From ec2c067e6009a3c83c7627a8e77cf3c42d0486e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:01:13 +0200 Subject: [PATCH 01/13] fix(retention): keep a tombstone while a live record still points at it The nightly purge nulled every non-cascading key into a purged row, so a live visit lost the practitioner it was with, a live dose its visit, a symptom its condition. Each purge entry now lists the keys that do not cascade, and a tombstone stays while a live row holds one. The register test derives those keys from the schema's relations, so a new nulling key into a purged model fails until it is listed. --- src/__tests__/helpers/prisma-schema-names.ts | 48 +++++- .../soft-delete-retention-register.test.ts | 152 ++++++++++++++++++ src/lib/jobs/soft-delete-retention.ts | 91 +++++++++-- tests/integration/soft-delete-purge.test.ts | 98 +++++++++-- 4 files changed, 365 insertions(+), 24 deletions(-) diff --git a/src/__tests__/helpers/prisma-schema-names.ts b/src/__tests__/helpers/prisma-schema-names.ts index fd7b5b448..b9e187e12 100644 --- a/src/__tests__/helpers/prisma-schema-names.ts +++ b/src/__tests__/helpers/prisma-schema-names.ts @@ -41,6 +41,20 @@ export interface DeclaredField { physicalName: string; } +/** The owning side of a relation: the field that carries `@relation(fields:)`. */ +export interface DeclaredRelation { + /** The relation field's name. */ + name: string; + /** The model it points at. */ + target: string; + /** Whether the relation field is optional (`Type?`). */ + optional: boolean; + /** The scalar fields holding the key, by their datamodel names. */ + fields: string[]; + /** The declared `onDelete`, or null when Prisma's default applies. */ + onDelete: string | null; +} + export interface DeclaredModel { name: string; mappedName: string | null; @@ -48,6 +62,8 @@ export interface DeclaredModel { physicalName: string; /** Scalar + enum fields only — relation fields carry no column. */ fields: DeclaredField[]; + /** Relations this model owns the key of. */ + relations: DeclaredRelation[]; } export interface DeclaredSchema { @@ -58,7 +74,9 @@ export interface DeclaredSchema { const BLOCK_MAP = /^\s*@@map\("([^"]+)"\)/m; const FIELD_MAP = /@map\("([^"]+)"\)/; /** `name Type[]? @attr…` — the two leading tokens are what matters. */ -const FIELD_LINE = /^\s*(\w+)\s+(\w+)(?:\[\])?\??\s*(.*)$/; +const FIELD_LINE = /^\s*(\w+)\s+(\w+)(?:\[\])?(\??)\s*(.*)$/; +const RELATION_FIELDS = /@relation\([^)]*\bfields:\s*\[([^\]]*)\]/; +const ON_DELETE = /@relation\([^)]*\bonDelete:\s*(\w+)/; /** Drop `//` and `///` lines so prose can never read as a declaration. */ function stripComments(block: string): string { @@ -112,15 +130,31 @@ export function readDeclaredSchema( const models = rawModels.map(({ name, body }) => { const mappedName = BLOCK_MAP.exec(body)?.[1] ?? null; const fields: DeclaredField[] = []; + const relations: DeclaredRelation[] = []; for (const line of body.split("\n")) { if (/^\s*@@/.test(line)) continue; const match = FIELD_LINE.exec(line); if (!match) continue; - const [, fieldName, baseType, rest] = match; + const [, fieldName, baseType, optional, rest] = match; // A field whose type is another model is a relation, not a column. The // back-reference side carries no `@relation` attribute at all, so the // type is what decides it — keying on `@relation` would miss half. - if (modelNames.has(baseType)) continue; + if (modelNames.has(baseType)) { + const keys = RELATION_FIELDS.exec(rest)?.[1]; + if (keys !== undefined) { + relations.push({ + name: fieldName, + target: baseType, + optional: optional === "?", + fields: keys + .split(",") + .map((key) => key.trim()) + .filter(Boolean), + onDelete: ON_DELETE.exec(rest)?.[1] ?? null, + }); + } + continue; + } const fieldMapped = FIELD_MAP.exec(rest)?.[1] ?? null; fields.push({ name: fieldName, @@ -128,7 +162,13 @@ export function readDeclaredSchema( physicalName: fieldMapped ?? fieldName, }); } - return { name, mappedName, physicalName: mappedName ?? name, fields }; + return { + name, + mappedName, + physicalName: mappedName ?? name, + fields, + relations, + }; }); return { enums, models }; diff --git a/src/lib/jobs/__tests__/soft-delete-retention-register.test.ts b/src/lib/jobs/__tests__/soft-delete-retention-register.test.ts index 70b86563b..a2eb70ae2 100644 --- a/src/lib/jobs/__tests__/soft-delete-retention-register.test.ts +++ b/src/lib/jobs/__tests__/soft-delete-retention-register.test.ts @@ -6,6 +6,13 @@ * tombstones stayed for as long as the instance ran. The set is derived from * `prisma/schema.prisma` (a `deletedAt` field), so a model that starts * soft-deleting cannot ship without saying how long its tombstones live. + * + * The second half holds the live references. A key into a purged model that + * does not cascade is a live record pointing at the tombstone (a visit naming + * a deleted practitioner); the purge would null it and the record would stop + * showing what it showed. Each such key must be one of the entry's + * `referencedBy`, which keeps the tombstone while a live row holds it, and the + * set is derived from the schema's `@relation` lines in both directions. */ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -19,10 +26,67 @@ import { import { TOMBSTONE_RETENTION_DAYS } from "@/lib/auth/native-client"; import { SOFT_DELETE_RETENTION, + purgeCondition, purgeModels, + type LiveReferrer, type SoftDeleteRetention, } from "@/lib/jobs/soft-delete-retention"; +/** + * Every key into a purged model that does not cascade, as the register spells + * a referrer. A cascading key belongs to the row it points at (a visit's link + * rows) and goes with it; any other `onDelete` (SetNull, Prisma's default for + * an optional relation, Restrict, NoAction) leaves a live row behind. + */ +function nullingKeys( + schema: DeclaredSchema, + register: Readonly>, +): Map { + const byTarget = new Map(); + for (const model of schema.models) { + const softDeletes = model.fields.some((f) => f.name === "deletedAt"); + for (const relation of model.relations) { + if (register[relation.target]?.kind !== "purge") continue; + if (relation.onDelete === "Cascade") continue; + for (const key of relation.fields) { + const column = + model.fields.find((f) => f.name === key)?.physicalName ?? key; + const list = byTarget.get(relation.target) ?? []; + list.push({ table: model.physicalName, column, softDeletes }); + byTarget.set(relation.target, list); + } + } + } + return byTarget; +} + +const refKey = (ref: LiveReferrer) => + `${ref.table}.${ref.column}${ref.softDeletes ? "" : " (no tombstone)"}`; + +function referrerProblems( + schema: DeclaredSchema, + register: Readonly>, +): string[] { + const found: string[] = []; + const derived = nullingKeys(schema, register); + for (const [model, entry] of Object.entries(register)) { + if (entry.kind !== "purge") continue; + const want = new Set((derived.get(model) ?? []).map(refKey)); + const have = new Set((entry.referencedBy ?? []).map(refKey)); + for (const key of want) { + if (!have.has(key)) { + found.push(`${model}: ${key} does not cascade and is not a referrer`); + } + } + for (const key of have) { + if (!want.has(key)) { + found.push(`${model}: referrer ${key} is not a nulling key`); + } + } + } + return found; +} + function problems( schema: DeclaredSchema, register: Readonly>, @@ -91,6 +155,94 @@ describe("soft-delete retention register", () => { } }); + it("holds every tombstone a live row still points at", () => { + const derived = nullingKeys(schema, SOFT_DELETE_RETENTION); + // A reader that stopped seeing relations would derive nothing and pass. + const count = [...derived.values()].reduce((n, l) => n + l.length, 0); + expect(count).toBeGreaterThanOrEqual(9); + expect(derived.get("Practitioner")?.map(refKey)).toEqual([ + "encounters.practitioner_id", + "vaccination_records.practitioner_id", + ]); + const found = referrerProblems(schema, SOFT_DELETE_RETENTION); + expect(found, found.join("\n")).toEqual([]); + }); + + it("turns every referrer into a live-row condition", () => { + const entry = SOFT_DELETE_RETENTION.Practitioner; + if (entry.kind !== "purge") throw new Error("Practitioner is purged"); + const sql = purgeCondition(entry); + expect(sql).toContain( + `NOT EXISTS (SELECT 1 FROM "encounters" r WHERE r."practitioner_id" = t."id" AND r."deleted_at" IS NULL)`, + ); + const symptoms = SOFT_DELETE_RETENTION.IllnessEpisode; + if (symptoms.kind !== "purge") throw new Error("IllnessEpisode is purged"); + // A table without tombstones: every row is live. + expect(purgeCondition(symptoms)).toContain( + `NOT EXISTS (SELECT 1 FROM "symptom_events" r WHERE r."episode_id" = t."id")`, + ); + expect(() => + purgeCondition({ + kind: "purge", + table: "widgets", + days: () => 75, + referencedBy: [{ table: "x; DROP", column: "a", softDeletes: true }], + }), + ).toThrow("Bad referrer"); + }); + + it("counter-check: names a new nulling key and a stale referrer", () => { + const dir = mkdtempSync(join(tmpdir(), "hl-retention-")); + try { + const path = join(dir, "schema.prisma"); + writeFileSync( + path, + `model Widget { + id String @id + userId String @map("user_id") + deletedAt DateTime? @map("deleted_at") + @@map("widgets") +} +model Note { + id String @id + widgetId String? @map("widget_id") + ownerId String? @map("owner_id") + deletedAt DateTime? @map("deleted_at") + widget Widget? @relation(fields: [widgetId], references: [id], onDelete: SetNull) + owner Widget? @relation("Owned", fields: [ownerId], references: [id]) + @@map("notes") +} +model Part { + id String @id + widgetId String @map("widget_id") + widget Widget @relation(fields: [widgetId], references: [id], onDelete: Cascade) + @@map("parts") +} +`, + ); + const synthetic = readDeclaredSchema(path); + const found = referrerProblems(synthetic, { + Widget: { + kind: "purge", + table: "widgets", + days: () => 75, + referencedBy: [ + { table: "notes", column: "widget_id", softDeletes: true }, + { table: "gone", column: "widget_id", softDeletes: false }, + ], + }, + }); + // The SetNull key is handled, the default-`onDelete` key is not, the + // cascading key is owned, and the stale referrer is named. + expect(found).toEqual([ + "Widget: notes.owner_id does not cascade and is not a referrer", + "Widget: referrer gone.widget_id (no tombstone) is not a nulling key", + ]); + } finally { + rmSync(dir, { recursive: true }); + } + }); + it("counter-check: names a new soft-delete model, a stale entry and a wrong table", () => { const dir = mkdtempSync(join(tmpdir(), "hl-retention-")); try { diff --git a/src/lib/jobs/soft-delete-retention.ts b/src/lib/jobs/soft-delete-retention.ts index 056f54c78..beab28258 100644 --- a/src/lib/jobs/soft-delete-retention.ts +++ b/src/lib/jobs/soft-delete-retention.ts @@ -24,9 +24,14 @@ * purged measurement. * * The delete runs per account under the restore lock, in bounded batches - * (`purgeTombstonesByAccount`). Foreign keys into these tables cascade or set - * null (none restricts), so a purged visit takes its link rows with it and a - * live vaccination that named it simply loses the pointer. + * (`purgeTombstonesByAccount`). A key that cascades belongs to the row it + * points at: a purged visit takes its link rows with it. A key that does not + * cascade is a live record's reference, and a live record keeps what it shows: + * a visit still names the practitioner it was with after the address-book + * entry is deleted, so that entry is not purged while a live visit or dose + * names it. Those references are each entry's `referencedBy`, and + * `soft-delete-retention-register.test.ts` derives the set from the schema, + * so a new nulling key into a purged model cannot ship without one. */ import type { PrismaClient } from "@/generated/prisma/client"; import { TOMBSTONE_RETENTION_DAYS } from "@/lib/auth/native-client"; @@ -37,6 +42,19 @@ import { type TombstonePurgeOutcome, } from "@/lib/jobs/measurement-tombstone-cleanup"; +/** + * A key into a purged model that does not cascade. While a row that is not + * itself tombstoned holds it, the row it names stays. + */ +export interface LiveReferrer { + /** The physical table holding the key; a literal, spliced into the SQL. */ + table: string; + /** The key column; a literal, spliced into the SQL. */ + column: string; + /** Whether the referring table soft-deletes (a tombstoned row is not live). */ + softDeletes: boolean; +} + export type SoftDeleteRetention = | { kind: "sync-feed"; days: number } | { kind: "document-grace"; days: number } @@ -51,14 +69,31 @@ export type SoftDeleteRetention = * the alias `t`. For a tombstone something live still depends on. */ keep?: string; + /** The live references that hold a tombstone back. */ + referencedBy?: readonly LiveReferrer[]; /** Why the horizon or the condition differs from the default. */ why?: string; }; -const standard = (table: string): SoftDeleteRetention => ({ +const standard = ( + table: string, + referencedBy?: readonly LiveReferrer[], +): SoftDeleteRetention => ({ kind: "purge", table, days: () => TOMBSTONE_RETENTION_DAYS, + ...(referencedBy ? { referencedBy } : {}), +}); + +const live = (table: string, column: string): LiveReferrer => ({ + table, + column, + softDeletes: true, +}); +const always = (table: string, column: string): LiveReferrer => ({ + table, + column, + softDeletes: false, }); export const SOFT_DELETE_RETENTION: Readonly< @@ -75,6 +110,7 @@ export const SOFT_DELETE_RETENTION: Readonly< kind: "purge", table: "menstrual_cycles", days: () => TOMBSTONE_RETENTION_DAYS, + referencedBy: [live("cycle_day_logs", "cycle_id")], keep: `NOT EXISTS (SELECT 1 FROM "menstrual_cycles" a WHERE a."id" = t."absorbed_into_id" AND a."deleted_at" IS NULL)`, @@ -88,26 +124,61 @@ export const SOFT_DELETE_RETENTION: Readonly< }, MentalHealthAssessment: standard("mental_health_assessments"), - MeasurementReminder: standard("measurement_reminders"), + MeasurementReminder: standard("measurement_reminders", [ + live("encounters", "reminder_id"), + live("vaccination_records", "reminder_id"), + ]), LabResult: standard("lab_results"), CustomMetric: standard("custom_metrics"), CustomMetricEntry: standard("custom_metric_entries"), CoachPlan: standard("coach_plans"), CoachReminder: standard("coach_reminders"), - IllnessEpisode: standard("illness_episodes"), + IllnessEpisode: standard("illness_episodes", [ + live("illness_episodes", "parent_condition_id"), + always("symptom_events", "episode_id"), + ]), IllnessDayLog: standard("illness_day_logs"), Allergy: standard("allergies"), FamilyHistoryEntry: standard("family_history_entries"), - Encounter: standard("encounters"), - Practitioner: standard("practitioners"), + Encounter: standard("encounters", [ + live("vaccination_records", "encounter_id"), + ]), + Practitioner: standard("practitioners", [ + live("encounters", "practitioner_id"), + live("vaccination_records", "practitioner_id"), + ]), VaccinationRecord: standard("vaccination_records"), - CustomVaccine: standard("custom_vaccines"), + CustomVaccine: standard("custom_vaccines", [ + live("vaccination_records", "custom_vaccine_id"), + ]), LifeEvent: standard("life_events"), }; const TABLE = /^[a-z_]+$/; const DAY_MS = 86_400_000; +/** + * The condition a purgeable row must meet, over the alias `t`: the entry's + * own `keep`, and no live row holding a key to it. Every identifier comes + * from the register's literals and is asserted against a closed pattern. + */ +export function purgeCondition( + entry: Extract, +): string { + const parts: string[] = []; + if (entry.keep) parts.push(entry.keep); + for (const ref of entry.referencedBy ?? []) { + if (!TABLE.test(ref.table) || !TABLE.test(ref.column)) { + throw new Error(`Bad referrer ${ref.table}.${ref.column}`); + } + const alive = ref.softDeletes ? ` AND r."deleted_at" IS NULL` : ""; + parts.push( + `NOT EXISTS (SELECT 1 FROM "${ref.table}" r WHERE r."${ref.column}" = t."id"${alive})`, + ); + } + return parts.map((part) => `AND ${part}`).join(" "); +} + /** One table's purge, for the run report. */ export interface SoftDeletePurgeLeg extends TombstonePurgeOutcome { model: string; @@ -130,7 +201,7 @@ export async function purgeSoftDeletedTable( } const table = `"${entry.table}"`; const cutoff = new Date(now.getTime() - entry.days() * DAY_MS); - const keep = entry.keep ? `AND ${entry.keep}` : ""; + const keep = purgeCondition(entry); const outcome = await purgeTombstonesByAccount({ prisma, findRows: (take, skipUserIds) => diff --git a/tests/integration/soft-delete-purge.test.ts b/tests/integration/soft-delete-purge.test.ts index bd14bbb06..d84cc7b01 100644 --- a/tests/integration/soft-delete-purge.test.ts +++ b/tests/integration/soft-delete-purge.test.ts @@ -3,10 +3,11 @@ * retention register hands to `soft-delete-purge`. * * Proves, against the migrated schema: every table's purge statement runs; an - * expired tombstone goes while a recent one and a live row stay; a purged - * visit cascades its links and leaves a live vaccination that named it in - * place; a start folded into a live cycle stays; the coach facts keep their - * longer horizon. + * expired tombstone goes while a recent one and a live row stay; a tombstone + * a live record still points at stays (a deleted practitioner a live visit + * names, a deleted visit a live dose names) and goes once nothing live names + * it; a purged visit cascades its links; a start folded into a live cycle + * stays; the coach facts keep their longer horizon. */ import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -75,34 +76,111 @@ describe("soft-delete purge", () => { expect(left.map((r) => r.id)).toEqual(["lab-live", "lab-recent"]); }); - it("cascades a purged visit's links and unhooks a live vaccination", async () => { + it("keeps a deleted practitioner a live visit still names", async () => { + const practitioner = (id: string) => + prisma.practitioner.create({ + data: { id, userId: USER, name: id, deletedAt: EXPIRED }, + }); + await practitioner("doc-live-visit"); + await practitioner("doc-gone-visit"); + await practitioner("doc-unused"); await prisma.encounter.create({ data: { - id: "visit-expired", + id: "visit-live", userId: USER, occurredAt: new Date("2026-01-01T08:00:00.000Z"), - deletedAt: EXPIRED, + practitionerId: "doc-live-visit", }, }); + // A visit that is itself a tombstone holds nothing back. + await prisma.encounter.create({ + data: { + id: "visit-tombstoned", + userId: USER, + occurredAt: new Date("2026-01-02T08:00:00.000Z"), + practitionerId: "doc-gone-visit", + deletedAt: RECENT, + }, + }); + + const leg = await purgeSoftDeletedTable(prisma, "Practitioner", NOW); + + expect(leg.deleted).toBe(2); + const left = await prisma.practitioner.findMany({ select: { id: true } }); + expect(left.map((r) => r.id)).toEqual(["doc-live-visit"]); + expect( + await prisma.encounter.findUniqueOrThrow({ + where: { id: "visit-live" }, + select: { practitionerId: true }, + }), + ).toEqual({ practitionerId: "doc-live-visit" }); + }); + + it("purges the practitioner once the visit naming it is gone", async () => { + await prisma.practitioner.create({ + data: { id: "doc", userId: USER, name: "doc", deletedAt: EXPIRED }, + }); + await prisma.encounter.create({ + data: { + id: "visit", + userId: USER, + occurredAt: new Date("2026-01-01T08:00:00.000Z"), + practitionerId: "doc", + }, + }); + await purgeSoftDeletedTable(prisma, "Practitioner", NOW); + expect(await prisma.practitioner.count()).toBe(1); + + await prisma.encounter.update({ + where: { id: "visit" }, + data: { deletedAt: EXPIRED }, + }); + // The visit goes first in the run's order, and the practitioner with it. + await purgeSoftDeletedRows(prisma, NOW); + expect(await prisma.encounter.count()).toBe(0); + expect(await prisma.practitioner.count()).toBe(0); + }); + + it("keeps a deleted visit a live dose names and cascades a purged visit's links", async () => { + const visit = (id: string) => + prisma.encounter.create({ + data: { + id, + userId: USER, + occurredAt: new Date("2026-01-01T08:00:00.000Z"), + deletedAt: EXPIRED, + }, + }); + await visit("visit-named"); + await visit("visit-linked"); await prisma.vaccinationRecord.create({ data: { id: "vacc-live", userId: USER, occurredAt: new Date("2026-01-01T08:00:00.000Z"), - encounterId: "visit-expired", + encounterId: "visit-named", + }, + }); + await prisma.encounterVaccinationLink.create({ + data: { + userId: USER, + encounterId: "visit-linked", + vaccinationId: "vacc-live", }, }); const leg = await purgeSoftDeletedTable(prisma, "Encounter", NOW); expect(leg.deleted).toBe(1); - expect(await prisma.encounter.count()).toBe(0); + const left = await prisma.encounter.findMany({ select: { id: true } }); + expect(left.map((r) => r.id)).toEqual(["visit-named"]); + expect(await prisma.encounterVaccinationLink.count()).toBe(0); expect( await prisma.vaccinationRecord.findUniqueOrThrow({ where: { id: "vacc-live" }, select: { encounterId: true, deletedAt: true }, }), - ).toEqual({ encounterId: null, deletedAt: null }); + ).toEqual({ encounterId: "visit-named", deletedAt: null }); }); it("keeps a start folded into a live cycle until that cycle goes", async () => { From a275a5bb65f1299469ee6384c4885a85c01ad248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:01:13 +0200 Subject: [PATCH 02/13] fix(records): show a deleted practitioner on the live visits that name it The web record and the doctor report named the practitioner of a live visit after the address-book entry was deleted; the MCP read, the search and the appointment reminder did not. All of them now name it, the entry stays out of the address book and the pickers, and a portable backup carries it as a tombstone instead of dropping the visit's reference. --- docs/api/openapi.yaml | 5 +- src/app/api/practitioners/[id]/route.ts | 7 ++- .../__tests__/appointment-reminder.test.ts | 32 ++++++++++ src/lib/encounters/appointment-reminder.ts | 9 ++- src/lib/export/visits-backup.ts | 18 +++++- .../mcp/__tests__/module-gate-sweep.test.ts | 8 ++- src/lib/mcp/__tests__/record-search.test.ts | 16 ++++- src/lib/mcp/record-search.ts | 27 ++++----- src/lib/mcp/tools.ts | 22 +++---- src/lib/openapi/routes/encounters.ts | 2 +- .../practitioner-contact-encryption.test.ts | 60 +++++++++++++++++++ 11 files changed, 159 insertions(+), 47 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 7e7c2d163..06661dca7 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -16369,8 +16369,9 @@ paths: tags: - Records summary: Soft-delete a practitioner - description: Stamps `deletedAt`. Every visit that named this entry survives with no practice attached, because the - reference nulls rather than cascades. Idempotent. Audits as `practitioner.contact.delete`. + description: Stamps `deletedAt`. The entry leaves the address book and the pickers; every live visit and vaccination + that named it keeps showing it, and the tombstone is kept while one does. Idempotent. Audits as + `practitioner.contact.delete`. parameters: - in: path name: id diff --git a/src/app/api/practitioners/[id]/route.ts b/src/app/api/practitioners/[id]/route.ts index 3377dd7b2..0d1f8819d 100644 --- a/src/app/api/practitioners/[id]/route.ts +++ b/src/app/api/practitioners/[id]/route.ts @@ -7,9 +7,10 @@ * read by id and then checked against the resolved user, so a foreign id and a * missing id give the same 404 and probing learns nothing. * - * Deleting a practitioner keeps every visit that named it. The foreign key is - * SetNull, so the visits survive with no practice attached rather than - * disappearing with the address-book entry. + * Deleting a practitioner takes it out of the address book and the pickers, + * not out of the history: every live visit and dose that named it keeps + * showing it, and the nightly purge keeps the tombstone for as long as one + * does (`soft-delete-retention.ts`). */ import { NextRequest } from "next/server"; diff --git a/src/lib/encounters/__tests__/appointment-reminder.test.ts b/src/lib/encounters/__tests__/appointment-reminder.test.ts index 6c3d881d2..47eada90e 100644 --- a/src/lib/encounters/__tests__/appointment-reminder.test.ts +++ b/src/lib/encounters/__tests__/appointment-reminder.test.ts @@ -18,6 +18,7 @@ import { describe, expect, it } from "vitest"; import { appointmentNextDueAfterFiring, appointmentNotifyHour, + appointmentReminderAddress, } from "@/lib/encounters/appointment-reminder"; const TZ = "Europe/Berlin"; @@ -84,3 +85,34 @@ describe("the hour the nudge fires", () => { expect(appointmentNotifyHour(APPOINTMENT, "UTC")).toBe(7); }); }); + +describe("the address an appointment reminder names", () => { + const reminder = (practitionerUserId: string) => ({ + userId: "u1", + origin: "ENCOUNTER", + location: null, + encounters: [ + { + userId: "u1", + practitioner: { + userId: practitionerUserId, + location: "Hauptstraße 1", + locationEncrypted: null, + }, + }, + ], + }); + + it("is the practice of the visit's practitioner, deleted from the address book or not", () => { + // The select carries no `deletedAt`: the booked visit still shows the + // practitioner, so the reminder names the same practice. + expect(appointmentReminderAddress(reminder("u1"))).toBe("Hauptstraße 1"); + }); + + it("counter-check: another account's practitioner names nothing", () => { + expect(appointmentReminderAddress(reminder("u2"))).toBeNull(); + expect( + appointmentReminderAddress({ ...reminder("u1"), encounters: [] }), + ).toBeNull(); + }); +}); diff --git a/src/lib/encounters/appointment-reminder.ts b/src/lib/encounters/appointment-reminder.ts index 2565edcea..0cae8c541 100644 --- a/src/lib/encounters/appointment-reminder.ts +++ b/src/lib/encounters/appointment-reminder.ts @@ -199,7 +199,6 @@ export const APPOINTMENT_REMINDER_ADDRESS_INCLUDE = { practitioner: { select: { userId: true, - deletedAt: true, location: true, locationEncrypted: true, }, @@ -212,7 +211,9 @@ export const APPOINTMENT_REMINDER_ADDRESS_INCLUDE = { /** * The address a reminder's push names. A checkup's own free-text `location`; * for an appointment, the owning visit's practitioner address, decrypted - * here. Null when the visit names no live practitioner of the same account. + * here. Null when the visit names no practitioner of the same account. A + * practitioner deleted from the address book is still the one the booked + * visit is with, and the visit still shows it, so the reminder names it too. */ export function appointmentReminderAddress(reminder: { userId: string; @@ -222,7 +223,6 @@ export function appointmentReminderAddress(reminder: { userId: string; practitioner: { userId: string; - deletedAt: Date | null; location: string | null; locationEncrypted: Uint8Array | null; } | null; @@ -235,8 +235,7 @@ export function appointmentReminderAddress(reminder: { !visit || !practitioner || visit.userId !== reminder.userId || - practitioner.userId !== reminder.userId || - practitioner.deletedAt !== null + practitioner.userId !== reminder.userId ) { return null; } diff --git a/src/lib/export/visits-backup.ts b/src/lib/export/visits-backup.ts index 2957bb947..2baa603e7 100644 --- a/src/lib/export/visits-backup.ts +++ b/src/lib/export/visits-backup.ts @@ -222,7 +222,19 @@ export async function buildVisitsBackupSection( conditionLinkRows, ] = await Promise.all([ prisma.practitioner.findMany({ - where: disasterRecovery ? { userId } : { userId, deletedAt: null }, + // A portable file leaves the address book's tombstones out, except one + // a live visit or dose still names: the record shows it, so the file + // carries it (with its `deletedAt`) instead of dropping the reference. + where: disasterRecovery + ? { userId } + : { + userId, + OR: [ + { deletedAt: null }, + { encounters: { some: { deletedAt: null } } }, + { vaccinations: { some: { deletedAt: null } } }, + ], + }, orderBy: { name: "asc" }, select: PRACTITIONER_BACKUP_SELECT, }), @@ -290,7 +302,9 @@ export async function buildVisitsBackupSection( }), createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), - ...(disasterRecovery + // A portable file names a tombstone only when a live visit holds it, + // and says so, so the restore keeps it out of the address book. + ...(disasterRecovery || row.deletedAt ? { deletedAt: row.deletedAt?.toISOString() ?? null } : {}), })), diff --git a/src/lib/mcp/__tests__/module-gate-sweep.test.ts b/src/lib/mcp/__tests__/module-gate-sweep.test.ts index 20d0bfdf6..88e88aa78 100644 --- a/src/lib/mcp/__tests__/module-gate-sweep.test.ts +++ b/src/lib/mcp/__tests__/module-gate-sweep.test.ts @@ -190,11 +190,13 @@ describe("get_visits", () => { expect(result.visits[0].conditions).toEqual(["Knee pain"]); }); - it("does not name a deleted practitioner", async () => { + it("names the practitioner a live visit was with, even once deleted from the address book", async () => { + // The web record and the doctor report show it; the agent reads the same + // record (the address-book entry is kept while a live visit names it). const result = (await tool("get_visits").run(CTX, {})) as { visits: Array<{ practitioner: string | null; specialty: string | null }>; }; - expect(result.visits[0].practitioner).toBeNull(); - expect(result.visits[0].specialty).toBeNull(); + expect(result.visits[0].practitioner).toBe("Dr. Gone"); + expect(result.visits[0].specialty).toBe("Cardiology"); }); }); diff --git a/src/lib/mcp/__tests__/record-search.test.ts b/src/lib/mcp/__tests__/record-search.test.ts index 7a7f5e9e1..67f3a9e34 100644 --- a/src/lib/mcp/__tests__/record-search.test.ts +++ b/src/lib/mcp/__tests__/record-search.test.ts @@ -797,16 +797,26 @@ describe("short query words", () => { }); describe("practitioners and planned visits", () => { - it("does not match or show a deleted practitioner's name", async () => { + it("finds and names the practitioner a live visit was with, even once deleted from the address book", async () => { + // Deleting the entry takes it out of the address book, not out of the + // visit: the web record shows it, and so does the agent's read. store.encounter[0].practitioner = { name: "Dr. Weber", specialty: "Orthopädie", - deletedAt: new Date(), }; + expect((await search(ME, "weber")).results.map((r) => r.id)).toEqual([ + "visit:v-knee", + ]); + const hit = await fetchId(ME, "visit:v-knee"); + expect(hit.metadata.practitioner).toBe("Dr. Weber"); + expect(hit.text).toContain("Weber"); + }); + + it("counter-check: a visit with no practitioner names none", async () => { + store.encounter[0].practitioner = null; expect((await search(ME, "weber")).results).toHaveLength(0); const hit = await fetchId(ME, "visit:v-knee"); expect(hit.metadata.practitioner).toBeNull(); - expect(hit.text).not.toContain("Weber"); }); it("lists past visits, not future planned ones, for an empty query", async () => { diff --git a/src/lib/mcp/record-search.ts b/src/lib/mcp/record-search.ts index 79cca5ebd..6edfa7102 100644 --- a/src/lib/mcp/record-search.ts +++ b/src/lib/mcp/record-search.ts @@ -322,27 +322,26 @@ function visitTitle(args: { } /** - * The practitioner columns a record read selects. `deletedAt` rides along so - * a practitioner the person removed contributes nothing: the visit keeps its - * scalar id, but the name is no longer something the record says. + * The practitioner columns a record read selects. A live visit or dose names + * the practitioner it was with even after the address-book entry is deleted, + * as the web record and the doctor report do: deleting the entry takes it out + * of the address book and the pickers, not out of the history. The purge keeps + * the entry for as long as a live record names it (`soft-delete-retention.ts`). */ const PRACTITIONER_SELECT = { - select: { name: true, specialty: true, deletedAt: true }, + select: { name: true, specialty: true }, } as const; type PractitionerRow = { name: string; specialty: string | null; - deletedAt: Date | null; } | null; -/** The practitioner as the record may still show it, or null. */ -export function livePractitioner( +/** The practitioner a live visit or dose names, or null. */ +export function visitPractitioner( row: PractitionerRow | undefined, ): { name: string; specialty: string | null } | null { - return row && row.deletedAt === null - ? { name: row.name, specialty: row.specialty } - : null; + return row ? { name: row.name, specialty: row.specialty } : null; } async function visitCandidates( @@ -368,7 +367,7 @@ async function visitCandidates( withinCap(rows, read), read, (row): SearchCandidate => { - const practitioner = livePractitioner(row.practitioner); + const practitioner = visitPractitioner(row.practitioner); const reason = decryptText(row.reasonEncrypted); const outcome = decryptText(row.outcomeEncrypted); const bodySite = decryptText(row.bodySiteEncrypted); @@ -728,7 +727,7 @@ async function vaccinationCandidates( withinCap(rows, read), read, (row): SearchCandidate => { - const practitioner = livePractitioner(row.practitioner); + const practitioner = visitPractitioner(row.practitioner); const entry = resolveVaccineEntry({ antigenSlug: row.antigenSlug }); const own = ownVaccine(row); const catalogEn = vaccineCatalogName(row.antigenSlug, "en"); @@ -1098,7 +1097,7 @@ async function fetchVisit( const bodySite = decryptText(row.bodySiteEncrypted); const day = userDayKey(row.occurredAt, owner.timezone); const kindLabel = encounterKindLabel(row.kind, "en"); - const live = livePractitioner(row.practitioner); + const live = visitPractitioner(row.practitioner); const practitioner = live?.name ?? null; const specialty = live?.specialty ?? null; @@ -1378,7 +1377,7 @@ async function fetchVaccination( const own = ownVaccine(row); const name = row.vaccineName ?? catalog ?? own?.name ?? row.antigenSlug ?? "Vaccination"; - const practitioner = livePractitioner(row.practitioner); + const practitioner = visitPractitioner(row.practitioner); const day = dateOnly(row.occurredAt); const dose = row.doseNumber !== null diff --git a/src/lib/mcp/tools.ts b/src/lib/mcp/tools.ts index 476d8a3ed..4eb2b3b17 100644 --- a/src/lib/mcp/tools.ts +++ b/src/lib/mcp/tools.ts @@ -2236,19 +2236,15 @@ export const MCP_TOOLS: McpToolDefinition[] = [ orderBy: [{ occurredAt: "asc" }, { id: "asc" }], take, include: { - practitioner: { - select: { name: true, specialty: true, deletedAt: true }, - }, + practitioner: { select: { name: true, specialty: true } }, }, }), ); const appointments = appointmentRows.map((r) => { const reason = decryptVisitText(r.reasonEncrypted); - // A practitioner the person deleted is no longer named. - const practitioner = - r.practitioner && r.practitioner.deletedAt === null - ? r.practitioner - : null; + // A live visit names its practitioner even after the address-book + // entry is deleted, as the web record does. + const practitioner = r.practitioner; return { occurredAt: r.occurredAt.toISOString(), practitioner: practitioner @@ -2578,7 +2574,7 @@ export const MCP_TOOLS: McpToolDefinition[] = [ take, include: { practitioner: { - select: { name: true, specialty: true, deletedAt: true }, + select: { name: true, specialty: true }, }, }, }), @@ -2613,11 +2609,9 @@ export const MCP_TOOLS: McpToolDefinition[] = [ const reason = decryptVisitText(r.reasonEncrypted); const outcome = decryptVisitText(r.outcomeEncrypted); const bodySite = decryptVisitText(r.bodySiteEncrypted); - // A practitioner the person deleted is no longer named. - const seenBy = - r.practitioner && r.practitioner.deletedAt === null - ? r.practitioner - : null; + // A live visit names its practitioner even after the address-book + // entry is deleted, as the web record does. + const seenBy = r.practitioner; return { occurredAt: r.occurredAt.toISOString(), status: r.status, diff --git a/src/lib/openapi/routes/encounters.ts b/src/lib/openapi/routes/encounters.ts index 36cd0c7d4..68136f0b2 100644 --- a/src/lib/openapi/routes/encounters.ts +++ b/src/lib/openapi/routes/encounters.ts @@ -705,7 +705,7 @@ export const encounterPaths: NonNullable = { tags: ["Records"], summary: "Soft-delete a practitioner", description: - "Stamps `deletedAt`. Every visit that named this entry survives with no practice attached, because the reference nulls rather than cascades. Idempotent. Audits as `practitioner.contact.delete`.", + "Stamps `deletedAt`. The entry leaves the address book and the pickers; every live visit and vaccination that named it keeps showing it, and the tombstone is kept while one does. Idempotent. Audits as `practitioner.contact.delete`.", requestParams: idPath, responses: { ...recordRefusal(), diff --git a/tests/integration/practitioner-contact-encryption.test.ts b/tests/integration/practitioner-contact-encryption.test.ts index d98363add..82df5cb39 100644 --- a/tests/integration/practitioner-contact-encryption.test.ts +++ b/tests/integration/practitioner-contact-encryption.test.ts @@ -322,4 +322,64 @@ describe("practitioner phone and address at rest", () => { ]); } }); + + it("carries a deleted practitioner a live visit names in a portable file, and no other tombstone", async () => { + const prisma = getPrismaClient(); + const gone = new Date("2026-09-01T00:00:00.000Z"); + for (const id of ["pr-live", "pr-named", "pr-orphan"]) { + await prisma.practitioner.create({ + data: { + id, + userId: OWNER_ID, + name: id, + deletedAt: id === "pr-live" ? null : gone, + }, + }); + } + await prisma.encounter.create({ + data: { + id: "visit-named", + userId: OWNER_ID, + occurredAt: new Date("2026-08-01T08:00:00.000Z"), + practitionerId: "pr-named", + }, + }); + + const portable = await buildVisitsBackupSection(prisma, OWNER_ID, { + purpose: "portable-export", + }); + + expect( + portable.practitioners + .map((p) => ({ id: p.id, deleted: Boolean(p.deletedAt) })) + .sort((a, b) => a.id.localeCompare(b.id)), + ).toEqual([ + { id: "pr-live", deleted: false }, + { id: "pr-named", deleted: true }, + ]); + expect(portable.encounters[0].practitionerId).toBe("pr-named"); + + // The restore brings the named entry back as a tombstone: on the visit, + // not in the address book. + await prisma.$transaction(async (tx) => { + await restoreVisitsData( + tx, + OWNER_ID, + { + practitioners: portable.practitioners, + encounters: portable.encounters, + encounterDocumentLinks: [], + encounterLabLinks: [], + encounterConditionLinks: [], + }, + [], + ); + }); + const visit = await prisma.encounter.findFirstOrThrow({ + where: { userId: OWNER_ID }, + select: { practitioner: { select: { id: true, deletedAt: true } } }, + }); + expect(visit.practitioner?.id).toBe("pr-named"); + expect(visit.practitioner?.deletedAt).not.toBeNull(); + }); }); From 2b81b4f23e0afd896997e6a45aba912db9b7eeb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:06:10 +0200 Subject: [PATCH 03/13] fix(migrations): retire the measurements prefix index without waiting at boot 0416 ran DROP INDEX CONCURRENTLY at boot, which waits without bound for every transaction on the table; a pg_dump or a backfill outlasted the start period and the next boot stopped at P3009. The migration now tries a plain drop with a two-second lock wait and tolerates the timeout, and the nightly measurement maintenance drops what is left concurrently under a bounded wait, retrying the next night. A fresh or shadow database takes the lock at once, so the history still ends where the schema does. The guard refuses a concurrent drop in a migration and ties every retired index to a tolerant one. --- docs/ops/measurement-maintenance.md | 60 +++++++- .../migration.sql | 34 ++++- .../migration-lock-timeout-guard.test.ts | 70 ++++++++- src/lib/jobs/measurement-maintenance.ts | 103 ++++++++++++- tests/integration/retired-index-drop.test.ts | 140 ++++++++++++++++++ 5 files changed, 388 insertions(+), 19 deletions(-) create mode 100644 tests/integration/retired-index-drop.test.ts diff --git a/docs/ops/measurement-maintenance.md b/docs/ops/measurement-maintenance.md index d6a7865eb..eb95cb053 100644 --- a/docs/ops/measurement-maintenance.md +++ b/docs/ops/measurement-maintenance.md @@ -151,7 +151,8 @@ SELECT pg_size_pretty(pg_total_relation_size('measurements')) AS total, ## The nightly check Since v1.43.7 the same queue also runs by itself at 03:40 (Europe/Berlin). -That run names no steps and changes nothing: vacuuming the table is +That run names no steps and changes nothing beyond dropping a retired index +(next section): vacuuming the table is autovacuum's job, which migration 0412 starts at 5 % dead rows instead of Postgres's 20 %. The check reads the table's live and dead rows and warns (`maintenance_autovacuum_behind` on the wide event) when there are at least @@ -185,6 +186,63 @@ FROM pg_stat_user_tables WHERE relname = 'measurements'; applies to pages written after the migration; a `VACUUM FULL` after the upgrade applies it to the whole table. +## Retired indexes + +An index the schema no longer declares has to leave the database too, and on +`measurements` the obvious migration is the wrong one. A plain `DROP INDEX` +needs an exclusive lock on the table; `DROP INDEX CONCURRENTLY` blocks nobody +but waits, without limit, for every transaction already running on the +table. A migration runs at boot, so either one waits for whatever holds the +table (a `pg_dump`, a long backfill, a `VACUUM FULL`); past the container's +start period the container is replaced with the migration half done, and the +next boot stops at `P3009` until someone runs `prisma migrate resolve`. + +So an index is retired in two halves (v1.43.8, migration 0416, for +`measurements_user_id_type_measured_at_idx`): + +1. The migration tries a plain `DROP INDEX IF EXISTS` inside a `DO` block with + a two-second `lock_timeout` and catches the timeout + (`EXCEPTION WHEN lock_not_available`). When nothing holds the table the + index is gone at once; otherwise the migration still succeeds and the + index stays. The longest the table's other statements can wait behind the + attempt is those two seconds. A new database and Prisma's shadow database + always take the lock, so the migration history ends without the index, + exactly as the schema does, and `prisma migrate dev` / `migrate diff` see + no drift and generate nothing. +2. The index name goes into `RETIRED_MEASUREMENT_INDEXES` in + `src/lib/jobs/measurement-maintenance.ts`. Every run of this job, the + nightly 03:40 check included, drops each name still present with + `DROP INDEX CONCURRENTLY IF EXISTS`, waiting at most 60 seconds for its + locks. A run that runs out of time leaves the index (possibly marked + invalid, which the next drop finishes), sets + `maintenance_retired_index_pending` on the wide event with a warning, and + the next night tries again. The index costs writes until then, as it did + before the release; nothing reads it. + +Until the job has run, the database holds one index the schema does not +declare. Nothing compares the two at runtime, and the only place they are +compared, a `migrate diff` against the migration history, never sees it. To +check by hand: + +```sql +SELECT indexname FROM pg_indexes + WHERE tablename = 'measurements' + AND indexname = 'measurements_user_id_type_measured_at_idx'; +``` + +No row: it is gone. To drop it now rather than at night, start a run from the +admin card (any steps), or run the same statement yourself: + +```sql +SET lock_timeout = '60s'; +DROP INDEX CONCURRENTLY IF EXISTS "measurements_user_id_type_measured_at_idx"; +``` + +For the next large index, follow the same two halves. +`src/__tests__/migration-lock-timeout-guard.test.ts` refuses a +`DROP INDEX CONCURRENTLY` in any migration and requires every retired name to +have a migration that bounds its wait and catches the timeout. + ## "could not resize shared memory segment" Up to v1.42.0 the run could stop at once with `could not resize shared memory diff --git a/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql b/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql index ff86f6c2c..62339c211 100644 --- a/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql +++ b/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql @@ -1,4 +1,4 @@ --- Drop `measurements_user_id_type_measured_at_idx`. +-- Retire `measurements_user_id_type_measured_at_idx`. -- -- `(user_id, type, measured_at)` is the leading part of the unique key -- `(user_id, type, measured_at, source, sleep_stage)`, and live reads have @@ -8,10 +8,28 @@ -- seeded table). The index cost every write to the densest table in the -- schema and, on a bloated instance, more space than the table itself. -- --- `DROP INDEX CONCURRENTLY` takes no lock that blocks reads or writes: it --- waits for the transactions already using the index to finish and then --- removes it. It refuses to run inside a transaction block, and under --- `migrate deploy` it does so whenever the file holds any other statement --- (Prisma 7 on Postgres 16 reports 25001), so this file holds exactly this --- one statement. `IF EXISTS` makes a re-run a no-op. -DROP INDEX CONCURRENTLY IF EXISTS "measurements_user_id_type_measured_at_idx"; +-- This runs at boot, so it must not wait on the table. A plain `DROP INDEX` +-- needs ACCESS EXCLUSIVE on `measurements`, held for the instant the catalog +-- changes; `DROP INDEX CONCURRENTLY` blocks nobody but waits, without bound, +-- for every transaction on the table (a pg_dump, a backfill), long enough for +-- the container to be replaced mid-migration and the next boot to stop at +-- P3009. So the drop is tried once with a two-second lock wait (the most the +-- table's other statements can queue behind the request), and a timeout is +-- swallowed: the migration then succeeds with the index still there, and +-- the nightly measurement maintenance drops it with +-- `DROP INDEX CONCURRENTLY IF EXISTS` (`RETIRED_MEASUREMENT_INDEXES` in +-- `src/lib/jobs/measurement-maintenance.ts`). A fresh database and Prisma's +-- shadow database take the lock at once, so the migration history ends +-- without the index, as the schema does. The pattern, for the next large +-- index: docs/ops/measurement-maintenance.md, "Retired indexes". +-- +-- @retry-on-lock-timeout + +DO $migration$ +BEGIN + PERFORM set_config('lock_timeout', '2s', true); + DROP INDEX IF EXISTS "measurements_user_id_type_measured_at_idx"; +EXCEPTION WHEN lock_not_available THEN + RAISE NOTICE 'measurements_user_id_type_measured_at_idx is in use; the nightly measurement maintenance drops it'; +END +$migration$; diff --git a/src/__tests__/migration-lock-timeout-guard.test.ts b/src/__tests__/migration-lock-timeout-guard.test.ts index 45c7595bf..919e7661f 100644 --- a/src/__tests__/migration-lock-timeout-guard.test.ts +++ b/src/__tests__/migration-lock-timeout-guard.test.ts @@ -23,6 +23,8 @@ import { readdirSync, readFileSync, existsSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; +import { RETIRED_MEASUREMENT_INDEXES } from "@/lib/jobs/measurement-maintenance"; + import { RETRY_MARKER } from "../../scripts/migrate-deploy.mjs"; const MIGRATIONS_DIR = join(process.cwd(), "prisma", "migrations"); @@ -114,13 +116,14 @@ describe("concurrent index statements", () => { expect(found, found.join("\n")).toEqual([]); }); - it("sees the concurrent migrations there are", () => { - const concurrent = migrations() - .filter(({ sql }) => - /\bCONCURRENTLY\b/i.test(topLevelStatements(sql).join(";")), - ) - .map(({ name }) => name); - expect(concurrent).toContain("0416_drop_measurements_type_prefix_index"); + it("reads the statements there are", () => { + // No migration holds a concurrent statement since 0416 stopped dropping + // concurrently; the rule stands for the next one. A reader that stopped + // splitting statements would see none and pass, so it must see many. + const statements = migrations().flatMap(({ sql }) => + topLevelStatements(sql), + ); + expect(statements.length).toBeGreaterThan(1000); }); it("refuses a concurrent statement beside another one", () => { @@ -139,6 +142,58 @@ describe("concurrent index statements", () => { }); }); +/** + * `DROP INDEX CONCURRENTLY` blocks no reader or writer, but it waits without + * bound for every transaction on the table, and in a migration that wait is + * the boot's: a pg_dump or a backfill outlasts the container's start period, + * the half-applied migration is left on record, and the next boot stops at + * P3009. An index is retired instead by a bounded `DROP INDEX` that tolerates + * the timeout, finished by the nightly measurement maintenance (0416). + */ +function concurrentDropViolations(name: string, sql: string): string[] { + return topLevelStatements(sql).some((statement) => + /^DROP INDEX CONCURRENTLY\b/i.test(statement), + ) + ? [`${name}: DROP INDEX CONCURRENTLY waits on the table at boot`] + : []; +} + +describe("retired indexes", () => { + it("are never dropped concurrently in the boot path", () => { + const found = migrations().flatMap(({ name, sql }) => + concurrentDropViolations(name, sql), + ); + expect(found, found.join("\n")).toEqual([]); + }); + + it("are each dropped by a migration that bounds its wait and tolerates the timeout", () => { + expect(RETIRED_MEASUREMENT_INDEXES.length).toBeGreaterThan(0); + for (const index of RETIRED_MEASUREMENT_INDEXES) { + const owner = migrations().find(({ sql }) => + new RegExp(`DROP INDEX IF EXISTS "${index}"`).test(sql), + ); + expect(owner, index).toBeDefined(); + expect(owner?.sql).toMatch(/set_config\(\s*'lock_timeout'/); + expect(owner?.sql).toMatch(/EXCEPTION WHEN lock_not_available THEN/); + } + }); + + it("counter-check: refuses a concurrent drop, not one in a comment", () => { + expect( + concurrentDropViolations( + "a", + 'DROP INDEX CONCURRENTLY IF EXISTS "measurements_x_idx";', + ), + ).toHaveLength(1); + expect( + concurrentDropViolations( + "b", + '-- DROP INDEX CONCURRENTLY "x";\nDROP INDEX IF EXISTS "x";', + ), + ).toEqual([]); + }); +}); + describe("migration lock waits", () => { it("bounds lock waits where they take effect and marks only repeatable files", () => { const found = migrations().flatMap(({ name, sql }) => @@ -154,6 +209,7 @@ describe("migration lock waits", () => { expect(marked).toContain("0412_measurement_hot_updates"); expect(marked).toContain("0414_drop_medication_compliance_rollups"); expect(marked).toContain("0415_foreign_key_and_prefix_indexes"); + expect(marked).toContain("0416_drop_measurements_type_prefix_index"); }); it("catches each shape it exists to refuse", () => { diff --git a/src/lib/jobs/measurement-maintenance.ts b/src/lib/jobs/measurement-maintenance.ts index a5e4efa0f..144427b66 100644 --- a/src/lib/jobs/measurement-maintenance.ts +++ b/src/lib/jobs/measurement-maintenance.ts @@ -46,6 +46,15 @@ * before the first rebuild. * - Every step's size before and after, and its duration, are on the run's * wide event; the totals are job facts. + * + * Retired indexes. An index the schema no longer declares is dropped by the + * migration that retires it only when the lock is there at once (a `DO` + * block with a two-second `lock_timeout` that swallows the timeout), so the + * boot never waits on a long reader of the table. Where the migration had to + * leave it, every run of this job, the nightly tick included, drops it with + * `DROP INDEX CONCURRENTLY IF EXISTS` under a bounded lock wait and tries + * again the next night if the wait runs out. Runbook: + * `docs/ops/measurement-maintenance.md`, "Retired indexes". */ import { Client } from "pg"; import type { Job } from "pg-boss"; @@ -153,6 +162,22 @@ export function maintenanceSessionStatements(): string[] { }); } +/** + * Indexes on `measurements` the schema no longer declares, which a run drops + * when the migration that retired them could not. Each one's migration names + * it; `IF EXISTS` makes the drop a no-op once it is gone. + */ +export const RETIRED_MEASUREMENT_INDEXES: readonly string[] = [ + // 0416: a prefix of the unique key `(user_id, type, measured_at, source, + // sleep_stage)`, which plans every query it served at the same cost. + "measurements_user_id_type_measured_at_idx", +]; + +/** How long a run waits for the locks that drop a retired index. */ +export const RETIRED_INDEX_LOCK_TIMEOUT = "60s"; + +const LOCK_TIMEOUT_VALUE = /^\d{1,6}(ms|s)$/; + /** The only shape an index name may have before it is spliced into SQL. */ const INDEX_NAME = /^[a-z0-9_]{1,63}$/; @@ -162,7 +187,7 @@ const CONCURRENT_COPY = /_ccnew\d*$|_ccold\d*$/; const PURGE_QUEUE = "compaction-tombstone-purge"; export interface MaintenanceStep { - step: "vacuum" | "reindex" | "drop_invalid"; + step: "vacuum" | "reindex" | "drop_invalid" | "drop_retired"; target: string; bytesBefore: number; bytesAfter: number; @@ -336,6 +361,62 @@ async function dropInvalidCopies( } } +/** + * Drop every retired index still present. `DROP INDEX CONCURRENTLY` takes no + * lock that stops reads or writes, but it waits for every transaction that + * may use the index, so the wait is bounded for the statement and the + * session's own setting is put back after it. A timed-out drop may leave the + * index marked invalid; the next run's `IF EXISTS` drop finishes it. + */ +export async function dropRetiredIndexes( + db: MaintenanceConnection, + steps: MaintenanceStep[], + lockTimeout: string = RETIRED_INDEX_LOCK_TIMEOUT, +): Promise { + if (!LOCK_TIMEOUT_VALUE.test(lockTimeout)) { + throw new Error(`refusing lock timeout of unexpected shape`); + } + for (const name of RETIRED_MEASUREMENT_INDEXES) { + const bytesBefore = await indexBytes(db, name); + const { rows } = await db.query<{ present: boolean }>( + `SELECT to_regclass($1) IS NOT NULL AS present`, + [name], + ); + if (!rows[0]?.present) continue; + const { rows: setting } = await db.query<{ value: string }>( + `SELECT current_setting('lock_timeout') AS value`, + ); + const startedAt = Date.now(); + try { + await db.query(`SET lock_timeout = '${lockTimeout}'`); + await db.query(`DROP INDEX CONCURRENTLY IF EXISTS ${quoted(name)}`); + steps.push({ + step: "drop_retired", + target: name, + bytesBefore, + bytesAfter: 0, + durationMs: Date.now() - startedAt, + ok: true, + }); + } catch (err) { + logCaught("measurements.maintenance.drop_retired_failed", err); + steps.push({ + step: "drop_retired", + target: name, + bytesBefore, + bytesAfter: await indexBytes(db, name), + durationMs: Date.now() - startedAt, + ok: false, + error: errorText(err), + }); + } finally { + await db.query(`SELECT set_config('lock_timeout', $1, false)`, [ + setting[0]?.value ?? "0", + ]); + } + } +} + /** * The pass itself, on a connection the caller opened with * {@link MAINTENANCE_SESSION_OPTIONS}. Exported for the integration test. @@ -343,7 +424,7 @@ async function dropInvalidCopies( export async function runMeasurementMaintenance( db: MaintenanceConnection, payload: MaintenanceSteps, - options: { shouldStop?: () => boolean } = {}, + options: { shouldStop?: () => boolean; retiredLockTimeout?: string } = {}, ): Promise { const before = await relationSizes(db); const steps: MaintenanceStep[] = []; @@ -359,6 +440,10 @@ export async function runMeasurementMaintenance( indexBytesAfter: after.indexes, }); + // Every run, the nightly tick included: a retired index costs every write + // until it is gone, and the drop blocks no reader or writer. + await dropRetiredIndexes(db, steps, options.retiredLockTimeout); + if (!payload.vacuum && !payload.reindex) return summary("nothing_due"); // The queue table exists wherever a worker has started; a database without @@ -406,7 +491,10 @@ export async function runMeasurementMaintenance( if (payload.reindex) { await dropInvalidCopies(db, steps); const targets = (await measurementIndexes(db)).filter( - (index) => index.valid && !CONCURRENT_COPY.test(index.name), + (index) => + index.valid && + !CONCURRENT_COPY.test(index.name) && + !RETIRED_MEASUREMENT_INDEXES.includes(index.name), ); for (const index of targets) { if (options.shouldStop?.()) break; @@ -509,6 +597,15 @@ export async function handleMeasurementMaintenance( ); } } + const retiredLeft = result.steps.filter( + (s) => s.step === "drop_retired" && !s.ok, + ); + if (retiredLeft.length > 0) { + evt.addMeta("maintenance_retired_index_pending", true); + evt.addWarning( + `retired index ${retiredLeft.map((s) => s.target).join(", ")} could not be dropped within ${RETIRED_INDEX_LOCK_TIMEOUT} (a long transaction on measurements); the next nightly run tries again`, + ); + } evt.addMeta("maintenance_table_bytes_before", result.tableBytesBefore); evt.addMeta("maintenance_table_bytes_after", result.tableBytesAfter); evt.addMeta("maintenance_index_bytes_before", result.indexBytesBefore); diff --git a/tests/integration/retired-index-drop.test.ts b/tests/integration/retired-index-drop.test.ts new file mode 100644 index 000000000..4ac6bb800 --- /dev/null +++ b/tests/integration/retired-index-drop.test.ts @@ -0,0 +1,140 @@ +/** + * Retiring an index on `measurements` without the boot waiting on the table. + * + * Migration 0416 drops `measurements_user_id_type_measured_at_idx` only when + * the lock is there at once and otherwise leaves it, successfully; the + * nightly measurement maintenance drops what is left with + * `DROP INDEX CONCURRENTLY IF EXISTS` under a bounded lock wait. Proven here + * against Postgres, each with the transaction that used to hold the boot: + * the migration finishes while a reader holds the table and drops the index + * once it does not; the maintenance step gives up within its wait, puts the + * session's lock timeout back, and drops the index on the next run. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { Client } from "pg"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + MAINTENANCE_SESSION_OPTIONS, + RETIRED_MEASUREMENT_INDEXES, + runMeasurementMaintenance, +} from "@/lib/jobs/measurement-maintenance"; + +const INDEX = "measurements_user_id_type_measured_at_idx"; +const MIGRATION = readFileSync( + join( + process.cwd(), + "prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql", + ), + "utf8", +); + +function connect(options?: string): Client { + const url = new URL(process.env.DATABASE_URL ?? ""); + url.searchParams.delete("schema"); + return new Client({ connectionString: url.toString(), options }); +} + +async function present(db: Client): Promise { + const { rows } = await db.query<{ present: boolean }>( + `SELECT to_regclass($1) IS NOT NULL AS present`, + [INDEX], + ); + return rows[0].present; +} + +let db: Client; +let holder: Client; + +beforeEach(async () => { + db = connect(MAINTENANCE_SESSION_OPTIONS); + holder = connect(); + await db.connect(); + await holder.connect(); + // The state on an instance that has not run 0416 yet. + await db.query( + `CREATE INDEX IF NOT EXISTS "${INDEX}" ON "measurements" ("user_id", "type", "measured_at")`, + ); +}); + +afterEach(async () => { + await holder.query("ROLLBACK").catch(() => {}); + await holder.end(); + await db.query(`DROP INDEX IF EXISTS "${INDEX}"`); + await db.end(); +}); + +describe("migration 0416", () => { + it("finishes while a reader holds the table and leaves the index", async () => { + await holder.query("BEGIN"); + await holder.query(`SELECT count(*) FROM "measurements"`); + + const startedAt = Date.now(); + await db.query(MIGRATION); + + expect(Date.now() - startedAt).toBeLessThan(10_000); + expect(await present(db)).toBe(true); + }); + + it("counter-check: drops the index when nothing holds the table", async () => { + await db.query(MIGRATION); + expect(await present(db)).toBe(false); + // And again, as a retried boot would. + await db.query(MIGRATION); + }); +}); + +describe("the maintenance step for a retired index", () => { + it("names the index the migration retires", () => { + expect(RETIRED_MEASUREMENT_INDEXES).toContain(INDEX); + }); + + it("gives up within its wait, restores the session, and drops it on the next run", async () => { + await holder.query("BEGIN"); + await holder.query( + `LOCK TABLE "measurements" IN SHARE UPDATE EXCLUSIVE MODE`, + ); + + const blocked = await runMeasurementMaintenance( + db, + { vacuum: false, reindex: false }, + { retiredLockTimeout: "300ms" }, + ); + expect(blocked.outcome).toBe("nothing_due"); + expect(blocked.steps).toEqual([ + expect.objectContaining({ + step: "drop_retired", + target: INDEX, + ok: false, + error: expect.stringContaining("55P03"), + }), + ]); + // The session's own wait (five minutes) is back for the rest of the run. + const { rows } = await db.query<{ value: string }>( + `SELECT current_setting('lock_timeout') AS value`, + ); + expect(rows[0].value).toBe("5min"); + + await holder.query("ROLLBACK"); + const next = await runMeasurementMaintenance(db, { + vacuum: false, + reindex: false, + }); + expect(next.steps).toEqual([ + expect.objectContaining({ + step: "drop_retired", + target: INDEX, + ok: true, + }), + ]); + expect(await present(db)).toBe(false); + + // Gone: the run after that has nothing to do. + const after = await runMeasurementMaintenance(db, { + vacuum: false, + reindex: false, + }); + expect(after.steps).toEqual([]); + }); +}); From 408ae35b7c4566afbdd29bfd4f4a65426112ecda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:06:10 +0200 Subject: [PATCH 04/13] fix(migrations): retry a marked migration that lost a deadlock A marked migration's DO block holds the locks it takes until it commits, so an app transaction taking two of its tables in the other order can deadlock with it. The block then rolled back whole, exactly as on a lock timeout, so the boot retries 40P01 for marked migrations as it does 55P03. Corrects 0415's note that the drops hold their lock for an instant. --- docs/ops/deploy.md | 5 ++ .../migration.sql | 20 ++++--- scripts/__tests__/migrate-deploy.test.ts | 60 ++++++++++++++++++- scripts/migrate-deploy.mjs | 45 ++++++++++---- 4 files changed, 108 insertions(+), 22 deletions(-) diff --git a/docs/ops/deploy.md b/docs/ops/deploy.md index d4a828b9e..c87d7926b 100644 --- a/docs/ops/deploy.md +++ b/docs/ops/deploy.md @@ -68,6 +68,11 @@ Database error code: 55P03 HealthLog: migration 0412_measurement_hot_updates timed out waiting for a lock (55P03) and changed nothing; marking it rolled back and retrying in 15s (retry 1 of 5). ``` +A marked migration that loses a deadlock against an app transaction +(`Database error code: 40P01`) is in the same state, its block rolled back +whole, and is handled the same way; the entrypoint's line then says +`was cancelled in a deadlock (40P01)`. + What happens on its own: such a migration is written so that a timed-out attempt changes nothing, and it says so in its file (`-- @retry-on-lock-timeout`). The entrypoint then marks that one migration diff --git a/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql b/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql index 1844f11a7..eb7fb4676 100644 --- a/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql +++ b/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql @@ -20,13 +20,19 @@ -- -- Lock wait. `CREATE INDEX` takes SHARE on its table (reads go on, writes -- wait while it builds; every table here is small, so the build is quick) --- and `DROP INDEX` takes ACCESS EXCLUSIVE for the instant the catalog --- changes. `CREATE INDEX CONCURRENTLY` cannot share a migration file with --- any other statement (0416), so the whole set runs in one `DO` block: one --- statement that commits or rolls back whole, with the lock wait bounded at --- five seconds so a long reader cannot queue the tables' writes behind the --- migration. Every statement is `IF [NOT] EXISTS`, so a lock timeout leaves --- the file safe to apply again, which the marker declares +-- and `DROP INDEX` takes ACCESS EXCLUSIVE. `CREATE INDEX CONCURRENTLY` +-- cannot share a migration file with any other statement, so the whole set +-- runs in one `DO` block: one statement that commits or rolls back whole, +-- with each lock wait bounded at five seconds so a long reader cannot queue +-- the tables' writes behind the migration. Inside the block every lock is +-- held until it commits, not just while its statement runs: the SHARE locks +-- of the builds stay through the drops, and the drops come last so their +-- ACCESS EXCLUSIVE locks are held only for the rest of the block. An app +-- transaction that takes two of these tables in the other order can +-- therefore deadlock with it; Postgres cancels one side, and when that is the +-- migration its block has rolled back whole, as on a lock timeout. Every +-- statement is `IF [NOT] EXISTS`, so either failure leaves the file safe to +-- apply again, which the marker declares and the boot acts on for both -- (docs/ops/deploy.md, "A migration that timed out on a lock"). -- -- @retry-on-lock-timeout diff --git a/scripts/__tests__/migrate-deploy.test.ts b/scripts/__tests__/migrate-deploy.test.ts index 46b7b7915..f1e776340 100644 --- a/scripts/__tests__/migrate-deploy.test.ts +++ b/scripts/__tests__/migrate-deploy.test.ts @@ -47,6 +47,11 @@ const P3018_UNIQUE = P3018_LOCK.replace("55P03", "23505").replace( 'duplicate key value violates unique constraint "x"', ); +const P3018_DEADLOCK = P3018_LOCK.replace("55P03", "40P01").replace( + "canceling statement due to lock timeout", + "deadlock detected", +); + const P3009 = `Error: P3009 migrate found failed migrations in the target database, new migrations will not be applied. Read more about how to resolve migration issues in a production database: https://pris.ly/d/migrate-resolve @@ -78,7 +83,11 @@ describe("which failure is retried", () => { failedRows: null, readMigration: reader(MARKED), }), - ).toEqual({ retry: true, migration: "0412_measurement_hot_updates" }); + ).toEqual({ + retry: true, + migration: "0412_measurement_hot_updates", + code: "55P03", + }); }); it("does not retry a lock timeout of an unmarked migration", () => { @@ -93,6 +102,31 @@ describe("which failure is retried", () => { }); }); + it("retries a deadlock of a marked migration, which rolled back whole", () => { + expect(lockTimeoutInReport(P3018_DEADLOCK)).toBe( + "0412_measurement_hot_updates", + ); + expect( + decideLockRetry({ + output: P3018_DEADLOCK, + failedRows: null, + readMigration: reader(MARKED), + }), + ).toEqual({ + retry: true, + migration: "0412_measurement_hot_updates", + code: "40P01", + }); + // Unmarked, a deadlock stays fatal like a lock timeout. + expect( + decideLockRetry({ + output: P3018_DEADLOCK, + failedRows: null, + readMigration: reader(UNMARKED), + }).retry, + ).toBe(false); + }); + it("does not retry any other database error", () => { expect( decideLockRetry({ @@ -123,7 +157,11 @@ describe("which failure is retried", () => { failedRows, readMigration: reader(MARKED), }), - ).toEqual({ retry: true, migration: "0412_measurement_hot_updates" }); + ).toEqual({ + retry: true, + migration: "0412_measurement_hot_updates", + code: "55P03", + }); // A stored failure of another kind stays fatal. expect( decideLockRetry({ @@ -295,6 +333,24 @@ process.exit(0); expect(result.stdout).toContain("timed out waiting for a lock (55P03)"); }); + it("does the same for a deadlock, and names it", () => { + const { run, calls } = setup( + [ + [1, P3018_DEADLOCK], + [0, "All migrations have been successfully applied."], + ], + MARKED, + ); + const result = run(); + expect(result.status).toBe(0); + expect(calls()).toEqual([ + "migrate deploy --schema x", + "migrate resolve --rolled-back 0412_measurement_hot_updates --schema x", + "migrate deploy --schema x", + ]); + expect(result.stdout).toContain("was cancelled in a deadlock (40P01)"); + }); + it("gives up after the last delay with Prisma's exit code", () => { const { run, calls } = setup( [ diff --git a/scripts/migrate-deploy.mjs b/scripts/migrate-deploy.mjs index 4f06202c2..d6f9f8973 100644 --- a/scripts/migrate-deploy.mjs +++ b/scripts/migrate-deploy.mjs @@ -21,9 +21,15 @@ * back whole. `src/__tests__/migration-lock-timeout-guard.test.ts` holds the * migrations to that. * - * So: on a 55P03 failure of a migration carrying the marker, mark that one - * migration rolled back and run `migrate deploy` again after a backoff (15, - * 30, 60, 120, 240 seconds). Any other failure, a lock timeout of a migration + * A deadlock (40P01) is the same failure seen from the other side: a marked + * migration's `DO` block holds the locks it took until it commits, and an app + * transaction that took two of the same tables in the other order makes + * Postgres cancel one of the two. When the migration is the one cancelled, + * its block rolled back whole, exactly as on a lock timeout. + * + * So: on a 55P03 or 40P01 failure of a migration carrying the marker, mark + * that one migration rolled back and run `migrate deploy` again after a + * backoff (15, 30, 60, 120, 240 seconds). Any other failure, a lock timeout of a migration * without the marker, or a fifth lock timeout in a row, ends the boot with * Prisma's exit code exactly as before. The failure is read from Prisma's * own report (`Migration name:` and `Database error code:`), or, on a boot @@ -58,20 +64,27 @@ const DEFAULT_RETRY_DELAYS_S = [15, 30, 60, 120, 240]; /** The line a migration carries when a lock-timeout retry is safe for it. */ export const RETRY_MARKER = /^--\s*@retry-on-lock-timeout\s*$/m; -const LOCK_TIMEOUT = "55P03"; +/** Lock timeout and deadlock: both leave a marked migration unapplied. */ +const LOCK_FAILURES = new Set(["55P03", "40P01"]); const TIME_ZONE_OPTION = "-c TimeZone=UTC"; const MIGRATION_NAME = /^[A-Za-z0-9_]+$/; +/** The SQLSTATE a P3018 report gives, or null. */ +export function errorCodeInReport(text) { + if (typeof text !== "string") return null; + return /^Database error code: (\S+)\s*$/m.exec(text)?.[1] ?? null; +} + /** - * The migration a P3018 report says failed on a lock timeout, or null. - * Prisma prints `Migration name: ` and `Database error code: ` - * for a migration that failed to apply. + * The migration a P3018 report says failed on a lock (a lock timeout or a + * deadlock), or null. Prisma prints `Migration name: ` and + * `Database error code: ` for a migration that failed to apply. */ export function lockTimeoutInReport(text) { if (typeof text !== "string") return null; const name = /^Migration name: (\S+)\s*$/m.exec(text)?.[1]; - const code = /^Database error code: (\S+)\s*$/m.exec(text)?.[1]; - return name && code === LOCK_TIMEOUT ? name : null; + const code = errorCodeInReport(text); + return name && code !== null && LOCK_FAILURES.has(code) ? name : null; } /** Whether Prisma refused to start because a failed migration is on record. */ @@ -89,14 +102,16 @@ function isFailedMigrationOnRecord(output) { * when they were not read * @param {(name: string) => string | null} input.readMigration * the migration's SQL, or null when the image does not ship it - * @returns {{ retry: true, migration: string } | { retry: false, reason: string }} + * @returns {{ retry: true, migration: string, code: string } | { retry: false, reason: string }} */ export function decideLockRetry({ output, failedRows, readMigration }) { let migration = lockTimeoutInReport(output); + let code = migration ? errorCodeInReport(output) : null; if (!migration && isFailedMigrationOnRecord(output) && failedRows) { // More than one failed row is not a state this boot can reason about. if (failedRows.length === 1) { migration = lockTimeoutInReport(failedRows[0].logs); + code = migration ? errorCodeInReport(failedRows[0].logs) : null; } } if (!migration) { @@ -115,7 +130,7 @@ export function decideLockRetry({ output, failedRows, readMigration }) { reason: `${migration} timed out on a lock but is not marked safe to re-run`, }; } - return { retry: true, migration }; + return { retry: true, migration, code }; } /** The backoff schedule, from the environment or the default. */ @@ -272,9 +287,13 @@ async function main({ return deploy.code; } const delay = delays[attempt]; + const cause = + decision.code === "40P01" + ? "was cancelled in a deadlock (40P01)" + : "timed out waiting for a lock (55P03)"; console.log( - `HealthLog: migration ${decision.migration} timed out waiting for a lock ` + - `(55P03) and changed nothing; marking it rolled back and retrying in ` + + `HealthLog: migration ${decision.migration} ${cause} ` + + `and changed nothing; marking it rolled back and retrying in ` + `${delay}s (retry ${attempt + 1} of ${delays.length}).`, ); const resolve = await runPrisma(cli, [ From 5185edca749647055e9a06d453e6b0932722b2ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:09:28 +0200 Subject: [PATCH 05/13] fix(rate-limit): fail the shared write ceiling open and skip reads over POST The shared write bucket threw on any SQL error, so a lock on rate_limits answered every authenticated write with 500. It is a loop cutter, not an access check: when the bucket cannot be read the write now goes ahead with a warn signal. The login and other auth-surface limits call the limiter directly and still fail closed. requireAuth now passes readOverPost to the meter, the draft-only document assist declares it, and the three chat routes, which meter their turns with buckets of their own, declare that instead of being counted twice. --- docs/api/openapi.yaml | 17 +++--- src/__tests__/read-over-post-guard.test.ts | 14 ++++- src/__tests__/write-rate-limit-guard.test.ts | 6 ++ .../api/documents/inbound/[id]/chat/route.ts | 4 +- .../documents/inbound/[id]/suggest/route.ts | 4 +- src/app/api/insights/chat/fenced/route.ts | 3 +- src/app/api/insights/chat/route.ts | 4 +- src/lib/api-handler.ts | 2 +- src/lib/auth/__tests__/write-meter.test.ts | 30 ++++++++++ src/lib/auth/write-meter.ts | 22 ++++++- src/lib/openapi/routes/shared.ts | 2 +- tests/integration/write-meter.test.ts | 59 ++++++++++++++++++- 12 files changed, 148 insertions(+), 19 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 06661dca7..795ae7379 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -398,14 +398,15 @@ paths: "429": &a12 description: "Rate limit exceeded. This route shares one per-account bucket with every other authenticated write — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns - their own allowance rather than the record owner's. Only the bulk ingest endpoints, which meter themselves - with a dedicated bucket, stay outside it. Nothing was written. `meta.errorCode` is - `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when - both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` - headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against - the bucket, refused ones included: the check runs before the body is read, so a client looping on a 422 - spends the same allowance as one that writes rows. A client with more than a handful of rows to send should - use the batch endpoint for its domain instead of looping this one." + their own allowance rather than the record owner's. Only the routes that meter themselves with a dedicated + bucket (the bulk ingest endpoints and the model-turn chats) and a POST that writes nothing stay outside it. + If the bucket cannot be read, the write goes ahead unmetered rather than failing. Nothing was written. + `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own + narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` + and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every + request counts against the bucket, refused ones included: the check runs before the body is read, so a + client looping on a 422 spends the same allowance as one that writes rows. A client with more than a handful + of rows to send should use the batch endpoint for its domain instead of looping this one." headers: *a8 content: application/json: diff --git a/src/__tests__/read-over-post-guard.test.ts b/src/__tests__/read-over-post-guard.test.ts index c699a9ad4..d118df810 100644 --- a/src/__tests__/read-over-post-guard.test.ts +++ b/src/__tests__/read-over-post-guard.test.ts @@ -2,8 +2,10 @@ * `readOverPost` lets a delegable route answer a POST at the grant's read * level. That widens what a read-only grant reaches, so the set of routes * declaring it is frozen here: adding one is a reviewed diff, not an - * accident. Today it is the record search alone, whose query is health data - * that must stay out of URLs. + * accident. Today it is the record search, whose query is health data that + * must stay out of URLs, and the document assist, whose POST writes nothing + * (under `requireAuth` the declaration only keeps it off the shared write + * ceiling). * * The matcher tolerates whitespace and line breaks, and the guard asserts it * matched something, so a rewrite that hides the declaration from the regex @@ -16,7 +18,13 @@ import { join, relative, sep } from "node:path"; const ROOT = process.cwd(); -const FROZEN = ["src/app/api/search/route.ts"]; +const FROZEN = [ + // The draft-only document assist: a POST that writes nothing. Under + // `requireAuth` the declaration only keeps it out of the shared write + // ceiling; there is no grant for it to widen. + "src/app/api/documents/inbound/[id]/suggest/route.ts", + "src/app/api/search/route.ts", +]; describe("readOverPost declarations", () => { it("are exactly the frozen set", () => { diff --git a/src/__tests__/write-rate-limit-guard.test.ts b/src/__tests__/write-rate-limit-guard.test.ts index a579655ee..3b3b3559b 100644 --- a/src/__tests__/write-rate-limit-guard.test.ts +++ b/src/__tests__/write-rate-limit-guard.test.ts @@ -75,6 +75,12 @@ const OWN_BUCKET: Record = { "The cycle HealthKit sweep sends 500-day slices in sequence; the route's own bulk bucket meters it.", "app/api/nutrients/batch/route.ts": "The nutrient sync sends 500-row batches in sequence; the route's own batch bucket meters it.", + "app/api/insights/chat/route.ts": + "A Coach turn is metered by the Coach's own buckets: 20 turns a minute (`coach-chat`) and one turn slot at a time.", + "app/api/insights/chat/fenced/route.ts": + "A fenced document chat turn is metered by its own per-account bucket, sized for model turns.", + "app/api/documents/inbound/[id]/chat/route.ts": + "A document chat turn is metered by its own per-account bucket, sized for model turns.", "app/api/insights/ecg/route.ts": "The ECG sweep sends one recording per request with no client-side delay, newest first, over the whole HealthKit history; the route's own 600-a-minute ingest bucket meters it.", }; diff --git a/src/app/api/documents/inbound/[id]/chat/route.ts b/src/app/api/documents/inbound/[id]/chat/route.ts index d37ff416e..49a24fdd7 100644 --- a/src/app/api/documents/inbound/[id]/chat/route.ts +++ b/src/app/api/documents/inbound/[id]/chat/route.ts @@ -71,7 +71,9 @@ type RouteParams = { params: Promise<{ id: string }> }; export const POST = apiHandler( async (request: NextRequest, { params }: RouteParams) => { - const { user } = await requireAuth(); + // Metered by the document chat's own bucket below, not the shared + // ceiling. + const { user } = await requireAuth(undefined, { writeBucket: "own" }); const userId = user.id; const gate = await requireModuleEnabled(userId, "inboundDocuments"); diff --git a/src/app/api/documents/inbound/[id]/suggest/route.ts b/src/app/api/documents/inbound/[id]/suggest/route.ts index da9d78fcc..47371f779 100644 --- a/src/app/api/documents/inbound/[id]/suggest/route.ts +++ b/src/app/api/documents/inbound/[id]/suggest/route.ts @@ -73,7 +73,9 @@ type RouteParams = { params: Promise<{ id: string }> }; export const POST = apiHandler( async (request: NextRequest, { params }: RouteParams) => { - const { user } = await requireAuth(); + // Writes nothing (the draft goes back to the edit form), so the shared + // write ceiling does not count it; the document-AI bucket meters it. + const { user } = await requireAuth(undefined, { readOverPost: true }); const gate = await requireModuleEnabled(user.id, "inboundDocuments"); if (!gate.enabled) return gate.response; diff --git a/src/app/api/insights/chat/fenced/route.ts b/src/app/api/insights/chat/fenced/route.ts index 1276a96de..66c5f1dc6 100644 --- a/src/app/api/insights/chat/fenced/route.ts +++ b/src/app/api/insights/chat/fenced/route.ts @@ -52,7 +52,8 @@ export const dynamic = "force-dynamic"; export const runtime = "nodejs"; export const POST = apiHandler(async (request: NextRequest) => { - const { user } = await requireAuth(); + // Metered by the fenced chat's own bucket below, not the shared ceiling. + const { user } = await requireAuth(undefined, { writeBucket: "own" }); const userId = user.id; const gate = await requireModuleEnabled(userId, "inboundDocuments"); diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index 3a3260f7a..5b8134950 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -81,7 +81,9 @@ const COACH_CONCURRENT_TURNS = 2; const COACH_TURN_SLOT_MS = TURN_LIMITS.user.wallMs + 30_000; async function handleChatRequest(request: NextRequest): Promise { - const auth = await requireAuth(); + // A turn is metered by the Coach's own buckets (`coach-chat`, the turn + // slot), not the shared write ceiling. + const auth = await requireAuth(undefined, { writeBucket: "own" }); // The `coach` capability, right after auth and before the rate limit and // the body parse. It folds in what the Coach module gate, the operator // switch and the `disableCoach` opt-out used to answer separately, plus diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 8166b60d5..5f0f9c9fa 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -546,7 +546,7 @@ export function isScopedCredential(auth: AuthContext): boolean { */ export async function requireAuth( requiredPermission?: string, - options?: Pick, + options?: WriteMeterOptions, ): Promise { const auth = await authenticateCaller(requiredPermission); const carrier = await readActingCarrier(auth); diff --git a/src/lib/auth/__tests__/write-meter.test.ts b/src/lib/auth/__tests__/write-meter.test.ts index ac6069b83..ddf381d31 100644 --- a/src/lib/auth/__tests__/write-meter.test.ts +++ b/src/lib/auth/__tests__/write-meter.test.ts @@ -16,6 +16,12 @@ vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn(), })); +const signals = vi.hoisted(() => ({ emitted: [] as unknown[] })); + +vi.mock("@/lib/logging/signal", () => ({ + emitSignal: (input: unknown) => signals.emitted.push(input), +})); + vi.mock("@/lib/rate-limit", () => ({ checkRecordWriteRateLimit: vi.fn(async () => ({ allowed: state.allowed, @@ -38,6 +44,7 @@ function request(method: string | undefined) { beforeEach(() => { vi.mocked(checkRecordWriteRateLimit).mockClear(); + signals.emitted.length = 0; state.allowed = true; state.event = null; }); @@ -99,4 +106,27 @@ describe("meterAccountWrite", () => { RECORD_WRITE_RATE_LIMITED_CODE, ); }); + + it("lets the write through with a warning when the bucket cannot be read", async () => { + vi.mocked(checkRecordWriteRateLimit).mockRejectedValueOnce( + new Error("canceling statement due to lock timeout"), + ); + request("POST"); + await expect(meterAccountWrite("account-1")).resolves.toBeUndefined(); + expect(signals.emitted).toEqual([ + expect.objectContaining({ + action: "rate_limit.record_write.unavailable", + level: "warn", + }), + ]); + }); + + it("counter-check: a readable, spent bucket still refuses, with no warning", async () => { + state.allowed = false; + request("POST"); + await expect(meterAccountWrite("account-1")).rejects.toBeInstanceOf( + HttpError, + ); + expect(signals.emitted).toEqual([]); + }); }); diff --git a/src/lib/auth/write-meter.ts b/src/lib/auth/write-meter.ts index 25c4a1895..8743739eb 100644 --- a/src/lib/auth/write-meter.ts +++ b/src/lib/auth/write-meter.ts @@ -31,9 +31,18 @@ * record owner's, and cannot collect a fresh one by switching records. A * refused request is counted too, since the charge happens before the body is * read. + * + * The bucket fails open. It cuts off a runaway loop; it is not what decides + * who may write, which the resolver that called it has already settled. So + * when the bucket itself cannot be read (the upsert fails: a lock on + * `rate_limits`, a pool at its limit), the write goes ahead and a `warn` + * signal says the ceiling was not applied, instead of every authenticated + * write in the app answering 500. The login, sign-up and other auth-surface + * limits call the limiter directly and keep failing closed. */ import { HttpError } from "@/lib/api-errors"; import { annotate, getEvent } from "@/lib/logging/context"; +import { emitSignal } from "@/lib/logging/signal"; import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; /** The code a refusal by the shared write bucket carries. */ @@ -76,7 +85,18 @@ export async function meterAccountWrite( if (charged.has(evt)) return; charged.add(evt); - const rl = await checkRecordWriteRateLimit(accountId); + let rl: Awaited>; + try { + rl = await checkRecordWriteRateLimit(accountId); + } catch (error) { + annotate({ meta: { record_write_rate_limit_unavailable: true } }); + emitSignal({ + action: "rate_limit.record_write.unavailable", + level: "warn", + error, + }); + return; + } if (!rl.allowed) { annotate({ meta: { record_write_rate_limited: true } }); throw new HttpError( diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index afa7f2533..1e929b18e 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -373,7 +373,7 @@ export const stdResponses = { export const recordWriteRateLimitResponse = { "429": { description: - "Rate limit exceeded. This route shares one per-account bucket with every other authenticated write — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns their own allowance rather than the record owner's. Only the bulk ingest endpoints, which meter themselves with a dedicated bucket, stay outside it. Nothing was written. `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against the bucket, refused ones included: the check runs before the body is read, so a client looping on a 422 spends the same allowance as one that writes rows. A client with more than a handful of rows to send should use the batch endpoint for its domain instead of looping this one.", + "Rate limit exceeded. This route shares one per-account bucket with every other authenticated write — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns their own allowance rather than the record owner's. Only the routes that meter themselves with a dedicated bucket (the bulk ingest endpoints and the model-turn chats) and a POST that writes nothing stay outside it. If the bucket cannot be read, the write goes ahead unmetered rather than failing. Nothing was written. `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against the bucket, refused ones included: the check runs before the body is read, so a client looping on a 422 spends the same allowance as one that writes rows. A client with more than a handful of rows to send should use the batch endpoint for its domain instead of looping this one.", content: { "application/json": { schema: errorEnvelope } }, headers: stdResponses["429"].headers, }, diff --git a/tests/integration/write-meter.test.ts b/tests/integration/write-meter.test.ts index e72265dbf..cd945a9c1 100644 --- a/tests/integration/write-meter.test.ts +++ b/tests/integration/write-meter.test.ts @@ -6,7 +6,9 @@ * before. What has to hold: a write is counted, a read is not, a spent bucket * refuses with the published code and the headers that describe it and * writes nothing, and a bulk route on its own bucket keeps working while the - * shared one is spent. + * shared one is spent. A POST that reads (`readOverPost`) is not counted on + * either resolver, and a bucket that cannot be read lets the write through + * while the limiter itself still fails for its other callers. */ import { NextRequest } from "next/server"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -183,3 +185,58 @@ describe("the shared write bucket on routes without a limiter of their own", () ).toBe(1); }); }); + +describe("what the shared bucket does not count", () => { + async function postThrough(options?: { readOverPost: true }) { + const { apiHandler, requireAuth } = await import("@/lib/api-handler"); + const { apiSuccess } = await import("@/lib/api-response"); + const POST = apiHandler(async (_request: NextRequest) => { + const { user } = await requireAuth(undefined, options); + return apiSuccess({ id: user.id }); + }); + return POST( + new NextRequest("http://localhost/api/probe", { + method: "POST", + body: "{}", + headers: { "content-type": "application/json" }, + }), + ); + } + + it("leaves a read-over-POST through requireAuth uncounted", async () => { + expect((await postThrough({ readOverPost: true })).status).toBe(200); + expect(await bucketCount()).toBe(0); + }); + + it("counter-check: the same POST without the declaration is counted", async () => { + expect((await postThrough()).status).toBe(200); + expect(await bucketCount()).toBe(1); + }); +}); + +describe("a bucket that cannot be read", () => { + it("lets the write through instead of answering 500, and the limiter still fails for its own callers", async () => { + const prisma = getPrismaClient(); + const { checkRateLimit } = await import("@/lib/rate-limit"); + await prisma.$executeRawUnsafe( + `ALTER TABLE "rate_limits" RENAME TO "rate_limits_away"`, + ); + try { + const res = await patchAllergy({ substance: "Birch pollen" }); + expect(res.status, await res.clone().text()).toBe(200); + const row = await prisma.allergy.findUniqueOrThrow({ + where: { id: allergyId }, + }); + expect(row.substance).toBe("Birch pollen"); + + // A login or sign-up limit calls the limiter directly: no fail-open. + await expect( + checkRateLimit("auth:login:probe", 5, 60_000), + ).rejects.toThrow(); + } finally { + await prisma.$executeRawUnsafe( + `ALTER TABLE "rate_limits_away" RENAME TO "rate_limits"`, + ); + } + }); +}); From a37d45ab3fb7181419e8fe870ea71647839f98c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 16:16:13 +0200 Subject: [PATCH 06/13] fix(insights): take the crosstab units from the shared unit table The cycle, mood-tag and mood-factor boards each kept a table of unit words, and the German cycle board printed S/min where every other screen says bpm. A board row now names the measurement it reads; MetricCrosstab resolves the unit through measurementUnitLabel after the reader's unit preference, and the callers use the same reading for their detail line. A rating or score shows no unit word, as everywhere else. The boards' own unit keys are gone from all bundles, and unit-label.test.ts refuses a component that keeps a unit table again. The pair label wraps instead of being cut off. --- messages/de.json | 35 ++--- messages/en.json | 35 ++--- messages/es.json | 35 ++--- messages/fr.json | 35 ++--- messages/it.json | 35 ++--- messages/ko.json | 35 ++--- messages/nl.json | 35 ++--- messages/pl.json | 35 ++--- src/__tests__/helpers/metric-unit-display.ts | 24 ++++ .../unit-preference-display-guard.test.ts | 10 +- .../__tests__/cycle-phase-crosstab.test.tsx | 4 +- src/components/cycle/cycle-phase-crosstab.tsx | 117 ++++++----------- .../__tests__/metric-crosstab.test.tsx | 37 +++++- src/components/insights/metric-crosstab.tsx | 123 ++++++++++++++++-- .../mood-tag-metric-crosstab.test.tsx | 8 +- .../mood/mood-factor-metric-crosstab.tsx | 55 ++++---- .../mood/mood-tag-metric-crosstab.tsx | 43 ++++-- .../measurements/__tests__/unit-label.test.ts | 62 +++++++++ 18 files changed, 426 insertions(+), 337 deletions(-) create mode 100644 src/__tests__/helpers/metric-unit-display.ts diff --git a/messages/de.json b/messages/de.json index 3aff7d309..1c92cceec 100644 --- a/messages/de.json +++ b/messages/de.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Erholung am Folgetag", "metricNextDayRestingHr": "Ruhepuls am Folgetag", "metricNextDayHrv": "Herzfrequenzvariabilität am Folgetag", - "unitHours": "Std.", - "unitKcal": "kcal", - "unitScore": "Pkt.", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "Durchschnittlich {withAvg} {unit} an den {withDays} Tagen mit diesem Tag gegenüber {withoutAvg} {unit} ohne.", - "detailNextDay": "Durchschnittlich {withAvg} {unit} am Tag nach den {withDays} Tagen mit diesem Tag gegenüber {withoutAvg} {unit} sonst." + "detailSameDay": "Durchschnittlich {withAvg} an den {withDays} Tagen mit diesem Tag gegenüber {withoutAvg} ohne.", + "detailNextDay": "Durchschnittlich {withAvg} am Tag nach den {withDays} Tagen mit diesem Tag gegenüber {withoutAvg} sonst." }, "factorCrosstab": { "title": "Deine bewerteten Faktoren und deine Vitalwerte", @@ -4239,15 +4234,8 @@ "metricHrv": "Herzfrequenzvariabilität", "metricWeight": "Gewicht", "metricBloodPressureSys": "Systolisch", - "unitHours": "Std.", - "unitScore": "Pkt.", - "unitSteps": "Schritte", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "Durchschnittlich {lowAvg} {unit} an den {lowDays} Tagen mit niedriger {factor}-Bewertung gegenüber {highAvg} {unit} an den Tagen mit hoher Bewertung.", - "detailInverse": "Durchschnittlich {lowAvg} {unit} an deinen {lowDays} schlechteren {factor}-Tagen gegenüber {highAvg} {unit} an deinen besseren." + "detail": "Durchschnittlich {lowAvg} an den {lowDays} Tagen mit niedriger {factor}-Bewertung gegenüber {highAvg} an den Tagen mit hoher Bewertung.", + "detailInverse": "Durchschnittlich {lowAvg} an deinen {lowDays} schlechteren {factor}-Tagen gegenüber {highAvg} an deinen besseren." }, "discovery": { "title": "Was auffällt", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Dein Zyklus und dein Körper", - "restingHeartRate": "Dein Ruhepuls liegt in der Lutealphase etwa {delta} {unit} {dir} als in der Follikelphase.", - "heartRateVariability": "Deine Herzfrequenzvariabilität liegt in der Lutealphase etwa {delta} {unit} {dir} als in der Follikelphase.", - "generic": "Dein/e {metric} liegt in der Lutealphase etwa {delta} {unit} {dir} als in der Follikelphase.", + "restingHeartRate": "Dein Ruhepuls liegt in der Lutealphase etwa {delta} {dir} als in der Follikelphase.", + "heartRateVariability": "Deine Herzfrequenzvariabilität liegt in der Lutealphase etwa {delta} {dir} als in der Follikelphase.", + "generic": "Dein/e {metric} liegt in der Lutealphase etwa {delta} {dir} als in der Follikelphase.", "dirHigher": "höher", "dirLower": "niedriger", "empty": "Noch nicht genug Zyklen und Messwerte für ein Phasenmuster. Logge weiter, das füllt sich, je länger deine Historie wird." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Wie sich jeder Messwert zwischen deiner Luteal- und Follikelphase vergleicht, auf deinen eigenen Daten.", "pairLabel": "{metric}: Luteal- vs. Follikelphase", - "detail": "Luteal {lutealAvg} {unit} ({lutealDays} Tage) vs. Follikel {follicularAvg} {unit} ({follicularDays} Tage).", + "detail": "Luteal {lutealAvg} ({lutealDays} Tage) vs. Follikel {follicularAvg} ({follicularDays} Tage).", "metricRestingHeartRate": "Ruhepuls", "metricHrv": "Herzfrequenzvariabilität", "metricSleepDuration": "Schlafdauer", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Handgelenkstemperatur nachts", "metricSkinTemperature": "Hauttemperatur", "metricBloodGlucose": "Blutzucker", - "metricMood": "Stimmung", - "unitHours": "Std.", - "unitSteps": "Schritte", - "unitBpm": "S/min", - "unitMs": "ms", - "unitMood": "Pkt." + "metricMood": "Stimmung" }, "title": "Zyklus-Erkenntnisse", "symptomPatternsTitle": "Symptommuster nach Phase", diff --git a/messages/en.json b/messages/en.json index b58302e22..20d1ac6a4 100644 --- a/messages/en.json +++ b/messages/en.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Next-day recovery", "metricNextDayRestingHr": "Next-day resting heart rate", "metricNextDayHrv": "Next-day heart-rate variability", - "unitHours": "h", - "unitKcal": "kcal", - "unitScore": "pts", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "Averages {withAvg} {unit} on the {withDays} days you tagged this vs {withoutAvg} {unit} without it.", - "detailNextDay": "Averages {withAvg} {unit} the day after the {withDays} days you tagged this vs {withoutAvg} {unit} otherwise." + "detailSameDay": "Averages {withAvg} on the {withDays} days you tagged this vs {withoutAvg} without it.", + "detailNextDay": "Averages {withAvg} the day after the {withDays} days you tagged this vs {withoutAvg} otherwise." }, "factorCrosstab": { "title": "Your rated factors and your vitals", @@ -4239,15 +4234,8 @@ "metricHrv": "Heart-rate variability", "metricWeight": "Weight", "metricBloodPressureSys": "Systolic", - "unitHours": "h", - "unitScore": "pts", - "unitSteps": "steps", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "Averages {lowAvg} {unit} on the {lowDays} days you rated {factor} low vs {highAvg} {unit} on the days you rated it high.", - "detailInverse": "Averages {lowAvg} {unit} on your {lowDays} worse {factor} days vs {highAvg} {unit} on your better ones." + "detail": "Averages {lowAvg} on the {lowDays} days you rated {factor} low vs {highAvg} on the days you rated it high.", + "detailInverse": "Averages {lowAvg} on your {lowDays} worse {factor} days vs {highAvg} on your better ones." }, "discovery": { "title": "What stands out", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Your cycle and your body", - "restingHeartRate": "Your resting heart rate runs about {delta} {unit} {dir} in your luteal phase than your follicular phase.", - "heartRateVariability": "Your heart-rate variability runs about {delta} {unit} {dir} in your luteal phase than your follicular phase.", - "generic": "Your {metric} runs about {delta} {unit} {dir} in your luteal phase than your follicular phase.", + "restingHeartRate": "Your resting heart rate runs about {delta} {dir} in your luteal phase than your follicular phase.", + "heartRateVariability": "Your heart-rate variability runs about {delta} {dir} in your luteal phase than your follicular phase.", + "generic": "Your {metric} runs about {delta} {dir} in your luteal phase than your follicular phase.", "dirHigher": "higher", "dirLower": "lower", "empty": "Not enough cycles and readings yet to surface a phase pattern. Keep logging; this fills in as your history grows." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "How each measurement compares between your luteal and follicular phases, on your own data.", "pairLabel": "{metric}: luteal vs follicular", - "detail": "Luteal {lutealAvg} {unit} ({lutealDays} days) vs follicular {follicularAvg} {unit} ({follicularDays} days).", + "detail": "Luteal {lutealAvg} ({lutealDays} days) vs follicular {follicularAvg} ({follicularDays} days).", "metricRestingHeartRate": "Resting heart rate", "metricHrv": "Heart-rate variability", "metricSleepDuration": "Sleep duration", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Overnight wrist temperature", "metricSkinTemperature": "Skin temperature", "metricBloodGlucose": "Blood glucose", - "metricMood": "Mood", - "unitHours": "h", - "unitSteps": "steps", - "unitBpm": "bpm", - "unitMs": "ms", - "unitMood": "pts" + "metricMood": "Mood" }, "title": "Cycle insights", "symptomPatternsTitle": "Symptom patterns by phase", diff --git a/messages/es.json b/messages/es.json index a682b92ab..38c7f6f1c 100644 --- a/messages/es.json +++ b/messages/es.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Recuperación al día siguiente", "metricNextDayRestingHr": "Frecuencia cardíaca en reposo al día siguiente", "metricNextDayHrv": "Variabilidad de la frecuencia cardíaca al día siguiente", - "unitHours": "h", - "unitKcal": "kcal", - "unitScore": "pts", - "unitBpm": "ppm", - "unitMs": "ms", - "detailSameDay": "Promedia {withAvg} {unit} en los {withDays} días que lo etiquetaste frente a {withoutAvg} {unit} sin él.", - "detailNextDay": "Promedia {withAvg} {unit} el día después de los {withDays} días que lo etiquetaste frente a {withoutAvg} {unit} el resto." + "detailSameDay": "Promedia {withAvg} en los {withDays} días que lo etiquetaste frente a {withoutAvg} sin él.", + "detailNextDay": "Promedia {withAvg} el día después de los {withDays} días que lo etiquetaste frente a {withoutAvg} el resto." }, "factorCrosstab": { "title": "Tus factores valorados y tus constantes", @@ -4239,15 +4234,8 @@ "metricHrv": "Variabilidad de la frecuencia cardíaca", "metricWeight": "Peso", "metricBloodPressureSys": "Sistólica", - "unitHours": "h", - "unitScore": "pts", - "unitSteps": "pasos", - "unitBpm": "ppm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "Promedia {lowAvg} {unit} en los {lowDays} días que valoraste {factor} bajo frente a {highAvg} {unit} en los días que lo valoraste alto.", - "detailInverse": "Promedia {lowAvg} {unit} en tus {lowDays} peores días de {factor} frente a {highAvg} {unit} en los mejores." + "detail": "Promedia {lowAvg} en los {lowDays} días que valoraste {factor} bajo frente a {highAvg} en los días que lo valoraste alto.", + "detailInverse": "Promedia {lowAvg} en tus {lowDays} peores días de {factor} frente a {highAvg} en los mejores." }, "discovery": { "title": "Lo que destaca", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Tu ciclo y tu cuerpo", - "restingHeartRate": "Tu frecuencia cardíaca en reposo es unos {delta} {unit} {dir} en tu fase lútea que en tu fase folicular.", - "heartRateVariability": "Tu variabilidad de la frecuencia cardíaca es unos {delta} {unit} {dir} en tu fase lútea que en tu fase folicular.", - "generic": "Tu {metric} es unos {delta} {unit} {dir} en tu fase lútea que en tu fase folicular.", + "restingHeartRate": "Tu frecuencia cardíaca en reposo es unos {delta} {dir} en tu fase lútea que en tu fase folicular.", + "heartRateVariability": "Tu variabilidad de la frecuencia cardíaca es unos {delta} {dir} en tu fase lútea que en tu fase folicular.", + "generic": "Tu {metric} es unos {delta} {dir} en tu fase lútea que en tu fase folicular.", "dirHigher": "más alta", "dirLower": "más baja", "empty": "Aún no hay suficientes ciclos ni registros para mostrar un patrón de fase. Sigue registrando: se completará a medida que crezca tu historial." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Cómo se compara cada medición entre tus fases lútea y folicular, con tus propios datos.", "pairLabel": "{metric}: fase lútea vs. folicular", - "detail": "Lútea {lutealAvg} {unit} ({lutealDays} días) vs. folicular {follicularAvg} {unit} ({follicularDays} días).", + "detail": "Lútea {lutealAvg} ({lutealDays} días) vs. folicular {follicularAvg} ({follicularDays} días).", "metricRestingHeartRate": "Frecuencia cardíaca en reposo", "metricHrv": "Variabilidad de la frecuencia cardíaca", "metricSleepDuration": "Duración del sueño", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Temperatura de la muñeca nocturna", "metricSkinTemperature": "Temperatura de la piel", "metricBloodGlucose": "Glucosa en sangre", - "metricMood": "Estado de ánimo", - "unitHours": "h", - "unitSteps": "pasos", - "unitBpm": "lpm", - "unitMs": "ms", - "unitMood": "pts" + "metricMood": "Estado de ánimo" }, "title": "Análisis del ciclo", "symptomPatternsTitle": "Patrones de síntomas por fase", diff --git a/messages/fr.json b/messages/fr.json index e0fd482d5..7f653d505 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Récupération le lendemain", "metricNextDayRestingHr": "Fréquence cardiaque au repos le lendemain", "metricNextDayHrv": "Variabilité de la fréquence cardiaque le lendemain", - "unitHours": "h", - "unitKcal": "kcal", - "unitScore": "pts", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "En moyenne {withAvg} {unit} les {withDays} jours avec ce tag contre {withoutAvg} {unit} sans.", - "detailNextDay": "En moyenne {withAvg} {unit} le lendemain des {withDays} jours avec ce tag contre {withoutAvg} {unit} sinon." + "detailSameDay": "En moyenne {withAvg} les {withDays} jours avec ce tag contre {withoutAvg} sans.", + "detailNextDay": "En moyenne {withAvg} le lendemain des {withDays} jours avec ce tag contre {withoutAvg} sinon." }, "factorCrosstab": { "title": "Vos facteurs notés et vos constantes", @@ -4239,15 +4234,8 @@ "metricHrv": "Variabilité de la fréquence cardiaque", "metricWeight": "Poids", "metricBloodPressureSys": "Systolique", - "unitHours": "h", - "unitScore": "pts", - "unitSteps": "pas", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "En moyenne {lowAvg} {unit} les {lowDays} jours où vous avez noté {factor} bas contre {highAvg} {unit} les jours où vous l'avez noté haut.", - "detailInverse": "En moyenne {lowAvg} {unit} sur vos {lowDays} pires jours de {factor} contre {highAvg} {unit} sur les meilleurs." + "detail": "En moyenne {lowAvg} les {lowDays} jours où vous avez noté {factor} bas contre {highAvg} les jours où vous l'avez noté haut.", + "detailInverse": "En moyenne {lowAvg} sur vos {lowDays} pires jours de {factor} contre {highAvg} sur les meilleurs." }, "discovery": { "title": "Ce qui ressort", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Ton cycle et ton corps", - "restingHeartRate": "Ta fréquence cardiaque au repos est d'environ {delta} {unit} {dir} en phase lutéale qu'en phase folliculaire.", - "heartRateVariability": "Ta variabilité de la fréquence cardiaque est d'environ {delta} {unit} {dir} en phase lutéale qu'en phase folliculaire.", - "generic": "Ton/ta {metric} est d'environ {delta} {unit} {dir} en phase lutéale qu'en phase folliculaire.", + "restingHeartRate": "Ta fréquence cardiaque au repos est d'environ {delta} {dir} en phase lutéale qu'en phase folliculaire.", + "heartRateVariability": "Ta variabilité de la fréquence cardiaque est d'environ {delta} {dir} en phase lutéale qu'en phase folliculaire.", + "generic": "Ton/ta {metric} est d'environ {delta} {dir} en phase lutéale qu'en phase folliculaire.", "dirHigher": "plus élevée", "dirLower": "plus basse", "empty": "Pas encore assez de cycles et de relevés pour faire ressortir un schéma de phase. Continue à enregistrer : cela se complétera à mesure que ton historique grandit." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Comment chaque mesure se compare entre tes phases lutéale et folliculaire, sur tes propres données.", "pairLabel": "{metric} : phase lutéale vs folliculaire", - "detail": "Lutéale {lutealAvg} {unit} ({lutealDays} jours) vs folliculaire {follicularAvg} {unit} ({follicularDays} jours).", + "detail": "Lutéale {lutealAvg} ({lutealDays} jours) vs folliculaire {follicularAvg} ({follicularDays} jours).", "metricRestingHeartRate": "Fréquence cardiaque au repos", "metricHrv": "Variabilité de la fréquence cardiaque", "metricSleepDuration": "Durée du sommeil", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Température du poignet la nuit", "metricSkinTemperature": "Température de la peau", "metricBloodGlucose": "Glycémie", - "metricMood": "Humeur", - "unitHours": "h", - "unitSteps": "pas", - "unitBpm": "bpm", - "unitMs": "ms", - "unitMood": "pts" + "metricMood": "Humeur" }, "title": "Analyses du cycle", "symptomPatternsTitle": "Tendances des symptômes par phase", diff --git a/messages/it.json b/messages/it.json index 19db4220d..6e3618cb8 100644 --- a/messages/it.json +++ b/messages/it.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Recupero il giorno dopo", "metricNextDayRestingHr": "Frequenza cardiaca a riposo il giorno dopo", "metricNextDayHrv": "Variabilità della frequenza cardiaca il giorno dopo", - "unitHours": "h", - "unitKcal": "kcal", - "unitScore": "pt", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "In media {withAvg} {unit} nei {withDays} giorni con questo tag contro {withoutAvg} {unit} senza.", - "detailNextDay": "In media {withAvg} {unit} il giorno dopo i {withDays} giorni con questo tag contro {withoutAvg} {unit} altrimenti." + "detailSameDay": "In media {withAvg} nei {withDays} giorni con questo tag contro {withoutAvg} senza.", + "detailNextDay": "In media {withAvg} il giorno dopo i {withDays} giorni con questo tag contro {withoutAvg} altrimenti." }, "factorCrosstab": { "title": "I tuoi fattori valutati e i tuoi parametri vitali", @@ -4239,15 +4234,8 @@ "metricHrv": "Variabilità della frequenza cardiaca", "metricWeight": "Peso", "metricBloodPressureSys": "Sistolica", - "unitHours": "h", - "unitScore": "pti", - "unitSteps": "passi", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "In media {lowAvg} {unit} nei {lowDays} giorni in cui hai valutato basso {factor} contro {highAvg} {unit} nei giorni in cui lo hai valutato alto.", - "detailInverse": "In media {lowAvg} {unit} nei tuoi {lowDays} giorni peggiori di {factor} contro {highAvg} {unit} in quelli migliori." + "detail": "In media {lowAvg} nei {lowDays} giorni in cui hai valutato basso {factor} contro {highAvg} nei giorni in cui lo hai valutato alto.", + "detailInverse": "In media {lowAvg} nei tuoi {lowDays} giorni peggiori di {factor} contro {highAvg} in quelli migliori." }, "discovery": { "title": "Cosa risalta", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Il tuo ciclo e il tuo corpo", - "restingHeartRate": "La tua frequenza cardiaca a riposo è di circa {delta} {unit} {dir} nella fase luteale rispetto alla fase follicolare.", - "heartRateVariability": "La tua variabilità della frequenza cardiaca è di circa {delta} {unit} {dir} nella fase luteale rispetto alla fase follicolare.", - "generic": "Il/la tuo/a {metric} è di circa {delta} {unit} {dir} nella fase luteale rispetto alla fase follicolare.", + "restingHeartRate": "La tua frequenza cardiaca a riposo è di circa {delta} {dir} nella fase luteale rispetto alla fase follicolare.", + "heartRateVariability": "La tua variabilità della frequenza cardiaca è di circa {delta} {dir} nella fase luteale rispetto alla fase follicolare.", + "generic": "Il/la tuo/a {metric} è di circa {delta} {dir} nella fase luteale rispetto alla fase follicolare.", "dirHigher": "più alta", "dirLower": "più bassa", "empty": "Non ci sono ancora abbastanza cicli e rilevazioni per mostrare uno schema di fase. Continua a registrare: si completerà man mano che cresce il tuo storico." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Come si confronta ogni misurazione tra la fase luteale e quella follicolare, sui tuoi dati.", "pairLabel": "{metric}: fase luteale vs follicolare", - "detail": "Luteale {lutealAvg} {unit} ({lutealDays} giorni) vs follicolare {follicularAvg} {unit} ({follicularDays} giorni).", + "detail": "Luteale {lutealAvg} ({lutealDays} giorni) vs follicolare {follicularAvg} ({follicularDays} giorni).", "metricRestingHeartRate": "Frequenza cardiaca a riposo", "metricHrv": "Variabilità della frequenza cardiaca", "metricSleepDuration": "Durata del sonno", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Temperatura del polso notturna", "metricSkinTemperature": "Temperatura della pelle", "metricBloodGlucose": "Glicemia", - "metricMood": "Umore", - "unitHours": "h", - "unitSteps": "passi", - "unitBpm": "bpm", - "unitMs": "ms", - "unitMood": "pts" + "metricMood": "Umore" }, "title": "Analisi del ciclo", "symptomPatternsTitle": "Pattern dei sintomi per fase", diff --git a/messages/ko.json b/messages/ko.json index fb03673be..d1c43962a 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "다음 날 회복도", "metricNextDayRestingHr": "다음 날 안정 시 심박수", "metricNextDayHrv": "다음 날 심박변이도", - "unitHours": "h", - "unitKcal": "kcal", - "unitScore": "점", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "이 태그를 붙인 {withDays}일의 평균은 {withAvg}{unit}, 붙이지 않은 날의 평균은 {withoutAvg}{unit}.", - "detailNextDay": "이 태그를 붙인 {withDays}일의 다음 날 평균은 {withAvg}{unit}, 그 외의 날은 {withoutAvg}{unit}." + "detailSameDay": "이 태그를 붙인 {withDays}일의 평균은 {withAvg}, 붙이지 않은 날의 평균은 {withoutAvg}.", + "detailNextDay": "이 태그를 붙인 {withDays}일의 다음 날 평균은 {withAvg}, 그 외의 날은 {withoutAvg}." }, "factorCrosstab": { "title": "직접 평가한 요인과 생체 지표", @@ -4239,15 +4234,8 @@ "metricHrv": "심박변이도", "metricWeight": "체중", "metricBloodPressureSys": "수축기", - "unitHours": "h", - "unitScore": "점", - "unitSteps": "걸음", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "{factor} 평가가 낮았던 {lowDays}일의 평균은 {lowAvg}{unit}, 높게 평가한 날의 평균은 {highAvg}{unit}.", - "detailInverse": "{factor} 상태가 나빴던 {lowDays}일의 평균은 {lowAvg}{unit}, 좋았던 날의 평균은 {highAvg}{unit}." + "detail": "{factor} 평가가 낮았던 {lowDays}일의 평균은 {lowAvg}, 높게 평가한 날의 평균은 {highAvg}.", + "detailInverse": "{factor} 상태가 나빴던 {lowDays}일의 평균은 {lowAvg}, 좋았던 날의 평균은 {highAvg}." }, "discovery": { "title": "눈에 띄는 점", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "내 주기와 내 몸", - "restingHeartRate": "황체기의 안정 시 심박수가 난포기보다 약 {delta}{unit} {dir}.", - "heartRateVariability": "황체기의 심박변이도가 난포기보다 약 {delta}{unit} {dir}.", - "generic": "황체기의 {metric} 값이 난포기보다 약 {delta}{unit} {dir}.", + "restingHeartRate": "황체기의 안정 시 심박수가 난포기보다 약 {delta} {dir}.", + "heartRateVariability": "황체기의 심박변이도가 난포기보다 약 {delta} {dir}.", + "generic": "황체기의 {metric} 값이 난포기보다 약 {delta} {dir}.", "dirHigher": "높아요", "dirLower": "낮아요", "empty": "아직 주기와 측정값이 부족해서 단계별 패턴을 보여줄 수 없어요. 계속 기록하면 기록이 쌓이면서 채워져요." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "내 데이터를 기준으로, 각 측정값이 황체기와 난포기에서 어떻게 다른지 비교했어요.", "pairLabel": "{metric}: 황체기와 난포기 비교", - "detail": "황체기 {lutealAvg}{unit} ({lutealDays}일), 난포기 {follicularAvg}{unit} ({follicularDays}일).", + "detail": "황체기 {lutealAvg} ({lutealDays}일), 난포기 {follicularAvg} ({follicularDays}일).", "metricRestingHeartRate": "안정 시 심박수", "metricHrv": "심박변이도", "metricSleepDuration": "수면 시간", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "야간 손목 온도", "metricSkinTemperature": "피부 온도", "metricBloodGlucose": "혈당", - "metricMood": "기분", - "unitHours": "h", - "unitSteps": "걸음", - "unitBpm": "bpm", - "unitMs": "ms", - "unitMood": "점" + "metricMood": "기분" }, "title": "주기 인사이트", "symptomPatternsTitle": "단계별 증상 패턴", diff --git a/messages/nl.json b/messages/nl.json index b70878026..f6540db5e 100644 --- a/messages/nl.json +++ b/messages/nl.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Herstel de volgende dag", "metricNextDayRestingHr": "Rusthartslag de volgende dag", "metricNextDayHrv": "Hartslagvariabiliteit de volgende dag", - "unitHours": "u", - "unitKcal": "kcal", - "unitScore": "ptn", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "Gemiddeld {withAvg} {unit} op de {withDays} dagen dat je dit hebt getagd, tegenover {withoutAvg} {unit} zonder.", - "detailNextDay": "Gemiddeld {withAvg} {unit} op de dag na de {withDays} dagen dat je dit hebt getagd, tegenover {withoutAvg} {unit} op andere dagen." + "detailSameDay": "Gemiddeld {withAvg} op de {withDays} dagen dat je dit hebt getagd, tegenover {withoutAvg} zonder.", + "detailNextDay": "Gemiddeld {withAvg} op de dag na de {withDays} dagen dat je dit hebt getagd, tegenover {withoutAvg} op andere dagen." }, "factorCrosstab": { "title": "Je beoordeelde factoren en je vitale waarden", @@ -4239,15 +4234,8 @@ "metricHrv": "Hartslagvariabiliteit", "metricWeight": "Gewicht", "metricBloodPressureSys": "Systolisch", - "unitHours": "u", - "unitScore": "ptn", - "unitSteps": "stappen", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "Gemiddeld {lowAvg} {unit} op de {lowDays} dagen dat je {factor} laag beoordeelde, tegenover {highAvg} {unit} op de dagen dat je die hoog beoordeelde.", - "detailInverse": "Gemiddeld {lowAvg} {unit} op je {lowDays} slechtere dagen voor {factor}, tegenover {highAvg} {unit} op je betere dagen." + "detail": "Gemiddeld {lowAvg} op de {lowDays} dagen dat je {factor} laag beoordeelde, tegenover {highAvg} op de dagen dat je die hoog beoordeelde.", + "detailInverse": "Gemiddeld {lowAvg} op je {lowDays} slechtere dagen voor {factor}, tegenover {highAvg} op je betere dagen." }, "discovery": { "title": "Wat opvalt", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Je cyclus en je lichaam", - "restingHeartRate": "Je rusthartslag ligt in je luteale fase ongeveer {delta} {unit} {dir} dan in je folliculaire fase.", - "heartRateVariability": "Je hartslagvariabiliteit ligt in je luteale fase ongeveer {delta} {unit} {dir} dan in je folliculaire fase.", - "generic": "Je {metric} ligt in je luteale fase ongeveer {delta} {unit} {dir} dan in je folliculaire fase.", + "restingHeartRate": "Je rusthartslag ligt in je luteale fase ongeveer {delta} {dir} dan in je folliculaire fase.", + "heartRateVariability": "Je hartslagvariabiliteit ligt in je luteale fase ongeveer {delta} {dir} dan in je folliculaire fase.", + "generic": "Je {metric} ligt in je luteale fase ongeveer {delta} {dir} dan in je folliculaire fase.", "dirHigher": "hoger", "dirLower": "lager", "empty": "Nog niet genoeg cycli en metingen om een patroon per fase te tonen. Blijf vastleggen; dit vult zich naarmate je geschiedenis groeit." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Hoe elke meting zich verhoudt tussen je luteale en folliculaire fase, op basis van je eigen gegevens.", "pairLabel": "{metric}: luteaal vs. folliculair", - "detail": "Luteaal {lutealAvg} {unit} ({lutealDays} dagen) vs. folliculair {follicularAvg} {unit} ({follicularDays} dagen).", + "detail": "Luteaal {lutealAvg} ({lutealDays} dagen) vs. folliculair {follicularAvg} ({follicularDays} dagen).", "metricRestingHeartRate": "Rusthartslag", "metricHrv": "Hartslagvariabiliteit", "metricSleepDuration": "Slaapduur", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Nachtelijke polstemperatuur", "metricSkinTemperature": "Huidtemperatuur", "metricBloodGlucose": "Bloedglucose", - "metricMood": "Stemming", - "unitHours": "u", - "unitSteps": "stappen", - "unitBpm": "bpm", - "unitMs": "ms", - "unitMood": "pnt" + "metricMood": "Stemming" }, "title": "Cyclusinzichten", "symptomPatternsTitle": "Symptoompatronen per fase", diff --git a/messages/pl.json b/messages/pl.json index 7c0d439f1..49f041e09 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -4221,13 +4221,8 @@ "metricNextDayRecovery": "Regeneracja następnego dnia", "metricNextDayRestingHr": "Tętno spoczynkowe następnego dnia", "metricNextDayHrv": "Zmienność rytmu serca następnego dnia", - "unitHours": "godz.", - "unitKcal": "kcal", - "unitScore": "pkt", - "unitBpm": "bpm", - "unitMs": "ms", - "detailSameDay": "Średnio {withAvg} {unit} w {withDays} dniach z tym tagiem wobec {withoutAvg} {unit} bez niego.", - "detailNextDay": "Średnio {withAvg} {unit} dzień po {withDays} dniach z tym tagiem wobec {withoutAvg} {unit} w pozostałe dni." + "detailSameDay": "Średnio {withAvg} w {withDays} dniach z tym tagiem wobec {withoutAvg} bez niego.", + "detailNextDay": "Średnio {withAvg} dzień po {withDays} dniach z tym tagiem wobec {withoutAvg} w pozostałe dni." }, "factorCrosstab": { "title": "Twoje ocenione czynniki i parametry życiowe", @@ -4239,15 +4234,8 @@ "metricHrv": "Zmienność rytmu serca", "metricWeight": "Waga", "metricBloodPressureSys": "Skurczowe", - "unitHours": "godz.", - "unitScore": "pkt", - "unitSteps": "kroki", - "unitBpm": "bpm", - "unitMs": "ms", - "unitKg": "kg", - "unitMmHg": "mmHg", - "detail": "Średnio {lowAvg} {unit} w {lowDays} dniach z niską oceną {factor} wobec {highAvg} {unit} w dniach z wysoką oceną.", - "detailInverse": "Średnio {lowAvg} {unit} w Twoich {lowDays} gorszych dniach {factor} wobec {highAvg} {unit} w lepszych." + "detail": "Średnio {lowAvg} w {lowDays} dniach z niską oceną {factor} wobec {highAvg} w dniach z wysoką oceną.", + "detailInverse": "Średnio {lowAvg} w Twoich {lowDays} gorszych dniach {factor} wobec {highAvg} w lepszych." }, "discovery": { "title": "Co się wyróżnia", @@ -9158,9 +9146,9 @@ "insights": { "headline": { "title": "Twój cykl i Twoje ciało", - "restingHeartRate": "Twoje tętno spoczynkowe jest o około {delta} {unit} {dir} w fazie lutealnej niż w fazie folikularnej.", - "heartRateVariability": "Twoja zmienność rytmu serca jest o około {delta} {unit} {dir} w fazie lutealnej niż w fazie folikularnej.", - "generic": "Twoje {metric} jest o około {delta} {unit} {dir} w fazie lutealnej niż w fazie folikularnej.", + "restingHeartRate": "Twoje tętno spoczynkowe jest o około {delta} {dir} w fazie lutealnej niż w fazie folikularnej.", + "heartRateVariability": "Twoja zmienność rytmu serca jest o około {delta} {dir} w fazie lutealnej niż w fazie folikularnej.", + "generic": "Twoje {metric} jest o około {delta} {dir} w fazie lutealnej niż w fazie folikularnej.", "dirHigher": "wyższe", "dirLower": "niższe", "empty": "Za mało cykli i pomiarów, aby pokazać wzorzec fazy. Rejestruj dalej, a uzupełni się to wraz z historią." @@ -9168,7 +9156,7 @@ "crosstab": { "description": "Jak każdy pomiar wypada między fazą lutealną a folikularną, na Twoich danych.", "pairLabel": "{metric}: faza lutealna vs folikularna", - "detail": "Lutealna {lutealAvg} {unit} ({lutealDays} dni) vs folikularna {follicularAvg} {unit} ({follicularDays} dni).", + "detail": "Lutealna {lutealAvg} ({lutealDays} dni) vs folikularna {follicularAvg} ({follicularDays} dni).", "metricRestingHeartRate": "Tętno spoczynkowe", "metricHrv": "Zmienność rytmu serca", "metricSleepDuration": "Czas snu", @@ -9178,12 +9166,7 @@ "metricWristTemperature": "Nocna temperatura nadgarstka", "metricSkinTemperature": "Temperatura skóry", "metricBloodGlucose": "Glukoza we krwi", - "metricMood": "Nastrój", - "unitHours": "godz.", - "unitSteps": "kroki", - "unitBpm": "ud./min", - "unitMs": "ms", - "unitMood": "pkt" + "metricMood": "Nastrój" }, "title": "Analizy cyklu", "symptomPatternsTitle": "Wzorce objawów wg fazy", diff --git a/src/__tests__/helpers/metric-unit-display.ts b/src/__tests__/helpers/metric-unit-display.ts new file mode 100644 index 000000000..0b3971e0e --- /dev/null +++ b/src/__tests__/helpers/metric-unit-display.ts @@ -0,0 +1,24 @@ +/** + * A `useUnitDisplay` result for the metric preference, for static renders + * that have no QueryClient (the real hook hangs off `useAuth`). Metric is the + * identity on values and yields the canonical display symbols. + */ +import * as dt from "@/lib/measurements/display-transform"; +import type { UnitDisplay } from "@/hooks/use-unit-display"; + +export function metricUnitDisplay( + glucoseUnit: UnitDisplay["glucoseUnit"] = "mg/dL", +): UnitDisplay { + const forType = (t: string) => dt.getDisplayTransform(t, "metric"); + return { + preference: "metric", + glucoseUnit, + transformFor: forType, + toDisplay: (t, v) => dt.applyDisplayTransform(v, forType(t)), + fromDisplay: (t, v) => dt.invertDisplayTransform(v, forType(t)), + toDisplayDelta: (t, v) => dt.applyDisplayTransformDelta(v, forType(t)), + unitFor: (t) => forType(t).displayUnit, + decimalsFor: (t) => forType(t).decimals, + isTransformed: (t) => dt.hasDisplayTransform(t), + }; +} diff --git a/src/__tests__/unit-preference-display-guard.test.ts b/src/__tests__/unit-preference-display-guard.test.ts index 4f6362757..b5febe503 100644 --- a/src/__tests__/unit-preference-display-guard.test.ts +++ b/src/__tests__/unit-preference-display-guard.test.ts @@ -328,8 +328,14 @@ const UNIT_LITERAL_ALLOWLIST: Record< reason: "a display-kind tag; the weight row converts through the hook", }, "components/insights/mood/mood-factor-metric-crosstab.tsx": { - lines: 2, - reason: "a display-kind tag; the weight row converts through the hook", + lines: 1, + reason: + "a display-kind tag; the weight row converts through crosstabReading", + }, + "components/insights/metric-crosstab.tsx": { + lines: 3, + reason: + "the boards' display kinds and the storage token each is in; rows convert through getReadingTransform and label through measurementUnitLabel", }, // ── Canonical by contract ── "lib/insights/metric-status-registry.ts": { diff --git a/src/components/cycle/__tests__/cycle-phase-crosstab.test.tsx b/src/components/cycle/__tests__/cycle-phase-crosstab.test.tsx index 1e16d70e6..70e93fd0d 100644 --- a/src/components/cycle/__tests__/cycle-phase-crosstab.test.tsx +++ b/src/components/cycle/__tests__/cycle-phase-crosstab.test.tsx @@ -86,7 +86,9 @@ describe("", () => { ); expect(html).toContain("Mood"); expect(html).toContain("Blood glucose"); - expect(html).toContain("pts"); + // A mood rating has no unit word, as everywhere else a rating is shown. + expect(html).not.toContain("pts"); + expect(html).toContain("Luteal 5.0 (12 days)"); expect(html).toContain("mg/dL"); // Never leaks the raw metric key or the raw i18n key on a covered row. expect(html).not.toContain("cycle.insights.crosstab"); diff --git a/src/components/cycle/cycle-phase-crosstab.tsx b/src/components/cycle/cycle-phase-crosstab.tsx index a46a12917..4762dcdaf 100644 --- a/src/components/cycle/cycle-phase-crosstab.tsx +++ b/src/components/cycle/cycle-phase-crosstab.tsx @@ -2,13 +2,13 @@ import { useFormatters, useTranslations } from "@/lib/i18n/context"; import type { Formatters } from "@/lib/format-locale"; -import { useUnitDisplay, type UnitDisplay } from "@/hooks/use-unit-display"; +import { useUnitDisplay } from "@/hooks/use-unit-display"; import { - applyDisplayTransform, - applyDisplayTransformDelta, - getReadingTransform, -} from "@/lib/measurements/display-transform"; -import { MetricCrosstab } from "@/components/insights/metric-crosstab"; + crosstabReading, + MetricCrosstab, + withUnit, + type CrosstabMeasure, +} from "@/components/insights/metric-crosstab"; /** * v1.15 — CYCLE_PHASE × vital crosstab card. @@ -57,18 +57,26 @@ const METRIC_LABEL_KEY: Record = { }; /** - * Bundle labels for the displays no preference changes. Weight, the - * temperatures and glucose take their symbol from the reader's transform - * (`DISPLAY_TO_TYPE` below) and have no bundle label to fall back on. + * The measurement each row reads (`src/lib/cycle/phase-crosstab.ts`), which + * is what its unit and its preference conversion come from. Mood has none. */ -const UNIT_KEY: Partial> = { - hours: "cycle.insights.crosstab.unitHours", - steps: "cycle.insights.crosstab.unitSteps", - bpm: "cycle.insights.crosstab.unitBpm", - ms: "cycle.insights.crosstab.unitMs", - mood: "cycle.insights.crosstab.unitMood", +const METRIC_TYPE: Record = { + restingHeartRate: "RESTING_HEART_RATE", + heartRateVariability: "HEART_RATE_VARIABILITY", + sleepDuration: "SLEEP_DURATION", + steps: "ACTIVITY_STEPS", + weight: "WEIGHT", + basalBodyTemp: "BODY_TEMPERATURE", + wristTemperature: "WRIST_TEMPERATURE", + skinTemperature: "SKIN_TEMPERATURE", + bloodGlucose: "BLOOD_GLUCOSE", + mood: null, }; +function measureOf(row: CyclePhaseCrosstabRow): CrosstabMeasure { + return { type: METRIC_TYPE[row.metricKey] ?? null, display: row.display }; +} + /** Whole numbers for steps; one decimal otherwise. */ function fmt( value: number, @@ -78,57 +86,6 @@ function fmt( return display === "steps" ? nf.integer(value) : nf.number(value, 1); } -/** - * v1.32.26 — the two displays whose values follow the metric/imperial - * preference, mapped to the measurement type that carries the transform. Weight - * is offset-free (kg→lb); temperature is affine (°C→°F), so an ABSOLUTE phase - * average takes `toDisplay` while a phase DELTA takes `toDisplayDelta` (the - * offset must never leak into a difference). - */ -const DISPLAY_TO_TYPE: Partial> = { - kg: "WEIGHT", - celsius: "BODY_TEMPERATURE", - glucose: "BLOOD_GLUCOSE", -}; - -/** - * Unit symbol and conversions for one row, from the reader's preferences. - * Glucose follows the glucose unit (it used to print the mg/dL label from - * the bundle beside raw mg/dL values for every reader); weight and - * temperature follow metric/imperial; every other display keeps its bundle - * label and its raw value. - */ -function rowUnits( - display: CyclePhaseCrosstabDisplay, - unitDisplay: UnitDisplay, - t: (key: string) => string, -): { - unit: string; - abs: (v: number) => number; - delta: (v: number) => number; -} { - const type = DISPLAY_TO_TYPE[display]; - if (!type) { - // Defensive: an unmapped display must never pass undefined to `t` - // (whose resolver does `key.split(".")`). Empty unit on a miss. - const unitKey = UNIT_KEY[display]; - return { - unit: unitKey ? t(unitKey) : "", - abs: (v) => v, - delta: (v) => v, - }; - } - const transform = getReadingTransform(type, { - system: unitDisplay.preference, - glucoseUnit: unitDisplay.glucoseUnit, - }); - return { - unit: transform.displayUnit, - abs: (v) => applyDisplayTransform(v, transform), - delta: (v) => applyDisplayTransformDelta(v, transform), - }; -} - const HEADLINE_KEY: Record = { restingHeartRate: "cycle.insights.headline.restingHeartRate", heartRateVariability: "cycle.insights.headline.heartRateVariability", @@ -160,7 +117,11 @@ export function CyclePhaseHeadline({ const metricLabel = t( METRIC_LABEL_KEY[headline.metricKey] ?? headline.metricKey, ); - const { unit, delta: deltaConv } = rowUnits(headline.display, unitDisplay, t); + const { unit, delta: deltaConv } = crosstabReading( + measureOf(headline), + unitDisplay, + t, + ); const up = headline.delta >= 0; const dir = t( up @@ -170,8 +131,10 @@ export function CyclePhaseHeadline({ const params = { metric: metricLabel, // A phase delta → factor-only conversion (no affine offset). - delta: fmt(Math.abs(deltaConv(headline.delta)), headline.display, nf), - unit, + delta: withUnit( + fmt(Math.abs(deltaConv(headline.delta)), headline.display, nf), + unit, + ), dir, }; const key = @@ -202,11 +165,12 @@ export function CyclePhaseCrosstab({ description={t("cycle.insights.crosstab.description")} rows={rows.map((row) => { const metricLabel = t(METRIC_LABEL_KEY[row.metricKey] ?? row.metricKey); + const measure = measureOf(row); const { unit, abs: absConv, delta: deltaConv, - } = rowUnits(row.display, unitDisplay, t); + } = crosstabReading(measure, unitDisplay, t); // `delta` = lutealAvg − follicularAvg: positive = the vital runs // higher in the luteal phase. return { @@ -217,13 +181,18 @@ export function CyclePhaseCrosstab({ }), up: row.delta >= 0, deltaValue: fmt(deltaConv(row.delta), row.display, nf), - unit, + measure, confidence: row.confidence, detail: t("cycle.insights.crosstab.detail", { metric: metricLabel, - lutealAvg: fmt(absConv(row.lutealAvg), row.display, nf), - follicularAvg: fmt(absConv(row.follicularAvg), row.display, nf), - unit, + lutealAvg: withUnit( + fmt(absConv(row.lutealAvg), row.display, nf), + unit, + ), + follicularAvg: withUnit( + fmt(absConv(row.follicularAvg), row.display, nf), + unit, + ), lutealDays: row.lutealDays, follicularDays: row.follicularDays, }), diff --git a/src/components/insights/__tests__/metric-crosstab.test.tsx b/src/components/insights/__tests__/metric-crosstab.test.tsx index 7fd1f334a..12fa7baff 100644 --- a/src/components/insights/__tests__/metric-crosstab.test.tsx +++ b/src/components/insights/__tests__/metric-crosstab.test.tsx @@ -1,7 +1,13 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; import { Moon } from "lucide-react"; +vi.mock("@/hooks/use-unit-display", async () => { + const { metricUnitDisplay } = + await import("@/__tests__/helpers/metric-unit-display"); + return { useUnitDisplay: () => metricUnitDisplay() }; +}); + import { I18nProvider } from "@/lib/i18n/context"; import { MetricCrosstab, type MetricCrosstabRow } from "../metric-crosstab"; @@ -12,7 +18,7 @@ function row(overrides: Partial = {}): MetricCrosstabRow { pairLabel: "Resting heart rate", up: true, deltaValue: "2,5", - unit: "bpm", + measure: { type: "RESTING_HEART_RATE", display: "bpm" }, confidence: "medium", detail: "62 vs 59.5 bpm", ...overrides, @@ -68,4 +74,31 @@ describe("", () => { const html = render([row({ patternId: "p1", dismissed: true })]); expect(html).not.toContain('data-slot="board-row"'); }); + + it("takes the unit from the measure, through the shared unit table", () => { + const html = render([ + row(), + row({ + key: "b", + measure: { type: "ACTIVITY_STEPS", display: "steps" }, + deltaValue: "120", + }), + row({ + key: "c", + measure: { type: null, display: "mood" }, + deltaValue: "0,4", + }), + ]); + expect(html).toContain("+2,5 bpm"); + // The word comes from `measurementUnitLabel` (`insights.units.steps`). + expect(html).toContain("+120 steps"); + // A rating has no unit word: the number stands alone, no trailing space. + expect(html).toContain(">+0,4<"); + }); + + it("wraps a long pair label instead of cutting it", () => { + const html = render([row({ pairLabel: "A very long custom mood tag" })]); + expect(html).toMatch(/data-slot="metric-crosstab-pair"/); + expect(html).not.toContain("truncate"); + }); }); diff --git a/src/components/insights/metric-crosstab.tsx b/src/components/insights/metric-crosstab.tsx index 3211ff5d3..3f3ceb66e 100644 --- a/src/components/insights/metric-crosstab.tsx +++ b/src/components/insights/metric-crosstab.tsx @@ -3,7 +3,15 @@ import type { LucideIcon } from "lucide-react"; import { CONFIDENCE_TONE, ScorePill } from "@/components/ui/score-pill"; +import { useUnitDisplay, type UnitDisplay } from "@/hooks/use-unit-display"; import { useTranslations } from "@/lib/i18n/context"; +import { + applyDisplayTransform, + applyDisplayTransformDelta, + getReadingTransform, + isUnitSensitiveType, +} from "@/lib/measurements/display-transform"; +import { measurementUnitLabel } from "@/lib/measurements/unit-label"; import { PatternDismissButton, PatternDismissedNotice, @@ -18,14 +26,101 @@ import { * same markup that had begun to differ (the cycle copy printed its numbers * with a full stop in every language). * - * The caller owns the meaning: it resolves the labels, converts the units and - * formats the numbers (`deltaValue` already in the reader's notation, signed - * when negative). This component owns the shape: the row, the neutral delta - * (a higher value is good on one row and bad on the next, so the number is - * never coloured; the sign and `data-direction` carry the direction), the - * confidence chip, and the optional per-row dismissal. + * The caller owns the meaning: it resolves the labels and formats the numbers + * (`deltaValue` already in the reader's notation, signed when negative). The + * unit is not the caller's: each row names the measurement it reads + * (`measure`), and the unit comes from `measurementUnitLabel`, the one table + * every other read-out uses, after the reader's unit preference + * ({@link crosstabReading}). The boards used to carry a unit table each, and + * the German cycle board printed "S/min" where every other screen says "bpm". + * This component owns the shape: the row, the neutral delta (a higher value + * is good on one row and bad on the next, so the number is never coloured; + * the sign and `data-direction` carry the direction), the confidence chip, + * and the optional per-row dismissal. */ +/** + * How the server sends a board's values: the scale, not the label. Most are + * the measurement's canonical unit; sleep arrives in hours (stored minutes), + * and a mood value is a rating with no measurement type. + */ +export type CrosstabDisplay = + | "hours" + | "kcal" + | "score" + | "bpm" + | "ms" + | "steps" + | "kg" + | "mmHg" + | "celsius" + | "glucose" + | "mood"; + +/** The storage token each display is in, for `measurementUnitLabel`. */ +const DISPLAY_TOKEN: Record = { + hours: "h", + kcal: "kcal", + score: "score", + bpm: "bpm", + ms: "ms", + steps: "steps", + kg: "kg", + mmHg: "mmHg", + celsius: "celsius", + glucose: "mg/dL", + mood: "rating", +}; + +/** What one row reads: the measurement type (null for mood) and its scale. */ +export interface CrosstabMeasure { + type: string | null; + display: CrosstabDisplay; +} + +/** + * The unit a row reads in, and the conversions of its absolute values and + * its differences, under the reader's preferences. A type whose number + * follows a preference (weight, temperature, glucose) takes the transform; any + * other keeps the server's value. Either way the label is + * `measurementUnitLabel`'s. + */ +export function crosstabReading( + measure: CrosstabMeasure, + units: UnitDisplay, + t: (key: string) => string, +): { + unit: string; + abs: (value: number) => number; + delta: (value: number) => number; +} { + const { type } = measure; + if (type && isUnitSensitiveType(type)) { + // The reading transform, not `units.transformFor`: glucose follows its + // own unit preference, not metric/imperial. + const transform = getReadingTransform(type, { + system: units.preference, + glucoseUnit: units.glucoseUnit, + }); + return { + unit: measurementUnitLabel(type, transform.displayUnit, t), + abs: (value) => applyDisplayTransform(value, transform), + // A difference takes the factor only, never an affine offset (°C→°F). + delta: (value) => applyDisplayTransformDelta(value, transform), + }; + } + return { + unit: measurementUnitLabel(type ?? "", DISPLAY_TOKEN[measure.display], t), + abs: (value) => value, + delta: (value) => value, + }; +} + +/** A formatted value with its unit, or the value alone when it has none. */ +export function withUnit(value: string, unit: string): string { + return unit ? `${value} ${unit}` : value; +} + export type MetricCrosstabConfidence = "low" | "medium" | "high"; export interface MetricCrosstabRow { @@ -39,7 +134,8 @@ export interface MetricCrosstabRow { up: boolean; /** The difference in the reader's notation, with its minus sign. */ deltaValue: string; - unit: string; + /** What the row reads; the unit beside the difference comes from it. */ + measure: CrosstabMeasure; confidence: MetricCrosstabConfidence; detail: string; /** A dismissible pattern; omitted on boards that cannot be dismissed. */ @@ -68,6 +164,7 @@ export function MetricCrosstab({ rows: readonly MetricCrosstabRow[]; }) { const { t } = useTranslations(); + const units = useUnitDisplay(); const dismissal = usePatternDismissalOverrides(); if (rows.length === 0) return null; @@ -93,6 +190,7 @@ export function MetricCrosstab({ ); } const Icon = row.icon; + const { unit } = crosstabReading(row.measure, units, t); return (
  • -
    +
    {Icon ? (