From da2cd6287650b4d63a79f2e0c431d90de8001989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 25 Oct 2026 00:33:55 +0200 Subject: [PATCH] fix(rollups): register read-path warm-ups with the background registry The mood rollup warm-up on the dashboard summary was started with a bare `void`, so `settleBackgroundTasks()` never saw it. On a stale account the warm-up started a full recompute that could still be writing when the statement counter closed its window, and the hot-route budget read 30 against 28 depending on where it landed. Every read-path start of that shape now goes through `trackBackgroundTask(runDetached(...))`: the measurement and mood rollup warm-ups (seven call sites), the dashboard's today-briefing warm, and the two delegated-access bookkeeping writes in the record guard. A guard refuses a bare `void` on any of them. `trackBackgroundTask` now wraps its handle in `Promise.resolve`: a test double that returns nothing used to leave `undefined` in the set, and settle spun on it at full CPU. The budget fixture mints the mood rollups in its seed, as it already did for the measurement rollups. Eight serial runs of query-budget-hot-routes read the same counts every time (dashboard summary 28, snapshot 99, day 47, health-status 59, correlations 43). --- .../background-task-settle-guard.test.ts | 8 +++ src/__tests__/detached-work-guard.test.ts | 53 +++++++++++++++++++ .../analytics/__tests__/mood-series.test.ts | 31 ++++++++++- src/lib/analytics/mood-series.ts | 6 ++- src/lib/analytics/response/default-body.ts | 4 +- src/lib/analytics/summaries-slice.ts | 4 +- src/lib/auth/guards/record.ts | 8 ++- src/lib/dashboard/snapshot-read.ts | 22 +++++--- src/lib/insights/comprehensive-aggregator.ts | 4 +- .../comprehensive-overview/mood-series.ts | 4 +- src/lib/insights/mood-daily-points.ts | 4 +- src/lib/logging/background-tasks.ts | 11 ++-- src/lib/targets/build-response.ts | 4 +- .../query-budget-hot-routes.test.ts | 6 +++ 14 files changed, 148 insertions(+), 21 deletions(-) diff --git a/src/__tests__/background-task-settle-guard.test.ts b/src/__tests__/background-task-settle-guard.test.ts index e6b564813..dad2f0e28 100644 --- a/src/__tests__/background-task-settle-guard.test.ts +++ b/src/__tests__/background-task-settle-guard.test.ts @@ -142,6 +142,14 @@ describe("background-task settle contract", () => { expect(pendingBackgroundTaskCount()).toBe(0); }); + it("drains a handle that is not a promise instead of spinning on it", async () => { + // A mocked background starter that returns nothing hands the registry + // `undefined`; left in the set, settle would loop on it forever. + trackBackgroundTask(undefined as unknown as Promise); + await settleBackgroundTasks(); + expect(pendingBackgroundTaskCount()).toBe(0); + }); + it("registers what fireAndForget starts", async () => { let release: () => void = () => {}; fireAndForget( diff --git a/src/__tests__/detached-work-guard.test.ts b/src/__tests__/detached-work-guard.test.ts index 8f3992048..c4903abf0 100644 --- a/src/__tests__/detached-work-guard.test.ts +++ b/src/__tests__/detached-work-guard.test.ts @@ -91,3 +91,56 @@ describe("detached work cannot rename the request's event", () => { expect(startsDetached(detached)).toBe(true); }); }); + +/** + * Read-path work a request starts and never awaits. Each of these runs real + * statements against Postgres after the response is already on its way, so a + * bare `void` hides it from `settleBackgroundTasks()`: the statement counter + * then closes its window while a stale account's rollup recompute is still + * writing, and the next test inherits the tail. The dashboard summary budget + * read 30 against 28 that way, depending on which side of the window a full + * mood recompute landed. + */ +const READ_PATH_BACKGROUND_STARTERS = [ + "ensureUserRollupsFresh", + "ensureUserMoodRollupsFresh", + "requestTodayBriefingWarm", + "touchGrantUsage", + "recordDelegatedAccess", +] as const; + +describe("read-path background work is registered", () => { + const files = readSourceFolder("src", { + floor: 3000, + include: (rel) => + !rel.startsWith("generated/") && + !rel.includes("__tests__/") && + !/\.test\.tsx?$/.test(rel), + }); + const bare = new RegExp( + String.raw`\bvoid\s+(${READ_PATH_BACKGROUND_STARTERS.join("|")})\s*\(`, + ); + + it("starts none of them with a bare void", () => { + const offenders = files.flatMap(([rel, text]) => + stripComments(text) + .split("\n") + .flatMap((line, i) => (bare.test(line) ? [`src/${rel}:${i + 1}`] : [])), + ); + expect( + offenders, + "wrap the call in trackBackgroundTask(runDetached(() => ...)) or " + + "fireAndForget(...) so the suite can await it", + ).toEqual([]); + }); + + it("still finds each starter at a detached call site", () => { + const joined = files.map(([, text]) => stripComments(text)).join("\n"); + for (const name of READ_PATH_BACKGROUND_STARTERS) { + expect( + new RegExp(String.raw`=>\s*${name}\s*\(`).test(joined), + `${name} is no longer started detached anywhere; drop it from the list`, + ).toBe(true); + } + }); +}); diff --git a/src/lib/analytics/__tests__/mood-series.test.ts b/src/lib/analytics/__tests__/mood-series.test.ts index e29667876..a3ba1fcd2 100644 --- a/src/lib/analytics/__tests__/mood-series.test.ts +++ b/src/lib/analytics/__tests__/mood-series.test.ts @@ -31,7 +31,14 @@ vi.mock("@/lib/rollups/mood-rollups", async () => { import { buildMoodDailySeries } from "../mood-series"; import { prisma } from "@/lib/db"; -import { readMoodDayRollups } from "@/lib/rollups/mood-rollups"; +import { + ensureUserMoodRollupsFresh, + readMoodDayRollups, +} from "@/lib/rollups/mood-rollups"; +import { + pendingBackgroundTaskCount, + settleBackgroundTasks, +} from "@/lib/logging/background-tasks"; beforeEach(() => { vi.mocked(prisma.moodEntry.findMany).mockResolvedValue([] as never); @@ -104,4 +111,26 @@ describe("buildMoodDailySeries", () => { expect(series.source).toBe("rollup"); expect(series.summary.count).toBe(0); }); + + it("files the rollup warm-up with the background registry", async () => { + // The warm-up is not awaited by the read, so a caller that has to know + // when the request's work is over (the statement counter, the suite's + // settle hook) can only see it through the registry. Left unregistered, + // a stale account's full recompute bled into whatever ran next. + let release: () => void = () => {}; + vi.mocked(ensureUserMoodRollupsFresh).mockImplementationOnce( + () => + new Promise((resolve) => { + release = () => resolve({ recomputed: true }); + }), + ); + + await buildMoodDailySeries("user-1"); + + expect(ensureUserMoodRollupsFresh).toHaveBeenCalledWith("user-1"); + expect(pendingBackgroundTaskCount()).toBe(1); + release(); + await settleBackgroundTasks(); + expect(pendingBackgroundTaskCount()).toBe(0); + }); }); diff --git a/src/lib/analytics/mood-series.ts b/src/lib/analytics/mood-series.ts index 8e57d64e3..e921062c5 100644 --- a/src/lib/analytics/mood-series.ts +++ b/src/lib/analytics/mood-series.ts @@ -12,6 +12,8 @@ */ import type { PrismaClient } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { summarize, type DataPoint } from "@/lib/analytics/trends"; import { ensureUserMoodRollupsFresh, @@ -94,7 +96,9 @@ export async function buildMoodDailySeries( ): Promise { // Fire-and-forget warm-up so the next cold mount for this user lands on // the rollup tier even when the boot-time backfill hasn't reached them. - void ensureUserMoodRollupsFresh(userId); + // Registered so a caller that must know when the request's work is over + // (the suite's settle hook, the statement counter) can await it. + trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId))); const since = new Date(Date.now() - MOOD_SERIES_WINDOW_MS); const rollups = await readMoodDayRollups(userId, since); diff --git a/src/lib/analytics/response/default-body.ts b/src/lib/analytics/response/default-body.ts index 4ba36afe1..e8c5bb87d 100644 --- a/src/lib/analytics/response/default-body.ts +++ b/src/lib/analytics/response/default-body.ts @@ -5,6 +5,8 @@ * the sleep stages. The stages run in the order the route always ran them. */ import type { SessionUser } from "@/lib/auth/session-user"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { annotate } from "@/lib/logging/context"; import type { Locale } from "@/lib/i18n/config"; import { DEFAULT_TIMEZONE } from "@/lib/tz/resolver"; @@ -42,7 +44,7 @@ export async function buildAnalyticsResponse( // measurement lands, and the next request returns the up-to-date // value once the background refresh completes. `ensureUserRollupsFresh` // wraps its own try/catch, so the void call cannot reject. - void ensureUserRollupsFresh(user.id); + trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(user.id))); // Single per-type coverage probe shared by the // bp_in_target / healthScore / correlations branches below. The diff --git a/src/lib/analytics/summaries-slice.ts b/src/lib/analytics/summaries-slice.ts index 031022a05..f7d481893 100644 --- a/src/lib/analytics/summaries-slice.ts +++ b/src/lib/analytics/summaries-slice.ts @@ -59,6 +59,8 @@ import pLimit from "p-limit"; import type { MeasurementType } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { HOURLY_MEAN_DAY_TYPES, usesHourlyMeanDay, @@ -284,7 +286,7 @@ export async function computeSummariesSlice( // coverage probe falls back to live SQL for uncovered types, so // correctness is preserved; the read just doesn't block waiting // for the background refresh. - void ensureUserRollupsFresh(userId); + trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(userId))); // v1.4.36 — per-type coverage probe replaces the legacy global // COUNT. The previous gate returned true as soon as ANY type had at diff --git a/src/lib/auth/guards/record.ts b/src/lib/auth/guards/record.ts index 076450841..44010e554 100644 --- a/src/lib/auth/guards/record.ts +++ b/src/lib/auth/guards/record.ts @@ -1,5 +1,7 @@ import type { AccountGrant } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { SESSION_USER_SELECT, type SessionUser } from "@/lib/auth/session-user"; import { annotate, getEvent } from "@/lib/logging/context"; import { recordDelegatedAccess } from "@/lib/auth/audit"; @@ -372,13 +374,15 @@ async function resolveSwitchedRecord( }); // Fire-and-forget, the `ApiToken.lastUsedAt` posture: the read must not wait // on the bookkeeping, and the bookkeeping failing must not fail the read. - void touchGrantUsage(grant.id); + trackBackgroundTask(runDetached(() => touchGrantUsage(grant.id))); // The owner's copy of the same fact, and the one they can read: a day- // coalesced row on their own record saying who opened it. It lands here // rather than in the handlers because a route cannot forget what it never // had to remember — every delegated request in the product passes through // this line, and only requests that got past the grant check reach it. - void recordDelegatedAccess(owner.id, auth.user.id); + trackBackgroundTask( + runDetached(() => recordDelegatedAccess(owner.id, auth.user.id)), + ); return { session: auth.session, diff --git a/src/lib/dashboard/snapshot-read.ts b/src/lib/dashboard/snapshot-read.ts index cd98809e0..4038d91cd 100644 --- a/src/lib/dashboard/snapshot-read.ts +++ b/src/lib/dashboard/snapshot-read.ts @@ -24,6 +24,8 @@ import type { SessionUser } from "@/lib/auth/session-user"; import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { annotate } from "@/lib/logging/context"; import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache"; import { dashboardSnapshotCacheKey } from "@/lib/cache/invalidate"; @@ -201,14 +203,18 @@ export async function readDashboardSnapshotCached( // its claim, never thrown. if (briefingAi.available) { const lastSeen = cached.tiles?.lastSeenByType ?? {}; - void requestTodayBriefingWarm({ - userId: user.id, - locale, - timezone: user.timezone, - generatedAt, - lastSeenAt: (type) => lastSeen[type]?.lastSeenAt ?? null, - now, - }).catch(() => undefined); + trackBackgroundTask( + runDetached(() => + requestTodayBriefingWarm({ + userId: user.id, + locale, + timezone: user.timezone, + generatedAt, + lastSeenAt: (type) => lastSeen[type]?.lastSeenAt ?? null, + now, + }), + ).catch(() => undefined), + ); } return { body, locale }; diff --git a/src/lib/insights/comprehensive-aggregator.ts b/src/lib/insights/comprehensive-aggregator.ts index 0c9e035a5..5f46f0cc0 100644 --- a/src/lib/insights/comprehensive-aggregator.ts +++ b/src/lib/insights/comprehensive-aggregator.ts @@ -98,6 +98,8 @@ * shape `summarize()` would return. */ import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import type { DataSummary } from "@/lib/analytics/trends"; import { liveTrend, @@ -261,7 +263,7 @@ export async function buildComprehensiveAggregate( // coverage probe falls back to live SQL for uncovered types, so // correctness is preserved; the read just doesn't block waiting // for the background refresh. - void ensureUserRollupsFresh(userId); + trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(userId))); // v1.4.36 — per-type coverage probe. The legacy global COUNT // returned true as soon as any one type had a DAY bucket, which made diff --git a/src/lib/insights/comprehensive-overview/mood-series.ts b/src/lib/insights/comprehensive-overview/mood-series.ts index 40efa5afd..6deda77c9 100644 --- a/src/lib/insights/comprehensive-overview/mood-series.ts +++ b/src/lib/insights/comprehensive-overview/mood-series.ts @@ -22,6 +22,8 @@ * left on it deliberately (see their call sites in `build.ts`). */ import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import type { DataPoint } from "@/lib/analytics/trends"; import { ensureUserMoodRollupsFresh, @@ -41,7 +43,7 @@ export async function readOverviewMoodSeries( userId: string, since: Date, ): Promise { - void ensureUserMoodRollupsFresh(userId); + trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId))); const moodRollupDayRows = await readMoodDayRollups(userId, since); if (moodRollupDayRows.length > 0) { return { diff --git a/src/lib/insights/mood-daily-points.ts b/src/lib/insights/mood-daily-points.ts index 00b91b87a..9309c1bb0 100644 --- a/src/lib/insights/mood-daily-points.ts +++ b/src/lib/insights/mood-daily-points.ts @@ -4,6 +4,8 @@ * Split out of `features.ts`, which builds the block from them. */ import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { ensureUserMoodRollupsFresh, readMoodDayRollups, @@ -39,7 +41,7 @@ export async function readMoodDailyPoints( totalEntries: number; latestScore: number | null; }> { - void ensureUserMoodRollupsFresh(userId); + trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId))); // Five-year window mirrors the mood-rollup writer default; covers // every realistic user history span without an unbounded scan. const fiveYearsAgo = new Date(Date.now() - 5 * 365 * 24 * 60 * 60 * 1000); diff --git a/src/lib/logging/background-tasks.ts b/src/lib/logging/background-tasks.ts index 5516ef44e..7b1dee275 100644 --- a/src/lib/logging/background-tasks.ts +++ b/src/lib/logging/background-tasks.ts @@ -56,9 +56,14 @@ const inFlight = new Set>(); export function trackBackgroundTask(settled: Promise): void { if (!TRACK_IN_FLIGHT) return; - inFlight.add(settled); - void settled.finally(() => { - inFlight.delete(settled); + // A test double that stands in for the background work often returns + // nothing at all. Filed as is, `undefined` would never leave the set and + // `settleBackgroundTasks` would spin on it forever at full CPU. A real + // promise passes through `Promise.resolve` as the same object. + const handle = Promise.resolve(settled); + inFlight.add(handle); + void handle.finally(() => { + inFlight.delete(handle); }); } diff --git a/src/lib/targets/build-response.ts b/src/lib/targets/build-response.ts index 1c28f8775..b236b894a 100644 --- a/src/lib/targets/build-response.ts +++ b/src/lib/targets/build-response.ts @@ -11,6 +11,8 @@ * are read in the user's own calendar. */ import { prisma } from "@/lib/db"; +import { runDetached } from "@/lib/logging/detached"; +import { trackBackgroundTask } from "@/lib/logging/background-tasks"; import { annotate } from "@/lib/logging/context"; import { COURSES_COMPLIANCE_SELECT, @@ -102,7 +104,7 @@ export async function buildTargetsResponse(user: AuthedUser) { queryNow.getTime() - 5 * 365 * 24 * 60 * 60 * 1000, ); - void ensureUserMoodRollupsFresh(userId); + trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId))); // All independent reads stay in one bounded fan-out. The medication event // read is the sole dependent query and remains chained to its medication diff --git a/tests/integration/query-budget-hot-routes.test.ts b/tests/integration/query-budget-hot-routes.test.ts index cb0390f26..0a50202d6 100644 --- a/tests/integration/query-budget-hot-routes.test.ts +++ b/tests/integration/query-budget-hot-routes.test.ts @@ -281,9 +281,15 @@ async function seed(): Promise { }), }); + // Both rollup tiers are minted here, the way a running account has them. + // Seeded rows without their rollups make the first read start a full + // recompute in the background, and that is a warm-up cost, not the read's. const { recomputeUserRollups } = await import("@/lib/rollups/measurement-rollups"); await recomputeUserRollups(USER_ID); + const { recomputeUserMoodRollups } = + await import("@/lib/rollups/mood-rollups"); + await recomputeUserMoodRollups(USER_ID); const session = await db.session.create({ data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) },