fix(selfhost): force JSON mode on OpenAI-compatible review calls and stop retrying deterministic-identical outputs

src/selfhost/ai.ts

@@ -33,6 +33,12 @@ interface AiRunOptions {
   // (embeddings, the subscription CLIs, Anthropic) ignores it, so it is safe to set unconditionally on a
   // call that ONLY ever targets an Ollama-backed binding (e.g. AI_VISION).
   providerOptions?: Record<string, unknown>;
+  // #8790: the caller expects a JSON object back (the PR-review prompt's contract). Only
+  // createOpenAiCompatibleAi's chat path reads this — forwarded as OpenAI's `response_format` so a local
+  // model (confirmed live: an Ollama fallback answering the review prompt in markdown prose on every
+  // attempt) is FORCED into JSON mode instead of merely asked. Every other provider ignores it (the
+  // subscription CLIs already comply via the prompt), so it is safe to set unconditionally on review calls.
+  responseFormat?: "json_object";
   // Correlation context for a provider-failure log (#codex-timeout-fields): purely observational, never read by a
   // provider's own request logic. The caller (runWorkersOpinion) passes whatever of these it already has in scope
   // for THIS review — job id and attempt are per-attempt, repoFullName/pullNumber identify the PR being reviewed —
@@ -366,18 +372,33 @@ export function createOpenAiCompatibleAi(opts: {
       }
       const repoOverride = opts.providerName ? resolveOpenAiCompatibleRepoOverride(opts.providerName, options) : undefined;
       const resolvedModel = resolveModel(firstConfigured(repoOverride, opts.model), model, opts.defaultModel ?? DEFAULT_OPENAI_COMPATIBLE_CHAT_MODEL);
-      const res = await fetch(`${base}/chat/completions`, {
-        method: "POST",
-        headers: headers(),
-        body: JSON.stringify({
+      const chatBody = (withResponseFormat: boolean): string =>
+        JSON.stringify({
           model: resolvedModel,
           messages: toMessages(options).map((message) => ({ role: message.role, content: toOpenAiMessageContent(message.content) })),
           max_tokens: options.max_tokens,
           temperature: options.temperature,
           ...(options.providerOptions ? { options: options.providerOptions } : {}),
-        }),
+          // #8790: force JSON mode when the caller declared a JSON contract — Ollama's and vLLM's
+          // OpenAI-compatible layers both honor it; servers that don't get the 400-fallback below.
+          ...(withResponseFormat && options.responseFormat === "json_object" ? { response_format: { type: "json_object" } } : {}),
+        });
+      let res = await fetch(`${base}/chat/completions`, {
+        method: "POST",
+        headers: headers(),
+        body: chatBody(true),
         signal: AbortSignal.timeout(120_000),
       });
+      // #8790: an older OpenAI-compatible server may reject the response_format parameter outright with a
+      // 400. Retry once without it — degrading to the pre-#8790 ask-nicely behavior beats failing the call.
+      if (!res.ok && res.status === 400 && options.responseFormat === "json_object") {
+        res = await fetch(`${base}/chat/completions`, {
+          method: "POST",
+          headers: headers(),
+          body: chatBody(false),
+          signal: AbortSignal.timeout(120_000),
+        });
+      }
       if (!res.ok) throw new Error(`ai_http_${res.status}`);
       const data = (await res.json()) as { choices?: Array<{ message?: { content?: string } }> };
       const usage = extractCliUsage(JSON.stringify(data));

src/services/ai-review.ts

@@ -476,7 +476,7 @@ export type ModelReview = {
 export type AiReviewDiagnostic = {
   model: string;
   attempt: number;
-  status: "parsed" | "empty_output" | "unparseable_output" | "provider_error" | "missing_assessment";
+  status: "parsed" | "empty_output" | "unparseable_output" | "provider_error" | "missing_assessment" | "identical_retry_skipped";
   responseChars?: number | undefined;
   hasJsonObject?: boolean | undefined;
   error?: string | undefined;
@@ -1189,6 +1189,12 @@ async function runWorkersOpinion(
     if (modelIndex > 0) {
       incr("loopover_ai_review_model_fallback_total", { primary, fallback: model });
     }
+    // #8790: the previous attempt's raw output for THIS model. Reviews run at temperature 0, so a
+    // byte-identical repeat is deterministic — the remaining retries are provably useless (confirmed live
+    // 2026-07-26: a fallback returned the same 2,814-char markdown response on all 3 attempts). Same
+    // stop-retrying-this-model reasoning as the deliberate-bail/timeout/429 breaks below; the next model
+    // still gets its own full budget.
+    let lastRawText: string | undefined;
     for (let attempt = 0; attempt < 3; attempt += 1) {
       try {
         const cliSystemAppend = selfHostCliSystemAppend(model, systemAppend);
@@ -1197,6 +1203,10 @@ async function runWorkersOpinion(
           {
             max_tokens: maxTokens,
             temperature: 0,
+            // #8790: the review prompt's contract is a JSON object — declare it so an OpenAI-compatible
+            // provider is forced into JSON mode (response_format) instead of merely asked. Ignored by every
+            // other provider (the subscription CLIs already comply via the prompt).
+            responseFormat: "json_object",
             messages: [
               { role: "system", content: system },
               { role: "user", content: toContentBlocks(user, images) },
@@ -1232,6 +1242,22 @@ async function runWorkersOpinion(
         const text = coerceAiText(result);
         const usage = coerceAiUsage(result);
         const usageFields = usage ? { usage } : {};
+        // #8790: byte-identical to the previous attempt's (necessarily failed — a success returns) output →
+        // deterministic repeat; stop this model's retries instead of burning the rest of the budget on it.
+        if (attempt > 0 && text.trim() !== "" && text === lastRawText) {
+          diagnostics.push({ model, attempt, status: "identical_retry_skipped", responseChars: text.length, hasJsonObject: Boolean(extractLastJsonObject(text)), ...usageFields });
+          console.warn(
+            JSON.stringify({
+              level: "warn",
+              event: "ai_review_provider_identical_retry_skipped",
+              model,
+              attempt,
+              responseChars: text.length,
+            }),
+          );
+          break;
+        }
+        lastRawText = text;
         const parsed = parseModelReview(text);
         if (parsed && parsed.assessment.trim() !== "") {
           diagnostics.push({ model, attempt, status: "parsed", responseChars: text.length, hasJsonObject: Boolean(extractLastJsonObject(text)), ...usageFields });

test/unit/ai-review-advisory.test.ts

@@ -760,6 +760,49 @@ describe("runAiReviewForAdvisory", () => {
     captureSpy.mockRestore();
   });
 
+  it("#8790: stops retrying a model after a byte-identical repeat of a failed attempt (deterministic at temperature 0)", async () => {
+    const adv = advisory();
+    let aiCalls = 0;
+    // The same unparseable markdown every time — the confirmed 2026-07-26 production shape. Attempt 0 fails,
+    // attempt 1 comes back byte-identical → the remaining retry budget is provably useless and is skipped.
+    const env = aiEnv(async () => {
+      aiCalls += 1;
+      return { response: "### Review of Code Changes\n\nProse, not JSON." };
+    });
+    const result = await runAiReviewForAdvisory(env, {
+      mode: "live",
+      settings: { aiReviewMode: "block" } as RepositorySettings,
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pr,
+      author: "alice",
+      confirmedContributor: true,
+    });
+    expect(result?.findings).toEqual([expect.objectContaining({ code: "ai_review_inconclusive" })]);
+    // This harness's plan resolves 4 model slots (two reviewers x primary+fallback); the invariant under
+    // test is per-slot: exactly 2 attempts each (attempt 0 + the identical attempt 1), never a third.
+    expect(aiCalls).toBe(8);
+  });
+
+  it("#8790: a model whose failed outputs DIFFER between attempts keeps its full retry budget (only determinism short-circuits)", async () => {
+    const adv = advisory();
+    let aiCalls = 0;
+    const env = aiEnv(async () => {
+      aiCalls += 1;
+      return { response: `### Prose attempt ${aiCalls}` };
+    });
+    await runAiReviewForAdvisory(env, {
+      mode: "live",
+      settings: { aiReviewMode: "block" } as RepositorySettings,
+      advisory: adv,
+      repoFullName: "acme/widgets",
+      pr,
+      author: "alice",
+      confirmedContributor: true,
+    });
+    expect(aiCalls).toBe(12); // 3 attempts x the same 4 model slots — pre-#8790 behavior pinned for varying output
+  });
+
   it("uses the non-cacheable block-mode inconclusive note when no reviewer returns public text", async () => {
     const adv = advisory();
     const result = await runAiReviewForAdvisory(aiEnv(async () => ({ response: "" })), {

test/unit/ai-review.test.ts

@@ -1449,8 +1449,9 @@ describe("Workers AI fallback + degraded output", () => {
     const result = await runLoopOverAiReview(env, baseInput);
     expect(result.status === "ok" && result.advisoryNotes).toBeNull();
     expect(result.status === "ok" && result.inconclusive).toBe(true);
-    // primary 3× + fallback 3× retries, all unparseable.
-    expect(run).toHaveBeenCalledTimes(6);
+    // #8790: identical unparseable output on every call → each model stops after its byte-identical
+    // attempt 1 (2 calls per model) instead of burning the full 3-attempt budget on a deterministic repeat.
+    expect(run).toHaveBeenCalledTimes(4);
   });
 });
 
@@ -3259,7 +3260,10 @@ describe("pure helpers", () => {
     // attempt ever produced the required assessment field.
     expect(parsed.review?.assessment).toBe("");
     expect(parsed.review?.nits).toEqual(["Edge case on empty input is untested.", "Naming could be clearer."]);
-    expect(run).toHaveBeenCalledTimes(6); // 3 attempts x 2 models -- the full budget, since nothing here is a deliberate bail.
+    // #8790: the mock returns byte-identical output every call, so each model stops after its identical
+    // attempt 1 (2 calls per model). The incomplete-review fallback + exhausted log below are unaffected —
+    // attempt 0 already captured bestIncompleteReview.
+    expect(run).toHaveBeenCalledTimes(4);
     const exhausted = logSpy.mock.calls
       .map((c) => c[0])
       .find((l) => typeof l === "string" && l.includes("ai_review_missing_assessment_exhausted"));

test/unit/selfhost-ai.test.ts

@@ -178,6 +178,48 @@ describe("createOpenAiCompatibleAi (#979)", () => {
     expect(first?.body.model).toBe("llama3.1");
   });
 
+  it("#8790: forwards response_format json_object when the caller declares a JSON contract, and omits it otherwise", async () => {
+    const bodies: Array<Record<string, unknown>> = [];
+    vi.stubGlobal("fetch", vi.fn(async (_url: string, init: { body: string }) => {
+      bodies.push(JSON.parse(init.body));
+      return new Response(JSON.stringify({ choices: [{ message: { content: "{}" } }] }), { status: 200 });
+    }));
+    const ai = createOpenAiCompatibleAi({ baseUrl: "http://o/v1" });
+    await ai.run("m", { prompt: "x", responseFormat: "json_object" });
+    expect(bodies[0]).toMatchObject({ response_format: { type: "json_object" } });
+    await ai.run("m", { prompt: "x" });
+    expect("response_format" in bodies[1]!).toBe(false);
+  });
+
+  it("#8790: retries ONCE without response_format when the server 400s on it — degrade to ask-nicely, never fail the call", async () => {
+    const bodies: Array<Record<string, unknown>> = [];
+    vi.stubGlobal("fetch", vi.fn(async (_url: string, init: { body: string }) => {
+      const body = JSON.parse(init.body) as Record<string, unknown>;
+      bodies.push(body);
+      if ("response_format" in body) return new Response("unknown parameter", { status: 400 });
+      return new Response(JSON.stringify({ choices: [{ message: { content: "ok" } }] }), { status: 200 });
+    }));
+    const ai = createOpenAiCompatibleAi({ baseUrl: "http://o/v1" });
+    const out = await ai.run("m", { prompt: "x", responseFormat: "json_object" });
+    expect(out.response).toBe("ok");
+    expect(bodies).toHaveLength(2);
+    expect("response_format" in bodies[1]!).toBe(false);
+  });
+
+  it("#8790: a 400 WITHOUT a declared JSON contract still throws ai_http_400 — the retry is format-rejection-specific", async () => {
+    vi.stubGlobal("fetch", vi.fn(async () => new Response("bad request", { status: 400 })));
+    const ai = createOpenAiCompatibleAi({ baseUrl: "http://o/v1" });
+    await expect(ai.run("m", { prompt: "x" })).rejects.toThrow("ai_http_400");
+  });
+
+  it("#8790: a NON-400 failure with the JSON contract set throws without a format-stripping retry (only 400 means parameter rejection)", async () => {
+    const fetchMock = vi.fn(async () => new Response("upstream broke", { status: 500 }));
+    vi.stubGlobal("fetch", fetchMock);
+    const ai = createOpenAiCompatibleAi({ baseUrl: "http://o/v1" });
+    await expect(ai.run("m", { prompt: "x", responseFormat: "json_object" })).rejects.toThrow("ai_http_500");
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
   it("attributes usage.provider from its own configured providerName (#ai-usage-provider-attribution) since an HTTP chat-completions response never reports one itself", async () => {
     vi.stubGlobal("fetch", vi.fn(async () =>
       new Response(JSON.stringify({ choices: [{ message: { content: "ok" } }] }), { status: 200 }),