feat(in-page-nav): extract the default in-page navigation template - #1740
Conversation
`docs/search-chrome-behaviour.md` names the DocumentViewer header as the default in-page navigation template for any mode page, but the markup implementing it was duplicated near-verbatim between DocumentViewer and the differentials detail page. Adopting it on the remaining information pages would have meant a third copy. Extract it into `InPageNavHeader`: back control, title carrying the active section behind a chevron disclosure, ellipsis page actions, and the weighted segment track, plus the `PhoneHeaderCollapsePortal` wrapper and both sheets. It composes the already-shared `DocumentSectionTrack` / `DocumentSectionList` / `Sheet` rather than reimplementing them, so pages pass data, not plumbing. Sections are declared as `PageSection` — `DocumentSection` with the document-only fields optional, because a service record's Referral section has no magnitude to report the way an 84-page PDF does. `toDocumentSections` fills the gaps, so `section-index.ts` is untouched and the document tests are unaffected. Omit `weight` and `usePageSectionWeights` measures the rendered heights; pass explicit weights for discrete tab panels, which have no on-screen height to measure, and the hook then observes nothing at all. Converts `differential-detail-page.tsx` onto it as a behaviour-neutral proof of the extraction. DocumentViewer keeps its own copy for now: it owns the page `<h1>`, uses the `edge-glass-header` treatment, and is pinned by visual baselines, so converging it is a separate change rather than half of this one. Three source-scanning contracts pinned that markup by reading the differentials page. They follow it to the shared component and additionally assert the page adopts the template, so a page that dropped its navigation entirely still fails. The addon-slot guard now scans for `InPageNavHeader` as well as the raw portal — without that it would have gone silent for every future adopter, which is the regression it exists to catch. Verification: verify:cheap — 5618 passed, 1 failed, the failure being tests/pr-handoff-stop.test.ts, which reproduces on a clean tree with this work stashed and is unrelated to this change. lint, typecheck and whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GyM49edHSNfm42u1SFDfG3
|
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: 53 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 (14)
Comment |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GyM49edHSNfm42u1SFDfG3
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b67f33f65e
ℹ️ 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".
|
@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. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/inpage-nav-info-pages-v8rhnd at starting commit d67451c; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/inpage-nav-info-pages-v8rhnd, then verify that the pull request head contains the published commit before reporting success. 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. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. 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. |
|
@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 #1740 — tip What I did
Gates
Required CI is re-running on the new tip after the fix push.   |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
toDocumentSections combined already-normalised measured weights with explicit ones without a second pass, so an 80% segment beside two measured halves reached DocumentSectionTrack as 0.8/0.5/0.5 and flexGrow painted it at ~44%. Renormalise the resolved set to sum to one; pure all-explicit or all-measured inputs that already sum to one are unchanged. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Cursoragent PR #1740 end-to-end summaryTip: Sync / merge-tree
Fixed vs dispositioned
Thread disposition (inline reply 403’d on shell Unresolved actionable threads: 0. Local gates (decisive lines)
Residual risks
|
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #9094 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
@cursoragent Unblock the current open PR. 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; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, 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, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels 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. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
PR #1740 tip Snapshot
Status
|
|
@cursoragent Unblock the current open PR. 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; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, 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, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels 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. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
PR #1740 is already merged — no further unblock work.
No push this pass — tip was already handled, merge-tree clean, auto-merge completed once required checks passed. Summary posted on the PR. |
Production UI (2) failed on DSM comparison remove: the Next Link click resolved to the correct href but fired no navigation (URL stuck on both ids). Same-route search-param soft-nav is unreliable under full-suite load; use explicit router.push for the remove control, align compare href building with URLSearchParams, and pin the visible page owner in the route-coverage action. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Cursoragent PR #1740 unblock summaryTip: What blocked
Fix
Proof
Residual risks
|
|
@cursoragent Unblock the current open PR. 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; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, 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, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels 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. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
PR #1740 is already merged — nothing left to unblock.
Stopping per the merged/closed stop rule. |
Behind-but-clean vs origin/main (fc399e2); clears GitHub DIRTY/CONFLICTING so squash auto-merge can proceed. No content conflict. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Cursoragent PR #1740 unblock summaryTip: Sync / merge-tree
What blocked / what fixed
No new product fixes this pass. Unresolved actionable review threads: 0. Required CIRe-queued on Residual risks
No force-push, rebase, or merge performed. |
Cursoragent PR #1740 unblock — already mergedOutcome: PR #1740 is MERGED (squash auto-merge). No further unblock work. Confirmed
Prior unblock actions (already on the merged tip)
Merge left to auto-merge / you — done. Stopping. |
Cursoragent unblock — PR already mergedPR #1740 is MERGED. No open PR to unblock; stopping.
No push, force-push, rebase, or further sync this pass. Stale “PR mergeability dirty” context for older tip |
… header (#1766) * feat(in-page-nav): convert the six information routes onto the shared header PR 2 of the in-page navigation series. PR 1 (#1740) extracted the DocumentViewer in-page navigation template into src/components/in-page-nav/ and converted the differentials detail page as proof. This converts the six routes still drawing the shell-owned "On this page" pill rail — services, forms, specifiers (record and catalogue reference), formulation, and the two DSM diagnosis routes — then deletes the rail behind them. Three things the conversion needed first: - InPageNavHeader's `actions` was a render prop. Four of the seven components in scope are Server Components, and React cannot pass a function across that boundary, so `actions` is widened to `ReactNode | ((close) => ReactNode)`. `onSelectSection` and `PageSection.icon` have the same problem, so those four pages mount the header through a small "use client" sibling module that owns the section table and the hooks and takes `actions` as a slot. - Both sheets now derive their open state from the current pathname, so navigating closes them. Server-passed action JSX is mostly <Link>s and has no way to call close(). - Information-page sections carried no scroll-mt at all. A shared `inPageAnchor` token consumes `--inpage-anchor-offset`, published from the live chrome height by useInPageChromeMetrics, which InPageNavHeader calls itself. The measurement behind both anchor offsets is now useStickyChromeMetrics in src/components/sticky-chrome-metrics.ts, shared with the document viewer. use-document-chrome-metrics.ts keeps its selector and custom-property literals verbatim, because header-scroll-hide-contract.test.ts pins that file's source. Two DSM routes declared section anchors that nothing rendered, so /dsm/diagnoses/<slug>/differentials drew one nav entry out of four declared and /dsm/diagnoses/<slug> drew three out of five. Both are wired in, asserted against rendered DOM rather than a source grep (/issues #256's stop rule) by the new tests/in-page-nav-route-sections.dom.test.tsx, which covers all seven components including present-and-absent cases for the conditional anchors. Rail removal: hasLocalInformationPageNavigation collapses to isInformationPage, because every information page now owns its navigation; the `section` kind and its scroll spy leave secondary-navigation.tsx. That leaves the component with no production constructor at all — RegistryModeNav renders ModeNav, not SecondaryNavigation — which is folded into /issues #271 rather than resolved here. Also corrects /issues #256: its Forms half was stale, all six form anchors render. Records /issues #285 for a pre-existing ui-smoke phone scroll-hide flake at base commit 9ab3b73, proven on pristine source in the same worktree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the in-page-nav PR 2 review Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(in-page-nav): repair the service-detail test mock and share the slug-detail test `InPageNavHeader` calls `usePathname()`, so every test that renders a converted page needs it on the `next/navigation` mock. `tests/service-detail-page.dom.test.tsx` mocked only `useRouter`, which failed three cases in CI's Unit coverage job with `No "usePathname" export is defined on the "next/navigation" mock`. It was the only such gap: the other eight test files mocking `next/navigation` without `usePathname` render no converted page. Review follow-ups: - `tests/in-page-nav-header.dom.test.tsx` resets its module-level `pathname` in `beforeEach`. The route-change case left it at `/specifiers/compare`, so outcomes depended on file order. - `header-addon-slot.ts` now calls the exported `isSlugDetail` from `information-pages.ts` instead of its own hand-copied equivalent. Every claimant route it names must also be `isInformationPage`, and two slug tests are how that agreement would silently diverge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…lready require jsdom@30 requires Node ^24.15.0, but engines.node said "24.x" and every gate compared majors only. A container on 24.13.0 therefore passed check:runtime and the preinstall hook, then failed `npm ci` with an opaque EBADENGINE for a transitive package. That blocked local verification on PRs #1611, #1697, #1705 and #1740, each worked around by hand. - package.json: engines.node "24.x" -> ">=24.15.0 <25", declaring the floor the dependency tree already imposes. This cannot newly break any environment that currently installs, since jsdom enforces the same floor during `npm ci`. - check-node-engine.cjs (npm preinstall): compare the full version against that range instead of the major, so the failure names the real requirement. Kept import-free — the Dockerfile COPYs this file alone before `npm ci` — with the range restated as a literal and pinned to package.json by test. - check-runtime.ts: add NODE_MINIMUM_VERSION and reject a matching major below it. This gate fronts verify:cheap, :ui, :release and :pr-local. - session-start.sh: provision 24.19.0, and make the install guard floor-aware. The old major-only test meant a cached container already on 24.13 skipped the install entirely, so bumping the pin alone would not have upgraded it. - check-codex-cloud-setup.mjs: accept the range form, still requiring its major to track .node-version. Closes #285. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XZHZjz6kX8Ly1qBvvCg29C
…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
…e claiming "0 commits ahead" when it isn't (#2117) * fix(claude): make the session-start hook runnable and give the agent config teeth `.claude/hooks/session-start.sh` was committed as mode 100644 while both its siblings were 100755, and it is the only hook registered by bare path rather than through `bash`. Its whole body is gated on CLAUDE_CODE_REMOTE=true, so the one environment it does any work in is a Linux web container — exactly where a non-executable checkout cannot run. It is also the script that provisions the Node 24 the engine floor requires, after npm ci EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. The defect was invisible locally: the primary workstation is a Windows ReFS Dev Drive with core.fileMode=false, so git ignores filesystem permission bits and a local `chmod +x` is a silent no-op. Only `git update-index --chmod=+x` can fix it. Fixed three ways so it cannot recur: the index mode, a `bash "..."` registration that stops the mode being load-bearing, and a contract test. Also in this change: - .claude/settings.json gains a permissions block. AGENTS.md's provider confirmation boundary was prose-only; this encodes it as deny/ask rules, including deny on reading .env* (a staging key leaked on 2026-08-18) and on the Supabase MCP write tools. The repo already learned that prose does not hold here — see the comment in pr-handoff-stop.sh. - check-base-freshness.mjs now emits its stale-base warning on stdout as hook JSON. Every human-readable branch used console.error, and Claude Code injects only stdout into context, so the tripwire never reached the agent. The origin/main fetch also gains a 10s timeout so a hung remote cannot burn the whole SessionStart budget. - clean-worktree.mjs gains list-only `--merged` and `--squashed`. Nothing reclaimed merged worktrees, so 49 accumulated, ~19 GB of duplicated node_modules on a 50 GB Dev Drive. Ancestor detection alone finds 2 of 49 because this repo squash-merges; the patch-id test finds 9. A `confidence:` line distinguishes proven from inferred, because the two are not the same claim and one candidate had 2 of 21 files still differing. - Explicit hook timeouts, a PreCompact hook that asks for /issues capture while the context still exists, and a push format guard that only fires where the .githooks pre-push guard is not wired. Removal stays a separate opt-in throughout; `runWorktreeCleanup()` is byte identical, so verify:preflight is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): stop the session-start hook test failing on every Windows run `tests/session-start-hook.test.ts` asserted that the written env file contains `join(home, ".node24", …)`. On Windows `home` comes from mkdtempSync(tmpdir()) as `C:\Users\…\AppData\Local\Temp\session-start-home-XXXX`, while the hook runs under Git Bash and writes the POSIX view of the same directory, `/tmp/ session-start-home-XXXX`. The assertion therefore failed on every Windows run regardless of the diff under test. That is worse than a red test: it is a red test everyone learns to ignore. It fails inside `npm run test`, which is the last step of `verify:pr-local`, so the whole gate goes red locally for every change and the only way to use it is to decide which failures do not count. Compare the path tail instead. The unique mkdtemp basename still pins the assertion to this test's own HOME, so it loses no strength, and it now holds on both platforms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the Claude Code environment review for PR #2113 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(claude): pin the permission boundary and make the PreCompact hook answerable Closes the four items left open when PR #2113 was opened, each with the smallest fix that actually resolves it rather than restating it. 1. The claim that no `allow` rule can reach a provider-backed script was asserted in review and never measured. It is now a test: 36 provider-backed scripts, derived from script names rather than hand-listed, each asserted unreachable through `allow` and covered by an explicit `ask`. Mutation-checked — injecting a broad `Bash(npm run check:*)` allow rule turns 4 of them red. 2. Hook registrations are pinned to invoke through an interpreter rather than a bare path, and to carry an explicit timeout. Bare-path registration is what made the `session-start.sh` mode bug reachable; the mutation check confirms both guards bite. 3. The PreCompact hook's known limit could not be closed by reading code — the installed CLI ships a compiled binary with no inspectable bundle, so whether the platform injects its stdout into model context is not determinable here. Instead of leaving that permanently unverified, the hook now appends one line per firing to a log under the git dir. After the next compaction the log distinguishes "hook never ran" from "hook ran but its output went nowhere", and both answers are actionable. The log lives outside the worktree so it can never be staged. 4. The `newtask` skill said "~40 worktrees". It was 48 on 2026-08-18 and reached 50 during one session, so the number now carries the reason it drifts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(issues): queue the five follow-ups this session could not close Each of these is blocked on something outside the repo, so capturing them is the smallest fix that resolves them — the alternative is that they die with the session context. - P1 rec: PR churn has exhausted both review bots' budgets, so PR #2113 landed with zero automated review and subsequent PRs will too. AGENTS.md already measured the CI half of this cost; this is the second bill and the worse one. Needs a decision on whether the bundling rule gets a gate. - P2 task: nine landed worktrees, ~4.5 GB on a 51%-full Dev Drive, deliberately not removed — re-verification immediately before deletion showed two held unmerged commits despite the scan reporting none minutes earlier, and a third had been switched branches mid-scan by a live session. - P3 task: confirm D:\.npm-cache is a registered Dev Drive trusted cache; fsutil needs elevation and the non-elevated registry fallback reads empty. - P3 task: read the PreCompact hook's own log after the next compaction to settle whether its output reaches model context. The log lives under the worktree's git dir, so check it before cleaning that worktree up. - P3 task: confirm on a real web session that session-start.sh now runs. The mode bug was proven; the failure it would cause on Linux was not, because no container was available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(clean-worktree): stop the candidate listing claiming "0 commits ahead" when it isn't Every `--squashed` candidate printed "0 commits ahead" while being genuinely ahead of origin/main — by 11, 2, 1 commits on the real fleet. A squash-merged branch keeps its original commits forever, so it stays ahead permanently; what is zero is the count of UNLANDED commits, which is a different claim. The line was therefore stating something a reader could disprove with one `git rev-list`, on a tool whose entire job is to be trusted before a deletion. It now reports both numbers: "11 ahead of origin/main, 0 unlanded commits". Also corrects the comment above the ahead check, which described it as belt-and-braces against the merge test. That is true in ancestor mode, where it is the real gate. In squash mode it is not a second opinion at all — gitAheadUnlandedCount returns 0 for any branch the squash test just accepted, so the check is satisfied by construction and can only fire on a candidate that was already skipped. Keeping it is correct; describing it as independent evidence was not. The raw count is reporting only and never gates, so no candidate set changes. Verified against the live fleet: same 9 candidates before and after, worktree count unchanged, --remove not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the follow-up review for PR #2117 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
3 participants
Summary
docs/search-chrome-behaviour.mdnames the DocumentViewer header as the default in-page navigation template for any mode page, but the markup implementing it was duplicated near-verbatim betweenDocumentViewer.tsx:1025-1091anddifferential-detail-page.tsx:893-979. Adopting it on the remaining information pages would have meant a third copy. This is groundwork: it extracts the template so the adoption PRs that follow have one component to mount.InPageNavHeader(src/components/in-page-nav/in-page-nav-header.tsx) — back control, title carrying the active section behind a chevron disclosure, ellipsis page actions, weighted segment track, thePhoneHeaderCollapsePortalwrapper, and both sheets with their open state and return-focus refs. It composes the already-sharedDocumentSectionTrack/DocumentSectionList/Sheetrather than reimplementing them, so pages pass data, not plumbing.PageSection+toDocumentSections(page-section-index.ts):DocumentSectionwith the document-only fields optional, because a service record's Referral section has no magnitude to report the way an 84-page PDF does. The adapter fills the gaps, sosection-index.tsis untouched and the document weight/presence tests are unaffected.usePageSectionWeights: measures rendered section heights so the track reads as "how far through" rather than "which of five", with a floor so a short section never becomes an invisible hairline. When every section carries an explicit weight — discrete tab panels, which have no on-screen height to measure — the hook observes nothing at all and those pages pay none of the cost.differential-detail-page.tsxonto the shared component as a behaviour-neutral proof of the extraction (its localHeaderChromeand both hand-rolled sheets are gone; 207 lines removed).docs/search-chrome-behaviour.mdto name the shared component as the thing to mount, and to record that DocumentViewer has not converged yet.Deliberately not in this PR:
DocumentViewerkeeps its own copy of the header. It owns the page<h1>, uses the distinctedge-glass-headertreatment, and is pinned by visual baselines, so converging it is a clean separate change rather than half of one. Also deferred: generalisinguseDocumentChromeMetricsfor anchor offsets, which only pays off once anchor-scroll pages consume--inpage-anchor-offset— shipping it here would be a hook with nothing on the other end.Test contracts followed the markup, and none were weakened. Three source-scanning contracts pinned the header by reading the differentials page's source; they now assert the structure against the shared component and that the page adopts it, so a page that dropped its navigation entirely still fails. The addon-slot guard in
tests/mode-nav-addon-slot.dom.test.tsxnow scans forInPageNavHeaderas well as the raw portal — without that it would have gone silent for every future adopter, which is precisely the regression it exists to catch.Verification
npm run verify:pr-local— exit 1, from a single environment-dependent failure, detailed below. Everything else in its plan passed:5618 passed | 1 failed | 4 skipped (5623).npm run verify:cheap— same single failure, same counts:Test Files 1 failed | 523 passed (524).npm run build— run separately, becauseverify:pr-localshort-circuits attestand so never reached its ownbuildandcheck:rag:fixturessteps:✓ Compiled successfully in 53s,Client bundle secret surface check passed.npm run verify:phone-chrome— exit 0. Stage 1 (browser/PWA ownership matrix + affected phone journeys):Test Files 9 passed (9),Tests 119 passed (119). Stage 2 (focused browser):7 passed (23.5s).npm run lint,npm run typecheck, and whole-treenpx prettier --check .— all clean.npm run verify:uiwas not selected. The phone-chrome smart selector reportedFull UI policy: auto (not selected)— "Focused ownership and journey coverage is sufficient for this page-local or test-infrastructure scope." No page's chrome changes behaviour in this PR beyond the one already-converted route, so that selection looks right; if a reviewer reads a new shared component as shared-foundation scope, the full Chromium gate is the escalation.eval:*commands andcheck:production-readiness.classifyPullRequestFilesreturnsclinicalRisk: false,ragRanking: false,operationalRisk: false,ui: truefor this diff, so noRAG impact:line or Clinical Governance Preflight is required and neither section is included.npm run verify:release— provider-backed, and no release or handoff-confidence claim is being made here.The one failing test, and why it is not this change
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file existsfails in this environment. It is not caused by this PR and is not repo breakage:chmodSync(gitDir, 0o555)to make the marker write fail, then asserts the hook fails open with no marker.id -u→0), and root bypasses permission bits, so the write succeeds andmarkerExistsreturnstrue.touchinto a0555directory succeeds here.Environment caveat on all local evidence
The lockfile could not be installed under the repo's
engine-strict=true:jsdom@30.0.1requires Node^22.22.2 || ^24.15.0 || >=26.0.0, and the newest Node in this container is 24.13.0 (the only alternative, 22.22.2, fails the repo's ownengines: {node: "24.x"}). With the user's explicit approval, dependencies were installed once with the engine check relaxed. This installs the exact locked versions and leavespackage.jsonandpackage-lock.jsonuntouched (confirmed viagit status), but every result above ran with jsdom below its declared floor. CI is the first run on a compliant runtime.Separately, the Playwright browser preflight initially failed because the image ships Chromium build 1194 while Playwright 1.62.1 expects headless-shell build 1234. The browser stage was run with
PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHpointed at the pre-installed Chromium 141 — the escape hatch named in the gate's own error message — rather than downloading browsers.Risk and rollout
relativeon the header is load-bearing (the track is absolutely positioned against it), and the component must never grow a scroll-hide hook of its own, since belowsmthe universal collapse row owns the motion. Both are pinned by tests.git revertthis single commit. It is self-contained — three new files plus one page conversion — and nothing else imports the new module yet.Notes
page-secondary-navigation.tsx/secondary-navigation.tsx; (3) convert the locally-owned routes — medications, factsheets, differentials presentations./dsm/diagnoses/<slug>declares five sections but only renders three anchors (key-featuresandrecord-summaryare missing; Key features carriesid="key-features-title"on its<h2>only), and/dsm/diagnoses/<slug>/differentialsdeclares four but renders onlydsm-differential-filters./factsheets/<slug>renders no section ids at all, and its list is dynamic per factsheet kind viatocFor(), so its anchors must be generated from that same source rather than hardcoded./issues #256's claim that/forms/<slug>renders no navigation is stale — all six anchors are present today (form-detail-page.tsx:700,823,835,859,882,891,897,904), andform-decision-context-mobileis now both an element id and a testId. Worth updating that ledger entry.MobileTabsis cross-route navigation (three of four links leave the page), whichdocs/search-chrome-behaviour.mdexplicitly excludes from this template. The plan is to move those destinations into the actions sheet so the page keeps one navigation owner — a real behaviour change, not a like-for-like swap.🤖 Generated with Claude Code
https://claude.ai/code/session_01GyM49edHSNfm42u1SFDfG3
Generated by Claude Code