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) <noreply@anthropic.com>
This commit is contained in:
Marc-André BombeckandClaude Opus 4.7 committed 2026-05-10 09:49:17 +02:00
1 parent 57de2177a9
commit 79bfa27d65
6 files changed
+210 -12

No files matched your search

@@ -267,6 +267,45 @@ describe("<InsightAdvisorCard> — 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(
<I18nProvider initialLocale="en">
<InsightAdvisorCard
title="Test card"
insight={legacy}
legacyPayload
onRegenerate={() => {}}
/>
</I18nProvider>,
);
// 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(
<I18nProvider initialLocale="en">
@@ -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 (
<Card data-slot="insight-legacy-card">
<CardHeader className="pb-3">
<div className="flex items-center justify-between">
<div className="flex items-center gap-2">
{icon ?? <Sparkles className="text-dracula-purple h-5 w-5" />}
<CardTitle className="text-lg">
{t("insights.aiAnalysisTitle")}
</CardTitle>
</div>
{onRegenerate && (
<Button
variant="ghost"
size="icon"
className="h-7 w-7"
onClick={onRegenerate}
disabled={regenerating}
title={t("insights.refreshAnalysis")}
>
{regenerating ? (
<Loader2 className="h-3.5 w-3.5 animate-spin" />
) : (
<RefreshCw className="h-3.5 w-3.5" />
)}
</Button>
)}
</div>
<p className="text-muted-foreground text-sm">{title}</p>
</CardHeader>
<CardContent>
<div
className="border-dracula-yellow/30 bg-dracula-yellow/10 flex items-start gap-2 rounded-md border px-3 py-2 text-sm"
data-slot="insight-legacy-payload-cta"
>
<Info className="text-dracula-yellow mt-0.5 h-4 w-4 shrink-0" />
<div className="space-y-2">
<p>{t("insights.recommendation.legacyPayloadCta")}</p>
{onRegenerate && (
<Button
variant="outline"
size="sm"
onClick={onRegenerate}
disabled={regenerating}
className="h-7 text-xs"
>
{regenerating ? (
<Loader2 className="mr-1.5 h-3 w-3 animate-spin" />
) : (
<Sparkles className="mr-1.5 h-3 w-3" />
)}
{t("insights.startAnalysis")}
</Button>
)}
</div>
</div>
</CardContent>
</Card>
);
}
// ── Full Insight Card ─────────────────────────────────
return (
<Card className="animate-insight-in">
@@ -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);
});
});
+31 -9
View File
@@ -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;
@@ -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", () => {
+21 -3
View File
@@ -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
* (`<p>{stripChartTokens(insight.summary)}</p>`) 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, " ")