diff --git a/src/__tests__/dashboard-summary-read-plan-guard.test.ts b/src/__tests__/dashboard-summary-read-plan-guard.test.ts new file mode 100644 index 000000000..44c9707d8 --- /dev/null +++ b/src/__tests__/dashboard-summary-read-plan-guard.test.ts @@ -0,0 +1,106 @@ +/** + * Structural guards on the two per-type reads behind the dashboard tiles. + * + * A database audit against a multi-year account measured both of them as + * full passes over the largest tables on the dashboard path. The cause is + * the same in both cases: PostgreSQL 16 has no index skip scan, so a + * query that wants one row per measurement type but never names a type + * cannot be served by any index on `(user_id, type, …)` and falls back to + * reading everything the account owns. + * + * Neither shape is a bug a behavioural test can see — both return the + * right rows. What changes is the plan, and the plan is decided by two + * properties of the SQL text: that the latest-reading read is driven FROM + * the type list rather than grouped down to it, and that the rollup read + * carries a `type` predicate at all. So the guards assert on the SQL. + * + * They are tripwires, not proofs. They cannot show a plan is good — only + * that the two shapes the audit found have not come back. The measured + * plans live in the integration companion + * (`tests/integration/dashboard-summary-reads.test.ts`), which runs the + * same functions against a real Postgres. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const READS = join(process.cwd(), "src/lib/dashboard/summary-reads.ts"); +const ROUTE = join(process.cwd(), "src/app/api/dashboard/summary/route.ts"); + +/** + * Source with every comment removed, so a guard matches the SQL the + * database actually receives and never the prose explaining it. Both + * files discuss `DISTINCT ON` in their comments on purpose — that is the + * shape being warned about. + */ +function code(path: string): string { + return readFileSync(path, "utf8") + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/(^|[^:])\/\/.*$/gm, "$1"); +} + +describe("dashboard summary reads — latest reading per type", () => { + it("does not group the account's whole live set down with DISTINCT ON", () => { + // The shape the audit measured: `SELECT DISTINCT ON (m."type") … FROM + // measurements`. Whitespace-tolerant, because a reformat must not be + // able to slip it back past the matcher. + const distinctOn = /DISTINCT\s+ON\s*\(/i; + expect(code(READS)).not.toMatch(distinctOn); + expect(code(ROUTE)).not.toMatch(distinctOn); + }); + + it("drives the read from the type list with a LATERAL lookup per type", () => { + const sql = code(READS); + const lateral = sql.match(/CROSS\s+JOIN\s+LATERAL/gi) ?? []; + expect(lateral.length).toBeGreaterThan(0); + // The type list has to reach SQL as the driving relation, and the + // per-type branch has to stop at the first row — without the LIMIT the + // LATERAL still reads every row of the type partition. + expect(sql).toMatch(/FROM\s+unnest\(\$\{/); + expect(sql).toMatch(/ORDER\s+BY\s+m\."measured_at"\s+DESC\s+LIMIT\s+1/); + }); +}); + +describe("dashboard summary reads — sparkline rollup buckets", () => { + it("carries a type predicate so the rollup key can drive the read", () => { + const sql = code(READS); + // `measurement_rollups` is keyed (user_id, type, granularity, + // bucket_start, source). Without an equality on `type` the key's + // second column is skipped and neither the primary key nor the + // user/type/granularity/bucket index can serve the read. + const rollupRead = sql.slice(sql.indexOf("FROM measurement_rollups r")); + expect(rollupRead.length).toBeGreaterThan(0); + const typePredicate = rollupRead.match(/r\."type"\s*=\s*ANY\(\$\{/g) ?? []; + expect(typePredicate.length).toBe(1); + // It has to sit alongside the user and granularity filters, not in a + // later subquery that the planner reaches only after the scan. + expect(rollupRead).toMatch( + /WHERE\s+r\."user_id"\s*=\s*\$\{[^}]+\}\s+AND\s+r\."type"\s*=\s*ANY\(/, + ); + }); + + it("binds the type list as a parameter rather than splicing it", () => { + // Raw SQL is parameter-bound or whitelist-spliced. The type list is a + // bound array with an explicit enum cast; a spliced list would also + // defeat statement reuse. + const sql = code(READS); + const casts = sql.match(/\}::"measurement_type"\[\]/g) ?? []; + expect(casts.length).toBe(2); + expect(sql).not.toMatch(/\$queryRawUnsafe/); + }); +}); + +describe("dashboard summary reads — the route keeps using them", () => { + it("calls the shared readers instead of inlining its own SQL", () => { + const route = code(ROUTE); + expect(route).toMatch(/readLatestEver\(prisma, userId, measurementTypes\)/); + expect(route).toMatch( + /readSparkBuckets\(\s*prisma,\s*userId,\s*measurementTypes,/, + ); + // No second copy of the rollup read can drift out of the guarded + // module. (`FROM measurements` still appears in the route's own + // streak-day aggregate, which is a bounded 365-row read and not part + // of this finding.) + expect(route.includes("FROM measurement_rollups")).toBe(false); + }); +}); diff --git a/tests/integration/dashboard-summary-reads.test.ts b/tests/integration/dashboard-summary-reads.test.ts new file mode 100644 index 000000000..6dcf1f840 --- /dev/null +++ b/tests/integration/dashboard-summary-reads.test.ts @@ -0,0 +1,247 @@ +/** + * v1.38.5 — the dashboard's two per-type reads, against a real Postgres. + * + * Both were rewritten for their query plans, not for their results: the + * latest-reading read moved from `DISTINCT ON (type)` to a LATERAL join + * over the type list, and the sparkline read gained the `type` predicate + * its rollup key needs. Neither change may move a single row. + * + * So the reference implementation is the OLD SQL, kept verbatim in this + * file and run side by side with the shipped readers over the same + * fixture. The rows have to match exactly — value, ordering, and the + * types that are present at all. + * + * The fixture deliberately carries the three cases that could break the + * rewrite: a type whose only rows are tombstoned (must drop out of BOTH + * results, as an inner join and a filtered group both do), a type with + * no rows at all (same), and a rollup table holding coarser + * granularities the DAY read must not pick up. + * + * Plan shape is asserted by the unit companion + * (`src/__tests__/dashboard-summary-read-plan-guard.test.ts`) rather than + * here: a testcontainer fixture is far too small for the planner to + * choose the same access paths a real account produces, so an EXPLAIN + * assertion at this size would pin nothing. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; +import { + readLatestEver, + readSparkBuckets, + type LatestEverRow, + type SparklineRow, +} from "@/lib/dashboard/summary-reads"; +import { measurementTypeEnum } from "@/lib/validations/measurement"; +import type { + MeasurementType, + RollupGranularity, +} from "@/generated/prisma/client"; + +const SPARK_DAYS = 7; +const TYPES = [...measurementTypeEnum.options] as MeasurementType[]; + +/** The pre-v1.38.5 latest-reading read, kept as the parity reference. */ +function legacyLatestEver(userId: string): Promise { + return getPrismaClient().$queryRaw` + SELECT DISTINCT ON (m."type") + m."type" AS type, + m."value"::double precision AS value, + m."measured_at" AS measured_at + FROM measurements m + WHERE m."user_id" = ${userId} + AND m."deleted_at" IS NULL + ORDER BY m."type", m."measured_at" DESC + `; +} + +/** The pre-v1.38.5 sparkline read, without the `type` predicate. */ +function legacySparkBuckets(userId: string): Promise { + return getPrismaClient().$queryRaw` + SELECT type, bucket_start, mean, count, sum_value + FROM ( + SELECT + r."type" AS type, + r."bucket_start" AS bucket_start, + r."mean"::double precision AS mean, + r."count"::int AS count, + r."sum_value"::double precision AS sum_value, + ROW_NUMBER() OVER ( + PARTITION BY r."type" + ORDER BY r."bucket_start" DESC + ) AS rn + FROM measurement_rollups r + WHERE r."user_id" = ${userId} + AND r."granularity" = 'DAY' + ) sub + WHERE rn <= ${SPARK_DAYS} + ORDER BY type, bucket_start ASC + `; +} + +const DAY = 86_400_000; + +async function seedAccount(): Promise { + const prisma = getPrismaClient(); + const user = await prisma.user.create({ + data: { + username: "summary-reads-user", + email: "summary-reads-user@example.test", + role: "USER", + }, + }); + + // Live readings across several types, several rows each, so the + // "latest wins" pick has something to choose between. + const live: MeasurementType[] = [ + "WEIGHT", + "PULSE", + "BLOOD_PRESSURE_SYS", + "ACTIVITY_STEPS", + "SLEEP_DURATION", + ]; + const base = Date.UTC(2026, 0, 1); + const rows = live.flatMap((type, ti) => + Array.from({ length: 12 }, (_, i) => ({ + userId: user.id, + type, + value: 50 + ti * 10 + i, + unit: "x", + source: "MANUAL" as const, + measuredAt: new Date(base + i * DAY + ti * 3_600_000), + deletedAt: null as Date | null, + })), + ); + // BODY_FAT exists only as tombstones: both reads must drop it. + rows.push( + ...Array.from({ length: 4 }, (_, i) => ({ + userId: user.id, + type: "BODY_FAT" as MeasurementType, + value: 20 + i, + unit: "%", + source: "MANUAL" as const, + measuredAt: new Date(base + i * DAY + 7_200_000), + deletedAt: new Date(base + 30 * DAY), + })), + ); + // A live type whose newest row is tombstoned — the read must fall back + // to the newest surviving row, not report the deleted one. + rows.push( + { + userId: user.id, + type: "BLOOD_GLUCOSE" as MeasurementType, + value: 95, + unit: "mg/dL", + source: "MANUAL" as const, + measuredAt: new Date(base + 5 * DAY), + deletedAt: null, + }, + { + userId: user.id, + type: "BLOOD_GLUCOSE" as MeasurementType, + value: 999, + unit: "mg/dL", + source: "MANUAL" as const, + measuredAt: new Date(base + 9 * DAY), + deletedAt: new Date(base + 30 * DAY), + }, + ); + await prisma.measurement.createMany({ data: rows }); + + // DAY buckets past the SPARK_DAYS window, plus coarser granularities + // the DAY read must ignore. + const bucketRows = live.flatMap((type, ti) => + Array.from({ length: 11 }, (_, i) => ({ + userId: user.id, + type, + granularity: "DAY" as RollupGranularity, + bucketStart: new Date(base + i * DAY), + source: "MANUAL" as const, + count: 3, + mean: 60 + ti + i, + minValue: 50, + maxValue: 70, + sumValue: 180 + ti, + })), + ); + bucketRows.push( + ...(["WEEK", "MONTH", "YEAR"] as const).flatMap((granularity) => + live.map((type, ti) => ({ + userId: user.id, + type, + granularity, + bucketStart: new Date(base + ti * DAY), + source: "MANUAL" as const, + count: 9, + mean: 1000 + ti, + minValue: 900, + maxValue: 1100, + sumValue: 9000 + ti, + })), + ), + ); + await prisma.measurementRollup.createMany({ data: bucketRows }); + + return user.id; +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); +}); + +describe("dashboard summary reads — parity with the pre-v1.38.5 SQL", () => { + it("returns the identical latest-reading rows the DISTINCT ON returned", async () => { + const userId = await seedAccount(); + + const [legacy, shipped] = await Promise.all([ + legacyLatestEver(userId), + readLatestEver(getPrismaClient(), userId, TYPES), + ]); + + expect(shipped).toEqual(legacy); + // Not a vacuous pass: the fixture has to have produced rows. + expect(shipped.length).toBe(6); + // Tombstone-only type is absent from both; a type with no rows too. + expect(shipped.map((r) => r.type)).not.toContain("BODY_FAT"); + expect(shipped.map((r) => r.type)).not.toContain("VO2_MAX"); + // The newest LIVE glucose row wins, not the newer tombstoned one. + expect(shipped.find((r) => r.type === "BLOOD_GLUCOSE")?.value).toBe(95); + }); + + it("returns the identical sparkline buckets the type-less read returned", async () => { + const userId = await seedAccount(); + + const [legacy, shipped] = await Promise.all([ + legacySparkBuckets(userId), + readSparkBuckets(getPrismaClient(), userId, TYPES, SPARK_DAYS), + ]); + + expect(shipped).toEqual(legacy); + // Five types × the trailing SPARK_DAYS buckets, and nothing coarser. + expect(shipped.length).toBe(5 * SPARK_DAYS); + expect(shipped.every((r) => r.mean < 1000)).toBe(true); + }); + + it("orders both reads the way the callers' per-type maps expect", async () => { + const userId = await seedAccount(); + + const latest = await readLatestEver(getPrismaClient(), userId, TYPES); + // Enum declaration order, exactly as `ORDER BY m."type"` produced it. + const enumRank = new Map(TYPES.map((t, i) => [t, i])); + const ranks = latest.map((r) => enumRank.get(r.type) ?? -1); + expect(ranks).toEqual([...ranks].sort((a, b) => a - b)); + + const spark = await readSparkBuckets( + getPrismaClient(), + userId, + TYPES, + SPARK_DAYS, + ); + for (const type of new Set(spark.map((r) => r.type))) { + const stamps = spark + .filter((r) => r.type === type) + .map((r) => r.bucket_start.getTime()); + expect(stamps).toEqual([...stamps].sort((a, b) => a - b)); + } + }); +});