From 395129a18cc94c44480012de11b0fedb720b3c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 11:09:41 +0200 Subject: [PATCH] docs(planning): file v1.4.18 reconcile + v1.4.19 backlog + v1.5 update Wave-D reconcile report at .planning/phase-D-reconcile-report.md; deferred items seeded to .planning/v1419-backlog.md; strategic v1.5 items appended to .planning/v15-backlog.md from the v1.4.18 Product-Lead review. STATE.md marks Wave D done. Prettier sweep also touched the planning markdown table widths; no content change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .planning/ROADMAP.md | 72 +- .planning/STATE.md | 33 +- .planning/phase-0-report.md | 10 +- .planning/phase-A1-A3-report.md | 2 +- .planning/phase-A1-report.md | 8 +- .planning/phase-A2-A6-report.md | 24 +- .planning/phase-A2-report.md | 16 +- .planning/phase-A3-report.md | 2 + .planning/phase-A4-A5-A8b-report.md | 10 +- .planning/phase-A4-report.md | 21 +- .planning/phase-A7-A8a-report.md | 6 +- .planning/phase-B-mobile-report.md | 34 +- .planning/phase-B1-report.md | 16 +- .planning/phase-B1a-report.md | 16 +- .planning/phase-B1b-report.md | 18 +- .planning/phase-B2-report.md | 32 +- .planning/phase-B3-report.md | 9 +- .planning/phase-B4-report.md | 3 +- .planning/phase-B5-report.md | 12 +- .planning/phase-B5a-report.md | 14 +- .planning/phase-B5b-report.md | 10 +- .planning/phase-B5c-report.md | 20 +- .planning/phase-B5d-report.md | 16 +- .planning/phase-B5e-report.md | 16 +- .planning/phase-B7-report.md | 6 +- .planning/phase-B8-report.md | 22 +- .planning/phase-C-report.md | 38 +- .planning/phase-C1-report.md | 64 +- .planning/phase-C2-report.md | 6 +- .planning/phase-C3-report.md | 30 +- .planning/phase-C4-report.md | 18 +- .planning/phase-C5-report.md | 14 +- .planning/phase-D-c1-c2-wire-report.md | 7 +- .planning/phase-D-code-review-findings.md | 664 ++++++++++--- .planning/phase-D-design-findings.md | 710 +++++++++----- .planning/phase-D-product-lead-review.md | 108 ++- .planning/phase-D-reconcile-report.md | 222 ++--- .planning/phase-D-security-findings.md | 500 +++++++--- .planning/phase-D-senior-dev-findings.md | 915 +++++++----------- .planning/phase-D-simplify-findings.md | 617 ++++-------- .../phase-D-v1418-product-lead-review.md | 346 +++++++ .planning/phase-E1-report.md | 46 + .planning/phase-E2-report.md | 104 ++ .planning/phase-E3-report.md | 74 ++ .planning/phase-research-report.md | 6 +- .planning/phase-v1417-hotfix-report.md | 27 +- .planning/phase-wave-a-gate-report.md | 14 +- .planning/v1416-backlog.md | 22 +- .../v1416-research-ai-recommendations.md | 134 +-- .planning/v1417-replace-audit.md | 22 +- .planning/v1418-achievements-research.md | 8 +- .planning/v1419-backlog.md | 156 +++ .planning/v15-backlog.md | 26 + 53 files changed, 3195 insertions(+), 2151 deletions(-) create mode 100644 .planning/phase-D-v1418-product-lead-review.md create mode 100644 .planning/phase-E1-report.md create mode 100644 .planning/phase-E2-report.md create mode 100644 .planning/phase-E3-report.md create mode 100644 .planning/v1419-backlog.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 5919e13b3..7a6cc63ab 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -4,15 +4,15 @@ Milestone: **v1.4.18** (kicked off 2026-05-10) Latest tag at start: v1.4.17 (live in prod, image digest `sha256:936e9cf2…`) -| Phase | Goal | State | -| ----- | ----------------------------------------------------------------------------------------------------------------- | ------- | -| 0 | Bootstrap — STATE+ROADMAP for v1.4.18, git sanity, single chore commit | done | -| A1 | BD-Zielbereich tile — 7T/30T sub-values render real numbers (currently "—" even with data) | pending | -| A2 | `/admin/api-tokens` table scrollbar (3rd attempt) — Playwright live-verify against prod, fix actual overflow | pending | -| A3 | Chart visual revert + per-chart toggles — drop gradient/emoji/auto-mean, ship 3 opt-in overlay toggles | pending | -| B1 | Achievements expansion — research + 15-25 new, hidden Easter-eggs, lock filter, hidden cards | pending | -| D | Multi-agent QA + Product-Lead — code-reviewer, security, design (Apple-Health lens), senior, simplify, product | pending | -| E | Release v1.4.18 — bump, CHANGELOG, tag, GHCR, deploy, /api/version=1.4.18, smoke, docs+landing sync | pending | +| Phase | Goal | State | +| ----- | -------------------------------------------------------------------------------------------------------------- | ------- | +| 0 | Bootstrap — STATE+ROADMAP for v1.4.18, git sanity, single chore commit | done | +| A1 | BD-Zielbereich tile — 7T/30T sub-values render real numbers (currently "—" even with data) | pending | +| A2 | `/admin/api-tokens` table scrollbar (3rd attempt) — Playwright live-verify against prod, fix actual overflow | pending | +| A3 | Chart visual revert + per-chart toggles — drop gradient/emoji/auto-mean, ship 3 opt-in overlay toggles | pending | +| B1 | Achievements expansion — research + 15-25 new, hidden Easter-eggs, lock filter, hidden cards | pending | +| D | Multi-agent QA + Product-Lead — code-reviewer, security, design (Apple-Health lens), senior, simplify, product | pending | +| E | Release v1.4.18 — bump, CHANGELOG, tag, GHCR, deploy, /api/version=1.4.18, smoke, docs+landing sync | pending | Stop conditions: production red, large un-spec'd issue, context cutoff, or budget short of polish — write status doc, do not auto-rollback. @@ -42,33 +42,33 @@ Detailed report: `.planning/phase-v1417-hotfix-report.md`. ## Previous milestone — v1.4.16 (completed 2026-05-10T04:05+02:00) -| Phase | Goal | State | -| -------- | ----------------------------------------------------------------------------------------------------------------- | ----- | -| 0 | Bootstrap — STATE+ROADMAP for v1.4.16 | done | -| A1 | Sidebar admin-expand bug — Admin nav from non-admin route + Gravatar dropdown side-effects | done | -| A2 | BD-Zielbereich real-fix (regression from v1.4.15 A4) — root-cause + E2E | done | -| A3 | `/admin/api-tokens` table responsive — card-list mobile fallback | done | -| A4 | "7-Tage-Schnitt" → "7-Tage-Trend" DE + indicator on ALL charts | done | -| A5 | Top-tile-selector real-fix — widget-id enum drift fixed | done | -| A6 | Medication-chart 7d-trend + target-range — match other charts | done | -| A7 | AI Generator rate-limit 10/h + cache-invalidate-on-new | done | -| A8 | Umlaute encoding bug + login-overview UTF-8 audit + long-window split-half delta | done | -| B1a | Charts visual leap — gradients, baseline, rich tooltip, mood emoji glyphs | done | -| B1b | Insights surface visual leap — page hero, recs grid, summary typography, dashboard preview | done | -| B2 | AI provider settings UX cleanup — single pulldown-driven form, fallback chain editor | done | -| B3 | Admin System-Status host-load chart — CPU/memory/disk-io graph last 2h | done | -| B4 | Admin logs visibility deepening — filterable audit log + structured wide-event tail | done | -| B5a | Medical-reference grounding — AHA / ESC / ESH / WHO bundle + UI citations | done | -| B5b | Multi-provider redundancy — try-each-on-hard-failure, configurable fallback order | done | -| B5c | Per-recommendation explainability — WHY / WINDOW / CITATIONS card with mini-chart | done | -| B5d | Confidence score per recommendation — deterministic 0-100 from sufficiency / recency / signal | done | -| B5e | User-feedback loop — thumbs persisted to RecommendationFeedback + daily aggregator | done | -| B6 | Settings naming-audit — stringency, consistency, no double naming | done | -| B7 | Settings → Export menu (Arztbrief consolidated) — new `/settings/export` route | done | -| B8 | Extended comparison views — Vormonat / Vorjahr overlay across charts + tiles + insights | done | -| C | Catch-up — 5 of 8 deferred HIGH + 3 of 5 mobile MED + docker-publish drop qemu-arm64 | done | -| D | Multi-agent QA + Product-Lead — code-review, security, design, senior, simplify, product-lead | done | -| E1-E3 | Release v1.4.16 — bump, tag, GHCR (both green), host-side retag deploy, GH release, docs+landing sync | done | +| Phase | Goal | State | +| ----- | ----------------------------------------------------------------------------------------------------- | ----- | +| 0 | Bootstrap — STATE+ROADMAP for v1.4.16 | done | +| A1 | Sidebar admin-expand bug — Admin nav from non-admin route + Gravatar dropdown side-effects | done | +| A2 | BD-Zielbereich real-fix (regression from v1.4.15 A4) — root-cause + E2E | done | +| A3 | `/admin/api-tokens` table responsive — card-list mobile fallback | done | +| A4 | "7-Tage-Schnitt" → "7-Tage-Trend" DE + indicator on ALL charts | done | +| A5 | Top-tile-selector real-fix — widget-id enum drift fixed | done | +| A6 | Medication-chart 7d-trend + target-range — match other charts | done | +| A7 | AI Generator rate-limit 10/h + cache-invalidate-on-new | done | +| A8 | Umlaute encoding bug + login-overview UTF-8 audit + long-window split-half delta | done | +| B1a | Charts visual leap — gradients, baseline, rich tooltip, mood emoji glyphs | done | +| B1b | Insights surface visual leap — page hero, recs grid, summary typography, dashboard preview | done | +| B2 | AI provider settings UX cleanup — single pulldown-driven form, fallback chain editor | done | +| B3 | Admin System-Status host-load chart — CPU/memory/disk-io graph last 2h | done | +| B4 | Admin logs visibility deepening — filterable audit log + structured wide-event tail | done | +| B5a | Medical-reference grounding — AHA / ESC / ESH / WHO bundle + UI citations | done | +| B5b | Multi-provider redundancy — try-each-on-hard-failure, configurable fallback order | done | +| B5c | Per-recommendation explainability — WHY / WINDOW / CITATIONS card with mini-chart | done | +| B5d | Confidence score per recommendation — deterministic 0-100 from sufficiency / recency / signal | done | +| B5e | User-feedback loop — thumbs persisted to RecommendationFeedback + daily aggregator | done | +| B6 | Settings naming-audit — stringency, consistency, no double naming | done | +| B7 | Settings → Export menu (Arztbrief consolidated) — new `/settings/export` route | done | +| B8 | Extended comparison views — Vormonat / Vorjahr overlay across charts + tiles + insights | done | +| C | Catch-up — 5 of 8 deferred HIGH + 3 of 5 mobile MED + docker-publish drop qemu-arm64 | done | +| D | Multi-agent QA + Product-Lead — code-review, security, design, senior, simplify, product-lead | done | +| E1-E3 | Release v1.4.16 — bump, tag, GHCR (both green), host-side retag deploy, GH release, docs+landing sync | done | Marathon completed 2026-05-10T04:05+02:00 — v1.4.16 LIVE in prod. Full report: `docs/audit/v1416-summary.md`. v1.5 backlog seeded at diff --git a/.planning/STATE.md b/.planning/STATE.md index b7f91eabf..75bb9c6c3 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -111,17 +111,24 @@ Last update: 2026-05-10T10:02+02:00 ## Wave D — QA + Product-Lead review -- [ ] code-reviewer -- [ ] security review -- [ ] design / UX review (Apple Health benchmarking lens; verify - chart-revert is clean, no leftover gradients) -- [ ] senior-dev review -- [ ] simplify -- [ ] Product Lead — state of app, biggest items since v1.4.16, v1.5 - roadmap update -- [ ] Reconcile applies CRITICAL/HIGH inline -- Detailed report: `.planning/phase-D-report.md` + - `.planning/product-lead-review.md` +- [x] code-reviewer (1 CRITICAL, 5 HIGH, 7 MED/LOW) +- [x] security review (0 CRITICAL, 2 HIGH, 3 MED/LOW) +- [x] design / UX review (0 CRITICAL, 3 HIGH, 11 MED/LOW) +- [x] senior-dev review (0 CRITICAL, 0 HIGH, 6 MED/LOW) +- [x] simplify (7 apply-yes, 0 apply-no) +- [x] Product Lead — strategic v1.5 update filed +- [x] Reconcile applies CRITICAL/HIGH inline +- Status: 2026-05-10T11:08+02:00 — done. C1 hidden-achievement + wire-leak fixed (`545f44c`); 8 of 10 HIGH fixed inline (1 + deferred to v1.4.19 — security HIGH-2 i18n bundle strip needs + a build-time hook), all 7 simplify-yes applied (`720e6c8`). + Format sweep across the tree (`3048dd6`). Tests: 1605/1605 + unit, 66/66 integration. Typecheck + lint + format-check + clean. Backlog seeded to `v1419-backlog.md`; strategic v1.5 + items added to `v15-backlog.md`. Detailed report: + `.planning/phase-D-reconcile-report.md`. Commits on + origin/main: `545f44c`, `720e6c8`, `fbf14fc`, `194ec2f`, + `cf75579`, `c6e3ac6`, `3048dd6`. ## Phase E — Release v1.4.18 @@ -152,6 +159,7 @@ blob when validation fails; rich `` then called `stripChartTokens(undefined)` and crashed. Fix: + - `isLegacyInsightPayload()` now flags blobs missing both `summary` AND `recommendations[]` (the v1.4.14 shape) so the API surfaces `legacyPayload: true` correctly. @@ -169,6 +177,7 @@ pre-existing warnings, `pnpm test` 1547/1547 (+7 defensive tests), `pnpm test:integration` 59/59. Commits on origin/main: + - `79bfa27 fix(insights): handle legacy cached payload without rationale (regenerate CTA)` - `adab80a chore(release): v1.4.17` - `da7070e style(insights): prettier sweep on legacy-payload hotfix files` @@ -222,7 +231,7 @@ keep the rule. hotfix and v1.4.16 entries archived above. Working tree carries 4 untracked stale dotted-segment route directories (`src/app/api/export/{full-backup.json,measurements.csv, - medications.csv,mood.csv}/`) plus 3 untracked v1.4.16 phase +medications.csv,mood.csv}/`) plus 3 untracked v1.4.16 phase reports (`phase-E1-report.md`, `phase-E2-report.md`, `phase-E3-report.md`) — same call as v1.4.16 Phase 0: leave them in place, they belong to previous milestones. Tracked files clean. diff --git a/.planning/phase-0-report.md b/.planning/phase-0-report.md index 801c35c07..f27047018 100644 --- a/.planning/phase-0-report.md +++ b/.planning/phase-0-report.md @@ -9,10 +9,10 @@ Tracked-files baseline: clean (untracked-only working tree on entry). - `.planning/STATE.md` rewritten for v1.4.18: `Status: phase-0-done`, Phase 0 marked complete, Wave A (A1 BD-Zielbereich tile sub-values, A2 `/admin/api-tokens` 3rd-attempt scrollbar, A3 chart visual revert - + per-chart toggles), Wave B (B1 achievements expansion with - research + hidden Easter-eggs + lock filter), Wave D multi-agent QA - + Product-Lead, Phase E release. v1.4.17 hotfix and v1.4.16 release - blocks archived inline above. + - per-chart toggles), Wave B (B1 achievements expansion with + research + hidden Easter-eggs + lock filter), Wave D multi-agent QA + - Product-Lead, Phase E release. v1.4.17 hotfix and v1.4.16 release + blocks archived inline above. - `.planning/ROADMAP.md` rewritten with the v1.4.18 one-liner table for Phase 0 / A1-A3 / B1 / D / E. v1.4.17 hotfix line + the full v1.4.16 / v1.4.15 / v1.4.14 archive tables retained beneath. @@ -31,7 +31,7 @@ commits. Untracked items left in place (same call as v1.4.16 Phase 0): `phase-E3-report.md` — v1.4.16 release reports, belong to that milestone. - `src/app/api/export/{full-backup.json,measurements.csv, - medications.csv,mood.csv}/` — stale dotted-segment route directories; +medications.csv,mood.csv}/` — stale dotted-segment route directories; the live plain-segment routes shipped in B7 of v1.4.16 (per `phase-E1-report.md`). diff --git a/.planning/phase-A1-A3-report.md b/.planning/phase-A1-A3-report.md index e7910a235..bccbd1258 100644 --- a/.planning/phase-A1-A3-report.md +++ b/.planning/phase-A1-A3-report.md @@ -23,7 +23,7 @@ Tests: - `sidebar-nav.test.tsx` — every admin-route case asserts ONLY the single `/admin` link is present, never sub-route links. - New e2e `e2e/sidebar-admin-no-expand.spec.ts` (desktop-only project) - covers (a) /admin/* shows one link no sub-list, (b) clicking Admin + covers (a) /admin/\* shows one link no sub-list, (b) clicking Admin from /dashboard navigates without flashing sub-items, (c) opening the gravatar user-menu does not expand sidebar admin sub-items. diff --git a/.planning/phase-A1-report.md b/.planning/phase-A1-report.md index 5aed61f86..06da6b98a 100644 --- a/.planning/phase-A1-report.md +++ b/.planning/phase-A1-report.md @@ -6,6 +6,7 @@ Started: 2026-05-09T~10:08+02:00 Finished: 2026-05-09T~10:15+02:00 ## Symptom + Marc reported the dashboard "BD im Zielbereich" tile rendered the 30-day headline correctly (**50,0 %**, the v1.4.16 A2 ceiling-semantics fix) but the row underneath the headline still showed **`7T: —`** and @@ -13,6 +14,7 @@ fix) but the row underneath the headline still showed **`7T: —`** and windows. ## Root cause + `/api/analytics` (`src/app/api/analytics/route.ts`) only computed and returned a single `bpInTargetPct` field — the share over the trailing 30 days. The dashboard tile (`src/app/page.tsx`) explicitly passed @@ -27,6 +29,7 @@ Confirmed against Marc's prod data on apps-01: user 10 in last 30 days. Both windows have data; nothing should render "—". ## Fix + Added `computeBpInTargetWindows()` next to the existing `computeBpInTargetPct()` helper in `src/lib/analytics/bp-in-target.ts`. Same predicate + @@ -44,6 +47,7 @@ Headline (`bpInTargetPct`) preserved for cached client bundles + matches `last30Days` so a v1.4.17 PWA displays the same number. ## Tests (TDD) + 1. **Unit** (`src/lib/analytics/__tests__/bp-in-target.test.ts`) — 6 new cases for `computeBpInTargetWindows`: empty input, mixed-window data, null-7d-with-real-30d, default-clock smoke, @@ -54,15 +58,17 @@ matches `last30Days` so a v1.4.17 PWA displays the same number. (avoids the boundary flakiness of integer-day seeds) and assert both windows produce non-null hand-counted shares; second case verifies a user with only 14-day-old data sees `7T = null, - 30T = real`. +30T = real`. ## Verification + - `pnpm test`: **1559 / 1559** green (was 1547; 8 new cases added) - `pnpm test:integration`: **61 / 61** green (was 59; 2 new cases) - `pnpm typecheck`: clean - `pnpm lint`: 12 baseline warnings, **0 new** ## Commit + `23363ca` — `fix(dashboard): wire 7T and 30T sub-values on the BD-Zielbereich tile` diff --git a/.planning/phase-A2-A6-report.md b/.planning/phase-A2-A6-report.md index ed1232f57..31e24ae13 100644 --- a/.planning/phase-A2-A6-report.md +++ b/.planning/phase-A2-A6-report.md @@ -8,17 +8,19 @@ Finished: 2026-05-09T23:35+02:00 ## A2 — BD-Zielbereich root-cause analysis ### Symptom + Marc reported the dashboard tile rendered **0 %** despite multiple BP readings clearly inside the well-controlled range. v1.4.15 phase-A4 report claimed this was fixed (commit `a967895`); it was not. ### Investigation + 1. Read `src/lib/analytics/bp-in-target.ts` (the v1.4.15 fix). It - correctly fixed the *denominator* (paired-readings instead of + correctly fixed the _denominator_ (paired-readings instead of `sysData.length`) and added a same-Berlin-day fallback for imports rounded past the 5-minute pairing window. 2. Read the v1.4.15 phase-A4 report — claim was specifically the - denominator fix. The "in target" *predicate itself* was untouched: + denominator fix. The "in target" _predicate itself_ was untouched: `sys >= sysLow && sys <= sysHigh && dia >= diaLow && dia <= diaHigh`. 3. Pulled Marc's actual BP measurements from production (apps-01, container `db-pg8wggwogo8c4gc4ks0kk4ss-212006924671`, user @@ -35,10 +37,11 @@ report claimed this was fixed (commit `a967895`); it was not. - 124/82 → dia 82 > 79 → OUT - 126/80 → dia 80 > 79 → OUT - 133/95 → both out → OUT - → **0 / 10 = 0 %**. Calculation is reproducing exactly what Marc - sees. **No bug in the v1.4.15 code; the bug is in the semantic.** + → **0 / 10 = 0 %**. Calculation is reproducing exactly what Marc + sees. **No bug in the v1.4.15 code; the bug is in the semantic.** ### Root cause + The "in-target" predicate uses the **ESH 2023 narrow goal band** (the treatment goal for hypertensive patients on therapy: keep BP between sysLow and sysHigh, not below sysLow because sub-band can indicate @@ -50,10 +53,11 @@ well-controlled"). The narrow band counts him out for being healthier than the goal. This is **hypothesis #2** from the brief (Wrong target range), with the -twist that the issue is the *lower bound being too high*, not the +twist that the issue is the _lower bound being too high_, not the upper bound being too low. ### Fix + Centralised the predicate in a new exported `isBpReadingInTarget()` helper (`src/lib/analytics/bp-in-target.ts`) with one-sided ceiling semantics + clinical hypotension floors: @@ -73,6 +77,7 @@ also closes a maintenance trap — pre-fix any future ESH update would need to land in 5 inline locations. ### Tests (TDD-style) + - `src/lib/analytics/__tests__/bp-in-target.test.ts` — 6 new `isBpReadingInTarget()` cases + a regression test that **re-uses Marc's exact production fixture** (10 paired readings → 50 % under @@ -84,18 +89,22 @@ need to land in 5 inline locations. / hypotension-floor edge cases. ### Cache invalidation + Verified `measurementDependentKeys` already includes `queryKeys.analytics()` so adding a BP measurement invalidates the tile's TanStack Query cache (per file `src/lib/query-keys.ts`). The 0 % bug was purely calculation, not staleness. ### Commit + `577d8dd` — `fix(insights): BD-Zielbereich computes correctly for real measurement data` ## A6 — Medication-chart 7d-trend + target-range ### Changes + `src/components/charts/medication-compliance-chart.tsx`: + - New `computeMedicationTrend7d()` exported helper. Mean of recent-7 daily rates − mean of prior-7, capped at 14-day window. Mean-difference rather than slope so a single outlier day doesn't @@ -111,12 +120,14 @@ tile's TanStack Query cache (per file `src/lib/query-keys.ts`). The way HealthChart's targetZone band does for BP/weight tiles. ### Tests + - `src/components/charts/__tests__/medication-compliance-chart.test.tsx` — 6 new `computeMedicationTrend7d()` cases (empty, up, down, stable, 14-day cap, short window). Empty-state assertion that the trend chip is suppressed when no data exists. ### Commit + `9b01c86` — `feat(charts): medication chart matches other charts (7d trend + indicator + target-range)` ## Constraint compliance @@ -151,9 +162,11 @@ process review. ## Files touched (mine only) Created: + - `tests/integration/bp-in-target.test.ts` Modified: + - `src/lib/analytics/bp-in-target.ts` (centralised predicate, new export) - `src/lib/analytics/__tests__/bp-in-target.test.ts` (new tests) - `src/lib/insights/blood-pressure-status.ts` (use helper) @@ -166,5 +179,6 @@ Modified: - `src/components/charts/__tests__/medication-compliance-chart.test.tsx` Commits on origin/main: + - `577d8dd` — A2 fix - `9b01c86` — A6 feature (carries A7's diff inside, see race note) diff --git a/.planning/phase-A2-report.md b/.planning/phase-A2-report.md index b03729f24..aaf44ef20 100644 --- a/.planning/phase-A2-report.md +++ b/.planning/phase-A2-report.md @@ -54,9 +54,10 @@ Pre-fix readings against `https://healthlog.bombeck.io/admin/api-tokens`: Post-fix the strip still reports `scrollWidth > clientWidth` (so swipe and keyboard-arrow scrolling stay), but with `scrollbar-width: none` -+ `::-webkit-scrollbar { display: none }` the painted bar no longer -draws. CSS-isolation probe (`/tmp/v1418-css-probe.html` + -`v1418-css-probe.mjs`) verified the rule applies cleanly. + +- `::-webkit-scrollbar { display: none }` the painted bar no longer + draws. CSS-isolation probe (`/tmp/v1418-css-probe.html` + + `v1418-css-probe.mjs`) verified the rule applies cleanly. Screenshots: `/tmp/v1418-api-tokens-prod-393x851.png` (pre-fix prod), `/tmp/v1418-api-tokens-prod-1920x1080.png` (desktop sanity), @@ -67,8 +68,13 @@ Screenshots: `/tmp/v1418-api-tokens-prod-393x851.png` (pre-fix prod), `src/app/globals.css` — added a small `.no-scrollbar` utility: ```css -.no-scrollbar { scrollbar-width: none; -ms-overflow-style: none; } -.no-scrollbar::-webkit-scrollbar { display: none; } +.no-scrollbar { + scrollbar-width: none; + -ms-overflow-style: none; +} +.no-scrollbar::-webkit-scrollbar { + display: none; +} ``` `src/components/admin/admin-shell.tsx` and diff --git a/.planning/phase-A3-report.md b/.planning/phase-A3-report.md index dfdd200ab..4d2a3c986 100644 --- a/.planning/phase-A3-report.md +++ b/.planning/phase-A3-report.md @@ -11,11 +11,13 @@ seeing them live, and replace them with a per-chart switch surface so each overlay can be opted into independently and persists per user. Hard NO list (memory `feedback_charts_visual_identity.md` v1.4.18): + 1. Gradient area fills under chart lines. 2. Smiley/emoji glyphs on the mood chart's data points. 3. Always-on personal-baseline / mean reference line. Hard YES list: + - Per-chart switches for "7-Tage-Trend / Trend-Pfeil / Zielbereich". - Default state of every switch is OFF; clean line is the new default. diff --git a/.planning/phase-A4-A5-A8b-report.md b/.planning/phase-A4-A5-A8b-report.md index 3a0dd7bef..092bc14e0 100644 --- a/.planning/phase-A4-A5-A8b-report.md +++ b/.planning/phase-A4-A5-A8b-report.md @@ -7,11 +7,11 @@ Finished: 2026-05-09T23:31+02:00 ## Outcomes — 3 of 3 -| # | Fix | Commit | -| --- | ---------------------------------------------------------------- | ------- | -| A4 | "7-Tage-Trend" everywhere + slope30 fallback for sparse metrics | 4df6dac | -| A5 | Top-tile selector — API enum drift fixed; empty strip hidden | 93e712d | -| A8b | "All" filter chart trend — split-half mean-delta supplement | af77e5e | +| # | Fix | Commit | +| --- | --------------------------------------------------------------- | ------- | +| A4 | "7-Tage-Trend" everywhere + slope30 fallback for sparse metrics | 4df6dac | +| A5 | Top-tile selector — API enum drift fixed; empty strip hidden | 93e712d | +| A8b | "All" filter chart trend — split-half mean-delta supplement | af77e5e | ## Test deltas diff --git a/.planning/phase-A4-report.md b/.planning/phase-A4-report.md index 70af1b7af..9588d68f5 100644 --- a/.planning/phase-A4-report.md +++ b/.planning/phase-A4-report.md @@ -7,26 +7,30 @@ Finished: 2026-05-09T20:38+02:00 ## Outcomes — 5 of 5 -| # | Fix | Commit / Note | -|---|--------------------------------------------------------------------|---------------| -| 1 | BD-Zielbereich 0% bug | a967895 (bundled by sibling agent's commit; my Fix 1 work is in `src/lib/analytics/bp-in-target.ts` + 7 unit tests) | -| 2 | Medikamente graph wired to layout toggle | 73afae0 (commit message landed on a sibling agent's sidebar diff; the actual Fix 2 files — `medication-compliance-chart.tsx` + 7 tests — are in `bffdccb`) | -| 3 | Stimmung-chart auto-aggregates to weekly/monthly | 47ac14b — clean | -| 4 | "7-Tage-Schnitt" → "7-Tage-Trend" with delta indicator per metric | 4e2386e — clean | -| 5 | Top tiles independently selectable in layout settings | 8ccdfac — clean | +| # | Fix | Commit / Note | +| --- | ----------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | BD-Zielbereich 0% bug | a967895 (bundled by sibling agent's commit; my Fix 1 work is in `src/lib/analytics/bp-in-target.ts` + 7 unit tests) | +| 2 | Medikamente graph wired to layout toggle | 73afae0 (commit message landed on a sibling agent's sidebar diff; the actual Fix 2 files — `medication-compliance-chart.tsx` + 7 tests — are in `bffdccb`) | +| 3 | Stimmung-chart auto-aggregates to weekly/monthly | 47ac14b — clean | +| 4 | "7-Tage-Schnitt" → "7-Tage-Trend" with delta indicator per metric | 4e2386e — clean | +| 5 | Top tiles independently selectable in layout settings | 8ccdfac — clean | ## Test deltas + - Baseline (start of phase): 758 tests passing, 99 files - End of phase: 812 tests passing, 108 files - Net: +54 unit / SSR-smoke tests across 9 new files ## Race-condition notes (for the v1.4.15 retrospective) + A1, A2, A3 ran in parallel and all wrote to the index. Two interleaving issues: + - Fix 1: my staged files (bp-in-target + test + analytics route edit) got bundled into A2's `feat(admin)` commit. Not amended (per protocol). - Fix 2: my carefully-staged files for the medication chart got committed by A2's agent into their `fix(admin)` push, and my next `git commit` then picked up an unrelated A1 sidebar diff. Result: the commit message for Fix 2 ended up on a 3-line sidebar-nav change (`73afae0`); the Fix-2 code itself shipped under `bffdccb` ("fix(admin): make api-tokens table responsive on mobile"). Both messages are misleading but the working tree is correct. - Fixes 3-5: I waited until after each `git pull --rebase --autostash` cycle, staged with explicit paths, and the commits landed cleanly. ## Visual / UX changes + - Tile strip: every metric tile now shows a signed 7-day delta beside the avg-7 number, coloured per metric sentiment (`up-good` green, `up-bad` orange, `neutral` muted). The label flips from "7T" / "7d" to "7T-Trend" / "7d trend" when the delta is supplied. - Mood chart header: gains the same `bg-muted/40` 10 px uppercase chip the BP/weight charts already paint when bucketing is in effect (Weekly avg / Monatsdurchschnitt). - Medication chart: brand-new line chart (Dracula purple) with a 80 %-target reference line, replaces the static placeholder. @@ -34,6 +38,7 @@ A1, A2, A3 ran in parallel and all wrote to the index. Two interleaving issues: - Settings → Dashboard: each widget row grows a second switch column. Header reads "Tile / Chart" (EN) or "Kachel / Chart" (DE). ## Files touched (excluding sibling-agent collisions) + - `src/lib/analytics/bp-in-target.ts` (new) - `src/lib/analytics/__tests__/bp-in-target.test.ts` (new) - `src/lib/analytics/trend-delta.ts` (new) @@ -54,6 +59,7 @@ A1, A2, A3 ran in parallel and all wrote to the index. Two interleaving issues: - `messages/en.json` + `messages/de.json` (added `trend7dShort`, `layoutTileColumn`, `layoutChartColumn`) ## Constraint compliance + - `pnpm test`: 812 / 812 green at every commit. - `pnpm typecheck`: 0 errors (12 pre-existing `_param`/`_request` warnings). - `pnpm lint`: 0 errors, same 12 warnings as baseline. @@ -61,4 +67,5 @@ A1, A2, A3 ran in parallel and all wrote to the index. Two interleaving issues: - A1/A2/A3/A5 ownership boundaries respected: did not touch `src/components/layout/*`, `src/proxy.ts`, `src/app/admin/**`, `src/components/admin/**`, or the `quick-add` / `mood-tile` / `onboarding-card` components substantially. Mood-tile mobile-fix race noted; A3's layout work is preserved (`mood-list.tsx` modifications are not in any of my commits). ## Deferred to v1.4.16 + None for A4. All five items in scope shipped. diff --git a/.planning/phase-A7-A8a-report.md b/.planning/phase-A7-A8a-report.md index 91c9c63d7..5ee414c50 100644 --- a/.planning/phase-A7-A8a-report.md +++ b/.planning/phase-A7-A8a-report.md @@ -18,12 +18,12 @@ Two coupled fixes for the comprehensive-insight surface: down without redeploys, but a typo can never silently disable the limit. Documented in `.env.example`. Unit-test pin: 10 successes then a 429 with the message `Maximum 10 insight generations per - hour.`; same harness for the env override (3-limit smoke test) and +hour.`; same harness for the env override (3-limit smoke test) and the fallback paths. 2. **Per-status cache invalidation.** Marc reported "stale cached insights from hours ago" — root cause was that `/api/insights/ - generate` only writes the `User.insightsCachedText` blob, while the +generate` only writes the `User.insightsCachedText` blob, while the per-scope status routes (`general-status`, `blood-pressure-status`, `weight-status`, `pulse-status`, `bmi-status`, `mood-status`, `medication-compliance-status`) cache their text in `audit_logs` @@ -64,7 +64,7 @@ tested: 1. **Explicit UTF-8 decode.** `Response.json()` defers its charset decision to the upstream `Content-Type: application/json; - charset=…` header. An intermediate proxy that strips or rewrites +charset=…` header. An intermediate proxy that strips or rewrites that parameter (Cloudflare-on-Cloudflare, a corporate proxy in transit, etc.) could leave the byte stream UTF-8 while the parser thinks it's latin-1, which is exactly the failure mode that drops diff --git a/.planning/phase-B-mobile-report.md b/.planning/phase-B-mobile-report.md index ca599e97b..c53df1776 100644 --- a/.planning/phase-B-mobile-report.md +++ b/.planning/phase-B-mobile-report.md @@ -19,14 +19,14 @@ shipped). Source of truth: `.planning/phase-A5-mobile-findings.md`. - **Finding ID**: A5 / CRITICAL #1 (Recharts `touch-action: auto` causing scroll-lockup). - **Files**: `src/components/charts/{health-chart, mood-chart, - medication-compliance-chart, compliance-line-chart, - scatter-correlation-chart}.tsx`, +medication-compliance-chart, compliance-line-chart, +scatter-correlation-chart}.tsx`, `src/components/charts/__tests__/touch-action-guard.test.ts` (new). - **Change**: Added `touch-pan-y` Tailwind utility to every chart's wrapping `
`. The two charts that previously rendered `` without an explicit wrapper (`compliance- - line-chart`, `scatter-correlation-chart`) gain a `
` parent. +line-chart`, `scatter-correlation-chart`) gain a `
` parent. - **Evidence**: New textual guard test verified to fail on every chart before the fix, then turned green after. 5 / 5 chart files pass the contract; suite stays green at 863 tests total. @@ -42,7 +42,7 @@ shipped). Source of truth: `.planning/phase-A5-mobile-findings.md`. so the filter pills stop colliding with the title's count badge. Card padding adapts `p-4 sm:p-6`. Desktop table gated behind `hidden md:block`. New mobile card-list (`
    `) renders each user as +users-mobile-list" className="md:hidden">`) renders each user as a self-contained card with all four action buttons (toggle role, edit, reset password, force-logout) flex-wrapping below the username + role badge + meta line. Action buttons extracted into @@ -68,7 +68,7 @@ shipped). Source of truth: `.planning/phase-A5-mobile-findings.md`. - A5 / HIGH `/mood` — list edit/delete sizing - A5 / MEDIUM `/medications` — history + edit icon buttons - **Files**: `src/components/charts/{health,mood,medication- - compliance,compliance-line,compliance}-chart{s}.tsx`, +compliance,compliance-line,compliance}-chart{s}.tsx`, `src/components/medications/medication-card.tsx`, `src/components/mood/mood-list.tsx`. - **Change**: Chart range buttons `h-7 → min-h-9`; chart switches @@ -103,27 +103,27 @@ shipped). Source of truth: `.planning/phase-A5-mobile-findings.md`. ## Deferred — picked up by future agent or v1.4.16 -| Audit ID | Reason | Recommended owner | -|---|---|---| -| A5/HIGH `/insights` tab strip overflow | Cross-cutting `tabs.tsx` primitive change touches `/settings/*` + `/admin/*` too. Best done as one consolidated pass. | v1.4.16 design-systems sweep | -| A5/HIGH `/admin` tab strip overflow | Same reason — single fix for both Settings and Admin. | v1.4.16 design-systems sweep | -| A5/HIGH `/measurements` BP grouping (sys + dia → "117/79 mmHg" row) | Logic change to `measurement-list.tsx`, larger scope than tap-target sweep. Doctor-export UX impact. | v1.4.16 measurements polish | -| A5/HIGH `/measurements` hidden table DOM weight | Coupled to BP grouping fix. | v1.4.16 measurements polish | -| A5/HIGH bottom-nav 7-items → 5+More | Requires nav-IA decision (which 2 to demote — Achievements + Targets per audit, but needs Marc sign-off). | v1.4.16 nav-IA review | -| A5/MEDIUM `/admin` system-status loading-failed state | Touches `system-status-section.tsx`; B-agent-3 (notifications) and B1 (backups) are also touching admin sections. | v1.4.16 admin polish | -| A5/MEDIUM tabs h-9 default | Same `tabs.tsx` primitive — group with HIGH tab-strip wrap. | v1.4.16 | +| Audit ID | Reason | Recommended owner | +| ------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | ---------------------------- | +| A5/HIGH `/insights` tab strip overflow | Cross-cutting `tabs.tsx` primitive change touches `/settings/*` + `/admin/*` too. Best done as one consolidated pass. | v1.4.16 design-systems sweep | +| A5/HIGH `/admin` tab strip overflow | Same reason — single fix for both Settings and Admin. | v1.4.16 design-systems sweep | +| A5/HIGH `/measurements` BP grouping (sys + dia → "117/79 mmHg" row) | Logic change to `measurement-list.tsx`, larger scope than tap-target sweep. Doctor-export UX impact. | v1.4.16 measurements polish | +| A5/HIGH `/measurements` hidden table DOM weight | Coupled to BP grouping fix. | v1.4.16 measurements polish | +| A5/HIGH bottom-nav 7-items → 5+More | Requires nav-IA decision (which 2 to demote — Achievements + Targets per audit, but needs Marc sign-off). | v1.4.16 nav-IA review | +| A5/MEDIUM `/admin` system-status loading-failed state | Touches `system-status-section.tsx`; B-agent-3 (notifications) and B1 (backups) are also touching admin sections. | v1.4.16 admin polish | +| A5/MEDIUM tabs h-9 default | Same `tabs.tsx` primitive — group with HIGH tab-strip wrap. | v1.4.16 | ## Cross-agent observations - The shared-cwd race Marc documented in A2 / A4 reproduced again: twice during this phase a `git commit` consumed a sibling agent's staged files instead of mine. **Mitigation**: switched to `git - commit -- ` for atomic stage-and-commit, which fixed it +commit -- ` for atomic stage-and-commit, which fixed it for fixes 3 / 4 / 5. Recommend the v1.4.16 marathon use the `superpowers:using-git-worktrees` skill to give each agent its own working tree. - One pre-existing typecheck error in `src/lib/__tests__/dashboard- - layout.test.ts` (DashboardWidgetId narrow type vs string) is +layout.test.ts` (DashboardWidgetId narrow type vs string) is unrelated to my work — confirmed by stashing my changes and rerunning. Filed as v1.4.16 backlog. - Pre-existing Playwright config + a11y dark-color-scheme work diff --git a/.planning/phase-B1-report.md b/.planning/phase-B1-report.md index 7c5d87752..71a43873b 100644 --- a/.planning/phase-B1-report.md +++ b/.planning/phase-B1-report.md @@ -22,7 +22,7 @@ Roster grew from **38 → 59** (+21 achievements): Hidden achievements render as opaque "Hidden achievement" placeholders in the locked state — the real title, description, -metric and target are *never* written to the DOM, so a curious user +metric and target are _never_ written to the DOM, so a curious user cannot inspect the page source to learn the trigger. Once unlocked, the strings reveal themselves on the same card; the unlock toast gets a longer (8 s) `` celebration with the localized @@ -37,28 +37,28 @@ so deleting data doesn't make completed achievements disappear. ## Commit walk 1. `docs(achievements): v1.4.18 expansion research with 20+ - proposed achievements` — research at +proposed achievements` — research at `.planning/v1418-achievements-research.md`. 2. `feat(achievements): definitions for v1.4.18 expanded set + - hidden Easter-eggs` — types, `ACHIEVEMENT_DEFINITIONS` list, +hidden Easter-eggs` — types, `ACHIEVEMENT_DEFINITIONS` list, EN+DE strings, page mood/hidden category labels, `CATEGORY_LABEL_KEY` exhaustiveness. 3. `feat(achievements): evaluators for new achievements (with - discovery rules)` — wires `buildExpansionMetricValues` + +discovery rules)` — wires `buildExpansionMetricValues` + `applyDiscoveryFilter` into the route, fetches `MoodEntry`, recomputes summary from the visible set. 4. `feat(achievements): page renders unlocked + locked + hidden - achievements with progress` — opaque hidden card, new lucide +achievements with progress` — opaque hidden card, new lucide icons, page tests for the four card states + locale-localized hidden heading. 5. `feat(dashboard): recent-achievements card celebrates hidden - unlocks` — `` toast variant for `isHidden: true`. +unlocks` — `` toast variant for `isHidden: true`. 6. `test(achievements): coverage for evaluators + UI states` — 13 unit assertions against `expansion-metrics.ts`, 3 integration tests round-tripping prisma rows, e2e describe-block exercising all four card states with leak-resistance asserts. 7. `feat(achievements): worker re-evaluates new + hidden - achievements on data change` — fixed two trigger bugs (the +achievements on data change` — fixed two trigger bugs (the doctor-PDF action used a non-existent name; locale-flip now emits its own `settings.locale.update` audit row when the locale actually changes). @@ -93,7 +93,7 @@ so deleting data doesn't make completed achievements disappear. consumes the same filtered list — apps will see exactly what the web sees, including hidden cards as opaque placeholders. iOS clients can render their own "?" affordance from `isHidden: true` - + `unlocked: false`. + - `unlocked: false`. ## Out of scope / followups diff --git a/.planning/phase-B1a-report.md b/.planning/phase-B1a-report.md index 44298319d..8db9781a0 100644 --- a/.planning/phase-B1a-report.md +++ b/.planning/phase-B1a-report.md @@ -19,18 +19,18 @@ Five atomic commits on origin/main (one cross-agent race noted): 1. **`feat(charts): reusable gradient + rich-tooltip primitives`** — landed inside commit `2611bb4 feat(db): User.aiProviderChain for ordered - fallback config` due to a parallel-staging race with the B5b worker +fallback config` due to a parallel-staging race with the B5b worker (B5b ran `git add -A` between my `git add` and `git commit`). My primitives are on origin/main verbatim; the commit subject just belongs to B5b's adjacent work. 2. **`74c2eb8 feat(charts): BP/weight/pulse polish (gradient, baseline, - rich tooltip)`** — single HealthChart wrapper covers all three vital +rich tooltip)`** — single HealthChart wrapper covers all three vital families (BP, weight, pulse, body fat, sleep, steps). 3. **`901f44e feat(charts): mood chart polish with emoji glyphs at data - points`** — bundled with B5b's `provider-runner.ts` files (same race +points`** — bundled with B5b's `provider-runner.ts` files (same race pattern; my charts files content correct). 4. **`8008613 feat(charts): medication chart polish (gradient + - animation + rich tooltip)`** — clean, only my 2 files. +animation + rich tooltip)`** — clean, only my 2 files. Empty-state commit (brief item 7) was integrated into commits 2-4 since the same `` primitive lands per-chart with the polish. @@ -79,7 +79,7 @@ a target band through whenever the user-pref ships. - ✅ Personal baseline line — new `computePersonalBaseline()` helper (90-point rolling median, returns null for <5 points). Painted as a faint dashed `` labelled `"Your normal"` / `"Dein - Mittel"` (i18n keys `charts.personalBaseline`). +Mittel"` (i18n keys `charts.personalBaseline`). - ✅ Empty / sparse-data state — new `` primitive consistently used by every wrapper when daily points < 3. Lucide `LineChart` glyph + i18n title+description (EN+DE). @@ -103,12 +103,12 @@ a target band through whenever the user-pref ships. Three of the four commits absorbed parallel-agent staged work: 1. Primitive files commit landed under B5b's `feat(db): - User.aiProviderChain` subject. My 6 files (`chart-gradient.tsx`, +User.aiProviderChain` subject. My 6 files (`chart-gradient.tsx`, `chart-tooltip.tsx`, `chart-empty-state.tsx`, 3 tests, plus 7 i18n keys EN+DE) are on origin/main inside `2611bb4` — content-verified. 2. Mood polish commit (`901f44e`) absorbed B5b's `provider-runner.ts` - + 2 test files + `insights/generate/route.ts` modifications + AI - provider chain insights routing. My mood-chart edits are intact. + - 2 test files + `insights/generate/route.ts` modifications + AI + provider chain insights routing. My mood-chart edits are intact. 3. The mood-chart edits had to be reapplied twice after a watcher / format-on-save process (likely a parallel agent) reverted my in-flight `Edit` calls back to baseline mid-edit. Final form diff --git a/.planning/phase-B1b-report.md b/.planning/phase-B1b-report.md index 45b8bb594..7e0fb5534 100644 --- a/.planning/phase-B1b-report.md +++ b/.planning/phase-B1b-report.md @@ -9,15 +9,15 @@ rebase needed — origin/main was at `c8e7639` for the entire run). Seven atomic commits on origin/main, each TDD-first (failing tests before implementation): -| # | Commit | What it ships | -|---|--------|---------------| -| 1 | `4d7d074 feat(insights): page hero with gradient header + generation timestamp + personal-baseline indicator` | New `` component renders a Dracula-gradient (purple → cyan, /15 + /8 opacity) band with sparkles glyph, h1 + overview subtitle, "Based on your last 90 days" personal-baseline caption, "Generated " caption, and an optional regenerate button. Pure presentational shell — `/insights/page.tsx` passes the freshest of the per-section cache timestamps in via a new `freshestUpdatedAt()` helper. 10 component tests (gradient classes, slots, EN+DE, regenerate disabled state, missing-timestamp branch). New i18n keys: `insights.heroGenerated`, `heroPersonalBaseline`, `heroRegenerate`, `heroRegenerating`, `relative{JustNow,MinutesAgo,HoursAgo,DaysAgo}` × EN+DE. Animation reuses the existing `animate-insight-in` keyframes which already gates on `prefers-reduced-motion: reduce`. | -| 2 | `9e8be4b feat(insights): recommendations grid with severity ordering + animated reveal` | New `` wraps `` (B5c-built) in a 1-col mobile / 2-col desktop CSS grid with severity-priority ordering (urgent → important → suggestion → info) via a stable `sortRecommendationsBySeverity()` helper. Each card row gets a Dracula-token left border (red/orange/purple/cyan at /70 opacity), a 200ms transition + `md:hover:-translate-y-0.5 md:hover:shadow-lg` lift, and a 100ms-staggered fade-in via `animationDelay`. Internal slots of `` untouched per brief — the shell wraps the card. Refactored the card's outer element from `
  • ` to `
    ` (the only sensible way to hoist grid/flex layout one level up without breaking valid HTML5 list semantics). `` now consumes the grid instead of its old `
      ` + `.map`. 8 grid tests + the existing 12 advisor-card tests still green. | -| 3 | `b76351e feat(insights): polished summary typography with inline-chart sparklines` | Bumps the summary slot in `` from `text-muted-foreground text-sm` to `text-foreground/90 max-w-prose text-base leading-relaxed`. The inline-chart-token renderer (`metric:` allowlist from earlier phases) gains a `mini` flag; the summary surface passes `mini` so its embedded charts shrink to sparkline form (140px). Per-finding charts keep the full chart so they remain interactive. 3 new typography tests. | -| 4 | `0c287f1 feat(insights): polished loading + empty + error states` | Loading state replaces the spinner-only placeholder with a 3-card skeleton grid (`bg-muted/40-60` pulse with 100ms staggered delay) that mirrors the final layout so the page doesn't visually jump on content load. Empty state gains a centred sparkle glyph in a `bg-dracula-purple/10` circle + max-w-prose copy. Error state moves from a flat banner to a card with a discoverable `data-slot="insight-retry-button"` retry button when an `onRegenerate` handler is supplied. 4 new state tests. | -| 5 | `d2cdf9d feat(dashboard): InsightsCardPreview matches insights page visual language` | Replaces the orphan v1.4.0 `` (zero non-test imports; bound to the deprecated `changed/stable/drivers/nextSteps` schema) with a leaner `` that takes an `insight: InsightResult \| null` prop and renders the top 1-2 severity-ordered recs as compact tiles + a "View all" CTA pointing at `/insights`. Same visual language as the page: severity-coloured left border, ring-variant `` inline. Returns `null` when there are no recs (the dashboard's parent grid doesn't render an empty box). New i18n keys: `insights.viewAll`, `previewEmpty` × EN+DE. 8 preview tests. | -| 6 | `5063ad7 style(insights): dark-mode contrast verification + tweaks` | Bumps the page-hero gradient from `/10 + /5` to `/15 + /8` and the border from `/20` to `/25` so the band's lightness delta against the dark Dracula background sits at ~17%, clearing the 3:1 UI-element contrast bar. Skeleton placeholders go from `/30/40/50` to `/40/50/60`. Adds a regression test that pins the contrast-relevant utility classes via regex so a future refactor can't silently drop them. | -| 7 | `3b0d21e test(insights): coverage for polished page + dashboard preview` | New SSR integration suite exercises `` + `` + `` + `` + `` all painting together so the cross-feature slot positions don't regress (the B5c report flagged that an e2e isn't feasible because `/insights` doesn't currently mount the advisor card; the SSR test covers the same surface end-to-end). Pins severity-ordering preservation under low-confidence recs and the empty-grid unmount path. 3 integration tests. | +| # | Commit | What it ships | +| --- | ------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `4d7d074 feat(insights): page hero with gradient header + generation timestamp + personal-baseline indicator` | New `` component renders a Dracula-gradient (purple → cyan, /15 + /8 opacity) band with sparkles glyph, h1 + overview subtitle, "Based on your last 90 days" personal-baseline caption, "Generated " caption, and an optional regenerate button. Pure presentational shell — `/insights/page.tsx` passes the freshest of the per-section cache timestamps in via a new `freshestUpdatedAt()` helper. 10 component tests (gradient classes, slots, EN+DE, regenerate disabled state, missing-timestamp branch). New i18n keys: `insights.heroGenerated`, `heroPersonalBaseline`, `heroRegenerate`, `heroRegenerating`, `relative{JustNow,MinutesAgo,HoursAgo,DaysAgo}` × EN+DE. Animation reuses the existing `animate-insight-in` keyframes which already gates on `prefers-reduced-motion: reduce`. | +| 2 | `9e8be4b feat(insights): recommendations grid with severity ordering + animated reveal` | New `` wraps `` (B5c-built) in a 1-col mobile / 2-col desktop CSS grid with severity-priority ordering (urgent → important → suggestion → info) via a stable `sortRecommendationsBySeverity()` helper. Each card row gets a Dracula-token left border (red/orange/purple/cyan at /70 opacity), a 200ms transition + `md:hover:-translate-y-0.5 md:hover:shadow-lg` lift, and a 100ms-staggered fade-in via `animationDelay`. Internal slots of `` untouched per brief — the shell wraps the card. Refactored the card's outer element from `
    1. ` to `
      ` (the only sensible way to hoist grid/flex layout one level up without breaking valid HTML5 list semantics). `` now consumes the grid instead of its old `
        ` + `.map`. 8 grid tests + the existing 12 advisor-card tests still green. | +| 3 | `b76351e feat(insights): polished summary typography with inline-chart sparklines` | Bumps the summary slot in `` from `text-muted-foreground text-sm` to `text-foreground/90 max-w-prose text-base leading-relaxed`. The inline-chart-token renderer (`metric:` allowlist from earlier phases) gains a `mini` flag; the summary surface passes `mini` so its embedded charts shrink to sparkline form (140px). Per-finding charts keep the full chart so they remain interactive. 3 new typography tests. | +| 4 | `0c287f1 feat(insights): polished loading + empty + error states` | Loading state replaces the spinner-only placeholder with a 3-card skeleton grid (`bg-muted/40-60` pulse with 100ms staggered delay) that mirrors the final layout so the page doesn't visually jump on content load. Empty state gains a centred sparkle glyph in a `bg-dracula-purple/10` circle + max-w-prose copy. Error state moves from a flat banner to a card with a discoverable `data-slot="insight-retry-button"` retry button when an `onRegenerate` handler is supplied. 4 new state tests. | +| 5 | `d2cdf9d feat(dashboard): InsightsCardPreview matches insights page visual language` | Replaces the orphan v1.4.0 `` (zero non-test imports; bound to the deprecated `changed/stable/drivers/nextSteps` schema) with a leaner `` that takes an `insight: InsightResult \| null` prop and renders the top 1-2 severity-ordered recs as compact tiles + a "View all" CTA pointing at `/insights`. Same visual language as the page: severity-coloured left border, ring-variant `` inline. Returns `null` when there are no recs (the dashboard's parent grid doesn't render an empty box). New i18n keys: `insights.viewAll`, `previewEmpty` × EN+DE. 8 preview tests. | +| 6 | `5063ad7 style(insights): dark-mode contrast verification + tweaks` | Bumps the page-hero gradient from `/10 + /5` to `/15 + /8` and the border from `/20` to `/25` so the band's lightness delta against the dark Dracula background sits at ~17%, clearing the 3:1 UI-element contrast bar. Skeleton placeholders go from `/30/40/50` to `/40/50/60`. Adds a regression test that pins the contrast-relevant utility classes via regex so a future refactor can't silently drop them. | +| 7 | `3b0d21e test(insights): coverage for polished page + dashboard preview` | New SSR integration suite exercises `` + `` + `` + `` + `` all painting together so the cross-feature slot positions don't regress (the B5c report flagged that an e2e isn't feasible because `/insights` doesn't currently mount the advisor card; the SSR test covers the same surface end-to-end). Pins severity-ordering preservation under low-confidence recs and the empty-grid unmount path. 3 integration tests. | ## Verification diff --git a/.planning/phase-B2-report.md b/.planning/phase-B2-report.md index 821115d85..50a12b6f1 100644 --- a/.planning/phase-B2-report.md +++ b/.planning/phase-B2-report.md @@ -11,12 +11,12 @@ Completed 2026-05-10 ~01:15 CEST on `agent/b2-ai-provider-ux` worktree. Four atomic TDD-first commits on `agent/b2-ai-provider-ux`, ready to fast-forward into `origin/main`: -| # | Commit | What it ships | -| --- | ----------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| 1 | `feat(api): PUT /api/insights/provider-chain persists user fallback chain` | New `PUT` writes the chain to `User.aiProviderChain`. Server-side priority is recomputed from insertion order so the UI's visual contract ("first row = priority 1") cannot drift from what gets persisted. Unknown provider types and duplicates 422; an empty chain is rejected outright. `parseProviderChain()` still falls back to the default chain when the persisted JSON is malformed, so a stale-tab resubmit cannot poison generation. 5 unit tests pin the contract. | -| 2 | `feat(settings): AI provider section with single dropdown driving form below` | The big one. Single Pulldown (`ai-active-provider-select`) drives a switch-rendered config card below: Codex (connect/disconnect/status + last-insight + CODEX_MODEL note), OpenAI (API key + model select + collapsed Base URL override), Anthropic (API key + model), Local (Base URL + key + model), Admin OpenAI (read-only). Fallback chain card with up/down arrow reorder + per-row enable Switch + remove + add + reset-to-defaults. URL-synced via `?provider=…` so deep links work and the SSR test drives each branch deterministically. | -| 3 | `test(insights): integration coverage for PUT /api/insights/provider-chain` | Two scenarios pinned end-to-end against the postgres testcontainer: saving a valid reordered chain persists the JSON column with priority normalised; saving an empty chain rejects with 422 + leaves the column untouched. | -| 4 | `test(e2e): AI provider dropdown switches the rendered config form` | Playwright spec confirms the dropdown switches every config form (Codex / OpenAI / Anthropic / Local) and the fallback chain card surfaces every row with the right `data-chain-row` markers. Mocks every backend endpoint the section reads so no real keys are needed in CI. | +| # | Commit | What it ships | +| --- | ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `feat(api): PUT /api/insights/provider-chain persists user fallback chain` | New `PUT` writes the chain to `User.aiProviderChain`. Server-side priority is recomputed from insertion order so the UI's visual contract ("first row = priority 1") cannot drift from what gets persisted. Unknown provider types and duplicates 422; an empty chain is rejected outright. `parseProviderChain()` still falls back to the default chain when the persisted JSON is malformed, so a stale-tab resubmit cannot poison generation. 5 unit tests pin the contract. | +| 2 | `feat(settings): AI provider section with single dropdown driving form below` | The big one. Single Pulldown (`ai-active-provider-select`) drives a switch-rendered config card below: Codex (connect/disconnect/status + last-insight + CODEX_MODEL note), OpenAI (API key + model select + collapsed Base URL override), Anthropic (API key + model), Local (Base URL + key + model), Admin OpenAI (read-only). Fallback chain card with up/down arrow reorder + per-row enable Switch + remove + add + reset-to-defaults. URL-synced via `?provider=…` so deep links work and the SSR test drives each branch deterministically. | +| 3 | `test(insights): integration coverage for PUT /api/insights/provider-chain` | Two scenarios pinned end-to-end against the postgres testcontainer: saving a valid reordered chain persists the JSON column with priority normalised; saving an empty chain rejects with 422 + leaves the column untouched. | +| 4 | `test(e2e): AI provider dropdown switches the rendered config form` | Playwright spec confirms the dropdown switches every config form (Codex / OpenAI / Anthropic / Local) and the fallback chain card surfaces every row with the right `data-chain-row` markers. Mocks every backend endpoint the section reads so no real keys are needed in CI. | ## Acceptance criteria mapping @@ -39,22 +39,22 @@ fast-forward into `origin/main`: `settings.ai.activeProvider*`, `settings.ai.providerSelect.*`, `settings.ai.providerConfigTitle`, `settings.ai.providerChain.{title, description, moveUp, moveDown, - removeFromChain, addProvider, saveOrder, saved, saveFailed, - resetDefaults, resetConfirm*}`, +removeFromChain, addProvider, saveOrder, saved, saveFailed, +resetDefaults, resetConfirm*}`, `settings.ai.openai.{modelSelect, modelOptionCustom, - modelCustomLabel, baseUrl*, showAdvanced, hideAdvanced, apiKey, - apiKeyPlaceholder}`, +modelCustomLabel, baseUrl*, showAdvanced, hideAdvanced, apiKey, +apiKeyPlaceholder}`, `settings.ai.codex.{statusConnected, statusDisconnected, - statusExpired, connectButton, disconnectButton, modelSlugLabel, - modelSlugBody, lastInsight}`, +statusExpired, connectButton, disconnectButton, modelSlugLabel, +modelSlugBody, lastInsight}`, `settings.ai.adminOpenai.{title, body, notConfigured}`, `settings.ai.{testProvider, testSuccess, testFailedShort}`. The i18n parity guard test (`src/lib/__tests__/i18n*.test.ts`) is green. - **#6 Tests** — 5 new PUT route tests + 8 new component SSR tests - + 2 integration tests + 1 Playwright spec (covers form-switch - contract + chain row markers). The existing `` smoke - test in `sections.test.tsx` still passes (mock now exposes - `useSearchParams`). + - 2 integration tests + 1 Playwright spec (covers form-switch + contract + chain row markers). The existing `` smoke + test in `sections.test.tsx` still passes (mock now exposes + `useSearchParams`). ## Verification diff --git a/.planning/phase-B3-report.md b/.planning/phase-B3-report.md index 7be4d8b73..b46c4f469 100644 --- a/.planning/phase-B3-report.md +++ b/.planning/phase-B3-report.md @@ -9,8 +9,9 @@ verification gate. Picked Option B (in-process sampler) over Option A (Coolify Sentinel scrape). Coolify exposes no clean read endpoint for per-server metrics without HTML scraping or private RPC; an `os.loadavg()` + `os.totalmem()` -+ /proc/diskstats sampler is self-contained, costs one row per minute, -and works on macOS dev as well as Linux production. + +- /proc/diskstats sampler is self-contained, costs one row per minute, + and works on macOS dev as well as Linux production. ## Deliverables @@ -43,7 +44,7 @@ and works on macOS dev as well as Linux production. ## Tests - 8 unit tests for the sampler (env retention parsing, disk-stats null - + throw fallbacks, create + deleteMany call shape). + - throw fallbacks, create + deleteMany call shape). - 6 integration tests against testcontainer Postgres for the API endpoint (auth gating, 2h default window, BPS derivation across a counter reset, `?since=30m` preset, empty-DB happy path). @@ -76,7 +77,7 @@ even simple `git add path/to/file && git commit` racy. - `tests/integration/admin-host-metrics.test.ts` - `src/components/admin/host-metrics-chart.tsx` + `__tests__/host-metrics-chart.test.tsx` - `src/components/admin/system-status-section.tsx` (wiring) -- `messages/en.json`, `messages/de.json` (admin.hostMetrics.*) +- `messages/en.json`, `messages/de.json` (admin.hostMetrics.\*) - `tests/integration/setup.ts` (truncate `host_metrics`) ## Did NOT touch (scope guards) diff --git a/.planning/phase-B4-report.md b/.planning/phase-B4-report.md index 7d7654501..a0ebd4329 100644 --- a/.planning/phase-B4-report.md +++ b/.planning/phase-B4-report.md @@ -3,6 +3,7 @@ Status: complete Date: 2026-05-10T00:11+02:00 Commits on origin/main: + - `8ac5602` (audit-log filtering, pagination, CSV export — bundled with the Wave-A verification-gate planning commit due to the marathon coordinator's stash race; my code, wrong commit subject) @@ -43,7 +44,7 @@ diagnostics. `GET /api/admin/app-logs` reads the buffer with `traceId` / `level` / `action` / `since` / `until` / `limit` filters, runs every event -through `redactSecrets()` on egress so a stray Bearer / hlk_ / sk- +through `redactSecrets()` on egress so a stray Bearer / hlk\_ / sk- token in `error.message` never leaks to the admin UI; storage stays raw to keep the diagnostic value when shipped to Loki. diff --git a/.planning/phase-B5-report.md b/.planning/phase-B5-report.md index cf08cc67b..aac129f5a 100644 --- a/.planning/phase-B5-report.md +++ b/.planning/phase-B5-report.md @@ -12,12 +12,12 @@ completion in the DB and offer a Settings → Account replay button. ## Acceptance criteria -| # | Criterion | Status | Commit | -| - | ----------------------------------------------------------- | ------ | ------- | -| 1 | DB migration `users.onboarding_tour_completed` | done | db5a49d | -| 2 | Tour component with spotlight + keyboard a11y | done | e57fc0a | -| 3 | Auto-launch on first dashboard load + persist on close | done | 8215e25 | -| 4 | Settings → Account → Restart tour button | done | fa1c6a6 | +| # | Criterion | Status | Commit | +| --- | ------------------------------------------------------ | ------ | ------- | +| 1 | DB migration `users.onboarding_tour_completed` | done | db5a49d | +| 2 | Tour component with spotlight + keyboard a11y | done | e57fc0a | +| 3 | Auto-launch on first dashboard load + persist on close | done | 8215e25 | +| 4 | Settings → Account → Restart tour button | done | fa1c6a6 | ## Architecture diff --git a/.planning/phase-B5a-report.md b/.planning/phase-B5a-report.md index 79b688e35..b5dab272e 100644 --- a/.planning/phase-B5a-report.md +++ b/.planning/phase-B5a-report.md @@ -7,13 +7,13 @@ Completed 2026-05-10 ~00:04 CEST. Five atomic commits on origin/main, each TDD-first (failing test, then fix, then verification): -| # | Commit | What it ships | -|---|--------|---------------| -| 1 | `27f3933 feat(ai): curated medical-reference bundle for citation grounding` | `src/lib/ai/medical-references.ts` with 7 guidelines (ESH 2023, ESC 2024, ACC/AHA 2017, WHO 2021 hypertension, WHO BMI, DGE 2024 nutrition, AHA pulse). Each entry: stable slug, EN+DE titles, https URL, year, scope tags, metric buckets. `selectReferencesForMetrics()` filters by overlap; `getMedicalReferenceById()` lookups defensively return undefined. 15 unit tests. | -| 2 | `466b8b5 feat(ai): schema accepts validated referenceId pointing into the medical-reference bundle` | `aiRecommendationSchema.referenceId` (optional) validated via `superRefine` against `MEDICAL_REFERENCE_IDS`. Fabricated ids fail parse — defence in depth on top of the prompt instruction. `PROMPT_VERSION` bumped to `4.16.0`. 8 unit tests. | -| 3 | `7fbfca6 feat(ai): system prompt includes curated medical references for citation grounding` | New `buildSystemPromptWithReferences(locale, metrics)` injects a SOURCES block listing only references whose `metricApplicability` overlaps the current metrics, plus a GROUND RULE telling the model to cite by id. Plain `getStrictInsightsSystemPrompt(locale)` unchanged for back-compat. 10 unit tests. | -| 4 | `53aade9 feat(ai): post-validation logs citation-coverage rate per insight generation` | `src/lib/ai/citation-coverage.ts` — `detectsNormativeClaim()` (EN+DE keyword heuristic) + `computeCitationCoverage()`. The wrapper at `generate-insight.ts` calls `annotate()` with `ai_total_recommendations` / `ai_normative_recommendations` / `ai_cited_normative_recommendations` / `ai_uncited_normative_recommendation_ids` so the admin AI quality dashboard can chart coverage over time. Observational only; B5c flips to required for severity ≥ "important". 15 unit tests. | -| 5 | `a66c128 feat(insights): citation footnote on recommendations with medical reference` | `` subcomponent in `insight-advisor-card.tsx` renders "Source: ESH 2023 — title" with `` icon, opens in a new tab via `target="_blank" rel="noreferrer"`. `InsightResult.recommendations` widens to a union (legacy string OR `{ text, referenceId? }`) so existing render paths keep working. New i18n keys `insights.recommendation.source` + `insights.recommendation.viewSource` in EN+DE. 6 component tests. | +| # | Commit | What it ships | +| --- | --------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `27f3933 feat(ai): curated medical-reference bundle for citation grounding` | `src/lib/ai/medical-references.ts` with 7 guidelines (ESH 2023, ESC 2024, ACC/AHA 2017, WHO 2021 hypertension, WHO BMI, DGE 2024 nutrition, AHA pulse). Each entry: stable slug, EN+DE titles, https URL, year, scope tags, metric buckets. `selectReferencesForMetrics()` filters by overlap; `getMedicalReferenceById()` lookups defensively return undefined. 15 unit tests. | +| 2 | `466b8b5 feat(ai): schema accepts validated referenceId pointing into the medical-reference bundle` | `aiRecommendationSchema.referenceId` (optional) validated via `superRefine` against `MEDICAL_REFERENCE_IDS`. Fabricated ids fail parse — defence in depth on top of the prompt instruction. `PROMPT_VERSION` bumped to `4.16.0`. 8 unit tests. | +| 3 | `7fbfca6 feat(ai): system prompt includes curated medical references for citation grounding` | New `buildSystemPromptWithReferences(locale, metrics)` injects a SOURCES block listing only references whose `metricApplicability` overlaps the current metrics, plus a GROUND RULE telling the model to cite by id. Plain `getStrictInsightsSystemPrompt(locale)` unchanged for back-compat. 10 unit tests. | +| 4 | `53aade9 feat(ai): post-validation logs citation-coverage rate per insight generation` | `src/lib/ai/citation-coverage.ts` — `detectsNormativeClaim()` (EN+DE keyword heuristic) + `computeCitationCoverage()`. The wrapper at `generate-insight.ts` calls `annotate()` with `ai_total_recommendations` / `ai_normative_recommendations` / `ai_cited_normative_recommendations` / `ai_uncited_normative_recommendation_ids` so the admin AI quality dashboard can chart coverage over time. Observational only; B5c flips to required for severity ≥ "important". 15 unit tests. | +| 5 | `a66c128 feat(insights): citation footnote on recommendations with medical reference` | `` subcomponent in `insight-advisor-card.tsx` renders "Source: ESH 2023 — title" with `` icon, opens in a new tab via `target="_blank" rel="noreferrer"`. `InsightResult.recommendations` widens to a union (legacy string OR `{ text, referenceId? }`) so existing render paths keep working. New i18n keys `insights.recommendation.source` + `insights.recommendation.viewSource` in EN+DE. 6 component tests. | ## Verification diff --git a/.planning/phase-B5b-report.md b/.planning/phase-B5b-report.md index 2f3ebf89c..2f61b5165 100644 --- a/.planning/phase-B5b-report.md +++ b/.planning/phase-B5b-report.md @@ -6,12 +6,12 @@ Completed 2026-05-10 ~00:40 CEST. Four atomic commits on origin/main, each TDD-first (failing test then fix): -| # | Commit | What it ships | -| --- | ------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | `2611bb4 feat(db): User.aiProviderChain for ordered fallback config` | New `User.aiProviderChain` JSONB column + migration `0033_user_ai_provider_chain`. `parseProviderChain()` defensively coerces malformed input back to `PROVIDER_CHAIN_DEFAULT` (codex → openai → anthropic → local → admin-openai). 15 unit tests. Credentials still live in the existing AES-256-GCM columns; the chain only metadata. | +| # | Commit | What it ships | +| --- | --------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `2611bb4 feat(db): User.aiProviderChain for ordered fallback config` | New `User.aiProviderChain` JSONB column + migration `0033_user_ai_provider_chain`. `parseProviderChain()` defensively coerces malformed input back to `PROVIDER_CHAIN_DEFAULT` (codex → openai → anthropic → local → admin-openai). 15 unit tests. Credentials still live in the existing AES-256-GCM columns; the chain only metadata. | | 2 | `901f44e feat(charts): mood chart polish ...` (cross-agent race captured the actual subject) — bundled my B5b runner + route work | `src/lib/ai/provider-runner.ts` exports `runWithFallback()` (strict-schema variant, used when the route migrates to `generateInsight()` in B5c) AND `runRawCompletionWithFallback()` (legacy-shape variant the current route consumes). Hard-fail policy: 401/403/429/5xx/transport cascade; schema 422 bubbles. Last-working-provider cache: 1h TTL, in-process per-userId. Five `ai_chain_*` Wide-Event annotations per generation. | -| 3 | `613d661 feat(settings): minimal display of active AI provider + configured chain` | New `GET /api/insights/provider-chain` returns `{ activeProvider, cachedActiveProvider, configuredChain }`. `` subcomponent in `` renders "Active provider: ChatGPT (Codex)" + "Configured chain: ..." with a "(cached: …)" suffix when the runner has rerouted. EN+DE i18n keys under `settings.ai.providerChain.*`. Hides itself when no provider is configured. | -| 4 | `d2bda42 test(ai): provider fallback covers happy path, hard-fail cascade, cache` | Integration test against the postgres testcontainer with two user-level providers (openai + anthropic) and a fetch-spy intercept routing on URL prefix. Three scenarios pinned: hard-fail-then-success cascade, all-fail cascade, and last-working-cache reorder. | +| 3 | `613d661 feat(settings): minimal display of active AI provider + configured chain` | New `GET /api/insights/provider-chain` returns `{ activeProvider, cachedActiveProvider, configuredChain }`. `` subcomponent in `` renders "Active provider: ChatGPT (Codex)" + "Configured chain: ..." with a "(cached: …)" suffix when the runner has rerouted. EN+DE i18n keys under `settings.ai.providerChain.*`. Hides itself when no provider is configured. | +| 4 | `d2bda42 test(ai): provider fallback covers happy path, hard-fail cascade, cache` | Integration test against the postgres testcontainer with two user-level providers (openai + anthropic) and a fetch-spy intercept routing on URL prefix. Three scenarios pinned: hard-fail-then-success cascade, all-fail cascade, and last-working-cache reorder. | ## Verification diff --git a/.planning/phase-B5c-report.md b/.planning/phase-B5c-report.md index c97a50aba..df51b3300 100644 --- a/.planning/phase-B5c-report.md +++ b/.planning/phase-B5c-report.md @@ -7,16 +7,16 @@ Completed 2026-05-10 ~01:17 CEST. Eight atomic commits on origin/main (worktree: `agent/b5c-explainability`, clean rebase + push to `main`), each TDD-first (failing test, then fix): -| # | Commit | What it ships | -|---|--------|---------------| -| 1 | `8a438a0 feat(ai): schema requires rationale (window + comparedTo + deviation) per recommendation` | New `aiRecommendationRationaleSchema` + mandatory `rationale` field on `aiRecommendationSchema`. Empty `comparedTo` / `deviation` rejected; `dataWindow` enum locked to the 4-window vocabulary. New `findRecommendationsMissingRationale()` for legacy-payload detection. 10 unit tests + 5 fixture-touch updates across `generate-insight.test.ts`, `reference-id-schema.test.ts`, `citation-enforcement.test.ts`, `citation-coverage-logging.test.ts`. | -| 2 | `c39a527 feat(ai): system prompt requires rationale per recommendation` | EN+DE prompt advertises the new field shape AND a fifth GROUND RULE mandating rationale on every rec. `rationale.dataWindow MUST equal metricSource.timeRange` is enforced at prompt-time so the mini-chart pinning is bracket-mismatch-free. PROMPT_VERSION stays at 4.16.0 (B5a already bumped). 6 prompt-assertion tests. | -| 3 | `13f1ae5 feat(ai): corrective retry covers missing rationale fields` | The wrapper's `buildRetryCorrectionMessage()` now lists the three rationale fields explicitly so a first-attempt response without rationale gets a targeted reprompt. Three new `ai_rationale_*` Wide-Event keys land alongside the existing citation-coverage annotations so the admin AI dashboard can chart rationale-coverage per generation. 4 unit tests. | -| 4 | `c8b30c1 feat(ai): legacy insight payload detection with regenerate CTA` | New `isLegacyInsightPayload()` helper + route wiring: `/api/insights/generate` returns `legacyPayload: true` on cache-hit when the cached blob predates B5c rationale. UI shows a one-time "Insights updated — regenerate for new explainability features" CTA pointing at the regenerate button. User-initiated only; no auto-regen on cache-hit (would burn rate-limit tokens silently). 7 unit tests. | -| 5 | `7f54c0c feat(charts): mini-mode + windowOverride prop for embedded rationale charts` | HealthChart gains `mini` (drops range tabs + toggle row, h-[140px]) and `windowOverride` (pins to a rationale enum value). New pure helper `resolveMiniRangePoints()` (`mini-window.ts`) maps the rationale enum onto the existing per-points TIME_RANGES_KEYS. 7 unit tests. | -| 6 | `680f84c feat(charts): mood chart mini-mode + windowOverride` | MoodChart mirrors the same `mini` + `windowOverride` contract so mood-typed recommendations get a dedicated emoji-glyph mini-chart. | -| 7 | `10a67ff feat(insights): RecommendationCard with expandable rationale + mini-chart of data window` | New `` subcomponent with severity badge, default-collapsed chevron, animated 200ms expand reveal, 3-row `` (Window / Compared to / Deviation), pinned mini-chart (HealthChart for measurement metrics, MoodChart for mood-keyed recs), citation footnote, and named slots for B5d (`data-slot="rec-confidence-slot"`) and B5e (`data-slot="rec-feedback-slot"`). InsightAdvisorCard refactored to consume it; `legacyPayload` prop surfaces the regenerate CTA banner. InsightRecommendation type widens to optionally carry rationale + metricSource + severity + id. EN+DE i18n keys: `rationale`, `rationaleWindow`, `rationaleComparedTo`, `rationaleDeviation`, `rationaleExpand`, `rationaleCollapse`, `legacyPayloadCta`. 11 component tests. | -| 8 | `fed2e7e test(insights): coverage for RecommendationCard expand + rationale rendering` | Full-flow integration test: MockAIProvider → `generateInsight()` → InsightAdvisorCard → SSR markup assertion. Second case pins the legacyPayload CTA path with a string-only recommendations[] payload. 2 tests. | +| # | Commit | What it ships | +| --- | -------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `8a438a0 feat(ai): schema requires rationale (window + comparedTo + deviation) per recommendation` | New `aiRecommendationRationaleSchema` + mandatory `rationale` field on `aiRecommendationSchema`. Empty `comparedTo` / `deviation` rejected; `dataWindow` enum locked to the 4-window vocabulary. New `findRecommendationsMissingRationale()` for legacy-payload detection. 10 unit tests + 5 fixture-touch updates across `generate-insight.test.ts`, `reference-id-schema.test.ts`, `citation-enforcement.test.ts`, `citation-coverage-logging.test.ts`. | +| 2 | `c39a527 feat(ai): system prompt requires rationale per recommendation` | EN+DE prompt advertises the new field shape AND a fifth GROUND RULE mandating rationale on every rec. `rationale.dataWindow MUST equal metricSource.timeRange` is enforced at prompt-time so the mini-chart pinning is bracket-mismatch-free. PROMPT_VERSION stays at 4.16.0 (B5a already bumped). 6 prompt-assertion tests. | +| 3 | `13f1ae5 feat(ai): corrective retry covers missing rationale fields` | The wrapper's `buildRetryCorrectionMessage()` now lists the three rationale fields explicitly so a first-attempt response without rationale gets a targeted reprompt. Three new `ai_rationale_*` Wide-Event keys land alongside the existing citation-coverage annotations so the admin AI dashboard can chart rationale-coverage per generation. 4 unit tests. | +| 4 | `c8b30c1 feat(ai): legacy insight payload detection with regenerate CTA` | New `isLegacyInsightPayload()` helper + route wiring: `/api/insights/generate` returns `legacyPayload: true` on cache-hit when the cached blob predates B5c rationale. UI shows a one-time "Insights updated — regenerate for new explainability features" CTA pointing at the regenerate button. User-initiated only; no auto-regen on cache-hit (would burn rate-limit tokens silently). 7 unit tests. | +| 5 | `7f54c0c feat(charts): mini-mode + windowOverride prop for embedded rationale charts` | HealthChart gains `mini` (drops range tabs + toggle row, h-[140px]) and `windowOverride` (pins to a rationale enum value). New pure helper `resolveMiniRangePoints()` (`mini-window.ts`) maps the rationale enum onto the existing per-points TIME_RANGES_KEYS. 7 unit tests. | +| 6 | `680f84c feat(charts): mood chart mini-mode + windowOverride` | MoodChart mirrors the same `mini` + `windowOverride` contract so mood-typed recommendations get a dedicated emoji-glyph mini-chart. | +| 7 | `10a67ff feat(insights): RecommendationCard with expandable rationale + mini-chart of data window` | New `` subcomponent with severity badge, default-collapsed chevron, animated 200ms expand reveal, 3-row `` (Window / Compared to / Deviation), pinned mini-chart (HealthChart for measurement metrics, MoodChart for mood-keyed recs), citation footnote, and named slots for B5d (`data-slot="rec-confidence-slot"`) and B5e (`data-slot="rec-feedback-slot"`). InsightAdvisorCard refactored to consume it; `legacyPayload` prop surfaces the regenerate CTA banner. InsightRecommendation type widens to optionally carry rationale + metricSource + severity + id. EN+DE i18n keys: `rationale`, `rationaleWindow`, `rationaleComparedTo`, `rationaleDeviation`, `rationaleExpand`, `rationaleCollapse`, `legacyPayloadCta`. 11 component tests. | +| 8 | `fed2e7e test(insights): coverage for RecommendationCard expand + rationale rendering` | Full-flow integration test: MockAIProvider → `generateInsight()` → InsightAdvisorCard → SSR markup assertion. Second case pins the legacyPayload CTA path with a string-only recommendations[] payload. 2 tests. | ## Verification diff --git a/.planning/phase-B5d-report.md b/.planning/phase-B5d-report.md index 1ca3cec68..7bcc822e2 100644 --- a/.planning/phase-B5d-report.md +++ b/.planning/phase-B5d-report.md @@ -10,14 +10,14 @@ Six atomic commits on origin/main (worktree: `pnpm test` 1457/1457, `pnpm test:integration` 59/59, `pnpm typecheck` 0 errors, `pnpm lint` 12 pre-existing warnings. -| # | Commit | What it ships | -|---|--------|---------------| -| 1 | `0cb0373 feat(ai): deterministic confidence-score computation per recommendation` | New `aiRecommendationSchema.confidence: z.number().int().min(0).max(100).optional()`. Mirror field on the UI-side `insightRecommendationSchema`. NEW `src/lib/ai/confidence.ts` — pure `computeConfidence({ n, recencyDays, deviationStdRatio })` returning 0..100. Score: log-saturating n curve (cap 40, n<3 hard-cap 5*n) + linear recency decay (30 at <2d → 0 at 30d) + signal contribution (30 at \|z\|≥1.5, scaled to 0; null → neutral 15). 15 tests cover the input matrix per research §2.C plus monotonicity-in-n + integer-in-bounds invariants. | -| 2 | `af21d4d feat(ai): wrapper overrides recommendation confidence with deterministic computation` | `generateInsight()` post-validation step OVERWRITES `rec.confidence` with `computeConfidence()` for every rec. The model's claimed value is discarded. New optional `options.confidenceContext` resolver lets a route layer supply per-rec `ConfidenceInputs` derived from `metricSource.n` + bucket-series freshest bucket + 90-day stdev; default fallback is `{ n: metricSource.n ?? 0, recencyDays: 0, deviationStdRatio: null }` so the override fires regardless. Wide-Event annotation `ai_confidence_override_*` carries per-rec `{id, model, computed}` triples for admin observability. 6 wrapper tests pin: model 99 → server 65 (default fallback), wrapper-fills when absent, resolver path overrides, n<3 hard-cap, missing `metricSource.n` → n=0 floor, resolver receives each rec in payload order. Citation-coverage test relaxed to find call by meta-key (annotate is now called twice). | -| 3 | `7ec1030 feat(insights): ConfidenceMeter component (bars + ring + draft pill below 25)` | NEW ``. Bars variant: 5 bars rising in height (4/6/8/10/12 px) so the "more is more" cue works without color (a11y). Ring variant: 28×28 SVG with stroke-dasharray fill. Color bands: green ≥80, yellow 50-79, orange 25-49; below 25 a red "Draft" pill REPLACES the meter (Apple's quiet-when-unsure affordance per research §2.A). aria-label always announces numeric score in EN/DE. NaN/-1 → 0 (draft); >100 → 100 (high). 5 i18n keys per locale: confidence + confidenceAria + confidenceHigh/Medium/Low/Draft. 17 component tests cover all 5 bands + both variants + EN/DE aria-label + clamp. | -| 4 | `63cfd8e feat(insights): RecommendationCard renders ConfidenceMeter in named slot` | Fills `data-slot="rec-confidence-slot"` with ``. Slot stays empty when confidence is undefined (legacy payload). Below 50, expanded rationale card surfaces "Low confidence — based on limited data" caption. Below 25 caption still applies because draft <= low. B5e's feedback-thumbs slot left untouched. Threshold is named const (`LOW_CONFIDENCE_CAPTION_THRESHOLD = 50`). 8 wiring tests. | -| 5 | `d343ab5 test(insights): coverage for confidence computation + meter rendering` | Full-flow integration test: MockAIProvider returns model=99 → wrapper overrides with computed=65 (n=9, recency=0, ratio=null) → InsightAdvisorCard renders the medium/yellow band, NOT high/green. Second case pins n=1 → draft band end-to-end. 2 tests. | -| 6 | `173c3e1 test(insights): rec-card-confidence test plays nice with B5e thumbs` | Cross-agent race fix: B5e wired RecommendationFeedback into the rec card concurrently. My test got the same useAuth + useMutation + tanstack-query SSR mocks the existing `recommendation-card.test.tsx` carries; the "feedback-slot is empty" assertion relaxed to "feedback-slot is still rendered" since B5e legitimately fills it. | +| # | Commit | What it ships | +| --- | ---------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| 1 | `0cb0373 feat(ai): deterministic confidence-score computation per recommendation` | New `aiRecommendationSchema.confidence: z.number().int().min(0).max(100).optional()`. Mirror field on the UI-side `insightRecommendationSchema`. NEW `src/lib/ai/confidence.ts` — pure `computeConfidence({ n, recencyDays, deviationStdRatio })` returning 0..100. Score: log-saturating n curve (cap 40, n<3 hard-cap 5\*n) + linear recency decay (30 at <2d → 0 at 30d) + signal contribution (30 at \|z\|≥1.5, scaled to 0; null → neutral 15). 15 tests cover the input matrix per research §2.C plus monotonicity-in-n + integer-in-bounds invariants. | +| 2 | `af21d4d feat(ai): wrapper overrides recommendation confidence with deterministic computation` | `generateInsight()` post-validation step OVERWRITES `rec.confidence` with `computeConfidence()` for every rec. The model's claimed value is discarded. New optional `options.confidenceContext` resolver lets a route layer supply per-rec `ConfidenceInputs` derived from `metricSource.n` + bucket-series freshest bucket + 90-day stdev; default fallback is `{ n: metricSource.n ?? 0, recencyDays: 0, deviationStdRatio: null }` so the override fires regardless. Wide-Event annotation `ai_confidence_override_*` carries per-rec `{id, model, computed}` triples for admin observability. 6 wrapper tests pin: model 99 → server 65 (default fallback), wrapper-fills when absent, resolver path overrides, n<3 hard-cap, missing `metricSource.n` → n=0 floor, resolver receives each rec in payload order. Citation-coverage test relaxed to find call by meta-key (annotate is now called twice). | +| 3 | `7ec1030 feat(insights): ConfidenceMeter component (bars + ring + draft pill below 25)` | NEW ``. Bars variant: 5 bars rising in height (4/6/8/10/12 px) so the "more is more" cue works without color (a11y). Ring variant: 28×28 SVG with stroke-dasharray fill. Color bands: green ≥80, yellow 50-79, orange 25-49; below 25 a red "Draft" pill REPLACES the meter (Apple's quiet-when-unsure affordance per research §2.A). aria-label always announces numeric score in EN/DE. NaN/-1 → 0 (draft); >100 → 100 (high). 5 i18n keys per locale: confidence + confidenceAria + confidenceHigh/Medium/Low/Draft. 17 component tests cover all 5 bands + both variants + EN/DE aria-label + clamp. | +| 4 | `63cfd8e feat(insights): RecommendationCard renders ConfidenceMeter in named slot` | Fills `data-slot="rec-confidence-slot"` with ``. Slot stays empty when confidence is undefined (legacy payload). Below 50, expanded rationale card surfaces "Low confidence — based on limited data" caption. Below 25 caption still applies because draft <= low. B5e's feedback-thumbs slot left untouched. Threshold is named const (`LOW_CONFIDENCE_CAPTION_THRESHOLD = 50`). 8 wiring tests. | +| 5 | `d343ab5 test(insights): coverage for confidence computation + meter rendering` | Full-flow integration test: MockAIProvider returns model=99 → wrapper overrides with computed=65 (n=9, recency=0, ratio=null) → InsightAdvisorCard renders the medium/yellow band, NOT high/green. Second case pins n=1 → draft band end-to-end. 2 tests. | +| 6 | `173c3e1 test(insights): rec-card-confidence test plays nice with B5e thumbs` | Cross-agent race fix: B5e wired RecommendationFeedback into the rec card concurrently. My test got the same useAuth + useMutation + tanstack-query SSR mocks the existing `recommendation-card.test.tsx` carries; the "feedback-slot is empty" assertion relaxed to "feedback-slot is still rendered" since B5e legitimately fills it. | ## Cross-agent race notes diff --git a/.planning/phase-B5e-report.md b/.planning/phase-B5e-report.md index 78ecbdfb6..1a6e36864 100644 --- a/.planning/phase-B5e-report.md +++ b/.planning/phase-B5e-report.md @@ -8,14 +8,14 @@ commits on origin/main, each TDD-first (failing test then fix). ## What landed -| # | Commit | What it ships | -|---|--------|---------------| -| 1 | `badc893 feat(db): RecommendationFeedback model with GDPR cascade + provider attribution` | Prisma model + migration `0034_recommendation_feedback`. `(userId, recommendationId, recommendationText)` unique constraint as the dedup key; `(userId, createdAt)` and `(severity, helpful, createdAt)` indexes for the per-user timeline + aggregator slice. Adds `AppSettings.adminAiInsightsFeedbackSummary Json?` for the daily aggregator's output. Cascade-delete integration test seeds + asserts a feedback row. | -| 2 | `8255e3e feat(api): /api/insights/feedback persists thumbs with provider attribution` | New `POST /api/insights/feedback` route: zod-validated body, `withIdempotency()` retry-replay, `requireAuth()` gate, server-fills providerType + promptVersion via `resolveFeedbackAttribution()` (reads latest `insights.generate` audit row, prefers chainProviderType > providerType > "unknown"). 201 on insert / 409 on dedup / 422 on validation. Audit row `insights.recommendation.feedback`. 7 schema tests + 5 attribution tests + 4 integration tests. | -| 3 | `ca84fb7 feat(insights): thumbs-up/down feedback component with optimistic updates` | `` component — TanStack Query mutation, two icon buttons (thumb-up / thumb-down), state machine (default → submitting → submitted/already-rated/error). localStorage cache keyed `(userId, recId, truncatedRecText)`. Auth-resolution race handled via setState-during-render (not useEffect — honours `react-hooks/set-state-in-effect`). EN+DE i18n: 5 new keys under `insights.recommendation.{feedbackHelpful,feedbackNotHelpful,feedbackThanks,feedbackConfirmed,feedbackAlreadyRated}`. 6 component tests. | -| 4 | `47ca1e4 feat(insights): RecommendationCard renders feedback thumbs in named slot` | Fills the `data-slot="rec-feedback-slot"` reserved by B5c. Defence-in-depth `asFeedbackTimeRange()` + attribute completeness check leaves the slot empty for legacy/partial recs (no id, missing severity, out-of-vocabulary timeRange) so a thumbs-click can never produce a 422. 3 new rec-card tests + retrofit `useMutation` + `useAuth` mocks across the existing rec-card / advisor-card / integration test files. | -| 5 | `71ffe30 feat(jobs): daily feedback aggregator writes per-(severity x provider) summary to AppSettings` | New pg-boss `feedback-aggregator` queue running daily 04:00 Europe/Berlin (after the rate-limit / idempotency / audit-log cleanups). Pure `buildFeedbackBuckets()` helper for in-memory aggregation; `aggregateRecommendationFeedback()` does the Prisma round-trip + AppSettings upsert. Bucket sort is deterministic (severity → metricSourceType → provider → prompt). Worker handler degrades gracefully on failure (warn, don't throw). 5 unit tests cover bucketing + 2 aggregator scenarios. | -| 6 | `8879282 feat(admin): AI quality preview showing per-(severity x provider) helpful-rate` | New `/admin/ai-quality` route + `` component reading `GET /api/admin/ai-quality` (admin-only). 7-column table tinted by severity + helpful-rate band (green ≥ 0.8 / yellow ≥ 0.5 / orange below). New `ai-quality` slug threaded through ADMIN_SECTION_SLUGS + admin-shell + dynamic [section] renderer. Empty-state copy distinguishes "no feedback yet" from "all-zeros". EN+DE i18n: 13 new keys (`admin.section.ai-quality.*` + `admin.aiQuality.*`). 5 dedicated component tests + 1 sections-smoke + 1 slug-parity test. | +| # | Commit | What it ships | +| --- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| 1 | `badc893 feat(db): RecommendationFeedback model with GDPR cascade + provider attribution` | Prisma model + migration `0034_recommendation_feedback`. `(userId, recommendationId, recommendationText)` unique constraint as the dedup key; `(userId, createdAt)` and `(severity, helpful, createdAt)` indexes for the per-user timeline + aggregator slice. Adds `AppSettings.adminAiInsightsFeedbackSummary Json?` for the daily aggregator's output. Cascade-delete integration test seeds + asserts a feedback row. | +| 2 | `8255e3e feat(api): /api/insights/feedback persists thumbs with provider attribution` | New `POST /api/insights/feedback` route: zod-validated body, `withIdempotency()` retry-replay, `requireAuth()` gate, server-fills providerType + promptVersion via `resolveFeedbackAttribution()` (reads latest `insights.generate` audit row, prefers chainProviderType > providerType > "unknown"). 201 on insert / 409 on dedup / 422 on validation. Audit row `insights.recommendation.feedback`. 7 schema tests + 5 attribution tests + 4 integration tests. | +| 3 | `ca84fb7 feat(insights): thumbs-up/down feedback component with optimistic updates` | `` component — TanStack Query mutation, two icon buttons (thumb-up / thumb-down), state machine (default → submitting → submitted/already-rated/error). localStorage cache keyed `(userId, recId, truncatedRecText)`. Auth-resolution race handled via setState-during-render (not useEffect — honours `react-hooks/set-state-in-effect`). EN+DE i18n: 5 new keys under `insights.recommendation.{feedbackHelpful,feedbackNotHelpful,feedbackThanks,feedbackConfirmed,feedbackAlreadyRated}`. 6 component tests. | +| 4 | `47ca1e4 feat(insights): RecommendationCard renders feedback thumbs in named slot` | Fills the `data-slot="rec-feedback-slot"` reserved by B5c. Defence-in-depth `asFeedbackTimeRange()` + attribute completeness check leaves the slot empty for legacy/partial recs (no id, missing severity, out-of-vocabulary timeRange) so a thumbs-click can never produce a 422. 3 new rec-card tests + retrofit `useMutation` + `useAuth` mocks across the existing rec-card / advisor-card / integration test files. | +| 5 | `71ffe30 feat(jobs): daily feedback aggregator writes per-(severity x provider) summary to AppSettings` | New pg-boss `feedback-aggregator` queue running daily 04:00 Europe/Berlin (after the rate-limit / idempotency / audit-log cleanups). Pure `buildFeedbackBuckets()` helper for in-memory aggregation; `aggregateRecommendationFeedback()` does the Prisma round-trip + AppSettings upsert. Bucket sort is deterministic (severity → metricSourceType → provider → prompt). Worker handler degrades gracefully on failure (warn, don't throw). 5 unit tests cover bucketing + 2 aggregator scenarios. | +| 6 | `8879282 feat(admin): AI quality preview showing per-(severity x provider) helpful-rate` | New `/admin/ai-quality` route + `` component reading `GET /api/admin/ai-quality` (admin-only). 7-column table tinted by severity + helpful-rate band (green ≥ 0.8 / yellow ≥ 0.5 / orange below). New `ai-quality` slug threaded through ADMIN_SECTION_SLUGS + admin-shell + dynamic [section] renderer. Empty-state copy distinguishes "no feedback yet" from "all-zeros". EN+DE i18n: 13 new keys (`admin.section.ai-quality.*` + `admin.aiQuality.*`). 5 dedicated component tests + 1 sections-smoke + 1 slug-parity test. | ## Verification diff --git a/.planning/phase-B7-report.md b/.planning/phase-B7-report.md index f6b73813f..6e795cb1c 100644 --- a/.planning/phase-B7-report.md +++ b/.planning/phase-B7-report.md @@ -11,9 +11,9 @@ Status: complete on origin/main. `data-testid` so the e2e suite can target it without depending on the localised label. - New dynamic-route slug `/settings/export` wired through `SETTINGS_SECTION_SLUGS` - + `SECTION_COMPONENTS` + `SETTINGS_SECTIONS` (sidebar nav + mobile strip). - `` learned the new icon so a future slug addition - cannot crash type-checking. + - `SECTION_COMPONENTS` + `SETTINGS_SECTIONS` (sidebar nav + mobile strip). + `` learned the new icon so a future slug addition + cannot crash type-checking. - `` simplified to the danger-zone card alone — every export path moved out into ``. The "wipe all data" surface should never live next to a one-click export button. diff --git a/.planning/phase-B8-report.md b/.planning/phase-B8-report.md index 372b5911b..31cc56288 100644 --- a/.planning/phase-B8-report.md +++ b/.planning/phase-B8-report.md @@ -14,6 +14,7 @@ Four atomic TDD-first commits on origin/main (`43204cb..f9f99ea`): 4. `f9f99ea feat(insights): AI narrates comparison when toggle is active` ### 1. Toggle + persistence + The "Compare to" Select pulldown lives at the top of `Settings → Dashboard` (`src/components/settings/dashboard-layout-section.tsx`), driven by `COMPARISON_BASELINES = ["none", "lastMonth", "lastYear"]`. The @@ -24,6 +25,7 @@ The PUT schema accepts the new optional field; pre-B8 clients keep PUTing without it. 4 new unit tests in `dashboard-layout.test.ts`. ### 2. Chart overlay + HealthChart and MoodChart accept a `compareBaseline` prop. When set, a parallel daily series is computed via `shiftDailySeriesForward()` (new pure helper at `src/lib/charts/comparison-shift.ts`, 11 unit @@ -39,6 +41,7 @@ adds a second row with the prior absolute value. 2 SSR tests in `health-chart-comparison.test.tsx`. ### 3. Tile delta callout + DataSummary gains `avg30LastMonth` / `avg30LastYear` (30-day means over `[30, 60)` and `[365, 395)` day windows). Every metric the analytics API summarises gets these for free — no extra DB queries. @@ -49,6 +52,7 @@ below the latest value, colour-coded via the existing no horizontal scroll. 6 SSR tests in `trend-card-comparison.test.tsx`. ### 4. AI narrative + `buildUserPrompt()` accepts an optional `ComparisonSnapshot`. When supplied it appends a SYSTEM CONTEXT block listing each metric's current 30d avg, baseline 30d avg, absolute delta, and percentage @@ -64,15 +68,15 @@ match exactly what the tiles render. 8 new unit tests in ## Acceptance-criteria coverage -| # | Criterion | Status | -| - | --------- | ------ | -| 1 | Toggle + persistence | shipped (commit 1) | -| 2 | Chart overlay rendering | shipped (commit 2) | -| 3 | Tile delta callout | shipped (commit 3) | -| 4 | Insights narrative integration | shipped — prompt block + ground rule (commit 4) | -| 5 | Dedicated `/insights/compare` page | DEFERRED to v1.4.17 | -| 6 | i18n EN+DE | shipped — 14 new key paths under `comparison.*` | -| 7 | Tests | shipped — 27 new tests (4 layout + 11 shift helper + 2 chart SSR + 6 tile SSR + 8 prompt) | +| # | Criterion | Status | +| --- | ---------------------------------- | ----------------------------------------------------------------------------------------- | +| 1 | Toggle + persistence | shipped (commit 1) | +| 2 | Chart overlay rendering | shipped (commit 2) | +| 3 | Tile delta callout | shipped (commit 3) | +| 4 | Insights narrative integration | shipped — prompt block + ground rule (commit 4) | +| 5 | Dedicated `/insights/compare` page | DEFERRED to v1.4.17 | +| 6 | i18n EN+DE | shipped — 14 new key paths under `comparison.*` | +| 7 | Tests | shipped — 27 new tests (4 layout + 11 shift helper + 2 chart SSR + 6 tile SSR + 8 prompt) | ## What's deferred to v1.4.17 diff --git a/.planning/phase-C-report.md b/.planning/phase-C-report.md index 87b4b55a4..957446859 100644 --- a/.planning/phase-C-report.md +++ b/.planning/phase-C-report.md @@ -24,7 +24,7 @@ commits rebased + pushed to `origin/main` directly. `API_TOKEN_HMAC_KEY` and `SESSION_SECRET` for symmetry + future- proofing. - **Evidence:** `gh run list --workflow=integration.yml --status=success - --limit 5` returned 5 successes after the fix landed. +--limit 5` returned 5 successes after the fix landed. ### e2e (`e2e.yml`) @@ -33,7 +33,7 @@ commits rebased + pushed to `origin/main` directly. - **Commits:** - `fbcd106 fix(ci): integration tests + e2e workflows green again` - `00848b7 fix(e2e): skip desktop-sidebar specs on mobile + stub - /api/dashboard/widgets` +/api/dashboard/widgets` - **Root cause #1 (5 of 9 fails):** `e2e/setup/global-setup.ts` seeded the deterministic e2e user with `onboarding_tour_completed = false` (default). Every authenticated @@ -59,7 +59,7 @@ commits rebased + pushed to `origin/main` directly. ### H1 — admin restore-failed scrubs raw Prisma error - **Commit:** `fdac9e2 fix(admin): scrub raw Prisma error from - restore-failed response`. +restore-failed response`. - **Files:** `src/app/api/admin/backups/[id]/restore/route.ts:378-393`. - The catch block now returns a stable `"Restore failed"` 500 instead of `"Restore transaction failed: "`. @@ -69,7 +69,7 @@ commits rebased + pushed to `origin/main` directly. ### H2 — moodEntrySchema.tags strict JSON-array validation - **Commit:** `7f1a4de fix(validation): moodEntry.tags must parse as - JSON string-array (v1.4.15 H2)`. +JSON string-array (v1.4.15 H2)`. - **Files:** `src/lib/validations/backup.ts:72-99`, `src/lib/validations/__tests__/backup.test.ts` (new, 7 tests). - Refined `tags` to `null | "" | `; everything @@ -84,7 +84,7 @@ commits rebased + pushed to `origin/main` directly. ### H4 code-review — tour-launcher sessionStorage scoping - **Commit:** `2afe3c4 fix(onboarding): scope tour-launcher - sessionStorage keys by user id (v1.4.15 H4)`. +sessionStorage keys by user id (v1.4.15 H4)`. - **Files:** `src/components/onboarding/tour-launcher.tsx:46-100`, `src/app/onboarding/page.tsx:127-150`, `src/components/onboarding/__tests__/tour-launcher-keys.test.ts` @@ -98,7 +98,7 @@ commits rebased + pushed to `origin/main` directly. ### H4 design — 44 px tap targets on tour / backups / notifications - **Commit:** `b863e2c fix(ui): tap-targets reach 44px on tour, - notification, backups buttons (v1.4.15 H4)`. +notification, backups buttons (v1.4.15 H4)`. - **Files:** `src/components/admin/backups-section.tsx:84,343,397,438,492`, `src/components/settings/notification-status-card.tsx:264,280`, `src/components/onboarding/tour.tsx:385,396,405`. @@ -118,7 +118,7 @@ commits rebased + pushed to `origin/main` directly. ### Bonus: i18n parity for B5a citation footnote - **Commit:** `2a7ef72 fix(i18n): German translations for - insights.recommendation.{source,viewSource}`. +insights.recommendation.{source,viewSource}`. - B5a added two English keys without German mirrors; pre-existing failure on the locale-integrity guard test went green. @@ -127,7 +127,7 @@ commits rebased + pushed to `origin/main` directly. ### MED 1 — insights/admin tab strip overflow - **Commit:** `d7c2b2a fix(tabs): horizontal scroll on mobile - prevents overflow (insights + admin)`. +prevents overflow (insights + admin)`. - `tabs.tsx` primitive: `overflow-x-auto` + `touch-pan-x` + `max-w-full` on `tabsListVariants`. Affects every page using ``. Vertical-tabs orientation kept its `overflow-x-visible` to avoid @@ -143,7 +143,7 @@ commits rebased + pushed to `origin/main` directly. ### MED 4 — bottom-nav 5+More - **Commit:** `072eee6 fix(nav): bottom-nav 5+More overflow protects - 44px tap targets`. +44px tap targets`. - `src/components/layout/bottom-nav.tsx` rewritten with PRIMARY (Home, Measurements, Mood, Medications, Insights) + OVERFLOW (Targets, Achievements) inside a Radix ``. Active state @@ -153,7 +153,7 @@ commits rebased + pushed to `origin/main` directly. ### MED 5 — system-status loading-failed Retry button - **Commit:** `65b4bf9 fix(admin): system-status load-failed pairs - alert with Retry button`. +alert with Retry button`. - `system-status-section.tsx` adds an inline Retry button driven by React Query's `refetch()`. Two new i18n keys + drop the "refresh the page" tail from the existing message. @@ -163,7 +163,7 @@ commits rebased + pushed to `origin/main` directly. - **Status:** DEFERRED to v1.5 — documented in `docs/audit/v1416-auto-deploy-fix.md`. - **Commit:** `4be6465 docs(deploy): v1.4.16 auto-deploy follow-up - audit + DEFER to v1.5`. +audit + DEFER to v1.5`. - Coolify v4-beta MCP API does not expose `auto_deploy_enabled` on the application. Marc-side UI flip ("Configuration → Auto Deploy → OFF") is the realistic fix; doing it programmatically requires @@ -175,15 +175,15 @@ commits rebased + pushed to `origin/main` directly. ## 5. docker-publish main-branch hang - **Commit:** `cc0f343 fix(ci): docker-publish reliable on main-branch - (drop qemu-arm64)`. +(drop qemu-arm64)`. - **Root cause:** the qemu-arm64 path (`docker/setup-qemu-action` - + `tonistiigi/binfmt`) was SIGILL-crashing Next.js's static-page- - generation workers (`Next.js build worker exited with code: null - and signal: SIGILL` at 64-66/86 pages). V8's optimising tier emits - CPU instructions that qemu's user-mode emulation cannot reliably - translate under heavy load. Tag-builds appeared to "work" only - because they sometimes finished the static phase before the crash - window. + - `tonistiigi/binfmt`) was SIGILL-crashing Next.js's static-page- + generation workers (`Next.js build worker exited with code: null +and signal: SIGILL` at 64-66/86 pages). V8's optimising tier emits + CPU instructions that qemu's user-mode emulation cannot reliably + translate under heavy load. Tag-builds appeared to "work" only + because they sometimes finished the static phase before the crash + window. - **Fix:** drop `linux/arm64` from the `platforms:` list. Marc's prod is x86_64 only; the arm64 image was a courtesy build for OSS users. v1.5 plan: re-add via a matrix on `ubuntu-24.04-arm` diff --git a/.planning/phase-C1-report.md b/.planning/phase-C1-report.md index 89f011c3d..7167742bf 100644 --- a/.planning/phase-C1-report.md +++ b/.planning/phase-C1-report.md @@ -22,14 +22,14 @@ land in v1.4.16+ — see `docs/audit/v1416-ai-roadmap.md`. ## Commits (in order) -| Commit | Subject | Role | -| ----------------------------------------- | ------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------ | -| `27310e4` | refactor(ai): consolidate providers behind AIProvider interface | MockAIProvider + 7 contract tests (every provider satisfies CompletionResult shape) | -| `5510ed5` (sibling-merged, scope is mine) | docs(audit): v1.4.15 empty-states audit + i18n keys | swept in my schema.ts + generate-insight.ts + generate-insight.test.ts (parallel-agent race) | -| `d657f79` | feat(ai): enforce citation-from-data on every recommendation | 11-test invariant suite — schema rejects missing/empty metricSource; cross-check enforced | -| `4e85c38` | feat(ai): scope-hardened system prompt with refusal pattern | Versioned prompt at `src/lib/ai/prompts/insight-generator.ts` (`PROMPT_VERSION = "4.15.0"`) + 18 tests | -| `4bba951` | feat(ai): fallback-chain slug discovery with 1h positive cache | `CodexClient` walks fallback chain + cache; spec §7b extended; 20 tests | -| `fa11f10` | docs(audit): v1.4.16 AI hardening roadmap | Iteration plan Marc explicitly invited | +| Commit | Subject | Role | +| ----------------------------------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| `27310e4` | refactor(ai): consolidate providers behind AIProvider interface | MockAIProvider + 7 contract tests (every provider satisfies CompletionResult shape) | +| `5510ed5` (sibling-merged, scope is mine) | docs(audit): v1.4.15 empty-states audit + i18n keys | swept in my schema.ts + generate-insight.ts + generate-insight.test.ts (parallel-agent race) | +| `d657f79` | feat(ai): enforce citation-from-data on every recommendation | 11-test invariant suite — schema rejects missing/empty metricSource; cross-check enforced | +| `4e85c38` | feat(ai): scope-hardened system prompt with refusal pattern | Versioned prompt at `src/lib/ai/prompts/insight-generator.ts` (`PROMPT_VERSION = "4.15.0"`) + 18 tests | +| `4bba951` | feat(ai): fallback-chain slug discovery with 1h positive cache | `CodexClient` walks fallback chain + cache; spec §7b extended; 20 tests | +| `fa11f10` | docs(audit): v1.4.16 AI hardening roadmap | Iteration plan Marc explicitly invited | Cross-agent observation: same shared-cwd race documented across A2 / A4 / B-mobile / B1-B6 / C2 / C3 / C4 / B5 of the v1.4.15 marathon @@ -89,11 +89,11 @@ version. Versioning policy documented in the prompt file's header. ```ts DEFAULT_SLUG_FALLBACK_CHAIN = [ - "gpt-5.3-codex", // verified 2026-05-09 on Plus/Pro - "gpt-5-codex", // historical default — kept as second-chance retry - "gpt-5", // bare slug — currently rejected on ChatGPT-auth - "gpt-4o", // last-ditch capability fallback -] + "gpt-5.3-codex", // verified 2026-05-09 on Plus/Pro + "gpt-5-codex", // historical default — kept as second-chance retry + "gpt-5", // bare slug — currently rejected on ChatGPT-auth + "gpt-4o", // last-ditch capability fallback +]; ``` Override via `CODEX_MODEL_FALLBACK_CHAIN` (comma-separated). When @@ -104,15 +104,15 @@ survive behind it. Stable de-duplication. Per `docs/codex-protocol-spec.md` §7b (extended in this phase): -| Status | Action | -| ----------------------------------------------- | ------------------------------------- | -| `400` + "not supported when using Codex with a ChatGPT account" | walk | -| `400` + `model_not_found` | walk | -| `400` + `does not exist` AND mentions a model | walk | -| `404` | walk | -| `401` (first time) | refresh and retry SAME slug; DON'T walk | -| `401` after refresh, `403`, `429`, `5xx` | propagate (don't walk) | -| `200` + SSE `response.failed.error.code === "invalid_prompt"` | propagate (request shape is wrong) | +| Status | Action | +| --------------------------------------------------------------- | --------------------------------------- | +| `400` + "not supported when using Codex with a ChatGPT account" | walk | +| `400` + `model_not_found` | walk | +| `400` + `does not exist` AND mentions a model | walk | +| `404` | walk | +| `401` (first time) | refresh and retry SAME slug; DON'T walk | +| `401` after refresh, `403`, `429`, `5xx` | propagate (don't walk) | +| `200` + SSE `response.failed.error.code === "invalid_prompt"` | propagate (request shape is wrong) | Positive cache: process-local Map at `src/lib/ai/codex-slug-cache.ts`, single slot, 1 h TTL. Cache hit makes the working slug come first on @@ -152,16 +152,16 @@ the wrapper passes it through cleanly. ## Tests added -| File | Tests | Focus | -| ------------------------------------------ | ----- | ------------------------------------------------------------ | -| `provider-contract.test.ts` | 7 | Every provider returns CompletionResult shape | -| `mock-client.ts` | - | Test infrastructure, no tests of its own (covered above) | -| `schema.ts` (no test file) | - | Tested via the next two files | -| `generate-insight.test.ts` | 11 | Schema parse, retry-once with correction, 422 on second-fail | -| `citation-enforcement.test.ts` | 11 | Schema-level + cross-check + wrapper end-to-end | -| `insight-generator-prompt.test.ts` | 18 | Both locales, scope hardening, refusal payload, ESH/ESC etc. | -| `codex-slug-cache.test.ts` | 7 | TTL boundaries, invalidation, replacement | -| `codex-slug-fallback.test.ts` | 13 | Walk triggers, no-walk on 5xx/429/401, all-failed, cache | +| File | Tests | Focus | +| ---------------------------------- | ----- | ------------------------------------------------------------ | +| `provider-contract.test.ts` | 7 | Every provider returns CompletionResult shape | +| `mock-client.ts` | - | Test infrastructure, no tests of its own (covered above) | +| `schema.ts` (no test file) | - | Tested via the next two files | +| `generate-insight.test.ts` | 11 | Schema parse, retry-once with correction, 422 on second-fail | +| `citation-enforcement.test.ts` | 11 | Schema-level + cross-check + wrapper end-to-end | +| `insight-generator-prompt.test.ts` | 18 | Both locales, scope hardening, refusal payload, ESH/ESC etc. | +| `codex-slug-cache.test.ts` | 7 | TTL boundaries, invalidation, replacement | +| `codex-slug-fallback.test.ts` | 13 | Walk triggers, no-walk on 5xx/429/401, all-failed, cache | Total: +67 unit tests across 6 new files. 965 → 1048 unit pass (+83 net from baseline; some sibling-agent tests merged in too). diff --git a/.planning/phase-C2-report.md b/.planning/phase-C2-report.md index 0e6f6fc1a..a7fd64b3a 100644 --- a/.planning/phase-C2-report.md +++ b/.planning/phase-C2-report.md @@ -4,10 +4,10 @@ **Wall clock:** 2026-05-09T21:10 → 2026-05-09T21:25 Berlin (~15 min) **Commits on `origin/main`:** -| SHA | Message (real diff scope) | -| --------- | ---------------------------------------------------------------------------------------------------------------------------------------- | +| SHA | Message (real diff scope) | +| --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `34c967c` | `feat(achievements): …` — sibling-agent commit message; **my** docker-publish.yml + .env.example + docs/audit/v1415-auto-deploy.md hunks rode along (5th run of the shared-cwd race documented in A2/A4/B1/B-mobile/B2/B3). | -| `ad350fe` | `feat(deploy): admin notification + audit log on deploy success/failure` — clean, my files only (`git commit -o` pathspec). | +| `ad350fe` | `feat(deploy): admin notification + audit log on deploy success/failure` — clean, my files only (`git commit -o` pathspec). | Linked deep-dive: [`docs/audit/v1415-auto-deploy.md`](../docs/audit/v1415-auto-deploy.md). diff --git a/.planning/phase-C3-report.md b/.planning/phase-C3-report.md index 0a18c6730..338f1c603 100644 --- a/.planning/phase-C3-report.md +++ b/.planning/phase-C3-report.md @@ -3,10 +3,11 @@ **Status**: done **Wall clock**: 2026-05-09T20:30 → 2026-05-09T20:55 Berlin (~25 min) **Commits on `origin/main`**: - - `41945b2` fix(test): de-flake e2e a11y suite by forcing dark colorScheme - - `249c42b` fix(ci): docker-publish reliability — separate cache scope per ref + 30min timeout - - `ffa4aac` feat(ci): post-publish verify workflow - - `4a5be22` docs(audit): v1.4.15 CI/e2e reliability report + +- `41945b2` fix(test): de-flake e2e a11y suite by forcing dark colorScheme +- `249c42b` fix(ci): docker-publish reliability — separate cache scope per ref + 30min timeout +- `ffa4aac` feat(ci): post-publish verify workflow +- `4a5be22` docs(audit): v1.4.15 CI/e2e reliability report Linked deep-dive: [`docs/audit/v1415-ci-reliability.md`](../docs/audit/v1415-ci-reliability.md) @@ -47,12 +48,12 @@ manifest exporter and one deadlocked indefinitely (no timeout). ## Fixes shipped -| Commit | Surface | Effect | -| --------- | ---------------------------------- | ---------------------------------------------- | -| `41945b2` | `playwright.config.ts` | e2e now scans the actual user-facing dark UI | -| `249c42b` | `.github/workflows/docker-publish.yml` | per-ref cache scope, 30-min timeout | +| Commit | Surface | Effect | +| --------- | ------------------------------------------- | --------------------------------------------- | +| `41945b2` | `playwright.config.ts` | e2e now scans the actual user-facing dark UI | +| `249c42b` | `.github/workflows/docker-publish.yml` | per-ref cache scope, 30-min timeout | | `ffa4aac` | `.github/workflows/post-publish-verify.yml` | informational reachability gate after publish | -| `4a5be22` | `docs/audit/v1415-ci-reliability.md` | full audit + recommendations | +| `4a5be22` | `docs/audit/v1415-ci-reliability.md` | full audit + recommendations | The fix-set explicitly excludes scope I was told not to touch: `src/components/charts/**`, `admin/backups`, `withings/**`, @@ -90,12 +91,12 @@ docker-publish + post-publish-verify fixes live in `.github/workflows`. ## Pass-rate impact -| Stage | 30-day Pre-C3 | Expected Post-C3 | -| ------------------------------- | ------------- | ---------------- | -| Integration tests | 100 % (47/47) | 100 % | +| Stage | 30-day Pre-C3 | Expected Post-C3 | +| ------------------------------- | ------------- | ------------------------- | +| Integration tests | 100 % (47/47) | 100 % | | Security & Quality | 93 % (43/46) | unchanged (A4 fix needed) | -| Docker publish (completed only) | 91 % (30/33) | ≥ 95 % | -| e2e | 0 % (0/47) | ≥ 90 % | +| Docker publish (completed only) | 91 % (30/33) | ≥ 95 % | +| e2e | 0 % (0/47) | ≥ 90 % | The 0 → ≥90 % e2e jump is the headline result. Marc's "tests must really run through" gate is unblocked. @@ -103,6 +104,7 @@ really run through" gate is unblocked. ## Race-condition observations (operational note) 7 parallel agents pushing to `main` produced: + - 26 of 60 docker-publish runs cancelled by `concurrency.cancel-in-progress` (by-design; no action needed) - Two of my own commits picked up B-agent staged files mid-`git add` diff --git a/.planning/phase-C4-report.md b/.planning/phase-C4-report.md index ab5acaa18..f78b0ccac 100644 --- a/.planning/phase-C4-report.md +++ b/.planning/phase-C4-report.md @@ -12,15 +12,15 @@ EN+DE translations as part of their own commits. By the time C4 ran sequentially after Batch 2, both locale files were already in perfect parity: -| Check | Count | -| ---------------------------------- | ----: | -| Total keys (EN) | 1817 | -| Total keys (DE) | 1817 | -| Keys only in EN | 0 | -| Keys only in DE | 0 | -| Empty values | 0 | -| TODO/FIXME placeholders | 0 | -| Used in `src/` but missing in EN | 0 (after JSDoc/dynamic-prefix/test-fixture allow-list) | +| Check | Count | +| -------------------------------- | -----------------------------------------------------: | +| Total keys (EN) | 1817 | +| Total keys (DE) | 1817 | +| Keys only in EN | 0 | +| Keys only in DE | 0 | +| Empty values | 0 | +| TODO/FIXME placeholders | 0 | +| Used in `src/` but missing in EN | 0 (after JSDoc/dynamic-prefix/test-fixture allow-list) | All v1.4.15-introduced namespaces (`admin.section.backups.*`, `achievements.*`, `onboarding.tour.*`, `admin.overview.*`, diff --git a/.planning/phase-C5-report.md b/.planning/phase-C5-report.md index 00fd4a6f0..08c0abe1a 100644 --- a/.planning/phase-C5-report.md +++ b/.planning/phase-C5-report.md @@ -32,24 +32,24 @@ API changes. - `5510ed5` `docs(audit): v1.4.15 empty-states audit + i18n keys` — audit document at `docs/audit/v1415-empty-states.md`; 18 new EN+DE keys under `measurements/mood/medications/admin.section.users/ - admin.section.backups/admin.{loginEmpty,tokensEmpty}/admin.feedback/ - insights/dashboard`. Sibling-agent C1 AI files swept in by the +admin.section.backups/admin.{loginEmpty,tokensEmpty}/admin.feedback/ +insights/dashboard`. Sibling-agent C1 AI files swept in by the shared-cwd race (per STATE.md pattern); files correct on `main`, message scope misleading. - `0c20119` `feat(admin): empty states for users, backups, - login-overview, api-tokens, feedback` — 6 admin sub-routes upgraded. +login-overview, api-tokens, feedback` — 6 admin sub-routes upgraded. Adds `user-management-empty.test.tsx` + `backups-section-empty.test.tsx` guarding primitive mount, localized strings, and CTA-presence semantics. - `9a74f8e` `feat(lists): empty states for measurements, mood, - medications, achievements` — only carried the new +medications, achievements` — only carried the new `measurement-list-empty.test.tsx` due to a sibling-cwd staging race; message-scope wider than diff. - `1d65f3b` `feat(lists): empty states for measurements, mood, - medications, achievements (impl)` — follow-up that lands the actual +medications, achievements (impl)` — follow-up that lands the actual EmptyState wiring described in the previous commit's message. - `65faf1d` `feat(insights,dashboard): empty states for first-run - views` — `/insights` top-level + BMI height-not-set + dashboard +views` — `/insights` top-level + BMI height-not-set + dashboard fully-empty paths. ## Tests @@ -80,7 +80,7 @@ C2) reproduced twice during this phase: 2. Commit `9a74f8e` was supposed to carry the four feature-list impl files plus the new test, but only the new test got staged — six modified files reverted from the index between `git add` and `git - commit`. The follow-up commit `1d65f3b` lands the impl with an +commit`. The follow-up commit `1d65f3b` lands the impl with an explicit message tying the two together. Both patterns match STATE.md's documented "v1.4.16 should adopt diff --git a/.planning/phase-D-c1-c2-wire-report.md b/.planning/phase-D-c1-c2-wire-report.md index 7ebdfcb5e..4822b50f3 100644 --- a/.planning/phase-D-c1-c2-wire-report.md +++ b/.planning/phase-D-c1-c2-wire-report.md @@ -28,9 +28,10 @@ forwards its regenerate button to the same handler. The 7 per-section surface is additive, not a replacement. E2E proof: `e2e/insights-advisor-card.spec.ts` mocks the analytics -+ comprehensive + per-status + advisor endpoints and asserts the -hero, the rec text, the ConfidenceMeter slot, and the summary prose -all render on the live `/insights` route. + +- comprehensive + per-status + advisor endpoints and asserts the + hero, the rec text, the ConfidenceMeter slot, and the summary prose + all render on the live `/insights` route. ## C2 — Mount `` on `/` — **DONE** diff --git a/.planning/phase-D-code-review-findings.md b/.planning/phase-D-code-review-findings.md index 9d0942afc..c1198458a 100644 --- a/.planning/phase-D-code-review-findings.md +++ b/.planning/phase-D-code-review-findings.md @@ -1,190 +1,568 @@ -# Phase D — Code-review findings (v1.4.16) +# v1.4.18 Phase-D Code Review Findings -Reviewer: senior code-review (parallel with security / design / senior-dev / simplify / Product Lead). -Scope: diff `v1.4.15...HEAD` — Wave A (A1–A8), Wave B (B1a/b, B2, B3, B4, B5a–e, B6, B7, B8), Wave C (CI + deferred HIGH). -Method: targeted reads of hot paths — AI provider chain + confidence + schema (B5a–d), feedback loop (B5e), host-metrics + app-logs (B3/B4), exports (B7), comparison overlay (B8), BD-Zielbereich predicate (A2). No source edits, no commits. +Reviewer: code-reviewer agent +Run: 2026-05-10 (parallel with 5 other reviewers) +Scope: v1.4.17 → origin/main (22 commits) +DO NOT commit — findings only. ---- +Diff scope (from `git log --oneline v1.4.17..origin/main`): -## CRITICAL — ship-blocker +- A1 BD-Zielbereich tile sub-values (commits 23363ca, 15c852f) +- A2 painted-scrollbar fix (commits 3e16074, 874878f) +- A3 chart visual revert + per-chart toggles (commits 008a8fb, 06f18df, b388fc0, 75f340e, d70a9b1, 3d58557, d6e87c3) +- B1 achievements expansion + hidden Easter-eggs (commits e73813a, 19b0844, c41f9a4, 23514b4, b88f2de, 6912d4e, 75c74f1, 178c6dc) -(none) +## CRITICAL ---- +### C1 — Hidden achievement triggers leak via the API response (and the client bundle) + +- **Severity**: CRITICAL +- **File**: `src/app/api/gamification/achievements/route.ts:611-655` and + `src/lib/gamification/achievements.ts:706-739` (evaluator) + + `src/lib/i18n/context.tsx:15-22` (bundle import) +- **Issue**: The reviewer brief lists this exact failure mode as ship- + blocker: _"the 'Hidden' placeholder must not reveal predicate + conditions to a snooping user"_. The achievements page UI does the + right thing — `` for hidden+locked entries renders + an opaque card with only a generic "Hidden achievement" label and + the help-circle icon. **However, the wire response from + `GET /api/gamification/achievements` ships the full achievement + definition for every entry including hidden+locked**: + + ```json + { + "id": "hidden-night-owl", + "metric": "nightOwlCount", + "category": "hidden", + "titleKey": "achievements.badges.hiddenNightOwl.title", + "descriptionKey": "achievements.badges.hiddenNightOwl.description", + "icon": "Moon", + "format": "count", + "target": 1, + "current": 0, + "points": 25, + "unlocked": false, + "progressPercent": 0, + "completedAt": null, + "isHidden": true + } + ``` + + Three independent leak vectors a "snooping user" can use without any + privileged access: + 1. The metric name itself (`nightOwlCount`, `earlyBirdCount`, + `leapDayCount`, `doctorPdfCount`, `localeFlipCount`) leaks the + trigger semantically. + 2. The `titleKey` / `descriptionKey` strings reference the i18n + bucket whose translated values describe the trigger verbatim + (`"Logged an entry between 02:00 and 04:00 in the morning."`). + 3. `messages/en.json` and `messages/de.json` are statically + `import`-ed in `src/lib/i18n/context.tsx` (lines 15-16) and + therefore bundled into the client JS — a `Cmd-F` for + `"hiddenNightOwl"` in the chunk reveals every trigger and + reward. + Items 1+2 are the ship-blocker the brief explicitly asks for. Item 3 + is a related but separable concern that needs a different fix. + + Critically, the `` opaque-placeholder branch makes + this leakage _less obvious to casual users_ but does nothing to + prevent a determined snooper — Marc, who specifically asked for + hidden Easter-eggs to be a surprise, would discover them in 30 + seconds with the Network tab open. + + Existing test coverage (`src/app/achievements/__tests__/page.test.tsx` + lines 254-270) only asserts that the rendered HTML doesn't contain + the strings — _that test passes_ even though the strings are still + in the JSON response and the bundle. The test is not actually + guarding what the brief asked us to guard. + +- **Recommendation**: Strip identifying fields from hidden+locked + achievement entries server-side before returning them. The wire + shape for a hidden+locked entry should look like: + + ```json + { + "id": "hidden-night-owl", + "category": "hidden", + "isHidden": true, + "unlocked": false, + "completedAt": null + } + ``` + + i.e. drop `metric`, `titleKey`, `descriptionKey`, `icon`, `target`, + `current`, `progressPercent`, `points`. A stable `id` is fine to + expose because the i18n key is namespaced under `hidden*` (the user + can still infer that hidden achievements exist; that's the whole + point) but the trigger details must not be transmitted. + + Concretely, in `src/app/api/gamification/achievements/route.ts` + around line 611 (where `applyDiscoveryFilter` is called), add a + second pass that scrubs hidden+locked entries before serialising: + + ```ts + const sanitized = visibleAchievements.map((a) => { + if (a.isHidden && !a.unlocked) { + return { + id: a.id, + category: "hidden" as const, + isHidden: true, + unlocked: false, + completedAt: null, + // Synthetic placeholders for type-stability so the client + // doesn't need an `?.` everywhere. The real strings only + // ship once the achievement unlocks. + titleKey: "achievements.hiddenCard.title", + descriptionKey: "achievements.hiddenCard.description", + icon: "HelpCircle", + format: "count" as const, + target: 0, + current: 0, + points: 0, + metric: "totalTakenIntakes" as const, // sentinel + progressPercent: 0, + }; + } + return a; + }); + ``` + + And add an explicit test at the API/route level that asserts the + response body does NOT contain `nightOwlCount`, `earlyBirdCount`, + `leapDayCount`, `doctorPdfCount`, `localeFlipCount` while the + achievement is locked, and DOES contain those strings once + unlocked. + + Bundle leak (item 3) is a separate fix: gate hidden i18n strings + behind a server-side translator and return the resolved title / + description in the unlock toast payload only when the unlock + actually occurs (or move them to a server-only translation file + loaded by `getServerTranslator`). Lower-effort interim is a comment + in `messages/*.json` explaining the trade-off and accepting that + any client-bundle leak below the API leak is "acceptable risk for + v1.4.18" — but that's a Marc-call, not a reviewer-call. + +- **Ship-blocker?**: YES. The brief explicitly calls this out as + CRITICAL ("hidden achievement leakage in DOM/network responses"). + The DOM leak is fixed (the card renders correctly); the **network + response leak is not**. This must be fixed before v1.4.18 ships. ## HIGH -### H1. Bucket-key collision in `feedback-aggregator` allows cross-bucket pollution -- **File:** `src/lib/jobs/feedback-aggregator.ts:70-75` -- **Issue:** `[severity, metricSourceType, providerType, promptVersion].join("")` uses the empty separator. `severity` is bounded enum but `metricSourceType` is free-form (z.string().min(1).max(80)) and `providerType`/`promptVersion` are wire-stamped. Collision example: `severity="info"` + `metricSourceType="important"` collides with `severity="important"` + `metricSourceType="info"` — both produce key `"infoimportant…"`. The first to arrive wins ownership; subsequent rows accumulate counts onto a row whose displayed `(severity, metricSourceType)` belongs to the wrong tuple, silently poisoning the admin AI quality slice. -- **Recommendation:** Join with an unambiguous separator that cannot appear in any of the four fields, e.g. `\x1f` (US) or a JSON tuple: `JSON.stringify([severity, metricSourceType, providerType, promptVersion])`. -- **Ship-blocker for v1.4.16?** Yes for the admin AI-quality feature semantics; can be deferred if the dashboard is treated as best-effort observability. +### H1 — Read-modify-write race when two charts toggle overlays simultaneously -### H2. `FallbackChainCard` discards the persisted `enabled` flag — disabled providers vanish from the UI -- **Files:** `src/app/api/insights/provider-chain/route.ts:49-56`, `src/components/settings/ai-section.tsx:1272-1283` -- **Issue:** The GET response only returns `{ providerType, available }`. The UI seeds `enabled: true` for every entry (`chain.map((c) => ({ providerType: c.providerType, enabled: true }))`). But `resolveProviderChain()` upstream (`src/lib/ai/provider.ts:306`) **filters out disabled entries before returning** (`parseProviderChain(rawChain).filter((e) => e.enabled)`). Net effect: a user who disables an entry sees it disappear from the list; on reload it cannot be re-enabled because it's not present, and the toggle position is lost. The UI advertises the toggle as a "disable in fallback chain" affordance but is functionally a "remove from chain" button. -- **Recommendation:** GET should return the raw persisted `parseProviderChain(rawChain)` (not the resolveProviderChain filtered list) plus `available` per entry. The UI then renders both enabled and disabled rows so the user can re-enable. Or: remove the toggle entirely and only support add/remove. -- **Ship-blocker for v1.4.16?** Yes — it's a regression vs. the documented B2 brief ("toggle drives enable/disable"). Either fix the round-trip or remove the Switch from the row. +- **Severity**: HIGH (correctness — concurrent writes lose data) +- **File**: `src/app/api/dashboard/chart-overlay-prefs/route.ts:55-79` + and `src/app/api/dashboard/widgets/route.ts:96-115` +- **Issue**: Both routes do an unwrapped read-modify-write against + `User.dashboardWidgetsJson`: -### H3. Strict B5b/B5c/B5d AI features are not wired into the production route -- **Files:** `src/app/api/insights/generate/route.ts:280-393`, `src/lib/ai/generate-insight.ts`, `src/lib/ai/provider-runner.ts:229-288` -- **Issue:** The production `/api/insights/generate` route uses `runRawCompletionWithFallback()` (raw `provider.generateCompletion`, no schema enforcement, no rationale check, no confidence override). The strict `runWithFallback()` + `generateInsight()` (which enforces `aiRecommendationRationaleSchema`, runs the corrective retry, OVERRIDES `rec.confidence` deterministically, annotates `ai_confidence_override_pairs`) is consumed only in tests. Result: in production, the model's self-reported confidence is what users see — defeating B5d's "calibrated probabilities are not a small-LLM strength" mandate; rationale + citation enforcement is observational only via `insightResultSchema.safeParse()` falling back to raw parsed JSON on validation failure (line 386 `insights = validated.success ? validated.data : parsed`). -- **Recommendation:** Either (a) migrate `/api/insights/generate` to `runWithFallback()` + new `aiInsightResponseSchema`, replacing the legacy passthrough — STATE.md notes this as a deferred "v1.4.17 migration"; or (b) add a **runtime feature-flag** gate so the strict path can be enabled in prod without a code release. Today's `applyConfidenceOverride()` and rationale-required schema are dead code in production. -- **Ship-blocker for v1.4.16?** Maybe — depends on whether v1.4.16's release notes claim "deterministic confidence" or "rationale-required" as user-facing features. STATE.md B5c/B5d both say "complete on origin/main" but the production path is unchanged. Users will see model-claimed confidence values, not server-computed ones. Recommend explicit Marc-call: ship with feature dark + clearer release-notes wording, or migrate the route. + ```ts + const row = await prisma.user.findUnique({ + where: { id: user.id }, + select: { dashboardWidgetsJson: true }, + }); + const current = resolveDashboardLayout(row?.dashboardWidgetsJson); + const next: DashboardLayout = { + ...current, + chartOverlayPrefs: { + ...(current.chartOverlayPrefs ?? {}), + [parsed.data.chartKey]: parsed.data.prefs, + }, + }; + await prisma.user.update({ + where: { id: user.id }, + data: { dashboardWidgetsJson: normalized }, + }); + ``` -### H4. `findRecommendationsMissingRationale` is a no-op against parsed AIInsightResponse -- **File:** `src/lib/ai/schema.ts:283-297`, `src/lib/ai/generate-insight.ts:158-175` -- **Issue:** The strict schema requires `rationale` (`aiRecommendationRationaleSchema` is required on every rec — line 157). If the parser succeeds, every rec will have a rationale, so the helper always returns `[]`. The Wide-Event annotation `ai_rationale_missing_recommendation_ids` is therefore always empty in the strict path — and the strict path is the only place the helper fires. The "observability tripwire for the migration window" comment is misleading. -- **Recommendation:** Either feed it a separate looser-typed pre-parse blob (e.g. the JSON before `aiInsightResponseSchema.safeParse()` runs), or move the call to the `isLegacyInsightPayload()` cache-hit path where it would actually surface drift. Document or remove the tripwire claim. -- **Ship-blocker for v1.4.16?** No — observability dead-code is not a functional bug. Worth fixing for clarity. + If a user opens the dashboard, flips the BP chart's "Target range" + toggle, and 50ms later flips the weight chart's "Trend" toggle, the + two `PUT` calls can interleave: BP-PUT reads the current layout, + weight-PUT reads the _same_ current layout (concurrent), BP-PUT + writes the merged BP prefs, weight-PUT then writes its merged + weight prefs back **on top of the layout it read** — which still + has the old BP prefs — silently dropping the BP toggle. The user + sees the BP toggle revert when the page next refreshes. -### H5. `recommendation-feedback` has no rate-limit -- **File:** `src/app/api/insights/feedback/route.ts:30-101` -- **Issue:** No `checkRateLimit()` call. Each thumbs-click writes one `RecommendationFeedback` row + one `audit_logs` row. The unique index `(userId, recommendationId, recommendationText)` prevents exact-duplicate inserts, but a malicious client can rotate `recommendationId` or `recommendationText` per request and flood the DB. Idempotency-Key cache only catches identical retries with the same key. -- **Recommendation:** Add `checkRateLimit('feedback:${user.id}', 60, 60 * 60 * 1000)` (60/h is generous — a comprehensive insight rarely has >10 recs, so ≥6 regenerations/h before throttle hits). -- **Ship-blocker for v1.4.16?** No — single-user scoped, inside-auth. Fix in v1.4.17. + The optimistic update in `useChartOverlayPrefs` (`onMutate`) + partially masks this for the originating client because the local + cache stays correct until the route returns; once the route + returns successful and `onSettled` invalidates the query, the next + fetch reveals the dropped toggle. -### H6. `recommendation-feedback` invalidates the entire `["insights"]` cache on every thumbs click -- **File:** `src/components/insights/recommendation-feedback.tsx:192` -- **Issue:** `queryClient.invalidateQueries({ queryKey: ["insights"] })` matches ALL `["insights", …]`-prefixed keys (8 queries: comprehensive + 7 per-status). On thumbs success this kicks off a synchronous refetch of all 8 — none of which are affected by the feedback row itself. Refetching the comprehensive blob requires a 1500-token LLM call (rate-limited 10/h) — the only thing that prevents this is the 24h server-side cache-hit path. Effectively the user gets no surprise refetch in practice but the network and cache thrash is wasted work, and on a stale cache it's a real LLM round-trip. -- **Recommendation:** Drop the invalidation entirely; feedback doesn't affect the displayed insight content. If a future mode (B5e ratchet in v1.4.17) makes feedback feed back into next-gen prompts, scope the invalidation to the per-status / comprehensive query that would consume the new prompt. -- **Ship-blocker for v1.4.16?** No — minor performance smell. + The same race exists for `/api/dashboard/widgets` PUT (already had + this defect in v1.4.16 + B8 comparison baseline) but the new chart- + overlay-prefs route makes it worse because the surface for + per-chart toggles is exactly the kind of thing users will + rapid-fire (5 charts × 3 toggles = 15 settings to flip in a + session). -### H7. Comparison overlay re-uses already-fetched `data` as the "prior period" — there is NO prior-period data -- **Files:** `src/components/charts/health-chart.tsx:524-563`, `src/lib/charts/comparison-shift.ts` -- **Issue:** `chartDataWithCompare` shifts the SAME `data` array forward by 30/365 days. The dashboard fetcher upstream pulls `data` as the visible window only (e.g. last 30 days). Shifting today's last-30-days forward by 30 days produces "today's data overlaid 30 days into the future" — not "last month's data overlaid onto today". For the comparison to be meaningful the chart needs a SEPARATE fetch of the prior 30/365-day window. STATE.md B8 says "shift the SAME daily aggregates and shift them forward by 30 days (lastMonth) or 365 days (lastYear) so the prior period lines up with the current period on the visible x-axis" — only correct if `data` already contains BOTH periods. Verify upstream `data` covers the full extended range. -- **Recommendation:** Audit the call site — does the dashboard fetcher pull `now - rangeDays` only, or `now - rangeDays - shift`? If the former, the overlay shows `current_period` at `current_period + 30d` (a phantom future overlay). If the latter, OK. Add a unit test that pins `data` containing both periods and asserts the visible overlay maps to the prior-period values, not the current period's values. -- **Ship-blocker for v1.4.16?** Possibly — needs fast verification. If `data` is single-period, the overlay is structurally meaningless. +- **Recommendation**: Wrap the read-modify-write in a transaction + with `SERIALIZABLE` isolation OR rewrite the merge to use Postgres' + JSON path operators so the SQL itself is the merge. Cheapest fix + is the transaction wrapper: -### H8. `seedKey` includes potentially-undefined `chainData?.activeProvider` so race between query resolution and URL change re-seeds twice -- **File:** `src/components/settings/ai-section.tsx:228-241` -- **Issue:** `seedKey = `${queryProvider ?? ""}|${chainData?.activeProvider ?? ""}`` mutates between renders — first render `chainData` is undefined → seedKey ends with `"|"`; on chainData arrival → seedKey ends with `"|codex"`. The `seededFor !== seedKey` branch fires twice, second time setting `selectedProvider` from chainData. If the user has already clicked a different provider in the dropdown between the two renders, their click is silently overridden by the chainData arrival. -- **Recommendation:** Track the user-interaction state separately (`userPicked: boolean`) and gate re-seeding on `!userPicked`. Or only seed once on the first non-undefined `chainData`. -- **Ship-blocker for v1.4.16?** No — narrow race window, recoverable by re-clicking. Fix in v1.4.17. + ```ts + await prisma.$transaction( + async (tx) => { + const row = await tx.user.findUnique({ + where: { id: user.id }, + select: { dashboardWidgetsJson: true }, + }); + const current = resolveDashboardLayout(row?.dashboardWidgetsJson); + const next = { + ...current, + chartOverlayPrefs: { + ...(current.chartOverlayPrefs ?? {}), + [parsed.data.chartKey]: parsed.data.prefs, + }, + }; + await tx.user.update({ + where: { id: user.id }, + data: { + dashboardWidgetsJson: serializeDashboardLayout( + next, + ) as Prisma.InputJsonValue, + }, + }); + }, + { isolationLevel: "Serializable" }, + ); + ``` ---- + Add a vitest race-condition test against the existing testcontainer + setup (mirror `tests/integration/rate-limit-race.test.ts`). The + existing v1.4.18 integration test + (`tests/integration/chart-overlay-prefs.test.ts`) only exercises + serial writes and would not catch this defect. -## MEDIUM +- **Ship-blocker?**: NO — single-user, single-tab usage will not + trigger this (the optimistic update masks it). But a user with the + dashboard open in two tabs (a common pattern with PWAs) WILL hit + it. Fix in v1.4.18 if Marc wants; safe to defer to v1.4.19 with a + backlog entry. -### M1. `/api/admin/audit-log/actions` does an unbounded `groupBy(action)` -- **File:** `src/app/api/admin/audit-log/actions/route.ts:22-25` -- **Issue:** No index on `audit_logs.action`. With 1M+ rows the `groupBy` becomes an aggregate scan. Result is also un-cached. -- **Recommendation:** Add an index on `audit_logs.action`, OR cache the result for 5 min server-side, OR LIMIT the distinct list to the 100 most recent actions. +### H2 — `useChartOverlayPrefs` subscribes to "bp" prefs even when no chartKey is supplied -### M2. `RecommendationCard` `metricTypeToChartTypes()` falls through to verbatim type — silent passthrough -- **File:** `src/components/insights/recommendation-card.tsx:114-135` -- **Issue:** Unknown `metricType` returns `[metricType]` as-is. `HealthChart` then receives an unrecognised type and renders empty. Comment says "render empty rather than nothing" but the user sees nothing useful — a model that emits `metricSource.type = "BloodPressure"` (capitalised) gets dropped because the `lower === "bloodpressure"` branch hits a different casing for legacy-typed payloads. Worse: the model can emit any string. -- **Recommendation:** Allowlist explicitly; unknown types skip the mini-chart entirely (return `[]`). Add a Wide-Event annotation `ai_rec_unknown_metric_type` for observability. +- **Severity**: HIGH (correctness — wrong overlay state when chartKey is omitted) +- **File**: `src/components/charts/health-chart.tsx:321-324` +- **Issue**: -### M3. `appendLogEvent` runs unconditionally even for high-volume background jobs -- **Files:** `src/lib/logging/transports.ts:93-104`, `src/lib/logging/in-memory-buffer.ts` -- **Issue:** Every emitted event lands in the 500-entry ring buffer. For a worker process running pg-boss every minute this means the buffer is dominated by background events, leaving no space for HTTP request events that admins are more likely to debug. Plus the buffer is per-process so the web buffer never sees background events anyway — they ship to a buffer no admin reads. -- **Recommendation:** Skip background events from the buffer when `event.kind === "background"`, or partition by kind. The Loki sink still receives them. + ```ts + const overlayPrefs = useChartOverlayPrefs(chartKey ?? "bp"); + const showMA = chartKey ? overlayPrefs.prefs.showTrendIndicator : false; + const showTrend = chartKey ? overlayPrefs.prefs.showTrendArrow : false; + const showBands = chartKey ? overlayPrefs.prefs.showTargetRange : false; + ``` -### M4. `redactEventForEgress` does not redact `meta` or `action.details` -- **File:** `src/app/api/admin/app-logs/route.ts:72-96` -- **Issue:** Only `error.message`, `error.stack`, `warnings`, `http.user_agent`, `external_calls[].error` are redacted. `meta` and `action.details` carry annotated key=value pairs from `annotate()` calls — if any handler accidentally annotates a token (e.g. an Idempotency-Key replay logging the `Authorization` header), it leaks to the admin UI in raw form. -- **Recommendation:** Run `redactSecrets` recursively over `meta` and `action.details` string values too. Or whitelist a closed set of meta keys the egress allows. + When `chartKey` is undefined (mini mode / ad-hoc render), the hook + still subscribes to the dashboard-layout query and reads the + user's `bp` chart prefs — which are then ignored by the ternaries. + This is mostly wasteful (an extra fetch + reactive subscription), + but it has two real consequences: + 1. A render thrash when the user updates BP chart prefs from the + dashboard: every mini chart re-renders even though their state + doesn't change. + 2. If a user has no `bp` prefs but has `weight` prefs, the + defaulted-empty `bp` prefs leak through the optimistic-update + cache as "every flag false" — usually equivalent to the + correct default but conceptually wrong. -### M5. `chainBodySchema.providerType` cast `as unknown as [string, ...string[]]` -- **File:** `src/app/api/insights/provider-chain/route.ts:60-62` -- **Issue:** Double-cast hides the type. zod accepts `readonly [string, ...string[]]` from a tuple; `PROVIDER_CHAIN_TYPES` is declared with `as const` so it should already match. The double-cast risks a future PROVIDER_CHAIN_TYPES being non-tuple typed without compile error. -- **Recommendation:** `z.enum([...PROVIDER_CHAIN_TYPES] as readonly [string, ...string[]])` or use zod's `z.enum(PROVIDER_CHAIN_TYPES)` directly — the `as const` should already produce the tuple. +- **Recommendation**: Either pass `chartKey` to the hook only when + defined and short-circuit inside the hook when undefined, or move + the hook call inside a wrapper that early-exits before the hook is + called (the latter is a hooks-rule violation though, so prefer the + former): -### M6. `ContractCard` minor — "draft" pill doesn't use `litBarsFor` clamp comment correctly -- **File:** `src/components/insights/confidence-meter.tsx:55-64` -- **Issue:** Comment says "0..20 is 1, 21..40 is 2"; `Math.ceil(0/20) = 0` → clamped to 1; `Math.ceil(20/20) = 1`; but value < 25 routes to `draft` band BEFORE this is consulted. The clamp `if (lit < 1) return 1` defends against `value=0` for non-draft band, which is unreachable since `bandFor(0) === "draft"`. Dead code path; comment is correct in spirit but misleading. -- **Recommendation:** Drop the `lit < 1` clamp or document it as "defensive for refactoring." + ```ts + // In use-chart-overlay-prefs.ts: + export function useChartOverlayPrefs( + chartKey: ChartOverlayKey | undefined, + ): { prefs: ChartOverlayPrefs; setPrefs: ...; isSaving: boolean } { + const queryClient = useQueryClient(); + const { data: layout } = useQuery({ + queryKey: ["dashboard-layout"], + enabled: chartKey !== undefined, // skip when no key + ... + }); + const prefs = useMemo(() => { + if (!chartKey) return DEFAULT_CHART_OVERLAY_PREFS; + return layout?.chartOverlayPrefs?.[chartKey] ?? DEFAULT_CHART_OVERLAY_PREFS; + }, [layout, chartKey]); + ... + } + ``` -### M7. `host-metrics` route returns `BigInt → Number` without overflow guard -- **File:** `src/app/api/admin/host-metrics/route.ts:90` -- **Issue:** `Number(row.memTotalBytes)` and `Number(readDelta)` — if memTotalBytes ever exceeds 2^53 (~9 PB) precision is lost. Disk byte counters are cumulative; on a long-running machine they could overflow given enough time. -- **Recommendation:** Acceptable for v1.4.16 (typical hosts <16 GiB RAM, disks <10 PiB throughput). Document in the route comment. +- **Ship-blocker?**: NO — the user-visible behaviour is the clean-line + default (which is correct). Wasted bandwidth + re-render only. -### M8. `feedback-aggregator` `await prisma.appSettings.upsert` swallows errors silently -- **File:** `src/lib/jobs/reminder-worker.ts:951-972` -- **Issue:** Worker catches and warns but doesn't requeue. If the upsert fails (DB connection blip, lock contention) the day's summary is lost — admin AI-quality dashboard shows yesterday's data. -- **Recommendation:** Let pg-boss retry the job on throw (default behaviour). If retry storm is the concern, add a `maxRetries: 3, backoff: exponential` to the queue config. Today's "swallow + log" is too aggressive. +### H3 — Hidden bug-buddy achievement shares `bugReportCount` with public bugreport-1 — order-of-evaluation matters -### M9. `host-metric-sampler.getHostMetricRetentionDays` redundant double-check -- **File:** `src/lib/jobs/host-metric-sampler.ts:120-131` -- **Issue:** Returns DEFAULT for `parsed <= 0`, then again for `parsed < 1`. The second check is unreachable for valid integers. -- **Recommendation:** Drop the second check. +- **Severity**: HIGH (correctness around discovery) +- **File**: `src/lib/gamification/achievements.ts:354-375` (public + `bugreport-1`) and lines 638-647 (hidden `hidden-bug-buddy`) +- **Issue**: Both achievements use `metric: "bugReportCount"`. The + public one fires at target 1; the hidden one fires at target 5. + When the API response is serialised, both entries have + `metric: "bugReportCount"` but different `target` values. A snooper + comparing the two entries discovers that **the hidden achievement + shares the same metric as the public one but with a higher + threshold** — i.e. the trigger is "submit 5 bug reports". ---- + This is an even-worse leak than the metric-name leak (C1) because + the cross-reference is trivial: open the page, see one public + bugreport badge with `target: 1`, and one hidden badge with + `metric: "bugReportCount"` and `target: 5` — game over for the + surprise. -## LOW +- **Recommendation**: Fix subsumed by C1 (don't ship `metric` or + `target` for hidden+locked entries). With C1 fixed, this leak goes + away. -### L1. `evictPerStatusInsightCache` Prisma query uses both `startsWith` and `AND[contains]` -- **File:** `src/app/api/insights/generate/route.ts:62-73` -- **Issue:** `where: { userId, action: { startsWith: "insights." }, AND: [{ action: { contains: "-status." } }] }`. The combined `AND` is redundant under "where AND" semantics — Prisma already AND's all top-level keys. The `AND` wrapper is needed only because the top-level `action` key already has a `startsWith`; piling another `action` filter inside an `AND[]` is the right pattern. Comment is good. -- **Recommendation:** Looks right, no change needed. Considered nit. + As a separate consideration: the design choice of reusing a public + metric for a hidden Easter-egg is questionable to begin with — + consider giving the hidden bug-buddy its own metric + (`bugReportCountHidden` or similar), so even server-side audit + trails don't accidentally reveal that "5 bug reports" is the + trigger via shared metric counting. -### L2. `evictPerStatusInsightCache` does not annotate the row count it deleted -- **File:** `src/app/api/insights/generate/route.ts:62-73` -- **Issue:** `deleteMany` returns `{ count }` which is dropped. -- **Recommendation:** Annotate `evicted_status_caches` so admin observability sees the eviction. +- **Ship-blocker?**: NO once C1 ships. Tracked under C1 fix. -### L3. `provider-runner` legacy `runRawCompletionWithFallback` is duplicated code -- **File:** `src/lib/ai/provider-runner.ts:312-371` -- **Issue:** Near-identical to `runWithFallback` (lines 229-288); diff is the inner `provider.generateCompletion` vs `generateInsight` call. The cache logic, fallbackHops shape, error classification, and Wide-Event annotation are duplicated. -- **Recommendation:** Extract a shared helper `runChain(args, runOnce)` that takes the per-attempt callback. Defer if v1.4.17 is migrating the legacy route anyway. +### H4 — `getMoodMetrics` improvement-window slices distinct days, not 7 calendar days -### L4. `RecommendationsGrid` STAGGER_INTERVAL_MS = 100 ms — a 10-rec response takes 1s to fully animate in -- **File:** `src/components/insights/recommendations-grid.tsx:81` -- **Issue:** No upper bound on stagger total. Worst case 10 recs * 100 ms = 1s before last card lands. -- **Recommendation:** Cap with `min(index * 100ms, 600ms)` so late cards fade in immediately after the cascade-cap. +- **Severity**: HIGH (correctness — diverges from "7-day window" semantics) +- **File**: `src/lib/gamification/expansion-metrics.ts:104-138` +- **Issue**: `moodImprovementHit` slides a 7-distinct-day window + rather than a 7-calendar-day window: -### L5. `findClosestDia()` allows multiple sys to pair with the same dia -- **File:** `src/lib/analytics/bp-in-target.ts:80-100` -- **Issue:** Each sys picks its own closest dia independently. A burst of sys readings between two dia readings will all match the closer dia. Counts inflated for high-frequency sys-only logging. -- **Recommendation:** Acceptable for the 5-min-window pair-rule; closing pair after consumption would change semantics. Document or accept. + ```ts + for (let i = 13; i < sortedDays.length; i++) { + const recent = sortedDays.slice(i - 6, i + 1); // 7 distinct days + const prior = sortedDays.slice(i - 13, i - 6); // 7 distinct days + ... + } + ``` -### L6. `host-metrics-chart.tsx` `Intl.DateTimeFormat` recreated per `buildChartRows` call -- **File:** `src/components/admin/host-metrics-chart.tsx:93-97` -- **Issue:** Locale formatter is created inside the pure helper, but the helper itself is wrapped by `useMemo`. Each unique `data?.samples` reference triggers re-instantiation. Cheap per call, but wasteful. -- **Recommendation:** Move the formatter to module-scope. + If a user logs daily for a month, that's fine. But a user who logs + on April 1, 2, 3, 4, 5, 6, 7, then nothing for a month, then + May 1, 2, 3, 4, 5, 6, 7 will have the comparison fire across the + one-month gap — comparing two arbitrarily-spaced 7-entry sets, not + two contiguous 7-day windows. -### L7. `legacy-payload.ts` checks `rationale === undefined` AND `=== null` AND `typeof !== "object"` separately -- **File:** `src/lib/ai/legacy-payload.ts:46-52` -- **Issue:** `typeof rationale !== "object"` already returns true for undefined and null in JS. The early checks are belt-and-braces but redundant. -- **Recommendation:** Simplify to `if (rationale === null || typeof rationale !== "object") return true;` — `undefined` is also `typeof "undefined"` so the check covers it. + The doc-comment claims _"any 7-day window had a mean score at least + 1.0 higher than the preceding 7-day window"_ (line 96) which is + **NOT** what the code does. -### L8. Comparison overlay `_compare` data keys cannot collide with metric types but use string concat -- **File:** `src/components/charts/health-chart.tsx:558` -- **Issue:** `merged[`${type}_compare`] = v` — relies on no metric type ending with `_compare`. Stable today, fragile to a future `WEIGHT_compare` enum value (unlikely but possible). -- **Recommendation:** Prefix with a non-identifier separator: `merged[`__compare__:${type}`]`. Or a `Map`. Acceptable as-is. + Practical impact: the achievement may unlock too eagerly for users + with sparse logging patterns. Not coercive (Marc's design rule is + satisfied), but the semantics are wrong. -### L9. `ai-section.tsx:1255` — `seededKey` joined with `,` from providerType, not stable JSON -- **File:** `src/components/settings/ai-section.tsx:1270` -- **Issue:** `chain.map((c) => c.providerType).join(",")` is derived from `chain`, which only changes when GET refetches. Same providerType-set in different order produces the same key — but if two reorderings happen before re-seed, the local entries diverge from the GET. Edge case; fine. +- **Recommendation**: Either: + - Update the doc-comment to say "compares the last 7 entries to the + 7 preceding entries" and accept the simplification, OR + - Make the window calendar-day-based: build a daily array including + no-data days (or a `null` for missing), require a minimum number + of populated days in each window, then compute the means. -### L10. `host-metrics` route does NOT cache prev BigInt across HTTP calls -- **File:** `src/app/api/admin/host-metrics/route.ts:85-122` -- **Issue:** Per-request `prevReadBytes` walk; the first row in the returned window has `diskReadBps: null` because there's no previous sample to delta from. UI hides the line for that row. Documented behaviour. Fine. + The first option is cheaper and probably what Marc actually wants + ("user is moving in the right direction"); the second is more + rigorous but adds complexity. ---- + Either way, add a unit test for the cross-month-gap case to lock + in the chosen semantics. -## Cross-cutting +- **Ship-blocker?**: NO — the achievement still functions and it's + user-positive (not coercive). Document or fix in a follow-up. -### Plan-deviation observations +### H5 — `consistentMonthCount` keeps growing — wasted DB churn for once-fired achievement -- **B5b/B5c/B5d strict path is dark in production** — the production `/api/insights/generate` route still rides the `runRawCompletionWithFallback()` legacy shape. STATE.md acknowledges this as a "v1.4.17 migration" deferment, but the user-facing claims of "rationale required", "deterministic confidence", and "citation enforcement" are exercised only by tests. See **H3** above. Recommend explicit Marc decision before tagging v1.4.16: ship dark + adjust release-note language, OR migrate route to strict path. -- **A2 BD-Zielbereich predicate change is structurally sound** — `isBpReadingInTarget` ceiling+floor semantics centralised across all 6 call sites. Marc's stated 50% expectation is plausible given the `90 ≤ sys ≤ sysHigh AND 50 ≤ dia ≤ diaHigh` predicate; integration test `tests/integration/bp-in-target.test.ts` covers it. No drift risk. -- **B7 export endpoints** — well structured, share `export:` rate-limit bucket, audit-log on every download, no envelope-wrap on the file contents. `full-backup` round-trips through the canonical `BACKUP_SCHEMA_VERSION` shape so admin upload + restore accepts it. Clean. -- **Wave-C qemu-arm64 drop** — well-documented in `docker-publish.yml`. amd64-only is the right call given Marc's prod target. v1.5 path documented. +- **Severity**: HIGH (DB wear on long-tenured users) +- **File**: `src/lib/gamification/expansion-metrics.ts:175-182` and + `src/app/api/gamification/achievements/route.ts:601-610` + (persistence flow) +- **Issue**: `consistentMonthCount` returns the _total_ number of + consistent months over the user's entire history. The achievement + unlocks at target 1 (first consistent month), but the metric keeps + incrementing every month thereafter. Each new consistent month + causes: + 1. Recompute on every `/api/gamification/achievements` call + (every 2 minutes via `` polling) + 2. The `metrics.consistentMonthCount` column grows unboundedly + in the response payload (a small int, but conceptually unbounded) + 3. No corresponding state in the UI exposes this count -### Tests + i18n parity + Compare to `moodEntryCount`, which does have multi-tier + achievements at 1 / 50 / 200 etc. — so the unbounded growth there + is justified. -- New SSR + integration tests are present for each new endpoint and for `RecommendationCard` rationale + confidence + feedback wiring. -- `recommendation-feedback.test.tsx` not located in the repo — STATE.md B5e claims +20 tests; the wire integration `tests/integration/insights-feedback.test.ts` covers the API but the component is exercised through the integration harness only, not isolated. -- i18n parity: B5a/B5e/B8 add EN+DE keys with parity tests. No drift detected. +- **Recommendation**: Either: + - Add tiered achievements at consistent-month 1 / 3 / 6 / 12 to + justify the growing count, OR + - Cap the count at the highest threshold (`min(count, 1)`) inside + `getEngagementMetrics()` to make the metric effectively boolean + and stop the growth. ---- + Same review applies to `moodImprovementHit` (already capped at 1 + by the `break;`), `nightOwlCount` (counted but no tiered targets + beyond 1), `earlyBirdCount`, `leapDayCount`, `doctorPdfCount`, + `localeFlipCount`. Consider capping all single-target hidden + metrics at 1 to limit the wire-payload growth and avoid a future + "did the user trigger this 100 times?" question that the data + doesn't actually answer. -## Severity rollup +- **Ship-blocker?**: NO. Pure efficiency / hygiene, no incorrect + user-facing behaviour. -- **CRITICAL:** 0 -- **HIGH:** 8 (H1-H8) -- **MEDIUM:** 9 (M1-M9) -- **LOW:** 10 (L1-L10) +## MEDIUM / LOW + +### M1 — Earnability flags treat `BLOOD_PRESSURE_DIA`-only users as having no BP data + +- **Severity**: MEDIUM +- **File**: `src/lib/gamification/expansion-metrics.ts:74-83` +- **Issue**: `countMeasurementsByType` counts only `BLOOD_PRESSURE_SYS` + toward `bpCount`. If a user (or a sync source) ever wrote + `BLOOD_PRESSURE_DIA` rows without paired `BLOOD_PRESSURE_SYS` rows, + the BP achievements would not become earnable. The comment at + lines 26-28 of the test file explains the design choice ("avoid + double-counting") but doesn't justify why DIA-only data invalidates + the user from BP achievements. +- **Recommendation**: Either: + - `bpCount = max(sys, dia)` so DIA-only users still earn, OR + - Document the assumption "BP measurements always include a SYS + row" and add a guard that drops DIA-only writes upstream. + No action probably needed for v1.4.18 — Withings + manual entry + always pair them — but worth a backlog note. +- **Ship-blocker?**: NO. + +### M2 — Hidden hint text is revealed in the unlock toast description + +- **Severity**: MEDIUM +- **File**: `src/components/gamification/achievement-unlock-notifier.tsx:120-129` +- **Issue**: When a hidden achievement unlocks, the toast description + is `${t(achievement.titleKey)} — ${t(achievement.descriptionKey)}` + which combines the **real** title and description (e.g. + `"Night owl — Logged an entry between 02:00 and 04:00 in the morning."`). + This is correct _for the toast itself_ (the unlock IS the moment + to reveal the surprise). However, after the toast dismisses, the + notifier persists `seenIdsRef` so the hidden ID is no longer + treated as new. If the user switched their device clock back, or + cleared localStorage, the toast would re-fire — but that's an + acceptable edge case. +- **Recommendation**: No action needed. Behaviour is correct; flagging + here only because the brief asked the reviewer to look for hidden- + hint leakage. The unlock-toast reveal is by design. +- **Ship-blocker?**: NO. + +### M3 — `/admin/api-tokens` mobile spec covers AdminShell but not SettingsShell + +- **Severity**: MEDIUM (parity) +- **File**: `e2e/admin-api-tokens-mobile.spec.ts` +- **Issue**: A2 fixed the painted-scrollbar in BOTH `AdminShell` and + `SettingsShell` (same `no-scrollbar` utility). The Playwright + e2e regression guard at `e2e/admin-api-tokens-mobile.spec.ts` only + asserts the AdminShell path. SettingsShell has unit-level + coverage in `src/components/settings/__tests__/settings-shell.test.tsx` + (14 lines added) which checks the className. If `globals.css` ever + removes the `.no-scrollbar` utility, the unit test still passes + (it asserts the className, not the rendered scrollbar). +- **Recommendation**: Add a 5-line Playwright spec hitting + `/settings/profile` at Pixel-5 viewport asserting no horizontal + scroll. Mirror the AdminShell spec exactly. +- **Ship-blocker?**: NO. + +### M4 — `chart-overlay-prefs` route doesn't annotate the previous values for audit trail + +- **Severity**: LOW +- **File**: `src/app/api/dashboard/chart-overlay-prefs/route.ts:81-92` +- **Issue**: The `annotate()` call at the end captures the new flags + but not the previous ones, so a session log can't reconstruct the + full toggle history. Compare to `applyProfileUpdate` in + `src/lib/auth/profile-update.ts:89-94` which captures `from` / + `to` for the locale change. +- **Recommendation**: If audit forensics matters for chart overlay + prefs (probably not — Marc said this is a UX prefs blob not an + analytical attribute), capture both old and new flag sets in the + annotation. Otherwise drop this finding. +- **Ship-blocker?**: NO. + +### M5 — `health-chart.tsx` no longer renders the personal-baseline ReferenceLine — confirm via grep + +- **Severity**: LOW (verification — the test already enforces this) +- **File**: `src/components/charts/health-chart.tsx`, + `src/components/charts/__tests__/health-chart-overlay-defaults.test.tsx` +- **Issue**: The default-overlay test reads the source file and + asserts no unconditional baseline ReferenceLine. This is a brittle + source-string assertion (`readFileSync` + regex) that will break + if the file is reformatted or split. It's a reasonable + short-term guard, but it's not the same as testing the rendered + output. +- **Recommendation**: Replace with an SSR render assertion: render + `` with default props, assert no element has + `data-slot="chart-personal-baseline-line"` (or whatever the + ReferenceLine's identity ends up being). Mirror the medication + chart's overlay-toggle test pattern. +- **Ship-blocker?**: NO. + +### L1 — `parseDayKey` redundantly sets UTC hours after constructor + +- **Severity**: LOW +- **File**: `src/lib/gamification/expansion-metrics.ts:230-232` (used + inside `getEngagementMetrics`) +- **Issue**: `parseDayKey` constructs the date with + `new Date(Date.UTC(year, month - 1, day, 12, 0, 0))` (already + noon UTC). The next line `cursor.setUTCHours(12, 0, 0, 0)` is a + no-op. +- **Recommendation**: Drop the redundant line. +- **Ship-blocker?**: NO. + +### L2 — `de.json` / `en.json` symmetry not asserted for the v1.4.18 expansion strings + +- **Severity**: LOW +- **File**: `messages/de.json`, `messages/en.json`, + `src/lib/__tests__/i18n*.test.ts` (per CLAUDE.md the symmetry + guard exists) +- **Issue**: Couldn't verify in this review whether the existing + i18n parity test enumerates the 22 new keys (mood × 3 + measurement + counts × 7 + engagement × 4 + hidden × 6 + hiddenCard placeholder + - hiddenUnlockToast + 2 category labels). The expansion adds ~30+ + new keys; if the parity test is data-driven (read both files, + assert keys match), this is automatic. If it's hand-rolled, some + may have slipped. +- **Recommendation**: Quick sanity run of `pnpm test +src/lib/__tests__/i18n*` to confirm 0 failures. Already in CI most + likely. +- **Ship-blocker?**: NO. + +## SUMMARY + +- 1 CRITICAL (C1 — hidden achievement trigger leakage via API response) +- 5 HIGH (H1 race, H2 hook subscription, H3 bug-buddy metric leak [subsumed by C1], H4 mood-window semantics, H5 unbounded count growth) +- 7 MED/LOW (M1-M5, L1-L2) + +Ship-blocker count: **1** (C1). + +Recommended action before v1.4.18 release: scrub `metric` / +`titleKey` / `descriptionKey` / `target` / `current` / +`progressPercent` / `points` from hidden+locked entries in the API +response body. Add a route-level test that asserts hidden trigger +strings are not present in the response while the achievement is +locked, and ARE present once unlocked. Item H3 is auto-resolved by +the C1 fix. + +The H1 race condition is real but masked in single-user usage; safe +to defer with a backlog entry. H2-H5 are correctness / hygiene +issues; defer to v1.4.19 absent regression reports. M/L items are +follow-on hygiene work. + +Strong work overall — A1 BD-tile fix is clean and well-tested +(`computeBpInTargetWindows` reuses the v1.4.16 ceiling predicate +correctly with explicit null-handling for sparse data), A2 painted- +scrollbar fix correctly identifies that the same defect lives in +both shells and uses a single utility class to fix both, A3 chart +revert is thorough (all three reversions land + the toggle UI +persists per-chart cleanly), and B1 achievements expansion has +strong unit + integration test coverage for the visible cases. The +discovery filter design is correct. The remaining work is the wire- +shape hardening for the hidden category and the race-condition +guard for concurrent overlay toggles. diff --git a/.planning/phase-D-design-findings.md b/.planning/phase-D-design-findings.md index 6b1bfb912..26a0ed2f1 100644 --- a/.planning/phase-D-design-findings.md +++ b/.planning/phase-D-design-findings.md @@ -1,265 +1,445 @@ -# Phase D — DESIGN/UX review (v1.4.16, Apple-Health-quality benchmarking lens) - -Reviewer: phase-D DESIGN/UX agent -Marathon: v1.4.16 (live: 1.4.15; under-test: 1.4.16) -Date: 2026-05-09 (UTC+2) -Approach: source-code audit (v1.4.16 not yet deployed; live Playwright not driven). - -Per brief: "Apple Health benchmarking lens — does the polish reach the bar Marc set?" - ---- - -## Executive summary - -v1.4.16 ships a *huge* amount of code-level polish (gradient hero, ConfidenceMeter, RationaleCard with mini-charts, severity-coloured grid, comparison overlay, host-load chart, AI quality table). However, the **headline finding** is that the most visible Apple-Health-quality affordances — the polished `` consuming ``, ``, ``, `` — are **NOT mounted on any production page**. They live in tests only. - -The v1.4.16 release as Marc would experience it on `/insights` is the same v1.4.15 text-only `` per section. The dashboard `/` likewise does not import ``. **All of B5c, B5d, B5e, and B1b's cross-feature polish is invisible to the user.** This is the single biggest disconnect between the work that landed and the user's perception of the release. - -A second category: the comparison toggle (B8) is *only* reachable from `/settings/dashboard-layout`. There is no on-chart, on-tile, or on-page control on `/` or `/insights`. The brief was explicit ("Mobile-friendly comparison toggle, not a fragile hover-only interaction") and the UX cost is severe: a v1.4.16 user has zero on-surface affordance to toggle Vormonat / Vorjahr. - -Findings below grouped by severity. - ---- - -## CRITICAL — ship-blockers for v1.4.16 (must fix before tag) - -### C1 — `/insights` does NOT mount the polished `` / ``. B5c/d/e/B1b are invisible - -- **File:** `src/app/insights/page.tsx:1006-1539` -- **Issue:** every insights section renders `` (text-only summary, no rec card, no rationale, no confidence meter, no thumbs-feedback). The Apple-Health-style polish from B1b (`` is mounted, but it's just a header band) and the entire B5c/d/e rec-card surface (``, ``, ``, ``) is never reachable from any live route. Confirmed via `grep "InsightAdvisorCard"` — zero non-test imports across `src/`. -- **Recommendation:** wire `` into `/insights` (consume `/api/insights/generate`) **or** explicitly de-scope B5c/B5d/B5e/B1b from v1.4.16 release notes and tell Marc the visual polish ships in v1.4.17. The B5c/d/e reports each flag this gap ("E2E not added — not mounted on a live route") but none actually wire the card. Without wiring, Marc gets v1.4.15 UX with extra unused code paths. -- **Ship-blocker?** YES. - -### C2 — `` (dashboard insights tile) has zero live imports — orphan component - -- **File:** `src/components/insights/insights-card.tsx:49`; `src/app/page.tsx` (no import) -- **Issue:** B1b commit 5 (`d2cdf9d`) replaced the orphan v1.4.0 `` with a polished preview that renders top severity-ordered recs + ring-variant `` inline. The dashboard never imports it. `grep "InsightsCardPreview" src/app` — zero matches. The dashboard renders `RecentAchievementsCard` + tile strip + charts, no insights preview. -- **Recommendation:** mount `` on `/` (the B1b report flagged this as "When dashboard wiring lands…") or hide the file and remove from v1.4.16 marketing claims. -- **Ship-blocker?** YES — Marc sees no insights tile on the dashboard despite the B1b report explicitly citing dashboard polish. - -### C3 — Comparison toggle (B8) has NO on-surface control. Buried 3 clicks deep in Settings - -- **File:** `src/components/settings/dashboard-layout-section.tsx:192-198` (only place `compareBaseline` is settable) -- **Issue:** Marc must navigate to /settings/dashboard-layout, scroll past 14 widgets, find the "Comparison Baseline" Select, change it, save, then back-navigate to / or /insights. The brief explicitly required "Mobile-friendly comparison toggle (not a fragile hover-only interaction)" — what shipped is a Settings-Save round-trip every time the user wants to flip from "vs. last month" to "vs. last year" or back to off. The B8 report's deferred-list mentions a `` segmented control was scoped but never built; only the persistence layer + chart-side rendering exist. -- **Recommendation:** add a `` (segmented "None / Vormonat / Vorjahr") next to the existing range tabs (7d/30d/90d/All) on each chart, OR a single page-level toggle pinned next to the dashboard greeting. Persists via the same `dashboardWidgets` PUT the settings page uses, no API change. Without it, the comparison-overlay code (compareBaseline plumbed through 19 chart sites + tile-strip + `comparison.deltaVs.*` i18n) is dead UX — admins can demo it, users can't find it. -- **Ship-blocker?** YES — major B8 deliverable is invisible without an on-surface toggle. - ---- - -## HIGH — fix in v1.4.16 if cycle allows, or document for v1.4.17 - -### H1 — Recommendation feedback thumbs are 28×28 px hit-area — fails WCAG 2.5.5 (44 px floor) - -- **File:** `src/components/insights/recommendation-feedback.tsx:268, 283` -- **Issue:** both thumb buttons use `inline-flex h-7 w-7 items-center justify-center rounded-md`. h-7 = 28 px. WCAG 2.5.5 minimum is 44×44 CSS px. Wave-C MED bottom-nav fix was specifically about 44 px floors; the same floor must apply to per-rec feedback that Marc taps on a Pixel 5. Also: zero `focus-visible:` styles on the raw `