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) | 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. 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..a29bb358f0 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 { motionIsSuppressed } from "@/lib/scroll-behavior"; import { answerLoading, sharedHomeEmptyState, @@ -231,6 +232,79 @@ 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; + * `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); + 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 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) { + // `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); + + 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. + 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; +} + /** * Single-line progress for the non-answer (library/document) search modes, the * flat sibling of AnswerProgress. @@ -318,7 +392,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 +432,9 @@ export function AnswerProgress({ - {evidencePreview ? : null} + {evidencePreview ? ( + + ) : null} )} diff --git a/src/lib/answer-preview.ts b/src/lib/answer-preview.ts index 308499a6a8..933ccdd6b4 100644 --- a/src/lib/answer-preview.ts +++ b/src/lib/answer-preview.ts @@ -17,34 +17,77 @@ export type { VerifiedUnit }; const evidencePreviewMaxSources = 12; +/** Is this one source danger-level on its own account? + * + * 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 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 })); +} + /** * 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, - relevance: args.relevance ?? null, - }); - if (hasDangerSourceGovernanceWarning(warnings)) return null; - const selected = args.results.slice(0, evidencePreviewMaxSources); + if (hasAnswerLevelDanger(args.relevance ?? null)) 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))); +} + +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 +95,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/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 ec99220969..da33db501d 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,49 @@ 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} + />, + ); +} + +/** 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) { + 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. */ +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 +162,98 @@ 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…"); + }); + + // 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", async () => { + document.documentElement.setAttribute("data-motion", "reduced"); + renderProgressWithPreview(evidencePreview(8)); + expect(screen.getAllByTestId("answer-evidence-preview-source")).toHaveLength(6); + + // `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"); + }); + + 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); + }); }); diff --git a/tests/answer-incremental-delivery.test.ts b/tests/answer-incremental-delivery.test.ts index 83d62fc4dc..fce39ef3dd 100644 --- a/tests/answer-incremental-delivery.test.ts +++ b/tests/answer-incremental-delivery.test.ts @@ -191,14 +191,83 @@ 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("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 + // 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-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", () => { diff --git a/tests/answer-progress-ui-smoke.spec.ts b/tests/answer-progress-ui-smoke.spec.ts index 85d68864f5..179b7544af 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,64 @@ 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; + 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)) 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 }); + }); + + 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 }) => {