From 58141cb48bf9a9faecefe2145e5d9a18114f2b80 Mon Sep 17 00:00:00 2001 From: haritowa <6107211+haritowa@users.noreply.github.com> Date: Mon, 26 Oct 2026 00:24:48 +0000 Subject: [PATCH 1/8] fix(documents): date an undated lab value by its document A lab report usually prints the collection date once, in its header. The extraction puts that date on the document (reportDate) and leaves each value's own date empty, and the confirm step filed every such value under the moment it was confirmed, so an old report showed up as today's results. The confirm route now reads the document's report date and filing date with the document it already loads, and an undated value takes the report date, else the filing date, else now: the order the day view places a document by. A stated date, a lab value's or a condition's onset, now counts only when it names a real calendar day. 2021-02-30 used to be stored as 2 March, and 2021-13-01 became an invalid date that failed the whole confirm. --- .../documents/inbound/[id]/confirm/route.ts | 16 +- src/lib/documents/__tests__/commit.test.ts | 189 +++++++++++++++++- src/lib/documents/commit.ts | 52 ++++- 3 files changed, 244 insertions(+), 13 deletions(-) diff --git a/src/app/api/documents/inbound/[id]/confirm/route.ts b/src/app/api/documents/inbound/[id]/confirm/route.ts index 564af8e62..919398228 100644 --- a/src/app/api/documents/inbound/[id]/confirm/route.ts +++ b/src/app/api/documents/inbound/[id]/confirm/route.ts @@ -43,7 +43,8 @@ async function confirmDocument( const document = await prisma.inboundDocument.findFirst({ where: { id, userId: user.id, deletedAt: null }, - select: { id: true }, + // The two dates date a value that states none of its own. + select: { id: true, reportDate: true, documentDate: true }, }); if (!document) { return apiError("Document not found", 404, { @@ -94,10 +95,15 @@ async function confirmDocument( try { // v1.43 — an OBSERVATION may name the marker the person chose // on the review screen; the label is then remembered as its alias. - const ref = await commitApprovedFact(user.id, fact, { - biomarkerId: decision.biomarkerId, - rememberAlias: decision.rememberAlias, - }); + const ref = await commitApprovedFact( + user.id, + fact, + { + biomarkerId: decision.biomarkerId, + rememberAlias: decision.rememberAlias, + }, + document, + ); await prisma.extractedFact.update({ where: { id: fact.id }, data: { diff --git a/src/lib/documents/__tests__/commit.test.ts b/src/lib/documents/__tests__/commit.test.ts index 591ebae47..81deb22dd 100644 --- a/src/lib/documents/__tests__/commit.test.ts +++ b/src/lib/documents/__tests__/commit.test.ts @@ -12,6 +12,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("@/lib/db", () => ({ prisma: { labResult: { create: vi.fn() }, + illnessEpisode: { create: vi.fn() }, }, })); @@ -27,8 +28,16 @@ import { commitApprovedFact, FactCommitError } from "@/lib/documents/commit"; import { prisma } from "@/lib/db"; import { resolveOrMintBiomarker } from "@/lib/labs/biomarker-store"; import { decryptFactData } from "@/lib/documents/store"; -import type { ExtractedFact } from "@/generated/prisma/client"; +import type { + ExtractedFact, + IllnessEpisode, + LabResult, +} from "@/generated/prisma/client"; import { userDayKey } from "@/lib/tz/format"; +import type { + ConditionFactData, + ObservationFactData, +} from "@/lib/validations/inbound-documents"; function observationFact(): ExtractedFact { return { @@ -213,3 +222,181 @@ describe("commitApprovedFact — OBSERVATION unit integrity", () => { ).resolves.toEqual({ recordType: "labResult", recordId: "lr-2" }); }); }); + +describe("commitApprovedFact — OBSERVATION date when the value states none", () => { + // A lab report that prints the collection date once in its header carries + // no date beside each value. Mutation check: drop the document day from + // `takenAt` → the report-date, filing-date and impossible-date cases file + // the reading under today. + function undated(effectiveDate: string | null = null) { + const data: ObservationFactData = { + label: "Ferritin", + code: null, + codeSystem: null, + value: 80, + valueText: null, + unit: "ng/mL", + referenceLow: null, + referenceHigh: null, + effectiveDate, + }; + vi.mocked(decryptFactData).mockReturnValue(data); + vi.mocked(resolveOrMintBiomarker).mockResolvedValue({ + id: "bm-3", + name: "Ferritin", + unit: "ng/mL", + lowerBound: null, + upperBound: null, + panel: null, + analyteKey: null, + }); + vi.mocked(prisma.labResult.create).mockResolvedValue({ + id: "lr-3", + } as LabResult); + } + + /** The dates as the confirm route reads them off the stored document. */ + function dates(reportDate: string | null, documentDate: string | null) { + return { + // An older report date sits at UTC midnight, newer ones at noon UTC. + reportDate: reportDate ? new Date(`${reportDate}T00:00:00.000Z`) : null, + documentDate: documentDate + ? new Date(`${documentDate}T12:00:00.000Z`) + : null, + }; + } + + function storedTakenAt(): Date { + return vi.mocked(prisma.labResult.create).mock.calls[0]![0].data + .takenAt as Date; + } + + it("keeps the value's own date", async () => { + undated("2021-03-04"); + + await commitApprovedFact( + "user-1", + observationFact(), + {}, + dates("2021-03-01", "2026-10-20"), + ); + + expect(storedTakenAt().toISOString()).toBe("2021-03-04T12:00:00.000Z"); + }); + + it("takes the report date the document states over its filing date", async () => { + undated(); + + await commitApprovedFact( + "user-1", + observationFact(), + {}, + dates("2021-03-01", "2026-10-20"), + ); + + expect(storedTakenAt().toISOString()).toBe("2021-03-01T12:00:00.000Z"); + }); + + it("takes the filing date when the document states no report date", async () => { + undated(); + + await commitApprovedFact( + "user-1", + observationFact(), + {}, + dates(null, "2021-02-15"), + ); + + expect(storedTakenAt().toISOString()).toBe("2021-02-15T12:00:00.000Z"); + }); + + it.each([ + ["not a date", "March 2021"], + ["a day the month does not have", "2021-02-30"], + ["a month that does not exist", "2021-13-01"], + ])( + "reads a value whose own date is %s as undated", + async (_label, effectiveDate) => { + undated(effectiveDate); + + await commitApprovedFact( + "user-1", + observationFact(), + {}, + dates("2021-03-01", null), + ); + + // Not 2 March for 30 February, and no Invalid Date for month 13. + expect(storedTakenAt().toISOString()).toBe("2021-03-01T12:00:00.000Z"); + }, + ); + + it("files the reading under now when neither the value nor the document has a date", async () => { + undated(); + + const before = Date.now(); + await commitApprovedFact( + "user-1", + observationFact(), + {}, + dates(null, null), + ); + const after = Date.now(); + + const takenAt = storedTakenAt().getTime(); + expect(takenAt).toBeGreaterThanOrEqual(before); + expect(takenAt).toBeLessThanOrEqual(after); + }); +}); + +describe("commitApprovedFact — CONDITION onset", () => { + // The onset goes through the same stated-day check as a lab value's date. + function condition(onsetDate: string | null) { + const data: ConditionFactData = { + label: "Iron deficiency", + code: null, + codeSystem: null, + clinicalStatus: null, + verificationStatus: null, + onsetDate, + }; + vi.mocked(decryptFactData).mockReturnValue(data); + vi.mocked(prisma.illnessEpisode.create).mockResolvedValue({ + id: "ep-1", + } as IllnessEpisode); + } + + function conditionFact(): ExtractedFact { + return { + id: "fact-2", + factType: "CONDITION", + dataEncrypted: new Uint8Array(), + } as unknown as ExtractedFact; + } + + function storedOnset(): Date { + return vi.mocked(prisma.illnessEpisode.create).mock.calls[0]![0].data + .onsetAt as Date; + } + + it("keeps a stated onset day", async () => { + condition("2021-03-04"); + + await commitApprovedFact("user-1", conditionFact()); + + expect(storedOnset().toISOString()).toBe("2021-03-04T12:00:00.000Z"); + }); + + it("files an onset that is not a real day under now, not an invalid date", async () => { + // 2021-13-01 used to become an invalid date that failed the whole confirm. + condition("2021-13-01"); + + const before = Date.now(); + await commitApprovedFact("user-1", conditionFact()); + const after = Date.now(); + + const onset = storedOnset().getTime(); + expect(onset).toBeGreaterThanOrEqual(before); + expect(onset).toBeLessThanOrEqual(after); + }); +}); diff --git a/src/lib/documents/commit.ts b/src/lib/documents/commit.ts index db39f3d31..17bae2712 100644 --- a/src/lib/documents/commit.ts +++ b/src/lib/documents/commit.ts @@ -44,7 +44,11 @@ import type { MedicationStatementFactData, ObservationFactData, } from "@/lib/validations/inbound-documents"; -import { dateOnlyAtNoonUtc, isDateOnlyKey } from "@/lib/tz/date-only"; +import { + dateOnlyAtNoonUtc, + dateOnlyKey, + isCalendarDateKey, +} from "@/lib/tz/date-only"; /** A per-fact commit failure the caller maps to a per-fact 422 entry. */ export class FactCommitError extends Error { @@ -61,14 +65,41 @@ export interface CommittedRecordRef { recordId: string; } +/** A stated day, or null when it is not a real calendar date. */ +function statedDay(date: string | null): string | null { + return date && isCalendarDateKey(date) ? date : null; +} + /** * A stated YYYY-MM-DD as the instant a date-only value is stored at (noon - * UTC), or now when the document states none. UTC midnight read back as the + * UTC), else the fallback day, else now. UTC midnight read back as the * previous day anywhere west of UTC. */ -function statedDateOrNow(date: string | null): Date { - if (date && isDateOnlyKey(date)) return dateOnlyAtNoonUtc(date); - return new Date(); +function statedDateOrNow( + date: string | null, + fallback: string | null = null, +): Date { + const day = statedDay(date) ?? fallback; + return day ? dateOnlyAtNoonUtc(day) : new Date(); +} + +/** The two dates a stored document carries, as the confirm route reads them. */ +export interface FactDocumentDates { + reportDate: Date | null; + documentDate: Date | null; +} + +/** + * The day a value from this document took place when it states none of its + * own: the report date the model read off the document, else the document's + * filing date, the same order the day view and the timeline place a document + * by. A lab report that prints the collection date once in its header carries + * no date beside each value, and every reading used to be filed under the day + * it was confirmed. + */ +function documentDay(dates: FactDocumentDates | null): string | null { + const day = dates?.reportDate ?? dates?.documentDate ?? null; + return day ? dateOnlyKey(day) : null; } /** @@ -84,6 +115,7 @@ async function commitObservation( userId: string, data: ObservationFactData, choice: ObservationChoice = {}, + fallbackDay: string | null = null, ): Promise { const isQualitative = typeof data.value !== "number"; // A numeric reading needs a unit so the catalog has something to mint with — @@ -228,7 +260,7 @@ async function commitObservation( sourceReferenceLow: storedSourceLow, sourceReferenceHigh: storedSourceHigh, sourceReferenceText: isQualitative ? null : sourceText, - takenAt: statedDateOrNow(data.effectiveDate), + takenAt: statedDateOrNow(data.effectiveDate, fallbackDay), source: "DOCUMENT", noteEncrypted: null, }, @@ -317,12 +349,18 @@ export async function commitApprovedFact( userId: string, fact: ExtractedFact, choice: ObservationChoice = {}, + documentDates: FactDocumentDates | null = null, ): Promise { // Decrypt the staged payload at confirm time to write it into the normal // structured store. Fail-closed: a bad key id throws and aborts the commit. const data = decryptFactData(fact.dataEncrypted); if (fact.factType === "OBSERVATION") { - return commitObservation(userId, data as ObservationFactData, choice); + return commitObservation( + userId, + data as ObservationFactData, + choice, + documentDay(documentDates), + ); } if (fact.factType === "CONDITION") { return commitCondition(userId, data as ConditionFactData); From 4ca7573f9093618dea7b601d850f4f2920b91d76 Mon Sep 17 00:00:00 2001 From: haritowa <6107211+haritowa@users.noreply.github.com> Date: Mon, 26 Oct 2026 00:24:49 +0000 Subject: [PATCH 2/8] test(documents): pin an undated lab value's date through the confirm route Against a real Postgres and the real confirm route: an undated value takes the report date over the filing date, a value's own date is kept, an impossible own date counts as none, and a document without a report date dates the value by its filing date. --- .../document-undated-lab-value.test.ts | 222 ++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 tests/integration/document-undated-lab-value.test.ts diff --git a/tests/integration/document-undated-lab-value.test.ts b/tests/integration/document-undated-lab-value.test.ts new file mode 100644 index 000000000..154bb4619 --- /dev/null +++ b/tests/integration/document-undated-lab-value.test.ts @@ -0,0 +1,222 @@ +/** + * A lab value without its own date, confirmed from a stored document, against + * a real Postgres and through the real confirm route. + * + * A lab report usually prints the collection date once, in its header. The + * extraction puts that date on the document (`reportDate`) and leaves each + * value's own date empty, so the confirm step has to date the value by its + * document. It used to file every such reading under the moment it was + * confirmed, so a report from years ago showed up as today's results. + * + * The route reads the document's two dates once and hands them to the commit + * step: the report date first, else the filing date (`documentDate`), the order + * the day view places a document by. A value's own date still wins, but only + * when it names a real calendar day. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const USER_ID = "user-undated-lab-value"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + const prisma = getPrismaClient(); + await prisma.user.create({ + data: { + id: USER_ID, + username: "undated-lab-value", + email: "undated-lab-value@example.test", + modulePreferencesJson: { inboundDocuments: true, labs: true }, + }, + }); + const session = await prisma.session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.set("healthlog_session", session.id); + for (const [name, unit] of [ + ["Ferritin", "ng/mL"], + ["Glucose", "mmol/L"], + ["Haemoglobin", "g/L"], + ]) { + await prisma.biomarker.create({ data: { userId: USER_ID, name, unit } }); + } +}); + +const noon = (day: string) => new Date(`${day}T12:00:00.000Z`); + +async function storedDocument(dates: { + reportDate: string | null; + documentDate: string | null; +}) { + return getPrismaClient().inboundDocument.create({ + data: { + userId: USER_ID, + filename: "befund.pdf", + mimeType: "application/pdf", + byteSize: 1024, + contentEncrypted: Buffer.from("stand-in for the stored bytes"), + status: "EXTRACTED", + reportDate: dates.reportDate ? noon(dates.reportDate) : null, + documentDate: dates.documentDate ? noon(dates.documentDate) : null, + }, + }); +} + +async function stagedValue( + documentId: string, + label: string, + value: number, + unit: string, + effectiveDate: string | null, +) { + const { encryptFactData, encryptFactProvenance } = + await import("@/lib/documents/store"); + return getPrismaClient().extractedFact.create({ + data: { + documentId, + userId: USER_ID, + factType: "OBSERVATION", + status: "PENDING", + confidence: 0.95, + needsReview: false, + dataEncrypted: encryptFactData({ + label, + code: null, + codeSystem: null, + value, + valueText: null, + unit, + referenceLow: null, + referenceHigh: null, + referenceText: null, + effectiveDate, + }), + provenanceEncrypted: encryptFactProvenance({ + sourceText: `${label} ${value} ${unit}`, + anchored: true, + sourceOffset: 0, + page: 0, + confidence: 0.95, + }), + }, + }); +} + +async function confirm(documentId: string, factIds: string[]) { + const { POST } = + await import("@/app/api/documents/inbound/[id]/confirm/route"); + const res = await POST( + new NextRequest( + `http://localhost/api/documents/inbound/${documentId}/confirm`, + { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + decisions: factIds.map((factId) => ({ factId, action: "approve" })), + }), + }, + ), + { params: Promise.resolve({ id: documentId }) }, + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.data.failed).toEqual([]); + expect(body.data.approved).toHaveLength(factIds.length); + return body.data.approved as { factId: string; recordId: string }[]; +} + +async function takenAtOf(recordId: string) { + const row = await getPrismaClient().labResult.findUniqueOrThrow({ + where: { id: recordId }, + }); + return row.takenAt.toISOString(); +} + +describe("an undated lab value is dated by its document", () => { + it("takes the report date over the filing date, keeps a value's own date, and ignores an impossible one", async () => { + const today = new Date().toISOString().slice(0, 10); + // Collection date printed once in the header; filed today (the upload-day + // default for documentDate). + const doc = await storedDocument({ + reportDate: "2021-03-01", + documentDate: today, + }); + const undated = await stagedValue(doc.id, "Ferritin", 80, "ng/mL", null); + const ownDate = await stagedValue( + doc.id, + "Glucose", + 5.1, + "mmol/L", + "2021-02-20", + ); + const impossible = await stagedValue( + doc.id, + "Haemoglobin", + 142, + "g/L", + "2021-02-30", + ); + + const approved = await confirm(doc.id, [ + undated.id, + ownDate.id, + impossible.id, + ]); + const recordOf = (factId: string) => + approved.find((a) => a.factId === factId)!.recordId; + + expect(await takenAtOf(recordOf(undated.id))).toBe( + "2021-03-01T12:00:00.000Z", + ); + expect(await takenAtOf(recordOf(ownDate.id))).toBe( + "2021-02-20T12:00:00.000Z", + ); + // Not 2 March: a day the month does not have is no date at all. + expect(await takenAtOf(recordOf(impossible.id))).toBe( + "2021-03-01T12:00:00.000Z", + ); + }); + + it("takes the filing date when the document has no report date", async () => { + const doc = await storedDocument({ + reportDate: null, + documentDate: "2021-02-15", + }); + const undated = await stagedValue(doc.id, "Ferritin", 80, "ng/mL", null); + + const [approved] = await confirm(doc.id, [undated.id]); + + expect(await takenAtOf(approved!.recordId)).toBe( + "2021-02-15T12:00:00.000Z", + ); + }); +}); From d88313e92996cf9aa2920a3c4b519a2f01f1640d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 29 Oct 2026 08:49:40 +0100 Subject: [PATCH 3/8] fix(labs): move document lab values filed under their confirmation day Before the confirm step dated an undated value by its document, a lab value confirmed from a report that prints its collection date only in the header was stored with takenAt set to the moment of confirmation. The fix covers new confirms; the values already filed that way stay on the wrong day until something moves them. A one-shot maintenance job, sent at boot only while there is something to repair, moves exactly those values to the document's report date (else its filing date) at noon UTC, the same rule the confirm step now uses. A value qualifies only when the chain is certain: written by the document confirm path, named by exactly one approved observation fact, takenAt still within five seconds of its own createdAt, no calendar date of its own in the decrypted fact, and a document day that differs from the day the value is filed under in the owner's zone. A value re-dated since, one really taken on its confirmation day, or one whose document has no date is left alone. A moved value no longer sits at its creation moment, so the pass converges and a second run changes nothing. The job definition lives in a new registry file beside maintenance.ts so that file stays under the size ratchet. --- .../labs-module-ai-gate-guard.test.ts | 2 + src/lib/documents/commit.ts | 4 +- .../__tests__/queue-policy-tables.test.ts | 9 + .../jobs/lab-document-date-repair-queue.ts | 10 + src/lib/jobs/lab-document-date-repair.ts | 234 ++++++++++++++++++ src/lib/jobs/queue-runtime.ts | 3 + .../jobs/registry/maintenance-discovery.ts | 9 + src/lib/jobs/registry/maintenance-repairs.ts | 25 ++ src/lib/jobs/registry/maintenance.ts | 2 + src/lib/jobs/subsystem-surface.ts | 1 + .../lab-document-date-repair.test.ts | 232 +++++++++++++++++ 11 files changed, 529 insertions(+), 2 deletions(-) create mode 100644 src/lib/jobs/lab-document-date-repair-queue.ts create mode 100644 src/lib/jobs/lab-document-date-repair.ts create mode 100644 src/lib/jobs/registry/maintenance-repairs.ts create mode 100644 tests/integration/lab-document-date-repair.test.ts diff --git a/src/__tests__/labs-module-ai-gate-guard.test.ts b/src/__tests__/labs-module-ai-gate-guard.test.ts index d798e69ef..70ca38c36 100644 --- a/src/__tests__/labs-module-ai-gate-guard.test.ts +++ b/src/__tests__/labs-module-ai-gate-guard.test.ts @@ -65,6 +65,8 @@ const NOT_MODEL_INPUT: Record = { "links stored rows to their biomarker; no model is involved", "src/lib/jobs/lab-biomarker-backfill.ts#enqueueBootTimeLabBiomarkerBackfill": "finds the accounts whose rows need linking; no model is involved", + "src/lib/jobs/lab-document-date-repair.ts#candidateQuery": + "finds document lab values filed at their confirmation moment to re-date them; no model is involved", "src/lib/jobs/soft-delete-retention.ts#SOFT_DELETE_RETENTION": "purges rows past the trash window; no model is involved", "src/lib/insights/correlation-channel-series.ts#fetchLabDraws": diff --git a/src/lib/documents/commit.ts b/src/lib/documents/commit.ts index 17bae2712..97633752b 100644 --- a/src/lib/documents/commit.ts +++ b/src/lib/documents/commit.ts @@ -66,7 +66,7 @@ export interface CommittedRecordRef { } /** A stated day, or null when it is not a real calendar date. */ -function statedDay(date: string | null): string | null { +export function statedDay(date: string | null): string | null { return date && isCalendarDateKey(date) ? date : null; } @@ -97,7 +97,7 @@ export interface FactDocumentDates { * no date beside each value, and every reading used to be filed under the day * it was confirmed. */ -function documentDay(dates: FactDocumentDates | null): string | null { +export function documentDay(dates: FactDocumentDates | null): string | null { const day = dates?.reportDate ?? dates?.documentDate ?? null; return day ? dateOnlyKey(day) : null; } diff --git a/src/lib/jobs/__tests__/queue-policy-tables.test.ts b/src/lib/jobs/__tests__/queue-policy-tables.test.ts index 7a728b6fb..41c5b2f3d 100644 --- a/src/lib/jobs/__tests__/queue-policy-tables.test.ts +++ b/src/lib/jobs/__tests__/queue-policy-tables.test.ts @@ -112,6 +112,15 @@ describe("maintenance group queue policies", () => { expectPolicy("maintenance", queue, policy); }); + // One-shot repairs on a fixed singleton key: a boot during a run must be + // able to queue the next pass, so only queued duplicates collapse. + it.each([["lab-document-date-repair", "short"]])( + "%s is %s", + (queue, policy) => { + expectPolicy("maintenance", queue, policy); + }, + ); + it("environment-fetch is deliberately left on standard", () => { // Its explicit-range backfill sends with no key at all, on purpose, so any // policy would collapse two different requested date ranges onto the shared diff --git a/src/lib/jobs/lab-document-date-repair-queue.ts b/src/lib/jobs/lab-document-date-repair-queue.ts new file mode 100644 index 000000000..34ccea6e6 --- /dev/null +++ b/src/lib/jobs/lab-document-date-repair-queue.ts @@ -0,0 +1,10 @@ +/** + * The queue facts of `lab-document-date-repair.ts`: its name and its one + * singleton key. They live apart from the job so the worker registry and the + * boot discovery can read them without loading it. + */ +export const LAB_DOCUMENT_DATE_REPAIR_QUEUE = "lab-document-date-repair"; + +/** One pass covers every account, so there is only ever one job. */ +export const LAB_DOCUMENT_DATE_REPAIR_SINGLETON_KEY = + "lab-document-date-repair"; diff --git a/src/lib/jobs/lab-document-date-repair.ts b/src/lib/jobs/lab-document-date-repair.ts new file mode 100644 index 000000000..57edb24e5 --- /dev/null +++ b/src/lib/jobs/lab-document-date-repair.ts @@ -0,0 +1,234 @@ +/** + * One-shot repair: lab values confirmed from a document before the confirm + * step learned to date an undated value by its document. + * + * A lab report usually prints the collection date once, in its header. The + * extraction stores that date on the document (`reportDate`) and leaves each + * value's own date empty, and the confirm step used to file such a value + * under the moment it was confirmed. The confirm step now takes the + * document's day (`documentDay` in `src/lib/documents/commit.ts`); this pass + * moves the values it filed wrongly before, by the same rule. + * + * A value is moved only when every link in the chain is certain: + * - the lab result was written by the document confirm path + * (`source = 'DOCUMENT'`, which no other writer sets) and is live; + * - exactly one APPROVED observation fact names it as its committed record, + * so the document it came from is known; + * - its `takenAt` still sits within a few seconds of its own `createdAt`, + * which is what a value filed under its confirmation moment looks like + * (both were stamped in the same create). A value the person re-dated + * since, or one that carried its own date, sits elsewhere; + * - the staged fact states no calendar date of its own (decrypted here; + * SQL cannot see it); + * - the document has a report date, else a filing date, and that day is + * not the day the value is filed under in the owner's time zone. A value + * really taken on the day it was confirmed stays where it is. + * + * The moved value lands at noon UTC on the document's day, the anchor every + * date-only lab value uses. Idempotent: a moved value no longer sits at its + * creation moment, so a second run finds nothing, and a boot with nothing to + * repair costs one query. + */ +import type { Job } from "pg-boss"; + +import { Prisma, type PrismaClient } from "@/generated/prisma/client"; +import { activeAccountSql } from "@/lib/account-deletion/active-account"; +import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; +import { prisma as defaultPrisma } from "@/lib/db"; +import { documentDay, statedDay } from "@/lib/documents/commit"; +import { decryptFactData } from "@/lib/documents/store"; +import { getGlobalBoss } from "@/lib/jobs/boss-instance"; +import { jobDone, jobFailed, type JobOutcome } from "@/lib/jobs/job-outcome"; +import { workerLog } from "@/lib/jobs/reminder/shared"; +import { annotate } from "@/lib/logging/context"; +import { withBackgroundEvent } from "@/lib/logging/background"; +import { dateOnlyAtNoonUtc } from "@/lib/tz/date-only"; +import { userDayKey } from "@/lib/tz/format"; +import { resolveUserTimezone } from "@/lib/tz/resolver"; +import type { ObservationFactData } from "@/lib/validations/inbound-documents"; + +import { + LAB_DOCUMENT_DATE_REPAIR_QUEUE, + LAB_DOCUMENT_DATE_REPAIR_SINGLETON_KEY, +} from "./lab-document-date-repair-queue"; + +/** + * How far `takenAt` may sit from `createdAt` and still count as the moment of + * confirmation. The two are stamped a few milliseconds apart in one create; a + * few seconds leaves room for a clock step and nothing a person would type. + */ +const CONFIRM_MOMENT_TOLERANCE_SECONDS = 5; + +interface CandidateRow { + id: string; + userId: string; + takenAt: Date; + dataEncrypted: Uint8Array; + reportDate: Date | null; + documentDate: Date | null; +} + +/** + * The SQL half of the rule. `limit` turns it into the boot's existence + * check. `.planning/v1.46.2-fragments/lab-repair-count.sql` is the same + * predicate as a read-only count. + */ +function candidateQuery(limit: number | null): Prisma.Sql { + // The day comparison runs here as well as in the repair, so a value that + // really was taken on its confirmation day drops out of the boot check and + // the pass converges to an empty set. A zone Postgres does not know is read + // as UTC rather than failing the query; the repair re-checks every row in + // the zone the app resolves. + return Prisma.sql` + WITH "known_zones" AS (SELECT "name" FROM pg_timezone_names) + SELECT r."id", + r."user_id" AS "userId", + r."taken_at" AS "takenAt", + f."data_encrypted" AS "dataEncrypted", + d."report_date" AS "reportDate", + d."document_date" AS "documentDate" + FROM "lab_results" r + JOIN "extracted_facts" f + ON f."committed_record_id" = r."id" + AND f."committed_record_type" = 'labResult' + AND f."status" = 'APPROVED' + AND f."fact_type" = 'OBSERVATION' + AND f."user_id" = r."user_id" + JOIN "inbound_documents" d + ON d."id" = f."document_id" + AND d."user_id" = r."user_id" + JOIN "users" u ON u."id" = r."user_id" + WHERE r."source" = 'DOCUMENT' + AND r."deleted_at" IS NULL + AND abs(extract(epoch FROM (r."taken_at" - r."created_at"))) <= ${CONFIRM_MOMENT_TOLERANCE_SECONDS} + AND COALESCE(d."report_date", d."document_date") IS NOT NULL + AND to_char(COALESCE(d."report_date", d."document_date"), 'YYYY-MM-DD') + <> to_char( + (r."taken_at" AT TIME ZONE 'UTC') AT TIME ZONE ( + CASE WHEN u."timezone" IN (SELECT "name" FROM "known_zones") + THEN u."timezone" ELSE 'UTC' END + ), + 'YYYY-MM-DD' + ) + AND ( + SELECT count(*) FROM "extracted_facts" other + WHERE other."committed_record_id" = r."id" + AND other."committed_record_type" = 'labResult' + ) = 1 + AND ${activeAccountSql('r."user_id"')} + ORDER BY r."id" + ${limit === null ? Prisma.empty : Prisma.sql`LIMIT ${limit}`} + `; +} + +export interface LabDocumentDateRepairResult { + /** Rows the SQL predicate returned. */ + scanned: number; + /** Rows moved to their document's day. */ + updated: number; + /** Rows left alone: an own date, or already on the document's day. */ + unchanged: number; +} + +/** The repair itself, across every account. */ +export async function repairUndatedDocumentLabValues( + prisma: PrismaClient = defaultPrisma, +): Promise { + const rows = await prisma.$queryRaw(candidateQuery(null)); + const touchedUsers = new Set(); + let updated = 0; + let unchanged = 0; + + for (const row of rows) { + const data = decryptFactData(row.dataEncrypted) as ObservationFactData; + // A value with a date of its own is never touched, whatever its row says. + if (statedDay(data.effectiveDate ?? null) !== null) { + unchanged += 1; + continue; + } + const day = documentDay({ + reportDate: row.reportDate, + documentDate: row.documentDate, + }); + const tz = await resolveUserTimezone(row.userId); + if (!day || userDayKey(row.takenAt, tz) === day) { + unchanged += 1; + continue; + } + // Conditional on the value read: a re-date that landed since the read + // wins over the repair. + const moved = await prisma.labResult.updateMany({ + where: { + id: row.id, + userId: row.userId, + takenAt: row.takenAt, + deletedAt: null, + }, + data: { takenAt: dateOnlyAtNoonUtc(day) }, + }); + if (moved.count === 0) { + unchanged += 1; + continue; + } + updated += 1; + touchedUsers.add(row.userId); + } + + for (const userId of touchedUsers) invalidateUserHealthScore(userId); + return { scanned: rows.length, updated, unchanged }; +} + +/** The queue handler: one run over every account. */ +export async function handleLabDocumentDateRepair( + _jobs: Job>[], + prisma: PrismaClient = defaultPrisma, +): Promise { + return withBackgroundEvent("job.lab_document_date_repair", async () => { + let result: LabDocumentDateRepairResult; + try { + result = await repairUndatedDocumentLabValues(prisma); + } catch (err) { + workerLog("error", "[lab-document-date-repair] failed", err); + return jobFailed("lab document date repair failed", err); + } + workerLog( + "info", + `[lab-document-date-repair] scanned=${result.scanned} updated=${result.updated} unchanged=${result.unchanged}`, + ); + annotate({ + action: { name: "labs.documentDate.repair" }, + meta: { ...result }, + }); + return jobDone({ + candidates_scanned: result.scanned, + updated: result.updated, + unchanged: result.unchanged, + }); + }); +} + +/** + * Boot discovery: one existence query, and the one job only when there is + * something to repair. + */ +export async function enqueueBootTimeLabDocumentDateRepair( + prisma: PrismaClient = defaultPrisma, +): Promise<{ enqueued: boolean; error: string | null }> { + const boss = getGlobalBoss(); + if (!boss) return { enqueued: false, error: null }; + try { + const any = await prisma.$queryRaw(candidateQuery(1)); + if (any.length === 0) return { enqueued: false, error: null }; + const id = await boss.send( + LAB_DOCUMENT_DATE_REPAIR_QUEUE, + {}, + { singletonKey: LAB_DOCUMENT_DATE_REPAIR_SINGLETON_KEY, retryLimit: 2 }, + ); + return { enqueued: id !== null, error: null }; + } catch (err) { + return { + enqueued: false, + error: err instanceof Error ? err.message : String(err), + }; + } +} diff --git a/src/lib/jobs/queue-runtime.ts b/src/lib/jobs/queue-runtime.ts index 49fc6d154..7d6a3ae9d 100644 --- a/src/lib/jobs/queue-runtime.ts +++ b/src/lib/jobs/queue-runtime.ts @@ -485,6 +485,9 @@ export const QUEUE_RUNTIME: Readonly> = { }, why: "One account's folded means, a day at a time; stops at three quarters of the default expiry and sends a follow-up that resumes at the next day.", }, + "lab-document-date-repair": short( + "Reads the document lab values still filed at their confirmation moment, a handful per account, and moves each with one update.", + ), "measurement-maintenance": { runtime: "long", expireInSeconds: MEASUREMENT_MAINTENANCE_SEND_OPTIONS.expireInSeconds, diff --git a/src/lib/jobs/registry/maintenance-discovery.ts b/src/lib/jobs/registry/maintenance-discovery.ts index c0f2c41e2..2de44262a 100644 --- a/src/lib/jobs/registry/maintenance-discovery.ts +++ b/src/lib/jobs/registry/maintenance-discovery.ts @@ -77,6 +77,15 @@ export const MAINTENANCE_DISCOVERY: readonly BootDiscovery[] = [ await import("@/lib/jobs/measurement-fold-repair") ).enqueueBootTimeMeasurementFoldRepair(), }, + // Converges to nothing once the misdated values are moved; a boot with + // none costs one query. + { + name: "lab-document-date-repair", + run: async () => + ( + await import("@/lib/jobs/lab-document-date-repair") + ).enqueueBootTimeLabDocumentDateRepair(), + }, // Self-limiting and idempotent; a boot with no backlog costs one query. { name: "compaction-tombstone-purge", diff --git a/src/lib/jobs/registry/maintenance-repairs.ts b/src/lib/jobs/registry/maintenance-repairs.ts new file mode 100644 index 000000000..2d3224e86 --- /dev/null +++ b/src/lib/jobs/registry/maintenance-repairs.ts @@ -0,0 +1,25 @@ +/** + * One-shot data repairs of the maintenance group, kept beside + * `maintenance.ts` so that file stays under the size ratchet. Each is sent + * by its boot discovery in `maintenance-discovery.ts` only while it has + * something to repair. + */ +import { LAB_DOCUMENT_DATE_REPAIR_QUEUE } from "@/lib/jobs/lab-document-date-repair-queue"; + +import { defineJob, type JobDefinition } from "./define-job"; + +export const MAINTENANCE_REPAIR_JOBS: readonly JobDefinition[] = [ + // Document lab values filed under their confirmation moment. + defineJob({ + queue: LAB_DOCUMENT_DATE_REPAIR_QUEUE, + policy: { + policy: "short", + reason: + "Fixed singleton key; collapse queued duplicates only, so a boot during a run can queue the next one.", + }, + work: { localConcurrency: 1 }, + load: async () => + (await import("@/lib/jobs/lab-document-date-repair")) + .handleLabDocumentDateRepair, + }), +]; diff --git a/src/lib/jobs/registry/maintenance.ts b/src/lib/jobs/registry/maintenance.ts index 587123022..574d48352 100644 --- a/src/lib/jobs/registry/maintenance.ts +++ b/src/lib/jobs/registry/maintenance.ts @@ -202,6 +202,7 @@ import { } from "@/lib/medications/intake-slot-dedup"; import { MAINTENANCE_DISCOVERY } from "./maintenance-discovery"; +import { MAINTENANCE_REPAIR_JOBS } from "./maintenance-repairs"; import { defineJob, type JobDefinition, @@ -524,6 +525,7 @@ const jobs: readonly JobDefinition[] = [ (await import("@/lib/jobs/measurement-fold-repair")) .handleMeasurementFoldRepair, }), + ...MAINTENANCE_REPAIR_JOBS, // The nightly check vacuums when dead rows pile up; the same singleton key // as the operator's run, so the two collapse. defineJob({ diff --git a/src/lib/jobs/subsystem-surface.ts b/src/lib/jobs/subsystem-surface.ts index ea14eac89..419541cca 100644 --- a/src/lib/jobs/subsystem-surface.ts +++ b/src/lib/jobs/subsystem-surface.ts @@ -120,6 +120,7 @@ const subsystemSurface = { "health-connect-import": { audience: "account" }, "compaction-tombstone-purge": { audience: "system" }, "measurement-fold-repair": { audience: "system" }, + "lab-document-date-repair": { audience: "system" }, "measurement-maintenance": { audience: "system" }, "pulse-day-mean": { audience: "system" }, "medication-intake-import": { audience: "system" }, diff --git a/tests/integration/lab-document-date-repair.test.ts b/tests/integration/lab-document-date-repair.test.ts new file mode 100644 index 000000000..82c6aa086 --- /dev/null +++ b/tests/integration/lab-document-date-repair.test.ts @@ -0,0 +1,232 @@ +/** + * The one-shot repair of lab values a document confirm filed under the + * moment of confirmation (real Postgres). + * + * Before the confirm step dated an undated value by its document, such a + * value got `takenAt = now`, the same instant as its `createdAt`. The repair + * moves exactly those to the document's report date (else its filing date) + * at noon UTC, and leaves alone a value with a date of its own, a value that + * really was taken on its confirmation day in the owner's zone, one the + * person re-dated since, and one whose document has no date. A second run + * changes nothing. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { encryptFactData, encryptFactProvenance } from "@/lib/documents/store"; +import { repairUndatedDocumentLabValues } from "@/lib/jobs/lab-document-date-repair"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const USER = "user-lab-document-date-repair"; +const noon = (day: string) => new Date(`${day}T12:00:00.000Z`); + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + await prisma.user.create({ + data: { + id: USER, + username: "lab-date-repair", + email: "lab-date-repair@example.test", + timezone: "Europe/Berlin", + }, + }); +}); + +/** + * A document, one confirmed value from it and the fact that links the two, + * as the confirm route left them. + */ +async function confirmedValue(opts: { + id: string; + reportDate: string | null; + documentDate: string | null; + effectiveDate: string | null; + createdAt: Date; + takenAt: Date; +}) { + const prisma = getPrismaClient(); + const doc = await prisma.inboundDocument.create({ + data: { + userId: USER, + filename: `${opts.id}.pdf`, + mimeType: "application/pdf", + byteSize: 1024, + contentEncrypted: Buffer.from("stand-in for the stored bytes"), + status: "CONFIRMED", + reportDate: opts.reportDate ? noon(opts.reportDate) : null, + documentDate: opts.documentDate ? noon(opts.documentDate) : null, + }, + }); + await prisma.labResult.create({ + data: { + id: opts.id, + userId: USER, + analyte: "Ferritin", + value: 80, + unit: "ng/mL", + source: "DOCUMENT", + takenAt: opts.takenAt, + createdAt: opts.createdAt, + }, + }); + await prisma.extractedFact.create({ + data: { + documentId: doc.id, + userId: USER, + factType: "OBSERVATION", + status: "APPROVED", + confidence: 0.95, + needsReview: false, + dataEncrypted: encryptFactData({ + label: "Ferritin", + code: null, + codeSystem: null, + value: 80, + valueText: null, + unit: "ng/mL", + referenceLow: null, + referenceHigh: null, + referenceText: null, + effectiveDate: opts.effectiveDate, + }), + provenanceEncrypted: encryptFactProvenance({ + sourceText: "Ferritin 80 ng/mL", + anchored: true, + sourceOffset: 0, + page: 0, + confidence: 0.95, + }), + committedRecordId: opts.id, + committedRecordType: "labResult", + }, + }); +} + +async function takenAt(): Promise> { + const rows = await getPrismaClient().labResult.findMany({ + where: { userId: USER }, + select: { id: true, takenAt: true }, + }); + return Object.fromEntries(rows.map((r) => [r.id, r.takenAt.toISOString()])); +} + +describe("lab document date repair", () => { + it("moves only values filed under their confirmation moment, and converges", async () => { + const confirmed = new Date("2026-10-20T08:00:00.123Z"); + + // Collection date in the header, value undated, filed at confirmation. + await confirmedValue({ + id: "misdated", + reportDate: "2021-03-01", + documentDate: "2026-10-18", + effectiveDate: null, + createdAt: confirmed, + takenAt: confirmed, + }); + // No report date: the filing date is the document's day. + await confirmedValue({ + id: "misdated-filing-date", + reportDate: null, + documentDate: "2021-02-15", + effectiveDate: null, + createdAt: confirmed, + takenAt: new Date(confirmed.getTime() + 2), + }); + // Its own date, even where the row looks like a confirmation moment. + await confirmedValue({ + id: "own-date", + reportDate: "2021-03-01", + documentDate: null, + effectiveDate: "2021-02-20", + createdAt: confirmed, + takenAt: confirmed, + }); + // Confirmed at 00:30 in Berlin on the report day itself: 22:30 UTC the + // evening before, which a UTC comparison would have moved. + const justAfterMidnight = new Date("2026-10-19T22:30:00.000Z"); + await confirmedValue({ + id: "taken-on-confirm-day", + reportDate: "2026-10-20", + documentDate: "2026-10-20", + effectiveDate: null, + createdAt: justAfterMidnight, + takenAt: justAfterMidnight, + }); + // Re-dated by the person after the confirm. + await confirmedValue({ + id: "re-dated", + reportDate: "2021-03-01", + documentDate: null, + effectiveDate: null, + createdAt: confirmed, + takenAt: new Date("2026-10-01T07:45:00.000Z"), + }); + // A document with no date at all leaves nothing to move to. + await confirmedValue({ + id: "undated-document", + reportDate: null, + documentDate: null, + effectiveDate: null, + createdAt: confirmed, + takenAt: confirmed, + }); + + const before = await takenAt(); + const first = await repairUndatedDocumentLabValues(getPrismaClient()); + + expect(first.updated).toBe(2); + expect(await takenAt()).toEqual({ + ...before, + misdated: "2021-03-01T12:00:00.000Z", + "misdated-filing-date": "2021-02-15T12:00:00.000Z", + }); + + const afterFirst = await takenAt(); + const second = await repairUndatedDocumentLabValues(getPrismaClient()); + expect(second.updated).toBe(0); + expect(await takenAt()).toEqual(afterFirst); + }); + + it("leaves a lab value alone when a second fact also names it", async () => { + const confirmed = new Date("2026-10-20T08:00:00.000Z"); + await confirmedValue({ + id: "linked-twice", + reportDate: "2021-03-01", + documentDate: null, + effectiveDate: null, + createdAt: confirmed, + takenAt: confirmed, + }); + const prisma = getPrismaClient(); + const second = await prisma.inboundDocument.create({ + data: { + userId: USER, + filename: "other.pdf", + mimeType: "application/pdf", + byteSize: 1, + contentEncrypted: Buffer.from("x"), + reportDate: noon("2020-01-01"), + }, + }); + const fact = await prisma.extractedFact.findFirstOrThrow({ + where: { committedRecordId: "linked-twice" }, + }); + await prisma.extractedFact.create({ + data: { + documentId: second.id, + userId: USER, + factType: "OBSERVATION", + status: "APPROVED", + dataEncrypted: fact.dataEncrypted, + provenanceEncrypted: fact.provenanceEncrypted, + committedRecordId: "linked-twice", + committedRecordType: "labResult", + }, + }); + + const result = await repairUndatedDocumentLabValues(prisma); + + expect(result.updated).toBe(0); + expect((await takenAt())["linked-twice"]).toBe(confirmed.toISOString()); + }); +}); From e2998183c8b304eec2f38657b6a1e704c4ecc236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 29 Oct 2026 08:56:19 +0100 Subject: [PATCH 4/8] fix(measurements): delete a blood-pressure reading as one Systolic, diastolic and the cuff's pulse are three rows with no link column, and the single DELETE tombstoned only the row it was given, so deleting a reading's systolic left its diastolic and pulse behind as half a reading. The bulk delete had the same gap. A reading is the rows with the same owner, the same source and the identical measuredAt: that is how the entry form, the MCP write, a Withings measure group and a HealthKit correlation all store one, and how the FHIR read pairs a diastolic onto its systolic. The looser five-minute pairing of the chart reads is right for drawing a point and wrong for taking rows away. Deleting a systolic or diastolic now tombstones the rest of its reading in the same transaction with the same deletedAt, each row with its own syncVersion bump so paired clients see every tombstone. The restore route brings back the rows deleted together with a named half (identical deletedAt), so the undo, which names only the row that was tapped, puts the whole reading back, from the web list and from any client that calls it the same way. A pulse is still deleted on its own, and a row deleted at another moment stays deleted. The response shapes do not change; the OpenAPI descriptions say what the counts now include. --- docs/api/openapi.yaml | 13 +- src/app/api/measurements/[id]/route.ts | 47 ++++- .../bulk-delete/__tests__/route.test.ts | 28 ++- src/app/api/measurements/bulk-delete/route.ts | 33 ++- .../restore/__tests__/route.test.ts | 28 ++- src/app/api/measurements/restore/route.ts | 43 +++- .../measurements/blood-pressure-reading.ts | 82 ++++++++ src/lib/openapi/routes/measurements.ts | 6 +- .../blood-pressure-reading-delete.test.ts | 198 ++++++++++++++++++ 9 files changed, 430 insertions(+), 48 deletions(-) create mode 100644 src/lib/measurements/blood-pressure-reading.ts create mode 100644 tests/integration/blood-pressure-reading-delete.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 479a40268..f8c36f55f 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -4825,7 +4825,10 @@ paths: tags: - Measurements summary: Delete a measurement - description: Soft-deletes the row (tombstone) so paired clients can reconcile the deletion. + description: "Soft-deletes the row (tombstone) so paired clients can reconcile the deletion. A blood-pressure systolic + or diastolic takes the rest of its reading with it: the other half and the pulse with the same source and the + identical `measuredAt`, tombstoned in the same transaction with the same `deletedAt`, each with its own + `syncVersion` bump. A pulse is deleted on its own." parameters: - in: path name: id @@ -4936,7 +4939,8 @@ paths: description: Soft-deletes (tombstones) up to 200 of the caller's measurement rows in one call, mirroring the single-DELETE contract. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:bulk-delete:`. Forged / foreign / already-deleted ids are silently skipped — `deleted` is - the count of rows actually tombstoned. The deletions surface in `/api/sync/changes` as tombstones. + the count of rows actually tombstoned, which includes the rest of any blood-pressure reading a selected systolic + or diastolic belongs to (as on the single DELETE). The deletions surface in `/api/sync/changes` as tombstones. parameters: - *a6 requestBody: @@ -4965,7 +4969,10 @@ paths: description: Un-tombstones up to 200 of the caller's soft-deleted measurement rows in one call — the delete-Undo counterpart to `/api/measurements/bulk-delete`. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:restore:`. Forged / foreign / not-deleted ids are silently skipped — `restored` is the - count of rows actually un-tombstoned. The restored rows re-surface in `/api/sync/changes` as upserts. + count of rows actually un-tombstoned. A blood-pressure systolic or diastolic also restores the rows of its + reading that were deleted with it (same source, identical `measuredAt` and `deletedAt`), so naming one half + undoes the whole reading; a row deleted at another moment stays deleted. The restored rows re-surface in + `/api/sync/changes` as upserts. parameters: - *a6 requestBody: diff --git a/src/app/api/measurements/[id]/route.ts b/src/app/api/measurements/[id]/route.ts index 84e18c624..be24ec3a2 100644 --- a/src/app/api/measurements/[id]/route.ts +++ b/src/app/api/measurements/[id]/route.ts @@ -23,6 +23,7 @@ import { } from "@/lib/measurements/apple-health-mirror"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutation"; +import { bloodPressureSiblings } from "@/lib/measurements/blood-pressure-reading"; import { Prisma, type MeasurementType } from "@/generated/prisma/client"; import { NextRequest } from "next/server"; import { z } from "zod"; @@ -237,18 +238,45 @@ export const DELETE = apiHandler( // (see `measurements/route.ts:100`), so the row is invisible to // normal reads from this point on. A row that is already tombstoned // re-bumps `syncVersion` harmlessly (idempotent re-delete). - await prisma.measurement.update({ - where: { id }, - data: { - deletedAt: new Date(), - syncVersion: { increment: 1 }, - }, + // + // A systolic or diastolic takes the rest of its reading with it (the + // other half and the cuff's pulse, `bloodPressureSiblings`), in the same + // transaction and with the same `deletedAt`, which is how the restore + // route knows to bring them back together. Each sibling bumps its own + // `syncVersion`, so a paired client sees every row's tombstone. + const deletedAt = new Date(); + const siblings = await prisma.$transaction(async (tx) => { + const found = await bloodPressureSiblings( + tx, + user.id, + [existing], + "live", + ); + await tx.measurement.update({ + where: { id }, + data: { deletedAt, syncVersion: { increment: 1 } }, + }); + if (found.length > 0) { + await tx.measurement.updateMany({ + where: { + id: { in: found.map((s) => s.id) }, + userId: user.id, + deletedAt: null, + }, + data: { deletedAt, syncVersion: { increment: 1 } }, + }); + } + return found; }); await auditLog("measurement.delete", { userId: user.id, ipAddress: getClientIp(request), - details: { measurementId: id, type: existing.type }, + details: { + measurementId: id, + type: existing.type, + readingRows: siblings.length, + }, }); annotate({ @@ -269,7 +297,10 @@ export const DELETE = apiHandler( // the rollup row when the day's measurement count goes to zero). await afterMeasurementMutation( user.id, - [{ type: existing.type, measuredAt: existing.measuredAt }], + [ + { type: existing.type, measuredAt: existing.measuredAt }, + ...siblings.map((s) => ({ type: s.type, measuredAt: s.measuredAt })), + ], { source: "manual" }, ); diff --git a/src/app/api/measurements/bulk-delete/__tests__/route.test.ts b/src/app/api/measurements/bulk-delete/__tests__/route.test.ts index b41d53ccb..f546f195d 100644 --- a/src/app/api/measurements/bulk-delete/__tests__/route.test.ts +++ b/src/app/api/measurements/bulk-delete/__tests__/route.test.ts @@ -10,15 +10,19 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; -vi.mock("@/lib/db", () => ({ - prisma: { +vi.mock("@/lib/db", () => { + const prisma = { measurement: { findMany: vi.fn(), updateMany: vi.fn(), }, auditLog: { create: vi.fn() }, - }, -})); + // The read and the write run in one transaction; the stub hands the + // callback the same mocked client. + $transaction: vi.fn(async (fn: (tx: unknown) => unknown) => fn(prisma)), + }; + return { prisma }; +}); vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); vi.mock("@/lib/auth/audit", () => ({ @@ -113,8 +117,16 @@ describe("POST /api/measurements/bulk-delete", () => { it("soft-deletes only owned, non-tombstoned rows and returns that count", async () => { // Caller asked for 3 ids but only 2 are owned + live. vi.mocked(prisma.measurement.findMany).mockResolvedValue([ - { type: "WEIGHT", measuredAt: new Date("2026-01-01T08:00:00Z") }, - { type: "WEIGHT", measuredAt: new Date("2026-01-01T20:00:00Z") }, + { + id: "m1", + type: "WEIGHT", + measuredAt: new Date("2026-01-01T08:00:00Z"), + }, + { + id: "m2", + type: "WEIGHT", + measuredAt: new Date("2026-01-01T20:00:00Z"), + }, ] as never); vi.mocked(prisma.measurement.updateMany).mockResolvedValue({ count: 2, @@ -129,7 +141,9 @@ describe("POST /api/measurements/bulk-delete", () => { // never 404s, it's just excluded from the count. const call = vi.mocked(prisma.measurement.updateMany).mock.calls[0][0]; expect(call.where).toMatchObject({ - id: { in: ["m1", "m2", "foreign"] }, + // The write names the rows the scoped read found, never the + // forged id the caller sent. + id: { in: ["m1", "m2"] }, userId: "user-1", deletedAt: null, }); diff --git a/src/app/api/measurements/bulk-delete/route.ts b/src/app/api/measurements/bulk-delete/route.ts index 03ea9abc3..16e2a22ee 100644 --- a/src/app/api/measurements/bulk-delete/route.ts +++ b/src/app/api/measurements/bulk-delete/route.ts @@ -7,6 +7,8 @@ * * Soft-deletes (tombstones) every owned, not-already-deleted row in one * `updateMany` — matching the single-DELETE iOS tombstone contract. A + * selected blood-pressure half takes the rest of its reading with it, as the + * single DELETE does, and `deleted` counts every row tombstoned. A * forged / foreign id is a silent no-op (never a 404 existence leak): the * mutation `where` pins `userId` so it only ever touches the caller's rows. * @@ -26,6 +28,7 @@ import { withIdempotency } from "@/lib/idempotency"; import { checkRateLimit } from "@/lib/rate-limit"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutation"; +import { bloodPressureSiblings } from "@/lib/measurements/blood-pressure-reading"; import type { MeasurementType } from "@/generated/prisma/client"; const MAX_IDS_PER_BATCH = 200; @@ -73,17 +76,27 @@ async function postBulkDelete(request: NextRequest): Promise { // a status-insight cache drop. Ownership-scoped + `deletedAt: null` so // a forged / foreign / already-tombstoned id is excluded here exactly // as it is from the mutation below — no existence leak. - const affected = await prisma.measurement.findMany({ - where: { id: { in: ids }, userId: user.id, deletedAt: null }, - select: { type: true, measuredAt: true }, - }); + // + // A selected systolic or diastolic brings the rest of its reading (the + // other half and the cuff's pulse, `bloodPressureSiblings`) into the same + // statement, so the shared `deletedAt` lets the restore route bring the + // reading back whole. + const { affected, count } = await prisma.$transaction(async (tx) => { + const selected = await tx.measurement.findMany({ + where: { id: { in: ids }, userId: user.id, deletedAt: null }, + select: { id: true, type: true, source: true, measuredAt: true }, + }); + const siblings = await bloodPressureSiblings(tx, user.id, selected, "live"); + const rowIds = [...selected, ...siblings].map((row) => row.id); - // Soft-delete (tombstone) in one statement. The `where` pins `userId` - // so a forged id for another user is silently a no-op; `deletedAt: null` - // keeps the count honest (an already-tombstoned id doesn't re-count). - const { count } = await prisma.measurement.updateMany({ - where: { id: { in: ids }, userId: user.id, deletedAt: null }, - data: { deletedAt: now, syncVersion: { increment: 1 } }, + // Soft-delete (tombstone) in one statement. The `where` pins `userId` + // so a forged id for another user is silently a no-op; `deletedAt: null` + // keeps the count honest (an already-tombstoned id doesn't re-count). + const { count } = await tx.measurement.updateMany({ + where: { id: { in: rowIds }, userId: user.id, deletedAt: null }, + data: { deletedAt: now, syncVersion: { increment: 1 } }, + }); + return { affected: [...selected, ...siblings], count }; }); await auditLog("measurement.delete.bulk", { diff --git a/src/app/api/measurements/restore/__tests__/route.test.ts b/src/app/api/measurements/restore/__tests__/route.test.ts index 0fa7e8427..73ce2540e 100644 --- a/src/app/api/measurements/restore/__tests__/route.test.ts +++ b/src/app/api/measurements/restore/__tests__/route.test.ts @@ -10,15 +10,19 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; -vi.mock("@/lib/db", () => ({ - prisma: { +vi.mock("@/lib/db", () => { + const prisma = { measurement: { findMany: vi.fn(), updateMany: vi.fn(), }, auditLog: { create: vi.fn() }, - }, -})); + // The read and the write run in one transaction; the stub hands the + // callback the same mocked client. + $transaction: vi.fn(async (fn: (tx: unknown) => unknown) => fn(prisma)), + }; + return { prisma }; +}); vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); vi.mock("@/lib/auth/audit", () => ({ @@ -110,8 +114,16 @@ describe("POST /api/measurements/restore", () => { it("un-tombstones only owned, tombstoned rows and returns that count", async () => { // Caller asked for 3 ids but only 2 are owned + tombstoned. vi.mocked(prisma.measurement.findMany).mockResolvedValue([ - { type: "WEIGHT", measuredAt: new Date("2026-01-01T08:00:00Z") }, - { type: "WEIGHT", measuredAt: new Date("2026-01-01T20:00:00Z") }, + { + id: "m1", + type: "WEIGHT", + measuredAt: new Date("2026-01-01T08:00:00Z"), + }, + { + id: "m2", + type: "WEIGHT", + measuredAt: new Date("2026-01-01T20:00:00Z"), + }, ] as never); vi.mocked(prisma.measurement.updateMany).mockResolvedValue({ count: 2, @@ -126,7 +138,9 @@ describe("POST /api/measurements/restore", () => { // forged / live id never 404s, it's just excluded from the count. const call = vi.mocked(prisma.measurement.updateMany).mock.calls[0][0]; expect(call.where).toMatchObject({ - id: { in: ["m1", "m2", "foreign"] }, + // The write names the rows the scoped read found, never the + // forged id the caller sent. + id: { in: ["m1", "m2"] }, userId: "user-1", deletedAt: { not: null }, }); diff --git a/src/app/api/measurements/restore/route.ts b/src/app/api/measurements/restore/route.ts index 3c2a7b92d..62aeab9a7 100644 --- a/src/app/api/measurements/restore/route.ts +++ b/src/app/api/measurements/restore/route.ts @@ -28,6 +28,7 @@ import { withIdempotency } from "@/lib/idempotency"; import { checkRateLimit } from "@/lib/rate-limit"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { afterMeasurementMutation } from "@/lib/rollups/after-measurement-mutation"; +import { bloodPressureSiblings } from "@/lib/measurements/blood-pressure-reading"; import type { MeasurementType } from "@/generated/prisma/client"; const MAX_IDS_PER_BATCH = 200; @@ -73,17 +74,39 @@ async function postRestore(request: NextRequest): Promise { // a status-insight cache drop. Ownership-scoped + `deletedAt != null` // so a forged / foreign / never-deleted id is excluded here exactly as // it is from the mutation below — no existence leak. - const affected = await prisma.measurement.findMany({ - where: { id: { in: ids }, userId: user.id, deletedAt: { not: null } }, - select: { type: true, measuredAt: true }, - }); + // + // A blood-pressure half brings back the rows of its reading that were + // tombstoned in the same statement (identical `deletedAt`, + // `bloodPressureSiblings`), so an undo that names only the row the person + // tapped restores the reading the delete took. A row deleted on its own, + // at another moment, stays deleted. + const { affected, count } = await prisma.$transaction(async (tx) => { + const selected = await tx.measurement.findMany({ + where: { id: { in: ids }, userId: user.id, deletedAt: { not: null } }, + select: { + id: true, + type: true, + source: true, + measuredAt: true, + deletedAt: true, + }, + }); + const siblings = await bloodPressureSiblings( + tx, + user.id, + selected, + "deletedWith", + ); + const rowIds = [...selected, ...siblings].map((row) => row.id); - // Un-tombstone in one statement. The `where` pins `userId` so a forged - // id for another user is silently a no-op; `deletedAt: { not: null }` - // keeps the count honest (a live row doesn't re-count). - const { count } = await prisma.measurement.updateMany({ - where: { id: { in: ids }, userId: user.id, deletedAt: { not: null } }, - data: { deletedAt: null, syncVersion: { increment: 1 } }, + // Un-tombstone in one statement. The `where` pins `userId` so a forged + // id for another user is silently a no-op; `deletedAt: { not: null }` + // keeps the count honest (a live row doesn't re-count). + const { count } = await tx.measurement.updateMany({ + where: { id: { in: rowIds }, userId: user.id, deletedAt: { not: null } }, + data: { deletedAt: null, syncVersion: { increment: 1 } }, + }); + return { affected: [...selected, ...siblings], count }; }); await auditLog("measurement.restore", { diff --git a/src/lib/measurements/blood-pressure-reading.ts b/src/lib/measurements/blood-pressure-reading.ts new file mode 100644 index 000000000..589a7fb66 --- /dev/null +++ b/src/lib/measurements/blood-pressure-reading.ts @@ -0,0 +1,82 @@ +/** + * One blood-pressure reading is up to three rows: systolic, diastolic and + * the pulse the cuff measured with them. No column links them. What makes + * them one reading is how every writer stores it: the entry form, the MCP + * write, a Withings measure group and a HealthKit correlation all write the + * halves with the same source at the same instant, and the FHIR read pairs a + * diastolic onto its systolic by exactly that instant. + * + * So the rule here is the strict one: same owner, same source, identical + * `measuredAt`. The series and in-target reads pair more loosely (nearest + * diastolic within five minutes) because they only draw a point; a delete + * takes rows away, so it takes only the rows that can be nothing else. + * + * Deleting a systolic or a diastolic takes the whole reading with it. + * Deleting a pulse takes only the pulse: a pulse is also a value of its own, + * and removing one is no reason to remove a blood pressure. + */ +import type { MeasurementType, Prisma } from "@/generated/prisma/client"; + +/** The types that anchor a reading: deleting one deletes the reading. */ +const BP_HALVES: ReadonlySet = new Set([ + "BLOOD_PRESSURE_SYS", + "BLOOD_PRESSURE_DIA", +]); + +/** Every type a reading's rows can carry. */ +const BP_READING_TYPES: MeasurementType[] = [ + "BLOOD_PRESSURE_SYS", + "BLOOD_PRESSURE_DIA", + "PULSE", +]; + +export interface ReadingAnchor { + id: string; + type: MeasurementType | string; + source: string; + measuredAt: Date; + deletedAt?: Date | null; +} + +type MeasurementReader = { + measurement: Pick; +}; + +export interface ReadingSibling { + id: string; + type: MeasurementType; + measuredAt: Date; +} + +/** + * The other rows of the readings `rows` belong to, not counting `rows` + * themselves. `state: "live"` finds the rows a delete takes with it; + * `state: "deletedWith"` finds the rows that were tombstoned in the same + * statement as their anchor (the identical `deletedAt`), which is what a + * restore brings back with it, and never a row deleted on its own. + */ +export async function bloodPressureSiblings( + db: MeasurementReader, + userId: string, + rows: readonly ReadingAnchor[], + state: "live" | "deletedWith", +): Promise { + const anchors = rows.filter( + (r) => BP_HALVES.has(r.type) && (state === "live" || r.deletedAt), + ); + if (anchors.length === 0) return []; + const found = await db.measurement.findMany({ + where: { + userId, + id: { notIn: rows.map((r) => r.id) }, + type: { in: BP_READING_TYPES }, + OR: anchors.map((a) => ({ + source: a.source as Prisma.MeasurementWhereInput["source"], + measuredAt: a.measuredAt, + deletedAt: state === "live" ? null : a.deletedAt, + })), + }, + select: { id: true, type: true, measuredAt: true }, + }); + return found; +} diff --git a/src/lib/openapi/routes/measurements.ts b/src/lib/openapi/routes/measurements.ts index c8a3dc8af..b04e40220 100644 --- a/src/lib/openapi/routes/measurements.ts +++ b/src/lib/openapi/routes/measurements.ts @@ -1055,7 +1055,7 @@ export const measurementPaths: NonNullable = { tags: ["Measurements"], summary: "Delete a measurement", description: - "Soft-deletes the row (tombstone) so paired clients can reconcile the deletion.", + "Soft-deletes the row (tombstone) so paired clients can reconcile the deletion. A blood-pressure systolic or diastolic takes the rest of its reading with it: the other half and the pulse with the same source and the identical `measuredAt`, tombstoned in the same transaction with the same `deletedAt`, each with its own `syncVersion` bump. A pulse is deleted on its own.", requestParams: { path: z.object({ id: z.string() }) }, responses: { ...recordRefusal(), @@ -1145,7 +1145,7 @@ export const measurementPaths: NonNullable = { tags: ["Measurements"], summary: "Bulk soft-delete measurements", description: - "Soft-deletes (tombstones) up to 200 of the caller's measurement rows in one call, mirroring the single-DELETE contract. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:bulk-delete:`. Forged / foreign / already-deleted ids are silently skipped — `deleted` is the count of rows actually tombstoned. The deletions surface in `/api/sync/changes` as tombstones.", + "Soft-deletes (tombstones) up to 200 of the caller's measurement rows in one call, mirroring the single-DELETE contract. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:bulk-delete:`. Forged / foreign / already-deleted ids are silently skipped — `deleted` is the count of rows actually tombstoned, which includes the rest of any blood-pressure reading a selected systolic or diastolic belongs to (as on the single DELETE). The deletions surface in `/api/sync/changes` as tombstones.", requestBody: { required: true, content: { @@ -1176,7 +1176,7 @@ export const measurementPaths: NonNullable = { tags: ["Measurements"], summary: "Restore soft-deleted measurements", description: - "Un-tombstones up to 200 of the caller's soft-deleted measurement rows in one call — the delete-Undo counterpart to `/api/measurements/bulk-delete`. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:restore:`. Forged / foreign / not-deleted ids are silently skipped — `restored` is the count of rows actually un-tombstoned. The restored rows re-surface in `/api/sync/changes` as upserts.", + "Un-tombstones up to 200 of the caller's soft-deleted measurement rows in one call — the delete-Undo counterpart to `/api/measurements/bulk-delete`. Idempotent via the `Idempotency-Key` header; rate-limited `measurements:restore:`. Forged / foreign / not-deleted ids are silently skipped — `restored` is the count of rows actually un-tombstoned. A blood-pressure systolic or diastolic also restores the rows of its reading that were deleted with it (same source, identical `measuredAt` and `deletedAt`), so naming one half undoes the whole reading; a row deleted at another moment stays deleted. The restored rows re-surface in `/api/sync/changes` as upserts.", requestBody: { required: true, content: { diff --git a/tests/integration/blood-pressure-reading-delete.test.ts b/tests/integration/blood-pressure-reading-delete.test.ts new file mode 100644 index 000000000..d7f4028d9 --- /dev/null +++ b/tests/integration/blood-pressure-reading-delete.test.ts @@ -0,0 +1,198 @@ +/** + * Deleting one row of a blood-pressure reading deletes the reading, through + * the real routes against a real Postgres. + * + * Systolic, diastolic and the cuff's pulse are three rows with no link + * column; they are one reading because every writer stores them with the + * same source at the same instant. Deleting the systolic used to tombstone + * only the systolic and leave a diastolic and a pulse behind. Now the + * single DELETE and the bulk delete take the whole reading, every row bumps + * its own syncVersion (so the sync feed carries each tombstone), and the + * undo, which names only the row the person tapped, brings the whole + * reading back. A reading of another source at the same instant, a reading + * a minute later, and a pulse deleted on its own are left alone. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +const USER = "user-bp-reading-delete"; +const AT = new Date("2026-10-20T07:30:00.000Z"); +const MINUTE_LATER = new Date(AT.getTime() + 60_000); + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + headerJar.clear(); + await prisma.user.create({ + data: { + id: USER, + username: "bp-reading-delete", + email: "bp-reading-delete@example.test", + }, + }); + const session = await prisma.session.create({ + data: { userId: USER, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.set("healthlog_session", session.id); + + const row = ( + id: string, + type: "BLOOD_PRESSURE_SYS" | "BLOOD_PRESSURE_DIA" | "PULSE", + value: number, + source: "MANUAL" | "WITHINGS", + measuredAt: Date, + ) => ({ + id, + userId: USER, + type, + value, + unit: type === "PULSE" ? "bpm" : "mmHg", + source, + measuredAt, + }); + await prisma.measurement.createMany({ + data: [ + // The reading under test. + row("sys", "BLOOD_PRESSURE_SYS", 128, "MANUAL", AT), + row("dia", "BLOOD_PRESSURE_DIA", 82, "MANUAL", AT), + row("pulse", "PULSE", 64, "MANUAL", AT), + // The same instant from another source: another reading. + row("w-sys", "BLOOD_PRESSURE_SYS", 131, "WITHINGS", AT), + row("w-dia", "BLOOD_PRESSURE_DIA", 84, "WITHINGS", AT), + // A minute later from the same source: another reading. + row("next-sys", "BLOOD_PRESSURE_SYS", 125, "MANUAL", MINUTE_LATER), + row("next-dia", "BLOOD_PRESSURE_DIA", 80, "MANUAL", MINUTE_LATER), + row("next-pulse", "PULSE", 62, "MANUAL", MINUTE_LATER), + ], + }); +}); + +async function rows(): Promise< + Record +> { + const all = await getPrismaClient().measurement.findMany({ + where: { userId: USER }, + select: { id: true, deletedAt: true, syncVersion: true }, + }); + return Object.fromEntries( + all.map((r) => [ + r.id, + { deleted: r.deletedAt !== null, syncVersion: r.syncVersion }, + ]), + ); +} + +const deletedIds = async () => + Object.entries(await rows()) + .filter(([, r]) => r.deleted) + .map(([id]) => id) + .sort(); + +async function deleteOne(id: string) { + const { DELETE } = await import("@/app/api/measurements/[id]/route"); + const res = await DELETE( + new NextRequest(`http://localhost/api/measurements/${id}`, { + method: "DELETE", + }), + { params: Promise.resolve({ id }) }, + ); + expect(res.status).toBe(200); +} + +async function post(path: "bulk-delete" | "restore", ids: string[]) { + const route = + path === "bulk-delete" + ? await import("@/app/api/measurements/bulk-delete/route") + : await import("@/app/api/measurements/restore/route"); + const res = await route.POST( + new NextRequest(`http://localhost/api/measurements/${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ ids }), + }), + ); + expect(res.status).toBe(200); + return (await res.json()).data as { deleted?: number; restored?: number }; +} + +describe("deleting a blood-pressure reading", () => { + it("takes the other half and the pulse with the systolic, and the undo brings all three back", async () => { + await deleteOne("sys"); + + expect(await deletedIds()).toEqual(["dia", "pulse", "sys"]); + const after = await rows(); + for (const id of ["sys", "dia", "pulse"]) { + expect(after[id]!.syncVersion).toBe(2); + } + for (const id of ["w-sys", "w-dia", "next-sys", "next-dia"]) { + expect(after[id]!.syncVersion).toBe(1); + } + + // The undo names only the row that was tapped. + const { restored } = await post("restore", ["sys"]); + expect(restored).toBe(3); + expect(await deletedIds()).toEqual([]); + }); + + it("takes the systolic and the pulse with the diastolic", async () => { + await deleteOne("dia"); + expect(await deletedIds()).toEqual(["dia", "pulse", "sys"]); + }); + + it("deletes a pulse on its own, and its undo leaves the reading as it was", async () => { + await deleteOne("pulse"); + expect(await deletedIds()).toEqual(["pulse"]); + await post("restore", ["pulse"]); + expect(await deletedIds()).toEqual([]); + }); + + it("does the same for the bulk delete, and restores only what went in the same delete", async () => { + // The pulse of the second reading was deleted on its own earlier. + await deleteOne("next-pulse"); + + const { deleted } = await post("bulk-delete", ["sys", "next-dia"]); + expect(deleted).toBe(5); + expect(await deletedIds()).toEqual([ + "dia", + "next-dia", + "next-pulse", + "next-sys", + "pulse", + "sys", + ]); + + const { restored } = await post("restore", ["sys", "next-dia"]); + expect(restored).toBe(5); + // The pulse deleted at another moment stays deleted. + expect(await deletedIds()).toEqual(["next-pulse"]); + }); +}); From bf0142a62672f672259868a1be09648e90fc1c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 29 Oct 2026 09:07:37 +0100 Subject: [PATCH 5/8] fix(ai): give background work the High room on a model that always reasons Background jobs ask for medium reasoning at most, and status cards ask for none. A model that cannot be switched down (GLM behind a vLLM gateway, for instance) ignores both, spends the whole room thinking and returns only reasoning, so the daily briefing and the status cards fail as reasoning_only and the chain pauses. The Coach on High works because it leaves 12k tokens to think in. The chain runner now marks every call of a job walk as unattended, in one place rather than at each call site. The gateway and local clients give such a call at least the High thinking room once its model has answered with reasoning alone. The room has to ride on the #1267 repeat with none: one call may send two requests, and a once-a-day job meets an expired lesson every morning, so a room that waited for the unswitchable lesson would never reach the first briefing of the day. It is a ceiling, not a charge: a model that does stop at none never touches it. Calls a person is reading, the Coach included, keep the room of their own level, and the requested effort is unchanged. The room calculation both clients shared is now one function. Reported by @Tio61 in #1276. --- .../gateway-unswitchable-reasoning.test.ts | 95 ++++++++++++++++++- .../provider-runner-background-bench.test.ts | 26 +++++ src/lib/ai/local-client.ts | 12 +-- src/lib/ai/openai-client.ts | 17 +--- src/lib/ai/provider-runner.ts | 11 ++- src/lib/ai/reasoning/answerless.ts | 48 ++++++++++ src/lib/ai/types.ts | 8 ++ 7 files changed, 191 insertions(+), 26 deletions(-) diff --git a/src/lib/ai/__tests__/gateway-unswitchable-reasoning.test.ts b/src/lib/ai/__tests__/gateway-unswitchable-reasoning.test.ts index 8e415bd80..329363eab 100644 --- a/src/lib/ai/__tests__/gateway-unswitchable-reasoning.test.ts +++ b/src/lib/ai/__tests__/gateway-unswitchable-reasoning.test.ts @@ -45,16 +45,22 @@ interface SeenRequest { * `always`: the thought fills the ceiling whatever the effort (gpt-oss, GLM). * `answers-unreasoned`: the same, except asked with `none` it answers * (Gemma 4 on Ollama). + * `answers-with-room`: ignores the effort and answers once the ceiling leaves + * it the `high` thinking room (GLM 5.3 behind a gateway, #1276). */ -let mode: "always" | "answers-unreasoned" = "always"; +let mode: "always" | "answers-unreasoned" | "answers-with-room" = "always"; let server: http.Server; let origin: string; let seen: SeenRequest[] = []; const ANSWER = JSON.stringify({ summary: "Steady." }); -function reply(effort: unknown) { - const answers = mode === "answers-unreasoned" && effort === "none"; +function reply(effort: unknown, maxTokens: unknown) { + const answers = + (mode === "answers-unreasoned" && effort === "none") || + (mode === "answers-with-room" && + typeof maxTokens === "number" && + maxTokens >= 12_000); return { id: "chatcmpl-1279", object: "chat.completion", @@ -87,7 +93,14 @@ beforeAll(async () => { maxTokens: body.max_tokens ?? body.max_completion_tokens, }); res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify(reply(body.reasoning_effort))); + res.end( + JSON.stringify( + reply( + body.reasoning_effort, + body.max_tokens ?? body.max_completion_tokens, + ), + ), + ); }); }); await new Promise((r) => server.listen(0, "127.0.0.1", r)); @@ -210,3 +223,77 @@ describe.each([ }); }, ); + +/** A daily briefing call as the chain runner hands it to a client. */ +function briefingParams(unattended: boolean): CompletionParams { + return { + ...singleUserTurn({ + system: "You write the daily briefing.", + user: "Snapshot: …", + maxTokens: 300, + timeoutMs: 30_000, + responseFormat: "json", + }), + reasoning: { effort: "medium", summaries: false }, + ...(unattended ? { unattended: true } : {}), + }; +} + +describe.each([ + ["the OpenAI-compatible gateway", gateway], + ["the Local client", local], +] as const)( + "#1276 — %s, background work for a model that cannot be switched down", + (_, make) => { + it("gives the #1267 repeat the High room, and answers on the first call", async () => { + mode = "answers-with-room"; + const result = await make().generateCompletion(briefingParams(true)); + + expect(result.content).toBe(ANSWER); + // Two requests, the call's budget: medium at its own room, then the + // repeat with `none` and the High room the reply showed it needs. + expect(seen).toEqual([ + { reasoningEffort: "medium", maxTokens: 4396 }, + { reasoningEffort: "none", maxTokens: 12_300 }, + ]); + // Every later background call starts with the High room. + seen = []; + await make().generateCompletion(briefingParams(true)); + expect(seen).toEqual([{ reasoningEffort: "none", maxTokens: 12_300 }]); + }); + + it("gives a status card, which asks no level, the same room", async () => { + mode = "answers-with-room"; + const result = await make().generateCompletion({ + ...statusParams(), + unattended: true, + }); + expect(result.content).toBe(ANSWER); + expect(seen.map((s) => s.maxTokens)).toEqual([300, 12_300]); + }); + + it("keeps the High room once even `none` is learned useless, and stays at one request", async () => { + const err = (await make() + .generateCompletion(briefingParams(true)) + .catch((e) => e)) as Record; + expect(err.kind).toBe("reasoning_only"); + expect(seen.map((s) => s.maxTokens)).toEqual([4396, 12_300]); + seen = []; + await make() + .generateCompletion(briefingParams(true)) + .catch(() => {}); + expect(seen).toEqual([{ reasoningEffort: "medium", maxTokens: 12_300 }]); + }); + + it("leaves a call a person is reading at the room of its level", async () => { + await make() + .generateCompletion(briefingParams(false)) + .catch(() => {}); + seen = []; + await make() + .generateCompletion(briefingParams(false)) + .catch(() => {}); + expect(seen).toEqual([{ reasoningEffort: "medium", maxTokens: 4396 }]); + }); + }, +); diff --git a/src/lib/ai/__tests__/provider-runner-background-bench.test.ts b/src/lib/ai/__tests__/provider-runner-background-bench.test.ts index e17e9c558..c7efb8110 100644 --- a/src/lib/ai/__tests__/provider-runner-background-bench.test.ts +++ b/src/lib/ai/__tests__/provider-runner-background-bench.test.ts @@ -272,3 +272,29 @@ describe("#1279: background work and a benched provider", () => { expect((err as AllProvidersFailedError).tokensSpent).toBe(5400); }); }); + +describe("#1276 — the runner marks background calls", () => { + class Recorder implements AIProvider { + readonly type = "openai-compatible" as const; + seen: CompletionParams[] = []; + async generateCompletion(p: CompletionParams): Promise { + this.seen.push(p); + return { content: "{}", tokensUsed: 1 } as CompletionResult; + } + } + + it.each([ + ["job", true], + ["coach", undefined], + ] as const)("a %s walk carries unattended: %s", async (surface, expected) => { + const instance = new Recorder(); + await walk( + surface, + `u-unattended-${surface}`, + "openai-compatible", + instance, + createInMemoryProviderHealthLedger(), + ); + expect(instance.seen[0]?.unattended).toBe(expected); + }); +}); diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index 6af86a14c..4fbe52d6b 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -27,11 +27,10 @@ import { noteReasoningSeen, refusesReasoningField, } from "./reasoning/dialect-cache"; -import { uncontrolledThinkingRoom } from "./reasoning/levels"; import { askAgainUnreasoned, knownReasoner, - reasonsUnasked, + uncontrolledCallThinkingRoom, repeatUnreasoned, } from "./reasoning/answerless"; import { @@ -149,11 +148,10 @@ export class LocalOpenAICompatibleClient implements AIProvider { // `none` sent for Off or after the #1267 lesson must not leave it the // bare answer budget to think in. A call without a level gets the `low` // room once the model has shown it reasons unasked. - const thinkingTokens = params.reasoning - ? uncontrolledThinkingRoom(params.reasoning.effort) - : reasonsUnasked(this.reasoningDialectKey()) - ? uncontrolledThinkingRoom("off") - : 0; + const thinkingTokens = uncontrolledCallThinkingRoom( + this.reasoningDialectKey(), + params, + ); const body = JSON.stringify({ model: this.config.model, messages, diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index feb6efd5c..1e144ac2c 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -21,15 +21,12 @@ import { import { askAgainUnreasoned, knownReasoner, - reasonsUnasked, + uncontrolledCallThinkingRoom, repeatUnreasoned, } from "./reasoning/answerless"; import { RequestHops } from "./request-hops"; import { learnFromRefusal } from "./wire-refusal"; -import { - REASONING_THINKING_BUDGET, - uncontrolledThinkingRoom, -} from "./reasoning/levels"; +import { REASONING_THINKING_BUDGET } from "./reasoning/levels"; import { answerOf, isOpenRouterEndpoint, @@ -573,19 +570,15 @@ export class OpenAIClient implements AIProvider { * included, and a `none` sent for the person's Off or after the #1267 * lesson must not leave it the bare answer budget to think in. Without a * level of its own, a call gets the `low` room once the model has shown it - * reasons unasked. + * reasons unasked, and background work the `high` room once the model has + * answered with reasoning alone (`unattendedThinkingRoom`). */ private thinkingRoom( params: CompletionParams, wire: OpenAIReasoningWire, ): number { if (this.isGateway) { - if (params.reasoning) { - return uncontrolledThinkingRoom(params.reasoning.effort); - } - return reasonsUnasked(this.reasoningDialectKey()) - ? uncontrolledThinkingRoom("off") - : 0; + return uncontrolledCallThinkingRoom(this.reasoningDialectKey(), params); } if (wire.kind === "none" || wire.effort === "none") return 0; const level = levelOfWireEffort(wire.effort); diff --git a/src/lib/ai/provider-runner.ts b/src/lib/ai/provider-runner.ts index 684853b5c..2aafb360d 100644 --- a/src/lib/ai/provider-runner.ts +++ b/src/lib/ai/provider-runner.ts @@ -743,14 +743,17 @@ export async function runStreamingRawCompletionWithFallback( onDelta: (delta: string) => void; }, ): Promise { + // #1276 — a job walk is background work; stamped here, not per call site. + const params: CompletionParams = + args.surface === "job" ? { ...args.params, unattended: true } : args.params; return runRawChain( args, // Prefer the provider's streaming wire; fall back to the buffered call for // providers (cloud clients today) that do not implement streaming. (candidate) => candidate.instance.generateCompletionStream - ? candidate.instance.generateCompletionStream(args.params, args.onDelta) - : candidate.instance.generateCompletion(args.params), + ? candidate.instance.generateCompletionStream(params, args.onDelta) + : candidate.instance.generateCompletion(params), (candidate) => ({ ai_chain_streamed: Boolean(candidate.instance.generateCompletionStream), }), @@ -789,7 +792,9 @@ export interface RunRawWithFallbackResult { export async function runRawCompletionWithFallback( args: ChainWalkArgs, ): Promise { + const params: CompletionParams = + args.surface === "job" ? { ...args.params, unattended: true } : args.params; return runRawChain(args, (candidate) => - candidate.instance.generateCompletion(args.params), + candidate.instance.generateCompletion(params), ); } diff --git a/src/lib/ai/reasoning/answerless.ts b/src/lib/ai/reasoning/answerless.ts index bcf2a2e18..492a94e3f 100644 --- a/src/lib/ai/reasoning/answerless.ts +++ b/src/lib/ai/reasoning/answerless.ts @@ -36,6 +36,11 @@ import { rememberReasoningDialect, type ReasoningDialectKey, } from "./dialect-cache"; +import { + REASONING_THINKING_BUDGET, + uncontrolledThinkingRoom, + type ReasoningLevel, +} from "./levels"; /** * What the client does next with an answerless reply. @@ -137,6 +142,49 @@ export function reasonsUnasked(key: ReasoningDialectKey): boolean { ); } +/** + * #1276 — the thinking room of an unattended call (`CompletionParams.unattended`) + * to a model that has answered with reasoning alone (`reasonsUnasked`): at + * least the `high` room the Coach gets on High. Background work asks for + * `medium` at most, and a model that cannot be switched down (GLM behind + * vLLM) spends all of it thinking and answers nothing, the daily briefing's + * `reasoning_only`. The room applies from the #1267 repeat with `none` on: + * one call may send two requests (`request-hops.ts`), and a once-a-day job + * meets an expired lesson every morning, so the room has to ride on the + * repeat or the first job of the day never gets it. A ceiling, not a charge: + * a model that does stop at `none` never touches it, one that does not + * thinks as long as it does either way and reaches the answer. Every call a + * person is reading keeps `room`. + */ +export function unattendedThinkingRoom( + key: ReasoningDialectKey, + room: number, + unattended: boolean | undefined, +): number { + return unattended && reasonsUnasked(key) + ? Math.max(room, REASONING_THINKING_BUDGET.high) + : room; +} + +/** + * #1276 — the thinking room of one call on a wire whose reasoning switch the + * client cannot verify (a gateway, a local server): by the call's level, not + * by what the wire says, the `low` room for a call without a level once the + * model has shown it reasons unasked, and the `high` room for background work + * once it has answered with reasoning alone (`unattendedThinkingRoom`). + */ +export function uncontrolledCallThinkingRoom( + key: ReasoningDialectKey, + params: { reasoning?: { effort: ReasoningLevel }; unattended?: boolean }, +): number { + const room = params.reasoning + ? uncontrolledThinkingRoom(params.reasoning.effort) + : reasonsUnasked(key) + ? uncontrolledThinkingRoom("off") + : 0; + return unattendedThinkingRoom(key, room, params.unattended); +} + /** * #1276 — true when the model has shown it reasons, for a request that did * (`asked`) or did not ask it to: a lesson that says it reasons unasked, or a diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index 4dcbdc1bf..f4a78a27a 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -383,6 +383,14 @@ export interface CompletionParams { * client sends exactly what it sent before the field existed. */ reasoning?: { effort: ReasoningLevel; summaries: boolean }; + /** + * #1276 — no person reads this reply as it streams: the call runs as + * background work. Set by the chain runner for every `"job"` walk, never by + * a call site. The gateway and local clients give such a call the `high` + * thinking room once its model has answered with reasoning alone + * (`unattendedThinkingRoom`); nothing else reads it. + */ + unattended?: boolean; /** * v1.41.2 — a stable key for calls that resend the same prompt prefix (the * rounds of one Coach turn, the turns of one conversation). Providers that From acf58923f7d15ba10fa926834943c682d62ff3b2 Mon Sep 17 00:00:00 2001 From: haritowa <6107211+haritowa@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:42:54 +0000 Subject: [PATCH 6/8] test(ai): pin the step-up when gpt-6.1-sol refuses effort none gpt-6.1-sol, a rung on the default list, answers reasoning "off" (effort none) with 400 unsupported_value and lists low as its lowest effort (seen 2026-10-24). The client must stay on the slug, retry with low and remember it, not walk on as if the model were retired. --- src/lib/ai/__tests__/codex-reasoning.test.ts | 43 ++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/lib/ai/__tests__/codex-reasoning.test.ts b/src/lib/ai/__tests__/codex-reasoning.test.ts index ac022be44..a4609130e 100644 --- a/src/lib/ai/__tests__/codex-reasoning.test.ts +++ b/src/lib/ai/__tests__/codex-reasoning.test.ts @@ -137,6 +137,49 @@ describe("Codex reasoning request", () => { expect(body.include).toEqual([]); expect(result.reasoning?.downgradedTo).toBeUndefined(); }); + + it("on a slug that refuses none, steps up to its lowest effort and stays on the slug", async () => { + // gpt-6.1-sol, a rung on the default list, refuses "none" (seen 2026-10-24). + // That refusal is about the effort, not the model: no walk to the next slug. + const refusal = JSON.stringify({ + error: { + message: + "Unsupported value: 'none' is not supported with the 'gpt-6.1-sol' model. Supported values are: 'low', 'medium', 'high', 'xhigh', and 'max'.", + type: "invalid_request_error", + param: "reasoning.effort", + code: "unsupported_value", + }, + }); + const mock = vi + .fn() + .mockResolvedValueOnce(errorResponse(400, refusal)) + .mockImplementation(async () => + sseResponse(fixture("codex-after-tool.sse")), + ); + vi.stubGlobal("fetch", mock); + const c = new CodexClient({ + accessToken: "test-token", + accountId: "acct-test", + onTokenRefresh: vi + .fn() + .mockResolvedValue({ accessToken: "x", accountId: "acct-test" }), + slugChain: ["gpt-6.1-sol", "gpt-6-sol"], + }); + + await c.generateCompletion( + params({ reasoning: { effort: "off", summaries: false } }), + ); + expect(sentBody(mock, 0).reasoning).toEqual({ effort: "none" }); + expect(sentBody(mock, 1).model).toBe("gpt-6.1-sol"); + expect(sentBody(mock, 1).reasoning).toEqual({ effort: "low" }); + + await c.generateCompletion( + params({ reasoning: { effort: "off", summaries: false } }), + ); + expect(mock).toHaveBeenCalledTimes(3); + expect(sentBody(mock, 2).model).toBe("gpt-6.1-sol"); + expect(sentBody(mock, 2).reasoning).toEqual({ effort: "low" }); + }); }); describe("Codex reasoning stream", () => { From 69d46f0add7ff20a4ecd2fad9f6844f667e78d77 Mon Sep 17 00:00:00 2001 From: haritowa <6107211+haritowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:09:02 +0100 Subject: [PATCH 7/8] test(ai): pin the walk past a retired model on the default Codex list OpenAI retired the 5.x line from Codex for ChatGPT sign-in in steps: GPT-5.2 and GPT-5.3-Codex on 2026-05-26, GPT-5.4 and GPT-5.4 mini on 2026-08-31, and GPT-5.5 on 2026-10-14. The backend answers a retired model with 400 model_not_found and "not supported when using Codex with a ChatGPT account". Pin that exact refusal as a reason to try the next slug, pin that an account refused the head of the default list walks on to the next rung and caches it, and keep the long-retired models off the default list. --- .../ai/__tests__/codex-slug-fallback.test.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/lib/ai/__tests__/codex-slug-fallback.test.ts b/src/lib/ai/__tests__/codex-slug-fallback.test.ts index 860cbfd39..6ac75d8a3 100644 --- a/src/lib/ai/__tests__/codex-slug-fallback.test.ts +++ b/src/lib/ai/__tests__/codex-slug-fallback.test.ts @@ -91,6 +91,48 @@ describe("Codex slug fallback chain", () => { clearCodexSlugCache(); }); + it("an account without the head of the default list walks on to the next rung", async () => { + const origModel = process.env.CODEX_MODEL; + const origChain = process.env.CODEX_MODEL_FALLBACK_CHAIN; + delete process.env.CODEX_MODEL; + delete process.env.CODEX_MODEL_FALLBACK_CHAIN; + try { + // The exact refusal a ChatGPT sign-in gets for a retired model. + const mockFetch = vi + .fn() + .mockResolvedValueOnce( + rejectionResponse( + 400, + '{"detail":"The \'gpt-5.6-terra\' model is not supported when using Codex with a ChatGPT account.","error":{"type":"invalid_request_error","code":"model_not_found"}}', + ), + ) + .mockResolvedValueOnce(sseSuccessResponse()); + vi.stubGlobal("fetch", mockFetch); + + const client = new CodexClient({ + accessToken: "test", + accountId: "acct", + onTokenRefresh, + }); + await client.generateCompletion( + singleUserTurn({ system: "s", user: "u" }), + ); + + expect(client.getLastDiagnostics()?.attempted).toEqual([ + "gpt-5.6-terra", + "gpt-6-sol", + ]); + expect(client.getLastDiagnostics()?.workingSlug).toBe("gpt-6-sol"); + expect(getCachedCodexSlug()).toBe("gpt-6-sol"); + } finally { + if (origModel === undefined) delete process.env.CODEX_MODEL; + else process.env.CODEX_MODEL = origModel; + if (origChain === undefined) + delete process.env.CODEX_MODEL_FALLBACK_CHAIN; + else process.env.CODEX_MODEL_FALLBACK_CHAIN = origChain; + } + }); + it("first slug succeeds — uses head-of-chain and caches it", async () => { const mockFetch = vi.fn().mockResolvedValue(sseSuccessResponse()); vi.stubGlobal("fetch", mockFetch); @@ -340,6 +382,14 @@ describe("Codex slug fallback chain", () => { ).toBe(true); expect(helper(400, '{"error":{"code":"model_not_found"}}')).toBe(true); expect(helper(400, "model gpt-5 does not exist")).toBe(true); + // The refusal a ChatGPT sign-in got for gpt-5.5 after its retirement + // (seen 2026-10-24). + expect( + helper( + 400, + '{"detail":"The \'gpt-5.5\' model is not supported when using Codex with a ChatGPT account.","error":{"type":"invalid_request_error","code":"model_not_found"}}', + ), + ).toBe(true); expect(helper(404, "anything")).toBe(true); expect(helper(500, "internal error")).toBe(false); expect(helper(429, "rate limited")).toBe(false); @@ -360,6 +410,17 @@ describe("Codex slug fallback chain", () => { "gpt-5.6-sol", "gpt-5.5", ]); + // OpenAI retired these from Codex for ChatGPT sign-in: 5.2 and 5.3-Codex + // on 2026-05-26, 5.4 and 5.4 mini on 2026-08-31. A retired rung costs + // every account that reaches it a refused request. + for (const retired of [ + "gpt-5.4", + "gpt-5.4-mini", + "gpt-5.3-codex", + "gpt-5.2", + ]) { + expect(__test.DEFAULT_SLUG_FALLBACK_CHAIN).not.toContain(retired); + } }); it("DEFAULT_SLUG_FALLBACK_CHAIN holds only slugs every family gate recognises", () => { From 0b32b567c09a2a54b003e33c3cb65dedcb35abf1 Mon Sep 17 00:00:00 2001 From: haritowa <6107211+haritowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:09:23 +0100 Subject: [PATCH 8/8] docs(ai): record the live check of the GPT-6 rungs and the retired 5.x line From a HealthLog container with its stored ChatGPT sign-in, gpt-6.1-sol, gpt-6-sol and gpt-6-luna each answered text and read a lab-report image, and the account's own model list offered them with image input. gpt-6.1-sol refuses effort none and names low as its lowest. The spec said the gpt-6 rungs had never been probed live, and still listed gpt-5.4, gpt-5.4 mini, gpt-5.3-codex and gpt-5.2 as accepted, all of which OpenAI has retired for ChatGPT sign-in, with the exact refusal the backend sends. Also note why a fixed chain goes stale: once GPT-5.5 retired, every model on the chain shipped at the time was retired with it. --- docs/codex-protocol-spec.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/codex-protocol-spec.md b/docs/codex-protocol-spec.md index de74b0198..67b45d4bb 100644 --- a/docs/codex-protocol-spec.md +++ b/docs/codex-protocol-spec.md @@ -631,10 +631,10 @@ Confirmed-rejected slugs (despite the source-tree references in the table above) Confirmed-accepted slugs on a ChatGPT Plus subscription (per `available_in_plans` in `models.json`): - `gpt-5.6-terra` — HealthLog's default (first chain entry) as of 2026-10-24; the backend rejected `gpt-5.5` and everything below it that day. `gpt-5.6-luna` and `gpt-5.6-sol` are its siblings on the same line -- `gpt-6-sol`, `gpt-6.1-sol`, `gpt-6-luna`, `gpt-6-astra` — the gpt-6 line, listed in `models.json` since 2026-09-22 (`gpt-6.1-sol` the catalog default since 2026-09-29). The catalog names `gpt-6-sol` as the upgrade of `gpt-5.6-terra` and `gpt-5.6-sol`, and `gpt-6-luna` as that of `gpt-5.6-luna`. Every gpt-6 slug lists `input_modalities: ["text", "image"]` and the same reasoning presets as the gpt-5.6 line (`low` to `max`, sol and astra also `ultra`). The presets omit `none`, as they do for `gpt-5.5`, which a live probe on 2026-10-06 showed accepts it; HealthLog sends `none` for "off" on the gpt-6 line too and learns a refusal per slug. The gpt-6 slugs were not probed against a live backend when they joined the chain -- `gpt-5.5` — the default from 2026-06 until 2026-10-24 -- `gpt-5.4` / `gpt-5.4-mini` / `gpt-5.2` — accepted per plan -- `gpt-5.3-codex` — was the default (verified 2026-05-09); the backend rotated off the `-codex` line on 2026-06-02 and this slug is now only accepted intermittently, so it sits on a late chain rung +- `gpt-6-sol`, `gpt-6.1-sol`, `gpt-6-luna`, `gpt-6-astra` — the gpt-6 line, listed in `models.json` since 2026-09-22 (`gpt-6.1-sol` the catalog default since 2026-09-29). The catalog names `gpt-6-sol` as the upgrade of `gpt-5.6-terra` and `gpt-5.6-sol`, and `gpt-6-luna` as that of `gpt-5.6-luna`. Every gpt-6 slug lists `input_modalities: ["text", "image"]` and the same reasoning presets as the gpt-5.6 line (`low` to `max`, sol and astra also `ultra`). The presets omit `none`, as they do for `gpt-5.5`, which a live probe on 2026-10-06 showed accepts it; HealthLog sends `none` for "off" on the gpt-6 line too and learns a refusal per slug. A live check on 2026-10-24 found the account's own list (`GET /backend-api/codex/models?client_version=…`) offering `gpt-6.1-sol`, `gpt-6-astra`, `gpt-6-sol`, `gpt-6-luna` and the `gpt-5.6` line, all with `input_modalities: ["text", "image"]`. From a HealthLog container with its stored sign-in, `gpt-6.1-sol`, `gpt-6-sol` and `gpt-6-luna` each answered text and read a lab-report image correctly. `gpt-6.1-sol` refuses `reasoning.effort: "none"` and names `low` as its lowest effort, which the client learns and steps up to on the same slug; the other two accept `none` +- `gpt-5.5` — the default from 2026-06 until 2026-10-24, kept as the last chain rung while plans differ + +Retired for ChatGPT sign-in (OpenAI's Codex models page): `gpt-5.2` and `gpt-5.3-codex` on 2026-05-26, `gpt-5.4` and `gpt-5.4-mini` on 2026-08-31, `gpt-5.5` on 2026-10-14 at 19:00 UTC. The backend answers each of them with 400 `model_not_found`, "The '' model is not supported when using Codex with a ChatGPT account.", which the client reads as a reason to try the next slug. The `codex-rs/tui/src/model_migration.rs` references to `gpt-5-codex` etc. are _migration prompts_ shown to existing users — they are NOT the slugs sent on the wire after migration. The wire slugs come from `models.json`. Always cross-check `models.json` (`gh api repos/openai/codex/contents/codex-rs/models-manager/models.json`) before picking a default. @@ -662,6 +662,8 @@ Every default rung must be a model family the reasoning, sampling and vision gat The shipped constant in `src/lib/ai/codex-slug-chain.ts` is authoritative; when the upstream rotates again, that file moves first and this listing follows. +A fixed chain goes stale. From 2026-10-14, when GPT-5.5 retired, every model on the chain HealthLog shipped then was retired, and every Codex request answered 503 "all configured Codex slugs were rejected" until the chain was updated or `CODEX_MODEL` was set by hand. Reading the account's live list (`GET /backend-api/codex/models`, as the Codex CLI does) would avoid this. + The chain is overridable via the env var `CODEX_MODEL_FALLBACK_CHAIN` (comma-separated). The first env-var-named slug supersedes `CODEX_MODEL`; if both are set, `CODEX_MODEL` is folded into position 0 and de-duplicated. **Trigger condition** (when to walk to the next slug):