fix(inbox): keep and announce an unsaved triage correction when its row leaves the list - #2698
Conversation
The Paper triage row editor now exposes `readDraft()`, which answers with the unsaved correction only while the capture text has loaded and something actually differs from it, and accepts a `restoredDraft` prop that it applies once over the freshly re-read server values before announcing `restored`. A getter rather than an emit on unmount: `close` fires on Cancel and on a completed Save as well as on an involuntary close, so an unmount emit would force the table to infer which kind of close it was. The table already knows the one moment the close is involuntary and can read the draft there. Refs #1999
…ow leaves the list Switching the Inbox board filter while a Paper row editor was open let the draft survive the replacement and then, once the new list landed without the edited row, the items watcher cleared editItemId and the correction went with the unmounted editor. Nothing said so. That was the silent loss the sibling edit gate and the decision block already refuse one row over. The table now reads the editor's unsaved draft at the one moment the close is involuntary and holds it, keyed by capture id, in component state. A role="status" receipt above the list names the capture by the excerpt the row showed and says the correction is kept and comes back with it. When the capture returns the editor reopens with the draft laid over freshly re-read server text, unless another editor is open, in which case the correction stays held and the receipt says so. It is dropped only when the capture comes back in a state this surface cannot edit, again with a receipt that names the status. Cancel and Save are unchanged and both release the held copy. The read-only history switch closes the editor through its own watcher, so it takes the correction the same way rather than leaving a second silent door. Nothing is persisted and nothing reaches the server: a restored editor's Save is the same explicit write it always was. Copy in inbox.triage.draft for en, es and it. Refs #1999
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Review record (alpha product-trust lane, review-and-ship round 1 at head Reviewer: one fresh-context independent reviewer subagent (read-only; Codex credits exhausted, SC-9), given the worktree at the head and the merge-base diff. Verdict: FIX-FIRST on two HIGHs; the premise holds (the table stays mounted across a board-filter change and a same-scope refresh, so the component-local kept-draft map survives; no new path calls the server; the getter covers every field Save submits; catalog parity clean). Findings, triaged once; the one fix round takes the HIGHs and the MEDIUMs that are holes in the new mechanism itself, since shipping a "never silently" promise with them would be worse than the silent destruction it replaces:
Round count: 2 after the fix push. Merge gate: |
…ry path Review round 2 on PR #2698. Every item below is one change to the same mechanism, so they land together: the editor's answer, the table's use of it, and what the surface says are not separable into commits that build. HIGH — readDraft conflated "nothing unsaved" with "I cannot tell". It returned null both when the draft matched the server text and when the editor was still loading, had failed to load, or had been refused. Two paths then destroyed a held correction silently: keepOpenDraft released it when a restore's detail read was still pending or had failed and the list changed again, and closeEdit released it on a close emitted by the load-error panel's Cancel or the refused panel's Close, where the reader had never seen the correction at all. The editor now answers { state: 'ready', draft } or { state: 'unavailable' }; the table releases a held copy only on a ready report with an empty draft, or on a close from a ready editor. An unavailable report changes nothing held. Design — the automatic restore is gone. A held correction comes back only through the reader's own Edit capture, the explicit action this surface requires everywhere else. That removes an editor opening by itself with text nobody asked for, a list arriving and cancelling a board pick in progress on another row, and a silent non-restore when the returning row happened to be action-disabled. What replaces it is a standing line per capture, derived from items, keptDrafts and the open editor rather than from a previous-versus- current diff, so it is always true, states itself for every capture that qualifies, and cannot miss a second one. MEDIUM — notices are one line per capture and merge instead of replacing, so a restored line cannot wipe another capture's discarded receipt. Standing lines disappear when they stop being true; receipts stand until that capture's next receipt, until the edit they describe ends from a loaded editor, or until dismissed. MEDIUM — the discard sweep skips a capture whose editor is open, and keys on the server's own rule rather than this list's action gate: only ProposalCreated, Converted and Ignored drop a correction, because those are the settled states CaptureService would refuse the edit in. Triaged keeps it (the Paper gate is a product policy the D-13 ruling can move), Triaging keeps it (a transient), and an unrecognised status keeps it. MEDIUM — the live region is mounted and silent from the start, so a notice is a mutation of a region that was already there rather than an insertion. MEDIUM — per-field dirty tracking. The held draft carries only the fields the reader changed, and the editor lays only those over the values it re-reads, so a text correction can no longer revert a due date or a label that moved on the server while it was held. The template comment claiming a restore is never a stale write is replaced by the truth: Save is last-write-wins, as it is from an editor opened fresh. LOW — dismiss returns focus to the region that survives it; the kept sentence says the correction is held while the reader stays on this Inbox list, which is what the Map's lifetime actually promises. Refs #1999
|
Round-2 record (alpha product-trust lane; fix head The mechanism is rewritten to be honest by construction:
Red-first on the round-1 components: 18 failed, 93 passed of 111 across the two component specs (the HIGH paths, the standing lines, one line per capture, the Triaged and Triaging keeps, the never-announce-over-an-open-editor rule, dismiss focus, forget-on-cancel; the editor's unavailable states and the touched-fields contract); eight of the 26 new or rewritten cases are guards that passed before. Verified at the head: the eight-suite command 8 files, 300 passed; typecheck, build, eslint and diff-check clean. Coverage claim corrected in the PR body: three table cases assert Not verified: no browser or screen-reader run (announcement asserted structurally, focus in an attached-DOM test); es/it wording not native-reviewed; the picker E2E leg stays out (not test-only). Merge gate remaining: |
|
Round-2 verification record (the whole rewritten mechanism at Verdict: SHIP; both round-1 HIGHs closed and spec-driven, no new HIGH or CRITICAL. What it confirmed: the held copy is released only on a ready empty answer or a ready-confirmed close, and a close from the error or blocked panel re-states the kept receipt (the exact sequences are driven: a return whose load rejects then a second list change; Cancel on the error panel; Close on the blocked panel; Cancel and a landed Save from a ready editor release); no path but the user's Edit writes the open-editor slot, and a list replacement can no longer cancel another row's board pick; the standing lines are a computed over the present rows and the held drafts, one per capture, shadowing that capture's receipt, with no code path deleting another capture's receipt; the discard set is exactly the settled statuses the server refuses minus the transient Triaging, Archived is a disposition kind, unknown statuses keep, the sweep skips an open editor; the restore applies only touched fields and Save gates metadata on a real change; the live region is always mounted and the store's list assignment and loading flag settle in the same synchronous continuation so lines never insert under Findings, triaged once at the two-round ceiling:
Round count: 2. Merge gate remaining: |
|
CI at |
1 participant
Summary
Switching the Inbox board filter while a Paper row editor was open destroyed the unsaved
correction without a word. The draft survived the list replacement itself, and then the
itemswatcher inPaperTriageTable.vuefound the edited row missing from the new list,cleared
editItemId, and the correction went with the unmounted editor. The table specpinned that as intended. It is the same silent loss the sibling-edit gate and the
decision block already refuse one row over, so the fate is now decided explicitly:
preserve, and announce.
The table reads the editor's unsaved correction at the moments the close is not the
reader's answer to it, and holds it, keyed by capture id, in component state. A
role="status"region above the list carries one line per capture, naming it by theexcerpt the row showed. A held correction comes back only through the reader's own
Edit capture on the row — never by an editor opening itself — and the region says it is
there to come back to. It is dropped only when the capture settles in a state the server
itself would refuse the edit in, with a receipt naming that status.
Nothing is persisted and nothing is sent. The held corrections live in the component for
as long as the table does, and a restored editor's Save is the same explicit write it
always was.
The contract chosen is a getter,
readDraft(), exposed by the editor and read by thetable, rather than an emit on unmount.
closeis emitted from four places with twodifferent meanings — Cancel and a landed Save from a loaded editor, against Cancel on the
load-error panel and Close on the refused panel, where the reader never saw a draft at
all — so an unmount emit would make the table infer intent from a flag rather than ask.
The getter answers
{ state: 'ready', draft }or{ state: 'unavailable' }, and thatdistinction is load-bearing: an editor that never showed a textarea is not evidence that
there is nothing unsaved.
Refs #1999 (item 3; item 2, the Triaged-row edit gate, stays behind the
OUTSTANDING_TASKS.mdD-13 ruling and is not touched here).
Refs #1995, #2426, #2668.
Changes
feat(inbox): let the row editor hand out and take back an unsaved draftPaperTriageRowEdit.vueexposesreadDraft()and accepts arestoredDraftprop that itapplies once, inside
load, on the editable path only and after the forced re-read, thenemits
restoredso the announcement is made after the restore is true.isDirtyis extracted as the one definition read bysaveBlock'sunchanged, so"nothing to save" and "nothing worth keeping" cannot disagree. No behaviour change to
saveBlock.<script>block alongside<script setup>, whichcannot export, so the table can name what it is holding.
fix(inbox): keep and announce an unsaved triage correction when its row leaves the listPaperTriageTable.vueholds corrections in a component-local map, renders therole="status"region above the list, and takes the correction on both involuntarycloses: the row leaving the list, and the read-only history switch, which closes the
editor through its own watcher.
inbox.triage.draftfor en, es and it.fix(inbox): make the held-correction mechanism answer honestly in every path(review round 2)
readDraftconflated "nothing unsaved" with "I cannot tell", returningnullfor both. Two paths then destroyed a held correction silently:
keepOpenDraftreleasedit when a restore's detail read was still pending or had failed and the list changed
again, and
closeEditreleased it on aclosefrom the load-error or refused panel,where the reader had never seen the correction. The report is now three-way and the
table releases only on a ready report — with an empty draft, or from a ready editor's
close. An
unavailablereport changes nothing held.action this surface requires everywhere else. That removes an editor opening by itself
with text nobody asked for, a list arriving and cancelling a board pick in progress on
another row, and a silent non-restore when the returning row was action-disabled for
some other reason. In its place, a standing line per capture derived from
items,keptDraftsand the open editor rather than from a previous-versus-current diff — alwaystrue, restated for every capture that qualifies, and unable to miss a second one.
restoredline cannotwipe another capture's
discardedreceipt. Standing lines vanish when they stop beingtrue; receipts stand until that capture's next receipt, until the edit they describe ends
from a loaded editor, or until dismissed.
server's rule, not this list's action gate. Only
ProposalCreated,ConvertedandIgnoreddrop a correction — the settled statesCaptureService.IsSuggestionEditableStatuswould refuse.
Triagedkeeps it and says this list does not edit that status (the D-13gate is a product policy a ruling can move, and dropping is irreversible);
Triagingkeeps it as a transient; an unrecognised status keeps it.
notice is a mutation of a region that was already there. It is never
display: none,which would take it back out of the accessibility tree.
changed, and the editor lays only those over the values it re-reads, so a text correction
can no longer revert a due date or a label that moved on the server while it was held.
The template comment claiming a restore is never a stale write is replaced by the truth:
Save is last-write-wins, exactly as from an editor opened fresh.
keptsentence saysthe correction is held while the reader stays on this Inbox list, which is what the map's
lifetime actually promises.
Test plan
Verified, from
frontend/taskdeck-webat the PR head:npx vitest --run --maxWorkers=2 src/tests/views/paper/inbox/PaperTriageTable.spec.ts src/tests/views/paper/inbox/PaperTriageRowEdit.spec.ts src/tests/views/paper/inbox/PaperTriageTable.degraded.spec.ts src/tests/views/paper/PaperInboxView.spec.ts src/tests/i18n/catalogs.spec.ts src/tests/guards/primaryActionGuards.spec.ts src/tests/guards/nativeBrowserDialogs.spec.ts src/tests/composables/useInboxOrchestrator.spec.ts— 8 files, 300 tests passed.PaperTriageTable.spec.tsalone is 86,PaperTriageRowEdit.spec.ts25.npm run typecheck— clean.npm run build— clean.npx eslinton the seven changed files — clean.git diff --check— clean.Red-first, round 2. Against the round-1 components the rewritten and added cases failed
18 of 26 (19 table cases, 7 editor cases; the eight that already passed cover behaviour
round 1 had right — the discard on a terminal status, a correction surviving repeated list
changes, the archived-history switch, and an untouched editor announcing nothing):
PaperTriageTable.spec.tsthere was a notice.
keptsentence did not yet scope its promise to this list.opened by itself and there was no
heldline.blockedline was event-pushed on return, not derived.
the correction was destroyed.
error panel's Cancel released it.
refused panel's Close released it.
both were discarded on the Paper action gate.
notices replaced the list wholesale.
removed and focus went to the document.
region's line set.
PaperTriageRowEdit.spec.tswas a whole snapshot.
failed / when the server refuses the edit (3 cases) — all three answered
null, the sameanswer as "nothing unsaved".
rest — the restore overwrote the freshly loaded metadata with the old load's values.
Coverage claim, stated exactly: 3 of the 19 table cases assert explicitly that
captureStore.updateSuggestionwas not called, and one of those also asserts that noaccept/keep/rejectleft the table. No case in the block clicks Save, so nonereaches a write — but that is an absence, not an assertion, and the earlier claim that
every case asserted it was wrong.
NOT verified:
is the condition a live region needs, but the announcement itself is asserted
structurally (role,
aria-live, text,data-noticekind), not observed in assistivetech. The focus move after dismiss is asserted against
document.activeElementin anattached-DOM test, not observed.
OUTSTANDING_TASKS.mdSeed an i18n translation layer (vue-i18n) with Italian and Spanish locales #1770). Thecatalog guard proves structural parity and placeholder membership only.
test-only addition to
tests/e2e/paper-responsive.spec.ts: a board-less capture needs anew helper in
tests/e2e/support/captureFlow.ts(the existingcreateCaptureItemrequires a
boardId), and the assertion would then be red until.paper-triage__board-pickgets a narrow-width rule, which is a second behaviour change.It stays listed on [Frontend] Paper triage-edit residuals from PR #1995's Codex round: mobile action stacking, Triaged-row edit, filter-change draft, stalled-detail Cancel #1999.
Boundaries and risks
Owned and touched:
PaperTriageTable.vue,PaperTriageRowEdit.vue, the threesrc/locales/*/inbox.tscatalogs, and the two triage specs.PaperInboxView.vue,useInboxOrchestrator.ts,captureStore.ts,PaperCaptureComposer.vueand the reviewfiles are untouched; the relay was not needed, because the board-filter change reaches this
table as a plain
itemsreplacement. The board-pick copy, the retry-board-load control andthe #2696 catalog keys are untouched.
Risks:
something, and has that row leave the list, and it dies with the table, so it is bounded
by deliberate action rather than by a cap. A cap would mean silently dropping a
correction, which is the behaviour this PR exists to remove.
Triaging, stays held untilthe table unmounts. That is the stated rule: only a settled server refusal drops one, and
dropping on a guess would be the silent loss again.
from any editor. Holding the correction adds no staleness, because untouched fields carry
the values the restore re-read.
v-for, so Vue maintains it as a list;only one editor is ever mounted, and the editor is read while it is still mounted — from
a pre-flush watcher, or from inside the
closehandler it just fired.Worktree:
.worktrees/codex-1999-draft-fate, ready for plaingit worktree removeafterthis PR. Its only ignored content is
frontend/taskdeck-web/dist/andnode_modules/,both regenerable build output; nothing was copied out.