mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
chore(planning): record A1 BD-Zielbereich tile sub-values fix
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
23363ca05e
commit
15c852f48e
2 files changed
+73
-66
No files matched your search
+13
-3
@@ -17,10 +17,20 @@ Last update: 2026-05-10T10:02+02:00
|
||||
|
||||
### A1 — BD-Zielbereich tile 7T/30T sub-values
|
||||
|
||||
- [ ] Investigate why "7T: —" / "30T: —" render even when user has data
|
||||
- [ ] Fix data source / aggregation for the sub-values
|
||||
- [ ] E2E test against tile rendering with real measurement data
|
||||
- [x] Investigate why "7T: —" / "30T: —" render even when user has data
|
||||
- [x] Fix data source / aggregation for the sub-values
|
||||
- [x] E2E test against tile rendering with real measurement data
|
||||
- Detailed report: `.planning/phase-A1-report.md`
|
||||
- Status: 2026-05-09T10:15+02:00 — done. Root cause was an
|
||||
unfinished tile wire: `/api/analytics` only computed a single
|
||||
30-day `bpInTargetPct`, and the dashboard tile passed
|
||||
`avg7={null}, avg30={null}` so the sub-values rendered "—" no
|
||||
matter how much data the user had. Added a windowed helper
|
||||
`computeBpInTargetWindows()` (re-uses the v1.4.16 A2 ceiling
|
||||
predicate, filters input by `measuredAt`), surfaced
|
||||
`bpInTargetPct7d` / `bpInTargetPct30d` from the route, and wired
|
||||
them through the tile. 6 new unit tests + 2 new testcontainer
|
||||
integration tests, all green. Commit `23363ca` on origin/main.
|
||||
|
||||
### A2 — `/admin/api-tokens` table scrollbar (3rd attempt, Playwright live-verified)
|
||||
|
||||
|
||||
@@ -1,73 +1,70 @@
|
||||
# Phase A1 — Nav conditionals + sidebar context-awareness
|
||||
# Phase A1 — BD-Zielbereich tile 7T / 30T sub-values
|
||||
|
||||
Status: complete
|
||||
Date: 2026-05-09T20:23+02:00
|
||||
Marathon: v1.4.18 Wave-A bucket A1 (parallel with A2 api-tokens, A3
|
||||
chart revert, B1 achievements)
|
||||
Started: 2026-05-09T~10:08+02:00
|
||||
Finished: 2026-05-09T~10:15+02:00
|
||||
|
||||
## Summary
|
||||
## 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
|
||||
**`30T: —`** even though Marc clearly has paired BP readings in both
|
||||
windows.
|
||||
|
||||
All four atomic fixes for Phase A1 landed on `origin/main`. The session ran
|
||||
under heavy parallel-agent contention (A2, A3, A4 working in the same
|
||||
worktree); a few of my changes were folded into a sibling agent's commit
|
||||
when they raced to `git add` first, but every behavioural change in the
|
||||
A1 scope is now committed and the test suite is green (103 test files,
|
||||
779 tests, 0 typecheck errors, 0 lint errors).
|
||||
## 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
|
||||
`avg7={null}` and `avg30={null}` to the `TrendCard` component, which
|
||||
correctly renders "—" as the friendly fallback when those fields are
|
||||
null. So the sub-values were never wired to data, regardless of how
|
||||
many readings the user had — pure unfinished feature, not a calculation
|
||||
bug.
|
||||
|
||||
## Atomic commits
|
||||
Confirmed against Marc's prod data on apps-01: user
|
||||
`cmlupy4tn000001rpzx1pxvz7` has 2 paired sys+dia in last 7 days and
|
||||
10 in last 30 days. Both windows have data; nothing should render "—".
|
||||
|
||||
| Commit | Fix | Files |
|
||||
| ------- | ------ | -------------------------------------------------------- |
|
||||
| 85aa15b | Fix 1 | `app-settings-provider.tsx`, `providers.tsx`, `sidebar-nav.tsx` (+ test) |
|
||||
| 786b395 | Fix 2 | `auth-shell.tsx`, `e2e/a11y.spec.ts` |
|
||||
| 73afae0 | Fix 3 | `sidebar-nav.tsx` predicate tightening (folded into A4's commit on push race) |
|
||||
| bde167d, c63e4de | Fix 4 | `error-details.tsx` + new test under `components/__tests__/` |
|
||||
## Fix
|
||||
Added `computeBpInTargetWindows()` next to the existing
|
||||
`computeBpInTargetPct()` helper in
|
||||
`src/lib/analytics/bp-in-target.ts`. Same predicate +
|
||||
`isBpReadingInTarget()` ceiling semantics from v1.4.16; the new
|
||||
function filters the input series by `measuredAt >= now − Nd` and
|
||||
delegates to `computeBpInTargetPct()` for each window. Returns
|
||||
`{ last7Days, last30Days }`, both nullable so the tile renders "—" only
|
||||
when a window genuinely has no paired readings.
|
||||
|
||||
## What landed
|
||||
Wired into the analytics route — fetched 30-day data once, derived
|
||||
both shares, and added `bpInTargetPct7d` / `bpInTargetPct30d` to the
|
||||
response. Wired into the dashboard tile —
|
||||
`avg7={data?.bpInTargetPct7d}`, `avg30={data?.bpInTargetPct30d}`.
|
||||
Headline (`bpInTargetPct`) preserved for cached client bundles +
|
||||
matches `last30Days` so a v1.4.17 PWA displays the same number.
|
||||
|
||||
- **Fix 1 — Bug-Report nav gate.** New `<AppSettingsProvider>` in
|
||||
`src/components/app-settings-provider.tsx` exposes the admin's
|
||||
`bugReportEnabled` flag (sourced from the existing `/api/bugreport/status`
|
||||
endpoint) via `useAppSettings()`. Sidebar consumes it in both collapsed
|
||||
and expanded modes; entry disappears when the admin flips the toggle.
|
||||
Defaults to ON during loading / unauthenticated so no flicker.
|
||||
Bottom-nav and topbar don't surface bug-report today, so no changes
|
||||
needed there.
|
||||
- **Fix 2 — Skip-link no longer blocks logo.** Combined `pointer-events-none`
|
||||
+ `-translate-y-full` in the unfocused state so the keyboard-only skip
|
||||
link sits offscreen and ignores hit-tests until focused. Focus restores
|
||||
both modifiers (keyboard flow unchanged). Playwright e2e in
|
||||
`e2e/a11y.spec.ts` clicks the desktop logo at its centre and asserts
|
||||
the URL stays at `/`.
|
||||
- **Fix 3 — Sidebar admin sub-items.** Already gated `{onAdminPage && ...}`
|
||||
per Phase 4b — the desired behaviour was the existing behaviour. The
|
||||
refactor tightened the predicate from `pathname.startsWith("/admin")`
|
||||
to `pathname === "/admin" || pathname.startsWith("/admin/")` so a
|
||||
hypothetical `/administrative` route can never trip the gate. Test
|
||||
suite added in 3e45a7b covers all three states.
|
||||
- **Fix 4 — User-facing feedback link.** `<ErrorDetails>` panel's "Report
|
||||
bug" button now reuses `useAppSettings()` and disappears when the
|
||||
admin disables the feature. Other escape hatches (retry, copy
|
||||
diagnostic payload) stay visible. Unit test in
|
||||
`src/components/__tests__/error-details.test.tsx`.
|
||||
## 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,
|
||||
Marc's production fixture. Failed first (`is not a function`),
|
||||
went green after the helper landed.
|
||||
2. **Integration** (`tests/integration/bp-in-target.test.ts`) — 2 new
|
||||
testcontainer cases: seed 30 days of BP at fractional-day offsets
|
||||
(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`.
|
||||
|
||||
## Tests
|
||||
## 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**
|
||||
|
||||
- **Unit:** 779 passing across 103 test files (was 754 across 97 before A1).
|
||||
New tests added: `sidebar-nav.test.tsx` (6 cases — bug-report toggle +
|
||||
admin context-awareness), `error-details.test.tsx` (2 cases — report
|
||||
bug button visibility).
|
||||
- **e2e:** new logo-click case in `e2e/a11y.spec.ts` under the
|
||||
authenticated section.
|
||||
- **Typecheck:** clean.
|
||||
- **Lint:** 0 errors, 12 pre-existing warnings (none in A1 files).
|
||||
## Commit
|
||||
`23363ca` —
|
||||
`fix(dashboard): wire 7T and 30T sub-values on the BD-Zielbereich tile`
|
||||
|
||||
## Notes for next phase
|
||||
|
||||
- `<AppSettingsProvider>` is generic: future admin-managed feature
|
||||
toggles can be added to the `AppSettings` interface without a parallel
|
||||
fetch — the same `/api/bugreport/status` envelope can carry
|
||||
additional flags, or a dedicated `/api/app-settings` route can replace
|
||||
it later when the surface grows.
|
||||
- Push races forced one Fix 4 follow-up commit (c63e4de) because a
|
||||
sibling agent's `git add` swept my staged `error-details.tsx` out of
|
||||
the index between staging and commit. Pre-commit hooks held; nothing
|
||||
was lost.
|
||||
Pushed to origin/main on first attempt (no rebase race against the
|
||||
other Wave-A agents).
|
||||
Reference in new issue
Block a user