spec: bulk delete chat history (#10359)#10457
spec: bulk delete chat history (#10359)#10457lonexreb wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec proposes conversation-list multi-select, bulk delete, undo, keyboard coverage, storage changes, tests, and telemetry. The scope is appropriate for the issue, but several requirements remain ambiguous or unresolved enough to risk an incorrect or unsafe implementation.
Concerns
- The scope and implementation contract for
Select allis ambiguous, especially with active filters/search and virtualized rows. - The storage/API decision for bulk delete is still open and the client fan-out proposal does not define failure, retry, atomicity, or undo semantics.
- The skip-confirmation wording is internally inconsistent and the fixed 5-second undo window needs accessibility requirements.
Security
- Client-side fan-out for an unbounded bulk destructive action needs explicit limits or failure semantics so a single user action cannot create unbounded work or partially committed destructive state.
Verdict
Found: 0 critical, 2 important, 2 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| - Header controls: | ||
| - "Select all visible" — selects only currently rendered rows | ||
| (respects active filter / search). | ||
| - "Select all" — selects every conversation, including |
There was a problem hiding this comment.
Select all means all conversations globally, all conversations matching the active filter/search, or all currently loaded conversations, and require how the exact count/IDs are computed before confirmation.
| - Disabled when N == 0. | ||
| - When N > 0 and the user has any existing | ||
| "skip delete confirmations" preference set, that preference | ||
| is **still respected**, but bulk delete ALWAYS confirms, |
There was a problem hiding this comment.
💡 [SUGGESTION] This says the skip-confirmation preference is respected and then ignored; rewrite to say single-row deletes keep the preference while bulk deletes always confirm.
| - Q1. Should bulk-delete be gated behind a feature flag for the | ||
| first preview rollout? Proposal: **yes**, gate to the Preview | ||
| channel for one cycle, then promote to Stable. | ||
| - Q2. Should the snackbar duration be user-configurable? V1 |
There was a problem hiding this comment.
💡 [SUGGESTION] If the undo window stays fixed at 5 seconds, add accessibility requirements for keyboard focus, screen-reader announcement, and whether users who miss the snackbar have any alternate recovery path.
| dialog already shows the count. | ||
| - Q4. Does the storage layer need a server-side bulk-delete | ||
| endpoint, or is per-row deletion fanned-out client-side | ||
| acceptable for V1? Proposal: V1 fans out client-side; if |
There was a problem hiding this comment.
Summary
Spec-only PR proposing multi-select and bulk-delete affordances for
the conversation list, with a 5-second undo window after each bulk
delete (matching macOS Finder's "Recover" semantics).
Resolves intent of #10359.
What's in this PR
specs/GH10359/SPEC.md— full behavior contract, keyboard map,acceptance criteria, tests, telemetry plan, and open questions.
No code changes. This PR is for spec review only.
Highlights
context menu, with checkbox + Cmd-click + Shift-click semantics.
even when the per-row skip-confirmation preference is set —
because the blast radius is larger.
pattern. Within the window, deletions are reversible; after, they
are permanent.
Cmd/Ctrl+Shift+A, Delete, Esc.
batch lands cleanly on the next-most-recent conversation.
commit on shutdown so there are no zombie undo affordances.
Telemetry
No new event. Extends the existing
conversation.deleteevent withcount,bulk, andundonefields. Bulk events fire after theundo window closes so
undoneis always known.Triage request
Requesting
ready-to-implementonce Q1 (Preview-channel featureflag) and Q4 (server-side batch endpoint vs client fan-out) are
resolved.
Test plan
tombstone-window approach is acceptable.
ready-to-implement.