From eaced8226bed46527b28b0dbd9dec0fa01c8e0fe Mon Sep 17 00:00:00 2001 From: BernhardBuckel <33808384+BernhardBuckel@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:31:22 +0200 Subject: [PATCH] fix(labs): block a lab-scan save while a selected reading is incomplete The review dialog silently dropped any selected reading that could not be written (no date, no unit, no value) and saved the rest. When nothing was left it showed "Select at least one reading to save", which described a different situation and said nothing about what was missing. A save is now all-or-nothing over the selected readings. When one is incomplete, nothing is written; the dialog states how many readings need attention, marks each missing field (aria-invalid, inline message, red border, required marker on the label) and moves focus to the first one. Marks appear after a save attempt and follow the edits. The "nothing selected" message is kept for that case only. Validation is a pure function next to the payload mapping, so the rule that blocks a save and the rule that builds the payload cannot drift apart. Adds EN and DE strings; the other locales are left to the maintainers. Refs #1055 --- messages/de.json | 6 + messages/en.json | 6 + .../__tests__/ocr-review-validation.test.ts | 142 ++++++++++++++++++ .../labs/__tests__/ocr-row-editor.test.tsx | 84 +++++++++++ src/components/labs/ocr-review-dialog.tsx | 81 +++++----- src/components/labs/ocr-review-validation.ts | 104 +++++++++++++ src/components/labs/ocr-row-editor.tsx | 42 ++++++ 7 files changed, 427 insertions(+), 38 deletions(-) create mode 100644 src/components/labs/__tests__/ocr-review-validation.test.ts create mode 100644 src/components/labs/__tests__/ocr-row-editor.test.tsx create mode 100644 src/components/labs/ocr-review-validation.ts diff --git a/messages/de.json b/messages/de.json index 096e51bd7..8e8fd6197 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9002,6 +9002,12 @@ "savedEmptyToast": "Nichts zu speichern", "noRows": "In diesem Befund konnten keine Werte gefunden werden.", "nothingToSave": "Wähle mindestens einen Wert zum Speichern aus.", + "validationSummary": "Betroffene Werte: {count}. Ergänze die markierten Felder oder wähle diese Werte ab.", + "errorDate": "Bitte ein Datum angeben.", + "errorAnalyte": "Bitte einen Namen angeben.", + "errorValue": "Bitte einen Wert angeben.", + "errorResult": "Bitte ein Ergebnis angeben.", + "errorUnit": "Bitte eine Einheit angeben.", "saveFailed": "Werte konnten nicht gespeichert werden. Bitte erneut versuchen.", "providerUnsupported": "Dein KI-Anbieter kann keine Bilder lesen. Konfiguriere ein Modell mit Bildverständnis.", "consentRequired": "Für das Scannen eines Befunds ist die Einwilligung zur KI-Verarbeitung erforderlich.", diff --git a/messages/en.json b/messages/en.json index 3261cdbe7..5e3ca017e 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9002,6 +9002,12 @@ "savedEmptyToast": "Nothing to save", "noRows": "No readings could be found in this report.", "nothingToSave": "Select at least one reading to save.", + "validationSummary": "Readings needing attention: {count}. Complete the marked fields or deselect those readings.", + "errorDate": "Add the sample date.", + "errorAnalyte": "Add a biomarker name.", + "errorValue": "Add a value.", + "errorResult": "Add a result.", + "errorUnit": "Add a unit.", "saveFailed": "Couldn't save the readings. Please try again.", "providerUnsupported": "Your AI provider can't read images. Configure a vision-capable model.", "consentRequired": "AI processing consent is required to scan a report.", diff --git a/src/components/labs/__tests__/ocr-review-validation.test.ts b/src/components/labs/__tests__/ocr-review-validation.test.ts new file mode 100644 index 000000000..1052269b2 --- /dev/null +++ b/src/components/labs/__tests__/ocr-review-validation.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from "vitest"; + +import type { OcrReviewRow } from "../ocr-review-types"; +import { + collectRowErrors, + planSave, + toCommitRow, + validateReviewRow, +} from "../ocr-review-validation"; + +function row(overrides: Partial = {}): OcrReviewRow { + return { + key: "LDL-0", + analyte: "LDL", + value: 2.4, + valueText: null, + unit: "mmol/L", + referenceLow: 0, + referenceHigh: 3, + referenceText: null, + takenAt: "2026-06-10", + confidence: { analyte: 1, value: 1, unit: 1, range: 1 }, + biomarkerMatch: "existing", + duplicateOf: null, + confirmed: true, + ...overrides, + }; +} + +describe("validateReviewRow", () => { + it.each([ + ["a complete numeric row", {}, {}], + [ + "a complete qualitative row", + { value: null, valueText: "negative", unit: null }, + {}, + ], + ["a missing date", { takenAt: null }, { date: true }], + ["an unparseable date", { takenAt: "not-a-day" }, { date: true }], + ["a blank analyte", { analyte: " " }, { analyte: true }], + ["a missing value", { value: null }, { value: true }], + ["a non-finite value", { value: Number.NaN }, { value: true }], + ["a blank unit", { unit: " " }, { unit: true }], + ["a missing unit", { unit: null }, { unit: true }], + [ + "a blank qualitative result", + { value: null, valueText: " ", unit: null }, + { valueText: true }, + ], + [ + "several problems at once", + { takenAt: null, unit: "", analyte: "" }, + { date: true, unit: true, analyte: true }, + ], + ] as const)("%s", (_name, overrides, expected) => { + expect(validateReviewRow(row(overrides))).toEqual(expected); + }); + + it("does not ask a qualitative row for a unit", () => { + expect( + validateReviewRow(row({ value: null, valueText: "trace", unit: null })), + ).toEqual({}); + }); +}); + +describe("toCommitRow agrees with validateReviewRow", () => { + const cases: Partial[] = [ + {}, + { takenAt: null }, + { takenAt: "nope" }, + { analyte: "" }, + { value: null }, + { unit: "" }, + { value: null, valueText: "negative", unit: null }, + { value: null, valueText: "", unit: null }, + ]; + + it.each(cases)("row %j", (overrides) => { + const r = row(overrides); + const blocked = Object.keys(validateReviewRow(r)).length > 0; + expect(toCommitRow(r) === null).toBe(blocked); + }); + + it("anchors the date at noon UTC", () => { + expect(toCommitRow(row())?.takenAt).toBe("2026-06-10T12:00:00.000Z"); + }); +}); + +describe("collectRowErrors", () => { + it("ignores rows that are not selected", () => { + const rows = [row({ key: "a", takenAt: null, confirmed: false })]; + expect(collectRowErrors(rows).size).toBe(0); + }); + + it("keys the problems by row", () => { + const rows = [row({ key: "ok" }), row({ key: "bad", takenAt: null })]; + expect([...collectRowErrors(rows).entries()]).toEqual([ + ["bad", { date: true }], + ]); + }); +}); + +describe("planSave", () => { + it("reports nothing selected only when no row is selected", () => { + expect(planSave([])).toEqual({ kind: "nothing-selected" }); + expect(planSave([row({ confirmed: false })])).toEqual({ + kind: "nothing-selected", + }); + }); + + it("blocks the whole save when a selected row has no date, even if the others are fine", () => { + const plan = planSave([ + row({ key: "ok" }), + row({ key: "bad", takenAt: null }), + ]); + expect(plan).toEqual({ kind: "blocked" }); + }); + + it("is not reported as 'nothing selected' when the only selected row lacks a date", () => { + expect(planSave([row({ takenAt: null })]).kind).toBe("blocked"); + }); + + it("ignores an unselected invalid row and saves the selected ones", () => { + const plan = planSave([ + row({ key: "ok" }), + row({ key: "skipped", takenAt: null, confirmed: false }), + ]); + expect(plan.kind).toBe("ready"); + if (plan.kind === "ready") expect(plan.payload).toHaveLength(1); + }); + + it("returns every selected row once all of them are complete", () => { + const plan = planSave([ + row({ key: "a" }), + row({ key: "b", analyte: "HDL" }), + ]); + expect(plan.kind).toBe("ready"); + if (plan.kind === "ready") { + expect(plan.payload.map((r) => r.analyte)).toEqual(["LDL", "HDL"]); + } + }); +}); diff --git a/src/components/labs/__tests__/ocr-row-editor.test.tsx b/src/components/labs/__tests__/ocr-row-editor.test.tsx new file mode 100644 index 000000000..844d66d0b --- /dev/null +++ b/src/components/labs/__tests__/ocr-row-editor.test.tsx @@ -0,0 +1,84 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; + +import { I18nProvider } from "@/lib/i18n/context"; + +import type { OcrReviewRow } from "../ocr-review-types"; +import { OcrRowEditor } from "../ocr-row-editor"; + +const baseRow: OcrReviewRow = { + key: "LDL-0", + analyte: "LDL", + value: 2.4, + valueText: null, + unit: "mmol/L", + referenceLow: 0, + referenceHigh: 3, + referenceText: null, + takenAt: null, + confidence: { analyte: 1, value: 1, unit: 1, range: 1 }, + biomarkerMatch: "existing", + duplicateOf: null, + confirmed: true, +}; + +function render( + errors: Parameters[0]["errors"], + locale: "en" | "de" = "en", + row: OcrReviewRow = baseRow, +): string { + return renderToStaticMarkup( + + {}} errors={errors} /> + , + ); +} + +describe(" required-field marking", () => { + it("shows no error state before a save was attempted", () => { + const html = render(undefined); + expect(html).not.toContain('aria-invalid="true"'); + expect(html).not.toContain('role="alert"'); + }); + + it("marks the missing date, states why, and links the message to the field", () => { + const html = render({ date: true }); + const invalid = html.match(/]*aria-invalid="true"[^>]*>/g) ?? []; + expect(invalid).toHaveLength(1); + const describedBy = invalid[0].match(/aria-describedby="([^"]+)"/)?.[1]; + expect(describedBy).toBeTruthy(); + expect(html).toContain(`id="${describedBy}"`); + expect(html).toContain("Add the sample date."); + expect(html).toContain('role="alert"'); + // The bordered wrapper is what the eye sees. + expect(html).toContain("border-destructive"); + }); + + it("marks only the fields that are wrong", () => { + const html = render({ unit: true }); + expect(html.match(/aria-invalid="true"/g)).toHaveLength(1); + expect(html).toContain("Add a unit."); + expect(html).not.toContain("Add the sample date."); + }); + + it("marks a blank qualitative result", () => { + const html = render({ valueText: true }, "en", { + ...baseRow, + value: null, + valueText: "", + unit: null, + }); + expect(html).toContain("Add a result."); + expect(html.match(/aria-invalid="true"/g)).toHaveLength(1); + }); + + it("carries a visible required marker on the fields a row needs", () => { + const html = render(undefined); + // Value, unit and date on a numeric row. + expect(html.match(/text-destructive"[^>]*> \* { + expect(render({ date: true }, "de")).toContain("Bitte ein Datum angeben."); + }); +}); diff --git a/src/components/labs/ocr-review-dialog.tsx b/src/components/labs/ocr-review-dialog.tsx index b17162702..0a0b0a17c 100644 --- a/src/components/labs/ocr-review-dialog.tsx +++ b/src/components/labs/ocr-review-dialog.tsx @@ -10,9 +10,9 @@ * row editor. */ import { DocumentReadingConsentPrompt } from "@/components/ai/document-reading-consent-prompt"; -import { useId, useMemo, useState } from "react"; +import { useEffect, useId, useMemo, useRef, useState } from "react"; -import { Loader2, ScanLine, Upload } from "lucide-react"; +import { AlertCircle, Loader2, ScanLine, Upload } from "lucide-react"; import { toast } from "sonner"; import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; @@ -24,11 +24,11 @@ import { useTranslations } from "@/lib/i18n/context"; import { OcrRowEditor } from "./ocr-row-editor"; import { seedReviewRows, type OcrReviewRow } from "./ocr-review-types"; +import { collectRowErrors, planSave } from "./ocr-review-validation"; import { useOcrCommit, useOcrExtract, useOcrTextExtract, - type OcrCommitRowInput, } from "./use-ocr-extract"; type Stage = "pick" | "review"; @@ -53,35 +53,6 @@ export function handleFilePickerChange( if (file) onFilePicked(file); } -/** Map a confirmed review row to the commit payload, or null when invalid. */ -function toCommitRow(row: OcrReviewRow): OcrCommitRowInput | null { - const analyte = row.analyte.trim(); - if (!analyte || !row.takenAt) return null; - // A calendar day → an ISO instant at noon UTC, avoiding a TZ day-shift. - const takenAt = new Date(`${row.takenAt}T12:00:00.000Z`); - if (Number.isNaN(takenAt.getTime())) return null; - - const isQualitative = row.valueText !== null && row.valueText !== undefined; - if (isQualitative) { - const valueText = (row.valueText ?? "").trim(); - if (!valueText) return null; - return { analyte, valueText, takenAt: takenAt.toISOString() }; - } - - if (row.value === null || !Number.isFinite(row.value)) return null; - const unit = (row.unit ?? "").trim(); - if (!unit) return null; - return { - analyte, - value: row.value, - unit, - takenAt: takenAt.toISOString(), - ...(row.referenceLow !== null ? { referenceLow: row.referenceLow } : {}), - ...(row.referenceHigh !== null ? { referenceHigh: row.referenceHigh } : {}), - ...(row.referenceText ? { referenceText: row.referenceText } : {}), - }; -} - /** * Translate an extract failure into a friendly, error-code-aware message. * Refusals are read by `meta.errorCode`: a 403 is a missing consent only when @@ -163,6 +134,12 @@ export function OcrReviewDialog({ * else: the panel commits with it unset exactly as before. */ const [visitId, setVisitId] = useState(null); + // Errors stay hidden until a save is attempted: a row is not "wrong" just + // because the person has not filled it in yet. Once shown they follow the + // edits, so fixing a field clears its mark at once. + const [showErrors, setShowErrors] = useState(false); + const [focusRequest, setFocusRequest] = useState(0); + const reviewListRef = useRef(null); // Text mode OCR's the image in the browser then POSTs the text; vision mode // uploads the image. Both resolve with the same proposed-rows DTO. @@ -191,8 +168,22 @@ export function OcrReviewDialog({ return dates.length > 0 ? `${dates[0]}T12:00:00.000Z` : null; }, [rows]); + /** Blocking fields per confirmed row; rows that are writable are absent. */ + const rowErrors = useMemo(() => collectRowErrors(rows), [rows]); + + // Move focus to the first marked field once the marks have rendered. + useEffect(() => { + if (focusRequest === 0) return; + const first = reviewListRef.current?.querySelector( + '[aria-invalid="true"]', + ); + first?.focus(); + first?.scrollIntoView({ block: "center" }); + }, [focusRequest]); + function reset() { setStage("pick"); + setShowErrors(false); setRows([]); setPickedFile(null); setVisitId(null); @@ -223,14 +214,17 @@ export function OcrReviewDialog({ } function onSave() { - const payload = rows - .filter((r) => r.confirmed) - .map(toCommitRow) - .filter((r): r is OcrCommitRowInput => r !== null); - if (payload.length === 0) { + const plan = planSave(rows); + if (plan.kind === "nothing-selected") { toast.error(t("labs.ocr.nothingToSave")); return; } + if (plan.kind === "blocked") { + setShowErrors(true); + setFocusRequest((n) => n + 1); + return; + } + const { payload } = plan; commit.mutate( { rows: payload, file: pickedFile, encounterId: visitId }, { @@ -354,11 +348,21 @@ export function OcrReviewDialog({ ) : ( -
+

{t("labs.ocr.foundCount", { count: rows.length })}

+ {showErrors && rowErrors.size > 0 ? ( +

+ + {t("labs.ocr.validationSummary", { count: rowErrors.size })} +

+ ) : null} + {/* Anchored on the draw date the scan read, so the offer is about the panel rather than about today. */} setRows((prev) => prev.map((r) => (r.key === next.key ? next : r)), diff --git a/src/components/labs/ocr-review-validation.ts b/src/components/labs/ocr-review-validation.ts new file mode 100644 index 000000000..8e48e69bc --- /dev/null +++ b/src/components/labs/ocr-review-validation.ts @@ -0,0 +1,104 @@ +/** + * Per-row validation for the Lab-OCR review screen. + * + * A confirmed row can only be written when it carries everything the commit + * payload needs. `validateReviewRow` reports WHICH fields are missing so the + * review screen can mark them, and `toCommitRow` is the same rule applied to + * build the payload: it returns null exactly when `validateReviewRow` reports + * at least one field. + */ +import type { OcrReviewRow } from "./ocr-review-types"; +import type { OcrCommitRowInput } from "./use-ocr-extract"; + +export type OcrRowField = "analyte" | "value" | "valueText" | "unit" | "date"; + +/** The fields of one row that block saving; empty when the row is writable. */ +export type OcrRowErrors = Partial>; + +/** A calendar day → an ISO instant at noon UTC, avoiding a TZ day-shift. */ +function dayAtNoonUtc(day: string | null): Date | null { + if (!day) return null; + const at = new Date(`${day}T12:00:00.000Z`); + return Number.isNaN(at.getTime()) ? null : at; +} + +function isQualitativeRow(row: OcrReviewRow): boolean { + return row.valueText !== null && row.valueText !== undefined; +} + +export function validateReviewRow(row: OcrReviewRow): OcrRowErrors { + const errors: OcrRowErrors = {}; + if (!row.analyte.trim()) errors.analyte = true; + if (!dayAtNoonUtc(row.takenAt)) errors.date = true; + + if (isQualitativeRow(row)) { + if (!(row.valueText ?? "").trim()) errors.valueText = true; + } else { + if (row.value === null || !Number.isFinite(row.value)) errors.value = true; + if (!(row.unit ?? "").trim()) errors.unit = true; + } + return errors; +} + +export function hasRowErrors(errors: OcrRowErrors): boolean { + return Object.keys(errors).length > 0; +} + +/** Map a confirmed review row to the commit payload, or null when invalid. */ +export function toCommitRow(row: OcrReviewRow): OcrCommitRowInput | null { + const analyte = row.analyte.trim(); + const takenAt = dayAtNoonUtc(row.takenAt); + if (!analyte || !takenAt) return null; + + if (isQualitativeRow(row)) { + const valueText = (row.valueText ?? "").trim(); + if (!valueText) return null; + return { analyte, valueText, takenAt: takenAt.toISOString() }; + } + + if (row.value === null || !Number.isFinite(row.value)) return null; + const unit = (row.unit ?? "").trim(); + if (!unit) return null; + return { + analyte, + value: row.value, + unit, + takenAt: takenAt.toISOString(), + ...(row.referenceLow !== null ? { referenceLow: row.referenceLow } : {}), + ...(row.referenceHigh !== null ? { referenceHigh: row.referenceHigh } : {}), + ...(row.referenceText ? { referenceText: row.referenceText } : {}), + }; +} + +/** Blocking fields per CONFIRMED row, keyed by the row's key; writable rows are absent. */ +export function collectRowErrors( + rows: readonly OcrReviewRow[], +): Map { + const byKey = new Map(); + for (const row of rows) { + if (!row.confirmed) continue; + const errors = validateReviewRow(row); + if (hasRowErrors(errors)) byKey.set(row.key, errors); + } + return byKey; +} + +/** + * What pressing Save does. A save is all-or-nothing over the selected rows: + * one unwritable row blocks the whole save instead of being dropped, because a + * reading that is silently left out is a reading the person believes was kept. + */ +export type OcrSavePlan = + | { kind: "nothing-selected" } + | { kind: "blocked" } + | { kind: "ready"; payload: OcrCommitRowInput[] }; + +export function planSave(rows: readonly OcrReviewRow[]): OcrSavePlan { + const confirmed = rows.filter((row) => row.confirmed); + if (confirmed.length === 0) return { kind: "nothing-selected" }; + if (collectRowErrors(confirmed).size > 0) return { kind: "blocked" }; + const payload = confirmed + .map(toCommitRow) + .filter((row): row is OcrCommitRowInput => row !== null); + return { kind: "ready", payload }; +} diff --git a/src/components/labs/ocr-row-editor.tsx b/src/components/labs/ocr-row-editor.tsx index f11aa5bb2..ea1a06281 100644 --- a/src/components/labs/ocr-row-editor.tsx +++ b/src/components/labs/ocr-row-editor.tsx @@ -23,6 +23,7 @@ import { TriangleAlert, } from "lucide-react"; +import { FieldError } from "@/components/forms/field-error"; import { Badge } from "@/components/ui/badge"; import { Checkbox } from "@/components/ui/checkbox"; import { DateField } from "@/components/ui/date-field"; @@ -31,19 +32,43 @@ import { Label } from "@/components/ui/label"; import { useTranslations } from "@/lib/i18n/context"; import type { OcrReviewRow } from "./ocr-review-types"; +import type { OcrRowErrors } from "./ocr-review-validation"; /** Below this per-field confidence the field is flagged for a second look. */ const CONFIDENCE_THRESHOLD = 0.6; +/** The visible marker that a field must be filled before the row can save. */ +function RequiredMark() { + return ( + + {" *"} + + ); +} + export function OcrRowEditor({ row, onChange, + errors, }: { row: OcrReviewRow; onChange: (next: OcrReviewRow) => void; + /** The blocking fields of this row, once a save has been attempted. */ + errors?: OcrRowErrors; }) { const { t } = useTranslations(); const fieldId = useId(); + const invalidProps = (field: keyof OcrRowErrors) => + errors?.[field] + ? { + "aria-invalid": true as const, + "aria-describedby": `${fieldId}-${field}-error`, + } + : {}; + const fieldError = (field: keyof OcrRowErrors, messageKey: string) => + errors?.[field] ? ( + + ) : null; const isQualitative = row.valueText !== null && row.valueText !== undefined; const lowValueConfidence = @@ -98,8 +123,10 @@ export function OcrRowEditor({ value={row.analyte} onChange={(e) => onChange({ ...row, analyte: e.target.value })} aria-label={t("labs.ocr.analyteLabel")} + {...invalidProps("analyte")} className="font-medium" /> + {fieldError("analyte", "labs.ocr.errorAnalyte")}
{row.biomarkerMatch === "existing" ? ( @@ -133,18 +160,22 @@ export function OcrRowEditor({
onChange({ ...row, valueText: e.target.value })} + {...invalidProps("valueText")} /> + {fieldError("valueText", "labs.ocr.errorResult")}
) : ( <>
+ {fieldError("value", "labs.ocr.errorValue")}
onChange({ ...row, unit: e.target.value })} + {...invalidProps("unit")} /> + {fieldError("unit", "labs.ocr.errorUnit")}
)}
onChange({ ...row, takenAt: value || null })} + {...invalidProps("date")} + // The bordered wrapper is the visible control; the input inside + // it only carries the aria state. + className={errors?.date ? "border-destructive" : undefined} /> + {fieldError("date", "labs.ocr.errorDate")}