diff --git a/src/lib/ai/generate-insight.ts b/src/lib/ai/generate-insight.ts index 3cb1444f1..4b8fb939c 100644 --- a/src/lib/ai/generate-insight.ts +++ b/src/lib/ai/generate-insight.ts @@ -46,7 +46,10 @@ export interface GenerateInsightOutcome { * what went wrong on the first attempt. Includes the violated zod * issues verbatim (truncated to 1KB) so the model can self-correct. */ -function buildRetryCorrectionMessage(reason: string, details: string): string { +export function buildRetryCorrectionMessage( + reason: string, + details: string, +): string { return ` Your previous response did not satisfy the required JSON schema. Reason: ${reason} diff --git a/src/lib/insights/__tests__/comprehensive-json-retry.test.ts b/src/lib/insights/__tests__/comprehensive-json-retry.test.ts new file mode 100644 index 000000000..047525350 --- /dev/null +++ b/src/lib/insights/__tests__/comprehensive-json-retry.test.ts @@ -0,0 +1,128 @@ +/** + * v1.18.7 (MEDIUM-5) — comprehensive JSON-retry robustness. + * + * The comprehensive path used to fail cold to `invalid-json` on a first-pass + * parse miss. It now reuses `buildRetryCorrectionMessage` for ONE corrective + * retry before declaring failure. These tests pin: a first-pass miss followed + * by a valid retry succeeds; two misses fail; and a first-pass success runs + * no retry. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const findUnique = vi.fn(); +const userUpdate = vi.fn(); +const resolveProviderChain = vi.fn(); +const resolveProvider = vi.fn(); +const runRawCompletionWithFallback = vi.fn(); +const extractFeatures = vi.fn(); + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { + findUnique: (...a: unknown[]) => findUnique(...a), + update: (...a: unknown[]) => userUpdate(...a), + }, + auditLog: { deleteMany: vi.fn() }, + }, +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain: (...a: unknown[]) => resolveProviderChain(...a), + resolveProvider: (...a: unknown[]) => resolveProvider(...a), +})); +vi.mock("@/lib/ai/provider-runner", () => ({ + AllProvidersFailedError: class extends Error {}, + runRawCompletionWithFallback: (...a: unknown[]) => + runRawCompletionWithFallback(...a), +})); +vi.mock("@/lib/insights/features", () => ({ + FeaturesPayloadTooLargeError: class extends Error { + sizeBytes = 0; + }, + extractFeatures: (...a: unknown[]) => extractFeatures(...a), +})); +vi.mock("@/lib/insights/glp1-plateau", () => ({ + detectGlp1Plateau: vi.fn(async () => null), + buildGlp1PlateauPrompt: vi.fn(() => ""), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextTextForUser: vi.fn(async () => null), + buildAboutMeInsightBlock: vi.fn(() => ""), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserInsights: vi.fn(), +})); + +import { generateComprehensiveInsight } from "../comprehensive-generate"; + +const FEATURES = { weight: { count: 12, latest: 81.4, mean30: 82.1 } }; + +beforeEach(() => { + vi.clearAllMocks(); + resolveProviderChain.mockResolvedValue([ + { providerType: "openai", instance: {} }, + ]); + resolveProvider.mockResolvedValue({ type: "none" }); + extractFeatures.mockResolvedValue(FEATURES); + userUpdate.mockResolvedValue({}); + // No cached text / hash → always runs a full generation (no gate hit). + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: null, + insightsCachedText: null, + insightsExcludeMetrics: [], + insightsSnapshotHash: null, + insightsBriefingRerollDate: null, + }); +}); + +const VALID = JSON.stringify({ dailyBriefing: { paragraph: "ok" } }); + +describe("comprehensive JSON-retry", () => { + it("recovers via one corrective retry after a first-pass JSON miss", async () => { + runRawCompletionWithFallback + .mockResolvedValueOnce({ + result: { content: "I'm sorry, here is the data:", tokensUsed: 5, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }) + .mockResolvedValueOnce({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "generated", providerType: "openai" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(2); + // The retry call appends the correction to the user prompt. + const retryParams = runRawCompletionWithFallback.mock.calls[1][0].params; + expect(retryParams.userPrompt).toContain("did not satisfy the required"); + }); + + it("fails with invalid-json when both attempts miss", async () => { + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: "still not json", tokensUsed: 5, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "failed", reason: "invalid-json" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(2); + }); + + it("runs no retry when the first pass is valid", async () => { + runRawCompletionWithFallback.mockResolvedValue({ + result: { content: VALID, tokensUsed: 10, model: "m" }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "generated", providerType: "openai" }); + expect(runRawCompletionWithFallback).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index 97c1b2ea9..7f263103f 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -34,6 +34,7 @@ import { type ComparisonSnapshot, } from "@/lib/ai/prompts/insight-system-prompt"; import { buildSystemPromptWithReferences } from "@/lib/ai/prompts/insight-generator"; +import { buildRetryCorrectionMessage } from "@/lib/ai/generate-insight"; import { buildAboutMeInsightBlock, getSelfContextTextForUser, @@ -234,6 +235,28 @@ async function rerollBriefingParagraph(args: { return { text: JSON.stringify(merged), providerType }; } +/** + * Parse + (best-effort) validate a comprehensive provider reply. Returns the + * parsed payload (validated when it matches `insightResultSchema`, else the + * raw object so the richer optional blocks survive), or null when the reply + * is not valid JSON even after fence-stripping. Null signals the caller to + * run its one corrective retry. + */ +function parseComprehensiveResult( + content: string, +): InsightResult | Record | null { + let parsed: unknown; + try { + parsed = JSON.parse(stripJsonFences(content)); + } catch { + return null; + } + const validated = insightResultSchema.safeParse(parsed); + return validated.success + ? validated.data + : (parsed as Record); +} + /** * Comparison-snapshot builder — shared with the on-demand route. Returns * null when the user's comparison toggle is off (most users), so the @@ -856,16 +879,45 @@ export async function generateComprehensiveInsight( return { status: "failed", reason: "provider-error" }; } - let insights: InsightResult | Record; - try { - // Anthropic + local have no native JSON mode, so a ```json-fenced or - // sentence-prefixed reply would otherwise fail the whole generation. - // Strip the fence before parsing; clean JSON passes through unchanged. - const parsed = JSON.parse(stripJsonFences(result.content)); - const validated = insightResultSchema.safeParse(parsed); - insights = validated.success ? validated.data : parsed; - } catch { - return { status: "failed", reason: "invalid-json" }; + // Anthropic + local have no native JSON mode, so a ```json-fenced or + // sentence-prefixed reply would otherwise fail the whole generation. + // Strip the fence before parsing; clean JSON passes through unchanged. + let insights = parseComprehensiveResult(result.content); + if (insights === null) { + // v1.18.7 (MEDIUM-5) — one corrective retry before declaring failure, + // reusing the same `buildRetryCorrectionMessage` the strict insight + // wrapper uses next door. The comprehensive path previously failed cold + // to `invalid-json` on a first-pass miss even though the recovery helper + // sat right there. The retry appends the correction to the user prompt + // and re-runs the chain once. + annotate({ + action: { name: "insights.generate.json_retry" }, + meta: { locale }, + }); + try { + const retry = await runRawCompletionWithFallback({ + userId, + providers: chain, + params: { + systemPrompt, + userPrompt: `${userPrompt}\n\n${buildRetryCorrectionMessage( + "Response was not valid JSON", + "The previous reply could not be parsed as a JSON object.", + )}`, + temperature: AI_BUDGETS.comprehensive.temperature, + maxTokens: AI_BUDGETS.comprehensive.maxTokens, + responseFormat: "json", + }, + }); + result = retry.result; + workingProviderType = retry.workingProvider.providerType; + } catch { + return { status: "failed", reason: "invalid-json" }; + } + insights = parseComprehensiveResult(result.content); + if (insights === null) { + return { status: "failed", reason: "invalid-json" }; + } } // v1.9.0 — the caller abandoned this generation (its bounded timeout