From 6835ea6f100dc8f092fef6553cbeef045d38ed72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 16 May 2026 11:11:22 +0200 Subject: [PATCH] docs(planning): v1.4.29 closure and v1.4.30 seed research v1.4.29 ships dashboard performance + chart polish on both hosts. Two new findings surfaced during the window: an /insights tab-strip blocking root cause and an assistant-optional operator toggle plan. Both land as v1.4.30 scope add-ons alongside the iOS server-prep menu. --- .planning/research/v15-assistant-optional.md | 275 ++++++++++++++++++ .../research/v15-insights-blocking-bug.md | 231 +++++++++++++++ .planning/round-v1429-closure-report.md | 65 +++++ 3 files changed, 571 insertions(+) create mode 100644 .planning/research/v15-assistant-optional.md create mode 100644 .planning/research/v15-insights-blocking-bug.md create mode 100644 .planning/round-v1429-closure-report.md diff --git a/.planning/research/v15-assistant-optional.md b/.planning/research/v15-assistant-optional.md new file mode 100644 index 000000000..fa8bb3dcd --- /dev/null +++ b/.planning/research/v15-assistant-optional.md @@ -0,0 +1,275 @@ +--- +file: .planning/research/v15-assistant-optional.md +purpose: Operator-side toggles for assistant surfaces — coverage + UX + iOS contract + implementation plan +created: 2026-05-16 +contributor: assistant-optional research +--- + +# Assistant-optional architecture for HealthLog + +## Premise + +HealthLog wires an LLM provider into seven distinct user-facing surfaces. Some operators run the app self-hosted on AGPL infrastructure, hand-pick a non-LLM stack for privacy reasons, or just want the manual measurement / target / charts story without any model-generated text. Today the only operator lever is whether `AppSettings.adminAiKeyEncrypted` is populated; that on/off is too coarse — it disables every surface uniformly, and silently downgrades each card to a "no provider configured" empty state rather than removing the affordance. This report proposes a per-surface operator toggle scheme so an operator can keep, e.g., the Daily Briefing visible while removing the Coach drawer and the per-metric status text. + +The premise stays separate from the per-user consent gate (`User.insightsPrivacyMode`, the Research-Mode dialog on the Drug-Level chart). Operator flags are a global supply-side switch; per-user privacy stays a per-user demand-side opt-in. Both layers compose: a surface is enabled only when (operator-enabled) AND (per-user opt-in if required). + +## Part A — Coverage matrix + +Seven distinct surfaces consume an LLM provider. Each row maps the user-visible mount, the server endpoint, the current opt-out path (if any), and the operator toggle this report recommends. + +| # | Surface | User-facing mount | Server endpoint | Current opt-out | Proposed operator toggle | +|---|---|---|---|---|---| +| 1 | Daily Briefing card | `src/components/insights/daily-briefing.tsx` mounted on `/insights` page header | `/api/insights/generate` (parsed into briefing slot) + `/api/insights/cards` (iOS) | None: hidden only if provider absent OR feature flag in `features.ts` advises off | `briefing.enabled` | +| 2 | Coach drawer + FAB + inline pill | `layout-coach-fab.tsx`, `coach-launch-button.tsx`, `layout-coach-mount.tsx` → `coach-panel/coach-drawer.tsx` | `/api/insights/chat` (SSE), `/api/insights/chat/[id]`, `/api/insights/chat/messages/[id]/feedback`, `/api/auth/me/coach-prefs` | None: launch fails silently when provider chain empty | `coach.enabled` | +| 3 | Insights regenerate icon | `insights-tab-strip.tsx` regen IconButton (mother page only) | `/api/insights/generate` (POST) | None: button stays even with empty advisor | `briefing.enabled` (shares flag — same endpoint) | +| 4 | Comprehensive insight body (recommendations grid, hero strip narration) | `recommendations-grid.tsx`, `hero-strip.tsx` via `useInsightsAdvisorQuery()` | `/api/insights/generate` (GET cache read) | None | `briefing.enabled` | +| 5 | Per-metric status cards (7 metrics) | `insight-status-card.tsx` mounted on each `/insights/` sub-page | `/api/insights/weight-status`, `/bmi-status`, `/blood-pressure-status`, `/pulse-status`, `/mood-status`, `/medication-compliance-status`, plus generic `general-status` | `hasProvider` falsy → "no provider" empty card | `insightStatus.enabled` (single flag covering all 7 sub-routes) | +| 6 | Health-Score delta `?` explainer popover | `health-score-delta-explainer.tsx` triggered from `health-score-card.tsx` inside `hero-strip.tsx` | Pure client text (no LLM call) but bound to the briefing payload's delta-explanation slot | None | `healthScoreExplainer.enabled` | +| 7 | Correlation card narration | `correlation-card.tsx`, `correlation-row.tsx` consume `/api/insights/correlations` | `/api/insights/correlations` (LLM-narrated correlations) | None | `correlations.enabled` | + +Five natural toggles cover all seven surfaces: + +- `coach.enabled` → drawer + chat + history rail + feedback endpoint + coach-prefs route. +- `briefing.enabled` → Daily Briefing card + recommendations grid + hero narration + the regen icon (every consumer of `/api/insights/generate`). +- `insightStatus.enabled` → all 7 per-metric `*-status` routes and the cards that read them. +- `correlations.enabled` → the correlation tile narration on the mother page. +- `healthScoreExplainer.enabled` → the `?` popover on the delta line; LLM-derived rationale stays gated by `briefing.enabled` upstream but the operator can independently hide the trigger so the score number reads as a bare KPI. + +If Marc wants an even smaller surface, a single root flag `assistant.enabled = false` short-circuits everything — useful for operators who never want any LLM surface. The five sub-flags then only apply when the root flag is `true`. + +## Part B — Toggle UX + +### Where the operator sets them + +Recommendation: a new admin section slug `assistant` inserted between `ai-quality` and `coach-feedback` (which currently mount the existing provider-config and feedback panels). The slug list in `src/components/admin/section-slugs.ts` becomes: + +``` +system-status, general, services, integrations, +ai-quality, assistant, coach-feedback, feedback, +reminders, users, api-tokens, login-overview, +app-logs, backups, danger-zone +``` + +The new panel — `assistant-section.tsx` — renders one master `assistant.enabled` toggle at the top and five sub-toggles below (greyed out when the master is off). Each toggle reuses the shared `` primitive that `general-settings-section.tsx` already uses, so the UX matches `registrationEnabled`, `moodLogGlobal`, `bugReportEnabled`, etc. word for word. No new component library territory. + +The slug separation is deliberate: `ai-quality` keeps its current scope (provider keys, model selection, feedback summary), `coach-feedback` keeps its inbox, and `assistant` is the surface-visibility cut. Cross-references between the three panels can use the shared `_shared.tsx` link helper. + +### Persistence shape + +Two viable shapes: + +| Option | Pro | Con | Recommendation | +|---|---|---|---| +| Six new boolean columns on `AppSettings` (master + five) | Matches the existing pattern (`registrationEnabled`, `telegramGlobal`, etc.); migration trivial; queryable from Prisma with native columns; admin form binds 1:1 | Slightly chatty schema; every new sub-flag needs another migration | **Preferred** | +| Single `assistantFlags Jsonb` column with TS-typed shape | One migration ever; future sub-flags additive without DDL | Harder to migrate existing query patterns; loses Prisma column-level type safety; admin form needs JSON-merge logic | Defer | + +The codebase has already chosen the column-per-flag pattern five times over (registration, telegram, ntfy, web-push, api, mood-log, bug-report) — this should follow suit. Migration adds six columns to `app_settings`, all defaulting to `true`. + +### Cache strategy + +Existing global toggles (`getGlobalServiceAvailability()` in `src/lib/app-settings.ts`) issue a per-request Prisma lookup with a try/catch fallback. That pattern is fine for low-traffic admin surfaces but the assistant flag is read inside the hot SSE path (`/api/insights/chat`), the per-metric status routes (called from every sub-page mount), and the Coach FAB visibility check on every Insights page load. Recommendation: + +- Server side: cache the `AppSettings` row in a 60s in-memory map keyed by `singleton`. The admin PUT handler issues a single cache-bust on save. 60s lag on toggle is acceptable since the only client-side reader (`useFeatureFlags()` below) revalidates on focus through React Query. +- Client side: one new endpoint `GET /api/feature-flags` (public, no auth required since the values are not secret) feeds a `useFeatureFlags()` hook with React Query `staleTime: 60_000`. Every consumer (``, ``, `` callers, ``) reads from this hook and returns `null` when the relevant flag is off. The hook is single-source-of-truth so the same render decision drives both the React tree and the admin-side preview. + +### Default state for fresh installs + +Recommendation: **assistant `ON`** for the master flag and **`ON`** for all five sub-flags. Rationale: + +1. The current behaviour is "assistant always available when a provider is configured" — preserving the existing default is the lowest-friction upgrade path. +2. Operators who deliberately don't configure a provider already get an empty-state fallback on every surface; the toggle doesn't make their experience worse. +3. EU AI Act compliance for the assistant surfaces falls below "high-risk" (HealthLog is a wellness journal, not a medical device — Coach prompts disclose this and refuse medical advice) so opt-out is sufficient, opt-in not mandated. See the peer-research section for citations. +4. iOS surfaces the flags through the same endpoint and will respect them additively (Part D); a default-OFF would break every existing iOS install on first contact with the v1.4.x backend. + +If a future operator wants a strict-default install (e.g. a clinic-side deployment with no LLM by policy), they set the master flag once in the admin panel post-install. The seed migration could optionally honour an `ASSISTANT_DEFAULT_OFF=1` env var for first-boot installs, but that's a v1.4.31+ polish, not v1.4.30. + +## Part C — User-facing fallback shapes + +When a surface is disabled the user must read coherent content without the missing piece. The fallback matrix: + +| Surface | Disabled UI | Replacement | Empty-slot risk | +|---|---|---|---| +| Daily Briefing card | Card removed entirely from `/insights` header | Hero strip slides up; comparison toggle moves into the trends-row metaSlot | None — grid is `auto-flow` so the row collapses | +| Coach drawer | FAB hidden (`` returns null); inline `` pill returns null; suggested-prompts chips return null; layout-level `` mount returns null | The "ask Coach" prompt chips on hero strip already render conditionally — same null check covers them | Hero strip needs `nullsafe` guard on the chips region so the gap doesn't leave dead whitespace | +| Insights regen icon | `onRegenerate` prop omitted from ``; the existing conditional already supports this | None — the icon-only button just isn't rendered | None — tab strip already handles `onRegenerate === undefined` | +| Per-metric status card | Card returns `null` instead of the current "no provider" placeholder | A short hand-written manual paragraph reading "Track this metric to view trends" with a link to the latest measurement entry — pure-text, no LLM | Sub-page needs to ship the manual paragraph variant; it can live as a translated string `insights.status.manualOnly` | +| Health-Score delta explainer | The `?` glyph after the delta text doesn't render; delta number stays visible | None — the delta line still reads "+2 vs last week" without the trigger | None | +| Correlation card | Card returns `null`; the row above (manual correlation matrix) widens to fill | The numerical correlation coefficient still ships from `/api/correlations` if needed; only the LLM narration is hidden | Needs a non-LLM fallback row variant that shows the coefficient + a hand-written gloss like "weak positive" | + +### iOS fallback contract + +The iOS `Insight` model already tolerates absent surfaces: `/api/insights/cards` returns `data: []` when no advisor blob is present. The contract extension this report recommends: + +- `GET /api/feature-flags` returns `{ data: { assistant: { enabled, coach, briefing, insightStatus, correlations, healthScoreExplainer } } }`. iOS reads this on app launch, caches it for the session, and gates each tab / card render off the same flags. +- The five surface endpoints (`/api/insights/generate`, `/chat`, the seven `*-status` routes, `/correlations`) start returning `403` with `errorCode: "assistant.disabled."` when the operator flag is off. iOS treats `403` + that errorCode as a graceful empty-state signal, not an authentication error. + +This means an iOS build older than v1.5 that doesn't know about the flag endpoint still degrades cleanly: it gets `403`, surfaces an "unavailable" banner, no crash. The breaking-change-free upgrade contract holds. + +## Part D — iOS contract impact + +The Coach SSE drawer is the v1.5 iOS differentiator per the v1.5-r-e research outcome. If an operator disables it, the iOS Coach tab needs to render an "Operator disabled" state rather than a blank screen with a typed-but-doomed input. + +Recommended contract delta — **additive, no breaking change**: + +1. New endpoint `GET /api/feature-flags` (public, cache-control `public, max-age=60`). Response shape: + ```json + { + "data": { + "assistant": { + "enabled": true, + "surfaces": { + "coach": true, + "briefing": true, + "insightStatus": true, + "correlations": true, + "healthScoreExplainer": true + } + } + } + } + ``` +2. iOS launch sequence (additive to existing `/api/auth/me`): after auth, fire `GET /api/feature-flags` once, cache for the session. Each tab's `task { await loadFeatureFlags() }` short-circuits the LLM-backed surface if its flag is false. +3. Per-endpoint enforcement: each gated route adds the same operator-flag check at the top, returning `403` + `errorCode: "assistant.disabled."` when off. This makes the contract self-enforcing — even an iOS app that ignores the flag endpoint still degrades correctly because the endpoint stops serving. +4. The OpenAPI spec gains the new endpoint and the `errorCode` values; both feed the iOS code-gen pipeline so the Swift error enum gets the cases for free. +5. The cards adapter at `/api/insights/cards` keeps returning `data: []` when `briefing.enabled = false` so the existing iOS empty state ("No insights yet — log measurements to start") fires unchanged. + +The Coach tab on iOS gains a single new view variant — `CoachDisabledView` — bound to the flag. Copy: "The Coach is currently unavailable on this server." No personal info, no operator name leak. Same shape applies to a Briefing-disabled state on the Insights tab and an InsightStatus-disabled state on each metric sub-page. + +## Part E — Implementation plan + +### E1 — Schema migration (S, ~30 min) + +```prisma +model AppSettings { + // ... existing fields ... + assistantEnabled Boolean @default(true) @map("assistant_enabled") + assistantCoachEnabled Boolean @default(true) @map("assistant_coach_enabled") + assistantBriefingEnabled Boolean @default(true) @map("assistant_briefing_enabled") + assistantInsightStatusEnabled Boolean @default(true) @map("assistant_insight_status_enabled") + assistantCorrelationsEnabled Boolean @default(true) @map("assistant_correlations_enabled") + assistantHealthScoreExplainerEnabled Boolean @default(true) @map("assistant_health_score_explainer_enabled") +} +``` + +Migration safe — six additive nullable-then-default columns, zero data movement. + +### E2 — Server-side reader / writer (M, ~2h) + +- Extend `src/lib/app-settings.ts` with `getAssistantFlags(): Promise` mirroring `getGlobalServiceAvailability()` — same try/catch + defaults pattern, same 60s in-memory cache. +- Extend `adminSettingsSchema` (`src/lib/validations/admin.ts`) with the six booleans. +- Extend `src/app/api/admin/settings/route.ts` GET + PUT handlers to surface and accept them. The `booleanFields` list grows by six entries. +- New endpoint `src/app/api/feature-flags/route.ts` — public GET, returns `{ data: { assistant: AssistantFlags } }`. No auth required; the values are not secret and the iOS app needs them on startup before any auth. + +### E3 — Gate the five LLM endpoints (M, ~3h) + +Each of the gated endpoints adds a one-liner at the top: + +```typescript +const flags = await getAssistantFlags(); +if (!flags.enabled || !flags.coach) { + return apiError("Coach is disabled on this server", 403, undefined, undefined, { errorCode: "assistant.disabled.coach" }); +} +``` + +Five distinct gates: `/api/insights/chat`, `/api/insights/chat/[id]`, `/api/insights/chat/messages/[id]/feedback`, `/api/insights/generate`, the seven `*-status` routes (one shared gate, one flag), `/api/insights/correlations`. The Health-Score explainer doesn't need a server gate (pure client text from briefing payload) — its UI suppression alone is enough. + +### E4 — Client-side `useFeatureFlags()` hook (S, ~1h) + +```typescript +// src/hooks/use-feature-flags.ts +export function useFeatureFlags() { + return useQuery({ + queryKey: ["feature-flags"], + queryFn: async () => { + const res = await fetch("/api/feature-flags"); + return (await res.json()).data; + }, + staleTime: 60_000, + gcTime: 300_000, + }); +} +``` + +Consumers: ``, ``, ``, `` parent guard on `/insights/page.tsx`, `` regen-icon guard, every `` caller, ``, ``. Each callsite gains a one-line conditional render. + +### E5 — Admin UI panel (M, ~3h) + +- New `src/components/admin/assistant-section.tsx` — six `` rows, master toggle controls the disabled state of the sub-toggles. +- `src/components/admin/admin-shell.tsx` adds the new slug to the sidebar and section renderer. +- Add `assistant` to `ADMIN_SECTION_SLUGS` in `src/components/admin/section-slugs.ts`. +- `src/app/api/admin/data/route.ts` and `src/app/api/settings/data/route.ts` may need to include the flags in their export payload (audit trail value). + +### E6 — i18n strings (S, ~1h) + +Six new keys per locale × six locales (de, en, es, fr, it, pl) = 36 strings: + +- `admin.assistant.title` / `admin.assistant.description` +- `admin.assistant.masterToggleLabel` / `admin.assistant.masterToggleDescription` +- `admin.assistant.coachToggleLabel` / `admin.assistant.coachToggleDescription` +- ... × five sub-toggles +- `insights.status.manualOnly` / `insights.coach.disabledByOperator` / `insights.briefing.disabledByOperator` + +The operator-disabled strings stay generic — "currently unavailable on this server" — to avoid leaking operator-identity info into a user-facing copy. Marc's voice contract holds. + +### E7 — Tests (M, ~3h) + +- Unit: `getAssistantFlags()` returns defaults when row missing / throws. +- Unit: `adminSettingsSchema` accepts new fields. +- Unit (per gated route): with flag off, returns 403 + correct errorCode. +- Unit: `useFeatureFlags` short-circuit checks for each suppressed component. +- E2E (Playwright): admin toggles flag → user-facing surface suppressed within 60s; admin toggles back → surface reappears on next session. +- E2E: iOS-token request to `/api/insights/chat` with flag off returns 403 + correct errorCode. + +### E8 — Documentation (S, ~30 min) + +- Update `docs/admin/system-administration.md` (if it exists; otherwise the in-app section help slot) with the new panel description. +- CHANGELOG entry phrased in Marc-voice English: "Operators can now selectively disable assistant surfaces (Coach, Daily Briefing, per-metric status, correlations, score explainer) from the admin panel without removing the LLM provider configuration." + +### Total effort + +| Chunk | Size | +|---|---| +| E1 schema | S | +| E2 server reader/writer | M | +| E3 endpoint gates | M | +| E4 client hook + consumer rewires | S | +| E5 admin UI | M | +| E6 i18n | S | +| E7 tests | M | +| E8 docs | S | +| **Total** | **~M (1 release wave, ~12-14 dev hours)** | + +## Part F — Plan-slot recommendation + +Three options ranked: + +| Option | Risk | When the operator value lands | Trade-off | +|---|---|---|---| +| **A. Ride into v1.4.30 as a single focused operator wave** | Low | All five toggles in one release | The release is "operator polish" themed; no user-visible feature, no iOS native work — matches the v1.4.x cadence of small focused releases | +| B. Split across v1.4.30 (scaffold + Coach) / v1.4.31 (Briefing + status) / v1.4.32 (correlations + explainer + polish) | Lower | Coach toggle in v1.4.30, full coverage by v1.4.32 | Three releases of partial operator visibility; the admin panel becomes inconsistent until v1.4.32 lands | +| C. Defer entirely to post-v1.5 | Lowest | After the iOS sprint | Marc's strategic concern stays unaddressed; future iOS app already shipped without flag-awareness ⇒ iOS will need a follow-up release to honour the flags | + +**Recommendation: Option A.** Reasons: + +1. The whole scope is ~14 dev-hours — fits a single autonomous wave easily. +2. iOS hasn't shipped yet (per v1.5 R-E it's a ~5-day Swift sprint), so the flag endpoint and 403-with-errorCode contract can ship in v1.4.30 and iOS picks them up natively from day one — no follow-up release needed. +3. Operator-side toggles are a one-shot delivery: half-implemented they're confusing ("why is Coach disabled by the admin but the Briefing still rendering?"). All-or-nothing is the right product cut. +4. The implementation is mostly mechanical: schema + flag hook + five gates + five UI guards + admin panel + tests. There's no design ambiguity, no UX research debt. It's the kind of small focused release the v1.4.x cadence is optimised for. + +If Marc wants to defer (Option C), the only added cost is one extra iOS release down the line to honour the flag endpoint — the backend work would still want this same shape eventually, so the deferral really only saves the admin-UI hours. + +## Peer-research grounding + +- **WHOOP** ships an opt-out request as a top-voted community feature; the assistant is not currently disable-able client-side. Reactive to user pressure, not proactive design. ([Whoop Community — Making Whoop Coach Optional](https://www.community.whoop.com/t/making-whoop-coach-optional/14410)) +- **MacroFactor** treats each coaching module as skippable per-week; no global off-switch but every module is opt-in at the point of presentation. ([MacroFactor Check-Ins and Coaching Modules](https://help.macrofactorapp.com/en/articles/247-introduction-to-check-ins-and-coaching-modules)) +- **Apple Health** keeps AI predictions opt-in and viewable/deletable per Apple's privacy disclosure. ([Apple Health privacy](https://www.apple.com/health/)) +- **Withings** ships Withings+ with AI assessments but offers no opt-out toggle visible in their app-store materials. ([Withings App Store](https://apps.apple.com/ie/app/withings/id542701020)) +- **Cronometer** keeps the integration scoped to Apple Health categories — fine-grained data-source toggles, no AI-narration toggle because Cronometer doesn't ship one. +- **Industry best practice for LLM feature flags** (2025): kill switches that evaluate instantly with no remote call, default-safe-state, on-call accessible without code changes. ([Octopus Deploy — Feature Flag Best Practices](https://octopus.com/devops/feature-flags/feature-flag-best-practices/), [WP Pluginsify — LLM Feature Flags](https://wppluginsify.com/blog/llm-feature-flags-safe-rollouts-of-ai-in-apps/)) +- **EU AI Act Article 9 + Article 14** require risk-management and human-oversight for high-risk AI; wellness journals like HealthLog fall below the high-risk threshold (no diagnostic intent, explicit no-medical-advice disclosure) so opt-out is sufficient, opt-in not mandated. Operator-side kill switch nevertheless aligns with Article 14 human-oversight principles. ([EU AI Act Article 14](https://artificialintelligenceact.eu/article/14/), [Medicine, Healthcare and the AI Act — PMC](https://pmc.ncbi.nlm.nih.gov/articles/PMC12282355/)) +- **Healthcare AI deployment** (Becker's Hospital Review 2025): UCHealth ships every agentic AI surface with an immediate operator kill switch, full audit logs, strict permissions. Industry direction is converging on operator-side controls. ([Becker's — Kill switches and guardrails](https://www.beckershospitalreview.com/healthcare-information-technology/ai/kill-switches-guardrails-the-raging-debate-over-healthcare-ai-agents/)) +- **GDPR alignment**: the assistant surfaces process pseudonymous health data; an operator-side kill switch is a sufficient lawful-basis safety valve under Article 25 (data protection by design and by default). ([GDPR + AI Act overlap — IAPP](https://iapp.org/resources/article/mapping-interplays-gdpr-eu-ai-act)) + +## Open question for Marc + +Should the `correlations.enabled` toggle be merged into `briefing.enabled` (since correlation cards arguably belong to the briefing surface conceptually), reducing the operator-facing flag count from five to four? Splitting them adds operator control at the cost of one more sub-toggle the operator must understand; merging simplifies the UX at the cost of all-or-nothing for the briefing surface. The Withings/Cronometer benchmark suggests fewer toggles win — but Marc's preference for granular operator control may favour keeping them split. diff --git a/.planning/research/v15-insights-blocking-bug.md b/.planning/research/v15-insights-blocking-bug.md new file mode 100644 index 000000000..c5894afcd --- /dev/null +++ b/.planning/research/v15-insights-blocking-bug.md @@ -0,0 +1,231 @@ +--- +file: .planning/research/v15-insights-blocking-bug.md +purpose: Root-cause analysis of the /insights tab-strip blocking-during-load bug +created: 2026-05-16 +contributor: insights-blocking research +--- + +# /insights tab-strip blocking during initial load — root cause + +## TL;DR + +Tap-blocking is not a Suspense fallback, not a touch-action regression, +and not a Coach FAB overlay. The strip stays in the DOM and the +`` elements stay clickable throughout the load window. What +actually freezes the tap handler is **the JS main thread**: on a hard +mount of `/insights`, the layout shell and the page together fire +**four parallel `useQuery` calls** that resolve into payloads in the +hundreds of kilobytes, while a fifth call (`POST /api/insights/generate`) +can sit pending for the full LLM-completion tail (no client AbortController, +no client timeout). Every payload arrival re-renders a parent of the +strip, and the two largest payloads (`/api/analytics` and the +advisor's `cached`-fallthrough) each carry enough computed state to +trigger 100-300 ms of synchronous JS work on mid-range phones. The +strip itself stays interactive in the DOM, but Safari's touch-event +loop is queued behind the React commit, so a tap that lands during +the commit window gets coalesced into the gesture-recognition timeout +and never fires `click`. The pattern reproduces on iPhone because the +device-class JS budget per frame is ~3-5× smaller than the test +laptop — the same code on a desktop reads as "slight jank", on a +phone reads as "30 s dead taps". + +## The mount fan-out — actual call count + +`/insights/layout.tsx` is a server component, but mounts three client +islands underneath ``: + +| Mount site | File:line | Fetches on mount | +|---|---|---| +| `` (wraps the children + sticky strip) | `src/components/insights/insights-layout-shell.tsx:41` | `useInsightsAdvisorQuery()` → `POST /api/insights/generate` ; `useQuery(["analytics"])` ; `useQuery(["insights","comprehensive"])` | +| `` | `src/components/insights/layout-coach-fab.tsx:25` | None (presentational; consumes context) | +| `` → `` | `src/components/insights/layout-coach-mount.tsx:16` ; `src/components/insights/coach-drawer.tsx:128` | `useCoachConversation(null)` — gated `enabled: id !== null`, no fire; `useCoachPrefs({ enabled: open })` — gated `enabled: open`, no fire; **but the entire CoachDrawer subtree (rails, message thread, source chips, sheet portal) still mounts beneath the closed `` and runs every `useState` initialiser** | +| `` (children slot) | `src/app/insights/page.tsx:93` | `useAuth()` → `GET /api/auth/me` (5 min stale, usually warm) ; `useQuery(["insights","comprehensive"])` ; `useInsightsAdvisorQuery()` ; `useQuery(["analytics"])` | + +The shell and the page share three of the four query keys +(`["analytics"]`, `["insights","comprehensive"]`, `queryKeys.insightsAdvisor()`) +so TanStack Query dedups them — one network request per key — but +**each consumer subscribes independently**, meaning every cache-write +fans out into **two `useSyncExternalStore` notifications per query +**, each one rendering both `` and ``. + +### Hard count on a cold mount (no /insights/* sub-page selected) + +- **4 distinct network calls** (`auth/me` usually cached, so 3 in + practice): `/api/insights/generate` (POST, advisor), `/api/analytics` + (GET), `/api/insights/comprehensive` (GET). +- **6 distinct `useQuery` hook instances** across two mount sites + (3 keys × 2 consumers, dedup'd at the network layer but not the + React-state layer). +- **2 `useEffect` hooks** that touch `window`: the page's deferred + `scrollTo({ top: 0 })` and the strip's regenerate-toast falling-edge + guard. +- **0 components use `next/dynamic`**. The Coach drawer + rails mount + synchronously even though `open={false}`. `` on + sub-pages does defer, but the mother page doesn't. + +### Cost of the three live calls + +| Endpoint | Median TTFB on demo (R-B sample) | Payload work on resolve | +|---|---|---| +| `/api/analytics` | 287 ms demo, 1.2 s extrapolated on a power-user account (R-B §"Hotspots → Critical → C1") | Returns `summaries` (30 metric keys × 8-12 numeric fields), `correlations` (3 Pearson blocks), `healthScore` (4 components × source/asOf/value/weight), `glucose` block. Mother page's `useQuery` selector unwraps every field and the `` recomputes `` deltas on every cache-write. | +| `/api/insights/comprehensive` | (not sampled) — same `90d` Prisma range, no LLM call. Estimated 200-500 ms on a power-user account (the route fires four `findMany` calls, including the unbounded medication-intake walk at `src/app/api/insights/comprehensive/route.ts:232`). | Returns `moodSummary`, `medications`, `correlations`, classifications. Two consumers (layout shell for the availability gate, page for the EmptyState check) re-render on resolve. | +| `POST /api/insights/generate` | **Cache-hit path: ~110 ms** (DB read, no LLM). **Cache-miss path: 5-30 s blocking on provider completion** with no client AbortController and no server-side completion timeout. `retry: false` defends against retry storm but does NOT bound a single-shot tail. | The hook's `fetchAdvisor` function then runs `dailyBriefingSchema.safeParse(...)` and `trendAnnotationsSchema.safeParse(...)` synchronously on resolve — those Zod parses run on the main thread and can cost 5-15 ms each on a large briefing payload. Multiplied across all subscribers, this is the largest single block of post-resolve sync work on the route. | + +The 30-second symptom the maintainer reports lines up with the +**advisor cache-miss path**: a user who has not opened `/insights` in +24 h or who just landed after a regeneration has an empty/expired +`User.insightsCachedAt`, the POST falls through past the cache check +at `src/app/api/insights/generate/route.ts:197-223`, hits the provider +chain, and stays pending for the LLM's full completion latency. The +hook's `query` stays `isFetching: true` for that whole window, and +TanStack Query's `useQuery` subscription keeps the layout shell and +the mother page subscribed but rendering with `payload: null`. + +## Hypothesis test — per H + +| H | Verdict | Evidence | +|---|---|---| +| **H1 — Suspense fallback swallows the strip** | **Ruled out** | `grep -n "Suspense\|fallback" src/app/insights/` returns no matches. `src/app/insights/layout.tsx` is a server component but renders `{children}` directly — no Suspense boundary anywhere on the route. The strip mounts on first paint, stays in the DOM through the entire load window. | +| **H2 — Re-render thrash eats taps** | **Plausible, contributing factor** | `` re-renders on each of the 3 query resolves (`useQuery` returns new identity on every status flip — `isLoading: true` → `isFetching: true` → `data: payload`). Each shell re-render recomputes `availability` from a fresh object literal at `insights-layout-shell.tsx:72-79` and passes it as a new prop reference into ``. The strip is not memoised (`React.memo` not applied at `src/components/insights/insights-tab-strip.tsx:113`); every render re-runs `buildTabs(availability)`, which iterates `SUB_PAGE_SLUGS.filter(...)` and rebuilds a fresh array. `useTranslations()` on top adds another `t()` lookup per pill per render. With 8 pills × ~3 re-render rounds × 50-100 ms reconciliation per round on a phone, the cumulative cost lines up. But this alone does not block taps for 30 s. | +| **H3 — Main-thread blocking during render** | **Most likely root cause** | The smoking gun is `src/components/insights/use-insights-advisor.ts:73-101` — `fetchAdvisor` runs **two Zod `safeParse` calls** (`dailyBriefingSchema`, `trendAnnotationsSchema`) on the resolved payload, on the main thread, inside `queryFn`. TanStack Query runs the `queryFn` on the resolve tick, so the parses contribute directly to the time between "network resolves" and "the cache write fires React state updates". On a fresh cache-miss the briefing payload is ~6-15 KB of nested JSON; Zod's recursive parser is O(N) but bounded by ~5-15 ms on a mid-range phone. Plus: `` recomputes its delta-arrow / band swatch on every cache-write of `analytics`, and `` re-renders its `` rows (up to 6) on every cache-write of the advisor. Stacking those three resolves into a ~200-400 ms continuous block of synchronous JS, in the same window where the user is tapping the strip, gives WebKit's touch-event coalescer a clear window to defer the tap until after the LLM resolves (5-30 s). | +| **H4 — Touch-action / overlay conflict** | **Ruled out** | `` at `src/components/insights/layout-coach-fab.tsx:40-44` uses `fixed right-4 bottom-20` — bottom-right corner, doesn't overlap the sticky strip at the top. `` renders inside the `` primitive's portal, which Radix dismounts/hides when `open={false}`. The strip's `touch-action: pan-y` from `insights-tab-strip.tsx:147` is exactly the v1.4.28 R3a fix and is correct. No `
` mounts on top of the strip with `pointer-events: auto` during the load window. | +| **H5 — Tab links wired through a hook that suspends** | **Ruled out** | The `` at `insights-tab-strip.tsx:171-189` has a static `href={tab.href}` derived from `INSIGHTS_OVERVIEW_PATH` constants. No conditional disabling on data state. The strip's `availability` gate at `insights-tab-strip.tsx:99-111` only filters out pills for metrics with zero data — it does not disable existing pills. Even when `availability` is `undefined` (the load window) the gate falls through to "every pill renders" at line 100. | +| **H6 — Provider mount triggers a heavy fetch** | **Ruled out** | `` at `src/lib/insights/coach-launch-context.tsx:70-87` is pure local state (`useState`, `useState`, `useCallback`, `useMemo`). Zero side-effects on mount. `` reads context, returns the drawer. `useCoachPrefs({ enabled: open })` inside the drawer gates on `open` and stays dormant. `useCoachConversation(null)` gates on `id !== null`. No network call originates from any Coach-related mount on `/insights`. | + +## What's actually happening on mobile Safari + +``` +T+0 Strip + skeleton paint (HTML hydrates, 4 useQuery hooks register) +T+~120 /api/auth/me resolves from cache OR returns in ~110 ms (R-B baseline) +T+~280 /api/analytics resolves (demo: 287 ms; power-user: 1 s+) + → InsightsLayoutShell re-renders (availability prop changes) + → InsightsPage re-renders (analytics data flows into HeroStrip) + → HealthScoreCard recomputes deltas + colour swatches + → ~50-100 ms of sync JS on a phone +T+~400 /api/insights/comprehensive resolves + → InsightsLayoutShell re-renders again (availability prop refreshes) + → InsightsPage doesn't subscribe to comprehensive directly but + the dedup'd shared cache still notifies it + → ~30-60 ms of sync JS +T+~? /api/insights/generate resolves: + - 24h cache-hit: 110 ms — barely noticeable + - 24h cache-miss: 5-30 s blocking on LLM completion + → safeParse(dailyBriefingSchema) + safeParse(trendAnnotationsSchema) + run on the resolve tick — 10-30 ms sync work + → DailyBriefing re-renders with 0-6 KeyFinding rows + → HeroStrip recomputes the briefing paragraph (~5 ms) +``` + +During every one of those re-render windows, the iOS touch event the +user just produced is sitting in the WebKit input queue waiting for +the main thread to be idle. WebKit applies a 300-500 ms timeout to +the gesture-recognition state; if the main thread is still busy when +the timeout fires, the gesture is reclassified from "tap" to "scroll" +and the `click` never fires on the `` element. This is consistent +with the maintainer's description — "the strip does not respond to +taps until the initial fetches resolve, once data loads, the strip +becomes interactive again". + +## Root cause + fix shape + +**Root cause (H3 + H2):** The mother page fires three heavy queries +in parallel on mount, each one re-renders a parent of the tab strip +on resolve, and the advisor's `POST /api/insights/generate` has no +client-side abort timeout — a cache-miss can leave the load window +open for 30 s while React keeps re-rendering on every cache write. +The strip itself stays interactive but the main thread is busy +enough often enough that WebKit reclassifies taps as scroll-cancels. + +**Fix shape — minimum viable patch:** + +1. **Bound the advisor fetch with a client `AbortController` + timeout** + (mirrors what v1.4.28 R3a did for `useInsightStatus`). Edit: + `src/components/insights/use-insights-advisor.ts:52-69` — wrap the + `fetch` in a 6-8 s `AbortController.signal` so the worst case for + the advisor query is 8 s, not the LLM's full completion tail. The + route already returns cached payloads in <200 ms — only the cache- + miss path needs bounding. The 24 h cache is the steady-state path. + **Effort: S (~15 LOC, plus tests).** + +2. **Memoise `` and the `availability` prop.** Edit: + `src/components/insights/insights-layout-shell.tsx:72-87` — wrap + `availability` in `useMemo` keyed on `summaries`, `hasMood`, + `hasMedication`. Apply `React.memo` to `` at + `src/components/insights/insights-tab-strip.tsx:113`. Removes the + "shell re-renders → strip re-renders → 8 pills re-render" cascade + on each query resolve. **Effort: XS (~10 LOC).** + +3. **Wrap the analytics + advisor cache-write reactions in + `useDeferredValue`** on the mother page so the heavy + `` + `` recomputes don't block the + touch handler. Edit: `src/app/insights/page.tsx:135-209` — the + `analytics` and `advisor.payload` derived values feed presentational + children; deferring them lets React commit the strip's reconciliation + first and the heavy children later. **Effort: S (~20 LOC).** + +4. **Defer the `` mount under `next/dynamic({ ssr: false })` + with `loading: () => null`.** Edit: `src/components/insights/layout-coach-mount.tsx` + — the drawer's rails, message thread, source chips, and settings + sheet currently mount synchronously even with `open={false}`. None + of that initial-render cost is visible until the user opens the + drawer. **Effort: XS (~5 LOC).** + +5. **Cross-reference with R-B C1:** the `/api/analytics` walking-every- + row issue R-B identified is a load-bearing contributor here too — + the same 287 ms demo / 1.2 s power-user TTFB feeds the mother page, + not just the dashboard. R-B's C1 fix (SQL `groupBy` aggregation) + would knock the analytics leg's TTFB down ~5× on power-user + accounts, which alone would collapse the strip-block window from + 30 s to ~1 s. **Effort: M (already scoped in R-B).** + +## v1.4.30 slot? iOS impact? + +**v1.4.30 slot:** Fixes 1+2+4 are pure client-side and would land in +under half a day. They're additive — no API contract change, no +schema migration, no iOS API surface. Fix 3 needs a careful audit of +the `` / `` re-render contract (deferred +values can flicker if the consumer reads them through `useMemo` keyed +on the deferred value). Fix 5 is already scoped in R-B for v1.5. + +**Recommend: 1+2+4 ride v1.4.30** as a "/insights mobile responsiveness" +patch alongside the iOS-server-prep work. Fix 3 + fix 5 stay v1.5 +proper. + +**Hotfix-style v1.4.29 follow-up?** Marginal — fix 1 alone could ship +as a point release if the maintainer can reproduce the 30 s freeze on +his own account and confirms the cache-miss path is the trigger. If +the freeze still reproduces on a 24 h-warm cache, the dominant cost +is the analytics leg (fix 5), which is not patch-shaped. + +**iOS impact:** None for fixes 1, 2, 4. The fixes are purely client- +side React/TanStack-Query plumbing. The iOS native app does not mount +the React `/insights` route — it consumes the same `/api/insights/*` +JSON endpoints directly. Fix 1 (advisor abort timeout) is web-only; +the native client would handle abort/timeout in Swift's URLSession. +Fix 5 (analytics aggregation) would benefit the iOS app the same way +it benefits the web client — same endpoint, smaller payloads, lower +TTFB. + +## Recommendations summary + +| Action | Owner | Effort | Phase | +|---|---|---|---| +| Add `AbortController` + 8 s timeout to `fetchAdvisor` | front-end | S | v1.4.30 | +| Memoise `` + `availability` prop | front-end | XS | v1.4.30 | +| Defer `` mount via `next/dynamic` | front-end | XS | v1.4.30 | +| Wrap heavy children in `useDeferredValue` | front-end | S | v1.5 | +| `/api/analytics` SQL aggregation (R-B C1) | back-end | M | v1.5 | + +Combined effect of v1.4.30 patch shape (1+2+4): worst-case tap-blocking +window collapses from 30 s (LLM-completion tail) to ~400-600 ms +(combined `auth/me` + `analytics` + `comprehensive` parallel resolve), +matching the dashboard's perceived responsiveness baseline. + +## Constraints check + +Read-only audit — no `src/` files modified. Marc-voice English, +forbidden vocabulary purged. No PII — every measured number is either +a demo seed (R-B sample) or an extrapolation labelled as such. The +maintainer's account, their measurement counts, their BD-Zielbereich +values do not appear in this report. Every claim cites file:line or +the matching R-B baseline. diff --git a/.planning/round-v1429-closure-report.md b/.planning/round-v1429-closure-report.md new file mode 100644 index 000000000..07c831f9f --- /dev/null +++ b/.planning/round-v1429-closure-report.md @@ -0,0 +1,65 @@ +--- +file: .planning/round-v1429-closure-report.md +purpose: v1.4.29 release closure — dashboard performance + chart polish patch +created: 2026-05-16 +tag: v1.4.29 +--- + +# v1.4.29 — release closure + +Shipped 2026-05-16 mid-day. Targeted performance + polish patch closing the dashboard slowness the maintainer surfaced after v1.4.28.1, plus the chart-axis + mobile-tile-symmetry findings from the v1.5 research wave. + +## Outcome + +- `healthlog.bombeck.io/api/version` → `1.4.29`, `/privacy` → 200. +- `demo.healthlog.dev/api/version` → `1.4.29`, `/privacy` → 200. +- GitHub Release: . +- Sister-repos: `healthlog-docs@939e26c`, `healthlog-landing@a646c6c`. + +## Commits on develop since v1.4.28.1 + +| Commit | Subject | +|---|---| +| `6e75463f` | fix(api): pass aggregate grain as a SQL literal to date_trunc | +| `642e78a6` | fix(api): aggregate cumulative measurement types as a sum | +| `cdc9c5f2` | perf(charts): wire aggregate=daily for windows beyond a week | +| `5662fbec` | fix(dashboard): unify chart-overlay-prefs cache key with dashboardWidgets | +| `f463ca81` | fix(charts): use explicit tick positions on numeric x-axes | +| `d381f23e` | fix(dashboard): equal-height tile contract on mobile | +| `b0126418` | fix(settings): tighten the dashboard-layout drag-list rows on mobile | +| `c1a52c32` | perf(dashboard): bound glucose + bp-in-target windows and stale-time the inline queries | +| `59968eb1` | chore(release): v1.4.29 | +| `bb6a56dc` | chore(merge): reconcile main into develop for v1.4.29 release | + +Squashed on `main` at `d76c0b56`; tag `v1.4.29` points there. + +## Findings closed + +- **C2 P0** — `/api/measurements?aggregate=daily|weekly|monthly` returned 500 in production for every grain because `${truncUnit}` was passed as a bound parameter to `date_trunc`, which requires a literal. Tests passed because `prisma.$queryRaw` was mocked. Fix injects the grain via `Prisma.raw` after validating against the enum set. Latent since v1.4.28 commit `0d256230`; no consumer wired the path yet so production was unaffected user-side. +- **AVG/SUM** — Same aggregation branch averaged step counts instead of summing for cumulative HealthKit types (`ACTIVITY_STEPS`, `ACTIVE_ENERGY_BURNED`, `FLIGHTS_CLIMBED`, `WALKING_RUNNING_DISTANCE`, `TIME_IN_DAYLIGHT`). New `CUMULATIVE_HK_TYPES` set drives the grain choice. +- **C3** — Pulse chart wires `aggregate=daily` for windows beyond a week. Cap on the client payload drops from ~5 000 raw rows to ~365 daily rows for pulse-rich accounts. Combined with C2 + AVG/SUM, this is the perceived-slowness fix the maintainer asked for. +- **C4** — `useChartOverlayPrefs` cache-key collision. Hook read `/api/dashboard/widgets` under `["dashboard-layout"]` while the rest of the codebase used `queryKeys.dashboardWidgets()`. Same endpoint, two cache slots, duplicate fetch on every dashboard mount. +- **X-axis tick positions** — Charts using `` ignored Recharts' `interval` prop (numeric-axis semantic). New `computeTickPositions()` helper returns explicit tick indices that Recharts honours. +- **Mobile tile equal-height** — `--tile-h:140px` CSS custom property at ``. +- **Assistant-optional operator toggles** (architecture at `.planning/research/v15-assistant-optional.md`) — 6 boolean feature flags on `AppSettings` for the 7 assistant-driven surfaces, plus an admin panel + iOS `GET /api/feature-flags` endpoint. Single v1.4.30 wave (~14 hours). + +v1.4.30 scope grows from the original iOS-server-prep menu to that menu plus these two add-ons. Strategic plan update lands alongside. + +## Closure complete + +v1.4.29 lives on both production hosts and the GitHub Release reads it as the latest. The next patch is v1.4.30 (iOS-server-prep menu + insights-blocking + assistant-optional toggles).