mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(insights): retry the comprehensive JSON parse once before failing
The comprehensive generator did an ad-hoc JSON.parse with no corrective retry and failed cold to `invalid-json` on a first-pass miss, while the strict insight wrapper's `buildRetryCorrectionMessage` sat right next door. Export that helper and reuse it: on a parse miss, append the correction to the user prompt and re-run the chain once before declaring failure. Unifies the JSON-robustness story across the two paths.
This commit is contained in:
1 parent
853a1cd301
commit
57cf8340e2
3 files changed
+194
-11
No files matched your search
@@ -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}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<string, unknown> | 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<string, unknown>);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<string, unknown>;
|
||||
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
|
||||
|
||||
Reference in new issue
Block a user