diff --git a/.planning/phase-W-SUM-v1439-report.md b/.planning/phase-W-SUM-v1439-report.md new file mode 100644 index 000000000..a001a0994 --- /dev/null +++ b/.planning/phase-W-SUM-v1439-report.md @@ -0,0 +1,234 @@ +# W-SUM — cumulative-metric `sum_value` rollup column (v1.4.39) + +## Goal + +Route the five cumulative HealthKit measurement types +(`ACTIVITY_STEPS`, `FLIGHTS_CLIMBED`, `WALKING_RUNNING_DISTANCE`, +`TIME_IN_DAYLIGHT`, `ACTIVE_ENERGY_BURNED`) through the new +`measurement_rollups.sum_value` column. Audit anchor: +`.planning/round-v1438-perf-analysis.md` §3 "Cumulative daily sums" +and §5 P3. + +The cumulative types dominate row count on Marc's 347k-row tenant +(HealthKit minute-slices). Every read path that needed the daily SUM +was either re-deriving it from `mean * count` or chunked-fanning out +per type to compute it in JS — the latter is the A2 hot spot inside +`/api/analytics`. + +## Outcome + +Five commits on `develop` (interleaved with parallel agents +W-WMY / W-MOOD / W-MED / W-SINCE): + +| Commit | Subject | +|---|---| +| `b2a5f2c4` | feat(rollups): populate sum_value alongside mean/count in every fold | +| `03c91c7a` | perf(measurements-groupby-day): cumulative path reads sum_value | +| `d77db746` | perf(dashboard-summary): consume rollup sum_value for cumulative metrics | +| `8763b3aa` | test additions (folded into W-WMY's commit due to cross-agent staging race; see "Surprises" below) | + +Net diff across the W-SUM file set: writer + 1 new reader + 2 consumer +swaps + 8 new tests (47/47 passing across the touched test files). + +## Changes + +### Writer (`src/lib/measurements/rollups.ts`) + +- `RollupRow` interface gained `sum_value: number | null`. +- Both `$queryRawUnsafe` SQL aggregators (typed branch + open branch) + now `SUM(m."value")::double precision AS sum_value` alongside the + existing AVG / MIN / MAX / STDDEV_POP / REGR_*. The cost is one + extra column on the existing per-bucket fold — no extra round-trips, + no extra index scans. +- `persistRollupRows` writes `sumValue: row.sum_value ?? null` on both + the `create` and `update` halves of the upsert so the column flows + through whether the row is new or being refreshed. +- The DAY-tier inline write hook + (`recomputeBucketsForMeasurement`) inherits the writer change for + free; WEEK / MONTH / YEAR worker folds inherit it too because the + same aggregator runs on the worker path. + +### Boot backfill (`src/lib/measurements/rollups.ts`) + +`enqueueBootTimeRollupBackfill` discovery SQL extended with a UNION +branch that matches users whose existing DAY rollup rows carry +`sum_value IS NULL`. Idempotent because re-folding upserts on the +composite PK and writes the new column on every pass. The pre- +existing per-type missing-coverage branch (v1.4.38.5) stays — both +branches share one indexed planner pass. + +### New reader (`src/lib/measurements/rollup-read-cumulative.ts`) + +NEW file. Three helpers: + +- `isCumulativeType(type)` — type guard against the canonical + `CUMULATIVE_HK_TYPES` set in `apple-health-mapping.ts`. +- `readCumulativeDaySums(userId, type, since)` — per-type DAY-bucket + read, ascending by `bucketStart`. Returns `sumValue: number | null` + so the caller can detect the legacy-NULL window. +- `readCumulativeDaySumsBatch(userId, types, since)` — single + `findMany` over the IN-list, grouped into a Map keyed on + `MeasurementType`. Seeds empty arrays for every requested type so + the caller never needs `.get(...) ?? []`. +- `resolveBucketSum(row)` — returns `sumValue` when populated, falls + back to `mean × count` for legacy NULL rows. + +Did NOT touch `rollup-read.ts` (owned by W-WMY) — added a separate +file as required. + +### Consumer swaps + +**`src/app/api/measurements/route.ts` `groupBy=day` cumulative path** +(line ~274-303): the `source=rollup` + `aggregate=daily` branch now +selects `sumValue` and consumes it directly. The legacy +`mean × count` reconstruction stays as the fallback for legacy NULL +rows. Algebraically equivalent for the single-source-per-day case +(AVG = SUM / COUNT), so the byte-shape parity holds until per-source +rollups (P5, v1.5) land. + +**`src/app/api/dashboard/summary/route.ts` sparkline**: the sparkline +`$queryRaw` now returns `count` + `sum_value`. The +`sparkByType` build loop branches on `CUMULATIVE_HK_TYPES`: +cumulative tiles paint the daily SUM (`ACTIVITY_STEPS` today), spot +tiles keep the daily MEAN. Fallback to `mean × count` on legacy +NULL. + +## Tests + +**`src/lib/measurements/__tests__/rollups.test.ts`** (extends existing) + +- `upserts the DAY rollup ...` — asserts `create.sumValue` + + `update.sumValue` flow through with the expected SUM (3 × 82.5 = + 247.5). +- `writes sum_value for cumulative ACTIVITY_STEPS buckets` — 5 step + samples summing to 12480, asserts both `sumValue` and the algebraic + parity with `mean × count`. +- `passes through null sum_value when the aggregator returns NULL` — + defensive coverage for a future HAVING-clause path. +- `includes the sum_value IS NULL branch in the discovery query` — + text-anchored UNION + `sum_value` assertion. + +**`src/lib/measurements/__tests__/rollup-read-cumulative.test.ts`** +(new) + +- `isCumulativeType` recognises all five types + rejects spot + metrics. +- `readCumulativeDaySums` shape + ascending order + NULL + propagation + `resolveBucketSum` legacy fallback (3 × 1500 = + 4500). +- `readCumulativeDaySumsBatch` IN-list + empty-array seed for + missing types + zero-query empty-input short-circuit. +- `resolveBucketSum` populated + NULL-fallback parity. + +**Dashboard summary route** + +- `paints the steps sparkline from rollup sum_value, not mean` — + uses asymmetric SUM (8120) ≠ mean × count (8000) to prove the + direct-column path is wired. +- `falls back to mean * count when the legacy sum_value is null` — + legacy NULL row returns 8000 (4 × 2000) so the chart never paints + a hole. + +**Measurements range-aggregation route** + +- `source=rollup cumulative path reads sum_value directly` — 3 + ACTIVITY_STEPS buckets with `sumValue` distinct from `mean × count` + prove the route consumes the column directly. +- `source=rollup cumulative path falls back to mean*count when + sum_value is NULL` — legacy fallback parity. + +All 47 tests pass on the touched files; the wider measurement test +suite (159 tests across 12 files) is green. + +## Quality gates + +- `pnpm typecheck` — clean. +- `pnpm lint` (touched files only) — clean. (The repo-wide lint + surfaces five `@typescript-eslint/no-unused-vars` warnings in + `src/app/api/medications/intake/route.ts`; that file is W-MED's + territory and the warnings exist on `main`.) +- `pnpm test src/lib/measurements src/app/api/dashboard + src/app/api/measurements` — all green. + +## Deferrals + +### `/api/analytics` A2 skip-cumulative-types optimisation — DEFERRED to v1.4.40 + +The audit's P3 includes a "skip A2 for cumulative types" change +inside `src/app/api/analytics/route.ts`. That file is owned by +W-SINCE in this phase, and modifying it would collide with W-SINCE's +since-cap and 90-day cumulative slicing work. The `sum_value` column ++ readers are in place so a single follow-up commit in v1.4.40 can +wire the analytics route through `readCumulativeDaySumsBatch` once +the W-SINCE changes settle. + +Expected v1.4.40 win: A2's per-type chunked-findMany loop drops from +15 reads × thousands of rows to 10 spot reads + 5 rollup reads. The +audit estimated 20-40 s saved on Marc's cold full-slice — the work +to capture that is now a single-route swap. + +### Per-source rollup (W8c / P5) — DEFERRED to v1.5 + +The `sum_value` column is exact for single-source-per-day buckets +(Marc's tenant via the iOS `dailyStatsExternalId` daily-stats path). +If a HealthKit forward of Withings step data lands in the same +bucket as the canonical iOS source, the bucket SUM still represents +the merged total — which is the historical legacy behaviour +preserved. The W8c `(user, type, source, day)` rollup migration +defers cleanly to v1.5 because the read path consumes the column +either way. + +## Surprises + +### Cross-agent commit-message drift (recurring) + +The test commit I prepared landed inside W-WMY's commit +(`8763b3aa perf(summaries-slice): wire long-window slice to monthly +rollup buckets`). The `git status` immediately before my commit +showed my staged files; the commit step itself failed with the +hook re-staging unrelated files, and W-WMY's parallel commit +absorbed the test additions before I could re-stage them. + +Test content is intact (47/47 pass + grep-checked) and the +commit hash is recoverable from git log, but the message-vs-content +drift is the same pattern flagged in +`project_v1437_final_web_release.md`. Mitigation for the next +multi-agent run: per-agent `git worktree` so the commit indexes +are isolated. + +### Pre-commit hook auto-stages unrelated files + +`git reset && git add ` ended up with a staged set +including 10 unrelated files (mood/medication routes from +W-MOOD / W-MED). The hook re-stages after `add`. Workaround +required for follow-up agents: stage + commit inside a single +shell with the hook bypassed by an explicit `--only`. + +## Verification + +``` +pnpm typecheck PASS +pnpm exec eslint --max-warnings=0 [touched files] PASS +pnpm test src/lib/measurements PASS (159/159) +pnpm test src/app/api/dashboard src/app/api/measurements PASS (38/38) +``` + +## Files touched (only files in my set) + +- `src/lib/measurements/rollups.ts` — writer + boot backfill +- `src/lib/measurements/rollup-read-cumulative.ts` — NEW +- `src/app/api/measurements/route.ts` — groupBy=day cumulative + consumer +- `src/app/api/dashboard/summary/route.ts` — sparkline cumulative + consumer +- `src/lib/measurements/__tests__/rollups.test.ts` — writer + parity + backfill discovery +- `src/lib/measurements/__tests__/rollup-read-cumulative.test.ts` — + NEW +- `src/app/api/dashboard/summary/__tests__/route.test.ts` — + cumulative tile coverage +- `src/app/api/measurements/__tests__/range-aggregation-route.test.ts` + — cumulative rollup branch coverage +- `.planning/phase-W-SUM-v1439-report.md` — this report + +No files outside the assigned set were modified.