Show the answer wait's evidence rail reliably, and let it accrue - #2588
Conversation
The answer wait was built to show the retrieved documents as cards under the status line, where the finished answer's own source rail lands. Two things kept that from reaching a reader. Governance suppression was answer-wide. buildEvidencePreviewUnit ran the canonical danger-level source-governance check over the whole retrieval set and returned null on any hit, so one outdated or badly-extracted chunk among twelve to twenty-four retrieved passages blanked the entire rail — and the sources it hid were the clean ones. The decision is now per document: a danger-level document is excluded from the preview and the rest are shown. That is strictly safer per card, because such a document can no longer appear in the preview at all, where the wide check merely delayed it until the answer's own rail. The answer-level verdict stays all-or-nothing: relevance.verdict === "none" is not a property of any one document, so no subset of the rail is safe to show. buildEvidencePreviewProgress no longer needs its governanceResults argument, so it and the one call site in rag.ts drop it. The cards also arrived as a single block. Every source reaches the browser in one stream event and the contract carries exactly one preview per answer, so the rail mounted whole and a 90ms CSS cascade finished before a reader watching an otherwise still screen registered it. useProgressiveReveal now reveals one card per --duration-moderate, with the first standing the instant the unit arrives so time to first useful content is not spent on the animation. The status line reads its count from the same hook, so the number always equals the cards beneath it — the wait's one copy rule. Motion suppressed returns the full count with no timer in the path, read through a subscription so the in-app Reduce motion toggle reaches a wait already on screen. RAG impact: no retrieval behaviour change — evidence-preview governance scope only; retrieval, ranking, selection and the final payload are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
The rail's stagger override was pinned here as a `--stagger-cascade-wide` animation-delay, which is the mechanism the reveal replaces. The contract worth keeping is that the two cannot compound: a delay override plus mount pacing puts the last card at twice the intended distance. Also pins the reveal interval to the `--duration-moderate` token rather than an invented rung, keeps the shared cascade rule unlayered so the rail's own component styles cannot take the entrance away, and keeps both halves of the reduced-motion guarantee — the CSS resets and the hook's own check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
The pacing observer matched only the added node itself, so the first card — which arrives as a child of the rail container rather than as its own insertion — was never recorded. The reveal looked one beat shorter than it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
sourceGovernanceWarnings ends with `.slice(0, limit ?? 8)`, a cap sized for a warnings banner. Deriving the danger-document set from that list made the cap a gate: with nine or more danger warnings — five documents that are both outdated and poorly extracted is enough, at two warnings each — the entries past the eighth were dropped, their documents never reached the exclusion set, and they were disclosed as preview cards. A document escaping on extraction_quality alone was worse still, because sourceStatusShortLabel keys only on document_status and would badge it "Current". Governance is now asked one source at a time, which cannot hit the cap and still runs the canonical helper rather than reimplementing its rules. The answer-level verdict is asked with no sources at all, so only the relevance-derived warning can be raised. Found in clinical-governance review of ecef490. RAG impact: no retrieval behaviour change — evidence-preview governance scope only; retrieval, ranking, selection and the final payload are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
The design doc stated in three places that any danger-level source emits zero evidence previews, and that the preview runs the same refusal as the final response. Neither is true since the per-document decision landed, and the doc is normative — a reader who trusts it would be reading the opposite of the code. Records the amendment, why the wide check had to go, and the residual it costs: when the final response refuses on source governance it blanks its source list, so a preview that showed clean cards has disclosed identities the final response then withholds. Says so plainly rather than leaving it to be found. Scoped to the evidence preview; Phase 2 answer sections keep the unamended rule. Also pins the cap trap that produced the review finding on ecef490, and records the paced reveal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1e073409-5323-4f3a-963a-8cea571fc2a4) |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f6588c799
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
…n-reveal Two findings from the Codex review of 9f6588c, both real. The reveal gated on prefersReducedMotion(), which recognises only data-motion="reduced" before falling through to the OS media query. The app's Motion preference has three states and its CSS honours an explicit "Full" over an OS reduce request (html:not([data-motion="full"])), so a reader in that combination had this one rail dumped whole while everything around it animated. settings-dialog.tsx already carries a local three-state copy with a comment saying the shared helper does not honour the opt-in; motionIsSuppressed() now puts that resolution in the shared module, beside the scroll helper it deliberately differs from, rather than adding a third copy. Motion is a live subscription, so the preference can change mid-generation. With the count restarting on that change, a reader who had suppressed motion — filling the rail — and then re-enabled it watched six cards they were already reading vanish and accrue again. The count is now monotonic for one preview: a preference change can complete the rail early, never take a card back. The interval also became a chain of timeouts keyed on the current count. It terminates on its own when the rail is full, needs no self-clearing bookkeeping inside a state updater, and survives the count moving for reasons other than a tick. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
…he change The preference is watched with a MutationObserver, whose callback is a microtask, so a synchronous act() let the assertion pass without the component ever re-rendering under the new preference. Awaiting act() makes the test fail against the defect it describes: without the monotonic count the rail drops from six cards to one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #15355 (success). That run's conclusion is an aggregate and did not exercise Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Summary
buildEvidencePreviewUnitran the canonical danger-level source-governance check over the whole retrieval set and returnednullon any hit, so one outdated or badly-extracted chunk among twelve to twenty-four retrieved passages blanked the entire rail — and the sources it hid were the clean ones. The decision is now per document: a danger-level document is excluded, the rest are shown. Per card that is stricter than before, because such a document can no longer appear in the preview at all, where the wide check merely delayed its appearance until the answer's own rail. The answer-level verdict (WEAK_EVIDENCE, fromrelevance.verdict === "none") still suppresses the whole preview — it is not a property of any one document.sourceGovernanceWarnings' returned list. That list ends with.slice(0, limit ?? 8), a cap sized for a warnings banner; reading the danger entries back out of it made the cap a gate, and with nine or more danger warnings a flagged document was disclosed as a card — badged "Current" if it escaped onextraction_qualityalone, sincesourceStatusShortLabelkeys only ondocument_status. Found in clinical-governance review of the first commit here, and pinned by a test proven to fail against the capped version.buildEvidencePreviewProgressno longer needsgovernanceResults; it and the one call site inrag.tsdrop it.useProgressiveRevealnow reveals one card per--duration-moderate, with the first standing the instant the unit arrives so time to first useful content is not spent on the animation. The status line reads its count from the same hook, so the number always equals the cards beneath it. Motion suppressed returns the full count with no timer in the path, read through a subscription so the in-app Reduce motion toggle reaches a wait already on screen.docs/verified-answer-incremental-delivery-design.mdstated the opposite of all this in three places. Amended, including the residual the narrowing costs — see The one judgement call below.The one judgement call
Calling this out here rather than burying it in a checkbox, because it is the part worth a second opinion before merge.
Per card the change is stricter than what shipped: an outdated or poorly-extracted document can no longer appear in the preview at all.
The residual is that the final response computes its refusal over the wide retrieval set (
answer.sources = answerInputResultsinrag.ts), not over the preview's candidate set. So a danger-level document outside the preview's candidates can still make the final response refuse and blanksources: []— after the preview had shown clean cards. Those cards are documents that passed governance individually and are reachable by ordinary search; they are removed the moment the answer lands, because the rail renders only while the request is in flight; and the refusal still blocks the synthesised answer either way.This is a deliberate narrowing of the invariant in
docs/verified-answer-incremental-delivery-design.md, approved by the owner, and now recorded there as a dated amendment rather than left as a doc that disagrees with the code. It is scoped to the evidence preview; Phase 2 answer sections keep the unamended rule.Verification
npm run verify:pr-local— all static, docs, ledger, lint and typecheck gates completed.testexits 1 on three pre-existing failures that are shallow-clone artefacts of the container, not this diff:clinical-hazard-controls,privacy-readiness-contractandrag-plan-package-parityall fail on missing git history (reviewedCommit does not exist 883f1007…,reconciledBase is unavailable locally f3d1a3cc…). Confirmed identical with this branch's changes stashed. Everything else:14369 passed | 3 failed | 3 skipped.npm run verify:cheap— same three pre-existing failures, nothing else.npm run build— passed, includingClient bundle secret surface check passed.npm run eval:rag:offline—Offline RAG fixture and manifest validation passed (36 golden cases, 26 suites),630 passed.npm run eval:rag:adversarial:offline—25 passed, 24 adversarial cases recorded.npm run plan:browserselected level: full becauseglobals.cssis a shared foundation, so the focused run below is explicitly not the full gate and the full Chromium suite is left to CI. Every journey the planner attributed to this diff was run on Chromium against the production server and passed:answer-progress-ui-smoke.spec.ts8 passed (36.7s),ui-phone-motion+ui-universal-search25 passed (46.7s),ui-smoke106 passed (3.2m).npm run eval:retrieval:quality— not applicable, not run. Retrieval, ranking, selection, chunking and scoring are untouched; the onlysrc/lib/rag/edit removes one argument from a progress-event call.npm run eval:rag -- --limit 15/npm run eval:quality -- --rag-only— not applicable, not run. Answer generation, the synthesis prompt and answer post-processing are unchanged; this changes what the wait discloses, not what is generated.npm run check:production-readiness— Verification not run: it cannot produce a signal in this container. It fails on absent local credentials (NEXT_PUBLIC_SUPABASE_URL,SUPABASE_SERVICE_ROLE_KEY,OPENAI_API_KEY), on the same shallow-clone ancestry error, and on the six pre-existing release-blocking privacy approvals tracked as#BS3SN9. None relate to this diff.Risk and rollout
RAG_INCREMENTAL_EVIDENCE_PREVIEW=falseto stop server emission first, thenNEXT_PUBLIC_RAG_INCREMENTAL_EVIDENCE_PREVIEW_RENDER=false(build-time, needs a rebuild) to stop client rendering. No schema or stored-data rollback is needed.src/lib/rag/rag.tsedit removes one argument from thestage: "ranking"progress event and nothing else.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Preflight notes
Evidence for each box above, in the same order:
trimSourceForClient, unchanged. The preview payload is a subset of what the previous code sent when it sent anything, and owner scoping happens upstream in retrieval, which this does not touch.Notes
clinical-governance-reviewer, which found the display-cap defect (P1, fixed ind0890d1with a mutation-proven regression test) and the invariant/doc drift (P2, addressed in9f6588c).tests/answer-progress-indicator-css.test.tspreviously pinned the pacing as a CSSanimation-delayoverride. That rule is gone, so the test now pins what replaced it — that mount pacing and a delay cannot compound, that the reveal interval is a named duration token rather than an invented rung, and that both halves of the reduced-motion guarantee survive.mainwas merged into this branch by branch-sync automation (da1cc81). The merge is clean and left every file in this change untouched; the affected tests plus the two source-presentation suites that arrived with it pass on the merged head (66 passed).🤖 Generated with Claude Code
https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa