diff --git a/.planning/phase-W-SINCE-v1439-report.md b/.planning/phase-W-SINCE-v1439-report.md new file mode 100644 index 000000000..1d23048e2 --- /dev/null +++ b/.planning/phase-W-SINCE-v1439-report.md @@ -0,0 +1,132 @@ +# Phase W-SINCE — `/api/analytics` live-fallback `since` cap + +## Scope + +Defense-in-depth row cap on the live-fallback per-type read inside +`/api/analytics`. The v1.4.38.8 per-type fast-path gate (commit +`8a8150d2`) narrowed `isFullyCovered` from "all types covered" to +"these specific types covered" inside the three downstream fast-paths +(`bp_in_target` / `health_score` / `correlations`). That made the +`fetchMeasurementSeriesChunked` walk against `measurements` +unreachable in the common case — but only on those three branches. +The A2 per-type loop (the route's own `Promise.all` over every +`MeasurementType`) has no fast-path gate at all; it always runs live +SQL. A regression on the fast-path gate would re-trigger Marc's 74.6 s +cold full-slice mount. + +## Change + +Single source file touched: `src/app/api/analytics/route.ts`. + +1. Introduced `ANALYTICS_LIVE_WINDOW_DAYS = 90` + `liveSince = new + Date(Date.now() - 90 d)` at the top of `buildAnalyticsResponse`. +2. Threaded `since: liveSince` into every `fetchMeasurementSeriesChunked` + call inside the per-type loop. The helper already accepted an + optional `since?: Date`; the per-type loop just never passed one. +3. Annotated `meta.analytics.bp_aggregate.live_since` (ISO) alongside + the existing `row_count` slot so the next perf-verify can prove + how far back the live read went without redeploying instrumentation. + +## Untouched + +- The three downstream fast-paths (`computeBpInTargetFastPath`, + `computeUserHealthScoreFastPath`, + `computeCorrelationHypothesesFastPath`) keep their own + rollup-coverage-gated `readRollupBuckets` reads. The cap only + affects the route-local A2 loop. +- The slim `?slice=summaries` branch is independent of the per-type + loop entirely (it resolves through `computeSummariesSlice`). +- `summaries-slice.ts`, `correlations-fast-path.ts`, + `bp-in-target-fast-path.ts`, `health-score-fast-path.ts` — none + modified. Owned by other agents. +- `fetchMeasurementSeriesChunked` helper signature already supported + `since`; no helper-side rewrite needed. + +## Before / after row-count bound + +| Scenario | Before | After | +|---|---|---| +| Marc's tenant cold full-slice (live-fallback path) | ~347 114 rows across 15 chunked reads | ~5 000 rows (trailing 90 days, comprehensive-aggregator parity) | +| Empty / new account | 0 | 0 | +| Tenant with 2-year history but no rows in trailing 90 days | every historical row (paginated) | 0 | + +Estimated cold-path savings on the live-fallback: 20-40 s elided per +the audit (§1 A2 + §6 quick-win bundle). Wall-clock parity confirmed +unchanged for the fast-path (rollup-backed) common case — the cap only +fires inside the per-type loop, which the fast-paths don't reach. + +## Trade-off documented in the file + +`summarize().avg30LastYear` (points 365-395 days ago) returns `null` +on the fall-through path. The dashboard tile's year-over-year overlay +shows "no prior data" until the rollup tier warms. Acceptable because +the live-fallback is already a degraded path and the slim slice + +comprehensive aggregator already use the same 90-day window. + +## Test additions + +Created `src/__tests__/api/analytics/since-cap.test.ts` (3 tests, all +passing). Mocks every downstream dependency so the test exercises the +route's per-type loop in isolation: + +1. **`passes a trailing-90-day measuredAt.gte to every per-type + findMany`** — asserts the chunked helper carries `since` to every + call. Identifies per-type calls via the `orderBy: [..]` array + shape that distinguishes them from the narrow glucose / sleep-stage + reads. +2. **`annotates meta.analytics.bp_aggregate.live_since`** — pins the + ISO surface so the next perf-verify can see the cap is in effect + on a live request. +3. **`does not invoke the per-type loop on the slim ?slice=summaries + branch`** — regression test for the slim-slice path; the rollup + common case is unaffected by the cap. + +Tests pass under `pnpm test src/__tests__/api/analytics`. + +### Integration-test update (out of file set but unavoidable) + +The existing `tests/integration/analytics-bp-aggregate-paged.test.ts` +seeded 6 000 PULSE rows from `2025-01-01` — all > 90 days old by +today (2026-05-18), so the new cap would zero them out. Updated the +seed to backfill 6 000 rows × 15-minute intervals (62.5 days, +comfortably inside the 90-day floor) so the test still spans two +chunks and still pins the existing paging-correctness contract. The +test name + assertions are unchanged. + +## Quality gates + +- `pnpm typecheck` — clean on my files (`src/app/api/analytics/route.ts`, + `src/__tests__/api/analytics/since-cap.test.ts`, + `tests/integration/analytics-bp-aggregate-paged.test.ts`). Two + pre-existing TS errors in `src/app/api/telegram/webhook/route.ts` + belong to a parallel agent (W-MED/W-MOOD area). +- `pnpm lint src/app/api/analytics/route.ts src/__tests__/api/analytics + tests/integration/analytics-bp-aggregate-paged.test.ts` — clean. +- `pnpm test src/__tests__/api/analytics` — 3/3 pass. +- `pnpm test src/__tests__/api/analytics src/lib/analytics` — 241/241 + pass (regression-free across every analytics-related unit test). + +## Self-review against `8a8150d2` + +The v1.4.38.8 commit narrowed three fast-path gates from +`isFullyCovered(coverage) && coverage.get("X")` to per-type +predicates. My diff doesn't touch any of those three files. The +`since` cap lives entirely inside the A2 per-type loop, which: + +- Runs unconditionally on the default slice (no coverage gate). +- Feeds `summarize()` per type — `summarize()` is pure JS on the + array it receives, so a smaller array means a smaller working set + and an accurate slope7/30/90 (slope365 isn't a thing). +- Does not feed the three downstream fast-paths — they each issue + their own probes against `measurement_rollups` (rollup path) or + `measurement.findMany` (live path) inside their own helper. + +The fast-path-gate behaviour from `8a8150d2` is therefore untouched; +the cap is purely a row-count guard on the route-local per-type +read. + +## Commit + +Single commit, Marc-Voice English, no Co-Authored-By, no +`--no-verify`, no emoji. To be authored by the marathon driver per +the standard release flow. diff --git a/src/__tests__/api/analytics/since-cap.test.ts b/src/__tests__/api/analytics/since-cap.test.ts new file mode 100644 index 000000000..5ad0f7f9c --- /dev/null +++ b/src/__tests__/api/analytics/since-cap.test.ts @@ -0,0 +1,277 @@ +/** + * v1.4.39 W-SINCE — defense-in-depth row cap on the live-fallback + * per-type read. + * + * The v1.4.38.7 perf audit (`.planning/round-v1438-perf-analysis.md` + * §1 A2) flagged the per-type `fetchMeasurementSeriesChunked` loop + * inside `/api/analytics` as the dominant cost of Marc's 74.6 s cold + * full-slice mount. v1.4.38.8 (commit 8a8150d2) relaxed the + * `isFullyCovered` gate inside the three downstream fast-paths so the + * walk became unreachable in the common case — but the A2 loop itself + * has no fast-path gate. A regression that flips a fast-path back to + * the all-types-required form would re-trigger the unbounded read. + * + * This file pins: + * + * 1. **The 90-day floor reaches `prisma.measurement.findMany`.** + * Every per-type read inside the default slice carries + * `where.measuredAt.gte` ≈ now − 90 days. If a future refactor + * drops the `since` option from the chunked helper this test + * breaks loudly. + * + * 2. **The `meta.analytics.bp_aggregate.live_since` annotate fires.** + * Lets the next perf-verify see how far back the live-fallback + * read actually went. A widened window in a regression would + * surface in this annotation. + * + * 3. **The slim slice (`?slice=summaries`) is unaffected.** That + * branch never hit the per-type loop in the first place and + * keeps its own 90-day SQL aggregator. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + measurement: { findMany: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +// Spy on the real `annotate` so we can assert the +// `meta.analytics.bp_aggregate.live_since` field while keeping the +// AsyncLocalStorage `eventStorage.run` wiring intact (the +// `apiHandler` opens an event scope inside `run()`). +vi.mock("@/lib/logging/context", async () => { + const actual = await vi.importActual< + typeof import("@/lib/logging/context") + >("@/lib/logging/context"); + return { + ...actual, + annotate: vi.fn(actual.annotate), + }; +}); + +// The three fast-paths and the rollup helpers are exercised by their +// own tests; here we stub them so the route's per-type read is the +// only Prisma traffic the test observes. +vi.mock("@/lib/measurements/rollups", () => ({ + ensureUserRollupsFresh: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/measurements/rollup-coverage", () => ({ + probeRollupCoverage: vi.fn().mockResolvedValue(new Map()), +})); + +vi.mock("@/lib/analytics/bp-in-target-fast-path", () => ({ + computeBpInTargetFastPath: vi.fn().mockResolvedValue({ + last7Days: null, + last30Days: null, + allTime: null, + priorMonth: null, + priorYear: null, + }), +})); + +vi.mock("@/lib/analytics/health-score-fast-path", () => ({ + computeUserHealthScoreFastPath: vi.fn().mockResolvedValue(null), +})); + +vi.mock("@/lib/analytics/correlations-fast-path", () => ({ + computeCorrelationHypothesesFastPath: vi + .fn() + .mockResolvedValue({ hypotheses: [] }), +})); + +vi.mock("@/lib/analytics/summaries-slice", () => ({ + computeSummariesSlice: vi.fn().mockResolvedValue({ + summaries: {}, + bmi: null, + bpInTargetPct: null, + lastSeenByType: {}, + }), +})); + +import { GET } from "@/app/api/analytics/route"; +import { getSession } from "@/lib/auth/session"; +import { prisma } from "@/lib/db"; +import { annotate } from "@/lib/logging/context"; +import { __resetAllCachesForTests } from "@/lib/cache/server-cache"; + +const callGet = GET as unknown as (req: Request) => Promise; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { + id: "user-since-1", + username: "marc", + role: "USER" as const, + locale: "en", + timezone: "Europe/Berlin", + heightCm: null, + dateOfBirth: null, + sourcePriorityJson: null, + }, +}; + +beforeEach(async () => { + vi.clearAllMocks(); + __resetAllCachesForTests(); + (getSession as ReturnType).mockResolvedValue(SESSION_OK); + // Default: every per-type findMany returns an empty page so the + // chunked loop terminates after the first call. + (prisma.measurement.findMany as ReturnType).mockResolvedValue( + [], + ); + + // Re-prime the fast-path / probe / slim-slice stubs because we use + // `clearAllMocks()` to wipe call history between tests; `vi.fn()` at + // module scope only sets the initial implementation, and the slim + // slice test must observe a resolved value not `undefined`. + const { ensureUserRollupsFresh } = await import( + "@/lib/measurements/rollups" + ); + const { probeRollupCoverage } = await import( + "@/lib/measurements/rollup-coverage" + ); + const { computeBpInTargetFastPath } = await import( + "@/lib/analytics/bp-in-target-fast-path" + ); + const { computeUserHealthScoreFastPath } = await import( + "@/lib/analytics/health-score-fast-path" + ); + const { computeCorrelationHypothesesFastPath } = await import( + "@/lib/analytics/correlations-fast-path" + ); + const { computeSummariesSlice } = await import( + "@/lib/analytics/summaries-slice" + ); + + (ensureUserRollupsFresh as ReturnType).mockResolvedValue( + undefined, + ); + (probeRollupCoverage as ReturnType).mockResolvedValue( + new Map(), + ); + (computeBpInTargetFastPath as ReturnType).mockResolvedValue({ + last7Days: null, + last30Days: null, + allTime: null, + priorMonth: null, + priorYear: null, + }); + (computeUserHealthScoreFastPath as ReturnType).mockResolvedValue( + null, + ); + ( + computeCorrelationHypothesesFastPath as ReturnType + ).mockResolvedValue({ hypotheses: [] }); + (computeSummariesSlice as ReturnType).mockResolvedValue({ + summaries: {}, + bmi: null, + lastSeenByType: {}, + }); +}); + +describe("GET /api/analytics — live-fallback `since` cap (W-SINCE)", () => { + it("passes a trailing-90-day `where.measuredAt.gte` to every per-type findMany", async () => { + const before = Date.now(); + const response = await callGet( + new Request("http://localhost/api/analytics"), + ); + expect(response.status).toBe(200); + const after = Date.now(); + + const findMany = prisma.measurement.findMany as ReturnType; + expect(findMany.mock.calls.length).toBeGreaterThan(0); + + const NINETY_DAYS_MS = 90 * 24 * 60 * 60 * 1000; + const expectedMinSince = before - NINETY_DAYS_MS; + const expectedMaxSince = after - NINETY_DAYS_MS; + + // The per-type loop (A2) issues chunked reads with an `orderBy` + // *array* (`(measuredAt asc, id asc)`); the 30-day glucose and + // 30-day sleep-stage reads use either a single-object orderBy or + // none at all. Filtering on the array shape isolates the chunked + // helper's traffic from the unrelated narrow reads. + const perTypeCalls = findMany.mock.calls.filter((call) => { + const arg = call[0] as { orderBy?: unknown }; + return Array.isArray(arg?.orderBy); + }); + + expect(perTypeCalls.length).toBeGreaterThan(0); + + for (const call of perTypeCalls) { + const arg = call[0] as { + where?: { measuredAt?: { gte?: Date } }; + }; + const gte = arg.where?.measuredAt?.gte; + expect(gte).toBeInstanceOf(Date); + const gteMs = (gte as Date).getTime(); + expect(gteMs).toBeGreaterThanOrEqual(expectedMinSince); + expect(gteMs).toBeLessThanOrEqual(expectedMaxSince); + } + }); + + it("annotates `meta.analytics.bp_aggregate.live_since` with the 90-day cutoff ISO", async () => { + const before = Date.now(); + await callGet(new Request("http://localhost/api/analytics")); + const after = Date.now(); + + const annotateFn = annotate as ReturnType; + const bpAggregateCall = annotateFn.mock.calls.find((call) => { + const fields = call[0] as + | { meta?: { analytics?: { bp_aggregate?: unknown } } } + | undefined; + return fields?.meta?.analytics?.bp_aggregate !== undefined; + }); + expect(bpAggregateCall).toBeDefined(); + + const fields = bpAggregateCall![0] as { + meta: { + analytics: { + bp_aggregate: { row_count: number; live_since: string }; + }; + }; + }; + const bpAggregate = fields.meta.analytics.bp_aggregate; + expect(typeof bpAggregate.live_since).toBe("string"); + + const NINETY_DAYS_MS = 90 * 24 * 60 * 60 * 1000; + const liveSinceMs = new Date(bpAggregate.live_since).getTime(); + expect(liveSinceMs).toBeGreaterThanOrEqual(before - NINETY_DAYS_MS); + expect(liveSinceMs).toBeLessThanOrEqual(after - NINETY_DAYS_MS); + }); + + it("does not invoke the per-type loop on the slim `?slice=summaries` branch", async () => { + const response = await callGet( + new Request("http://localhost/api/analytics?slice=summaries"), + ); + expect(response.status).toBe(200); + + // The slim slice resolves through `computeSummariesSlice` only; + // the per-type findMany loop is skipped entirely so the rollup- + // path semantics are unaffected by the `since` cap. + const findMany = prisma.measurement.findMany as ReturnType; + expect(findMany).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/analytics/route.ts b/src/app/api/analytics/route.ts index c60e1db7f..827e18675 100644 --- a/src/app/api/analytics/route.ts +++ b/src/app/api/analytics/route.ts @@ -205,6 +205,43 @@ async function buildAnalyticsResponse(user: AuthedUser) { // when the column is null or malformed. const sourcePriorityJson = user.sourcePriorityJson; + // v1.4.39 W-SINCE — defense-in-depth row cap on the per-type live + // read. The v1.4.38.8 per-type fast-path gate (commit 8a8150d2) + // narrowed `isFullyCovered` from "all types covered" to "these + // specific types covered" inside the three downstream fast-paths + // (bp_in_target / health_score / correlations). That made the + // unbounded `fetchMeasurementSeriesChunked` walk against + // `measurements` unreachable in the common case — but only on those + // three branches. This per-type loop (A2 in the v1.4.38 perf audit) + // has no fast-path gate at all; it always runs live SQL. + // + // Before the cap: a power-user account with 347 k rows split across + // 15 measurement types fanned out 70+ chunked round-trips of 5 000 + // rows each, dominating the cold full-slice critical path (Marc's + // 74.6 s cold mount). + // + // After the cap: the trailing 90 days mirror the comprehensive + // aggregator + summaries-slice windows; the worst case drops to + // roughly the same row count those paths already absorb (~5 k + // estimated on Marc's account). Cumulative HealthKit types + // (ACTIVITY_STEPS, ACTIVE_ENERGY, …) collapse to one-row-per-day + // after the per-day-sum pass downstream so the 90-day window is + // ample for every consumer summarise() feeds. + // + // Trade-off: `summarize().avg30LastYear` (points 365-395 days ago) + // returns null on this fall-through path; the dashboard tile's + // year-over-year overlay shows "no prior data" until the rollup + // tier warms. This is acceptable because the live-fallback is + // already a degraded path — users hit it only when the per-type + // coverage gate flips false, which v1.4.38.8 made rare. The slim + // slice (`?slice=summaries`) and the rollup-backed comprehensive + // aggregator already operate on the 90-day window, so the cap + // brings the full slice into line with the slim slice's contract. + const ANALYTICS_LIVE_WINDOW_DAYS = 90; + const liveSince = new Date( + Date.now() - ANALYTICS_LIVE_WINDOW_DAYS * 24 * 60 * 60 * 1000, + ); + // v1.4.34 IW-B — capture the most-recent `measuredAt` per type so the // dashboard tile strip can render an "Letzter Wert vor Xd" caption on // tiles whose latest reading is older than a week. The series read @@ -225,6 +262,11 @@ async function buildAnalyticsResponse(user: AuthedUser) { types.map((type) => fetchMeasurementSeriesChunked(user.id, type, { includeSleepStage: true, + // v1.4.39 W-SINCE — trailing 90-day floor for the live read. + // Mirrors the comprehensive-aggregator + summaries-slice + // windows so the full slice stops over-reading on the + // fall-through path. See comment block above. + since: liveSince, }).then((measurements) => { totalRowsReadForAggregate += measurements.length; // v1.4.34 IW-B — record the freshest `measuredAt` for this type @@ -317,10 +359,19 @@ async function buildAnalyticsResponse(user: AuthedUser) { // v1.4.23 Sr-H1 — slow-query attribution. Total rows pulled across // every per-type chunked read so ops can spot outlier users whose // analytics requests dominate the route's tail latency. + // + // v1.4.39 W-SINCE — also surface `live_since` (ISO) so the next + // perf-verify can see how far back the live-fallback read actually + // went. With the 90-day cap in place this should be ~now-90d on + // every full-slice request; an older value would signal that a + // future refactor accidentally widened the window again. annotate({ meta: { analytics: { - bp_aggregate: { row_count: totalRowsReadForAggregate }, + bp_aggregate: { + row_count: totalRowsReadForAggregate, + live_since: liveSince.toISOString(), + }, }, }, }); diff --git a/tests/integration/analytics-bp-aggregate-paged.test.ts b/tests/integration/analytics-bp-aggregate-paged.test.ts index 64b03259b..a31f8df4f 100644 --- a/tests/integration/analytics-bp-aggregate-paged.test.ts +++ b/tests/integration/analytics-bp-aggregate-paged.test.ts @@ -109,16 +109,26 @@ describe("GET /api/analytics — chunked per-type reads (Sr-H1)", () => { // `(measuredAt, id)` order produces a deterministic chronological // sequence that `summarize()` consumes the same way it would have // consumed a single-shot read. + // + // v1.4.39 W-SINCE — the route now caps the live-fallback per-type + // read at the trailing 90 days. To keep this paging contract test + // exercising both chunks the rows are seeded backwards from `now` + // at 15-minute intervals (6 000 × 15 min = 62.5 days, comfortably + // inside the 90-day floor). const ROW_COUNT = 6000; - const baseMs = new Date("2025-01-01T00:00:00.000Z").getTime(); - const HOUR_MS = 60 * 60 * 1000; + const STEP_MS = 15 * 60 * 1000; + const nowMs = Date.now(); const rows = Array.from({ length: ROW_COUNT }, (_, i) => ({ userId: user.id, type: "PULSE" as const, value: 60 + (i % 40), // deterministic 60..99 unit: "bpm", source: "MANUAL" as const, - measuredAt: new Date(baseMs + i * HOUR_MS), + // i = ROW_COUNT - 1 is the freshest row (just before `now`); + // i = 0 is the oldest. The chunked helper reads + // `(measuredAt asc, id asc)` so the latest value is the last + // entry in the sorted stream — which the test asserts below. + measuredAt: new Date(nowMs - (ROW_COUNT - i) * STEP_MS), })); // createMany batches efficiently; default Postgres parameter limit // accommodates this volume in a single statement.