mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
Merge origin/main
This commit is contained in:
commit
98324b0eff
5 files changed
+554
-90
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);
|
||||
});
|
||||
});
|
||||
@@ -22,14 +22,16 @@
|
||||
* - the set of YYYY-MM-DD day-keys with any activity in 365 days
|
||||
*
|
||||
* v1.4.38 swaps the two unbounded reads for SQL aggregates:
|
||||
* - `DISTINCT ON (type)` → one row per type carrying the latest
|
||||
* value + measuredAt (≤ N_metrics rows). REG-11 (v1.4.44) dropped
|
||||
* the trailing-7-day window from the WHERE clause; the tile now
|
||||
* surfaces the all-time-latest reading per type so a 60-day-old BP
|
||||
* measurement still feeds the tile (paired with the stale-caption
|
||||
* hint built off `lastSeenAt`).
|
||||
* - `measurement_rollups` DAY buckets keyed `(user_id, granularity,
|
||||
* bucketStart)` → at most 7 buckets per metric × N_metrics rows.
|
||||
* - one row per type carrying the latest value + measuredAt
|
||||
* (≤ N_metrics rows). REG-11 (v1.4.44) dropped the trailing-7-day
|
||||
* window from the WHERE clause; the tile now surfaces the all-time-
|
||||
* latest reading per type so a 60-day-old BP measurement still feeds
|
||||
* the tile (paired with the stale-caption hint built off
|
||||
* `lastSeenAt`). v1.38.5 replaced the `DISTINCT ON (type)` that
|
||||
* produced those rows with a LATERAL join over the type list; both
|
||||
* per-type reads now live in `@/lib/dashboard/summary-reads`.
|
||||
* - `measurement_rollups` DAY buckets keyed `(user_id, type,
|
||||
* granularity, bucketStart)` → at most 7 buckets per metric × N_metrics rows.
|
||||
* Sparkline points become the bucket means rather than individual
|
||||
* raw samples, which is a *smoother* trend signal for high-volume
|
||||
* metrics like ACTIVITY_STEPS and bounded for every other metric.
|
||||
@@ -63,6 +65,10 @@ import { defaultLocale, locales, type Locale } from "@/lib/i18n/config";
|
||||
import { userDayKey, DEFAULT_TIMEZONE } from "@/lib/tz/resolver";
|
||||
import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache";
|
||||
import { buildMedsTodayBlock } from "@/lib/dashboard/meds-today";
|
||||
import {
|
||||
readLatestEver,
|
||||
readSparkBuckets,
|
||||
} from "@/lib/dashboard/summary-reads";
|
||||
import { resolveModuleMap } from "@/lib/modules/gate";
|
||||
import { gateMetricCardsByModules } from "@/lib/dashboard/widget-modules";
|
||||
import {
|
||||
@@ -295,48 +301,6 @@ function computeStreak(activityDays: Set<string>, userTz: string): StreakInfo {
|
||||
return { currentDays, longest };
|
||||
}
|
||||
|
||||
/** v1.4.38 W-F — `DISTINCT ON (type)` row for the single most recent
|
||||
* reading per measurement type, irrespective of age. One row per
|
||||
* metric the user has ever touched; replaces the legacy unbounded
|
||||
* `prisma.measurement.findMany`.
|
||||
*
|
||||
* REG-11 (v1.4.44): dropped the trailing-7-day window from the WHERE
|
||||
* clause. The old shape returned `latestValue: null` + an empty
|
||||
* sparkline for accounts whose last BP / pulse reading was older than
|
||||
* 7 days, leaving the iOS tile blank even though the historical row
|
||||
* was still in the database. The all-time aggregate (`groupBy` on
|
||||
* `measurements`) already proves the row exists; the row count is
|
||||
* still bounded by `|measurementTypes|` via `DISTINCT ON`. */
|
||||
interface LatestEverRow {
|
||||
type: MeasurementType;
|
||||
value: number;
|
||||
measured_at: Date;
|
||||
}
|
||||
|
||||
/** v1.4.38 W-F — per-day measurement_rollup bucket feeding the dashboard
|
||||
* sparkline. At most 7 buckets per metric × N metrics — bounded by
|
||||
* `SPARK_DAYS * |measurementTypes|` rather than the raw row count.
|
||||
*
|
||||
* v1.4.39 W-SUM — `sum_value` rides along so the cumulative tile
|
||||
* (ACTIVITY_STEPS) renders the daily SUM rather than the per-bucket
|
||||
* MEAN. Spot metrics ignore the column; cumulative tiles fall back to
|
||||
* `mean * count` when the legacy NULL hits (boot-backfill convergence
|
||||
* window).
|
||||
*
|
||||
* REG-11 (v1.4.44): switched from a calendar-window filter
|
||||
* (`bucket_start >= sevenDaysAgo`) to a `ROW_NUMBER() OVER (PARTITION
|
||||
* BY type ORDER BY bucket_start DESC)` window so the sparkline takes
|
||||
* the last `SPARK_DAYS` daily buckets per type regardless of age. An
|
||||
* account whose last BP reading is 60 days old now still gets the
|
||||
* trailing 7 days of historical buckets feeding the tile chart. */
|
||||
interface SparklineRow {
|
||||
type: MeasurementType;
|
||||
bucket_start: Date;
|
||||
mean: number;
|
||||
count: number;
|
||||
sum_value: number | null;
|
||||
}
|
||||
|
||||
/** v1.4.38 W-F — distinct activity day-keys from the streak window.
|
||||
* The route only needs YYYY-MM-DD strings, so the aggregate runs
|
||||
* `date_trunc('day', m.measured_at AT TIME ZONE $userTz)` and returns
|
||||
@@ -478,9 +442,9 @@ async function buildDashboardSummary(
|
||||
|
||||
// v1.4.38 W-F — six bounded sub-queries replace the legacy 4
|
||||
// unbounded ones. Row counts are now:
|
||||
// - latestEver: ≤ N metric types (one row per type via DISTINCT ON,
|
||||
// REG-11: dropped the 7-day window so a stale-but-
|
||||
// valid historical reading still feeds the tile)
|
||||
// - latestEver: ≤ N metric types (one indexed lookup per type via
|
||||
// LATERAL, REG-11: dropped the 7-day window so a
|
||||
// stale-but-valid historical reading still feeds the tile)
|
||||
// - sparkBuckets: ≤ SPARK_DAYS × N metric types (typically <80,
|
||||
// REG-11: taken via ROW_NUMBER window, no calendar
|
||||
// filter, so a 60-day-old metric still paints a chart)
|
||||
@@ -498,43 +462,15 @@ async function buildDashboardSummary(
|
||||
measurementStreakDays,
|
||||
sleepStageRows,
|
||||
] = await Promise.all([
|
||||
time(
|
||||
"latestEver",
|
||||
() =>
|
||||
prisma.$queryRaw<LatestEverRow[]>`
|
||||
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
|
||||
`,
|
||||
),
|
||||
time(
|
||||
"sparkline",
|
||||
() =>
|
||||
prisma.$queryRaw<SparklineRow[]>`
|
||||
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
|
||||
`,
|
||||
// Both reads live in `@/lib/dashboard/summary-reads` — the SQL is a
|
||||
// contract the integration suite exercises directly against Postgres,
|
||||
// and each carries the reason it names the type list at its
|
||||
// definition. Short version: PostgreSQL 16 has no index skip scan, so
|
||||
// "one row per type" has to be driven FROM the type list or it
|
||||
// degrades to a full pass over the account.
|
||||
time("latestEver", () => readLatestEver(prisma, userId, measurementTypes)),
|
||||
time("sparkline", () =>
|
||||
readSparkBuckets(prisma, userId, measurementTypes, SPARK_DAYS),
|
||||
),
|
||||
time("allTime", () =>
|
||||
prisma.measurement.groupBy({
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
/**
|
||||
* The two per-type reads behind the dashboard summary's metric tiles.
|
||||
*
|
||||
* Both used to live inline in `src/app/api/dashboard/summary/route.ts`.
|
||||
* They are lifted out for the same reason `buildMedsTodayBlock` is: the
|
||||
* SQL is the contract, and a contract that only exists inside a route
|
||||
* handler cannot be exercised against a real Postgres. The integration
|
||||
* suite runs THESE functions, not a transcription of them.
|
||||
*
|
||||
* Both reads are shaped by one property of the schema: PostgreSQL 16 has
|
||||
* no index skip scan. An index on `(user_id, type, …)` cannot hand back
|
||||
* "one row per type" on its own — something has to supply the type list.
|
||||
* So both queries name it. `measurementTypes` is the complete canonical
|
||||
* enum, so naming it filters nothing out; it exists to give the planner
|
||||
* an equality on the leading type column, which turns a full pass over
|
||||
* the account into one bounded lookup per type.
|
||||
*/
|
||||
import type { PrismaClient } from "@/generated/prisma/client";
|
||||
import type { MeasurementType } from "@/generated/prisma/client";
|
||||
|
||||
/** v1.4.38 W-F — the single most recent reading per measurement
|
||||
* type, irrespective of age. One row per metric the user has ever
|
||||
* touched; replaces the legacy unbounded `prisma.measurement.findMany`.
|
||||
*
|
||||
* REG-11 (v1.4.44): dropped the trailing-7-day window from the WHERE
|
||||
* clause. The old shape returned `latestValue: null` + an empty
|
||||
* sparkline for accounts whose last BP / pulse reading was older than
|
||||
* 7 days, leaving the iOS tile blank even though the historical row
|
||||
* was still in the database. The all-time aggregate (`groupBy` on
|
||||
* `measurements`) already proves the row exists; the row count is
|
||||
* still bounded by `|measurementTypes|`, now because the query is
|
||||
* driven FROM that list (v1.38.5) rather than grouped down to it. */
|
||||
export interface LatestEverRow {
|
||||
type: MeasurementType;
|
||||
value: number;
|
||||
measured_at: Date;
|
||||
}
|
||||
|
||||
/** v1.4.38 W-F — per-day measurement_rollup bucket feeding the dashboard
|
||||
* sparkline. At most `sparkDays` buckets per metric × N metrics —
|
||||
* bounded by `sparkDays * |measurementTypes|` rather than the raw row
|
||||
* count.
|
||||
*
|
||||
* v1.4.39 W-SUM — `sum_value` rides along so the cumulative tile
|
||||
* (ACTIVITY_STEPS) renders the daily SUM rather than the per-bucket
|
||||
* MEAN. Spot metrics ignore the column; cumulative tiles fall back to
|
||||
* `mean * count` when the legacy NULL hits (boot-backfill convergence
|
||||
* window).
|
||||
*
|
||||
* REG-11 (v1.4.44): switched from a calendar-window filter
|
||||
* (`bucket_start >= sevenDaysAgo`) to a `ROW_NUMBER() OVER (PARTITION
|
||||
* BY type ORDER BY bucket_start DESC)` window so the sparkline takes
|
||||
* the last `sparkDays` daily buckets per type regardless of age. An
|
||||
* account whose last BP reading is 60 days old now still gets the
|
||||
* trailing 7 days of historical buckets feeding the tile chart. */
|
||||
export interface SparklineRow {
|
||||
type: MeasurementType;
|
||||
bucket_start: Date;
|
||||
mean: number;
|
||||
count: number;
|
||||
sum_value: number | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Latest live reading per measurement type.
|
||||
*
|
||||
* v1.38.5 — a LATERAL join over the type list, replacing the
|
||||
* `DISTINCT ON (m."type")` this read used to be.
|
||||
*
|
||||
* The DISTINCT-ON shape has to produce every live row of the account in
|
||||
* `(type, measured_at DESC)` order before it can throw all but the first
|
||||
* of each group away. With no skip scan there is no index that hands it
|
||||
* one row per type, so on a multi-year account it degrades to a
|
||||
* sequential scan plus an in-memory sort that spills to disk once it
|
||||
* outgrows `work_mem`.
|
||||
*
|
||||
* Driving the read from the type list instead makes it one index lookup
|
||||
* per type against `measurements_live_covering_idx` (the partial
|
||||
* `(user_id, type, measured_at DESC) WHERE deleted_at IS NULL` index from
|
||||
* migration 0183), each stopping at the first row. `CROSS JOIN LATERAL`
|
||||
* is an inner join, so a type with no live reading drops out exactly as
|
||||
* DISTINCT ON dropped it, and `ORDER BY mt."type"` reproduces the enum
|
||||
* ordering the old `ORDER BY m."type"` emitted. Same rows, same order.
|
||||
*
|
||||
* The driving relation is aliased `mt`, not `t`: a one-letter `t` alias
|
||||
* makes `t("type")` in the SQL text look exactly like a `t()` translation
|
||||
* call to the i18n call-site scanner, which then reports a missing key.
|
||||
*/
|
||||
export function readLatestEver(
|
||||
db: PrismaClient,
|
||||
userId: string,
|
||||
measurementTypes: readonly MeasurementType[],
|
||||
): Promise<LatestEverRow[]> {
|
||||
return db.$queryRaw<LatestEverRow[]>`
|
||||
SELECT
|
||||
mt."type" AS type,
|
||||
l."value" AS value,
|
||||
l."measured_at" AS measured_at
|
||||
FROM unnest(${[...measurementTypes]}::"measurement_type"[]) AS mt("type")
|
||||
CROSS JOIN LATERAL (
|
||||
SELECT
|
||||
m."value"::double precision AS value,
|
||||
m."measured_at" AS measured_at
|
||||
FROM measurements m
|
||||
WHERE m."user_id" = ${userId}
|
||||
AND m."type" = mt."type"
|
||||
AND m."deleted_at" IS NULL
|
||||
ORDER BY m."measured_at" DESC
|
||||
LIMIT 1
|
||||
) l
|
||||
ORDER BY mt."type"
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Trailing `sparkDays` DAY rollup buckets per measurement type.
|
||||
*
|
||||
* v1.38.5 — the `type` predicate is what makes this read indexable.
|
||||
*
|
||||
* `measurement_rollups` is keyed `(user_id, type, granularity,
|
||||
* bucket_start, source)`. Filtering on `user_id` and `granularity` alone
|
||||
* skips the second key column, so on a single-account host — where
|
||||
* `user_id` selects nearly the whole table — neither the primary key nor
|
||||
* `measurement_rollups_user_type_granularity_bucket_desc_idx` can drive
|
||||
* the read, and Postgres falls back to a sequential scan over every
|
||||
* granularity the account has ever rolled up.
|
||||
*
|
||||
* Naming the full enum restores the equality on `type` and the read
|
||||
* becomes a bitmap index scan over the DAY slice. Deliberately NOT
|
||||
* narrowed to the types the metric cards happen to render: the full enum
|
||||
* keeps the returned row set provably identical to the unfiltered read,
|
||||
* and a new measurement type stays included by construction.
|
||||
*/
|
||||
export function readSparkBuckets(
|
||||
db: PrismaClient,
|
||||
userId: string,
|
||||
measurementTypes: readonly MeasurementType[],
|
||||
sparkDays: number,
|
||||
): Promise<SparklineRow[]> {
|
||||
return db.$queryRaw<SparklineRow[]>`
|
||||
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."type" = ANY(${[...measurementTypes]}::"measurement_type"[])
|
||||
AND r."granularity" = 'DAY'
|
||||
) sub
|
||||
WHERE rn <= ${sparkDays}
|
||||
ORDER BY type, bucket_start ASC
|
||||
`;
|
||||
}
|
||||
Reference in new issue
Block a user