mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
test: pin the dashboard summary read shapes against a plan regression
Neither read is a bug a behavioural test can see: both shapes return the right rows, and only the plan differs. So the guards split by what each can actually answer. The unit guard asserts on the SQL the readers build, with comments stripped so it matches the query text and never the prose describing it: no DISTINCT ON on this path, a LATERAL driven from an unnested type list with a LIMIT on the per-type branch, a bound type predicate on the rollup read sitting alongside the user and granularity filters, and the route still calling the shared readers rather than growing a second copy. Four of its five cases fail against the pre-change SQL. The integration companion runs the shipped readers and the pre-change SQL side by side over one fixture on the real Postgres and requires identical rows. The fixture carries the cases the rewrite could have broken: a type whose only rows are tombstoned, a type with no rows, a type whose newest row is a tombstone over a surviving one, and coarser rollup granularities the DAY read must ignore. Dropping the soft-delete filter or shortening the type list makes both cases fail. Plan assertions are deliberately not in the integration file: a testcontainer fixture is too small for the planner to choose the access paths a real account produces, so an EXPLAIN there would pin nothing.
This commit is contained in:
1 parent
f2e98e3885
commit
83b6ce4b15
2 files changed
+353
No files matched your search
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user