diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index b407f67b8..7938a5939 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -3936,9 +3936,11 @@ paths: "401": *a1 "409": *a7 "422": - description: The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation, or + description: "The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation + (`measurement.batch.invalid`, with every issue under `details.issues`), or (`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow - `measurements:write` credential. Nothing was written in any of the three cases. + `measurements:write` credential. Nothing was written in any of the three cases, and all three are permanent + for the batch as sent: resending it unchanged is refused the same way." content: application/json: schema: @@ -30284,7 +30286,7 @@ components: labs.update.linkedFieldsImmutable, labs.update.numericExpected, labs.update.qualitativeExpected, labs.update.referenceRangeInvalid, labs.update.sourceReferenceRangeInvalid; managed_profile — managed_profile.guardian.duplicate, managed_profile.guardian.managed_invitee, - managed_profile.guardian.required, managed_profile.not_found; measurement — + managed_profile.guardian.required, managed_profile.not_found; measurement — measurement.batch.invalid, measurement.batch.source_not_permitted, measurement.batch.too_large, measurement.bulk-delete.invalid, measurement.create.invalid, measurement.create.source_not_permitted, measurement.delete.too_large, measurement.duplicate_timestamp, measurement.restore.invalid, measurement.update.server_owned_source; @@ -39736,6 +39738,28 @@ components: not advance the entry cursor; the response is marked `Cache-Control: no-store`." reason: type: string + convertedValue: + description: "Only on a `value_out_of_range` skip (since v1.39.1): the entry's value after conversion into the stored + unit, which is the number the band was checked against. A HealthKit fraction that should have been a percent + shows up here a hundred times off." + type: number + range: + description: "Only on a `value_out_of_range` skip (since v1.39.1): the plausibility band that refused the entry, in the + stored unit. Both edges are inclusive. A skip is permanent for the value as sent; resending it unchanged + skips it again." + type: object + properties: + min: + type: number + max: + type: number + unit: + type: string + required: + - min + - max + - unit + additionalProperties: false required: - index - status diff --git a/src/app/api/measurements/batch/__tests__/refusal-detail.test.ts b/src/app/api/measurements/batch/__tests__/refusal-detail.test.ts new file mode 100644 index 000000000..921cda502 --- /dev/null +++ b/src/app/api/measurements/batch/__tests__/refusal-detail.test.ts @@ -0,0 +1,231 @@ +/** + * `POST /api/measurements/batch` — what a refusal tells the client. + * + * Two gaps the native client reported against v1.39.0: + * + * - The whole-batch validation 422 carried no `meta.errorCode`, so the + * client could not tell a permanent refusal from a transient one and had + * to hold its anchor for several syncs before giving up on the rows. + * The too-large and source refusals already had codes; this one now has + * `measurement.batch.invalid`. + * - A `value_out_of_range` skip said neither what the value became after + * conversion nor which band it broke, so a unit-mapping slip (a fraction + * sent where a percent was expected) looked the same as a sensor glitch. + * The entry now carries `convertedValue` and `range`. + * + * Both are additive: status codes, the envelope and every existing field + * are unchanged. + */ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { update: vi.fn() }, + measurement: { findMany: vi.fn() }, + $transaction: vi.fn(async (fn: unknown) => { + if (typeof fn === "function") { + return (fn as (tx: unknown) => unknown)(prisma); + } + }), + }, +})); + +vi.mock("@/lib/measurements/reconcile-external-measurement", () => ({ + reconcileExternalMeasurement: vi.fn( + async (_tx: unknown, input: Record) => ({ + status: "inserted", + row: { id: "row-1", ...input }, + }), + ), +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn() })); +vi.mock("@/lib/jobs/pr-detection", () => ({ + enqueuePrDetection: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/jobs/reminder-satisfy", () => ({ + enqueueReminderSatisfy: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserMeasurements: vi.fn(), +})); +vi.mock("@/lib/rollups/after-measurement-mutation", () => ({ + afterMeasurementMutation: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/daily/morning-refresh-trigger", () => ({ + maybeEnqueueMorningRefresh: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/arrivals/emit-shared", () => ({ + emitDataArrival: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "testuser", role: "USER" as const }, +}; + +function post(body: unknown) { + return POST( + new NextRequest("http://localhost/api/measurements/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }), + ); +} + +const AT = new Date(Date.now() - 3_600_000).toISOString(); + +function entry(overrides: Record = {}) { + return { + hkIdentifier: "HKQuantityTypeIdentifierHeartRate", + value: 64, + unit: "count/min", + startDate: AT, + endDate: AT, + externalId: "uuid-1", + ...overrides, + }; +} + +type EntryResult = { + index: number; + status: string; + reason?: string; + convertedValue?: number; + range?: { min: number; max: number; unit: string }; +}; + +async function entriesFor(entries: unknown[]): Promise { + const res = await post({ entries }); + expect(res.status).toBe(200); + return ((await res.json()) as { data: { entries: EntryResult[] } }).data + .entries; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + limit: 60, + remaining: 60, + resetAt: Date.now() + 60_000, + }); + vi.mocked(prisma.measurement.findMany).mockResolvedValue([]); +}); + +describe("POST /api/measurements/batch — validation refusal code", () => { + it("names a malformed batch measurement.batch.invalid and keeps the 422 envelope", async () => { + const res = await post({ entries: [entry({ startDate: "yesterday" })] }); + expect(res.status).toBe(422); + const body = (await res.json()) as { + data: null; + error: string; + details: { issues: unknown[] }; + meta?: { errorCode?: string }; + }; + expect(body.data).toBeNull(); + expect(typeof body.error).toBe("string"); + expect(body.details.issues.length).toBeGreaterThan(0); + expect(body.meta?.errorCode).toBe("measurement.batch.invalid"); + }); + + it("names an empty entries array the same way", async () => { + const res = await post({ entries: [] }); + expect(res.status).toBe(422); + expect( + ((await res.json()) as { meta?: { errorCode?: string } }).meta?.errorCode, + ).toBe("measurement.batch.invalid"); + }); + + it("keeps measurement.batch.too_large for an over-cap batch", async () => { + const entries = Array.from({ length: 501 }, (_, i) => + entry({ externalId: `uuid-${i}` }), + ); + const res = await post({ entries }); + expect(res.status).toBe(422); + expect( + ((await res.json()) as { meta?: { errorCode?: string } }).meta?.errorCode, + ).toBe("measurement.batch.too_large"); + }); +}); + +describe("POST /api/measurements/batch — value_out_of_range detail", () => { + it("reports the converted value and the band it broke, in the stored unit", async () => { + // HealthKit reports body fat as a fraction; 0.9 becomes 90 %, above 80. + const [result] = await entriesFor([ + entry({ + hkIdentifier: "HKQuantityTypeIdentifierBodyFatPercentage", + value: 0.9, + unit: "%", + }), + ]); + expect(result).toEqual({ + index: 0, + status: "skipped", + reason: "value_out_of_range", + convertedValue: 90, + range: { min: 1, max: 80, unit: "%" }, + }); + }); + + it("reports the lower edge too", async () => { + const [result] = await entriesFor([entry({ value: 12 })]); + expect(result).toMatchObject({ + reason: "value_out_of_range", + convertedValue: 12, + range: { min: 20, max: 300, unit: "bpm" }, + }); + }); + + it("carries the same detail when the write-side gate is the one that refuses", async () => { + vi.mocked(reconcileExternalMeasurement).mockResolvedValueOnce({ + status: "rejected_range", + rejection: { type: "PULSE", direction: "above_max" }, + }); + const [result] = await entriesFor([entry({ value: 64 })]); + expect(result).toMatchObject({ + status: "skipped", + reason: "value_out_of_range", + convertedValue: 64, + range: { min: 20, max: 300, unit: "bpm" }, + }); + }); + + it("adds nothing to an entry that landed or was skipped for another reason", async () => { + const results = await entriesFor([ + entry({ externalId: "uuid-ok" }), + entry({ hkIdentifier: "HKQuantityTypeIdentifierNotAThing" }), + ]); + expect(results[0]).toEqual({ index: 0, status: "inserted" }); + expect(results[1]).toEqual({ + index: 1, + status: "skipped", + reason: "unmappable_identifier", + }); + }); +}); diff --git a/src/app/api/measurements/batch/route.ts b/src/app/api/measurements/batch/route.ts index 776e870b7..ffba6897d 100644 --- a/src/app/api/measurements/batch/route.ts +++ b/src/app/api/measurements/batch/route.ts @@ -71,7 +71,11 @@ import { UNSTABLE_EXTERNAL_ID_REASON, type UnstableExternalIdShape, } from "@/lib/validations/external-id"; -import { validateMeasurementRange } from "@/lib/validations/measurement"; +import { + VALUE_RANGES, + getUnitForType, + validateMeasurementRange, +} from "@/lib/validations/measurement"; import { deviceTypeEnum } from "@/lib/validations/source-priority"; import { checkRateLimit } from "@/lib/rate-limit"; import { enqueuePrDetection } from "@/lib/jobs/pr-detection"; @@ -216,6 +220,36 @@ interface EntryResult { index: number; status: EntryStatus; reason?: string; + /** + * Only on a `value_out_of_range` skip: the value after conversion into the + * stored unit, which is what the band was checked against. + */ + convertedValue?: number; + /** Only on a `value_out_of_range` skip: the band applied, in the stored unit. */ + range?: { min: number; max: number; unit: string }; +} + +/** + * The `value_out_of_range` skip, with what a client needs to say which limit + * applied. The value is the entry's own reading after conversion, handed back + * to the account that sent it; a fraction sent where a percent was expected + * shows up here as a converted value a hundred times too small or too large. + */ +function outOfRangeResult( + index: number, + type: string, + convertedValue: number, +): EntryResult { + const band = VALUE_RANGES[type]; + return { + index, + status: "skipped", + reason: "value_out_of_range", + ...(Number.isFinite(convertedValue) ? { convertedValue } : {}), + ...(band + ? { range: { min: band.min, max: band.max, unit: getUnitForType(type) } } + : {}), + }; } export const POST = apiHandler(withIdempotency<[NextRequest]>(postBatch)); @@ -276,10 +310,13 @@ async function postBatch(request: NextRequest): Promise { const parsed = batchPayloadSchema.safeParse(rawBody); if (!parsed.success) { + // The code is what lets a client tell this refusal from a transient one: + // the batch as sent will never be accepted, so resending it is pointless. return apiValidationError( parsed.error.issues[0]?.message ?? "Invalid batch", sanitiseZodIssues(parsed.error.issues), 422, + { errorCode: "measurement.batch.invalid" }, ); } @@ -363,11 +400,7 @@ async function postBatch(request: NextRequest): Promise { // sensor reading shouldn't poison an otherwise clean ingest. const rangeError = validateMeasurementRange(mapped.type, mapped.value); if (rangeError !== null) { - results[index] = { - index, - status: "skipped", - reason: "value_out_of_range", - }; + results[index] = outOfRangeResult(index, mapped.type, mapped.value); continue; } @@ -666,11 +699,11 @@ async function postBatch(request: NextRequest): Promise { // already applied above, so reaching here means the two disagreed — // report it exactly as the earlier guard would have, and let the // wide-event tally record which end refused. - results[p.index] = { - index: p.index, - status: "skipped", - reason: "value_out_of_range", - }; + results[p.index] = outOfRangeResult( + p.index, + p.row.type as string, + p.row.value as number, + ); continue; } diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index 1957d5bb0..49d374343 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -198,6 +198,7 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "managed_profile.not_found", ], measurement: [ + "measurement.batch.invalid", "measurement.batch.source_not_permitted", "measurement.batch.too_large", "measurement.bulk-delete.invalid", diff --git a/src/lib/openapi/routes/measurements.ts b/src/lib/openapi/routes/measurements.ts index ffd097081..340673fc4 100644 --- a/src/lib/openapi/routes/measurements.ts +++ b/src/lib/openapi/routes/measurements.ts @@ -147,6 +147,22 @@ const batchEntryResult = z "`inserted`/`duplicate` — the row landed (advance the cursor). `updated` — a `stats:` aggregate overwrote an existing row. `skipped` — validation no-op; see `reason`. `failed` — retryable database failure that must not advance the entry cursor; the response is marked `Cache-Control: no-store`.", ), reason: z.string().optional(), + convertedValue: z + .number() + .optional() + .describe( + "Only on a `value_out_of_range` skip (since v1.39.1): the entry's value after conversion into the stored unit, which is the number the band was checked against. A HealthKit fraction that should have been a percent shows up here a hundred times off.", + ), + range: z + .object({ + min: z.number(), + max: z.number(), + unit: z.string(), + }) + .optional() + .describe( + "Only on a `value_out_of_range` skip (since v1.39.1): the plausibility band that refused the entry, in the stored unit. Both edges are inclusive. A skip is permanent for the value as sent; resending it unchanged skips it again.", + ), }) .meta({ id: "AppleHealthBatchEntryResult" }); @@ -1011,7 +1027,7 @@ export const measurementPaths: NonNullable = { ...stdResponses, "422": { description: - "The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation, or (`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow `measurements:write` credential. Nothing was written in any of the three cases.", + "The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation (`measurement.batch.invalid`, with every issue under `details.issues`), or (`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow `measurements:write` credential. Nothing was written in any of the three cases, and all three are permanent for the batch as sent: resending it unchanged is refused the same way.", content: { "application/json": { schema: errorEnvelope } }, }, },