diff --git a/src/app/api/biomarkers/[id]/route.ts b/src/app/api/biomarkers/[id]/route.ts index d3aec468b..506333ad3 100644 --- a/src/app/api/biomarkers/[id]/route.ts +++ b/src/app/api/biomarkers/[id]/route.ts @@ -17,6 +17,7 @@ import { decryptContextFromBytes, encryptContextToBytes, } from "@/lib/labs/biomarker-store"; +import { matchAnalyteKey } from "@/lib/labs/analyte-match"; import { annotate } from "@/lib/logging/context"; import { updateBiomarkerSchema } from "@/lib/validations/biomarkers"; import { effectiveBound, isInvertedRange } from "@/lib/validations/labs"; @@ -176,6 +177,16 @@ export const PUT = apiHandler( if (d.upperBound !== undefined) data.upperBound = d.upperBound; if (d.panel !== undefined) data.panel = d.panel; if (d.hidden !== undefined) data.hidden = d.hidden; + // The analyte a marker is follows from its name and unit, the same match + // the creation paths use. A rename must move it, or a marker renamed from + // "Glucose" to "Cholesterol" would keep converting readings with the + // glucose factor; a name that now matches nothing clears it. + if (d.name !== undefined || d.unit !== undefined) { + data.analyteKey = matchAnalyteKey({ + name: d.name ?? existing.name, + unit: d.unit ?? existing.unit, + }); + } // Inverted-range guard for a PARTIAL bound update. The schema refine only // fires when both bounds arrive together; moving a single bound past the diff --git a/src/lib/labs/analyte-key-store.ts b/src/lib/labs/analyte-key-store.ts index f11720e53..dfefa416d 100644 --- a/src/lib/labs/analyte-key-store.ts +++ b/src/lib/labs/analyte-key-store.ts @@ -1,37 +1,53 @@ /** * Link a marker to its analyte for the unit conversion. * - * The key is set when a marker is created and, for a marker that already - * existed, the first time a reading needs it. It is never overwritten: a key a - * person (or an earlier pass) set stands, and `updateMany` is gated on the - * column still being null, so two concurrent writers converge. A marker whose - * name matches no analyte keeps a null key and gets no conversion. + * There is no manual choice of analyte: the key is a function of the marker's + * name and unit (`matchAnalyteKey`). It is set when a marker is created and + * re-derived whenever its name or unit changes, and this step brings a stored + * key back in line with that function the first time a reading needs it. So a + * marker that never had a key gets one, and a key that no longer agrees with + * the name (written before a rename re-derived it, or in an older form) is + * replaced, or cleared when the name now matches nothing. A key that agrees is + * left alone, so the step is idempotent. + * + * The write is gated on the column still holding what the caller read, so two + * concurrent writers converge, and a caller holding a stale copy re-reads the + * row rather than writing over a newer value. */ import type { PrismaClient } from "@/generated/prisma/client"; import { matchAnalyteKey } from "./analyte-match"; +interface MarkerKeyState { + id: string; + name: string; + unit: string; + analyteKey?: string | null; +} + export async function ensureAnalyteKey( client: Pick, - marker: { - id: string; - name: string; - unit: string; - analyteKey?: string | null; - }, + marker: MarkerKeyState, ): Promise { - if (marker.analyteKey) return marker.analyteKey; - const key = matchAnalyteKey({ name: marker.name, unit: marker.unit }); - if (!key) return null; - const { count } = await client.biomarker.updateMany({ - where: { id: marker.id, analyteKey: null }, - data: { analyteKey: key }, - }); - if (count > 0) return key; - // Another writer set it first; read what stands. - const current = await client.biomarker.findUnique({ - where: { id: marker.id }, - select: { analyteKey: true }, - }); - return current?.analyteKey ?? null; + let current: MarkerKeyState = marker; + // Two rounds: the caller's copy, then the row as it stands if that copy was + // stale. A third writer racing both is vanishingly rare; the derived key is + // still the right answer for the name this call saw. + for (let round = 0; round < 2; round++) { + const derived = matchAnalyteKey({ name: current.name, unit: current.unit }); + const stored = current.analyteKey ?? null; + if (stored === derived) return derived; + const { count } = await client.biomarker.updateMany({ + where: { id: current.id, analyteKey: stored }, + data: { analyteKey: derived }, + }); + if (count > 0) return derived; + const row = await client.biomarker.findUnique({ + where: { id: current.id }, + select: { id: true, name: true, unit: true, analyteKey: true }, + }); + if (!row) return null; + current = row; + } + return matchAnalyteKey({ name: current.name, unit: current.unit }); } diff --git a/src/lib/labs/ocr-extract.ts b/src/lib/labs/ocr-extract.ts index 657445fd3..59740c81a 100644 --- a/src/lib/labs/ocr-extract.ts +++ b/src/lib/labs/ocr-extract.ts @@ -146,16 +146,18 @@ export async function annotateRow( // Biomarker match — case-insensitive on the catalog identity, presence-only. const existingBiomarker = await prisma.biomarker.findFirst({ where: { userId, name: { equals: analyte, mode: "insensitive" } }, - select: { id: true, name: true, unit: true, analyteKey: true }, + select: { id: true, name: true, unit: true }, }); // The analyte the marker is, so the review can say whether a row in another - // unit converts. Read only: the key is filled when a reading is written. + // unit converts. Derived from the name and unit exactly as the commit derives + // it (a stored key is only that derivation, written down), so the review and + // the commit cannot disagree. Read only: the key is stored when a reading is + // written. const markerAnalyteKey = existingBiomarker - ? (existingBiomarker.analyteKey ?? - matchAnalyteKey({ + ? matchAnalyteKey({ name: existingBiomarker.name, unit: existingBiomarker.unit, - })) + }) : null; // Duplicate check: a live lab_results row with the same analyte (fuzzy, diff --git a/tests/integration/labs-analyte-key.test.ts b/tests/integration/labs-analyte-key.test.ts index df12eb7c3..595c70261 100644 --- a/tests/integration/labs-analyte-key.test.ts +++ b/tests/integration/labs-analyte-key.test.ts @@ -1,15 +1,19 @@ /** - * A marker is linked to its analyte, once, and the link is never overwritten. + * A marker is linked to its analyte, and the link follows its name and unit. * * `Biomarker.analyteKey` says which analyte a marker is, so a reading in another - * unit can be converted (#1095). It is set when a marker is created, by all - * three creation paths, and, for a marker that already exists, the first time - * it is needed. Against a real Postgres: + * unit can be converted (#1095). There is no manual choice: the key is derived + * from the name and unit. It is set when a marker is created, by all three + * creation paths, re-derived when a rename or unit change moves it, and, for a + * marker that already exists, brought in line the first time it is needed. + * Against a real Postgres: * - a marker created by name is linked when the name is a known analyte and * its unit is one the analyte reaches, and left null otherwise; * - the key reaches the API response; - * - `ensureAnalyteKey` fills a null key once and never replaces a key that is - * set, even by a different match, and two concurrent calls converge; + * - a rename moves the key, or clears it, and the next reading in another + * unit converts with the new analyte's factor; + * - `ensureAnalyteKey` fills a null key, replaces one that disagrees with the + * name, and two concurrent calls converge; * - the mint path used by the lab writes and the legacy backfill set it too. */ import { NextRequest } from "next/server"; @@ -100,6 +104,109 @@ describe("POST /api/biomarkers", () => { }); }); +async function patch(id: string, body: Record) { + const s = await prisma.session.create({ + data: { userId: USER, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.clear(); + cookieJar.set("healthlog_session", s.id); + const { PUT } = await import("@/app/api/biomarkers/[id]/route"); + return PUT( + new NextRequest(`http://localhost/api/biomarkers/${id}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }), + { params: Promise.resolve({ id }) }, + ); +} + +async function postReading(body: Record) { + const s = await prisma.session.create({ + data: { userId: USER, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.clear(); + cookieJar.set("healthlog_session", s.id); + const { POST } = await import("@/app/api/labs/route"); + return POST( + new NextRequest("http://localhost/api/labs", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }), + ); +} + +describe("PUT /api/biomarkers/[id]", () => { + it("moves the key with a rename, and the next reading converts with the new analyte", async () => { + const created = await post({ name: "Blood sugar", unit: "mg/dL" }); + const { data: marker } = (await created.json()) as { + data: { id: string; analyteKey: string | null }; + }; + expect(marker.analyteKey).toBe("fasting-glucose"); + + const res = await patch(marker.id, { name: "Cholesterol" }); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { analyteKey: string | null }; + }; + expect(body.data.analyteKey).toBe("total-cholesterol"); + expect( + (await prisma.biomarker.findUnique({ where: { id: marker.id } })) + ?.analyteKey, + ).toBe("total-cholesterol"); + + // 5.2 mmol/L of cholesterol is 201.07 mg/dL (C27H46O, 386.664 g/mol). With + // the glucose key left over it would have been stored as 93.7. + const reading = await postReading({ + biomarkerId: marker.id, + value: 5.2, + unit: "mmol/L", + takenAt: "2026-05-03T08:00:00.000Z", + }); + expect(reading.status).toBe(201); + const { data: dto } = (await reading.json()) as { + data: { value: number; unit: string; converted: boolean }; + }; + expect(dto.converted).toBe(true); + expect(dto.unit).toBe("mg/dL"); + expect(dto.value.toFixed(2)).toBe("201.07"); + }); + + it("clears the key when the new name matches no analyte", async () => { + const created = await post({ name: "Harnstoff", unit: "mg/dL" }); + const { data: marker } = (await created.json()) as { + data: { id: string; analyteKey: string | null }; + }; + expect(marker.analyteKey).toBe("urea"); + const res = await patch(marker.id, { name: "Harnstoff (eigene Notiz)" }); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { analyteKey: string | null } }; + expect(body.data.analyteKey).toBeNull(); + }); + + it("re-derives the key when the unit changes on a marker without readings", async () => { + const created = await post({ name: "Glukose", unit: "mEq/L" }); + const { data: marker } = (await created.json()) as { + data: { id: string; analyteKey: string | null }; + }; + expect(marker.analyteKey).toBeNull(); + const res = await patch(marker.id, { unit: "mg/dL" }); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { analyteKey: string | null } }; + expect(body.data.analyteKey).toBe("fasting-glucose"); + }); + + it("leaves the key alone on an edit that touches neither name nor unit", async () => { + const created = await post({ name: "Kalzium gesamt", unit: "mmol/L" }); + const { data: marker } = (await created.json()) as { data: { id: string } }; + const res = await patch(marker.id, { lowerBound: 2.1 }); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { analyteKey: string | null } }; + expect(body.data.analyteKey).toBe("calcium-total"); + }); +}); + describe("ensureAnalyteKey", () => { it("fills a null key once", async () => { const marker = await prisma.biomarker.create({ @@ -113,7 +220,12 @@ describe("ensureAnalyteKey", () => { ).toBe("uric-acid"); }); - it("never replaces a key that is set, even when the name would match another", async () => { + // This used to pin the opposite: a set key was never replaced. With no + // manual choice of analyte, a key that disagrees with the name can only be + // left over from a rename (or written in an older form), and keeping it + // converted readings with the wrong analyte's factor. So the key follows the + // name. + it("replaces a key that disagrees with the marker's name", async () => { const marker = await prisma.biomarker.create({ data: { userId: USER, @@ -122,16 +234,47 @@ describe("ensureAnalyteKey", () => { analyteKey: "hba1c", }, }); - // The caller read the row with the key set. - expect(await ensureAnalyteKey(prisma, marker)).toBe("hba1c"); - // A caller holding a stale copy with a null key does not overwrite either. - expect( - await ensureAnalyteKey(prisma, { ...marker, analyteKey: null }), - ).toBe("hba1c"); + expect(await ensureAnalyteKey(prisma, marker)).toBe("fasting-glucose"); expect( (await prisma.biomarker.findUnique({ where: { id: marker.id } })) ?.analyteKey, - ).toBe("hba1c"); + ).toBe("fasting-glucose"); + }); + + it("clears a key when the name matches no analyte", async () => { + const marker = await prisma.biomarker.create({ + data: { + userId: USER, + name: "Some other marker", + unit: "mg/dL", + analyteKey: "fasting-glucose", + }, + }); + expect(await ensureAnalyteKey(prisma, marker)).toBeNull(); + expect( + (await prisma.biomarker.findUnique({ where: { id: marker.id } })) + ?.analyteKey, + ).toBeNull(); + }); + + it("re-reads the row when the caller's copy is stale", async () => { + const marker = await prisma.biomarker.create({ + data: { + userId: USER, + name: "Urea", + unit: "mg/dL", + analyteKey: "urea", + }, + }); + // A copy read before the key was set: the gated write misses, the row as it + // stands already agrees, and nothing is overwritten. + expect( + await ensureAnalyteKey(prisma, { ...marker, analyteKey: null }), + ).toBe("urea"); + expect( + (await prisma.biomarker.findUnique({ where: { id: marker.id } })) + ?.analyteKey, + ).toBe("urea"); }); it("converges when two callers fill the same marker", async () => {