fix(labs): strict numeric parsing, atomic confirm guard, test integrity
- normalize.ts num(): parseFloat truncated at the first comma, so "1,200" silently became 1 (1000x error) and "5,5" became 5. Now strictly matches either US thousands-grouping or a plain number spanning the whole string; anything else (incl. ambiguous "5,5") returns null instead of a wrong value. - labs.ts confirm handler: the pending-status check ran before the request body was read, so two concurrent confirms could both pass it and double-insert. Added a guarded UPDATE ... WHERE status = 'pending' as the first statement inside the existing synchronous transaction; zero rows affected throws and the route returns 409, with the fast-path check kept for the common case. - Added missing `await` on two rejects.toThrow assertions (llm.test.ts, extract.test.ts) that were previously resolving before the assertion settled. - Bumped the 11th-failed-login rate-limit test to a 30s timeout — 10 sequential argon2id verifies can exceed bun:test's 5s default under load.
This commit is contained in:
@@ -24,9 +24,27 @@ export interface NormMarker {
|
||||
canonicalUnit: string | null;
|
||||
}
|
||||
|
||||
const THOUSANDS_GROUPED = /^\d{1,3}(,\d{3})+(\.\d+)?$/;
|
||||
const PLAIN_NUMBER = /^-?\d+(\.\d+)?$/;
|
||||
|
||||
// Strict numeric parsing: parseFloat alone stops at the first non-numeric
|
||||
// character, so "1,200" silently became 1 (a 1000x error) and "5,5" (a
|
||||
// European decimal) silently became 5. Instead: strip comparators/whitespace,
|
||||
// then only accept (a) US thousands-grouping, comma-stripped, or (b) a plain
|
||||
// number that spans the ENTIRE remaining string. Anything else — including
|
||||
// ambiguous "5,5" — returns null so no canonical value is computed rather
|
||||
// than a silently wrong one.
|
||||
const num = (v: string): number | null => {
|
||||
const n = parseFloat(v.replace(/[<>≤≥]/g, "").trim());
|
||||
return Number.isFinite(n) ? n : null;
|
||||
const stripped = v.replace(/[<>≤≥\s]/g, "");
|
||||
if (THOUSANDS_GROUPED.test(stripped)) {
|
||||
const n = parseFloat(stripped.replace(/,/g, ""));
|
||||
return Number.isFinite(n) ? n : null;
|
||||
}
|
||||
if (PLAIN_NUMBER.test(stripped)) {
|
||||
const n = parseFloat(stripped);
|
||||
return Number.isFinite(n) ? n : null;
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
export function normalizeMarker(raw: RawMarker): NormMarker {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { desc, eq } from "drizzle-orm";
|
||||
import type { Changes } from "bun:sqlite";
|
||||
import { and, desc, eq } from "drizzle-orm";
|
||||
import { Hono } from "hono";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { existsSync, mkdirSync } from "node:fs";
|
||||
@@ -88,20 +89,39 @@ export function labsRoutes(deps: LabsDeps) {
|
||||
panel: m.panel, name: m.name, value: m.value, unit: m.unit,
|
||||
referenceRange: m.referenceRange, flagged: m.flagged,
|
||||
}));
|
||||
deps.db.transaction((tx) => {
|
||||
tx.insert(labDraws).values({
|
||||
id: drawId, collectedAt: body.data.collectedDate, labName: body.data.labName,
|
||||
draftId: row.id, createdAt: Date.now(),
|
||||
}).run();
|
||||
for (const n of norm) {
|
||||
tx.insert(biomarkers).values({
|
||||
drawId, panel: n.panel, name: n.name, marker: n.marker, analyteKey: n.analyteKey,
|
||||
value: n.value, valueNum: n.valueNum, unit: n.unit, referenceRange: n.referenceRange,
|
||||
flagged: n.flagged ? 1 : 0, valueCanonical: n.valueCanonical, canonicalUnit: n.canonicalUnit,
|
||||
try {
|
||||
deps.db.transaction((tx) => {
|
||||
// Guarded status transition (pending -> confirmed) inside the same
|
||||
// synchronous transaction as the inserts below, so two concurrent
|
||||
// confirms of the same draft can't both pass the earlier status
|
||||
// check (before the request body was even read) and double-insert.
|
||||
// Only the request that actually flips the row gets to write rows.
|
||||
// drizzle-orm's bun-sqlite types pin TRunResult to `void`, but at
|
||||
// runtime bun:sqlite's Statement.run() actually returns a
|
||||
// `{ changes, lastInsertRowid }` Changes object — confirmed in
|
||||
// node_modules/bun-types/sqlite.d.ts. Cast to the real runtime type.
|
||||
const updated = tx.update(labDrafts).set({ status: "confirmed" })
|
||||
.where(and(eq(labDrafts.id, row.id), eq(labDrafts.status, "pending"))).run() as unknown as Changes;
|
||||
if (updated.changes === 0) throw new Error("draft_not_pending");
|
||||
|
||||
tx.insert(labDraws).values({
|
||||
id: drawId, collectedAt: body.data.collectedDate, labName: body.data.labName,
|
||||
draftId: row.id, createdAt: Date.now(),
|
||||
}).run();
|
||||
for (const n of norm) {
|
||||
tx.insert(biomarkers).values({
|
||||
drawId, panel: n.panel, name: n.name, marker: n.marker, analyteKey: n.analyteKey,
|
||||
value: n.value, valueNum: n.valueNum, unit: n.unit, referenceRange: n.referenceRange,
|
||||
flagged: n.flagged ? 1 : 0, valueCanonical: n.valueCanonical, canonicalUnit: n.canonicalUnit,
|
||||
}).run();
|
||||
}
|
||||
});
|
||||
} catch (e) {
|
||||
if (e instanceof Error && e.message === "draft_not_pending") {
|
||||
return c.json({ error: "draft is not pending" }, 409);
|
||||
}
|
||||
tx.update(labDrafts).set({ status: "confirmed" }).where(eq(labDrafts.id, row.id)).run();
|
||||
});
|
||||
throw e;
|
||||
}
|
||||
return c.json({ drawId }, 201);
|
||||
});
|
||||
|
||||
|
||||
@@ -54,7 +54,7 @@ describe("auth", () => {
|
||||
// The global window applies to everyone, including a request with the correct password.
|
||||
const blocked = await app.request("/api/login", json({ password: "hunter2hunter2" }));
|
||||
expect(blocked.status).toBe(429);
|
||||
});
|
||||
}, 30000); // 10 sequential argon2id verifies can exceed the 5s default timeout
|
||||
|
||||
test("unknown /api/* returns 404 when authed, 401 when unauthed", async () => {
|
||||
const app = makeApp();
|
||||
|
||||
@@ -41,6 +41,6 @@ describe("extractFromText", () => {
|
||||
test("malformed LLM output → llm_error, not a crash", async () => {
|
||||
const mock = (async () => new Response(JSON.stringify({ choices: [{ message: { content: '{"markers": "not an array"}' } }] }), { status: 200 })) as unknown as typeof fetch;
|
||||
const dir = mkdtempSync(join(tmpdir(), "helios-"));
|
||||
expect(extractFromText({ db: openDb(dir), key: loadOrCreateKey(dir), fetchImpl: mock }, "x")).rejects.toThrow(/llm_error/);
|
||||
await expect(extractFromText({ db: openDb(dir), key: loadOrCreateKey(dir), fetchImpl: mock }, "x")).rejects.toThrow(/llm_error/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -64,6 +64,13 @@ describe("draft review", () => {
|
||||
body: JSON.stringify({ collectedDate: "2026-01-15", labName: null, markers: [{ panel: null, name: "X", value: "1", unit: null, referenceRange: null, flagged: false }] }),
|
||||
});
|
||||
expect(again.status).toBe(409);
|
||||
|
||||
// Second (rejected) confirm must not have inserted a second draw or any
|
||||
// extra biomarker rows — exactly one draw, one set of biomarkers.
|
||||
const drawsAfter = await db.select().from(labDraws);
|
||||
expect(drawsAfter).toHaveLength(1);
|
||||
const rowsAfter = await db.select().from(biomarkers);
|
||||
expect(rowsAfter).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("discard marks draft discarded", async () => {
|
||||
|
||||
@@ -46,6 +46,6 @@ describe("chatJSON", () => {
|
||||
|
||||
test("non-2xx throws llm_error", async () => {
|
||||
const mock = (async () => new Response("nope", { status: 401 })) as unknown as typeof fetch;
|
||||
expect(chatJSON(deps(mock), { system: "s", user: "u" })).rejects.toThrow(/llm_error/);
|
||||
await expect(chatJSON(deps(mock), { system: "s", user: "u" })).rejects.toThrow(/llm_error/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -61,4 +61,21 @@ describe("normalizeMarker", () => {
|
||||
const n = normalizeMarker({ name: "DHEA-S", value: "250", unit: "ug/dL" });
|
||||
expect(n.valueCanonical).toBeCloseTo(6.78, 1); // 250 * 0.02713
|
||||
});
|
||||
test("thousands-grouped value parses as 1200, not 1 (parseFloat truncation bug)", () => {
|
||||
const n = normalizeMarker({ name: "Vitamin B12", value: "1,200", unit: "pg/mL" });
|
||||
expect(n.valueNum).toBe(1200);
|
||||
expect(n.analyteKey).toBe("vitamin_b12");
|
||||
expect(n.valueCanonical).toBeCloseTo(885.35, 1);
|
||||
expect(n.canonicalUnit).toBe("pmol/L");
|
||||
});
|
||||
test("ambiguous European-decimal-looking value is left unmapped, not silently wrong", () => {
|
||||
const n = normalizeMarker({ name: "Glucose", value: "5,5", unit: "mg/dL" });
|
||||
expect(n.valueNum).toBeNull();
|
||||
expect(n.valueCanonical).toBeNull();
|
||||
expect(n.value).toBe("5,5"); // raw value preserved
|
||||
});
|
||||
test("comparator value still parses after strict-parsing rewrite", () => {
|
||||
const n = normalizeMarker({ name: "hs-CRP", value: "<0.3", unit: "mg/L" });
|
||||
expect(n.valueNum).toBeCloseTo(0.3);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user