mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
docs(planning): W6 wave report for the v1.4.37 settings + misc fixes
Per-commit list, tests delta, code-review pass, brief-back — documents the W6 outcome (timezone-override removal, BMI status timeout-sentinel + skeleton, cf-connecting-ip branch, geo-backfill schedule). Notes the three-way race-condition with adjacent waves that landed W6 file content under neighbouring commit messages.
This commit is contained in:
Marc-André Bombeck committed
1 parent
0280369952
commit
e33bb41c6d
1 file changed
+195
@@ -0,0 +1,195 @@
|
||||
# Wave W6 — settings cleanup + misc bug fixes (v1.4.37)
|
||||
|
||||
Implementation report for the three audit items owned by W6 (items 6,
|
||||
8, 10 in `.planning/research/v1437-ux-audit.md`).
|
||||
|
||||
## Per-commit list
|
||||
|
||||
The wave landed five logical commits. Three of them got swept into
|
||||
adjacent agents' commits because the develop branch was hot with
|
||||
parallel waves; the file content is identical to what W6 staged and
|
||||
the diffs match the audit recommendations line for line, just under a
|
||||
different commit message.
|
||||
|
||||
| Commit | Subject | Owner |
|
||||
|---------|---------------------------------------------------------------------------------------|-------------------|
|
||||
| 23273322 | (chore-settings-tz files swept in) — `timezone-picker` + `account-section` + tests | W6 (raced) |
|
||||
| 675c084b | (chore-i18n files swept in) — `timezoneDetect` + `timezoneDetectAria` removed (×6) | W6 (raced) |
|
||||
| bc95e8fc | (fix-insights files swept in) — BMI timeout-sentinel + structured loading skeleton | W6 (raced) |
|
||||
| a43f7046 | `feat(audit): cf-connecting-ip branch in getClientIp behind TRUST_CF_CONNECTING_IP` | W6 |
|
||||
| d3c9eaaf | `chore(queue): schedule geo-backfill via pg-boss on the :40 hourly slot` | W6 |
|
||||
|
||||
The race-condition pattern is documented in the marathon coordination
|
||||
notes; future waves can preempt it by staging + committing inside the
|
||||
same `git` invocation under a write lock. Marc retains attribution
|
||||
because every author is the same.
|
||||
|
||||
## Per-item outcome
|
||||
|
||||
### Item 8 — Timezone override removal — resolved
|
||||
|
||||
- `src/components/settings/timezone-picker.tsx` dropped the
|
||||
`<Button>` + `<Compass>` block (plus the fallback branch's twin)
|
||||
and the `useState(detectBrowserTimezone)` it relied on. The
|
||||
picker reads cleanly as `<Label>` + `<NativeSelect>` + hint.
|
||||
- `src/components/settings/account-section.tsx` now bootstraps the
|
||||
timezone field via the new pure helper `resolveInitialTimezone`,
|
||||
which auto-seeds the form with the browser zone whenever the
|
||||
stored value is still the `Europe/Berlin` default and the browser
|
||||
reports a different IANA zone. No toast, no banner, no opt-in.
|
||||
- `messages/{de,en,es,fr,it,pl}.json` lost `settings.timezoneDetect`
|
||||
+ `settings.timezoneDetectAria`. The i18n-parity test required
|
||||
matching deletes across all six locale bags; the `timezone`,
|
||||
`timezoneHint`, `timezoneInvalid` strings stay because the picker
|
||||
label, hint, and IANA-validation error still surface them.
|
||||
- Two new tests:
|
||||
- `src/components/settings/__tests__/timezone-picker.test.tsx`
|
||||
pins the button removal in both EN and DE plus the
|
||||
custom-value preservation contract.
|
||||
- `src/components/settings/__tests__/account-section-timezone-seed.test.ts`
|
||||
pins the auto-seed decision matrix (7 cases — default→non-default,
|
||||
null/empty handling, Berlin→Berlin no-op, non-default respected).
|
||||
|
||||
### Item 6 — BMI status stuck on "laden" — resolved
|
||||
|
||||
- `src/lib/insights/bmi-status.ts` now persists a sentinel row keyed
|
||||
to today's Berlin day-key whenever the 20 s provider race times
|
||||
out or returns null. The user-facing payload is identical (same
|
||||
`getNoKeyBmiStatusText(locale)` body); only the cached-row write
|
||||
is new. The row carries `model: "timeout-stub"` and `timeout: true`
|
||||
so the daily 02:20 pre-warm job can recognise + overwrite the
|
||||
stub rather than respect it as a real assessment. The persist
|
||||
is wrapped in `try/catch` (best-effort — a write miss returns
|
||||
the same payload, the next mount falls back to the race).
|
||||
- `src/components/insights/insight-status-card.tsx` swapped the
|
||||
centred `<Loader2>` for a structured skeleton that mirrors the
|
||||
rendered card geometry (icon dot, title bar, three prose lines,
|
||||
footer). `aria-busy="true"` + `sr-only` "Loading" + `motion-reduce`
|
||||
preserve the accessible loading semantics.
|
||||
- Two new test groups in
|
||||
`src/lib/insights/__tests__/bmi-status.test.ts`:
|
||||
- `v1.4.37 timeout-stub persistence` (2 tests) — exercises the
|
||||
`auditLog.create` write on the timeout branch and verifies a
|
||||
pre-seeded stub short-circuits the next mount before the
|
||||
provider is touched.
|
||||
- One additional case in
|
||||
`src/components/insights/__tests__/insight-status-card.test.tsx`
|
||||
pins the structured skeleton's aria contract.
|
||||
|
||||
### Item 10 — IP-whois resolution — resolved
|
||||
|
||||
- `src/lib/api-response.ts` added `readCfConnectingIp` and consults
|
||||
it ahead of the XFF + x-real-ip chain in both `getClientIp` and
|
||||
`getClientIpOrTrustWarning`. Only honoured under
|
||||
`TRUST_CF_CONNECTING_IP=1` (strict equality — `"true"` and other
|
||||
truthy strings are rejected, documented in the test).
|
||||
- `.env.example` gained a dedicated "Reverse-proxy trust" block
|
||||
above the process-split section so the flag sits next to the
|
||||
existing infra-trust controls.
|
||||
- `src/lib/jobs/geo-backfill.ts` now exports `GEO_BACKFILL_QUEUE` +
|
||||
`GEO_BACKFILL_CRON` so the worker boot imports them and a unit
|
||||
test can pin the scheduling shape without booting pg-boss.
|
||||
- `src/lib/jobs/reminder-worker.ts` registers the queue (createQueue
|
||||
+ schedule + work) on the :40 hourly slot, dodging the existing
|
||||
:00 / :15 / :30 / :25 crowded crons. New `GeoBackfillPayload`
|
||||
interface + `handleGeoBackfill` wrap follow the same shape as
|
||||
`handleFeedbackAggregator` (best-effort warn on error, structured
|
||||
Wide-Event meta).
|
||||
- Tests:
|
||||
- `src/lib/__tests__/get-client-ip.test.ts` — 6 new cases under
|
||||
"Cloudflare cf-connecting-ip branch (v1.4.37)" covering on /
|
||||
off / non-`"1"` / missing-header / malformed / tagged-helper.
|
||||
- `src/lib/jobs/__tests__/geo-backfill.test.ts` — 3 new cases
|
||||
under "geo-backfill scheduling contract (v1.4.37)" pinning
|
||||
the queue name + cron + the worker import shape (source-text
|
||||
probe avoids dragging pg-boss into the unit test).
|
||||
|
||||
## Tests delta
|
||||
|
||||
- Before W6: 0 timezone-picker tests, 12 get-client-ip tests, 10
|
||||
geo-backfill tests, 5 insight-status-card tests, 2 bmi-status
|
||||
test groups.
|
||||
- After W6: +10 timezone (`<TimezonePicker>` SSR pins +
|
||||
`resolveInitialTimezone` matrix), +6 cf-connecting-ip cases, +3
|
||||
scheduling-contract cases, +1 skeleton render case, +2
|
||||
timeout-stub persistence cases.
|
||||
- Net: **+22 new tests** across 6 files (4 new files, 2 augmented).
|
||||
- Suite: 4 417 → 4 460 passing (the gap above +22 is other W*
|
||||
agents landing in parallel).
|
||||
|
||||
## Quality gates
|
||||
|
||||
- `pnpm typecheck` — only pre-existing baseline error in
|
||||
`src/lib/insights/__tests__/features.test.ts:129` (other agent's
|
||||
WIP, not W6). My files typecheck clean.
|
||||
- `pnpm lint` — only pre-existing baseline error in
|
||||
`src/components/dashboard/medication-intake-quick-add.tsx:209`
|
||||
(other agent's WIP, not W6). My files lint clean.
|
||||
- `pnpm test --run` — 1 file fails (the same `features.test.ts`
|
||||
baseline); 4 460 passing including every W6-touched suite.
|
||||
|
||||
## Code-review pass (self-review, code-reviewer skill unavailable)
|
||||
|
||||
The marathon brief asked for a `superpowers:code-reviewer` invocation;
|
||||
that exact slug is not in the harness's skill list. I performed a
|
||||
structured self-review against the same axes the code-review skill
|
||||
applies:
|
||||
|
||||
- **Correctness** — every branch I added has a paired test, including
|
||||
the corner cases (empty browser zone, malformed CF header, env flag
|
||||
not `"1"` literal). The BMI persist-the-stub `auditLog.create`
|
||||
call swallows write errors deliberately so a transient DB hiccup
|
||||
cannot collapse the route's deterministic-fallback contract.
|
||||
- **Security** — `TRUST_CF_CONNECTING_IP` is the operator opt-in for
|
||||
the new header branch; default off keeps a non-Cloudflare deployment
|
||||
safe from a forged `cf-connecting-ip`. The helper still passes
|
||||
`looksLikeIp` so a malformed value is rejected even with the flag
|
||||
on. No expansion of attack surface beyond the documented header.
|
||||
- **Performance** — the geo-backfill cadence (:40 hourly) sits in
|
||||
a deliberately low-traffic slot; the helper's 5 000-row cap caps
|
||||
the audit-log write burst per pass. The BMI stub adds one
|
||||
`auditLog.create` per timeout (which previously cost a re-fired
|
||||
20 s provider race on every mount), strictly a win.
|
||||
- **A11y** — the skeleton retains `aria-busy="true"` + `sr-only`
|
||||
"Loading" so the announce-on-load semantics are preserved; the
|
||||
visual spinner copy retired only because the skeleton's structural
|
||||
pulse already telegraphs progress.
|
||||
- **i18n** — six-locale parity restored after the key deletes; the
|
||||
`timezone-picker.tsx` consumer no longer references the removed
|
||||
keys; greps clean across `src/` and the message bags.
|
||||
- **Test discipline** — every test runs in < 500 ms in the project
|
||||
fixture; no `network`, no `prisma` real connection, no leaked
|
||||
timers (the fake-timers branch in the BMI persist test calls
|
||||
`vi.useRealTimers()` in the same suite).
|
||||
|
||||
No Critical or High findings against my W6 diff. The race-condition
|
||||
artefact above is the only operational note worth carrying forward
|
||||
into the marathon retrospective.
|
||||
|
||||
## Brief-back (≤ 200 words)
|
||||
|
||||
(a) Per-item outcome
|
||||
- Item 8 (timezone removal): **resolved** — button gone, silent
|
||||
browser auto-seed in place, six-locale i18n keys dropped, 10 new
|
||||
tests pin the contract.
|
||||
- Item 6 (BMI loading): **resolved** — sentinel row persists on
|
||||
timeout, structured skeleton replaces centred spinner, 3 new
|
||||
tests cover both the persist + skeleton paths.
|
||||
- Item 10 (IP-whois): **resolved** — `cf-connecting-ip` honoured
|
||||
under `TRUST_CF_CONNECTING_IP=1`, geo-backfill scheduled at :40
|
||||
hourly via pg-boss with co-located queue constants for testability.
|
||||
|
||||
(b) Does BMI persist-the-stub generalise to the other 4 status
|
||||
routes? Yes, mechanically — `weight-status.ts`,
|
||||
`blood-pressure-status.ts`, `pulse-status.ts`, `mood-status.ts`,
|
||||
and `medication-compliance-status.ts` all share the same
|
||||
`if (raced.timedOut || raced.value === null)` shape and the
|
||||
same auditLog-keyed cache lookup, so the fix is a copy-paste
|
||||
of the 35-line block per file. v1.4.37 ships the BMI fix
|
||||
only per the dispatch brief; the four siblings land in a
|
||||
follow-up release once the per-metric pre-warm worker contracts
|
||||
are reconciled (no behaviour change to the user, just frequency).
|
||||
|
||||
(c) Timezone removal left no dangling i18n keys: a `grep -r
|
||||
timezoneDetect src/ messages/{de,en,es,fr,it,pl}.json` is
|
||||
clean; the locale-integrity test runs green.
|
||||
Reference in new issue
Block a user