Resolve #336/#222/#321 flagged by PR #2081; leave #322 to clinical review - #2084
Conversation
…view - #336: decide responsive breakpoint windows stay raw min-[]/max-[] rather than gaining named --breakpoint-* tokens; record the decision and rationale in docs/design-system/GATES.md (§3 row + new §3b). - #222: decide mode-home-template.tsx and search-results-header-band.tsx are permanently outside the PageHeader vocabulary, for the structural reasons the row itself laid out; record as DECISIONS.md C7. - #321: fix the one group that was genuinely mechanical — convert the document image filmstrip's "Page unknown" chip from native `disabled` to the stated-reason aria-disabled + ignoreUnavailableActivation pattern (docs/wiring-conventions.md), with a focused DOM test. The other three follow-up groups stay open — they are coupled to a planned rewrite, an unmade classification decision, and a capacity-state judgement call, none of which this change makes. - #322 (Warfarin catalogue dedup) is deliberately untouched: the row itself says a clinician must decide whether to merge/delete/relabel the two records, and PR #2069 already targets it — duplicating that work here would be exactly the #292 failure mode. Queues the corresponding issues:done/issues:update inbox requests; canonical ledger edits happen later via npm run issues:reconcile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsYJaPamTdCXMBh9hW3Rnm
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 101 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. 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 (10)
Comment |
There was a problem hiding this comment.
Pull request overview
Resolves three previously-flagged outstanding-issues items by recording design-system decisions (#336, #222) and fixing a specific accessibility/wiring defect in the DocumentViewer filmstrip control (#321), while intentionally leaving the clinically-scoped dedup item (#322) untouched. This aligns with the repo’s “serial-write” outstanding-issues workflow by queuing immutable inbox requests rather than editing the canonical ledger directly.
Changes:
- Convert the DocumentViewer filmstrip “page unknown” chip from native
disabledtoaria-disabled+ignoreUnavailableActivation+sr-onlyreason, and add a focused DOM test. - Record the breakpoint-token decision for
min-[…]/max-[…]vs named breakpoint tokens indocs/design-system/GATES.md(#336). - Record the “out of PageHeader vocabulary” decision for
mode-home-template.tsxandsearch-results-header-band.tsxindocs/design-system/DECISIONS.md(#222), and queue corresponding outstanding-issues inbox updates/done markers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/document-viewer/document-image-filmstrip.tsx | Makes the “page unknown” chip reachable-but-inert via aria-disabled + inert click handler + aria-describedby reason. |
| tests/document-image-filmstrip.dom.test.tsx | Adds DOM coverage asserting the chip is focusable, aria-disabled, not natively disabled, and click is a no-op. |
| docs/design-system/GATES.md | Records decision to keep responsive breakpoint windows as raw Tailwind arbitrary min-[…]/max-[…] values (#336). |
| docs/design-system/DECISIONS.md | Records decision to keep mode-home-template.tsx and search-results-header-band.tsx outside PageHeader vocabulary (#222). |
| docs/outstanding-issues-inbox/993a1c72-96bd-4e48-b1a4-3ea3b603acac.json | Marks #336 as done with recorded outcome + fingerprint. |
| docs/outstanding-issues-inbox/4302ff5b-582a-4204-94bf-d554c953900f.json | Marks #222 as done with recorded outcome + fingerprint. |
| docs/outstanding-issues-inbox/d25147c8-3da0-4062-9ce0-356e59a16c63.json | Updates #321 as partial, noting the filmstrip fix and leaving the other groups open. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
PR #2081 (already merged) queued "update" flags for #222, #321, and #336 noting they were still open after PR #2072. This PR queues its own newer, more complete requests for the same three rows (two "done" decisions, one updated "PARTIAL" status), which the ledger inbox tool correctly refuses to auto-reconcile as a silent overwrite — it requires an explicit cancel decision per row. Cancels the three now-superseded #2081 flags in favor of this PR's requests, fixing the docs:check-links / ledger-inbox failure in Static PR checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qDHwBNZYyRLdJHqSkiYTg
Summary
PR #2081 flagged four outstanding-issues rows (
#336,#222,#321,#322) as merged-but-unresolved or in-progress. This PR actually resolves the three that were an agent's to resolve, and deliberately leaves the fourth to a human:#336(named vs. raw responsive breakpoint tokens) — made the decision the row asked for: stay rawmin-[…]/max-[…]everywhere rather than adding named--breakpoint-*tokens (a Tailwind 4 named breakpoint would add both amin-widthand amax-widthvariant to every utility in the build for what is currently nine single-purpose consumers). Recorded indocs/design-system/GATES.md§3 (prohibition-table row) and new §3b with full rationale, covering all nine current call sites.#222(headers surface convergence) — decidedmode-home-template.tsx(the centred hero the in-flow phone composer sits beneath) andsearch-results-header-band.tsx(a status/count/filter spine, not a title stack) are permanently outside thePageHeadervocabulary, for the structural reasons the row itself laid out. Recorded asdocs/design-system/DECISIONS.md§C7, which also notes thatModeHomeStatusNoticein the same file already converged onto the DSEmptyStatevia PR docs(ui): record DS adoption evidence and overlay/links checks (221,233,234,235,238,245) #1842 — a separate, already-closed conversion.#321(four follow-up groups from#291) — fixed the one group that was genuinely mechanical:document-image-filmstrip.tsx's "Page unknown" chip used the nativedisabledattribute for a stated data-absence reason (no recorded page number), which is exactly the patterndocs/wiring-conventions.mdreserves foraria-disabled="true"+ignoreUnavailableActivation+ ansr-onlyreason, so a keyboard/screen-reader user can still reach and hear why. Added a focused DOM test. The other three groups (differential comparison page controls coupled to a planned rewrite, DocumentViewer's persistent-vs-transient classification split, the pin-limit capacity-state judgement) are unchanged — they are not single-PR-sized and stay open.#322(two catalogue records both named Warfarin) — deliberately not touched. The row's own text says this is a clinician's call (merge/delete/relabel the two records) and explicitly "not an agent's call"; PR feat(medications): reconcile Warfarin records and connect catalogue drugs into interaction graph (#322, #323) #2069 is already open and targets this exact row. Duplicating that work here would reproduce the failure mode#292already describes (two sessions building the same fix twice).Also queues the corresponding
issues:done/issues:updateinbox requests (docs/outstanding-issues-inbox/*.json) recording this outcome; canonical ledger edits happen later via a dedicatednpm run issues:reconcilebranch, per this repo's serial-write discipline.Verification
npm run verify:pr-local— full run, all steps green:check:runtime,check:installed-lock-parity,format:changed,sitemap:check,docs:check-index,docs:check-inventory,docs:check-scripts,docs:check-links,check:branch-review-ledger,check:outstanding-issues,check:ledger-write-discipline("Ledger write discipline passed for b8474c0..HEAD."),lint,typecheck,test("Test Files 649 passed (649) / Tests 6999 passed | 4 skipped (7003)"),build(exit 0),check:rag:fixtures("36 golden cases, 25 suites"),check:medication-interactions,check:medication-lexicon-report.npm run verify:ui— not run: the only behavioural change is a document-viewer chip's disabled-state encoding, covered by the new focused DOM test (tests/document-image-filmstrip.dom.test.tsx, 3 passed) per this repo's verification pyramid (Tier 2, focused behavioural).npm run verify:release— not applicable, not a release/handoff gate.npm run eval:retrieval:quality/eval:rag— not applicable, no retrieval/ranking/answer-generation behaviour changed (classifyPullRequestFileson the full changed-file list returnsragRanking: false).Risk and rollout
Notes
classifyPullRequestFileson the full diff returnsclinicalRisk: false,ragRanking: false— no Clinical Governance Preflight orRAG impact:line applies.#322remains open and is expected to close via PR feat(medications): reconcile Warfarin records and connect catalogue drugs into interaction graph (#322, #323) #2069 once its clinical-authorship decision is made, not via this PR.Generated by Claude Code