Never show an all-clear over a medication check that could not run - #2538
Conversation
… run The patient-considerations panel could render a green "no matching considerations" banner directly above an "Enter eGFR…" sentence, and dropped most unread gates before they ever reached the panel. Engine (medication-patient-alerts): - Collect a row's missing gates whether or not the row fired. A row that fired on age while eGFR was blank previously reported no gap at all, contraindication rows included. - Partition those gates into two exported fields. `unassessed` keeps its MEANING exactly — contraindication rows only, never the advisory tier, so the result-row verdict's green-to-grey degrade still says the same thing — but it is not untouched: collecting gates from fired rows too makes it a strict superset of what it held before, so the degrade now fires on medications where it previously stayed green. That direction is deliberate and only ever more conservative; it never clears a degrade that used to fire. The new `unassessedAdvisory` carries dose-adjust, caution and monitor rows, 227 of which were previously discarded outright. `action: "info"` rows are excluded from both; any unknown future action lands in the advisory tier rather than being dropped. `unassessedAdvisoryCount` reports the contributing row count, which the clinician-facing sentence states and the input list cannot supply. - The bare-renal fail-safe and its rationale are untouched. Panel (medication-considerations): - The empty state degrades from success to neutral whenever either tier has an unread gate, mirroring what the interactions block below it already does for unresolved rows. - The contraindication sentence is rewritten as a "Not assessed." statement, and the advisory gap renders as its own neutral notice instead of being silently absent. - The advisory sentence closes on what is missing, not on whether the entry applies: "…, which this profile does not include. Enter it to complete that check." Because gates are now collected from rows that fired, an advisory row can be on screen as a live alert while another of its criteria is unread; the old "Enter it to see whether it applies" then invited deferring an alert that already applied. Real in the corpus — lithium-carbonate-ir-sr's renal dose-adjust row fires on an NSAID allergy while eGFR stays blank. The input list also gains a serial "and" so it cannot run into the clause that follows it. Profile panel (patient-profile-panel): - Hepatic impairment gains an explicit "Not recorded" segment and is defaulted to it. Selecting "None" now stores "none", which the engine already treats as present-and-non-firing; storing it as null made an answered question indistinguishable from an unanswered one and showed "None" for a profile that recorded nothing. Tests: the two assertions that pinned the discard by name are replaced with equally strict assertions on the new partition (advisory contains the row, blocking still does not, verdict tone unchanged), plus DOM coverage that the empty state is not the success tone while a gate is unread, that a recorded "None" differs from a blank, that a fired advisory row never renders "to see whether" beneath it, and that verb and pronoun agreement key on the entry and input counts respectively. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL
|
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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…uncating Verified in Chromium at a 320px viewport: the five-segment `layout="equal"` control gives each segment ~50px, and "Not recorded" rendered as "Not recor…" directly beside a fully legible "None". That is the worst possible place for an ellipsis. This control exists to separate "no answer recorded" from "answered: no hepatic impairment", and "Not recorded" is the default state, so the truncated string is what a clinician sees before touching anything. Shortening by wording rather than by ellipsis keeps both states legible at 320, 360 and 390px. Caught by browser inspection, not by measurement — a scrollWidth comparison reported no truncation at any width while the rendered control was visibly clipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL
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_4329ed0d-08aa-4f38-8b2b-e3a6a7fe6906) |
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_ccf50e7c-2231-44c9-b634-e117065a724d) |
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_fbc4bf34-3b8e-4a2f-a75f-5e0beb470cd7) |
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_b4500cf3-28ff-46f8-a69e-5dad90988a3d) |
…aude/medication-gates
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #15029 (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_33d879be-a7f1-4449-afe2-8405f19f1f01) |
…r review (#2544) * issues: queue seven verified follow-ups from PRs #2538, #2536 and #2531 Seven immutable inbox requests, no canonical ledger edit. Every claim was checked against the code before it was written: three against origin/main, four against the PR heads the follow-up belongs to (#2538 ef7c55a, #2536 e24e0ee, #2531 60f5e8e), since none of the three has merged yet. - P2 issue: differential-records.ts asserts validation_status "locally_reviewed" from a literal over a snapshot that says "Pending review" — the sibling of the fix already in medication-records.ts. - P2 issue: registry-records.ts and differential-records.ts both return the frozen source_status column verbatim, and derive it with a substring test that also matches "not checked"/"unchecked". - P3 issue: a stored source_status of "outdated" can never be cleared, since nothing writes that column back on any of the three record tables. - P3 rec: patient-alert rows with action "info" reach neither unassessed tier, leaving a green all-clear for nystatin, levetiracetam and lorazepam. - P3 issue: isProfileEmpty treats a recorded hepatic "none" as no information, disagreeing with the engine, which treats it as an answer. - P3 task: the considerations panel's two not-assessed sentences format their input lists differently. - P3 rec: the forms PDF manifest records only passwordProtected, though the committed bytes also say modification, text extraction and assembly are blocked while printing and form-filling are permitted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: queue the recurring docs/scripts-index.md counts collision (#Y090R5 sibling) The generated counts sentence at the top of docs/scripts-index.md conflicts on every concurrent PR — measured five times on PR #2531 in about three hours, at five different main heads, each time as the sole conflicting line. Same root cause as #Y090R5 for data/outstanding-issues-snapshot.json: a single-line generated artefact every PR must regenerate. PR #2530 fixed the snapshot half by moving regeneration into the serialised reconcile step; this half is untouched. Records the two traps found while resolving it: taking main's whole file silently drops the branch's own new script entry, and a bundled verify:cheap:internal union merge desynchronises the gate counts that check:gate-manifest reads. Append-only inbox request; no canonical ledger edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: cancel the OCR-routing claim and replace it with a corrected record Codex review finding on this PR, verified against the code it cited rather than taken on trust. The finding is correct. Request e3133c1b asserted that disabled text extraction is "exactly the condition that sends the worker down the OCR fallback path". It is not. should_ocr_page() in worker/python/extract_pdf_assets.py (line 186) decides on extracted text length and image coverage ratio only, and never reads the /P permission bits. The extractor has no needs_pass or authenticate handling at all, so a user-password PDF fails before any OCR decision is reached. A wrong causal mechanism in a durable record is worse than no record: the next person plans ingestion work from it. Cancelled rather than edited, because inbox requests are immutable by design and the cancel action exists for exactly this — the audit trail keeps the wrong claim, its refutation, and the correction. The permission-bits finding itself stands unchanged and is restated in the replacement request, which makes no ingestion claim and says plainly that what happens to these files on ingestion is untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: record the reproducible mobile /documents/search LCP breach Two confirmed measurements on different heads of PR #2536 — +466ms (+20.4%) and +471ms (+20.6%) against a +20%/+100ms tolerance, five milliseconds apart. That is reproducible, not noise, and the passing re-run between them was the outlier. Ruled out as PR #2536's doing: its diff is six medication files, and documents/search/page.tsx imports one symbol (Metadata from next). Nothing outside medication-named files calls /api/medications. Recorded as a hypothesis, not a finding: lighthouse-budget.json was last refreshed 2026-08-27 and main has taken heavy change since, so the PR carrying the newest main absorbs the blame. Same disease as #QSHHGK for the bundle budget. Main itself was never measured, and the record says so. Needs an owner decision — find the regression, or refresh the baseline on a schedule rather than reactively to clear a red PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: cancel the Lighthouse regression claim and replace it with the measured variance A third graded run refutes the "reproducible regression" reading in request d1b3491f. CI run 33684986161 (head 957a038) measured mobile /documents/search at 2323ms against the 2282ms baseline — +41ms — and reported "Every graded route is within tolerance of the committed baseline." The cell has now produced two breaches near 2750ms and at least two passes near 2320ms on the same pinned Chromium and the same route. That is a bimodal measurement whose two modes straddle the +20%/+100ms tolerance, not a page that became half a second slower. The gate's 2-of-3 sampling already tolerates noise within a run; this split is between runs, which that design does not cover. Cancelled rather than edited, because inbox requests are immutable and the cancel action exists for this. The replacement states all four outcomes and asks for the variance to be characterised — repeat the dispatch-only baseline-refresh job against main and compare the spread — rather than for a regression hunt or a reactive baseline refresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: record the measured encryption state of the committed WA MHA form PDFs A concurrent review comment on this PR asserted that PyMuPDF does not enforce /P permission bits and that these files therefore "open normally and yield their text layer". Measured against the committed bytes with PyMuPDF 1.28.0, the library the worker actually uses, that is false for 50 of the 51 files. needs_pass=1, is_encrypted=True, page_count=0 doc.authenticate("") -> 0 (the empty user password is rejected) load_page(0) -> ValueError('document closed or encrypted') form-12a.pdf is the sole exception: no /Encrypt, opens, 3274 characters of first-page text — which is exactly the file PR #2531 corrected. The distinction the comment missed is that /P alone would not block opening; these files also carry a non-empty /U, and that does. The practical consequence is that extraction fails at the open call, before should_ocr_page() is ever reached, so the OCR fallback cannot rescue them. The JavaScript fallback was not measured and is not claimed either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * issues: record the intermittent caring-contacts-guidance strict-mode violation Observed once on PR #2536 (CI run 33690849576 attempt 1, Production UI shard 2) and cleared by a single re-run of the same commit: strict mode violation: getByTestId("caring-contacts-guidance") resolved to 2 elements The source renders that test id in exactly one place, and both the page and the shell interpolate it once, so the duplicate is not a second render site. The likely window is React relocating out-of-order streamed content from the page's next/dynamic shell, which the test's waitUntil:"load" does not wait past — but that is a hypothesis and the record says so; the retained trace should confirm it before anyone edits the test. Not PR #2536's: its diff is medication and documentation files only, and the same shard passed on its previous head. The record asks for the three unscoped locators to be scoped to the main landmark, and explicitly rules out quarantining — one reproduction is below the repository's three-on-the-same-SHA bar, and a locator fix is not a suppression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL * docs(ledger): record Run PR sweep review for PR #2544 Merged origin/main into claude/issues-followups (clean, no conflicts) and verified the narrow gates for this append-only inbox PR; both review threads were already resolved from a prior pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137etKe6X2p9HkXsXoYY8TA --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
Summary
medication-considerations.tsxrendered asuccess-toned "No matching considerations" notice whenever the considerations list was empty, and the "Enter eGFR…" hint was a sibling node — so it printed directly beneath the green banner. The empty state now degrades toneutralwhenever any gate went unevaluated, mirroring what the interactions block twelve lines below it already did for unresolved rows.evaluateRowcollected each row's unevaluated inputs, then dropped them if the row had fired on some other gate (contraindication rows included — a row firing on age while eGFR was blank reported no gap at all), and again for every non-contraindication row. Both discards are closed.unassessedkeeps its exact meaning — contraindication rows only — so the result-row verdict's green-to-grey degrade keeps its meaning too. The newunassessedAdvisorycarries dose-adjust, caution and monitor rows, 227 of which were previously dropped outright.action: "info"is excluded from both; an unrecognised future action lands in the advisory tier rather than being dropped.HepaticSeverityalready includes"none"and the engine already treated it as present-and-non-firing, but the panel wrotevalue === "none" ? null : valueand displayedprofile.hepatic ?? "none"— so selecting None stored exactly what never touching the field stored, and the control showed "None" for a profile that had recorded nothing. It now has an explicit Not set state and writes"none"through as a real answer.The single widened set the ledger asked for was measured and rejected: it would have taken the share of medications reading "we could not check something" from 39% to 66% of the catalogue, making grey the background state and diluting the contraindication signal that already works. Splitting the tiers keeps that signal intact.
Governance review changed the copy before push
The advisory sentence originally ended "Enter it to see whether it applies." Because this change now also collects gates from rows that fired, a live clinical alert can render directly above that sentence — and it already applies. A clinician could reasonably read the invitation as permission to defer acting on the alert above it. Confirmed against the corpus, not assumed: profile
{ allergies: ["nsaid"] }againstlithium-carbonate-ir-srfires the renal dose-adjust row on "NSAID allergy" whileeGFRstays unread.The sentence now reads: "Not assessed. 1 dosing and monitoring entry uses eGFR, which this profile does not include. Enter it to complete that check." No user-visible string claims a medication was assessed and found acceptable; none uses "clear", "safe", or "no issues".
Deliberate trade-off, stated
The advisory tier does not feed
composeMedicationVerdict, so a prescribing-search row can still show a green verdict while dosing or monitoring gates went unread. That is unchanged frommain— but the code now knows and chooses silence, so it belongs in the record rather than only in a test comment. Folding the advisory tier in would grey roughly two thirds of the catalogue on a thin profile and destroy the band's meaning.unassessedis nonetheless a strict superset of what it was, so the verdict degrade fires in more cases than before, never fewer.Verification
npm run test(full unit suite) —Test Files 947 passed (947)/Tests 12067 passed | 1 skipped (12068)Test Files 4 passed (4)/Tests 77 passed (77)(re-run after the label change; still 77 passed)npm run typecheck—recorded a pass for "typecheck:internal"npx eslinton the changed source files — exit 0npm run format(whole tree) — no files changed afterwardsTwo negative controls. Reintroducing both original defects (forcing the unassessed flag false, restoring the fired-row
continue) produced exactly two failures — the empty-state tone assertion and the "keeps collecting a row's remaining gates after it fires" assertion. Restoring the old advisory wording failed the new DOM guard with the defect printed in one line:That is the fired dose-adjust row rendering as a live alert, immediately followed by the invitation to defer it. Both controls were reverted and the suite re-verified green.
npm run test:focusedrefuses by design here ("Focused test selection is unsafe: test or configuration paths changed") and instructs the full suite, so the full suite is what ran.Two failures seen on an earlier run of this branch (
tests/clinical-hazard-controls.test.ts,tests/rag-plan-package-parity.test.ts) were a shallow-clone artefact — the container held 98 commits and both tests name commits absent from it. Aftergit fetch --deepen=2000the suite is green as quoted above, with no code change.Verification not run:
npm run check:production-readinesscannot pass in this repository.check:privacy-readiness:releaseblocks on six release-blocking legal/provider items (OpenAI ZDR, OpenAI and Railway DPAs, APP 8 cross-border basis, APP 1/APP 5 notice, PHI minimisation). The same command on unmodifiedmainat45a3dca, with no changes, produces identical output — repository state, not a consequence of this diff, and none of those items is touched here.Verification not run:
npm run verify:pr-localwas not run; its risk-routed selection adds no failure class this diff can reach beyond the full unit suite already run above.UI verified in a browser.
npm run ensure(server identity confirmed via/api/local-project-id), then Chromium at 320px, 360px and 390px on/medications/lithium-carbonate-ir-srwith the Patient details sheet open.This caught a real defect that measurement missed. The hepatic segment label was "Not recorded", and in a five-segment
layout="equal"control each segment is ~50px at 320px, so it rendered as "Not recor…" directly beside a fully legible "None" — truncating the default state of the one control whose entire purpose is separating "no answer" from "answered: no impairment". AscrollWidthcomparison reported no truncation at any width; the screenshot showed it plainly. The label is now "Not set" and all five segments render in full at 320, 360 and 390px.UI verification not run: the full Chromium journey gate
npm run verify:uiwas not run because the remaining rendered changes are notice tone and sentence copy, pinned by the new DOM assertions. No routing, layout, chrome-ownership or tap-target change;SegmentedControlsupplies the handler,role="radio",aria-checkedandmin-h-tap, so button-wiring and tap-target contracts are inherited unchanged.Risk and rollout
considerations,countsandhighestToneare byte-identical across the whole corpus.sanitizeProfileandHEPATIC_LEVELSare untouched, and the stored byte for both "None" and "never touched" was alwaysnull.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Two knowingly-retained gaps, neither a regression, both worth recording rather than discovering later:
action: "info"rows are excluded from both tiers, so a green empty state stays reachable for them. 63 info rows exist, only 8 carry a readable gate, and they affect three medications where both other tiers are empty (nystatin,levetiracetam,lorazepam). Their own source notes justify it — lorazepam's hepatic row describes the drug as the safer option in hepatic impairment, to be shown as information rather than a caution.isProfileEmptyis unchanged, so a profile whose only entry is hepatic "None" still shows "Enter patient details above" rather than evaluating. Both surfaces degrade conservatively there (no verdict at all in the workspace, no all-clear in the panel), and changing it would make a one-field profile start producing evaluated output — its own change, with its own tests.This branch is two commits behind
main(569d8565,a1aa449a).git merge-tree --write-tree origin/mainreports a clean merge, andcheck:gate-manifestreads onlyCLAUDE.mdand.claude/skills/gates/SKILL.md, neither of which this branch touches — so it is behind, not conflicted, and deliberately not re-synced to avoid restarting CI for nothing.🤖 Generated with Claude Code
https://claude.ai/code/session_0183EiexuZe6uKqoACXGuANL
Note
Medium Risk
Changes clinician-facing safety messaging on the medication detail panel and alert evaluation semantics; impact is conservative (more gaps surfaced, no green all-clear over unread gates) but touches clinical decision-support presentation.
Overview
Stops the medication Considerations panel from showing a green “no matching considerations” notice when contraindication or dosing/monitoring gates were never evaluated. The empty state drops to neutral and calls out incomplete checks, aligned with the interactions block.
evaluatePatientAlertsnow keeps missing inputs even when a row fired on another criterion, and splits gaps intounassessed(contraindication only, still driving search verdict grey degrade) versusunassessedAdvisory/unassessedAdvisoryCount(dose-adjust, caution, monitor). The UI shows separate copy for each tier; advisory wording avoids “to see whether it applies” when an alert already fired above.The hepatic control gains a Not set segment (stored as
null) so None (hepatic: "none") is a real assessed answer, not the same as leaving the field blank.Tests cover tone degradation, tier copy, fired-row gaps, verdict partition, and hepatic UI behavior.
Reviewed by Cursor Bugbot for commit d5099a9. Configure here.