Correct three contract claims the published spec got wrong

The `type` query parameter on GET /api/insights/derived carried the eleven
VITALS_BASELINE vitals and described itself as being for that metric only.
SAME_TIME_BASELINE takes a `type` too, and its four cumulative day metrics
were absent from the enum, so a client generated against the spec could not
send them at all. The parameter now publishes the union of both registry
sets and the description says which values belong to which metric. A guard
reads the enum back out of the assembled document and pins it to the
registry, so a type added to either engine reaches the contract with it.

The idempotency wrapper's two 409 arms sent `error` as `{ message }`. That
was the only object-shaped error the application emitted; every other route
sends the documented `{ data, error: <string> }` envelope, which is also
what the spec has always promised. The code was wrong, not the spec. Both
arms now build their response through one helper that sends the string, so
every route under withIdempotency serves the envelope its clients decode.

The hero score-ring comment still described MED_COMPLIANCE as pooled 7-day
adherence. It has been today's dose progress since v1.27.8.
This commit is contained in:
Marc-André Bombeck committed 2026-08-01 11:42:07 +02:00
1 parent fb1bf710e6
commit 43c591fe19
5 files changed
+266 -28

No files matched your search

@@ -0,0 +1,136 @@
/**
* The bytes a client actually receives from an idempotency 409.
*
* `POST /api/anamnesis/facts` is the route the drift was reported against
* (iOS #75), but nothing here is specific to it — the 409 comes from
* `withIdempotency`, so every route under that wrapper serves the same
* envelope. Until v1.35.1 both in-flight arms sent `error` as
* `{ message: … }`, the only object-shaped error the app emitted and not the
* `{ data, error: <string> }` envelope the published contract promises.
*
* This runs the REAL chain — the real route module, the real `apiHandler`,
* the real `withIdempotency` — with only the data layer and the session
* mocked, and asserts on the parsed response body. A schema test would not
* have caught this: the schema was already right.
*
* Both 409 arms are covered:
* - a `PENDING_STATUS` row already in the table (the in-flight lookup);
* - a lost claim race (the row appears between the lookup and the insert).
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
vi.stubEnv("ENCRYPTION_KEYS", "");
vi.stubEnv("ENCRYPTION_ACTIVE_KEY_ID", "");
vi.stubEnv("ENCRYPTION_KEY", "0".repeat(64));
const mocks = vi.hoisted(() => ({
findUnique: vi.fn(),
create: vi.fn(),
deleteMany: vi.fn(),
updateMany: vi.fn(),
getSession: vi.fn(),
createFact: vi.fn(),
factFindFirst: vi.fn(),
}));
vi.mock("@/lib/db", () => ({
prisma: {
idempotencyKey: {
findUnique: mocks.findUnique,
create: mocks.create,
deleteMany: mocks.deleteMany,
updateMany: mocks.updateMany,
delete: vi.fn(),
update: vi.fn(),
},
apiToken: { findUnique: vi.fn() },
healthProfileFactRevision: { findFirst: mocks.factFindFirst },
},
}));
vi.mock("@/lib/auth/session", () => ({ getSession: mocks.getSession }));
vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
vi.mock("@/lib/profile/health-facts", () => ({
readHealthProfileFacts: vi.fn(),
createHealthProfileFact: mocks.createFact,
correctHealthProfileFact: vi.fn(),
removeHealthProfileFact: vi.fn(),
}));
import { NextRequest } from "next/server";
import { POST } from "../route";
/** The pending sentinel `findCached` recognises as "another call is running". */
const PENDING_STATUS = 0;
function request(): NextRequest {
return new NextRequest("http://localhost/api/anamnesis/facts", {
method: "POST",
headers: {
"content-type": "application/json",
"idempotency-key": "key-in-flight",
},
body: JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }),
});
}
beforeEach(() => {
vi.clearAllMocks();
mocks.getSession.mockResolvedValue({
user: { id: "user-1", role: "USER" },
});
mocks.findUnique.mockResolvedValue(null);
mocks.create.mockResolvedValue({});
mocks.deleteMany.mockResolvedValue({ count: 0 });
mocks.updateMany.mockResolvedValue({ count: 1 });
mocks.factFindFirst.mockResolvedValue(null);
mocks.createFact.mockResolvedValue({ id: "fact-1" });
});
describe("idempotency 409 envelope, over the real route", () => {
it("serves `error` as a string when a request is already in flight", async () => {
mocks.findUnique.mockResolvedValue({
id: "idem-1",
responseStatus: PENDING_STATUS,
responseBody: "",
expiresAt: new Date(Date.now() + 60_000),
});
const response = await POST(request());
const body = await response.json();
expect(response.status).toBe(409);
expect(body.data).toBeNull();
expect(typeof body.error).toBe("string");
expect(body.error).toBe(
"A request with this Idempotency-Key is already in progress",
);
// The shape the drift produced, named explicitly so a revert cannot
// pass by satisfying only the `typeof` check above.
expect(body.error).not.toHaveProperty("message");
expect(response.headers.get("X-Idempotent-Replay")).toBe("false");
// The side effect must not have run on a refused call.
expect(mocks.createFact).not.toHaveBeenCalled();
});
it("serves `error` as a string when the claim race is lost", async () => {
// No row on lookup, but the claiming insert collides with the racing
// request that got there first.
mocks.findUnique.mockResolvedValue(null);
mocks.create.mockRejectedValue(
Object.assign(new Error("Unique constraint failed"), { code: "P2002" }),
);
const response = await POST(request());
const body = await response.json();
expect(response.status).toBe(409);
expect(body.data).toBeNull();
expect(typeof body.error).toBe("string");
expect(body.error).toBe(
"A request with this Idempotency-Key is already in progress",
);
expect(body.error).not.toHaveProperty("message");
expect(response.headers.get("X-Idempotent-Replay")).toBe("false");
expect(mocks.createFact).not.toHaveBeenCalled();
});
});
+6 -3
View File
@@ -406,9 +406,12 @@ function coerceChartOverlayPrefsMap(value: unknown): ChartOverlayPrefsMap {
*
* - `READINESS` / `RECOVERY_SCORE` / `SLEEP_SCORE` resolve through the
* derived registry's engines (module-gated like the derived routes);
* - `MED_COMPLIANCE` is the pooled 7-day medication adherence from the
* canonical compliance engine — the ring that absorbs the retired
* hero dose row's information role.
* - `MED_COMPLIANCE` is TODAY's dose progress (taken of scheduled) from
* the snapshot's own `medsToday` block — the ring that absorbs the
* retired hero dose row's information role. The v1.27.7 first cut put
* the pooled 7-day adherence percentage here; it has been today's
* progress since v1.27.8. Resolution lives in
* `@/lib/dashboard/score-rings`.
*
* The preference piggy-backs on the layout blob like `chartOverlayPrefs`
* (a UI affordance, no Prisma migration). The resolver drops unknown ids,
+27 -20
View File
@@ -329,6 +329,31 @@ export async function defaultUserIdResolver(): Promise<string | null> {
*
* No-op when the header is missing or the value is malformed.
*/
/**
* The 409 both in-flight arms return.
*
* `error` is a STRING, matching the envelope `@/lib/api-response` defines
* for every other route (`{ data: null, error: <prose>, meta? }`). Until
* v1.35.1 these two literals sent `error` as `{ message }` — the only
* object-shaped error the app ever emitted, inherited by every route under
* `withIdempotency`, and never what the published contract promised. A
* client decoding the documented envelope now gets what it was generated
* against.
*
* Built once rather than twice so the two arms cannot drift apart again.
* The response is hand-built rather than routed through `apiError` because
* the 409 carries the `X-Idempotent-Replay` header.
*/
function inflightConflictResponse(): Response {
return NextResponse.json(
{
data: null,
error: "A request with this Idempotency-Key is already in progress",
},
{ status: 409, headers: { "X-Idempotent-Replay": "false" } },
);
}
export function withIdempotency<
Args extends [Request | NextRequest, ...unknown[]],
>(
@@ -368,16 +393,7 @@ export function withIdempotency<
action: { name: "idempotency.inflight_conflict" },
meta: { method: ctx.method, path: ctx.path },
});
return NextResponse.json(
{
data: null,
error: {
message:
"A request with this Idempotency-Key is already in progress",
},
},
{ status: 409, headers: { "X-Idempotent-Replay": "false" } },
);
return inflightConflictResponse();
}
// Claim the key before running the handler. If a racing request beats
@@ -389,16 +405,7 @@ export function withIdempotency<
action: { name: "idempotency.inflight_conflict" },
meta: { method: ctx.method, path: ctx.path },
});
return NextResponse.json(
{
data: null,
error: {
message:
"A request with this Idempotency-Key is already in progress",
},
},
{ status: 409, headers: { "X-Idempotent-Replay": "false" } },
);
return inflightConflictResponse();
}
let response: Response;
@@ -0,0 +1,73 @@
/**
* The `type` query parameter on `GET /api/insights/derived` sub-targets the
* single measurement type a baseline metric works over. Two engines define
* one: `VITALS_BASELINE` over a vital, `SAME_TIME_BASELINE` over a cumulative
* day metric. For most of a year the published enum carried the vitals alone,
* so a spec-conformant client could not send the four same-time types at all
* (iOS #75).
*
* This guard asserts the published parameter enumerates exactly the union of
* the two registry sets, read out of the assembled OpenAPI document rather
* than out of the schema module — the same object the generator writes to
* `docs/api/openapi.yaml`, so a value that reaches the registry but not the
* contract fails here.
*/
import { describe, expect, it } from "vitest";
import { buildOpenApiDocument } from "../registry";
import {
VITALS_BASELINE_TYPES,
SAME_TIME_BASELINE_TYPES,
} from "@/lib/insights/derived/registry";
/** Pull the `type` query parameter's enum out of the assembled document. */
function publishedTypeEnum(): string[] {
const doc = buildOpenApiDocument() as {
paths?: Record<
string,
{ get?: { parameters?: unknown[]; requestParams?: unknown } }
>;
};
const operation = doc.paths?.["/api/insights/derived"]?.get;
expect(
operation,
"GET /api/insights/derived is missing from the document",
).toBeDefined();
const parameters = (operation?.parameters ?? []) as Array<{
name?: string;
in?: string;
schema?: { enum?: string[] };
}>;
const typeParam = parameters.find(
(p) => p.name === "type" && p.in === "query",
);
expect(typeParam, "the `type` query parameter is missing").toBeDefined();
const values = typeParam?.schema?.enum;
expect(Array.isArray(values), "`type` is not published as an enum").toBe(
true,
);
return values as string[];
}
describe("derived `type` parameter stays in sync with the registry", () => {
it("publishes exactly the union of the two baseline type sets", () => {
const expected = [
...VITALS_BASELINE_TYPES,
...SAME_TIME_BASELINE_TYPES,
].map(String);
expect([...publishedTypeEnum()].sort()).toEqual([...expected].sort());
});
it("publishes every same-time baseline type", () => {
// Named separately: this is the half that was missing, and a regression
// here is the one a client feels immediately (a 422-shaped dead end on
// a value the server accepts).
const published = publishedTypeEnum();
for (const type of SAME_TIME_BASELINE_TYPES) {
expect(published).toContain(String(type));
}
});
});
+24 -5
View File
@@ -14,6 +14,7 @@ import { METRIC_STATUS_IDS } from "@/lib/insights/metric-status-registry";
import {
DERIVED_METRIC_IDS,
VITALS_BASELINE_TYPES,
SAME_TIME_BASELINE_TYPES,
} from "@/lib/insights/derived/registry";
import { ANALYTICS_RANGES } from "@/lib/analytics/range-delta";
@@ -108,8 +109,26 @@ export const metricStatusResponse = z
// v1.10.0 — generic derived-wellness-metric route. The query enum is
// derived from the same registry the route validates against, so spec +
// route + cache scope cannot drift. `type` sub-targets the single vital
// a baseline metric (VITALS_BASELINE) bands over.
// route + cache scope cannot drift. `type` sub-targets the single
// measurement type a baseline metric works over.
/**
* Every value the `type` query parameter accepts, assembled from the two
* registry sets that define one: `VITALS_BASELINE` bands a single vital,
* `SAME_TIME_BASELINE` compares one cumulative day metric against the same
* hour of a typical day. Published as one enum because `type` is a single
* parameter shared by both metrics — a client picks the subset that belongs
* to the `metric` it names.
*
* Built from the registry rather than restated here, so a type added to
* either engine reaches the published contract in the same commit.
* `derived-type-enum-sync.test.ts` pins the equality.
*/
export const DERIVED_TYPE_PARAM_VALUES: string[] = [
...VITALS_BASELINE_TYPES,
...SAME_TIME_BASELINE_TYPES,
];
export const derivedMetricQuery = z
.object({
metric: z
@@ -118,10 +137,10 @@ export const derivedMetricQuery = z
"Derived-metric id to compute (e.g. VITALS_BASELINE, FITNESS_AGE, VASCULAR_AGE_DELTA, HRV_BALANCE, BMI, READINESS). Closed enum: an unknown id 422s. Metrics whose compute has not yet landed return an `insufficient` value with reason `not_implemented`.",
),
type: z
.enum(VITALS_BASELINE_TYPES as [string, ...string[]])
.enum(DERIVED_TYPE_PARAM_VALUES as [string, ...string[]])
.optional()
.describe(
"For VITALS_BASELINE only — the single vital to band (defaults to RESTING_HEART_RATE). Ignored by composites. An unsupported value yields an `insufficient` value rather than a 422 so iOS metric combinations stay forgiving.",
"The single measurement type a baseline metric works over. `VITALS_BASELINE` takes one of the eleven vitals (`RESTING_HEART_RATE` through `WEIGHT`; defaults to `RESTING_HEART_RATE`). `SAME_TIME_BASELINE` takes one of the four cumulative day metrics — `ACTIVITY_STEPS`, `ACTIVE_ENERGY_BURNED`, `WALKING_RUNNING_DISTANCE`, `FLIGHTS_CLIMBED`. Ignored by the composite metrics. A type the named metric does not support yields an `insufficient` value rather than a 422, so client metric combinations stay forgiving.",
),
})
.meta({ id: "DerivedMetricQuery" });
@@ -253,7 +272,7 @@ export const derivedBatchQuery = z
.min(1)
.max(1024)
.describe(
"Comma-separated derived-metric tokens. Each is a `<DERIVED_METRIC_ID>` or `<DERIVED_METRIC_ID>:<MeasurementType>` (the colon sub-targets a VITALS_BASELINE vital). An unknown id 422s; a `type` outside the MeasurementType enum 422s; at most 24 tokens; duplicates collapse.",
"Comma-separated derived-metric tokens. Each is a `<DERIVED_METRIC_ID>` or `<DERIVED_METRIC_ID>:<MeasurementType>` (the colon sub-targets the single measurement type a baseline metric works over — a vital for `VITALS_BASELINE`, a cumulative day metric for `SAME_TIME_BASELINE`; see the `type` parameter on `GET /api/insights/derived`). An unknown id 422s; a `type` outside the MeasurementType enum 422s; at most 24 tokens; duplicates collapse.",
),
})
.meta({ id: "DerivedBatchQuery" });