feat(ds): adopt the design system across the six PR 13 surfaces - #1595
Merged
Conversation
PR-J Phase 1 blocker 1 of 2. `answerStateFromRetrieval` read only overdue
sources and `answerQualityTier`, so an answer the pipeline itself reports as
ungrounded — `grounded: false`, `confidence: "unsupported"`, or non-empty
`unverifiedNumericTokens` — projected to `ready` over current sources.
Adopting `AnswerCard` on that projection would have silently retired the
"Review source match" caution the live product shows today
(`evidence-panels.tsx`, `answer-thread-turn.tsx`).
Adds a fifth kind rather than a companion flag on `ready`: a flag keeps the
"ready" vocabulary for an answer that is not ready and is missable in the
card's exhaustiveness, which is the point of the union.
- `AnswerState` gains `{ kind: "ungrounded"; reason: UngroundedReason; sourceCount }`.
- `AnswerStateInput` gains optional `grounded`, `confidence`,
`unverifiedNumericTokens` and a caller-derived `weakEvidence`. Still
structurally typed — the design-system bundle does not import `RagAnswer`.
- Precedence: stale_evidence > partial_retrieval > ungrounded > source_only >
ready. Ungrounded outranks source-only because an unsupported source-only
answer must not read as "evidence complete, synthesis weak";
`stale_evidence` stays the outer kind when an answer is both, so one answer
never stacks two alarms.
- Absent grounding fields are not ungrounding, so call sites that have not been
widened do not acquire a caution on every answer.
- `VerificationNotice` gains an approved `ungrounded` wording in both audiences
and joins the caution role, matching the amber the product paints today.
- `RetrievalStateBanner` renders one headline per reason under the group label
"Source match status", with the same read-the-passages instruction.
- `answerClipboardText` carries a per-reason caveat: the banner does not travel
with a paste, and unattributed prose in a record reads as clinician-endorsed.
No `src/lib/rag/**` change: every field read was already on the payload. The
`RagAnswer` assignability proof in the contract test is extended to the three
grounding fields so a rename there fails this test rather than silently
projecting `ready` again.
Wording in both surfaces remains open to the clinical owner's revision at the
PR 13 glance; the channel, the precedence and the pins do not.
Local: typecheck exit 0; targeted vitest 82 passed (2 files).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>…208) PR-J Phase 1 blocker 2 of 2. Two copy formatters exist and they are not interchangeable: `formatAnswerRenderCopyText()` is what the product copies today and carries the render policy's warnings, trust line, numbered sources with match strength, clinical tables and displayed evidence; `answerClipboardText()` carries unconditional attribution, the `AnswerState` caveat and the single-document provenance suppression rule, and none of the warnings. Decision: the render-policy string stays the primary product payload, and `composeAnswerClipboardText()` (`src/lib/answer-clipboard.ts`) wraps it with the three rules it lacks. Adopting `answerClipboardText` as the product copy path — the tempting simplification — would drop warnings the UI has already decided the clinician must see, which is the "clean prose in the chart" hazard SPEC records. - `renderCopyText` passes through byte-for-byte; the composer never edits, reorders or re-derives warnings, trust, sources, tables or evidence. - Attribution and the caveat sit above the render block: a truncated or quoted paste keeps its head more reliably than its tail, and those two lines are the ones that must survive. - The provenance audit line goes last, still through the single `clipboardProvenanceLine()` implementation, still suppressed on a multi-source stale answer where it would read as a correction of the caveat. - Attribution, caveat and provenance-suppression move into the shared module, so `answerClipboardText()` and the composer cannot drift — one implementation of each rule, two callers. `answerClipboardText` keeps its DS role for AnswerCard demos and unit contracts. The answer surface's `onCopy` is wired to the composer when that surface is adopted (controller-owned, last), so no product copy behaviour changes in this commit. The clinical owner confirms the composed payload reads correctly in an EMR paste at the PR 13 glance. Local: typecheck exit 0; targeted vitest 122 passed (4 files, including the unchanged `answer-render-policy` suite). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… step 0) The registration commit. Nothing may be adopted before this exists: it is the contract every adoption commit — builder or controller — codes against, and the reason integration can reject a stray diff wholesale instead of hand-trimming it. `docs/design-system/ADOPTION.md` records: - The six-surface adoption order (forms, headers, catalogues, docs, source provenance, answer last) and who owns each. Provenance and answer are controller-only: both are on the repo's clinical-risk list. - An explicit file allowlist per surface, verified against this tip rather than copied from the prep inventory — three paths in that inventory were wrong (`differentials-home.tsx`, `favourites-command-library-page.tsx` and `DocumentViewer.tsx` do not sit where it said), and an allowlist naming a file that does not exist is a trap for whoever reads it. - What is deliberately NOT in the headers allowlist — the shell, the master search header and the shell-props modules. Those own composer placement and phone collapse geometry; changing them is a search-chrome change, not a header adoption. - Which built-but-unregistered components must be registered before the surface that first imports them, including the `Select`/`Checkbox` design-sync and test gap, closed in the forms commit rather than deferred past it. - Exclusions (mockups, `src/lib/rag/**`, wrapping `GlobalSearchShell`, half- component adoption, bare `answerClipboardText` as the product copy path, adopting answer before #207, and the four specified-not-built components). - The invariants each commit is checked against, the per-surface test-pin files that must flip in the same commit, and the expected proof shots. The load-bearing pin is called out by name: the live "Review source match" assertion on `answer-support-card`. It must still pass after the answer surface adopts `AnswerCard`, and if adoption moves the caution to a new carrier the pin moves with it in the same commit. SPEC's PR 13 row now points here, and `docs/README.md` indexes it. `docs:check-links` passes: 1610 repo path references resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nce surface) PR 13 adoption surface 5 of 6, controller-owned. The live support-priority card and the design system's `RetrievalStateBanner` describe the same fact from the same payload through two unrelated code paths. Once the answer surface adopts `AnswerCard`, that is a drift waiting to happen, so `answerSupportPriority()` now reads the `AnswerState` projection as well. It is an addition, not a replacement, and the distinction is the whole point. Deriving the caution from the state alone loses cases: the projection's precedence collapses an answer that is both stale and ungrounded to `stale_evidence`, so a `kind === "ungrounded"` check would find nothing and silently drop the very warning #207 was raised to protect. The three original signals — source-only tier, not grounded, weak evidence — still fire on their own. Any degraded kind now asks for source review, which makes the caution a strict superset of the previous condition. One case is newly covered: an answer over overdue sources that is otherwise grounded. That is a deliberate, conservative widening — the DS banner already treats it as caution, and a clinician should verify a stale-sourced answer for the same reason. Flagging it for the glance because it is a visible change to live product behaviour, not a refactor. No behaviour changes yet in this commit: `answerState` is optional and nothing passes it until the answer surface adopts (next commit). Also records in ADOPTION.md that `SourceProvenance` dropping unknown segments while the clipboard line stays explicit is deliberate and not a defect to reconcile, and adds `answer-result-surface.tsx` to the answer allowlist — it calls `answerSupportPriority()` and was missing when the registration record was first written. Added openly rather than edited silently. Local: typecheck exit 0; new `tests/answer-support-priority.dom.test.tsx` 5 passed, covering every degraded kind, each legacy signal alone, the stale-and-ungrounded case, and safety findings still outranking source review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR 13 adoption surface 6 of 6, controller-owned, last. Turns on everything Phase 1 built. - `answer-result-surface.tsx` builds the `AnswerState` from the payload the app already receives — grounded, confidence, unverifiedNumericTokens, plus the render policy's own weakEvidence passed through rather than re-derived — and feeds it to `answerSupportPriority()`. The live "Review source match" card and the DS banner are now two renderings of one state instead of two independent readings of the same fields. - `VerificationNotice` renders above the prose in document order with the system-owned wording; the call site chooses the state, never the words. - `RetrievalStateBanner` renders under it whenever the state is not `ready`, wired to `onScopeDocument`, so a caution is never raised with nowhere to go. - Both product copy paths — the current answer in `ClinicalDashboard` and prior thread turns — go through `composeAnswerClipboardText()`. The render-policy string stays primary and unedited; the composer adds attribution, the state caveat and the provenance line. Prior turns get identical treatment because a copied old answer lands in a record exactly like a fresh one. Both paths fall back to the previous behaviour when there is no answer or no render text. Defect found while adopting, and fixed: clipboard attribution cannot be keyed on the `AnswerState` kind. #207 precedence puts `ungrounded` above `source_only`, so an extractive answer that is also weakly supported reports `ungrounded` — and the paste then claimed "AI-generated" over passages no model wrote. That is a false provenance claim in a clinical record. `composeAnswerClipboardText()` now takes an explicit `sourceOnly` tier flag and both callers pass it. A DOM test caught this, not review. Deliberately deferred, recorded in ADOPTION.md rather than left implicit: the `AnswerCard` container swap. Replacing the `answerSurface` wrapper with `AnswerCard`'s article restructures the primary clinical screen's chrome, collides with the surface style contract and the phone geometry pins, and would not give an honest signal until `verify:ui` at the end of the wave. It needs its own commit and its own UI gate. This narrows the surface's scope, so it is stated plainly here and belongs in the glance. Local: typecheck exit 0; targeted vitest 105 passed across the five answer-safety files; docs link check passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deferred from PR-J with reasons in docs/design-system/ADOPTION.md 2.6: the surface-treatment collision is the design owner's call, the --measure clamp moves the phone scroll-runway pins that cost PR-V two CI cycles, and bundling it would make a red verify:ui unattributable across the wave's other surfaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Captured while the context was fresh, so the findings survive the wave. All were found by the builders or by adoption itself, and every one was reported rather than fixed in-flight because fixing it needed a file outside the surface's allowlist, a design decision, or a gate that could not run. #217 EmptyState has no heading, blocking heading-bearing empty states #218 cn() has no tailwind-merge, so className size overrides resolve by stylesheet order (live instance: metadataPill dual text-* at four sites) #219 DocumentViewer preview error panel announces nothing after load #220 Chip type scale: globals.css says 11px, ui/chip.tsx is 12px #221 Local EmptyState/LoadingState/Chip duplicates still unconverged #222 Headers surface partially converged; two files declined with reasons #223 MatchExplanationChips exported with zero call sites Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wave 5 / PR-J surface 3. Convergence only — no catalogue was redesigned and no component that is specified-but-unbuilt was approximated. Tools launcher: StatusChip carried its own copy of the chip recipe, including literal duplicates of toneSuccess/toneWarning/toneInfo and its own 12px icons. It now maps the launcher's status vocabulary (source/safety/high/neutral) onto the design system's Chip tones and icon slot, so geometry, tone palette and truncation have one owner. The hand-rolled "no tools match" panel becomes EmptyState; that state is introduced by a filter or query edit rather than a navigation, so the primitive's polite announcement is correct here. DSM search: the hand-rolled no-results panel becomes EmptyState. Its heading dropped from an h2 to EmptyState's emphasised paragraph; the page keeps its h1 from DsmPageHeader, and no test pins that heading. The "Browse all diagnoses" recovery link moves into the primitive's actions slot unchanged. Favourites: the same "No favourites match" markup existed three times — twice in responsive table cells and once in the mobile card list. All three now render one local FavouritesEmptyMatches wrapper around EmptyState. Only one of the three is displayed at any breakpoint, so this does not multiply the live region. No test pins needed flipping for this surface: nothing in the catalogue pin set asserts the markup or copy these changes touch.
Wave 5 / PR-J surface 4. Most of what this surface was scoped to do is blocked, and the blockers are reported to the controller rather than worked around; what lands here is the one adoption that is both safe and worth having. The filtered-out state in document search was a local subtle panel with no role and no live region, so toggling a facet until the last matching document dropped out changed the page silently. It now renders the registered EmptyState, which carries the polite announcement for a state introduced without a navigation. The copy is unchanged, and the panel keeps a testId so a later pin can target it. Deliberately not changed, with reasons: - The "No matching documents" empty state stays hand-rolled. EmptyState renders its title as an emphasised paragraph, not a heading, and tests/ui-smoke.spec.ts pins that string as a heading role. Flipping that pin needs a file outside this commit's allowlist, and keeping the h3 while adopting the rest would be the half-component adoption ADOPTION.md forbids. - Every aria-live node and the role="alert" for the unavailable/auth case are untouched. LiveAnnouncer has zero mounts anywhere in src/, so routing any of them through announce() would replace a working announcement with silence. - DocumentViewer.tsx has no change. Its bespoke preview loading and preview error panels are the DocumentFrame-shaped restructure this surface is explicitly not allowed to attempt yet, and the rest of the file already composes InlineNotice, PanelHeading, Sheet and the shared panel recipes. No test pins needed flipping: nothing in the docs pin set asserts the markup or copy this change touches.
… fields TextField, SearchField and Select each carried a private copy of the field shell, and every copy carried the same defect (COMPONENTS §0.4): the hint was rendered only while `hint && !error`, so the format rule disappeared at exactly the moment the user got the format wrong. The field then said "that is not a date" with nothing left on screen saying what a date looks like. Folding the three onto `FormField` closes that, because `FormField` keeps both nodes in the DOM and both ids in `aria-describedby`. The fold picks up the rest of that ledger row for free: a caller's `aria-describedby` is now merged ahead of the hint instead of being overwritten, an external `id` can be supplied so an `ErrorSummary` entry can link to a field, required/optional is stated in the label text, and `autoComplete` reaches the control. `FormField` gains `hideLabel`, which the three controls each carried on their own shell and which a search field beside a heading still needs. Checkbox and RadioGroup keep `<fieldset>`/`<legend>` rather than folding onto a `<label htmlFor>` that would name nothing, but take the parts of the shell they were missing: group-level `FieldHint` + `FieldError` present together when invalid, merged rather than overwritten descriptions, and option ids derived from a sanitised key instead of the raw value. Then the product adoption, onto the folded controls only — these five controls had zero production mounts, so this is their first: - patient-profile-panel: the hand-rolled `NumberField` becomes `TextField`. The out-of-range message keeps its `role="alert"` and its describedby link, and now gains the non-colour error icon. The unit moves into the label string because the shared shell takes a string label, so the accessible name is one phrase. - settings-dialog: the sign-in email input becomes `TextField`, and `SettingsSelect` becomes the DS `Select`. The settings row already prints the visible label, so the row's text is no longer a `<label htmlFor>` and the select carries its own `sr-only` label — one control, one accessible name. - formulation builder: the mechanism filter input and the domain select take `TextField`/`Select`. The filter stays a text input rather than `SearchField` because it filters in place and never submits, so it is not a page composer. Test pins flip in this commit so the surface stays a single revert unit. The "swaps the description to the error" pin is inverted to assert both, which is the defect closing, and `Select` gains the dedicated test ADOPTION §3 records as missing.
PageHeader and Breadcrumb had zero product mounts, and InformationPageHeader was defined and never used, so three implementations of one page-title stack were drifting with nothing holding them together. This gives PageHeader its first real mounts and closes the two COMPONENTS §9.16 defects that made adopting it a downgrade rather than a convergence. PageHeader: the `<h1>` no longer truncates and the actions no longer starve it. Those were the same defect twice — a `shrink-0` actions row beside a shrinkable title meant a long diagnosis name lost its ending to an ellipsis while a pair of buttons kept their full width. The title column is now `minmax(0, 1fr)` and the actions wrap onto their own row. The title also takes the display scale the hand-rolled headers already shipped, so a page that converges keeps its heading. Breadcrumb: a crumb is a link whenever it has an `href`, rather than whenever it is not last. That is the semantic the information pages already shipped, and deciding on position instead would silently turn a linked parent crumb into dead text. Crumb links are `min-h-tap` — on a phone this row is the way back out of a record, and a text-height hit area is not a target. `Crumb.icon` exists so the home crumb keeps its back-arrow. InformationPageBreadcrumbs is now a projection onto Breadcrumb rather than a second implementation of one, keeping the back-arrow, the trailing `aria-current="page"`, and the tap height. InformationPageHeader and the DsmPageHeader title stack are projections onto PageHeader; DsmPageHeader keeps the one breadcrumb nav it already had rather than gaining PageHeader's as a second. Pins flip in this commit. The existing breadcrumb assertions pass unchanged, which is the point, and a new one locks the link-vs-text rule that the fold now depends on. Not converged, deliberately, and reported to the controller rather than guessed: the ModeHomeTemplate hero and the search-results header band. The hero is a centred display hero on the fluid `text-hero` token that also owns the in-flow phone composer slot, and the band is a results spine carrying status and filters, not a page title stack. Converging either is a redesign of a shared chrome surface, not a header adoption.
…aller ref Two defects found while integrating the builder diffs, both invisible to a diff read and to a typecheck. tests/information-page-shell.test.tsx matched neither vitest project. The node project collects tests/**/*.test.ts and the jsdom project collects tests/**/*.dom.test.tsx, so a plain *.test.tsx was collected by nothing and had never run. It was the only pin covering the breadcrumb and title-stack fold, and the header adoption added an assertion to a file that could not execute. Renamed onto the jsdom glob; all five of its tests now run and pass, including the new linked-intermediate-crumb assertion. Checkbox declared a ref prop and then discarded it. The component owns the input ref to set `indeterminate`, which has no HTML attribute, and that callback sits after the props spread — so a caller ref typechecked, rendered, and did nothing. Forwarded by hand, with a test. Also adds the running proof that a caller ref still reaches the input through TextField after the fold onto FormField, since the input now sits inside a render-prop child.
#224 records that the DSM search empty state lost its h2 when it adopted EmptyState — the first live instance of #217 rather than a separate defect. #225 records that the favourites no-matches state now renders three role=status live regions at once, one per breakpoint container. Also repoints ADOPTION.md's forms pin list at the renamed tests/information-page-shell.dom.test.tsx.
verify:phone-chrome fails tests/ui-smoke.spec.ts:2224 with maxOffset 271 against a <200 ceiling. Not finding L, which records 99 on this machine — below the floor, not above the ceiling. Attributed to the additive VerificationNotice on answer-result-surface.tsx, which now renders on every phone answer including ready. The ceiling is derived from the 128px collapse budget plus the 72px in-flow activation band, so it is not a number that can simply be widened.
Measured, not estimated. On a 390px phone the notice cost 160px above the answer prose: tests/ui-smoke.spec.ts:2224 read maxOffset 271 against a <200 ceiling with the notice rendered, and 111 with it hidden. That ceiling is derived rather than arbitrary — 128px collapse budget plus the 72px in-flow chrome activation band — so the answer had outgrown the band the phone chrome hide/reveal contract is written against. Drops to text-xs/leading-5 on phones and keeps text-sm/leading-6 from sm up, and hides the "Based on N cited sources." count on phones only. That count is not a warning, the Sources control directly below it already states the same number, and it stays in the DOM, returning at sm and in print where it is part of the audit artefact. Every word of the warning survives at every width. The notice is not clamped, not collapsed behind a disclosure, and not dropped on small screens — only its type scale changes. Showing it solely for non-ready states would have restored the geometry too, and was rejected: ready is not verified, and removing the disclaimer from ordinary grounded answers is the reduction #207 exists to prevent. Local: the focused pin passes (1 passed); typecheck, lint exit 0; 82 passed across ui-v2-answer-safety and answer-state-contract. Local geometry on this machine reads 41-81px below CI for this test (finding L), so CI remains the binding verdict on the pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every record page rendered the error boundary in a production build after the
headers adoption: formulation mechanism guides, specifiers detail, DSM diagnosis
and DSM comparison, all "Something went wrong", digest 443291805. The server log
carries the real cause:
Functions cannot be passed directly to Client Components unless you explicitly
expose it by marking it with "use server".
{$$typeof: ..., render: function, displayName: ...}
That object is a lucide icon. page-header.tsx was marked "use client" while both
of its importers — information-page-shell.tsx and dsm-page-header.tsx — are
server components, and the adoption started passing icons as components rather
than as rendered elements: `{ label, href, icon: ArrowLeft }` into Breadcrumb,
`icon={BookOpenCheck}` into PageHeader. An element serialises across that
boundary; a component is a function and does not.
The directive was inert before adoption and wrong after it. PageHeader and
Breadcrumb hold no state, no effects and no event handlers, so the fix is to
drop it rather than to reshape the icon contract or to push "use client" up into
the record pages. The client bundle gets smaller as a side effect.
Nothing in the diff looked wrong — `icon={BookOpenCheck}` is exactly what the
component's own type asks for — and typecheck, lint and every unit pin passed
over it. Only a production build surfaces this class of defect, which is why it
took the broad browser stage rather than the focused pins.
Local: tests/ui-formulation.spec.ts + tests/ui-specifiers.spec.ts +
tests/ui-route-coverage.spec.ts, 24 passed, with no digest and no boundary error
in the server log; typecheck and lint exit 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>On a weak-evidence answer the phone surface rendered VerificationNotice, then RetrievalStateBanner, then the live "Review source match" card — three renderings of the same caution, each telling the reader to check every clinical number, dose, timing and threshold against the cited passages. Measured against the deliberately one-sentence fixture in tests/ui-smoke.spec.ts:2056: eleven lines of warning around one line of answer, 147px of scroll where the phone budget is 8. Reproduced twice with identical numbers. The projection was never wrong — #207 precedence put weak_evidence on top exactly as specified. The adoption was: both DS surfaces were switched on while the legacy caution card stayed in place. The banner now renders only for `stale_evidence` and `partial_retrieval`, the two states where it says something the notice cannot — which sources are overdue and how much of the retrieval was missed. For `ungrounded` and `source_only` it restated the notice almost word for word. No wording changed, no state left the projection, and an ungrounded answer still carries the strongest wording the system has plus the Review source match card and its action. What is gone is the second copy. Three identical alarms teach a reader to skip all three, so on a clinical screen the duplicate is the dangerous one, not the missing one. Local: 93 passed across ui-v2-answer-safety, answer-support-priority, answer-clipboard-product-path and answer-state-contract; typecheck and lint exit 0. Phone geometry 147 -> 29 against an 8px budget, and ui-smoke:2224 now passes. The residual 29px is the notice itself, which is deliberate; per finding L that bound is re-pinned from CI-reported numbers, never from a local reading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bel query Two defects the full unit suite found, both introduced by this wave and both invisible to typecheck, lint, the focused pins and the browser gate. src/lib must not import @/components — tests/lib-layering.test.ts pins it — and #208 broke that when it put composeAnswerClipboardText() in src/lib/answer-clipboard.ts importing AnswerState from the design system. It has been on the branch since ba61ef8; nothing ran the whole suite until the PR mirror. The vocabulary moves down a layer into src/lib/answer-state-types.ts and the DS module re-exports it, so all nine existing importers are untouched and there is still exactly one definition. Copying the union into src/lib was the obvious cheap fix and the wrong one: the clipboard reads it deeply — state.overdue, state.retrieved, state.reason — so a second copy would drift against the #207 precedence rules, on the code that decides which caveat lands in a clinical record. answerStateFromRetrieval() and its structural input types stay with the design system, because the projection is UI-facing policy while these types are shared vocabulary. settings-dialog-actions.dom.test.tsx queried the email field by the exact label "Email address". The shared field shell states requirement in the label text, so the accessible name is now "Email address (required)". Matched loosely: pin the field, not the optionality marker. That test sits outside Builder A's pin list, which is why the forms adoption never saw it. Local: format, typecheck, lint exit 0; 47 passed across lib-layering, settings-dialog-actions, answer-state-contract and both clipboard suites; full suite 5048 passed with only the two known Windows codex-cloud path-mangling failures, which this diff does not touch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three are visible changes the design owner ruled on. Optionality marker. FormField appended "(required)" or "(optional)" to every visible label, so the patient profile read "Age (years) (optional)", "Weight (kg) (optional)" down a two-column grid on a panel that already declares itself optional. There was no per-field opt-out to stop passing and adding one was ruled out, so the marker is now required-only: mark the requirement, leave the rest unmarked, which is the ordinary convention and carries the same information because "unmarked" now means exactly one thing. The DS pin moves with it. Only the requirement is still stated in text rather than by colour. Settings rows. Jurisdiction and Default population get click-to-focus back without giving up a correct accessible name. The visible row text is a real <label htmlFor> again, and aria-labelledby points at that same label so it owns the name too — the DS Select keeps its own sr-only label, but aria-labelledby takes precedence, so the name is those words once instead of two <label for> elements concatenated. Correct name and clickable label were never a trade; the earlier fold just picked the wrong side of one. Pinned by a new test that asserts the label element, the id wiring and the resulting accessible name. DSM header icon. The hand-rolled header hid its icon tile below sm; the shared PageHeader rendered it at every width, which put a 36px tile on phones next to a title that also grew and now wraps. Restored as max-sm:hidden — not "hidden sm:grid", because iconTilePremium already carries grid and cn() here is a plain join with no tailwind-merge (ledger #218), so three display utilities would race and be settled by stylesheet order. The muted converged eyebrow stays as adopted. Local: format, typecheck, lint exit 0; 107 passed across the settings, form field, answer safety, clipboard and patient profile suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Card paste trap Both from the clinical governance review of this branch. The notice and the banner are the two governance statements on the answer surface and they render adjacent, but they counted sources differently — the notice from the render model's primary sources, the banner from the projection's distinct document keys. On stale_evidence that let the surface print "Based on 3 cited sources." directly above "2 of 7 sources for this answer are past their review date", leaving a clinician unable to tell how much of the evidence base is overdue. Both now read the projection. source_only carries no count, hence the `in` guard. answerClipboardText() keyed attribution on the AnswerState kind alone, so an extractive answer that is also weakly supported — which #207 precedence reports as ungrounded, not source_only — would paste "AI-generated from the cited sources." over passages no model wrote. That is the exact false provenance claim in a clinical record that #208 added an explicit sourceOnly flag to the product composer to prevent; the sibling primitive that shares its helpers was left without one. AnswerCard has zero product imports today, so this is latent rather than live, and it would have become live the moment #216 adopts the container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#228 (P2) from the clinical governance review: the answer notice can claim AI-generated directly above the Source-only chip, because #207 precedence puts ungrounded above source_only and the on-screen notice has no equivalent of the clipboard sourceOnly flag. Needs approved wording, so it is recorded rather than fixed. #229 (P3) from the frontend review: the DSM eyebrow moved beside the icon. #226 carries the user decision not to re-pin phone geometry from local numbers.
Both repo review subagents ran against the integrated branch. Clinical: net additive, nothing on origin/main weakened, dropped or narrowed, src/lib/rag untouched. Frontend: no tap-target, hex, z-index or raw-anchor violations, and the page-header "use client" removal confirmed as a real fix rather than a moved crash. Four findings between them: two fixed in 08e8b6b, one in b978d4c, and the notice-versus-Source-only attribution conflict recorded as #228 because it needs approved clinical wording.
Restoring click-to-focus put two labels on one control by design: the visible row text is a <label htmlFor> so clicking it focuses the select, and the DS Select keeps its own sr-only label because a field without one is not a field. aria-labelledby points at the visible one, so the accessible name is those words once rather than the two concatenated. The mobile layout helper measured "the label" by exact text, which now matches both and fails Playwright strict mode. It asserts where the *visible* label sits relative to the control, so it addresses that one by id. The pin travels with the surface change that caused it. Focused re-run: account settings ok, short-runway ok, and the known phantom-scroll residual unchanged at 29 against its 8px budget.
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:18 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR registers design-system adoption, adds ungrounded answer-state handling, centralizes clipboard composition, standardizes shared form controls, and adopts shared UI components across dashboard surfaces. ChangesDesign-system adoption and governance
Answer safety and clipboard behavior
Shared form controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AnswerResultSurface
participant answerStateFromRetrieval
participant VerificationNotice
participant RetrievalStateBanner
participant composeAnswerClipboardText
participant Clipboard
AnswerResultSurface->>answerStateFromRetrieval: retrieval and evidence fields
answerStateFromRetrieval-->>AnswerResultSurface: projected answer state
AnswerResultSurface->>VerificationNotice: state and attribution
AnswerResultSurface->>RetrievalStateBanner: stale or ungrounded state
AnswerResultSurface->>composeAnswerClipboardText: render text and state
composeAnswerClipboardText-->>Clipboard: composed answer text
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7894 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…hboard CI failed check:maintainability-budgets: ClinicalDashboard.tsx reached 4148 lines against a 4140-line no-growth budget. The answer adoption put the copy path there, in a file already at its cap, and the gate asks for a cohesive module rather than a bigger monolith. answer-copy-payload.ts now owns both the projection input and the payload. Three surfaces copy an answer and each was hand-assembling the same answerStateFromRetrieval() input from the same nine payload fields; three hand-assembled copies is how copy paths drift, and #208 exists because one of them once claimed "AI-generated" over passages no model wrote. It sits in the clinical-dashboard layer deliberately. It cannot live in src/lib because it imports @/components/ui/answer-state and tests/lib-layering.test.ts forbids that, and it should not live in the design system for the mirror-image reason recorded on AnswerStateInput: the DS projection takes a structural shape so the design-system bundle never pulls the retrieval layer in, and RagAnswer is the retrieval layer. No behaviour change: the same fields, the same sourceOnly tier flag read from answerQualityTier rather than from the state kind. Local: 4130/4140 lines, budgets pass; typecheck, lint exit 0; 41 passed across both clipboard suites, lib-layering and the answer-state contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI run 30820496984 measured phantom-scroll 97 against an 8px budget and short-runway maxOffset 251 against a 200px ceiling. This machine read 29 and passing for the same tree, so per finding L the pins move from CI's numbers and never from a local reading. Bare phantom budget 8 -> 112, maxOffset ceiling 200 -> 280, post-collapse ceiling 72 -> 160. The collapse budget is untouched: chrome height did not change, only the content below it. What moved is a real contract, not a convenience. The old ceiling was derived as collapse budget plus the 72px in-flow activation band, and the post-collapse runway no longer fits inside that band because every answer now carries an unconditional verification notice above the prose (#207). The guard still pins that the page is sized by its content rather than by the viewport, so a genuine phantom runway would still fail. Recorded in ledger #226, including the question this leaves open for the clinical owner: a one-sentence answer now carries roughly 97px of notice above it on a phone, which is the shape the phantom-scroll guard was written to catch.
BigSimmo
commented
Aug 3, 2026
OwnerAuthor
@coderabbitai thoroughly review PR please |
Contributor
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 54 minutes. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances PR 13’s design-system rollout by introducing a formally-tested adoption contract, adopting DS primitives across multiple UI surfaces (answer safety, forms, headers/breadcrumbs, launcher, empty states), and tightening clinical “answer state”/clipboard provenance handling so cautions and copy payloads remain conservative and consistent during adoption.
Changes:
- Adds an
ungroundedchannel toAnswerState(with ordered reasons) and propagates it throughVerificationNotice,RetrievalStateBanner, and clipboard text composition. - Folds multiple form controls onto the shared
FormFieldshell (preserving hint+error, mergingaria-describedby, ref reachability), and converges page headers/breadcrumbs onto shared components. - Replaces several one-off UI implementations with DS primitives (
EmptyState,Chip), adds/renames tests to ensure the adoption behaviors are actually executed.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ui-v2-form-field.dom.test.tsx | Updates FormField tests for “required marked / optional unmarked” labeling contract. |
| tests/ui-v2-components.dom.test.tsx | Expands DS component DOM tests (TextField/Select/Checkbox/RadioGroup describedBy, ids, refs). |
| tests/ui-v2-answer-safety.dom.test.tsx | Extends answer safety tests to cover ungrounded wording/tone, attribution, banner behavior, clipboard caveats. |
| tests/ui-smoke.spec.ts | Updates smoke assertions for settings label targeting and phone short-answer geometry pins. |
| tests/settings-dialog-actions.dom.test.tsx | Pins settings select row label semantics (click-to-focus + single accessible name) and loosens Email label match. |
| tests/information-page-shell.dom.test.tsx | Adds Breadcrumb link-vs-text regression coverage after folding onto DS Breadcrumb. |
| tests/answer-support-priority.dom.test.tsx | Adds regression tests ensuring “Review source match” support-priority remains a superset across legacy + AnswerState signals. |
| tests/answer-state-contract.test.ts | Extends AnswerState projection contract to include grounding fields and new ungrounded kind + precedence. |
| tests/answer-clipboard-product-path.dom.test.tsx | Ensures product copy button goes through the #208 composer and carries attribution/caveats correctly. |
| tests/answer-clipboard-composition.test.ts | Pins composer “compose, don’t replace” decision and shared-rule helpers for attribution/caveat/provenance. |
| src/lib/answer-state-types.ts | Moves AnswerState vocabulary into src/lib to satisfy layering rules and avoid drift across callers. |
| src/lib/answer-clipboard.ts | Implements shared clipboard attribution/caveat/provenance helpers and the product composer. |
| src/components/ui/verification-notice.tsx | Adds ungrounded state + attribution override to avoid incorrect “AI-generated” provenance for extractive answers. |
| src/components/ui/text-field.tsx | Folds TextField/SearchField onto FormField with merged describedBy, external id, and ref pass-through. |
| src/components/ui/select.tsx | Folds Select onto FormField to preserve hint+error and shared accessibility behavior. |
| src/components/ui/retrieval-state-banner.tsx | Adds ungrounded body/headlines and aligns caution treatment with product semantics. |
| src/components/ui/page-header.tsx | Removes "use client" to fix server-component import crash; adds DS PageHeader/Breadcrumb convergence. |
| src/components/ui/form-field.tsx | Adds hideLabel and updates label requirement marker behavior. |
| src/components/ui/choice.tsx | Adds ref forwarding for Checkbox indeterminate and introduces group-level hint/error + id sanitization for RadioGroup. |
| src/components/ui/answer-state.ts | Re-exports AnswerState types from src/lib and adds new grounding-related projection inputs. |
| src/components/ui/answer-card.tsx | Routes clipboard attribution/caveat/provenance through shared helpers; adds sourceOnly flag. |
| src/components/information-page-shell.tsx | Adopts DS Breadcrumb/PageHeader and tightens title/icon prop contracts. |
| src/components/formulation/formulation-builder-page.tsx | Adopts DS TextField/Select for mechanism filtering controls. |
| src/components/dsm/dsm-search-page.tsx | Uses DS EmptyState for no-results content. |
| src/components/dsm/dsm-page-header.tsx | Converges DSM header onto PageHeader while preserving breadcrumb semantics and mobile icon behavior. |
| src/components/ClinicalDashboard.tsx | Switches answer copy to composed clipboard payload preserving render-policy warnings + DS caveats/provenance. |
| src/components/clinical-dashboard/settings-dialog.tsx | Adopts DS inputs and restores correct label/name behavior for clinical default selects. |
| src/components/clinical-dashboard/patient-profile-panel.tsx | Folds numeric inputs onto DS TextField while preserving out-of-range error behavior. |
| src/components/clinical-dashboard/favourites-command-library-page.tsx | Replaces duplicate empty states with shared EmptyState primitive. |
| src/components/clinical-dashboard/evidence-panels.tsx | Extends “Review source match” priority logic to incorporate AnswerState without dropping legacy signals. |
| src/components/clinical-dashboard/document-search-results.tsx | Uses DS EmptyState for filter-empty results with polite announcement. |
| src/components/clinical-dashboard/answer-thread-turn.tsx | Applies AnswerState projection + composer to prior-turn copy path. |
| src/components/clinical-dashboard/answer-result-surface.tsx | Renders VerificationNotice and conditionally RetrievalStateBanner using AnswerState projection; avoids duplicate cautions. |
| src/components/clinical-dashboard/answer-copy-payload.ts | Centralizes answer-state + clipboard composition for multiple copy surfaces to prevent drift. |
| src/components/applications-launcher-page.tsx | Adopts DS Chip/EmptyState and maps launcher status vocabulary onto DS tones/icons. |
| docs/README.md | Adds ADOPTION.md to docs index. |
| docs/design-system/SPEC.md | Updates spec to include ADOPTION.md and records resolution of blockers (#207/#208). |
| docs/design-system/COMPONENTS.md | Updates component inventory/spec text for the new ungrounded state and clipboard composition decision. |
| docs/branch-review-ledger.md | Appends a review ledger entry for this PR’s integrated branch review. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Database/src/components/ui/answer-state.ts
Lines 159 to 160 in 7295fa3
Distinguish empty citations from missing citationsWhen an answer producer deliberately clears citations after an unsupported high-risk claim, this fallback treats every retrieved candidate as supporting evidence. If any unrelated candidate is overdue, stale-state precedence reports stale_evidence, claims that cited sources are outdated, and suppresses the more accurate ungrounded notice even though the citation list is explicitly empty. Distinguish undefined from [] or project from the retained claim-support IDs; a fixture with grounded: false, empty citations, and one stale retrieved candidate reproduces the misclassification.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo
commented
Aug 3, 2026
OwnerAuthor
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/ds-v2-adopt at starting commit 7295fa3; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/ds-v2-adopt, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/clinical-dashboard/answer-thread-turn.tsx (1)
70-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDerive
needsSourceReviewfromturnStateto avoid banner/clipboard drift.
turnState(lines 70-84) now derives the fullAnswerState, includingungroundedforconfidence === "unsupported"or a non-emptyunverifiedNumericTokens.needsSourceReview(lines 85-89) still uses its own separate condition, which does not checkconfidenceorunverifiedNumericTokensdirectly.For a turn where
confidence === "unsupported"(orunverifiedNumericTokensis non-empty) butgroundedis notfalseandrenderModel.trustis not"low"/"unsupported",needsSourceReviewisfalse, so the on-screen "Review source match" note does not render. The pasted clipboard text for the same turn still carries the ungrounded caveat (viacomposeAnswerClipboardTextat line 126-134). This mismatch is exactly the regression the design-system docs warn against: the caution banner must not be silently retired while the underlyingAnswerStateisungrounded.Derive
needsSourceReviewfromturnState.kindso the visible banner and the clipboard caveat always agree.🐛 Proposed fix to align the banner with `turnState`
- const needsSourceReview =- turn.answer.answerQualityTier === "source_only" ||- turn.answer.grounded === false ||- renderModel.trust === "low" ||- renderModel.trust === "unsupported";+ const needsSourceReview = turnState.kind === "ungrounded" || turnState.kind === "source_only";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/clinical-dashboard/answer-thread-turn.tsx` around lines 70 - 89, Update needsSourceReview in the answer-thread turn component to derive its value from turnState.kind, treating the ungrounded state as requiring source review. Remove the duplicated quality, groundedness, and trust checks while preserving the existing banner behavior for grounded turns.src/components/clinical-dashboard/answer-result-surface.tsx (1)
116-121: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the projected source count for
NaturalLanguageAnswer.
NaturalLanguageAnswerrenderssourceCountin its visible source badge. Pass the same projected count used byVerificationNotice; two chunks from one document can otherwise display2and1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/clinical-dashboard/answer-result-surface.tsx` around lines 116 - 121, Update the sourceCount calculation in the answer-result surface to use the same projected source count passed to VerificationNotice, rather than prioritizing raw primarySources.length. Ensure NaturalLanguageAnswer receives that projected count so duplicated chunks from one document display the same visible source total.
🧹 Nitpick comments (1)
tests/answer-clipboard-product-path.dom.test.tsx (1)
92-108: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider asserting the on-screen banner alongside the copied text.
These two cases (
confidence: "unsupported"andunverifiedNumericTokens: ["12.5 mg"]) are the exact fixtures that would expose theneedsSourceReview/turnStatedivergence flagged insrc/components/clinical-dashboard/answer-thread-turn.tsx. Once that fix lands, add an assertion here thatscreen.queryByTestId("prior-answer-source-review")is present for these fixtures, so a future regression in the banner condition is caught here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/answer-clipboard-product-path.dom.test.tsx` around lines 92 - 108, Extend the tests for copied answers with confidence "unsupported" and unverifiedNumericTokens ["12.5 mg"] in copiedTextFor to also assert that screen.queryByTestId("prior-answer-source-review") is present, covering the needsSourceReview/turnState banner condition in answer-thread-turn.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/clinical-dashboard/answer-result-surface.tsx`:
- Line 28: Replace the direct answerStateFromRetrieval assembly in the useMemo
of the answer-result surface with the shared answerStateForAnswer({ answer,
sources, weakEvidence }) helper from answer-copy-payload.ts. Remove the
now-unused answerStateFromRetrieval import while preserving the existing
memoized result and inputs.
In `@tests/ui-v2-components.dom.test.tsx`:
- Around line 44-46: Update the comment above the getByLabelText call in the UI
components test to remove the incorrect claim that the accessible name includes
“(optional)”. Document that the label is simply “Publisher”, while preserving
the existing loose /Publisher/ assertion.
---
Outside diff comments:
In `@src/components/clinical-dashboard/answer-result-surface.tsx`:
- Around line 116-121: Update the sourceCount calculation in the answer-result
surface to use the same projected source count passed to VerificationNotice,
rather than prioritizing raw primarySources.length. Ensure NaturalLanguageAnswer
receives that projected count so duplicated chunks from one document display the
same visible source total.
In `@src/components/clinical-dashboard/answer-thread-turn.tsx`:
- Around line 70-89: Update needsSourceReview in the answer-thread turn
component to derive its value from turnState.kind, treating the ungrounded state
as requiring source review. Remove the duplicated quality, groundedness, and
trust checks while preserving the existing banner behavior for grounded turns.
---
Nitpick comments:
In `@tests/answer-clipboard-product-path.dom.test.tsx`:
- Around line 92-108: Extend the tests for copied answers with confidence
"unsupported" and unverifiedNumericTokens ["12.5 mg"] in copiedTextFor to also
assert that screen.queryByTestId("prior-answer-source-review") is present,
covering the needsSourceReview/turnState banner condition in
answer-thread-turn.tsx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38be915a-eb75-4704-aa9b-dc68254d50a4
📒 Files selected for processing (41)
docs/README.mddocs/branch-review-ledger.mddocs/design-system/ADOPTION.mddocs/design-system/COMPONENTS.mddocs/design-system/SPEC.mddocs/outstanding-issues.mdsrc/components/ClinicalDashboard.tsxsrc/components/applications-launcher-page.tsxsrc/components/clinical-dashboard/answer-copy-payload.tssrc/components/clinical-dashboard/answer-result-surface.tsxsrc/components/clinical-dashboard/answer-thread-turn.tsxsrc/components/clinical-dashboard/document-search-results.tsxsrc/components/clinical-dashboard/evidence-panels.tsxsrc/components/clinical-dashboard/favourites-command-library-page.tsxsrc/components/clinical-dashboard/patient-profile-panel.tsxsrc/components/clinical-dashboard/settings-dialog.tsxsrc/components/dsm/dsm-page-header.tsxsrc/components/dsm/dsm-search-page.tsxsrc/components/formulation/formulation-builder-page.tsxsrc/components/information-page-shell.tsxsrc/components/ui/answer-card.tsxsrc/components/ui/answer-state.tssrc/components/ui/choice.tsxsrc/components/ui/form-field.tsxsrc/components/ui/page-header.tsxsrc/components/ui/retrieval-state-banner.tsxsrc/components/ui/select.tsxsrc/components/ui/text-field.tsxsrc/components/ui/verification-notice.tsxsrc/lib/answer-clipboard.tssrc/lib/answer-state-types.tstests/answer-clipboard-composition.test.tstests/answer-clipboard-product-path.dom.test.tsxtests/answer-state-contract.test.tstests/answer-support-priority.dom.test.tsxtests/information-page-shell.dom.test.tsxtests/settings-dialog-actions.dom.test.tsxtests/ui-smoke.spec.tstests/ui-v2-answer-safety.dom.test.tsxtests/ui-v2-components.dom.test.tsxtests/ui-v2-form-field.dom.test.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
RagAnswer.sources is a required array, so "not populated" arrives as [] and nullish coalescing kept it — dropping the search-result fallback and any overdue-source warnings only that fallback still carried. Resolve through one helper shared by the live surface, thread turns, and clipboard path. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
commented
Aug 3, 2026
OwnerAuthor
@cursoragent Work the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Goal: review and Bugbot the tip, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, prove fixes with focused local checks, push, append ledger, and post one PR summary. Use broader offline gates only when the delta or a failed required check warrants them. Regenerate large assets only when a fix requires it. Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main, required checks on that tip including Production UI when selected, unresolved actionable threads. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under the review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. After any sync or push, re-snapshot before declaring done. No sync thrash. Review high-confidence issues only. Separate PR-introduced defects from pre-existing re-emitted data. For generated assets, review contracts, aliases, and cache—not every generated line. Flag RAG or governance before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prove each fix with the smallest targeted check; escalate to cheap or PR-local verification only when the change set, risk surface, or CI failure needs that breadth. Format and commit before push. Push only this PR’s fix commits. Append ledger for the final GitHub head. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, required CI, decisive local proof, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no actionable threads remain, required checks are green or clearly in progress, summary posted, merge left to me. |
Contributor
PR #1595 workedConfirmed single open PR in scope: #1595 ( Fixed ( |
| Tip | ed4f8435 (ledger) · fix at cb898bec |
| Merge-tree | clean · MERGEABLE · auto-merge off |
| Threads | 0 unresolved (inline replies forbidden by token; dispositions in PR comment) |
| CI | re-running on new tip |
| Proof | vitest 111 passed |
Merge left to you.
…enance The retrieval banner's Open source button was wired to scopeOnlyDocument, which only replaces selectedDocumentIds and ignores the page locator while the control is labelled as opening the cited page. Route it through citedDocumentHref instead. Also pass single-document source_metadata into the product clipboard composer so the promised Designation/Review status audit line actually appears, apply the resolved RadioGroup id to the fieldset, and correct the optional-label comment. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
marked this pull request as draft
August 3, 2026 15:09
Contributor
Review-and-fix summary — PR #1595PR:#1595 · branch: Fixed
Dispositioned (no code)
Local proof
Residual risks
Merge left to you. |
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…asses The bugbot commit 407c8e7 added tests/answer-copy-payload.test.ts with a partial source_metadata literal. Its tests pass and lint passes, but `npm run typecheck` fails: ClinicalSourceMetadata's governance fields are required-and-nullable rather than optional, so the type refuses a partial literal — deliberately, so a fixture cannot quietly omit the provenance a real source always carries. Missing: source_title, publisher, jurisdiction, version, publication_date, uploaded_at, indexed_at, uploaded_by, clinical_validation_status, extraction_quality. Filled with explicit nulls plus the two "unknown" enums, which is what an unreviewed overdue source actually looks like. No assertion changed. The underlying fix in that commit is sound and I am keeping it: RagAnswer.sources is a required array, so an unpopulated cited set arrives as [] and `??` never fell back to the search-result set. That defect predates this PR — the extraction copied `answer.sources ?? sources` verbatim from the original call sites.
BigSimmo
marked this pull request as ready for review
August 3, 2026 15:21
BigSimmo
commented
Aug 3, 2026
OwnerAuthor
@codex thoroughly the PR for me and all the code I formation and fix any issues please and address them. Be meticulous. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:ed4f843556
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The two bugbot commits (407c8e7, cb898be) each added a partial source_metadata literal. Their tests pass and lint passes, but npm run typecheck fails on both: ClinicalSourceMetadata's governance fields are required-and-nullable rather than optional, deliberately, so a fixture cannot quietly omit the provenance a real source always carries. One annotated base fixture now, varied by spread. Spreading overdueSource.source_metadata instead does not work — source_metadata is optional on SearchResult, so the spread widens every field back to optional and the result stops being assignable. No assertion changed. The underlying bugbot fix is sound and stays: RagAnswer .sources is a required array, so an unpopulated cited set arrives as [] and the nullish fallback never fired — a defect that predates this PR, since the extraction copied `answer.sources ?? sources` from the original call sites.
BigSimmo
marked this pull request as draft
August 3, 2026 16:36
…ntrols Both findings are correct and both sit on code added by the bugbot commits in this PR. Verified against the cited evidence rather than taken on trust. P1 — the stale-source "Open source" control could open the wrong page. citedDocumentHref took the first candidate for the document while taking the page from the locator, emitting ?page=12&chunk=<a page-4 chunk>. Confirmed in src/lib/document-detail.ts: `effectivePage = selectedChunk?.page_number ?? requestedPage`, so the chunk wins and a control labelled "p. 12" lands on page 4 — while the clinician is reviewing an overdue source. Now prefers the candidate whose page matches the locator, and omits the chunk entirely when none does, so the viewer opens where the label promised. The existing test asserted the defective URL, so it encoded the bug; replaced with the two-candidate case the review asked for plus the omit-chunk case. P2 — the clipboard lost its provenance audit line on ordinary answers. singleDocumentClipboardMetadata was fed the resolved candidate list, but RagAnswer.sources retains every retrieval candidate while citations name the supporting set (ui/answer-state.ts). One uncited candidate from another document made a one-document answer look like two and suppressed the Designation/Review status line — on the normal payload shape. Now filtered to the cited set first, falling back to the full set when nothing identifies the citations. Local: typecheck, lint exit 0; 53 passed across the href, payload, clipboard composition/product-path and answer-state contract suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo
marked this pull request as ready for review
August 3, 2026 17:17
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 4, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Registers the design-system adoption contract.
docs/design-system/ADOPTION.mdrecords the adoption order, the per-surface file allowlists, the exclusions, the invariants every adoption commit is checked against, the per-surface test pins, and the expected proof shots. Every other commit in this PR codes against it.#207— givesAnswerStatean ungrounded channel. A fifth kind, not a companion flag onready, with reasonsgrounded_false | confidence_unsupported | unverified_numeric | weak_evidenceread in that order. Precedence isstale_evidence>partial_retrieval>ungrounded>source_only>ready. Absent or null grounding fields are explicitly not ungrounding, so callers that have not been widened do not acquire a caution on every answer. Without this kind the projection reported agrounded: falseanswer asready, and adopting the design system would have silently retired the live "Review source match" caution.#208— composes the clipboard payload rather than replacing it.formatAnswerRenderCopyText()stays the primary product payload and passes through byte-for-byte.composeAnswerClipboardText()adds the three things it lacks: unconditional attribution and the state caveat above the render block, so a truncated paste keeps its head rather than its tail, and the provenance audit line last under the multi-source-stale suppression rule. Swapping the product path toanswerClipboardText()alone was rejected because it drops the render policy's warnings.Adopts the source-provenance surface.
answerSupportPriority()now readsAnswerStatein addition to the three legacy signals, never instead of them: deriving from the state alone loses the stale-and-ungrounded case, which the projection collapses tostale_evidence.Adopts the answer surface.
VerificationNoticeandRetrievalStateBannerrender on the live answer surface driven by a realanswerStateFromRetrieval()projection, and both product copy paths go throughcomposeAnswerClipboardText()with an explicitsourceOnlytier flag.Adopts the forms surface. The five design-system controls fold onto the shared
FormFieldshell, closing the recorded defect where the hint was dropped the moment an error appeared — exactly when the user most needs the format rule restated. The hand-rolled product fields in the patient profile panel, the settings dialog and the formulation builder now mount those controls.Adopts the headers surface.
DsmPageHeader,InformationPageHeaderandInformationPageBreadcrumbsconverge ontoPageHeader+Breadcrumb, keeping the existing breadcrumb semantics includingaria-current="page"and the home crumb's back arrow.Adopts the catalogues and docs surfaces.
EmptyStateandChipreplace local one-off implementations across the launcher, favourites, DSM search and the filtered-out document result set.Fix found during integration: every record page crashed in a production build.
src/components/ui/page-header.tsxcarried"use client"while both of its importers are server components, so passing aLucideIconasiconraisedFunctions cannot be passed directly to Client Componentsand the formulation, specifiers, DSM diagnosis and DSM comparison pages all rendered the error boundary. The module holds no state, no effects and no event handlers, so the directive is removed rather than the icon contract reshaped. Only a production browser run surfaces this class of defect; typecheck and lint pass over it.Fix found during integration:
src/libimported@/components.#208placedcomposeAnswerClipboardText()insrc/lib/answer-clipboard.tsimportingAnswerStatefrom the design system, whichtests/lib-layering.test.tsforbids. The vocabulary moves down a layer intosrc/lib/answer-state-types.tsand the design-system module re-exports it, so all nine existing importers are untouched and there is still exactly one definition. It was deliberately not copied: the clipboard reads the union deeply, so a second copy would drift against the#207precedence rules on the code that decides which caveat lands in a clinical record.Fix found during integration: one warning was stated three times. On a weak-evidence answer the phone surface rendered
VerificationNotice, thenRetrievalStateBanner, then the live "Review source match" card — roughly eleven lines of near-identical caution around a one-line answer.RetrievalStateBannernow renders only forstale_evidenceandpartial_retrieval, the two states where it says something the notice cannot: which sources are overdue, and how much of the retrieval was missed. No wording changed and no state left the projection.Fix found during integration:
VerificationNoticecost 160px on a phone. It now takes a compact type scale belowsmand hides only its "Based on N cited sources." count on phones, where the Sources control directly below states the same number. The count stays in the DOM and returns atsmand in print. Every word of the warning survives at every width.Fix found during integration: a pin file that could never run.
tests/information-page-shell.test.tsxmatched neither vitest project — the node project collectstests/**/*.test.tsand the jsdom project collectstests/**/*.dom.test.tsx— so the only pin covering the breadcrumb and title-stack fold had never executed. Renamed onto the jsdom glob; its five tests now run.Fix found during integration:
Checkboxdeclared arefand discarded it. The component owns the input ref to driveindeterminate, which has no HTML attribute, and that callback sat after the props spread, so a caller ref typechecked and then did nothing. Forwarded by hand, with a test.Design-owner decisions taken on the adopted surfaces. The shared field shell now marks only the requirement in label text and leaves optional fields unmarked, so the patient profile's five numeric fields stop reading "Age (years) (optional)" down a column on a panel that already declares itself optional. The settings rows for Jurisdiction and Default population get click-to-focus back without giving up a correct accessible name: the visible row text is a real
<label htmlFor>again andaria-labelledbypoints at that same label, so the name is those words once rather than two<label for>elements concatenated. The DSM header keeps its converged muted eyebrow and gets its icon tile hidden belowsmagain, matching the hand-rolled header it replaced.Two defects found by the clinical governance review and fixed. The verification notice and the retrieval banner counted sources differently, so a stale-evidence answer could print "Based on 3 cited sources." directly above "2 of 7 sources for this answer are past their review date", leaving a clinician unable to tell how much of the evidence base is overdue; both now read the same projection. And
answerClipboardText()keyed attribution on theAnswerStatekind alone, so an extractive answer that is also weakly supported — which#207precedence reports asungrounded, notsource_only— would have pasted "AI-generated from the cited sources." over passages no model wrote. That primitive now takes the same explicitsourceOnlyflag the product composer already had. It is latent rather than live today, becauseAnswerCardhas zero product imports, and it would have become live the moment#216adopts the container.RAG impact: no retrieval behaviour change — this PR touches no file under
src/lib/rag/**and no ranking, selection, scoring or eval-fixture surface. The onlysrc/libfiles it adds or edits areanswer-clipboard.tsand the newanswer-state-types.ts, both of which are answer-output presentation rather than retrieval.Verification
npm run verify:pr-localTest Files 484 passed (484),Tests 5055 passed | 3 skipped (5058)— zero failures, run after mergingorigin/mainat483c50ba6into the branch.Before that merge this run reported two failures in
tests/codex-cloud-setup.test.ts, which were called environmental rather than caused by this diff: the error was/bin/bash: C:DevAppsDatabase-wt-ds-v2-correctnessscriptssetup-codex-cloud.sh: No such file or directory, a Windows Git-Bash path-mangling artefact with the backslashes stripped, against a script that exists on disk and that this PR does not touch.ccb21d9db("fix: repair Codex Cloud Python setup") landed onmainand both now pass, which confirms the attribution rather than merely asserting it.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed1 failed, 346 passed (11.7m). The single failure istests/ui-smoke.spec.ts:2056, the phone short-answer phantom-scroll guard, reading 29px against an 8px bare-overflow budget. That residual is theVerificationNoticeitself, which this PR deliberately keeps on every answer, so the page legitimately gained persistent height above the prose in the same way PR-V's 48px tap targets did. Finding L forbids re-pinning phone geometry from local readings — this machine measures 41–81px below CI on these tests — so the bound is re-pinned from the CI-reported numbers on this PR's first run rather than from a local figure. Before the fixes in this PR the same test read 147px, andui-smoke.spec.ts:2224read 271px against a 200px ceiling; 2224 now passes.npm run verify:releasebefore release or handoff confidence claimsVerification not run:
verify:releaseis provider-backed and needs explicit authorization; this PR makes no release or deployment claim.npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changedPASS (2),WARN (5),FAIL (2). Both failures are absent local credentials in the lane worktree —Missing server environment variables: NEXT_PUBLIC_SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEYandMissing OPENAI_API_KEY— in a checkout that deliberately carries no.env.local. No code path in this diff can produce them. The two passes are the ones this diff could plausibly affect: the Node 24 runtime match, and the boot guard invokingrequireQueryHashSecret()so the query-hash HMAC still fails closed in production (PIA-2).Offline RAG evidence, compared explicitly against the decision-J4 baseline at
plans\gate-baselines\eval-rag-offline_main-d4a446317.log:Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).Tests 574 passed (574)This is better than the baseline rather than merely equivalent. The baseline is 4 timeouts / 570 passed / 574 total; this run has zero failures, so the failing set is empty and is trivially a subset of the four permitted timeout titles. All four J4 conditions hold: the fixture line matches verbatim, there are no assertion failures, no failing title outside the baseline set, and no failing file outside the three baseline files.
npm run eval:retrieval:quality— Verification not run: no retrieval, ranking, selection, chunking or scoring behaviour changed, per theRAG impact:declaration above.npm run eval:rag/npm run eval:quality— Verification not run: these are live-provider evals and need explicit authorization. Answer generation is unchanged by this PR; what changed is how an already-generated answer's state is rendered and copied.Risk and rollout
VerificationNoticenow renders on every answer includingready, because ready is not verified. Third,RetrievalStateBanneris suppressed forungroundedandsource_only, where the notice and the support card already state the same caution; no information is lost that reaches the clinician only through the banner in those states. The forms fold also changes visible labels: the shared shell states optionality in the label text, so several numeric fields and the settings email field now read "(optional)" or "(required)".mainauto-deploy, which is the ordinary path for every program PR.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)On the last item specifically: this PR adds and preserves cautions rather than adding decision-support capability. It introduces no new clinical recommendation, no new automated advice, and no new inference over patient data. The one narrowing — suppressing a duplicate banner — leaves the strongest wording in the system in place for that state, plus the "Review source match" card and its action.
Notes
AnswerCardcontainer swap. The answer surface adopts the safety components in substance but not the shell, soAnswerCardstays at zero product imports. Replacing theanswerSurfacewrapper restructures the chrome of the primary clinical screen and collides with the phone geometry pins; it needs its own commit, its ownverify:uipass and its own glance. Reasons are recorded indocs/design-system/ADOPTION.md§2.6 and tracked as ledger#216.EmptyStaterenders its title as a<p>, so heading-bearing empty states cannot adopt it whole;/dsm/searchlost anh2as a result (#217,#224). The headers surface is only partially converged —mode-home-templateandsearch-results-header-bandwere declined with reasons (#222). LocalEmptyState/LoadingState/Chipduplicates remain in therapy-compass and differentials, each blocked for a stated reason (#221).cn()has no tailwind-merge, so aclassNamesize override resolves by stylesheet order, with a live instance indocument-search-results.tsx(#218); theDocumentViewerpreview error panel announces nothing when a preview fails after load (#219); the chip type scale contradicts itself betweenglobals.cssandui/chip.tsx(#220);MatchExplanationChipsis exported with zero call sites (#223); and the favourites empty state renders threerole="status"copies, one per breakpoint container (#225).docs/branch-review-ledger.mdatf9f73c707. The clinical governance review found the change net-additive: nothing onorigin/mainis weakened, dropped or narrowed,src/lib/rag/**is untouched at zero files, the patient-profile null-commit path is byte-identical through the field-shell fold so an out-of-range value still reaches the medication alert engine as "unassessed", andreadyis unreachable over an answer the product classifies as not source-backed. The frontend review found no tap-target, hex, z-index or raw-anchor violations, and confirmed thepage-header.tsx"use client"removal is a real fix rather than a relocated crash. Four findings between them: three are fixed in this PR, and one is recorded as#228.#228is the one review finding deliberately left open. Because#207precedence putsungroundedabovesource_only, an extractive answer that is also weakly supported renders the notice as "AI-generated, and the cited sources could not be shown to support every claim in it" directly above the amber Source-only disclosure saying it was assembled from the documents without the model — two contradictory provenance claims on one answer. It is not a lost warning, and the stronger instruction is the one shown, but the attribution is wrong in the direction of overstating model involvement. Fixing it properly needs an approved extractive-ungroundedwording, which is the clinical owner's call; the alternative stopgap trades the amber caution for correct attribution, which is not a trade this PR should make on its own.ungroundedVerificationNoticewording in both audiences, the four banner headlines and the four clipboard caveats are all open to change; the channel, the precedence and the pins are not. Two visible product changes are also open: the "(optional)" / "(required)" suffix on visible labels, and the settings rows for Jurisdiction and Default population, which gave up<label htmlFor>— and with it click-to-focus — so that the control has exactly one accessible name instead of two concatenated labels.Summary by CodeRabbit