fix(medication-safety): fail-safe physiological bounds for patient-profile inputs (FV-03) - #1045
Conversation
…ofile inputs (FV-03) Patient-considerations profile numeric fields (age/eGFR/CrCl/QTc/serum creatinine) fed the medication-safety alert engine with no validation, so a garbage-but-present value in a gate's non-firing direction (e.g. a negative QTc) could silently flip a contraindication from the fail-safe "unassessed" state to a false all-clear. - sanitizeProfile now rejects physiologically impossible / out-of-range values to null (never clamps), routing them into the existing fail-closed unassessed path. Bounds are input-validity only and kept separate from the clinical firing thresholds. Serum creatinine is unit-aware (µmol/L canonical, mg/dL normalised x88.4). - Close a latent false-all-clear hole in the bare-renal factor: the "eGFR or CrCl" unassessed signal now fires when EITHER renal input is missing and neither fired (was: both missing). No behaviour change on the current corpus (0 bare-renal contraindication rows) — pure hardening. - Patient profile form gains min/max + aria-invalid + an inline out-of-range message; the invalid entry is never committed to the shared store. Fix design adversarially verified before implementation: null-routing can never convert a firing contraindication into a false all-clear (once the bare-renal hole is closed), and the bounds never reject a legitimate clinical value. Verified offline: typecheck, lint, format:check, full unit+jsdom suite (349 files / 3120 passed / 0 failed), design-system-contract (baselines unchanged), type-scale, icon-scale, check:production-readiness (READY). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughPatient profile numeric fields now enforce physiological bounds with reject-to-null storage, unit-aware serum creatinine validation, accessible dashboard errors, and reset behavior. Renal contraindication evaluation marks partial eGFR/CrCl input as unassessed, with tests and review documentation covering the changes. ChangesPatient Profile Safety
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Clinician
participant PatientProfilePanel
participant PatientProfileContext
participant PatientProfileStorage
participant evaluatePatientAlerts
Clinician->>PatientProfilePanel: enter profile values
PatientProfilePanel->>PatientProfileContext: persist profile update
PatientProfileContext->>PatientProfileStorage: sanitize and store profile
PatientProfileStorage-->>evaluatePatientAlerts: provide sanitized profile
evaluatePatientAlerts-->>Clinician: contraindication or unassessed result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
There was a problem hiding this comment.
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 `@src/components/clinical-dashboard/patient-profile-panel.tsx`:
- Around line 139-149: Update the scrUnit change handling around updateField so
the existing scr value is converted to the newly selected unit or cleared when
conversion is invalid, rather than persisted unchanged. Ensure the profile’s
serum creatinine is normalized immediately during a unit switch and remains
consistent with scrBounds and the storage-layer validation.
🪄 Autofix (Beta)
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 Plus
Run ID: cbc6aba3-67d6-4461-804f-9347876867d9
📒 Files selected for processing (7)
docs/branch-review-ledger.mdsrc/components/clinical-dashboard/patient-profile-panel.tsxsrc/lib/medication-patient-alerts.tssrc/lib/patient-profile-storage.tstests/medication-patient-alerts.test.tstests/patient-profile-panel.dom.test.tsxtests/patient-profile-storage.test.ts
Uh oh!
There was an error while loading. Please reload this page.
…e-input-bounds-123366 # Conflicts: # docs/branch-review-ledger.md
…03 follow-up)
Toggling the serum-creatinine unit (µmol/L ↔ mg/dL) previously called
updateField("scrUnit", …) but left the stored `scr` number unchanged, so
the alert engine re-read the same value on the new scale — a silent
misread of the renal input (a narrow window can flip a firing renal
contraindication to a false all-clear; the sanitiser fail-safes the rest
to "unassessed").
Add `convertScrValue` (µmol/L canonical, same ×88.4 factor the engine
uses) and a `setScrUnit` context action that converts `scr` and changes
the unit in a single atomic write, so the sanitiser never reads the
mismatched intermediate. The panel's unit buttons now call `setScrUnit`.
Tests: storage-level conversion + round-trip/validity, and a DOM test
proving a switch converts the stored value (90 µmol/L ↔ 1.02 mg/dL) and
round-trips.
Addresses the CodeRabbit review finding on PR #1045.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LRZ1EyBZW1ADXrZvMxEEsCThere was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/clinical-dashboard/patient-profile-panel.tsx (2)
221-229: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the active unit in serum-creatinine validation messaging.
NumberFieldonly appends a unit whenunitis provided, but the serum-creatinine call omits it. The error currently renders asEnter 0.17–33.93.without indicating whether the values are mg/dL or µmol/L.Proposed fix
<NumberField key={`scr-${resetNonce}-${scrUnit}`} label="Serum creatinine" + unit={scrUnit} value={profile.scr}🤖 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 `@src/components/clinical-dashboard/patient-profile-panel.tsx` around lines 221 - 229, Update the serum-creatinine NumberField in the patient profile panel to pass the active scrUnit through its unit prop, so validation messages identify whether bounds are in mg/dL or µmol/L. Preserve the existing scrBounds, value, and change-handling behavior.
91-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep Clear available for invalid buffered input.
An out-of-range value is committed as
null, so a profile containing only that buffered value becomesisEmptyand disables Clear at Line 317. The new reset/remount path is therefore unreachable until the user manually edits the field. Keep Clear enabled while any numeric field has a validation error, or lift that transient state to the panel.Also applies to: 313-316
🤖 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 `@src/components/clinical-dashboard/patient-profile-panel.tsx` around lines 91 - 101, Update the patient profile panel’s empty-state/Clear availability logic near the relevant isEmpty check and the numeric field change handler so buffered invalid or out-of-range input keeps Clear enabled even when the committed value is null. Track or lift transient validation-error state from the numeric field handler, preserve it until the field is corrected or reset, and use it when determining whether the profile is empty.
🤖 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.
Outside diff comments:
In `@src/components/clinical-dashboard/patient-profile-panel.tsx`:
- Around line 221-229: Update the serum-creatinine NumberField in the patient
profile panel to pass the active scrUnit through its unit prop, so validation
messages identify whether bounds are in mg/dL or µmol/L. Preserve the existing
scrBounds, value, and change-handling behavior.
- Around line 91-101: Update the patient profile panel’s empty-state/Clear
availability logic near the relevant isEmpty check and the numeric field change
handler so buffered invalid or out-of-range input keeps Clear enabled even when
the committed value is null. Track or lift transient validation-error state from
the numeric field handler, preserve it until the field is corrected or reset,
and use it when determining whether the profile is empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 636b3e71-0794-4951-9bf5-88bd1800f5ae
📒 Files selected for processing (5)
src/components/clinical-dashboard/patient-profile-context.tsxsrc/components/clinical-dashboard/patient-profile-panel.tsxsrc/lib/patient-profile-storage.tstests/patient-profile-panel.dom.test.tsxtests/patient-profile-storage.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/patient-profile-storage.test.ts
- tests/patient-profile-panel.dom.test.tsx
- src/lib/patient-profile-storage.ts
…e-input-bounds-123366
Uh oh!
There was an error while loading. Please reload this page.
Summary
Fail-safe input validation for the patient-considerations profile numeric fields (age / eGFR / CrCl / QTc / serum creatinine) that feed the medication-safety alert engine (finding FV-03).
sanitizeProfileinpatient-profile-storage.ts): a physiologically impossible or out-of-range entry becomesnull— never clamped — so the alert engine treats it as a missing input and surfaces a contraindication row as "unassessed" rather than reading garbage as a false all-clear. Bounds are input-validity only (age 0–130, eGFR 0–250, CrCl 0–400, QTc 240–800, SCr 15–3000 µmol/L, unit-aware for mg/dL) and are kept strictly separate from the clinical firing thresholds.renalfactor (medication-patient-alerts.ts): the "eGFR or CrCl" unassessed signal now fires when either renal input is missing and neither fired (was: only when both were missing). A renal contraindication therefore clears only when both eGFR and CrCl are present and non-firing. No behaviour change on the current corpus (0 bare-renal contraindication rows) — pure future-proofing.patient-profile-panel.tsx):min/max+aria-invalid+ an inline out-of-range message; the typed text stays visible for correction, and the invalid value is never committed to the shared store.The fix design was adversarially verified before implementation by an independent clinical-governance review, which confirmed that (1) null-routing can never convert a firing contraindication into a false all-clear once the bare-renal hole is closed, and (2) the bounds never reject a legitimate clinical value (neonate age 0, anuric eGFR/CrCl 0, short-QT ~250 ms, augmented renal clearance CrCl ~350, severe-AKI creatinine ~2200 µmol/L all pass).
Verification
npm run typecheck,npm run lint,npm run format:checknpm run test— full unit + jsdom suite: 349 files / 3120 passed / 0 failed, including the new FV-03 tests (storage bounds, the end-to-end garbage→unassessed regression guard for the adversarial counterexample, the engine bare-renal fail-safe, and the form validation/Clear behaviour)npm run check:design-system-contract(baselines unchanged: raw colors 9, literal shadows 1, legacy tap 28),npm run check:type-scale,npm run check:icon-scalenpm run check:production-readiness— READY (demo-mode env warnings only; a UI-input validation change cannot affect env/config)npm run verify:uiruns in CI (Chromium). The change is jsdom-tested and adds no layout/token/routing change; the only rendered difference (a danger border + inline message) appears solely on out-of-range input.Risk and rollout
RENAL_IMPAIRMENT_EGFR/QTC_PROLONGED_MS/ age cut-offs). Reject-to-null (never clamp) preserves the existing fail-closedunassessedpath. The engine&&→||change is a proven no-op on the current medication corpus.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)🤖 Generated with Claude Code
Summary by CodeRabbit