issues: rewrite #287 from the full phase 3 survey - #1764
Closed
BigSimmo wants to merge 2 commits into
Closed
Conversation
Records the owner decision that /medications keeps tab-swapping, which makes that route the cheapest of the three rather than the blocked one, and adds the findings from reading all three components end to end: factsheets has 17 sections not 6 and an unguarded heading-to-id contract, and presentations is a Server Component whose conversion touches four shared panels and five pinned test assertions. #256 is deliberately untouched — it was corrected on PR #1761 before merge and that version is better than the one this branch carried. 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. |
Contributor
📝 WalkthroughWalkthroughIssue ChangesInformation-page route planning
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/outstanding-issues.md`:
- Line 329: Move heading-ID uniqueness validation from the data generator to the
final per-factsheet ID-building path used by FactsheetDetailPage, so it includes
both static headings and medLite-generated sections. Ensure collisions across
all rendered sections fail validation, using a rendered-DOM test only if no
shared ID builder exists.
- Line 329: Update the factsheets page’s generated “On this page” list to render
each entry as a keyboard-accessible navigation control targeting the
corresponding generated section ID, rather than plain li text. Keep the list
derived from rendered sections instead of tocFor, and ensure each target uses
the same IDs assigned by the section generator.
🪄 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: fc6f43ee-a872-47ec-a04c-0ea0073c2baa
📒 Files selected for processing (1)
docs/outstanding-issues.md
| | #285 | P2 | issue | Fresh remote/Cloud containers cannot run npm ci — shipped Node 24.13.0 is below the ^24.15.0 floor that main's jsdom@30.0.1 now requires | Observed 2026-08-08 in a Claude Code web container while syncing PR #1730. npm ci --include=dev aborts with EBADENGINE on jsdom@30.0.1 (needs Node ^22.22.2, ^24.15.0 or >=26); the container ships v24.13.0, so node_modules stays stale and the pre-push static guard then fails typecheck on the missing tailwind-merge added by #1678. Worked around by nvm install 24.19.0 plus a PATH prefix (nvm use alone does not stick — system node shadows it). Next action: raise the engines.node floor in package.json to >=24.15 so the mismatch fails loudly at the declared contract, and provision a compatible Node in the remote/Cloud setup path so a fresh container is not blocked at first install. | session 2026-08-08 (PR #1730) | 2026-08-08 | | ||
| | #286 | P2 | task | PR 2 of the in-page nav series - convert the six pill-rail information pages onto InPageNavHeader | **Outcome:** the six routes still drawing the shell-owned pill rail use the documented default in-page navigation template, and the rail behind them is deleted. **Detail:** PR 1 landed as 2806d5e (#1740), extracting InPageNavHeader plus PageSection/toDocumentSections and usePageSectionWeights into src/components/in-page-nav/, and converting differential-detail-page.tsx as proof. Remaining routes: /services/[slug], /forms/[slug], /specifiers/[slug] (record and reference), /formulation/[slug], /dsm/diagnoses/[slug], /dsm/diagnoses/[slug]/differentials. Settled with the owner: replace the rail at every width, drop the InformationPageBreadcrumbs row but keep the large in-body h1, move each record's actions into the ellipsis sheet. **Next:** FIRST widen InPageNavHeader actions from the render prop (close) => ReactNode to ReactNode or ((close) => ReactNode) and close both sheets on pathname change - four of the seven components in scope are Server Components (specifier-record-page, specifier-reference-page, formulation-mechanism-page, dsm-diagnosis-page carry no use client) and React refuses to pass a function across that boundary, so all four fail to build against the current signature. Then add use-resolved-page-sections and use-in-page-section-nav, generalise useDocumentChromeMetrics for an --inpage-anchor-offset, add scroll-mt to every anchor (information-page sections carry none today), convert each page, then delete informationPageSectionDefinitions and the section kind in secondary-navigation.tsx. Two DSM routes declare anchors nothing renders - see #256. Full brief: artifact 77de9f5b-d195-409d-8156-2e4b41cd2f45. **Stop:** do not convert DocumentViewer in this PR (its chrome-metric property names are contract-pinned by tests/header-scroll-hide-contract.test.ts:110-113), and do not verify anchors by grepping for id= - assert against the rendered DOM per route. | session 2026-08-08; PR #1740 (2806d5e) | 2026-08-08 | | ||
| | #287 | P2 | task | PR 3 of the in-page nav series - the three locally-owned routes each need a decision, not just a conversion | **Outcome:** every information page uses the documented in-page navigation template, or has a recorded reason not to. **Detail:** the last three routes each own a different bespoke pattern, and none is a mechanical port. (1) /medications/[slug] - SectionTabs at medication-record-page.tsx:183 SWAPS CONTENT rather than scrolling: sectionsByTab[activeTab] at :391 filters record.sections by type, so a different set mounts per tab. The InPageNavHeader track is scroll-spy over anchors that all exist at once, so adopting it means either driving tab state from the track (the track stops meaning where am I on the page) or flattening to one scrolling page - a real behaviour change to a clinical record, and a product call. (2) /differentials/presentations/[slug] - MobileTabs at differential-presentation-workflow-page.tsx plus the xl review sidebar; the old `differentialPresentationSections` shell set is gone and the route is locally owned (`page-secondary-navigation.tsx`). Remaining work is the product decision to adopt `InPageNavHeader` (or keep the tab/sidebar model with a recorded reason), not resurrecting deleted section targetIds. (3) /factsheets/[slug] - the On this page list at factsheet-detail-page.tsx:365-371 is li text with no link, button or handler, and the sections themselves carry no ids at all (:214, :267, :313, :447, :453, :471); the tail is data-driven via factsheet.sections.map keyed on section.heading (:538), so anchor ids must be generated deterministically from headings and that generator becomes the contract the section list depends on. Therapy Compass is deliberately excluded from the whole series - ModeNav is a different multi-route pattern. **Next:** decide the medications tab model first (owner decision, blocks planning); decide whether presentations keep MobileTabs/sidebar or adopt InPageNavHeader; choose the factsheets heading-to-id scheme. Then convert. **Stop:** do not port medications mechanically - swapping the tablist for a scroll track silently changes what a clinician sees on a medication record. | session 2026-08-08; follows #286 | 2026-08-08 | | ||
| | #287 | P2 | task | PR 3 of the in-page nav series - the three locally-owned routes each need a decision, not just a conversion | **Outcome:** every information page uses the documented in-page navigation template, or has a recorded reason not to. **Detail:** the last three routes each own a different bespoke pattern. Fully surveyed 2026-08-08 (all three components and their tests read end to end); this supersedes the initial sketch. (1) /medications/[slug] - SectionTabs (medication-record-page.tsx:175-181; tabs summary/dosing/safety/more) SWAPS CONTENT: sectionsByTab at :378-389 buckets record.sections by type, activeTab is local useState at :376 with no URL sync, and only activeSections is mapped into the single role=tabpanel at :444-464, so non-active sections have NO DOM nodes. OWNER DECISION 2026-08-08: keep tab-swapping and drive it from the header track. That makes this the CHEAPEST of the three and unblocks it - differential-detail-page.tsx:1041-1066 is an exact precedent (also panel-swap, maps activeId onto tab state, passes explicit weights because there is nothing on screen to measure, per page-section-index.ts:31-36). Sections carry no id in the data (MedicationSection = title/type/rows, src/lib/medications.ts:20-24, confirmed across all 328 snapshot records), so build the four PageSections from the tab ids. The page has NO page-level actions and no role=status notice, so omit actions entirely. Watch two data facts: the safe type in safetyTypes occurs zero times, and spec/sel/comp appear in 7/1/1 records, so the more tab is empty for most drugs. Placement needs width=bleed or mounting outside InformationPageShell (:504), and testId medication-page-<slug> is load-bearing for ui-smoke.spec.ts:129. Needs NOTHING from #286 - buildable against merged PR 1 alone. (2) /differentials/presentations/[slug] - the shell set is gone and the route is locally owned; remaining work is a product decision, not a bug fix. This is the HARDEST of the three. MobileTabs (:595-630) is a CROSS-ROUTE link strip: Overview/Map/Related navigate to the first candidate diagnosis, only Compare is a self-link, so those three destinations must move into the actions sheet or be dropped - they are not sections. Blockers: the page is a Server Component (no use client) so it needs converting or a client wrapper; four panels take only {workflow} and each render 2-3 times across breakpoints, so ids must be threaded as props (SafetySnapshot :288, HighestUrgencyPanel :372, ReviewPanel :397, SourceStatusPanel :454); ReviewPanels has NO phone render path (:753 tablet, :759 desktop only), which is why three of the old sixteen targetIds were unbuildable - do not repeat that; and five pinned assertions must change deliberately (four MobileTabs strings at tests/mobile-interaction-regressions.test.ts:54-70, which encode a past regression, plus tests/mode-nav-addon-slot.dom.test.tsx:46 asserting the route does not own the slot). Leave the phone footer and the main className alone - both contract-pinned. Fold in one real a11y gap: CopyAfterReviewButton signals success only by swapping its own label, with no live region anywhere in the feature. (3) /factsheets/[slug] - there are 17 section elements, not the 6 first listed, and NONE carries an id (the only DOM id in the 854-line file is unrelated). The On this page list at :364-373 is li text with no link, button or handler, and it escapes require-button-wiring because it is not a button. Its source tocFor (factsheets-data.ts:750-770) is a hand-maintained per-kind list, wrong in both directions: it names four blocks that are divs not sections (:431, :512, :602, :719), mislabels three, and omits two real sections (:267, :313). Build the list from what renders, not from tocFor; note :267 is conditional on moreInTopic.length. Only medLite has data-driven sections (2 of 8 sheets, 6 total, factsheets-data.ts:83-87 = {heading, body}, no id/slug), and headings are unique today but UNASSERTED - escitalopram's What it is / What to expect are byte-identical to static therapy headings at :628/:660 - so ship a uniqueness assertion with the generator. Moving Save into the sheet breaks tests/factsheet-save.dom.test.tsx:17-25 and orphans the sr-only live region at :158, which must stay outside the sheet. Do NOT rename or extract FactsheetPrintSheet (:733): design-system-contract-utils.test.ts:372-386 fails closed without it. Back destination changes deliberately from the breadcrumb's /factsheets/search to appModeHomeHref(factsheets). Therapy Compass stays excluded - ModeNav is a different multi-route pattern. **Next:** recommended order medications, then factsheets, then presentations as its own PR - cheapest and safest first, now that presentations is known not to carry a live bug. Factsheets and presentations need #286's scroll-spy hooks and anchor-offset generalisation; medications does not. Full brief: the PR 3 handover artifact published 2026-08-08. **Stop:** do not re-add a section set for the presentations route - tests/page-secondary-navigation.dom.test.tsx:78-90 forbids it. | session 2026-08-08; follows #286 | 2026-08-08 | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate heading IDs at the rendered-page boundary.
Line 329 places the uniqueness assertion in the generator, but it also identifies static headings in FactsheetDetailPage at lines 628 and 660. factsheets-data.ts cannot validate those headings or collisions between static and medLite sections. Validate the final per-factsheet section list in the shared ID builder or a rendered-DOM test.
🤖 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 `@docs/outstanding-issues.md` at line 329, Move heading-ID uniqueness
validation from the data generator to the final per-factsheet ID-building path
used by FactsheetDetailPage, so it includes both static headings and
medLite-generated sections. Ensure collisions across all rendered sections fail
validation, using a rendered-DOM test only if no shared ID builder exists.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the generated factsheet list navigable.
Line 329 records that the current On this page entries are plain <li> text with no link, button, or handler. The next step only replaces tocFor with rendered sections. Require each entry to target its generated section ID with an anchor or equivalent keyboard-accessible control. Otherwise, adding section IDs does not complete factsheet navigation.
🤖 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 `@docs/outstanding-issues.md` at line 329, Update the factsheets page’s
generated “On this page” list to render each entry as a keyboard-accessible
navigation control targeting the corresponding generated section ID, rather than
plain li text. Keep the list derived from rendered sections instead of tocFor,
and ensure each target uses the same IDs assigned by the section generator.
![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: ecc2f7a4f2
ℹ️ 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".
| | #285 | P2 | issue | Fresh remote/Cloud containers cannot run npm ci — shipped Node 24.13.0 is below the ^24.15.0 floor that main's jsdom@30.0.1 now requires | Observed 2026-08-08 in a Claude Code web container while syncing PR #1730. npm ci --include=dev aborts with EBADENGINE on jsdom@30.0.1 (needs Node ^22.22.2, ^24.15.0 or >=26); the container ships v24.13.0, so node_modules stays stale and the pre-push static guard then fails typecheck on the missing tailwind-merge added by #1678. Worked around by nvm install 24.19.0 plus a PATH prefix (nvm use alone does not stick — system node shadows it). Next action: raise the engines.node floor in package.json to >=24.15 so the mismatch fails loudly at the declared contract, and provision a compatible Node in the remote/Cloud setup path so a fresh container is not blocked at first install. | session 2026-08-08 (PR #1730) | 2026-08-08 | | ||
| | #286 | P2 | task | PR 2 of the in-page nav series - convert the six pill-rail information pages onto InPageNavHeader | **Outcome:** the six routes still drawing the shell-owned pill rail use the documented default in-page navigation template, and the rail behind them is deleted. **Detail:** PR 1 landed as 2806d5e (#1740), extracting InPageNavHeader plus PageSection/toDocumentSections and usePageSectionWeights into src/components/in-page-nav/, and converting differential-detail-page.tsx as proof. Remaining routes: /services/[slug], /forms/[slug], /specifiers/[slug] (record and reference), /formulation/[slug], /dsm/diagnoses/[slug], /dsm/diagnoses/[slug]/differentials. Settled with the owner: replace the rail at every width, drop the InformationPageBreadcrumbs row but keep the large in-body h1, move each record's actions into the ellipsis sheet. **Next:** FIRST widen InPageNavHeader actions from the render prop (close) => ReactNode to ReactNode or ((close) => ReactNode) and close both sheets on pathname change - four of the seven components in scope are Server Components (specifier-record-page, specifier-reference-page, formulation-mechanism-page, dsm-diagnosis-page carry no use client) and React refuses to pass a function across that boundary, so all four fail to build against the current signature. Then add use-resolved-page-sections and use-in-page-section-nav, generalise useDocumentChromeMetrics for an --inpage-anchor-offset, add scroll-mt to every anchor (information-page sections carry none today), convert each page, then delete informationPageSectionDefinitions and the section kind in secondary-navigation.tsx. Two DSM routes declare anchors nothing renders - see #256. Full brief: artifact 77de9f5b-d195-409d-8156-2e4b41cd2f45. **Stop:** do not convert DocumentViewer in this PR (its chrome-metric property names are contract-pinned by tests/header-scroll-hide-contract.test.ts:110-113), and do not verify anchors by grepping for id= - assert against the rendered DOM per route. | session 2026-08-08; PR #1740 (2806d5e) | 2026-08-08 | | ||
| | #287 | P2 | task | PR 3 of the in-page nav series - the three locally-owned routes each need a decision, not just a conversion | **Outcome:** every information page uses the documented in-page navigation template, or has a recorded reason not to. **Detail:** the last three routes each own a different bespoke pattern, and none is a mechanical port. (1) /medications/[slug] - SectionTabs at medication-record-page.tsx:183 SWAPS CONTENT rather than scrolling: sectionsByTab[activeTab] at :391 filters record.sections by type, so a different set mounts per tab. The InPageNavHeader track is scroll-spy over anchors that all exist at once, so adopting it means either driving tab state from the track (the track stops meaning where am I on the page) or flattening to one scrolling page - a real behaviour change to a clinical record, and a product call. (2) /differentials/presentations/[slug] - MobileTabs at differential-presentation-workflow-page.tsx plus the xl review sidebar; the old `differentialPresentationSections` shell set is gone and the route is locally owned (`page-secondary-navigation.tsx`). Remaining work is the product decision to adopt `InPageNavHeader` (or keep the tab/sidebar model with a recorded reason), not resurrecting deleted section targetIds. (3) /factsheets/[slug] - the On this page list at factsheet-detail-page.tsx:365-371 is li text with no link, button or handler, and the sections themselves carry no ids at all (:214, :267, :313, :447, :453, :471); the tail is data-driven via factsheet.sections.map keyed on section.heading (:538), so anchor ids must be generated deterministically from headings and that generator becomes the contract the section list depends on. Therapy Compass is deliberately excluded from the whole series - ModeNav is a different multi-route pattern. **Next:** decide the medications tab model first (owner decision, blocks planning); decide whether presentations keep MobileTabs/sidebar or adopt InPageNavHeader; choose the factsheets heading-to-id scheme. Then convert. **Stop:** do not port medications mechanically - swapping the tablist for a scroll track silently changes what a clinician sees on a medication record. | session 2026-08-08; follows #286 | 2026-08-08 | | ||
| | #287 | P2 | task | PR 3 of the in-page nav series - the three locally-owned routes each need a decision, not just a conversion | **Outcome:** every information page uses the documented in-page navigation template, or has a recorded reason not to. **Detail:** the last three routes each own a different bespoke pattern. Fully surveyed 2026-08-08 (all three components and their tests read end to end); this supersedes the initial sketch. (1) /medications/[slug] - SectionTabs (medication-record-page.tsx:175-181; tabs summary/dosing/safety/more) SWAPS CONTENT: sectionsByTab at :378-389 buckets record.sections by type, activeTab is local useState at :376 with no URL sync, and only activeSections is mapped into the single role=tabpanel at :444-464, so non-active sections have NO DOM nodes. OWNER DECISION 2026-08-08: keep tab-swapping and drive it from the header track. That makes this the CHEAPEST of the three and unblocks it - differential-detail-page.tsx:1041-1066 is an exact precedent (also panel-swap, maps activeId onto tab state, passes explicit weights because there is nothing on screen to measure, per page-section-index.ts:31-36). Sections carry no id in the data (MedicationSection = title/type/rows, src/lib/medications.ts:20-24, confirmed across all 328 snapshot records), so build the four PageSections from the tab ids. The page has NO page-level actions and no role=status notice, so omit actions entirely. Watch two data facts: the safe type in safetyTypes occurs zero times, and spec/sel/comp appear in 7/1/1 records, so the more tab is empty for most drugs. Placement needs width=bleed or mounting outside InformationPageShell (:504), and testId medication-page-<slug> is load-bearing for ui-smoke.spec.ts:129. Needs NOTHING from #286 - buildable against merged PR 1 alone. (2) /differentials/presentations/[slug] - the shell set is gone and the route is locally owned; remaining work is a product decision, not a bug fix. This is the HARDEST of the three. MobileTabs (:595-630) is a CROSS-ROUTE link strip: Overview/Map/Related navigate to the first candidate diagnosis, only Compare is a self-link, so those three destinations must move into the actions sheet or be dropped - they are not sections. Blockers: the page is a Server Component (no use client) so it needs converting or a client wrapper; four panels take only {workflow} and each render 2-3 times across breakpoints, so ids must be threaded as props (SafetySnapshot :288, HighestUrgencyPanel :372, ReviewPanel :397, SourceStatusPanel :454); ReviewPanels has NO phone render path (:753 tablet, :759 desktop only), which is why three of the old sixteen targetIds were unbuildable - do not repeat that; and five pinned assertions must change deliberately (four MobileTabs strings at tests/mobile-interaction-regressions.test.ts:54-70, which encode a past regression, plus tests/mode-nav-addon-slot.dom.test.tsx:46 asserting the route does not own the slot). Leave the phone footer and the main className alone - both contract-pinned. Fold in one real a11y gap: CopyAfterReviewButton signals success only by swapping its own label, with no live region anywhere in the feature. (3) /factsheets/[slug] - there are 17 section elements, not the 6 first listed, and NONE carries an id (the only DOM id in the 854-line file is unrelated). The On this page list at :364-373 is li text with no link, button or handler, and it escapes require-button-wiring because it is not a button. Its source tocFor (factsheets-data.ts:750-770) is a hand-maintained per-kind list, wrong in both directions: it names four blocks that are divs not sections (:431, :512, :602, :719), mislabels three, and omits two real sections (:267, :313). Build the list from what renders, not from tocFor; note :267 is conditional on moreInTopic.length. Only medLite has data-driven sections (2 of 8 sheets, 6 total, factsheets-data.ts:83-87 = {heading, body}, no id/slug), and headings are unique today but UNASSERTED - escitalopram's What it is / What to expect are byte-identical to static therapy headings at :628/:660 - so ship a uniqueness assertion with the generator. Moving Save into the sheet breaks tests/factsheet-save.dom.test.tsx:17-25 and orphans the sr-only live region at :158, which must stay outside the sheet. Do NOT rename or extract FactsheetPrintSheet (:733): design-system-contract-utils.test.ts:372-386 fails closed without it. Back destination changes deliberately from the breadcrumb's /factsheets/search to appModeHomeHref(factsheets). Therapy Compass stays excluded - ModeNav is a different multi-route pattern. **Next:** recommended order medications, then factsheets, then presentations as its own PR - cheapest and safest first, now that presentations is known not to carry a live bug. Factsheets and presentations need #286's scroll-spy hooks and anchor-offset generalisation; medications does not. Full brief: the PR 3 handover artifact published 2026-08-08. **Stop:** do not re-add a section set for the presentations route - tests/page-secondary-navigation.dom.test.tsx:78-90 forbids it. | session 2026-08-08; follows #286 | 2026-08-08 | |
There was a problem hiding this comment.
When a session selects #287 from the recommended execution queue, line 167 still says After #286; medications needs an owner product call and instructs the reader to decide the tab model, while this rewritten row says the owner decision is complete and medications needs nothing from #286. Because this ledger declares task order and dependencies canonical, the contradictory entry can unnecessarily block the medication conversion or reopen an already-settled decision; update the queue row alongside this detail.
Useful? React with 👍 / 👎.
BigSimmo
enabled auto-merge
August 9, 2026 01:46
BigSimmo
disabled auto-merge
August 9, 2026 02:05
BigSimmo
enabled auto-merge
August 9, 2026 02:11
BigSimmo
disabled auto-merge
August 9, 2026 02:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to PR #1761. That PR captured the in-page navigation series as
#286–#288from a sketch;#287(phase 3 — medications, factsheets, presentations) has since been planned properly, so this replaces its detail with what the survey actually found./medications/[slug]. The row previously said the tab model "blocks planning". It has been decided: keep tab-swapping and drive it from the header track. That inverts the route's difficulty — it becomes the cheapest of the three, becausedifferential-detail-page.tsx:1041-1066is an exact precedent (also panel-swap, also mapsactiveIdonto tab state, also passes explicit weights). It also needs nothing from#286, so it can be built against merged PR feat(in-page-nav): extract the default in-page navigation template #1740 alone.<section>elements, not the 6 the row listed, and none carries an id. The "On this page" list's source,tocFor, is hand-maintained and wrong in both directions — it names four blocks that are<div>s rather than sections, mislabels three, and omits two real sections. OnlymedLitehas data-driven sections, and their headings are unique today but unasserted, with two already byte-identical to static headings in another kind — so the heading-to-id generator needs a uniqueness assertion shipped with it.idprop while rendering 2–3 times across breakpoints,ReviewPanelshas no phone render path at all, and five pinned test assertions must change deliberately.MobileTabsis a cross-route link strip, so three of its four destinations are not sections and must be re-homed or dropped.#286.#256is deliberately untouched. It was corrected on PR #1761 before merge, and that version is more accurate than the one this branch started from; re-applying the local edit would have regressed it.Verification
Docs-only change to
docs/outstanding-issues.md(1 insertion, 1 deletion — a single row rewritten). No executable, RAG-ranking, or clinical-risk paths are touched.npm run check:outstanding-issues—Outstanding-issues guard passed: 286 rows (142 open, 144 archived), unique ids, next-id=289 above the highest, no merge driver, no ids deleted from base 8db1e53937c6.npx prettier --check .—All matched files use Prettier code style!(whole tree)git diffconfirmed to touch no row other than#287.Not run, and why: no behavioural gate applies to a ledger edit.
verify:pr-local,verify:ui,verify:release, the retrieval/answer evals andcheck:production-readinessall cover failure classes this diff cannot reach.Risk and rollout
Negligible. One row of the outstanding-work ledger; no product code, schema, or configuration. Rollback is
git revertof the single commit.The branch was restarted from
origin/mainafter PR #1761 merged and its head was auto-deleted. The row was rewritten withnpm run issues:updaterather than by hand, and re-applied onto currentmainrather than cherry-picked, becausedocs/outstanding-issues.mdhas no merge driver by design and a cherry-pick conflicted — the repo's documented resolution is to rebuild frommainand re-apply only the changed row.Notes
The findings behind this rewrite come from reading all three components and their tests end to end, including which tests each conversion will break. That detail lives in a handover document rather than the ledger row, which carries only what a future session needs to plan from.
Generated by Claude Code
Summary by CodeRabbit