feat(navigation): roll the shared mode nav out to DSM, Specifiers, Formulation and Differentials

[BigSimmo]

Summary

Completes the rollout #1642 started for Therapy. The four multi-destination modes now use the shared header-integrated ModeNav instead of an in-flow strip or a page-owned subnav.

• RegistryModeNav adapter — maps modeSecondaryNavigationRegistry onto ModeNavItem so the shell and the mode cannot drift onto different URLs for the same tab. Its icon map is keyed by a type derived from the registry literal, so adding a routed entry without choosing an icon fails the typecheck rather than shipping a FileText fallback nobody picked. Action-only entries are dropped, not adapted — ModeNavItem takes an href by design.
• DSM adopts — two destinations, so no More slot and no overflow band to calibrate.
• Specifiers and Formulation adopt — SpecifierSubnav and FormulationSubnav are removed, along with the GlobalSearchShell exclusion that skipped PageSecondaryNavigation for both modes (it existed only to prevent a duplicate row next to those strips; with them gone it would have suppressed the modes' only navigation).
• Differentials adopts — last, because it is the only mode containing a route that portals its own header into the universal header's single addon slot.
• Record-page section anchors (7891fa7) — the declared specifier/formulation section target ids are now rendered on the record, reference and mechanism pages. See "A regression this PR introduced and fixed" below.
• Density spec drives every adopting mode — tests/ui-mode-nav-density.spec.ts previously drove only /therapy-compass/*.

Adoption is an explicit list (MODE_NAV_ADOPTED_MODES), not a "has two or more routed entries" derivation, so a registry edit cannot silently move a mode onto a different navigation surface. Its membership is pinned by test, not just the criterion — the criterion alone is satisfied by any subset. The eight single-destination modes keep their existing SecondaryNavigation strip untouched: dropping it is a deletion decision about their lone entry, not a port, and is deliberately out of scope.

RAG impact: no retrieval behaviour change — navigation and layout only.

Desktop labels shorten

Specifiers and Formulation showed richer wording via their own subnavs ("Build wording", "Build formulation", "Mechanism map"); they now show the registry's "Build" and "Map". ModeNavItem has no shortLabel by design — a slot shows its real word or folds into More. Lengthening the registry instead was the alternative, but "Build formulation" plus "Mechanism map" do not fit the calibrated 42rem band, and ledger #113 is explicit that the thresholds are measured sums plus ~8% CI font-metric headroom, not padding to be widened.

A regression this PR introduced and fixed

Removing the page-owned subnavs left /specifiers/<slug>, /formulation/<slug> and the specifier reference pages with breadcrumbs and nothing else. The intended fallback — the shared "On this page" sections nav — silently rendered nothing, because specifierSections/formulationSections declared ten target ids (specifier-overview, formulation-five-ps, and eight more) that no component rendered. AvailableInformationPageNavigation filtered every item out and returned null. The dead config predated this PR; the subnav was masking it.

Fixed by wiring those ids into the pages, with binding tests per page family and a render test asserting the bar actually appears on a specifier record. Raised in review by Devin.

An invariant that was never actually stated

The header addon slot holds one page-owned header. What enforced that was an accident: documents and differentials, the only modes with a claimant, had fewer than MODE_NAV_MIN_ITEMS destinations, so ModeNav rendered nothing. Differentials now has three.

I first read /differentials/diagnoses/<slug> as a live collision and added an explicit guard at the mode branch. Mutating it away failed no test — because page-secondary-navigation.tsx returns on locallyOwnedInformationNavigation well before the mode branch, so the guard was unreachable. The guard is gone. What replaced it is isHeaderAddonSlotOwnedRoute, naming the claimants, plus assertions that the two separately-maintained lists actually agree route for route, and the render-level outcome. Breaking the cover now fails three assertions. When a future claimant falls outside it, that is the signal the mode branch needs a guard of its own — and the test says so.

Verification

• npm run verify:pr-local — stops at check:installed-lock-parity on a pre-existing container mismatch (playwright: installed 1.62.0 does not match locked 1.62.1), unrelated to this diff. Every remaining stage was run individually on the current tip:
  • check:runtime — PASS: Node runtime 24.13.0 / PASS: npm runtime 11.17.0
  • format:changed — All matched files use Prettier code style!
  • lint — clean, --max-warnings 0
  • typecheck — clean
  • test — Test Files 514 passed (514) / Tests 5443 passed | 4 skipped (5447)
  • build — completed, Client bundle secret surface check passed.
  • check:rag:fixtures — Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites).
• npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed — the wrapper aborts at the same lock-parity check, so the affected specs were run directly against Chromium:
  • ui-mode-nav-density.spec.ts — 48 passed, including 28 new per-mode cases and two new folded-active cases
  • ui-specifiers.spec.ts, ui-formulation.spec.ts, ui-route-coverage.spec.ts — 24 passed
  • ui-phone-scroll.spec.ts, ui-phone-scroll-routes.spec.ts, ui-phone-scroll-page-owned.spec.ts, ui-chrome-scroll.spec.ts — 85 passed across two runs, including phone differential detail navigation uses the universal collapse owner
  • ui-accessibility.spec.ts, ui-universal-search.spec.ts — 33 passed

UI verification not run: the full verify:ui project set and non-Chromium browsers — this container has Chromium build 1194 where playwright 1.62.1 expects 1234. CI is the authoritative browser run.

New contract assertions were mutation-checked rather than assumed: removing map from the icon map fails the typecheck; dropping differentials from the density spec table fails the coverage pin; breaking the addon-slot cover fails three assertions; removing formulation from the adopted list fails the membership pin. The one assertion that failed its mutation check — the original explicit slot guard — was removed rather than kept as unreachable code.

Note on CI: several Production UI and PR required failures on earlier heads were GitHub Actions infrastructure (Failed to resolve action download info. Error: Service Unavailable / Bad Gateway), with no test executed, and one PR required was the CANCELLED with no failing job supersession path after a branch sync. Those are not diff failures.

Risk and rollout

• Risk: this is the only navigation these four modes have, so a regression strands pages rather than degrading them, and ModeNav is shared — a mistake in the component lands everywhere at once. The mode-level exposure is bounded by MODE_NAV_ADOPTED_MODES. The shared component is touched once, additively: activeId accepts null as an explicit "no current page", with undefined still deriving from the path, so Therapy's shipped behaviour is unchanged.
• Rollback: remove a mode's id from MODE_NAV_ADOPTED_MODES to return it to the in-flow strip. For Specifiers and Formulation that also needs the subnav-removal commit reverted, since their page-owned subnavs were deleted. Reverting the navigation commits restores the pre-feat(therapy): standardise every Therapy page on the shared mode nav, centred on desktop #1642 state for these modes with Therapy untouched.
• Provider or production effects: None.

Notes

• Mode homes stay bar-free (invariant I3). /dsm, /specifiers, /formulation and /differentials already surface their destinations as tiles, so nothing is stranded by the removal.
• Specifier and formulation record routes keep the information-page sections nav, matching DSM diagnosis records; their Compare and Build actions remain on the pages themselves. Find and Map stay reachable from mode homes and workflow routes, where the bar does mount.
• PR feat(navigation): standardise multi-page mode menus in the header #1645 is an earlier attempt at the same work and is untouched by this branch. Its adapter shape informed this one; its three defects (mode homes moved into the nav for five modes, the addon-slot comment rewritten without a test, and the length < 2 early return that silently deletes the submitted-search nav for the eight single-destination modes) are not carried over. Closing it is the maintainer's call.
• Therapy is deliberately left alone. Its registry entry stays inert and useTherapyNavItems remains its live list.
• Known follow-up, not addressed here: other modes declare section target ids that appear unrendered (form-overview, differential-presentation-*). Pre-existing and unchanged by this PR; worth a separate audit.

Summary by CodeRabbit

• New Features

  • Added shared secondary navigation for Specifiers and Formulation pages.
  • Added consistent “On this page” links for key record sections.
  • Improved navigation across supported modes, including query-parameter preservation and clearer active-page handling.
  • Prevented duplicate navigation bars on pages with their own header navigation.

• Bug Fixes

  • Unmatched routes no longer incorrectly highlight a default navigation item.
  • Improved navigation behavior during pending page transitions.

• Tests

  • Expanded coverage for navigation ownership, active states, page anchors, density, and duplicate-bar prevention.

[supabase]

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 ↗︎.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Specifier and Formulation pages now use shared registry-driven navigation. Local subnavigation was removed, section anchors were added, and route ownership plus active-state handling were expanded. Tests cover navigation ownership, registry coverage, page targets, and responsive density.

Changes

Secondary navigation migration

Layer / File(s)	Summary
Navigation contracts and route resolution src/components/mode-nav/*, src/lib/mode-secondary-navigation.ts	Added route ownership detection, adopted-mode metadata, routed destination counts, and explicit active-item suppression.
Registry navigation integration src/components/mode-nav/registry-mode-nav.tsx, src/components/page-secondary-navigation.tsx, src/components/clinical-dashboard/global-search-shell.tsx	Added registry-based mode navigation and rendered it for adopted modes while preserving local navigation guards and pending-navigation suppression.
Specifier and Formulation page migration src/components/specifiers/*, src/components/formulation/*	Removed local subnavigation, rendered breadcrumbs directly, and added stable section IDs for shared in-page navigation.
Navigation and density validation tests/mode-nav-addon-slot.dom.test.tsx, tests/mode-nav-contract.test.ts, tests/mode-secondary-navigation.test.ts, tests/page-secondary-navigation.dom.test.tsx, tests/ui-mode-nav-density.spec.ts, tests/ui-formulation.spec.ts, tests/ui-specifiers.spec.ts	Added coverage for route ownership, active destinations, adopted modes, page targets, registry counts, responsive density, and focused section-label assertions. Updated review ledger records and outstanding issue entries.

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

Sequence Diagram(s)

sequenceDiagram
participant Page
participant PageSecondaryNavigation
participant RegistryModeNav
participant ModeNav
Page->>PageSecondaryNavigation: provide mode and pathname
PageSecondaryNavigation->>RegistryModeNav: select adopted-mode navigation
RegistryModeNav->>ModeNav: pass registry items and activeId
ModeNav-->>PageSecondaryNavigation: render shared navigation

Loading

Possibly related PRs

• BigSimmo/Database#1645: Builds the shared registry-backed header navigation extended by this PR.
• BigSimmo/Database#1642: Modifies shared ModeNav and secondary-navigation active-route handling.
• BigSimmo/Database#1607: Modifies secondary-navigation visibility during pending mode navigation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.93% 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 rollout of shared mode navigation to the four affected modes.
Description check	✅ Passed	The description covers the change, verification results and limitations, risk, rollback, production effects, and relevant notes.
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/new-session-6fz57i

---

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

[github-actions]

CI triage

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

• Visual baselines (advisory) — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
• Production UI (1) — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
• Production UI (2) — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
• PR required — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #8427 (cancelled).

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

[BigSimmo]

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

[Copilot]

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply t...

Implemented the requested review-thread fix in 9563f13: tests/mode-nav-addon-slot.dom.test.tsx now has a suite-level afterEach that always removes leftover #phone-header-collapse-addon-slot nodes to prevent slot leakage between tests when a case exits early.

[BigSimmo]

Review disposition (follow-up on Devin + pasted findings)

Addressed in 7891fa79 / tip 8124ddf2:

1. Empty On-this-page bar on specifier/formulation records — fixed. Wired the declared section target IDs into the record/reference/mechanism pages and added binding + render tests so a definition without a DOM target fails closed.
2. MODE_NAV_ADOPTED_MODES comment vs missing test — fixed. tests/mode-secondary-navigation.test.ts now asserts every adopted mode has ≥2 routed entries (and single-routed modes stay off the list; Therapy remains the documented exception).
3. activeId falling back to Find/Search on unmatched routes — fixed. activeModeSecondaryNavigationId returns null for record/unknown paths; ModeNav treats null as explicit no-aria-current.
4. isHeaderAddonSlotOwnedRoute has no production caller — intentionally kept. It is the named claimant set that tests/mode-nav-addon-slot.dom.test.tsx asserts against the hasLocalInformationPageNavigation early return; shipping it under src/ is the contract surface, not dead code.
5. Band arithmetic for the new modes — agreed; no change.

Not changing: putting header ModeNav back onto specifier/formulation record routes. The PR deliberately replaces the old Subnav with the shared information-page sections nav on those routes (same pattern as DSM diagnosis records). Restoring ModeNav there would reverse that product choice; workflow CTAs (Compare / Build) remain on the pages themselves, and Find/Map stay reachable from mode homes and workflow routes where ModeNav does mount.

[coderabbitai]

Actionable comments posted: 1

🤖 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 675-684: The branch-review ledger must remain append-only: restore
the existing temp-rebase, PR `#1649`, and PR `#1641` rows to their prior order, then
add only the new PR `#1647` entry using the repository ledger command. Do not
hand-edit, delete, or reorder existing rows, and run post-main deduplication
only when the documented workflow requires it.

🪄 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: 684db5e7-c781-4e88-a642-dafecfbff7e7

📥 Commits

Reviewing files that changed from the base of the PR and between 9563f13 and 3ee1f17.

📒 Files selected for processing (7)

• docs/branch-review-ledger.md
• docs/outstanding-issues.md
• src/components/clinical-dashboard/global-search-shell.tsx
• src/lib/mode-secondary-navigation.ts
• tests/mode-secondary-navigation.test.ts
• tests/ui-formulation.spec.ts
• tests/ui-specifiers.spec.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• src/components/clinical-dashboard/global-search-shell.tsx
• tests/mode-secondary-navigation.test.ts
• src/lib/mode-secondary-navigation.ts