Fix guaranteed JSON retry recovery (#52)

Moskize91 · web-flow · commit 686c0d91fe2d · 2026-06-09T23:33:39.000+08:00
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "spinedigest",
-  "version": "0.1.7",
+  "version": "0.1.8",
   "description": "CLI-first tool for digesting long-form text and ebooks into compressed text, EPUB, or reusable .sdpub archives.",
   "license": "Apache-2.0",
   "type": "module",
diff --git a/src/editor/review.ts b/src/editor/review.ts
@@ -1,6 +1,8 @@
 import { z } from "zod";
 
+import { getLogger } from "../common/logging.js";
 import {
+  GuaranteedRequestFailureError,
   requestGuaranteedJson,
   RESPONSE_INTENT_CLASSIFIER_PROMPT_TEMPLATE,
 } from "../guaranteed/index.js";
@@ -125,31 +127,54 @@ export class CompressionReviewer<S extends string> {
           previousHistory,
         );
 
-        return await requestGuaranteedJson({
-          messages,
-          parse: (data) => ({
-            rawResponse: JSON.stringify(data),
+        try {
+          return await requestGuaranteedJson({
+            messages,
+            parse: (data) => ({
+              rawResponse: JSON.stringify(data),
+              review: {
+                clueId: clueReviewer.clueId,
+                issues: data.issues.map((issue) => ({
+                  ...issue,
+                  severity: expectReviewSeverity(issue.severity),
+                })),
+                weight: clueReviewer.weight,
+              },
+            }),
+            responseIntentClassifierPrompt: this.#llm.loadSystemPrompt(
+              RESPONSE_INTENT_CLASSIFIER_PROMPT_TEMPLATE,
+            ),
+            request: async (retryMessages, retryIndex, retryMax) =>
+              await this.#llm.request(retryMessages, {
+                retryIndex,
+                retryMax,
+                scope: this.#reviewScope,
+                useCache: false,
+              }),
+            schema: reviewResponseSchema,
+          });
+        } catch (error) {
+          if (!(error instanceof GuaranteedRequestFailureError)) {
+            throw error;
+          }
+
+          getLogger({
+            clueId: clueReviewer.clueId,
+            component: "editor-review",
+          }).warn(
+            { error },
+            "Compression reviewer failed to produce valid JSON; treating it as no issues.",
+          );
+
+          return {
+            rawResponse: undefined,
             review: {
               clueId: clueReviewer.clueId,
-              issues: data.issues.map((issue) => ({
-                ...issue,
-                severity: expectReviewSeverity(issue.severity),
-              })),
+              issues: [],
               weight: clueReviewer.weight,
             },
-          }),
-          responseIntentClassifierPrompt: this.#llm.loadSystemPrompt(
-            RESPONSE_INTENT_CLASSIFIER_PROMPT_TEMPLATE,
-          ),
-          request: async (retryMessages, retryIndex, retryMax) =>
-            await this.#llm.request(retryMessages, {
-              retryIndex,
-              retryMax,
-              scope: this.#reviewScope,
-              useCache: false,
-            }),
-          schema: reviewResponseSchema,
-        });
+          };
+        }
       }),
     );
     const rawResponses = Object.create(null) as Record<
diff --git a/src/guaranteed/request.ts b/src/guaranteed/request.ts
@@ -24,14 +24,16 @@ import {
 } from "./response.js";
 import type { GuaranteedRequestOptions } from "./types.js";
 
-const DEFAULT_MAX_RETRIES = 7;
+const DEFAULT_MAX_RETRIES = 12;
+const MAX_MALFORMED_JSON_REPAIR_HISTORY_ATTEMPTS = 3;
 
 export async function requestGuaranteedJson<TData, TResult>(
   options: GuaranteedRequestOptions<TData, TResult>,
 ): Promise<TResult> {
   const initialMessages = [...options.messages];
   let currentMessages = [...options.messages];
   let consecutiveProtocolDerailments = 0;
+  let consecutiveMalformedJsonFailures = 0;
   const maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES;
 
   for (let index = 0; index <= maxRetries; index += 1) {
@@ -54,6 +56,7 @@ export async function requestGuaranteedJson<TData, TResult>(
 
       if (intent === "natural_language") {
         consecutiveProtocolDerailments += 1;
+        consecutiveMalformedJsonFailures = 0;
 
         if (consecutiveProtocolDerailments >= 2 || index >= maxRetries) {
           const reason =
@@ -76,17 +79,20 @@ export async function requestGuaranteedJson<TData, TResult>(
       }
 
       consecutiveProtocolDerailments = 0;
+      consecutiveMalformedJsonFailures += 1;
       currentMessages = buildRetryMessages(
         initialMessages,
         response,
         intent === "malformed_json"
           ? buildMalformedJsonMessage(asSyntaxError(error))
           : buildSyntaxErrorMessage(asSyntaxError(error)),
-        true,
+        consecutiveMalformedJsonFailures <
+          MAX_MALFORMED_JSON_REPAIR_HISTORY_ATTEMPTS,
       );
       continue;
     }
     consecutiveProtocolDerailments = 0;
+    consecutiveMalformedJsonFailures = 0;
 
     const validation = await options.schema.safeParseAsync(parsedData);
 
@@ -98,6 +104,7 @@ export async function requestGuaranteedJson<TData, TResult>(
 
         if (intent === "natural_language") {
           consecutiveProtocolDerailments += 1;
+          consecutiveMalformedJsonFailures = 0;
 
           if (consecutiveProtocolDerailments >= 2 || index >= maxRetries) {
             const reason =
diff --git a/test/editor/review.test.ts b/test/editor/review.test.ts
@@ -105,6 +105,73 @@ describe("editor/review", () => {
       ["system", "user", "assistant", "user"],
     );
   });
+
+  it("falls back to no issues when a reviewer cannot produce valid JSON", async () => {
+    const llm = new ScriptedLLM<SpineDigestScope>(Array(13).fill(""));
+    const reviewer = new CompressionReviewer(
+      llm as never,
+      createSerialFragments(),
+      {
+        review: SPINE_DIGEST_EDITOR_SCOPES.review,
+        reviewGuide: SPINE_DIGEST_EDITOR_SCOPES.reviewGuide,
+      },
+      Language.English,
+    );
+
+    const result = await reviewer.reviewCompression(
+      "Current compressed text",
+      [
+        {
+          clueId: 1,
+          label: "Alpha clue",
+          reviewerInfo: "Check continuity",
+          weight: 0.8,
+        },
+      ],
+      {},
+    );
+
+    expect(result.rawResponses["1"]).toBeUndefined();
+    expect(result.reviews).toStrictEqual([
+      {
+        clueId: 1,
+        issues: [],
+        weight: 0.8,
+      },
+    ]);
+  });
+
+  it("does not hide regular LLM request errors during review", async () => {
+    const llm = new ScriptedLLM<SpineDigestScope>([
+      () => {
+        throw new Error("network failed");
+      },
+    ]);
+    const reviewer = new CompressionReviewer(
+      llm as never,
+      createSerialFragments(),
+      {
+        review: SPINE_DIGEST_EDITOR_SCOPES.review,
+        reviewGuide: SPINE_DIGEST_EDITOR_SCOPES.reviewGuide,
+      },
+      Language.English,
+    );
+
+    await expect(
+      reviewer.reviewCompression(
+        "Current compressed text",
+        [
+          {
+            clueId: 1,
+            label: "Alpha clue",
+            reviewerInfo: "Check continuity",
+            weight: 0.8,
+          },
+        ],
+        {},
+      ),
+    ).rejects.toThrow("network failed");
+  });
 });
 
 function createChunk(
diff --git a/test/guaranteed/request.test.ts b/test/guaranteed/request.test.ts
@@ -91,6 +91,21 @@ describe("guaranteed/request", () => {
     expect(request).toHaveBeenCalledTimes(2);
   });
 
+  it("uses twelve retries by default", async () => {
+    const request = vi.fn(() => Promise.resolve(""));
+
+    await expect(
+      requestGuaranteedJson({
+        messages: [],
+        parse: (data) => data.value,
+        responseIntentClassifierPrompt: "classifier prompt",
+        request,
+        schema,
+      }),
+    ).rejects.toBeInstanceOf(GuaranteedEmptyResponseError);
+    expect(request).toHaveBeenCalledTimes(13);
+  });
+
   it("throws a parse validation error after exhausting retries", async () => {
     await expect(
       requestGuaranteedJson({
@@ -138,6 +153,48 @@ describe("guaranteed/request", () => {
     expect(secondCallMessages?.[1]?.content).toContain("malformed JSON");
   });
 
+  it("drops malformed JSON history after repeated repair failures", async () => {
+    const request = vi
+      .fn<
+        (
+          messages: readonly LLMessage[],
+          retryIndex: number,
+          retryMax: number,
+        ) => Promise<string>
+      >()
+      .mockResolvedValueOnce('{"value": "\\uZZZZ"}')
+      .mockResolvedValueOnce('{"value": "\\uZZZZ"}')
+      .mockResolvedValueOnce('{"value": "\\uZZZZ"}')
+      .mockResolvedValueOnce('{"value": 11}');
+
+    const result = await requestGuaranteedJson({
+      messages: [
+        {
+          role: "user",
+          content: "Return JSON",
+        },
+      ],
+      parse: (data) => data.value,
+      responseIntentClassifierPrompt: "classifier prompt",
+      request,
+      schema,
+    });
+
+    expect(result).toBe(11);
+
+    const fourthCallMessages = request.mock.calls[3]?.[0];
+
+    expect(fourthCallMessages).toHaveLength(2);
+    expect(fourthCallMessages?.[0]).toMatchObject({
+      role: "user",
+      content: "Return JSON",
+    });
+    expect(fourthCallMessages?.[1]).toMatchObject({
+      role: "user",
+    });
+    expect(fourthCallMessages?.[1]?.content).toContain("malformed JSON");
+  });
+
   it("uses classifier fallback for ambiguous replies before deciding to keep history", async () => {
     const request = vi
       .fn<

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "spinedigest",`
`3`		`- "version": "0.1.7",`
	`3`	`+ "version": "0.1.8",`
`4`	`4`	`"description": "CLI-first tool for digesting long-form text and ebooks into compressed text, EPUB, or reusable .sdpub archives.",`
`5`	`5`	`"license": "Apache-2.0",`
`6`	`6`	`"type": "module",`