From f9996552edd743830654dd62249d79adb395413c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 21 May 2026 11:20:24 +0200 Subject: [PATCH] =?UTF-8?q?chore(release):=20v1.4.39.4=20=E2=80=94=20dashb?= =?UTF-8?q?oard=20symmetry=20for=20daily-schedule=20intake=20projection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .planning/round-v1439-arch-qa-frontend.md | 140 ++++++ .planning/round-v1439-arch-qa-ghosts.md | 188 ++++++++ .planning/round-v1439-arch-qa-infra-db.md | 317 +++++++++++++ .planning/round-v1439-arch-qa-organization.md | 107 +++++ .planning/round-v1439-empirical-trace.md | 435 ++++++++++++++++++ CHANGELOG.md | 13 + package.json | 2 +- .../migration.sql | 57 +++ prisma/schema.prisma | 9 + .../dashboard/summary/__tests__/route.test.ts | 156 ++++++- src/app/api/dashboard/summary/route.ts | 111 ++++- .../intake/__tests__/route.test.ts | 124 ++++- src/app/api/medications/intake/route.ts | 79 +++- src/lib/__tests__/medication-schedule.test.ts | 110 +++++ src/lib/medication-schedule.ts | 179 +++++++ 15 files changed, 2016 insertions(+), 11 deletions(-) create mode 100644 .planning/round-v1439-arch-qa-frontend.md create mode 100644 .planning/round-v1439-arch-qa-ghosts.md create mode 100644 .planning/round-v1439-arch-qa-infra-db.md create mode 100644 .planning/round-v1439-arch-qa-organization.md create mode 100644 .planning/round-v1439-empirical-trace.md create mode 100644 prisma/migrations/0073_v1439_medication_intake_unique_per_source/migration.sql diff --git a/.planning/round-v1439-arch-qa-frontend.md b/.planning/round-v1439-arch-qa-frontend.md new file mode 100644 index 000000000..14232d102 --- /dev/null +++ b/.planning/round-v1439-arch-qa-frontend.md @@ -0,0 +1,140 @@ +# Frontend + Data-Flow Audit — HealthLog v1.4.39.3 + +**Reviewer scope.** Senior frontend + data-flow review of the dashboard composition tree, TanStack Query topology, hydration model, and mobile-specific paint behaviour. Marc reports a "two-wave paint" (mood + medications first, then everything else simultaneously) and suspects a structural problem, not a code bug. **He's right.** + +## Top 3 findings + +1. **Critical — Duplicate fetch fan-out: every visible tile + chart re-fetches the same data through three independent cache keys.** The dashboard mother queries `/api/analytics` twice (slim + thick), reads mood via `/api/mood/analytics` once, and then *every visible chart card* fires another query against either `/api/measurements?type=…&aggregate=daily` (`["chart-data", …]`) or `/api/mood/analytics` again (`["mood-chart-data"]`). For an account with weight + BP + pulse + body-fat + sleep + steps charts visible, the dashboard fires **~10–12 parallel client requests** on cold mount, not the 4 the brief assumes. The "two waves" Marc describes are: (a) light slices (slim analytics, mood analytics, dashboard widgets, medications/intake) resolve in <500 ms together, then (b) the thick analytics + the six `["chart-data", …]` per-chart calls land later as one HTTP-coalesced burst. This is the visible symptom of the data-flow problem. + +2. **High — `queryKeys` factory is bypassed 87 % of the time (154 bare vs 23 factory).** Despite a centralised `src/lib/query-keys.ts` with `measurementDependentKeys` / `moodDependentKeys` invalidation bundles, only 23 of 177 `queryKey:` declarations route through the factory. The chart-data and mood-chart-data keys (`["chart-data", …]`, `["mood-chart-data"]`) are bare and **not in `measurementDependentKeys`** — so after creating a measurement, the chart caches stay stale until `staleTime` expires (60 s) or the page is hard-reloaded. The dashboard's tile-strip refreshes via the analytics invalidation, but the chart row below does not. This is a latent cache-coherence bug that also drives unnecessary refetches when the factory bundle is invalidated and the chart still re-fetches on next focus. + +3. **High — No `` boundaries anywhere in the app; zero `useSuspenseQuery` usage; 32 `"use client"` boundaries under `app/`.** The dashboard is entirely client-rendered: `RootLayout` is async server-side, but `page.tsx` is `"use client"` at line 1 and every child is too. The initial HTML payload carries **no above-the-fold data** — every tile waits for client hydration + a fetch round trip. Per-tile suspense boundaries would let mood + medication tiles paint independently from the thick analytics tile. The brief's diagnosis ("fundamental composition issue") is exactly this: a flat client tree with shared loading semantics, not a layered server-streamed shell. + +## Verdict + +**Marc's intuition is correct.** Post-v1.4.39.3 the dashboard *does* split into two waves, but not for the reason the v1.4.39.2 split was meant to fix. The slim/thick split helped the tile strip but the **six per-chart `["chart-data", …]` queries below it are still firing in lockstep with the thick query** and dominate the second wave. The architecture is sound at the seam (slim + thick + merge helper) but the *whole row beneath* (charts) is invisible to that contract and keeps the "blocked-then-burst" pattern alive. v1.4.39.4 should either (a) push the per-chart fetches into a shared analytics-derived store keyed on the slim+thick payload, or (b) introduce per-tile `` boundaries so each chart streams in independently rather than the whole row blocking on the last fetch. + +--- + +## Critical + +### C1 — Per-chart `useQuery` fan-out duplicates analytics data +- **File:** `src/components/charts/health-chart.tsx:543`, `src/components/charts/mood-chart.tsx:315` +- **Pattern:** every `` (one per metric: weight, BP, pulse, body-fat, sleep, steps) declares its own `useQuery({ queryKey: ["chart-data", …] })` against `/api/measurements?type=X&aggregate=daily&source=rollup`. `` declares another `useQuery({ queryKey: ["mood-chart-data"] })` against `/api/mood/analytics` — a route the dashboard `page.tsx` already fetched at line 290. +- **Impact:** the dashboard fires ~10–12 cold requests instead of 4. Even with rollup-source coverage every per-chart call is ~80–200 ms server-side; six in parallel land as one HTTP/2 multiplex burst that *looks* simultaneous but blocks the row's paint until the slowest one finishes. +- **Why it's "two waves":** mood + medications + slim analytics + widgets are light + cached + share the same `staleTime: 60_000`; thick + chart-data + medication-compliance all hit heavier server work. Network-tab waterfall: ~200 ms cluster, then a ~800 ms gap, then the rest land together. This is what Marc sees. +- **Fix sketch:** the slim analytics route already returns per-type DataSummary headlines, lastSeenByType, and could carry the daily sparkline series the charts re-derive. Move the chart's daily-aggregate fetch into a single analytics-shaped consolidated endpoint (or piggy-back on the rollup tier the route already reads) and have charts consume the same `useAnalyticsQuery`-driven store rather than each fetching its own slice. + +### C2 — `chart-data` and `mood-chart-data` queryKeys aren't in `measurementDependentKeys` +- **File:** `src/lib/query-keys.ts:116-122` +- **Issue:** `measurementDependentKeys` lists `measurements()`, `analytics()`, `insightsRoot()`, `insightsTargets()`, `gamificationAchievements()`. It does **not** include `["chart-data", …]` or `["mood-chart-data"]`. After a measurement form save, `invalidateKeys(queryClient, measurementDependentKeys)` runs but the chart caches stay live until `staleTime` (60 s) expires. +- **Impact:** add a measurement → dashboard tile strip updates immediately (analytics invalidated) but the chart line below shows yesterday's data for up to a minute. Marc has not flagged this explicitly, but it likely reads as "the chart didn't update" in subjective UX. +- **Fix:** either move chart fetches onto the analytics-derived path (C1's fix supersedes this), or add `["chart-data"]` + `["mood-chart-data"]` prefix entries to the dependent-keys bundle. + +--- + +## High + +### H1 — 154/177 `queryKey:` declarations bypass `queryKeys` factory +- **Search:** `grep -rn "queryKey:" src --include="*.tsx" --include="*.ts" | grep -v "queryKeys\."` → 154 hits. +- **Factory usage:** `grep "queryKey: queryKeys\."` → 23 hits. +- **Examples of drift:** + - `useAuth` uses `["auth", "me"]` but `queryKeys.auth()` returns `["auth"]`. The factory is not the source of truth. + - `src/components/insights/coach-panel/use-coach.ts` declares its own local `QUERY_KEYS` constant — parallel factory. + - `src/app/insights/medikamente/page.tsx` writes `["insights", "comprehensive"]` inline; the factory has `insightsComprehensive()` for that exact shape. + - `src/components/settings/thresholds-editor-section.tsx` invalidates `["user", "thresholds"]` + `["insights"]` directly — neither in the factory. +- **Impact:** TanStack treats `["auth"]` and `["auth", "me"]` as different cache slots for *reads*, but a prefix invalidation matches both. This is the v1.4.32 TanStack queryKey-collision class Marc already has a memory-note about. Right now the codebase is one rename or typo away from the silent cache-poisoning failure mode. +- **Fix:** lint rule (`no-restricted-syntax`) blocking literal-array queryKeys outside `src/lib/query-keys.ts` + a one-shot migration adding the missing factory entries (notifications-prefs, ntfy-settings, global-services, passkeys, user-thresholds, coachConversation, glp1-timeline, personal-records, api-version, insights-provider-chain, measurement-drilldown, chart-data, mood-chart-data, medication-compliance-chart). + +### H2 — No `` boundaries, no `useSuspenseQuery`, app is 100 % client-fetched +- **Search:** `grep -rn "useSuspenseQuery" src` → 0 hits. `grep "` boundaries to the existing client tile-strip component. Keep the thick analytics + per-chart queries client-side. This is a real refactor (~2 days) but it's the structural answer to the "two waves" question. + +### H3 — Loading gates are implicit and shared via memoised `data` +- **File:** `src/app/page.tsx:248-276` +- **Pattern:** `mergeSlimAndThickAnalytics(slim, thick)` returns `undefined` until at least one resolves. Then `const data = useMemo(...)` becomes defined. *Every downstream gate* (`hasWeight`, `hasBp`, `showWeightTile`, etc.) reads `data?.summaries?.X`. There is no explicit "slim-loaded" vs "thick-loaded" distinction at the gate level. +- **Subtle consequence:** when slim resolves with non-empty summaries (e.g. `{ WEIGHT: {…}, PULSE: {…} }`) but thick is still pending, every tile *that depends only on summaries* will paint correctly. But the BD-Zielbereich tile gate (`hasBpInTarget`) reads `data?.bpInTargetPct` which only thick populates → that single tile is correctly held back. Good. +- **Concern:** the gate is *correct* for the intended split, but it's invisible — a future contributor adding a thick-only field (say `correlations`) to a tile that reads only summaries today won't realise they've just made the tile wait for the thick slice. A typed boundary (`SlimAnalytics` vs `ThickAnalytics` instead of the union) at the page level would make this explicit. + +### H4 — `getHourForTimeZone` runs on every render (no memo) +- **File:** `src/app/page.tsx:562` +- **Issue:** `const hour = user?.timezone ? getHourForTimeZone(user.timezone) : null;` instantiates a fresh `Intl.DateTimeFormat` and parses parts on every render. The greeting text is derived from it and *included in tile keys nowhere*, so the cost is small per render — but combined with H6 below (every range/band recomputed on every render) the per-mount work is non-trivial. +- **Fix:** `useMemo(() => getHourForTimeZone(user?.timezone), [user?.timezone])`. + +--- + +## Medium + +### M1 — Dashboard prop-drilling for `compareBaseline` + `userTimezone` +- **File:** `src/app/page.tsx:1129-1300` — every `` call forwards `compareBaseline` and `userTimezone`. Same for `` and ``. +- **Impact:** a layout-toggle change re-renders all six chart components even though the values are conceptually app-wide. Context (`DashboardLayoutProvider`) carrying `compareBaseline` + `userTimezone` would let charts subscribe directly. +- **Severity:** medium because charts already wrap in `next/dynamic` so the re-render cost is bounded. + +### M2 — `DASHBOARD_QUERY_OPTS` is a fresh object every render +- **File:** `src/app/page.tsx:221-224` +- **Issue:** `const DASHBOARD_QUERY_OPTS = { staleTime: 60_000, refetchOnWindowFocus: false } as const;` — declared inside the component body, so it's a fresh object each render. TanStack's `useQuery` does shallow-compare options; same values, same behaviour, but it's slot poisoning waiting to happen if a future field uses a callback. +- **Fix:** hoist to module scope. + +### M3 — `useAnalyticsQuery` declares per-component defaults but is also called inline by sub-pages without the slim slice +- The hook is shared, but the slim/thick split is dashboard-only. The Insights page and sub-pages (`grep "useAnalyticsQuery" src/app/insights/` — 6 sites) all hit the thick slice. If the user navigates dashboard → insights → dashboard, the dashboard remounts and re-issues *both* slim + thick. The thick result from `/insights` is in cache → instant. The slim result has never been fetched anywhere else → still ~300 ms. +- **Fix:** server-prewarm slim alongside thick from the insights routes, or pre-warm via `queryClient.prefetchQuery` in a layout effect when the user lands authenticated. + +### M4 — `dynamic(() => import(...), { ssr: false, loading: })` for MoodChart + MedicationComplianceChart but not the others +- **File:** `src/app/page.tsx:51-64` +- **Inconsistency:** `MoodChart` + `MedicationComplianceChart` are dynamically imported inline in `page.tsx`. `HealthChartDynamic` is a separate file that wraps the same pattern. Three different ways to lazy-load the same shape. +- **Fix:** consolidate into the `HealthChartDynamic`-style wrappers so the loading skeleton, ssr flag, and bundle-split contract are uniform. + +### M5 — Mobile-specific paths: none, but the symptom is real +- The brief asks "why does Marc say especially mobile?" — there are no mobile-specific code paths (no `if (isMobile)` fetches, no viewport-aware fetch logic). The waterfall is the same on desktop and mobile; mobile **feels** worse because: + - Mobile RTT is 80–200 ms vs desktop 20–50 ms. + - The thick slice is the heavy fetch — multiply RTT by ~5–10 vs desktop. + - 10–12 parallel TLS handshakes on a fresh PWA cold-mount on mobile is a measurable JS-thread cost too. +- **No code change to recommend specifically for mobile** — the structural fix (C1, H2) helps both, mobile disproportionately. + +### M6 — Service-worker cache + VersionPoller +- `VersionPoller` polls `/api/version` every 60 s and on mismatch unregisters every SW and reloads. The SW caches Next.js shell + static assets but not API responses. +- **No interference** with the dev tooling I can see. This is healthy. + +--- + +## Low + +### L1 — `viewport` themeColor uses raw hex literals +- `src/app/layout.tsx:86-89` — `themeColor` hex literals don't match a centralised token. If the Dracula palette token shifts, this comment-note path drifts silently. + +### L2 — Inline IIFE `(() => { ... })()` in `page.tsx:719-1437` +- The whole dashboard render body is one ~720-line IIFE inside the JSX. This is readable but hostile to React DevTools — every tile and chart is "anonymous" in the component tree. Extracting a `` component would help debuggability. + +### L3 — `RangeDisplayConfig` and the `getRangeColorClass` / `getRangeHint` helpers re-evaluate every render +- `src/app/page.tsx:146-195` — `weightRange`, `bpSysRange`, `bpDiaRange`, `pulseDisplayRange`, `pulseBands`, `bodyFatBands` are all rebuilt every render from `user.heightCm` / `user.dateOfBirth`. None are memoised. The objects are stable inputs, so a `useMemo` group at the top of the component would freeze them across re-renders and stop the deep child re-renders inside Recharts. + +### L4 — `medication-compliance-chart` invalidation chain +- After a medication intake POST, `medicationDependentKeys` invalidates `medications()`, `analytics()`, `insightsRoot()`, `insightsTargets()`, `medicationIntakeSummary()`, `gamificationAchievements()`. The compliance chart subscribes to `["medication-compliance-chart", days]` — not in the bundle. Same class as C2. + +### L5 — `MeasurementIntakeQuickAdd` uses a `useState` for the footer DOM ref instead of a callback ref +- `src/app/page.tsx:206-214` — `setMeasurementFooterEl` triggers a re-render of the page on portal-target mount. Functionally correct, but a `useRef` + imperative `appendChild` would skip the extra render. Cosmetic. + +--- + +## Method notes + +- **Files read end-to-end:** `src/app/page.tsx`, `src/app/layout.tsx`, `src/components/layout/auth-shell.tsx`, `src/components/providers.tsx`, `src/hooks/use-auth.ts`, `src/hooks/use-workouts.ts`, `src/lib/queries/use-analytics-query.ts`, `src/lib/query-keys.ts`, `src/lib/analytics/merge-slim-thick.ts`, `src/components/dashboard/recent-workouts-tile.tsx`. +- **Files spot-checked:** `src/components/charts/health-chart.tsx` (useQuery block + window derivation), `src/components/charts/mood-chart.tsx`, `src/components/charts/medication-compliance-chart.tsx`, `src/components/charts/health-chart-dynamic.tsx`, `src/app/api/dashboard/summary/route.ts`. +- **Greps run:** + - `useQuery`: 173 hits, `useMutation`: 85, `invalidateQueries`: 81, `useSuspenseQuery`: **0**. + - `queryKey:` bare: 154, factory-routed: 23. + - `"use client"` under `src/app`: 32. + - `` boundaries per tile group (tile strip, BD-Zielbereich, chart row). This is the deeper architectural answer. +4. **H1** — factory migration + lint rule. Defensive, blocks future cache poisoning. +5. **M2/L3/H4** — small wins, batched. diff --git a/.planning/round-v1439-arch-qa-ghosts.md b/.planning/round-v1439-arch-qa-ghosts.md new file mode 100644 index 000000000..4b2015159 --- /dev/null +++ b/.planning/round-v1439-arch-qa-ghosts.md @@ -0,0 +1,188 @@ +# Architecture QA — Ghost Paths + Dead Code + +v1.4.39.3 read-only audit. 457 source files scanned, 914 unique top-level +exports, 182 API routes, 2 437 i18n keys, 51 Prisma indexes, ~30 pg-boss +queues. Findings below are reconciler-grade evidence (file:line + +zero-caller proof) — not stylistic cleanup. + +## Findings + +### Truly dead (recommend deletion — 0 callers anywhere) + +- `src/lib/measurements/rollup-read-cumulative.ts:121` — + `readCumulativeDaySumsBatch`. Only its own test references it; the live + cumulative read path uses `readCumulativeDaySums` (singular). +- `src/lib/measurements/rollup-coverage.ts:77` — `typesMissingCoverage`. + Never imported; the live fast-path guards use `isFullyCovered` (which + *is* alive) or per-type `coverage.get(type)`. +- `src/app/api/medications/intake-summary/route.ts` — full GET handler. + Only reference is `queryKeys.medicationIntakeSummary` in + `src/lib/query-keys.ts:61` (never consumed in any `useQuery({queryKey: + …})`). No `fetch("/api/medications/intake-summary")` anywhere. +- `src/app/api/monitoring/umami/test/route.ts` — superseded by + `/api/admin/monitoring/umami-test` (the one the UI calls). +- `src/app/api/monitoring/glitchtip/test/route.ts` — same pattern; + superseded by `/api/admin/monitoring/glitchtip-test`. +- `TELEGRAM_CLEANUP_QUEUE` ghost in `src/lib/jobs/reminder-worker.ts` + (`const = "telegram-message-cleanup"` at line 164, handler at line + 1098, registered at line 1672, worker bound at line 1832). **No + `boss.send(TELEGRAM_CLEANUP_QUEUE, …)` and no cron schedule**: the + queue will receive zero jobs in production. Doc comment claims "the + Telegram sender schedules cleanup", but `src/lib/notifications/ + senders/telegram.ts` and `src/lib/telegram.ts` never call `boss.send`. +- `src/lib/glucose.ts:18` — `mmolToMgdl` exported but never called + (UI only converts mg/dL → mmol/L via `convertGlucose`; the reverse + helper is dead). +- `messages/de.json` namespace `format.{dateShort,timeShort,dateTime}` — + no `t("format.…")` consumer; `format-locale.ts` builds the patterns + via `Intl.DateTimeFormat`, the literal strings in messages/de.json + duplicate that intent and are stale. (Mirror keys in + en/es/fr/it/pl.json.) + +### Test-only (legit contract verification, no production caller) + +- `src/lib/analytics/correlations.ts:126` — `weeklyAverages`. Charts have + their own local `movingAverageByPoints`; production never imports + `weeklyAverages`. Keep if the contract is intentional; otherwise prune. +- `src/lib/analytics/trends.ts:13` — `movingAverage` (test-only). + `health-chart.tsx`/`mood-chart.tsx` define local + `movingAverageByPoints`; the lib-level helper is parallel-implemented. +- `src/lib/analytics/trends.ts:119` — `detectAnomalies` (called only from + `summarize` inside same file and from its test); no UI surface + consumes anomaly counts. +- `src/lib/measurements/rollup-read-wmy.ts:125-149` — `readWeekRollups`, + `readMonthRollups`, `readYearRollups`. Only the test file imports + them; production uses `readBestGranularityRollups` (the dispatcher). + These three are likely test fixtures left over from the v1.4.35 W-WMY + scaffolding. +- `src/lib/analytics/berlin-day.ts:29` — `berlinDayKey`. Production + switched to user-timezone `wallClockInTz` in v1.4.38; the legacy + helper survives only behind its own test. Comment at + `src/app/api/analytics/route.ts:185` confirms the legacy import was + intentionally dropped. +- `src/lib/insights/prompt.ts:90` — `buildComparisonBlock`. Used inside + the file (line 50) and its test; could be downgraded from `export` to + non-export. + +### Intentional v1.5 scaffolding (document, do not delete) + +- `src/lib/analytics/summaries-slice.ts:615` — + `computeLongWindowSummary`. Awaits v1.5 multi-year card. Tests only. + Already flagged in the brief. + +### Duplicate helpers + +- `wallClockInTz` defined twice: + - `src/lib/medications/compliance-rollups.ts:73` + - `src/lib/medications/scheduling/cadence.ts:107` + + v1.4.39 QA simplifier already flagged this — consolidate into + `src/lib/tz/format.ts` (which exports the canonical version). The two + in-place definitions drift independently and increase the bug surface + for the cross-tz ±3 h guard. + +- `startOfUtcDay` defined twice: + - `src/lib/measurements/rollups.ts:524` + - `src/lib/mood/rollups.ts:620` + + Both have identical bodies. Promote one to a shared `src/lib/tz/` + helper (or `src/lib/measurements/rollup-coverage.ts` — but a fresh + `src/lib/tz/utc-day.ts` is cleaner). + +### Orphaned API routes (no UI consumer) + +- `/api/medications/intake-summary` (full handler, no fetch). +- `/api/monitoring/umami/test` (duplicate of admin variant). +- `/api/monitoring/glitchtip/test` (duplicate of admin variant). +- `/api/admin/import-apple-health-export`, `/api/admin/backup/test`, + `/api/admin/drain-per-sample-cumulative` — operator-curl only, no UI + button. Not strictly dead (documented as operator endpoints) but they + are easy to mistake for orphans. Recommend doc-comment marker + `@operator-only` so future audits skip them. + +### Dead i18n keys + +Approx. 139/2 437 keys (≈ 5.7 %) have no `t("…")` consumer when scanned +for last-segment or full-path string literals. Most are false positives +(computed keys like `medications.sideEffects.entries.${i18nKey(row)}`). +Confirmed orphans after manual spot-check: + +- `format.dateShort`, `format.timeShort`, `format.dateTime` — date + patterns duplicated in messages/de.json but the runtime uses + `Intl.DateTimeFormat` directly. Same in en/es/fr/it/pl. + +A second-pass scan with translator-aware AST (instead of plain grep) is +needed before any mass-prune. Recommend deferring to a follow-up run +with a Babel-based extractor. + +### Unused indexes / columns + +Spot-checked all 51 `@@index([…])` against grep of column names — every +index has at least one WHERE/ORDER consumer in `src/lib` or `src/app`. +No removable indexes found in this pass. Columns with `// legacy …` or +`// TODO(v1.5 iOS):` markers (cf. the schema review above) carry their +own justification and read paths. + +### Long-standing TODOs + +Clean. Only one TODO marker in production source +(`src/components/i18n/maintainership-banner.tsx:14`, and that's +documentation of a test invariant — not a deferred bug). One +`TODO(v1.5 iOS):` in `prisma/schema.prisma` (cross-source workout +dedup) is intentional pre-v1.5 reminder. + +## Top 5 deletion candidates + +Ranked by safety (zero-risk first) × impact (biggest cognitive-noise +reduction first): + +1. **`/api/monitoring/umami/test` + `/api/monitoring/glitchtip/test` + routes.** Both have admin-namespaced replacements that the UI + already uses. Zero callers, zero test fixtures referencing them + as live targets. Safest single deletion in this audit. +2. **`/api/medications/intake-summary` route.** Full GET handler with + no `fetch()` caller and no `useQuery` consumer of its query key. + Safe to delete; query-key entry in `src/lib/query-keys.ts:61` also + removable as a tag-along. +3. **`TELEGRAM_CLEANUP_QUEUE` orphan in `reminder-worker.ts`.** + Handler + register + worker binding for a queue that has no + sender and no cron. Worst case it consumes a queue connection at + boot; best case (after removal) the worker boot is one queue + lighter and the operator-confusion footprint shrinks. Verify with + the operator that the cleanup feature was never wired before + removal — but it clearly isn't running today. +4. **`weeklyAverages`, `movingAverage`, `detectAnomalies`, + `readWeekRollups`/`readMonthRollups`/`readYearRollups`, + `readCumulativeDaySumsBatch`, `typesMissingCoverage`, `mmolToMgdl`, + `berlinDayKey`.** 9 dead exports across `src/lib/analytics/` + + `src/lib/measurements/` + `src/lib/glucose.ts`. Each one has a + live sibling that's the production path. Delete the orphans (and + their tests if the contract is no longer being verified + elsewhere); keep `berlinDayKey` if Marc wants the legacy + reference, downgrade the others. +5. **Duplicate `wallClockInTz` + `startOfUtcDay`.** Consolidate to + `src/lib/tz/` (one source of truth). The cross-tz guard work in + v1.4.38 hinges on these helpers staying in lock-step; today they + drift independently because the two copies aren't even imports of + each other. + +(Bonus, not in top 5: prune the `format.{dateShort,timeShort,dateTime}` +i18n keys from messages/de.json + the 5 locale siblings, with the +caveat that locale tests pin key parity — adjust those tests first.) + +## Process recommendation + +The biggest accumulator of ghost code in this codebase has been the +"replace, but leave the legacy helper for one more release just in +case" pattern (v1.4.35 W-WMY scaffolding, v1.4.36 read-swap probe, +v1.4.38 W-F dashboard summary). Each marathon adds two helpers in +parallel; the deletion of the older one routinely slips because the +release has shipped and "no caller" is hard to prove during the next +marathon kick-off. **The fix is a pre-marathon `unused-exports` CI +job** (use `ts-prune` or `knip` — both work against `tsconfig.json` and +respect dynamic imports; knip additionally covers API routes and i18n +keys). Run it once on every release branch; route new findings into +the v1.x.y.z+1 backlog as triage items. That converts ghost-hunting +from a quarterly archaeology dig into a recurring 5-minute review, +and crucially gives the read-swap-then-delete pattern a finite +horizon: if it's still parallel after two releases, CI flags it. diff --git a/.planning/round-v1439-arch-qa-infra-db.md b/.planning/round-v1439-arch-qa-infra-db.md new file mode 100644 index 000000000..d1de54450 --- /dev/null +++ b/.planning/round-v1439-arch-qa-infra-db.md @@ -0,0 +1,317 @@ +# Architecture QA — Infra + DB + +Scope: independent senior-level review of HealthLog v1.4.39.3 +infrastructure + database layer. Read targets: `prisma/schema.prisma` +(1 910 lines, 44 models), migrations `0067 / 0070 / 0071 / 0072` +(rollup foundation), `src/lib/db.ts`, `src/lib/cache/`, +`src/lib/jobs/reminder-worker.ts`, `src/lib/measurements/rollups.ts`, +`src/lib/mood/rollups.ts`, and the five hottest API routes +(`analytics`, `dashboard/summary`, `measurements`, `mood/analytics`, +`medications/intake`). Spot-checks across `src/app/api/insights/*`, +`src/app/api/sync/*`, and `.planning/round-v1438-perf-analysis.md`. + +## Verdict + +**sound-with-debt** — the rollup tier architecture (v1.4.34.4 → v1.4.39) +is well-designed and correct (composite PKs, idempotent upserts, +boot-time auto-converge, per-write inline DAY hooks + async WEEK+ +worker). Production caching layout, queue topology, FK cascades, and +SQL-injection hygiene are clean. The structural issues are (a) the +rollup tier is **half-consumed** — WEEK/MONTH/YEAR buckets are written +and never read, and the dashboard's progressive-load complaint traces +to several unbounded reads that still live alongside the rollup +fast-paths; (b) a handful of v1.4.30 iOS-future columns +(`syncVersion`, full `deletedAt` consumption across reads) are +write-only ghosts; (c) Insights / aggregator routes have not received +the rollup retrofit and still scan raw `MoodEntry`/`Measurement` per +mount. None of this is a data-risk problem; it is "the architecture +landed correctly but only half the read surface migrated to it." + +## Strengths + +- **Rollup tier composite PK + descending index is correct.** + `measurement_rollups @@id([userId, type, granularity, bucketStart])` + + `@@index([..., bucketStart(sort: Desc)])` guarantees idempotent + upserts and serves "last N buckets" cheaply. Mirrors hold for + `mood_entry_rollups` and `medication_compliance_rollups`. +- **FK + onDelete cascades are uniform.** Every per-user child table + (`Measurement`, `MoodEntry`, `MedicationIntakeEvent`, `Workout`, + `MedicationInventoryItem`, `MeasurementRollup`, `MoodEntryRollup`, + `MedicationComplianceRollup`, `Device`, `Passkey`, `Session`, + `RefreshToken`, `CoachConversation`, `IdempotencyKey`, …) cascades + on user deletion. `Feedback.user` and `AuditLog.user` use + `SetNull` to keep the audit trail past account deletion — correct + GDPR-balanced choice. `PersonalRecord.sourceMeasurementId` is a + loose pointer (no FK) which is intentional and documented. +- **Boot-time backfill is auto-converging and idempotent.** The + v1.4.39.1 widening to per-`(user, type, day)` discovery + (`rollups.ts:769-791`) plus the v1.4.39 W-SUM null-row discovery + branch is the correct shape — discovery query only matches users + with gaps; once filled they drop off. Idempotency guaranteed by + `singletonKey: boot-backfill|${id}` inside pg-boss. +- **SQL injection hygiene is clean.** Every `$queryRawUnsafe` call + inlines only whitelisted enum values (`measurement_type` cast, + `date_trunc('day', …)` unit literal validated against a closed + set) or feeds user data through bound parameters. The grain + injection at `measurements/route.ts:464` uses `Prisma.raw` with + a typed lookup map. No interpolated user input found. +- **Cache invalidation fans out correctly per write surface.** + `invalidateUserMeasurements / Mood / Medications` evict every + cache that reflects the user's set (analytics, mood-analytics, + achievements, workouts, insightsTargets, medicationsIntake). + v1.4.38 W-F added analytics eviction on medication writes for + the dashboard streak/compliance feed — that fix shows the model + works. +- **Single-flight in-process LRU is sound.** `ServerCache.wrap` + registers the in-flight promise on `pending` and removes it on + reject so a transient failure never poisons the key — the same + pattern `ensureUserRollupsFreshInFlight` reuses one layer up. + +## Issues + +### Critical + +- *(none observed)* — no data-risk findings. + +### High + +- **`measurement_rollups` WEEK / MONTH / YEAR rows are write-only.** + Every measurement write enqueues all three (`rollups.ts:200-221`), + the worker folds them, and the boot backfill writes them with + default opts. **No read path consults them** — per the v1.4.38 + perf audit the dashboard's "All / 1 yr" tab still falls through + to live SQL. Net effect: ~3× write amplification per measurement + for zero read-side win, plus storage for buckets nothing reads. + Either wire the WMY reader (`rollup-read-wmy.ts` exists but is + not the hot-path default) or stop populating WMY until a reader + ships. The fix is local — gate the async enqueue on a feature + flag, or remove the WMY granularities from `ALL_GRANULARITIES`. + This is the single biggest "growth introduced coupling, then we + didn't finish the harvest" finding. + +- **Unbounded `prisma.moodEntry.findMany({ where: { userId } })` in + six routes.** Pre-rollup-tier shape persisted in: + - `src/app/api/insights/targets/route.ts:983` (no window) + - `src/app/api/insights/comprehensive/route.ts:88` + - `src/app/api/insights/glp1-timeline/route.ts:95` + - `src/app/api/gamification/achievements/route.ts:607` + - `src/app/api/mood-entries/route.ts:57` (has `take` so OK) + - `src/app/api/mood/analytics/route.ts:126` (live fallback only) + + The W-MOOD rollup tier (v1.4.39) exists; only `mood/analytics` + consumes it. The Insights cluster still scans every mood the + user ever wrote on every mount. The progressive-load complaint + on mobile fits this exactly: dashboard tiles backed by analytics + / mood-analytics paint fast (≤ 500 ms), Insights / Health-Score + surfaces wait on cold mood scans. + +- **Unbounded `prisma.measurement.findMany({ where: { userId, + type:{in:types} } })` for "latest ever per type".** + `src/app/api/insights/targets/route.ts:191` runs + `findMany({ orderBy: measuredAt: desc, distinct: ["type"] })` + WITHOUT a `measuredAt >=` floor. On Marc's 347 k-row tenant + Postgres still has to sort the full set before applying + `DISTINCT ON`. `DISTINCT ON (type) … ORDER BY type, measuredAt + DESC` with the existing `(userId, type, measuredAt)` index is + the targeted fix — Prisma's `distinct` does not compile to + PG's `DISTINCT ON`; it dedups in the driver after pulling the + rows. Same anti-pattern in `gamification/achievements/route.ts`. + +- **Cumulative rollup write-amplification under `Promise.all` is + fine but the batch ingest hook in `measurements/route.ts:599-608` + awaits `recomputeBucketsForMeasurement` sequentially.** A 500-row + Apple-Health batch hitting 10 distinct (type, day) keys serialises + 10 DAY recomputes + 30 pg-boss sends. Either wrap in `Promise.all` + (cheap — the DAY upsert is a single bounded aggregate) or hand + off the whole batch to `recomputeUserRollups` once at the end the + way `apple-health-import-worker.ts:218` already does for imports. + +- **`Measurement.deletedAt` is set by the iOS soft-delete path + (`by-external-ids` route) but no analytics / dashboard read filters + on it.** `src/app/api/sync/state/route.ts` is the only consumer + that distinguishes live vs tombstoned. Every other read path + (`/api/analytics`, `/api/dashboard/summary`, the rollup populator + aggregate at `rollups.ts:383-414`) includes tombstoned rows in + the count / mean / sum. This is a latent correctness bug the + moment iOS soft-deletes the first row: the deleted reading still + appears in dashboard sparklines and pulls down the average. + +### Medium + +- **`MeasurementRollup.sumValue` reader coverage is partial.** + `readRollupBuckets` (`rollups.ts:283-303`) DOES NOT select + `sumValue`. The two consumers that need the sum + (`dashboard/summary` and the `measurements?source=rollup` branch) + select it directly via Prisma. Every other rollup reader silently + drops the column. Either expose it through `readRollupBuckets` + or document that it is not part of the canonical reader shape. + +- **`MeasurementRollup.r2 / slope` set by `runRollupAggregate` but + no reader currently consumes them off the rollup table** — the + comprehensive aggregator + summaries-slice still compute slope/r2 + via the live `$queryRaw` (slope/r2 don't compose linearly across + buckets). The columns are precomputed dead weight. Two options: + (a) drop them and accept the live compute, (b) document that the + rollup's slope/r2 is "per-bucket internal stat" and explicitly + not the trailing-window slope the readers want. + +- **`ensureUserRollupsFresh` warms only DAY for 90 days.** Anything + the user opens for the "1y" tab on a metric whose DAY buckets are + stale beyond the 90-day window falls through to live SQL even + though the WMY buckets sit there fresh from the worker. Same + half-coupling pattern as the High finding above. + +- **`Workout` unique key dedups on `(userId, source, externalId)` + which means the same workout from MANUAL + iOS HealthKit lands + twice.** Documented as a TODO at `schema.prisma:605-613`. Real- + world impact lands the moment iOS Health passthrough ships in + v1.5. The fix needs to mirror the measurement cross-source + picker — schedule it now, not after v1.5 ships. + +- **`prisma.$transaction([…upserts])` chunk size in + `persistRollupRows` (`rollups.ts:469`) wraps 500 upserts in a + single interactive transaction.** This holds a Postgres + connection for the duration of 500 round-trips. On the Apple + Health import worker (which then runs a full multi-year fold) + this can be measured in seconds. Replace with a single multi- + row `INSERT ... ON CONFLICT DO UPDATE` via `$executeRaw` for + the bulk-import path; the per-write inline DAY recompute (one + row) is already fine without `$transaction`. + +- **`User.lastSyncedAt` (v1.4.30 column) has exactly one consumer + (`/api/sync/state`).** It's defended as iOS-foundation. Not a + bug, but worth flagging: iOS v0.5.4 connected; if the column + still sees no writer post-iOS-launch, mark it for cleanup. + +- **`User.syncVersion` on `Measurement` (v1.4.30, defaults to 1) + has zero readers or writers in `src/` that increment it.** The + column was added so iOS can do last-writer-wins; no route bumps + it on update. Ghosting risk — either wire the increment in the + PATCH path or drop the column. + +- **`caches.moodAnalytics` is keyed `user.id`** (`mood/analytics/ + route.ts:166`) while every other cache is keyed + `${user.id}|`. `invalidateUserMood` does + `deleteByPrefix(userId)` which works for both — but `userA` and + `userAB` (substring) would collide if cuids ever shared a prefix. + cuids are 25 char fixed and the substring case is theoretical; + still, standardise the key shape across the eight caches. + +- **`caches.bugreportStatus.deleteByPrefix("")` evicts every entry + on any admin-settings write** (`invalidate.ts:96-98`). That's + fine for a singleton cache, but with the comment "the cache is + keyed on the singleton" the empty-prefix call is opaque. Rename + to `caches.bugreportStatus.clear()` (add the method) or pass + `"singleton"` explicitly. + +### Low + +- **Migration `0061_audit_log_carrier` adds `asn` + `carrier` as + nullable WITHOUT a backfill of the geo-backfill cron's first + run.** Acceptable (the cron auto-converges) but worth noting: + the v1.4.27 release announced the columns are populated, but + every audit row pre-cron is null until the hourly job catches + up. The hourly cap of 5 000 rows means a tenant with > 5 k + legacy audit rows takes hours to converge. + +- **`Measurement.deviceType` is `String?` rather than an enum** — + documented decision, but the v1.4.25 W8c enum lives in TypeScript + (`deviceTypeEnum`). DB-side check constraint would catch a + client that ships a typo like `"Watch"` (capitalised). + +- **`MedicationComplianceRollup.day` is `String` (YYYY-MM-DD) not + `DATE`.** Documented as multi-instance-safe, fine for the read + path. Cost: range queries can't use a `DATERANGE` operator and + bytewise lex compare relies on the fixed 10-char shape. Add a + CHECK constraint that day matches `^\d{4}-\d{2}-\d{2}$` so a + bad writer can't slip a non-conforming string in. + +- **`HostMetric` table has no TTL trigger.** Documented as "7-day + retention enforced inside the sampler" — that's an application- + layer DELETE that runs per sample. Lightweight, but a forgotten + worker turns this into an unbounded table. Add a Postgres + partition or row-level expire trigger for defence-in-depth. + +- **`prisma/migrations/0067_v1434_measurement_rollups/migration.sql` + uses `IF NOT EXISTS` everywhere** — good for replay safety. But + the FK `ALTER TABLE` is not guarded by an `EXCEPTION WHEN + duplicate_object` block (0070 / 0071 do guard it). Inconsistent. + If the migration is replayed on an instance that already + carries the FK, 0067 throws. Make the three rollup migrations + identical in guard shape. + +- **`AuditLog @@index([userId, createdAt])` covers the per-user + view, but `@@index([action, createdAt])` lets the admin "all + recent logins" view scan only one action. The admin audit + endpoint at `src/app/api/admin/audit-log/route.ts` should be + verified to actually use both branches — confirm via `EXPLAIN`. + +## Ghosting / dead code + +- **`User.syncVersion` on `Measurement`** — column added in v1.4.30 + to support last-writer-wins reconciliation; zero readers, zero + writers that bump it on update. +- **`MeasurementRollup.slope` and `MeasurementRollup.r2`** — + populated by `runRollupAggregate`, no current reader. +- **`MeasurementRollup` WEEK / MONTH / YEAR rows** — written by + worker + boot backfill, no read path consumes them. +- **`MedicationCategory.GLP1`-related side-effect logging coverage** + — surface is wired and the schema is sound, but the side-effect + read in the Coach snapshot path is not yet on the rollup tier. + Low priority (cardinality is small). +- **`RateLimit @@index([resetAt])`** — used by a hypothetical + cleanup query; the actual rate-limit code (`src/lib/rate-limit.ts`) + upserts by `key` and never queries by `resetAt`. The cleanup + cron (`RATE_LIMIT_CLEANUP_QUEUE`) does use it, so the index is + paid-for. Keep, but verify the cleanup cron is actually + registered (it is — `reminder-worker.ts:1675`). +- **`AppSettings.adminAiInsightsFeedbackSummary`** — written by the + feedback aggregator; the admin dashboard reads it. Not dead, but + the column type is a free-form `Json?` with the shape documented + only in a comment. Worth a Zod parse at read time. + +## Top 3 recommendations + +1. **Read-swap the Insights cluster onto the rollup tier** + (`/api/insights/targets`, `/api/insights/comprehensive`, + `/api/insights/cards`, `/api/insights/glp1-timeline`, + `/api/insights/generate`, plus the gamification mood read). The + pattern is documented and proven (W-MOOD + dashboard/summary). + Replace the unbounded mood / measurement `findMany` calls with + `readRollupBuckets / readMoodDayRollups` + per-type fast-paths. + Impact: 5-10× wall-clock improvement on Insights cold mount, + removes the "burst all at once vs progressive" UX complaint. + Effort: 1-2 days, mechanical pattern. + +2. **Either harvest or stop populating WEEK / MONTH / YEAR + buckets.** Either (a) wire `rollup-read-wmy.ts` into the + `aggregate=weekly|monthly` branch of `/api/measurements` and the + "All / 1y" dashboard tabs and reap the read-side win, or (b) + shrink `ALL_GRANULARITIES` to `["DAY"]` until the reader ships. + Today is the worst of both worlds — write cost, no read win. + Impact: 3× reduction in per-measurement-write work *or* unlocks + the long-range-tab perf. Effort: 0.5 day either direction. + +3. **Wire `Measurement.deletedAt` filtering into every analytics + read path or drop the column.** Either add + `deletedAt: null` to every `where: { userId, … }` on + `Measurement` (and the SQL aggregates), or remove the soft- + delete flag from the iOS sync endpoint and use hard deletes. + The current state — column exists, one route filters, every + other reader returns tombstoned rows — is a correctness bug + that lands the moment iOS soft-deletes its first reading. + Impact: correctness, prerequisite to v1.5 iOS launch. Effort: + 0.5 day grep-and-patch. + +--- + +Reviewer's broader read: the v1.4 cycle's rapid sub-release cadence +shipped a structurally sound rollup architecture, but the read-side +migration is half-done. Marc's "burst on mobile" intuition is correct +— it traces to (a) Insights/aggregator routes not yet on rollup, (b) +WMY tier write-cost without read benefit, (c) batch ingest hooks +serialising recomputes. None of this is a "wrong architecture" +problem; it's a "finish migrating the read surface to the tier you +already built" problem. The strangler-fig pattern is correctly used +where it has been applied (`source=rollup` opt-in, coverage probe +gate, live fallback on miss). Apply it to the remaining routes. diff --git a/.planning/round-v1439-arch-qa-organization.md b/.planning/round-v1439-arch-qa-organization.md new file mode 100644 index 000000000..91dc15a9f --- /dev/null +++ b/.planning/round-v1439-arch-qa-organization.md @@ -0,0 +1,107 @@ +# Architecture QA — Codebase Organization (Outside-Eye) + +Snapshot: v1.4.39.3, ~681 TS/TSX source files, 36 top-level `src/lib/` modules, 132 `__tests__/` co-located dirs, 60 integration tests, 33 e2e specs. + +## First-impression verdict + +The codebase is **organized, opinionated, and surprisingly disciplined for the pace it has been shipped at**. The App-Router layout is by-the-book; `src/lib/` is partitioned by domain (analytics, ai, insights, measurements, medications, mood, withings, jobs, notifications, auth, …) rather than by technical kind (no `services/`, `helpers/`, `utils/` dumping grounds — `utils.ts` is 169 bytes); every meaningful module owns its `__tests__/` directory; `AGENTS.md` is an actively-maintained architecture statement that matches what the tree actually looks like. That is uncommon at this churn level — most codebases six weeks into a marathon would already have a `lib/misc/` or a `components/old/`. There is none here. + +Where it shows wear is at the **seams between adjacent domains**. `src/lib/analytics/`, `src/lib/insights/`, `src/lib/ai/`, `src/lib/measurements/` (rollups), and `src/lib/medications/` (compliance-rollups) all live as peers, but the rollup tier introduced in v1.4.34 is now scattered across at least three of them (`measurements/rollups.ts`, `measurements/rollup-read*.ts`, `measurements/rollup-coverage.ts`, `analytics/*-fast-path.ts`, `medications/compliance-rollups.ts`, `mood/rollups.ts`). It works, but it is no longer a recognizable subsystem from a single directory. Similarly, three distinct `interface AnalyticsData {…}` declarations coexist (`src/app/page.tsx`, `src/app/insights/page.tsx`, `src/components/onboarding/getting-started-checklist.tsx`) alongside the shared `src/types/SubPageAnalyticsData` — that was an intentional partial extraction and the comment header acknowledges it. + +The other wear-pattern is **file size at the busiest junctions**: `reminder-worker.ts` 2 175 LOC, `src/app/page.tsx` 1 440 LOC, `health-chart.tsx` 1 722 LOC, `ai-section.tsx` 1 739 LOC, `insights/targets/route.ts` 1 223 LOC, `measurements/rollups.ts` 846 LOC, `insights/features.ts` 929 LOC. None of these is "broken," but each is a single-author cognitive load that has grown one marathon at a time. + +## Module boundaries + +- **`src/app/`** is clean App-Router idiom: every route file is `page.tsx` / `route.ts` / `[id]/`, the `api/` tree mirrors the URL space, and `proxy.ts` (the renamed-from-middleware file documented in AGENTS.md) lives at the right level. +- **`src/lib/`** is partitioned by **domain**, not by kind — exactly the right call for a health app. 36 top-level subdirs, but each one has a coherent name (`measurements`, `mood`, `medications`, `withings`, `insights`, `ai`, `analytics`, `notifications`, `jobs`, `auth`, `cache`, `charts`, `i18n`, `logging`, `tz`, `openapi`, `validations` …). The few loose `.ts` files at the lib root (`crypto.ts`, `db.ts`, `api-response.ts`, `api-handler.ts`, `format.ts`, `geo.ts`, `telegram.ts`, `glucose.ts`, `gravatar.ts`, `query-keys.ts`, `timezone.ts`, `idempotency.ts`, `rate-limit.ts`) are genuinely cross-cutting primitives — not a dumping ground. +- **`src/components/`** mirrors the page tree (`measurements/`, `medications/`, `mood/`, `insights/`, `dashboard/`, `settings/`, `admin/`, `onboarding/`, `targets/`, `charts/`, `layout/`, `ui/`). The contract holds. +- **`src/generated/prisma/`** is honored — only **3 components** (`personal-record-badge.tsx`, `SideEffectsSection.tsx`, `health-chart.tsx`) import directly from `@/generated/prisma`. That is impressively low for an app this size. +- **Cross-layer violations**: I found **one** — `src/components/admin/backups-section.tsx` imports `BackupRow, BackupsList` from `@/app/api/admin/backups/route`. That is a component depending on a route handler's exported types, the textbook anti-pattern. It is the only one in the tree. Otherwise the direction is strict: `components → lib`, `app → lib`, `app → components`, never `lib → components` (except in one feature-flag test file, which is fine). +- **Analytics / Insights / AI separation** has **leaked**. `src/lib/analytics/` holds the SQL-aggregation primitives + the new `*-fast-path.ts` rollup readers. `src/lib/insights/` holds the metric-specific status modules (`blood-pressure-status.ts`, `bmi-status.ts`, `pulse-status.ts`, `weight-status.ts`, `mood-status.ts`, `medication-compliance-status.ts`, `general-status.ts`) **plus** `correlations.ts` (24 KB) **plus** `features.ts` (929 LOC) **plus** `comprehensive-aggregator.ts` (775 LOC) **plus** `prompt.ts` + `prompt-compact.ts`. `src/lib/ai/` is the provider/prompt layer (`prompts/`, `coach/`, `provider-runner.ts`, `provider-chain.ts`, `schema.ts`). The boundary "insights = domain-aware feature extraction; ai = LLM mechanics" is **roughly** there but `insights/prompt*.ts` and `ai/prompts/` are two prompt directories that a new reader would not be able to distinguish from filenames alone. +- **Rollup tier** is the youngest subsystem and the most scattered. It lives in `measurements/rollups.ts` + `measurements/rollup-read*.ts` + `measurements/rollup-coverage.ts` + `analytics/*-fast-path.ts` + `medications/compliance-rollups.ts` + `mood/rollups.ts` + boot-backfill in `jobs/`. Each piece is well-named in isolation; together they are not a directory. + +## Type / DTO single-source + +- **Zod-first validation is honored**: `src/lib/validations/` holds 17 schemas (`measurement.ts`, `medication.ts`, `moodlog.ts`, `coach-prefs.ts`, `workout.ts`, `source-priority.ts`, …) and they are consumed both by API routes (runtime) and by clients (inferred types). This is the strongest single source-of-truth signal in the codebase. +- **API response envelope** is single-sourced in `src/lib/api-response.ts` (`apiSuccess` / `apiError` / `safeJson`) and the `{ data, error, meta }` shape is universal. +- **DTO single-source-of-truth is partially leaked**: + - `interface AnalyticsData` is declared 3× (`src/app/page.tsx`, `src/app/insights/page.tsx`, `src/components/onboarding/getting-started-checklist.tsx`) — `src/types/analytics.ts` extracted *only* the slim sub-page variant, intentionally per its header comment. + - One component imports a route handler's exported type directly (`components/admin/backups-section.tsx ← app/api/admin/backups/route.ts`). Once that pattern starts, it grows. +- **Prisma types are not wrapped** in a domain layer. Direct `@/generated/prisma/client` imports across analytics + ai + insights + measurements — that is the right call for an internal app of this size (a domain layer would be ceremony). The fact that only 3 components reach for Prisma directly means the wrapping happens *organically* at the lib boundary. + +## AI / Insights structure (Marc's domain priority) + +- **`src/lib/ai/`** is well-shaped: one file per provider (`anthropic-client.ts`, `openai-client.ts`, `codex-client.ts`, `local-client.ts`, `mock-client.ts`), a `provider.ts` orchestrator (16 KB), `provider-chain.ts`, `provider-runner.ts`, `schema.ts` (21 KB Zod schemas for AI responses — single source of truth), `prompts/` directory with one file per metric (`blood-pressure.ts`, `bmi.ts`, `mood.ts`, `pulse.ts`, `weight.ts`, `medication-compliance.ts`, `general-status.ts`, `native-prompts.ts`) plus `base-system.ts`, `insight-generator.ts` (carries `PROMPT_VERSION`), and `safety-contracts.{de,en,es,fr,it,pl}.yaml` localized policy files. `coach/` is the conversational adjunct with its own `system-prompt.ts`, `snapshot.ts`, `budget.ts`, `refusal.ts`, `keyvalues.ts`, `persistence.ts`. This is **adapter-pattern done right**. +- **`PROMPT_VERSION`** is single-sourced in `src/lib/ai/prompts/insight-generator.ts` and consumed by `feedback-attribution.ts`, `schema.ts`, `validations/recommendation-feedback.ts`. No duplication. +- **`src/lib/insights/`** is the bridge between domain data and AI input. The seven `*-status.ts` files (blood-pressure, bmi, pulse, weight, mood, medication-compliance, general) form a consistent pattern — each owns its metric's status derivation. `features.ts` (929 LOC) + `comprehensive-aggregator.ts` (775 LOC) are the two large files here; both are pure aggregation logic and look like they grew organically from the marathon adds (W7b, IW-G, etc.). +- **The weak seam**: `src/lib/insights/prompt.ts` + `prompt-compact.ts` overlap conceptually with `src/lib/ai/prompts/`. A new reader cannot tell from the filenames which one to extend. +- **`src/components/insights/`** has 37 files — large but every name reads cleanly (`coach-panel/`, `daily-briefing.tsx`, `health-score-card.tsx`, `recommendation-card.tsx`, `correlation-card.tsx`, `trends-row.tsx`, …). The `coach-panel/` sub-directory is a clean module of its own (drawer, body, input, history-rail, message-thread, sources-rail, settings-sheet, mobile-rail-tray, source-chips, use-coach hook). + +## Naming + conventions + +- **App Router idiom**: 100% consistent — `route.ts`, `page.tsx`, `[id]/`, `[section]/`, `__tests__/` colocated. +- **Files**: 95% kebab-case. The **outliers** are all in `src/components/medications/`: `DrugLevelChart.tsx`, `MedicationCardHeader.tsx`, `ResearchModeAcknowledgmentDialog.tsx`, `SchedulingSection.tsx`, `SideEffectsSection.tsx`, `TitrationSection.tsx`. Six PascalCase files in one directory among 681. Looks like a single sub-marathon's author preference that never got normalized. +- **Hooks**: two homes, two conventions — + - `src/hooks/` holds **12** generic-cross-cutting hooks (`use-auth`, `use-feature-flags`, `use-is-mobile`, `use-viewport-width`, `use-coach-prefs`, `use-coach-handoff`, `use-insights-analytics`, `use-insights-layout-prefs`, `use-chart-overlay-prefs`, `use-insight-status`, `use-scroll-reset-on-route`, `use-workouts`). + - `src/lib/queries/` holds **2** TanStack-Query wrappers (`use-achievements-query`, `use-analytics-query`). + - Plus component-local hooks (`use-coach.ts` inside `coach-panel/`, `use-chart-tooltip-active.ts`, `use-insights-advisor.ts` inside `components/insights/`). + - The `*-query` suffix is used in 2 files in `lib/queries/`, but **not** in `hooks/use-workouts.ts` (also a TanStack-Query hook). That is a soft inconsistency — not broken, but it would bite a refactor. +- **`src/types/`** holds **a single file** (`analytics.ts`). Either commit to it as the shared DTO home or fold it back in. Right now it looks half-built. +- **API route directories** mirror the URL space cleanly. `__tests__/` directories sit next to every non-trivial route. + +## Marathon scar tissue + +- "v1.4.X — …" comments are pervasive (53 in `api/analytics/route.ts`, 46 hand-written `//` comments in `reminder-worker.ts`, 17 in `comprehensive-aggregator.ts`, 15 in `rollups.ts`, 8 in `insights/features.ts`). They explain *why* a fast-path was added, *which* release a refactor landed in, *what* the prior shape looked like. For the *author* during a marathon they are gold. For a *future reader six months removed from the marathon vocabulary* they are noise — "v1.4.34 IW-G", "v1.4.37 W2", "QA F-H-01", "BK-F-M1" decode to nothing without `.planning/` context. +- The `.planning/` directory holds **328** files. Pruning it isn't the answer (it's the audit trail), but the **in-source comment density that references it** has crossed into "the comments are the changelog now." +- The header style is consistent enough that a one-pass regex strip would be cheap. The `// v1.4.X — explanation` pattern is the single most prevalent comment style in the modified files. +- The good news: the marathon scars **never bled into filenames or directory names**. Nothing is called `bp-in-target-v2-fast-path-2026-05-17.ts`. The version-tagging stayed in comments, where it can be stripped without churning git blame. + +## What's solidly built + +- Domain-partitioned `src/lib/` with no `utils/`, `helpers/`, `services/` dumping grounds. +- Multi-provider AI adapter (`src/lib/ai/`) — clean per-provider file, single `provider.ts` orchestrator, single `schema.ts` Zod source. +- Zod-as-single-source for runtime validation + inferred types (`src/lib/validations/`). +- `{ data, error, meta }` envelope universal via `src/lib/api-response.ts`. +- Co-located `__tests__/` in 132 directories — consistent convention. +- `getSession()` + `proxy.ts` + `requireAuth()` auth seam intact. +- `AGENTS.md` is an **actively-maintained** architecture statement that matches the tree (Gotchas section is invaluable: Prisma 7 / Next.js 16 / Zod v4 / pg-boss / SimpleWebAuthn). +- App-Router idiom 100% consistent. +- Only 3 components reach for `@/generated/prisma` — the wrapping happens at lib boundary organically. +- Coach + Insights pages have their own coherent component sub-trees (`coach-panel/`). +- i18n discipline: messages are still partitioned across 6 locale files; no duplicated translation tables. + +## What's accreted / debt'd + +- **Rollup tier is scattered across 5 directories** (`measurements/`, `analytics/`, `medications/`, `mood/`, `jobs/`) with no `src/lib/rollups/` umbrella. Each file is well-named; the *system* is not a directory. +- **Two prompt homes**: `src/lib/ai/prompts/` and `src/lib/insights/prompt*.ts`. The split makes sense to whoever built it but is undiscoverable from filenames. +- **Three `interface AnalyticsData` declarations** coexist alongside the shared slim type in `src/types/analytics.ts`. The header comment in `types/analytics.ts` acknowledges this is intentional; that doesn't change the fact that a new contract change has to be applied in three places. +- **One component → route-handler type import** (`admin/backups-section.tsx`). Anti-pattern. Once it normalizes, it spreads. +- **`src/types/` has one file**. Decide: extract more DTOs here, or fold this one back. +- **PascalCase filename island** in `src/components/medications/` (6 files). Looks like a single-author sub-marathon. +- **Five files over 1 000 LOC**: `reminder-worker.ts` (2 175), `health-chart.tsx` (1 722), `ai-section.tsx` (1 739), `page.tsx` dashboard (1 440), `insights/targets/route.ts` (1 223), `medication-form.tsx` (1 162), `measurement-list.tsx` (1 187). Each is a single-author cognitive load. +- **Hook home is forked**: `src/hooks/` vs `src/lib/queries/` vs component-local `use-*.ts`. Naming convention drift (`-query` suffix in 2 files, absent elsewhere). +- **In-source release-version comments**: dense, valuable to the author, opaque to a six-months-later reader without `.planning/` cross-reference. + +## Strategic recommendations + +1. **Create `src/lib/rollups/`** as the umbrella for the v1.4.34+ rollup tier. Move (or re-export from) `measurements/rollups.ts`, `measurements/rollup-read*.ts`, `measurements/rollup-coverage.ts`, `analytics/*-fast-path.ts`, `medications/compliance-rollups.ts`, `mood/rollups.ts`. Highest-impact structural move: it makes the youngest, most-scattered subsystem **a recognizable directory**. Do this **before iOS** so the contract surface (what reads from rollups vs what reads live) is visible from one place. +2. **Promote `src/types/` to the actual DTO home** — or delete it. Extract the three `interface AnalyticsData` declarations, the `BackupRow`/`BackupsList` from `admin/backups/route.ts`, and a handful of other shapes that drift between server and client. The cost is one afternoon; the payoff is that "where do shared shapes live?" has one answer. +3. **Unify prompt directories**: collapse `src/lib/insights/prompt*.ts` into `src/lib/ai/prompts/insight-*.ts` (or rename `ai/prompts/` to `ai/prompt-templates/` and move `insights/prompt*.ts` there). One prompt home, one mental model — important because *prompts are the product*. +4. **Soft cap files at ~800 LOC** with a lint warning (not error). The 7 files >1 000 LOC are not "bad code" — they are the busiest junctions and likely candidates for natural extraction. `reminder-worker.ts` at 2 175 LOC is the standout — it already has phase modules in `reminder-phases.ts`; finish the extraction. +5. **Marathon-comment policy**: keep `// v1.4.X — why` for **non-obvious decisions** (cache invalidation rules, race-condition workarounds, contract-locked APIs). Strip them on `// trivial rename`, `// extracted body to`, `// moved from`. The rule is: if the comment would not make sense to someone who has never read `.planning/`, either inline the rationale or drop it. Cheap one-pass cleanup. +6. **Normalize the 6 PascalCase files** in `src/components/medications/` to kebab-case. Trivial, removes the only naming-convention island. +7. **Hook home decision**: pick one — `src/hooks/` for all React hooks (including TanStack-Query wrappers) **or** `src/lib/queries/` for query hooks + `src/hooks/` for everything else. Apply the `-query` suffix uniformly (or drop it). Avoid component-local `use-*.ts` outside of single-use cases. + +## What NOT to refactor (warning bells) + +- **`src/lib/ai/` provider files** — multi-provider adapter looks repetitive (5 client files), but each one carries provider-specific authentication, retry, and tokenization quirks. Don't DRY them. +- **The three `AnalyticsData` interfaces** — the header comment in `src/types/analytics.ts` explicitly explains why the dashboard's variant stays separate (it carries `bpInTargetPct*` aggregates). Recommendation 2 above means *extracting them where they are duplicated*, not forcing them to merge. +- **`prisma.config.ts` not `schema.prisma`** — the DB URL lives in the config file per Prisma 7's contract. AGENTS.md flags this as a hard-won gotcha. Don't "fix" it. +- **`proxy.ts` not `middleware.ts`** — Next.js 16 rename. Same gotcha class. +- **`zod/v4` imports** — `import from "zod/v4"` looks weird but is required by Zod v4's transitional namespace. Don't refactor to plain `zod`. +- **`src/generated/prisma/`** — generated, never hand-edit. +- **The 132 co-located `__tests__/` dirs** — this is **the** test convention. Don't move tests to a central tree. +- **`PROMPT_VERSION` exported from `insight-generator.ts`** — single source. The fact that `feedback-attribution.ts` imports it is the contract, not a smell. +- **The `safety-contracts.{locale}.yaml` files in `ai/prompts/`** — YAML alongside TS in the same dir looks odd; it is the localized AI policy and the runtime expects it there. +- **`src/lib/jobs/reminder-worker.ts` size** — 2 175 LOC is large, but the phase logic was already extracted to `reminder-phases.ts`; what remains is a single pg-boss entry point. Don't shrink for shrink's sake; shrink only when a coherent sub-module can be lifted (e.g. APNs vs Telegram vs ntfy vs Web Push dispatcher branches). +- **`.planning/` 328 files** — this is the audit trail. Don't prune. diff --git a/.planning/round-v1439-empirical-trace.md b/.planning/round-v1439-empirical-trace.md new file mode 100644 index 000000000..9c4184060 --- /dev/null +++ b/.planning/round-v1439-empirical-trace.md @@ -0,0 +1,435 @@ +# v1.4.39 — empirical dashboard cold-mount waterfall trace + +Read-only Playwright-driven investigation of the dashboard cold-mount +network/render behaviour Marc observed post-v1.4.39.2: +"mood + medication paint first, then all other tiles arrive faster +than before but still in a single burst — *etwas blockiert noch*". + +Captured on `/Users/marc/Projects/HealthLog` against a locally-seeded +fixture mirroring the shape of Marc's production tenant (82 490 +measurements across 8 high-volume types over 2 years, 500 mood +entries) on `localhost:3000` (Next.js dev mode). Production +(`healthlog.bombeck.io` v1.4.39.3) was probed read-only for header / +status / version parity but Marc's authenticated routes were not +exercised against prod. + +Raw trace artifact: `/tmp/playwright-dashboard-trace.json`. +Visual progression: `/tmp/dashboard-mobile-{500,1000,1500,2500,4000,6000,8000}ms.png`. +Playwright driver: `/tmp/playwright-dashboard-trace.js`. + +--- + +## 1 — Test setup + +* **Server**: `pnpm dev` on `localhost:3000`, Node v25.9.0, + `NODE_ENV=development`, version 1.4.39.3. The Prisma client lives in + `src/lib/db.ts` and is constructed via + `new PrismaClient({ adapter: new PrismaPg({ connectionString }) })` + with **no `max` connection-limit override** — `pg.Pool` falls + back to its default of **10 concurrent connections**. +* **DB**: PostgreSQL on localhost:5432, schema `public`. +* **User**: `e2e-tester` (admin), seeded via + `/Users/marc/Projects/HealthLog/e2e/setup/storageState.json` cookie + jar (Playwright `globalSetup.ts` pattern). +* **Fixture shape** (seed script reproduced inline below): + * `WEIGHT` × 730, `BLOOD_PRESSURE_SYS` × 1460, `BLOOD_PRESSURE_DIA` + × 1460, `PULSE` × 1460, `BODY_FAT` × 730, `SLEEP_DURATION` × 730, + `ACTIVITY_STEPS` × 70 080 (96/day × 730 days), `OXYGEN_SATURATION` + × 5 840 → **82 490 measurements, 8 types**. + * 500 mood entries (1/day). + * No rollups (deleted up-front to force `path:"live"` on every + fast-path). +* **Browser**: Playwright Chromium, headless. Trace captured on + desktop viewport (1280×900); visual screenshots also on iPhone-15-Pro + mobile viewport (393×852, DSR 3, isMobile=true). +* **Cold vs warm**: cold = first navigation after Playwright context + creation; warm = same context, second navigation 8 s later. + +--- + +## 2 — Annotated COLD waterfall + +Listed in order of request **start** (ms from `page.goto` call). + +| start | end | dur | size | TTFB | URL | +|---:|---:|---:|---:|---:|---| +| 356 | 484 | 128 | 0.1 KB | 131 | `GET /api/monitoring/settings` | +| 356 | 521 | 165 | 0.5 KB | 166 | `GET /api/auth/me` | +| 357 | 496 | 139 | 0 | 138 | `POST /api/internal/web-vitals` | +| 357 | 495 | 138 | 0 | 137 | `POST /api/internal/web-vitals` | +| **573** | 716 | 143 | 14.4 KB | 153 | `GET /api/gamification/achievements` | +| **573** | 977 | 405 | 8.3 KB | 411 | `GET /api/analytics?slice=summaries` | +| **573** | 713 | 140 | 0 | 147 | `GET /api/medications` | +| **573** | 722 | 149 | 0.1 KB | 107 | `GET /api/workouts?limit=3` | +| **573** | **7240** | **6667** | 10.4 KB | **6562** | **`GET /api/analytics`** (thick) | +| **573** | 1088 | 515 | 1.0 KB | 375 | `GET /api/dashboard/widgets` | +| **573** | 1109 | 536 | 21.8 KB | 378 | `GET /api/mood/analytics` *(call 1, page.tsx)* | +| **573** | 1109 | 536 | 0.1 KB | 373 | `GET /api/feature-flags` | +| **573** | 1169 | 596 | 0.1 KB | 199 | `GET /api/bugreport/status` | +| 983 | 1175 | 192 | 1.4 KB | 85 | `GET /api/medications/intake?scope=compliance&days=30` | +| **1439** | **7290** | **5851** | 2.5 KB | **5855** | `GET /api/measurements?type=WEIGHT&…source=rollup` | +| **1439** | **7288** | **5849** | 2.5 KB | **5854** | `GET /api/measurements?type=WEIGHT&…source=rollup` *(dup)* | +| **1439** | **7295** | **5855** | 3.0 KB | **5858** | `GET /api/measurements?type=BLOOD_PRESSURE_SYS&…source=rollup` | +| **1440** | **7295** | **5855** | 3.0 KB | **5858** | `GET /api/measurements?type=BLOOD_PRESSURE_DIA&…source=rollup` | +| **1440** | **7295** | **5855** | 2.6 KB | **5858** | `GET /api/measurements?type=PULSE&…source=rollup` | +| **1440** | **7355** | **5915** | 2.5 KB | **90** | `GET /api/measurements?type=BODY_FAT&…source=rollup` | +| **1440** | **7355** | **5915** | 21.8 KB | **40** | `GET /api/mood/analytics` *(call 2, mood-chart.tsx)* | +| 5356 | 7354 | 1999 | 0.3 KB | 36 | `GET /api/version` | + +**Three waves clearly emerge**: +* **Wave A (start 356)** — auth shell / cookie probe / web-vitals + beacons. Fires before page hydration. +* **Wave B (start 573, +217 ms)** — 9 requests, all useQuery hooks in + `src/app/page.tsx` plus the auth-context-level `bugreport/status` + + `feature-flags`. They all start at the same instant (TanStack + parallel-mounts every `useQuery` whose `enabled` predicate flips + true together). +* **Wave C (start 1439-1440, +866 ms after Wave B)** — 7 requests, + 6× chart-tile `/api/measurements?…source=rollup` calls (one per + type from `health-chart.tsx:604`) **plus a second + `/api/mood/analytics`** that did not exist when the dashboard was + on a single useAnalytics hook. + +The **6 Wave-C measurements requests all finish within 65 ms of each +other** (7288–7355 ms) — i.e. they all unblock at the moment thick +`/api/analytics` releases its share of the Prisma connection pool. + +--- + +## 3 — WARM mount (60 s cache hits) + +| start | dur | URL | +|---:|---:|---| +| 197 | 47 | `/api/analytics` (thick) | +| 197 | 10 | `/api/analytics?slice=summaries` | +| 197 | 47 | `/api/mood/analytics` (page) | +| 308 | 20–26 | every `/api/measurements?…source=rollup` | +| 610 | 13 | `/api/mood/analytics` (mood-chart, dup) | + +Total wall-clock 8.1 s including 8 s settle, real activity ends at +~610 ms. The 60 s LRU on `caches.analytics` / `caches.moodAnalytics` +collapses the entire fan-out to ~50 ms each. **Cold→warm delta +≈ 6.7 s** — the entire mount-time cost is in the cold thick analytics ++ pool-blocked chart waterfall. + +--- + +## 4 — The three actual bottlenecks (root cause, not symptom) + +### B1 — `/api/analytics` thick **monopolises the Prisma pool for 6.5 s** + +`buildAnalyticsResponse` (`src/app/api/analytics/route.ts:161`) +fan-out: +* 15× `fetchMeasurementSeriesChunked` in parallel + (`route.ts:261-269`, capped to a trailing 425-day window) — each + paginates `prisma.measurement.findMany` at chunks of 5 000 rows. +* `probeRollupCoverage` + 3 fast-path branches + (`bp_in_target` / `correlations` / `healthScore`). Coverage was empty + in our fixture (`row_count: 1490, path: "live"`) so all three + branches fell to the live SQL fallback — production logs + (`v1.4.38.7`-era) show this is also the common case for Marc when + the `isFullyCovered` gate is false (see § 4 of the v1.4.38 perf + audit). + +Server logs: +``` +GET /api/analytics 200 in 6.5s (next.js: 2ms, proxy.ts: 6ms, application-code: 6.5s) +``` + +All 6.5 s land inside `application-code` — pure Prisma DB time. While +this single request runs it is holding ≥ 8 of the 10 pool slots +(15 chunked findMany + 3 fast-paths fire in parallel via +`Promise.all`); every other useQuery hook in Wave B + every +Wave-C `/api/measurements` request has to **queue behind it**. + +### B2 — Default Prisma `pg.Pool` ceiling = **10 connections** + +`src/lib/db.ts:8-13`: +```ts +function createPrismaClient() { + const adapter = new PrismaPg({ + connectionString: process.env.DATABASE_URL!, + }); + return new PrismaClient({ adapter }); +} +``` + +No `max` override is passed → `pg.Pool` keeps its library default +of 10. With B1 holding 8+ slots, Wave-C's 6 rollup chart reads +(individually ≤ 20 ms of `application-code` per the warm trace) +all stall for the duration of thick analytics. Their `application-code: +11ms` next to `proxy.ts: 1009ms` in the dev log is what a +pool-starved cold path looks like. + +### B3 — Duplicate `/api/mood/analytics` call from two different + TanStack query keys + +* `src/app/page.tsx:291` uses `queryKey: queryKeys.moodAnalytics()` + → `["mood-analytics"]` (defined in `src/lib/query-keys.ts:30`). +* `src/components/charts/mood-chart.tsx:316` uses + `queryKey: ["mood-chart-data"]`. + +Both `queryFn`s `fetch("/api/mood/analytics")` and unwrap +`json.data`. TanStack Query treats different keys as separate +queries → **the dashboard cold mount issues two identical requests**. +The second is part of Wave C; on warm mount it still fires (cache +key 2 is empty) and lands at start+610 ms. This is the same class of +silent cache poisoning Marc's memory dual-location notes flagged +for HealthLog conventions (`feedback_react_query_key_collision.md`). +At 22 KB body × 2 = 44 KB doubled mobile data + one extra connection +out of the already-saturated pool. + +--- + +## 5 — Why Marc's UX matches the trace exactly + +* **Mood + medication tiles paint first** — `/api/mood/analytics` + (first call) returns at +1109 ms, `/api/medications` at +713 ms. + Both are early Wave-B with low TTFB. ✓ matches Marc's report. +* **"All other tiles arrive faster than before but in a single + burst"** — the tile-strip headlines paint when the slim + `/api/analytics?slice=summaries` lands at +977 ms (well-handled + by v1.4.39.2's slim/thick split). But the chart row beneath the + tile strip waits for the 6 individual `/api/measurements?source=rollup` + calls, which **all unblock simultaneously when thick `/api/analytics` + releases the pool at ~7.3 s**. ✓ "single burst". +* **"Etwas blockiert noch"** — yes, thick `/api/analytics` is the + block. The slim/thick frontend split was correct but the **server- + side resource contention** wasn't decoupled. ✓. +* **"Mobile especially noticeable"** — mobile chart cards are bigger + vertically, so the empty placeholder area below the painted tile + strip is more obvious. Trace timings are identical (mobile + screenshots at 1000/1500/2500/4000 ms show empty chart row; only + at 6000-8000 ms do the charts populate). + +--- + +## 6 — Recommended fixes ranked by impact / effort + +### F1 — Cap thick `/api/analytics` Prisma concurrency to ≤ 4 (S, blocking-fix) + +Wrap the `types.map(t => fetchMeasurementSeriesChunked(…))` fan-out +(`route.ts:261-269`) in a bounded-concurrency helper (e.g. `p-limit` +with limit 4). Same total work but **at most 4 pool slots held by +analytics at any moment**, leaving ≥ 6 slots for the 6 Wave-C chart +reads. Expected impact: **the chart-tile burst moves from +7.3 s +to ~+1.6 s** (their individual `application-code` is 11-15 ms; +they'd start finishing as soon as the first analytics chunk +completes a slot). **Highest impact for lowest effort.** +* Files: `src/app/api/analytics/route.ts`. +* Dev: ~30 min including a unit test that asserts ≤ N inflight. +* Risk: trivial; thick analytics gets ~10-15 % slower in absolute + wall-clock (10 chunks at concurrency 4 = 3 batches instead of 1) + but it was already cache-wrapped at 60 s so the user-visible cost + is the cold mount only, and the chart row paints 5 s earlier in + exchange. + +### F2 — Raise `pg.Pool` `max` to ≥ 20 (XS, complementary) + +Pass `max: 20` (or env-driven) to `new PrismaPg({ connectionString, +max: 20 })`. Coolify's Postgres container has ample room (default +`max_connections` 100), and the current 10-slot ceiling is the +library default, not a tuned value. +* Files: `src/lib/db.ts`. +* Dev: ~5 min. +* Risk: nil (the upper bound is set by Postgres' `max_connections`, + well above 20). Complements F1 — even with F1 the pool ceiling + remains tight for any second concurrent power-user. + +### F3 — Unify mood-analytics queryKey (XS, dedup) + +`src/components/charts/mood-chart.tsx:316` should use +`queryKey: queryKeys.moodAnalytics()` (the canonical key in +`src/lib/query-keys.ts:30`) instead of its own `["mood-chart-data"]`. +Both share the same endpoint + same envelope-unwrap pattern; the +shape difference (`MoodAnalyticsData` vs `{ entries, summary }`) is a +type-narrowing concern, not a cache concern. Both already pull the +same JSON. +* Files: `src/components/charts/mood-chart.tsx`, + `src/lib/query-keys.ts` (if a typed-key variant is preferred). +* Dev: ~15 min including the e2e regression that asserts only one + network call per mount. +* Risk: nil — TanStack will warn if `queryFn` shapes diverge in dev, + and the test catches it. + +### F4 — Move 6 chart-tile rollup reads into the analytics envelope (M, eliminates Wave C entirely) + +The 6 `/api/measurements?…&aggregate=daily&source=rollup` calls each +ask for one type's last-30-day daily aggregate. The slim analytics +slice already pulls daily DAY-bucket data +(`computeSummariesSlice` → `summaries-slice.ts:309`). Extend the slim +envelope with a `daily30dByType: Record` +field — the buckets are already loaded in-memory, so no extra DB +hit. `health-chart.tsx` consumes from the existing `data` prop +instead of issuing per-type fetches. +* Files: `src/lib/analytics/summaries-slice.ts`, + `src/lib/analytics/merge-slim-thick.ts`, + `src/components/charts/health-chart.tsx`, + `src/app/page.tsx` (pass `daily30dByType` to each chart). +* Dev: ~half day. +* Risk: slim envelope payload grows ~12-15 KB per user (8 types × + ~30 points × ~50 B), well below the 50 KB ceiling for "slim". + Eliminates 6 request round-trips entirely and **removes the source + of Wave C**. + +### F5 — Defer `version-poller` initial check until idle (XS, UX polish) + +`src/components/version-poller.tsx:109` schedules the first poll at ++5 000 ms. In our trace the call **landed at +5356 ms but waited +1999 ms in the pool** (it competes for the same Prisma client even +though `/api/version` doesn't touch the DB — but middleware does). +Move the initial check to `requestIdleCallback` with a 10 s fallback +so it doesn't fire during the cold-mount network burst. +* Files: `src/components/version-poller.tsx`. +* Dev: ~10 min. +* Risk: nil — version-poller is a "every 60 s" deploy-mismatch heal + loop; delaying its first run by ≤ 10 s doesn't affect correctness. + +--- + +## 7 — Reproduction commands + +```bash +# 1. Start dev server (uses dev DB at localhost:5432) +cd /Users/marc/Projects/HealthLog +PORT=3000 pnpm dev > /tmp/healthlog-dev.log 2>&1 & + +# 2. Seed Marc-shaped fixture for the e2e-tester user (8 types, +# 82 490 measurements). The seed nukes ALL prior data for that +# one user and writes ~80 k rows in ~10 s. +# (The seed JS is captured below in §8 — copy to project root +# because the script `require`s `pg` from node_modules.) +cat > seed-heavy-fixture.tmp.js <<'EOF' +... (see §8) +EOF +node seed-heavy-fixture.tmp.js +rm seed-heavy-fixture.tmp.js + +# 3. Run Playwright trace (cold + warm). Uses the e2e storageState +# cookie jar already on disk. +cd /Users/marc/.claude/skills/playwright-skill +node run.js /tmp/playwright-dashboard-trace.js + +# 4. Inspect the trace JSON +node -e "const d=require('/tmp/playwright-dashboard-trace.json'); \ + d.cold.requests.sort((a,b)=>a.startedAt-b.startedAt) \ + .forEach(r=>console.log([Math.round(r.startedAt), \ + Math.round(r.duration_ms), Math.round((r.timing?.responseStart \ + - r.timing?.requestStart)||0), r.url].join('\t')))" +``` + +The trace script `/tmp/playwright-dashboard-trace.js` is the +empirical capture tool — captures `request.timing()` per `/api/*`, +two passes (cold + warm), JSON to `/tmp/playwright-dashboard-trace.json`. + +--- + +## 8 — Reproducible seed JS (for §7 step 2) + +The seed used to produce this trace (drop into project root so `pg` +resolves from `node_modules`): + +```js +// drop into /Users/marc/Projects/HealthLog/seed-heavy-fixture.tmp.js +const { Pool } = require('pg'); +const pool = new Pool({ + connectionString: process.env.DATABASE_URL + || 'postgresql://healthlog:healthlog@localhost:5432/healthlog?schema=public', +}); +const TYPES = [ + ['WEIGHT', 1, 85, 1.5, 'kg'], + ['BLOOD_PRESSURE_SYS', 2, 132, 8, 'mmHg'], + ['BLOOD_PRESSURE_DIA', 2, 82, 6, 'mmHg'], + ['PULSE', 2, 68, 10, 'bpm'], + ['BODY_FAT', 1, 22, 0.8, '%'], + ['SLEEP_DURATION', 1, 450, 60, 'minutes'], + ['ACTIVITY_STEPS', 96, 80, 60, 'steps'], + ['OXYGEN_SATURATION', 8, 97, 0.7, '%'], +]; +const DAYS = 730; +function cuid(){const c="abcdefghijklmnopqrstuvwxyz0123456789";let i="c";for(let n=0;n<24;n++)i+=c[Math.floor(Math.random()*c.length)];return i;} +function randn(m,s){const u1=Math.random(),u2=Math.random();return m+s*Math.sqrt(-2*Math.log(u1))*Math.cos(2*Math.PI*u2);} +(async () => { + const client = await pool.connect(); + try { + const { rows } = await client.query( + "SELECT id FROM users WHERE username='e2e-tester'"); + const userId = rows[0].id; + await client.query('BEGIN'); + await client.query('DELETE FROM measurements WHERE user_id=$1', [userId]); + await client.query('DELETE FROM measurement_rollups WHERE user_id=$1', [userId]); + await client.query('DELETE FROM mood_entries WHERE user_id=$1', [userId]); + const now = new Date(); + for (const [type, perDay, mean, sd, unit] of TYPES) { + const rowsBatch = []; + for (let d = 0; d < DAYS; d++) { + const day = new Date(now); day.setDate(day.getDate() - d); day.setHours(0,0,0,0); + for (let i = 0; i < perDay; i++) { + const measuredAt = new Date(day.getTime() + (i/perDay)*86400000 + Math.random()*300000); + let v = randn(mean, sd); + v = (type === 'ACTIVITY_STEPS' || type === 'SLEEP_DURATION') + ? Math.max(0, Math.round(v)) + : Math.round(v*10)/10; + rowsBatch.push({ measuredAt, value: v }); + } + } + for (let i = 0; i < rowsBatch.length; i += 800) { + const slice = rowsBatch.slice(i, i + 800); + const values = [], params = []; + let p = 1; + for (const r of slice) { + values.push(`($${p++},$${p++},$${p++}::measurement_type,$${p++},$${p++},$${p++},$${p++},$${p++})`); + params.push(cuid(), userId, type, r.value, unit, r.measuredAt, r.measuredAt, r.measuredAt); + } + await client.query( + `INSERT INTO measurements (id,user_id,type,value,unit,measured_at,created_at,updated_at) + VALUES ${values.join(',')}`, params); + } + } + const moodMap = { 1:'TERRIBLE', 2:'BAD', 3:'NEUTRAL', 4:'GOOD', 5:'GREAT' }; + const moodRows = []; + for (let d = 0; d < 500; d++) { + const day = new Date(now); day.setDate(day.getDate() - d); + const score = Math.max(1, Math.min(5, Math.round(randn(3.2, 0.9)))); + moodRows.push([cuid(), userId, day.toISOString().split('T')[0], score, day.toISOString()]); + } + for (let i = 0; i < moodRows.length; i += 200) { + const slice = moodRows.slice(i, i + 200); + const values = [], params = []; + let p = 1; + for (const r of slice) { + values.push(`($${p++},$${p++},$${p++},$${p++},$${p++},$${p++},NOW(),NOW())`); + params.push(r[0], r[1], r[2], moodMap[r[3]], r[3], r[4]); + } + await client.query( + `INSERT INTO mood_entries (id,user_id,date,mood,score,mood_logged_at,created_at,updated_at) + VALUES ${values.join(',')}`, params); + } + await client.query('COMMIT'); + console.log('seeded.'); + } catch (e) { await client.query('ROLLBACK').catch(()=>{}); throw e; } + finally { client.release(); await pool.end(); } +})().catch(e => { console.error(e); process.exit(1); }); +``` + +--- + +## 9 — Cross-references + +* The v1.4.38 perf audit + (`.planning/round-v1438-perf-analysis.md`) §1 § 4 already + identified `fetchMeasurementSeriesChunked × 15` as the dominant + cold path and `isFullyCovered` as the gate keeping the fast-path + on `path:"live"`. This trace **confirms empirically** that the + pool-saturation effect of that fan-out — not the wall-clock of + thick analytics itself — is what blocks every other useQuery on + the dashboard. +* `feedback_react_query_key_collision.md` (memory note) — the + duplicate mood-analytics call is a fresh instance of the same + pattern Marc has flagged before. +* The v1.4.39.2 slim/thick split was a **correct frontend + decoupling**, but the trace shows that without **server-side + Prisma fan-out throttling** (F1) and pool sizing (F2) the + improvement only buys the tile-strip ~5-6 s of head-start while + the chart row still arrives in a single burst at thick-analytics- + release time. diff --git a/CHANGELOG.md b/CHANGELOG.md index cf45a2c64..bd8002438 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## [1.4.39.4] — 2026-05-21 — Dashboard symmetry for daily-schedule intake projection + +v1.4.39.3 fixed `/api/medications/intake?scope=today` to project active schedules through the new `expandTodayIntakes` helper and idempotently backfill missing rows for daily meds (`daysOfWeek: null`). `/api/dashboard/summary` reads the same today-window for its compliance tile but had no projection step, so the iOS Dashboard tile fell to "Heute nichts geplant" even when the intake route surfaced the same meds correctly. + +### Fixed + +- **`/api/dashboard/summary` compliance tile** now mirrors the intake route's projection. Reuses the canonical `expandTodayIntakes` helper plus an idempotent `createMany({ skipDuplicates: true })` that survives a concurrent intake-route hit racing the same `(userId, medicationId, scheduledFor, REMINDER)` row in before the existence probe converges. + +### Operator notes + +- No migration. No env-var change. No API contract break. +- Daily meds with `daysOfWeek: null` (DB convention for "every day") now surface in the iOS Dashboard tile + Erfassen sheet the moment the user opens the app, instead of waiting for the reminder worker to enter RED phase at the end of the dose window. + ## [1.4.39.3] — 2026-05-21 — Dashboard slim/thick merge robustness and list-page value precision The post-deploy CI for v1.4.39.2 surfaced eight e2e failures across two diff --git a/package.json b/package.json index 3f41bffba..af725755c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.4.39.3", + "version": "1.4.39.4", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "AGPL-3.0-only", "homepage": "https://healthlog.dev", diff --git a/prisma/migrations/0073_v1439_medication_intake_unique_per_source/migration.sql b/prisma/migrations/0073_v1439_medication_intake_unique_per_source/migration.sql new file mode 100644 index 000000000..df174c3f3 --- /dev/null +++ b/prisma/migrations/0073_v1439_medication_intake_unique_per_source/migration.sql @@ -0,0 +1,57 @@ +-- v1.4.39 W-SERVER-FIX-2 — unique constraint on +-- (user_id, medication_id, scheduled_for, source) for +-- medication_intake_events so the projection-then-backfill code path +-- can't mint duplicate REMINDER rows under a concurrent two-route +-- hit. +-- +-- Background. Two read routes now project active schedules through +-- `expandTodayIntakes` and idempotently `createMany`-backfill any +-- missing rows: +-- +-- GET /api/medications/intake?scope=today +-- GET /api/dashboard/summary +-- +-- The iOS Dashboard mounts both at roughly the same wall-clock when +-- the compliance ring + Erfassen sheet hydrate together. Pre-fix the +-- table carried only an INDEX on `(user_id, medication_id, +-- scheduled_for)`, NOT a UNIQUE constraint, so both routes could pass +-- their existence probes simultaneously and `createMany` the same +-- `(medicationId, scheduledFor, REMINDER)` row twice — corrupting the +-- compliance bucket counts on the next read. +-- +-- `source` is part of the key so the reminder worker's REMINDER row +-- doesn't collide with manual user inserts (WEB / TELEGRAM / IMPORT) +-- on the same slot. Distinct intake sources at the same scheduled +-- instant are a legitimate shape — e.g. a Telegram-bot logged take + +-- a Web-UI correction on the same dose. +-- +-- The query-side `(user_id, medication_id, scheduled_for)` index +-- stays in place — most reads filter on those three columns without +-- caring about `source`, so the legacy index keeps its read-path +-- value even after the unique constraint creates its own +-- (user_id, medication_id, scheduled_for, source) covering index. +-- +-- Production data check. The reminder worker is the only writer that +-- ever produces REMINDER-source rows pre-this-fix; it queries pending +-- rows by `(medicationId, scheduledFor)` before each insert so a +-- self-double-mint is structurally impossible. The new projection +-- code paths (intake route + dashboard route) are the only new +-- sources of potential duplicates, and both ship with the +-- `skipDuplicates: true` flag alongside this constraint. If +-- `migrate deploy` reports a unique-constraint violation, a follow-up +-- one-shot data-cleanup pass would dedup the offending rows by +-- (user_id, medication_id, scheduled_for, source) keeping the +-- lowest-id row; that cleanup is intentionally NOT bundled here so +-- the migration stays additive + reviewable. +-- +-- Additive only. IF NOT EXISTS / EXCEPTION-on-duplicate guards +-- mirror the patterns in 0067 / 0071. + +DO $$ BEGIN + ALTER TABLE "medication_intake_events" + ADD CONSTRAINT "medication_intake_events_user_id_medication_id_scheduled_for_source_key" + UNIQUE ("user_id", "medication_id", "scheduled_for", "source"); +EXCEPTION + WHEN duplicate_object THEN NULL; + WHEN duplicate_table THEN NULL; +END $$; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 328e04373..b0509aee8 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -838,6 +838,15 @@ model MedicationIntakeEvent { user User @relation(fields: [userId], references: [id], onDelete: Cascade) medication Medication @relation(fields: [medicationId], references: [id], onDelete: Cascade) + /// v1.4.39 W-SERVER-FIX-2 — both the intake route and the dashboard + /// summary route project active schedules through `expandTodayIntakes` + /// and idempotently `createMany`-backfill missing rows. Without a + /// unique constraint the two reads could race a duplicate REMINDER + /// row in for the same `(medicationId, scheduledFor)` between the + /// existence probe and the insert. `source` is in the key so the + /// reminder worker (REMINDER) doesn't collide with manual user + /// inserts (WEB / TELEGRAM / IMPORT) on the same slot. + @@unique([userId, medicationId, scheduledFor, source]) @@index([userId, medicationId, scheduledFor]) @@map("medication_intake_events") } diff --git a/src/app/api/dashboard/summary/__tests__/route.test.ts b/src/app/api/dashboard/summary/__tests__/route.test.ts index 105c3420d..07d6b4717 100644 --- a/src/app/api/dashboard/summary/__tests__/route.test.ts +++ b/src/app/api/dashboard/summary/__tests__/route.test.ts @@ -4,7 +4,20 @@ import { NextRequest } from "next/server"; vi.mock("@/lib/db", () => ({ prisma: { measurement: { groupBy: vi.fn() }, - medicationIntakeEvent: { findMany: vi.fn() }, + medicationIntakeEvent: { + findMany: vi.fn(), + // v1.4.39 W-SERVER-FIX-2 — the dashboard route now backfills + // missing today-rows for daily schedules (parity with the + // intake route's `expandTodayIntakes` projection) so the iOS + // Dashboard compliance tile leaves "Heute nichts geplant" the + // moment a daily med is configured. + createMany: vi.fn(), + }, + medication: { + // v1.4.39 W-SERVER-FIX-2 — projection source for the today's- + // intakes backfill. + findMany: vi.fn(), + }, // v1.4.38 W-F — the route now uses `$queryRaw` for the per-type // latest reading, the 7-day rollup-bucket sparkline, and the // 365-day distinct-day streak set. Three raw calls per request, @@ -72,6 +85,13 @@ beforeEach(() => { vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue( [] as never, ); + // v1.4.39 W-SERVER-FIX-2 — default the projection prisma reads to + // "no active medications" so the legacy tests stay in their original + // branch (empty today bucket, no backfill firing). + vi.mocked(prisma.medication.findMany).mockResolvedValue([] as never); + vi.mocked(prisma.medicationIntakeEvent.createMany).mockResolvedValue({ + count: 0, + } as never); }); describe("GET /api/dashboard/summary", () => { @@ -293,6 +313,140 @@ describe("GET /api/dashboard/summary", () => { expect(steps?.sparkline).toEqual([8000]); }); + it("backfills today's intakes for daily meds (v1.4.39 W-SERVER-FIX-2)", async () => { + // The dashboard compliance tile is fed by `todaysIntakes`, the + // same window the intake route covers. Pre-fix this only read the + // pre-existing `MedicationIntakeEvent` rows, so a daily med + // (`daysOfWeek: null`) had `scheduledToday=0` until the reminder + // worker minted the RED-phase row at the end of the dose window. + // Post-fix the route projects active schedules through + // `expandTodayIntakes` + idempotently backfills missing rows, so + // the iOS Dashboard tile leaves "Heute nichts geplant" the moment + // a daily med is configured. + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.medication.findMany).mockResolvedValue([ + { + id: "med-ramipril", + schedules: [ + { + id: "sched-morgens", + medicationId: "med-ramipril", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }, + ], + }, + ] as never); + // The dashboard route now issues three findMany calls in the + // today path: + // 1. existence probe (no `OR`) → empty (no rows yet) + // 2. final read after backfill (no `OR`) → one minted row + // 3. streak fetch (with `OR`) → empty + vi.mocked(prisma.medicationIntakeEvent.findMany).mockImplementation((( + args: unknown, + ) => { + const a = args as { + where: { OR?: unknown }; + select?: { id?: boolean }; + }; + if (a.where.OR) return Promise.resolve([]) as never; + // No `id` field on the existence-probe `select` → the route + // only asked for `(medicationId, scheduledFor)`. + if (!a.select?.id) return Promise.resolve([]) as never; + return Promise.resolve([ + { + id: "e-new", + takenAt: null, + skipped: false, + }, + ]) as never; + }) as never); + vi.mocked(prisma.medicationIntakeEvent.createMany).mockResolvedValue({ + count: 1, + } as never); + + const res = await callGet(makeReq()); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { compliance: { scheduledToday: number; takenToday: number } }; + }; + expect(body.data.compliance.scheduledToday).toBe(1); + expect(body.data.compliance.takenToday).toBe(0); + expect(prisma.medicationIntakeEvent.createMany).toHaveBeenCalledTimes(1); + // Race-defense backstop: the createMany call must pass + // `skipDuplicates: true` so a concurrent intake-route hit can't + // race a duplicate row in before the existence probe converges. + const createManyArgs = vi.mocked(prisma.medicationIntakeEvent.createMany) + .mock.calls[0][0] as { + data: Array<{ + userId: string; + medicationId: string; + skipped: boolean; + source: string; + }>; + skipDuplicates?: boolean; + }; + expect(createManyArgs.skipDuplicates).toBe(true); + expect(createManyArgs.data).toEqual([ + expect.objectContaining({ + userId: "user-1", + medicationId: "med-ramipril", + skipped: false, + source: "REMINDER", + }), + ]); + }); + + it("is idempotent on a second pass (v1.4.39 W-SERVER-FIX-2)", async () => { + // When the existence probe already sees a row for the projected + // slot (e.g. the intake route or the reminder worker minted it + // first), the dashboard route's backfill `createMany` must not + // fire — the route simply re-reads + returns. + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + const scheduledFor = new Date("2026-05-21T05:00:00.000Z"); + vi.mocked(prisma.medication.findMany).mockResolvedValue([ + { + id: "med-ramipril", + schedules: [ + { + id: "sched-morgens", + medicationId: "med-ramipril", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }, + ], + }, + ] as never); + vi.mocked(prisma.medicationIntakeEvent.findMany).mockImplementation((( + args: unknown, + ) => { + const a = args as { + where: { OR?: unknown }; + select?: { id?: boolean }; + }; + if (a.where.OR) return Promise.resolve([]) as never; + if (!a.select?.id) { + // Existence probe — the projected slot already exists. + return Promise.resolve([ + { medicationId: "med-ramipril", scheduledFor }, + ]) as never; + } + return Promise.resolve([ + { id: "e-existing", takenAt: null, skipped: false }, + ]) as never; + }) as never); + + const res = await callGet(makeReq()); + expect(res.status).toBe(200); + expect(prisma.medicationIntakeEvent.createMany).not.toHaveBeenCalled(); + const body = (await res.json()) as { + data: { compliance: { scheduledToday: number; takenToday: number } }; + }; + expect(body.data.compliance.scheduledToday).toBe(1); + }); + it("computes intake compliance for today", async () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(prisma.medicationIntakeEvent.findMany).mockImplementation((( diff --git a/src/app/api/dashboard/summary/route.ts b/src/app/api/dashboard/summary/route.ts index 458330162..bf4233277 100644 --- a/src/app/api/dashboard/summary/route.ts +++ b/src/app/api/dashboard/summary/route.ts @@ -48,6 +48,7 @@ import { getServerTranslator } from "@/lib/i18n/server-translator"; import { defaultLocale, type Locale } from "@/lib/i18n/config"; import { userDayKey, DEFAULT_TIMEZONE } from "@/lib/tz/resolver"; import { cached, caches, type ServerCache } from "@/lib/cache/server-cache"; +import { expandTodayIntakes } from "@/lib/medication-schedule"; const SPARK_DAYS = 7; const STREAK_WINDOW_DAYS = 365; @@ -268,6 +269,98 @@ function buildContext( return { greetingName, locale }; } +/** + * v1.4.39 W-SERVER-FIX-2 — project today's active schedules through + * the canonical `expandTodayIntakes` helper, idempotently backfill any + * missing `MedicationIntakeEvent` rows, and re-read the today-window + * so the caller sees both pre-existing rows + the freshly minted ones. + * + * Mirrors the post-fix behaviour of `/api/medications/intake?scope= + * today` so the iOS Dashboard tile (fed by this route) and the iOS + * Erfassen sheet (fed by the intake route) converge on the same row + * set the moment a daily med becomes active. Without this projection + * the dashboard route returned `[]` for daily meds (`daysOfWeek = null`) + * even after the intake route's projection landed, leaving the iOS + * Dashboard tile in its "Heute nichts geplant" empty state all + * morning. + * + * `skipDuplicates: true` guards against a concurrent intake-route hit + * minting the same `(userId, medicationId, scheduledFor, REMINDER)` + * row in the gap between the existence probe and the `createMany`. + * The schema-level `@@unique` is the structural backstop; this flag + * is the defense-in-depth that keeps the route returning 2xx instead + * of bubbling a constraint violation during the race window. + */ +async function projectAndReadTodaysIntakes( + userId: string, + userTz: string, + todayStart: Date, + todayEnd: Date, +): Promise> { + const activeMedications = await prisma.medication.findMany({ + where: { userId, active: true }, + select: { + id: true, + schedules: { + select: { + id: true, + medicationId: true, + windowStart: true, + windowEnd: true, + daysOfWeek: true, + }, + }, + }, + }); + const projected = expandTodayIntakes( + activeMedications.flatMap((m) => m.schedules), + new Date(), + userTz, + ); + + if (projected.length > 0) { + const existing = await prisma.medicationIntakeEvent.findMany({ + where: { + userId, + scheduledFor: { gte: todayStart, lt: todayEnd }, + }, + select: { medicationId: true, scheduledFor: true }, + }); + const existingKey = new Set( + existing.map((e) => `${e.medicationId}|${e.scheduledFor.toISOString()}`), + ); + const missing = projected.filter( + (p) => + !existingKey.has(`${p.medicationId}|${p.scheduledFor.toISOString()}`), + ); + if (missing.length > 0) { + await prisma.medicationIntakeEvent.createMany({ + data: missing.map((m) => ({ + userId, + medicationId: m.medicationId, + scheduledFor: m.scheduledFor, + takenAt: null, + skipped: false, + // `REMINDER` mirrors the intake-route projection — the same + // `IntakeSource` literal the reminder worker uses for + // RED-phase rows. Doctor-report + analytics filters stay + // byte-stable across both code paths. + source: "REMINDER", + })), + skipDuplicates: true, + }); + } + } + + return prisma.medicationIntakeEvent.findMany({ + where: { + userId, + scheduledFor: { gte: todayStart, lt: todayEnd }, + }, + select: { id: true, takenAt: true, skipped: true }, + }); +} + async function buildDashboardSummary( userId: string, userTz: string, @@ -352,14 +445,18 @@ async function buildDashboardSummary( _max: { measuredAt: true }, }), ), + // v1.4.39 W-SERVER-FIX-2 — the dashboard compliance tile pulled + // `MedicationIntakeEvent` rows for the today-window only. Daily + // schedules (`daysOfWeek: null`) had no row to read until the + // reminder worker entered the RED phase at the end of the dose + // window, so the iOS Dashboard tile fell to "Heute nichts geplant" + // even when the intake route (post-`expandTodayIntakes` fix) was + // already projecting + backfilling correctly. Re-uses the same + // helper + idempotent `createMany` (with `skipDuplicates: true` + // so a concurrent intake-route hit can't race a duplicate row in + // before the existence probe converges). time("todaysIntakes", () => - prisma.medicationIntakeEvent.findMany({ - where: { - userId, - scheduledFor: { gte: todayStart, lt: todayEnd }, - }, - select: { id: true, takenAt: true, skipped: true }, - }), + projectAndReadTodaysIntakes(userId, userTz, todayStart, todayEnd), ), time("streakIntakes", () => prisma.medicationIntakeEvent.findMany({ diff --git a/src/app/api/medications/intake/__tests__/route.test.ts b/src/app/api/medications/intake/__tests__/route.test.ts index 3c99a7b7f..f1fc4fc75 100644 --- a/src/app/api/medications/intake/__tests__/route.test.ts +++ b/src/app/api/medications/intake/__tests__/route.test.ts @@ -7,8 +7,17 @@ vi.mock("@/lib/db", () => ({ findMany: vi.fn(), findUnique: vi.fn(), update: vi.fn(), + // v1.4.39 W-SERVER-FIX — `scope=today` now backfills missing + // rows for schedules whose window opens today (covers daily + // meds with `daysOfWeek: null` whose reminder hasn't fired yet). + createMany: vi.fn(), + }, + medication: { + update: vi.fn(), + // v1.4.39 W-SERVER-FIX — today's projection reads active meds + + // their schedules to know what to backfill. + findMany: vi.fn(), }, - medication: { update: vi.fn() }, medicationComplianceRollup: { findFirst: vi.fn(), findMany: vi.fn(), @@ -65,6 +74,13 @@ beforeEach(() => { vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue( [] as never, ); + // v1.4.39 W-SERVER-FIX — default the today-projection prisma reads + // to "no active medications" so existing compliance / POST tests + // exercise their original paths without an unmocked-call failure. + vi.mocked(prisma.medication.findMany).mockResolvedValue([] as never); + vi.mocked(prisma.medicationIntakeEvent.createMany).mockResolvedValue({ + count: 0, + } as never); // v1.4.39 W-MED — default the coverage probe to "uncovered" so the // legacy compliance test still exercises the live-fallback branch // and finds the legacy mocked intake events. @@ -123,6 +139,112 @@ describe("GET /api/medications/intake", () => { expect(body.data[0].status).toBe("pending"); }); + it("backfills missing today rows for daily meds (daysOfWeek=null)", async () => { + // v1.4.39 W-SERVER-FIX regression — the operator's Ramipril + // (Morgens) ships `daysOfWeek: null` in the DB ("every day" per + // schema). Pre-fix, the endpoint returned `[]` until the reminder + // worker entered RED phase. Post-fix, the GET projects the schedule + // and idempotently mints any missing rows so the iOS Dashboard + + // "Erfassen" sheet have an intake row to mark. + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.medication.findMany).mockResolvedValue([ + { + id: "med-ramipril", + schedules: [ + { + id: "sched-morgens", + medicationId: "med-ramipril", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }, + ], + }, + ] as never); + // First findMany call (pre-backfill existence probe) returns empty; + // second call (post-backfill list) returns the freshly minted row. + vi.mocked(prisma.medicationIntakeEvent.findMany) + .mockResolvedValueOnce([] as never) + .mockResolvedValueOnce([ + { + id: "e-new", + medicationId: "med-ramipril", + scheduledFor: new Date("2026-05-21T05:00:00.000Z"), + takenAt: null, + skipped: false, + medication: { id: "med-ramipril", snoozedUntil: null }, + }, + ] as never); + vi.mocked(prisma.medicationIntakeEvent.createMany).mockResolvedValue({ + count: 1, + } as never); + + const res = await GET( + new NextRequest("http://localhost/api/medications/intake?scope=today"), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: Array<{ id: string; status: string }>; + }; + expect(body.data).toHaveLength(1); + expect(body.data[0].id).toBe("e-new"); + expect(body.data[0].status).toBe("pending"); + expect(prisma.medicationIntakeEvent.createMany).toHaveBeenCalledTimes(1); + expect( + vi.mocked(prisma.medicationIntakeEvent.createMany).mock.calls[0][0], + ).toMatchObject({ + data: [ + { + userId: "user-1", + medicationId: "med-ramipril", + skipped: false, + source: "REMINDER", + }, + ], + }); + }); + + it("does not double-mint when today's rows already exist", async () => { + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + const scheduledFor = new Date("2026-05-21T05:00:00.000Z"); + vi.mocked(prisma.medication.findMany).mockResolvedValue([ + { + id: "med-ramipril", + schedules: [ + { + id: "sched-morgens", + medicationId: "med-ramipril", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }, + ], + }, + ] as never); + // The existence probe sees a row for the exact projected slot, so + // createMany must not fire. + vi.mocked(prisma.medicationIntakeEvent.findMany) + .mockResolvedValueOnce([ + { medicationId: "med-ramipril", scheduledFor }, + ] as never) + .mockResolvedValueOnce([ + { + id: "e-existing", + medicationId: "med-ramipril", + scheduledFor, + takenAt: null, + skipped: false, + medication: { id: "med-ramipril", snoozedUntil: null }, + }, + ] as never); + + const res = await GET( + new NextRequest("http://localhost/api/medications/intake?scope=today"), + ); + expect(res.status).toBe(200); + expect(prisma.medicationIntakeEvent.createMany).not.toHaveBeenCalled(); + }); + it("returns compliance buckets for the last N days", async () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue([ diff --git a/src/app/api/medications/intake/route.ts b/src/app/api/medications/intake/route.ts index d290166d9..73dc12607 100644 --- a/src/app/api/medications/intake/route.ts +++ b/src/app/api/medications/intake/route.ts @@ -33,6 +33,7 @@ import { enqueueUserMedicationComplianceBackfill, type ComplianceBucket as RollupComplianceBucket, } from "@/lib/medications/compliance-rollups"; +import { expandTodayIntakes } from "@/lib/medication-schedule"; const querySchema = z.object({ scope: z.enum(["today", "compliance"]), @@ -85,6 +86,78 @@ export const GET = apiHandler(async (request: NextRequest) => { if (scope === "today") { const todayStart = startOfDayInTz(new Date(), userTz); const todayEnd = new Date(todayStart.getTime() + 86_400_000); + + // v1.4.39 W-SERVER-FIX — project pending intake rows for every + // active schedule whose window opens today, then idempotently + // backfill any missing rows. Pre-fix the endpoint returned `[]` + // for daily meds (`schedule.daysOfWeek = null` in the DB) until + // the reminder worker entered the RED phase at the end of the + // dose window — leaving the iOS Dashboard tile + the "Erfassen" + // sheet empty for the whole morning. The projection re-uses the + // canonical `parseScheduleRecurrence` (null/empty = every day) + // so the daysOfWeek semantics stay aligned across every reader. + const activeMedications = await prisma.medication.findMany({ + where: { userId: user.id, active: true }, + select: { + id: true, + schedules: { + select: { + id: true, + medicationId: true, + windowStart: true, + windowEnd: true, + daysOfWeek: true, + }, + }, + }, + }); + const projected = expandTodayIntakes( + activeMedications.flatMap((m) => m.schedules), + new Date(), + userTz, + ); + + const existing = await prisma.medicationIntakeEvent.findMany({ + where: { + userId: user.id, + scheduledFor: { gte: todayStart, lt: todayEnd }, + }, + select: { medicationId: true, scheduledFor: true }, + }); + const existingKey = new Set( + existing.map((e) => `${e.medicationId}|${e.scheduledFor.toISOString()}`), + ); + const missing = projected.filter( + (p) => + !existingKey.has(`${p.medicationId}|${p.scheduledFor.toISOString()}`), + ); + if (missing.length > 0) { + await prisma.medicationIntakeEvent.createMany({ + data: missing.map((m) => ({ + userId: user.id, + medicationId: m.medicationId, + scheduledFor: m.scheduledFor, + takenAt: null, + skipped: false, + // `REMINDER` is the same source the reminder worker uses to + // mint RED-phase rows — semantically "the server projected + // this slot before the user logged anything". The IntakeSource + // enum has no separate `SCHEDULER` literal; reusing REMINDER + // keeps the doctor-report + analytics filters byte-stable. + source: "REMINDER", + })), + // v1.4.39 W-SERVER-FIX-2 — paired with the schema-level + // @@unique([userId, medicationId, scheduledFor, source]). A + // concurrent dashboard-summary hit (the second route that now + // runs the same projection) can race a duplicate row in + // between this route's existence probe and createMany; the + // flag tells Postgres to swallow the rejected rows so the + // request still returns 2xx. The unique constraint is the + // structural backstop, this is the defense-in-depth. + skipDuplicates: true, + }); + } + const events = await prisma.medicationIntakeEvent.findMany({ where: { userId: user.id, @@ -96,7 +169,11 @@ export const GET = apiHandler(async (request: NextRequest) => { annotate({ action: { name: "medications.intake.today" }, - meta: { count: events.length }, + meta: { + count: events.length, + projected: projected.length, + backfilled: missing.length, + }, }); return apiSuccess( diff --git a/src/lib/__tests__/medication-schedule.test.ts b/src/lib/__tests__/medication-schedule.test.ts index 9d6154486..8023f4b43 100644 --- a/src/lib/__tests__/medication-schedule.test.ts +++ b/src/lib/__tests__/medication-schedule.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from "vitest"; import { + expandTodayIntakes, parseScheduleRecurrence, serializeScheduleRecurrence, + type TodayExpandableSchedule, } from "@/lib/medication-schedule"; describe("medication schedule recurrence", () => { @@ -52,3 +54,111 @@ describe("medication schedule recurrence", () => { ).toBe("i4;"); }); }); + +describe("expandTodayIntakes", () => { + // 2026-05-21 is a Thursday → weekday 4. Pin a reference instant + // safely inside the Europe/Berlin day so every test below shares + // the same "today" anchor regardless of the host clock. + const REFERENCE = new Date("2026-05-21T12:00:00Z"); + const TZ = "Europe/Berlin"; + + const daily: TodayExpandableSchedule = { + id: "sched-daily", + medicationId: "med-daily", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }; + + it("emits a slot for a schedule with daysOfWeek=null (every day)", () => { + // This is the bug the W-SERVER-FIX patch addresses: the operator's + // Ramipril (Morgens) + Metformin (Abends) both ship `daysOfWeek: + // null` in the DB ("every day" per the schema annotation) and the + // today-intake endpoint returned `[]`. With the fix, the projector + // emits one slot per schedule. + const slots = expandTodayIntakes([daily], REFERENCE, TZ); + expect(slots).toHaveLength(1); + expect(slots[0]).toMatchObject({ + scheduleId: "sched-daily", + medicationId: "med-daily", + }); + }); + + it('treats daysOfWeek="" (empty string) as every day', () => { + const slots = expandTodayIntakes( + [{ ...daily, daysOfWeek: "" }], + REFERENCE, + TZ, + ); + expect(slots).toHaveLength(1); + }); + + it("treats all-seven-days (0..6) as every day", () => { + const slots = expandTodayIntakes( + [{ ...daily, daysOfWeek: "0,1,2,3,4,5,6" }], + REFERENCE, + TZ, + ); + expect(slots).toHaveLength(1); + }); + + it("respects an explicit weekday filter that excludes today", () => { + // Thursday is weekday 4; "1" = Monday only → no slot today. + const slots = expandTodayIntakes( + [{ ...daily, daysOfWeek: "1" }], + REFERENCE, + TZ, + ); + expect(slots).toHaveLength(0); + }); + + it("includes a slot when the explicit weekday filter contains today", () => { + // Thursday = weekday 4; allow {2,4} → today matches. + const slots = expandTodayIntakes( + [{ ...daily, daysOfWeek: "2,4" }], + REFERENCE, + TZ, + ); + expect(slots).toHaveLength(1); + }); + + it("emits slots for two daysOfWeek-null schedules (Ramipril + Metformin shape)", () => { + // The operator's real fixture: morning + evening daily meds. + const morning: TodayExpandableSchedule = { + id: "sched-ramipril", + medicationId: "med-ramipril", + windowStart: "07:00", + windowEnd: "09:00", + daysOfWeek: null, + }; + const evening: TodayExpandableSchedule = { + id: "sched-metformin", + medicationId: "med-metformin", + windowStart: "19:00", + windowEnd: "21:00", + daysOfWeek: null, + }; + const slots = expandTodayIntakes([morning, evening], REFERENCE, TZ); + expect(slots).toHaveLength(2); + expect(slots.map((s) => s.medicationId).sort()).toEqual([ + "med-metformin", + "med-ramipril", + ]); + }); + + it("anchors scheduledFor at the schedule's windowStart in the user tz", () => { + const slots = expandTodayIntakes([daily], REFERENCE, TZ); + expect(slots).toHaveLength(1); + const iso = slots[0].scheduledFor.toISOString(); + // 07:00 Europe/Berlin on 2026-05-21 (DST in effect → UTC+2) → 05:00Z. + expect(iso).toBe("2026-05-21T05:00:00.000Z"); + }); + + it("skips multi-week cadence (handled by the reminder worker)", () => { + const biweekly: TodayExpandableSchedule = { + ...daily, + daysOfWeek: "i2;1,3,5", + }; + expect(expandTodayIntakes([biweekly], REFERENCE, TZ)).toEqual([]); + }); +}); diff --git a/src/lib/medication-schedule.ts b/src/lib/medication-schedule.ts index 6d775be7b..e12976944 100644 --- a/src/lib/medication-schedule.ts +++ b/src/lib/medication-schedule.ts @@ -3,6 +3,41 @@ export interface ScheduleRecurrence { intervalWeeks: number; } +/** + * Minimal schedule shape consumed by `expandTodayIntakes`. Mirrors the + * relevant subset of the Prisma `MedicationSchedule` row (`id`, + * `windowStart`, `windowEnd`, `daysOfWeek`) plus the parent + * `medicationId` so the projected slots carry enough context for the + * intake-route caller to upsert into `MedicationIntakeEvent` without + * re-joining. + * + * `daysOfWeek` is the encoded string per `serializeScheduleRecurrence` + * (e.g. `null` for daily, `"1,3,5"` for Mon/Wed/Fri, `"i2;1"` for every + * other Monday). The DB schema column documents `null = daily` as the + * convention every reader honours. + */ +export interface TodayExpandableSchedule { + id: string; + medicationId: string; + /** "HH:mm" 24h, user-tz reference. */ + windowStart: string; + /** "HH:mm" 24h. May wrap midnight (`windowEnd < windowStart`). */ + windowEnd: string; + /** Encoded recurrence string per `serializeScheduleRecurrence`. */ + daysOfWeek: string | null; +} + +/** + * One projected dose slot for "today" in the user's timezone. The + * `scheduledFor` is the UTC instant that corresponds to the schedule's + * `windowStart` applied to today's local-day boundary in `timeZone`. + */ +export interface TodayExpandedSlot { + scheduleId: string; + medicationId: string; + scheduledFor: Date; +} + const ENCODED_PATTERN = /^i([1-4]);(.*)$/; function normalizeDays(days: number[]): number[] { @@ -55,3 +90,147 @@ export function serializeScheduleRecurrence( if (intervalWeeks === 1) return days.join(","); return `i${intervalWeeks};${days.join(",")}`; } + +/** + * Project the supplied schedules to the dose slots they would produce + * for "today" in the user's timezone. Treats `daysOfWeek` of `null`, + * `""`, or "no valid weekdays after normalization" as "every day" — the + * documented DB convention (`prisma/schema.prisma` annotates the column + * `null = daily`). Explicit weekday arrays (`"1,3,5"`) still filter as + * before, so a Monday-only schedule emits nothing on a Sunday. + * + * Used by `/api/medications/intake?scope=today` to backfill placeholder + * `MedicationIntakeEvent` rows for active medications whose reminder + * window has not yet opened — without this projection the endpoint + * returned `[]` for daily meds before the reminder worker entered the + * RED phase, leaving the iOS Dashboard tile + the "Erfassen" sheet + * empty for the whole morning. + * + * The function is pure (no DB access) so the test surface stays inside + * this file. The route owns the upsert. + */ +export function expandTodayIntakes( + schedules: TodayExpandableSchedule[], + reference: Date, + timeZone: string, +): TodayExpandedSlot[] { + const slots: TodayExpandedSlot[] = []; + const today = localDayParts(reference, timeZone); + + for (const schedule of schedules) { + const recurrence = parseScheduleRecurrence(schedule.daysOfWeek); + + // Day-of-week filter. Empty array (the parsed shape for `null` / + // `""` / "no valid weekdays") means "every day" — no filter. + if ( + recurrence.daysOfWeek.length > 0 && + !recurrence.daysOfWeek.includes(today.weekday) + ) { + continue; + } + + // Multi-week cadence isn't anchored without a medication start + // date here, so `intervalWeeks > 1` is conservatively skipped from + // the today projection. The reminder worker still mints those rows + // when their window opens because it has the medication context; + // those meds are GLP-1-style weeklies which the iOS today list + // doesn't depend on as a primary surface. + if (recurrence.intervalWeeks > 1) continue; + + const [h, m] = schedule.windowStart.split(":").map(Number); + if (!Number.isFinite(h) || !Number.isFinite(m)) continue; + + const scheduledFor = instantAtLocalHm(today, h, m, timeZone); + slots.push({ + scheduleId: schedule.id, + medicationId: schedule.medicationId, + scheduledFor, + }); + } + + return slots; +} + +interface LocalDayParts { + year: number; + month: number; + day: number; + weekday: number; +} + +const WEEKDAY_MAP: Record = { + Sun: 0, + Mon: 1, + Tue: 2, + Wed: 3, + Thu: 4, + Fri: 5, + Sat: 6, +}; + +function localDayParts(date: Date, timeZone: string): LocalDayParts { + const parts = new Intl.DateTimeFormat("en-US", { + timeZone, + year: "numeric", + month: "2-digit", + day: "2-digit", + weekday: "short", + }).formatToParts(date); + const get = (type: Intl.DateTimeFormatPartTypes): string => + parts.find((p) => p.type === type)?.value ?? "0"; + return { + year: Number(get("year")), + month: Number(get("month")), + day: Number(get("day")), + weekday: WEEKDAY_MAP[get("weekday")] ?? 0, + }; +} + +function instantAtLocalHm( + day: LocalDayParts, + hour: number, + minute: number, + timeZone: string, +): Date { + // Two-pass solver — first guess assumes wall-clock equals UTC, then + // correct by the zone's offset at that approximate instant. Two + // iterations suffice for every IANA zone (the second pass adjusts + // across DST transitions). + let guess = new Date( + Date.UTC(day.year, day.month - 1, day.day, hour, minute, 0, 0), + ); + for (let i = 0; i < 2; i++) { + const offsetMin = tzOffsetMinutes(guess, timeZone); + guess = new Date( + Date.UTC(day.year, day.month - 1, day.day, hour, minute, 0, 0) - + offsetMin * 60_000, + ); + } + return guess; +} + +function tzOffsetMinutes(date: Date, timeZone: string): number { + const parts = new Intl.DateTimeFormat("en-US", { + timeZone, + year: "numeric", + month: "2-digit", + day: "2-digit", + hour: "2-digit", + minute: "2-digit", + second: "2-digit", + hour12: false, + }).formatToParts(date); + const get = (type: Intl.DateTimeFormatPartTypes): number => + Number(parts.find((p) => p.type === type)?.value ?? "0"); + let hour = get("hour"); + if (hour === 24) hour = 0; + const asIfUtc = Date.UTC( + get("year"), + get("month") - 1, + get("day"), + hour, + get("minute"), + get("second"), + ); + return Math.round((asIfUtc - date.getTime()) / 60000); +}