From ecef490b07397562ad514071ccc68e83cf7fe410 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 19:38:19 +0000 Subject: [PATCH 1/8] feat(answer): show the evidence rail reliably, and let it accrue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- src/app/globals.css | 33 +++--- .../answer-evidence-preview.tsx | 30 +++++- .../clinical-dashboard/answer-status.tsx | 79 +++++++++++++- src/lib/answer-preview.ts | 64 ++++++++--- src/lib/rag/rag.ts | 1 - tests/answer-evidence-preview.dom.test.tsx | 100 +++++++++++++++--- tests/answer-incremental-delivery.test.ts | 43 +++++++- tests/answer-progress-ui-smoke.spec.ts | 71 +++++++++---- 8 files changed, 349 insertions(+), 72 deletions(-) diff --git a/src/app/globals.css b/src/app/globals.css index 4a433a54e8..74a6678995 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -4407,24 +4407,21 @@ html[data-motion="reduced"] .answer-progress-dot { animation-delay: calc(var(--stagger-index, 0) * var(--stagger-cascade)); } -/* The answer wait's source rail arrives one card at a time rather than as a single movement. - The default `--stagger-cascade` rung is right for a results grid the reader is about to - scan, but six cards in 175ms reads as one block appearing. On `--stagger-cascade-wide` the - last card lands around 450ms, so each is separately noticeable, and the whole rail is still - standing long before generation ends. - - Scoped to this rail deliberately — the shared class also drives the prose skeleton bars - immediately above it and the search result grids, which keep the 35ms rung. Declared here, - unlayered and adjacent to the rule it overrides, because `.answer-sources-arriving` itself - lives in @layer components and a layered rule would lose to this unlayered one whatever its - specificity. - - It cannot reintroduce motion when motion is suppressed: both reduced-motion paths below set - `animation: none !important` on `.stagger-item`, and that shorthand resets animation-delay - to 0s with it, so the cards stay immediately and fully visible. */ -.answer-sources-arriving .stagger-item { - animation-delay: calc(var(--stagger-index, 0) * var(--stagger-cascade-wide)); -} +/* The answer wait's source rail arrives one card at a time, and the pacing is NOT here. + + This used to be a `--stagger-cascade-wide` delay override, because every card mounted at + once and only an animation delay could separate them. Six cards then landed inside 450ms — + correct to the millisecond, and over before a reader looking at an otherwise still screen + registered that anything had happened. The rail is now revealed a card at a time by + `useProgressiveReveal` in `answer-status.tsx`, one per `--duration-moderate`, so each card + animates on mount with the shared `.stagger-item` rule and no delay of its own. The cards + deliberately set no `--stagger-index`: mount timing and an animation delay would compound, + and the last card would arrive at twice the intended distance. + + The pacing owner moved, not the reduced-motion contract. The hook returns the full count + immediately when motion is suppressed, and both reduced-motion paths below independently set + `animation: none !important` on `.stagger-item`. Suppressing motion must never withhold + content on this surface — that is the rule this rail was built under. */ @media (prefers-reduced-motion: reduce) { html:not([data-motion="full"]) .animate-skeleton-shimmer::after, diff --git a/src/components/clinical-dashboard/answer-evidence-preview.tsx b/src/components/clinical-dashboard/answer-evidence-preview.tsx index 7586498c22..991c8dfb18 100644 --- a/src/components/clinical-dashboard/answer-evidence-preview.tsx +++ b/src/components/clinical-dashboard/answer-evidence-preview.tsx @@ -41,9 +41,28 @@ export const visiblePreviewSourceLimit = 6; * * Each card is a real link to the real page, so a reader who recognises a * document can open it without waiting for the answer at all. + * + * **The cards arrive one at a time, and that pacing is presentation.** Retrieval is a + * single call, and the stream contract carries exactly one evidence preview per answer + * (`sequence: 0`), so every source in this rail was found in the same instant. What the + * pacing buys is a wait that accrues instead of a screen that sits still for several + * seconds and then blinks. Nothing here claims live discovery: the status line reads + * "N sources found", and N is always the number of cards standing beside it, which is the + * one rule the wait's copy is held to. */ -export function AnswerEvidencePreview({ preview }: { preview: VerifiedEvidencePreviewUnit }) { - const visibleSources = preview.sources.slice(0, visiblePreviewSourceLimit); +export function AnswerEvidencePreview({ + preview, + revealedCount, +}: { + preview: VerifiedEvidencePreviewUnit; + /** How many of the capped sources are on screen right now. The rail does not own this + * number: `AnswerProgress` paces it so the status line and the cards can never disagree + * about how many sources have been found. Omitted, every capped source shows at once, + * which is what a caller with no pacing state (and every reduced-motion reader) gets. */ + revealedCount?: number; +}) { + const cappedSources = preview.sources.slice(0, visiblePreviewSourceLimit); + const visibleSources = cappedSources.slice(0, revealedCount ?? cappedSources.length); if (visibleSources.length === 0) return null; return ( @@ -53,7 +72,7 @@ export function AnswerEvidencePreview({ preview }: { preview: VerifiedEvidencePr aria-label={`Sources found so far, ${visibleSources.length}. Not yet numbered — the answer decides the final list.`} className="answer-sources-arriving flex gap-1.5 overflow-x-auto pb-1" > - {visibleSources.map((source, index) => { + {visibleSources.map((source) => { const title = cleanDisplayTitle(source.title); // Freshness, not the section heading. A section heading is orientation a // reader gets anyway once the card is opened; whether the document is @@ -68,7 +87,10 @@ export function AnswerEvidencePreview({ preview }: { preview: VerifiedEvidencePr href={sourceResultHref(source)} data-testid="answer-evidence-preview-source" aria-label={`Open source found so far: ${title}, page ${source.page_number ?? "unknown"}, ${status}`} - style={{ "--stagger-index": index } as React.CSSProperties} + // No `--stagger-index`: the cascade is carried by mount timing now, not by an + // animation delay. Each card animates the moment it appears, so keeping an index + // here would delay it a second time and the last card would arrive twice as late + // as the pacing intends. The default `var(--stagger-index, 0)` is what we want. className={cn( "stagger-item inline-flex min-h-12 shrink-0 items-center gap-2 rounded-xl border border-[color:var(--border)]", "bg-[color:var(--surface-raised)] px-2.5 text-left transition", diff --git a/src/components/clinical-dashboard/answer-status.tsx b/src/components/clinical-dashboard/answer-status.tsx index 468b2d289e..02147c9e8a 100644 --- a/src/components/clinical-dashboard/answer-status.tsx +++ b/src/components/clinical-dashboard/answer-status.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useState, type CSSProperties } from "react"; +import { useEffect, useState, useSyncExternalStore, type CSSProperties } from "react"; import { History, Square } from "lucide-react"; import { @@ -22,6 +22,7 @@ import { cn } from "@/components/ui-primitives"; import { appModeIcons } from "@/lib/app-mode-icons"; import type { AppModeId } from "@/lib/app-modes"; import { consolidatedModeSearchPath } from "@/lib/consolidated-mode-home-redirect"; +import { prefersReducedMotion } from "@/lib/scroll-behavior"; import { answerLoading, sharedHomeEmptyState, @@ -231,6 +232,72 @@ function useSlowNotice(active: boolean, startedAt: number | null) { return active && startedAt !== null && slowRun === startedAt; } +/** One card per rung. `--duration-moderate` is an existing duration token (200ms) rather than + * a new one — `globals.css` says in as many words not to invent a rung — and six cards + * therefore land over about 1.2s. That is long enough that each is separately noticeable and + * short enough to be standing well before generation ends, which is the window the rail has + * to be readable in. */ +const evidenceRevealIntervalMs = 200; + +/** The motion preference as a subscription rather than a snapshot, so the in-app Reduce motion + * toggle takes effect on the wait already on screen. `use-app-preferences.ts` mirrors that + * toggle onto ``, and the OS request arrives through the media query; + * `prefersReducedMotion()` already reads both, this only watches them for changes. */ +function subscribeToMotionPreference(onChange: () => void) { + const media = window.matchMedia?.("(prefers-reduced-motion: reduce)"); + media?.addEventListener("change", onChange); + const observer = new MutationObserver(onChange); + observer.observe(document.documentElement, { attributes: true, attributeFilter: ["data-motion"] }); + return () => { + media?.removeEventListener("change", onChange); + observer.disconnect(); + }; +} + +/** + * How many source cards are on screen right now. + * + * The sources all arrive in one stream event, so this is a reveal, not live discovery — see + * the note on `AnswerEvidencePreview`. It lives here rather than in the rail because the + * status line prints this same number, and the wait's one copy rule is that no number appears + * that the reader cannot reconcile with something on screen. One owner, one count. + * + * Keyed by preview identity, in the same shape `useSlowNotice` uses for the run: a retry or a + * new question hands over a different unit, the count reads as zero again on identity alone, + * and no reset is written into an effect body. Nothing here calls setState synchronously + * during an effect — the only writes come from the interval callback. + * + * Motion suppressed reveals everything immediately, with no timer in the path at all. That is + * the hard-won rule on this surface: Reduce Motion once left a dead panel on a physical iPhone + * mid-generation, and a JS reveal could withhold content in a way a CSS delay never could. + */ +function useProgressiveReveal(total: number, preview: VerifiedEvidencePreviewUnit | null) { + const reducedMotion = useSyncExternalStore(subscribeToMotionPreference, prefersReducedMotion, () => false); + const [revealed, setRevealed] = useState<{ unit: VerifiedEvidencePreviewUnit; count: number } | null>(null); + + useEffect(() => { + if (reducedMotion || !preview || total <= 0) return undefined; + // Starts at one because the first card is already drawn below; the interval only has the + // rest to bring in. + let shown = 1; + const timer = window.setInterval(() => { + shown += 1; + setRevealed({ unit: preview, count: shown }); + if (shown >= total) window.clearInterval(timer); + }, evidenceRevealIntervalMs); + return () => window.clearInterval(timer); + }, [preview, total, reducedMotion]); + + if (reducedMotion) return total; + if (!preview || total <= 0) return 0; + // The first card is on screen the instant the unit arrives. This preview exists to shorten + // time to first useful content, and holding the whole rail back for a rung to make the + // animation tidier would spend exactly what it was built to buy. Identity, not a stale + // count, decides the rest: a new question's unit reads as one card again rather than + // inheriting the previous rail's number. + return Math.min(revealed?.unit === preview ? revealed.count : 1, total); +} + /** * Single-line progress for the non-answer (library/document) search modes, the * flat sibling of AnswerProgress. @@ -318,7 +385,11 @@ export function AnswerProgress({ // sources while the rail draws six, and a line reading "8 sources found" above six cards // is a number the reader cannot reconcile with anything on screen. const previewSourceCount = Math.min(evidencePreview?.sources.length ?? 0, visiblePreviewSourceLimit); - const previewMessage = latest ? answerProgressPreviewMessage(previewSourceCount, latest.stage) : null; + // The cards are revealed one at a time, so the line counts what is currently standing + // beneath it rather than what the unit carries. Before the first card lands there is no + // count to print and the line falls back to the stage clause. + const revealedSourceCount = useProgressiveReveal(previewSourceCount, evidencePreview); + const previewMessage = latest ? answerProgressPreviewMessage(revealedSourceCount, latest.stage) : null; const currentMessage = previewMessage ?? (latest ? answerProgressDisplayMessage(latest) : "Reading your question…"); const details = events .map((event) => ({ ...event, displayMessage: answerProgressDisplayMessage(event) })) @@ -354,7 +425,9 @@ export function AnswerProgress({ - {evidencePreview ? : null} + {evidencePreview ? ( + + ) : null} )} diff --git a/src/lib/answer-preview.ts b/src/lib/answer-preview.ts index 308499a6a8..68f0ddac59 100644 --- a/src/lib/answer-preview.ts +++ b/src/lib/answer-preview.ts @@ -11,40 +11,78 @@ import { trimSourceForClient } from "@/lib/answer-client-payload"; import { env } from "@/lib/env"; import { hasDangerSourceGovernanceWarning, sourceGovernanceWarnings } from "@/lib/source-governance"; import type { VerifiedEvidencePreviewUnit, VerifiedUnit } from "@/lib/answer-stream-contract"; -import type { EvidenceRelevance, SearchResult } from "@/lib/types"; +import type { EvidenceRelevance, SearchResult, SourceGovernanceWarning } from "@/lib/types"; export type { VerifiedUnit }; const evidencePreviewMaxSources = 12; +/** Danger-level warnings split into the two kinds the preview must treat differently. + * + * A danger warning that names a `document_id` is attributable: that document is outdated or + * poorly extracted, and excluding it removes the hazard exactly. A danger warning with no + * document is an answer-level verdict — today only `WEAK_EVIDENCE`, raised when the retrieved + * evidence does not back the question at all — and no per-document exclusion can answer it, + * so it suppresses the whole preview. + */ +function partitionDangerWarnings(warnings: readonly SourceGovernanceWarning[]) { + const danger = warnings.filter((warning) => warning.severity === "danger"); + return { + hasAnswerLevelDanger: danger.some((warning) => !warning.document_id), + dangerDocumentIds: new Set(danger.map((warning) => warning.document_id).filter((id): id is string => Boolean(id))), + }; +} + /** * Build the retrieval-complete evidence preview, or null when nothing may be disclosed. * - * Deliberately stricter than the final response's refusal: the final gate only refuses - * grounded, supported answers over danger-level sources, because unsupported/evidence-gap - * responses withhold content anyway. At preview time the answer's support level is not yet - * known, so ANY danger-level governance warning suppresses the preview — a preview must - * never disclose source content the final governed response might withhold. + * **The governance decision is per document, not per answer.** An earlier cut ran the + * canonical danger check over the whole retrieval set and returned null on any hit, which + * made one outdated or badly-OCR'd chunk anywhere in twelve-to-twenty-four retrieved + * passages blank the entire rail — the feature read as broken rather than conservative, and + * the sources it hid were the clean ones. Excluding the flagged documents instead is + * strictly safer per card: a danger-level source can no longer appear as a preview card at + * all, where the wide check merely delayed its appearance until the answer's own rail. + * + * What stays all-or-nothing is the answer-level verdict. `relevance.verdict === "none"` + * says the retrieved evidence is unbacked, which is not a property of any one document, so + * there is no subset that is safe to show. + * + * Still deliberately stricter than the final response's refusal in one respect: the final + * gate only refuses grounded, supported answers, because unsupported/evidence-gap responses + * withhold content anyway. At preview time the answer's support level is not yet known, so + * the danger decision is applied unconditionally. */ export function buildEvidencePreviewUnit(args: { results: SearchResult[]; - /** The full source set the final answer may expose; used to fail closed early. */ - governanceResults?: SearchResult[]; relevance?: EvidenceRelevance | null; }): VerifiedEvidencePreviewUnit | null { if (!args.results.length) return null; const warnings = sourceGovernanceWarnings({ - results: args.governanceResults ?? args.results, + results: args.results, relevance: args.relevance ?? null, }); - if (hasDangerSourceGovernanceWarning(warnings)) return null; - const selected = args.results.slice(0, evidencePreviewMaxSources); + if (!hasDangerSourceGovernanceWarning(warnings)) { + return buildUnit(args.results); + } + + const { hasAnswerLevelDanger, dangerDocumentIds } = partitionDangerWarnings(warnings); + if (hasAnswerLevelDanger) return null; + return buildUnit(args.results.filter((result) => !dangerDocumentIds.has(result.document_id))); +} + +function buildUnit(results: SearchResult[]): VerifiedEvidencePreviewUnit | null { + if (!results.length) return null; + const selected = results.slice(0, evidencePreviewMaxSources); return { schemaVersion: 1, kind: "evidence_preview", sequence: 0, sources: selected.map(trimSourceForClient), - selectedContextCount: args.results.length, + // Counts what survived governance, never the wider retrieval set: the stream contract + // requires selectedContextCount >= sources.length, and a count drawn from passages that + // were excluded would describe evidence the preview is deliberately not showing. + selectedContextCount: results.length, }; } @@ -52,14 +90,12 @@ export function buildEvidencePreviewUnit(args: { export function buildEvidencePreviewProgress(args: { normalResults: SearchResult[]; fallbackResults: SearchResult[]; - governanceResults: SearchResult[]; relevance?: EvidenceRelevance | null; }): { verifiedUnit?: VerifiedEvidencePreviewUnit } { if (!env.RAG_INCREMENTAL_EVIDENCE_PREVIEW) return {}; const fallbackIds = new Set(args.fallbackResults.map((result) => result.id)); const verifiedUnit = buildEvidencePreviewUnit({ results: args.normalResults.filter((result) => fallbackIds.has(result.id)), - governanceResults: args.governanceResults, relevance: args.relevance, }); return verifiedUnit ? { verifiedUnit } : {}; diff --git a/src/lib/rag/rag.ts b/src/lib/rag/rag.ts index 1ddb92b145..d367031406 100644 --- a/src/lib/rag/rag.ts +++ b/src/lib/rag/rag.ts @@ -3505,7 +3505,6 @@ ${qualityRetryInstruction}` ...buildEvidencePreviewProgress({ normalResults: modelContextResults, fallbackResults: generationFallbackResults, - governanceResults: answerInputResults, relevance, }), }); diff --git a/tests/answer-evidence-preview.dom.test.tsx b/tests/answer-evidence-preview.dom.test.tsx index ec99220969..99e7301e5a 100644 --- a/tests/answer-evidence-preview.dom.test.tsx +++ b/tests/answer-evidence-preview.dom.test.tsx @@ -1,7 +1,10 @@ -import { render, screen, within } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { act, render, screen, within } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { AnswerEvidencePreview } from "@/components/clinical-dashboard/answer-evidence-preview"; +import { + AnswerEvidencePreview, + visiblePreviewSourceLimit, +} from "@/components/clinical-dashboard/answer-evidence-preview"; import { AnswerProgress } from "@/components/clinical-dashboard/answer-status"; import { incrementalEvidencePreviewRenderingEnabled } from "@/lib/client-env"; import type { VerifiedEvidencePreviewUnit } from "@/lib/answer-stream-contract"; @@ -30,7 +33,44 @@ function evidencePreview(sourceCount = 4): VerifiedEvidencePreviewUnit { }; } +/** One card lands per `--duration-moderate`; see `useProgressiveReveal` in answer-status.tsx. */ +const revealIntervalMs = 200; + +function renderProgressWithPreview(preview: VerifiedEvidencePreviewUnit) { + return render( + {}} + evidencePreview={preview} + />, + ); +} + +function advanceReveal(cards: number) { + act(() => { + vi.advanceTimersByTime(revealIntervalMs * cards); + }); +} + +/** Past the last card the rail can draw, so assertions read the settled state. */ +function settleReveal() { + advanceReveal(visiblePreviewSourceLimit + 1); +} + describe("incremental answer evidence preview", () => { + // The reveal is timer-driven, so every render in this file needs a clock it controls. + // Real timers would make the card count depend on how long the assertion took to run. + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + document.documentElement.removeAttribute("data-motion"); + }); + // Inverted 2026-08-27 when Phase 1 was enabled by default. The rail is the wait's most // useful content and every unit reaching the browser has already passed the stream // contract's structural validation, so an unset variable renders it. Only the literal @@ -117,18 +157,54 @@ describe("incremental answer evidence preview", () => { // draws six, so the line has to read the rail's cap and not the unit's length — "8 // sources found" above six cards is the one thing this surface promises never to do. it("counts only the sources the reader can see, not every source in the unit", () => { - render( - {}} - evidencePreview={evidencePreview(8)} - />, - ); + renderProgressWithPreview(evidencePreview(8)); + settleReveal(); const cards = screen.getAllByTestId("answer-evidence-preview-source"); expect(cards).toHaveLength(6); expect(screen.getByTestId("answer-progress-line")).toHaveTextContent("6 sources found · writing the answer…"); }); + + // The wait's one copy rule: no number appears that the reader cannot reconcile with + // something on screen. The cards are revealed one at a time, so the count has to move with + // them — a line reading "6 sources found" above two cards is the same broken promise as + // reading the unit's length above the rail's cap. + it("reveals the cards one at a time with the count tracking what is on screen", () => { + renderProgressWithPreview(evidencePreview(8)); + + const countedSources = () => { + const match = /(\d+) sources? found/.exec(screen.getByTestId("answer-progress-line").textContent ?? ""); + return match ? Number(match[1]) : 0; + }; + + // The first card is standing the instant the unit arrives — the preview exists to shorten + // time to first useful content, so nothing is held back for a rung to tidy the animation. + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(1); + expect(countedSources()).toBe(1); + + advanceReveal(1); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(2); + expect(countedSources()).toBe(2); + + advanceReveal(3); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(5); + expect(countedSources()).toBe(5); + + // And it stops at the rail's cap rather than walking on toward the unit's eight. + settleReveal(); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + expect(countedSources()).toBe(6); + }); + + // The hard-won rule on this surface: suppressing motion must never withhold content. + // Reduce Motion once left a dead panel on a physical iPhone mid-generation, which is why + // the reveal fails toward showing everything rather than showing nothing. + it("shows every card immediately when motion is suppressed", () => { + document.documentElement.setAttribute("data-motion", "reduced"); + + renderProgressWithPreview(evidencePreview(8)); + + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + expect(screen.getByTestId("answer-progress-line")).toHaveTextContent("6 sources found · writing the answer…"); + }); }); diff --git a/tests/answer-incremental-delivery.test.ts b/tests/answer-incremental-delivery.test.ts index 83d62fc4dc..5eba1cfaed 100644 --- a/tests/answer-incremental-delivery.test.ts +++ b/tests/answer-incremental-delivery.test.ts @@ -191,14 +191,53 @@ describe("evidence preview builder (#100 Phase 1 server gate)", () => { expect(buildEvidencePreviewUnit({ results: [outdated] })).toBeNull(); }); - it("suppresses a preview when another potential final source fails governance", () => { + it("excludes the danger-level document and still shows the clean sources beside it", () => { + // The behaviour this replaces suppressed the whole rail whenever any retrieved passage + // failed governance, which on a real corpus meant one badly-OCR'd chunk hid every good + // source in the answer. Excluding the flagged document is strictly safer per card: it can + // no longer appear in the preview at all, where the old wide check only delayed it until + // the answer's own rail. const safe = makeSource(); const outdated = makeSource({ id: "chunk-outdated", + document_id: "doc-outdated", source_metadata: { document_status: "outdated" } as SearchResult["source_metadata"], }); - expect(buildEvidencePreviewUnit({ results: [safe], governanceResults: [safe, outdated] })).toBeNull(); + const unit = buildEvidencePreviewUnit({ results: [safe, outdated] }); + expect(unit).not.toBeNull(); + expect(unit!.sources.map((source) => source.document_id)).toEqual(["doc-1"]); + // Counts what survived, never the wider set: the contract requires + // selectedContextCount >= sources.length, and a count including the excluded document + // would describe evidence the preview is deliberately not showing. + expect(unit!.selectedContextCount).toBe(1); + }); + + it("excludes every chunk of a danger-level document, not only the flagged chunk", () => { + const safe = makeSource(); + const poorFirst = makeSource({ + id: "chunk-poor-1", + document_id: "doc-poor", + source_metadata: { extraction_quality: "poor" } as SearchResult["source_metadata"], + }); + // Same document, no flag of its own — governance is a property of the document, so this + // chunk must go with it rather than standing in as a clean card for the same PDF. + const poorSecond = makeSource({ id: "chunk-poor-2", document_id: "doc-poor" }); + + const unit = buildEvidencePreviewUnit({ results: [safe, poorFirst, poorSecond] }); + expect(unit).not.toBeNull(); + expect(unit!.sources.map((source) => source.id)).toEqual(["chunk-1"]); + }); + + it("suppresses the whole preview when the danger verdict is answer-level, not per document", () => { + // `WEAK_EVIDENCE` from relevance.verdict === "none" says the retrieved evidence does not + // back the question at all. That is not a property of any one document, so no subset of + // the rail is safe to show and the all-or-nothing path must survive. + const unit = buildEvidencePreviewUnit({ + results: [makeSource(), makeSource({ id: "chunk-2", document_id: "doc-2" })], + relevance: { isSourceBacked: false, verdict: "none" } as never, + }); + expect(unit).toBeNull(); }); it("emits zero units for empty retrieval", () => { diff --git a/tests/answer-progress-ui-smoke.spec.ts b/tests/answer-progress-ui-smoke.spec.ts index 85d68864f5..e38e831981 100644 --- a/tests/answer-progress-ui-smoke.spec.ts +++ b/tests/answer-progress-ui-smoke.spec.ts @@ -682,7 +682,9 @@ test("the sources arrive during the wait and hand over to the answer's own rail" await expect(page.getByTestId("answer-source-rail")).toBeVisible(); }); -test("the arriving sources are paced apart, and are simply present when motion is suppressed", async ({ page }) => { +test("the arriving sources are simply present when motion is suppressed", async ({ page }) => { + // The suite runs reduced-motion by default (see the dual-mode note on contextOptions in + // playwright.config.ts), so this is that half of the pair. await page.setViewportSize({ width: 390, height: 844 }); await mockDashboardApis(page); await installEvidencePreviewAnswerStream(page); @@ -695,12 +697,11 @@ test("the arriving sources are paced apart, and are simply present when motion i const cards = page.getByTestId("answer-evidence-preview").getByTestId("answer-evidence-preview-source"); await expect(cards.first()).toBeVisible({ timeout: 8_000 }); - // Reduced motion first, because the suite runs that way by default (see the - // dual-mode note on contextOptions in playwright.config.ts). Suppressing motion must - // never withhold the content: the cards stop animating and are immediately, fully - // visible — not held invisible for the length of the cascade, which is exactly what a - // delay on a `both`-filled animation would do if the reduced-motion reset did not also - // zero the delay. + // Suppressing motion must never withhold the content. The pacing that reveals the cards one + // at a time is JS, so unlike a CSS delay it could genuinely hold cards back from a + // reduced-motion reader — the reveal therefore returns the full count immediately, and every + // card is here at once, not animating, fully opaque. + await expect(cards).toHaveCount(6); const suppressed = await cards.evaluateAll((nodes) => nodes.map((node) => ({ name: getComputedStyle(node).animationName, @@ -708,25 +709,59 @@ test("the arriving sources are paced apart, and are simply present when motion i opacity: getComputedStyle(node).opacity, })), ); - expect(suppressed).toHaveLength(6); for (const card of suppressed) { expect(card.name).toBe("none"); expect(card.delay).toBe("0s"); expect(card.opacity).toBe("1"); } + await expect(page.getByTestId("answer-progress-line")).toContainText("6 sources found"); +}); - // With motion allowed, cards arrive one at a time rather than as a single block. The - // shared `.stagger-item` rung is 35ms, which reads as one movement across six cards; - // this rail overrides it so each card is separately noticeable. +test("the arriving sources are paced apart when motion is allowed", async ({ page }) => { await page.emulateMedia({ reducedMotion: "no-preference" }); - const delays = await cards.evaluateAll((nodes) => - nodes.map((node) => Number.parseFloat(getComputedStyle(node).animationDelay)), + await page.setViewportSize({ width: 390, height: 844 }); + await mockDashboardApis(page); + await installEvidencePreviewAnswerStream(page); + await page.goto("/?mode=answer", { waitUntil: "domcontentloaded" }); + await dismissBlockingPwaNotice(page); + + // Record when each card is inserted, rather than sampling the count from the test side. + // A MutationObserver sees every insertion, so the proof does not depend on how fast this + // machine happens to poll — the pacing either produced separated insertions or it did not. + await page.evaluate(() => { + const stamps: number[] = []; + (window as unknown as { __evidenceCardStamps: number[] }).__evidenceCardStamps = stamps; + new MutationObserver((records) => { + for (const record of records) { + for (const node of record.addedNodes) { + if (node instanceof HTMLElement && node.matches('[data-testid="answer-evidence-preview-source"]')) { + stamps.push(performance.now()); + } + } + } + }).observe(document.body, { childList: true, subtree: true }); + }); + + const submit = await fillHydratedAnswerQuestion(page, "Lithium dosing"); + await submit.click(); + + const cards = page.getByTestId("answer-evidence-preview").getByTestId("answer-evidence-preview-source"); + await expect(cards).toHaveCount(6, { timeout: 8_000 }); + + const stamps = await page.evaluate( + () => (window as unknown as { __evidenceCardStamps: number[] }).__evidenceCardStamps, ); - expect(delays[0]).toBe(0); - expect(delays[1] ?? 0).toBeGreaterThan(0.035); - expect(delays[5] ?? 0).toBeGreaterThan(delays[1] ?? 0); - // …and the whole rail is still standing well before a normal generation wait ends. - expect(delays[5] ?? 0).toBeLessThan(1); + expect(stamps).toHaveLength(6); + // One card per --duration-moderate (200ms), so first to last spans about a second. The + // bound is deliberately loose at both ends: the point is that the rail accrues rather than + // blinking into place, and that it is still standing well before generation ends. + const span = (stamps.at(-1) ?? 0) - (stamps[0] ?? 0); + expect(span).toBeGreaterThan(500); + expect(span).toBeLessThan(3_000); + + // The wait's one copy rule, checked where it is easiest to break: the number in the line is + // the number of cards beneath it, all the way to the end of the reveal. + await expect(page.getByTestId("answer-progress-line")).toContainText("6 sources found"); }); test("a completion frame cannot mark a previous answer complete when final is invalid", async ({ page }) => { From dbbc00984dc8f7dce042fcd61b76d4f161c2693c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 19:55:25 +0000 Subject: [PATCH 2/8] test(answer): pin the rail's pacing to mount timing, not a CSS delay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- tests/answer-progress-indicator-css.test.ts | 71 ++++++++++++--------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/tests/answer-progress-indicator-css.test.ts b/tests/answer-progress-indicator-css.test.ts index 4079bc0002..d05902fff6 100644 --- a/tests/answer-progress-indicator-css.test.ts +++ b/tests/answer-progress-indicator-css.test.ts @@ -46,7 +46,8 @@ function keyframes(name: string) { /** The character range of the `@layer components { … }` block, so a rule can be shown to sit * outside it. Layered rules lose to unlayered ones in the cascade regardless of specificity, - * which is the whole reason the rail's override is declared where it is. */ + * which is what keeps the shared cascade rule authoritative over the rail's own component + * styles. */ function componentsLayerRange() { const start = globalsCss.indexOf("@layer components {"); expect(start, "@layer components is missing").toBeGreaterThanOrEqual(0); @@ -122,35 +123,45 @@ describe("answer progress indicator CSS", () => { expect(answerStatusSource).toContain("answer-progress-dot grid"); }); - it("paces the arriving source rail apart from the shared cascade rung", () => { - // The rail's cards must arrive one at a time, not as one block. `.stagger-item` - // ships 35ms, which is right for the prose skeleton bars directly above the rail - // and for search result grids; six cards at that interval is 175ms and reads as a - // single movement. The override is pinned here so a later edit to the shared rung - // cannot silently re-collapse the rail into one beat. - const railRule = globalsCss.match(/\.answer-sources-arriving \.stagger-item\s*{([^}]*)}/); - expect(railRule, "the rail's stagger override is missing").not.toBeNull(); - expect(railRule?.[1]).toContain("var(--stagger-cascade-wide)"); - - // Both rungs are tokens, so the pacing is nameable and the design-system contract's - // hardcoded-duration ratchet stays satisfied. The wide rung must actually be wider — - // pointing it at the same value would leave the rule in place and the defect back. - const rung = (name: string) => Number(globalsCss.match(new RegExp(`--${name}:\\s*(\\d+)ms`))?.[1]); - expect(rung("stagger-cascade")).toBeGreaterThan(0); - expect(rung("stagger-cascade-wide")).toBeGreaterThan(rung("stagger-cascade")); - - // Declared UNLAYERED. `.answer-sources-arriving` itself lives in @layer components, - // and a layered override loses to the unlayered `.stagger-item` rule whatever its - // specificity — the rail would silently keep the 35ms rung. - const railIndex = globalsCss.indexOf(".answer-sources-arriving .stagger-item"); - expect(railIndex).toBeGreaterThan(0); - expect(componentsLayerRange().contains(railIndex), "the override must not sit in @layer components").toBe(false); - - // And the reduced-motion resets must still come after it, so a suppressed rail - // shows every card at once rather than holding six invisible cards for 450ms. - expect(globalsCss.lastIndexOf('html[data-motion="reduced"] .stagger-item')).toBeGreaterThan( - globalsCss.indexOf(".answer-sources-arriving .stagger-item"), - ); + it("paces the arriving source rail by mount timing, and never twice over", () => { + // The rail's cards must arrive one at a time, not as one block. That used to be a + // `--stagger-cascade-wide` animation-delay override here, because every card mounted at + // once and only a delay could separate them; six cards then landed inside 450ms, which is + // over before a reader looking at an otherwise still screen registers it. The pacing now + // lives in `useProgressiveReveal`, which mounts one card per rung. + // + // What this pins is that the two mechanisms cannot compound. A delay override plus mount + // pacing would put the last card at twice the intended distance, which is the defect that + // reintroducing the old rule would cause. + expect(globalsCss).not.toContain(".answer-sources-arriving .stagger-item"); + expect(answerStatusSource).toContain("useProgressiveReveal"); + + // The reveal interval is a named duration token, not an invented rung — globals.css says + // in as many words not to invent one — so the pacing stays nameable and the design-system + // contract's hardcoded-duration ratchet stays satisfied. + const revealInterval = Number(answerStatusSource.match(/evidenceRevealIntervalMs = (\d+)/)?.[1]); + expect(revealInterval, "the reveal interval must be declared as a number").toBeGreaterThan(0); + expect(Number(globalsCss.match(/--duration-moderate:\s*(\d+)ms/)?.[1])).toBe(revealInterval); + + // Each card still animates on mount through the shared rule, so a revealed card eases in + // rather than snapping. + const staggerIndex = globalsCss.indexOf("\n.stagger-item {"); + expect(staggerIndex, "the shared cascade rule is missing").toBeGreaterThan(0); + const staggerRule = globalsCss.match(/\n\.stagger-item\s*{([^}]*)}/); + expect(staggerRule?.[1]).toContain("cascade-fade-up"); + + // Declared UNLAYERED. `.answer-sources-arriving` lives in @layer components, and a layered + // rule loses to an unlayered one whatever its specificity — moving the cascade into a + // layer would let the rail's own component styles silently take the entrance away. + expect(componentsLayerRange().contains(staggerIndex), "the cascade must not sit in @layer components").toBe(false); + + // And the reduced-motion resets must still be present, so a suppressed rail shows every + // card at once. The hook independently returns the full count when motion is suppressed — + // a JS reveal could withhold content in a way a CSS delay never could — but the CSS half + // of that guarantee is pinned here. + expect(globalsCss).toContain('html[data-motion="reduced"] .stagger-item'); + expect(globalsCss).toContain('html:not([data-motion="full"]) .stagger-item'); + expect(answerStatusSource).toContain("prefersReducedMotion"); }); it("keeps the retired ECG trace and its animation deleted", () => { From b928e5bbb2f410f11aeca0482c312904bc3fda9d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:19:34 +0000 Subject: [PATCH 3/8] test(answer): count the first source card, which mounts inside the rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- tests/answer-progress-ui-smoke.spec.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/answer-progress-ui-smoke.spec.ts b/tests/answer-progress-ui-smoke.spec.ts index e38e831981..179b7544af 100644 --- a/tests/answer-progress-ui-smoke.spec.ts +++ b/tests/answer-progress-ui-smoke.spec.ts @@ -731,12 +731,17 @@ test("the arriving sources are paced apart when motion is allowed", async ({ pag await page.evaluate(() => { const stamps: number[] = []; (window as unknown as { __evidenceCardStamps: number[] }).__evidenceCardStamps = stamps; + const card = '[data-testid="answer-evidence-preview-source"]'; new MutationObserver((records) => { for (const record of records) { for (const node of record.addedNodes) { - if (node instanceof HTMLElement && node.matches('[data-testid="answer-evidence-preview-source"]')) { - stamps.push(performance.now()); - } + if (!(node instanceof HTMLElement)) continue; + // Descendants too, not only the node itself. The first card arrives inside the rail + // container, so the observer sees the container added and the card only as its + // child — matching the added node alone silently loses card one and makes the + // pacing look one beat shorter than it is. + const added = node.matches(card) ? 1 : node.querySelectorAll(card).length; + for (let index = 0; index < added; index += 1) stamps.push(performance.now()); } } }).observe(document.body, { childList: true, subtree: true }); From d0890d10daa971d81d26c979de06cffae8c6bafd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:29:18 +0000 Subject: [PATCH 4/8] fix(answer): do not read the preview's exclusion set from a display cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- src/lib/answer-preview.ts | 49 +++++++++++++---------- tests/answer-incremental-delivery.test.ts | 30 ++++++++++++++ 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/lib/answer-preview.ts b/src/lib/answer-preview.ts index 68f0ddac59..933ccdd6b4 100644 --- a/src/lib/answer-preview.ts +++ b/src/lib/answer-preview.ts @@ -11,26 +11,36 @@ import { trimSourceForClient } from "@/lib/answer-client-payload"; import { env } from "@/lib/env"; import { hasDangerSourceGovernanceWarning, sourceGovernanceWarnings } from "@/lib/source-governance"; import type { VerifiedEvidencePreviewUnit, VerifiedUnit } from "@/lib/answer-stream-contract"; -import type { EvidenceRelevance, SearchResult, SourceGovernanceWarning } from "@/lib/types"; +import type { EvidenceRelevance, SearchResult } from "@/lib/types"; export type { VerifiedUnit }; const evidencePreviewMaxSources = 12; -/** Danger-level warnings split into the two kinds the preview must treat differently. +/** Is this one source danger-level on its own account? * - * A danger warning that names a `document_id` is attributable: that document is outdated or - * poorly extracted, and excluding it removes the hazard exactly. A danger warning with no - * document is an answer-level verdict — today only `WEAK_EVIDENCE`, raised when the retrieved - * evidence does not back the question at all — and no per-document exclusion can answer it, - * so it suppresses the whole preview. + * Asked one source at a time, and deliberately so. `sourceGovernanceWarnings` ends with + * `.slice(0, limit ?? 8)` — a cap for a warnings BANNER, where eight lines is already more + * than a reader will take in. Passing the whole candidate set and reading the danger entries + * back out of that list would make 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 + * ones past the eighth are dropped, their documents never reach the exclusion set, and they + * are disclosed as preview cards. Worse, `sourceStatusShortLabel` keys only on + * `document_status`, so a document escaping the cap on `extraction_quality` alone would be + * badged "Current" on its card. + * + * One source per call cannot hit the cap — a single result yields at most a handful of + * warnings — and it still runs the canonical helper rather than reimplementing its rules. */ -function partitionDangerWarnings(warnings: readonly SourceGovernanceWarning[]) { - const danger = warnings.filter((warning) => warning.severity === "danger"); - return { - hasAnswerLevelDanger: danger.some((warning) => !warning.document_id), - dangerDocumentIds: new Set(danger.map((warning) => warning.document_id).filter((id): id is string => Boolean(id))), - }; +function isDangerLevelSource(source: SearchResult) { + return hasDangerSourceGovernanceWarning(sourceGovernanceWarnings({ results: [source] })); +} + +/** The answer-level verdict, asked with no sources at all so only the relevance-derived + * warning can be raised. `WEAK_EVIDENCE` says the retrieved evidence does not back the + * question — not a property of any one document, so no subset of the rail is safe to show. */ +function hasAnswerLevelDanger(relevance: EvidenceRelevance | null) { + return hasDangerSourceGovernanceWarning(sourceGovernanceWarnings({ results: [], relevance })); } /** @@ -58,16 +68,11 @@ export function buildEvidencePreviewUnit(args: { relevance?: EvidenceRelevance | null; }): VerifiedEvidencePreviewUnit | null { if (!args.results.length) return null; - const warnings = sourceGovernanceWarnings({ - results: args.results, - relevance: args.relevance ?? null, - }); - if (!hasDangerSourceGovernanceWarning(warnings)) { - return buildUnit(args.results); - } + if (hasAnswerLevelDanger(args.relevance ?? null)) return null; - const { hasAnswerLevelDanger, dangerDocumentIds } = partitionDangerWarnings(warnings); - if (hasAnswerLevelDanger) return null; + // Governance is a property of the document, so every chunk of a flagged document goes with + // it. A clean-looking second chunk of a badly extracted PDF is the same PDF. + const dangerDocumentIds = new Set(args.results.filter(isDangerLevelSource).map((result) => result.document_id)); return buildUnit(args.results.filter((result) => !dangerDocumentIds.has(result.document_id))); } diff --git a/tests/answer-incremental-delivery.test.ts b/tests/answer-incremental-delivery.test.ts index 5eba1cfaed..fce39ef3dd 100644 --- a/tests/answer-incremental-delivery.test.ts +++ b/tests/answer-incremental-delivery.test.ts @@ -229,6 +229,36 @@ describe("evidence preview builder (#100 Phase 1 server gate)", () => { expect(unit!.sources.map((source) => source.id)).toEqual(["chunk-1"]); }); + it("excludes danger-level documents past the warnings display cap", () => { + // `sourceGovernanceWarnings` ends with `.slice(0, limit ?? 8)`, a cap sized for a warnings + // banner. An earlier cut of this filter read its exclusion set out of that capped list, so + // the ninth danger warning onwards was silently dropped and its document was disclosed as + // a preview card. Five documents that are both outdated and poorly extracted produce ten + // danger warnings, which is enough to push the last one out. + const flagged = Array.from({ length: 5 }, (_unused, index) => + makeSource({ + id: `chunk-flagged-${index}`, + document_id: `doc-flagged-${index}`, + source_metadata: { + document_status: "outdated", + extraction_quality: "poor", + } as SearchResult["source_metadata"], + }), + ); + // Poor extraction only, so `document_status` stays "current" — the card badge reads only + // that field, so a document escaping the cap this way would be shown labelled "Current". + const badlyExtracted = makeSource({ + id: "chunk-poor-ocr", + document_id: "doc-poor-ocr", + source_metadata: { document_status: "current", extraction_quality: "poor" } as SearchResult["source_metadata"], + }); + const safe = makeSource(); + + const unit = buildEvidencePreviewUnit({ results: [...flagged, badlyExtracted, safe] }); + expect(unit).not.toBeNull(); + expect(unit!.sources.map((source) => source.document_id)).toEqual(["doc-1"]); + }); + it("suppresses the whole preview when the danger verdict is answer-level, not per document", () => { // `WEAK_EVIDENCE` from relevance.verdict === "none" says the retrieved evidence does not // back the question at all. That is not a property of any one document, so no subset of From 9f6588c79944a02458ae687ef0a4f8ebe86269cc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:32:21 +0000 Subject: [PATCH 5/8] docs(answer): record the per-document preview governance decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- ...fied-answer-incremental-delivery-design.md | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/docs/verified-answer-incremental-delivery-design.md b/docs/verified-answer-incremental-delivery-design.md index ce113877f2..de464514c8 100644 --- a/docs/verified-answer-incremental-delivery-design.md +++ b/docs/verified-answer-incremental-delivery-design.md @@ -95,6 +95,32 @@ boundary. If it refuses the authoritative response, the stream emits zero verifi through the existing refusal/fallback contract; a preview must never disclose source content that the final governed response withholds. +**Amendment, 2026-09-03 (owner decision), for the evidence preview only.** As first shipped, the +preview ran the danger-level check over the whole retrieval set (`answerInputResults`) and emitted +nothing on any hit. Because that is also the set the final response governs +(`answer.sources = answerInputResults` in `rag.ts`), it held the invariant above exactly — and it made +the feature unusable: one outdated or badly-extracted chunk among twelve to twenty-four retrieved +passages blanked the whole rail, which on a real corpus is most questions, and the sources it hid were +the clean ones. + +The decision is now **per document**. A danger-level document is excluded from the preview; the +remaining sources are shown. Per card this is stricter than before — 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`, from `relevance.verdict === "none"`) still +suppresses the entire preview, because it is not a property of any one document. + +The residual, stated plainly rather than left to be discovered: when the final response refuses on +source governance it blanks `sources: []`, so a preview that showed clean cards has disclosed +document identities the final response then withholds. Those cards are documents that passed +governance individually and are reachable by ordinary search; the refusal blocks the synthesised +answer, not access to the documents. They are removed the moment the answer lands, because the rail +renders only while the request is in flight. This is a deliberate, owner-approved narrowing of the +invariant above, not an oversight, and it applies to the evidence preview only — Phase 2 answer +sections carry generated clinical prose and keep the unamended rule. + +The exclusion set must never be derived from `sourceGovernanceWarnings`' returned list, which is +capped at eight for display. Evaluate one source at a time; see `src/lib/answer-preview.ts`. + Do not implement a second, weaker “stream-safe” verifier. If the current gates cannot operate on an independent section, that section stays buffered until the final answer. Cross-section comparisons, conflicts, and conclusions that depend on later sections are not independently emit-able in v1. @@ -107,8 +133,11 @@ conflicts, and conclusions that depend on later sections are not independently e of `token` / `revising`. - Add reconciliation tests proving every preview is an exact subset of `final` and is discarded on error, cancellation, retry, unknown schema version, or mismatch. -- Add a source-governance refusal fixture proving an outdated or poorly extracted danger-level source - emits zero evidence previews and zero answer-section units. +- Add a source-governance fixture proving an outdated or poorly extracted danger-level source is + excluded from every evidence preview and emits zero answer-section units, that an answer-level + `WEAK_EVIDENCE` verdict emits zero evidence previews, and that exclusion holds past the + eight-warning display cap. (Amended 2026-09-03 with the per-document decision above; before that + amendment any such source emitted zero evidence previews.) - Add owner-boundary fixtures proving private source fields and cross-owner identifiers cannot cross the route boundary. @@ -117,9 +146,10 @@ This phase is provider-free and must land before either visible phase. ### Phase 1 — retrieval-complete evidence preview - After answer evidence is ranked and the final context pack is selected, run the same danger-level - source-governance refusal used by the authoritative final response. Only when it permits disclosure, - build a preview through the existing client-source trimming policy and emit it as - `progress.verifiedUnit`. + source-governance decision used by the authoritative final response, per document (see the + 2026-09-03 amendment above). Emit the sources it permits, through the existing client-source + trimming policy, as `progress.verifiedUnit`; emit nothing when the answer-level verdict is + danger-level or when no source survives. - Render it where the answer's own source rail will land, so arrival swaps content in place rather than moving it. As shipped this is `AnswerEvidencePreview`, a rail of unnumbered cards under the status line and prose placeholder — not the labelled “Selected evidence” panel this design first @@ -127,6 +157,13 @@ This phase is provider-free and must land before either visible phase. prose, do not mark the answer complete, and do not number the cards: the preview is retrieval order while the final list is rebuilt from what the answer cites, so an early number can end up pointing at a different document. +- The cards are revealed one at a time, one per `--duration-moderate`, with the first standing the + instant the unit arrives. Retrieval is a single call and the contract carries exactly one preview + per answer, so this is presentation, not live discovery: the pacing exists because a rail that + mounts whole is over before a reader watching an otherwise still screen registers it. The status + line's count is read from the same reveal, so the number always equals the cards beneath it. + Suppressing motion returns the full count with no timer in the path — a JS reveal could withhold + content in a way a CSS delay never could. - Preserve the current final source list, source governance warnings, feedback token, telemetry, and persistence behaviour. From 2305ffd11b06ccef8d33ad39fd29da1454ee0b7e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 20:57:34 +0000 Subject: [PATCH 6/8] chore(ledger): record the clinical-governance review of this branch Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- ...7381f870e96a503c8ee36f825732c97de9c954dc90885fde588.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/ed1a74a8381f97381f870e96a503c8ee36f825732c97de9c954dc90885fde588.record.md diff --git a/docs/branch-review-records/ed1a74a8381f97381f870e96a503c8ee36f825732c97de9c954dc90885fde588.record.md b/docs/branch-review-records/ed1a74a8381f97381f870e96a503c8ee36f825732c97de9c954dc90885fde588.record.md new file mode 100644 index 0000000000..be152c3a02 --- /dev/null +++ b/docs/branch-review-records/ed1a74a8381f97381f870e96a503c8ee36f825732c97de9c954dc90885fde588.record.md @@ -0,0 +1 @@ +| 2026-09-03 | claude/answer-loading-sources-animation-4obfpq | da1cc81620afc9db0dc6c9ff71061b3a471a193e | answer-preview source governance, evidence rail | P1 governance display-cap defect fixed with a mutation-proven test; P2 invariant/doc drift recorded as a dated design-doc amendment | clinical-governance-reviewer; verify:cheap; verify:pr-local; build; eval:rag:offline; eval:rag:adversarial:offline; focused chromium (answer-progress-ui-smoke 8, ui-phone-motion+ui-universal-search 25, ui-smoke 106) | From 4181d669adc2e8b8421d1edd0d2543cc87468e28 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 21:05:49 +0000 Subject: [PATCH 7/8] fix(answer): resolve the reveal's motion gate three ways, and never un-reveal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- .../clinical-dashboard/answer-status.tsx | 57 +++++++++++-------- src/lib/scroll-behavior.ts | 23 ++++++++ tests/answer-evidence-preview.dom.test.tsx | 52 ++++++++++++++++- 3 files changed, 104 insertions(+), 28 deletions(-) diff --git a/src/components/clinical-dashboard/answer-status.tsx b/src/components/clinical-dashboard/answer-status.tsx index 02147c9e8a..a29bb358f0 100644 --- a/src/components/clinical-dashboard/answer-status.tsx +++ b/src/components/clinical-dashboard/answer-status.tsx @@ -22,7 +22,7 @@ import { cn } from "@/components/ui-primitives"; import { appModeIcons } from "@/lib/app-mode-icons"; import type { AppModeId } from "@/lib/app-modes"; import { consolidatedModeSearchPath } from "@/lib/consolidated-mode-home-redirect"; -import { prefersReducedMotion } from "@/lib/scroll-behavior"; +import { motionIsSuppressed } from "@/lib/scroll-behavior"; import { answerLoading, sharedHomeEmptyState, @@ -242,7 +242,7 @@ const evidenceRevealIntervalMs = 200; /** The motion preference as a subscription rather than a snapshot, so the in-app Reduce motion * toggle takes effect on the wait already on screen. `use-app-preferences.ts` mirrors that * toggle onto ``, and the OS request arrives through the media query; - * `prefersReducedMotion()` already reads both, this only watches them for changes. */ + * `motionIsSuppressed()` reads both, this only watches them for changes. */ function subscribeToMotionPreference(onChange: () => void) { const media = window.matchMedia?.("(prefers-reduced-motion: reduce)"); media?.addEventListener("change", onChange); @@ -263,39 +263,46 @@ function subscribeToMotionPreference(onChange: () => void) { * that the reader cannot reconcile with something on screen. One owner, one count. * * Keyed by preview identity, in the same shape `useSlowNotice` uses for the run: a retry or a - * new question hands over a different unit, the count reads as zero again on identity alone, - * and no reset is written into an effect body. Nothing here calls setState synchronously - * during an effect — the only writes come from the interval callback. + * new question hands over a different unit and the count starts again. The reset is a + * render-phase adjustment, not a write inside an effect — nothing here calls setState + * synchronously during an effect, where it would cascade renders. + * + * **The count for one unit never goes backwards.** Motion is a live subscription, so a reader + * can change the preference mid-generation; if suppressing motion filled the rail and then + * re-enabling it restarted the count, six cards a reader was already reading would vanish and + * re-accrue. The count only ever rises, so a preference change can complete the rail early but + * can never take back a card. * * Motion suppressed reveals everything immediately, with no timer in the path at all. That is * the hard-won rule on this surface: Reduce Motion once left a dead panel on a physical iPhone * mid-generation, and a JS reveal could withhold content in a way a CSS delay never could. */ function useProgressiveReveal(total: number, preview: VerifiedEvidencePreviewUnit | null) { - const reducedMotion = useSyncExternalStore(subscribeToMotionPreference, prefersReducedMotion, () => false); + // `motionIsSuppressed`, not `prefersReducedMotion`: this gates a JS animation, and the CSS + // animations around it honour an explicit in-app "Full" over an OS reduce request. Reading + // the weaker form here would freeze this rail alone while the rest of the interface animates. + const suppressed = useSyncExternalStore(subscribeToMotionPreference, motionIsSuppressed, () => false); const [revealed, setRevealed] = useState<{ unit: VerifiedEvidencePreviewUnit; count: number } | null>(null); - useEffect(() => { - if (reducedMotion || !preview || total <= 0) return undefined; - // Starts at one because the first card is already drawn below; the interval only has the - // rest to bring in. - let shown = 1; - const timer = window.setInterval(() => { - shown += 1; - setRevealed({ unit: preview, count: shown }); - if (shown >= total) window.clearInterval(timer); - }, evidenceRevealIntervalMs); - return () => window.clearInterval(timer); - }, [preview, total, reducedMotion]); - - if (reducedMotion) return total; - if (!preview || total <= 0) return 0; + const cap = preview && total > 0 ? total : 0; + const stored = revealed?.unit === preview ? revealed.count : 0; // The first card is on screen the instant the unit arrives. This preview exists to shorten // time to first useful content, and holding the whole rail back for a rung to make the - // animation tidier would spend exactly what it was built to buy. Identity, not a stale - // count, decides the rest: a new question's unit reads as one card again rather than - // inheriting the previous rail's number. - return Math.min(revealed?.unit === preview ? revealed.count : 1, total); + // animation tidier would spend exactly what it was built to buy. + const floor = cap === 0 ? 0 : suppressed ? cap : Math.min(1, cap); + const count = Math.max(stored, floor); + if (preview && count !== stored) setRevealed({ unit: preview, count }); + + useEffect(() => { + if (suppressed || !preview || count >= cap) return undefined; + // One card per rung, as a chain of timeouts keyed on the current count rather than a + // self-clearing interval: it terminates on its own when the rail is full, and the state + // write is the only thing the callback does. + const timer = window.setTimeout(() => setRevealed({ unit: preview, count: count + 1 }), evidenceRevealIntervalMs); + return () => window.clearTimeout(timer); + }, [preview, cap, count, suppressed]); + + return count; } /** diff --git a/src/lib/scroll-behavior.ts b/src/lib/scroll-behavior.ts index 01d3a964cd..8e21003727 100644 --- a/src/lib/scroll-behavior.ts +++ b/src/lib/scroll-behavior.ts @@ -28,3 +28,26 @@ export function prefersReducedMotion(): boolean { export function resolveScrollBehavior(): ScrollBehavior { return prefersReducedMotion() ? "auto" : "smooth"; } + +/** + * The same question, answered against the app's THREE-state Motion preference. + * + * {@link prefersReducedMotion} above deliberately does not honour the in-app + * `data-motion="full"` opt-in — scroll animation is suppressed under an OS request either + * way, and `settings-dialog.tsx` documents that divergence at its own local copy. But the + * app's CSS animations use the three-state form (`html[data-motion="reduced"]` always + * suppressed, `html:not([data-motion="full"])` + the media query otherwise), so anything + * gating a JS-driven animation must resolve it the same way or it will freeze an animation + * the surrounding interface is still running. + * + * Mirrors that CSS exactly: an explicit in-app choice wins in both directions, and the OS + * request decides only when the reader has expressed none. Safe on the server (returns + * `false`). + */ +export function motionIsSuppressed(): boolean { + if (typeof document === "undefined") return false; + const preference = document.documentElement.getAttribute("data-motion"); + if (preference === "reduced") return true; + if (preference === "full") return false; + return typeof window !== "undefined" && window.matchMedia?.("(prefers-reduced-motion: reduce)").matches === true; +} diff --git a/tests/answer-evidence-preview.dom.test.tsx b/tests/answer-evidence-preview.dom.test.tsx index 99e7301e5a..27c508b2b4 100644 --- a/tests/answer-evidence-preview.dom.test.tsx +++ b/tests/answer-evidence-preview.dom.test.tsx @@ -48,10 +48,15 @@ function renderProgressWithPreview(preview: VerifiedEvidencePreviewUnit) { ); } +/** Each card is a separate timeout, scheduled only once the previous card has rendered, so the + * clock has to be advanced one rung at a time. Advancing the whole span in a single act block + * lands exactly one card — the next timeout does not exist yet. */ function advanceReveal(cards: number) { - act(() => { - vi.advanceTimersByTime(revealIntervalMs * cards); - }); + for (let card = 0; card < cards; card += 1) { + act(() => { + vi.advanceTimersByTime(revealIntervalMs); + }); + } } /** Past the last card the rail can draw, so assertions read the settled state. */ @@ -207,4 +212,45 @@ describe("incremental answer evidence preview", () => { expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); expect(screen.getByTestId("answer-progress-line")).toHaveTextContent("6 sources found · writing the answer…"); }); + + // The app's Motion preference has three states, and an explicit in-app choice wins over the + // OS request in BOTH directions — that is what the CSS does + // (`html:not([data-motion="full"])`), so a JS-gated animation reading the weaker two-state + // form would freeze this rail alone while the rest of the interface animates. + it("honours an explicit Full choice over an OS reduce-motion request", () => { + window.matchMedia = ((query: string) => + ({ + matches: query.includes("prefers-reduced-motion"), + media: query, + addEventListener: () => {}, + removeEventListener: () => {}, + }) as unknown as MediaQueryList) as typeof window.matchMedia; + document.documentElement.setAttribute("data-motion", "full"); + + renderProgressWithPreview(evidencePreview(8)); + + // Paced, not dumped: the reader asked for motion and the OS request does not override it. + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(1); + advanceReveal(2); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(3); + }); + + // A reader can change the preference while an answer is generating. Suppressing motion fills + // the rail; re-enabling it must not then take those cards back and re-accrue them. + it("never takes back a card when the motion preference changes mid-wait", () => { + document.documentElement.setAttribute("data-motion", "reduced"); + renderProgressWithPreview(evidencePreview(8)); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + + act(() => { + document.documentElement.setAttribute("data-motion", "full"); + }); + + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + expect(screen.getByTestId("answer-progress-line")).toHaveTextContent("6 sources found"); + + // And it stays put rather than restarting on the next rung. + advanceReveal(2); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + }); }); From bc434cff6a5b33ee570811dec8b1a43830762d45 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 21:08:57 +0000 Subject: [PATCH 8/8] test(answer): make the mid-wait motion-change case actually observe the 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 Claude-Session: https://claude.ai/code/session_01LkwSYXTqbQAZ7rLmvT8XSa --- tests/answer-evidence-preview.dom.test.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/answer-evidence-preview.dom.test.tsx b/tests/answer-evidence-preview.dom.test.tsx index 27c508b2b4..da33db501d 100644 --- a/tests/answer-evidence-preview.dom.test.tsx +++ b/tests/answer-evidence-preview.dom.test.tsx @@ -237,12 +237,15 @@ describe("incremental answer evidence preview", () => { // A reader can change the preference while an answer is generating. Suppressing motion fills // the rail; re-enabling it must not then take those cards back and re-accrue them. - it("never takes back a card when the motion preference changes mid-wait", () => { + it("never takes back a card when the motion preference changes mid-wait", async () => { document.documentElement.setAttribute("data-motion", "reduced"); renderProgressWithPreview(evidencePreview(8)); expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); - act(() => { + // `await`, because the preference is watched with a MutationObserver and its callback is a + // microtask. A synchronous act() would let this assertion pass without the component ever + // having re-rendered under the new preference — the test would prove nothing. + await act(async () => { document.documentElement.setAttribute("data-motion", "full"); });