From 79bfa27d6518475f0877728e1a2de19b75d28e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 09:49:17 +0200 Subject: [PATCH] fix(insights): handle legacy cached payload without rationale (regenerate CTA) Cached insight blobs from before the v1.4.16 strict schema landed crashed /insights with `Cannot read properties of undefined (reading 'replace')` on `stripChartTokens(insight.summary)`. The pre-strict shape (`{changed, stable, drivers, nextSteps, confidence, limitations}`) has no `summary`, no `recommendations[]`, and no `findings[]`, so the rich card had nothing to render and the chart-token helper crashed on undefined input. The advisor card now short-circuits to a self-contained legacy-payload card with a one-click regenerate CTA when the shape is unrenderable. `isLegacyInsightPayload()` also flags the v1.4.14 pre-strict shape so the API surfaces the right `legacyPayload: true` flag from a cache hit. Defense-in-depth: `stripChartTokens()` and `parseChartTokens()` treat null/undefined as the empty string instead of crashing. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__tests__/insight-advisor-card.test.tsx | 39 ++++++++++ .../insights/insight-advisor-card.tsx | 74 +++++++++++++++++++ .../legacy-payload-detection.test.ts | 22 ++++++ src/lib/ai/legacy-payload.ts | 40 +++++++--- .../insights/__tests__/chart-tokens.test.ts | 23 ++++++ src/lib/insights/chart-tokens.ts | 24 +++++- 6 files changed, 210 insertions(+), 12 deletions(-) diff --git a/src/components/insights/__tests__/insight-advisor-card.test.tsx b/src/components/insights/__tests__/insight-advisor-card.test.tsx index 8b3ebaa8a..1b9085ed4 100644 --- a/src/components/insights/__tests__/insight-advisor-card.test.tsx +++ b/src/components/insights/__tests__/insight-advisor-card.test.tsx @@ -267,6 +267,45 @@ describe(" — polished loading / empty / error states (B1b) expect(html).toMatch(/data-slot="insight-retry-button"/); }); + it("renders the legacy v1.4.14 payload without crashing (regenerate CTA)", () => { + // Reproduces Marc's production /insights crash on 2026-05-10: + // + // TypeError: Cannot read properties of undefined (reading 'replace') + // at stripChartTokens(insight.summary) + // + // The cached blob in the DB predates v1.4.16's strict insight schema + // (no `summary`, no `recommendations[]`, no `findings[]`, no + // `dataQuality`, no `disclaimer` — it carries the old `changed`, + // `stable`, `drivers`, `nextSteps`, `confidence`, `limitations` + // shape). The route's `safeParse(parsed)` fails and falls through + // to `insights = parsed`, so the legacy blob reaches the card with + // `summary === undefined`. The card must surface a regenerate CTA + // and skip rendering the prose, NOT crash. + const legacy = { + changed: "Long-term improvement on weight and BP.", + stable: "Pulse remains stable.", + drivers: "Weight reduction may have contributed.", + nextSteps: "Keep going.", + confidence: "hoch", + limitations: "Correlations don't imply causation.", + } as unknown as InsightResult; + + const html = renderToStaticMarkup( + + {}} + /> + , + ); + + // No crash means the test got here. Surface the regenerate CTA so + // the user has a one-click escape from the legacy state. + expect(html).toContain('data-slot="insight-legacy-payload-cta"'); + }); + it("error state without onRegenerate hides the retry button", () => { const html = renderToStaticMarkup( diff --git a/src/components/insights/insight-advisor-card.tsx b/src/components/insights/insight-advisor-card.tsx index 98111c910..dffae88e9 100644 --- a/src/components/insights/insight-advisor-card.tsx +++ b/src/components/insights/insight-advisor-card.tsx @@ -432,6 +432,80 @@ export function InsightAdvisorCard({ ); } + // ── Legacy-Payload State ────────────────────────────── + // v1.4.17 hotfix: when the cached blob predates the v1.4.16 strict + // schema (no `summary`, no `recommendations[]`, no `findings[]` — + // the v1.4.14 `{changed, stable, drivers, ...}` shape), the rich-card + // render path below would crash on `stripChartTokens(undefined)`. + // Surface the regenerate CTA in a self-contained card and skip the + // rich body entirely. The `legacyPayload` prop is the explicit + // signal from the API; we also defensively short-circuit when the + // shape doesn't carry the modern `summary` string at all. + const isUnrenderable = + typeof insight.summary !== "string" || + !Array.isArray(insight.findings) || + !Array.isArray(insight.recommendations); + if (legacyPayload || isUnrenderable) { + return ( + + +
+
+ {icon ?? } + + {t("insights.aiAnalysisTitle")} + +
+ {onRegenerate && ( + + )} +
+

{title}

+ + +
+ +
+

{t("insights.recommendation.legacyPayloadCta")}

+ {onRegenerate && ( + + )} +
+
+ + + ); + } + // ── Full Insight Card ───────────────────────────────── return ( diff --git a/src/lib/ai/__tests__/legacy-payload-detection.test.ts b/src/lib/ai/__tests__/legacy-payload-detection.test.ts index 43974d3b7..9f5a3b01a 100644 --- a/src/lib/ai/__tests__/legacy-payload-detection.test.ts +++ b/src/lib/ai/__tests__/legacy-payload-detection.test.ts @@ -127,4 +127,26 @@ describe("isLegacyInsightPayload()", () => { it("returns false when recommendations is missing entirely (not legacy, just empty)", () => { expect(isLegacyInsightPayload({ summary: "x" })).toBe(false); }); + + it("returns true for the v1.4.14 pre-strict-schema shape (Marc's prod blob)", () => { + // The actual cached blob that crashed /insights for Marc on + // 2026-05-10. No `summary`, no `recommendations[]` — just the + // pre-v1.4.16 `{changed, stable, drivers, nextSteps, confidence, + // limitations}` shape. The route's `safeParse` failed and fell + // through to the raw blob, so the rich card got `summary === + // undefined` and crashed inside `stripChartTokens(undefined)`. + const payload = { + changed: "Long-term improvement on weight and BP.", + stable: "Pulse remains stable.", + drivers: "Weight reduction may have contributed.", + nextSteps: "Keep going.", + confidence: "hoch", + limitations: "Correlations don't imply causation.", + }; + expect(isLegacyInsightPayload(payload)).toBe(true); + }); + + it("returns true for an empty-summary v1.4.14-style blob (no fields)", () => { + expect(isLegacyInsightPayload({})).toBe(true); + }); }); diff --git a/src/lib/ai/legacy-payload.ts b/src/lib/ai/legacy-payload.ts index 875f0c6bd..9f7e45a15 100644 --- a/src/lib/ai/legacy-payload.ts +++ b/src/lib/ai/legacy-payload.ts @@ -16,23 +16,45 @@ */ /** - * Returns true when the payload is non-empty and at least one - * recommendation lacks a rationale object (or is a plain string from - * the v1.4.14/v1.4.15 InsightResult shape). + * Returns true when the payload predates the v1.4.16 strict insight + * shape and the UI must surface the regenerate CTA instead of trying + * to render the rich card. * + * Detection branches: * - Non-object / null / undefined input → false (nothing to migrate). - * - Missing recommendations[] → false (refusal payload, not legacy). - * - Empty recommendations[] → false (no recs to migrate). - * - Any rec is a plain string → true. - * - Any rec is an object missing `rationale` → true. + * - **v1.4.14 pre-strict shape** (`{changed, stable, drivers, ...}` — + * no `summary`, no `recommendations[]`, no `findings[]`): true. This + * is the production crash Marc hit on 2026-05-10 — the route's + * `safeParse` failed against `insightResultSchema`, fell through to + * the raw blob, and the rich card called `.replace()` on + * `undefined`. Detect by absence of *both* `summary` (string) and + * `recommendations` (array). + * - **v1.4.15 mid-shape** (recommendations[] present, all strings): + * true (plain-string recs predate B5c rationale). + * - **v1.4.16 pre-B5c shape** (recommendations[] present, structured, + * missing `rationale`): true. * - Otherwise → false. */ export function isLegacyInsightPayload(payload: unknown): boolean { if (payload === null || payload === undefined) return false; if (typeof payload !== "object") return false; - const recs = (payload as { recommendations?: unknown }).recommendations; - if (!Array.isArray(recs)) return false; + const obj = payload as { summary?: unknown; recommendations?: unknown }; + + const recs = obj.recommendations; + const hasSummary = typeof obj.summary === "string" && obj.summary.length > 0; + const hasRecsArray = Array.isArray(recs); + + // v1.4.14 / pre-strict shape: neither the modern `summary` string nor + // a `recommendations` array is present. The blob still parses as + // JSON but the rich card surface has nothing to render. + if (!hasSummary && !hasRecsArray) return true; + + // Modern-shape but recs missing entirely — refusal payload, not + // legacy. Don't gate the regenerate CTA on this. + if (!hasRecsArray) return false; + if (!Array.isArray(recs)) return false; // narrowing for TS if (recs.length === 0) return false; + for (const rec of recs) { // Legacy InsightResult shape: string-only recommendations[] if (typeof rec === "string") return true; diff --git a/src/lib/insights/__tests__/chart-tokens.test.ts b/src/lib/insights/__tests__/chart-tokens.test.ts index b2ce4ec57..326330427 100644 --- a/src/lib/insights/__tests__/chart-tokens.test.ts +++ b/src/lib/insights/__tests__/chart-tokens.test.ts @@ -55,6 +55,29 @@ describe("stripChartTokens", () => { it("trims leading and trailing whitespace", () => { expect(stripChartTokens(" metric:WEIGHT ")).toBe(""); }); + + // v1.4.17 hotfix — defensive: reading `insight.summary` on a legacy + // cached payload (no `summary` field) returns undefined. Crashing + // with `Cannot read properties of undefined (reading 'replace')` was + // the production /insights bug Marc hit on 2026-05-10. Treat + // null/undefined as the empty string instead. + it("returns empty string for undefined input", () => { + expect(stripChartTokens(undefined)).toBe(""); + }); + + it("returns empty string for null input", () => { + expect(stripChartTokens(null)).toBe(""); + }); +}); + +describe("parseChartTokens — defensive (v1.4.17)", () => { + it("returns empty array for undefined input", () => { + expect(parseChartTokens(undefined)).toEqual([]); + }); + + it("returns empty array for null input", () => { + expect(parseChartTokens(null)).toEqual([]); + }); }); describe("tokenToMetric", () => { diff --git a/src/lib/insights/chart-tokens.ts b/src/lib/insights/chart-tokens.ts index d4cd58686..d4535c193 100644 --- a/src/lib/insights/chart-tokens.ts +++ b/src/lib/insights/chart-tokens.ts @@ -49,14 +49,32 @@ const TOKEN_REGEX = /metric:[A-Z_]+/g; * Find every well-formed chart token in `text` and return only those that * are in the allowlist. Hallucinated tokens (e.g. `metric:NUKE`) are * silently dropped. + * + * v1.4.17 hotfix: legacy cached payloads from before the strict insight + * schema landed can deliver `undefined` here when the consumer reads a + * field that didn't exist in the v1.4.14 shape (e.g. `insight.summary` + * on a `{changed, stable, drivers, ...}` blob). Rather than crashing + * the entire `/insights` page on a `text.match` of undefined, treat + * non-string input as empty — the legacy-payload CTA owns the user- + * facing recovery path. */ -export function parseChartTokens(text: string): ChartToken[] { +export function parseChartTokens(text: string | null | undefined): ChartToken[] { + if (typeof text !== "string") return []; const matches: string[] = text.match(TOKEN_REGEX) ?? []; return matches.filter((m): m is ChartToken => ALLOWED_SET.has(m)); } -/** Strip the chart tokens out of the visible insight text. */ -export function stripChartTokens(text: string): string { +/** + * Strip the chart tokens out of the visible insight text. + * + * v1.4.17 hotfix: same defensive contract as `parseChartTokens()` — + * non-string input returns the empty string instead of crashing the + * caller. The empty result keeps the surrounding JSX render path + * (`

{stripChartTokens(insight.summary)}

`) alive so the legacy- + * payload CTA above it gets a chance to surface to the user. + */ +export function stripChartTokens(text: string | null | undefined): string { + if (typeof text !== "string") return ""; return text .replace(TOKEN_REGEX, "") .replace(/\s{2,}/g, " ")