From 3b4a5bc35a7105a1cba243426c47960a9c7cfe25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 21 May 2026 09:01:59 +0200 Subject: [PATCH] fix(rollups): boot-backfill discovery anchors on per-(user, type, day) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-fix boot-time discovery used a `(user, type)` LEFT JOIN against `measurement_rollups`: an account surfaced only when the ENTIRE type had zero rollup rows. Once any single day for that type had a row (e.g. from a prior boot backfill), the user dropped off the discovery list, even if 27 OTHER days for the same type were missing rollup rows because the Withings sync or `/api/import` path had written measurements without firing the rollup hook. Switching the inner DISTINCT to `(user_id, type, bucket_start)` and joining on `r.bucket_start = mt.bucket_start` surfaces every per-day gap. Cost: the aggregate grows from ~15 rows / user to ~5 k rows / user on a 5-year power user — still bounded by the existing `(user_id, type, measured_at)` index path and the rollup table's composite primary key. The legacy `sum_value IS NULL` branch stays on the same UNION so v1.4.39 write-tier converges continue working. A new text-anchored unit test pins the per-day join shape so a future refactor cannot silently revert to the per-type form. --- .../measurements/__tests__/rollups.test.ts | 28 +++++++++++-- src/lib/measurements/rollups.ts | 39 ++++++++++++------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/lib/measurements/__tests__/rollups.test.ts b/src/lib/measurements/__tests__/rollups.test.ts index 06382789e..cdc550aa8 100644 --- a/src/lib/measurements/__tests__/rollups.test.ts +++ b/src/lib/measurements/__tests__/rollups.test.ts @@ -546,8 +546,8 @@ describe("enqueueBootTimeRollupBackfill", () => { // v1.4.39 W-SUM — the discovery query now unions in users whose // existing DAY rollup rows carry `sum_value IS NULL`. Re-folding // converges those rows because `persistRollupRows` always writes the - // new column on upsert; the union keeps the per-type missing-coverage - // gap (v1.4.38.5) and the legacy-NULL backfill on the same indexed + // new column on upsert; the union keeps the per-day missing-coverage + // gap (v1.4.39.1) and the legacy-NULL backfill on the same indexed // pass. it("includes the sum_value IS NULL branch in the discovery query", async () => { getGlobalBossMock.mockReturnValue({ send: bossSend }); @@ -557,7 +557,7 @@ describe("enqueueBootTimeRollupBackfill", () => { const result = await enqueueBootTimeRollupBackfill(); expect(result).toEqual({ enqueued: 1, skipped: 0, error: null }); - // The discovery SQL surfaces both per-type missing coverage AND + // The discovery SQL surfaces both per-day missing coverage AND // legacy NULL sum_value rows under one UNION. The text-anchor is // the only durable assertion at the unit level — the integration // suite covers the planner shape on real Postgres. @@ -566,4 +566,26 @@ describe("enqueueBootTimeRollupBackfill", () => { expect(sqlText).toContain("sum_value"); expect(sqlText).toContain("UNION"); }); + + // v1.4.39.1 — the discovery anchor moved from per-type to per-day so + // accounts whose Withings sync / `/api/import` / admin restore wrote + // measurements without firing the rollup write hook get re-enqueued + // on the next worker boot. Pre-fix, the per-type LEFT JOIN matched + // on `(user, type)` and the user dropped off the discovery list as + // soon as ONE rollup row existed for the type — even if 27 OTHER + // days for that type were missing rollup rows. + it("anchors the missing-coverage join on (user, type, bucket_start) so per-day gaps surface", async () => { + getGlobalBossMock.mockReturnValue({ send: bossSend }); + queryRaw.mockResolvedValueOnce([{ id: "withings-stranded-user" }]); + bossSend.mockResolvedValueOnce("job-id"); + + await enqueueBootTimeRollupBackfill(); + + const sqlParts = queryRaw.mock.calls[0][0] as TemplateStringsArray; + const sqlText = Array.isArray(sqlParts) ? sqlParts.join("?") : ""; + // Inner DISTINCT widens to `(user_id, type, bucket_start)`. + expect(sqlText).toContain('date_trunc(\'day\', m."measured_at")'); + // LEFT JOIN now compares bucket_start on both sides. + expect(sqlText).toContain('r."bucket_start" = mt."bucket_start"'); + }); }); diff --git a/src/lib/measurements/rollups.ts b/src/lib/measurements/rollups.ts index 9cb3fe394..74f9bb455 100644 --- a/src/lib/measurements/rollups.ts +++ b/src/lib/measurements/rollups.ts @@ -741,35 +741,46 @@ export async function enqueueBootTimeRollupBackfill(): Promise<{ } try { - // Users where at least one measurement type lacks DAY-rollup - // coverage. Anchored on the per-user `DISTINCT type` set so the - // planner uses the `(user_id, type, measured_at)` index path; the - // LEFT JOIN onto `measurement_rollups` uses the - // `(user_id, type, granularity, bucket_start)` composite primary - // key. `measurement_rollups` has no surrogate `id` column — - // `r."bucket_start" IS NULL` (any column from the right side of - // the LEFT JOIN) marks the unmatched partitions. A user surfaces - // here when ANY type is missing — including the brand-new-type - // case that v1.4.35.1's zero-rollup-only shape silently stranded. + // Users where at least one (type, day) pair in `measurements` + // has no matching DAY rollup row. + // + // v1.4.39.1 — tightened from per-type to per-day. The legacy shape + // matched users where ANY type had zero rollup rows. That stranded + // accounts whose Withings sync / `/api/import` / admin restore + // wrote measurements without firing the rollup write hook: as soon + // as ONE rollup row existed for the type (even from a prior boot + // backfill), the LEFT JOIN found a match and the user dropped off + // the discovery list — leaving the dashboard chart's + // `source=rollup` fast-path under-counting the missing days. + // Switching the join to `(user, type, bucketStart)` surfaces every + // per-day gap. Cost: the inner DISTINCT widens from + // `(user_id, type)` (≈15 rows/user) to + // `(user_id, type, day)` (≈5 k rows/user on a 5-year power user), + // which is still bounded by the `(user_id, type, measured_at)` + // index path. The LEFT JOIN walks `measurement_rollups` against + // its composite primary key. // // v1.4.39 W-SUM — the discovery also surfaces users whose existing // DAY rollup rows carry `sum_value IS NULL`. Those rows pre-date // the v1.4.39 writer change; re-folding the user converges them // because `persistRollupRows` always writes the new column on - // upsert. The union runs in one query so the existing per-type - // coverage gap and the legacy-NULL backfill share a single planner - // pass. + // upsert. The union runs in one query so the per-day coverage gap + // and the legacy-NULL backfill share a single planner pass. const users = await prisma.$queryRaw>` SELECT DISTINCT id FROM ( SELECT DISTINCT mt."user_id" AS id FROM ( - SELECT DISTINCT m."user_id", m."type" + SELECT DISTINCT + m."user_id", + m."type", + date_trunc('day', m."measured_at") AS bucket_start FROM measurements m ) mt LEFT JOIN measurement_rollups r ON r."user_id" = mt."user_id" AND r."type" = mt."type" AND r."granularity" = 'DAY' + AND r."bucket_start" = mt."bucket_start" WHERE r."bucket_start" IS NULL UNION SELECT DISTINCT r2."user_id" AS id