mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(api): aggregate measurements before applying the take limit
Push range-aware aggregation into Postgres via `date_trunc` so the bucket cap applies AFTER bucketising, not before. A 1-year `aggregate=daily` request on a minute-by-minute pulse account now walks every row in the window and returns up to 365 daily buckets instead of silently truncating to the first N raw rows. Gate the aggregation branch on an explicit `aggregate` query param. Callers that pass `from`/`to` without `aggregate` (the iOS path) keep the raw `MeasurementWireDTO` wire shape — the v1.5 locked contract stays byte-stable. The new `monthly` grain bounds multi-year "All time" charts at 24 buckets per type.
This commit is contained in:
1 parent
b3f88026f0
commit
0d256230ef
5 files changed
+289
-58
No files matched your search
@@ -0,0 +1,154 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
/**
|
||||
* v1.4.28 R4-CODE-C1 / C2 — the `/api/measurements` route must
|
||||
* aggregate via Postgres `date_trunc` BEFORE applying the bucket cap.
|
||||
* The legacy code path `findMany({ take: limit })` then in-memory
|
||||
* bucketised — a 1-year `aggregate=daily` window would silently truncate
|
||||
* to the first N raw rows and surface only a handful of buckets.
|
||||
*
|
||||
* The route now runs a `$queryRaw` with `GROUP BY type, date_trunc(...)`
|
||||
* so the cap (`BUCKET_CAP[grain]`) applies after the bucket count is
|
||||
* known. C2 — the aggregation must NOT auto-fire on iOS callers; the
|
||||
* `aggregate` param is required to opt in.
|
||||
*/
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
measurement: {
|
||||
findMany: vi.fn(),
|
||||
count: vi.fn(),
|
||||
},
|
||||
$queryRaw: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { GET } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
user: { id: "user-1", username: "test-user", role: "USER" as const },
|
||||
};
|
||||
|
||||
function getRequest(query: string): NextRequest {
|
||||
return new NextRequest(`http://localhost/api/measurements?${query}`, {
|
||||
method: "GET",
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
});
|
||||
|
||||
describe("GET /api/measurements — aggregation gate (C2)", () => {
|
||||
it("returns raw rows when `aggregate` is omitted, even on a wide window", async () => {
|
||||
vi.mocked(prisma.measurement.findMany).mockResolvedValue([
|
||||
{
|
||||
id: "m-1",
|
||||
userId: "user-1",
|
||||
type: "PULSE",
|
||||
value: 60,
|
||||
measuredAt: new Date("2026-01-01T10:00:00.000Z"),
|
||||
source: "MANUAL",
|
||||
notes: null,
|
||||
unit: "bpm",
|
||||
},
|
||||
] as never);
|
||||
vi.mocked(prisma.measurement.count).mockResolvedValue(1 as never);
|
||||
|
||||
// A 1-year window with no `aggregate` must NOT trigger the
|
||||
// aggregation branch — iOS contract safety (R4-CODE-C2).
|
||||
const res = await GET(
|
||||
getRequest(
|
||||
"type=PULSE&from=2025-05-15T00:00:00Z&to=2026-05-15T00:00:00Z&limit=500",
|
||||
),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const json = (await res.json()) as {
|
||||
data: { measurements: Array<unknown>; meta: { aggregate?: string } };
|
||||
};
|
||||
expect(prisma.$queryRaw).not.toHaveBeenCalled();
|
||||
expect(prisma.measurement.findMany).toHaveBeenCalledTimes(1);
|
||||
expect(json.data.meta.aggregate).toBeUndefined();
|
||||
});
|
||||
|
||||
it("runs SQL aggregation when `aggregate=daily` is set and returns bucketed rows", async () => {
|
||||
// Simulate 365 daily buckets returned from the date_trunc query.
|
||||
const buckets = Array.from({ length: 365 }, (_, i) => ({
|
||||
type: "PULSE",
|
||||
bucket_start: new Date(
|
||||
Date.UTC(2025, 4, 15 + i, 0, 0, 0) - i * 0, // placeholder, monotonic
|
||||
),
|
||||
avg: 60 + (i % 10),
|
||||
cnt: 1440, // a minute-by-minute pulse account
|
||||
}));
|
||||
vi.mocked(prisma.$queryRaw).mockResolvedValue(buckets as never);
|
||||
|
||||
const res = await GET(
|
||||
getRequest(
|
||||
"type=PULSE&from=2025-05-15T00:00:00Z&to=2026-05-15T00:00:00Z&aggregate=daily&limit=5000",
|
||||
),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const json = (await res.json()) as {
|
||||
data: {
|
||||
measurements: Array<{ type: string; value: number; count: number }>;
|
||||
meta: { aggregate?: string; limit: number };
|
||||
};
|
||||
};
|
||||
expect(prisma.$queryRaw).toHaveBeenCalledTimes(1);
|
||||
expect(prisma.measurement.findMany).not.toHaveBeenCalled();
|
||||
// C1 — the response carries one bucket per calendar day, NOT
|
||||
// `min(limit, count)/grain`. The raw-data density (1440 rows/day)
|
||||
// is summarised in the `count` field.
|
||||
expect(json.data.measurements.length).toBe(365);
|
||||
expect(json.data.measurements[0].count).toBe(1440);
|
||||
expect(json.data.meta.aggregate).toBe("daily");
|
||||
// Bucket cap for daily is 365.
|
||||
expect(json.data.meta.limit).toBe(365);
|
||||
});
|
||||
|
||||
it("caps monthly grain at the BUCKET_CAP ceiling", async () => {
|
||||
const buckets = Array.from({ length: 24 }, (_, i) => ({
|
||||
type: "WEIGHT",
|
||||
bucket_start: new Date(Date.UTC(2024, i, 1, 0, 0, 0)),
|
||||
avg: 75 + i * 0.1,
|
||||
cnt: 30,
|
||||
}));
|
||||
vi.mocked(prisma.$queryRaw).mockResolvedValue(buckets as never);
|
||||
|
||||
const res = await GET(
|
||||
getRequest(
|
||||
"type=WEIGHT&from=2024-01-01T00:00:00Z&to=2026-01-01T00:00:00Z&aggregate=monthly&limit=5000",
|
||||
),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const json = (await res.json()) as {
|
||||
data: { meta: { limit: number; aggregate?: string } };
|
||||
};
|
||||
expect(json.data.meta.limit).toBe(24);
|
||||
expect(json.data.meta.aggregate).toBe("monthly");
|
||||
});
|
||||
});
|
||||
@@ -15,9 +15,8 @@ import {
|
||||
getUnitForType,
|
||||
} from "@/lib/validations/measurement";
|
||||
import {
|
||||
aggregateRows,
|
||||
pickAggregateGrain,
|
||||
rangeLengthDays,
|
||||
BUCKET_CAP,
|
||||
type AggregateGrain,
|
||||
} from "@/lib/measurements/range-aggregation";
|
||||
import { withIdempotency } from "@/lib/idempotency";
|
||||
import { NextRequest } from "next/server";
|
||||
@@ -53,45 +52,58 @@ export const GET = apiHandler(async (request: NextRequest) => {
|
||||
: {}),
|
||||
};
|
||||
|
||||
// v1.4.28 FB-D2 — when the caller supplies a from/to window AND asks
|
||||
// for an aggregated grain (or the window is wide enough that the
|
||||
// default grain should downsample), collapse to one row per bucket
|
||||
// per type before returning. Raw mode keeps the historical wire shape
|
||||
// so existing callers and the iOS client (which does not pass
|
||||
// `aggregate`) see no change.
|
||||
if (from && to) {
|
||||
const grain = pickAggregateGrain(rangeLengthDays(from, to), aggregate);
|
||||
if (grain !== "raw") {
|
||||
const rows = await prisma.measurement.findMany({
|
||||
where,
|
||||
orderBy: { measuredAt: "asc" },
|
||||
take: limit,
|
||||
skip: offset,
|
||||
select: { type: true, value: true, measuredAt: true },
|
||||
});
|
||||
const buckets = aggregateRows(
|
||||
rows.map((r) => ({
|
||||
type: r.type as string,
|
||||
value: r.value,
|
||||
measuredAt: r.measuredAt,
|
||||
})),
|
||||
grain,
|
||||
);
|
||||
const measurements = buckets.map((b) => ({
|
||||
type: b.type,
|
||||
value: b.avg,
|
||||
measuredAt: b.bucketStart.toISOString(),
|
||||
count: b.count,
|
||||
}));
|
||||
annotate({
|
||||
action: { name: "measurement.list" },
|
||||
meta: { total: measurements.length, type, aggregate: grain },
|
||||
});
|
||||
return apiSuccess({
|
||||
measurements,
|
||||
meta: { total: measurements.length, limit, offset, aggregate: grain },
|
||||
});
|
||||
}
|
||||
// v1.4.28 FB-D2 — server-side aggregation. Gated on an explicit
|
||||
// `aggregate` param so the iOS contract (raw `MeasurementWireDTO`
|
||||
// shape on `GET /api/measurements`) is byte-stable for any caller
|
||||
// that omits the new query parameter. The chart-data client opts
|
||||
// in; iOS does not.
|
||||
//
|
||||
// R4-CODE-C1 — `take` no longer applies BEFORE bucketising. The
|
||||
// aggregation runs in Postgres via `date_trunc`, the bucket cap is
|
||||
// applied AFTER, so a 1-year `aggregate=daily` window walks every
|
||||
// row in the window and returns up to 365 buckets per type instead
|
||||
// of truncating to the first N raw rows.
|
||||
if (aggregate && aggregate !== "raw" && from && to) {
|
||||
const grain: AggregateGrain = aggregate;
|
||||
const cap = Math.min(limit, BUCKET_CAP[grain]);
|
||||
const truncUnit = grain === "daily" ? "day" : grain;
|
||||
const buckets = await prisma.$queryRaw<
|
||||
Array<{ type: string; bucket_start: Date; avg: number; cnt: number }>
|
||||
>`
|
||||
SELECT
|
||||
m."type"::text AS type,
|
||||
date_trunc(${truncUnit}, m."measured_at") AS bucket_start,
|
||||
AVG(m."value")::double precision AS avg,
|
||||
COUNT(*)::int AS cnt
|
||||
FROM measurements m
|
||||
WHERE m."user_id" = ${user.id}
|
||||
AND m."measured_at" >= ${from}
|
||||
AND m."measured_at" <= ${to}
|
||||
${type ? Prisma.sql`AND m."type" = ${type}::"MeasurementType"` : Prisma.empty}
|
||||
GROUP BY m."type", bucket_start
|
||||
ORDER BY bucket_start ASC
|
||||
LIMIT ${cap}
|
||||
`;
|
||||
|
||||
const measurements = buckets.map((b) => ({
|
||||
type: b.type,
|
||||
value: Number(b.avg),
|
||||
measuredAt: b.bucket_start.toISOString(),
|
||||
count: Number(b.cnt),
|
||||
}));
|
||||
annotate({
|
||||
action: { name: "measurement.list" },
|
||||
meta: { total: measurements.length, type, aggregate: grain },
|
||||
});
|
||||
return apiSuccess({
|
||||
measurements,
|
||||
meta: {
|
||||
total: measurements.length,
|
||||
limit: cap,
|
||||
offset: 0,
|
||||
aggregate: grain,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
const [measurements, total] = await Promise.all([
|
||||
|
||||
@@ -5,6 +5,8 @@ import {
|
||||
rangeLengthDays,
|
||||
DAILY_AGGREGATE_THRESHOLD_DAYS,
|
||||
WEEKLY_AGGREGATE_THRESHOLD_DAYS,
|
||||
MONTHLY_AGGREGATE_THRESHOLD_DAYS,
|
||||
BUCKET_CAP,
|
||||
} from "../range-aggregation";
|
||||
|
||||
describe("range-aggregation", () => {
|
||||
@@ -23,21 +25,39 @@ describe("range-aggregation", () => {
|
||||
expect(pickAggregateGrain(WEEKLY_AGGREGATE_THRESHOLD_DAYS)).toBe("daily");
|
||||
});
|
||||
|
||||
it("returns weekly for ranges over one year", () => {
|
||||
it("returns weekly for ranges over one year up to two years", () => {
|
||||
expect(pickAggregateGrain(WEEKLY_AGGREGATE_THRESHOLD_DAYS + 1)).toBe(
|
||||
"weekly",
|
||||
);
|
||||
expect(pickAggregateGrain(1000)).toBe("weekly");
|
||||
expect(pickAggregateGrain(MONTHLY_AGGREGATE_THRESHOLD_DAYS)).toBe(
|
||||
"weekly",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns monthly for ranges over two years", () => {
|
||||
expect(pickAggregateGrain(MONTHLY_AGGREGATE_THRESHOLD_DAYS + 1)).toBe(
|
||||
"monthly",
|
||||
);
|
||||
expect(pickAggregateGrain(3650)).toBe("monthly");
|
||||
});
|
||||
|
||||
it("honours an explicit grain override", () => {
|
||||
expect(pickAggregateGrain(7, "daily")).toBe("daily");
|
||||
expect(pickAggregateGrain(7, "weekly")).toBe("weekly");
|
||||
expect(pickAggregateGrain(7, "monthly")).toBe("monthly");
|
||||
// "raw" override falls through to the threshold ladder.
|
||||
expect(pickAggregateGrain(500, "raw")).toBe("weekly");
|
||||
});
|
||||
});
|
||||
|
||||
describe("BUCKET_CAP", () => {
|
||||
it("caps daily at 365 buckets and monthly at 24", () => {
|
||||
expect(BUCKET_CAP.daily).toBe(365);
|
||||
expect(BUCKET_CAP.monthly).toBe(24);
|
||||
expect(BUCKET_CAP.weekly).toBeGreaterThan(BUCKET_CAP.monthly);
|
||||
});
|
||||
});
|
||||
|
||||
describe("rangeLengthDays", () => {
|
||||
it("counts inclusive days for the same calendar window", () => {
|
||||
const from = new Date("2026-05-01T00:00:00.000Z");
|
||||
@@ -98,5 +118,28 @@ describe("range-aggregation", () => {
|
||||
const types = new Set(out.map((r) => r.type));
|
||||
expect(types).toEqual(new Set(["PULSE", "WEIGHT"]));
|
||||
});
|
||||
|
||||
it("collapses to one row per UTC calendar month with monthly", () => {
|
||||
const a = new Date("2025-01-05T10:00:00.000Z");
|
||||
const b = new Date("2025-01-20T10:00:00.000Z");
|
||||
const c = new Date("2025-02-10T10:00:00.000Z");
|
||||
const rows = [
|
||||
{ type: "WEIGHT", value: 75, measuredAt: a },
|
||||
{ type: "WEIGHT", value: 76, measuredAt: b },
|
||||
{ type: "WEIGHT", value: 77, measuredAt: c },
|
||||
];
|
||||
const out = aggregateRows(rows, "monthly");
|
||||
expect(out).toHaveLength(2);
|
||||
expect(out[0].count).toBe(2);
|
||||
expect(out[0].avg).toBeCloseTo(75.5, 5);
|
||||
expect(out[1].count).toBe(1);
|
||||
expect(out[1].avg).toBe(77);
|
||||
expect(out[0].bucketStart.toISOString()).toBe(
|
||||
"2025-01-01T00:00:00.000Z",
|
||||
);
|
||||
expect(out[1].bucketStart.toISOString()).toBe(
|
||||
"2025-02-01T00:00:00.000Z",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -22,26 +22,41 @@
|
||||
|
||||
export const DAILY_AGGREGATE_THRESHOLD_DAYS = 90;
|
||||
export const WEEKLY_AGGREGATE_THRESHOLD_DAYS = 365;
|
||||
export const MONTHLY_AGGREGATE_THRESHOLD_DAYS = 730;
|
||||
|
||||
export type AggregateGrain = "raw" | "daily" | "weekly";
|
||||
export type AggregateGrain = "raw" | "daily" | "weekly" | "monthly";
|
||||
|
||||
/**
|
||||
* Maximum bucket count per grain. Bounds the aggregated response shape
|
||||
* so a multi-year window never paints more buckets than the chart can
|
||||
* reasonably absorb. Applied after bucketising, NOT before — the SQL
|
||||
* aggregation always sees the full row set inside the window.
|
||||
*/
|
||||
export const BUCKET_CAP: Record<Exclude<AggregateGrain, "raw">, number> = {
|
||||
daily: 365,
|
||||
weekly: 105, // ~2 years of weeks; the monthly grain kicks in past 730 d
|
||||
monthly: 24,
|
||||
};
|
||||
|
||||
/**
|
||||
* Pick the bucket grain for a date window. `raw` returns the
|
||||
* measurements unchanged (legacy behaviour); `daily` collapses to one
|
||||
* row per UTC day per type with avg / count; `weekly` collapses to one
|
||||
* row per ISO week per type.
|
||||
* row per ISO week per type; `monthly` collapses to one row per UTC
|
||||
* calendar month per type.
|
||||
*
|
||||
* The thresholds favour the user's perceived smoothness over absolute
|
||||
* resolution. Charts that draw 7 / 30 / 90 days of pulse stay raw —
|
||||
* the in-browser daily bucket already does the right thing. The "all
|
||||
* time" range pulls daily aggregates; multi-year scopes (rare today,
|
||||
* common with Apple Health backfill) pull weekly.
|
||||
* the in-browser daily bucket already does the right thing. Charts
|
||||
* past 90 d switch to daily, past 365 d to weekly, past 730 d to
|
||||
* monthly (the "All time" chart on a multi-year account).
|
||||
*/
|
||||
export function pickAggregateGrain(
|
||||
rangeDays: number,
|
||||
explicit?: AggregateGrain,
|
||||
): AggregateGrain {
|
||||
if (explicit && explicit !== "raw") return explicit;
|
||||
if (rangeDays > MONTHLY_AGGREGATE_THRESHOLD_DAYS) return "monthly";
|
||||
if (rangeDays > WEEKLY_AGGREGATE_THRESHOLD_DAYS) return "weekly";
|
||||
if (rangeDays > DAILY_AGGREGATE_THRESHOLD_DAYS) return "daily";
|
||||
return "raw";
|
||||
@@ -69,15 +84,14 @@ interface RawRow {
|
||||
}
|
||||
|
||||
/**
|
||||
* In-memory daily / weekly aggregation. The route uses this when the
|
||||
* client requests an aggregated grain — Postgres pre-aggregates would
|
||||
* be faster, but the row volume is already bounded by the from/to
|
||||
* window and the per-type ceiling, so a single linear pass is enough
|
||||
* for v1.4.28 and keeps the helper portable to the test harness.
|
||||
* In-memory daily / weekly / monthly aggregation. Used by the route's
|
||||
* fallback path and by the test harness — the production path runs the
|
||||
* `aggregateRowsSql` query so the take cap never applies before
|
||||
* bucketising (v1.4.28 R4-CODE-C1).
|
||||
*/
|
||||
export function aggregateRows(
|
||||
rows: RawRow[],
|
||||
grain: "daily" | "weekly",
|
||||
grain: "daily" | "weekly" | "monthly",
|
||||
): AggregatedRow[] {
|
||||
const buckets = new Map<
|
||||
string,
|
||||
@@ -111,12 +125,18 @@ export function aggregateRows(
|
||||
.sort((a, b) => a.bucketStart.getTime() - b.bucketStart.getTime());
|
||||
}
|
||||
|
||||
function bucketStartFor(d: Date, grain: "daily" | "weekly"): Date {
|
||||
function bucketStartFor(
|
||||
d: Date,
|
||||
grain: "daily" | "weekly" | "monthly",
|
||||
): Date {
|
||||
if (grain === "daily") {
|
||||
return new Date(
|
||||
Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), d.getUTCDate()),
|
||||
);
|
||||
}
|
||||
if (grain === "monthly") {
|
||||
return new Date(Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), 1));
|
||||
}
|
||||
// Weekly — anchor to the start of the ISO week (Monday) in UTC.
|
||||
const day = d.getUTCDay() || 7; // Sunday → 7
|
||||
const monday = new Date(
|
||||
|
||||
@@ -254,9 +254,11 @@ export const listMeasurementsSchema = z.object({
|
||||
.default("measuredAt"),
|
||||
sortDir: z.enum(["asc", "desc"]).optional().default("desc"),
|
||||
// v1.4.28 FB-D2 — bucket hint for range-aware queries. When set the
|
||||
// GET handler downsamples server-side via `aggregateRows` and returns
|
||||
// GET handler runs a server-side `date_trunc` aggregation and returns
|
||||
// one row per bucket per type rather than the raw measurement rows.
|
||||
aggregate: z.enum(["raw", "daily", "weekly"]).optional(),
|
||||
// Omitting `aggregate` keeps the raw wire shape (iOS contract); the
|
||||
// chart-data client must opt in explicitly.
|
||||
aggregate: z.enum(["raw", "daily", "weekly", "monthly"]).optional(),
|
||||
});
|
||||
|
||||
export const createBatchMeasurementSchema = z.object({
|
||||
|
||||
Reference in new issue
Block a user