fix(review): the Paper rail never shows the previous proposal's data, and says loading or failed instead of claiming emptiness

[Chris0Jeky]

Summary

The Paper review right rail used to show the previous proposal's evidence under the new proposal's header, and its cards claimed emptiness whether the read behind them was pending, had failed, or had genuinely found nothing. Both are the two residuals recorded with PR #2662's release on 2026-09-05, and both come from the same missing fact: the rail received selector values with no way to say which proposal they belonged to or whether the read that produced them had landed.

usePaperReviewSelectors now exposes one keyed snapshot, railEvidence. It carries the state of the batch for the ACTIVE key (idle, loading, failed with the outcome kept, settled), the key the values belong to, and the two rail values, withheld in every state but settled. PaperReviewView passes that snapshot to ReviewRightRail, which forwards the state to both cards. So a switch from proposal A to proposal B shows B's loading state, never A's confidence number, A's confidence-source sentence or A's similar-past rows, and the cards say what is true of a pending or failed read instead of asserting an absence they cannot know.

Mechanism: a keyed snapshot returned by the composable, not gating in the view. The composable is the only place that knows both which key a value was read for and how that read ended, so gating there makes the mistake unrepresentable at the boundary rather than merely unlikely: a caller cannot pass a value without the state that says whether it may be believed. Gating in the view would have needed the view to reconstruct the key comparison the composable already does, on every consumer, forever.

Read semantics are unchanged. The #2460 keying, the automatic refresh, the same-action retry and waitForCoreBatch behave exactly as before; the bare isLoading boolean is replaced by one record holding the last batch's key and status, and loading is derived from it so the two views of the same batch cannot disagree.

Refs #1940 (the two 2026-09-05 residuals). D-9 (waves, request-edit fields, defer durations) stays a maintainer ruling and is not touched; the parked P-key residual on issue-1940/provenance-shortcut stays separate. This does not close the issue.
Refs PR #2662 (the hoisted honest-empty sentences this makes truthful in every state).

Changes

Commit 1, usePaperReviewSelectors.ts and its spec: the PaperReviewEvidenceStatus and PaperReviewRailEvidence types, the exported SelectorKey, the batchRecord that replaces isLoading, and the railEvidence computed. Five write sites keep their previous meaning: fast path settled, new batch loading, failure and success terminal, no active proposal clears the record. A record for another key reads as loading, which is also the safe default if a future edit ever leaves a gap where no batch was started.

Commit 2, the rail seam and its specs:

• PaperReviewView.vue (rail wiring only): breakdown, similarPast, similarPastApplyRate and the new evidenceState all come from selectors.railEvidence, and authorMeta (which carries a confidence number into the rail) reads the snapshot instead of the bare breakdown.
• ReviewRightRail.vue: one new evidenceState prop, forwarded verbatim to both cards. Its default is loading, the conservative one, so a caller that does not know the state gets cards that withhold their claims.
• ReviewAuthorCard.vue: the confidence-source sentence is reserved for a settled read; loading and failed each get their own sentence in a new paper-review-author-confidence-state line; idle states nothing. Bars on screen already show where the number came from, so no state line is added to them.
• ReviewSimilarPast.vue: the same three states for the empty sentence, in a new paper-review-similar-past-state line. The "(none found)" disclosure label is the same claim in miniature and is now reserved for the settled empty result too. The region opens onto a matching line in each state rather than onto nothing, so the disclosure and count semantics from fix(review): tell the truth before the similar-past and confidence disclosures open #2662 are unchanged.
• The KNOWN GAP comments in both cards are replaced by what the code now does.
• locales/{en,it,es}/review.ts: author.confidenceLoading, author.confidenceFailed, similarPast.{loading,loadingDetail,failed,failedDetail}. The failure copy is a plain statement that the read failed and the values are unknown, because the rail has no retry affordance for the page-load batch; evidenceUnavailable is the Apply-time refresh and keeps its own prop.

Test plan

Red-first. The new assertions were run against the unmodified source (the eight source files stashed, the four spec files kept): 14 failed | 265 passed (279), 4 files. The failures were, per file:

• usePaperReviewSelectors.spec.ts, all five new cases: railEvidence did not exist. The load-bearing one is "withholds the previous proposal values while the new key is still loading", which asserts in the same test that the bare reads STILL hold proposal A's #PAST-A row and A's 0.84 overall while B loads. That is the residual itself, and it is why the gate has to exist.
• ReviewAuthorCard.spec.ts, four cases: with a loading or failed state the card still rendered paper-review-author-confidence-source saying "No model confidence reported" (and the deterministic sentence), and rendered it for idle too.
• ReviewSimilarPast.spec.ts, three cases: with a loading or failed state the card still rendered paper-review-similar-past-empty saying "No comparable past decisions." and the "(none found)" label, and rendered them for idle too.
• PaperReviewView.spec.ts, two cases: after switching to proposal B mid-flight the rail still contained A's 0.84 model-reported average, A's similar-past row title and A's confidence-source sentence; with B's read rejected it claimed emptiness instead of saying the read failed.

The third view case, "keeps the settled empty sentences from #2662 exactly as they were", passed before and after by design: it is the regression guard on #2662's shipped behaviour, not a red-first assertion.

Verified after the fix, on the exact pushed head (which includes a merge of origin/main at f74514fd1, disjoint boards-list work):

• npx vitest --run --maxWorkers=2 src/tests/composables/usePaperReviewSelectors.spec.ts src/tests/views/paper/review/PaperReviewView.spec.ts src/tests/views/paper/review/PaperReviewView.language.spec.ts src/tests/i18n/catalogs.spec.ts src/tests/views/paper/review/ReviewAuthorCard.spec.ts src/tests/views/paper/review/ReviewSimilarPast.spec.ts src/tests/views/paper/review/ReviewWhyNow.spec.ts — 7 files, 309 tests passed, Test Files matching the seven paths named.
• npx vitest --run --maxWorkers=2 src/tests/views/paper/review — 16 files, 402 tests passed (the whole Paper review directory, run before the merge).
• npm run typecheck — clean, before and after the merge.
• npx eslint on all twelve changed files — clean, zero warnings.
• git diff --check origin/main...HEAD — clean.

NOT verified: the Spanish and Italian wording is not native-reviewed (OUTSTANDING_TASKS #1770); the catalog spec proves structural parity, not tone. No screen-reader run and no real-browser pass: the state lines are plain text with no live region, so a reviewer whose focus is elsewhere is not announced when a state changes. The required E2E was not run locally; it drives the similar-past disclosure and asserts the region is visible once opened, which now holds in all three states because each renders a detail line, but that is reasoned from the source, not observed. npm run build was not run.

Boundaries and risks

Owned and changed: usePaperReviewSelectors.ts, PaperReviewView.vue (the rail wiring only), ReviewRightRail.vue (the prop pass-through), ReviewAuthorCard.vue, ReviewSimilarPast.vue, the three review catalogs and the four specs. The rail is mounted directly in PaperReviewView, so nothing flows through ReviewMain.vue and it is untouched. useReviewProposals.ts, the receipt recorders, the diff pane and the sticky seam are untouched. ReviewWhyNow.spec.ts mounts the rail and was NOT edited: the new prop is optional there and its populated fixture renders no state line, so it is an unmodified regression gate on this change.

Risks. The rail's evidenceState default is loading rather than settled, so a future caller that forgets the prop gets withheld claims instead of false ones; the trade is that such a caller would show the in-flight sentence indefinitely, which is visible rather than silent. ReviewMain still receives the bare confidence read, so its badge can show the previous proposal's number for the same in-flight window — same root, a different component, outside this slice's file boundary and recorded on the issue. The failure state is reachable only through a rejected page-load batch; a reviewer sees the failure sentence with no retry button, because the rail has none for that batch, and the recovery today is leaving the proposal and returning.

Human-action file OUTSTANDING_TASKS.md: unchanged by this PR. Open items it already carries that touch this work are D-9 (a) to (c) for the rest of #1940 and #1770 for the untranslated-tone review.

Round 2

One MEDIUM from the fresh-context review, fixed here in 688bedc58.

The finding: railEvidence asked "does this record cover the active key" with selectorKeysEqual, which treats a revision identity of rev-Y and one of null as different keys, while the watcher decides whether to start a batch with proposalRevisionMoved, which is deliberately asymmetric — rev-Y to null is not a move, because a revision identity can only reach null by the proposal leaving PendingReview. On exactly that transition the two disagreed: no batch starts, no record is ever written for the new key, and the snapshot reported loading with no end. That is a regression from this PR's own lines, since the pre-PR rail rendered the settled values there.

Proven red first at the composable level before fixing: a proposal with latestRevisionId: 'rev-1' settles, the rejected DTO replaces it with both revision fields null, and railEvidence.status came back loading. Exact failure: AssertionError: expected 'loading' to be 'settled'. It now returns settled with the values intact and issues no new read.

The fix is one shared predicate, selectorKeyStillCovers (same proposal id, same capture reference, and a revision that either matches or did not move), used by railEvidence AND by the watcher's no-op condition, so the two cannot drift apart again. Which transitions start a batch is unchanged: for two present proposal ids the new condition is exactly the previous three flags, and when either id is absent both forms proceed. The settled snapshot now names the key the values were READ under rather than the active one, because after a decision retires the revision those are no longer identical.

The fast path was checked as asked. ensureCoreBatch's selectorKeysEqual(settledCoreKey, key) carries the same asymmetry and is deliberately left alone: it is pre-existing and untouched by this PR, the watcher never calls it on this transition, and its only reachable caller there is waitForCoreBatch from an Apply-class action, where an exact mismatch costs one redundant re-read rather than producing a stuck state. Exact matching is also the correct test in that position and in the publication guards, which gate Apply on evidence taken for the exact revision identity being applied (the #2460 contract). The helper's doc says so, so the next reader does not "fix" it.

Recorded, not fixed: if a decision lands while the batch is still in flight, the in-flight batch's publication guard still compares exactly, returns superseded, publishes nothing and leaves the record at loading for a key that no longer arrives, so the rail's two cards can hold their in-flight line until the reviewer selects another proposal. It is an honest line rather than a false claim, it needs a decision inside the read window (Approve and Apply cannot reach it, since the #2460 barrier settles the batch first), and closing it means changing publication semantics, which is outside this round's one-finding scope.

The view-level case added alongside is a GUARD, not a red-first case: it passed before this fix too. Measured on the pre-fix head, the reject issues a second evidence batch (one read before the decision, two after), so that flow was repopulated by a fresh read rather than by the record it already held. Nothing in the flow asks for that restart, which is why the snapshot must not depend on it; the case now fails the day it stops happening.

Proof at 688bedc58: the same seven named specs — 7 files, 311 tests passed; the whole Paper review directory — 16 files, 403 passed; npm run typecheck clean; npx eslint on the three changed files clean; git diff --check clean.

[chatgpt-codex-connector]

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.

[Chris0Jeky]

Review record (alpha product-trust lane; one fresh-context read-only reviewer subagent since Codex credits are exhausted, SC-9; round 1 at head 3a3257fb4, inputs: the clean worktree at the head and the merge-base diff, 12 files).

Verdict at round 1: SHIP on the merge-blocking bar, with one MEDIUM taken as a round-2 fix because it is a regression from the changed lines that contradicts this PR's purpose:

• MEDIUM, fix-now (round 2): railEvidence decides whether the record covers the active key with selectorKeysEqual, which treats 'rev-Y' vs null as different, while the key watcher decides whether to start a batch with proposalRevisionMoved, for which rev-Y → null is deliberately not a move. On that one transition (a proposal with a saved revision is rejected: the rejected DTO comes back with latestRevisionId: null and no approvedRevisionId, performReject swaps it in and the decision receipt keeps it active) no batch starts, no record is written for the new key, and the rail shows "Reading the confidence evidence…" and "Reading comparable past decisions…" permanently while the centre column's badge still shows the bare breakdown. Before this PR that path rendered the settled evidence. Fix: one shared predicate for "does this record cover the active key", used by both the watcher and the snapshot, plus a red-first composable regression (settle with rev-Y, swap in the rejected DTO, assert settled with values and no new request).
• LOW, handled outside the diff: no docs/STATUS.md line for the four new reviewer-facing sentences; the lane's coordinator records this PR in the next STATUS block, as for every lane PR, so no commit here.
• LOW, pre-existing, tracked on #1940: with source: 'model-reported' and zero components the author card renders "0.84 model-reported average" directly above "No model confidence reported"; the new settled gate narrows when the pair appears but does not remove it, and one new spec codifies it. Predates this PR.

Clean lenses corroborated by the reviewer against the tree at the head: the four batchRecord write sites plus the null-proposal reset each hold that key's data at every settled write; supersede, abort and return-to-a-settled-proposal paths (the fast path invalidates the superseded batch first, so its late continuation writes nothing); the failure branch cannot report failed over valid same-key data; computed dependency ordering (data written before the record); loading has no non-test consumer and matches the old isLoading in every state; waitForCoreBatch, the same-action retry and the Apply preflight unchanged; a caller-aborted batch labelled failed is never rendered because evidenceUnavailable hides both cards; the settled-empty and pending sentences are mutually exclusive by construction; idle is unreachable at the rail with an active proposal; the "(none found)" reservation changes only the visible label, not the disclosure's aria wiring or the #2662 count semantics; the failed copy promises no retry; the three components have one production consumer chain, which always passes the state; six keys × three catalogs with parity enforced by catalogs.spec.ts; the two view specs drive the real composable through a real queue-row click and fail on the unmodified source.

Unverified by the read-only reviewer, run by the lane before merge: the spec, typecheck and eslint claims at the head (the worker's report gives 7 files / 309 tests, 16 files / 402 tests, typecheck and eslint clean); the finding above is confirmed by the red-first spec in round 2.

Round count: 2 (this round's fix changes logic, so one read-only verification pass scoped to the fix diff follows). Merge gate remaining: the fix head's CI green, that pass, and the three-minute age.

[Chris0Jeky]

Verification pass scoped to the round-2 fix diff (3a3257fb4..688bedc58, one commit, 3 files; a second fresh-context read-only reviewer with the clean worktree at the fix head). Verdict: SHIP; no correctness defect introduced by the fix.

What it checked: selectorKeyStillCovers is exactly same proposal id, same capture reference, and revision equal or not moved, with both call sites passing (read key, active key) in the right order and no third call site; the old and new watcher conditions agree on every row of the transition table (same, X→Y, X→null, null→X, null→null, capture changed, proposal changed, either id absent, empty id), so which batches start is unchanged; the widened leg is only the revision X→null direction and the proposal id and capture reference stay exact, the five record write sites write the key active at write time, and the six reads are proposal-scoped, so the widened case cannot surface another proposal's or capture's values; the fast path, the three mid-flight publication guards, the abort, the capture-metadata refresh, waitForCoreBatch and the same-action retry all still use exact matching, so the #2460 Apply barrier is untouched; railEvidence.key has no non-spec consumer; loading and railEvidence.status derive from the same record and cannot disagree; the other proposalRevisionMoved consumers (the view, useReviewActions, useProposalRevisions) share the same asymmetry, so no cross-surface split; the composable regression drives a real watcher transition, its value assertions and its "no new read" assertion are both falsifiable, and the sibling A→B cases close the always-true hole; the view-level guard is labelled honestly and asserts nodes that exist only in the settled state.

Triage at the two-round ceiling (no third round):

• MEDIUM, residual, not introduced by the fix (the same state rendered loading before it): a decision landing while the batch is in flight leaves the record at loading under a key the predicate now covers, because the in-flight continuation's exact guard returns superseded and publishes nothing; Reject, Defer and Dismiss are not gated by the waitForCoreBatch barrier (Approve and Apply are). Already recorded on #1940 from the worker's own disclosure; reachability through the shipped view (whether the post-decision list refresh transiently nulls the active proposal and self-heals it) stays unverified.
• LOW, tracked on #1940: the exported PaperReviewRailEvidence.key doc does not say it is the key the values were read under (the explanation lives only at the return site).
• LOW, tracked on #1940: the helper's "strictly weaker than selectorKeysEqual" comment is not total (whitespace-only revision-id differences trim on one side and not the other); unreachable with backend GUIDs and self-healing.

Not run by the reviewer, covered by the worker's run at 688bedc58 (seven named specs, 7 files / 311 tests; the Paper review directory 16 files / 403; typecheck and eslint clean) and by CI's frontend unit job at the same head. Round count: 2. Merge gate remaining: CI green at 688bedc58 and the three-minute age.