fix(inbox): make background batch poll reconciliation tell the truth - #2567
Conversation
A batch whose immediate post-POST refresh exhausted its retries left listError set, and both skins hide every row behind that message. The later successful quiet poll replaced items but never cleared the error, so the rows stayed invisible until the user pressed Retry. Clear listError only on the accepted success path of the background list read. Superseded, aborted, 401 and 403 responses never reach it. Closes #2305
The single-item triage poll and an explicit detail load write a fresher summary through upsertSummary without recording a capture write, so the list write generation did not protect them. A slower batch-list response landing afterwards put the row back to its pre-poll status. Stamp every summary write with the shared monotonic generation and merge a background snapshot per row: the list still owns membership, order and scope, and only a row newer than the read keeps its local value. Closes #2301
The bounded batch poll only refreshed the badge counts once every tracked item was terminal, so one lagging item held the sidebar and Home counts stale for up to a minute, and the 60-second deadline stop left them stale for good. Count each newly observed terminal outcome instead, once, and sweep again at the deadline for an outcome whose tick was aborted before it reconciled. A snapshot that observes nothing new notifies nobody. Closes #2303
…anel refreshTerminalDetails reads details for items the user may not have open, and every one of those reads took the store-wide loadingDetail flag. The Legacy detail panel renders from it, so an unrelated open capture lost its body to a Refreshing spinner and had its Refresh Detail button disabled by background work. Add a trackLoading option, default true, and pass false from the reconciliation. Foreground fetchDetail loading and error behavior is unchanged. Closes #2304
The count sweep reads the same post-enqueue observation rule isComplete uses. Prove it: a poll whose every list read fails reaches the deadline without refreshing the workload counts off pre-batch cached rows. Refs #2303
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Review gate (Codex credits exhausted, SC-9): one fresh-context adversarial reviewer on head Findings by bin:
Evidence at this head (from the implementation run): |
Summary
Four Inbox defects reported against the bounded batch-triage poll share one root cause: the
background reconciliation in
captureStorewrites store-wide state as if it were a foregroundload, and it reads that state as if nothing else could have moved it. All four fixes land in
pollBatchTriageCompletion,refreshTerminalDetailsand the generation guards they read, so theyship as one narrow change to one production file with one spec file.
listErrorset. Both skinshide every row behind that message, so the later successful quiet poll replaced the rows but the
user still saw a broken empty inbox until pressing Retry.
upsertSummarywithout recording a capture write, so the list write generation added in fix(inbox): preserve capture writes across list races #2517did not protect them. A slower batch-list response landing afterwards put the row back to its
pre-poll status.
held the sidebar and Home counts stale for up to a minute, and the 60-second deadline stop left
them stale for good.
refreshTerminalDetailsreads details for items the user may not have open, and each read tookthe store-wide
loadingDetailflag. The Legacy detail panel renders from it, so an unrelated opencapture lost its body to a "Refreshing detail..." spinner and had its Refresh Detail button
disabled by background work.
Closes #2305
Closes #2301
Closes #2303
Closes #2304
Changes
frontend/taskdeck-web/src/store/captureStore.tslistError. Only that pathclears it. An aborted response, one superseded by a newer explicit load or a newer capture write,
and 401/403 either fail
isCurrent()or land in the catch, so a foreground error stands.upsertSummarystamps each per-item summary write with the shared monotonic generation(the counter
recordCaptureWritealready used, renamednextCaptureGeneration). The pollobserves that generation before its request and applies the response through
applyBackgroundListSnapshot, a per-row merge: the snapshot still owns membership, order, scopeand the newest-first cap, and only a row whose own summary is newer than the read keeps its local
value. No new timer and no new async path.
notifyTriageCountChangedonce per newly observed terminal outcome, replacing theall-terminal-only notification.
stopAtDeadlinesweeps the same way for an outcome whose tick wasaborted before it reconciled. An outcome only counts once the poll has observed a post-enqueue
list, the same trust rule
isCompleteuses, so cached pre-batch state never moves the badge. Anunchanged snapshot notifies nobody.
DetailLoadOptionsgainstrackLoading(default true) andrefreshTerminalDetailspassesfalse. Foreground
fetchDetailloading, error recording and toasts are unchanged.frontend/taskdeck-web/src/tests/store/captureStore.spec.ts— a newbackground batch poll list truthblock with 10itdeclarations, 11 cases (oneit.eachcovers 401 and 403). Each defectspec was confirmed red against the current implementation before its fix.
Test plan
Verified (worktree
.worktrees/codex-2305-inbox-poll-truth, head of this branch):npx vitest --run --maxWorkers=2 src/tests/store/captureStore.spec.ts— 76 passed (65 cases at thebase commit, 11 added).
npx vitest --run --maxWorkers=2 src/tests/store/captureStore.spec.ts src/tests/store/captureStore.integration.spec.ts src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/InboxView.spec.ts src/tests/views/paper/PaperInboxView.spec.ts src/tests/components/paper/PaperSidebar.inboxBadge.spec.ts— 6 files, 303 passed.npx vitest --run --maxWorkers=2 src/tests/resilience/degradedMode.spec.ts src/tests/resilience/slowApiAndStorage.spec.ts src/tests/property/storeResilience.spec.ts src/tests/composables/useInboxCounts.spec.ts src/tests/components/inbox src/tests/views/paper/inbox src/tests/composables/useCaptureQueueSync.spec.ts— 13 files, 257 passed.npm run typecheck— clean.npx eslint src/store/captureStore.ts src/tests/store/captureStore.spec.ts— clean.npm run build— built.git diff --check— clean.Red-before-green, per defect:
expected 'Failed to load inbox items' to be null.expected 'Triaging' to be 'ProposalCreated'and
expected 'Triaging' to be 'Failed'.expected "vi.fn()" to be called 1 times, but got 0 times.expected true to be false.NOT verified:
tests/e2e/capture-loop.spec.tsand friends) — needs a running stack; not run.loadingDetail, so [Frontend][Inbox] Background batch reconciliation obscures an unrelated open detail #2304's user-visible effect is proven at thestore flag, not through a Paper render.
Boundaries and risks
frontend/taskdeck-web. No docs, locale catalogs, backend, viewor composable files are touched. Paper views and
useInboxOrchestratorwere read, not edited.trackLoading: falsealso applies to therefreshTerminalDetailscall insidebatchTriage, notjust the poll. That is the same reconciliation for the same reason; it already ran with
recordError: falseandshowToast: false.completed batch still refreshes counts at least once, because completion implies each tracked id
was observed terminal; the refresh now happens when the outcome is first seen rather than only at
the end, and an unchanged snapshot no longer re-notifies.
latestSummaryGenerationByIdgrows with the number of distinct capture ids seen in a session, thesame shape as the existing
latestDetailWriteGenerationById.isCurrent()rule (not stopped, not aborted, still theactive request, no newer explicit list load, no newer list write) plus the orchestrator cancelling
polls on a board or archived-history scope change. Pre-existing and unchanged here:
batchTriagerefreshes with an unscoped
fetchItems()while the poll reads the scoped query.shouldCache); this changeadds monotonic protection for summaries only, which is what [Frontend][Inbox] Single and batch triage pollers can overwrite newer summary state #2301 specifies.