Skip to content

DS V2 PR-E: answer safety, form foundation, announcements (slices 6+7+8) - #1576

Merged
BigSimmo merged 7 commits into
mainfrom
claude/ds-v2-answer-safety
Aug 2, 2026
Merged

DS V2 PR-E: answer safety, form foundation, announcements (slices 6+7+8)#1576
BigSimmo merged 7 commits into
mainfrom
claude/ds-v2-answer-safety

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Answer safety (SPEC §13 slice 6). AnswerCard now takes state: AnswerState and verification: VerificationNoticeProps as required props, so a generated answer that renders without its verification wording, or a degraded answer that renders without its caveat, is unrepresentable rather than merely discouraged. A degraded state additionally cannot be constructed without onOpenSource, because the clinician's next act after "this might be stale" is re-verification, not abandonment (DECISIONS §Q1). VerificationNotice owns the words — there is no children prop and no text prop, and the wording is never interpolated with a model or vendor name. RetrievalStateBanner renders the caution above the prose and gives every affected source a one-click route back to its cited page. MissingValue replaces the em-dash with four named phrases for absence, and DateDisplay puts every clinician-facing date through one locale, one timezone, and one absence rule.
  • Staleness is read, never inferred. answerStateFromRetrieval() projects the payload the app layer already receives onto the state the answer surface renders. It reads the server-set document_status governance enum and never compares a review date to the clock, so the review policy stays in the source-governance layer where it belongs. A source means a document, not a chunk: RagAnswer.sources is chunk-level and several chunks per document is normal, so the projection dedupes and counts by document_id. Chunk-level counting is wrong in both directions, and the direction that inflates the denominator under-warns.
  • Form foundation (slice 7). The FormField family keeps hint and error both in the DOM and both in describedBy when invalid, merges caller-supplied ids ahead of its own, supports external ids, renders required/optional as label text rather than colour, and gives ErrorSummary focus rather than an announcement.
  • Announcements and route focus (slice 8). A singleton announce() with a dedupe window and a queue gap, two visually-hidden live regions, and a RouteAnnouncer that moves focus to the new <h1> on navigation unless focus sits inside a dialog or a data-preserve-focus workflow.
  • Clinical governance review fixes (second commit). Eight findings from the review of the first commit, all in code this branch introduced. outdated (superseded) no longer renders in the review_due vocabulary; DoseRow replaces an optional overdue boolean with a required status: DocumentStatus whose overdue values also require source; stale_evidence with an empty overdue list now throws in development and states the caution without a count in production, instead of announcing "0 of 3 sources are past their review date"; VerificationNotice falls back to the most cautionary wording for an unrecognised state rather than failing open to ready; DateDisplay no longer invents a time for a date-only ISO; and two wordings that over-claimed were softened.
  • Clipboard hardened beyond the original spec scope. answerClipboardText() now carries attribution and "Verify against the linked source documents before clinical use." on every state including ready, enumerates the cited documents, and suppresses the single-document provenance line where it would contradict a multi-source stale caveat. A copied answer loses the banner, the notice and the links, and unattributed AI clinical prose pasted into a psychiatric record reads as clinician-endorsed. SPEC §13 originally scoped the copied caveat to non-ready states; exceeding that is deliberate and is recorded as a deviation in docs/design-system/SPEC.md.
  • Nothing is registered and nothing is adopted. No route, page, or product component imports any of these modules — verified mechanically across src/, worker/ and scripts/, where the only importers are the new modules themselves. Adoption is PR 13's job, and three constraints it must satisfy first are recorded in SPEC/COMPONENTS and in the outstanding-issues ledger as fix(ui): clinical UI design-review fixes (differentials, services, forms, favourites) #208, Wire Services/Forms to a Supabase registry (hybrid); label Differentials demo content #209 and fix(eval): resolve tsx runtime from ancestor node_modules in worktrees #210.

RAG impact: no retrieval behaviour change — nothing under src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, the golden fixture, or the retrieval RPCs is touched. answer-state.ts reads the app-facing RagAnswer shape structurally (it does not import the retrieval layer) and projects already-decided fields onto a render state; tests/answer-state-contract.test.ts pins that a real RagAnswer satisfies that shape so the two cannot drift.

Verification

  • npm run verify:pr-local
  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed

verify:ui reported 342 passed / 5 failed. Those five are not attributable to this diff, and the argument is mechanical rather than statistical: no rendered surface imports any of these modules, so Next never bundles them, and a test exercising rendered surfaces cannot execute code that is not in the bundle. On top of that, the same five specs were re-run scoped on this branch and on unmodified origin/main (dd7b65e2a) in a separate worktree with its own build root:

Spec verify:ui (347 tests) base dd7b65e2a scoped this branch scoped
ui-smoke.spec.ts:1053 mobile search focus fail pass pass
ui-smoke.spec.ts:3849 smart summary badges fail pass pass
ui-smoke.spec.ts:3894 source text accordion fail pass pass
ui-phone-scroll.spec.ts:30 Tools mode fail pass pass
ui-phone-scroll.spec.ts:318 shared header fail pass fail

The three ui-smoke assertion failures pass on this branch when run scoped, so the diff demonstrably does not produce them. ui-phone-scroll.spec.ts:30 is parameterised over 13 modes and fails a different subset on every run on both trees — the base tree failed Answer mode, which this branch passed twice — and where the failure text was captured it reads addPhoneScrollRunway: #main-content was not rendered, a render-timing failure rather than a geometry assertion. Nothing was re-pinned from a local number.

One residual is left open rather than papered over: ui-phone-scroll.spec.ts:318 failed both runs on this branch and passed the single base run. That is 2-vs-1 with no causal mechanism, inside the file already recorded as diverging locally from CI. Local re-runs cannot settle it; CI in the reference environment is the binding verdict and should be read before merge.

  • Full offline unit suite, lint, typecheck, format, design-system contract, docs surface, outstanding-issues and branch-review-ledger guards (all via verify:pr-local)
  • tests/answer-state-contract.test.ts — the standing proof that the typed state contract reaches the app layer with no change to src/lib/rag/** or src/lib/source-review.ts
  • npm run eval:retrieval:qualitynot applicable: no retrieval, ranking, selection, chunking, or scoring behaviour changed.
  • npm run eval:rag / npm run eval:qualitynot run: provider-backed, and no answer generation, synthesis prompt, or answer post-processing changed.
  • npm run verify:release — not run; no release or handoff confidence is claimed here.
  • npm run check:production-readiness — not run: nothing in this diff reaches production behaviour, since no module is imported by any shipped surface.

Risk and rollout

  • Risk: Low. Every file added or changed is either unreferenced by production code or documentation. The eight new modules have no importer outside their own set, so the shipped bundle is unchanged and no rendered surface can reach them. The residual risk is entirely in the future: if PR 13 adopts these components without first satisfying the three recorded constraints, it could ship a copy path weaker than today's formatAnswerRenderCopyText (Wire Services/Forms to a Supabase registry (hybrid); label Differentials demo content #209) or retire the "Review source match" warning the product shows for ungrounded answers (fix(ui): clinical UI design-review fixes (differentials, services, forms, favourites) #208). Both are recorded in docs/design-system/SPEC.md, docs/design-system/COMPONENTS.md and the outstanding-issues ledger precisely so that adoption cannot inherit this merge as clearance for them.
  • Rollback: revert the two commits on this branch. Nothing else depends on them, no migration runs, no data shape changes, and no configuration is touched.
  • Provider or production effects: None. No OpenAI, Supabase, Railway or hosted-CI mutation was performed. The Supabase target is unchanged.

Clinical Governance Preflight

classifyPullRequestFiles returns clinicalRisk: false for this file list, so this section is not machine-required. It is completed in full because the change defines the wording and the degradation behaviour of the clinical answer surface, and the plan requires the preflight on this PR regardless of what the classifier returns.

  • Source-backed claims still require linked source verification before clinical use — the verification wording is unconditional and is a required prop rather than a call-site convention. ready explicitly does not mean verified: its wording still reads "Verify every clinical claim against the linked source before acting on it." The copied text now carries the same instruction on every state, so an answer pasted out of the app cannot lose it.
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval — no document, upload, storage, or patient-data path is touched. The components render values handed to them and originate no data.
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — no Supabase configuration, migration, schema, RPC, or environment value is touched by this diff.
  • Service-role keys and private document access remain server-only — no key, credential, client, or authorization path is touched. The new modules are presentational and take no client.
  • Demo/synthetic content remains clearly separated from real clinical sources — no demo corpus, fixture, or fallback data path is touched, and no component synthesises clinical content. Where a value is absent, MissingValue names the absence rather than substituting a plausible default.
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative — this is the substance of the change. Staleness is read from the server-set document_status and never inferred from a date. unknown is treated as unknown, never as current and never as overdue. outdated is rendered as superseded, in its own shape and its own words, rather than being collapsed into the softer review_due vocabulary. An overdue source with no recorded review date is kept and shown as "Not recorded" rather than dropped, because a source known to be past review with no review commitment at all is the most alarming case, not the least. Where governance data is internally inconsistent — a stale answer naming no stale source, chunks of one document disagreeing on status — the components fail toward the more cautionary reading and, in development, throw rather than render a caution that argues against itself. An unrecognised state falls back to the strongest available disclaimer, never the neutral one.
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — no shipped behaviour changes, because no shipped surface imports these modules. The classification question belongs to PR 13, where adoption actually changes what a clinician sees; the constraints recorded here are what that assessment will need to start from.

Notes

  • Gate 4 in docs/design-system/GATES.md is recorded as implemented-partial rather than being flipped to blocking. The non-colour status encoding it governs is genuinely in place for the surfaces this PR builds, but the gate cannot honestly block repository-wide while the components are unregistered and the older surfaces are unconverted. The label is deliberately conservative and is called out here so it can be overruled rather than discovered later.
  • partial_retrieval has no producer. Nothing app-facing names which expected sources were unavailable — retrievalDiagnostics carries candidate counts and conflictsOrGaps carries prose. The component is built to its specified contract, but adoption can only emit ready, stale_evidence and source_only until a separate RAG contract PR adds a named missing-source signal. tests/answer-state-contract.test.ts pins the gap so it cannot be papered over in the component layer, and the gap is recorded as issue test(eval): recognize real Discharge Planning docs in golden alias #206.
  • The logged-once Sets used for enum-resilience warnings are module-level, so on the server they are per-process and unbounded: a persistent data defect logs once at boot and is then swallowed. Acceptable while these components are unregistered; recorded in fix(eval): resolve tsx runtime from ancestor node_modules in worktrees #210 with the Gate 1 contrast pair for --warning used at text tier.

Open in Devin Review

Summary by CodeRabbit

  • New Features

    • Added clearer verification notices, source attribution, retrieval-status messaging, and safe fallback states for answers.
    • Added structured dose information with status indicators and source-opening actions.
    • Added accessible form fields, error summaries, missing-value labels, consistent date display, and live announcements.
    • Improved clipboard exports with verification guidance, caveats, and source details.
  • Documentation

    • Updated design-system specifications, maturity tracking, review records, and outstanding issue documentation.
  • Tests

    • Added comprehensive coverage for answer safety, accessibility, forms, dates, announcements, sources, and dose statuses.

BigSimmo and others added 3 commits August 2, 2026 14:30
Build the five specified safety components so a degraded answer, an absent
clinical value, an invalid field and a route change all become states the type
system and assistive technology can see.

Slice 6 - answer safety. VerificationNotice owns eight approved strings (four
states x clinician/plain); AnswerCard requires both `state: AnswerState` and
`verification`, and a degraded state cannot be constructed without the
`onOpenSource` route that turns the caution into an affordance. DoseLine takes a
structured dose model, composes Quantity, and marks overdue in three channels
(amber rule + "Source review overdue" + StatusMark shape). MissingValue replaces
the bare dash with four phrases. DateDisplay takes ISO only and renders <time>.
AnswerFooter now names an absent field instead of dropping it - on a provenance
strip the absence is the governance signal. answerClipboardText carries the
degraded caveat out of the app and routes provenance through the one existing
clipboardProvenanceLine() implementation.

Slice 7 - FormField/FieldHint/FieldError/ErrorSummary. Hint and error are both
in the DOM and both in describedBy when invalid; caller ids merge ahead of them
rather than being overwritten; required/optional is label text; ErrorSummary
takes focus rather than announcing.

Slice 8 - LiveAnnouncer/RouteAnnouncer/announce. One singleton owns every
announcement, with a dedupe window and a queue gap; route change moves focus to
the new <h1> unless focus sits inside a dialog or a preserve-focus workflow.

Step 0 contract pre-check passed with no change to src/lib/rag/** or
source-review: ready, stale_evidence and source_only all project from fields the
retrieval layer already decides, so no component infers staleness from a date.
partial_retrieval has no producer and is recorded as such in SPEC, COMPONENTS,
/issues #206 and tests/answer-state-contract.test.ts.

All components are built, not registered - registration and product adoption
stay in PR 13.

Co-authored-by: Cursor <cursoragent@cursor.com>
…view

Eight findings from the PR 6 clinical governance review, all in code this
branch introduced, all safety-positive. No product surface imports any of
these modules yet, so nothing rendered changes.

Counting and identity
- answerStateFromRetrieval() counts and keys by document, not chunk.
  RagAnswer.sources is chunk-level, so the previous projection produced
  duplicate React keys, repeated a document as several overdue rows, and
  reported fractions that were wrong in both directions — including the
  direction that under-warns ("1 of 6" when half the documents are stale).
- Where chunks of one document disagree on governance status, the more
  severe reading wins.

Governance status carried, not collapsed
- OverdueSource gains `status`, and DoseRow replaces the optional
  `overdue` boolean with a required `status: DocumentStatus`. An overdue
  status additionally requires `source`, so a warned row with no route
  back to the document is unrepresentable. `outdated` (superseded) now
  renders the slashed mark and the word "superseded" rather than the
  half-ring "review overdue" vocabulary of a document still in force.

Cautions that cannot argue against themselves
- stale_evidence with an empty overdue list now throws in development
  (mirroring the partial_retrieval guard) and, in production, states the
  caution without a count instead of "0 of 3 sources are past their
  review date". Same rule in answerClipboardText().
- VerificationNotice falls back to the most cautionary wording for an
  unrecognised state, and logs once, rather than failing open to `ready`.
- The stale clinician wording says "some of which are past their review
  date" rather than asserting every cited source is overdue.
- SourceOnlyBody no longer claims "nothing has been paraphrased": the
  tier is inferred from the routing mode and the extractive builder
  composes sections, so the surface cannot stand behind verbatim fidelity.

Provenance precision
- DateDisplay renders a date-only ISO as a date, even for kind="generated".
  It previously printed "14/03/2026, 08:00" for "2026-03-14" — a precision
  that was never recorded, on a provenance strip.

Clipboard (exceeds the original SPEC slice-8 scope, deliberately)
- answerClipboardText() now carries attribution and "Verify against the
  linked source documents before clinical use." on every state including
  `ready`, enumerates the cited documents, and suppresses the
  single-document provenance line where it would contradict a
  multi-source stale caveat. A copied answer loses the banner, the notice
  and the links; unattributed clinical prose in a record reads as
  clinician-endorsed. It remains narrower than formatAnswerRenderCopyText
  and is not a replacement for it.

Recorded rather than fixed, so PR 13 cannot inherit them as clearance:
AnswerState has no channel for an ungrounded answer (#208), the clipboard
constraint above (#209), and the Gate 1 contrast pair for --warning at
text tier plus the per-process logged-once Sets (#210). All three are
also written into SPEC.md and COMPONENTS.md.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@supabase

supabase Bot commented Aug 2, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 34 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ba55c7b-4af0-40f1-9295-ebca8db0a3df

📥 Commits

Reviewing files that changed from the base of the PR and between 715d478 and 79cd040.

📒 Files selected for processing (11)
  • docs/design-system/COMPONENTS.md
  • src/components/ui/answer-card.tsx
  • src/components/ui/answer-state.ts
  • src/components/ui/form-field.tsx
  • src/components/ui/live-announcer.tsx
  • src/components/ui/missing-value.tsx
  • src/components/ui/retrieval-state-banner.tsx
  • tests/answer-state-contract.test.ts
  • tests/ui-v2-announcements.dom.test.tsx
  • tests/ui-v2-answer-safety.dom.test.tsx
  • tests/ui-v2-form-field.dom.test.tsx
📝 Walkthrough

Walkthrough

The PR adds typed answer-state and provenance contracts, safety-focused answer components, shared form accessibility components, live route announcements, comprehensive tests, and updated design-system records.

Changes

Answer safety and provenance

Layer / File(s) Summary
Answer-state contracts and projection
.design-sync/config.json, src/components/ui/answer-state.ts, tests/answer-state-contract.test.ts, docs/design-system/SPEC.md, docs/outstanding-issues.md
Answer retrieval data now maps to typed states with source deduplication, governance status handling, fallback classification, and citation-scoped evaluation.
Safety-aware answer components
src/components/ui/answer-card.tsx, src/components/ui/answer-state.ts, src/components/ui/retrieval-state-banner.tsx, src/components/ui/verification-notice.tsx, src/components/ui/date-display.tsx, src/components/ui/missing-value.tsx
Answer rendering now includes verification, retrieval-state messaging, provenance, source actions, structured dose rows, machine-readable dates, and explicit missing values.
Answer safety validation and records
tests/ui-v2-answer-safety.dom.test.tsx, tests/ui-v2-components.dom.test.tsx, docs/design-system/*, docs/branch-review-ledger.md
Tests cover answer safety, clipboard output, dose statuses, footer values, and component contracts. Design-system records document implementation status, gates, and deferred adoption work.

Form accessibility

Layer / File(s) Summary
Form field and error summary
src/components/ui/form-field.tsx, docs/design-system/COMPONENTS.md
FormField and ErrorSummary now provide accessible descriptions, validation state, autocomplete forwarding, labels, error rendering, and focus management.
Form accessibility tests
tests/ui-v2-form-field.dom.test.tsx
DOM tests cover descriptions, labels, autocomplete, invalid state, error navigation, and repeated submission focus.

Live announcements and route focus

Layer / File(s) Summary
Announcement runtime and route handling
src/components/ui/live-announcer.tsx, docs/design-system/COMPONENTS.md
LiveAnnouncer now queues and deduplicates announcements. RouteAnnouncer manages route focus, title announcements, initial-render silence, and dialog focus preservation.
Announcement behavior tests
tests/ui-v2-announcements.dom.test.tsx
Tests cover live-region priority, duplicate handling, singleton behavior, navigation focus, title announcements, and dialog focus preservation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the answer safety, form foundation, and announcements changes covered by the pull request.
Description check ✅ Passed The description covers the required summary, verification, risk, rollback, governance, and notes sections with detailed evidence and justified exceptions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aef5cf85a6

ℹ️ 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 thread src/components/ui/answer-state.ts
Comment thread src/components/ui/date-display.tsx Outdated
Comment thread src/components/ui/live-announcer.tsx
Comment thread src/components/ui/form-field.tsx Outdated
@BigSimmo

BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@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-answer-safety at starting commit aef5cf8; 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-answer-safety, 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.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread src/components/ui/live-announcer.tsx
Comment thread src/components/ui/answer-card.tsx
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: aef5cf85a6

ℹ️ 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".

@BigSimmo

BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent Please meticulously resolve all comments and ensure CI passes and address any other issues

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Taking a look!

Open in Web Open in Cursor 

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR builds (but does not yet register/adopt) a DS V2 safety-and-foundation slice set: answer safety primitives (verification wording, degraded-answer banner/state, provenance rendering, clipboard safety), a shared form field shell, and a centralized live-announcement + route-focus mechanism, with accompanying DOM/unit tests and design-system documentation updates.

Changes:

  • Introduces AnswerState + answer-safety UI components (AnswerCard, VerificationNotice, RetrievalStateBanner, DateDisplay, MissingValue, clipboard text/provenance behavior) with contract and DOM tests.
  • Adds a shared FormField + ErrorSummary foundation component with accessibility-driven behaviors, plus tests.
  • Adds a singleton announce() + LiveAnnouncer + RouteAnnouncer pattern for screen-reader announcements and route focus, plus tests and DS docs/ledger updates.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/ui-v2-form-field.dom.test.tsx New DOM tests for FormField/ErrorSummary behaviors.
tests/ui-v2-components.dom.test.tsx Updates existing component tests to new DoseLine/AnswerFooter contracts.
tests/ui-v2-answer-safety.dom.test.tsx New DOM tests covering answer-safety components and clipboard text.
tests/ui-v2-announcements.dom.test.tsx New DOM tests for LiveAnnouncer/RouteAnnouncer and dedupe behavior.
tests/answer-state-contract.test.ts New contract test pinning the projection shape from app payload to AnswerState.
src/components/ui/verification-notice.tsx New system-owned verification wording component with state/audience variants.
src/components/ui/retrieval-state-banner.tsx New degraded-answer banner rendering overdue/missing-source details and actions.
src/components/ui/missing-value.tsx New “missing value” component with explicit absence phrases + enum resilience.
src/components/ui/live-announcer.tsx New singleton announcement store + route focus/announce behavior.
src/components/ui/form-field.tsx New shared field shell (hint/error/ids/required labeling) + error summary.
src/components/ui/date-display.tsx New centralized date formatting + missing/invalid handling + relative companion.
src/components/ui/answer-state.ts New projection (answerStateFromRetrieval) and state types for answer surfaces.
src/components/ui/answer-card.tsx Tightens answer surface contracts (required state/verification, actions, provenance, clipboard text, dose model).
docs/outstanding-issues.md Records new follow-up items and advances next-id.
docs/design-system/SPEC.md Updates SPEC phase table and records PR 6 notes/constraints.
docs/design-system/GATES.md Updates Gate 4 status/evidence and clarifies partial-blocking scope.
docs/design-system/COMPONENTS.md Updates component inventory to reflect newly-built/tested symbols.
docs/branch-review-ledger.md Appends a review record for this branch/PR scope.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/ui-v2-form-field.dom.test.tsx Outdated
Comment thread src/components/ui/retrieval-state-banner.tsx Outdated
Comment thread src/components/ui/retrieval-state-banner.tsx Outdated
Filter staleness to cited supporting sources, reject rolled calendar dates,
make duplicate production announcers a no-op, re-focus ErrorSummary on
repeated submits, clear-then-set identical live announcements, and realign
AnswerCard/DoseLine dtsPropsFor declarations.
Keep production duplicate ownership in an effect with a targeted
set-state-in-effect exemption, matching the repo pattern, so the
duplicate never registers a listener and unmounts its live regions.
Import ComponentProps from react, require exact overdue/sourceCount
equality for the totality headline, and derive the partial-retrieval
numerator from the rendered missing list.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (1)
src/components/ui/live-announcer.tsx (1)

124-132: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Move the reactive setState out of the effect body or justify it explicitly.

Static analysis flags setSuppressed(true) as a synchronous setState call inside a useEffect body, which can trigger cascading renders. The call is reachable at runtime whenever a second LiveAnnouncer mounts in production.

Consider deriving the suppression flag through useSyncExternalStore against store.mounted, or, if the effect-based detection is intentional because it must run after commit to observe sibling mounts, add an explicit eslint-disable-next-line react-hooks/set-state-in-effect with a short justification comment so the exception is documented rather than silently tripping the rule on every lint run.

🛠️ Proposed fix (documented suppression)
       console.warn(JSON.stringify({ level: "warn", message: "live-announcer: duplicate instance mounted" }));
+      // eslint-disable-next-line react-hooks/set-state-in-effect -- reacts to a sibling
+      // instance mounting (external condition), not to committed props/state.
       setSuppressed(true);
🤖 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/ui/live-announcer.tsx` around lines 124 - 132, Document the
intentional effect-based update in the duplicate-instance branch of the
LiveAnnouncer mount effect: add a brief justification comment and a narrowly
scoped eslint-disable-next-line for react-hooks/set-state-in-effect immediately
before setSuppressed(true). Keep the post-commit sibling detection and existing
cleanup behavior unchanged.

Source: Linters/SAST tools

🤖 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/COMPONENTS.md`:
- Around line 203-220: Fix the documentation comment immediately above the
OverdueSource type by removing the premature closing marker between the
reviewDueOn and status paragraphs, leaving one continuous block comment that
closes only before the type declaration.

In `@src/components/ui/answer-card.tsx`:
- Around line 179-190: Update the provenance construction in answerClipboardText
so clipboardProvenanceLine is called only when metadata is supplied, omitting
provenance entirely when metadata is null or undefined. Preserve the existing
multi-source stale_evidence suppression and normalized metadata behavior for
provided metadata.

In `@src/components/ui/answer-state.ts`:
- Around line 135-148: Update overdueSourceFrom so review_due and outdated
sources with synthetic __unidentified_ keys are retained instead of returning
null. Preserve the synthetic key in OverdueSource, and update
RetrievalStateBanner to render these entries without invoking onOpenSource.
Ensure unidentified overdue sources no longer allow the answer state to project
as ready.

In `@src/components/ui/form-field.tsx`:
- Around line 152-165: Update the error-summary container in the form-field
component to expose an appropriate heading role and accessible name via the
heading text. Move the useId() call to the top of the component alongside the
other hooks, apply its generated id to the heading, and connect the focusable
container with aria-labelledby while preserving the existing focus behavior.

In `@src/components/ui/live-announcer.tsx`:
- Around line 119-140: Update the LiveAnnouncer singleton lifecycle around the
useEffect and shared store so suppressed instances can become active when the
current active instance unmounts. Track instance ownership/order and notify
mounted instances when active ownership changes, ensuring exactly one remaining
instance registers a listener and renders announcements while duplicates remain
suppressed; preserve duplicate detection and cleanup behavior.

In `@src/components/ui/missing-value.tsx`:
- Around line 30-43: Update missingValuePhrase to validate the resolved
PHRASES[reason] value is a string before returning it, so inherited keys such as
"toString" and "constructor" follow the unknown-reason fallback and logging
path. Preserve the existing known-phrase behavior and return PHRASES.unknown for
non-string or absent values.

In `@src/components/ui/retrieval-state-banner.tsx`:
- Around line 71-87: Update the everySourceOverdue/scope logic in the
retrieval-state banner so a positive overdue count with sourceCount less than or
equal to overdue.length is treated as totality, preventing contradictory “N of
0” wording. Preserve the existing zero-overdue fallback and use the totality
wording for the superseded and review-date headline branches.

---

Nitpick comments:
In `@src/components/ui/live-announcer.tsx`:
- Around line 124-132: Document the intentional effect-based update in the
duplicate-instance branch of the LiveAnnouncer mount effect: add a brief
justification comment and a narrowly scoped eslint-disable-next-line for
react-hooks/set-state-in-effect immediately before setSuppressed(true). Keep the
post-commit sibling detection and existing cleanup behavior unchanged.
🪄 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: f788a2b2-6ce7-4506-98cc-0c073e0c3e80

📥 Commits

Reviewing files that changed from the base of the PR and between dd7b65e and 715d478.

📒 Files selected for processing (19)
  • .design-sync/config.json
  • docs/branch-review-ledger.md
  • docs/design-system/COMPONENTS.md
  • docs/design-system/GATES.md
  • docs/design-system/SPEC.md
  • docs/outstanding-issues.md
  • src/components/ui/answer-card.tsx
  • src/components/ui/answer-state.ts
  • src/components/ui/date-display.tsx
  • src/components/ui/form-field.tsx
  • src/components/ui/live-announcer.tsx
  • src/components/ui/missing-value.tsx
  • src/components/ui/retrieval-state-banner.tsx
  • src/components/ui/verification-notice.tsx
  • tests/answer-state-contract.test.ts
  • tests/ui-v2-announcements.dom.test.tsx
  • tests/ui-v2-answer-safety.dom.test.tsx
  • tests/ui-v2-components.dom.test.tsx
  • tests/ui-v2-form-field.dom.test.tsx

Comment thread docs/design-system/COMPONENTS.md
Comment thread src/components/ui/answer-card.tsx
Comment thread src/components/ui/answer-state.ts
Comment thread src/components/ui/form-field.tsx Outdated
Comment thread src/components/ui/live-announcer.tsx Outdated
Comment thread src/components/ui/missing-value.tsx
Comment thread src/components/ui/retrieval-state-banner.tsx Outdated
@BigSimmo

BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent Please review all the open comments from coderabbit AI and address all of them and then resolve them for me

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

All 7 open CodeRabbit comments on PR #1576 are fixed and resolved. CI is green on 79cd0405 (PR required pass).

Finding Fix
Broken OverdueSource doc comment Merged into one continuous block in COMPONENTS.md
Clipboard provenance with no metadata Omit the line unless metadata is supplied
Unidentified overdue sources → ready Keep them in stale_evidence; banner skips open action
ErrorSummary focus without a name role="group" + h2 + aria-labelledby
LiveAnnouncer silence after owner unmount Ownership failover via useSyncExternalStore
missingValuePhrase("toString") throws Object.hasOwn + string-type check
"N of 0 sources" headline Totality when overdue.length >= sourceCount

No review threads remain open.

Open in Web Open in Cursor 

Merge the broken OverdueSource doc comment, omit clipboard provenance
when metadata is absent, keep unidentified overdue sources as stale,
give ErrorSummary an accessible heading name, fail over LiveAnnouncer
ownership when the active instance unmounts, harden MissingValue phrase
lookup against prototype keys, and treat sourceCount underflow as totality.
@BigSimmo
BigSimmo enabled auto-merge (squash) August 2, 2026 10:15
@BigSimmo
BigSimmo disabled auto-merge August 2, 2026 10:16
@BigSimmo
BigSimmo enabled auto-merge (squash) August 2, 2026 10:19
@BigSimmo
BigSimmo disabled auto-merge August 2, 2026 10:19
@BigSimmo
BigSimmo enabled auto-merge (squash) August 2, 2026 10:20
@BigSimmo
BigSimmo disabled auto-merge August 2, 2026 10:20
@BigSimmo
BigSimmo enabled auto-merge (squash) August 2, 2026 10:22
@BigSimmo
BigSimmo disabled auto-merge August 2, 2026 10:22
@BigSimmo
BigSimmo enabled auto-merge (squash) August 2, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants