Make the nav density profile a required choice, and state review status on the two therapy listings that quote a record - #2728
Conversation
…te review status on the two listings that quote a record Two root causes from the PR #2686 session, both small. 1. A rail could inherit a density profile it never chose. InPageSectionRail took `density` as an optional prop with a default, so medication's rail sat on Therapy's `extended` profile without naming it. That coupling was invisible to `registryModeNavDensityProfiles`, which lists only the modes that render the top bar, so checking that registry said the change was Therapy-only when it was not. Retuning `extended` unfolded medication's counted labels early, and only ui-smoke's prescribing journey caught it. `density` is now required at both the rail and the header layer. A default cannot be safe here: the value is calibrated against one label family's measured widths, so inheriting another's is always a guess. Making it required costs each call site one line and makes the consumer set answerable by `git grep "density:"`. The compiler immediately found two more silent inheritors in tests, which is the fix demonstrating itself. 2. Two listings quoted a record's clinical prose without its review state. Related therapies and linked pathway steps both print another record's name beside a sentence of its `bestUsedFor` or `clinicalSummary`, with nothing saying whether that record has been reviewed. The 2026-09-02 audit recorded this under L03c as a consistency gap, which it was while a catalogue-wide notice also stated the caveat. PR #2686 removed that notice at the owner's request, leaving the per-record badge as the whole disclosure, so these two surfaces became the only places a clinician can read therapy guidance with no review state at all. Both now render the same StatusBadge every other therapy surface renders, in a compact size added for dense rows. The default size is unchanged, so no existing surface moves. An unlinked pathway step names no record and so states nothing. Both fixes are pinned: the disclosure contract test now covers the two listings, and a new contract case asserts the profile is named rather than defaulted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUKKfk7hcKZ2UuqF3pwxMY
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8c053fd3-f513-4438-8eef-76a75bc3001a) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
2 participants
Summary
Two follow-ups from the PR #2686 session, each fixed at its root cause rather than patched at the symptom.
InPageSectionRailtookdensityas an optional prop with a default, so medication's section rail sat on Therapy'sextendedprofile without ever naming it.densityis now required at both the rail and the header layer.StatusBadgeevery other therapy surface renders, in a compact size added for dense rows.Root cause 1: an optional prop with a default made a calibration decision inheritable
registryModeNavDensityProfileslists the modes that render the top tab bar. Checking it says Therapy is the onlyextendedconsumer — and that is true of that registry. ButInPageSectionRaildeclareddensity = "extended"as a default parameter, andmedication-nav-header.tsxpassed no density, so it inherited the profile through a code path no registry describes. Retuningextendedfor Therapy in #2686 therefore unfolded medication's counted labels early, andui-smoke's prescribing journey was the only thing that caught it.A default cannot be safe here. The value is a calibration against one label family's measured slot widths, so inheriting another family's is always a guess. The four-slot medication rail carries an icon, a label and a count badge per slot; Therapy's bar carries five plain wordmarks. They are not interchangeable and never were.
Making
densityrequired costs each call site one line and makes the consumer set answerable bygit grep "density:". It is also self-demonstrating: the typecheck immediately found two further silent inheritors intests/in-page-nav-header.dom.test.tsxthat no one knew were there.Call sites after the change:
medication-nav-header.tsxextended-countedtherapy-record-nav-header.tsxbalanced-fourin-page-nav-header.dom.test.tsx×2balanced-fourRoot cause 2: the disclosure contract had two surfaces it did not reach
The contract is that every surface showing a record states its review state.
RelatedTherapiesandPathwayStepStackboth print another record's name beside a sentence of itsbestUsedFororclinicalSummary, with nothing saying whether that record has been reviewed. The 2026-09-02 audit recorded this under L03c as a consistency gap.It was a consistency gap while a catalogue-wide notice also stated the caveat. PR #2686 removed that notice at the owner's request, leaving the per-record badge as the whole disclosure — which turned these two into the only places in Therapy where a clinician can read treatment guidance with no review state at all. Same code, materially different consequence.
Both now carry the badge. An unlinked pathway step names no record and so states nothing.
record-sections.tsxrenders the current record's own body and its header already carries the badge, so it is untouched.StatusBadgegains an optionalsize, defaulting to"standard", so no existing surface moves.Worth the reviewer's attention: with 205 of 205 records unreviewed, every one of these badges currently reads "Needs source review". That is truthful and it is what the contract asks for, but it is repetitive today and will stay so until sign-off progresses (
#SBKXZ7). It differs from the removed banner in the way that matters: the banner was a library-level claim repeated above every search regardless of what you were reading, whereas this is attached to the specific record whose words are on screen. If it reads as noise in practice, the honest lever is per-record sign-off, not hiding the state.Verification
npm run verify:cheap— exit 0.Test Files 1268 passed | 1 skipped (1269),Tests 18037 passed | 2 expected fail | 3 skipped (18042).npm run typecheck— exit 0 (and is itself the enforcement mechanism for root cause 1).npm run format— whole tree, no changes left.density = "extended-counted"→mode-nav-contractgoes red (1 failed | 25 passed)related-therapies.tsx→therapy-review-regressionsgoes red (1 failed | 6 passed)tests/ui-therapy-pathways.spec.ts— 2 passed. The specplan:browserattributes topathway-step-stack.tsx, covering phone step scrolling and the desktop split view.tests/ui-smoke.spec.ts --grep @critical— 10 passed, includingprescribing workflow uses in-app medication routes, the exact test that caught the Therapy search: shared Filter button, no review banner, and a tab bar that fits a phone #2686 regression and the reasonmedication-nav-header.tsxis in this diff.npm run verify:ui— not run locally by choice.plan:browserfails closed tofullbecause several changed files are unattributable to any spec, and CI runs the complete Chromium suite on a non-draft PR. PerAGENTS.md"Do not pay twice for the verdict GitHub is about to reach", the focused proof above is what was spent locally. Last time this call was right and my reading of what a focused run proved was wrong, so: the runs above cover the pathway steps and the medication rail, and say nothing about the other five changed files.eval:retrieval:quality,eval:rag,check:production-readiness,check:deployment-readiness. No retrieval, ranking, ingestion, answer-generation, environment or deployment surface is touched.Risk and rollout
sizeprop defaults to the current value.Clinical Governance Preflight
This change increases clinical disclosure, so the section is completed.
Clinical KB Database(sjrfecxgysukkwxsowpy) — untouched#SBKXZ7(clinician sign-off outstanding for 205 therapy records) is unaffected and still open. This PR does not reduce that backlog; it makes the backlog visible everywhere a record's words appear.Notes
mode-nav-contractcase asserts the profile is named rather than defaulted, and that both production call sites name one.🤖 Generated with Claude Code
https://claude.ai/code/session_01HUKKfk7hcKZ2UuqF3pwxMY
Generated by Claude Code
Note
Low Risk
Type-level nav calibration with explicit profiles at existing call sites, plus additive UI badges on two therapy listings—no auth, data, or backend changes.
Overview
In-page section rails can no longer inherit a default nav density profile.
InPageSectionRailandInPageNavHeader'srailprop now require an explicitModeNavDensityProfile, fixing silent coupling where medication's counted-label rail picked up Therapy's profile after #2686. Medication now namesextended-counted; tests and amode-nav-contractguard ensure every call site stays greppable.Therapy disclosure: Related therapies and pathway step rows that quote another record's clinical text now show a compact
StatusBadgefor that record's review state (linked steps only).StatusBadgeaccepts an optionalsize(defaults unchanged for existing surfaces).Reviewed by Cursor Bugbot for commit 253da4c. Configure here.