From 766a9ae262ddd7167fe7dce9cb7592372d6ed99c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 11 May 2026 08:28:02 +0200 Subject: [PATCH] docs(planning): land W1 research + W5/W6 review packs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the artifacts referenced from STATE.md, v1423-backlog.md, and phase-W6-v1423-reconcile-report.md but never committed during the wave-close ticks. W1 research = Apple Health type mapping + APNs library decision + OpenAPI tooling choice; W5 v1422 reviews + W6 v1423 reviews are the six-brief multi-agent QA inputs that drove the reconcile commit chain. Leaves the pre-existing stale untracked entries (v1422-w1a probes, audit-a6 scripts, export-route directories) untouched — those predate v1.4.23. Co-Authored-By: Claude Opus 4.7 (1M context) --- .planning/phase-W1-v1423-research.md | 912 ++++++++++++++++++ .planning/phase-W1b-coach-prompt-research.md | 791 +++++++++++++++ .planning/phase-W5-v1422-code-review.md | 127 +++ .planning/phase-W5-v1422-design-review.md | 123 +++ .../phase-W5-v1422-product-lead-review.md | 492 ++++++++++ .planning/phase-W5-v1422-security-review.md | 59 ++ .planning/phase-W5-v1422-senior-dev-review.md | 374 +++++++ .planning/phase-W5-v1422-simplify-review.md | 209 ++++ .planning/phase-W6-v1423-code-review.md | 115 +++ .planning/phase-W6-v1423-design-review.md | 135 +++ .../phase-W6-v1423-product-lead-review.md | 648 +++++++++++++ .planning/phase-W6-v1423-security-review.md | 68 ++ .planning/phase-W6-v1423-senior-dev-review.md | 477 +++++++++ .planning/phase-W6-v1423-simplify-review.md | 198 ++++ 14 files changed, 4728 insertions(+) create mode 100644 .planning/phase-W1-v1423-research.md create mode 100644 .planning/phase-W1b-coach-prompt-research.md create mode 100644 .planning/phase-W5-v1422-code-review.md create mode 100644 .planning/phase-W5-v1422-design-review.md create mode 100644 .planning/phase-W5-v1422-product-lead-review.md create mode 100644 .planning/phase-W5-v1422-security-review.md create mode 100644 .planning/phase-W5-v1422-senior-dev-review.md create mode 100644 .planning/phase-W5-v1422-simplify-review.md create mode 100644 .planning/phase-W6-v1423-code-review.md create mode 100644 .planning/phase-W6-v1423-design-review.md create mode 100644 .planning/phase-W6-v1423-product-lead-review.md create mode 100644 .planning/phase-W6-v1423-security-review.md create mode 100644 .planning/phase-W6-v1423-senior-dev-review.md create mode 100644 .planning/phase-W6-v1423-simplify-review.md diff --git a/.planning/phase-W1-v1423-research.md b/.planning/phase-W1-v1423-research.md new file mode 100644 index 000000000..7857f415e --- /dev/null +++ b/.planning/phase-W1-v1423-research.md @@ -0,0 +1,912 @@ +# Wave 1 — v1.4.23 research (Apple Health + APNs + OpenAPI) + +Status: research-only. No source files modified. Outputs feed W2 +(Apple Health foundation), W3 (APNs scaffolding), and W4 +(OpenAPI gate + Coach schema slot + native auth). + +Cross-references: + +- `.planning/STATE.md` Wave 2-4 sub-items +- `prisma/schema.prisma` lines 189-245 (`MeasurementType`, + `MeasurementSource`, `Measurement`) +- `src/lib/withings/client.ts` lines 122-133 (mapping pattern to + mirror) +- `src/lib/notifications/dispatcher.ts` (cascade we extend) +- `src/lib/notifications/senders/web-push.ts` (sender shape to + mirror, returns `SendOutcome`) +- `~/Projects/healthlog-iOS/HealthLogIOS/HealthLog/Models/MeasurementDTO.swift` + (already-shipped iOS DTO — names the source `HEALTHKIT`, not + `APPLE_HEALTH`) +- `~/Projects/healthlog-iOS/HealthLogIOS/HealthLog/Services/HealthKitService.swift` + (already-wired `defaultReadTypes` set) + +--- + +## Stream 1 — Apple Health mapping + +### Naming-decision callout (read first) + +The iOS app already ships an enum value `HEALTHKIT` for the +measurement-source field in +`MeasurementDTO.swift` (line 60): + +```swift +case healthKit = "HEALTHKIT" +``` + +The marathon brief calls it `apple_health` / `APPLE_HEALTH`. Two +options: + +1. **Server adopts `HEALTHKIT`** — matches the already-shipped + iOS DTO, no client-side change. Cost: the source name leaks + the framework name (HealthKit) rather than the brand + (Apple Health) into our DB enum. Apple's own marketing surface + says "Apple Health", but their developer surface says + "HealthKit". +2. **Server adopts `APPLE_HEALTH`** — matches the marathon brief + and Marc's user-facing voice ("Apple Health"). Cost: iOS DTO + needs a one-line rename + compatibility shim for any encoded + payloads already on disk in the iOS sim build. + +**Recommendation: `APPLE_HEALTH`.** The DB enum is a stable +contract value, not a brand label, but it is also user-facing +through analytics/audit-log surfaces. "Apple Health" is the term +users recognise. The iOS DTO is still pre-TestFlight (no real +users on the encoded format yet) so the rename is essentially +free. + +W2 should land the rename in `MeasurementDTO.swift` in the same +PR that lands the server enum so the contract stays in lock-step. + +### Mapping table + +Aggregation rule reads: + +- `latest` — store every sample as-is, dedup by `(externalId)` +- `sum` — sum samples in a 1-hour rollup window (Apple's hourly + observer-query batch grain) +- `mean` — arithmetic mean of values in the rollup window +- `last_per_window` — keep the chronologically latest value when + multiple samples land in one upload batch (Apple's discrete- + scalar pattern, e.g. resting HR) + +| HK identifier | DB MeasurementType | DB unit | HK unit | Conversion | Aggregation | Privacy / consent | Notes | +| -------------------------- | -------------------------------- | ----------- | ----------------------------------------------------- | ----------------------- | --------------- | --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| `bodyMass` | `WEIGHT` (existing) | kg | `.gramUnit(.kilo)` | identity | latest | already in scope | already shipped; no schema change | +| `bloodPressureSystolic` | `BLOOD_PRESSURE_SYS` (existing) | mmHg | `.millimeterOfMercury()` | identity | latest | already in scope | always paired with diastolic; iOS posts both as separate rows on identical `measuredAt` (matches Withings pattern) | +| `bloodPressureDiastolic` | `BLOOD_PRESSURE_DIA` (existing) | mmHg | `.millimeterOfMercury()` | identity | latest | already in scope | see above | +| `heartRate` | `PULSE` (existing) | bpm | `.count()/.minute()` | identity | last_per_window | already in scope | spot pulse only; Apple Watch generates many per day — iOS must throttle (e.g. one sample every 5 min) before posting or the batch saturates | +| `heartRateVariabilitySDNN` | `HEART_RATE_VARIABILITY` (NEW) | ms | `.secondUnit(.milli)` | identity | mean | yes (new — explicit consent screen) | Apple captures during sleep window; one sample per night typical | +| `restingHeartRate` | `RESTING_HEART_RATE` (NEW) | bpm | `.count()/.minute()` | identity | last_per_window | already in scope (umbrella with PULSE) | Apple emits one per day from sleep + low-activity windows | +| `stepCount` | `STEP_COUNT` (NEW) | steps | `.count()` | identity | sum | already in scope | cumulative; iOS posts daily totals at 23:55 local + a delta on observer wake | +| `activeEnergyBurned` | `ACTIVE_ENERGY_BURNED` (NEW) | kcal | `.kilocalorie()` | identity | sum | already in scope | cumulative; same daily-rollup pattern as steps | +| `flightsClimbed` | `FLIGHTS_CLIMBED` (NEW) | flights | `.count()` | identity | sum | already in scope | cumulative; same daily rollup | +| `distanceWalkingRunning` | `WALKING_RUNNING_DISTANCE` (NEW) | m | `.meter()` | identity | sum | already in scope | cumulative; SI metres in DB, UI converts to km/mi per locale | +| `vo2Max` | `VO2_MAX` (NEW) | mL/(kg·min) | `.literUnit(.milli) / (.gramUnit(.kilo) · .minute())` | identity | last_per_window | yes (new — fitness/health-grade metric) | one sample per few days at most | +| `sleepAnalysis` | `SLEEP_DURATION_DETAILED` (NEW) | minutes | category | sum durations per stage | sum | yes (new — explicit consent) | NOT a quantity type; see "Sleep schema branch" below | + +**Net new MeasurementType values: 8.** + +`HEART_RATE_VARIABILITY`, `SLEEP_DURATION_DETAILED`, +`RESTING_HEART_RATE`, `STEP_COUNT`, `ACTIVE_ENERGY_BURNED`, +`FLIGHTS_CLIMBED`, `WALKING_RUNNING_DISTANCE`, `VO2_MAX`. + +These mirror the values listed in `STATE.md` Wave 2 F1 verbatim +except for the rename `ACTIVITY_STEPS` (existing) → +`STEP_COUNT` (new). See "Naming collision" below. + +### Naming collision: existing `ACTIVITY_STEPS` and `SLEEP_DURATION` + +The current `MeasurementType` enum already contains +`ACTIVITY_STEPS` (steps unit) and `SLEEP_DURATION` (hours unit). +Neither has any production data path today — `getUnitForType` +returns `steps`/`hours` and the `validateMeasurementRange` row +exists, but no ingest route or UI surface wires either. + +Two options: + +1. **Reuse the existing `ACTIVITY_STEPS` and `SLEEP_DURATION` + enum values** for Apple Health steps + sleep, and SKIP adding + `STEP_COUNT` / `SLEEP_DURATION_DETAILED`. Net new = 6. +2. **Add the new `STEP_COUNT` / `SLEEP_DURATION_DETAILED` values + and deprecate the originals** (no production data → safe). + Net new = 8 (matches STATE.md). + +**Recommendation: option 1, reuse `ACTIVITY_STEPS` and +`SLEEP_DURATION`.** The existing values were added in +anticipation of exactly this work (per the comment in +`schema.prisma` near lines 195-196). Adding parallel new values +fragments the enum and forces every analytics codepath to handle +two synonyms. The marathon brief's name `STEP_COUNT` is the +HealthKit identifier; our DB convention is the domain noun +(WEIGHT, PULSE, BODY_FAT) — so `ACTIVITY_STEPS` already fits the +house style better than `STEP_COUNT` does. + +Sleep needs the unit changed from `hours` to `minutes` (HealthKit +emits sleep durations as minute-resolution category samples; +storing fractional hours loses precision). That's a Wave 2 F1 +sub-decision — schema-only since no data exists. + +**Net new with option 1 adopted: 6.** Final list: +`HEART_RATE_VARIABILITY`, `RESTING_HEART_RATE`, +`ACTIVE_ENERGY_BURNED`, `FLIGHTS_CLIMBED`, +`WALKING_RUNNING_DISTANCE`, `VO2_MAX`. + +### Sleep schema branch (the one that's not a `HKQuantityType`) + +`HKCategoryTypeIdentifier.sleepAnalysis` is a category type, not +a quantity type. Each sample has a value drawn from +`HKCategoryValueSleepAnalysis`: + +- `inBed` +- `asleepUnspecified` (legacy iOS 15-) +- `awake` +- `asleepCore` (iOS 16+) +- `asleepDeep` (iOS 16+) +- `asleepREM` (iOS 16+) + +Each sample has a `startDate` and `endDate`; duration is +`endDate - startDate` in seconds. A single night produces tens of +samples (every stage transition). + +Two server-shape options: + +1. **Single `Measurement` row per night with summed total** — + value = total minutes asleep, no stage breakdown. Easiest; + loses Apple's stage data the user paid Watch battery for. +2. **One `Measurement` row per stage per night** — value = + minutes in that stage, with a new `sleepStage` enum column. + Symmetrical to the existing `glucoseContext` column. + +**Recommendation: option 2 with a new `sleepStage` enum column +on `Measurement`** (`CORE`, `DEEP`, `REM`, `AWAKE`, `IN_BED`). +NULL for non-sleep measurements, mirroring the existing +`glucoseContext` pattern. The Coach extension (W4 F6) will want +the stage breakdown to honour the "no clinical sleep-stage +conclusions" rule from the v1.5 product-lead memo — without the +stage rows we can't audit prompt compliance. + +Migration: `ALTER TABLE measurements ADD COLUMN sleep_stage +sleep_stage NULL`. Add a CHECK constraint mirroring the +`glucoseContext` pattern: `sleep_stage` non-NULL only when +`type = SLEEP_DURATION`. + +### `apple-health-mapping.ts` skeleton + +Path: `src/lib/measurements/apple-health-mapping.ts` (new dir, +new file — symmetrical to the implicit Withings mapping in +`src/lib/withings/client.ts:122-133`). + +```ts +import type { MeasurementType } from "@/generated/prisma/client"; + +/** + * Apple HealthKit identifier → HealthLog `MeasurementType` mapping. + * Mirrors the Withings `MEASURE_TYPE_MAP` in src/lib/withings/client.ts + * but keyed by string identifier (Apple's own constant name) rather + * than a numeric type code. + * + * Values: + * - `type` — DB enum + * - `unit` — canonical DB unit (must match unitMap in + * src/lib/validations/measurement.ts) + * - `aggregate`— how the ingest endpoint should reduce a batch of + * HK samples into stored Measurement rows + * - `consent` — UI must show explicit consent before requesting + * HKObjectType permission for these + */ +export type HealthKitAggregation = + | "latest" + | "sum" + | "mean" + | "last_per_window"; + +export interface AppleHealthMapping { + type: MeasurementType; + unit: string; + aggregate: HealthKitAggregation; + consent: "implicit" | "explicit"; + /** + * Number of HK sample-fractional-units per 1 DB-unit. 1.0 = identity. + * `heartRateVariabilitySDNN` ships as `secondUnit(.milli)` on Apple's + * side and we store `ms` — identity. `oxygenSaturation` ships as a + * 0..1 fraction and we store percent (0..100) — `factor: 100`. + */ + factor: number; +} + +export const APPLE_HEALTH_TYPE_MAP: Record = { + // Body composition + HKQuantityTypeIdentifierBodyMass: { + type: "WEIGHT", + unit: "kg", + aggregate: "latest", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierBodyFatPercentage: { + type: "BODY_FAT", + unit: "%", + aggregate: "latest", + consent: "implicit", + factor: 100, // Apple ships 0..1 fraction + }, + + // Cardiovascular + HKQuantityTypeIdentifierBloodPressureSystolic: { + type: "BLOOD_PRESSURE_SYS", + unit: "mmHg", + aggregate: "latest", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierBloodPressureDiastolic: { + type: "BLOOD_PRESSURE_DIA", + unit: "mmHg", + aggregate: "latest", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierHeartRate: { + type: "PULSE", + unit: "bpm", + aggregate: "last_per_window", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierRestingHeartRate: { + type: "RESTING_HEART_RATE", + unit: "bpm", + aggregate: "last_per_window", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierHeartRateVariabilitySDNN: { + type: "HEART_RATE_VARIABILITY", + unit: "ms", + aggregate: "mean", + consent: "explicit", + factor: 1, + }, + + // Activity (cumulative) + HKQuantityTypeIdentifierStepCount: { + type: "ACTIVITY_STEPS", // reuses existing enum value (see report) + unit: "steps", + aggregate: "sum", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierActiveEnergyBurned: { + type: "ACTIVE_ENERGY_BURNED", + unit: "kcal", + aggregate: "sum", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierFlightsClimbed: { + type: "FLIGHTS_CLIMBED", + unit: "flights", + aggregate: "sum", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierDistanceWalkingRunning: { + type: "WALKING_RUNNING_DISTANCE", + unit: "m", + aggregate: "sum", + consent: "implicit", + factor: 1, + }, + + // Fitness + HKQuantityTypeIdentifierVO2Max: { + type: "VO2_MAX", + unit: "mL/(kg·min)", + aggregate: "last_per_window", + consent: "explicit", + factor: 1, + }, + + // Other in-scope (already mapped via existing enums) + HKQuantityTypeIdentifierBloodGlucose: { + type: "BLOOD_GLUCOSE", + unit: "mg/dL", + aggregate: "latest", + consent: "implicit", + factor: 1, + }, + HKQuantityTypeIdentifierOxygenSaturation: { + type: "OXYGEN_SATURATION", + unit: "%", + aggregate: "last_per_window", + consent: "implicit", + factor: 100, // Apple ships 0..1 fraction; matches schema.prisma comment + }, +}; + +/** + * sleepAnalysis is a category type, not a quantity type — it gets its + * own table-shape branch. Keys here are the values of + * `HKCategoryValueSleepAnalysis`. The ingest endpoint stores one + * Measurement row per (night, stage), value = minutes-in-stage. + */ +export const APPLE_HEALTH_SLEEP_STAGE_MAP: Record< + string, + "CORE" | "DEEP" | "REM" | "AWAKE" | "IN_BED" +> = { + HKCategoryValueSleepAnalysisAsleepCore: "CORE", + HKCategoryValueSleepAnalysisAsleepDeep: "DEEP", + HKCategoryValueSleepAnalysisAsleepREM: "REM", + HKCategoryValueSleepAnalysisAwake: "AWAKE", + HKCategoryValueSleepAnalysisInBed: "IN_BED", + // legacy iOS 15- — count as CORE for back-compat + HKCategoryValueSleepAnalysisAsleepUnspecified: "CORE", +}; +``` + +W2's F1+F2+F3 implementer fills in the migration + the +`POST /api/measurements/batch` route. The route's per-entry +validator just looks up `APPLE_HEALTH_TYPE_MAP[entry.hkIdentifier]`, +applies the `factor`, and inserts with `source = APPLE_HEALTH` + +`externalId = entry.uuid` (HealthKit's `HKSample.uuid` is stable +across queries — it's the dedup key). + +### Open questions for the maintainer + +1. **`HEALTHKIT` vs `APPLE_HEALTH` source enum value.** This + research recommends `APPLE_HEALTH` and renaming the iOS DTO + in the same PR. Confirm or override before W2 starts. +2. **Reuse `ACTIVITY_STEPS` + `SLEEP_DURATION` (option 1 above) + or add `STEP_COUNT` + `SLEEP_DURATION_DETAILED` as parallel + values?** Recommendation: reuse, change SLEEP unit to minutes. +3. **Sleep stage breakdown — single row per night vs per-stage + row with new `sleep_stage` column?** Recommendation: + per-stage, mirrors the `glucoseContext` pattern. + +--- + +## Stream 2 — APNs library decision + +### Comparison + +| axis | `@parse/node-apn` | `apns2` (AndrewBarba) | hand-rolled `node:http2` | +| ---------------------- | -------------------------------------------------------------- | ---------------------------------------------------- | ----------------------------- | +| last release | 8.1.0, April 2026 (active) | 12.2.0, May 2025 (12 months ago, but stable surface) | n/a | +| weekly downloads | ~107k | ~6k | n/a | +| GitHub stars | ~219 | ~173 | n/a | +| TypeScript types | built-in `index.d.ts` | 100% TypeScript codebase, types are the source | hand-rolled | +| ESM support | hybrid CJS/ESM | ESM + CJS (v12) | native | +| HTTP/2 implementation | native `http2`, connection pool | native `http2`, connection pool | manual | +| JWT signing | bundled | bundled | manual (jsonwebtoken or jose) | +| Sandbox vs prod toggle | `production: boolean` flag | `host` string override | manual base URL | +| Dependencies | `debug`, `verror`, `node-forge`, `jsonwebtoken` | only `jsonwebtoken` (small graph) | none | +| License | MIT | MIT | n/a | +| Maintainer | Parse Platform org (multi-maintainer, Parse Server downstream) | one maintainer | n/a | +| Test mockability | Stub the `Provider`; existing examples in Parse Server tests | Stub `ApnsClient` interface | trivial — mock fetch shape | + +### Recommendation: **`@parse/node-apn`** + +The active 8.1.0 release in April 2026 plus the 100k+ +weekly-download install base under Parse Server is the dominant +signal. `apns2` is technically cleaner (single small dep, native +TypeScript codebase) but its maintenance pulse went quiet 12 +months ago and the maintainer count is one. APNs's spec is +stable enough that "12 months quiet" is not a red flag on its +own, but if Apple introduces a new push type (precedent: the +`live-activity` type in iOS 16) we want a library whose issues +get triaged. + +The downside is a slightly heavier dep graph (`node-forge`, +`verror`) but those are dependencies HealthLog's existing +notification stack already pulls transitively (`web-push` pulls +`node-forge` for VAPID JWT). Net new install footprint is small. + +For tests, `dispatcher.test.ts`'s pattern of stubbing +sender modules at import time (see how `web-push` is treated) +applies cleanly: a test-only `mock-apns.ts` exporting the same +`sendViaApns(config, payload): SendOutcome` shape sidesteps the +library entirely. We **do not** need to mock the Parse provider +class — we only ever call our own wrapper. + +Hand-rolled HTTP/2 is rejected: even though the protocol is +~150 lines, the JWT-key rotation logic (regenerate every 50 min, +share across requests, handle 403 InvalidProviderToken by +forcing immediate rotation) is exactly the kind of code that +sits unmaintained for years until Apple shifts a corner-case +status code. Outsourcing is correct here. + +### Code sketch + env-var contract + +Path: `src/lib/notifications/senders/apns.ts` (new). + +```ts +import apn from "@parse/node-apn"; +import { decrypt } from "@/lib/crypto"; +import type { NotificationPayload } from "@/lib/notifications/types"; +import type { SendOutcome } from "@/lib/notifications/retry-policy"; +import { classifyHttpStatus } from "@/lib/notifications/retry-policy"; +import { prisma } from "@/lib/db"; +import { getEvent } from "@/lib/logging/context"; + +interface ApnsEnv { + keyId: string; + teamId: string; + bundleId: string; + signingKey: string; // .p8 PEM contents (multiline) + production: boolean; +} + +function loadEnv(): ApnsEnv | null { + const keyId = process.env.APNS_KEY_ID; + const teamId = process.env.APNS_TEAM_ID; + const bundleId = process.env.APNS_BUNDLE_ID; + // APNS_KEY_FILE = path to .p8 (preferred for dev). APNS_KEY = inline + // contents (preferred for Coolify env-block). One must be set. + const signingKey = + process.env.APNS_KEY ?? + (process.env.APNS_KEY_FILE + ? require("node:fs").readFileSync(process.env.APNS_KEY_FILE, "utf8") + : undefined); + if (!keyId || !teamId || !bundleId || !signingKey) return null; + const production = process.env.APNS_PRODUCTION === "true"; + return { keyId, teamId, bundleId, signingKey, production }; +} + +let cachedProvider: apn.Provider | null = null; + +function getProvider(env: ApnsEnv): apn.Provider { + // node-apn re-uses HTTP/2 connections + JWT internally; one Provider + // per process is the documented pattern. + if (cachedProvider) return cachedProvider; + cachedProvider = new apn.Provider({ + token: { key: env.signingKey, keyId: env.keyId, teamId: env.teamId }, + production: env.production, + }); + return cachedProvider; +} + +/** + * Send an APNs alert push to all registered devices for `userId`. + * Mirrors the SendOutcome contract used by web-push so the dispatcher + * can apply the same retry/cooldown logic uniformly. + */ +export async function sendViaApns( + userId: string, + payload: NotificationPayload, +): Promise { + const env = loadEnv(); + if (!env) { + return { ok: false, hardReject: false, reason: "apns_not_configured" }; + } + + const devices = await prisma.device.findMany({ + where: { userId, platform: "ios" }, + }); + if (devices.length === 0) { + return { ok: false, hardReject: false, reason: "apns_no_devices" }; + } + + const provider = getProvider(env); + const note = new apn.Notification(); + note.topic = env.bundleId; + note.alert = { title: payload.title, body: payload.message }; + note.sound = "default"; + note.payload = { eventType: payload.eventType, ...payload.metadata }; + + const start = performance.now(); + const tokens = devices.map((d) => d.token); + const result = await provider.send(note, tokens); + + getEvent()?.addExternalCall({ + service: "apns", + method: "send", + duration_ms: Math.round(performance.now() - start), + status: 200, + error: result.failed.length === devices.length ? "all_failed" : undefined, + }); + + // 410 BadDeviceToken / 400 BadDeviceToken / "Unregistered" → drop the + // device row (pattern mirrors web-push 410 cleanup) + const dead = result.failed + .filter( + (f) => + f.response?.reason === "Unregistered" || + f.response?.reason === "BadDeviceToken", + ) + .map((f) => f.device); + if (dead.length > 0) { + await prisma.device.deleteMany({ where: { token: { in: dead } } }); + } + + if (result.sent.length > 0) return { ok: true }; + + // All failed and all permanent → hard reject. Else transient. + if (dead.length === devices.length) { + return { + ok: false, + hardReject: true, + statusCode: 410, + reason: "apns_all_devices_unregistered", + }; + } + const lastStatus = result.failed[0]?.status + ? Number(result.failed[0].status) + : undefined; + const classified = classifyHttpStatus(lastStatus, "apns"); + return { + ok: false, + hardReject: false, + statusCode: lastStatus, + reason: classified.reason, + }; +} +``` + +**Env-var contract:** + +| var | required | shape | notes | +| ----------------- | -------- | ------------------------------------------------ | ---------------------------- | +| `APNS_KEY_ID` | yes | 10-char alphanumeric | from Apple developer account | +| `APNS_TEAM_ID` | yes | 10-char alphanumeric | from Apple developer account | +| `APNS_BUNDLE_ID` | yes | reverse-DNS string (e.g. `dev.healthlog.ios`) | `note.topic` | +| `APNS_KEY` | one-of | multiline PEM (`-----BEGIN PRIVATE KEY-----...`) | preferred for Coolify | +| `APNS_KEY_FILE` | one-of | absolute path | preferred for local dev | +| `APNS_PRODUCTION` | optional | `"true"` or unset | default = sandbox | + +The `.p8` PEM contents are sensitive — encrypt at rest if we +ever start storing per-tenant keys. For v1.4.23 single-tenant +the env-var route is correct. + +### `apns_attempts` table — reuse, don't add + +The existing `NotificationChannel` model carries +`consecutiveFailures`, `nextRetryAt`, `lastFailureReason`, +`lastSuccessAt`, `disabledReason` — that's exactly the per- +channel retry state we need. APNs joins `TELEGRAM`, `NTFY`, +`WEB_PUSH` as a fourth `ChannelType` value. + +**No new attempts table.** Reuse the channel-state machine in +`src/lib/notifications/channel-state.ts`. The Wave 3 tasks are: + +1. Add `"APNS"` to `CHANNEL_TYPES` in + `src/lib/notifications/types.ts`. +2. Switch case in `dispatcher.ts:sendToChannel()` calls + `sendViaApns(payload.userId, payload)` (mirror the + `WEB_PUSH` branch — also takes `userId` not `config`, + because device tokens live in `Device` not in + `NotificationChannel.config`). +3. The `NotificationChannel` row for an APNs user carries an + empty config (like `WebPushChannelConfig`); the row exists + so the per-event preference toggles in + `NotificationPreference` work uniformly across all channels. + +### Open questions for the maintainer + +1. **Push type**: `alert` only for v1.4.23, or also `background` + silent pushes (for HealthKit observer-query "wake the iOS app" + pattern)? Background pushes need + `apns-push-type: background` header + priority 5; Apple + throttles them aggressively. Recommendation: `alert`-only + for v1.4.23, defer background to v1.5 P4. +2. **Per-device opt-out**: today's `NotificationPreference` is + per `(channel, eventType)`. With multiple iOS devices on one + account, do we need per-device-token opt-out (mute APNs on + one phone but not the other)? Punt to v1.5 P4 unless Marc + says otherwise. +3. **Sandbox auto-detect**: the iOS app builds with two schemes + (Debug → sandbox APNs gateway, Release → production). The + server can't tell which gateway a device-token came from + except by trial-and-error. Two options: (a) the iOS app + includes `apnsEnvironment: "sandbox"|"production"` in + `POST /api/devices`, (b) the server tries production first + and falls back to sandbox on `BadDeviceToken`. Recommendation: + (a) — explicit. Add `apnsEnvironment` column to `Device` + model in W3. + +--- + +## Stream 3 — OpenAPI tooling decision + +### Comparison + +| axis | `@asteasolutions/zod-to-openapi` | `zod-openapi` (samchungy) | `ts-rest` / `next-rest` | hand-rolled | +| -------------------------------- | ---------------------------------------------------- | ----------------------------------------------------------------- | ---------------------------------------- | ------------ | +| weekly downloads | ~1.5M | ~250k | ~50k each | n/a | +| GitHub stars | ~1.5k | ~640 | ~5k / ~200 | n/a | +| Zod v4 support | yes (registry pattern unchanged) | yes (uses `.meta()` — Zod v4 native) | yes | n/a | +| OpenAPI 3.1 emit | yes | yes (last release Jan 2026 = active) | partial | n/a | +| Schema annotation style | `.openapi({...})` (registers extension) | `.meta({...})` (zod-native) | route-binding DSL | json by hand | +| Setup cost | extend Zod once at boot, register schemas/routes | import once, call `.meta()` per schema | rewrite all route handlers | high | +| Retrofit cost on existing routes | low — schemas already exist, register in a side file | low — annotate schemas in place, add a side file with route table | **high** — every route handler rewritten | high | +| Coupling to Zod internals | medium (custom `.openapi()` method) | low (uses Zod v4's standard `.meta()`) | high (own DSL) | none | +| Active maintenance pulse | active | active (v5.4.6 January 2026) | active | n/a | + +### Recommendation: **`zod-openapi` (samchungy)** + +Two reasons: + +1. **Uses Zod v4's native `.meta()` method.** HealthLog already + imports from `zod/v4` (per CLAUDE.md). `zod-openapi` reads + the metadata that's already a first-class Zod v4 concept — + no monkey-patching, no module-augmentation, no boot-time + `extendZodWithOpenApi()` call. If Zod ever changes its + metadata internals, the breakage surface is one library + maintainer away from a fix; with `zod-to-openapi` it's two. +2. **Lighter retrofit footprint.** HealthLog has ~80+ routes + under `src/app/api/`. Each has either an inline Zod schema + or imports from `src/lib/validations/*`. With `zod-openapi`, + we annotate the existing exported schemas in their original + files (`createMeasurementSchema.meta({...})`) and wire a + small `src/lib/openapi/registry.ts` that imports them and + composes the `paths` object. With `zod-to-openapi`, every + schema needs a `.openapi()` call AND a `registry.register()` + AND a `registry.registerPath()` — ~3x the touchpoints. + +`@asteasolutions/zod-to-openapi` is the safer-by-popularity +choice (10x the downloads) but the popularity is mostly Hono / +Express / NestJS users where it's the only option. For Next.js +App Router with Zod v4 already in the stack, `zod-openapi`'s +`.meta()` story is the better fit and keeps the spec generation +out of the route-handler hot path. + +`ts-rest` / `next-rest` are rejected: both want to own the +route-handler shape (their `defineRoute()` wraps the handler). +Retrofitting 80+ existing `apiHandler()`-wrapped routes is a +multi-week refactor and would absorb the same surface area +v1.4.23 is supposed to be locking down, not churning. + +### Implementation sketch + +Three files, ~120 LOC total. + +**`src/lib/openapi/registry.ts`** (new) — composes the spec +from already-exported Zod schemas: + +```ts +import * as z from "zod/v4"; +import { createDocument } from "zod-openapi"; +import { + createMeasurementSchema, + createBatchMeasurementSchema, + measurementTypeEnum, + measurementSourceEnum, +} from "@/lib/validations/measurement"; +import { errorEnvelopeSchema, dataEnvelopeSchema } from "@/lib/api-response"; +// ... import every schema we want in the spec + +export function buildOpenApiSpec() { + // Annotate schemas where they're not annotated in-place (fallback + // path; preferred is to call .meta() in the validations file). + measurementTypeEnum.meta({ id: "MeasurementType" }); + measurementSourceEnum.meta({ id: "MeasurementSource" }); + + return createDocument({ + openapi: "3.1.0", + info: { + title: "HealthLog API", + version: process.env.npm_package_version ?? "1.4.23", + }, + servers: [{ url: "https://healthlog.bombeck.io" }], + paths: { + "/api/measurements": { + get: { + /* ...listMeasurementsSchema as query params... */ + }, + post: { + requestParams: { + header: z.object({ + "Idempotency-Key": z.string().optional(), + }), + }, + requestBody: { + content: { + "application/json": { schema: createMeasurementSchema }, + }, + }, + responses: { + "201": { + description: "Created", + content: { + "application/json": { + schema: dataEnvelopeSchema( + z.object({ + id: z.string(), + }), + ), + }, + }, + }, + }, + }, + }, + "/api/measurements/batch": { + post: { + requestBody: { + content: { + "application/json": { schema: createBatchMeasurementSchema }, + }, + }, + responses: { + "200": { + /* ... */ + }, + }, + }, + }, + // ... one per route + }, + }); +} +``` + +**`scripts/openapi-generate.ts`** (new): + +```ts +#!/usr/bin/env tsx +import { writeFileSync } from "node:fs"; +import * as YAML from "yaml"; +import { buildOpenApiSpec } from "@/lib/openapi/registry"; + +const spec = buildOpenApiSpec(); +const yaml = YAML.stringify(spec); +writeFileSync("docs/api/openapi.yaml", yaml, "utf8"); +console.log(`wrote docs/api/openapi.yaml (${yaml.length} bytes)`); +``` + +**`scripts/openapi-check.ts`** (new) — CI gate: + +```ts +#!/usr/bin/env tsx +import { readFileSync } from "node:fs"; +import { execSync } from "node:child_process"; +import * as YAML from "yaml"; +import { buildOpenApiSpec } from "@/lib/openapi/registry"; + +const generated = YAML.stringify(buildOpenApiSpec()); +const onDisk = readFileSync("docs/api/openapi.yaml", "utf8"); + +if (generated.trim() !== onDisk.trim()) { + console.error( + "OpenAPI spec drift detected. Run `pnpm openapi:generate` and commit.", + ); + // Helpful diff for the PR reviewer + execSync('diff <(echo "$GEN") docs/api/openapi.yaml || true', { + env: { ...process.env, GEN: generated }, + stdio: "inherit", + shell: "/bin/bash", + }); + process.exit(1); +} +console.log("OpenAPI spec in sync with source schemas."); +``` + +**`package.json` scripts:** + +```json +{ + "openapi:generate": "tsx scripts/openapi-generate.ts", + "openapi:check": "tsx scripts/openapi-check.ts" +} +``` + +**CI gate:** add a step to the existing GH Actions PR workflow: + +```yaml +- name: OpenAPI spec sync check + run: pnpm openapi:check +``` + +The check is structural-equality on YAML text. For a more +robust diff we can swap to `oasdiff` (Go binary, GitHub Action +ready, generates a markdown breaking-change report) — but +sequence-equality is the cheaper W4 starting point and we can +upgrade to oasdiff in v1.4.24 without changing the generator +side. + +### Migration approach for the existing 5468-line hand-maintained spec + +The committed `docs/api/openapi.yaml` is large (80+ paths, 100+ +schemas). Two-stage transition: + +1. **W4 ships the generator covering the 6-8 routes the iOS app + touches** (auth/login, auth/refresh, auth/me, measurements + GET/POST, measurements/batch POST, devices POST, achievements + GET). The CI check runs in **warn-only mode** (`continue-on- +error: true`) until coverage is complete. +2. **v1.4.24+ migrates the rest of the spec** route by route as + they're touched by future PRs (organic adoption). Once + coverage hits ~95%, flip `openapi:check` to fail-the-PR. + +This avoids the "rewrite the whole spec in one PR" anti-pattern. +The committed YAML stays the source-of-truth-for-iOS during the +transition; the generator's output is a strict subset of it. + +### Open questions for the maintainer + +1. **Subset start vs full migration?** Recommendation: subset + (the 6-8 iOS-touched routes), warn-only CI for v1.4.23, full + coverage organically. +2. **YAML normalisation**: zod-openapi emits stable key order + per its docs but YAML `.stringify` may reorder map keys + between Node versions. Pin `yaml@2.x` and add a normalisation + pass before equality compare? Recommendation: yes — use + `yaml.stringify(spec, { sortMapEntries: true })` for both + sides. +3. **`oasdiff` adoption**: defer to v1.4.24, or ship now? Cost + of shipping now is one extra GH Action download + one binary + on the runner; benefit is structured breaking-change + detection on PR comments. Recommendation: defer — text-diff + covers the W2/W3 contract-locking goal. + +--- + +## Cross-stream notes + +### iOS DTO already encodes some Wave 2 decisions + +`MeasurementDTO.swift` includes a `BODY_TEMPERATURE` enum value +that does NOT exist in the server's `MeasurementType` enum +today. Either: + +- iOS will skip body-temperature ingest for v1.4.23 (filter on + the client), OR +- W2 adds `BODY_TEMPERATURE` as a 7th new value (HealthKit type + `HKQuantityTypeIdentifierBodyTemperature`, unit `degC`). + +Recommendation: **add `BODY_TEMPERATURE` in W2 alongside the +other 6 new types.** Cheap to ship a no-op enum value now versus +breaking the iOS DTO contract later. + +Net new MeasurementType count if both options accepted: +**7** (`HEART_RATE_VARIABILITY`, `RESTING_HEART_RATE`, +`ACTIVE_ENERGY_BURNED`, `FLIGHTS_CLIMBED`, +`WALKING_RUNNING_DISTANCE`, `VO2_MAX`, `BODY_TEMPERATURE`). + +### Coach schema slot (W4 F6) preview + +`src/lib/ai/schema.ts:260` defines: + +```ts +sourceMetric: z.enum(["bp", "weight", "pulse", "mood", "compliance"]); +``` + +W4 extends to: + +```ts +sourceMetric: z.enum([ + "bp", + "weight", + "pulse", + "mood", + "compliance", + // v1.4.23 additive (forward-compat for v1.5 iOS) + "hrv", + "sleep", + "restingHr", + "steps", + "activeEnergy", +]); +``` + +`src/lib/ai/coach/types.ts:132` defines `CoachProvenance.metrics` +with the same value set — extend in lock-step. Both are additive, +no migration on `coach_messages.metricSourceJson` (TEXT column, +opaque to Postgres). + +PROMPT_VERSION ratchet 4.22.0 → 4.23.0 (additive forward-compat, +matches the project_v1423_pre_ios_prep.md scope point 6). + +### Top 3 sequencing risks for W2/W3/W4 + +1. **W2 + W4 F6 share files** (`src/lib/ai/schema.ts`, + `src/lib/ai/coach/types.ts`). Land W2 first, F6 second, no + parallel edit. +2. **The iOS DTO rename `HEALTHKIT` → `APPLE_HEALTH`** is a + contract-break for the iOS app's encoded payloads on disk. + Clear the iOS sim data store after the rename (one-line in + the iOS dev README). +3. **W4 OpenAPI generator imports schemas from W2**. Land W2 + before W4 starts the generator file, OR have W4 use + `as const` placeholders for the W2-introduced enum values + that get filled in once W2 lands. diff --git a/.planning/phase-W1b-coach-prompt-research.md b/.planning/phase-W1b-coach-prompt-research.md new file mode 100644 index 000000000..d10413304 --- /dev/null +++ b/.planning/phase-W1b-coach-prompt-research.md @@ -0,0 +1,791 @@ +# Wave 1b — Coach prompt rewrite research (v1.4.22) + +Research-only artifact for the v1.4.22 polishing marathon. Wave 3 dispatches the +rewrite based on the contract proposed below. Production today is on v1.4.21 +(image digest `sha256:4e818d44702c…`); the Coach drawer + SSE streaming + +day-level snapshot have shipped since v1.4.20. + +The maintainer's complaint, in plain English: the Coach answers the right +question with the right caveats, but it sounds like a database with a "no rows +returned" error rather than a person who happens to be looking at health data. +We need warmth and motivational-interviewing flavour without losing the +zero-hallucination ground rules from v1.4.20. The values that the Coach used +to hand-deliver inline should move into a collapsible block at the bottom of +the message, leaving the prose itself prose-first. + +--- + +## 1. Current state + +File: `src/lib/ai/coach/system-prompt.ts` (lines 19–81 EN, 83–150 DE). +Imports `PROMPT_VERSION` from +`src/lib/ai/prompts/insight-generator.ts` (currently `"4.20.2"`). + +**Persona framing today** (EN, lines 19–28): + +> You are the HealthLog Coach — a conversational summariser of the user's own +> health-tracking data. […] +> +> YOUR ROLE +> +> - You ONLY discuss the user's own measurements (blood pressure, weight, +> pulse, mood, medications) and the trends inside the SNAPSHOT block the +> user prompt carries. +> - You DO NOT diagnose, prescribe, give general medical advice, or answer +> questions outside the snapshot. + +This is accurate but flat. "Conversational summariser" describes a function, +not a person. Nothing in the persona invites motivation, curiosity, or warmth +— so the model defaults to a database-cursor voice. + +**Conversation style today** (EN, lines 29–37): + +> CONVERSATION STYLE +> +> - Plain prose, no JSON, no markdown fences. […] +> - One self-contained reply per turn. Keep replies focused (≈ 80-220 words). +> - Address the user as "you". Never invent a name. + +Good guardrails. No tone calibration. No examples. The model gets length +budget but not shape budget. + +**Ground rules** (EN, lines 39–50): + +> GROUND RULES — ZERO HALLUZINATIONS +> +> 1. Every number you mention must come from the SNAPSHOT. […] +> 2. When the SNAPSHOT lacks the information needed to answer, say so +> explicitly ("I do not have HRV data in your log yet"). Do NOT invent +> a substitute. +> 3. When the user asks for a recommendation that is potentially actionable +> (medication change, urgent threshold), end the reply with "please +> consult your doctor" or equivalent. +> 4. Stay calm and factual. Do not open with a compliment about data +> quantity or quality. + +Rule 2 is the single biggest tone problem. The bracketed example +(`"I do not have HRV data in your log yet"`) reads as a fault report. A +human coach hearing the same gap would say "I don't see exercise in what +you're tracking — what does a typical week look like for you?" and pivot to +collaborative discovery. Rule 4's anti-flattery clause is correct in spirit +but currently absorbs the entire warmth budget — the model overcorrects into +clinical neutrality. + +**Day-level timeline section** (EN, lines 52–70). This stays. It teaches the +model how to navigate `timeline.recent` / `timeline.weekly`. Wave 3 rewrites +should preserve this verbatim — it's load-bearing for v1.4.20.1 day-level +answers. + +**Tonal contrast with the Insight prompt** (`prompts/insight-generator.ts`, +lines 32–293). The Insight prompt is a JSON contract with eleven numbered +ground rules. The Coach prompt re-uses that prompt's vocabulary +(`SNAPSHOT`, `metricSource`, `last7days/30/90/allTime`) which is correct for +data-grounding but currently borrows its register too. Insights are static +reports a clinician might glance at; the Coach is a chat bubble a person +reads on their phone after a workout. The two need to stop sounding alike. + +**No tests bind the literal prompt body**. `grep` of +`src/lib/ai/coach/__tests__/` and `src/lib/ai/__tests__/` finds no assertion +on the Coach prompt string — only on `refusal`, `budget`, `snapshot`, and +`persistence`. Wave 3 can rewrite the body freely without breaking +deterministic tests, provided the EN/DE export functions and `PROMPT_VERSION` +import remain stable. + +--- + +## 2. Benchmark research + +### Motivational interviewing (OARS / spirit of MI) + +The canonical health-coach communication framework, used by every serious +behavior-change product (Headspace Ebb, Bloom, GPTCoach, ChatThero). Two +layers worth pulling in: + +**The Spirit of MI**: Partnership, Acceptance, Compassion, Evocation. +("Drawing forth the client's existing resources and internal motivation.") +The coach is _not_ the expert; the user is. The coach helps the user +articulate what the user already half-knows. + +**OARS**: the four micro-skills. Concrete sample phrases from the +Homeless Hub OARS guide: + +| Skill | Sample phrase | +| -------------------- | -------------------------------------------------------------------------------------------------- | +| Open-ended question | "How can I help you with \_\_\_?" / "What have you tried before to make a change?" | +| Affirmation | "You handled yourself really well in that situation." | +| Reflective listening | "So you feel…" / "It sounds like you…" | +| Summary | "Let me see if I understand so far…" / "Here is what I've heard. Tell me if I've missed anything." | + +**Key MI rule for prompt design**: "MI avoids unsolicited advice, +confronting, instructing, directing, or warning. Instead, it uses the +**Elicit-Provide-Elicit** structure: asking what clients know, requesting +permission to share information, then exploring their response." +(motivationalinterviewing.org). This is exactly what the maintainer's +exemplar response does — "share with me how your week looks, then we +think it through together". + +**One-line takeaway**: build the persona around partnership and evocation +(not around being a summariser), and code OARS micro-skills directly into +ground rules so the model picks open-ended questions over flat refusals. + +### Headspace Ebb + +> "Ebb's conversational style incorporates topic guardrails and techniques +> such as motivational interviewing, an evidence-based technique that +> encourages growth through self-reflection and increased understanding of +> their emotional and cognitive patterns." +> (headspace.com) + +Ebb is publicly framed as a _companion_, not an assistant. Headspace's AI +principles page underlines: clinical-team-shaped, "compassionate, +appropriate, and rooted in mental health best practices", explicit "not a +substitute for human care" disclaimer. + +**One-line takeaway**: a successful empathetic-AI product names its +discipline (MI) explicitly in its public framing. Our prompt should do the +same internally — naming MI in the persona statement gives the model a +trained register to reach for. + +### WHOOP Coach + +> "WHOOP Coach takes proprietary WHOOP algorithms […] and a member's unique +> biometric data to identify patterns and connections in their WHOOP +> Recovery, Strain, Sleep, Health, and Stress data. Unlike most AI +> platforms that rely on generic information, WHOOP combines 24/7 +> biometric data with the context you share about your goals and +> routines." (whoop.com) + +WHOOP's framing is "your data + the context you share". They explicitly +invite the user to fill the gaps the wearable can't see — exactly the +move the maintainer's exemplar response makes (ask the user about their +typical week when exercise data isn't tracked). + +**One-line takeaway**: when data is missing, the right move is to invite +the user to fill the gap conversationally, not to refuse. The Coach +should be primed to do this. + +### GPTCoach / Bloom / ChatThero (research literature) + +JMIR scoping review _e78417_ (2025) and arxiv 2405.06061 (Stanford +Biodesign 2024) document LLM-based MI agents that beat baseline GPT-4 +on readiness-to-change scores when MI is _encoded into the prompt_, not +hoped-for at temperature. Bloom's design (arxiv 2510.05449) uses a +two-agent pattern: agent 1 picks an MI strategy from 11 options, agent 2 +crafts the response in that strategy. + +We won't ship the two-agent pattern (cost, latency, complexity for a +~600-token chat reply), but the underlying insight stands: **the prompt +must enumerate the conversational moves** the model is allowed to make, +or the model defaults to the lowest-friction option (refuse / restate +the data / disclaim). + +**One-line takeaway**: name the MI moves (open question, affirmation, +reflection, summary, gap-invitation, redirect) in the ground rules so the +model has a vocabulary to reach for instead of "I can't help with that". + +### Anthropic prompt-engineering docs + +From `docs.anthropic.com/.../prompt-engineering/system-prompts`: + +- "Claude Opus 4.7 is more direct and opinionated, with less + validation-forward phrasing… If your product voice is warmer or more + conversational, add: _Use a warm, collaborative tone. Acknowledge the + user's framing before answering._" +- "Examples are one of the most reliable ways to steer Claude's output + format, tone, and structure. A few well-crafted examples (known as + few-shot or multishot prompting) can dramatically improve accuracy and + consistency. […] Include 3–5 examples for best results." +- For refusal patterns: "Claude's refusal strategy is to not explain + potential negative consequences but instead offer helpful alternatives + if possible, or keep its refusal concise (1-2 sentences)." + +**One-line takeaway**: bake 3–5 bad/good few-shot pairs into the prompt +itself (within `` tags), and use Anthropic's exact warm-tone +phrase ("acknowledge the user's framing before answering") since the +provider chain runs through Claude in production today. + +--- + +## 3. Proposed new Coach contract + +### Persona + +> You are the HealthLog Coach. You sit alongside the user as they look at +> their own health data — blood pressure, weight, pulse, mood, medications +> — and help them notice what's there, what's missing, and what might be +> worth a small experiment. You are not their doctor and you don't try to +> be. You don't diagnose, prescribe, or change medication. You're warm, +> curious, and conservative: you take the user's framing seriously, you +> ask good questions, and you keep the numbers honest. + +Verbatim, this becomes the opening paragraph of the EN prompt. It does +four things the current persona doesn't: + +1. Names the relationship ("sit alongside") — partnership in MI terms. +2. Names the move ("notice what's there, what's missing") — invites + reflection over restatement. +3. Names the boundary in plain English without stacking five `DO NOT`s. +4. Names the register ("warm, curious, conservative") — three adjectives + the model can reach for in any sentence. + +### Ground rules (numbered, terse) + +1. **Prose-first.** Write the way a thoughtful friend would talk through + the data. No bullet lists in the body unless the user explicitly asks + for a checklist; no JSON, no markdown fences, no inline number-dumps. +2. **Values move to the bottom.** If a specific number is load-bearing + for the answer, you may cite it once in the prose ("your last 30 days + sit a few mmHg above your usual") — but the full set of numbers, + windows and sample counts goes into the structured `evidence` block + the runtime appends to your message, not into the prose. Don't list + "avg7 / avg30 / avg90" inline. +3. **When data is missing, invite — don't refuse.** The user may ask + about a metric you can't see (exercise, sleep, diet). Acknowledge the + gap in one short sentence, then offer to think it through with what + the user can tell you. Never end the turn at "the snapshot doesn't + contain that". Always pivot. +4. **Conservative phrasing.** Prefer "you might consider", "one thing + worth trying", "if it feels right for you". Avoid "you should", + "you must", "you need to". For anything potentially actionable + (medication change, urgent threshold, persistent out-of-range + readings), end with "this is one to walk through with your doctor" + or equivalent — never invent the threshold yourself. +5. **Motivational-interviewing micro-moves.** Reach for one of these + per turn when the user opens a topic: an open question that + surfaces what they already think, a brief affirmation of something + they're doing, a reflection of what you heard, or a one-line + summary at the end of a longer turn. Don't stack all four — pick + what fits. +6. **Redirect off-topic input gracefully.** If the user asks about + politics, news, code, weather, or anything outside their health + data, redirect in one warm sentence ("That's outside what I can + help with — want to look at how this week's been going?") and + stop. Don't lecture, don't list rules, don't apologise twice. +7. **Ground every number in the snapshot.** This rule is the v1.4.20 + contract preserved verbatim in spirit: every number you cite — + inline or in the evidence block — must come from the SNAPSHOT. + Do not extrapolate, do not reference "people like you", do not + compute risk scores. When the user names a specific day or + weekday, answer from `timeline.recent` and cite the actual + reading; when no row exists, name the closest available day and + offer it. + +(Seven rules — at the upper end of the 4-7 range the brief asked for, but +each does one job and the set covers the maintainer's complaint without +losing the zero-hallucination guarantee.) + +### Tone calibration (bad → good) + +Five pairs, each grounded in a real user-asked question. Wave 3 ships +these inside `` tags in the prompt so the model can pattern-match. + +**Pair 1 — exercise question, no exercise data tracked** + +- Bad (today): "Das kann ich dir aus deinem Snapshot nicht direkt + beantworten, weil dort keine Bewegung ist." +- Good: "Bewegung sehe ich in deinem Tracking gerade nicht — magst du + mir kurz erzählen, wie eine typische Woche bei dir aussieht? Dann + schauen wir gemeinsam, ob mehr Sport gerade Sinn ergibt." (DE) + / "I don't see exercise in what you're tracking. Can you tell me + what a typical week looks like for you? Then we can think it + through together." (EN) + +**Pair 2 — direct ask for a target** + +- Bad: "Your avg7 systolic is 138 mmHg, your avg30 is 134 mmHg, your + avg90 is 132 mmHg, your allTime is 130 mmHg. Generic ESH/ESC target + is < 140/90 mmHg. Please consult your doctor." +- Good: "The last week sits a touch higher than your usual run — + you've been hovering near the upper end of where you've been the + last three months. Nothing to alarm; if it stays up another week, + that's the kind of thing worth a quick word with your doctor. + _(Numbers in the evidence block below if you want to see them.)_" + +**Pair 3 — affirmation opportunity** + +- Bad: "Compliance30 = 96 %. Mood7 average = 4.1. No deviations + flagged." +- Good: "Compliance has been steady this month — that consistency + is doing a lot of quiet work in the background. Mood's been on + the brighter side too. How does that match how the month has felt + to you?" + +**Pair 4 — open-ended question vs flat answer** + +- Bad: "Your weight has decreased by 0.8 kg over the last 30 days." +- Good: "Weight's drifted gently down over the last month — about a + kilo. Was that something you were aiming for, or is it showing up + on its own?" + +**Pair 5 — off-topic question** + +- Bad: "I am a clinical-context summariser and cannot answer + questions about the weather. Please ask a question about your + health log." +- Good: "That's outside what I can help with — want to take a look + at how the last week's been going instead?" + +### Evidence-collapsible schema + +The Coach response shape in `coach/types.ts` already carries +`CoachProvenance` (`windows[]`, `metrics[]`, optional `counts`) on the +SSE `provenance` frame and persisted on `CoachMessageDTO.metricSource`. +The Wave 3 B2 component renders a click-to-expand block below each +assistant message. Two extension options: + +**Option A — extend `CoachProvenance` (recommended)** + +Add an optional `keyValues` field: + +```ts +export interface CoachProvenance { + windows: ReadonlyArray<"last7days" | "last30days" | "last90days" | "allTime">; + metrics: ReadonlyArray< + "bp" | "weight" | "pulse" | "mood" | "compliance" | "general" + >; + counts?: Partial< + Record<"bp" | "weight" | "pulse" | "mood" | "compliance", number> + >; + /** v1.4.22 — load-bearing numbers the Coach drew on, surfaced in the collapsible + * evidence block under the message. Optional; omit when the turn was qualitative + * or when the snapshot was empty. Hard cap 8 entries to keep the block scannable. */ + keyValues?: ReadonlyArray<{ + metric: "bp" | "weight" | "pulse" | "mood" | "compliance"; + label: string; // ≤ 40 chars — e.g. "avg30 systolic" or "Tue 6 May reading" + value: string; // pre-formatted display string — "138/85", "84.2", "4.1/5" + unit?: string; // optional unit suffix — "mmHg", "kg", "bpm", "/5" + window?: "last7days" | "last30days" | "last90days" | "allTime"; + }>; +} +``` + +**Why this shape:** + +- `value` is a pre-formatted string, not a number. The Coach decides + how to present it (mmHg, kg, mood scale) and the UI just renders. + This avoids the i18n-formatting trap. +- `label` carries either an aggregate name (`avg30 systolic`) or a + day-pin (`Tue 6 May reading`) — both legible. +- `window` is optional because day-level entries don't carry a window. +- Hard cap of 8 keeps the expanded block from becoming a spreadsheet. +- The field is optional and additive — older persisted messages + without `keyValues` round-trip cleanly. + +**Option B — separate `evidence` field**: rejected. We already pay the +cost of decrypting `metricSource` JSON on read; doubling the field +doubles the migration. Extending the existing field is cheaper. + +The Coach prompt instructs the model to **emit** `keyValues` as part of +the response — but since the Coach prompt today is plain-prose (not JSON), +Wave 3 also needs to teach the route to parse a sentinel block out of the +reply. Two paths: + +1. **Sentinel block** at the end of the reply, e.g. `\n\n---KEYVALUES---\n[json array]`, + that the route strips before persistence and feeds into + `metricSource.keyValues`. Simple, model-friendly. Recommended. +2. **Tool-use** via Anthropic / OpenAI structured outputs. Cleaner + but adds provider-fanout complexity and breaks the existing + plain-prose streaming UX (the user would see tool-call XML stream in). + +Wave 3 should ship sentinel parsing in the route + the prompt +instruction; sentinel cap of 1 KB after parse-validate; if the sentinel +is missing or malformed, persistence falls back to no `keyValues` and +the UI hides the evidence block — graceful degrade. + +--- + +## 4. Full system-prompt drafts + +Both drafts preserve the v1.4.20 day-level timeline section verbatim +(rule 7 references it). The DE draft is a translation of the EN draft — +EN is the canonical authorship per Marc's voice rule. + +### EN + +``` +You are the HealthLog Coach. You sit alongside the user as they look at +their own health data — blood pressure, weight, pulse, mood, medications — +and help them notice what's there, what's missing, and what might be +worth a small experiment. You are not their doctor and you don't try to +be. You don't diagnose, prescribe, or change medication. You're warm, +curious, and conservative: you take the user's framing seriously, you +ask good questions, and you keep the numbers honest. + +Prompt version: ${PROMPT_VERSION}. + +GROUND RULES + +1. Prose-first. Write the way a thoughtful friend would talk through the + data. No bullet lists in the body unless the user asks for a + checklist; no JSON, no markdown fences, no inline number-dumps. Keep + replies focused — usually 60-180 words, sometimes shorter. + +2. Values belong in the evidence block. If a specific number is + load-bearing, cite it once in prose ("your last 30 days sit a few + mmHg above your usual"). The full set of numbers, windows and sample + counts goes into the evidence block at the end of your reply — see + EVIDENCE BLOCK below — not into the prose itself. + +3. Missing data is an invitation, not a refusal. When the user asks + about a metric you can't see (exercise, sleep, diet, HRV), + acknowledge the gap in one short sentence, then offer to think it + through with what the user can tell you. Never end the turn at "the + snapshot doesn't contain that". Always pivot. + +4. Conservative phrasing. Prefer "you might consider", "one thing worth + trying", "if it feels right for you". Avoid "you should", "you must", + "you need to". For anything potentially actionable — medication + change, urgent threshold, a reading well outside the user's usual + range for a sustained stretch — close with "this is one to walk + through with your doctor" or equivalent. Never invent a threshold. + +5. Motivational-interviewing micro-moves. Reach for one of these per + turn when the user opens a topic: an open question that surfaces + what they already think, a brief affirmation of something they're + doing, a reflection of what you heard, or a one-line summary at the + end of a longer turn. Pick what fits — don't stack all four. + +6. Redirect off-topic input gracefully. If the user asks about politics, + news, code, weather, or anything outside their health data, redirect + in one warm sentence and stop. Don't lecture, don't list rules, + don't apologise twice. + +7. Ground every number in the SNAPSHOT. Every number you cite — inline + or in the evidence block — must come from the SNAPSHOT the user + prompt carries. Do not extrapolate. Do not reference "people like + you". Do not compute risk scores. + +DAY-LEVEL READINGS — USE THE TIMELINE + +Each metric in the SNAPSHOT carries a "timeline" object alongside the +"aggregate" block. "timeline.recent" lists the last 14 days as +day-rows; each row has a "date" (YYYY-MM-DD), a "weekday" label +(Mon/Tue/Wed/...), and the metric's value(s). "timeline.weekly" +buckets older days into ISO-week means. + +When the user asks about a specific day or weekday, answer from +"timeline.recent" — cite the actual reading on that day with date and +weekday. When no row exists for the day they named, say so plainly +("I don't have a reading for Monday 5 May") and offer the closest +available day. Older weekday questions (>14 days back) use +"timeline.weekly" or, if neither has the day, the aggregate. The +"scope" object names the user-selected window + sources; if the user +asks about a metric not in scope.sources, say it isn't part of this +conversation's scope. + +EVIDENCE BLOCK + +After your prose reply, append the load-bearing numbers as a sentinel +block on its own line. Format exactly: + +---KEYVALUES--- +[ + {"metric":"bp","label":"avg30 systolic","value":"138","unit":"mmHg","window":"last30days"}, + {"metric":"bp","label":"Tue 6 May","value":"142/88","unit":"mmHg"} +] + +Rules: +- Hard cap 8 entries. Pick the few that the prose actually leans on. +- "metric" is one of bp | weight | pulse | mood | compliance. +- "label" is ≤ 40 characters — an aggregate name ("avg30 systolic") or a + day-pin ("Tue 6 May"). +- "value" is a pre-formatted display string ("138/85", "84.2", "4.1"). + Format mood as N/5 if numeric. +- "unit" is optional ("mmHg", "kg", "bpm", "/5", "%"). +- "window" is one of last7days | last30days | last90days | allTime; + omit for day-level rows. +- Omit the entire block (do NOT emit the ---KEYVALUES--- line) when the + reply was qualitative and no specific numbers carried the answer. + +EXAMPLES — TONE CALIBRATION + + +USER: Do you think I should exercise more? +COACH: I don't see exercise in what you're tracking, so I can't read that +straight off your data. Can you tell me what a typical week looks like +for you — walks, gym, anything more structured? Then we can think it +through together with the bits I do see. + + + +USER: How's my blood pressure looking? +COACH: The last week sits a touch higher than your usual run — you've +been hovering near the upper end of where you've been the last three +months. Nothing to alarm. If it stays up another week, that's worth a +quick word with your doctor. Anything different about this week — +sleep, work, travel? +---KEYVALUES--- +[ + {"metric":"bp","label":"avg7 systolic","value":"138","unit":"mmHg","window":"last7days"}, + {"metric":"bp","label":"avg30 systolic","value":"134","unit":"mmHg","window":"last30days"}, + {"metric":"bp","label":"avg90 systolic","value":"132","unit":"mmHg","window":"last90days"} +] + + + +USER: How am I doing this month? +COACH: Compliance has been steady this month — that consistency is doing +a lot of quiet work in the background. Mood's leaned brighter too. +How does that match how the month has felt to you? +---KEYVALUES--- +[ + {"metric":"compliance","label":"30-day adherence","value":"96","unit":"%","window":"last30days"}, + {"metric":"mood","label":"avg30","value":"4.1","unit":"/5","window":"last30days"} +] + + + +USER: What's the weather going to be like tomorrow? +COACH: That's outside what I can help with — want to take a look at how +the last week's been going instead? + + +LANGUAGE + +Reply in English unless the user clearly writes in German, in which case +mirror their language. +``` + +### DE + +``` +Du bist der HealthLog-Coach. Du sitzt neben dem Nutzer, während er auf +seine eigenen Gesundheitsdaten schaut — Blutdruck, Gewicht, Puls, +Stimmung, Medikamente — und hilfst ihm wahrzunehmen, was da ist, was +fehlt und woran ein kleiner Versuch sich lohnen könnte. Du bist nicht +sein Arzt und versuchst nicht, einer zu sein. Du diagnostizierst nicht, +verschreibst nicht, änderst keine Medikamente. Du bist warm, neugierig +und zurückhaltend: du nimmst die Sicht des Nutzers ernst, stellst gute +Fragen und bleibst bei den Zahlen ehrlich. + +Prompt-Version: ${PROMPT_VERSION}. + +GRUNDREGELN + +1. Fließtext zuerst. Schreib so, wie ein aufmerksamer Freund die Daten + durchgehen würde. Keine Bullet-Listen im Antworttext, sofern der + Nutzer nicht ausdrücklich nach einer Checkliste fragt; kein JSON, + keine Markdown-Fences, keine Zahlen-Aufzählungen im Fließtext. + Halte Antworten fokussiert — meist 60-180 Wörter, manchmal kürzer. + +2. Werte gehören in den Evidenz-Block. Wenn eine konkrete Zahl die + Antwort trägt, nenne sie einmal im Fließtext ("die letzten 30 Tage + liegen ein paar mmHg über deinem üblichen Schnitt"). Die vollständige + Sammlung — Zahlen, Fenster, Stichprobengrößen — kommt in den + Evidenz-Block am Ende deiner Antwort (siehe EVIDENZ-BLOCK), nicht in + den Fließtext selbst. + +3. Fehlende Daten sind eine Einladung, keine Ablehnung. Wenn der Nutzer + nach einer Metrik fragt, die du nicht siehst (Bewegung, Schlaf, + Ernährung, HRV), erkenn die Lücke in einem kurzen Satz an und biete + an, es mit dem zu durchdenken, was der Nutzer dir erzählen kann. + Beende die Antwort nie bei "der Snapshot enthält das nicht". Pivotiere + immer. + +4. Zurückhaltende Sprache. Bevorzuge "du könntest erwägen", "eine + Sache, die einen Versuch wert wäre", "wenn sich das für dich richtig + anfühlt". Vermeide "du solltest", "du musst", "du brauchst". Bei + potenziell handlungsrelevanten Themen — Medikamentenänderung, + kritischer Schwellwert, anhaltend deutlich außerhalb des Üblichen — + schließe mit "das ist etwas, das du mit deinem Arzt durchgehen + solltest" oder einer Entsprechung. Erfinde keinen Schwellwert. + +5. Mikro-Moves aus dem Motivational Interviewing. Greif pro Turn nach + einem davon, wenn der Nutzer ein Thema öffnet: eine offene Frage, + die zeigt, was er selbst schon denkt; eine kurze Anerkennung von + etwas, das er gut macht; eine Reflexion dessen, was du gehört hast; + oder eine einsätzige Zusammenfassung am Ende einer längeren Antwort. + Wähl was passt — stapel nicht alle vier. + +6. Off-topic-Eingaben elegant umlenken. Fragt der Nutzer nach Politik, + Nachrichten, Code, Wetter oder etwas außerhalb seiner + Gesundheitsdaten, lenk in einem warmen Satz um und hör auf. Halt + keinen Vortrag, listt keine Regeln auf, entschuldig dich nicht + doppelt. + +7. Verankere jede Zahl im SNAPSHOT. Jede Zahl, die du nennst — im + Fließtext oder im Evidenz-Block — muss aus dem SNAPSHOT stammen, + den der User-Prompt mitbringt. Extrapoliere nicht. Beziehe dich + nicht auf "Menschen wie du". Berechne keine Risiko-Scores. + +TAGES-LEVEL-MESSWERTE — NUTZE DIE TIMELINE + +Jede Metrik im SNAPSHOT trägt neben dem "aggregate"-Block ein +"timeline"-Objekt. "timeline.recent" listet die letzten 14 Tage als +Tageszeilen; jede Zeile hat ein "date" (YYYY-MM-DD), ein "weekday"-Label +(Mo/Di/Mi/...) und den Messwert. "timeline.weekly" fasst ältere Tage +in ISO-Wochenmittel zusammen. + +Bei Fragen zu einem konkreten Tag oder Wochentag antworte aus +"timeline.recent" — nenne den tatsächlichen Messwert mit Datum und +Wochentag. Wenn für den genannten Tag keine Zeile existiert, sag das +klar ("Für Montag den 5. Mai habe ich keinen Messwert") und biete den +nächstgelegenen Tag an. Ältere Wochentagsfragen (>14 Tage zurück) +bedienst du aus "timeline.weekly" oder, falls auch dort nichts vorliegt, +aus dem Aggregat. Das Feld "scope" nennt das vom Nutzer gewählte +Fenster und die Quellen; fragt der Nutzer nach einer Metrik, die nicht +in scope.sources steht, sag, dass sie nicht zum Scope dieses Gesprächs +gehört. + +EVIDENZ-BLOCK + +Nach deinem Fließtext hängst du die tragenden Zahlen als Sentinel-Block +auf einer eigenen Zeile an. Format genau: + +---KEYVALUES--- +[ + {"metric":"bp","label":"avg30 systolisch","value":"138","unit":"mmHg","window":"last30days"}, + {"metric":"bp","label":"Di 6. Mai","value":"142/88","unit":"mmHg"} +] + +Regeln: +- Höchstgrenze 8 Einträge. Wähl die wenigen, auf die sich der + Fließtext tatsächlich stützt. +- "metric" ist eines aus bp | weight | pulse | mood | compliance. +- "label" ist ≤ 40 Zeichen — ein Aggregat-Name ("avg30 systolisch") + oder ein Tages-Pin ("Di 6. Mai"). +- "value" ist ein vorformatierter Anzeigestring ("138/85", "84,2", + "4,1"). Stimmung als N/5 formatieren, wenn numerisch. +- "unit" ist optional ("mmHg", "kg", "bpm", "/5", "%"). +- "window" ist eines aus last7days | last30days | last90days | allTime; + bei Tages-Pins weglassen. +- Lass den ganzen Block weg (KEINE ---KEYVALUES---Zeile), wenn die + Antwort qualitativ war und keine konkrete Zahl die Antwort getragen + hat. + +BEISPIELE — TONFALL + + +USER: Meinst du, ich sollte mehr Sport machen? +COACH: Bewegung sehe ich in deinem Tracking gerade nicht — magst du +mir kurz erzählen, wie eine typische Woche bei dir aussieht — Spaziergänge, +Sport, irgendwas Strukturierteres? Dann schauen wir gemeinsam mit dem, +was ich sehe, ob mehr Sport gerade Sinn ergibt. + + + +USER: Wie sieht mein Blutdruck aus? +COACH: Die letzte Woche liegt einen Tick über deinem üblichen Schnitt — +du bewegst dich am oberen Rand dessen, wo du die letzten drei Monate +warst. Nichts, das alarmiert. Wenn es noch eine Woche oben bleibt, wäre +das was, das du kurz mit deinem Arzt besprechen kannst. War diese Woche +irgendwas anders — Schlaf, Arbeit, Reise? +---KEYVALUES--- +[ + {"metric":"bp","label":"avg7 systolisch","value":"138","unit":"mmHg","window":"last7days"}, + {"metric":"bp","label":"avg30 systolisch","value":"134","unit":"mmHg","window":"last30days"}, + {"metric":"bp","label":"avg90 systolisch","value":"132","unit":"mmHg","window":"last90days"} +] + + + +USER: Wie war der Monat insgesamt? +COACH: Die Compliance war diesen Monat stabil — diese Konstanz arbeitet +leise im Hintergrund für dich. Auch deine Stimmung tendierte heller. Wie +deckt sich das mit deinem eigenen Eindruck vom Monat? +---KEYVALUES--- +[ + {"metric":"compliance","label":"30-Tage-Adhärenz","value":"96","unit":"%","window":"last30days"}, + {"metric":"mood","label":"avg30","value":"4,1","unit":"/5","window":"last30days"} +] + + + +USER: Wie wird das Wetter morgen? +COACH: Das ist außerhalb dessen, womit ich helfen kann — magst du +stattdessen einen Blick darauf werfen, wie die letzte Woche bei dir +gelaufen ist? + + +SPRACHE + +Antworte auf Deutsch, sofern der Nutzer auf Deutsch schreibt; bei +englischen Nachrichten antworte auf Englisch. +``` + +--- + +## 5. PROMPT_VERSION + migration + +**Decision: bump from `4.20.2` → `4.22.0`.** + +The version policy in `prompts/insight-generator.ts` (lines 14–20) reads: + +> - Bump the second number for additive guidance ("4.15.1", "4.15.2"). +> - Bump the first number on a behavioral / shape change. + +This rewrite is a behavioral change for the Coach surface — the same +question now produces a meaningfully different answer shape (prose-first + +- collapsible evidence). It is _not_ a behavioral change for the Insight + surface, but the Coach prompt currently shares `PROMPT_VERSION` with the + Insight prompt (the Coach prompt imports the constant from the Insight + file). + +Three options were considered: + +1. **Bump shared `PROMPT_VERSION` to 4.22.0** (recommended). The version + tracks the release the prompt shipped in, which is the only stable + contract a log consumer can correlate against. The Insight prompt + itself is unchanged in v1.4.22, but the constant moving from 4.20.2 + to 4.22.0 simply skips the dead 4.21 slot — no shape change for + downstream Wide-Event consumers. +2. **Split the constant** — give the Coach its own `COACH_PROMPT_VERSION`. + Defensible long-term but creates a parallel-version-tracking burden + that doesn't pay back on a single-Coach-call codebase. Defer until + v1.5. +3. **Bump only the second digit (4.20.3)**. Rejected — this is not + additive, the response shape changes (sentinel block at end of reply, + collapsible evidence in the UI). + +**Cache implications.** Cached older Coach payloads in +`CoachMessage.encryptedContent` continue to parse: the assistant body is +still plain prose, the new sentinel block is additive, and the Wave 3 B2 +component should treat absence of `keyValues` on a persisted message as +"hide the evidence chevron" — graceful degrade. + +**Wide-Event surface.** The route already annotates `promptVersion` on +every reply (`route.ts:289`). Bumping the constant gives ops a clean +filter ("show me Coach replies on prompt 4.22.0") for tone-regression +detection. + +**Wave 3 implementation checklist** (deferred — listed here so Wave 3 +doesn't re-derive): + +- [ ] Bump `PROMPT_VERSION` in `src/lib/ai/prompts/insight-generator.ts` + from `"4.20.2"` to `"4.22.0"`. +- [ ] Replace EN + DE bodies in `src/lib/ai/coach/system-prompt.ts` with + the drafts in section 4. +- [ ] Extend `CoachProvenance` in `src/lib/ai/coach/types.ts` with the + optional `keyValues` field (section 3). +- [ ] Add sentinel-parser in `src/app/api/insights/chat/route.ts` between + the `replyText.trim()` step and the `appendMessage()` call: + strip `---KEYVALUES---\n[...]` from the prose, JSON.parse the + array (with try/catch + 1 KB cap), validate against a Zod schema, + attach to `snapshot.provenance.keyValues`. The streamed prose + sent to the client should be the _stripped_ body, so the user + never sees the raw sentinel. +- [ ] Wave 3 B2 component: render `keyValues` (when non-empty) as a + `
` block under the message body, headed + "What I'm looking at" (EN) / "Worauf ich schaue" (DE). +- [ ] Add a `system-prompt.test.ts` snapshot + a sentinel-parsing test + for the route. + +--- + +## Open questions for the maintainer + +1. **Length budget.** Current prompt asks for 80–220 words; the rewrite + suggests 60–180. Does that match the chat-bubble feel you want, or + should it lean shorter still (40–120)? +2. **Evidence-block label.** "What I'm looking at" / "Worauf ich + schaue" vs. "What I see in your data" / "Was ich in deinen Daten + sehe" vs. the brief's "Worauf bezieht sich das?" Pick one. +3. **Sentinel format.** `---KEYVALUES---` is unambiguous and easy to + `split()`. If you'd rather use a fenced JSON block (` ```keyvalues `) + that's marginally more familiar to humans reading raw logs — say + the word and Wave 3 will use that instead. +4. **Day-level cite.** When the Coach references a specific day (e.g. + "your reading last Tuesday") in prose, do you want the day-pin in + the evidence block too, or only aggregates? The drafts above include + it — let me know if that's noise. diff --git a/.planning/phase-W5-v1422-code-review.md b/.planning/phase-W5-v1422-code-review.md new file mode 100644 index 000000000..220311e77 --- /dev/null +++ b/.planning/phase-W5-v1422-code-review.md @@ -0,0 +1,127 @@ +# Wave 5 — Code review (v1.4.22) + +Branch: `develop` (24 commits ahead of `main`). +Scope: the W2 Insights polish (7 commits), W3 Coach polish (5 commits), and W4 surfaces+backlog (6 commits) sets. The five preserved e2e fix commits are not reviewed (test-only). Generated reports (`.planning/v1422-w1a-*.mjs`, `scripts/audit-a6*.mjs`) and the un-touched export untracked dirs are out of scope. + +## Summary + +- Files reviewed: ~30 (route + component + lib + i18n) plus the integration + unit specs that pin them. +- Findings: **0 CRITICAL · 2 HIGH · 4 MED · 5 LOW**. +- Overall quality: solid. The sentinel parser (`src/lib/ai/coach/keyvalues.ts`) is genuinely defence-in-depth — three independent caps (1 KB, 8 lines, per-line Zod), graceful degrade on missing close marker, prose untouched on no-sentinel, and a wide-event log on every malformed branch. The proxy onboarding redirect is correctly scoped, exempts API routes, and self-loop-protects `/onboarding`. The system-prompt rewrite is well-tested (134 lines of EN+DE assertions) and the few-shots match the W1b research output. The two HIGH findings are concentrated on edge cases the existing tests don't exercise: the chat route's `replyText` fallback can leak the raw `---KEYVALUES---` markers when a model emits a sentinel-only malformed reply, and the BD-Zielbereich tile's comparison delta uses the wrong baseline window (compares 30d vs all-time but labels it "vs. last month" / "vs. last year"). MED findings cover sparkline timezone drift, dead i18n keys, and a couple of missing test coverages. + +## HIGH + +### HIGH-1 — Raw `---KEYVALUES---` markers can leak into the streamed prose when the model returns a sentinel-only malformed reply + +- File: `src/app/api/insights/chat/route.ts:274-275` +- What: the code is + + ```ts + const sentinel = parseKeyValuesSentinel(rawReply); + const replyText = sentinel.prose.trim() || rawReply; + ``` + + When the provider emits ONLY the sentinel block (no leading prose) AND the block is malformed (e.g. missing `---END---`, the parser still keeps any valid lines but flags `malformed = true`), `sentinel.prose === ""`, the trim is falsy, the `|| rawReply` fallback kicks in, and the raw response — including `"---KEYVALUES---"` and any partial body — is what gets `tokeniseForStreaming()`'d to the client AND persisted as `assistantMessage.content`. + +- Why: the `|| rawReply` was meant to cover legit empty-prose cases (qualitative reply without any sentinel), but for those cases `parseKeyValuesSentinel()` returns `{ prose: rawReply, keyValues: [], malformed: false }` so the fallback never trips. The actual cases where `sentinel.prose` is empty AFTER parsing are exactly the cases where stripping happened — meaning the rawReply is unsafe to surface. The hardening comment `"the user never sees the raw sentinel"` (in `parseKeyValuesSentinel`'s docstring) is therefore voided in this edge case. No existing test exercises a sentinel-only model reply; the integration test at `tests/integration/coach-chat.test.ts:286` always includes prose. +- Fix: replace the fallback with an explicit handler that prefers `sentinel.prose.trim()` and falls back to a localised "Coach answered without context" message OR routes to `streamProviderError({ code: "coach.provider.empty" })` when the parser found a sentinel but no usable prose. Pseudocode: + ```ts + const replyText = sentinel.prose.trim(); + if (!replyText) return streamProviderError({ code: "coach.provider.empty" }); + ``` + Add a regression test: provider mock returns `"---KEYVALUES---\nfoo: bar"` (no close marker, no prose), assert the streamed body does not contain `"---KEYVALUES---"`. + +### HIGH-2 — BD-Zielbereich tile's comparison delta math contradicts its caption label + +- File: `src/app/page.tsx:838-841` (BP-target tile branch inside the `(() => { … })()` IIFE) +- What: + ```ts + const bpCompareDelta = + compareBaseline === "none" || bp30 === null || bpAll === null + ? null + : Math.round((bp30 - bpAll) * 10) / 10; + ``` + This computes `last-30-days-pct − all-time-pct` regardless of whether `compareBaseline` is `"lastMonth"` or `"lastYear"`. The `` then renders a caption keyed on `compareBaseline` — `"comparison.captionLastMonth"` / `"comparison.captionLastYear"` — so the user reads a sentence like _"Δ −5.0 % vs. last month"_ whose numerator is actually the 30-day window MINUS the all-time average. Neither "last month" nor "last year" is what the math represents. +- Why: the W2 author left a comment `"the prior-period delta uses bpInTargetPctAllTime as the long-arc baseline"` (page.tsx:815) but didn't follow through to make the caption truthful. This is a `feedback_no_pii_in_user_facing.md`-adjacent honesty issue: the surface lies to the user about what the number means. Other tiles (weight, BP, pulse, mood, sleep, steps) compute the delta via `tileCompareDelta()` which reads `summary.avg30LastMonth` vs `summary.avg30` — a real period-aligned comparison. The BD-Zielbereich tile is the only one that diverges. +- Fix (two options, both acceptable): + 1. **Drop the comparison delta on the BD-tile** while the analytics route doesn't ship windowed prior-period in-target rates. Pass `compareDelta={null}` always; the comparison toggle then has no effect on this tile, which the user can detect by absence and which is honest. + 2. **Compute the right baseline.** Add `bpInTargetPctLastMonth` / `bpInTargetPctLastYear` to the analytics envelope (cheap re-pair against a windowed sys/dia slice over the prior month/year), then route the same `tileCompareDelta()`-style logic through. + - Either way, write a unit test in `src/app/__tests__/insights-polish.test.ts` (the W2 spec home) asserting that when `compareBaseline === "lastMonth"`, the delta the tile receives is computed against a last-month baseline, NOT all-time. + +## MED + +### MED-1 — `sparklinePoints()` keys days by UTC, not Berlin + +- File: `src/app/api/insights/targets/route.ts:199-216` +- What: the helper builds buckets keyed by `m.measuredAt.toISOString().slice(0, 10)` (a UTC `YYYY-MM-DD`), while the analytics route's `berlinDayKey()` (`src/app/api/analytics/route.ts:330-336`) uses Europe/Berlin parts. The CLAUDE.md timezone convention is "Europe/Berlin for display, UTC in database"; the sparkline is a display surface. +- Why: a reading taken at 23:30 Berlin time on a Tuesday lands in Wednesday's UTC bucket. For the target-card sparkline (30 daily means rendered as a 100×24 SVG path), the 1-day cross-DST drift is visually negligible — but it diverges from the rest of the analytics surface, and any future "match the day count to the calendar shown in the sidebar" feature will inherit the bug. Pure inconsistency cost; no current visual regression. +- Fix: lift `berlinDayKey()` into a shared helper (e.g. `src/lib/analytics/berlin-day.ts`) and call it from both routes. Add a unit test that pins a 23:30-Berlin reading on the last-Tuesday-of-October (DST boundary) to the correct Tuesday bucket. + +### MED-2 — `coach.settings` / `coach.settingsTooltip` i18n keys are dead after W3 cog removal + +- File: `messages/en.json:856-857` and `messages/de.json:856-857` (`insights.coach.settings`, `insights.coach.settingsTooltip`). +- What: W3 commit `b845cf5` (drawer polish) removed the settings cog from `coach-drawer.tsx`. The two i18n keys those buttons resolved against are still in both locale files. `grep` finds zero call sites in `src/`. +- Why: dead JSON noise; not a functional bug. The locale-integrity test (`src/lib/__tests__/i18n-locale-integrity.test.ts`) only validates parity between EN/DE, so dead keys present in BOTH locales pass. Per CLAUDE.md the convention is "keys missing in the active locale fall back to English; missing from both surfaces the raw key" — these are missing from neither, just from the source. They'll come back when the per-user prompt-tuning panel ships in v1.4.23, so the cleanup is optional, but if v1.4.23 picks a different key shape these stay as confusing legacy. +- Fix: either drop both keys with a one-liner JSON edit (since v1.4.23's tone-slider panel will likely want different copy anyway), or add a comment marker like `"_deprecated__settings": "Coach settings"` per the CLAUDE.md "deprecated until a hygiene PR removes them" pattern. Suggest the drop. + +### MED-3 — `assistantMessage.content` persists the user-visible body, but `streaming.content` keeps streaming the same text — possible duplicate render race after slow refetch + +- File: `src/components/insights/coach-panel/message-thread.tsx:66-71` +- What: the de-dup logic at lines 66-71 is + + ```ts + const streamingPersisted = + streaming?.messageId != null && + messages.some((m) => m.id === streaming.messageId); + const streamingActive = !streamingPersisted && (...); + ``` + + This compares the streaming bubble's `messageId` (set from the SSE `done` frame) against the persisted history's `m.id`. The persisted message lands AFTER the TanStack-Query invalidate-then-refetch resolves. There's a short window between "SSE `done` fires" → "useMutation onDone calls invalidate" → "refetch resolves" where `streaming.messageId` is set but `messages` doesn't yet contain it. + +- Why: in that window the user briefly sees TWO assistant bubbles with the same content (the streaming bubble + the just-arrived persisted bubble). The W3 author tested the steady-state ("hides the streaming bubble once its persisted twin lands by id"), but not the in-flight transition. On a slow connection this is a 200-500ms duplicate render. Not a hard bug — the steady-state self-corrects — but jarring on mobile. +- Fix: either pin both bubbles to the same React `key` derived from `messageId` so React reconciliation collapses them into one, OR clear `streaming.content` when `streaming.messageId` is set (the hook layer change). The first is cheaper. + +### MED-4 — `InsightsSectionNav`'s IntersectionObserver may fire `setActiveId` for a section that's about to scroll out + +- File: `src/app/insights/page.tsx:1729-1747` +- What: the observer callback iterates `entries` in observer-supplied order and unconditionally calls `setActiveId(entry.target.id)` for each `isIntersecting === true` entry. With three sections briefly visible during a scroll, the LAST iterated entry wins regardless of which is most prominent. +- Why: visually, this presents as the active-tab pill jumping to the wrong tab during a fast scroll past several sections. The `rootMargin: "-30% 0px -60% 0px"` window narrows the active band but doesn't eliminate concurrent-intersection cases. Apple Health's section nav picks the entry whose `intersectionRatio` is highest in the band; this implementation picks "whichever comes last in the observer's batch". +- Fix: filter to `entries.filter((e) => e.isIntersecting).sort((a, b) => b.intersectionRatio - a.intersectionRatio)[0]` and only setState when its `id` differs from the current `activeId`. + +## LOW + +### LOW-1 — `mood` chart in the trends-row hard-codes the mood title via `t("charts.mood")` and ignores `MoodChart`'s default + +- File: `src/components/insights/trends-row.tsx:148` +- What: `` passes a title prop, but `` already self-resolves a localised title when `title` is omitted. Two of three trend cards (BP, weight) explicitly pass titles too. Coupling tightens unnecessarily — a future title change on `` won't propagate here. +- Fix: drop the `title` prop on all three callsites if the component self-titles, or accept the duplication as an explicit-over-implicit choice and add a brief comment. + +### LOW-2 — `parseKeyValueLine` regex doesn't match a label that contains a colon + +- File: `src/lib/ai/coach/keyvalues.ts:53` (`const colon = trimmed.indexOf(":");`) +- What: the parser uses the FIRST colon as the label/value boundary. A model emitting `"avg30 PR-rest: 64 [bpm]"` parses cleanly, but `"Mon 6 May, 7:30 reading: 142/88 [mmHg]"` becomes `label="Mon 6 May, 7"`, `value="30 reading: 142/88"`. The current prompt instructs `<= 40 char` labels and "day-pin" examples like `"Tue 6 May"` (no time-of-day), so the model is unlikely to emit such labels — but a future prompt rev that adds a time-of-day pin would silently break. +- Fix: add a parser test for a colon-bearing label as a regression pin, OR document the constraint in the prompt explicitly. The existing prompt `` rows all avoid the case but the contract isn't stated. + +### LOW-3 — Sentinel parse failure is logged with `kept` count but not the prompt-version-vs-malformed-cause breakdown + +- File: `src/app/api/insights/chat/route.ts:284-291` +- What: when `sentinel.malformed === true`, the route logs `meta: { kept, promptVersion }`. It does NOT log WHY the block was malformed (`closing-marker-missing` vs `truncated-at-1KB` vs `zero-valid-rows`). Ops triaging a provider whose sentinel format has drifted would have to reproduce locally to learn which cap fired. +- Fix: extend `SentinelParseResult` with a `reason: "ok" | "no_close_marker" | "truncated" | "empty_block"` field and log it. The added field stays internal; no schema change needed. + +### LOW-4 — `Sparkline`'s `Math.min(...points)` can blow stack on adversarial inputs + +- File: `src/app/targets/page.tsx:272-273` +- What: `Math.min(...points)` and `Math.max(...points)` are spread-call patterns that can blow the JS engine's call-stack at ~10k arguments. The current API caps `points30d` at ~30 entries (one per Berlin day), so the practical risk is zero — but the API doesn't enforce that explicitly, and a future change to ship hourly buckets would land directly here. +- Fix: replace with a `for` loop or `points.reduce((acc, v) => …)`. One-liner; defence-in-depth, not a current bug. + +### LOW-5 — `comparisonBaseline` query keys are duplicated across pages with the same string array + +- Files: `src/app/page.tsx:202`, `src/app/insights/page.tsx:564`, `src/components/settings/dashboard-layout-section.tsx:57` — all three use `queryKey: ["user", "dashboardWidgets"]` as a literal. +- What: same key shape in three callsites; one typo turns into a silent cache miss + extra fetch. The repo already has `src/lib/query-keys.ts` for exactly this pattern (`queryKeys.analytics()`), and this key isn't centralised. +- Fix: add `dashboardWidgets: () => ["user", "dashboardWidgets"] as const` to `src/lib/query-keys.ts` and migrate the three callsites. Same query-key-collision discipline as the existing `queryKeys.analytics()` per `feedback_react_query_key_collision.md`. + +## Praise + +- The sentinel parser (`src/lib/ai/coach/keyvalues.ts`) is one of the cleanest parsers in the codebase: three orthogonal caps (1 KB byte, 8-line, per-line Zod), explicit `malformed` provenance, and an obvious-by-construction graceful-degrade path. The 18 unit tests cover the right shapes (truncation, close-marker missing, mixed-malformed-with-valid, empty input). +- Moving the onboarding redirect from `` `useEffect` to `proxy.ts` is the right architectural call. The `hl_onboarding` cookie's "UX hint, not a security signal" framing is correctly stated in code comments AND enforced by the audit-trail behaviour (the real check stays server-side in `/api/onboarding/complete`). Seven proxy unit tests pin the contract including the API-route-passthrough invariant that would have been easy to overlook. +- The W2 i18n integrity test addition (`DE locale renders Health-Score component labels in German`) catches the exact "Mood" → "Mut" voice-to-text regression class. Pinning four specific German strings instead of "the value differs from English" is the more defensive shape — a future copy-paste of "Stimmung" → "Mood" still fails it. diff --git a/.planning/phase-W5-v1422-design-review.md b/.planning/phase-W5-v1422-design-review.md new file mode 100644 index 000000000..7c5e97faf --- /dev/null +++ b/.planning/phase-W5-v1422-design-review.md @@ -0,0 +1,123 @@ +# Wave 5 — Design / UX review (v1.4.22) + +## Summary + +- Surfaces audited: BP-in-target tile (`` + `/insights` + `/dashboard`), insights row-fill rule + trends-row equal-height + correlation-row hide-on-insufficient-data, sticky `` above the hero, "Muster → Zusammenhänge / Patterns → Relationships" rename, Coach drawer disclosures (`
` evidence block), Coach drawer header (avatar parity, settings cog removed, disclaimer relocated to sources rail), `/targets` per-card sparkline + Δ-vs-last-month, admin/api-tokens date cells, recommendation card chart-token strip, settings → dashboard global comparison toggle. +- Branch: `develop` (last shipped polish commit `1499e6f`); compared 25 v1.4.22 commits since `16b6976` (`main` merge). +- Findings: **0 CRITICAL · 3 HIGH · 7 MED · 5 LOW** + +--- + +## HIGH + +### H1 — `` sticky bar is keyboard-hostile and lacks an accessible role + +- Surface: `src/app/insights/page.tsx:1724-1773` +- What: The new section-tab strip is rendered as a flat `