feat(recorder/library): clip trash bin with 30-day retention

[rabble]

Closes #4547

Summary

Replaces the recorder's destructive "delete last clip" gesture with a
two-layer safety net:

1. Snackbar Undo (5 s) — taps to the bottom-left button now schedule a
   soft-delete and show a "Clip moved to trash • Undo" snackbar. Undo
   restores the clip to its original tray position.
2. 30-day trash bin — clips that aren't undone land in a new
   Recently deleted screen (accessible from the clips library), where
   they sit for 30 days before auto-purge. Each row offers Restore /
   Delete now, plus a single "Empty trash" action.

Library-tab deletes are routed through the same softDelete path so
both surfaces share one delete behavior. Icon swapped from
arrowCounterClockwise → trash so the affordance reads as
destructive-but-recoverable instead of undo my last tap.

Motivation: in-app feedback from 723glimmers — accidentally tapped
"undo" while assembling a brownie-batter compilation and lost a step.
The undo button's icon promised reversibility but the action was a
single-tap, no-confirm, on-disk hard delete with no recovery.

Architecture

• Schema: clips.deleted_at (nullable timestamp) + idx_clip_deleted_at.
  All existing read paths now filter deleted_at IS NULL.
• ClipLibraryService split into softDelete / restore /
  hardDelete / getTrashedClips / purgeExpiredTrash. softDelete
  optionally clears draft_id so a recorder-trashed clip lands in the
  library on restore rather than a stale session.
• ClipManagerNotifier tracks the pending deletion in state, commits
  it on addClip / reorderClip / clearAll / dispose so a fresh
  recording never leaks a half-undone state.
• ClipsLibraryBloc gains four events: TrashLoadRequested,
  RestoreClips, HardDeleteClip, EmptyTrash, plus trashedClips
  and lastDeletedClipIds on state.
• purgeExpiredTrash(retention: 30d) runs on app startup from
  app_lifecycle_handler.dart (best-effort, idempotent).

Test plan

• flutter test test/services/clip_library_service_test.dart — softDelete / restore / hardDelete / purgeExpiredTrash coverage
• flutter test test/blocs/clips_library — 64 tests, including state-prop shape after new fields
• flutter test test/providers/clip_manager_provider_test.dart — scheduleDeleteLastClip smoke
• flutter test test/widgets/video_recorder — both recording modes
• flutter test test/widgets/library test/screens/library_screen_test.dart — library entry-point unchanged for existing flows
• flutter test test/l10n/arb_consistency_test.dart — new keys allowlisted
• packages/db_client migration test — schema add safe
• Manual: capture-mode tap → snackbar → Undo → restored
• Manual: capture-mode tap → snackbar dismiss → clip in Recently deleted → Restore → library
• Manual: classic-mode same flow
• Manual: trash → Delete now → file gone
• Manual: trash → Empty trash → all files gone
• Manual: kill app, relaunch → trashed clips persisted

l10n

Eight new keys: videoRecorderClipUndoLabel + 7 libraryTrash* keys.
Allowlisted in arb_consistency_test.dart; translators will pick them
up in the next localization pass. videoRecorderClipDeletedMessage
English copy updated from "Clip deleted" → "Clip moved to trash";
other locales fall back to the prior translation until refreshed.

🤖 Generated with Claude Code

[rabble]

Review summary

Solid architecture overall — soft/restore/hard/purge belong on the service, the BLoC orchestrates, the UI stays thin, and the DAO does the right thing with the indexed deleted_at column. CI is green, tests cover the new service surface, and the recorder undo flow is well-thought-out (drafts decoupled on soft-delete, pending-deletion committed on addClip/reorderClip/clearAll/dispose so a fresh recording can't leak a half-undone state).

Two real consistency gaps in the library-tab flow and one nit on the trash UI, none of which prevent a merge but the first one I'd push for before shipping.

Disposition: Comment (not request-changes).

---

1. (Important) Library-tab "Delete" still says "cannot be undone" — and is now a lie

mobile/lib/l10n/app_en.arb:

"libraryDeleteClipsWarning": "This action cannot be undone. The video files will be permanently removed from your device."

That dialog is shown by _confirmDeleteSelectedClips (library_screen.dart:321), whose confirm path dispatches ClipsLibraryDeleteSelected — which this PR re-routes through softDelete (a 30-day trash). The PR description even calls this out as intentional ("Library-tab deletes are routed through the same softDelete path"), but the user-facing copy still promises a hard delete.

This is the kind of mismatch users notice the first time they hit the trash bin and find clips they were told "the video files will be permanently removed" still sitting there. Either:

• update the copy to "Moved to trash. You can restore from Recently deleted for 30 days." and probably drop the destructive-button styling, or
• drop the dialog entirely now that the action is reversible (the snackbar/undo is the appropriate affordance for a reversible delete; a confirm dialog plus trash bin is double-paying for safety).

I'd lean toward the second — the recorder path doesn't confirm, and there's no reason the library path should be more aggressive than the recorder when both are soft-deletes.

2. (Important) lastDeletedClipIds is dead state

ClipsLibraryState.lastDeletedClipIds is set by _onDeleteSelected and _onDeleteClip but has zero consumers:

$ grep -rn 'lastDeletedClipIds' mobile/lib mobile/test --include='*.dart'
# only the BLoC and state files themselves

The naming (and the PR description's framing of "Library-tab deletes are routed through the same softDelete path so both surfaces share one delete behavior") strongly suggests the intent was to wire an undo snackbar on the library tab parallel to the recorder's. That snackbar isn't here. Combined with finding #1, the library tab is currently the worst of both worlds: copy that says "permanent", behavior that's soft, and no surfaced undo to bridge the gap.

Either wire the undo snackbar via BlocListener<ClipsLibraryBloc> in library_screen.dart (which restores by dispatching ClipsLibraryRestoreClips(state.lastDeletedClipIds)), or delete the lastDeletedClipIds field + the population sites so we don't carry dead state. Personally I'd ship the undo — that's the whole point of the safety net, and the field is already populated.

3. (Nit) Trash UI shows recorded date, not deleted date / purge countdown

_TrashedClipTile._formatRecordedAt shows clip.recordedAt — but in a trash bin, what the user wants is "deleted N days ago" (or, even better, "auto-deletes in M days"). Recorded date is the wrong signal for this surface.

This is a model gap as much as a UI gap — DivineVideoClip doesn't carry deletedAt (it lives only on the row), so the screen literally can't render it without plumbing. Worth a follow-up: either expose deletedAt on the clip model or have getTrashedClips return a wrapper that pairs the clip with its deletedAt.

4. (Nit) app_lifecycle_handler.dart purge: await inside post-frame callback

WidgetsBinding.instance.addPostFrameCallback((_) async {
  ...
  await ref.read(clipLibraryServiceProvider).purgeExpiredTrash();
});

The comment says "must never block app startup" and the binding-level callback is fire-and-forget so this is mostly defensible — but ClipLibraryService.purgeExpiredTrash only wraps individual row failures in try/catch. If getTrashedClipsOlderThan(cutoff) itself throws (DB locked mid-migration, schema mismatch, anything), the exception propagates out of the post-frame callback as an unhandled async error. Wrap the call site (or the whole method body) in try/catch to honor the "best-effort, idempotent" contract the comment promises.

5. (Nit) _TrashedClipTile thumbnail has no-op callbacks

VideoClipThumbnailCard(
  clip: clip,
  showSelectionIndicator: false,
  onTap: () {},
  onLongPress: () {},
)

Empty () {} callbacks present a tappable affordance that does nothing — slightly confusing UX. Either pass null (if the card accepts it) or make a non-interactive variant. Could also be a good place for a "preview" tap if you wanted to flesh out the trash UX.

6. (Nit) recoverMissingAssets + trashed clips

ClipLibraryService.recoverMissingAssets calls saveClip (which is upsertClip under the hood) on incomplete clips. Drift's insertOnConflictUpdate with Value.absent() for deletedAt does NOT clobber the column on update, so trashed clips can't be silently resurrected this way. But the invariant is non-obvious and load-bearing; worth pinning it with a test that asserts a trashed clip surviving a recovery pass still has deletedAt != null.

---

Things done well

• Layering is clean. softDelete / restore / hardDelete / getTrashedClips / purgeExpiredTrash all live on the service, the BLoC just orchestrates, and the UI is just glue. Matches architecture.md.
• Pending-deletion lifecycle is paranoid in a good way. Committing the in-flight deletion on addClip / reorderClip / removeClipById / clearAll / dispose is exactly right — those are the windows where a stale undo would leak state into a fresh session.
• clearDraftId flag. Decoupling a recorder-trashed clip from its draft so Undo lands in the library, not a stale session, is a subtle correctness point and the right call.
• Persistence-key pattern preserved. The new ClipsLibraryStatus enum cases are added in-order and a new status is added rather than overloading loaded — clean.
• getTrashedClipsOlderThan is a separate DAO method, not a re-filter in service. Good push of selection logic down to the database where it belongs.
• Test coverage for the new service methods is real, not perfunctory. Both the "within retention" and "past retention" cases are pinned; restore roundtrip is tested.

[rabble]

Addressed self-review in 5f40dd1.

Finding 1 (dialog lie): Dropped the confirm dialog from both library
delete paths (multi-select toolbar and per-clip preview). With a 30-day
trash bin the dialog was double-paying for safety and the copy was a
lie; the recorder path already skipped confirm so library now matches.
Removed the now-orphan libraryDeleteClipsTitle/Message/Warning,
libraryDeleteClipTitle/Message, and libraryDeleteConfirm keys
from all 18 ARB locales.

Finding 2 (dead state): Wired the snackbar. The existing
lastDeletedCount BlocListener in library_screen.dart now also reads
lastDeletedClipIds and surfaces an Undo action on the snackbar that
dispatches ClipsLibraryRestoreClips(deletedIds) on the bloc. Reuses
the existing DivineSnackbarContainer actionLabel/onActionPressed
slots — same shape the recorder undo snackbar already uses.

Finding 3 (no purge countdown): Added DateTime? deletedAt on
DivineVideoClip, plumbed row.deletedAt through
ClipLibraryService.getTrashedClips() via copyWith(deletedAt: ...),
and replaced the recorded-date subtitle in _TrashedClipTile with an
ICU plural libraryTrashAutoDeletes (today / tomorrow / in N days). Lifted the 30-day window into
ClipLibraryService.trashRetention so the UI countdown and the purge
sweep can't drift apart. No schema migration needed — the
deleted_at column already exists; only the model surface was missing.

Nits: wrapped the startup purgeExpiredTrash() await in a
try/catch with Log.error to honor the "best-effort" comment; dropped
the no-op onTap/onLongPress on the trash thumbnail by making both
callbacks nullable on VideoClipThumbnailCard. Skipped the
resurrect-invariant test — production already keeps recoverMissingAssets
strictly on the getAllClips() path which filters deletedAt IS NULL,
so the invariant is enforced by the DAO query, not by the model.

l10n: new keys (libraryClipsDeletedUndoLabel, libraryTrashAutoDeletes)
added to app_en.arb and added to the _knownUntranslatedDebt
allowlist in arb_consistency_test.dart, matching the pattern other
recent UI passes use.

Scoped tests pass locally:
flutter test test/blocs/clips_library test/services/clip_library_service_test.dart test/widgets/library test/l10n/arb_consistency_test.dart test/screens/library_screen_test.dart test/widgets/video_clip/video_clip_thumbnail_card_test.dart test/providers/app_lifecycle_provider_test.dart test/models
→ all green; flutter analyze clean.

[github-actions]

Mobile PR Preview

Preview refreshed for 5f40dd1

Last refresh: 5f40dd1 at 2026-05-19 09:51:20 UTC (preview run)

Property	Value
Preview URL	https://67312f78.openvine-app.pages.dev
Pages project	openvine-app
Preview branch	pr-4546
PR branch	feat/clip-trash-bin
Commit	5f40dd1

[rabble]

Re-review after 5f40dd11 ("fix(library): drop confirm dialog, wire undo snackbar, show purge countdown").

Prior findings — resolution

#	Finding	Status
1	Library delete dialog said "permanently removed" but soft-deleted	Resolved. Confirm dialogs removed from both single (clips_tab.dart _softDeleteClip) and batch (library_screen.dart) paths. Orphan libraryDeleteClip* keys removed from app_en.arb.
2	lastDeletedClipIds populated but unconsumed	Resolved. BlocListener in library_screen.dart:424 triggers a snackbar with Undo on lastDeletedCount transitions, dispatching ClipsLibraryRestoreClips(deletedIds). The pre-emit clearDeletedCount: true in both single- and batch-delete handlers correctly drives N → null → M so consecutive deletes of the same size still refire.
3	Trash UI showed recordedAt, no purge countdown	Resolved. DivineVideoClip.deletedAt plumbed from clips.deleted_at. _daysUntilPurge derives from deletedAt + ClipLibraryService.trashRetention. Countdown renders via libraryTrashAutoDeletes ICU plural (today / tomorrow / in N days). Retention constant lives on the service — purge sweep and UI countdown share one source of truth.
4a	Best-effort purge not wrapped in try/catch	Resolved. app_lifecycle_handler.dart:68 now wraps purgeExpiredTrash in try/catch with a logged failure path.
4b	No-op onTap/onLongPress on trash thumbnail	Resolved. VideoClipThumbnailCard.onTap/onLongPress made nullable (VoidCallback?); trash tile passes neither.
4c	Invariant test: recoverMissingAssets can't resurrect trashed	Indirect coverage only. No explicit test, but the guarantee follows structurally: recoverMissingAssets(List<DivineVideoClip>) operates on the parameter; trashed rows never reach it because getLibraryClips / getAllClips / watchLibraryClips / getClipsByDraftId all filter deleted_at IS NULL (verified in clips_dao.dart). One dedicated test would still be cheap insurance — not a blocker.

New observations on this revision

1. libraryClipsDeletedUndoLabel and libraryTrashAutoDeletes are English-only + allowlisted. The other 6 library-trash keys + videoRecorderClipUndoLabel and the rewritten videoRecorderClipDeletedMessage ARE translated across all 17 locales (631ea01d). Mixed approach — fine per the documented allowlist policy, but worth a follow-up to translate these two as well so the snackbar Undo and the countdown match the rest of the surface.

2. _daysUntilPurge returns 0 for deletedAt == null. The comment frames it as "a degraded row state — surface as 'today' rather than crashing". That's safe but slightly misleading — a clip with deletedAt == null should not be in getTrashedClips() (DAO filter is deletedAt IS NOT NULL). Worth either an assert(clip.deletedAt != null) to catch programming errors, or trusting the DAO contract and dropping the null branch. Minor.

3. Schema migration safety. _addColumnIfMissing('clips', 'deleted_at', 'INTEGER') plus CREATE INDEX IF NOT EXISTS is idempotent and tolerates re-runs. schemaVersion stays at 1, so existing installs hit the ADD COLUMN on first launch via the migration step. Reads with the new deletedAt IS NULL filter will treat all pre-migration rows as active (NULL = active), which is correct. No data migration needed.

4. Cross-device sync is out of scope. Trash is local-only — deleting on Phone A does not propagate to Phone B's library. The PR is explicit about this being a local recovery feature, not a Nostr-level retraction, so this is acknowledged scope, not a gap.

5. Test coverage for the new service surface (softDelete, restore, hardDelete, purgeExpiredTrash) is solid in clip_library_service_test.dart. Bloc tests cover the new events. No tests for the trash screen itself (library_trash_screen.dart) — adding one widget test for the countdown rendering and Restore/Delete-now button wiring would close that gap.

Verdict

Approve. All four prior findings addressed cleanly. The two open items (translation of the two self-review-pass keys, trash screen widget test) are minor follow-ups rather than blockers. Schema migration is safe, snackbar undo race-condition is handled via the pre-emit null-clear, and the retention constant is correctly centralized.

[hm21]

I tested it and everything works well. I left a few comments, but nothing blocking.

@Chardot, this change affects the design, so it’s something you should be aware so we can improve that part later as well.

[hm21]

It seems the formatting in this file changed, which added whitespace everywhere. That’s why the PR shows this feature as adding +60k lines of new code.

[hm21]

Consider using here the DiVineAppBar

[hm21]

Maybe better using the BrandedLoadingIndicator

[hm21]

Because of this padding, the button became super small in my tests since there wasn’t enough space for it, which caused the button itself to shrink. I think we can completely remove that padding as well.

[hm21]

Consider using DivineSnackbarContainer.snackBar( instant of just the DivineSnackbarContainer