feat(differentials): adopt the default in-page navigation template - #1715
Conversation
PR #1688 codified the DocumentViewer header as the default in-page navigation template for every mode page, but deliberately left Differential detail as a legacy special case. This migrates it. The detail page owned the phone collapse slot and put almost nothing in it: a back chevron on the left, a `+` on the right, and empty space between. The real navigation was a separate in-flow tab strip that scrolled away with the content and clipped "Source" off the right edge at phone width. The header now carries the four template slots: back control, title with the active section on line two behind a chevron disclosure, ellipsis page actions, and a weighted segment track pinned to the bottom edge. The labelled strip is retained from `sm` up; phones navigate from the header disclosure and its sheet, so there is nothing left to clip at 320px. - New `detail-section-index.ts` builds the five tabs as `DocumentSection`s so `DocumentSectionTrack` and `DocumentSectionList` render them unchanged. Weights derive from real panel content — review-section count, related count, overlap links — normalised so the track reads as proportion rather than "which of five". - No scroll spy: these are discrete panels, not scroll positions, so the active id is just the active tab. - Both sheets are siblings of `PhoneHeaderCollapsePortal`, never children, so they are not carried away when the shared chrome scroll-hides. - The header moves from `max-sm:static` to `relative`, matching DocumentViewer. The track is absolutely positioned against the header, and a static phone header would let it escape to another ancestor. - The tab panel is now named by `aria-label` rather than `aria-labelledby`: below `sm` the strip is `display:none`, and assistive tech ignores a hidden labelling element. - "New differentials search" moves from the bare `+` into the actions sheet, alongside Save, Copy and Compare. The regenerated adoption manifest drops `tests/ui-tools.spec.ts` from the design-system `Tabs` component's `testFiles`. That association came from a capital-T word in a comment this change rewrote; the differentials strip is a local `function Tabs`, not the design-system component, so the manifest is now more accurate rather than less. Verification: typecheck clean; lint clean at --max-warnings 0; unit suite 5556 passed with the one pre-existing pr-handoff-stop failure confirmed by stashed re-run on the clean base; verify:phone-chrome green through lock-parity, runtime, 119 contract tests, changed-browser ui-tools and focused-browser 7 passed; live browser proof shows zero horizontal overflow at 320/390/768px and the same collapse behaviour as DocumentViewer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnXJpp8BpZSYychgi18EnN
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:44 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe differential detail page now uses a five-section index, responsive phone sheets, desktop tabs, diagnosis actions, and updated accessibility behavior. Tests cover section metadata, navigation, header contracts, and responsive URL state. Design-system adoption records and review ledger entries were updated. ChangesDifferential detail navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DifferentialDetailPage
participant SectionIndex
participant NavigationSheet
User->>DifferentialDetailPage: Open differential detail page
DifferentialDetailPage->>SectionIndex: Build section metadata
SectionIndex-->>DifferentialDetailPage: Return ordered sections and weights
User->>DifferentialDetailPage: Open phone section navigation
DifferentialDetailPage->>NavigationSheet: Render section choices
NavigationSheet-->>DifferentialDetailPage: Return selected section
DifferentialDetailPage-->>User: Update active panel and tab URL
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
…67c492 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnXJpp8BpZSYychgi18EnN
BigSimmo
commented
Aug 8, 2026
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
PR #1715 — end-to-end pass completePR:#1715 — Blocker clearedGitHub reported
ReviewLedger lookup at Verification (on |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:c67c4927d8
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Cursor Cloud review-and-fix summary (PR #1715)PR:#1715 — Snapshot (before → after)
Blocker fixed
Review findings (delta)No P0/P1 defects found in the PR delta. Spot-check of high-risk areas:
Pre-existing / out of scope: CodeRabbit rate-limited; no actionable human or Bugbot threads on this tip. Local verification (offline, on |
BigSimmo
commented
Aug 8, 2026
@copilot resolve the merge conflicts on this branch. |
…inpage-navigation-h2u8fq # Conflicts: # docs/branch-review-ledger.md Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
Merge conflicts resolved. There was one real conflict in |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/header-scroll-hide-contract.test.ts (1)
368-384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMatch structure, not exact JSX formatting.
Lines 368 and 382-384 assert exact source literals. The formatter controls both. Adding one utility class to the
navpushes theclassNamepast the print width, and Prettier then splits the attribute across lines. The assertion fails although the behaviour is unchanged. The same applies to the single-line<DocumentSectionTrack ... />call.Assert the individual tokens instead.
♻️ Proposed change
- expect(differentialDetailSource).toContain("<DocumentSectionTrack sections={sections} activeId={activeTab} />");+ expect(differentialDetailSource).toMatch(/<DocumentSectionTrack\b[\s\S]*?activeId=\{activeTab\}/); expect(differentialDetailSource).toContain("<DocumentSectionList"); @@ - expect(differentialDetailSource).toContain(- 'className="hidden border-b border-[color:var(--border)] text-sm font-bold text-[color:var(--text-muted)] sm:flex"',- );+ const tabStripClasses = /role="tablist"[\s\S]{0,400}?className="([^"]+)"/.exec(differentialDetailSource)?.[1] ?? "";+ expect(tabStripClasses.split(/\s+/)).toEqual(expect.arrayContaining(["hidden", "sm:flex"]));🤖 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 `@tests/header-scroll-hide-contract.test.ts` around lines 368 - 384, Update the source-structure assertions in the header scroll contract test to match JSX semantics rather than exact formatting: replace the single-line DocumentSectionTrack and labelled-strip className string checks with assertions for their individual required tokens, while preserving the existing portal sibling and Sheet/DocumentSectionList checks.src/components/differentials/detail-section-index.ts (1)
74-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared "expandable section" predicate.
counts.reviewSectionsrepeats the exact filter used byexpandableSectionIdsinsrc/components/differentials/differential-detail-page.tsx(lines 1086-1090). The comment states the two must match. A change to one site will silently desynchronise the Overview segment weight and theN sectionsdetail from the rendered panel.Export one helper from
src/lib/differential-detail.tsand call it from both places.♻️ Suggested shape
// src/lib/differential-detail.tsexportfunctionexpandableSectionIds(record: DifferentialRecord): string[]{returnrecord.sections.filter((section)=>visibleSectionItems(section,record).length>0).map((s)=>s.id);}- reviewSections: record.sections.filter((section) => visibleSectionItems(section, record).length > 0).length,+ reviewSections: expandableSectionIds(record).length,🤖 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/differentials/detail-section-index.ts` around lines 74 - 80, Extract the shared expandable-section logic into an exported expandableSectionIds helper in differential-detail.ts, using visibleSectionItems to filter sections with items and returning their IDs. Update the counts.reviewSections calculation in the detail section and the existing expandableSectionIds usage in differential-detail-page.tsx to call this helper, keeping both the Overview count and rendered panel synchronized.tests/ui-tools.spec.ts (1)
2062-2078: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd phone coverage for the actions sheet.
This block covers the section disclosure well. The PR also moves "New differentials search" from the header row into the ellipsis actions sheet, and that path has no browser coverage at 320px. A regression that hides or mis-sizes
differential-actions-triggerwould pass this gate.Add a short assertion for the actions trigger and sheet.
As per path instructions: "Use Playwright for end-to-end tests and run the appropriate Chromium UI gate, starting the project with npm run ensure rather than assuming a port."💚 Suggested addition after line 2078
await expect(page).toHaveURL(/[?&]tab=map/); ++ const actionsTrigger = page.getByTestId("differential-actions-trigger");+ await expect(actionsTrigger).toBeVisible();+ await expectMinTouchTarget(actionsTrigger);+ await actionsTrigger.click();+ const actionsSheet = page.getByTestId("differential-actions-sheet");+ await expect(actionsSheet).toBeVisible();+ await expect(actionsSheet.getByRole("link", { name: "New differentials search" })).toBeVisible();+ await page.keyboard.press("Escape");+ await expect(actionsSheet).toBeHidden();🤖 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 `@tests/ui-tools.spec.ts` around lines 2062 - 2078, Extend the 320px viewport test after the existing section-sheet assertions to cover the differential actions flow: assert differential-actions-trigger is visible and has the minimum touch target, click it, and assert the actions sheet becomes visible. Use the existing page/test-id locators and Playwright assertions without changing the section disclosure coverage.Source: Path instructions
src/components/differentials/differential-detail-page.tsx (1)
1261-1267: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
sectionMetaas the single label source
detailTabsandsectionMetacurrently use the same labels. Derive tab labels fromsectionMetato prevent future drift.🤖 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/differentials/differential-detail-page.tsx` around lines 1261 - 1267, Update the tab-label construction in the differential detail page to derive labels from sectionMeta rather than duplicating label values in detailTabs. Keep sectionMeta as the single source of truth and preserve the existing activeSection aria-label fallback behavior.
🤖 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 `@tests/differential-section-nav.dom.test.tsx`:
- Around line 30-46: Update buildContext so ...overrides is merged before
reconstructing source, then spread the default source values with
overrides.source last. Preserve all default source fields when callers provide
partial source overrides, while allowing complete source values to replace them.
---
Nitpick comments:
In `@src/components/differentials/detail-section-index.ts`:
- Around line 74-80: Extract the shared expandable-section logic into an
exported expandableSectionIds helper in differential-detail.ts, using
visibleSectionItems to filter sections with items and returning their IDs.
Update the counts.reviewSections calculation in the detail section and the
existing expandableSectionIds usage in differential-detail-page.tsx to call this
helper, keeping both the Overview count and rendered panel synchronized.
In `@src/components/differentials/differential-detail-page.tsx`:
- Around line 1261-1267: Update the tab-label construction in the differential
detail page to derive labels from sectionMeta rather than duplicating label
values in detailTabs. Keep sectionMeta as the single source of truth and
preserve the existing activeSection aria-label fallback behavior.
In `@tests/header-scroll-hide-contract.test.ts`:
- Around line 368-384: Update the source-structure assertions in the header
scroll contract test to match JSX semantics rather than exact formatting:
replace the single-line DocumentSectionTrack and labelled-strip className string
checks with assertions for their individual required tokens, while preserving
the existing portal sibling and Sheet/DocumentSectionList checks.
In `@tests/ui-tools.spec.ts`:
- Around line 2062-2078: Extend the 320px viewport test after the existing
section-sheet assertions to cover the differential actions flow: assert
differential-actions-trigger is visible and has the minimum touch target, click
it, and assert the actions sheet becomes visible. Use the existing page/test-id
locators and Playwright assertions without changing the section disclosure
coverage.
🪄 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: 4bd152d4-3ae7-4672-b717-dbce79ad5b22
📒 Files selected for processing (8)
docs/branch-review-ledger.mddocs/design-system/COMPONENTS.mddocs/design-system/adoption-manifest.jsonsrc/components/differentials/detail-section-index.tssrc/components/differentials/differential-detail-page.tsxtests/differential-section-nav.dom.test.tsxtests/header-scroll-hide-contract.test.tstests/ui-tools.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
Pass live sourceStatus into buildDifferentialSectionIndex so the in-page nav Source row matches FooterStatus when owner governance differs from the bundled snapshot. Fix test buildContext merge order so partial source overrides keep required metadata fields. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Resolve the sole merge conflict in docs/branch-review-ledger.md via the merge=ledger driver (append-only union). Other incoming changes from main (lighthouse budget pin, specifiers breadcrumb removal) merge cleanly with no product overlap. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…inpage-navigation-h2u8fq
…gation-h2u8fq' into claude/differentials-inpage-navigation-h2u8fq
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #9022 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
BigSimmo
commented
Aug 8, 2026
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues. Please try again later or Open in Web to view the status.   Learn more about Cursor Agents |
The live-governance fix passes `{ source: { sourceStatus } }` into the test
helper, but `Partial<DifferentialDetailContext>` only makes `source` optional
as a whole — once supplied it still demands every field, so the three call
sites failed typecheck and took Static PR checks / PR required red with them.
Type the override one level deeper so a partial `source` is actually legal,
which is what the helper's own rebuild-the-object logic already assumed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VnXJpp8BpZSYychgi18EnNUh oh!
There was an error while loading. Please reload this page.
Summary
docs/search-chrome-behaviour.md. That PR explicitly left this page behind as a legacy special case ("Existing simpler collapse headers (for example Differential detail) remain special cases"); this closes that gap and makes the page the template's second adopter afterDocumentViewer.+on the right, and empty space between. The real navigation was a separate in-flow tab strip that scrolled away with the content and clipped "Source" off the right edge at phone width. The header now carries all four template slots: back control, page title with the active section on line two behind a chevron disclosure, ellipsis page actions, and a weighted segment track pinned to the bottom edge.src/components/differentials/detail-section-index.ts, which shapes the five tabs asDocumentSections so the existingDocumentSectionTrackandDocumentSectionListrender them unchanged rather than growing a parallel implementation. Segment weights derive from real panel content — clinical-review section count, related-diagnosis count, overlap links — and are normalised, so the track reads as proportion rather than as "which of five". The Source segment is deliberately the smallest on every record because it is a governance footer, not a body of content.smup, and hides it below. Phones navigate from the header disclosure and its sheet, which is one affordance per breakpoint and leaves nothing to clip at 320px.useDocumentSectionSpyis not involved.PhoneHeaderCollapsePortal, never as children, so they are not carried away when the shared chrome scroll-hides.DocumentSectionSheetis intentionally not reused because it hard-requires the document-only compact/density toggle; the section sheet composes the sharedSheetaroundDocumentSectionListinstead.max-sm:statictorelative, matchingDocumentViewer. This is load-bearing rather than cosmetic: the segment track is absolutely positioned against the header, and astaticphone header would let it escape to whichever ancestor happens to be positioned.aria-labelinstead ofaria-labelledby. Belowsmthe strip isdisplay:none, and assistive technology ignores a hidden labelling element, which would have left the panel unnamed on phones.+and into the actions sheet, alongside Save, Copy after review, and Compare.MobilePrimaryActions. That last one is a content call-to-action rather than navigation chrome, and removing it would drop the prominent "Compare (n)" button.Verification
npm run verify:pr-local— reaches the full unit suite and stops only attests/pr-handoff-stop.test.ts, a pre-existing failure unrelated to this change. Confirmed by stashing the whole diff and re-running on the clean base, where it fails identically; it is also already recorded as pre-existing indocs/branch-review-ledger.md(PR refactor(navigation): remove the vestigial one-button mode strip #1679 row). Everything else is green:Test Files 1 failed | 522 passed (523),Tests 1 failed | 5557 passed | 4 skipped (5562).npm run verify:phone-chrome— the gateAGENTS.mdnames for phone-chrome changes.check:installed-lock-parityPASS (playwright 1.62.1 parity),check:runtimePASS, static/unit contracts119 passed (9 files), changed-browsertests/ui-tools.spec.tspassed, focused-browser7 passed (26.1s). Its smart selector reported "Focused ownership and journey coverage is sufficient for this page-local scope" and did not escalate to the full Chromium gate.npx tsc --noEmit— exit 0, no output.npm run lint— clean at--max-warnings 0.tests/ui-tools.spec.ts --grep "diagnosis detail actions stay tappable"—1 passed.tests/differential-section-nav.dom.test.tsx—9 passed; combined with the updated contract test,40 passed (2 files)re-run afternpm run format.UI verification beyond the above: manual browser proof against the running dev server at 320px, 390px and 768px. Zero horizontal overflow at all three widths. The section sheet opens from the header disclosure, selecting Map sets
?tab=mapand updates the header subtitle, the actions sheet opens, and Escape restores focus to its trigger. The phone collapse contract was checked side by side against the reference implementation: after real wheel scrolling, both/differentials/diagnoses/deliriumand the document viewer reportdata-scroll-hidden="true"with the collapse stack atbottom: 0, and the differentials header sits insideuniversal-header-collapse, so a single owner still moves both. No second scroll-hide hook was added.npm run verify:releasenot run: not a release or handoff-confidence claim. Retrieval, ranking, answer-generation and production-readiness gates not run: none of those surfaces are touched by this diff.Risk and rollout
?tab=query-param sync and the static-generation-safe read-on-mount are unchanged, so deep links such as?tab=mapfrom the presentation workflow page keep working. The main regression surface is phone chrome, which is whyverify:phone-chromeand the side-by-side collapse comparison are the primary evidence.Notes
docs/design-system/adoption-manifest.jsondropstests/ui-tools.spec.tsfrom the design-systemTabscomponent'stestFiles. This is not lost coverage.scripts/generate-design-system-adoption.mjsassociates tests with components by matching\bTabs\bagainst file text, and the only match in that spec was a capital-T word in a comment (// Tabs: no page overflow …) that this change rewrote. The differentials strip is a localfunction Tabsinside the page and never imports@/components/ui/tabs, so the association was coincidental and the manifest is now more accurate.Tabsretains its two genuine proofs.docs/design-system/COMPONENTS.mdmovesSheetusage from 20 to 21, reflecting the new section sheet.docs/codebase-index.mdwas not edited. Nothing in it describes the differentials header at a granularity this change alters, so there was no accurate line to update.tests/mode-nav-addon-slot.dom.test.tsxenumeration guard, which hard-codes the set of components allowed to claim the phone collapse slot, passes unchanged: the page already claimed the slot and still does.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH=/opt/pw-browsers/chromium. That is a container gap, not a product signal.Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests