Skip to content

feat(in-page-nav): convert medications and factsheets, close the series (PR 3 of 3) - #1781

Merged
BigSimmo merged 10 commits into
mainfrom
claude/in-page-nav-pr-3-i6gi8n
Aug 9, 2026
Merged

feat(in-page-nav): convert medications and factsheets, close the series (PR 3 of 3)#1781
BigSimmo merged 10 commits into
mainfrom
claude/in-page-nav-pr-3-i6gi8n

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Final PR of the in-page navigation series. PR 1 (#1740) extracted the template into src/components/in-page-nav/ and converted differentials; PR 2 (#1766) converted the six information routes and deleted the shell-owned pill rail. Three routes remained — this lands two, and records why the third is not a candidate.

  • /medications/[slug] — converted (panel-swap). The owner's call: keep the tab swap, drive it from the header's segment track, exactly as differentials/differential-detail-page.tsx does. SectionTabs (a 58-line roving-tabindex tablist) is deleted, activeTab lifts to MedicationRecordPage so the header above the shell can drive it, and the InformationPageBreadcrumbs row goes with it. The tab→section-type grouping moves into the new medication-nav-header.tsx sibling so the segment weights and the rendered panel cannot disagree about what a tab contains. Weights are explicit, because only the active panel is ever mounted and measurement would report one full-width segment beside three empty ones. The panel keeps a per-tab id but drops role="tabpanel"/aria-labelledby: the control is now a list of buttons, so claiming the role would name a tab that no longer exists.

  • /factsheets/[slug] — converted (anchor-scrolling). It already mounted InPageNavHeader in the breadcrumb shape; this gives it a real section index across all five kinds. tocFor is deleted rather than ported — it returned display strings with no anchors behind them, painted into an inert <li> "On this page" list, and was wrong in both directions (it named "What is this medicine?" where the page renders "What is <title>?", and never listed the Sources, More-in-topic or Related sections every sheet renders). The replacement derives sections from what each kind actually renders, with medLite's headings coming from the record.

  • /differentials/presentations/[slug] — recorded exception, not converted. The premise did not survive reading the file. It was carried forward as "a SectionTabs page that swaps panels"; it swaps nothing — MobileTabs is four <Link>s to other routes with "Compare" hardcoded active, which is the multi-route ModeNav pattern docs/search-chrome-behaviour.md already carves out. Its candidate sections are also rendered two or three times per breakpoint in different DOM parents (an xl <aside>, an mdlg grid, and a phone copy nested inside another section's anchor), which PageSection.targetIds does not model. Reasoned, lasting exception recorded in docs/search-chrome-behaviour.md alongside the DocumentViewer one.

  • Deleted the orphaned SecondaryNavigation component (/issues #271). Test-only since PR 2 removed the section kind. The row's two stated side-conditions turned out not to exist: nothing outside its own test imports it, and tests/mode-nav-contract.test.ts string-matches page-secondary-navigation.tsx, a different file that is untouched here. Its test fixture was literally the medication tab bar (["summary","dosing","safety","more"], labelled "Medication sections"), so it is the tail of this same thread rather than an unrelated chore, and it carries no governance statement of its own.

  • Guards. tests/in-page-nav-route-sections.dom.test.tsx grows from 7 routes to 12 (one factsheet case per kind) and gains a panel-swap suite for medications, so both halves of /issues #256's stop rule hold — declared ids proven against rendered DOM, never a source grep. A new absent field asserts the therapy and procedure sheets genuinely do not render More-in-topic rather than skipping the check. Both new routes are registered in isHeaderAddonSlotOwnedRoute and in the claimant enumeration in tests/mode-nav-addon-slot.dom.test.tsx.

  • Docs. docs/search-chrome-behaviour.md gains a "Panel-swap adopters" section (explicit weights, no inPageAnchor, no useResolvedPageSections, never collapsible) and the presentations exception; the adoption list and docs/codebase-index.md are updated. Two now-stale claims in in-page-nav-header.tsx's docstring are corrected: DocumentViewer's non-adoption is settled rather than "a separate change", and the breadcrumb-shape page count no longer says "eight".

Both new section tables live in colocated "use client" nav-header siblings that own and export them, per the rule pinned in docs/search-chrome-behaviour.md — not inline in the page, and not in a separate section-index module (the shape closed PR #1767 proposed).

Verification

  • npm run verify:pr-local — every stage green except one pre-existing, environment-only failure (below). docs:check-links caught a real defect on the first run (my codebase-index.md note still cited the deleted path); fixed and re-run.

    • [installed-lock-parity] next 16.2.12 / react 19.2.8 / playwright 1.62.1 / typescript 6.0.3 / vitest 4.1.10
    • docs/codebase-index.md coverage OK: all 50 repository roots/modules/routes and all schema tables are indexed.
    • docs link check — 1717 references checked, 0 missing (after the fix)
    • lint, typecheck — completed, no findings
    • npm run testTest Files 1 failed | 547 passed (548) / Tests 1 failed | 5932 passed | 4 skipped (5937)
    • npm run build — completed; Client bundle secret surface check passed.
    • npm run check:rag:fixturesOffline RAG fixture and manifest validation passed (36 golden cases, 23 suites).
    • npm run check:bundle-budgetproduction (what users download, 101 routes): 1281.5 KiB gzip — baseline 1279.1 KiB, within tolerance.

    The one failure is pre-existing and cannot be caused by this diff. tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists chmod 0o555s a git dir to force a marker write to fail, then asserts the marker is absent. This container runs as uid 0, and root ignores permission bits — verified directly: creating a 0555 dir and touching a file inside it succeeds here. git diff --stat origin/main -- tests/pr-handoff-stop.test.ts .claude/ scripts/ is empty, so the test and the hook it exercises are byte-identical to main.

  • Per-route DOM proof (/issues #256 stop rule) — npx vitest run tests/in-page-nav-route-sections.dom.test.tsx: Test Files 1 passed (1) / Tests 29 passed (29)

  • Directly affected suites — mode-nav-addon-slot, factsheets-data, factsheet-detail-header, medication-record-page, factsheet-save: Test Files 5 passed (5) / Tests 30 passed (30)

  • npm run verify:phone-chrome — 3 of 4 stages green with output; the 4th could not run (see below).

    • [phone-chrome:lock-parity] and [phone-chrome:runtime]PASS: Node runtime 24.19.0 / PASS: npm runtime 11.17.0
    • [phone-chrome:contracts]Test Files 9 passed (9) / Tests 123 passed (123) (includes header-scroll-hide-contract, mobile-composer-reserve, ui-overlay-css-contract)
    • [phone-chrome:focused-browser]could not run. Playwright browser-revision drift, /issues #255: the image ships chromium-1194 / chromium_headless_shell-1194 in /opt/pw-browsers, while the pinned Playwright 1.62.1 requires revision 1234. docs/testing.md says not to force a mismatched Chromium path, and this environment forbids playwright install, so browser proof is delegated to CI Production UI. The selector had chosen tests/ui-phone-scroll-page-owned.spec.ts --project=chromium with 4 journeys.
  • UI verification not run: Playwright browser-revision drift (/issues #255) — the container's Chromium is revision 1194 against the pinned 1234, and forcing a mismatched binary is explicitly disallowed by docs/testing.md. Browser proof is delegated to CI Production UI. In its place, both converted routes were exercised against the real dev server (npm run ensurehttp://localhost:4598, identity confirmed via /api/local-project-id):

    • /medications/sertraline → 200, renders medication-detail-header, medication-section-trigger, id="medication-panel-summary"
    • /factsheets/sertraline (medRich) → 200, renders all five body anchors plus factsheet-sources, factsheet-more-in-topic, factsheet-related
    • /factsheets/ssri (medLite) → renders factsheet-timing, factsheet-section-0/1/2 (the record-derived headings)
    • /factsheets/cbt (therapy) and /factsheets/lithium-monitoring (procedure) → render their own body anchors and, correctly, no factsheet-more-in-topic — the absent case, confirmed in the running app as well as in jsdom
    • Control: the already-shipped /services/community-team and /formulation/rumination also render 0 section triggers in SSR, so factsheets matching them is useResolvedPageSections filling in on mount, not a regression
  • Not run, with reason: verify:release, eval:*, check:supabase-project, test:live — provider-backed and out of scope; no src/lib/rag/**, retrieval, ingestion, auth or privacy path is touched. check:production-readiness — no clinical workflow, privacy, environment, Supabase, source-governance or deployment behaviour changed.

Risk and rollout

  • Risk: Low, and UI-local. The largest behavioural change is that the medication record page's tab control moves from an in-body tablist into the shared header, which changes the keyboard model for that one control (arrow-key roving tabindex → the header's section sheet, matching the other nine adopters). Factsheets gain navigation they did not have; nothing they had is removed except an inert list. The SecondaryNavigation deletion removes code with no production caller.
  • Rollback: git revert the single commit. The two new modules are additive and have no importers outside the two pages and the contract test.
  • Provider or production effects: None.

Notes

  • Classification confirmed by running scripts/pr-policy.mjs's own classifyPullRequestFiles / evaluatePullRequestPolicy against the exact changed-file list rather than assuming: clinicalRisk: false, operationalRisk: false, ragRanking: false, ui: true, errors: [], ok: true. No RAG impact: line and no Clinical Governance Preflight are required.
  • The usePathname mock trap that produced PR 2's CI failure was re-scanned after conversion: eight test files mock next/navigation without usePathname, and none of them renders a route converted here.
  • docs/outstanding-issues.md is deliberately untouched. Open PR feat(design-system): ratchet raw scale literals and gate type-step selection (#262 parts 2 and 3) #1780 modifies it, and the file has no merge driver by design, so /issues #271's row is left for a session that owns the file. Its disposition is recorded here and in docs/codebase-index.md instead.
  • Gate B of the handover (the aria-disabled a11y pass) had already landed on main, so no placeholder buttons were avoided or skipped on that account.

Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added medication page navigation with Summary, Dosing, Safety, and More tabs.
    • Added medication section rails with counts and responsive navigation.
    • Added patient details access through a dedicated sheet.
    • Added factsheet-specific section navigation with stable anchors.
    • Improved factsheet actions, including download and reading-level controls.
  • Enhancements

    • Improved responsive action menus and mobile view-mode access.
    • Added clearer grouped actions and improved navigation accessibility.

…es (PR 3 of 3)

Completes the in-page navigation series. PR 1 (#1740) extracted the template
into src/components/in-page-nav/ and converted differentials; PR 2 (#1766)
converted the six information routes and deleted the shell-owned pill rail.
Three routes were left; this lands two of them and records why the third is
not a candidate.

Medications (/medications/[slug]) — converted, panel-swap
The owner's call: keep the tab swap and drive it from the header's segment
track, exactly as differentials/differential-detail-page.tsx does. SectionTabs
(a 58-line roving-tabindex tablist) is deleted; activeTab lifts to
MedicationRecordPage so the header above the shell can drive it, and the
InformationPageBreadcrumbs row goes with it. The tab->section-type grouping
moves into the new medication-nav-header.tsx sibling so the segment weights and
the rendered panel cannot disagree about what a tab contains. The panel keeps a
per-tab id but drops role="tabpanel"/aria-labelledby: the control is now a list
of buttons, so claiming the role would name a tab that no longer exists.

Factsheets (/factsheets/[slug]) — converted, anchor-scrolling
Already mounted InPageNavHeader in its breadcrumb shape; this gives it a real
section index. tocFor is deleted rather than ported: it returned display strings
with no anchors behind them, painted into an inert <li> "On this page" list, and
was wrong in both directions (it named "What is this medicine?" where the page
renders "What is <title>?", and never listed the Sources, More-in-topic or
Related sections every sheet renders). The replacement derives sections from
what each of the five kinds actually renders, with medLite's headings coming
from the record.

Differentials presentations — recorded exception, not converted
The premise did not survive reading the file. It was carried as "a SectionTabs
page that swaps panels"; it swaps nothing — MobileTabs is four <Link>s to other
routes with "Compare" hardcoded active, which is the multi-route ModeNav pattern
the template already carves out. Its candidate sections are also rendered two or
three times per breakpoint in different DOM parents, one nested inside another
section's anchor, which PageSection.targetIds does not model. Reasoned exception
recorded in docs/search-chrome-behaviour.md.

Also: delete the orphaned SecondaryNavigation component (/issues #271)
Test-only since PR 2 removed the section kind. Its two stated side-conditions
turned out not to exist — nothing outside its own test imports it, and
tests/mode-nav-contract.test.ts string-matches page-secondary-navigation.tsx, a
different file. Its test fixture was literally the medication tab bar, so it
lands with the conversion that retired it.

Guards
tests/in-page-nav-route-sections.dom.test.tsx grows from 7 routes to 12 (one
factsheet case per kind) and gains a panel-swap suite for medications, so both
halves of /issues #256's stop rule hold: declared ids are proven against
rendered DOM, never a source grep. A new `absent` field asserts the therapy and
procedure sheets genuinely do not render More-in-topic rather than skipping it.
Both new routes are registered in isHeaderAddonSlotOwnedRoute and the claimant
enumeration.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q
@supabase

supabase Bot commented Aug 9, 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 9, 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: 54 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: 9605b599-7906-4e6b-810d-caa985b7ec2a

📥 Commits

Reviewing files that changed from the base of the PR and between 2a28b57 and 5910a19.

📒 Files selected for processing (6)
  • src/components/in-page-nav/in-page-nav-header.tsx
  • tests/in-page-nav-header.dom.test.tsx
  • tests/in-page-nav-playwright-contract.test.ts
  • tests/playwright-settlement-contract.test.ts
  • tests/ui-forms-section-nav.spec.ts
  • tests/ui-smoke.spec.ts
📝 Walkthrough

Walkthrough

Factsheet and medication detail pages now use route-specific navigation headers. Shared navigation adds responsive rails, weighted sections, action groups, and mobile mode controls. DOM tests cover anchors, panel switching, route ownership, and patient-detail sheets.

Changes

Shared navigation infrastructure

Layer / File(s) Summary
Responsive navigation infrastructure
src/components/in-page-nav/..., src/components/design-system/...
InPageNavHeader now supports section rails, numeric counts, icon-only primary actions, joined action groups, and responsive view-mode controls.
Factsheet navigation flow
src/components/factsheets/..., tests/factsheet-detail-header.dom.test.tsx, tests/factsheets-data.test.ts
Factsheets use FactsheetNavHeader with stable anchors for all supported factsheet variants. The old tocFor helper and static sidebar list were removed.
Medication navigation flow
src/components/clinical-dashboard/..., docs/design-system/adoption-manifest.json
Medication pages use typed tabs, weighted sections, a desktop rail, lifted tab state, and a controlled patient-details sheet.
Route ownership and validation
src/components/mode-nav/header-addon-slot.ts, tests/in-page-nav-route-sections.dom.test.tsx, tests/mode-nav-addon-slot.dom.test.tsx, tests/medication-record-page.dom.test.tsx, tests/ui-smoke.spec.ts
Tests cover factsheet anchors, absent sections, medication panel swaps, rail state, route ownership, patient-sheet placement, and the updated medication link.
Adoption documentation and cleanup
docs/search-chrome-behaviour.md, docs/codebase-index.md, docs/branch-review-ledger.md, docs/design-system/COMPONENTS.md
Documentation records navigation adoption, panel-swapping rules, ownership conventions, review decisions, and updated component import counts.

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

Possibly related PRs

Suggested labels: codex

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% 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 in-page navigation conversion for medications and factsheets and indicates this is the final series PR.
Description check ✅ Passed The description includes a detailed summary, verification results, risk and rollback details, production effects, and notes explaining why governance checks do not apply.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/in-page-nav-pr-3-i6gi8n

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

  • Production UI (2)needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • Production UI (1)main-side: the same job also failed on the latest completed main CI run.
  • PR requiredmain-side: the same job also failed on the latest completed main CI run.

Compared with main CI run #9522 (failure).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

claude added 2 commits August 9, 2026 11:28
…phone reading level

Design follow-up on the PR-3 conversions, from review of the shipped phone
headers.

Medications: the two-rail header
`InPageNavHeader` gains an optional `rail={{ label }}` that swaps the thin
weighted track for `InPageSectionRail` — every section named in a visible row
with its icon, label and a count badge, active one underlined. Medications is
the only adopter and the prop exists so it stays that way by choice rather than
drift: factsheets' eight anchored sections would overflow the row this is meant
to simplify.

The rail changes the row above it. From `sm`, where the whole rail fits, the
title stops being a disclosure — the chevron would open a list of the same
destinations — and renders as plain text. Below `sm` the rail scrolls and the
disclosure returns as its overflow, which is the two-rail shape. The rail is
deliberately not a `role="tablist"`: the same sections are reachable from the
sheet on a phone, so a roving-tabindex group would put half the destinations
behind arrow keys and half behind Tab.

`PageSection.count` is a new field, separate from `detail`. The sheet row has
room for "3 sections" and a badge does not, and parsing the digits back out of
the prose would break the first time a route worded its detail differently.

Patient details move into the header
Both body cards — `PatientProfilePanel` and `MedicationConsiderations` — are
removed from the page body and now open from a patients control in rail one.
They are a per-patient overlay on a reference record rather than part of the
record, and inline they pushed every section below a permanently-empty prompt.
The feature is relocated, not retired.

Factsheets: the phone band is gone
`mode` now renders inside the actions sheet below `sm` instead of claiming a
full-width band under the row — it was the only second phone row on any
converted page. Both copies are always in the DOM with CSS choosing one per
breakpoint, so there is no state to keep in step. `mode` consequently requires
`actions`: with no sheet to move into, a phone would have no way to reach it.

One joined control group
`primaryAction` and `actions` now render inside a single bordered, clipped group
with a hairline between them, replacing a bordered promoted action sitting
beside a borderless ellipsis. `primaryActionIconOnly` drops the label at every
width for a glyph that carries its own meaning.

Two things found while building, both real
The rail first used `focus-ring-tab`, which sets `border-radius` on all four
corners and rendered the 2px active underline as a detached pill; grouped
buttons had the same utility and painted a second corner against the group's
clipped edge. Both now carry explicit focus-visible outlines and no radius. The
rail also briefly used `min-h-11` with a comment rationalising it — that is the
exact substitution AGENTS.md forbids because it reintroduces a known `ui-smoke`
sub-pixel flake. It is `min-h-12` like every other production tap target.

Guards
Rail contents and counts, rail-driven panel swap, the joined group's membership,
the sheet-hosted reading level driving body copy, and the patient panels being
absent from the body but present in the sheet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q
@BigSimmo
BigSimmo requested a balanced review from Copilot August 9, 2026 11:30
@BigSimmo
BigSimmo enabled auto-merge (squash) August 9, 2026 11:33
@BigSimmo
BigSimmo disabled auto-merge August 9, 2026 11:33

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

Converts the remaining eligible medication and factsheet detail routes to the shared in-page navigation pattern, completing the three-PR series.

Changes:

  • Adds panel-swap navigation for medications and anchor navigation for all factsheet kinds.
  • Deletes the unused SecondaryNavigation component and tests.
  • Expands route contracts and documents adoption rules and exceptions.

Browser verification remains delegated to CI due the reported Playwright revision mismatch.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/components/clinical-dashboard/medication-nav-header.tsx Adds medication panel navigation.
src/components/clinical-dashboard/medication-record-page.tsx Moves tab control into the shared header.
src/components/factsheets/factsheet-nav-header.tsx Defines factsheet sections and header wiring.
src/components/factsheets/factsheet-detail-page.tsx Adds section anchors and shared navigation.
src/components/factsheets/factsheets-data.ts Removes the inert table-of-contents projection.
src/components/in-page-nav/in-page-nav-header.tsx Updates component documentation.
src/components/mode-nav/header-addon-slot.ts Registers both new header owners.
src/components/secondary-navigation.tsx Deletes the orphaned component.
tests/in-page-nav-route-sections.dom.test.tsx Covers factsheet anchors and medication panels.
tests/mode-nav-addon-slot.dom.test.tsx Updates slot ownership contracts.
tests/factsheets-data.test.ts Removes obsolete TOC assertions.
tests/secondary-navigation.dom.test.tsx Removes tests for the deleted component.
docs/search-chrome-behaviour.md Documents panel swapping and the presentations exception.
docs/codebase-index.md Updates the navigation architecture map.
docs/branch-review-ledger.md Records the branch review entry.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

cursoragent and others added 3 commits August 9, 2026 11:39
GitHub marked the branch CONFLICTING while merge-tree was clean; sync clears stale mergeability.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
`in-page-nav-header.tsx` no longer imports `Button` (the promoted action is now
a plain button inside the joined control group) and `medication-record-page.tsx`
gained `Sheet` for the relocated patient panels. The manifest is generated, so
this is the recorded consequence of both, not a hand edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GHmzgER6rv8Va9HbQZw87Q
@BigSimmo
BigSimmo enabled auto-merge August 9, 2026 11:40

@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: 1

🧹 Nitpick comments (2)
src/components/in-page-nav/in-page-section-rail.tsx (1)

95-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the rail badge size to a theme token.

h-[1.125rem] and min-w-[1.125rem] introduce component-local design values. Define or reuse a Tailwind theme token in src/app/globals.css, then use that token here.

As per coding guidelines, “Use Tailwind 4 @theme tokens in src/app/globals.css … rather than introducing hardcoded design values.”

🤖 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/in-page-nav/in-page-section-rail.tsx` around lines 95 - 107,
Replace the hardcoded 1.125rem height and minimum width in the section count
badge within the in-page section rail with a shared Tailwind 4 theme token.
Define or reuse the appropriate token in globals.css, then reference it through
the badge’s Tailwind classes while preserving the existing layout and styling.

Source: Coding guidelines

src/components/in-page-nav/in-page-nav-header.tsx (1)

49-81: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Require actions when mode is provided.

InPageNavHeaderProps accepts mode without actions. Below sm, the inline control is hidden and the mode control exists only inside the actions sheet. Without actions, users cannot change the mode. Encode this dependency in the prop type, or render a mode-only sheet.

🤖 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/in-page-nav/in-page-nav-header.tsx` around lines 49 - 81,
Update InPageNavHeaderProps so mode is only permitted when actions is also
provided, using a discriminated union or equivalent type-level dependency;
preserve the existing independent props for action-only and
neither-mode-nor-actions usage.

Source: Coding guidelines

🤖 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/branch-review-ledger.md`:
- Around line 840-846: Restore the original ordering of existing rows in the
branch-review ledger, leaving their text unchanged. Add only the new record
using the established ledger command rather than editing
docs/branch-review-ledger.md directly, and preserve its append-only merge-driver
configuration.

---

Nitpick comments:
In `@src/components/in-page-nav/in-page-nav-header.tsx`:
- Around line 49-81: Update InPageNavHeaderProps so mode is only permitted when
actions is also provided, using a discriminated union or equivalent type-level
dependency; preserve the existing independent props for action-only and
neither-mode-nor-actions usage.

In `@src/components/in-page-nav/in-page-section-rail.tsx`:
- Around line 95-107: Replace the hardcoded 1.125rem height and minimum width in
the section count badge within the in-page section rail with a shared Tailwind 4
theme token. Define or reuse the appropriate token in globals.css, then
reference it through the badge’s Tailwind classes while preserving the existing
layout and styling.
🪄 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: ac977374-dd6b-4a78-9a01-9c0c95d91b35

📥 Commits

Reviewing files that changed from the base of the PR and between 122cddf and 2a28b57.

📒 Files selected for processing (22)
  • docs/branch-review-ledger.md
  • docs/codebase-index.md
  • docs/design-system/COMPONENTS.md
  • docs/design-system/adoption-manifest.json
  • docs/search-chrome-behaviour.md
  • src/components/clinical-dashboard/medication-nav-header.tsx
  • src/components/clinical-dashboard/medication-record-page.tsx
  • src/components/factsheets/factsheet-detail-page.tsx
  • src/components/factsheets/factsheet-nav-header.tsx
  • src/components/factsheets/factsheets-data.ts
  • src/components/in-page-nav/in-page-nav-header.tsx
  • src/components/in-page-nav/in-page-section-rail.tsx
  • src/components/in-page-nav/page-section-index.ts
  • src/components/mode-nav/header-addon-slot.ts
  • src/components/secondary-navigation.tsx
  • tests/factsheet-detail-header.dom.test.tsx
  • tests/factsheets-data.test.ts
  • tests/in-page-nav-route-sections.dom.test.tsx
  • tests/medication-record-page.dom.test.tsx
  • tests/mode-nav-addon-slot.dom.test.tsx
  • tests/secondary-navigation.dom.test.tsx
  • tests/ui-smoke.spec.ts
💤 Files with no reviewable changes (2)
  • tests/secondary-navigation.dom.test.tsx
  • src/components/secondary-navigation.tsx

Comment thread docs/branch-review-ledger.md
Complete the prescribing smoke back-link assertion (aria-label form on
phone too), floor the icon-only phone back control with min-w-tap, and
scope form-detail-header through visibleByTestId for #93 duplicates.
Add offline contracts so a half-patched smoke or bare testid cannot
regress the same red required CI.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Unblock summary — PR 1781

Tip: 5910a195de340af14282998cfda57221775074c1 on claude/in-page-nav-pr-3-i6gi8n
Sync / merge-tree: behind origin/main by 0; git merge-tree --write-tree clean; GitHub MERGEABLE (mergeability not the blocker). Auto-merge was already armed — pushed only because required Production UI was red.

What blocked required CI

  1. Production UI (2)prescribing workflow shows full mobile action text… looked for link name "Medications" after the InPageNavHeader conversion. The control’s accessible name is always Back to medications (aria-label); the visible label is hidden sm:inline. Autofix Align prescribing smoke test with back-link accessible name #1787 patched only the desktop critical test, so phone stayed red. After the name fix, the same control failed expectMinTouchTarget at 40px wide (icon-only, no min-w-tap).
  2. Production UI (1)ui-forms-section-nav bare getByTestId('form-detail-header') hit Playwright strict mode with 2 matches under full-suite load (ledger chore(deps): bump actions/checkout from 4 to 7 #93 streaming / phone-portal twin). Forms product code was not in this PR’s diff; the locator was the flake surface.

What I fixed (pushed)

  • Phone + desktop prescribing smoke → Back to medications + visible filter.
  • InPageNavHeader back control → min-w-tap (+ phone padding) so icon-only width meets the tap floor.
  • Forms section-nav → visibleByTestId('form-detail-header').
  • Offline contracts: tests/in-page-nav-playwright-contract.test.ts + settlement-contract coverage so a half-patched smoke / bare testid cannot regress the same red.

Proof (local)

  • Vitest: 31 passed (in-page-nav-header + both contracts)
  • Playwright: forms breakpoint variant 1 passed; prescribing smoke 2 passed
  • No provider-backed gates

Required CI now

Change scope QUEUED on the new tip; other required jobs should re-run. Merge left to you.

Threads / residual

  • CodeRabbit ledger-reorder note: dispositioned as non-blocking (merge=ledger / prior main sync churn). Not a required-CI failure.
  • Ledger row for this unblock recorded locally for 5910a195 under scope PR 1781 unblock and not pushed (avoid ledger-only tip cancelling CI).
  • Residual risk: full Production UI suite still needs the in-flight CI run to finish green; only the two failing owners were re-proven locally.

@BigSimmo
BigSimmo merged commit 441a2df into main Aug 9, 2026
26 checks passed
@BigSimmo
BigSimmo deleted the claude/in-page-nav-pr-3-i6gi8n branch August 9, 2026 13:41
cursor Bot pushed a commit that referenced this pull request Aug 10, 2026
Production UI (2) timed out waiting for Breadcrumb/Medications exact
inside medication-page-*; MedicationNavHeader exposes
aria-label "Back to medications" outside that test id. Align with the
in-page-nav Playwright contract from #1781 and guard the quick-links
block against the old selector.
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.

5 participants