fix(differentials): unify filtering, urgency, and audit follow-ups - #2729
Conversation
The rail previously read two lists: Check next followed the applied filter while Highest urgency deliberately read the whole result set, so a lens could never hide an emergent differential from the safety net. On review the owner wants one rule instead, so the rail always describes the list on screen rather than a wider one the clinician cannot see. Both cards now read the filtered results. A lens that excludes every emergent differential empties the urgency card, and the applied-filter chips above the results say why it is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FnUcNddudVxqJQbcxP1Vn8
PR #2665 merged and deployed, and three things it surfaced were recorded nowhere. The session-start hook asked for a capture twice and never got one, so this is that capture. Queued through npm run issues:add/update only; docs/outstanding-issues.md itself is untouched and these apply on the next npm run issues:reconcile. P1 — ten locally authored differential records are live with no clinician sign-off. src/lib/differential-curated.ts seeds delirium, hypoactive delirium, NMS, serotonin toxicity, catatonia in mood disorder, akathisia, alcohol withdrawal, clozapine adverse effects, postpartum psychosis and lithium physiological withdrawal tremor. The labelling and the two enforcing tests are done; the attestation is not, and the content reached psychiatry.tools when main deployed. Shaped after #SBKXZ7, including its rule that assistants never tick the clinical checks on their own behalf. P2 — the differentials export leaves 194 of 201 records with an empty section summary, 150 with no investigations and 110 with no immediate actions, and lithium-physiological-withdrawal-tremor carries akathisia's content outright. The detail page now degrades honestly around this, which is why it is not P1, but the per-slug overlay treats symptoms and no row owned the generator. #QSHHGK updated: the baseline it was opened around has now been refreshed, so its points (2) and (3) are superseded and only the missing refresh owner keeps it open. The update records that it was measured from main at fd81561 rather than from a feature branch — a feature-branch baselineSource is deleted by squash-merge, which is how the previous one became unresolvable — and it deliberately preserves the row's caution that the refresh absorbed growth never attributed to a reviewed change set. That was the owner's call. #ZWJ71W updated with a cross-reference: the two morning specs it names are now test.skip on main as well (18a42d2), so Advisory UI is green and the owner ruling behind those skips is less visible than while they were failing, not more. Verified: check:outstanding-issues in step (121 open); working tree contains nothing outside docs/outstanding-issues-inbox/; npm run format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMHX5TDG34BBn2yDdi81s5
…porting its catalogue Four follow-ups from the filter panel work, each the smallest change that actually closes the item. Derive the segmented renderer instead of opting into it. A one-of-N filter whose options all carry counts is a segmented bar because of what it is, not because a call site asked; `renderAs` is gone and the option list decides. That was always the stated end state — the flag existed only so modes could move one at a time — and leaving it in kept twelve surfaces on the wrapping counted chips section 5 rejects. Bounded at five options, where the chip tier ends: past that a segmented bar wraps into rows and stops reading as one control. Split the count on SegmentedControl. It used one field for both the visible figure and the accessible name, so a count carrying a unit had to choose between an unreadable segment and a name that dropped the unit — which is why documents' Source locality stayed on chips. It now takes the same `hint`/`hintLabel` pair an option already has: "1 loaded source" announced, "1" displayed. Report the catalogue size from /api/differentials. `total` measured the records being returned, which under a query are the ranked matches, so a caller asking how many differentials exist got its own result count back — while the route's other three branches all reported the catalogue. That is what left the differentials filter unable to state the figure. Pinned by a test that fails on the old code with "expected 10 to be 201". Drop the redundant "together" from the differentials filter subtitle. Two defects found and fixed while doing the above, both of which would otherwise have shipped: A state-dependent renderer made the control change shape mid-interaction. Documents' Source locality marks an option dead the moment its count reaches zero, and the first derivation vetoed the segmented bar on any dead end — so the filter morphed from a bar into a chip row while the reader was using it. The renderer now depends on the option list alone. Fixing that by redefining `disabled` was wrong. `SegmentedControl` is shared well beyond filters, and skipping genuinely unavailable options with the arrow keys is a deliberate contract with a test pinning it. Dead ends are a different state and now have their own field: `disabled` leaves the arrow path, `deadEnd` stays on it with `aria-disabled` and a stated reason, as section 3 requires. No consumer passed `disabled` to a segment, so existing output is unchanged. Verified: typecheck, lint, whole-tree prettier, check:design-system-contract, and the full offline unit suite — 18,038 passing, no failures. The three failures that were red on main earlier in this work are gone, fixed upstream. Full Chromium suite still running at commit time; result reported separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMaWYk6iF8bsMwEx38egVX
|
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: Advanced 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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
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. |
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_311d51b0-69d8-42b5-829b-8e224efb8fef) |
…r than act on it Two of the four PR items had owner decisions that could be executed. Only one of them survived contact with the repository. CLOSED #QSHHGK. The owner is the named refresh owner and the weekly report-only run is sufficient, which resolves the last of the three reasons it stayed open; the other two were resolved by the 2026-09-06 refresh from main at fd81561. The closing note deliberately carries the row's own caution forward: that refresh absorbed roughly two weeks of growth never attributed to a reviewed change set, and closing the row does not un-record it. DID NOT retire the fixed morning view, though that was the decision given. Two independent reasons, either sufficient on its own. First, the question was mis-framed when it was put to the owner, and the mis-framing is mine. Spec D9 is not the fixed/live morning view: the phase-4 design spec defines D9 as the SHIFT HANDOVER page, which is already built at components/ward-management/handover/ and imports none of the morning freeze machinery. The actual open D9 question, in the spec file's own words, is "whether the morning board and the shift handover still owe each other a cross-link once folded" — a navigation question. The fixed/live split is not an open question at all; owner decision WB-DB-11 already reversed it. The two skipped specs are therefore parked on the cross-link ruling, and morning-page.tsx explicitly instructs against the three moves that would pre-empt it. Second, npm run check:dead-code-candidate REFUSED all five candidates — FrozenMorning, buildFrozenMorning, ViewControl, NoHandoverYet, MorningView. Each was introduced 2026-09-03, four days ago and well inside the 30-day threshold; ViewControl is additionally pinned by two committed tests. Per docs/agents/dead-code-deletion.md a refusal is a stop and the threshold is not to be tuned to make a diff pass. Earliest eligible re-check is 2026-10-03. #ZWJ71W updated with both findings and with the question the owner actually needs to answer, so the next reader does not repeat the mis-framing. Its earlier reference to "the D5 half of #YTR84P" is corrected too: D5 in that spec is release-a-hold and cancel-a-transport, unrelated to any of this. No source or test file is touched by this commit. Verified: npm run format clean; working tree contains nothing outside docs/outstanding-issues-inbox/; the dead-code gate output above is quoted from a real run, not assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMHX5TDG34BBn2yDdi81s5
…g mutation Fixes the Static PR checks failure on 9581700. The failing step was `docs:check-links`, which builds the projected ledger markdown and therefore plans the pending request batch: Error: multiple pending mutations require an explicit cancellation decision (#ZWJ71W: 2ab09b4d, ce667f09; #QSHHGK: 2e5e2bc1, a4593f03). Self-inflicted: I queued two mutations against each of those rows across two commits, and the reconciler refuses to guess which one wins. Reproduced locally before fixing, and the same command is green after. CANCELLED 2ab09b4d (#ZWJ71W, first update). Verified superseded verbatim — ce667f09 contains its full 2678 characters plus the D9 correction. Nothing is lost. CANCELLED 2e5e2bc1 (#QSHHGK, the closure). This is ordering, not a reversal: the owner's decision to close that row stands. Its sibling update a4593f03 corrects a statement in the row that is now false — that the baseline was deliberately not refreshed — and a `done` request cannot carry a detail change, so the correction has to land first. The closure is re-queued after the next reconcile. Cancelling the update instead would have left the false statement in the archived row, which is the opposite of why the update exists. Why CI caught this and I did not: I ran the ledger gates and `ledger-inbox.mjs check` locally and all passed, but `ledger-inbox check` validates request files without planning the batch, and I never ran `docs:check-links` — the one gate that does. Running the ledger checks alone was not sufficient evidence. Verified after the fix: docs:check-links, docs:check-scripts, check:outstanding-issues, check:branch-review-ledger, check:ledger-stamp-retention, check:diff-integrity all pass; `ledger-inbox.mjs check` reports 42 pending, 1080 applied; whole-tree `prettier --check .` clean. check:ledger-write-discipline re-run below on the committed tree, since it refuses a verdict on an uncommitted one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GMHX5TDG34BBn2yDdi81s5
BigSimmo
changed the title
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_e3f099c1-00e4-4cf1-939b-86902310b8d5) |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #16665 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
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_f469f2d8-c2fd-4983-ab3e-0c7a079009a0) |
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_a370b2fb-a4c1-4db6-8ce7-6c0615eb791f) |
BigSimmo
left a comment
BigSimmo
left a comment
There was a problem hiding this comment.
Summary
Exact-head Copilot-style review of a9d9a384fcb6092c0d50fd0f92a877cd348bd8e5 (Batch G / differentials urgency rail + filter consolidation). Scope reviewed: differentials-home.tsx interpretation rail, /api/differentials total, result-filter-control derived segmented rendering, SegmentedControl deadEnd/hintLabel, and the matching DOM/route tests. Do not merge until the clinical urgency-rail caveat below is addressed or explicitly owner-waived with UI copy.
Note: GitHub blocked --request-changes (own PR). Treat this COMMENT as a would-be REQUEST_CHANGES on the P1 clinical finding.
Filter/a11y work looks solid. The urgency-rail behaviour change is the load-bearing clinical risk.
Findings
P1 — Clinical: Highest-urgency rail silently vanishes under lenses (safety-net removed)
Files: src/components/clinical-dashboard/differentials-home.tsx (InterpretationRail, call site ~1423); UrgencyCard (~701–706); tests/differentials-interpretation-rail.dom.test.tsx
Previously the Highest urgency card read the unfiltered result set so a result-type / urgency lens could not hide an emergent differential from the safety net. This PR feeds only relevanceResults into both rail cards. UrgencyCard already return null when no emergents remain in its input, with an explicit “drop the card” comment — so under the new wiring the card disappears with no clinical notice.
Measured by the updated test: selecting urgency High removes differentials-highest-urgency from the DOM entirely, even though emergent rows still exist in the broader hit set and only the filter chips explain why.
Risk: A clinician narrowing by result type (e.g. Presentations) or by urgency band other than Emergent can lose the emergent safety cue without an affirmative “N emergent differentials hidden by current filters” banner. Silent absence reads like “nothing emergent,” which is the opposite of the prior contract.
Ask before merge (pick one):
- Restore unfiltered input for
UrgencyCardonly (keep Check-next on filtered), or - Keep filtered semantics but when unfiltered emergents exist and filtered emergents are empty, render an explicit clinical empty-state (not
null) naming that filters hid emergent items, or - Written owner waiver on the PR that accepts silent drop + relies solely on filter chips — recorded as clinical-governance acceptance.
Until (1)/(2)/(3), this is a real P1 clinical regression relative to the previous safety-net design.
P2 — Dead-end SR text is generic; may lose filter-specific reason
File: src/components/ui/segmented-control.tsx (dead-end sr-only: “Not selectable from here.”)
deadEnd / aria-disabled / stay-on-arrow-path is the right a11y model vs native disabled. The announced reason is fixed boilerplate. If isDeadEnd / option metadata carries a richer count-driven reason, prefer plumbing it into aria-describedby so differentials urgency dead-ends explain why (zero under current narrowing) rather than a generic refusal.
P2 — API total semantics fixed; scoped count still absent
File: src/app/api/differentials/route.ts (total: differentialRecords.length)
Catalogue-size total under q is correct and pinned by tests/differentials-route.test.ts. Docs already note the client still cannot state “how many of N survive the current urgency selection” without bundling the snapshot — fine as follow-up, not blocking, but any future consumer of total must not treat it as filtered scope.
P2 — Process: inbox cancel-of-done for #QSHHGK
Files: docs/outstanding-issues-inbox/2e5e2bc1-… (done) + 7c9df6d8-… (cancel) + a4593f03-… (update)
Intent (update before close) is documented. Confirm post-merge reconcile order so the closure is re-queued and not lost; Batch reconcile is currently failing on the PR checks surface (likely shared infra — verify it is not this inbox conflict).
Non-findings / looks good
- Derived segmented rendering (≤5 counted options; dead ends no longer morph chips↔bar) matches
docs/filter-contract.md§5 intent. hint/hintLabelsplit onSegmentedControlcloses the unit-vs-display gap that kept documents Source locality on chips.- No RAG / retrieval / claim-support surfaces touched; RAG impact: none as declared.
- Never merge from this review.
Verdict
Would REQUEST_CHANGES on the P1 urgency-rail clinical caveat (posted as COMMENT because own-PR restriction). Filter/a11y consolidation is otherwise merge-ready after that is fixed or explicitly waived.
BigSimmo
left a comment
BigSimmo
left a comment
There was a problem hiding this comment.
Adversarial exact-head review — a9d9a384fcb6092c0d50fd0f92a877cd348bd8e5
Verdict: GO_WITH_CAVEATS (does not approve or merge). Prior notes at e1d5c34 / 7503bae9 are superseded. behind_by=0, ahead 11 vs main. Feature delta vs main unchanged (+342/−108, 16 files).
MUST FLAG — Clinical / urgency rail
Highest urgency no longer safety-nets unfiltered results. Both interpretation-rail cards now read relevanceResults (filtered only). A lens that excludes every emergent differential empties the Highest urgency card (UrgencyCard returns null); DOM test locks that disappearance. Applied-filter chips are the remaining cue. safetyLead is also taken from the filtered list.
This is intentional (owner-requested one-rule: rail describes the list on screen) but is a deliberate safety-property downgrade vs the prior design. Joshua must explicitly accept before merge.
Other adversarial lenses
| Lens | Finding |
|---|---|
| Privacy / PHI / keys / Supabase | None observed |
| Silent-fail / API | GET /api/differentials total = catalogue size even with q (pinned by route test) — LOW–MEDIUM contract change |
| RAG | None in this PR’s unique delta |
| CI-weakening | None (no workflow/coverage files in delta) |
| Shared UI blast radius | renderAs removed; segmented derived; deadEnd on shared SegmentedControl — MEDIUM |
| Merge hazards | Consolidation PR; update-branch only if main moves; empty reviewDecision |
CI on this exact head (one-shot; no further poll)
Run https://github.com/BigSimmo/Database/actions/runs/34686198269 (head_sha = a9d9a384…). At wrap: Safety ✓; Unit coverage / Build / Static / Production UI critical still pending. Batch reconcile fail/cancel — ignore. Do not merge on CI grounds until required checks are green on this SHA.
Pre-merge checklist
- Accept urgency-rail clinical caveat
- Required CI green on
a9d9a384(Unit coverage, Static, Build, Safety, Production UI critical, PR required) - Human approve if branch protection requires it
Full write-up: workspace /workspace/pr-2729-review.md.
The PR was not merged. Final merge is left to the user.
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_7cdd6b73-21d6-4fa1-a6e9-0242dc6e264b) |
…cy rail The highest-urgency card still follows the visible list, but it no longer disappears without saying that active filters hid emergent differentials.
|
Urgency-rail P1 fix ( Highest urgency still follows the visible (filtered) list — no unfiltered safety-net rows were put back. If a lens hides every emergent, the card no longer vanishes: it stays with a short notice that emergents are hidden by active filters, plus Show all results. When the search itself has no emergents, the card still drops (unchanged). Focused rail DOM test updated. Not merged. |
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_1b15c256-50bf-474c-8627-1ed1ab99de25) |
2 participants
Summary
Consolidates server-side filter normalization, shared filter counts, and urgency behavior based on visible results. Retains the related differential audit requests in the same review.
Consolidates #2729 (2ab2c2d), #2719 (ee2aef2), #2722 (5200644). Original heads are retained as ancestors; absorbed PRs are closed only after publication is verified.
Verification
npm run test -- tests/differentials-route.test.ts tests/ui-v2-components.dom.test.tsx tests/differentials-interpretation-rail.dom.test.tsx— 111 passed in 3 files.npm run format— passed; formatting committed.git diff --check— passed.Risk and rollout
npm run check:production-readinesson the shared current-base governance integration stopped at six existing privacy release blockers (five pending and one partial). The production-readiness stage did not execute. These approvals remain outstanding.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)This PR edits
src/app/api/differentials/route.ts, so it classifies as clinical-risk and carries the full preflight. Each box above was checked on its own merits — the basis for each, in the same order:supabase/changes on this PR.publicDifferentialPayload, the public/demo path. No key handling, auth resolution, rate limiting or access check is altered, and the owner-scoped branches are untouched.demoMode: trueexactly as before.governancefield is untouched.totalhas no consumer (verified: nothing insrc/reads it), so no clinical decision-support behaviour changes.Two points worth stating beyond the checklist:
totalmakes the API more truthful, not less — it previously reported a figure that was wrong whenever a query was present. Filter counts themselves are untouched and still come from the same predicate as the filter, per §3.Note
Medium Risk
Changes clinical urgency surfacing when filters hide emergents and alters shared filter/segmented-control keyboard semantics; the differentials API
totalmeaning changes though callers may not consume it yet.Overview
Filter lenses no longer accept
renderAs: "segmented". Counted one-of-N groups with at most five options automatically render asSegmentedControl; dead-end options no longer force a chip fallback, and counts can usehint/hintLabelin the segmented bar.SegmentedControladdsdeadEnd(focusable,aria-disabled, no selection) separate from nativedisabled, plushintLabelfor short visible counts while keeping units in the accessible name.Differentials search aligns the interpretation rail with the filtered list: “Check next” and “Highest urgency” both follow active lenses. When filters hide emergent rows, the urgency card stays visible with a status message and Show all results. Filter groups drop explicit segmented opt-in.
/api/differentialstotalon the diagnosis branch now reports full catalogue size even whenqis set (was match count); covered bytests/differentials-route.test.ts.docs/filter-contract.mddocuments derived rendering and the API fix.docs/outstanding-issues-inbox/adds reconciler mutations (clinical sign-off tasks, bundle-budget closure, ward-flow test notes).Reviewed by Cursor Bugbot for commit 2857fbf. Configure here.