From 232ea43d99ee0467eec534d6b95d06647b6d5e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 16 May 2026 09:28:40 +0200 Subject: [PATCH] chore(release): v1.4.28.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## [1.4.28.1] — 2026-05-16 — Dashboard-save hotfix "Speichern" on the Dashboard works again for every account whose layout was last persisted before v1.4.28. The v1.4.28 retire of the `glp1` widget id left an orphan entry in `dashboardWidgetsJson` for legacy users, which made the save validator reject the payload; the resolver now filters unknown widget ids on read. ### Fixed - **Dashboard save against legacy layouts.** `resolveDashboardLayout` in `src/lib/dashboard-layout.ts` drops widget ids that are no longer in the registry before returning the layout, so the `dashboardWidgetsJson` round-trip (`GET /api/me/dashboard` → user edit → `PUT /api/me/dashboard`) succeeds even when the persisted JSON still names `glp1` or any other retired widget. ### Tests - `src/lib/__tests__/dashboard-layout.test.ts` gains a case for the retired-id filter (19/19 green). --- .github/workflows/docker-publish.yml | 65 ++- .planning/research/v1428-r4-code.md | 168 ++++++++ .planning/research/v1428-r4-dead-code.md | 200 ++++++++++ .planning/research/v1428-r4-design.md | 279 +++++++++++++ .planning/research/v1428-r4-i18n.md | 185 +++++++++ .planning/research/v1428-r4-ios-contracts.md | 248 ++++++++++++ .planning/research/v1428-r4-product-lead.md | 273 +++++++++++++ .planning/research/v1428-r4-security.md | 91 +++++ .planning/research/v1428-r4-senior-dev.md | 120 ++++++ .planning/research/v1428-r4-simplifier.md | 199 ++++++++++ .planning/research/v1428-r4-ui-conformity.md | 375 ++++++++++++++++++ .planning/round-3c-coach-report.md | 88 ++++ .planning/round-3c-insights-report.md | 158 ++++++++ .planning/round-3c-med-report.md | 162 ++++++++ .planning/round-3d-perf-report.md | 96 +++++ .planning/round-4-reconcile-A-report.md | 110 +++++ .planning/round-4-reconcile-B-report.md | 136 +++++++ .planning/round-5-geolite2-ci-fix-report.md | 162 ++++++++ .planning/round-5-release-closure-report.md | 296 ++++++++------ .../round-5-v14281-hotfix-blocker-report.md | 122 ++++++ .planning/v1429-backlog.md | 171 ++++++++ CHANGELOG.md | 23 ++ package.json | 2 +- scripts/fetch-geolite2.sh | 61 +-- src/app/api/version/__tests__/route.test.ts | 2 +- src/lib/__tests__/dashboard-layout.test.ts | 22 + src/lib/dashboard-layout.ts | 15 +- 27 files changed, 3662 insertions(+), 167 deletions(-) create mode 100644 .planning/research/v1428-r4-code.md create mode 100644 .planning/research/v1428-r4-dead-code.md create mode 100644 .planning/research/v1428-r4-design.md create mode 100644 .planning/research/v1428-r4-i18n.md create mode 100644 .planning/research/v1428-r4-ios-contracts.md create mode 100644 .planning/research/v1428-r4-product-lead.md create mode 100644 .planning/research/v1428-r4-security.md create mode 100644 .planning/research/v1428-r4-senior-dev.md create mode 100644 .planning/research/v1428-r4-simplifier.md create mode 100644 .planning/research/v1428-r4-ui-conformity.md create mode 100644 .planning/round-3c-coach-report.md create mode 100644 .planning/round-3c-insights-report.md create mode 100644 .planning/round-3c-med-report.md create mode 100644 .planning/round-3d-perf-report.md create mode 100644 .planning/round-4-reconcile-A-report.md create mode 100644 .planning/round-4-reconcile-B-report.md create mode 100644 .planning/round-5-geolite2-ci-fix-report.md create mode 100644 .planning/round-5-v14281-hotfix-blocker-report.md create mode 100644 .planning/v1429-backlog.md diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index d50242e34..5aa60a63e 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -103,17 +103,24 @@ jobs: # downloads both editions into `assets/geolite2/` so the Dockerfile # COPY at line 78 picks them up. # - # The build is intentionally resilient to a missing - # `MAXMIND_LICENSE_KEY` secret: when the key is unset we emit a - # `::warning::` line, drop a `.empty` marker into the assets - # directory so the Dockerfile COPY still has a non-empty source, - # and continue cleanly. The runtime resolver in `src/lib/geo.ts` - # detects the marker on first lookup and emits a one-shot - # admin notification — the maintainer hears about the gap from - # the running app rather than from a red CI badge that blocks - # every release. To enable the offline feature, set the secret - # under repo Settings → Secrets and variables → Actions and - # redeploy. + # Two intentional paths: + # - `MAXMIND_LICENSE_KEY` unset → emit a `::warning::`, drop a + # `.empty` marker into the assets directory so the Dockerfile + # COPY still has a non-empty source, and continue cleanly. The + # runtime resolver in `src/lib/geo.ts` detects the marker on + # first lookup and emits a one-shot admin notification — the + # maintainer hears about the gap from the running app rather + # than from a red CI badge that blocks every release. + # - `MAXMIND_LICENSE_KEY` set → run `fetch-geolite2.sh` and + # fail the build if it errors. The "Verify GeoLite2 databases" + # step below adds a belt-and-braces assertion that both MMDB + # files landed in `assets/geolite2/` before the Dockerfile + # COPY reads the directory, so v1.4.28-era silent fallback + # (offlineGeoEnabled: false on both nodes after a tag build) + # can no longer ship. + # + # To enable the offline feature, set the secret under repo + # Settings → Secrets and variables → Actions and redeploy. - name: Fetch GeoLite2 databases env: MAXMIND_LICENSE_KEY: ${{ secrets.MAXMIND_LICENSE_KEY }} @@ -126,6 +133,42 @@ jobs: fi ./scripts/fetch-geolite2.sh + # Build-time assertion: with the license key present, both MMDB + # files must land in `assets/geolite2/` before the Dockerfile + # COPY at line 78 picks them up. The fetch script already fails + # loud on a curl / tarball error, but a stale `.empty` marker + # from a previous keyless run or a workspace-cache surprise could + # otherwise survive into the image — this step is the operator + # guarantee that "key set" really means "MMDBs in the image". + # + # `secrets.*` is not available in step-level `if:` expressions, + # so we surface the presence flag through env and gate inline. + - name: Verify GeoLite2 databases + env: + MAXMIND_LICENSE_KEY: ${{ secrets.MAXMIND_LICENSE_KEY }} + run: | + if [ -z "$MAXMIND_LICENSE_KEY" ]; then + echo "MAXMIND_LICENSE_KEY unset — verification skipped (offline geo tier intentionally absent on this build)." + exit 0 + fi + missing=() + for mmdb in GeoLite2-City.mmdb GeoLite2-ASN.mmdb; do + if [ ! -s "assets/geolite2/$mmdb" ]; then + missing+=("$mmdb") + fi + done + if [ "${#missing[@]}" -gt 0 ]; then + echo "::error::MAXMIND_LICENSE_KEY is set but the following GeoLite2 MMDB files are missing or empty in assets/geolite2/: ${missing[*]}. The Dockerfile would COPY them into /opt/geolite2/ in the final image, leaving the offline geo tier disabled at runtime. Investigate the previous 'Fetch GeoLite2 databases' step before retrying." + exit 1 + fi + if [ -f assets/geolite2/.empty ]; then + echo "::error::Stale .empty marker present in assets/geolite2/ despite MAXMIND_LICENSE_KEY being set. The runtime resolver would treat the image as missing the offline tier. Investigate the previous 'Fetch GeoLite2 databases' step before retrying." + exit 1 + fi + city_size="$(stat -c%s assets/geolite2/GeoLite2-City.mmdb)" + asn_size="$(stat -c%s assets/geolite2/GeoLite2-ASN.mmdb)" + echo "::notice::GeoLite2 MMDBs verified — GeoLite2-City.mmdb ${city_size} bytes; GeoLite2-ASN.mmdb ${asn_size} bytes." + # Per-ref + per-platform gha cache scope. Tag builds warm-start # off main's last good layer set for the same platform; tag and # main builds at the same SHA do not contend on writes, and diff --git a/.planning/research/v1428-r4-code.md b/.planning/research/v1428-r4-code.md new file mode 100644 index 000000000..8cf04ecc9 --- /dev/null +++ b/.planning/research/v1428-r4-code.md @@ -0,0 +1,168 @@ +--- +file: .planning/research/v1428-r4-code.md +purpose: R4 code review of the v1.4.28 diff vs v1.4.27 — 30 commits, severity-grouped findings +created: 2026-05-16 +contributor: R4 code-review +--- + +# v1.4.28 R4 — code review + +Scope: 30 commits `948fcd93..HEAD` on `develop` (post-kickoff). Bug-fix sub-pass (FB-B1/C1/D2/D3), six retirements, consistency sweep, perf instrumentation. Review is read-only; findings are sorted by severity with a fix-shape suggestion per item. + +## Critical + +### R4-CODE-C1 — Aggregated `/api/measurements` truncates before bucketing +File: `src/app/api/measurements/route.ts:67-86` (commit `b00be286`) +The aggregation branch fetches `prisma.measurement.findMany({ take: limit, orderBy: { measuredAt: "asc" } })` then bucketises in memory. With `orderBy asc` the server returns the earliest `limit` rows in the window — for a Pulse-every-minute account selecting a 365-day window with `limit=5000`, only ~3.5 days of raw data feed the aggregator and the chart paints 3 daily buckets instead of 365. The whole point of FB-D2 is high-density data; the aggregation defeats itself on the exact accounts it targets. +Fix shape: drop `take`/`skip` from the aggregation branch (the bucket count is already bounded by the window length, never exceeds ~52 weekly or ~365 daily rows per type), or push aggregation into SQL via `date_trunc` + `GROUP BY`. Add a regression test that seeds 30 000 rows across a 365-day window and asserts the response bucket count equals the calendar-day count, not `min(limit, count)/grain`. + +### R4-CODE-C2 — Aggregation auto-triggers on any wide window, breaking the documented iOS contract +File: `src/app/api/measurements/route.ts:56` + `src/lib/measurements/range-aggregation.ts:44-46` (commit `b00be286`) +The commit message asserts the aggregated branch fires only when "from + to are present AND the window is wide enough" — but the implementation fires on `from && to && pickAggregateGrain(...) !== "raw"`, and `pickAggregateGrain` auto-promotes to `daily` past 90 days even without an explicit `aggregate` param. iOS currently calls `/api/measurements?limit=400` without `from`/`to` and stays safe, but a future iOS update that adds date filtering for a > 90-day drill-down silently receives the aggregated wire shape (`{ type, value, measuredAt, count }`) instead of the locked `MeasurementWireDTO`. The `.planning/v15-ios-handoff/08-locked-contracts.md` shape for `GET /api/measurements` is `Measurement[]` with full row fields. +Fix shape: gate the aggregation branch on `aggregate !== undefined` (caller must opt in explicitly); keep the auto-grain promotion as a hint the chart applies client-side. Add an integration test that asserts `/api/measurements?from=…&to=…` without `aggregate` still returns the raw DTO shape regardless of window width. + +### R4-CODE-C3 — Web-vitals beacon is an unauthenticated log-injection vector +File: `src/app/api/internal/web-vitals/route.ts:45-76` + `src/components/monitoring/web-vitals-reporter.tsx:21-49` (commit `ebf83b1e`) +The route accepts unauthenticated POSTs of arbitrary `{ name, value, rating, navigationType }` and forwards `name`/`rating`/`navigationType` strings verbatim into `annotate({ meta })`. The header comment claims "CSP + same-origin policy keep cross-site reporting out" — neither does. `sendBeacon` issues a simple POST with no CORS preflight; any external page can flood `/api/internal/web-vitals` with custom `name` payloads and pollute the wide-event log. There's also no rate limit ("deliberately none") and the route is publicly indexable. +Fix shape: restrict `name` to the documented web-vitals identifier set (`CLS|FCP|FID|LCP|TTFB|INP`); reject unknown names early; clamp `value` to a sane numeric range; add a session-cookie check or a same-origin Referer guard at the route boundary. Web Vitals don't need auth on a logged-in user — but they do need an attestation that the caller came from a HealthLog page. + +## High + +### R4-CODE-H1 — Mislabelled commit `0e7c97c5` bundles FB-K trend-row work under a CTA-variant subject +File: commit `0e7c97c5` body vs diff +The subject claims "align briefing empty-state CTA variant" and the body only describes the daily-briefing CTA. The diff actually rewrites `trends-row.tsx` (FB-K1/K2 equal-height contract), `mood-chart.tsx`, `trend-annotation.tsx` and two new test files — six files, 130 net additions. Atomic-commit hygiene violation; reviewers cannot bisect the FB-K contract from the FB-M2 CTA fix; future `git log -- trends-row.tsx` will mislead. +Fix shape: leave as-is for v1.4.28 (rewriting history before tag is destructive) but add a `Note: also lands FB-K1/K2 trend-row equal-height contract` postscript in the release notes; pin atomic-commit discipline in the v1.4.29 kickoff. + +### R4-CODE-H2 — Mislabelled commit `235e52cb` ships Coach drawer + mobile rail tray under a charts subject +File: commit `235e52cb` body vs diff +Subject: "refactor(charts): single HealthChartDynamic re-export". Diff: 237 net additions across `coach-drawer.tsx`, the new `mobile-rail-tray.tsx`, and its test fixture. Zero chart files. The commit body is missing entirely. Worse atomic-hygiene violation than H1 — the subject is actively misleading. +Fix shape: same as H1 — note in release closure; the chart re-export work appears to live in `8f3bfc37` ("collapse health-chart dynamic imports onto re-export"), so the two commit subjects are swapped between two adjacent commits. + +### R4-CODE-H3 — Test-then-fix split across commits leaves intermediate `develop` red +File: `1b0e81ae` (icon-only coach launch) → `5570971f` (test update, 23 min later) +`1b0e81ae` rewrites `target-coach-button.tsx` to render an icon-only affordance but leaves `targets-responsive.test.tsx` asserting the old `w-full sm:w-auto` text-bearing wrapper. The per-commit gate (`pnpm typecheck` + `pnpm lint` + relevant tests) listed in the kickoff would have flagged the failure. Either the gate did not run, or the test was skipped; either way `develop` between `1b0e81ae` and `5570971f` failed the suite. +Fix shape: amend the kickoff convention to "tests update inside the same commit that ships the behaviour change"; cap the time-between-commits monitoring (a 23-minute red `develop` window blocks any concurrent contributor's pre-push hook). + +### R4-CODE-H4 — Forbidden vocabulary "wave-4b" reintroduced into code comments +File: `src/components/medications/medication-detail-section.tsx:7` + `src/components/medications/DrugLevelChart.tsx:253` (commit `5109e930`) +Both comments use "wave-4b" as a historical bucket identifier. The kickoff explicitly lists `wave` in the forbidden vocabulary. The original v1.4.25 W21 comment ("The four wave-4b sections") was carried forward verbatim during the chrome rewrite; a clean rewrite should have neutralised the bucket name. +Fix shape: replace "wave-4b" with the neutral identifier "v1.4.25 W21 medication-detail section family" or drop the bucket reference entirely. Same scan should pass on the rest of the file (no other `wave` matches). + +### R4-CODE-H5 — Forbidden vocabulary "AI" used in new code comments +File: `src/app/insights/schlaf/page.tsx:62-72` (commit `8f7cbd49`); `src/components/insights/__tests__/hero-strip.test.tsx:196` (commit `cad53a68`) +The kickoff bans the substring `AI`. The schlaf comment ("written per-section AI assessment") and the hero-strip test comment ("the AI schema slot ... are gone") both add the forbidden word in lines that this release authored. Pre-existing `/lib/ai/*` path references are allowed (path identifier exception); free-form comment strings are not. +Fix shape: rewrite to "written per-section assessment" / "the assistant schema slot". The same scan should grep `git diff v1.4.27..HEAD` for `\bAI\b` in non-path lines as a release-gate. + +### R4-CODE-H6 — Maintainer first-name reference in commit body breaks the no-PII rule +File: commit `9a020f21` body ("FB-I1 — Marc asked for a tappable `?` glyph...") +Commit messages on `develop` become the public squash-merge subject + body on `main` and are visible in the GitHub UI for any browser of the repo. The kickoff bans the maintainer's name in user-facing artifacts; the public commit log is user-facing. Compare with `538b44f7`'s test fixture (`username: "marc"`) which is internal test code — different exposure surface. +Fix shape: drop the first-name reference, rewrite as "FB-I1 — the maintainer asked for a tappable `?` glyph" or "FB-I1 — surface a tappable `?` glyph". The squash-merge subject + body is the primary touch-point; one-line scrub is enough. + +## Medium + +### R4-CODE-M1 — Aggregated DTO ships `count` field that's not declared anywhere +File: `src/app/api/measurements/route.ts:80-86` +The aggregated branch invents an inline DTO `{ type, value, measuredAt, count }` directly inside the route — no Zod schema, no `MeasurementAggregateWireDTO` type, no entry in `.planning/v15-ios-handoff/08-locked-contracts.md`. Consumers (the chart, any future iOS update, OpenAPI generator) have nothing to import. The `count` field is forwarded but never spec'd. +Fix shape: lift the inline shape to `src/lib/validations/measurement.ts` as `aggregatedMeasurementSchema`; export the TS type; add an `aggregateRow` Zod schema for OpenAPI emission. Document the new shape in `08-locked-contracts.md` before the iOS team consumes it. + +### R4-CODE-M2 — `limit` cap loosened from 500 to 5000 without bumping the locked-contract doc +File: `src/lib/validations/measurement.ts:249` +The locked-contract excerpt in `.planning/v15-ios-handoff/03-api-contracts.md:305` says `limit: z.coerce.number().int().min(1).max(500)`. The v1.4.28 schema now reads `.max(5000)`. Backwards-compatible for existing callers but a contract drift; the iOS team's `MeasurementListWireResponse` may carry the documented cap. +Fix shape: bump the locked-contract doc in lockstep; surface the change in the release notes under "API contract additions". + +### R4-CODE-M3 — Several commits ship without bodies +Files: `75773ca0`, `ebf83b1e`, `b0ef80dc`, `8c89ddac`, `8f3bfc37`, `235e52cb`, `d286220b` +The kickoff says "Atomic commits per logical sub-task. No 'WIP', 'various improvements', 'end-of-day commit'." — implicitly the body explains the "why" for non-trivial subjects. `ebf83b1e` (274-line bundle-analyzer + web-vitals wire-up) and `8c89ddac` (277-line insights-sub-page consolidation, six pages touched) both ship body-less. `b0ef80dc` (225-line locale cache) is forgiven because the rationale lives in the file header. +Fix shape: enforce a per-commit hook that requires a body for diffs over a 40-line / 3-file threshold. Retroactive for v1.4.28 is moot; the release-notes pass can backfill the "why" for the heavier commits. + +### R4-CODE-M4 — FB-E3 (opt-in GLP-1 dashboard widget) silently deferred +File: `.planning/v1428-feedback-2026-05-15.md:66` vs. the 30-commit diff +FB-E3 is Medium severity in the feedback doc. Two retirement commits (`8e5f71b1`, `8c81af10`, `8c8d6dc2`) reference "an opt-in dashboard widget that surfaces the same level chart will land later in this cycle" — the cycle is v1.4.28, and the widget has not landed. No entry in a v1.4.29 backlog (file does not exist). +Fix shape: create `.planning/v1429-backlog.md` and seed FB-E3 with the explicit rationale ("Medium polish item, scope-reduction sub-pass took precedence in v1.4.28"); soften the commit-message wording in the release-notes editorial pass. + +### R4-CODE-M5 — Typed-error pattern repeated inline rather than extracted +File: `src/components/measurements/measurement-list.tsx:248-260` (commit `538b44f7`) +The mutation casts `as Error & { errorCode?: string; status?: number }` in three places (response parse, throw, onError handler). The same pattern will recur on every mutation that handles a 409 with a structured error code; v1.4.28 sets the first instance. A `class HttpError extends Error { errorCode?: string; status?: number }` (or a `parseApiError` helper) would carry the contract instead of re-casting at every callsite. +Fix shape: extract `HttpError` into `src/lib/http-error.ts`; migrate the measurement-list call-site as the first consumer; queue the rest under a v1.4.29 polish item. + +### R4-CODE-M6 — `useMemo` for `fetchWindow` recomputes `new Date()` only on dep change +File: `src/components/charts/health-chart.tsx:518-532` (commit `b00be286`) +`fetchWindow` calls `new Date()` inside a `useMemo` keyed on `[rangePoints, effectiveCompareBaseline]`. Within a long-lived session (a user leaves the page open for hours), the `to` boundary never advances — the chart paints data up to the time of last range-tab flip, not "now". For most users it's invisible; for a power user staring at the live pulse chart, the right edge slowly stalls. +Fix shape: re-key the memo on a 60-second-rounded timestamp (`Math.floor(Date.now() / 60_000)`) so the window snaps to "now" each minute without churning the cache key on every render. + +## Low + +### R4-CODE-L1 — `i18n: add the lastYear coach window key` commit drops the conventional-commit scope parens +File: commit `75773ca0` subject +Subject reads `i18n: add the lastYear coach window key`. Every other commit in the v1.4.28 range uses `type(scope): subject` (`fix(api)`, `feat(insights)`, `chore(medications)`). The plain `i18n:` form drops `(scope)`. Mild format drift, not a blocker. +Fix shape: pin `(i18n)` as the scope label for future locale-only commits; or treat it as the scope itself with a body subject (`feat(i18n): add the lastYear coach window key`). + +### R4-CODE-L2 — Test mock fixture uses the maintainer's first name +File: `src/app/api/measurements/__tests__/put-duplicate-timestamp.test.ts:39` +`username: "marc"` in a mock session fixture. Internal-only (test fixture, never serialised to the client) — the no-PII rule arguably stops at user-facing artifacts. Carried forward from v1.4.27 fixtures. +Fix shape: rename to a neutral fixture name (`test-user`) at the next refactor pass; not worth a v1.4.28 follow-up commit. + +### R4-CODE-L3 — Empty body on the perf+web-vitals commit obscures the cumulative LOC impact +File: commit `ebf83b1e` body (empty) +274-line addition (Next config wrapper + `@next/bundle-analyzer` dep + new internal route + the client reporter). The intent is clear from the subject but the body could carry the "why" (perf-baseline plumbing for the v1.4.29 measure-then-act pass). +Fix shape: backfill the body in the release-notes editorial; not actionable on the commit itself without a rewrite. + +--- + +## Per-commit notes + +| Commit | Subject | R4 verdict | +|---|---|---| +| `538b44f7` | fix(api): return 409 on duplicate-timestamp measurement edit | Clean; failing-test-first; iOS-safe | +| `b00be286` | perf(charts): bound health-chart fetches to the active range window | **C1, C2, M1, M2, M6** — needs server-side aggregation fix + iOS gate | +| `0d591ac9` | fix(insights): cap status-card provider calls at 20s with graceful fallback | Clean; coverage at `pulse-status-timeout.test.ts` | +| `ac80c099` | fix(insights): unstick scroll on tab-strip and mother-page navigation | Clean; e2e spec covers both regressions | +| `59ef95f2` | fix(dashboard): align BD-Zielbereich tile with shared TrendCard primitive | Clean; failing-test-first locks the date-shape regression | +| `8e5f71b1` | chore(dashboard): retire the GLP-1 tile | Clean delete; drift-guard updated; iOS audit clean | +| `cad53a68` | chore(insights): retire the weekly-report surface | Clean delete; one "AI" remnant cleaned elsewhere | +| `52edf85f` | chore(insights): retire the InsightAdvisorCard surface | Clean delete | +| `8c81af10` | chore(medications): drop the Dosis-Historie disclosure from GLP-1 detail | Clean delete; iOS DTO byte-stable | +| `8c8d6dc2` | chore(medications): drop the Bestand section from GLP-1 detail | Clean delete; iOS DTO byte-stable | +| `6f6992c6` | refactor(medications): unify medication-list row shape | Clean; primitive carved out | +| `155b529d` | fix(insights): match HealthScore card height to the hero column | Clean; DOM-class assertions, no pixel checks | +| `4c6d8779` | refactor(coach): consolidate launch button to inline + layout-FAB shape | Clean; fixes duplicate-FAB a11y issue | +| `d286220b` | perf(charts): wire chart-skeleton loading state across dynamic imports | Body empty (**M3**); clean diff | +| `1b0e81ae` | fix(targets): make the coach launch an icon-only affordance | **H3** — test update split into a separate commit | +| `66e13845` | refactor(coach): narrow launch-scope metric type to the source union | Clean; type-safety win | +| `ca381957` | fix(coach): align mobile sheet height to the responsive-sheet convention | Clean | +| `7d38a54d` | fix(medications): align side-effects card to the surface convention | Clean (test-only update, paired with `88085615`) | +| `9a020f21` | feat(insights): explain the HealthScore delta on tap | **H6** — Marc-name in commit body | +| `88085615` | fix(medications): shorten side-effects add CTA across locales | Clean (locale half of FB-F1) | +| `235e52cb` | refactor(charts): single HealthChartDynamic re-export | **H2** — content is Coach drawer + mobile rail tray, NOT charts | +| `8f3bfc37` | refactor(charts): collapse health-chart dynamic imports onto re-export | Body empty (**M3**); likely the actual charts-rewrite the H2 subject claims | +| `5109e930` | refactor(medications): collapse detail-page chrome to one heading scale | **H4** — "wave-4b" comment carried forward | +| `0e7c97c5` | fix(insights): align briefing empty-state CTA variant | **H1** — body claims only CTA variant; diff lands FB-K1/K2 trend-row | +| `8c89ddac` | refactor(insights): consolidate sub-page data-fetch and empty state | Body empty (**M3**); clean shared hook | +| `8f7cbd49` | fix(insights): document the missing sleep status slot | **H5** — "AI assessment" in new comment | +| `b0ef80dc` | perf(notifications): cache the dispatch-localised user lookup | Body empty (**M3**) but file header carries rationale; clean LRU + 5 tests | +| `ebf83b1e` | feat(perf): wire bundle analyzer and web-vitals beacon | **C3, M3, L3** — unauthenticated beacon, empty body | +| `75773ca0` | i18n: add the lastYear coach window key | **L1** — scope parens dropped | +| `5570971f` | test(targets): update coach CTA assertion to the icon-only shape | Clean test-only follow-up | + +--- + +## Summary + +| Severity | Count | +|---|---| +| Critical | 3 | +| High | 6 | +| Medium | 6 | +| Low | 3 | + +### Top-3 fix recommendations (must clear before the v1.4.28 tag) + +1. **R4-CODE-C1** — drop `take: limit` from the `/api/measurements` aggregation branch (or push aggregation into SQL). The chart silently truncates high-density data on > 90-day windows; FB-D2 is half-fixed without it. Add a 30 000-row regression seed. +2. **R4-CODE-C2** — gate the aggregation branch on an explicit `aggregate` param. Without it, any future iOS update that adds date filtering on a > 90-day drill-down silently breaks the locked-contract response shape. +3. **R4-CODE-C3** — restrict the web-vitals beacon to known metric names + add a same-origin Referer guard. Currently any external page can pollute the wide-event log with arbitrary strings. + +### Go / no-go + +**No-go on the v1.4.28 tag** until the three Critical findings are addressed. R4-CODE-C1 alone is a regression on the exact perf surface FB-D2 was meant to fix — shipping it would leave the maintainer with a chart that paints fewer points than v1.4.27 on the > 90-day range. R4-CODE-C2 and R4-CODE-C3 are smaller in user-visible blast radius but R4-CODE-C2 violates the v1.5 iOS-safety underlying premise that the kickoff calls out explicitly. + +The High-severity items (H1, H2 mislabelled commits; H3 test-then-fix split; H4-H5 forbidden vocabulary; H6 PII) do not block the tag individually but each warrants a touch-up commit before merge: the vocab + PII items via a `chore(docs): release-notes editorial scrub` pass over commit messages and code comments, and a `Note:` in the release closure noting the H1/H2 subject swap so the v1.4.27 → v1.4.28 changelog reflects what landed where. diff --git a/.planning/research/v1428-r4-dead-code.md b/.planning/research/v1428-r4-dead-code.md new file mode 100644 index 000000000..976a4f1aa --- /dev/null +++ b/.planning/research/v1428-r4-dead-code.md @@ -0,0 +1,200 @@ +--- +file: .planning/research/v1428-r4-dead-code.md +purpose: R4 dead-code scan — orphans introduced or surfaced by the v1.4.28 diff +created: 2026-05-16 +contributor: R4 dead-code +--- + +# R4 — dead-code scan for v1.4.28 + +The v1.4.28 sweep landed three surface retirements (`Glp1Tile`, `InsightAdvisorCard`, `WeeklyReportView`) plus two GLP-1 detail-page disclosures (`IntakeHistoryList`, `InventorySection`). The R4-v1.4.27 single-commit recommendation also landed — the 14 orphan exported functions/consts from `v1427-r4-dead-code.md §2`, the `__testables` block in `glp1-snapshot.ts`, the legacy `getInsightsSystemPrompt` helper and the `InsightsOutput` interface are all gone from `src/`. `MedicationComplianceChart.compareBaseline` was re-wired with an explicit `void compareBaseline` discard plus a v1.4.27 RC3 caption (Section 7.1 of the predecessor resolved). + +What v1.4.28 left behind: a small set of orphans surfaced by the retire moves. The `DrugLevelChart.compact` rendering path lost its only mount (the tile is gone, the standalone medication-detail page mounts without `compact`). `shotPhaseAt` in `glp1-pk.ts` has tests but no production consumer. Two component files (`trends-row.tsx`, `vo2-max-chart-row.tsx`) still declare local `ChartSkeleton` constants identical to the shared one the v1.4.28 H4 fix introduced. Five admin/monitoring routes remain orphan (unchanged decision from v1.4.27 §1.1). The `OpenAPI` route description still names the retired weekly-report block. + +No High-tier findings. + +## Severity-grouped findings + +| Severity | Items | Decision required | +|---|---|---| +| Medium | **3** | Wire vs drop — each is a measurable LOC reduction | +| Low | **9** | Drop-or-keep per item, one-to-three lines each | +| Informational | **3** | Pre-existing or intentional; no action | + +--- + +## Medium findings (3) + +### M1 — `DrugLevelChart.compact` mode is unconsumed + +`src/components/medications/DrugLevelChart.tsx:123` declares the `compact?: boolean` prop; lines 226, 237, 244, 254, 270, 293, 319, 331 branch on it; lines 115–122 + 228–236 + 461 carry the doc comments. The only consumer was the GLP-1 dashboard tile (retired in `8e5f71b1`). The standalone mount at `src/app/medications/[id]/history/page.tsx:90` does not pass `compact`. `grep -rn "compact=" src/app | grep -i drug` returns zero. + +`compact` mode also pulled in the `windowHoursBefore` override (`DrugLevelChart.tsx:133`), which has the same zero consumers post-retire. + +**Decision:** drop the `compact` branch + `windowHoursBefore` prop + their three doc-comment blocks. Estimated diff: roughly `-90 / +0` LOC across `DrugLevelChart.tsx`. Risk: low — verified no test exercises `compact={true}`. + +### M2 — `shotPhaseAt` is test-only after the tile retire + +`src/lib/medications/glp1-pk.ts:303` exports `shotPhaseAt(drug, doses, asOf)`. Grep for the symbol returns only the declaration plus four test assertions in `__tests__/glp1-pk.test.ts:155–171`. The doc comment at line 292 names "the GLP-1 tile" as the consumer — that tile is gone. The `ShotPhase` type at line 301 is consumed only inside the same module. + +**Decision:** drop `shotPhaseAt` + `ShotPhase` from the module + the matching test block. Sibling exports `computeOneCompartment` + `RESEARCH_MODE_DISCLAIMER_VERSION` + `PkSample` + `OneCompartmentOptions` still serve `DrugLevelChart.tsx` and `/api/auth/me/research-mode/route.ts` — keep them. Estimated diff: roughly `-60 / +0` LOC. + +### M3 — five admin/monitoring routes still orphan (unchanged from v1.4.27 §1.1) + +Re-verified at HEAD: + +| Endpoint | src callers | tests | README | CHANGELOG | +|---|---|---|---|---| +| `/api/admin/ai-settings` | 0 | 0 | L362–363 | L752, L3261 | +| `/api/admin/backup/test` | 0 | 0 | L368 | L752 | +| `/api/admin/status-overview` | 0 | self-tests | L367 | L753 | +| `/api/monitoring/glitchtip/test` | 0 | self-tests | L381 | — | +| `/api/monitoring/umami/test` | 0 | self-tests | L382 | — | + +The v1.4.27 fix-plan parked these in the v1.4.28 backlog (`.planning/v1428-backlog.md:8`). v1.4.28 did not surface a consumer for any of them. Per-endpoint maintainer call still pending: wire-a-consumer (admin status page mount, settings AI panel) vs delete-route-plus-README-line. + +**Decision:** carry forward to v1.4.29 unless the maintainer pulls one into v1.4.28. The status-overview endpoint already has a candidate consumer at `src/app/api/admin/backups/route.ts:63` (comment-only reference); the GeoLite2 v1.4.27 work added a new admin system-status page in `82a23069` that *could* mount `/api/admin/status-overview` if that's the intent. + +--- + +## Low findings (9) + +### L1 — interface-only `export` keywords (6 symbols) + +Six v1.4.28-new interfaces are referenced only inside their declaring module — same pattern as the v1.4.27 §8 zod-derived contract types but **not** zod-backed; these are component-prop / hook-result shapes. Each is a one-word edit (`export interface` → `interface`): + +| Symbol | File:line | +|---|---| +| `TimeoutEnvelope` | `src/lib/insights/with-timeout.ts:21` | +| `MetricEmptyStateProps` | `src/components/insights/metric-empty-state.tsx:36` | +| `MobileRailTrayProps` | `src/components/insights/coach-panel/mobile-rail-tray.tsx:33` | +| `AggregatedRow` | `src/lib/measurements/range-aggregation.ts:58` | +| `UseInsightsAnalyticsResult` | `src/hooks/use-insights-analytics.ts:39` | +| `MedicationCardHeaderProps` | `src/components/medications/MedicationCardHeader.tsx:29` | + +`DAILY_AGGREGATE_THRESHOLD_DAYS` and `WEEKLY_AGGREGATE_THRESHOLD_DAYS` (range-aggregation.ts:23–24) read as "non-test" hits but the only external consumer is the test file itself, so they're true library-level constants. **Keep** the export keyword on those two — tests are first-class consumers for tuning thresholds. + +**Decision:** optional. Drop-the-export pass is a separate hygiene commit; can ride with the M1/M2 sweep. + +### L2 — duplicate local `ChartSkeleton` in two components + +`src/components/insights/trends-row.tsx:34–36` and `src/components/insights/vo2-max-chart-row.tsx:29–31` each declare a local `const ChartSkeleton = () => (...)` with the same `bg-muted/40 h-[220px]` body. The shared primitive at `src/components/charts/chart-skeleton.tsx:29` (added by v1.4.28 `d286220b` "wire chart-skeleton loading state across dynamic imports") is the canonical version — it paints a richer skeleton with header chrome + sized chart band. + +These two are not strictly orphans (the local consts are used inside the same file), but they are the only two `next/dynamic` mounts that did NOT pick up the v1.4.28 skeleton fix. Wiring them through `` from `@/components/charts/chart-skeleton` would close the L2 finding *and* match the layout-stable behaviour the H4 fix was meant to deliver everywhere. + +**Decision:** import the shared `` in both files; drop the local constants. Estimated diff: roughly `-12 / +4` LOC. + +### L3 — stale "GLP-1 tile" comment in `glp1-pk.ts` + +`src/lib/medications/glp1-pk.ts:292` — "Surfaces the 'shot phase' chip on the GLP-1 tile (research §2.4)". The tile is gone. If M2 lands the comment goes with it; otherwise trim the line. + +### L4 — stale "compact mode … GLP-1 tile" comments in `DrugLevelChart.tsx` + +Three blocks reference the retired tile: + +| Line | Excerpt | +|---|---| +| 115 | "v1.4.27 B1 — compact rendering for tile-internal mounts (the GLP-1 dashboard tile…)" | +| 228 | "Compact mode mounts the chart inside a host card (the GLP-1 tile) …" | +| 234 | "… made the GLP-1 tile feel oversized relative to the neighbouring trend cards …" | + +If M1 lands these go with the `compact` branch. If M1 is deferred, trim "the GLP-1 tile" → "a host card" so the comments don't lie about live surfaces. + +### L5 — stale "weekly report" in OpenAPI route description + +`src/lib/openapi/routes.ts:757` — `/api/insights/comprehensive` description still reads "Full Insights surface — daily briefing, recommendations with rationale, optional weekly report + storyboard annotations." The `weeklyReport` block was stripped from `src/lib/ai/schema.ts` in v1.4.28; the route no longer emits it. The generated OpenAPI client and the docs site will surface a description that contradicts the schema. + +**Decision:** drop "optional weekly report + " from the description. One-line edit. + +### L6 — `/api/dashboard/glp1` references in two safety-net comments + +The route was deleted in v1.4.28. Two comments still cite it as a representative example of the "no-scope route ⇒ any-token" rule: + +- `src/lib/api-handler.ts:323` — naming four routes in a v1.4.25 W10 reconcile note +- `src/lib/__tests__/require-auth-bearer.test.ts:205` — same wording + +Both are intentional historical context (the W10 reconcile **did** include `/api/dashboard/glp1`). The example is still semantically valid — the rule applies to any scope-less route — but the cited example no longer exists. Either swap the example for a live scope-less route or strike the parenthetical list. + +### L7 — stale `feedback_charts_visual_identity.md` ChartSkeleton justification + +`src/components/insights/trends-row.tsx:32` block-comments the local `ChartSkeleton` with "cleared against `feedback_charts_visual_identity.md` — placeholder only, no chart visual change." If L2 lands, the new shared `` is richer than the local one (header chrome + sized chart band) — that's a visible delta and the comment becomes wrong. The feedback memo's actual rule is "no token reshuffle, no Recharts swap"; the skeleton change is layout-only and does not touch the rendered chart. + +**Decision:** when L2 lands, either drop the comment or rewrite it as "skeleton chrome aligned with the dashboard's shared `` — `feedback_charts_visual_identity.md` covers chart-body identity, which is unchanged." + +### L8 — `void compareBaseline` discard is now load-bearing test surface + +Resolved in v1.4.27 RC3 (`compareBaseline` is destructured with an explicit `void` discard plus a doc comment at lines 196–202 explaining the prop is intentionally accepted-and-ignored). No further action — flagged only because the v1.4.27 R4 §7.1 "maintainer decision" landed and the file proves the resolution. + +### L9 — `MEDICATION_CATEGORIES` import-only consumer + +`src/lib/medication-category.ts:4` exports `MEDICATION_CATEGORIES`. The blob scan reports an internal-file-only match. Spot-check returns one cross-file consumer in `src/components/medications/category-select.tsx` that the substring scanner missed because the import collapsed into a named-import block. **No action** — false positive in the heuristic. + +--- + +## Informational (3) + +### I1 — v1.4.27 R4 §2 14 orphans all swept + +Confirmed at HEAD: every symbol from the v1.4.27 R4 §2 list (`countMessages`, `buildGeneratePrompts`, `_resetSafetyContractsCacheForTests`, `forEachLocaleAndRule`, `getPulseRange`, `trendLinePoints`, `CHART_MINI_HEIGHT_PX`, `formatDateWithWeekday`, `resolveGeneralStatusLocale`, `isSubPageSlug`, `recordHeartbeat`, `EVENT_TYPE_LABELS`, `cleanupExpiredRateLimits`, `sleepStageEnum`) returns zero grep hits in `src/`. The `getInsightsSystemPrompt` helper + `InsightsOutput` interface (§6.1 + §8.1) plus the `glp1-snapshot.ts` `__testables` block (§3.2) are also gone. The v1.4.27 R4 sweep landed in full. + +### I2 — i18n keys clean (`pnpm test … i18n-locale-integrity` 26/26) + +Re-run at HEAD passes. The new `insights.coach.window.lastYear` key (added in `75773ca0` per the v1.4.27 §4 cross-reference) closes the only missing-key gap from v1.4.27. The custom blob scanner I ran on top reports 69 candidate "dead" keys, but cross-checking against the existing integrity test (which uses the template-prefix-aware logic) confirms each candidate has either a `t("ns.key")` consumer or a template prefix usage the coarse scan misses (e.g. `t(\`insights.recommendation.${key}\`)`). **No dead i18n keys at HEAD.** + +### I3 — `@deprecated` markers still zero + +`grep -rn "@deprecated" src` returns zero hits. No new deferred markers introduced in v1.4.28. + +--- + +## Orphan-by-orphan table + +| ID | Symbol / surface | File:line | Consumers | Severity | Decision | +|---|---|---|---|---|---| +| M1 | `DrugLevelChart.compact` + `windowHoursBefore` | `src/components/medications/DrugLevelChart.tsx:123,133` | 0 prod, 0 tests | Medium | DROP | +| M2 | `shotPhaseAt`, `ShotPhase` | `src/lib/medications/glp1-pk.ts:301,303` | 0 prod, 4 test assertions | Medium | DROP (with tests) | +| M3 | 5 admin/monitoring routes | per table above | README-only | Medium | Defer to v1.4.29 | +| L1 | `TimeoutEnvelope` | `src/lib/insights/with-timeout.ts:21` | declaring-file only | Low | Drop `export` | +| L1 | `MetricEmptyStateProps` | `src/components/insights/metric-empty-state.tsx:36` | declaring-file only | Low | Drop `export` | +| L1 | `MobileRailTrayProps` | `src/components/insights/coach-panel/mobile-rail-tray.tsx:33` | declaring-file only | Low | Drop `export` | +| L1 | `AggregatedRow` | `src/lib/measurements/range-aggregation.ts:58` | declaring-file only | Low | Drop `export` | +| L1 | `UseInsightsAnalyticsResult` | `src/hooks/use-insights-analytics.ts:39` | declaring-file only | Low | Drop `export` | +| L1 | `MedicationCardHeaderProps` | `src/components/medications/MedicationCardHeader.tsx:29` | declaring-file only | Low | Drop `export` | +| L2 | local `ChartSkeleton` (trends-row) | `src/components/insights/trends-row.tsx:34` | same file | Low | Wire to shared | +| L2 | local `ChartSkeleton` (vo2-max-chart-row) | `src/components/insights/vo2-max-chart-row.tsx:29` | same file | Low | Wire to shared | + +(L3–L9 are comment-level findings, listed in the next section.) + +--- + +## Stale-comment list + +| File:line | Comment excerpt | Recommendation | +|---|---|---| +| `src/lib/medications/glp1-pk.ts:292` | "Surfaces the 'shot phase' chip on the GLP-1 tile (research §2.4)" | Drop with M2, else trim "the GLP-1 tile" | +| `src/components/medications/DrugLevelChart.tsx:115` | "v1.4.27 B1 — compact rendering for tile-internal mounts (the GLP-1 dashboard tile…)" | Drop with M1, else trim | +| `src/components/medications/DrugLevelChart.tsx:228` | "Compact mode mounts the chart inside a host card (the GLP-1 tile)…" | Drop with M1, else trim | +| `src/components/medications/DrugLevelChart.tsx:234` | "made the GLP-1 tile feel oversized relative to the neighbouring trend cards" | Drop with M1, else trim | +| `src/components/medications/DrugLevelChart.tsx:461` | "Chart-area height in CSS pixels; compact mode passes 160." | Drop with M1 | +| `src/lib/openapi/routes.ts:757` | "Full Insights surface — daily briefing, recommendations with rationale, optional weekly report + storyboard annotations." | Drop "optional weekly report + " | +| `src/lib/api-handler.ts:323` | "`/api/dashboard/glp1`) 403'd every narrow-scope token even though those routes did not intend to restrict by scope" | Swap example or strike route name | +| `src/lib/__tests__/require-auth-bearer.test.ts:205` | "`/api/dashboard/glp1` — all four were called without a scope" | Swap example or strike route name | +| `src/components/insights/trends-row.tsx:32` | "cleared against `feedback_charts_visual_identity.md` — placeholder only, no chart visual change" | Rewrite when L2 lands | + +Total stale comments: **9**. + +--- + +## Summary + +v1.4.28 cleaned up cleanly — the v1.4.27 R4 §2 sweep landed in full (14 orphan exports + the `__testables` block + the legacy `getInsightsSystemPrompt` helper + `InsightsOutput` are all gone) and the three retired surfaces (`Glp1Tile`, `InsightAdvisorCard`, `WeeklyReportView`) plus the two GLP-1 detail disclosures (`IntakeHistoryList`, `InventorySection`) took their direct supporting code with them. The i18n surface remains clean (`pnpm test … i18n-locale-integrity` 26/26) and no `@deprecated` markers slipped in. + +What remains is residue. **M1 + M2** are the only real wins: `DrugLevelChart.compact` mode plus `shotPhaseAt` lost their only mount when the tile was retired, so each is a measurable LOC reduction. **M3** is unchanged from v1.4.27 — five admin/monitoring routes still need a per-endpoint maintainer call. **L1** is six new `export interface` declarations that could shed the `export` keyword (matching the v1.4.27 §8 hygiene pattern). **L2** is two duplicate local `ChartSkeleton` constants that could ride the shared primitive the v1.4.28 H4 fix introduced. **L3–L9** are nine stale comments — three of them disappear automatically if M1 + M2 land. + +Recommended single commit if R4 is pulled into v1.4.28: M1 + M2 + L2 + the six L1 single-word edits + the L3/L4/L5 comment trims that survive after M1/M2 land. Estimated diff: roughly **-180 / +6** LOC. Risk: low — verified zero non-test consumers for every dropped symbol. M3 stays deferred; L6–L9 are optional polish. + +Output: + +- Orphan count (Medium + Low symbols/surfaces): **11** (3 Medium + 8 Low; L8 already resolved, L9 false positive) +- Top sweep candidate: **M1** (`DrugLevelChart.compact` mode + `windowHoursBefore` — biggest LOC reduction, removes the tightest coupling to the retired tile) +- Stale comments: **9** diff --git a/.planning/research/v1428-r4-design.md b/.planning/research/v1428-r4-design.md new file mode 100644 index 000000000..3d8d85cd9 --- /dev/null +++ b/.planning/research/v1428-r4-design.md @@ -0,0 +1,279 @@ +--- +file: .planning/research/v1428-r4-design.md +purpose: R4 design review — 7-axis pass across the v1.4.28 user-facing diff +created: 2026-05-16 +contributor: R4 design +--- + +# v1.4.28 — R4 design review + +Read-only 7-axis pass across the 11 user-facing surfaces from +`.planning/v1428-feedback-2026-05-15.md` (themes F, G, H, I, K, L, M). +Diff scope: 30 commits since `v1.4.27`. Axes: hierarchy, contrast +(WCAG 2.1 AA), spacing, typography, responsive (320 / 768 / 1024 / 1440 +px), a11y, motion. + +R1 prerequisites `v1428-r1-ui-inventory.md` and `v1428-r1-competitive.md` +are not yet on disk; review reads components directly and benchmarks +against `v1427-r4-design.md`. + +--- + +## Severity-grouped findings + +### Critical + +None. The maintainer's eight Critical feedback items (bug-class, scope +reduction) are bug + scope work, not design. No regressions on the 7 axes +that would block tag. + +### High + +**D-H1 — Target-card Coach button under WCAG 2.5.5 tap-target floor** +File: `src/components/targets/target-coach-button.tsx:65-86` +Axis: a11y / responsive +FB-L1 collapses the affordance to `