fix(review): short-circuit the retry loop on a deliberate INCOHERENT_DIFF_ASSESSMENT bail

src/services/ai-review.ts

@@ -793,6 +793,23 @@ export function parseModelReview(text: string): ModelReview | null {
   }
 }
 
+/** True when the model's raw response is specifically the deliberate INCOHERENT_DIFF_ASSESSMENT bail (see that
+ *  constant's own prompt text) rather than a generic parse failure. parseModelReview collapses both into the
+ *  same `null` -- correct for its own contract, since neither yields a usable review -- but the retry loop needs
+ *  to tell them apart: an incoherent-diff bail is the model's deliberate, confident answer about THIS diff and
+ *  will not change on a same-model retry, unlike a truncated/malformed response that might parse fine next time.
+ *  Mirrors parseModelReview's own extraction so this can never disagree with what that function actually parsed. */
+export function isIncoherentDiffBail(text: string): boolean {
+  const jsonText = extractLastJsonObject(text);
+  if (!jsonText) return false;
+  try {
+    const obj = JSON.parse(jsonText) as Record<string, unknown>;
+    return typeof obj.assessment === "string" && obj.assessment.trim() === INCOHERENT_DIFF_ASSESSMENT;
+  } catch {
+    return false;
+  }
+}
+
 // Aggregate ceiling across ALL optional context sections combined (#3900). Each section below already
 // enforces its OWN per-section cap (FILE_CONTENT_BUDGET, MAX_CONTEXT_CHARS, MAX_PROMPT_CHARS,
 // MAX_ENRICHMENT_PROMPT_SECTION_CHARS...), but nothing previously bounded the COMBINED total: with every
@@ -1141,6 +1158,11 @@ async function runWorkersOpinion(
             }),
           );
         }
+        // #ops-review-burst: an INCOHERENT_DIFF_ASSESSMENT bail is the model's deliberate, confident answer about
+        // THIS diff -- not a truncated/malformed response that might parse fine on a same-model retry. Stop
+        // retrying this model (same reasoning as the CLI-timeout/429/structural-config breaks below); the
+        // fallback model below still gets its own full retry budget, since it may reach a different verdict.
+        if (isIncoherentDiffBail(text)) break;
       } catch (error) {
         // Fail-LOUD (#1566): a provider/CLI failure (e.g. the claude-code CLI absent → spawn ENOENT, or an auth/API
         // error) must be VISIBLE, not silently swallowed into a "no usable output" review. Log every failed attempt;

test/unit/ai-review.test.ts

@@ -4,6 +4,8 @@ import {
   BEST_REVIEW_MODELS,
   buildTestEvidencePromptSection,
   callAiProvider,
+  INCOHERENT_DIFF_ASSESSMENT,
+  isIncoherentDiffBail,
   isStructuralProviderConfigError,
   resolveEffectiveAiReviewOnMerge,
   resolveEffectiveAiReviewPlan,
@@ -3074,6 +3076,49 @@ describe("pure helpers", () => {
     expect(run).toHaveBeenCalledTimes(2); // 1 primary (structural failure) + 1 fallback (succeeded on its first try).
   });
 
+  it("runWorkersOpinion stops retrying a model after ONE INCOHERENT_DIFF_ASSESSMENT bail, but the fallback still gets its full retry budget (#ops-review-burst)", async () => {
+    let primaryAttempts = 0;
+    const run = vi.fn(async (model: string) => {
+      if (model === "fallback") return { response: reviewJson() };
+      primaryAttempts += 1;
+      return { response: reviewJson({ assessment: INCOHERENT_DIFF_ASSESSMENT, blockers: [], nits: [], suggestions: [] }) };
+    });
+    const env = createTestEnv({ AI: { run } as unknown as Ai });
+    const diagnostics: Array<{ status: string; model: string }> = [];
+    const parsed = await runWorkersOpinion(env, "primary", "fallback", "sys", "user", 256, diagnostics as never);
+    expect(parsed.review?.assessment).toContain("reasonable");
+    expect(primaryAttempts).toBe(1); // NOT 3 -- the model's own deliberate bail will not change on a same-model retry.
+    expect(run).toHaveBeenCalledTimes(2); // 1 primary (incoherent-diff bail) + 1 fallback (succeeded on its first try).
+    expect(diagnostics[0]).toMatchObject({ model: "primary", attempt: 0, status: "unparseable_output" });
+  });
+
+  it("runWorkersOpinion exhausts all providers when EVERY model bails on INCOHERENT_DIFF_ASSESSMENT, without burning the full retry budget on either", async () => {
+    let totalAttempts = 0;
+    const run = vi.fn(async () => {
+      totalAttempts += 1;
+      return { response: reviewJson({ assessment: INCOHERENT_DIFF_ASSESSMENT, blockers: [], nits: [], suggestions: [] }) };
+    });
+    const env = createTestEnv({ AI: { run } as unknown as Ai });
+    const parsed = await runWorkersOpinion(env, "primary", "fallback", "sys", "user", 256);
+    expect(parsed.review).toBeNull();
+    expect(totalAttempts).toBe(2); // 1 per model, NOT 3 per model (6 total) -- each model's own bail is deliberate.
+  });
+
+  it("isIncoherentDiffBail recognizes exactly the model's own INCOHERENT_DIFF_ASSESSMENT text, not a generic parse failure or a look-alike assessment", () => {
+    expect(isIncoherentDiffBail(reviewJson({ assessment: INCOHERENT_DIFF_ASSESSMENT }))).toBe(true);
+    // Real-world shape (LOOPOVER-29): empty blockers/nits/suggestions alongside the bail assessment.
+    expect(isIncoherentDiffBail(reviewJson({ assessment: INCOHERENT_DIFF_ASSESSMENT, blockers: [], nits: [], suggestions: [] }))).toBe(true);
+    expect(isIncoherentDiffBail(reviewJson({ assessment: "The change looks reasonable and focused." }))).toBe(false);
+    expect(isIncoherentDiffBail(reviewJson({ assessment: `${INCOHERENT_DIFF_ASSESSMENT} (extra prose)` }))).toBe(false);
+    expect(isIncoherentDiffBail("not json at all")).toBe(false);
+    expect(isIncoherentDiffBail("")).toBe(false);
+    // A JSON object whose assessment isn't a string at all (extractLastJsonObject still finds the object).
+    expect(isIncoherentDiffBail(JSON.stringify({ assessment: 42 }))).toBe(false);
+    // Brace-balanced (extractLastJsonObject finds it) but not valid JSON (single-quoted, not double-quoted) --
+    // exercises the try/catch around JSON.parse, not just the "no JSON object at all" early return above.
+    expect(isIncoherentDiffBail("{ 'assessment': 'not valid JSON' }")).toBe(false);
+  });
+
   it("isStructuralProviderConfigError matches only codex's own structural-config error messages, not other Errors or non-Error throws (GITTENSORY-K/8)", () => {
     expect(isStructuralProviderConfigError(new Error("codex_auth_not_configured: ~/.codex/auth.json not found"))).toBe(true);
     expect(isStructuralProviderConfigError(new Error("codex_no_auth: auth.json missing or expired"))).toBe(true);