From 324d4bb41119e6f4b3c198160657912768ce5605 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 21 May 2026 22:07:32 +0200 Subject: [PATCH] =?UTF-8?q?chore(release):=20v1.4.44=20=E2=80=94=20REG-11?= =?UTF-8?q?=20dashboard=20summary=20fix=20(iOS=20Home=20tile=20sparkline?= =?UTF-8?q?=20+=20value)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iOS-operator-blocking hotfix for REG-11 — Home dashboard tile renders neither chart nor latest value for BP / Puls / Körperfett when the most recent reading is older than 7 days. Root cause was in `/api/dashboard/summary` SQL gates; `latestIn7d` + `sparkBuckets` both required `measured_at >= sevenDaysAgo`, which returned empty for sparse accounts, leaving the iOS tile with `latestValue: null` + `sparkline: []` to render from. Detail-screen (`/api/measurements/series?kind=…`) escaped because of its broader window. Server-side changes: - `latestIn7d` → `latestEver` (drops 7-day filter; DISTINCT ON returns latest reading EVER per type, bounded at N_metrics rows). - `sparkBuckets` rewritten via ROW_NUMBER() OVER (PARTITION BY type ORDER BY bucket_start DESC) so the trailing N daily buckets per type render regardless of age, bounded at SPARK_DAYS × N_metrics. - BP + Pulse metrics.push gated on `latest || allTimeCount > 0` like BodyFat already is — accounts with zero readings ever no longer get empty placeholder tiles. - 4 new regression tests in summary/__tests__/route.test.ts cover sparse accounts, zero-data accounts, within-7d guards. Other develop work bundled in this release: - W4-QoL — copy nits. - W5-WORKOUTS — user source priority threaded through write-time picker + batch route. - W7-WITHINGS-RELIABILITY — response classifier + sync hardening. - W8-OPS — ops phase docs. - B11 — NEXT_PUBLIC_APP_VERSION build-arg routing so /api/version cannot drift from CI tag. - env-check workflow — manifest / example lockstep enforcement. Resolves PR #194 (squash-merge produced via local -X theirs strategy because GitHub's UI couldn't auto-resolve develop's post-v1.4.42 contiguous work against the v1.4.42 squash on main; resulting tree is byte-identical to develop's HEAD). --- .env.production.example | 62 +++ .github/workflows/docker-publish.yml | 9 + .github/workflows/env-check.yml | 73 ++++ .gitignore | 3 + .planning/phase-W4-QoL-COPY-v1443-report.md | 101 +++++ ...se-W7-WITHINGS-RELIABILITY-v1443-report.md | 122 ++++++ .planning/phase-W8-OPS-v1443-report.md | 45 +++ ...round-v1443-AUDIT-analytics-9s-findings.md | 265 +++++++++++++ .../round-v1443-AUDIT-mobile-ui-findings.md | 148 ++++++++ .planning/round-v1443-AUDIT-qol-findings.md | 152 ++++++++ .../round-v1443-AUDIT-security-findings.md | 359 ++++++++++++++++++ .planning/v1443-handoff.md | 76 ++++ Dockerfile | 19 + messages/de.json | 32 +- messages/en.json | 20 +- messages/es.json | 26 +- messages/fr.json | 26 +- messages/it.json | 26 +- messages/pl.json | 26 +- .../api/auth/login/__tests__/route.test.ts | 115 ++++++ src/app/api/auth/login/route.ts | 5 +- .../dashboard/summary/__tests__/route.test.ts | 231 ++++++++++- src/app/api/dashboard/summary/route.ts | 168 +++++--- .../dashboard/widgets/__tests__/route.test.ts | 23 +- src/app/api/dashboard/widgets/route.ts | 69 +++- src/app/api/version/__tests__/route.test.ts | 21 + src/app/api/version/route.ts | 10 +- .../workouts/__tests__/batch-create.test.ts | 72 ++++ src/app/api/workouts/batch/route.ts | 14 + src/app/global-error.tsx | 20 +- src/app/not-found.tsx | 18 +- src/components/admin/admin-shell.tsx | 4 +- .../health-chart-empty-state-gate.test.tsx | 121 ++++++ ...compliance-chart-empty-state-gate.test.tsx | 93 +++++ .../mood-chart-empty-state-gate.test.tsx | 94 +++++ .../charts/chart-overlay-controls.tsx | 2 +- src/components/charts/health-chart.tsx | 63 ++- .../charts/medication-compliance-chart.tsx | 36 +- src/components/charts/mood-chart.tsx | 38 +- .../insights/coach-panel/message-thread.tsx | 4 +- .../measurements/measurement-list.tsx | 2 +- src/components/mood/mood-form.tsx | 2 +- src/components/mood/mood-list.tsx | 2 +- .../settings/integration-status-pill.tsx | 22 +- src/components/settings/settings-shell.tsx | 4 +- src/components/ui/dialog.tsx | 2 +- src/components/ui/sheet.tsx | 2 +- src/components/ui/switch.tsx | 8 +- src/lib/__tests__/motion.test.ts | 44 +++ .../__tests__/summaries-slice.wmy-cap.test.ts | 103 +++++ src/lib/analytics/summaries-slice.ts | 35 +- src/lib/i18n/relative-time.ts | 30 +- src/lib/motion.ts | 13 + .../__tests__/response-classifier.test.ts | 17 + .../sync-typed-classification.test.ts | 226 +++++++++++ src/lib/withings/response-classifier.ts | 8 +- src/lib/withings/sync-activity.ts | 55 +-- src/lib/withings/sync-sleep.ts | 42 +- src/lib/withings/sync.ts | 2 +- .../workouts/__tests__/canonical-rows.test.ts | 56 +++ src/lib/workouts/canonical-rows.ts | 84 ++-- 61 files changed, 3322 insertions(+), 248 deletions(-) create mode 100644 .env.production.example create mode 100644 .github/workflows/env-check.yml create mode 100644 .planning/phase-W4-QoL-COPY-v1443-report.md create mode 100644 .planning/phase-W7-WITHINGS-RELIABILITY-v1443-report.md create mode 100644 .planning/phase-W8-OPS-v1443-report.md create mode 100644 .planning/round-v1443-AUDIT-analytics-9s-findings.md create mode 100644 .planning/round-v1443-AUDIT-mobile-ui-findings.md create mode 100644 .planning/round-v1443-AUDIT-qol-findings.md create mode 100644 .planning/round-v1443-AUDIT-security-findings.md create mode 100644 .planning/v1443-handoff.md create mode 100644 src/app/api/auth/login/__tests__/route.test.ts create mode 100644 src/components/charts/__tests__/health-chart-empty-state-gate.test.tsx create mode 100644 src/components/charts/__tests__/medication-compliance-chart-empty-state-gate.test.tsx create mode 100644 src/components/charts/__tests__/mood-chart-empty-state-gate.test.tsx create mode 100644 src/lib/__tests__/motion.test.ts create mode 100644 src/lib/analytics/__tests__/summaries-slice.wmy-cap.test.ts create mode 100644 src/lib/motion.ts create mode 100644 src/lib/withings/__tests__/sync-typed-classification.test.ts diff --git a/.env.production.example b/.env.production.example new file mode 100644 index 000000000..028b12a45 --- /dev/null +++ b/.env.production.example @@ -0,0 +1,62 @@ +# ============================================================================= +# HealthLog -- production .env example +# ============================================================================= +# Mirrors `scripts/env-manifest.json`. The `pnpm check-env --file +# .env.production.example` CI step (see `.github/workflows/env-check.yml`) +# enforces every variable declared `required: true` in the manifest has a +# non-empty placeholder here, so a contributor who introduces a new +# required var without updating this file fails the workflow. +# +# Placeholders are syntactic only -- replace with real secrets before +# deploying. The check-env presence rule treats any non-empty, +# non-whitespace string as "set", so the `` style below +# satisfies the manifest without leaking a usable credential. +# +# Generate the two crypto secrets locally with: +# $ openssl rand -hex 32 +# ============================================================================= + + +# ----------------------------------------------------------------------------- +# Core -- required for boot +# ----------------------------------------------------------------------------- +DATABASE_URL="postgresql://healthlog:@db:5432/healthlog?schema=public" +POSTGRES_PASSWORD="" +NEXT_PUBLIC_APP_URL="https://healthlog.example.com" +APP_URL="https://healthlog.example.com" +ENCRYPTION_KEY="" +API_TOKEN_HMAC_KEY="" + + +# ----------------------------------------------------------------------------- +# Withings OAuth -- optional integration +# ----------------------------------------------------------------------------- +# WITHINGS_CLIENT_ID="" +# WITHINGS_CLIENT_SECRET="" +# WITHINGS_WEBHOOK_SECRET="" + + +# ----------------------------------------------------------------------------- +# APNs -- iOS push (optional, all-or-none) +# ----------------------------------------------------------------------------- +# APNS_KEY_ID="" +# APNS_TEAM_ID="" +# APNS_BUNDLE_ID="" +# APNS_KEY="" # OR APNS_KEY_FILE -- either one satisfies the manifest +# APNS_KEY_FILE="" + + +# ----------------------------------------------------------------------------- +# Deploy webhook -- Coolify -> HealthLog auto-deploy feedback +# ----------------------------------------------------------------------------- +# DEPLOY_WEBHOOK_SECRET="" + + +# ----------------------------------------------------------------------------- +# Off-host backups -- all-or-none +# ----------------------------------------------------------------------------- +# BACKUP_S3_ENDPOINT="" +# BACKUP_S3_BUCKET="" +# BACKUP_S3_ACCESS_KEY="" +# BACKUP_S3_SECRET_KEY="" +# BACKUP_ENCRYPTION_KEY="" diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 4473b70d1..2622a05c9 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -194,6 +194,15 @@ jobs: file: ./Dockerfile platforms: ${{ matrix.platform }} labels: ${{ steps.meta.outputs.labels }} + # v1.4.43 B11 — thread the tag ref into the build as + # NEXT_PUBLIC_APP_VERSION. The Dockerfile picks it up as an + # ARG/ENV and /api/version reads it at runtime. The build-arg + # becomes part of the BuildKit layer cache key, so a release + # tag bump invalidates every downstream layer that depends on + # it (`pnpm build`) and the prior release's version string + # can never survive a cache hit into the new image. + build-args: | + NEXT_PUBLIC_APP_VERSION=${{ github.ref_name }} outputs: | type=image,name=${{ env.IMAGE_REF }},push-by-digest=true,name-canonical=true,push=${{ github.event_name != 'pull_request' }} cache-from: | diff --git a/.github/workflows/env-check.yml b/.github/workflows/env-check.yml new file mode 100644 index 000000000..5612c2fc0 --- /dev/null +++ b/.github/workflows/env-check.yml @@ -0,0 +1,73 @@ +name: Env-manifest <-> example lockstep + +# v1.4.43 B5 -- enforce that `.env.production.example` stays in lockstep +# with `scripts/env-manifest.json`. A contributor adding a new required +# variable to the manifest must also add a placeholder to the example +# file in the same PR, otherwise the workflow fails with a grep-friendly +# list of missing required vars. Optional vars are reported but do not +# fail the build -- they exist for operator discovery, not deploy +# blocking. + +on: + push: + branches: [main, develop] + pull_request: + branches: [main, develop] + workflow_dispatch: + +permissions: + contents: read + +jobs: + env-check: + name: pnpm check-env against .env.production.example + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Install pnpm + uses: pnpm/action-setup@v4 + + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Assert example file exists + run: | + if [ ! -f .env.production.example ]; then + echo "::error::.env.production.example is missing -- contributors must keep this file in lockstep with scripts/env-manifest.json. See docs/ops/env-check.md." + exit 1 + fi + + # The check-env script exits non-zero when at least one required + # variable from the manifest is missing/empty in the example file. + # We additionally grep the rendered output for `[MISSING-REQUIRED]` + # rows and surface them as GitHub Actions error annotations so the + # PR reviewer can see the missing var names without expanding the + # log. + - name: pnpm check-env --file .env.production.example + id: check + run: | + set +e + OUT=$(pnpm --silent check-env --file .env.production.example) + EC=$? + echo "$OUT" + if [ $EC -ne 0 ]; then + echo "" + echo "::group::Missing required variables" + echo "$OUT" | grep -E '^\s*\[MISSING-REQUIRED\]' | while IFS= read -r line; do + name=$(echo "$line" | sed -E 's/.*\[MISSING-REQUIRED\] ([A-Z0-9_]+).*/\1/') + echo "::error file=.env.production.example::Required env var $name missing from .env.production.example -- add a placeholder so the file stays in lockstep with scripts/env-manifest.json." + done + echo "::endgroup::" + echo "" + echo "::error::pnpm check-env reported missing required variables in .env.production.example. Either add a placeholder for each missing var, or downgrade the manifest entry to required:false if the variable is genuinely optional. See docs/ops/env-check.md." + exit $EC + fi diff --git a/.gitignore b/.gitignore index e37e7cb9e..f32cec364 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,9 @@ # testing /coverage +# HAR captures from production debugging (may contain auth headers/cookies) +*.har + # playwright e2e /test-results /playwright-report diff --git a/.planning/phase-W4-QoL-COPY-v1443-report.md b/.planning/phase-W4-QoL-COPY-v1443-report.md new file mode 100644 index 000000000..2a93aafa6 --- /dev/null +++ b/.planning/phase-W4-QoL-COPY-v1443-report.md @@ -0,0 +1,101 @@ +# W4-QoL-COPY — v1.4.43 phase report + +Branch: `worktree-agent-a2db58cdde5fee530` (pushed). +Base: `develop` @ `7daff01a`. +Commits: 6 atomic. + +## Six Highs — outcome + +### H1 — DayDrillDown error toast wrong key +`src/components/measurements/measurement-list.tsx:1058` now reads +`measurements.loadError` in the GET-failure branch. A new key +`measurements.loadError` was added inside the existing `measurements.*` +namespace of every locale (de/en/es/fr/it/pl). German reads "Fehler beim +Laden", English "Could not load measurements". + +Commit `a2bd6fdb`. + +### H2 — German "Provider" / "rate-limited" leak +`messages/de.json` only — three locations: +- `providerRateLimitTitle` / `providerRateLimitBody` → "Anbieter + gedrosselt" / "Anbieter ist temporär überlastet; wieder verfügbar in + ~5 min." +- `:1620 description` → "Anbieter, Modell, API-Schlüssel." +- `rawDataOnDescription` / `OffDescription` / `Warning` → "Provider" + replaced with "Anbieter" throughout. + +Commit `d8258fc2`. + +### H3 — Persistent failure-kind has no surface +Brief allowed only `integration-status-pill.tsx` + locale files. Added a +fourth pill state `"warning"` with locale key +`settings.integrationPill.warningServerError` ("Verbunden, aber +Serverfehler" / "Connected, server error") and an amber +`border-dracula-yellow/30 bg-dracula-yellow/15 text-dracula-yellow` +chip. Variant defaults match `connected` (custom-class instead of +shadcn destructive) so the warning visual is distinguishable from the +red reconnect chip. + +**Deferred (out of scope per file allow-list):** wiring at +`integrations-section.tsx:123-136` — `pillStateFor` still collapses +`error_transient` → `"error"`. To enable the warning chip end-to-end, +the next phase must: +1. Add `lastFailureKind` to `IntegrationStatus` (Prisma migration) and + plumb it through `recordSyncFailure` + `getIntegrationStatus`. +2. Surface it in the `/api/integrations/status` payload. +3. Update `pillStateFor` to return `"warning"` when + `failureKind === "persistent"`. + +Commit `d1428650`. + +### H4 — not-found.tsx English-only +Switched `src/app/not-found.tsx` to an `async` server component that +calls `resolveServerLocale()` + `getServerTranslator()` (already +existed under `src/lib/i18n/`). Three new keys per locale under a new +top-level `errors.notFound.{title,body,backToDashboard}` namespace. + +Commit `430f5374`. + +### H5 — global-error.tsx bilingual +Cannot reach an i18n resolver before root-layout boots, so doubled the +four user-facing strings as `Deutsch · English`. Updated heading, body +paragraph, both button labels, copy-fallback `window.prompt`, and the +unknown-error fallback. No new dependency on the i18n stack — the file +remains provider-less by contract. + +Commit `ee0c4e8c`. + +### H6 — relativeMinutes/Hours/DaysAgo singular form +Helper `src/lib/i18n/relative-time.ts` now branches on `count === 1` +and reads `*AgoOne` vs `*AgoOther`. Each locale's three plural-only +keys were split into the two-form pair (matching the existing +`dashboard.staleHintWeeksOne` / `Other` pattern). Polish kept a +pragmatic singular/plural pair; the helper's two-form contract matches +the brief's recipe. + +No other call-site references the old keys (grep confirmed), so the +removal is safe. + +Commit `886957a0`. + +## Quality gate + +- `pnpm typecheck` — clean. Required a one-off `pnpm prisma generate` + on the worktree because `src/generated/prisma` is git-ignored. +- `pnpm lint` — clean. +- `pnpm vitest run src/components/settings/__tests__/integration-status-pill.test.tsx src/lib/i18n src/components/measurements` — 28 passed / 1 skipped. +- All six `messages/*.json` files parse. + +## File-allow-list discipline + +Touched only: `src/components/measurements/measurement-list.tsx`, +`src/components/settings/integration-status-pill.tsx`, +`src/app/not-found.tsx`, `src/app/global-error.tsx`, +`src/lib/i18n/relative-time.ts`, `messages/{de,en,es,fr,it,pl}.json`. + +Nothing else. + +## Commit-message hygiene + +No `Co-Authored-By: Claude`. No `--no-verify`. No first-name in commit +bodies. Marc-voice, English, no AI references. diff --git a/.planning/phase-W7-WITHINGS-RELIABILITY-v1443-report.md b/.planning/phase-W7-WITHINGS-RELIABILITY-v1443-report.md new file mode 100644 index 000000000..97370dd87 --- /dev/null +++ b/.planning/phase-W7-WITHINGS-RELIABILITY-v1443-report.md @@ -0,0 +1,122 @@ +# W7-WITHINGS-RELIABILITY — v1.4.43 phase report + +## Scope shipped (B3 only) + +- **B3 — typed-classification catch-block migration** for the two + sibling Withings sync paths (activity + sleep). +- Exported `recordWithingsSyncFailure(userId, err)` from + `src/lib/withings/sync.ts` so the sibling paths can share the helper. +- Migrated `src/lib/withings/sync-activity.ts` to consume the shared + helper. Dropped the local `extractWithingsStatus` + + `isWithingsRefreshReauthFailure` imports; the helper internally + routes through `classifyError` (typed `WithingsApiError` first, regex + fallback for un-prototyped errors via pg-boss JSON round-trip). +- Migrated `src/lib/withings/sync-sleep.ts` the same way. +- BL-P3-2 defence-in-depth — a Withings body-status 403 on either + scope-gated endpoint still forces `kind: "reauth_required"`. The + response-classifier's body-status taxonomy doesn't include 403 (it + isn't a documented Withings response code; only the HTTP layer emits + 403 for scope-gated resources), so the override stays at the call + site rather than being added to the shared `REAUTH_CODES` set — that + would have meant a `response-classifier.ts` edit, outside this + wave's allow-list. +- New regression suite + `src/lib/withings/__tests__/sync-typed-classification.test.ts` pins + the typed-classification verdict for body-status 100 + (reauth_required), 102 (reauth_required), 293 (persistent), 601 + (transient) across both sync paths. The 293 + 601 cases were + silently `transient` under the legacy regex chain — the new tests + guard against a future refactor reverting to the regex. + +### Quality bar + +- `pnpm lint` — clean. +- `pnpm typecheck` — clean. +- `pnpm test --run src/lib/withings` — **6 test files, 118 tests, + all green** (112 pre-existing + 6 new). + +### File diff surface (strict) + +- `src/lib/withings/sync.ts` — single-keyword `export` added; no + behavioural change. +- `src/lib/withings/sync-activity.ts` — import set trimmed + catch + block refactored. +- `src/lib/withings/sync-sleep.ts` — same pattern. +- `src/lib/withings/__tests__/sync-typed-classification.test.ts` — + new file. +- **NO touch** to `src/lib/integrations/status.ts`, + `src/lib/withings/response-classifier.ts`, or any other file + outside the four listed above. + +## Deferred to v1.4.44 + +### B4 — IntegrationStatus surface unification + +**Status:** DEFERRED — scope discipline / context budget. + +**Architecturally-correct path the v1.4.44 wave should pick up:** + +1. The five callers that today hand-roll + `recordSyncFailure({ ..., kind, errorCode })` payloads inline + (`getValidToken` reauth-park, `syncUserMeasurements` fetch catch, + `syncUserActivity` 403 short-circuit, `syncUserSleep` 403 + short-circuit, plus the route-layer caller in + `src/app/api/withings/status/route.ts`) duplicate the + classification-to-FailureKind mapping. +2. Lift the mapping into a single `withingsFailureKindFor(err)` helper + in `src/lib/withings/sync.ts` (or in `response-classifier.ts` if + the helper stays pure — preferred). The helper returns + `{ kind: FailureKind, errorCode?: string }` directly so each call + site collapses to `recordSyncFailure({ userId, integration: + "withings", message, ...withingsFailureKindFor(err) })`. +3. Migrate `src/app/api/withings/status/route.ts:85` off + `isWithingsRefreshReauthFailure(message)` onto the typed helper + (the route currently re-parses the message string a second time + after the sync layer already classified it). +4. Once every call site uses the helper, delete the legacy regex + exports `isWithingsRefreshReauthFailure` + `extractWithingsStatus` + from `sync.ts`. The route-layer migration in step 3 is the last + blocker; once it's gone the regex helpers are unreferenced and can + be removed in the same commit. + +### B7 — IntegrationStatus row freshness audit + +**Status:** DEFERRED — scope discipline / context budget. + +**Architecturally-correct path the v1.4.44 wave should pick up:** + +1. Inventory every code path that writes `IntegrationStatus` and + compare against the integrations the user can reach from the + Settings page. Today the `withings` row is the only one fully + wired through the failure ladder; the `apple_health` and `manual` + surfaces lean on the same row but never refresh `lastSyncedAt` + when ingest succeeds. +2. Add a `recordIntegrationSuccess(userId, integration)` helper + alongside `recordSyncSuccess` in + `src/lib/integrations/status.ts` that ALWAYS bumps + `lastSyncedAt` even when the integration doesn't run on a sync + cadence (Apple Health is push-only — no `syncUser*` wrapper to + hook). +3. Wire the helper into the Apple Health batch-ingest endpoint + (`src/app/api/healthkit/batch/route.ts` and the per-route + counterparts) so the Settings page surfaces a meaningful + "last synced" timestamp for the iOS path. +4. Add a freshness-staleness probe to the `/api/integrations/status` + reader that flags rows where `lastSyncedAt` is older than the + integration's expected cadence (Withings: 1h; Apple Health: 24h + since iOS only batches on a daily wake). The probe drives an + amber-state badge on the Settings card without changing the + underlying success / failure / reauth taxonomy. + +### Reason for the split + +The B4 + B7 work touches `src/lib/integrations/status.ts` (B4: helper +addition + removal of the regex helpers) and the API surface (B7: +freshness probe). Either change is one bucket on its own — bundling +all three into v1.4.43 would have meant the wave touched the +classifier, the integration-status writer, AND the API reader, well +past the "scope discipline" budget the v1.4.43 marathon was +calibrated for. B3 was the standalone unit that delivered a +measurable de-duplication of the classification logic without +expanding the surface, so it shipped on its own and the dependent B4 ++ B7 work moves to v1.4.44 where it gets the focus it needs. diff --git a/.planning/phase-W8-OPS-v1443-report.md b/.planning/phase-W8-OPS-v1443-report.md new file mode 100644 index 000000000..ea79630fe --- /dev/null +++ b/.planning/phase-W8-OPS-v1443-report.md @@ -0,0 +1,45 @@ +# W8-OPS phase report — v1.4.43 + +Branch: `worktree-agent-abf6af166a9d95b13` (pushed). +Base: `develop` at `7daff01a`. +Commits: 3 atomic, one per B-item. + +## B2 — Widgets audit-ledger rate-limit + +- `src/app/api/dashboard/widgets/route.ts`: added a `Map` dedup memo with a 60 s `(userId, action)` window. The audit-row write is gated through `shouldEmitAuditRow(...)`; the 422 response is unchanged. Opportunistic GC at >512 entries drops expired rows. Exported `__resetAuditDedupMemoForTests` for the unit test. +- Test: new case "dedups the audit-ledger write across two sequential 422s for the same user". Two PUTs in succession → exactly one `prisma.auditLog.create` call. Existing W2 tests still green (4 → 5 tests). +- Commit: `6301a640 feat(widgets): dedup validation-failed audit rows per user 60s`. + +## B11 — Docker BuildKit cache fix (option 2) + +- `Dockerfile`: `ARG NEXT_PUBLIC_APP_VERSION` + `ENV` in builder and runner stages. Comment notes that the build-arg becomes part of the layer cache key, forcing per-release invalidation. +- `.github/workflows/docker-publish.yml`: added `build-args: NEXT_PUBLIC_APP_VERSION=${{ github.ref_name }}` to the build-push step. +- `src/app/api/version/route.ts`: `process.env.NEXT_PUBLIC_APP_VERSION?.trim() || packageJson.version`. Whitespace-only env value falls through to the package.json read. +- Tests: two new cases — env-var preference + blank-fallthrough. 5 → 7 tests, all green. +- Commit: `88e80949 fix(docker): bake CI tag into image, prefer it at runtime`. + +## B5 — pnpm check-env CI integration + +- New `.env.production.example` with placeholder strings for every required manifest var; optional groups commented out. Verified locally: `pnpm check-env --file .env.production.example` exits 0 with 6/6 required `[OK]` rows. +- New `.github/workflows/env-check.yml` that runs on push + PR against `main`/`develop`. Runs `pnpm check-env --file .env.production.example`, greps `[MISSING-REQUIRED]` from the rendered output, and surfaces each missing var as a GitHub Actions error annotation with file pinning. +- Existing `scripts/__tests__/check-env.test.ts` (19 tests) pin the dedup + anyOf + all-or-none logic; left untouched — no new branches added. +- Commit: `1db52d0c ci(env-check): enforce manifest <-> example lockstep`. + +## Quality gates + +- `pnpm typecheck`: green (no output). +- `pnpm lint`: green (no output). +- Impacted-subset tests: `src/app/api/dashboard/widgets src/app/api/version scripts/__tests__/check-env.test.ts` → 3 files, 31 tests passed. + +## Touched files + +- `src/app/api/dashboard/widgets/route.ts` +- `src/app/api/dashboard/widgets/__tests__/route.test.ts` +- `src/app/api/version/route.ts` +- `src/app/api/version/__tests__/route.test.ts` +- `Dockerfile` +- `.github/workflows/docker-publish.yml` +- `.github/workflows/env-check.yml` (new) +- `.env.production.example` (new) + +No unrelated files touched. Marc-voice, no Claude attribution, no `--no-verify`. diff --git a/.planning/round-v1443-AUDIT-analytics-9s-findings.md b/.planning/round-v1443-AUDIT-analytics-9s-findings.md new file mode 100644 index 000000000..f3a1ec651 --- /dev/null +++ b/.planning/round-v1443-AUDIT-analytics-9s-findings.md @@ -0,0 +1,265 @@ +# v1.4.43 /api/analytics 9s perf investigation + +## Verdict + +**URGENT** — both `/api/analytics` (default + `?slice=summaries`) wait +~9 030 ms server-side on every dashboard mount of the live v1.4.42 +tree. The regression has been latent since v1.4.40 W-WMY-WIRE and was +masked through v1.4.41/42 because no other endpoint moved. It is the +single largest user-visible regression on the v1.4.42 deploy and must +land in v1.4.43. + +## Root cause + +`src/lib/analytics/summaries-slice.ts:731-750` +(`computeAvg30LastYearMap`) is an **unbounded `Promise.all` fan-out** +over the per-type list of measurement types with data. It was +introduced in v1.4.40 W-WMY-WIRE (commit `24568c80`, +`perf(summaries-slice): consume monthly rollup buckets for long-window +slice`). Both `computeFromRollups` +(`src/lib/analytics/summaries-slice.ts:421`) and +`computeFromLiveAggregate` +(`src/lib/analytics/summaries-slice.ts:584`) call it unconditionally +on every slim-slice request. + +On Marc's tenant `typesWithData.length` is ~15 (every measurement type +the dashboard tile-strip surfaces). For each type the helper invokes +`readBestGranularityRollups(userId, type, 395)` +(`src/lib/rollups/measurement-read-wmy.ts:133-155`), which **sequentially** +walks `GRANULARITY_FLOORS` from coarsest to finest. With `windowDays=395` +the loop tries MONTH (floor 181) first, then WEEK (91), then DAY (0) +— up to **three sequential Prisma round-trips per type** when MONTH / +WEEK coverage is partial. The only coverage the boot-time inline +refresh (`ensureUserRollupsFresh`, +`src/lib/rollups/measurement-rollups.ts:632-642`) guarantees is DAY +(trailing 90 d); WEEK/MONTH/YEAR depend on the v1.4.35.1 background +`rollup-full-backfill` queue and per-write hooks having converged. + +So on a hot dashboard mount with two concurrent `/api/analytics` +requests: + +- **Slim** (`computeFromRollups`) holds 3 base slots (narrows / + latests / dayBuckets `$queryRaw`) and then **bursts 15 simultaneous + `findMany`s** against `measurement_rollups`. Each can serialise into + 2-3 round-trips on coverage miss. +- **Thick** (`buildAnalyticsResponse`) probes coverage, fans out 15-way + per-type live walks behind `p-limit(4)` (4 slots), runs two parallel + `computeBpInTargetFastPath` calls (4 slots for the rollup pair-read), + `correlations-fast-path` (3 slots), `health-score-fast-path` (4 + parallel slots), plus stand-alone `sleepStages` + `glucoseByContext` + findManys (2 slots). +- **Background**: each route fires `void + ensureUserRollupsFresh(userId)` which itself holds 2 slots for the + watermark probe and, when stale, a `recomputeUserRollups` of the + DAY window. + +Peak concurrency easily exceeds the `pg.Pool` max of 20 (see +`src/lib/db.ts:35`). The W-POOL `p-limit(4)` cap on the per-type +live walk inside the thick route (`route.ts:55`, +`ANALYTICS_TYPE_FETCH_CONCURRENCY`) was added in v1.4.40 explicitly to +prevent this — but the same release added the W-WMY-WIRE fan-out in +the slim slice WITHOUT any equivalent cap. The two concurrent routes +now drown the pool that v1.4.40 raised from 10→20 specifically to +absorb this kind of burst. + +Secondary contributors (audited but NOT the dominant cost): + +- `computeFromRollups`'s three lead `$queryRaw`s are correctly + parallelised in one `Promise.all`. They are not the bottleneck. +- The thick route's `summarize()` per-type fan-out is correctly capped + at 4. Its 425-day cap (`route.ts:264`) is in place. +- `probeRollupCoverage` (`src/lib/rollups/measurement-coverage.ts:45`) + is one indexed query and lands sub-50 ms in practice. +- `glucoseByContext` and `sleepStages` are both bounded to 30 days and + narrow projections — fine. +- The cache (`caches.analytics`, 60s TTL) is keyed + `{userId}|summaries` vs `{userId}|default`. Both miss simultaneously + on cold mount; both are single-flighted **per key** but NOT across + keys, so the two slices fire concurrently. Correct behaviour, but + exposes the pool to the combined fan-out below. + +## Recommended fix + +Two-line minimum-delta change — cap the WMY fan-out with the same +`p-limit` discipline v1.4.40 W-POOL applied to the thick route. + +### `src/lib/analytics/summaries-slice.ts` + +Add `import pLimit from "p-limit";` at the top of the file (alongside +the existing imports — already a project dependency, used in +`src/app/api/analytics/route.ts:1`). + +Replace `computeAvg30LastYearMap` +(`src/lib/analytics/summaries-slice.ts:731-750`) with: + +```ts +/** + * v1.4.43 — cap concurrent per-type WMY reads at 4. + * + * Pre-fix this helper ran an unbounded Promise.all over every type + * the user has data for. On a 15-type tenant the burst held 15+ + * Prisma slots simultaneously, and with the slim and thick analytics + * slices firing in parallel on dashboard mount the combined fan-out + * drowned the pg.Pool max=20 even after the v1.4.40 W-POOL raise. + * + * Capping at 4 keeps the same total work but holds at most 4 slots + * for the WMY branch of the slim slice at any moment, leaving + * headroom for the concurrent thick slice's own fan-outs. + */ +const WMY_FANOUT_CONCURRENCY = 4; + +async function computeAvg30LastYearMap( + userId: string, + types: ReadonlyArray, +): Promise> { + const out = new Map(); + if (types.length === 0) return out; + const limit = pLimit(WMY_FANOUT_CONCURRENCY); + const results = await Promise.all( + types.map((type) => + limit(async () => { + const value = await computeAvg30LastYearForType( + userId, + type as MeasurementType, + ); + return [type, value] as const; + }), + ), + ); + for (const [type, value] of results) { + out.set(type, value); + } + return out; +} +``` + +That alone collapses the slim-slice burst back to 4-at-a-time, which +matches the thick slice's existing W-POOL cap. + +### Optional follow-up (defer to v1.4.44 if v1.4.43 needs minimum-delta) + +If perf-verify confirms the cap fix is insufficient on its own, two +incremental escalations are available: + +1. **Pin the WMY router to the granularity the boot backfill + actually mints.** Today `readBestGranularityRollups(395)` falls + through MONTH → WEEK → DAY on coverage miss. If the tenant has + converged DAY but not MONTH, the helper pays the cost of two + miss probes per type before landing on DAY. The cheap fix is to + issue a single MONTH-or-DAY read using the per-type coverage map + the route already probed (`route.ts:209` / + `summaries-slice.ts:221`) instead of a sequential floor walk. + See `.planning/round-v1438-perf-analysis.md` §2 for the original + "WMY tier sits as write amplification" framing. + +2. **Hoist `computeAvg30LastYearMap` behind a per-(userId,date) cache.** + The 395-day window is anchored on `Date.now()` so the result is + stable across an entire calendar day. A 24h TTL in + `caches.analytics` keyed on a `${userId}|avg30-last-year-${ymd}` + shape would collapse the burst to zero on warm days. The current + `caches.analytics` envelope already carries the full slice; the + per-call sub-helper just doesn't see the cache because it lives + inside `computeFromRollups`. + +Neither follow-up is required to close the 9 s regression; the +`p-limit(4)` cap is the single minimum-delta land. + +## Expected delta + +Empirical reference points from the v1.4.39 cold-mount trace +(`.planning/round-v1439-empirical-trace.md` § B1-B3) and the v1.4.40 +W-POOL closure show: + +- W-POOL `p-limit(4)` on the thick route's 15-way per-type walk took + the pool-wait from 6.5 s → ~1.5 s under a similar burst. +- The slim slice's pre-W-WMY-WIRE timing (v1.4.37) was 1.5-3 s cold, + <100 ms warm. Adding the 15-way WMY fan-out in v1.4.40 widened the + cold-mount window proportionally. + +Projecting the same `p-limit(4)` shape onto the slim slice: + +| state | before | after (est) | +|---|---|---| +| dashboard mount, both slices cold | 9.0 s + 9.0 s in parallel | ~2.0-3.0 s + ~2.0-3.0 s in parallel | +| dashboard mount, slim warm + thick cold | ~9.0 s thick | ~2.0-3.0 s thick | +| dashboard mount, both warm (60s TTL hit) | ~50 ms | ~50 ms | +| WMY tier converged + cap applied | n/a | <500 ms cold (best case) | + +The cap restores the v1.4.37 target band (1.5-3 s) under combined +slim+thick concurrent load. Warm-cache behaviour is unchanged. + +## Test plan + +1. **Unit** — extend + `src/lib/analytics/__tests__/summaries-slice.test.ts` with a test + that mocks `computeAvg30LastYearForType` and asserts the helper + never holds more than 4 in-flight calls simultaneously. Pattern + mirrors the v1.4.40 `route.test.ts` test pinning + `ANALYTICS_TYPE_FETCH_CONCURRENCY=4` + (`src/app/api/analytics/__tests__/route.test.ts` — + `pin concurrency cap and pool ceiling`, commit `433874e3`). + +2. **Integration** — add a smoke test that issues a concurrent + `?slice=summaries` + default request against a seeded + 15-type tenant and asserts both resolve under 5 s. Place + alongside the existing `route.test.ts` integration suite. + +3. **Wide-event probe** — in production, after deploy, confirm both + routes emit `meta.analytics.slim_summaries.path:"rollup"` AND a new + `wmy_fanout.concurrency: 4` annotate (add to the helper's annotate + call alongside `slim_summaries` block so ops can prove the cap + fired). Compare wall-clock to the HAR baseline. + +4. **HAR re-capture** — Marc to re-record the live tree post-deploy + and replace `.planning/v1442-postdeploy-new1.har` with the + post-fix capture. Both `/api/analytics*` rows should land in the + 2-3 s band. + +## Related findings + +- **WMY rollup tier dead-write-amplification risk**: the v1.4.38 perf + audit (`round-v1438-perf-analysis.md` § 2, P6) flagged that + WEEK/MONTH/YEAR rollups are written on every measurement but never + read. v1.4.39 W-WMY readers + v1.4.40 W-WMY-WIRE wiring closed + that — but the wiring landed without a concurrency cap, which is + exactly the regression this report documents. The pattern + recurrence (Read-swap should replace, not parallel-run — see the + memo `feedback_read_swap_replace_not_parallel.md`) is worth + surfacing for the v1.4.43 marathon retrospective. + +- **Boot backfill MONTH/WEEK coverage**: `ensureUserRollupsFresh` + (`src/lib/rollups/measurement-rollups.ts:632`) inlines only the + DAY refresh. If the per-write hooks or boot backfill have not + drained the MONTH partition for a user, every WMY call pays a + miss-probe penalty before landing on DAY. The v1.4.35.1 + `rollup-full-backfill` queue (`enqueueBootTimeRollupBackfill`, + `measurement-rollups.ts:688`) should be doing this on each worker + boot — operator verification via `pg-boss` queue depth on + `ROLLUP_FULL_BACKFILL_QUEUE` would confirm. Not on the v1.4.43 + critical path; deferrable to a v1.4.44 ops doc. + +- **Cache key splitting**: the analytics cache uses + `${userId}|summaries` and `${userId}|default` as separate keys + (`route.ts:96` and `route.ts:115`), so the two concurrent dashboard + fetches do NOT share a single-flight. Each must compute + independently. Acceptable today (different response shapes), but + if v1.5 unifies the analytics envelope a single-flight collapse + would halve the cold-mount cost outright. + +- **`mood/analytics` at 200 ms (HAR row 3)** is healthy — it does + not share the WMY fan-out and serves directly from + `mood_entry_rollups` (`src/lib/rollups/mood-rollups.ts`). No + action. + +- **`ensureUserRollupsFresh` void-fire**: the v1.4.37.1 hotfix + decoupling was correct and remains. The in-flight dedup map + (`measurement-rollups.ts:582-588`) handles the concurrent slim+thick + case correctly — both join the same promise. No action. + +- **Charts intermittent empty-state ("Erfasse mindestens 3 + Einträge…")**: traceable to the slim slice's 9 s tail letting the + chart's `count >= 3` predicate evaluate against an undefined + summary before the response lands. Capping the fan-out closes this + symptom as a side effect — the chart will see the populated + `summary` within the 2-3 s window TanStack Query's `useQuery` + loading skeleton already accommodates. No separate fix needed. diff --git a/.planning/round-v1443-AUDIT-mobile-ui-findings.md b/.planning/round-v1443-AUDIT-mobile-ui-findings.md new file mode 100644 index 000000000..909c99aea --- /dev/null +++ b/.planning/round-v1443-AUDIT-mobile-ui-findings.md @@ -0,0 +1,148 @@ +# v1.4.43 Mobile UI / responsive audit findings + +## Verdict +APPROVE_WITH_FIXES + +Mobile-UI hygiene is in good shape overall — v1.4.27/33/34 mobile passes have closed the major touch-target / iOS-zoom / safe-area gaps. The user-reported "Erfasse mindestens 3 Einträge" finding is a real semantic bug (not a slow-data artifact), and there are a few residual sub-44 px tap targets and motion-handling inconsistencies worth addressing before the tag. None block the release on their own. + +--- + +## Critical (must fix before tag) + +### C1 — Chart empty-state fires on the 7-day tab even when the user has lots of data +- **File**: `/Users/marc/Projects/HealthLog/src/components/charts/health-chart.tsx:1251` (gate) + `:672-770` (chartData derivation) + `:675` (slice) +- **What's wrong**: The empty state `t("charts.emptyStateTitle")` = "Erfasse mehr Messungen, um Trends zu sehen" + `t("charts.emptyStateDescription")` = "Erfasse mindestens 3 Einträge, um die Trendlinie freizuschalten." paints when `chartData.length >= 1 && chartData.length < 3`. `chartData` is the *daily-aggregated* slice over the active range tab. A user with 50 BP readings in the last 30 days but only 2 distinct days inside the 7-day tab still sees this message even though raw count is ≥ 50. The message is factually wrong in that case (they have many entries; what they don't have is points across distinct days). The same condition holds in `mood-chart.tsx:679` and `medication-compliance-chart.tsx:391`. +- **Recommended fix**: Gate the message on raw measurement count (or `summary.count`), not aggregated `chartData.length`. Two options: + 1. When `chartData.length < 3 && rawCount >= 3`, render a different copy: "Mehr Messtage erforderlich" / "Erfasse an mehreren Tagen für eine Trendlinie" (the underlying constraint is *day diversity*, not entry count). + 2. Render the chart line anyway with the 1-2 points and skip only the regression overlay — the empty-state was added in v1.4.16 B1a for genuinely brand-new accounts; an established user just doesn't need it. +- **Severity rationale**: Marc explicitly flagged this as confusing under "even when data exists". It misleads experienced users into thinking the app forgot their data. + +--- + +## High (should fix before tag) + +### H1 — Switch tap area is 18×32 px, well below WCAG 2.5.5 +- **File**: `/Users/marc/Projects/HealthLog/src/components/ui/switch.tsx:20` +- **What's wrong**: `data-[size=default]:h-[1.15rem] data-[size=default]:w-8` = 18.4×32 px. 24 instances across the app (settings sections, sources-section, dashboard-layout-section, coach-settings-sheet, notification toggles). On a touch device a 18 px tall target is one finger-pad's height — the Radix-Switch root *is* the hit area; the label next to it is not part of the tappable region. v1.4.34 IW-G floored Inputs at 44 px but Switches were missed. +- **Recommended fix**: Wrap the Switch root in a 44×44 padded `