fix(review): name an unavailable pinned proposal in Legacy and hold the loading announcement

[Chris0Jeky]

Summary

Two Legacy review truths, plus the half of the second that is identical in Paper.

views/LegacyReviewView.vue contained no occurrence of unavailableProposalId. The composable exports it and Paper consumes it, but Legacy did not, so a hash-pinned proposal the server refused with 403 or 404 fell through to the generic empty queue. The reviewer followed a link to a specific proposal and was told there was nothing to review, rather than that the thing they asked for is gone. Legacy now renders an explicit target-unavailable state with role="status" and a return-to-queue control, and recovers on its own when the pin resolves again, because the state is derived from unavailableProposalId and the rendered queue rather than latched.

The copy reuses the review.empty.unavailable.* keys Paper already renders (eyebrow, title, body, return), which exist in en, it and es. No locale keys are added or changed. The file's own copy is otherwise hardcoded English, and a comment there claimed $t appears nowhere in the file and that its specs install no i18n plugin. The second half was already untrue: src/tests/setup.ts installs the i18n plugin globally for every mounted component. The comment is corrected to match what the file now does. Forking Paper's wording into a fourth hardcoded English string would have drifted the two skins apart on the same state, which is exactly what the #1124 / ADR-0038 rule forbids.

Second, both queue live regions announced under the loading state. Legacy's sr-only role=status region renders above the v-else-if="proposalsLoading" skeleton, and Paper's ReviewQueueRail.vue renders its review.queueRail.liveAnnounce region with no loading gate at all. In both, the awaiting count is 0 while the queue read is in flight because nothing has been read yet, so a screen-reader user heard "0 proposals awaiting review." under the skeleton and then the real count. The first of those was never true. Both regions now withhold their content while loading.

The regions stay mounted and only their text is withheld. Gating with v-if would insert the live region at the same moment its text appears, which is unreliably announced, and would trade one defect for another.

Refs #2214

Closes the Legacy unavailable-pin state and the loading-announcement gate in both skins. The other residuals on the issue's claim comment remain, so this does not close the issue.

Changes

• frontend/taskdeck-web/src/views/LegacyReviewView.vue — consumes unavailableProposalId; adds a returnToReview handler that mirrors PaperReviewView.returnToReview (clears the deep link against the id the state names, not against whatever the hash holds by then); renders the target-unavailable panel ordered before the empty state and still requiring an empty render, so a pin that has already resolved shows its proposal; withholds the live region's content while proposalsLoading.
• frontend/taskdeck-web/src/views/paper/review/ReviewQueueRail.vue — new optional loading prop, default false, and the same content gate on its live region.
• frontend/taskdeck-web/src/tests/views/ReviewView.spec.ts — three regressions: the pin-unavailable state replacing the generic empty queue, the return-to-queue control clearing the hash, recovery when the target resolves again on the next background read, and one for the loading announcement.
• frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts — the loading gate, plus a guard that an omitted flag still announces.

PaperReviewView.vue, useReviewProposals.ts and usePaperReviewSelectors.ts are untouched.

Known gap, stated deliberately

ReviewQueueRail's loading prop is optional and defaults to false so PaperReviewView.vue needs no edit, because that file is held by open PR #2576. The rail therefore has the gate but Paper does not yet pass the flag, so Paper's live region still announces under its loading state in production. The remaining work is one attribute on the <ReviewQueueRail> element in PaperReviewView.vue:

:loading="proposalsLoading"

proposalsLoading is already destructured there. This is a one-line follow-up once #2576 lands, not a design question.

Test plan

Verified, from frontend/taskdeck-web after npm ci:

• Red first: npx vitest --run --maxWorkers=2 src/tests/views/ReviewView.spec.ts src/tests/views/paper/review/ReviewQueueRail.spec.ts before the fix — 5 failed, 70 passed (75). The failures were the three pin tests (expected false to be true on the unavailable panel), the Legacy loading announcement, and the rail loading announcement (both expected '0 proposals awaiting review.' to be ''). The two guard tests, that a loaded rail and a rail with no flag still announce, passed before the fix.
• After the fix, same command: 75 passed (75).
• npx vitest --run --maxWorkers=2 src/tests/composables/useReviewActions.spec.ts src/tests/views/ReviewView.spec.ts src/tests/views/ReviewView.coverage.spec.ts src/tests/components/review src/tests/views/paper/review/ReviewQueueRail.spec.ts src/tests/views/paper/review/PaperReviewView.spec.ts src/tests/i18n/catalogs.spec.ts — 16 files passed, 436 tests passed. PaperReviewView.spec.ts was run as an unmodified neighbour to prove the rail's new prop does not disturb the Paper view.
• npm run typecheck — clean.
• npx eslint src/views/LegacyReviewView.vue src/views/paper/review/ReviewQueueRail.vue src/tests/views/ReviewView.spec.ts src/tests/views/paper/review/ReviewQueueRail.spec.ts — clean, exit 0.
• npm run build — succeeded, PWA precache 142 entries.
• git diff --check — clean.

NOT verified: Playwright E2E (not run, no stack up). No real screen-reader pass; the announcement behaviour is asserted through the rendered live region's role and text, not through assistive technology. Paper's live region under its own loading state is not fixed in production, per the gap above. No backend tests run (no backend change). The full frontend vitest suite was not run, only the specs listed.

Boundaries and risks

• Files held by PR fix(review): bound the post-revision truth refresh and keep it retryable #2576 were not touched.
• The rail's new prop is additive and optional; every existing call site behaves exactly as before.
• The Legacy unavailable branch is ordered after the access-revoked and loading branches and before the empty state, and additionally requires renderedProposals.length === 0, so it cannot mask a hydrated proposal if the pin state briefly lags the hash.
• The corrected comment in LegacyReviewView.vue is a factual fix: the claim that its specs install no i18n plugin was already false at src/tests/setup.ts.

Round 2

Review found one MEDIUM and two LOWs. All three are addressed at 284de851b.

MEDIUM — the gate covered loading but not revocation. Round 1 gated on proposalsLoading alone. A current-scope 403 sets queueAccessRevoked and clears the queue in the same block, so the announcement changed from a real count to "0 proposals awaiting review." — a change, therefore spoken — beside a panel saying the queue is gone and has stopped updating. The same held in the Paper rail whenever the revoked state clears the queue. Reproduced before fixing: both new tests failed with expected '0 proposals awaiting review.' to be ''.

Both skins now gate on whether the count is a real count, not on one cause of it being unreal. Legacy computes countIsAnnounceable from !proposalsLoading && !queueAccessRevoked; the rail computes the same from its props.

The rail takes a second optional prop, queueUnavailable, rather than a single renamed boolean. Two props let PaperReviewView pass its two real states directly, so the reason a count is unspeakable survives at the call site instead of being flattened into a derived expression. Both remain optional and default to false, so PaperReviewView.vue still needs no edit now.

LOW, taken — the rail prop doc read as though the defect were closed. It now states plainly that the wiring is not yet in place, that Paper still announces under both states in production, and gives the exact follow-up: :loading="proposalsLoading" :queue-unavailable="queueAccessRevoked" on the <ReviewQueueRail> element, pending on #2214 and blocked on #2576.

LOW, taken with a correction — the renderedProposals.length === 0 half of the panel guard. The finding is right that nothing reddens if it is deleted. The suggested scenario does not fix that, and I measured it rather than assuming: with the half deleted, a test that moves from a refused pin X to a renderable pin Y still passes.

The reason is that the state is not observable in the DOM. The window where the recorded id still names X while the hash names a renderable Y exists only while proposalsLoading is true, because that is exactly when openProposalFromHash early-returns — and the loading branch precedes the panel in the same v-if chain, so the skeleton renders there, with or without the length half. When the read settles, loadProposalsWithOutcome sets proposalsLoading = false in its finally and then calls openProposalFromHash, which clears the recorded id synchronously in the same job, so Vue never flushes an intermediate render.

The half is therefore defence-in-depth against that ordering changing, not a live guard, and no honest spec can redden it. It is kept, because the alternative is a branch whose safety depends on a distant invariant in another module. The test is kept too, retitled and commented to say exactly what it pins — that moving from a dead pin to a live one shows the proposal and not the panel, which would redden if the clearing regressed or the panel were reordered ahead of the loading branch — and to record, in the spec itself, what it does not pin and why.

Not fixed, as directed: the blank-and-restore re-announcing an unchanged count on an explicit reload, and the return control dropping focus to body. Both are tracked by the reviewer; Paper has the same focus gap.

Round 2 verification

• Red first: npx vitest --run --maxWorkers=2 src/tests/views/ReviewView.spec.ts src/tests/views/paper/review/ReviewQueueRail.spec.ts — 2 failed, 76 passed (78), both revoked cases failing with expected '0 proposals awaiting review.' to be ''. After the fix: 78 passed (78).
• npx vitest --run --maxWorkers=2 src/tests/composables/useReviewActions.spec.ts src/tests/views/ReviewView.spec.ts src/tests/views/ReviewView.coverage.spec.ts src/tests/components/review src/tests/views/paper/review/ReviewQueueRail.spec.ts src/tests/views/paper/review/PaperReviewView.spec.ts src/tests/i18n/catalogs.spec.ts — 16 files passed, 439 tests passed, PaperReviewView.spec.ts again as an unmodified neighbour.
• Guard probe: with && renderedProposals.length === 0 temporarily deleted, ReviewView.spec.ts -t "resolvable pinned proposal" still passed. The file was restored from a byte-for-byte copy and git diff against the committed state confirmed no residue.
• npm run typecheck — clean. npx eslint on all four changed files — clean, exit 0. npm run build — succeeded. git diff --check — clean.

NOT verified in round 2: unchanged from round 1, and the known gap stands — Paper's live region is still ungated in production until #2576 releases PaperReviewView.vue. No Playwright, no screen-reader pass, no full frontend suite.

The Windows Frontend Unit red on this PR is the dev-up launcher suite (dev-up.test.mjs, the invalid-Vite-outcome and foreign-port-owner cases, 30 s timeout), the known CI-control cohort, unrelated to this change and untouched here.

Round 2 addendum — both skins wired, the known gap is closed

PR #2576 merged (35c7ba2b5), releasing PaperReviewView.vue. origin/main is merged into this branch as a deliberate merge commit (638de3ae8, no rebase); the merge was clean with no conflicts, and it touched no dependency manifests.

The deferred half has landed at 804683be3. The <ReviewQueueRail> element in PaperReviewView.vue now carries :loading="proposalsLoading" and :queue-unavailable="queueAccessRevoked" — two added lines, nothing else in that file, and specifically not the barrier region #2576 changed there.

The "Known gap, stated deliberately" section above is now closed. Paper no longer announces "0 proposals awaiting review." under its own loading state, nor when a 403 clears the queue beside the access-revoked panel. Both skins are wired: Legacy gates its own region on proposalsLoading || queueAccessRevoked; Paper passes the same two states into the rail, which withholds the announcement when either holds. The rail's prop docs drop the wiring-pending pointer and now name PaperReviewView as the caller.

The new PaperReviewView.spec.ts case walks all three states in one narrative: still reading (region mounted, text empty), read landed (the real count), access revoked (region mounted, text empty, revoked panel present). It was red against the unwired view with expected '0 proposals awaiting review.' to be '', failing on the loading phase before it ever reached the revoked phase.

Addendum verification

• Red first: npx vitest --run --maxWorkers=2 src/tests/views/paper/review/PaperReviewView.spec.ts -t "real count" before the wiring — 1 failed, 167 skipped (168).
• After the wiring: npx vitest --run --maxWorkers=2 src/tests/views/paper/review/PaperReviewView.spec.ts src/tests/views/paper/review/ReviewQueueRail.spec.ts src/tests/views/ReviewView.spec.ts — 3 files passed, 246 tests passed.
• npm run typecheck — clean. npx eslint src/views/paper/PaperReviewView.vue src/views/paper/review/ReviewQueueRail.vue src/tests/views/paper/review/PaperReviewView.spec.ts — clean, exit 0. npm run build — succeeded. git diff --check — clean.

NOT verified, unchanged: Playwright, screen-reader pass, full frontend suite, backend. The remaining non-blocking items are the reviewer's two tracked LOWs (blank-and-restore re-announcing an unchanged count on an explicit reload; the return control dropping focus to body, which Paper shares).

[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 gate (Codex credits exhausted, SC-9): one fresh-context adversarial reviewer on head 14a86d3 (merge base 2803355). Verdict: SHIP. Confirmed: unavailableProposalId is set only at the mount identity-mismatch/404 sites and the poll's pin 403/404, wrong-identity or cross-scope site, and cleared on every recovery path, so the Legacy panel is derived, never latched; its v-else-if position below the revoked panel and the loading skeleton and above ReviewEmptyState matches Paper's order and makes the stale-id-while-loading window unreachable in Legacy; the v-else-if chain survives the interleaved HTML comment; returnToReview mirrors Paper's clearProposalDeepLink and does not leave the pin armed; refreshProposals never raises proposalsLoading, so background polls cannot blank the region and no state shows a count while an explicit load is in flight; the four locale keys exist in en, it and es with the same placeholder and the corrected i18n comment is accurate; the rail prop is additive and defaulted; the recovery spec's fake timers leave setTimeout real like the sibling #2194 specs; four files only.

Findings by bin:

• Fixed in the same round (same defect class, one branch over, and the rail's suppression contract must cover it before Paper wires it): MEDIUM, the gate covers loading but not queueAccessRevoked, so a current-scope 403 that clears the queue changes the region text to "0 proposals awaiting review." (spoken, because it is a change) under the revoked panel; both skins' regions now withhold the count while loading or revoked. Two LOWs folded in: the rail's prop doc gains the pointer to the pending Paper wiring on Review queue refresh residuals: deep-link re-auth, poll restart after recovery, stale indicator (from PR #2208) #2214; the renderedProposals half of the panel guard gains a spec.
• Tracked: [Frontend][A11y][Review] Queue live-region residuals from PR #2593 review: re-announce on explicit reload, focus after the unavailable-pin return control #2599 (re-announcement of an unchanged count on explicit reload; focus drops to body after the return control, Paper too).
• Coordinator-owned: docs/STATUS.md:241's "Legacy renders no translated strings" sentence is corrected in the next alpha STATUS block (Legacy now renders four $t calls in this panel; the summary-card labels were already translated through the composable).

Evidence at the reviewed head (implementation run): five regressions red first (5 failed / 70 passed) then 75/75; 16 files / 436 including the untouched PaperReviewView spec; typecheck, ESLint, build, diff check. Not verified: Playwright, any screen-reader run (STATUS records none has ever been used on this region). The Windows Frontend Unit red on this head is the dev-up launcher suite (foreign frontend-port owner case, 30 s timeout), the CI-control cohort, unrelated. Merge after the fix commit ages and ci-required is green at the new head.

[Chris0Jeky]

Round 2 (head 284de85): the fix diff was verified by the coordinator reading it. Both skins now gate the queue live region on whether the count is a real count: Legacy computes countIsAnnounceable = !proposalsLoading && !queueAccessRevoked; the rail takes a second optional, defaulted prop queueUnavailable alongside loading so Paper can pass its two real states without flattening the reason, and PaperReviewView.vue still needs no edit now. Both revoked cases were red first (expected '0 proposals awaiting review.' to be '') and are green after. The rail's prop doc names the pending Paper wiring (:loading and :queue-unavailable on the ReviewQueueRail element, pending on #2214, blocked on #2576). On the renderedProposals half of the panel guard: the worker measured that deleting it changes nothing DOM-observable (the only window where the recorded id names X while the hash names a renderable Y is while proposalsLoading is true, when the loading branch precedes the panel, and the read's finally clears the id in the same job), so the guard stays as a structural safety with the spec retitled to say what it pins and does not pin. Counts at this head (implementation run): 78/78 on the two view specs (red first 2 failed / 76 passed), 16 files / 439 including the untouched PaperReviewView spec, typecheck, scoped ESLint, build, diff check. Round count: 2. Merge after ci-required is green at 284de85 and the head has aged.

[Chris0Jeky]

Round 2 addendum (head 804683b, after a clean merge of main 35c7ba2): the deferred Paper half is in. PaperReviewView passes :loading="proposalsLoading" and :queue-unavailable="queueAccessRevoked" to ReviewQueueRail (the diff on that file is exactly those two attribute lines; the #2576 barrier region is untouched), the rail's prop doc no longer says the wiring is pending, and a PaperReviewView spec walks the three states (reading: region mounted with empty text; landed: the real count; 403 poll: region empty beside the revoked panel), red first against the unwired view (expected '0 proposals awaiting review.' to be ''). Verified at this head: PaperReviewView, ReviewQueueRail and ReviewView specs 3 files / 246 passed; typecheck; scoped ESLint; build; diff check. The #2214 known gap stated in round 1 is closed; both skins are gated. Merge after ci-required is green at 804683b and the head has aged.