fix(review): the #2214 residual pass, and the three open items it confirmed - #2710
Conversation
The explicit list read's 403 raised the durable revoked panel AND a generic "Failed to load proposals" toast: two reports for one fact, the shape #2694 removed on the pin leg. The panel names the revocation and its remedy and stays; the toast named neither and was gone seconds later. Only the 403 branch loses its toast, and the 'failed' outcome every action composable depends on is unchanged. The route-hash watcher had no queueAccessRevoked guard, while the explicit load's own openProposalFromHash call site has had one since #2694, so a #proposal- link followed under the revoked panel still asked the by-id route about a target inside a board the server had refused wholesale and wrote its refusal into unavailableProposalId as a second, narrower and wrong account of the same fact. Latent rather than visible today only because the revoked panel is the first branch of both skins' empty chains. Adds awaitingProposalIds and queueAnnouncementKey: the ordered ids of the proposals both skins count as awaiting review, and that identity as one primitive. One predicate for the count and for what it stands for, so the skins cannot drift (#1124 / ADR-0038) and a count-neutral queue replacement has something to key an announcement on (#2214 item 4). Refs #2214
Both queue live regions derived their sentence from the pending count alone, so a poll that removed one pending proposal and added another in the same response rendered a byte-identical "3 proposals awaiting review.": no DOM mutation, nothing announced, the queue moved under the reviewer in silence. That is #2214 item 4, and the false-negative class #2194 exists to remove. The sentence and its count are unchanged. Each skin now wraps it in a node keyed on the composable's queueAnnouncementKey (the ordered awaiting ids), so a changed queue replaces that node inside a live region that itself stays mounted -- the node addition aria-live's default aria-relevant="additions text" announces -- while a byte-identical queue keeps its key and stays silent. Blanking the text for a frame and restoring it was rejected: it recreates the "inserted together with its text" shape #2593 and #2630 both call unreliably announced. ReviewQueueRail takes the key as an optional prop rather than deriving it: its items prop is the whole visible queue, not the awaiting set the count is about, and a rail-local derivation is exactly how the two skins drift. Both views now take awaitingCount from the same composable predicate instead of recomputing it (Paper) or reading it off summaryCards (Legacy); the rendered value is identical. Refs #2214
|
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; one fresh-context read-only reviewer subagent since Codex credits are exhausted, SC-9; round 1 at head Verdict at round 1: SHIP; no CRITICAL or HIGH, and no realistic path from the changed lines to a lost report, a lost deep link, a spurious per-poll announcement or a skin drift. Triage, one pass:
Clean lenses corroborated against the tree at the head: the keyed node is the direct child of a live element that is never Unverified by the read-only reviewer, run by the worker at the head (six named specs 6 files / 492 tests; typecheck, build, eslint, diff-check clean) and by CI's frontend unit job. Round count: 1. Merge gate remaining: |
1 participant
The
#2214residual pass the 2026-09-04 coordinator note asked for, plus the composable- andview-scoped open items that need no ruling.
Summary
Every residual ever recorded on
#2214— the issue body's items 1-5, the "Fixed in the PR's finalround" trail, and all 24 comments — was checked against
mainata8a3f3496, together with thethree
#2599rows the coordinator folded into this pass.37 residuals: 23 shipped, 1 superseded, 13 open. This PR fixes 3 of the 13 open; the other 10
have a row saying why they stay out. Nothing was struck on a PR body's word — every "shipped" row
names a line or a spec on
main.Attribution correction for the record: the pin-leg-truth work (a 400 on the pin leg as
pinUnavailable,expectedStatuseson the background pin read) merged as PR#2658(
issue-2214/pin-leg-truth), not#2668; the refusal-retraction sentence and the recoveryretirement rule merged as PR
#2702(issue-2638/recovery-retire).Residual table (condensed)
Full table with per-row evidence: posted separately by the coordinator.
Shipped, verified against
main(23)main#2357useReviewProposals.ts:1174-1249; specuseReviewProposals.spec.ts:1362#2298useReviewProposals.ts:1340-1344, called at:787-789#2388:60threshold, raised:883-894;LegacyReviewView.vue:379-388,PaperReviewView.vue:2753-2763#2388:53REVIEW_QUEUE_REQUEST_DEADLINE_MS = 8_000;awaitQueueRefreshRequest:1014-1039on both legs#2593LegacyReviewView.vue:234-236; specReviewView.spec.ts:1852useReviewProposals.spec.ts:1678(success),:1925(403)#2658useReviewProposals.ts:1194-1198expectedStatuses: [400, 403, 404]#2658isMalformedTargetError:182-185; pin-leg branch:1230-1239#2593LegacyReviewView.vue:429-448#2630queueRefreshRecoveredKind:329;LegacyReviewView.vue:344-358+ Paper twin#2694isRefusedQueueRefreshFailure:221-226;queueRefreshRefusedraised:859-867#2445recordQueueRefreshFailure:876-882#2630.paper-review-deep__queue-stale--pinned { position: sticky }PaperReviewView.vue:3157-3158#2630review.queue.degraded.bodyin en/it/es;PaperReviewView.vue:2762/:2978,LegacyReviewView.vue:387#2630PaperReviewView.spec.ts:4217-4254#2357sentencedocs/STATUS.md:221#2694isRefusedQueueRefreshFailure:221-226#2694unavailableProposalMalformed:288;LegacyReviewView.vue:439-440#2694openProposalFromHash:718-727#2702QueueRecoveryKind:254;raiseQueueRecovery('refused', …):947#2638: recovery retired by any successful load#2702backgroundQueueReadCount:374-386, retirement:991-1004#2599(3): Paper does not pass the rail's suppression props#2593PaperReviewView.vue:2730-2731#2298useReviewProposals.ts:786-789Superseded (1)
:1110-1116to "EVERY single-proposal decision path (… in useReviewActions)", which is true and scoped.upsertProposal(:620-635) still writes in place and is still not named, but no false statement remains and its only caller is stillopenProposalFromHash(:696)Open (13) — 3 fixed here
PaperReviewView.vue:2980-2986renders the revoked title as a plain<h2>with no role, noaria-liveand no focus move, and the only status region in that branch isv-if'd off while revoked. Needs a new always-mounted region per skin (the#2630pattern) plus a focus decision: its own slice#2694round 2)#2694round 2)badgeSyncArmed)LegacyReviewView.vue:244-252,PaperReviewView.vue:2538-2547; the catch never assignsproposals, so the first poll success is consumed arming the flag. Workload-badge wiring in both skins, not a live regionbadgeSyncArmedfirst-assignment skipsrc/api/http.ts:132logs in theerror.requestbranch, whichexpectedStatuses(:87-93) does not guard.src/api/**is not owned by this claimdocs/STATUS.md:428still says the Paper stale copy's localization "is still open as#2214item 4", false since#2630and mis-numbered. Docs are outside this claim#2638decision-rail sticky offset inert#2599(1) re-announcement of an unchanged count on an explicit reload#2599" below#2599(2) focus after the unavailable-pin return controlChanges per item
1. Item 4 — announce a count-neutral queue replacement, in both skins
Both live regions rendered a sentence derived from the pending count alone. A live region only
speaks when its text changes, so a poll that removed one pending proposal and added another in the
same response produced a byte-identical
"3 proposals awaiting review.", mutated nothing, andannounced nothing. The queue moved under the reviewer in silence — the false-negative class
#2194exists to remove.
useReviewProposalsgainsawaitingProposalIds(the ORDERED ids of the proposals both skins countas awaiting review) and
queueAnnouncementKey(that identity as one primitive). One predicate nowowns the count AND what it stands for: Legacy read the number off
summaryCards'pending-reviewcard and Paper recomputed the same filter inline, so neither skin had any notion of WHICH proposals
the number was about. Both views now take
awaitingCountfromawaitingProposalIds.length; therendered value is identical.
(
LegacyReviewView.vue,ReviewQueueRail.vue). Re-keying replaces the node inside a live regionthat itself stays mounted, and a node addition is what
aria-live's defaultaria-relevant="additions text"announces. The sentence and its count are unchanged. Abyte-identical queue keeps its key and stays silent; the loading/revoked gate from
#2593stillwithholds the whole thing and its specs are untouched.
with its text" shape that
#2593and#2630both call unreliably announced, and it would need atimer that every existing fake-timer spec would have to know about.
ReviewQueueRailtakes the key as an optional prop (default'') rather than deriving it: itsitemsprop is the whole visible queue, not the awaiting set the count is about, and a rail-localderivation is exactly how the two skins drift (
#1124/ ADR-0038).2. The explicit list read's 403 no longer toasts beside the revoked panel
useReviewProposals.ts:808-810raised the durable revoked panel and the genericreview.toast.loadProposalsFailed— two reports for one fact, the shape#2694removed on the pinleg, and the weaker of the two: the toast names neither the revocation nor its remedy and is gone
seconds later, contradicted by a panel that stays. Only the 403 branch loses its toast; every other
explicit failure keeps it, and the
'failed'outcome every action composable depends on isunchanged. The LIST-leg authority path (
recordQueueAccessRevoked) is untouched.3. The hash watcher no longer marks a pin unavailable under a revoked queue
The route-hash watcher had no
queueAccessRevokedguard, while the explicit load's ownopenProposalFromHashcall site has had one since#2694. A#proposal-link followed while therevoked panel is up — a stale rail row, a bookmark, the back button — still asked the by-id route
about a target inside a board the server had refused wholesale, and wrote that refusal into
unavailableProposalIdas a second, narrower and wrong account of the same fact. Latent rather thanvisible today only because the revoked panel is the first branch of both skins' empty chains: the
state was written, and the next surface to consume it would read a lifecycle claim ("applied,
archived, or removed") about a proposal that was none of those things.
4. The two LOWs from PR
#2694's round 2 — stated exactly, both left open"Double announcement." Recorded text: "a refusal rising while the degraded warning is visible
announces the sentence twice (the visible region's text change plus the new sr-only region); the
alternative strips
aria-livefrom the visible warning, which#2630's specs pin." Onmain: withqueueRefreshStalealready true the visible panel (LegacyReviewView.vue:379-388,PaperReviewView.vue:2971-2979) is mounted witharia-live="polite" aria-atomic="true"showingreview.queue.degraded.body; whenqueueRefreshRefusedrises its text changes toreview.queue.refused.body(spoken) and the always-mountedsr-onlyregion(
LegacyReviewView.vue:365-371) goes from''to the same sentence (spoken again).Not fixed, because no variant is a correct one-liner: gating the
sr-onlyregion on!queueRefreshStalere-announces spuriously whenstalelater clears whilerefusedstill stands;latching on whether the panel was already mounted needs new state; and moving the announcement wholly
into the
sr-onlyregion means strippingrole/aria-live/aria-atomicfrom the visible panel,which
#2630deliberately pinned (PaperReviewView.spec.ts:4237-4239,ReviewView.spec.ts:377-379)and which would silence the degraded ONSET. The issue comment says a deliberate choice is needed; it
still is.
"Empty-queue wording." Recorded text: "the refused body's 'these are the last proposals it
confirmed' reads oddly beside an empty queue on the boardId-400 entry path (the initial load 400s
too); the disclosure is true (the confirmed set is empty) but the two sentences read as
contradictory." On
mainthe one string isreview.queue.refused.body= "This review queue hasstopped updating. The server is refusing the refresh rather than failing temporarily, so these are
the last proposals it confirmed. Reload the page, or check the board filter in the address bar."
Not fixed: the recorded remedy is "a count-aware variant of
review.queue.refused.body" — a newsibling key authored in en, it AND es, a count predicate branched in both skins, and view specs in
both. Not a one-liner, and it needs Italian and Spanish copy this lane cannot native-review. It is
also copy for the REFUSAL disclosure, not for the awaiting-count announcement this PR changes, so the
two need not land together.
No locale catalog changed in this PR. The awaiting-count sentence keeps its exact wording in all
three catalogs.
#2599Audited as instructed. Item 3 is already shipped (
PaperReviewView.vue:2730-2731, PR#2593).Item 2 (focus after the return control) is focus management in both skins and stays out. Item 1 is
not a one-line consequence of the identity key, so it stays out too, and this PR carries no
Refs #2599:and announces once;
unmounting during a reload, i.e. replacing the
!proposalsLoadingterm with "no read has landedfor the CURRENT board scope yet". That means a new load-scope signal in the composable, a gate
change in both skins and a third rail prop, and it interacts with the board-filter change (where
the count on screen genuinely stops being real mid-load) and with a failed entry load. It changes
what
#2593pinned and wants its own review lens.Test plan
Red first. All seven new assertions were run against the unmodified source before any fix:
One further rail spec — "keeps the whole announcement withheld while the count is unspeakable, key
or no key" — was green on arrival and is a pin, not a fix: it stops the re-announcement from
ever speaking under the
#2593loading/revoked gate.The count-neutral view specs assert the real mechanism, not a proxy: the announcement node's DOM
element is
.not.toBe()the previous one after a swap,.toBe()it after a byte-identical poll, andthe live region's own element is
.toBe()itself throughout (the region is never remounted).Green at the head, all run in this worktree:
useReviewProposals.spec.tsalone was also run immediately after the composable fix: 167 passed.Not verified
keyed node is genuinely replaced inside a region that stays mounted — not that a given AT speaks
it. That a node addition inside a polite live region is announced is
aria-live's documenteddefault (
aria-relevant="additions text"), reasoned from the spec, never observed here.(
review-queue-announcement,paper-review-queue-announcement); every existing one, includingreview-queue-liveandpaper-review-queue-liveand their text contracts, is untouched.main; they were noteach re-exercised at runtime.
docs/orOUTSTANDING_TASKS.mdwas touched, so the two open docs rows above(
docs/STATUS.md:428, and the#2214residual list itself) are left for the coordinator.Boundaries
Owned and changed:
src/composables/useReviewProposals.ts;src/views/LegacyReviewView.vue;src/views/paper/PaperReviewView.vue;src/views/paper/review/ReviewQueueRail.vue; and the fourspecs
src/tests/composables/useReviewProposals.spec.ts,src/tests/views/ReviewView.spec.ts,src/tests/views/paper/review/PaperReviewView.spec.ts,src/tests/views/paper/review/ReviewQueueRail.spec.ts.Not touched:
usePaperReviewSelectors.tsand the rail cards (#2705's files), the receiptrecorders, the diff pane, the sticky seam (D-15),
src/api/**, the locale catalogs,docs/**,OUTSTANDING_TASKS.md.Refs #2214