Skip to content

chore: organize dirty work from claude/navigation-pane-mockups-0600af - #1311

Merged
BigSimmo merged 31 commits into
mainfrom
claude/navigation-pane-mockups-0600af
Jul 28, 2026
Merged

chore: organize dirty work from claude/navigation-pane-mockups-0600af#1311
BigSimmo merged 31 commits into
mainfrom
claude/navigation-pane-mockups-0600af

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Organized dirty work for claude/navigation-pane-mockups-0600af\n\nRAG impact: no retrieval behaviour change\n\nThis branch organizes local dirty work for main.

Summary by CodeRabbit

  • New Features
    • Added new mockup pages and route entries for document navigation studies (contract, pane, perfected, final review, and multiple phone variants).
    • Introduced section-aware document navigation in the viewer, including a sticky section track and a phone “jump to section” sheet.
  • Improvements
    • Refreshed the document viewer rail and refined scroll/jump behavior so section offsets track the collapsing header/chrome state.
  • Documentation
    • Expanded the “Search chrome behaviour contract” for document section navigation ownership and invariants.
  • Tests
    • Added/updated coverage for section indexing, navigation UI, jump/scroll behavior, header/chrome offset contracts, and authorization-gated rail controls.

@supabase

supabaseBot commented Jul 28, 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

coderabbitaiBot commented Jul 28, 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:41 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: d815e741-439a-4de0-b53d-432140686ad8

📥 Commits

Reviewing files that changed from the base of the PR and between 0a5f9fe and 98ecc72.

📒 Files selected for processing (4)
  • docs/branch-review-ledger.md
  • src/components/document-viewer/document-clinical-summary.tsx
  • tests/document-section-nav.dom.test.tsx
  • tests/ui-smoke.spec.ts
📝 Walkthrough

Walkthrough

Adds document-navigation mockup routes and responsive showcase components, wires those routes into the mockup shell, and introduces production document section indexing, navigation controls, active-section tracking, chrome offset metrics, and an extracted viewer rail with supporting tests and documentation.

Changes

Document navigation and viewer integration

Layer / File(s)Summary
Mockup routes and shell wiring
docs/site-map.md, src/app/mockups/..., src/app/mockups/mockups-layout-client.tsx
Adds metadata-bearing routes for document navigation mockups, registers them in the sitemap, and updates document mode and shared composer visibility.
Responsive navigation studies
src/components/document-navigation-*-mockups.tsx, src/components/document-phone-*-mockups.tsx
Adds desktop, tablet, and phone showcases for navigation contracts, pane concepts, final layouts, title treatments, fused directions, review variants, perfected states, and zero-chrome states.
Section model and controls
src/components/document-viewer/section-index.ts, src/components/document-viewer/section-nav.tsx, src/components/document-viewer/use-section-spy.ts
Adds conditional weighted section models, navigation lists and tracks, phone sheets, accordion-aware jumping, and active-section observation.
Viewer integration and rail extraction
src/components/DocumentViewer.tsx, src/components/document-viewer/document-rail-panels.tsx, src/components/document-viewer/use-document-chrome-metrics.ts, src/components/document-viewer/source-panels.tsx
Integrates section navigation and chrome metrics into the viewer, replaces inline rail markup with DocumentViewerRail, updates anchors, and removes the prior anchor component.
Contracts, documentation, and tests
docs/*.md, tests/*
Documents section-navigation chrome and offset rules, records review ledger entries, and tests section indexing, navigation semantics, access gating, and header-scroll contracts.

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

Possibly related PRs

Suggested labels:codex

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 30.58% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check⚠️ WarningThe description does not follow the repository template and is missing Summary, Verification, Risk, and Clinical Governance sections.Add the required template sections and include verification status, risk/rollback notes, and any relevant governance preflight items.
Title check❓ InconclusiveThe title is related to the branch work but is too vague and doesn't state the actual change.Rename it to something specific like 'Add document navigation mockups and section navigation to the viewer'.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

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

@coderabbitaicoderabbitaiBot 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: 8

🧹 Nitpick comments (2)
src/components/document-phone-fused-directions-mockups.tsx (1)

87-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

focusRing, UniversalHeader, PaneList, and Canvas are duplicated across sibling mockup files.

The same focusRing string, near-identical UniversalHeader/Canvas/PaneList markup, and the sections/documentTitle data show up again almost verbatim in document-phone-title-mockups.tsx and document-phone-title-refined-mockups.tsx. Extracting these into a shared mockup-kit module would cut drift risk across this whole cohort.

Also applies to: 90-106, 116-154, 205-239

🤖 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/document-phone-fused-directions-mockups.tsx` around lines 87 -
88, Extract the duplicated focusRing constant, UniversalHeader, PaneList,
Canvas, and shared sections/documentTitle data from the sibling document phone
mockup files into a shared mockup-kit module. Update
document-phone-fused-directions-mockups.tsx, document-phone-title-mockups.tsx,
and document-phone-title-refined-mockups.tsx to import and reuse these shared
symbols while preserving each mockup’s unique content and behavior.
src/components/document-navigation-contract-mockups.tsx (1)

155-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

UniversalTopBar and PhoneComposerPill are duplicated verbatim in document-navigation-final-mockups.tsx.

Both components (plus focusRing, documentTitle, and the sections data array) are byte-for-byte identical to their counterparts in document-navigation-final-mockups.tsx (Lines 104-142 and 294-310 there). Since final is explicitly derived from this candidate study (per its own lineage narrative), extracting these shared primitives into a common module would prevent silent drift between the two studies.

Also applies to: 243-259

🤖 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/document-navigation-contract-mockups.tsx` around lines 155 -
193, Extract the duplicated UniversalTopBar and PhoneComposerPill components,
along with focusRing, documentTitle, and sections, into a shared module used by
both mockup files. Update document-navigation-contract-mockups.tsx and
document-navigation-final-mockups.tsx to import these shared symbols, preserving
their current behavior and data.
🤖 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/site-map.md`:
- Around line 991-992: Add the six missing mockup
routes—document-navigation-contract, document-navigation-final,
document-phone-fused-directions, document-phone-title,
document-phone-title-refined, and document-phone-zero-chrome—to the mockup
routes section of docs/site-map.md alongside the existing
document-navigation-pane and document-search entries.
In `@src/components/document-navigation-contract-mockups.tsx`:
- Around line 215-229: Replace the no-op onClick handlers in the phone title
disclosure and the SectionRows, SectionCards, and IconRail item controls with
the disabled-placeholder pattern: mark each control disabled or aria-disabled,
add a coming-soon title, and include an accessible sr-only explanation. Preserve
their existing labels, expanded state, and visual structure while ensuring
unfinished controls are not presented as actionable.
In `@src/components/document-navigation-final-mockups.tsx`:
- Around line 192-219: Replace the no-op onClick handlers in DocumentHeaderRow’s
disclosure button and the SectionGrid items with the required
disabled-placeholder pattern: mark each control disabled or aria-disabled, add a
coming-soon title, and include an accessible sr-only explanation. Preserve the
existing visual layout and disclosure state presentation.
In `@src/components/document-phone-fused-directions-mockups.tsx`:
- Around line 116-154: Update all affected interactive buttons, including
PaneList and the pane-toggle controls, so each either performs its intended
action or uses the required disabled/aria-disabled coming-soon placeholder with
title and sr-only explanation. Replace no-op onClick handlers accordingly.
Ensure aria-controls references an element that exists, either by keeping Pane
mounted while closed or omitting the attribute when the pane is closed.
In `@src/components/document-phone-title-mockups.tsx`:
- Around line 110-148: The mockup’s non-functional navigation triggers lack the
required disabled-placeholder accessibility treatment, and their aria-controls
references conditionally unmounted panes. Update PaneList, FusedApproach,
BreadcrumbApproach, and SheetApproach to use the explicit disabled or
aria-disabled placeholder pattern, including a coming-soon title and sr-only
explanation, and ensure each aria-controls reference only points to a mounted
pane or is removed when unavailable.
In `@src/components/document-phone-title-refined-mockups.tsx`:
- Around line 158-172: Replace the no-op handlers in ActionsButton, PaneList,
the three variant triggers, and the Stepper prev/next buttons with the explicit
disabled-placeholder pattern: mark each unfinished control disabled or
aria-disabled, add a coming-soon title, and include an accessible sr-only
explanation. Remove or correct any aria-controls references to paneId when the
corresponding pane is not rendered, so triggers never reference absent DOM
elements.
In `@src/components/document-phone-zero-chrome-mockups.tsx`:
- Around line 104-118: Replace the inert onClick handlers on the buttons in the
document mockup, including the control around documentTitle and the buttons at
the referenced sections, with functional local mock-state actions that update
their associated UI state. If a control cannot be implemented, convert it to the
established disabled or aria-disabled placeholder pattern with a coming-soon
title and sr-only explanation.
- Around line 315-323: Update the DocumentHeader and ActionsSheet rendering to
keep the title disclosure associated only with the sections state: pass sheetId
to DocumentHeader only when state is "sheet", and omit it for "actions". Ensure
ActionsSheet provides its own trigger when the actions sheet is interactive.
---
Nitpick comments:
In `@src/components/document-navigation-contract-mockups.tsx`:
- Around line 155-193: Extract the duplicated UniversalTopBar and
PhoneComposerPill components, along with focusRing, documentTitle, and sections,
into a shared module used by both mockup files. Update
document-navigation-contract-mockups.tsx and
document-navigation-final-mockups.tsx to import these shared symbols, preserving
their current behavior and data.
In `@src/components/document-phone-fused-directions-mockups.tsx`:
- Around line 87-88: Extract the duplicated focusRing constant, UniversalHeader,
PaneList, Canvas, and shared sections/documentTitle data from the sibling
document phone mockup files into a shared mockup-kit module. Update
document-phone-fused-directions-mockups.tsx, document-phone-title-mockups.tsx,
and document-phone-title-refined-mockups.tsx to import and reuse these shared
symbols while preserving each mockup’s unique content and behavior.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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: a76fdbe5-efcc-4e2c-b9ce-da4c3e0964b1

📥 Commits

Reviewing files that changed from the base of the PR and between c9bec8f and 75a796d.

📒 Files selected for processing (16)
  • docs/site-map.md
  • src/app/mockups/document-navigation-contract/page.tsx
  • src/app/mockups/document-navigation-final/page.tsx
  • src/app/mockups/document-navigation-pane/page.tsx
  • src/app/mockups/document-phone-fused-directions/page.tsx
  • src/app/mockups/document-phone-title-refined/page.tsx
  • src/app/mockups/document-phone-title/page.tsx
  • src/app/mockups/document-phone-zero-chrome/page.tsx
  • src/app/mockups/mockups-layout-client.tsx
  • src/components/document-navigation-contract-mockups.tsx
  • src/components/document-navigation-final-mockups.tsx
  • src/components/document-navigation-pane-mockups.tsx
  • src/components/document-phone-fused-directions-mockups.tsx
  • src/components/document-phone-title-mockups.tsx
  • src/components/document-phone-title-refined-mockups.tsx
  • src/components/document-phone-zero-chrome-mockups.tsx

Comment threaddocs/site-map.md
Comment threadsrc/components/document-navigation-contract-mockups.tsx
Comment threadsrc/components/document-navigation-final-mockups.tsx
Comment threadsrc/components/document-phone-fused-directions-mockups.tsx
Comment threadsrc/components/document-phone-title-mockups.tsx
Comment threadsrc/components/document-phone-title-refined-mockups.tsx
Comment threadsrc/components/document-phone-zero-chrome-mockups.tsx
Comment threadsrc/components/document-phone-zero-chrome-mockups.tsx Outdated
@github-actions

github-actionsBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

CI triage

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

  • Production UIneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #6343 (success).

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

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:30541ae488

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threaddocs/site-map.md
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit:30541ae488

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

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:6bb13bd850

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadsrc/components/document-viewer/use-document-chrome-metrics.ts Outdated
Comment threadsrc/components/DocumentViewer.tsx Outdated
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Inspect the PR and repository state, identify the root cause of every failing CI check, unresolved review comment, merge conflict, and related defect. Apply the smallest robust fix using existing project patterns and best practices, without unrelated changes. Run Bugbot and all relevant lint, type-check, test, build, and CI-equivalent commands, fix any issues found, then resolve only the review comments that are fully addressed. Summarise the root causes, changes made, validation results, and any remaining blockers.

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Taking a look!

Open in WebOpen in Cursor

@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

❌ Failed to clone repository into sandbox. Please try again.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 28, 2026 12:43
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

⚠️Branch updated during autofix.

The branch was updated while autofix was in progress. Please try again.

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:8ad5e53728

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadsrc/components/document-viewer/section-index.ts
Comment threadsrc/components/document-viewer/section-index.ts Outdated
cursoragentand others added 2 commits July 28, 2026 12:48
…rome
Extract the sticky rail into document-rail-panels, hide the section index
card below lg, and clear the page-owned sticky header in anchor/rail
offsets when the shared collapse row hides. Remove accidental scratch
file .shootG.tmp.mjs.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…600af'
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent fix failing CI testing

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Taking a look!

Open in WebOpen in Cursor

cursoragentand others added 2 commits July 28, 2026 13:04
Unblocks Static PR format:check / CircleCI verify after the section-nav
extract and mockup a11y follow-ups.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmoBigSimmo mentioned this pull request Jul 28, 2026
3 tasks

@coderabbitaicoderabbitaiBot 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: 2

🧹 Nitpick comments (2)
src/components/document-viewer/document-rail-panels.tsx (1)

84-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Hardcoded 69px fallback undercuts the dynamic-offset goal.

headerHidden correctly derives its offset from the live --document-sticky-header-height custom property, but the visible-bar branch still hardcodes lg:top-[69px]. The entire point of useDocumentChromeMetrics is to avoid fixed offsets that "strand headings whenever the row is a different height" (per the new invariant in docs/search-chrome-behaviour.md), yet the rail's sticky offset for the common (chrome-visible) case still relies on a magic number that will silently drift out of sync if the universal header's height ever changes.

♻️ Suggested direction
- headerHidden ? "lg:top-[var(--document-sticky-header-height,0px)]" : "lg:top-[69px]",+ headerHidden ? "lg:top-[var(--document-sticky-header-height,0px)]" : "lg:top-[var(--universal-header-height,69px)]",

Publish a --universal-header-height (or reuse an existing token) from wherever the universal bar's height is already measured, keeping 69px only as a fallback for the first paint before the observer runs.

🤖 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/document-viewer/document-rail-panels.tsx` around lines 84 -
93, Update the visible-bar branch in the document rail’s aside class logic to
use the live universal header height custom property published by the existing
document chrome metrics flow, retaining 69px only as its CSS fallback before
measurement is available. Keep the headerHidden branch’s document sticky header
offset unchanged.
src/components/document-viewer/use-document-chrome-metrics.ts (1)

28-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

collapse is resolved once and never re-queried if it mounts later.

collapse is captured via a single querySelector at effect setup, and the effect's dependency array is [rootRef] — a stable ref that never changes identity, so this effect runs once for the component's lifetime. If [data-testid="universal-header-collapse"] isn't in the DOM yet at that moment (e.g. a hydration/mount-order edge case), collapse stays null forever and headerHidden/the anchor-offset metrics silently stop tracking the shared header. Contrast this with the shellScrollContainer sync pattern in DocumentViewer.tsx (a MutationObserver on document.body that keeps re-resolving #main-content for exactly this class of problem) — this hook has no equivalent fallback for collapse.

This is likely fine in practice since the shared header wraps every document route, but worth confirming since a race here degrades silently (no error, just a stale rail offset).

Also applies to: 88-88

🤖 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/document-viewer/use-document-chrome-metrics.ts` around lines
28 - 33, Update the effect containing the collapseSelector query in the document
chrome metrics hook so the shared collapse element is re-resolved when it mounts
after setup, rather than relying on the initial query. Add a suitable DOM
observation or equivalent retry mechanism, and ensure headerHidden and
anchor-offset metric listeners use the newly resolved element while preserving
cleanup and existing 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/document-section-nav.dom.test.tsx`:
- Around line 108-112: Update the “takes an explicit selection immediately” test
to invoke selectSection within act and assert result.current.activeId equals
"source-images" after the update, replacing the static sections membership
assertion.
- Around line 94-106: Add an IntersectionObserver mock for the
useDocumentSectionSpy test, either in tests/setup/jsdom.setup.ts or within this
test file, so the hook does not take its fallback path when the API is
unavailable. Ensure the mock supports the hook’s observer setup and allows the
assertion to exercise the collapsed-section handling branch.
---
Nitpick comments:
In `@src/components/document-viewer/document-rail-panels.tsx`:
- Around line 84-93: Update the visible-bar branch in the document rail’s aside
class logic to use the live universal header height custom property published by
the existing document chrome metrics flow, retaining 69px only as its CSS
fallback before measurement is available. Keep the headerHidden branch’s
document sticky header offset unchanged.
In `@src/components/document-viewer/use-document-chrome-metrics.ts`:
- Around line 28-33: Update the effect containing the collapseSelector query in
the document chrome metrics hook so the shared collapse element is re-resolved
when it mounts after setup, rather than relying on the initial query. Add a
suitable DOM observation or equivalent retry mechanism, and ensure headerHidden
and anchor-offset metric listeners use the newly resolved element while
preserving cleanup and existing behavior.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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: da05bcce-238d-4707-a94c-5c1be01bd1f5

📥 Commits

Reviewing files that changed from the base of the PR and between f833621 and 4b65fbb.

📒 Files selected for processing (21)
  • docs/branch-review-ledger.md
  • docs/search-chrome-behaviour.md
  • docs/site-map.md
  • src/app/mockups/document-navigation-final-review/page.tsx
  • src/app/mockups/document-navigation-perfected/page.tsx
  • src/app/mockups/mockups-layout-client.tsx
  • src/components/DocumentViewer.tsx
  • src/components/document-navigation-final-review-mockups.tsx
  • src/components/document-navigation-perfected-mockups.tsx
  • src/components/document-phone-title-mockups.tsx
  • src/components/document-phone-zero-chrome-mockups.tsx
  • src/components/document-viewer/document-rail-panels.tsx
  • src/components/document-viewer/section-index.ts
  • src/components/document-viewer/section-nav.tsx
  • src/components/document-viewer/source-panels.tsx
  • src/components/document-viewer/use-document-chrome-metrics.ts
  • src/components/document-viewer/use-section-spy.ts
  • tests/account-access-model.test.ts
  • tests/document-section-index.test.ts
  • tests/document-section-nav.dom.test.tsx
  • tests/header-scroll-hide-contract.test.ts
💤 Files with no reviewable changes (1)
  • src/components/document-viewer/source-panels.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/app/mockups/mockups-layout-client.tsx
  • src/components/document-phone-title-mockups.tsx
  • src/components/document-phone-zero-chrome-mockups.tsx

Comment threadtests/document-section-nav.dom.test.tsx
Comment threadtests/document-section-nav.dom.test.tsx
@coderabbitai

coderabbitaiBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

⚠️Branch updated during autofix.

The branch was updated while autofix was in progress. Please try again.

cursoragentand others added 4 commits July 28, 2026 13:13
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Mock IntersectionObserver and element rects so collapsed-section filtering
is actually covered, and assert selectSection updates activeId under act().
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent please resolve failing CI tests and ensure it does not happen again

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR #1311 is already fixed and merged (6230c4db). The Production UI reds in that GitHub context were from an older tip.

What failed (and was fixed on merge):

  1. Duplicate id="document-overview" on clinical summary + overview wrapper → DOM integrity failure
  2. ui-smoke still used the retired “Document viewer sections” nav and required a “Tables and diagrams” sheet row for the lithium demo (image_count: 0), which omits that row

Prevention (so it doesn’t recur): opened follow-up draft #1371#1371

  • Cheap source contracts in tests/document-section-nav-contract.test.ts
  • DOM assertion that clinical summary does not claim document-overview
  • Contract note in docs/search-chrome-behaviour.md §22

Validation:npm run verify:cheap green — 420 files / 4262 tests passed.

Open in WebOpen in Cursor

cursoragentand others added 2 commits July 28, 2026 13:41
Remove the duplicate document-overview id from DocumentClinicalSummary
and retarget phone section navigation in ui-smoke to the title sheet.
Open Tables and diagrams via its details summary when the demo doc has
no visuals (omitted from the section index).
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent failing CI fix

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

CI is green.

Draft PR: #1371

Open in WebOpen in Cursor

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:bdddc44b4f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadsrc/components/DocumentViewer.tsx
@coderabbitai

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.

CopilotAIand others added 4 commits July 28, 2026 21:57
Copilot pushed an empty tip that skipped pull_request CI; required
Static PR / Production UI / PR required checks never queued on the tip.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Empty tip commits skipped Actions pull_request CI; record the prior
green parent tip and force a non-empty push so required checks queue.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Unstick GitHub CONFLICTING (merge-tree clean) so pull_request CI can
queue required Static PR / Production UI / PR required checks.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo
BigSimmo merged commit 6230c4d into mainJul 28, 2026
19 checks passed
@BigSimmo
BigSimmo deleted the claude/navigation-pane-mockups-0600af branch July 28, 2026 14:32
cursorBot pushed a commit that referenced this pull request Jul 28, 2026
* test: guard document section nav against Production UI drift
PR #1311 red Production UI twice from duplicate document-overview ids and
smoke still targeting the retired in-flow section nav / missing Images row.
Add cheap ownership contracts and a DOM assertion so verify:cheap catches
that class of regression before the browser gate.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
* style: prettier document-section-nav-contract test
CircleCI format:check failed on the multi-line regex matcher.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@BigSimmo@cursoragent