From ab081d2a8590f262c78b9cfec7814cca251f6b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 17:53:36 +0200 Subject: [PATCH 01/15] fix(workouts): keep a deleted workout's insight slot spent The insight claim row is now the daily-cap ledger. Its workout_id is nullable with ON DELETE SET NULL (migration 0367), so deleting a workout no longer frees the day's slot. The cap counts claims that reached the provider plus live leases, and drops the separate count of insight rows. A daily workout-insight-claim-cleanup queue removes claim rows untouched for seven days. --- .../migration.sql | 15 ++ prisma/schema.prisma | 9 +- src/lib/export/backup-plan.ts | 2 +- .../workout-insight-generate.test.ts | 82 +++++-- src/lib/jobs/queue-runtime.ts | 1 + src/lib/jobs/reminder/cleanup-handlers.ts | 25 +++ src/lib/jobs/reminder/register-maintenance.ts | 27 +++ src/lib/jobs/subsystem-surface.ts | 1 + src/lib/jobs/workout-insight-claim-cleanup.ts | 30 +++ src/lib/jobs/workout-insight-generate.ts | 59 +++-- .../integration/workout-insight-claim.test.ts | 208 ++++++++++-------- 11 files changed, 324 insertions(+), 135 deletions(-) create mode 100644 prisma/migrations/0367_workout_insight_claim_set_null/migration.sql create mode 100644 src/lib/jobs/workout-insight-claim-cleanup.ts diff --git a/prisma/migrations/0367_workout_insight_claim_set_null/migration.sql b/prisma/migrations/0367_workout_insight_claim_set_null/migration.sql new file mode 100644 index 000000000..d561de0cf --- /dev/null +++ b/prisma/migrations/0367_workout_insight_claim_set_null/migration.sql @@ -0,0 +1,15 @@ +-- The per-workout insight claim is the ledger behind the daily insight cap. +-- It used to cascade with its workout, so deleting a workout freed the day's +-- slot and a delete-and-re-add loop could buy more paragraphs than the cap +-- allows. The claim now outlives the workout with a null `workout_id`; a +-- daily retention job drops rows older than a week. Idempotent: a re-run +-- finds the column already nullable and the constraint already re-created. +ALTER TABLE "workout_insight_generation_claims" + ALTER COLUMN "workout_id" DROP NOT NULL; + +ALTER TABLE "workout_insight_generation_claims" + DROP CONSTRAINT IF EXISTS "workout_insight_generation_claims_workout_id_fkey"; + +ALTER TABLE "workout_insight_generation_claims" + ADD CONSTRAINT "workout_insight_generation_claims_workout_id_fkey" + FOREIGN KEY ("workout_id") REFERENCES "workouts"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 11b59cb98..bde9dbecc 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -2409,7 +2409,10 @@ model WorkoutInsight { model WorkoutInsightGenerationClaim { id String @id @default(cuid()) userId String @map("user_id") - workoutId String @unique @map("workout_id") + /// Null once the workout is deleted. The row stays as the day's ledger so + /// the delete cannot free an insight slot; the retention job drops it after + /// a week. + workoutId String? @unique @map("workout_id") localDate String @map("local_date") claimId String? @unique @map("claim_id") claimedAt DateTime? @map("claimed_at") @@ -2418,8 +2421,8 @@ model WorkoutInsightGenerationClaim { createdAt DateTime @default(now()) @map("created_at") updatedAt DateTime @updatedAt @map("updated_at") - user User @relation(fields: [userId], references: [id], onDelete: Cascade) - workout Workout @relation(fields: [workoutId], references: [id], onDelete: Cascade) + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + workout Workout? @relation(fields: [workoutId], references: [id], onDelete: SetNull) @@index([userId, localDate]) @@map("workout_insight_generation_claims") diff --git a/src/lib/export/backup-plan.ts b/src/lib/export/backup-plan.ts index a6e83e2d2..8287dd748 100644 --- a/src/lib/export/backup-plan.ts +++ b/src/lib/export/backup-plan.ts @@ -579,7 +579,7 @@ export const DERIVED_MODELS: Readonly> = { WorkoutInsight: "Generated narrative over `Workout`; regenerates on demand. Carrying it would restore an interpretation of data rather than the data.", WorkoutInsightGenerationClaim: - "A concurrency claim guarding the generator above. Meaningless outside the run that took it.", + "A concurrency claim and the daily-cap ledger for the generator above. It outlives a deleted workout for at most a week and means nothing on another instance or day.", InsightNarrative: "Generated prose over measurements. Same reasoning as WorkoutInsight — the record restores, the essay about it does not.", InsightStatusCache: diff --git a/src/lib/jobs/__tests__/workout-insight-generate.test.ts b/src/lib/jobs/__tests__/workout-insight-generate.test.ts index bd3798933..f1443b204 100644 --- a/src/lib/jobs/__tests__/workout-insight-generate.test.ts +++ b/src/lib/jobs/__tests__/workout-insight-generate.test.ts @@ -54,7 +54,10 @@ vi.mock("@/lib/measurements/pick-canonical-workout-rows", () => ({ pickCanonicalWorkoutRows: vi.fn((rows) => rows), })); -import { runWorkoutInsightGenerate } from "../workout-insight-generate"; +import { + runWorkoutInsightGenerate, + workoutInsightCapWhere, +} from "../workout-insight-generate"; import { prisma } from "@/lib/db"; import { aiCapabilityForJob } from "@/lib/ai/capabilities/gate"; import { resolveUserTimezone } from "@/lib/tz/resolver"; @@ -224,8 +227,10 @@ describe("Activity Insight — the gate stack", () => { expect(outcome.status).toBe("generated"); }); - it("gate 3 — refuses once four paragraphs exist for the local day", async () => { - vi.mocked(prisma.workoutInsight.count).mockResolvedValue(4 as never); + it("gate 3 — refuses once four slots are spent for the local day", async () => { + vi.mocked(prisma.workoutInsightGenerationClaim.count).mockResolvedValue( + 4 as never, + ); const outcome = await runWorkoutInsightGenerate( { userId: USER, workoutId: WORKOUT }, @@ -237,36 +242,33 @@ describe("Activity Insight — the gate stack", () => { }); it("gate 3 — counts in the USER's timezone, not UTC", async () => { - // 00:30 Berlin on the 19th is 22:30 UTC on the 18th. A UTC-keyed window - // would still be counting the 18th's paragraphs against the new local day. + // 00:30 Berlin on the 19th is 22:30 UTC on the 18th. A UTC-keyed day + // would still be counting the 18th's claims against the new local day. vi.mocked(resolveUserTimezone).mockResolvedValue("Europe/Berlin"); await runWorkoutInsightGenerate( { userId: USER, workoutId: WORKOUT }, new Date("2026-07-18T22:30:00.000Z"), ); - const where = vi.mocked(prisma.workoutInsight.count).mock.calls[0][0] - ?.where as { generatedAt: { gte: Date } }; - // Midnight Berlin on the 19th == 22:00 UTC on the 18th. - expect(where.generatedAt.gte.toISOString()).toBe( - "2026-07-18T22:00:00.000Z", - ); + const where = vi.mocked(prisma.workoutInsightGenerationClaim.count).mock + .calls[0][0]?.where as { localDate: string }; + expect(where.localDate).toBe("2026-07-19"); }); - it("gate 3 — starts a midday cap window at exact local midnight", async () => { - vi.mocked(resolveUserTimezone).mockResolvedValue("Europe/Berlin"); - vi.mocked(prisma.workoutInsight.count).mockResolvedValue(4 as never); + it("gate 3 — the cap reads the claim ledger, never the insight rows", async () => { + // A paragraph whose workout was deleted is gone from `workout_insights`, + // so counting those rows would hand the slot back. Only the claims count. + vi.mocked(prisma.workoutInsightGenerationClaim.count).mockResolvedValue( + 4 as never, + ); + vi.mocked(prisma.workoutInsight.count).mockResolvedValue(0 as never); const outcome = await runWorkoutInsightGenerate( { userId: USER, workoutId: WORKOUT }, new Date("2026-07-18T15:00:45.678Z"), ); - const where = vi.mocked(prisma.workoutInsight.count).mock.calls[0][0] - ?.where as { generatedAt: { gte: Date } }; - expect(where.generatedAt.gte.toISOString()).toBe( - "2026-07-17T22:00:00.000Z", - ); + expect(prisma.workoutInsight.count).not.toHaveBeenCalled(); expect(outcome).toEqual({ status: "skipped", reason: "daily_cap" }); expect(runStatusCompletion).not.toHaveBeenCalled(); }); @@ -304,7 +306,9 @@ describe("Activity Insight — the gate stack", () => { vi.clearAllMocks(); arrangeHappyPath(); - vi.mocked(prisma.workoutInsight.count).mockResolvedValue(0 as never); + vi.mocked(prisma.workoutInsightGenerationClaim.count).mockResolvedValue( + 0 as never, + ); vi.mocked(prisma.workoutInsight.findFirst).mockResolvedValue({ id: "wi-1", inputHash: stored.inputHash, @@ -321,7 +325,9 @@ describe("Activity Insight — the gate stack", () => { // The mirror of the test above: a fresh workout (no stored row, so the hash // gate cannot fire) is still refused once the day's count is spent. vi.mocked(prisma.workoutInsight.findFirst).mockResolvedValue(null as never); - vi.mocked(prisma.workoutInsight.count).mockResolvedValue(4 as never); + vi.mocked(prisma.workoutInsightGenerationClaim.count).mockResolvedValue( + 4 as never, + ); const outcome = await runWorkoutInsightGenerate( { userId: USER, workoutId: WORKOUT }, @@ -660,3 +666,37 @@ describe("Activity Insight — own-history comparison", () => { expect(where.id).toEqual({ not: WORKOUT }); }); }); + +describe("Activity Insight — the cap predicate", () => { + const staleBefore = new Date("2026-07-18T14:50:00.000Z"); + + it("counts every provider attempt and every live lease, nothing narrower", () => { + const where = workoutInsightCapWhere({ + userId: USER, + localDate: "2026-07-18", + staleBefore, + }); + expect(where).toEqual({ + userId: USER, + localDate: "2026-07-18", + OR: [ + { providerInvokedAt: { not: null } }, + { claimId: { not: null }, claimedAt: { gte: staleBefore } }, + ], + }); + // A completed claim whose workout is gone is still a spent slot: the + // predicate must not narrow on the workout or on completion. + expect(JSON.stringify(where)).not.toContain("workoutId"); + expect(JSON.stringify(where)).not.toContain("completedAt"); + }); + + it("leaves the reclaiming row out of its own count", () => { + const where = workoutInsightCapWhere({ + userId: USER, + localDate: "2026-07-18", + staleBefore, + excludeClaimRowId: "claim-row-1", + }); + expect(where).toMatchObject({ id: { not: "claim-row-1" } }); + }); +}); diff --git a/src/lib/jobs/queue-runtime.ts b/src/lib/jobs/queue-runtime.ts index caf376c3d..82e919086 100644 --- a/src/lib/jobs/queue-runtime.ts +++ b/src/lib/jobs/queue-runtime.ts @@ -291,6 +291,7 @@ export const QUEUE_RUNTIME: Readonly> = { "idempotency-cleanup": short("One retention DELETE."), "step-up-elevation-cleanup": short("One retention DELETE."), "coach-message-cleanup": short("One retention DELETE."), + "workout-insight-claim-cleanup": short("One retention DELETE."), "mcp-token-cleanup": short("One retention DELETE."), "audit-log-cleanup": short("At most 40 batches of 5 000 rows per leg."), "measurement-tombstone-cleanup": short( diff --git a/src/lib/jobs/reminder/cleanup-handlers.ts b/src/lib/jobs/reminder/cleanup-handlers.ts index 14c86bb90..0c6927965 100644 --- a/src/lib/jobs/reminder/cleanup-handlers.ts +++ b/src/lib/jobs/reminder/cleanup-handlers.ts @@ -31,6 +31,7 @@ import { cleanupExpiredIdempotencyKeys } from "@/lib/jobs/idempotency-cleanup"; import { cleanupExpiredMcpTokens } from "@/lib/jobs/mcp-token-cleanup"; import { cleanupOldAuditLogs } from "@/lib/jobs/audit-log-cleanup"; import { cleanupOldCoachMessages } from "@/lib/jobs/coach-message-cleanup"; +import { cleanupOldWorkoutInsightClaims } from "@/lib/jobs/workout-insight-claim-cleanup"; import { cleanupExpiredWithingsOAuthStates } from "@/lib/jobs/withings-oauth-state-cleanup"; import { cleanupExpiredOidcNativeHandoffs } from "@/lib/jobs/oidc-handoff-cleanup"; import { @@ -395,6 +396,30 @@ export async function handleCoachMessageCleanup( }); } +export interface WorkoutInsightClaimCleanupPayload { + triggeredAt: string; +} + +export async function handleWorkoutInsightClaimCleanup( + jobs: Job[], +): Promise { + void jobs; + return withBackgroundEvent( + "job.workout_insight_claim_cleanup", + async (evt) => { + const p = getWorkerPrisma(); + try { + const deleted = await cleanupOldWorkoutInsightClaims(p); + evt.addMeta("workout_insight_claim_cleanup_deleted", deleted); + return jobDone({ deleted }); + } catch (err) { + evt.addWarning(`workout-insight-claim-cleanup failed: ${err}`); + return jobFailed("workout-insight-claim cleanup failed", err); + } + }, + ); +} + export interface McpTokenCleanupPayload { triggeredAt: string; } diff --git a/src/lib/jobs/reminder/register-maintenance.ts b/src/lib/jobs/reminder/register-maintenance.ts index e9fdc896d..e157839a4 100644 --- a/src/lib/jobs/reminder/register-maintenance.ts +++ b/src/lib/jobs/reminder/register-maintenance.ts @@ -189,6 +189,8 @@ import { handlePushAttemptCleanup, ArrivalReactionCleanupPayload, handleArrivalReactionCleanup, + WorkoutInsightClaimCleanupPayload, + handleWorkoutInsightClaimCleanup, MeasurementTombstoneCleanupPayload, handleMeasurementTombstoneCleanup, handleRateLimitCleanup, @@ -330,6 +332,13 @@ const MEASUREMENT_TOMBSTONE_CLEANUP_CRON = "40 3 * * *"; const COACH_MESSAGE_CLEANUP_QUEUE = "coach-message-cleanup"; const COACH_MESSAGE_CLEANUP_CRON = "50 3 * * *"; +// v1.40 — daily prune of the workout-insight claim ledger. Claims outlive a +// deleted workout so the delete cannot free a daily slot; rows older than a +// week are never read by the cap again. + +const WORKOUT_INSIGHT_CLAIM_CLEANUP_QUEUE = "workout-insight-claim-cleanup"; + +const WORKOUT_INSIGHT_CLAIM_CLEANUP_CRON = "52 3 * * *"; // v1.23 — converging backfill that migrates the free-text health-note columns // (mood + measurement) from plaintext to AES-256-GCM at rest. Boot discovery // enqueues one per-user job; a daily 03:55 Europe/Berlin discovery tick (empty @@ -445,6 +454,10 @@ const allQueues = [ // the daily schedule silently no-ops and the encrypted coach_messages // table grows unbounded. COACH_MESSAGE_CLEANUP_QUEUE, + // v1.40 — workout-insight claim ledger retention prune. Without this entry + // the daily schedule silently no-ops and the claim table keeps every row + // whose workout was deleted. + WORKOUT_INSIGHT_CLAIM_CLEANUP_QUEUE, // v1.23 — free-text health-note encryption backfill. Boot discovery + a // daily discovery cron enqueue one per-user job per account still holding a // plaintext note; without this entry pg-boss never provisions the queue and @@ -605,6 +618,12 @@ const schedules: ScheduleEntry[] = [ [MEASUREMENT_TOMBSTONE_CLEANUP_QUEUE, MEASUREMENT_TOMBSTONE_CLEANUP_CRON], // v1.18.7 — daily 03:50 Europe/Berlin prune for stale Coach history. [COACH_MESSAGE_CLEANUP_QUEUE, COACH_MESSAGE_CLEANUP_CRON, cronIsTheRetry], + // v1.40 — daily 03:52 Europe/Berlin prune for the workout-insight claims. + [ + WORKOUT_INSIGHT_CLAIM_CLEANUP_QUEUE, + WORKOUT_INSIGHT_CLAIM_CLEANUP_CRON, + cronIsTheRetry, + ], // v1.23 — daily 03:55 Europe/Berlin note-encryption backfill discovery. // The empty cron payload (no `userId`) is the handler's signal to fan out // one per-user job per account still holding a plaintext note. @@ -918,6 +937,14 @@ export async function registerMaintenanceQueues( { localConcurrency: 1 }, handleCoachMessageCleanup, ); + // v1.40 — daily prune of the workout-insight claim ledger. Single-flight + // like every other cleanup queue. + await createAndWork( + boss, + WORKOUT_INSIGHT_CLAIM_CLEANUP_QUEUE, + { localConcurrency: 1 }, + handleWorkoutInsightClaimCleanup, + ); // MCP Phase 3 (M2) — daily prune of dead MCP connector access tokens + // long-revoked connection anchors. Single-flight like every other cleanup. await createAndWork( diff --git a/src/lib/jobs/subsystem-surface.ts b/src/lib/jobs/subsystem-surface.ts index a67847565..42703adc5 100644 --- a/src/lib/jobs/subsystem-surface.ts +++ b/src/lib/jobs/subsystem-surface.ts @@ -124,6 +124,7 @@ const subsystemSurface = { "achievement-unlock-sweep": { audience: "system" }, "measurement-tombstone-cleanup": { audience: "system" }, "coach-message-cleanup": { audience: "system" }, + "workout-insight-claim-cleanup": { audience: "system" }, "note-encryption-backfill": { audience: "system" }, "encryption-key-rotate": { audience: "system" }, "mcp-token-cleanup": { audience: "system" }, diff --git a/src/lib/jobs/workout-insight-claim-cleanup.ts b/src/lib/jobs/workout-insight-claim-cleanup.ts new file mode 100644 index 000000000..b51bd799b --- /dev/null +++ b/src/lib/jobs/workout-insight-claim-cleanup.ts @@ -0,0 +1,30 @@ +/** + * Daily retention for `workout_insight_generation_claims`. + * + * The claim row is the ledger behind the per-workout insight daily cap, and it + * outlives its workout on purpose: deleting a workout nulls the claim's + * `workout_id` instead of removing the row, so the delete cannot free the + * day's slot. The cap only ever reads today's rows, so anything older than the + * retention window is dead weight and is dropped here. + * + * Keyed on `updated_at`, which moves on every claim, reclaim, provider + * invocation and completion. A row written within the window is kept whatever + * its `local_date` says, so no row that the cap can still read is ever + * deleted. + */ +import type { PrismaClient } from "@/generated/prisma/client"; + +export const WORKOUT_INSIGHT_CLAIM_RETENTION_DAYS = 7; + +export async function cleanupOldWorkoutInsightClaims( + prisma: Pick, + now: Date = new Date(), +): Promise { + const cutoff = new Date( + now.getTime() - WORKOUT_INSIGHT_CLAIM_RETENTION_DAYS * 86_400_000, + ); + const { count } = await prisma.workoutInsightGenerationClaim.deleteMany({ + where: { updatedAt: { lt: cutoff } }, + }); + return count; +} diff --git a/src/lib/jobs/workout-insight-generate.ts b/src/lib/jobs/workout-insight-generate.ts index cff606fe0..083f77cd2 100644 --- a/src/lib/jobs/workout-insight-generate.ts +++ b/src/lib/jobs/workout-insight-generate.ts @@ -55,7 +55,6 @@ import { withBackgroundEvent } from "@/lib/logging/background"; import { aiCapabilityForJob } from "@/lib/ai/capabilities/gate"; import { userDayKey } from "@/lib/tz/format"; import { resolveUserTimezone } from "@/lib/tz/resolver"; -import { startOfLocalDayInTz } from "@/lib/tz/local-day"; import { buildWorkoutHrSeries } from "@/lib/workouts/hr-series"; import { buildWorkoutInsightEvidence, @@ -98,10 +97,42 @@ type ClaimWorkoutInsightGenerationArgs = { userId: string; workoutId: string; localDate: string; - dayStart: Date; now: Date; }; +/** + * Which claim rows spend one of the day's slots. + * + * The claim row is the ledger. A slot is spent by every attempt that reached + * the provider (completed or not, and whether or not its workout still exists: + * deleting a workout nulls the claim's `workout_id` instead of removing the + * row, so the delete cannot buy the slot back) plus every live lease that may + * still reach the provider. A lease past its expiry that never invoked the + * provider spent nothing and is reclaimable. Rows older than a week are + * dropped by `workout-insight-claim-cleanup`. + */ +export function workoutInsightCapWhere({ + userId, + localDate, + staleBefore, + excludeClaimRowId, +}: { + userId: string; + localDate: string; + staleBefore: Date; + excludeClaimRowId?: string; +}) { + return { + userId, + localDate, + ...(excludeClaimRowId ? { id: { not: excludeClaimRowId } } : {}), + OR: [ + { providerInvokedAt: { not: null } }, + { claimId: { not: null }, claimedAt: { gte: staleBefore } }, + ], + }; +} + /** * Serialize the local-day capacity check with the durable claim insert. * PostgreSQL advisory transaction locks keep the count and write atomic across @@ -112,7 +143,6 @@ export async function claimWorkoutInsightGeneration({ userId, workoutId, localDate, - dayStart, now, }: ClaimWorkoutInsightGenerationArgs): Promise { const claimId = randomUUID(); @@ -153,20 +183,15 @@ export async function claimWorkoutInsightGeneration({ return { status: "skipped", reason: "already_claimed" }; } - const [generatedToday, claimedToday] = await Promise.all([ - tx.workoutInsight.count({ - where: { userId, generatedAt: { gte: dayStart } }, + const spentToday = await tx.workoutInsightGenerationClaim.count({ + where: workoutInsightCapWhere({ + userId, + localDate, + staleBefore, + excludeClaimRowId: existing?.id, }), - tx.workoutInsightGenerationClaim.count({ - where: { - userId, - localDate, - completedAt: null, - ...(existing ? { workoutId: { not: workoutId } } : {}), - }, - }), - ]); - if (generatedToday + claimedToday >= MAX_INSIGHTS_PER_DAY) { + }); + if (spentToday >= MAX_INSIGHTS_PER_DAY) { return { status: "skipped", reason: "daily_cap" }; } @@ -270,13 +295,11 @@ export async function runWorkoutInsightGenerate( // The capacity check and durable claim write are one serialized PostgreSQL // transaction. A count performed before the write would let concurrent // workouts all observe the same free slot and exceed the cap. - const dayStart = startOfLocalDayInTz(now, tz); const localDate = userDayKey(now, tz); const claim = await claimWorkoutInsightGeneration({ userId, workoutId, localDate, - dayStart, now, }); if (claim.status === "skipped") { diff --git a/tests/integration/workout-insight-claim.test.ts b/tests/integration/workout-insight-claim.test.ts index 1fe49840f..534af38f6 100644 --- a/tests/integration/workout-insight-claim.test.ts +++ b/tests/integration/workout-insight-claim.test.ts @@ -11,8 +11,9 @@ * a paragraph that outlived its workout would be an orphaned description of * a session the user believes they erased. That is a data-retention claim, * not a tidiness one, and only the database enforces it. - * - **The daily-cap count** reads an index over `(user_id, generated_at)`. - * Whether the window actually selects the right rows is a SQL fact. + * - **The daily-cap count** reads the claim ledger, which outlives a deleted + * workout (`ON DELETE SET NULL`). Whether a delete frees a slot is an FK + * fact, so only the database can prove it does not. * * Every test writes through Prisma the way the worker does, so a schema change * that dropped a constraint fails here rather than in production. @@ -21,6 +22,7 @@ import { beforeEach, describe, expect, it } from "vitest"; import { getPrismaClient, truncateAllTables } from "./setup"; import { claimWorkoutInsightGeneration } from "@/lib/jobs/workout-insight-generate"; +import { cleanupOldWorkoutInsightClaims } from "@/lib/jobs/workout-insight-claim-cleanup"; beforeEach(async () => { await truncateAllTables(getPrismaClient()); @@ -189,67 +191,8 @@ describe("WorkoutInsight — cascade", () => { }); }); -describe("WorkoutInsight — the daily-cap count", () => { - it("counts only this user's rows inside the window", async () => { - const prisma = getPrismaClient(); - const mine = await createUser("wi-cap-mine"); - const other = await createUser("wi-cap-other"); - - // Three of mine today, one of mine yesterday, one belonging to someone else. - const today = new Date("2026-07-18T09:00:00Z"); - for (let i = 0; i < 3; i++) { - const w = await createWorkout( - mine.id, - new Date(today.getTime() + i * 3600_000), - `ext-today-${i}`, - ); - await prisma.workoutInsight.create({ - data: insightData( - mine.id, - w.id, - new Date(today.getTime() + i * 3600_000), - ), - }); - } - const yesterdayWorkout = await createWorkout( - mine.id, - new Date("2026-07-17T09:00:00Z"), - "ext-yesterday", - ); - await prisma.workoutInsight.create({ - data: insightData( - mine.id, - yesterdayWorkout.id, - new Date("2026-07-17T09:30:00Z"), - ), - }); - const otherWorkout = await createWorkout( - other.id, - new Date("2026-07-18T09:00:00Z"), - "ext-other", - ); - await prisma.workoutInsight.create({ - data: insightData( - other.id, - otherWorkout.id, - new Date("2026-07-18T10:00:00Z"), - ), - }); - - const dayStart = new Date("2026-07-18T00:00:00Z"); - const count = await prisma.workoutInsight.count({ - where: { userId: mine.id, generatedAt: { gte: dayStart } }, - }); - - // Not 4 (yesterday's is outside the window) and not 5 (the other user's is - // outside the tenancy narrow). - expect(count).toBe(3); - }); -}); - describe("WorkoutInsightGenerationClaim — concurrent ownership and cap", () => { const now = new Date("2026-07-18T15:00:00.000Z"); - const dayStart = new Date("2026-07-17T22:00:00.000Z"); const localDate = "2026-07-18"; it("atomically grants at most four different workouts for one user-local day", async () => { @@ -271,7 +214,6 @@ describe("WorkoutInsightGenerationClaim — concurrent ownership and cap", () => userId: user.id, workoutId: workout.id, localDate, - dayStart, now, }), ), @@ -315,7 +257,6 @@ describe("WorkoutInsightGenerationClaim — concurrent ownership and cap", () => userId: user.id, workoutId: workout.id, localDate, - dayStart, now, }); @@ -329,49 +270,132 @@ describe("WorkoutInsightGenerationClaim — concurrent ownership and cap", () => expect(rows[0]?.claimId).toBe(outcome.claimId); }); - it("counts a pre-migration insight row against the local-day cap", async () => { + it("keeps a deleted workout's slot spent, so a re-add hits the cap", async () => { + // Generate four, delete them all, add a fifth: the deletes must not buy + // the day's slots back. The claim row outlives its workout with a null + // workout id and is still counted. const prisma = getPrismaClient(); - const user = await createUser("wi-legacy-cap"); - const legacyWorkout = await createWorkout( - user.id, - new Date("2026-07-18T07:00:00.000Z"), - "ext-legacy", - ); - await prisma.workoutInsight.create({ - data: insightData( - user.id, - legacyWorkout.id, - new Date("2026-07-18T08:00:00.000Z"), - ), - }); - const candidates = await Promise.all( + const user = await createUser("wi-delete-cap"); + const first = await Promise.all( Array.from({ length: 4 }, (_, index) => createWorkout( user.id, new Date(`2026-07-18T${10 + index}:00:00.000Z`), - `ext-new-${index}`, + `ext-first-${index}`, ), ), ); + for (const workout of first) { + const outcome = await claimWorkoutInsightGeneration({ + userId: user.id, + workoutId: workout.id, + localDate, + now, + }); + expect(outcome.status).toBe("claimed"); + // The worker marks the provider call and completes the claim. + await prisma.workoutInsightGenerationClaim.update({ + where: { workoutId: workout.id }, + data: { + providerInvokedAt: now, + completedAt: now, + claimId: null, + claimedAt: null, + }, + }); + await prisma.workoutInsight.create({ + data: insightData(user.id, workout.id, now), + }); + } - const outcomes = await Promise.all( - candidates.map((workout) => - claimWorkoutInsightGeneration({ + await prisma.workout.deleteMany({ + where: { id: { in: first.map((w) => w.id) } }, + }); + + expect(await prisma.workoutInsight.count()).toBe(0); + const orphans = await prisma.workoutInsightGenerationClaim.findMany({ + where: { userId: user.id }, + }); + expect(orphans).toHaveLength(4); + expect(orphans.every((row) => row.workoutId === null)).toBe(true); + + const readded = await createWorkout( + user.id, + new Date("2026-07-18T14:30:00.000Z"), + "ext-readded", + ); + const outcome = await claimWorkoutInsightGeneration({ + userId: user.id, + workoutId: readded.id, + localDate, + now, + }); + expect(outcome).toEqual({ status: "skipped", reason: "daily_cap" }); + }); + + it("does not count a lapsed lease that never reached the provider", async () => { + const prisma = getPrismaClient(); + const user = await createUser("wi-lapsed-lease"); + const lapsed = await Promise.all( + Array.from({ length: 4 }, (_, index) => + createWorkout( + user.id, + new Date(Date.UTC(2026, 6, 18, 6 + index)), + `ext-lapsed-${index}`, + ), + ), + ); + for (const workout of lapsed) { + await prisma.workoutInsightGenerationClaim.create({ + data: { userId: user.id, workoutId: workout.id, localDate, - dayStart, - now, - }), - ), + claimId: `dead-${workout.id}`, + claimedAt: new Date("2026-07-18T12:00:00.000Z"), + }, + }); + } + const fresh = await createWorkout( + user.id, + new Date("2026-07-18T14:00:00.000Z"), + "ext-fresh", ); - expect( - outcomes.filter((outcome) => outcome.status === "claimed"), - ).toHaveLength(3); - expect(outcomes).toContainEqual({ - status: "skipped", - reason: "daily_cap", + const outcome = await claimWorkoutInsightGeneration({ + userId: user.id, + workoutId: fresh.id, + localDate, + now, }); + expect(outcome.status).toBe("claimed"); + }); +}); + +describe("WorkoutInsightGenerationClaim — retention", () => { + it("drops rows untouched for a week and keeps the rest", async () => { + const prisma = getPrismaClient(); + const user = await createUser("wi-claim-retention"); + const now = new Date(); + const old = await prisma.workoutInsightGenerationClaim.create({ + data: { userId: user.id, workoutId: null, localDate: "2000-01-01" }, + }); + const recent = await prisma.workoutInsightGenerationClaim.create({ + data: { userId: user.id, workoutId: null, localDate: "2000-01-02" }, + }); + // `updated_at` is Prisma-managed; age one row through raw SQL. + await prisma.$executeRaw` + UPDATE "workout_insight_generation_claims" + SET "updated_at" = ${new Date(now.getTime() - 8 * 86_400_000)} + WHERE "id" = ${old.id} + `; + + const deleted = await cleanupOldWorkoutInsightClaims(prisma, now); + + expect(deleted).toBe(1); + const left = await prisma.workoutInsightGenerationClaim.findMany({ + select: { id: true }, + }); + expect(left).toEqual([{ id: recent.id }]); }); }); From 6bb65824bf8722471ed7c07a05189d62bf4d1ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 17:54:04 +0200 Subject: [PATCH 02/15] fix(workouts): serialise record detection with a workout delete The workout-slot detection reads the best workout and writes its record inside one transaction under a per-user advisory lock, and the workout DELETE takes the same lock before removing records and the workout. A delete can no longer land between the read and the write and leave a record for a workout that is gone. The DELETE's match for a legacy record without an external id is scoped to the slots of the deleted workout's sport, so a record another sport set at the same instant stays. --- src/app/api/workouts/[id]/route.ts | 25 +++- src/app/api/workouts/__tests__/delete.test.ts | 26 +++- .../__tests__/pr-detection-worker.test.ts | 9 +- .../personal-records/pr-detection-worker.ts | 138 +++++++++++------- 4 files changed, 140 insertions(+), 58 deletions(-) diff --git a/src/app/api/workouts/[id]/route.ts b/src/app/api/workouts/[id]/route.ts index 21f04d8a5..a5ff667fa 100644 --- a/src/app/api/workouts/[id]/route.ts +++ b/src/app/api/workouts/[id]/route.ts @@ -36,6 +36,10 @@ import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { enqueuePrDetection } from "@/lib/jobs/pr-detection"; +import { + lockPersonalRecordsForUser, + workoutPrSlotsForSport, +} from "@/lib/personal-records/pr-detection-worker"; import { pickCanonicalWorkoutRows } from "@/lib/measurements/pick-canonical-workout-rows"; import { requireModuleEnabled } from "@/lib/modules/gate"; import { aiCapabilityToServe } from "@/lib/ai/capabilities/gate"; @@ -380,7 +384,9 @@ export const GET = apiHandler( * (`module-route-gate-inventory.test.ts`). * * The workout table carries no tombstone, so the row is removed outright; - * route, samples, insight and claim rows cascade with it. A personal record + * route, samples and insight rows cascade with it. The insight's generation + * claim stays behind with a null workout id: it is the day's cap ledger, and + * a delete must not hand the slot back. A personal record * the workout set would otherwise outlive it (the detector only ever raises * a stored best), so the records keyed on this workout go in the same * transaction and a silent detection pass re-derives the honest best. @@ -421,14 +427,25 @@ export const DELETE = apiHandler( } const removedRecords = await prisma.$transaction(async (tx) => { + // The detector reads the best workout and writes its record under this + // same lock, so it either wrote before this delete (and the record goes + // below) or reads after it (and never sees the workout). + await lockPersonalRecordsForUser(tx, user.id); const records = await tx.personalRecord.deleteMany({ where: { userId: user.id, - metricSlot: { not: null }, source: "MANUAL", ...(row.externalId !== null - ? { externalId: row.externalId } - : { externalId: null, achievedAt: row.startedAt }), + ? { metricSlot: { not: null }, externalId: row.externalId } + : { + // A legacy record carries no external id, so the start time + // is all that ties it to this workout. Scope it to the slots + // of this workout's sport so a record another sport set at + // the same instant stays. + metricSlot: { in: workoutPrSlotsForSport(row.sportType) }, + externalId: null, + achievedAt: row.startedAt, + }), }, }); await tx.workout.delete({ where: { id: row.id } }); diff --git a/src/app/api/workouts/__tests__/delete.test.ts b/src/app/api/workouts/__tests__/delete.test.ts index 340fd2cee..8af47cf6a 100644 --- a/src/app/api/workouts/__tests__/delete.test.ts +++ b/src/app/api/workouts/__tests__/delete.test.ts @@ -13,6 +13,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { NextRequest } from "next/server"; const tx = { + $queryRaw: vi.fn(async () => []), personalRecord: { deleteMany: vi.fn() }, workout: { delete: vi.fn() }, }; @@ -129,8 +130,16 @@ describe("DELETE /api/workouts/{id}", () => { expect(tx.personalRecord.deleteMany).toHaveBeenCalledWith({ where: { userId: "user-1", - metricSlot: { not: null }, source: "MANUAL", + // Only the running slots: a cycling record set at the same instant + // is not this workout's. + metricSlot: { + in: [ + "longest_run_duration", + "longest_distance_run", + "fastest_5km_time", + ], + }, externalId: null, achievedAt: MANUAL_ROW.startedAt, }, @@ -139,6 +148,21 @@ describe("DELETE /api/workouts/{id}", () => { expect(enqueuePrDetection).not.toHaveBeenCalled(); }); + it("takes the personal-records lock before it deletes anything", async () => { + vi.mocked(prisma.workout.findUnique).mockResolvedValue(MANUAL_ROW as never); + + await del(); + + expect(tx.$queryRaw).toHaveBeenCalledTimes(1); + const lockOrder = tx.$queryRaw.mock.invocationCallOrder[0]; + expect(lockOrder).toBeLessThan( + tx.personalRecord.deleteMany.mock.invocationCallOrder[0], + ); + expect(lockOrder).toBeLessThan( + tx.workout.delete.mock.invocationCallOrder[0], + ); + }); + it.each(["APPLE_HEALTH", "WITHINGS", "WHOOP", "EXTERNAL"])( "refuses a %s workout with 409 and touches nothing", async (source) => { diff --git a/src/lib/personal-records/__tests__/pr-detection-worker.test.ts b/src/lib/personal-records/__tests__/pr-detection-worker.test.ts index 9b3281d3d..cbaf12d2b 100644 --- a/src/lib/personal-records/__tests__/pr-detection-worker.test.ts +++ b/src/lib/personal-records/__tests__/pr-detection-worker.test.ts @@ -143,7 +143,7 @@ function makeFakePrisma(state: { return true; } - return { + const fake = { // v1.29.6 — the cumulative-kind path no longer runs a `$queryRaw` // UTC `date_trunc` aggregate; it reads every matching row via // `measurement.findMany` and does the source-collapse + tz-aware @@ -277,6 +277,13 @@ function makeFakePrisma(state: { ), }, } as unknown as PrismaClient; + // The workout-slot leg runs under a per-user advisory lock inside a + // transaction; the fake runs the callback against itself. + Object.assign(fake, { + $queryRaw: vi.fn(async () => []), + $transaction: vi.fn(async (cb: (tx: PrismaClient) => unknown) => cb(fake)), + }); + return fake; } const USER = "user-1"; diff --git a/src/lib/personal-records/pr-detection-worker.ts b/src/lib/personal-records/pr-detection-worker.ts index ae7ef6f80..d55e5c6dc 100644 --- a/src/lib/personal-records/pr-detection-worker.ts +++ b/src/lib/personal-records/pr-detection-worker.ts @@ -386,58 +386,65 @@ export async function detectPersonalRecordsForUser( [slot.field]: { gt: 0 } as { gt: number }, } as const; - const sampleCount = await prisma.workout.count({ - where: baseWhere, + // Read the best workout and write its record under the same per-user + // lock the workout DELETE takes. Without it a delete could land between + // the read and the write and leave a record for a workout that no longer + // exists (PersonalRecord has no FK to Workout to stop it). + const outcome = await prisma.$transaction(async (tx) => { + await lockPersonalRecordsForUser(tx, userId); + + const sampleCount = await tx.workout.count({ + where: baseWhere, + }); + if (sampleCount < PR_DETECTION_WARMUP_THRESHOLD) return null; + + const best = await findBestWorkout(tx, baseWhere, slot); + if (!best) return null; + + const value = + slot.field === "durationSec" + ? best.durationSec + : (best.totalDistanceM ?? 0); + if (!Number.isFinite(value) || value <= 0) return null; + + const currentPR = await tx.personalRecord.findFirst({ + where: { userId, metricType: slot.metricType, metricSlot: slot.slot }, + orderBy: + slot.direction === PersonalRecordDirection.MAX + ? { value: "desc" } + : { value: "asc" }, + }); + + const comparison = compareToCurrentBest( + value, + currentPR?.value, + slot.direction, + ); + if (comparison === "no-improvement") return null; + + const result = await tx.personalRecord.createMany({ + data: [ + { + userId, + metricType: slot.metricType, + metricSlot: slot.slot, + direction: slot.direction, + value, + unit: slot.unit, + achievedAt: best.startedAt, + sourceMeasurementId: null, + source: best.source, + externalId: best.externalId, + }, + ], + skipDuplicates: true, + }); + return result.count > 0 ? comparison : null; }); - if (sampleCount < PR_DETECTION_WARMUP_THRESHOLD) continue; - - const best = await findBestWorkout(prisma, baseWhere, slot); - if (!best) continue; - - const value = - slot.field === "durationSec" - ? best.durationSec - : (best.totalDistanceM ?? 0); - if (!Number.isFinite(value) || value <= 0) continue; - - const currentPR = await prisma.personalRecord.findFirst({ - where: { userId, metricType: slot.metricType, metricSlot: slot.slot }, - orderBy: - slot.direction === PersonalRecordDirection.MAX - ? { value: "desc" } - : { value: "asc" }, - }); - - const outcome = compareToCurrentBest( - value, - currentPR?.value, - slot.direction, - ); - if (outcome === "no-improvement") continue; - - const result = await prisma.personalRecord.createMany({ - data: [ - { - userId, - metricType: slot.metricType, - metricSlot: slot.slot, - direction: slot.direction, - value, - unit: slot.unit, - achievedAt: best.startedAt, - sourceMeasurementId: null, - source: best.source, - externalId: best.externalId, - }, - ], - skipDuplicates: true, - }); - if (result.count > 0) { - if (outcome === "tie") { - ties += 1; - } else { - inserted += 1; - } + if (outcome === "tie") { + ties += 1; + } else if (outcome === "improvement") { + inserted += 1; } } @@ -669,7 +676,7 @@ async function findBestCumulativeDay( } async function findBestWorkout( - prisma: PrismaClient, + prisma: Pick, where: Prisma.WorkoutWhereInput, slot: WorkoutSlotDefinition, ): Promise { @@ -704,6 +711,33 @@ async function findBestWorkout( }; } +/** + * The per-user lock shared by the workout-slot detection and the workout + * DELETE. Transaction-scoped: it releases on commit or rollback. + */ +export async function lockPersonalRecordsForUser( + tx: Pick, + userId: string, +): Promise { + await tx.$queryRaw` + SELECT pg_advisory_xact_lock( + hashtext('personal-records'), + hashtext(${userId}) + )::text AS locked + `; +} + +/** + * The workout-PR slot names the detector derives for one sport. The DELETE + * uses it to scope a legacy record match (no external id) to the sport of the + * workout being removed. + */ +export function workoutPrSlotsForSport(sportType: string): string[] { + return WORKOUT_SLOTS.filter((slot) => slot.sportType === sportType).map( + (slot) => slot.slot, + ); +} + /** Exposed for tests + the integration harness. */ export const __test__ = { WORKOUT_SLOTS, From 916f918674fc96cd9515b51f2e812f03e9e33804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 17:59:20 +0200 Subject: [PATCH 03/15] feat(workouts): save an edited resubmit of a hand-entered workout The manual form sends one manual: id per opening and resends it after an edit. The batch route now treats a MANUAL entry with a manual: id as an overwrite, the way stats: measurements are: the stored row takes the new values and the entry reports "updated", or "duplicate" when nothing changed. Records keyed on the workout are removed under the personal-records lock and re-derived. Every other id stays first-write-wins. The status enum gains "updated". --- docs/api/openapi.yaml | 20 ++- src/app/api/workouts/batch/route.ts | 170 +++++++++++++++++- .../__tests__/manual-workout-form.test.tsx | 12 ++ .../workouts/manual-workout-form.tsx | 7 +- src/lib/openapi/routes/workouts.ts | 6 +- .../integration/workout-manual-entry.test.ts | 75 ++++++++ 6 files changed, 271 insertions(+), 19 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index f66443ff6..d066ed6f3 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -4706,13 +4706,14 @@ paths: Server-side ingest endpoint for HKWorkout records (iOS) and Withings activity rows (server-to-server). Up to 100 workouts per call; nested route geometry (GeoJSON LineString) is capped at 20 000 points and stored in a 1:1 `WorkoutRoute` row keyed by `workoutId`. Idempotent via the `Idempotency-Key` header (replay window 24h). - Per-entry status (`inserted | duplicate | enriched | skipped`) lets the iOS sync cursor checkpoint accurately. - An entry matching a stored workout is first-write-wins for the workout row; when it carries a `samples` array - and the stored workout has no heart-rate series, the series is attached and the entry reports `enriched` - (repeatable: a workout that already has a series is a no-op). An entry whose `samples` array fails validation is - refused on its own with `reason: "invalid_samples"` — the rest of the batch still lands. The request body - ceiling is 5 MB enforced at the HTTP layer — clients above the ceiling receive a 413 with - `workout.batch.payload_too_large`. + Per-entry status (`inserted | duplicate | enriched | updated | skipped`) lets the iOS sync cursor checkpoint + accurately. An entry matching a stored workout is first-write-wins for the workout row, except a hand-entered + one (`source: MANUAL` with a `manual:` externalId), which a resend with other values overwrites and reports + `updated`; when it carries a `samples` array and the stored workout has no heart-rate series, the series is + attached and the entry reports `enriched` (repeatable: a workout that already has a series is a no-op). An entry + whose `samples` array fails validation is refused on its own with `reason: "invalid_samples"` — the rest of the + batch still lands. The request body ceiling is 5 MB enforced at the HTTP layer — clients above the ceiling + receive a 413 with `workout.batch.payload_too_large`. **Narrow token.** Also reachable with a `workouts:write` Bearer (minted at `POST /api/tokens/workouts`), the @@ -43190,11 +43191,14 @@ components: - inserted - duplicate - enriched + - updated - skipped description: "`inserted`: a new workout row. `duplicate`: the workout was already stored and nothing was written. `enriched`: the workout was already stored and the heart-rate series this entry carried was attached to it; the workout row itself was not written, and the entry also counts towards the top-level `duplicates`. - `skipped`: the server refused the entry, see `reason`." + `updated` (since v1.40.0): a workout entered by hand (`source: MANUAL`, `externalId` starting `manual:`) was + sent again with different values and the stored row now carries them; no row was inserted, so it also counts + towards `duplicates`. `skipped`: the server refused the entry, see `reason`." reason: description: "Why a `skipped` entry was refused: `unstable_external_id` for an id that does not survive a client restart, `invalid_samples` for a heart-rate series that failed validation." diff --git a/src/app/api/workouts/batch/route.ts b/src/app/api/workouts/batch/route.ts index 8c42af76a..5b885aa9e 100644 --- a/src/app/api/workouts/batch/route.ts +++ b/src/app/api/workouts/batch/route.ts @@ -77,6 +77,7 @@ import { import { withIdempotency } from "@/lib/idempotency"; import { checkRateLimit } from "@/lib/rate-limit"; import { enqueuePrDetection } from "@/lib/jobs/pr-detection"; +import { lockPersonalRecordsForUser } from "@/lib/personal-records/pr-detection-worker"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { emitDataArrival } from "@/lib/arrivals/emit-shared"; import { MAX_WORKOUTS_PER_BATCH } from "@/lib/validations/workout"; @@ -129,8 +130,63 @@ const MAX_BODY_BYTES = 5 * 1024 * 1024; * to a workout that had none, which is what lets a history sweep count * its own progress. `skipped` covers entries the server refused: an * unusable external id, or a `samples` array that failed validation. + * `updated` says a hand-entered workout (`manual:` id, source MANUAL) was + * sent again with different values and the stored row now carries them. */ -type EntryStatus = "inserted" | "duplicate" | "enriched" | "skipped"; +type EntryStatus = + "inserted" | "duplicate" | "enriched" | "updated" | "skipped"; + +/** The workout columns a hand-entered workout's resubmit may overwrite. */ +const MANUAL_OVERWRITE_FIELDS = [ + "sportType", + "startedAt", + "endedAt", + "durationSec", + "totalEnergyKcal", + "totalDistanceM", + "avgHeartRate", + "maxHeartRate", + "minHeartRate", + "stepCount", + "elevationM", + "pauseDurationSec", +] as const; +type ManualOverwriteField = (typeof MANUAL_OVERWRITE_FIELDS)[number]; +type ManualOverwriteValues = { + [K in ManualOverwriteField]: string | number | Date | null; +}; + +/** + * A workout entered by hand carries one id per form opening, and the form + * resends it with the edited values when the person corrects a submit. Such a + * resubmit is an edit of the stored row, the way a `stats:` measurement + * re-post is. Every other id stays first-write-wins. + */ +function isManualOverwrite(source: unknown, externalId: string | null) { + return ( + source === "MANUAL" && + externalId !== null && + externalId.startsWith("manual:") + ); +} + +/** The changed columns of a manual resubmit, or null when nothing differs. */ +function manualOverwriteChanges( + stored: ManualOverwriteValues, + sent: Prisma.WorkoutCreateManyInput, +): Partial | null { + const changes: Partial = {}; + for (const field of MANUAL_OVERWRITE_FIELDS) { + const next = (sent[field] ?? null) as string | number | Date | null; + const prev = stored[field]; + const same = + next instanceof Date && prev instanceof Date + ? next.getTime() === prev.getTime() + : next === prev; + if (!same) changes[field] = next; + } + return Object.keys(changes).length > 0 ? changes : null; +} interface EntryResult { index: number; status: EntryStatus; @@ -483,6 +539,7 @@ async function postBatch(request: NextRequest): Promise { let duplicateCount = droppedByWriteDedup.length; let insertedCount = 0; let enrichedCount = 0; + let updatedCount = 0; const insertedIdByIndex = new Map(); if (survivors.length > 0) { @@ -510,16 +567,45 @@ async function postBatch(request: NextRequest): Promise { source: true, externalId: true, samples: { select: { workoutId: true } }, + sportType: true, + startedAt: true, + endedAt: true, + durationSec: true, + totalEnergyKcal: true, + totalDistanceM: true, + avgHeartRate: true, + maxHeartRate: true, + minHeartRate: true, + stepCount: true, + elevationM: true, + pauseDurationSec: true, }, }) : []; - const existingByKey = new Map(); + const existingByKey = new Map< + string, + { id: string; hasSeries: boolean; values: ManualOverwriteValues } + >(); for (const row of existing) { if (row.externalId === null) continue; existingByKey.set(`${row.source}::${row.externalId}`, { id: row.id, hasSeries: row.samples !== null, + values: { + sportType: row.sportType, + startedAt: row.startedAt, + endedAt: row.endedAt, + durationSec: row.durationSec, + totalEnergyKcal: row.totalEnergyKcal, + totalDistanceM: row.totalDistanceM, + avgHeartRate: row.avgHeartRate, + maxHeartRate: row.maxHeartRate, + minHeartRate: row.minHeartRate, + stepCount: row.stepCount, + elevationM: row.elevationM, + pauseDurationSec: row.pauseDurationSec, + }, }); } @@ -530,6 +616,13 @@ async function postBatch(request: NextRequest): Promise { workoutId: string; samples: NonNullable; }> = []; + /** Hand-entered workouts this batch edits in place. */ + const toOverwrite: Array<{ + index: number; + workoutId: string; + externalId: string; + changes: Partial; + }> = []; /** Workouts already claimed by an earlier entry of THIS batch. */ const enrichClaimed = new Set(); for (const p of survivors) { @@ -537,7 +630,27 @@ async function postBatch(request: NextRequest): Promise { ? `${p.dedupKey.source}::${p.dedupKey.externalId}` : null; const known = keyTuple !== null ? existingByKey.get(keyTuple) : undefined; - if (known) { + if ( + known && + p.dedupKey && + isManualOverwrite(p.dedupKey.source, p.dedupKey.externalId) + ) { + const changes = enrichClaimed.has(known.id) + ? null + : manualOverwriteChanges(known.values, p.row); + if (changes) { + enrichClaimed.add(known.id); + toOverwrite.push({ + index: p.index, + workoutId: known.id, + externalId: p.dedupKey.externalId, + changes, + }); + } else { + results[p.index] = { index: p.index, status: "duplicate" }; + duplicateCount += 1; + } + } else if (known) { if (p.samples && !known.hasSeries && !enrichClaimed.has(known.id)) { enrichClaimed.add(known.id); // Enrichment. The status is assigned after the write, because @@ -560,12 +673,45 @@ async function postBatch(request: NextRequest): Promise { } } - if (toInsert.length > 0 || toEnrich.length > 0) { + if (toInsert.length > 0 || toEnrich.length > 0 || toOverwrite.length > 0) { const CHUNK = 100; /** Workout ids whose series row this request actually wrote. */ const seriesLanded = new Set(); + /** Batch indices whose manual edit this request actually wrote. */ + const overwritten = new Set(); await prisma.$transaction(async (tx) => { + // A hand-entered workout sent again with other values. Columns are + // copied one by one from the validated entry; the records the old + // values set go with them and the detection pass below re-derives + // the honest best, as the DELETE does. + if (toOverwrite.length > 0) { + await lockPersonalRecordsForUser(tx, user.id); + } + for (const o of toOverwrite) { + const data: Prisma.WorkoutUpdateManyMutationInput = {}; + for (const field of MANUAL_OVERWRITE_FIELDS) { + if (field in o.changes) { + (data as Record)[field] = o.changes[field]; + } + } + const written = await tx.workout.updateMany({ + where: { id: o.workoutId, userId: user.id, source: "MANUAL" }, + data, + }); + if (written.count === 1) { + overwritten.add(o.index); + await tx.personalRecord.deleteMany({ + where: { + userId: user.id, + source: "MANUAL", + metricSlot: { not: null }, + externalId: o.externalId, + }, + }); + } + } + const withExternalId = toInsert.filter((p) => p.dedupKey !== null); const withoutExternalId = toInsert.filter((p) => p.dedupKey === null); @@ -686,6 +832,18 @@ async function postBatch(request: NextRequest): Promise { if (seriesLanded.has(e.workoutId)) enrichedCount += 1; duplicateCount += 1; } + + // Like an enrichment, an edit inserts no row, so it counts towards + // `duplicates`; the per-entry status says the stored row changed. + for (const o of toOverwrite) { + const landed = overwritten.has(o.index); + results[o.index] = { + index: o.index, + status: landed ? "updated" : "duplicate", + }; + if (landed) updatedCount += 1; + duplicateCount += 1; + } } } @@ -701,6 +859,7 @@ async function postBatch(request: NextRequest): Promise { inserted: insertedCount, duplicates: duplicateCount, enriched: enrichedCount, + updated: updatedCount, skipped: skipped.length, }, }); @@ -739,6 +898,7 @@ async function postBatch(request: NextRequest): Promise { inserted: insertedCount, duplicates: duplicateCount, enriched: enrichedCount, + updated: updatedCount, skipped: skipped.length, }, }); @@ -750,7 +910,7 @@ async function postBatch(request: NextRequest): Promise { // An enrichment changes what the workout reads as — the detail seam // gains its curve, the list gains its glyph — so it busts the cache // too, even though no workout row was written. - if (insertedCount > 0 || enrichedCount > 0) { + if (insertedCount > 0 || enrichedCount > 0 || updatedCount > 0) { invalidateUserMeasurements(user.id); } diff --git a/src/components/workouts/__tests__/manual-workout-form.test.tsx b/src/components/workouts/__tests__/manual-workout-form.test.tsx index 64734040b..15f0d3818 100644 --- a/src/components/workouts/__tests__/manual-workout-form.test.tsx +++ b/src/components/workouts/__tests__/manual-workout-form.test.tsx @@ -138,6 +138,18 @@ describe("saveManualWorkout — the wire", () => { ).resolves.toBe("duplicate"); }); + it("treats an edited resubmit (`updated`) as saved and refreshes", async () => { + fetchSpy.mockResolvedValue( + envelope({ entries: [{ index: 0, status: "updated" }] }), + ); + const client = fakeClient(); + + await expect( + saveManualWorkout(ENTRY, client as unknown as QueryClient), + ).resolves.toBe("updated"); + expect(client.invalidateQueries).toHaveBeenCalled(); + }); + it("throws when the route skipped the entry, and refreshes nothing", async () => { fetchSpy.mockResolvedValue( envelope({ entries: [{ index: 0, status: "skipped" }] }), diff --git a/src/components/workouts/manual-workout-form.tsx b/src/components/workouts/manual-workout-form.tsx index 9902cac66..a254605e4 100644 --- a/src/components/workouts/manual-workout-form.tsx +++ b/src/components/workouts/manual-workout-form.tsx @@ -59,7 +59,7 @@ interface BatchResult { entries?: Array<{ status: string }>; } -export type ManualWorkoutSaveOutcome = "inserted" | "duplicate"; +export type ManualWorkoutSaveOutcome = "inserted" | "duplicate" | "updated"; /** * Post the entry and refresh what shows workouts. A `duplicate` is a success: @@ -74,7 +74,7 @@ export async function saveManualWorkout( workouts: [entry], }); const status = result?.entries?.[0]?.status; - if (status !== "inserted" && status !== "duplicate") { + if (status !== "inserted" && status !== "duplicate" && status !== "updated") { throw new Error(`workout entry not stored: ${status ?? "unknown"}`); } // The list, the dashboard tile and every detail read sit under this @@ -112,7 +112,8 @@ export function ManualWorkoutForm({ const timezone = user?.timezone || DEFAULT_TIMEZONE; // Minted once per opened form and sent with every submit, so a second tap - // or a retried request lands as a duplicate rather than a second row. + // or a retried request lands as a duplicate rather than a second row, and + // a submit after an edit updates the row the first submit stored. const [externalId] = useState(newManualWorkoutExternalId); const [draft, setDraft] = useState(() => emptyManualWorkoutDraft(new Date(), timezone), diff --git a/src/lib/openapi/routes/workouts.ts b/src/lib/openapi/routes/workouts.ts index e8bda4fe9..037b49bb5 100644 --- a/src/lib/openapi/routes/workouts.ts +++ b/src/lib/openapi/routes/workouts.ts @@ -38,9 +38,9 @@ const workoutBatchEntryResult = z .object({ index: z.number().int().nonnegative(), status: z - .enum(["inserted", "duplicate", "enriched", "skipped"]) + .enum(["inserted", "duplicate", "enriched", "updated", "skipped"]) .describe( - "`inserted`: a new workout row. `duplicate`: the workout was already stored and nothing was written. `enriched`: the workout was already stored and the heart-rate series this entry carried was attached to it; the workout row itself was not written, and the entry also counts towards the top-level `duplicates`. `skipped`: the server refused the entry, see `reason`.", + "`inserted`: a new workout row. `duplicate`: the workout was already stored and nothing was written. `enriched`: the workout was already stored and the heart-rate series this entry carried was attached to it; the workout row itself was not written, and the entry also counts towards the top-level `duplicates`. `updated` (since v1.40.0): a workout entered by hand (`source: MANUAL`, `externalId` starting `manual:`) was sent again with different values and the stored row now carries them; no row was inserted, so it also counts towards `duplicates`. `skipped`: the server refused the entry, see `reason`.", ), reason: z .string() @@ -349,7 +349,7 @@ export const workoutPaths: NonNullable = { tags: ["Measurements"], summary: "Typed workout batch ingest (v1.4.25 W16b)", description: - 'Server-side ingest endpoint for HKWorkout records (iOS) and Withings activity rows (server-to-server). Up to 100 workouts per call; nested route geometry (GeoJSON LineString) is capped at 20 000 points and stored in a 1:1 `WorkoutRoute` row keyed by `workoutId`. Idempotent via the `Idempotency-Key` header (replay window 24h). Per-entry status (`inserted | duplicate | enriched | skipped`) lets the iOS sync cursor checkpoint accurately. An entry matching a stored workout is first-write-wins for the workout row; when it carries a `samples` array and the stored workout has no heart-rate series, the series is attached and the entry reports `enriched` (repeatable: a workout that already has a series is a no-op). An entry whose `samples` array fails validation is refused on its own with `reason: "invalid_samples"` — the rest of the batch still lands. The request body ceiling is 5 MB enforced at the HTTP layer — clients above the ceiling receive a 413 with `workout.batch.payload_too_large`.\n\n' + + 'Server-side ingest endpoint for HKWorkout records (iOS) and Withings activity rows (server-to-server). Up to 100 workouts per call; nested route geometry (GeoJSON LineString) is capped at 20 000 points and stored in a 1:1 `WorkoutRoute` row keyed by `workoutId`. Idempotent via the `Idempotency-Key` header (replay window 24h). Per-entry status (`inserted | duplicate | enriched | updated | skipped`) lets the iOS sync cursor checkpoint accurately. An entry matching a stored workout is first-write-wins for the workout row, except a hand-entered one (`source: MANUAL` with a `manual:` externalId), which a resend with other values overwrites and reports `updated`; when it carries a `samples` array and the stored workout has no heart-rate series, the series is attached and the entry reports `enriched` (repeatable: a workout that already has a series is a no-op). An entry whose `samples` array fails validation is refused on its own with `reason: "invalid_samples"` — the rest of the batch still lands. The request body ceiling is 5 MB enforced at the HTTP layer — clients above the ceiling receive a 413 with `workout.batch.payload_too_large`.\n\n' + "**Narrow token.** Also reachable with a `workouts:write` Bearer (minted at `POST /api/tokens/workouts`), the supported way to push workouts from a bridge that is not the phone. This route is the scope's only one. Such a credential writes only on its owner's own record; its workouts are attributed `EXTERNAL`, resolved from the credential, and it may not name a source itself, so a batch in which any entry carries one, `MANUAL` and `APPLE_HEALTH` alike, is refused 422 (`workout.batch.source_not_permitted`) and nothing is written. `EXTERNAL` gives those writes their own `(userId, source, externalId)` namespace, so a bridge's ids can never collide with the phone's. Every other narrow token, `measurements:write` included, is refused 403.", requestBody: { required: true, diff --git a/tests/integration/workout-manual-entry.test.ts b/tests/integration/workout-manual-entry.test.ts index b02b03f71..e6ee0bd20 100644 --- a/tests/integration/workout-manual-entry.test.ts +++ b/tests/integration/workout-manual-entry.test.ts @@ -9,6 +9,8 @@ * - the entry lands as `MANUAL`, with the start, duration and metres the * form computed, and the list serves it; * - a second submit of the same form is a `duplicate`, not a second row; + * - a submit after an edit is `updated` and the one row carries the edit, + * while a synced id with changed values stays first-write-wins; * - delete removes a hand-entered workout and refuses a synced one. */ import { NextRequest } from "next/server"; @@ -163,6 +165,79 @@ describe("a workout entered by hand", () => { ).toBe(1); }); + it("updates the stored row when the same form is sent again with an edit", async () => { + const prisma = getPrismaClient(); + const entry = formEntry(newManualWorkoutExternalId()); + await post({ workouts: [entry] }); + const stored = await prisma.workout.findFirstOrThrow({ + where: { userId: TEST_USER_ID }, + }); + // A record the first values set; the edit must not leave it behind. + await prisma.personalRecord.create({ + data: { + userId: TEST_USER_ID, + metricType: "WALKING_RUNNING_DISTANCE", + metricSlot: "longest_distance_run", + direction: "MAX", + value: 8046.7, + unit: "m", + achievedAt: stored.startedAt, + source: "MANUAL", + externalId: entry.externalId, + }, + }); + + const edited = { ...entry, totalDistanceM: 6000, totalEnergyKcal: 380 }; + const res = await post({ workouts: [edited] }); + + expect((await res.json()).data.entries).toEqual([ + { index: 0, status: "updated" }, + ]); + const rows = await prisma.workout.findMany({ + where: { userId: TEST_USER_ID }, + }); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + id: stored.id, + totalDistanceM: 6000, + totalEnergyKcal: 380, + durationSec: 45 * 60, + }); + expect( + await prisma.personalRecord.count({ + where: { userId: TEST_USER_ID, externalId: entry.externalId }, + }), + ).toBe(0); + + // The same edit once more changes nothing and says so. + const again = await post({ workouts: [edited] }); + expect((await again.json()).data.entries).toEqual([ + { index: 0, status: "duplicate" }, + ]); + }); + + it("keeps a synced workout first-write-wins under the same resend", async () => { + const prisma = getPrismaClient(); + const entry = { + ...formEntry("hk-uuid-1"), + source: "APPLE_HEALTH" as const, + externalId: "8F14E45F-CEEA-467A-9575-0F5D6C5A1B2C", + }; + await post({ workouts: [entry] }); + + const res = await post({ + workouts: [{ ...entry, totalDistanceM: 1234 }], + }); + + expect((await res.json()).data.entries).toEqual([ + { index: 0, status: "duplicate" }, + ]); + const row = await prisma.workout.findFirstOrThrow({ + where: { userId: TEST_USER_ID }, + }); + expect(row.totalDistanceM).not.toBe(1234); + }); + it("can be deleted, and is gone from the list", async () => { await post({ workouts: [formEntry(newManualWorkoutExternalId())] }); const stored = await getPrismaClient().workout.findFirstOrThrow({ From 7022b698e70829ca7c93df421af237b338ed3dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 18:04:09 +0200 Subject: [PATCH 04/15] fix(ai): forget the remembered working provider when the chain is saved The in-memory memo of the last provider that answered reorders the chain for up to an hour. Saving a new chain now drops it for that account, so a provider the person removed or disabled no longer leads the chain they just arranged. --- .../api/insights/provider-chain/__tests__/put.test.ts | 9 +++++++++ .../api/insights/provider-chain/__tests__/route.test.ts | 1 + src/app/api/insights/provider-chain/route.ts | 6 +++++- src/lib/ai/provider-runner.ts | 9 +++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/app/api/insights/provider-chain/__tests__/put.test.ts b/src/app/api/insights/provider-chain/__tests__/put.test.ts index 7721d1d46..510fa5fe7 100644 --- a/src/app/api/insights/provider-chain/__tests__/put.test.ts +++ b/src/app/api/insights/provider-chain/__tests__/put.test.ts @@ -61,8 +61,14 @@ vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn(), })); +vi.mock("@/lib/ai/provider-runner", () => ({ + getLastWorkingProvider: vi.fn(), + forgetLastWorkingProvider: vi.fn(), +})); + import { PUT } from "../route"; import { prisma } from "@/lib/db"; +import { forgetLastWorkingProvider } from "@/lib/ai/provider-runner"; import { annotate } from "@/lib/logging/context"; interface Envelope { @@ -108,6 +114,9 @@ describe("PUT /api/insights/provider-chain", () => { ], }); expect(annotate).toHaveBeenCalled(); + // The remembered working provider would reorder the chain the person + // just arranged; a save drops it for this account. + expect(forgetLastWorkingProvider).toHaveBeenCalledWith("u-1"); }); it("rejects unknown provider types with 422", async () => { diff --git a/src/app/api/insights/provider-chain/__tests__/route.test.ts b/src/app/api/insights/provider-chain/__tests__/route.test.ts index fff8209ea..efb66afab 100644 --- a/src/app/api/insights/provider-chain/__tests__/route.test.ts +++ b/src/app/api/insights/provider-chain/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/ai/provider", () => ({ vi.mock("@/lib/ai/provider-runner", () => ({ getLastWorkingProvider: vi.fn(), + forgetLastWorkingProvider: vi.fn(), })); vi.mock("@/lib/logging/context", () => ({ diff --git a/src/app/api/insights/provider-chain/route.ts b/src/app/api/insights/provider-chain/route.ts index b5afc4379..8ea535990 100644 --- a/src/app/api/insights/provider-chain/route.ts +++ b/src/app/api/insights/provider-chain/route.ts @@ -10,7 +10,10 @@ import { } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { resolveProviderChain } from "@/lib/ai/provider"; -import { getLastWorkingProvider } from "@/lib/ai/provider-runner"; +import { + forgetLastWorkingProvider, + getLastWorkingProvider, +} from "@/lib/ai/provider-runner"; import { prisma } from "@/lib/db"; import { parseProviderChain, @@ -149,6 +152,7 @@ export const PUT = apiHandler(async (request: NextRequest) => { where: { id: user.id }, data: { aiProviderChain: normalised }, }); + forgetLastWorkingProvider(user.id); annotate({ meta: { diff --git a/src/lib/ai/provider-runner.ts b/src/lib/ai/provider-runner.ts index 37270e54f..7bf914106 100644 --- a/src/lib/ai/provider-runner.ts +++ b/src/lib/ai/provider-runner.ts @@ -98,6 +98,15 @@ export function getLastWorkingProvider( return entry.providerType; } +/** + * Drop one user's "last working provider" memo. Called when the person saves + * a new chain: the memo reorders the chain they just arranged, and a provider + * they removed or disabled must not keep leading it for up to an hour. + */ +export function forgetLastWorkingProvider(userId: string): void { + lastWorkingCache.delete(userId); +} + function rememberWorkingProvider( userId: string, providerType: ProviderChainType, From dd7636e14764993ed6fe7ab5ee049e3fac6180cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 18:19:50 +0200 Subject: [PATCH 05/15] feat(labs): refuse a biomarker unit change while readings are linked Readers label every linked reading with the marker's unit, so changing it relabelled stored numbers without converting them. PUT /api/biomarkers/{id} now answers 422 biomarkers.unit.locked with meta.readingCount when the unit differs and any reading (tombstoned ones included) is linked. The marker editor says so under the unit field. --- docs/api/openapi.yaml | 54 ++++++++------- src/app/api/biomarkers/[id]/route.ts | 22 ++++++ src/components/labs/biomarker-form.tsx | 10 +++ src/lib/openapi/error-codes.ts | 1 + src/lib/openapi/routes/biomarkers.ts | 2 +- tests/integration/biomarker-detail.test.ts | 81 +++++++++++++++++++++- 6 files changed, 143 insertions(+), 27 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index d066ed6f3..4e702ad03 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -14369,8 +14369,11 @@ paths: tags: - Labs summary: Edit a biomarker - description: Partial edit; omitted fields are untouched, an explicit null clears `context` / `panel` / a bound. Audits - as `biomarker.update`. + description: "Partial edit; omitted fields are untouched, an explicit null clears `context` / `panel` / a bound. Audits + as `biomarker.update`. Since v1.40.0 a `unit` different from the stored one is refused with 422 + `biomarkers.unit.locked` while any reading is linked to the marker (`meta.readingCount` carries how many): + readers label every linked reading with the marker's unit, so a change would relabel stored numbers without + converting them. Sending the current unit, or changing it on a marker with no readings, is accepted." parameters: - in: path name: id @@ -31719,29 +31722,30 @@ components: auth.reproof.failed, auth.reproof.required, auth.reproof.too_weak, auth.scope.insufficient, auth.stepup.required, auth.token.expired, auth.token.invalid; backup — backup.foreign_reference, backup.key.missing, backup.payload.undecryptable, backup.restore.active, backup.section.missing, - backup.upload.too_large; body-site — body-site.invalid; coach — coach.budget.exceeded, - coach.conversation.invalidTitle, coach.conversation.notFound, coach.fenced.attachmentConflict, - coach.fenced.attachmentLimit, coach.fenced.attachmentNotFound, coach.fenced.attachmentNotIndexed, - coach.fenced.attachmentRequired, coach.fenced.attachmentUnavailable, coach.fenced.invalid, - coach.request.invalid; coach-prefs — coach-prefs.body.invalid_json, coach-prefs.body.invalid_shape; - consent — consent.ai.required; cycle — cycle.bulk.invalid, cycle.bulk.too_large, cycle.calendar.invalid, - cycle.calendar.range, cycle.cycles.invalid, cycle.day-log.conflict, cycle.day-log.invalid, - cycle.disabled, cycle.period.invalid, cycle.symptom.custom.invalid, cycle.symptom.custom.limit; - cycle-prefs — cycle-prefs.body.invalid_json, cycle-prefs.invalid; disable-coach — - disable-coach.body.invalid_json; documents — documents.chat.conversationNotFound, - documents.chat.invalid, documents.inbound.alreadyConfirmed, documents.inbound.alreadyPartlyConfirmed, - documents.inbound.budgetExceeded, documents.inbound.decryptFailed, documents.inbound.encounterNotFound, - documents.inbound.episodeNotFound, documents.inbound.extractFailed, documents.inbound.factNotFound, - documents.inbound.factNotPending, documents.inbound.factTypeMismatch, documents.inbound.fileTooLarge, - documents.inbound.fileType, documents.inbound.invalidMetadata, documents.inbound.invalidQuery, - documents.inbound.invalidUpdate, documents.inbound.localOcrDisabled, documents.inbound.notFound, - documents.inbound.notIndexed, documents.inbound.pdfNeedsAnthropic, - documents.inbound.providerUnsupported, documents.inbound.quotaExceeded, documents.inbound.rateLimited, - documents.inbound.restoreDuplicate, documents.inbound.restoreGone, documents.inbound.sourceAliasLimit, - documents.inbound.uploadBusy, documents.inbound.uploadTimeout, documents.inbound.vaccinationNotFound, - documents.sources.authRefused, documents.sources.badResponse, documents.sources.browserOnly, - documents.sources.invalidAddress, documents.sources.linkTargetNotFound, documents.sources.notConnected, - documents.sources.notFound, documents.sources.organizationRequired, documents.sources.originNotAllowed, + backup.upload.too_large; biomarkers — biomarkers.unit.locked; body-site — body-site.invalid; coach — + coach.budget.exceeded, coach.conversation.invalidTitle, coach.conversation.notFound, + coach.fenced.attachmentConflict, coach.fenced.attachmentLimit, coach.fenced.attachmentNotFound, + coach.fenced.attachmentNotIndexed, coach.fenced.attachmentRequired, coach.fenced.attachmentUnavailable, + coach.fenced.invalid, coach.request.invalid; coach-prefs — coach-prefs.body.invalid_json, + coach-prefs.body.invalid_shape; consent — consent.ai.required; cycle — cycle.bulk.invalid, + cycle.bulk.too_large, cycle.calendar.invalid, cycle.calendar.range, cycle.cycles.invalid, + cycle.day-log.conflict, cycle.day-log.invalid, cycle.disabled, cycle.period.invalid, + cycle.symptom.custom.invalid, cycle.symptom.custom.limit; cycle-prefs — cycle-prefs.body.invalid_json, + cycle-prefs.invalid; disable-coach — disable-coach.body.invalid_json; documents — + documents.chat.conversationNotFound, documents.chat.invalid, documents.inbound.alreadyConfirmed, + documents.inbound.alreadyPartlyConfirmed, documents.inbound.budgetExceeded, + documents.inbound.decryptFailed, documents.inbound.encounterNotFound, documents.inbound.episodeNotFound, + documents.inbound.extractFailed, documents.inbound.factNotFound, documents.inbound.factNotPending, + documents.inbound.factTypeMismatch, documents.inbound.fileTooLarge, documents.inbound.fileType, + documents.inbound.invalidMetadata, documents.inbound.invalidQuery, documents.inbound.invalidUpdate, + documents.inbound.localOcrDisabled, documents.inbound.notFound, documents.inbound.notIndexed, + documents.inbound.pdfNeedsAnthropic, documents.inbound.providerUnsupported, + documents.inbound.quotaExceeded, documents.inbound.rateLimited, documents.inbound.restoreDuplicate, + documents.inbound.restoreGone, documents.inbound.sourceAliasLimit, documents.inbound.uploadBusy, + documents.inbound.uploadTimeout, documents.inbound.vaccinationNotFound, documents.sources.authRefused, + documents.sources.badResponse, documents.sources.browserOnly, documents.sources.invalidAddress, + documents.sources.linkTargetNotFound, documents.sources.notConnected, documents.sources.notFound, + documents.sources.organizationRequired, documents.sources.originNotAllowed, documents.sources.permissionMissing, documents.sources.rateLimited, documents.sources.redirected, documents.sources.tokenRequired, documents.sources.unavailable, documents.sources.unreachable, documents.sources.versionTooOld; encounter — encounter.invalid, encounter.link.invalid, diff --git a/src/app/api/biomarkers/[id]/route.ts b/src/app/api/biomarkers/[id]/route.ts index 45cc9f98a..a17d0b015 100644 --- a/src/app/api/biomarkers/[id]/route.ts +++ b/src/app/api/biomarkers/[id]/route.ts @@ -143,6 +143,28 @@ export const PUT = apiHandler( } } + // A marker's unit labels every reading linked to it: readers resolve the + // unit from the marker, and the stored numbers are in the old unit. A + // change while readings exist would relabel history without converting a + // single value, so it is refused. Tombstoned readings count too, since an + // undo brings them back under the new label. + if (d.unit !== undefined && d.unit !== existing.unit) { + const readingCount = await prisma.labResult.count({ + where: { userId: user.id, biomarkerId: id }, + }); + if (readingCount > 0) { + annotate({ + action: { name: "labs.biomarker.update.unit-locked" }, + meta: { biomarkerId: id, readingCount }, + }); + return apiError( + "The unit of a biomarker with readings cannot be changed", + 422, + { errorCode: "biomarkers.unit.locked", readingCount }, + ); + } + } + // Build `data` field-by-field. `undefined` → leave the column untouched; // an explicit `null` on `context` / `panel` / a bound clears it. const data: Record = {}; diff --git a/src/components/labs/biomarker-form.tsx b/src/components/labs/biomarker-form.tsx index 220c6b927..51503c242 100644 --- a/src/components/labs/biomarker-form.tsx +++ b/src/components/labs/biomarker-form.tsx @@ -70,6 +70,7 @@ export function BiomarkerForm({ }: BiomarkerFormProps) { const { t } = useTranslations(); const formId = useId(); + const unitHintId = `${formId}-unit-hint`; const [name, setName] = useState(existing?.name ?? ""); const [unit, setUnit] = useState(existing?.unit ?? ""); @@ -225,7 +226,16 @@ export function BiomarkerForm({ placeholder={t("labs.biomarker.form.unitPlaceholder")} maxLength={40} required + aria-describedby={existing ? unitHintId : undefined} /> + {existing ? ( + // The server refuses a unit change while readings are linked: + // every reading is shown in the marker's unit, so a new unit + // would relabel stored numbers without converting them. +

+ {t("labs.biomarker.form.unitLockedHint")} +

+ ) : null}