Skip to content

docs(issues): confirm #256's dead section sets; capture two mode-nav follow-ups - #1685

Merged
BigSimmo merged 10 commits into
mainfrom
claude/handover-review-nlhuln
Aug 7, 2026
Merged

docs(issues): confirm #256's dead section sets; capture two mode-nav follow-ups#1685
BigSimmo merged 10 commits into
mainfrom
claude/handover-review-nlhuln

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

Ledger-only. docs/outstanding-issues.md, +4/−2, no code.

  • #256 moves from "suspected" to confirmed, with the evidence. It previously recorded form-overview and the differential-presentation-* set as suspected remaining after PR feat(navigation): roll the shared mode nav out to DSM, Specifiers, Formulation and Differentials #1647 fixed specifiers/formulation. Both are now confirmed dead, so two live clinical routes draw no section navigation at all:

    • /forms/<slug>form-detail-page.tsx contains zero id= attributes. form-decision-context-mobile exists at line 881 but as a testId prop, which PathwayContextCard renders as data-testid only (line 294) — never an id. The other five declared targetIds are rendered nowhere.
    • /differentials/presentations/<slug> — all six differential-presentation-* targetIds absent from differential-presentation-workflow-page.tsx.

    #256's own guidance warns against auditing by grepping id= alone, because some live sections render through a sectionId prop. That was checked rather than skipped: the only template-literal section id in this family is differential-section-${section.id} in differential-detail-page.tsx — a different prefix on a different route — so nothing is hiding behind indirection in either file. The row now carries that check so the next person doesn't repeat it.

  • #261 (P3, new) — decide whether to delete SecondaryNavigationActionItem. PR refactor(navigation): remove the vestigial one-button mode strip #1679 removed its last live consumer. Kept deliberately there: it carries the tablist roving-focus behaviour and has its own component tests, and check:knip runs without --include exports so the dead-code gate will never flag it either way. The row records the keep-or-delete decision and the stop rule — don't delete the branch while leaving its tests, or vice versa.

  • #262 (P3, new) — the header addon-slot single-owner rule is enforced by two independently maintained lists agreeing by coincidence, not by a guard. The original incidental protection (a claimant mode having fewer than MODE_NAV_MIN_ITEMS destinations) has now expired twice, in feat(navigation): roll the shared mode nav out to DSM, Specifiers, Formulation and Differentials #1647 and feat(navigation): adopt the shared mode nav for Factsheets #1674. No action needed while the lists agree; the row names the trigger and the fix so it isn't rediscovered from scratch.

Opened rather than left local because the container is ephemeral and the #256 diagnosis took real digging — the same stranding failure #260 already tracks.

RAG impact: no retrieval behaviour change — documentation only.

Verification

  • npm run check:outstanding-issuesOutstanding-issues guard passed: 260 rows (119 open, 141 archived), unique ids, next-id=263 above the highest, no merge driver, no ids deleted from base 1ff9ed206456. The "no ids deleted from base" clause is the one that matters for a ledger edit — it is what catches a merge silently dropping rows.
  • npx prettier --check docs/outstanding-issues.mdAll matched files use Prettier code style!

All three rows were written through scripts/outstanding-issues.mjs (add / update), never hand-edited, per the ledger's own rule. #207, #226 and #231 were reviewed in the same pass and deliberately not touched — they are existing P1 rows and re-adding them would fragment live issues into duplicates.

No code gates run: this diff contains no executable code.

Risk and rollout

  • Risk: None to the product. docs/outstanding-issues.md has no merge driver by design, so an overlapping edit conflicts loudly rather than silently concatenating — if this sits while another session edits the ledger, resolve by re-applying these three rows through the writer script, never by taking one side wholesale.
  • Rollback: Single revert.
  • Provider or production effects: None.

Notes

Closes out the mode-navigation handover (#1642#1647#1674#1679). All 13 modes now carry navigation matching what they actually contain, and the loose ends found along the way are recorded rather than left in chat.

#256 is now specced end to end — element-by-element mapping for both routes, including the two places where no element exists to anchor and the -mobile variants on /differentials/presentations/ that can never resolve because ReviewPanels renders twice, not three times. One open question needs an operator call before that work starts: whether that route wants an in-page section nav at all, given it already renders a <nav aria-label="Differential presentation sections"> below xl whose links go to other routes rather than to in-page anchors.


Generated by Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a review record documenting the latest investigation, findings, and verification checks.
    • Updated the outstanding-issues ledger with confirmed inactive navigation targets.
    • Added tracking items for an unused navigation option and the universal header add-on slot behavior.
    • Advanced the issue tracking marker to reflect the newly recorded items.

…n-kind decision
#256 was 'suspected remaining' for form-overview and the
differential-presentation-* set. Both are now confirmed dead, so two live
routes draw no section nav at all: /forms/<slug> (one anchor is a testId
rather than an element id, the other five are rendered nowhere) and
/differentials/presentations/<slug> (all six absent, and the only dynamic
section id in that family uses a different prefix on a different route,
so nothing is hiding behind a sectionId prop).
Also captures #261: whether to delete SecondaryNavigationActionItem,
which lost its last live consumer in PR #1679 and was deliberately kept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01823Ctwj4vinGhGuRNyK7oE
@supabase

supabaseBot commented Aug 7, 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 ↗︎.

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@coderabbitai

coderabbitaiBot commented Aug 7, 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:54 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: 4dae30e6-f00a-4d45-bb26-25075c066679

📥 Commits

Reviewing files that changed from the base of the PR and between e200e58 and 805b968.

📒 Files selected for processing (1)
  • docs/branch-review-ledger.md
📝 Walkthrough

Walkthrough

The pull request updates two documentation ledgers. It records the mode-navigation follow-up, confirms missing navigation anchors, advances the issue marker, and adds issues for navigation and header ownership decisions.

Changes

Ledger documentation

Layer / File(s)Summary
Review and issue ledger updates
docs/branch-review-ledger.md, docs/outstanding-issues.md
The outstanding-issues ledger advances its next issue marker, expands issue #256, and adds issues #271 and #272. The branch ledger records the follow-up investigation and validation checks.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

Suggested reviewers:claude, cursoragent

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Description check⚠️ WarningThe description includes the required sections and verification details, but it uses outdated issue numbers and ledger counts that do not match the final changes.Update the description to use issues #271 and #272 and the final ledger totals, including next-id=273 and 270 rows.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately identifies the documentation change, confirmed dead section targets, and two follow-up issues.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/handover-review-nlhuln

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

Resolve outstanding-issues conflict by keeping main's #261-#270 design-system tracks and renumbering this PR's SecondaryNavigation and addon-slot follow-ups to #271/#272. Preserve the confirmed #256 diagnosis from this branch.
Co-authored-by: Cursor <cursoragent@cursor.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

Merge conflict resolved by merging origin/main.

docs/outstanding-issues.md collided because main had already taken #261#270 for the design-system tracks from PR #1678. Kept those rows, preserved this PR's confirmed #256 diagnosis, and renumbered this PR's follow-ups:

  • SecondaryNavigation action-kind decision: #261#271
  • Header addon-slot single-owner guard: #262#272

issues:next-id is now 273. Local gate: Outstanding-issues guard passed: 270 rows (129 open, 141 archived), unique ids, next-id=273 above the highest.

Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 704: Append a superseding correction to the branch-review ledger using
the ledger command, updating the references from `#261/`#262 to `#271/`#272 and the
validation counts to 270 rows, 129 open, and 141 archived. Do not modify or
delete the existing row; follow the append-only workflow rather than editing the
file directly.
🪄 Autofix

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6c568c7b-ab93-4614-a293-b9c4f50212c4

📥 Commits

Reviewing files that changed from the base of the PR and between 1c979df and e200e58.

📒 Files selected for processing (2)
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md

Comment threaddocs/branch-review-ledger.md
@BigSimmo
BigSimmo enabled auto-merge August 7, 2026 14:42
@coderabbitai

coderabbitaiBot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

  • docs/branch-review-ledger.md

Commit:ad218df50c0af8fc6e5a5b44d6a147b0d209549b

The changes have been pushed to the claude/handover-review-nlhuln branch.

Time taken:2m 57s

coderabbitaiBotand others added 2 commits August 7, 2026 14:47
Fixed 1 file(s) based on 1 unresolved review comment.
Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
@BigSimmo
BigSimmo merged commit 0dcb5c5 into mainAug 7, 2026
23 checks passed
@BigSimmo
BigSimmo deleted the claude/handover-review-nlhuln branch August 7, 2026 15:01
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@claude@cursoragent