mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
chore(release): v1.4.39.4 — dashboard symmetry for daily-schedule intake projection
This commit is contained in:
1 parent
dbb5979a97
commit
f9996552ed
15 files changed
+2016
-11
No files matched your search
@@ -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 `<Suspense>` 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 `<Suspense>` 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 `<HealthChartDynamic>` (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`. `<MoodChart>` 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 `<Suspense>` boundaries, no `useSuspenseQuery`, app is 100 % client-fetched
|
||||
- **Search:** `grep -rn "useSuspenseQuery" src` → 0 hits. `grep "<Suspense" src/components` → 0 hits. Only commentary in `app/insights/page.tsx` *justifies* not using Suspense.
|
||||
- **Layout model:** `app/layout.tsx` is an async RSC and *does* resolve the initial locale server-side, but every child page (including `app/page.tsx`) is `"use client"`. RootLayout never streams data into the dashboard.
|
||||
- **Impact:** zero benefit from React 19 streaming. Initial HTML is shell-only; tiles paint only after `useAuth` → `useAnalyticsQuery` (slim + thick) → `useQuery(moodAnalytics)` → `useQuery(layoutData)` complete. On 4G mobile that's a full 3–5× RTT chain before pixels.
|
||||
- **Fix sketch:** convert `app/page.tsx` to an RSC that fetches the slim analytics + widgets server-side, streams them through `<Suspense fallback>` 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 `<HealthChartDynamic>` call forwards `compareBaseline` and `userTimezone`. Same for `<MoodChart>` and `<MedicationComplianceChart>`.
|
||||
- **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: <ChartSkeleton /> })` 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 `<DashboardComposition>` 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.
|
||||
- `<Suspense`: 0 in `src/components`.
|
||||
- **Cross-references with infra brief:** the slim/thick split (v1.4.39.2/.3) lives on the *analytics* route — the chart-data route (`/api/measurements?aggregate=daily&source=rollup`) is on the same rollup tier but is fanned out via per-chart queries. Both perspectives agree this is the next ROI win.
|
||||
|
||||
---
|
||||
|
||||
## Recommended sequence for v1.4.39.4
|
||||
|
||||
1. **C1 first** — single biggest paint-time win. Consolidate per-chart fetches behind the slim+thick analytics contract OR a new chart-bundle endpoint keyed on the visible-metrics set.
|
||||
2. **C2 + L4** — bundle the chart caches into the dependent-keys lists so post-mutation freshness matches the tile strip.
|
||||
3. **H2** — convert `app/page.tsx` to an RSC + `<Suspense>` 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.
|
||||
@@ -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.
|
||||
@@ -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}|<discriminator>`. `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.
|
||||
@@ -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.
|
||||
@@ -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<MeasurementType, Point[]>`
|
||||
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.
|
||||
@@ -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
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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 $$;
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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(((
|
||||
|
||||
@@ -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<Array<{ id: string; takenAt: Date | null; skipped: boolean }>> {
|
||||
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({
|
||||
|
||||
@@ -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([
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
@@ -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<string, number> = {
|
||||
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);
|
||||
}
|
||||
Reference in new issue
Block a user