mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 21:32:18 +00:00
fix(labs): re-derive a marker's analyte key when its name or unit changes
The key is a function of the marker's name and unit, but it was only ever filled, never replaced, so a marker renamed from Glucose to Cholesterol kept converting readings with the glucose factor. The marker edit now re-derives it with the same match the creation paths use (clearing it when the new name matches nothing), and the fill step a reading runs brings a stored key back in line with the name, so a key written before this fix is corrected the first time it matters. The scan review derives the key the same way as the commit.
This commit is contained in:
1 parent
747e0964ad
commit
cc0313c955
4 files changed
+216
-44
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<PrismaClient, "biomarker">,
|
||||
marker: {
|
||||
id: string;
|
||||
name: string;
|
||||
unit: string;
|
||||
analyteKey?: string | null;
|
||||
},
|
||||
marker: MarkerKeyState,
|
||||
): Promise<string | null> {
|
||||
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 });
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
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<string, unknown>) {
|
||||
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 () => {
|
||||
|
||||
Reference in new issue
Block a user