Skip to content

Keep retained Inbox rows after same-scope refresh failures - #2484

Merged
Chris0Jeky merged 2 commits into
mainfrom
issue-2044/inbox-refresh
Sep 4, 2026
Merged

Keep retained Inbox rows after same-scope refresh failures#2484
Chris0Jeky merged 2 commits into
mainfrom
issue-2044/inbox-refresh

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Closes #2044

Summary

  • distinguish route-scope replacement loads from ordinary Inbox refreshes in the orchestrator
  • keep retained rows and their count visible alongside a retryable same-scope refresh error
  • continue hiding retained rows while a replacement scope is loading or failed
  • cover latest-load, retry, Paper table, and Paper Inbox behavior with deterministic tests

Verification

  • npm ci --no-audit --no-fund
  • npm test -- --maxWorkers=2 src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/paper/inbox/PaperTriageTable.spec.ts src/tests/views/paper/PaperInboxView.spec.ts (182 passed)
  • npm run typecheck
  • npx eslint on all six changed files
  • git diff --check

No persistence, API filtering, or latest-request-wins store behavior changed.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

captureStore.items survives leaving the Inbox route and the board/archived
watchers are not immediate, so re-entering Inbox at a different board with a
failed first load left isScopeReplacement false while listError was set and
retained rows were present. PaperTriageTable then rendered the previous
board's rows and their count under the new board's scope label.

Make the mount-time load a scope replacement so a failed first load hides the
stale rows and suppresses their count.
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Fix round — HIGH addressed, MEDIUM/LOW tracked

Head: 1900e90e0

Fixed (HIGH)

useInboxOrchestrator.tsonMounted called loadInbox() instead of loadInboxForScopeReplacement().

captureStore.items survives leaving the Inbox route (there is no store reset), and the board / archived-history watchers are not immediate. So re-entering Inbox at a different board with a failing first load left isScopeReplacement=false while listError was set and hasItems=true. PaperTriageTable.vue:581 then rendered the previous board's rows, and :543 their count, under the new board's scope label (PaperScopeDisclosure, PaperInboxView.vue:504-506).

This was a regression introduced by this PR — pre-diff, loadingList || listError hid the rows.

The mount-time load is now unconditionally a scope replacement, which is also the semantically correct classification: mount is always a fresh scope.

Test added

src/tests/composables/useInboxOrchestrator.spec.ts — in the lifecycle block:

  • treats the mount load as a scope replacement so a failed first load hides rows retained from a previous board

It pre-seeds captureStore.items with rows from a previous board, mounts with boardId=board-2, rejects the first fetchItems, and asserts the scoped fetch happened and isScopeReplacement stays true (which is what makes PaperTriageTable hide the rows and suppress the count).

Confirmed the test is load-bearing: reverting only the onMounted line makes it fail, restoring it makes it pass.

The view-side rendering half is already covered by PaperTriageTable.spec.ts (prioritizes an error and hides retained rows and their count after replacement fails).

Verification

Command Result
npx vitest --run --maxWorkers=2 src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/paper/inbox/PaperTriageTable.spec.ts src/tests/views/paper/PaperInboxView.spec.ts 3 files passed, 183 tests passed
npm run typecheck (vue-tsc -b) clean
npx eslint on both changed files clean, exit 0

Tracked, not fixed (deliberate — being filed as a follow-up issue)

  • MEDIUM useInboxOrchestrator.ts:363-373isScopeReplacement is cleared whenever fetchItems resolves, even if captureStore discarded the response via its observedListWriteGeneration guard (captureStore.ts:151,167) after a nib capture landed during the replacement fetch.
  • MEDIUM PaperInboxView.vue:98,484-494 — the header eyebrow counts from useInboxCounts(items) and ignore scope replacement, so a stale count can still surface there.
  • LOW — no visible in-flight affordance during a same-scope refresh with retained rows; only aria-busy changes.

None of these are merge blockers under the severity bar: each needs a specific concurrent-write or cosmetic path, and none makes the shipped retained-rows behavior wrong on the direct path this PR changes.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Round 2: the fix is the exact one-line change the review asked for (onMounted -> loadInboxForScopeReplacement) plus a load-bearing lifecycle spec (proven red without the fix, 183 targeted tests green). No logic beyond the reviewed finding, so no further review pass is owed. The two MEDIUMs and one LOW from round 1 are tracked in #2501. Merging when CI at 1900e90 is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Keep valid Inbox rows visible after same-scope refresh failures

1 participant