feat(answer): adopt AnswerCard on the live answer surface; resolve #207 vs #227 - #1658
Merged
Conversation
… vs #227 The answer shell was half-adopted: `VerificationNotice` and `RetrievalStateBanner` were mounted, `AnswerCard` was not. Completing it surfaced a direct conflict between two documented, measured clinical decisions that had never been reconciled — neither referenced the other. - `#207` (tests/ui-v2-answer-safety.dom.test.tsx) required a retrieval banner on EVERY degraded state: "the adoption failure this guards is a silent one — the card renders, the prose is fine, and the caution the product shows today is simply gone." - `#227` (answer-result-surface.tsx) deliberately suppressed the banner for `ungrounded` and `source_only`, having measured the duplicate on a one-sentence answer: three renderings of one warning, eleven lines of caution around one line of answer, 147px of scroll against a phone budget of 8. Resolved in favour of `#227`, on the owner's decision. `#207` protects that the caution survives, not that a *banner* carries it — and it does survive: `VerificationNotice` states `ungrounded` and `source_only` in words, and the card carries `data-state`. The banner now renders for exactly the two kinds where it says something the notice cannot: `stale_evidence` names WHICH sources are overdue, `partial_retrieval` names HOW MUCH was missed. `onOpenSource` stays required for every degraded state (DECISIONS §Q1) — a degraded answer must remain re-verifiable whether or not a banner renders. The `#207` test now pins the two channels that genuinely carry the state and pins the duplicate OUT, so a banner reappearing for `ungrounded` fails as the `#227` regression it would be. Added coverage for `source_only` and for both kinds that keep the banner. The surface splits on `ready` vs degraded only because `AnswerCardProps` discriminates on `state` to make `onOpenSource` required for a degraded card; a union-typed `state` cannot narrow that at the call site. Verification props and prose are built once so both arms stay identical. `#228`'s attribution derivation (quality tier, never state kind) is preserved verbatim. Adoption: 27 -> 29 of 53 registered components product-adopted. `AnswerFooter` and `DoseLine` remain unadopted — the live surface has no provenance or dose payload to pass them yet. The reference-only assertion in design-system-adoption.test.ts is a snapshot of adoption state, so `AnswerCard`/`Quantity` moving out of it is the expected shape of an adoption change. Replaced with the inverse guard: an adopted component must also carry a production v2 mount, so "adopted" can never mean an import with no shell behind it. Verification: typecheck 0 · lint 0 · design-system-adoption 51 passed · ui-v2-answer-safety 69 passed. Full unit suite before the manifest regen: 5 444 passed, the only unrelated failure being tests/installed-lock-parity.test.ts (playwright 1.62.0 vs locked 1.62.1, issue #149, environmental). check:production-readiness fails on absent local Supabase/OpenAI env in this worktree — the documented offline-profile gap, not this diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 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 (3)
📝 WalkthroughWalkthroughChangesAnswerCard answer rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AnswerResultSurface
participant AnswerCard
participant VerificationNotice
participant RetrievalStateBanner
AnswerResultSurface->>AnswerCard: provide answer prose and verification metadata
AnswerCard->>VerificationNotice: render verification state
AnswerCard->>RetrievalStateBanner: render stale_evidence or partial_retrieval details
AnswerCard-->>AnswerResultSurface: provide degraded-answer source navigation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
BigSimmo
enabled auto-merge (squash)
August 6, 2026 17:35
Prior CI run cancelled/timed out while queued during the GitHub Actions major outage. Empty commit to re-fire checks on current tip. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/design-system/adoption-manifest.json`:
- Around line 3053-3058: Update the sanctioned patterns record for this
answer-surface entry to remove VerificationNotice, leaving the adopted
AnswerResultSurface and RetrievalStateBanner patterns unchanged.
In `@src/components/clinical-dashboard/answer-result-surface.tsx`:
- Around line 271-289: Pass query={query} to both AnswerCard instances in the
answer-state conditional so the query echo renders for ready and non-ready
states. Ensure answer-result-surface shows only this single AnswerCard-provided
query echo, removing or avoiding any duplicate visible echo on the surrounding
surface.
In `@src/components/ui/answer-card.tsx`:
- Around line 91-110: Correct the Ledger date wording in the shared rationale
comments: in src/components/ui/answer-card.tsx lines 91-110, replace the
inaccurate completed-decision claim with the actual decision date or explicitly
describe August 7, 2026 as planned/effective; align the related rationale
comments in tests/ui-v2-answer-safety.dom.test.tsx lines 525-530 and
tests/design-system-adoption.test.ts lines 1145-1149 with the same corrected
date and status.
🪄 Autofix
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: 5b869876-dec3-4f5b-8de9-30b51e0db424
📒 Files selected for processing (6)
docs/design-system/COMPONENTS.mddocs/design-system/adoption-manifest.jsonsrc/components/clinical-dashboard/answer-result-surface.tsxsrc/components/ui/answer-card.tsxtests/design-system-adoption.test.tstests/ui-v2-answer-safety.dom.test.tsx
Remove VerificationNotice from answer-surface sanctioned patterns now that AnswerCard owns that composition; pass query into AnswerCard and drop the duplicate UserQuestionBubble echo; correct #227 decision date to 3 Aug 2026. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Contributor
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8618 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
AnswerCard nested inside padded answerSurface with dual pad-panel stacks added ~60px phantom scroll (#227). Drop the outer surface pad, tighten card vertical density, and keep the query echo above table-aside grids so desktop prose/table alignment stays under the smoke budget. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
added a commit
that referenced
this pull request
Aug 7, 2026
…family (#1663) Section D, the half that carries no conflict with the two open PRs: this touches only `src/app/globals.css` and the contract scripts, none of which #1655 or #1658 holds. Corrects the measurement first. The backlog and the audit both quote the letterspacing sprawl as ~371 sites across 31 values; that number counts `*-mockups.tsx`, which is design scratch and gate-exempt by policy. Production only: 110 sites across 18 values. The gate's AST pass — which sees statically resolvable class roots — counts 68 across 40 files, and that is the number a ratchet can act on. The real defect turns out to be sharper than sprawl. 57 of the 110 production sites are one value, 0.06em, while the single pre-existing token `--tracking-eyebrow` is declared at 0.08em. The token has almost no consumers because it was named for a number nobody writes. Adds five named roles sized to the measured production mass, not to the mockup-inflated spread: --tracking-display -0.02em absorbs the six-site negative cluster --tracking-normal 0 --tracking-label 0.06em absorbs 66 sites including the 57-site mass --tracking-eyebrow 0.08em pre-existing, value deliberately unchanged --tracking-kicker 0.12em absorbs 23 sites Five roles rather than seven: production does not justify a second wide step or a separate tight-display step. No fold moves a call site by more than 0.04em. `--tracking-eyebrow` keeps 0.08em on purpose. Retuning it to the more common 0.06em would silently move every current consumer of the token — a different change from naming the scale, and one that wants its own measured step. Adds an `arbitraryTracking` metric to the design-system contract, mirroring the existing literal-shadow ratchet exactly, baselined per path at today's 68 so the family cannot regrow while the call-site migration waits for the open PRs to land. `tracking-[var(--…)]` is the sanctioned token form and is not counted, the same exemption `text-[color:var(--…)]` has in the type-scale check. No call sites migrated in this commit — that is 40 files including `ui-primitives.tsx`, which #1655 owns. Ratchet proven to bite, not assumed: injecting `tracking-[0.077em]` into mode-nav.tsx failed the checker with "arbitraryTracking increased from 68 to 69" and the per-path line, then the file was restored clean. Verification: check:design-system-contract exit 0 · typecheck 0 · lint 0 · ckb-v2-token-contract + design-token-contract + design-system-contract-utils 85 passed. Pre-existing baseline metrics were asserted unchanged before the baseline was rewritten; only `arbitraryTracking` was introduced. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 7, 2026
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 7, 2026
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This was referenced Aug 7, 2026
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
2 participants
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
Completes the answer-shell adoption.
VerificationNoticeandRetrievalStateBannerwere already mounted on the live surface;AnswerCardwas not, so the design system owned the wording but the product still owned the composition and ordering. The card now owns the query echo, the notice, the banner and their document order, on screen and on print alike. Adoption moves from 27 to 29 of 53 registered components.Resolves a direct conflict between two documented clinical decisions that had never been reconciled. Neither referenced the other:
#207(pinned intests/ui-v2-answer-safety.dom.test.tsx) required a retrieval banner on every degraded state, on the reasoning that "the adoption failure this guards is a silent one — the card renders, the prose is fine, and the caution the product shows today is simply gone."#227(inanswer-result-surface.tsx) deliberately suppressed the banner forungroundedandsource_only, having measured the duplicate on a one-sentence answer: three renderings of one warning, eleven lines of caution around one line of answer, 147px of scroll against a phone budget of 8.Resolved in favour of
#227.#207protects that the caution survives, not that a banner carries it — and it does survive:VerificationNoticestatesungroundedandsource_onlyin words, and the card carriesdata-state. The banner now renders for exactly the two kinds where it says something the notice cannot:stale_evidencenames which sources are overdue,partial_retrievalnames how much was missed. Three identical alarms teach a reader to skip all three, so the duplicate is the dangerous outcome, not the omission.onOpenSourcestays required for every degraded state (DECISIONS §Q1). A degraded answer must remain re-verifiable whether or not a banner renders.The
#207test now pins the duplicate OUT rather than in. It asserts the two channels that genuinely carry the state forungrounded, and asserts the banner is absent — so a banner reappearing there fails as the#227regression it would be. Added coverage forsource_onlyand for both kinds that keep the banner.#228's attribution derivation is preserved verbatim — attribution comes from the quality tier, never the state kind, sosource_onlycannot announce "AI-generated" directly above a disclosure saying no model wrote it.Test-guard change is an inversion, not a removal. The reference-only assertion in
design-system-adoption.test.tsis a snapshot of adoption state, soAnswerCard/Quantitymoving out of it is the expected shape of an adoption change. It is replaced with the inverse guard: an adopted component must also carry a production v2 mount, so "adopted" can never mean an import with no production shell behind it.AnswerFooterandDoseLineremain unadopted — the live surface has no provenance or dose payload to pass them yet. Not stubbed.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy) — no Supabase, migration, or env change in this diff.stale_evidenceandpartial_retrieval, which keep the banner naming overdue and missing sources. Forungroundedandsource_onlythe caution is still stated byVerificationNoticeand still exposed asdata-state; only the verbatim second rendering is removed. Degradation remains conservative: no state loses its caution, andonOpenSourceremains required so every degraded answer keeps its route back to source.scripts/pr-policy.mjsreturnsclinicalRisk: falsefor these paths, so this preflight is not machine-required. It is included because the repo's broader PR risk detection covers clinical output, and this diff changes what a clinician sees when an answer is ungrounded — the narrow classifier is a floor, not a closed enumeration.RAG impact: no retrieval behaviour change — this diff touches only client-side answer presentation. No file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture is modified.Verification
npm run typecheck— exit 0npm run lint— exit 0tests/ui-v2-answer-safety.dom.test.tsx— 69 passedtests/design-system-adoption.test.ts— 51 passedtests/answer-support-priority.dom.test.tsx,tests/answer-state-contract.test.ts— passed in the same run (146 passed across the four suites before the guard update)npm run test(full suite, before the manifest regeneration) — 5 444 passed, 3 skipped. The one unrelated failure istests/installed-lock-parity.test.ts(installed playwright 1.62.0 vs locked 1.62.1), environmental and pre-existing — issue#149.Not run, with reasons:
npm run verify:ui— this box had 83 node processes active; a contended Chromium run yields false reds rather than signal. This PR changes the visual structure of the primary answer screen (the answer now sits inside the card's bordered, raised surface rather than the transparentanswerSurface), so the Chromium job on this PR is the meaningful check and should be read carefully before merge.npm run check:production-readiness— fails on absent local Supabase/OpenAI env in this worktree, the documented offline-profile gap. Not introduced by this diff, and provider keys were deliberately not set.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation