fix(rag): deterministic ranking tiebreak + alias cache hardening (Phase 1) - #218
Merged
Conversation
… deferral fetchEnabledRagAliases no longer caches an empty result on a transient rag_aliases read failure. Caching [] suppressed alias-based query expansion (and could let an alias-rescuable query short-circuit) for the whole TTL; it now returns empty for the failing call only and retries next call. Also documents in docs/process-hardening.md why finding #11 (nondeterministic "unsupported" retrieval — bipolar/anorexia intermittently returning 0 results) has no safe Phase-1 fix: the deterministic classifier cannot distinguish in-corpus from out-of-corpus topics, and removing the LLM-gated soft-tail short-circuit regresses eval unsupported_correct 1.0->0.79 in this broad corpus. Deferred to Phase 2 corpus-grounded relevance (IDF/semantic + RC6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
finalScore is clamped to [0,1], so heavily-boosted results saturate at 1.0 and tie. rankClinicalResults now falls through to the pre-clamp boost magnitude (recovering the boost engineering the clamp discards) and finally a stable id comparison, so fully-tied results no longer order by arbitrary retrieval order (a residual run-to-run nondeterminism). Both levels fire only after score/tieBreakScore have tied, so ranking quality is unchanged: golden retrieval stays 23/23 with mrr@10=0.7283 identical to baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # docs/process-hardening.md
BigSimmo
enabled auto-merge (squash)
July 3, 2026 03:44
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38bfdacd2
ℹ️ 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".
Comment on lines
+2005
to
2008
| // Do not cache an empty result on a transient rag_aliases read failure: caching [] would suppress | ||
| // alias-based query expansion (and could let an alias-rescuable query short-circuit) for the whole | ||
| // TTL. Return empty for this call only and retry on the next call. | ||
| return []; |
There was a problem hiding this comment.
When this catch returns [] after a transient rag_aliases read error, an alias-rescuable unsupported query can still enter the short-circuit path and setCachedSearch(args, [], ...) below. If the alias canonical is already the primary variant (for example an alias like bipolar → bipolar disorder on the query bipolar disorder), a successful alias read on the next request builds the same queryVariants cache key and receives the cached zero-result response for RAG_SEARCH_CACHE_TTL_MS, so the failure is not limited to the current call. Carry an alias-read-failed signal and skip caching the unsupported result (or include the alias-read status in the cache key) for this path.
Useful? React with 👍 / 👎.
BigSimmo
added a commit
that referenced
this pull request
Aug 3, 2026
* feat(ds): give AnswerState an ungrounded channel (#207) 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> * feat(ds): compose the answer clipboard path instead of replacing it (#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> * docs(ds): register PR 13 adoption surfaces and their allowlists (PR-J 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> * feat(ds): tie the Review source match caution to AnswerState (provenance 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> * feat(ds): adopt the answer safety components and the composed copy path 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> * issues: record the deferred AnswerCard container swap as #216 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> * issues: record the seven design and defect findings from PR-J adoption 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> * feat(ds): adopt EmptyState and Chip across the catalogue surfaces 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. * feat(ds): adopt EmptyState for the filtered-out document result set 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. * feat(ds): fold the form controls onto FormField and adopt the product 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. * feat(ds): converge the record headers and breadcrumbs onto PageHeader 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. * fix(ds): make the shell pin executable and stop Checkbox dropping a caller 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. * issues: record the two adoption findings from the builder hunk read #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. * issues: record the phone short-runway blocker as #226 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. * fix(ds): give VerificationNotice a compact phone treatment 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> * fix(ds): keep PageHeader server-side so record pages stop crashing 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> * fix(ds): stop the answer surface stating one warning three times 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> * issues: close #227 and record the #226 residual #227 resolved in PR-J: the banner now renders only where it adds information the notice cannot. #226 keeps its residual — ui-smoke:2056 reads 29px against an 8px budget, which is the notice itself and must be re-pinned from CI numbers rather than from this machine. * fix(ds): move the answer-state vocabulary into src/lib and repin a label 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> * fix(ds): take the three product decisions on the adopted surfaces 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> * fix(ds): one source count on the answer surface, and close the AnswerCard 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> * issues: record the two review findings and the #226 decision #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. * ledger: record the PR-J clinical and frontend reviews at f9f73c7 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. * test(ui): address the visible settings label by id, not by text 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. * refactor(ds): extract the answer clipboard payload out of ClinicalDashboard 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> * test(ui): re-pin the phone answer geometry from CI numbers 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. * fix(ds): stop the notice claiming a model wrote an extractive answer #228. #207 precedence puts stale_evidence, partial_retrieval and ungrounded above source_only, so an extractive answer that is also stale, partial or unsupported reports one of those kinds — and every one of their wordings opened with "AI-generated". The surface therefore announced model authorship directly above the amber Source-only disclosure saying no model wrote it. Two contradictory claims about the one fact that decides how a clinician weighs the answer, and the false one overstates model involvement. VerificationNotice takes an explicit `attribution` prop, and the answer surface passes the quality tier rather than the state kind — the same reason the clipboard composer takes an explicit sourceOnly flag (#208). Attribution is not derivable from the kind; that is the whole defect. Each extractive string keeps its state's full instruction and changes only the provenance clause, in both audiences. Wording is a spec change per this component's own header, so these six strings are for the clinical owner to sign off at the glance; the channel and precedence are not open. Default stays "model", so an un-widened caller renders exactly as before. Local: typecheck, lint exit 0; 90 passed across answer safety, the answer-state contract and the product clipboard path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * issues: close #228 as fixed and #229 as accepted #228 fixed in this PR: the notice takes an explicit attribution and the answer surface passes the quality tier, so an extractive answer no longer claims a model wrote it. The six new wordings still need the clinical owner sign-off at the glance. #229 accepted: the DSM eyebrow beside the icon is the converged PageHeader treatment, and special-casing one caller costs more than the cosmetic difference. * fix(ds): treat empty answer.sources as unpopulated for state projection 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> * ledger: record bugbot assessment of PR #1595 at 407c8e7 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(ds): open stale sources at the cited page and wire clipboard provenance 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> * ledger: record review-and-fix of PR #1595 at cb898be Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(test): complete the ClinicalSourceMetadata fixture so typecheck passes 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. * fix(test): make the bugbot fixtures typecheck 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. * fix(ds): resolve the two Codex review findings on the stale-source controls 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> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
added a commit
that referenced
this pull request
Aug 7, 2026
`cn()` was a plain `classes.filter(Boolean).join(" ")`, so a later class could
never override an earlier one — every size or colour override had to be worked
around at the call site, and the winner was decided by Tailwind's stylesheet
emission order rather than by intent. Five comments across four files documented
that constraint.
The substance is not the `cn()` body, which is a one-line change keeping its
exact previous signature and falsy filtering. It is `src/lib/tailwind-merge.ts`:
stock tailwind-merge classifies any unrecognised `text-<x>` as a text COLOUR,
and this repo's `@theme` defines scales it has never seen. `text-sm-minus`,
`text-base-minus`, `text-lg-minus`, `text-2xl-minus`, `text-2xl-compact`,
`text-3xl-minus` and `text-hero` were each measured being deleted when they met
`text-[color:var(--text-muted)]` in the same call — the `eyebrowText` recipe is
exactly that pair. `size-icon-*`, `tracking-*`, `leading-*`, `ease-*`,
`animate-*` and the `pt-safe` family were not recognised at all, so they never
merged. All of that is silent: no type error, no lint error.
`--spacing-tap` is deliberately NOT declared. Tailwind emits `.min-h-tap` after
every numeric `.min-h-*`, so the 48px tap token wins today at the 22 call sites
that pair them; declaring it would hand the win to the later class and drop 18
production targets to 32/36/40/42px, which AGENTS.md forbids. The omission is
documented in the config and pinned by a test.
Every other family was measured across all 1 409 `cn()` call sites and
introduces zero new class deletions. The 339 sites where twMerge drops
`focus-visible:outline` beside `focus-visible:outline-2` are CSS-identical:
compiling Tailwind shows both utilities emit the same two properties, with
`--tw-outline-style` defaulting to `solid`.
The five workaround comments now record that the constraint is lifted. The
dodges themselves are kept — reverting `max-sm:hidden` or collapsing the
three-branch chip changes which utilities render, and belongs in a change whose
Chromium job is being read, not in a dependency swap. One of the five is only
half lifted: tailwind-merge scores bare `border` and `border-t` as separate
groups, so `document-search-results.tsx` still needs its `border-0`.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
BigSimmo
added a commit
that referenced
this pull request
Aug 9, 2026
…lection (#262 parts 2 and 3) (#1780) * docs(issues): close #218 and #270, both shipped before this session Both rows were still open in docs/outstanding-issues.md while their work was already live on main, which had scoped a third session from them. #218 (cn() lacks tailwind-merge) shipped in PR #1678, aeba5a2. src/components/ui-primitives.tsx:37 is twMergeClinical(...) rather than a plain join, package.json carries tailwind-merge ^3.6.0, and src/lib/tailwind-merge.ts declares the repo's @theme scales to twMerge. #270 (declare the tap spacing token) shipped in PR #1738, 80cf781, an ancestor of origin/main. "tap" is present in CLINICAL_TWMERGE_THEME.spacing, and tests/tailwind-merge-config.test.ts was inverted rather than deleted so the merge behaviour is now asserted rather than pinned out. Verified in source at origin/main 7aaf934, not inferred from the handover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): ratchet raw padding, radius and line-height literals (#262 part 3) The design-system contract ratcheted colour, shadow, tap and tracking but not spacing, radius or line-height, so a value could bypass the scale as a bare literal in either a class or a stylesheet and nothing objected. Adds three per-path ratchets, covering both halves the way the colour and legacy-shadow metrics already do: rawPaddingLiterals 67 (17 CSS declarations, 50 class utilities) rawRadiusLiterals 24 (22 CSS declarations, 2 class utilities) rawLineHeightLiterals 3 (3 CSS declarations) The exemption is deliberately "contains no CSS function", not the narrower `(?!var\()` the tracking rule uses. Padding is not only ever a token or a literal: production ships pb-[env(safe-area-inset-bottom)], pt-[max(0.75rem,var(--safe-area-top))], pt-[clamp(1.5rem,5vh,3rem)] and pb-[calc(7rem+env(safe-area-inset-bottom))]. Those are computed from the viewport or the safe-area inset, cannot be spelled as a scale step, and a `var(`-only lookahead would have flagged every one of them. On the CSS side, zero in any unit, the CSS-wide keywords and custom-property declarations (the token definitions themselves) are exempt for the same reason. Every one of the 94 baseline entries was verified present at its cited line before pinning, and the baseline change is additive: all fifteen pre-existing metrics and every pre-existing debtByPath entry are byte-identical. Mutation-tested rather than assumed. Class side, in a file with no prior debt: - rawPaddingLiterals increased from 67 to 68 - rawPaddingLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawRadiusLiterals increased from 24 to 25 - rawRadiusLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawLineHeightLiterals increased from 3 to 4 - rawLineHeightLiterals at src/components/ui-primitives.tsx increased from 0 to 1 The CSS half fails the same way. Both probes also carried the sanctioned computed forms, and each count rose by exactly one, so the exemptions are proved by the same runs rather than argued. No new npm script: the metrics live inside check:design-system-contract, so docs:check-inventory and check:gate-manifest are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): gate type-step selection on the decidable half (#262 part 2) check:type-scale blocks arbitrary text-[12px] values. Nothing has stopped the scale itself growing a step no surface ever picks, which is the drift that makes a wrong selection possible in the first place. Whether a heading should have chosen text-sm over text-sm-minus is not mechanically decidable, and this does not pretend otherwise. A step that is declared and consumed by nobody is decidable, and there is one today: --text-2xl-compact (globals.css:112) has zero consumers -- no utility use, no var() use -- while the next-rarest step, text-hero, has one real consumer. The analyzer reports every bare text-<name> it sees and does not decide which names are steps; the checker intersects that against the @theme block it parses from globals.css. So the scale is never written down twice, and a step added to globals.css is covered without touching this gate. Retiring the dead step edits @theme, so it gets its own revertible PR rather than riding along here: it is carried in UNUSED_TYPE_STEP_EXEMPTIONS and tracked as docs/outstanding-issues.md #295. The exemption cannot rot silently -- the gate also fails if an exempted step stops being declared or gains a consumer. Mutation-tested, three ways: - type steps are declared in globals.css @theme but no production surface selects them: --text-2xl-compact (text-2xl-compact). Retire the step or use it; do not leave the scale carrying a step nobody picks. - (a newly added --text-probe-step fails identically, so this catches future drift rather than only today's known case) - --text-2xl-compact is exempted as unused but production now selects text-2xl-compact -- drop the exemption Measurement note, since three different figures were in circulation for this row: the "1318 sites" is a repo-wide grep INCLUDING mockups, which the gate excludes (1360 at this HEAD). Production consumers of the nine non-standard steps total 705 -- text-2xs 421, sm-minus 160, base-minus 57, 3xs 42, 2xl-minus 9, 3xl-minus 9, lg-minus 6, hero 1, 2xl-compact 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(design-system): correct GATES.md for the two new scale gates GATES.md's own §1 is the list of what actually runs, and this series' recurring failure is that list lagging the code: four of #264's six prohibitions were already gated while it said "planned". Records the padding/radius/line-height ratchets and the type-step selection rule in the contract row, and rewrites the type-scale callout, which claimed a step-selection lint "does not exist". The decidable half now ships; the half that asks whether text-sm-minus was the right pick over text-sm still does not, and cannot. Also corrects the "1 318 call sites" figure quoted there. It was a repo-wide grep including src/app/mockups/**, which every one of these gates excludes (1 360 at 7aaf934). Production consumers total 705, and there are nine non-standard steps, not eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the #262 parts 2/3 gate work (PR #1780) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(design-system): close scale-ratchet and unused-step review gaps Cover Tailwind arbitrary-property forms and modern CSS zero units in the raw scale ratchets, and validate unused-step exemptions against the same class-or-CSS consumer predicate used for ordinary steps. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Two small, isolated RAG fixes from a Phase-1 bug-sweep, plus a debt writeup.
Deterministic ranking tiebreak (
src/lib/clinical-search.ts).finalScoreis clamped to[0,1], so heavily-boosted results saturate at 1.0 and tie.rankClinicalResultsnow falls through to the pre-clamp boost magnitude (recovering the boost engineering the clamp discards) and finally a stableidcomparison, so fully-tied results no longer order by arbitrary retrieval order — a residual run-to-run nondeterminism. Both new levels fire only afterscoreand the engineeredrankingTieBreakScorehave tied, so ranking quality is unchanged.Alias cache hardening (
src/lib/rag.ts).fetchEnabledRagAliasesno longer caches an empty result on a transientrag_aliasesread failure — caching[]suppressed alias-based query expansion (and could let an alias-rescuable query short-circuit) for the whole TTL. It now returns empty for the failing call only and retries next call.Debt writeup (
docs/process-hardening.md). Documents why finding Improve Clinical KB dashboard and RAG hardening #11 (nondeterministic "unsupported" retrieval — bipolar/anorexia intermittently returning 0 results) has no safe Phase-1 fix and is deferred to Phase 2's corpus-grounded relevance work. Root cause: an LLM query classifier nondeterministically gates the soft-tail short-circuit; removing it regresseseval:quality --rag-onlyunsupported_correct1.0→0.79 in this broad corpus because lexical signals can't separate in-corpus from out-of-corpus topics.Validation
npm run eval:retrieval:quality: 23/23 pass, doc/content recall@5 = 1.0, mrr@10 = 0.7283 (identical to baseline), 0 failures.npm run typecheck: clean. Ranking/routing/selection unit tests: 113/113 pass on the merged-with-main state.npm run verify:cheap: green apart from a pre-existing flaky/slow test (rag-cache-invalidationtimes out at 15s on cleanmaintoo, confirmed via stash) and load-induced timeouts that pass in isolation.Clinical governance preflight
eval:quality --rag-onlymetrics (grounded-supported 0.933, unsupported-correct 1.0, citation-failure 0, numeric-grounding 0) are unaffected by ranking-tiebreak-only reordering.🤖 Generated with Claude Code