From 6abb905a4e5d96b98e1eafb676cdd55946e73465 Mon Sep 17 00:00:00 2001
From: RealDiligent <brave.challenge007@gmail.com>
Date: Sun, 26 Jul 2026 08:39:05 +0800
Subject: [PATCH] fix(orb): break to the fallback model on a rate limit in
 e2e-test-gen
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

runWorkersE2eTestGen's per-model retry catch had no rate-limit check, so a 429
burned all 3 attempts against the same rate-limited model (half the total call
budget) before reaching the fallback model — despite the doc comment claiming
it "mirrors runWorkersSlopOpinion's exact shape", which does break on a 429.
Adds the same isRateLimitError(error) break the sibling loops use.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/services/ai-e2e-test-gen.ts   |  9 +++++++--
 test/unit/ai-e2e-test-gen.test.ts | 21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/services/ai-e2e-test-gen.ts b/src/services/ai-e2e-test-gen.ts
index 9e714af11c..3c796a128a 100644
--- a/src/services/ai-e2e-test-gen.ts
+++ b/src/services/ai-e2e-test-gen.ts
@@ -39,6 +39,7 @@ import {
   coerceAiUsage,
   estimateNeurons,
   isEnabled,
+  isRateLimitError,
   utcDayStartIso,
 } from "./ai-review";
 
@@ -214,8 +215,12 @@ async function runWorkersE2eTestGen(env: Env, system: string, user: string, maxT
         );
         const parsed = parseE2eTestGenResponse(coerceAiText(result));
         if (parsed) return { testSource: parsed, usage: coerceAiUsage(result) };
-      } catch {
-        /* retry / fall through to fallback */
+      } catch (error) {
+        // #8672: a 429 will not have cleared by the next attempt a few hundred ms later, so retrying THIS
+        // model burns the remaining budget for zero additional chance of success -- move straight to the
+        // fallback model instead (the same guard runWorkersSlopOpinion/runWorkersOpinion already apply).
+        if (isRateLimitError(error)) break;
+        /* non-rate-limit error: retry this model / fall through to the fallback model */
       }
     }
   }
diff --git a/test/unit/ai-e2e-test-gen.test.ts b/test/unit/ai-e2e-test-gen.test.ts
index 20b9fca3d3..6b6bc410dc 100644
--- a/test/unit/ai-e2e-test-gen.test.ts
+++ b/test/unit/ai-e2e-test-gen.test.ts
@@ -558,4 +558,25 @@ describe("runWorkersE2eTestGen (internal)", () => {
     const finalFlags = run.mock.calls.map((c) => ((c as unknown[])[1] as { finalAttempt?: boolean }).finalAttempt);
     expect(finalFlags).toEqual([false, false, false, false, false, true]);
   });
+
+  it("breaks to the fallback model immediately on a rate-limit error instead of burning all per-model attempts (#8672)", async () => {
+    // Every call 429s. Before #8672 this burned all 3 attempts per model (6 calls); now a rate limit short-
+    // circuits the inner retry loop after the first attempt on each model — 2 models × 1 attempt = 2 calls.
+    const run = vi.fn(async () => {
+      throw new Error("workers_ai_http_429");
+    });
+    const env = enabledEnv(run);
+    await expect(runWorkersE2eTestGen(env, "system", "user", 1024)).resolves.toEqual({ testSource: null });
+    expect(run).toHaveBeenCalledTimes(2);
+  });
+
+  it("still burns all per-model attempts on a NON-rate-limit error (the retry path is unchanged) (#8672)", async () => {
+    // A transient non-429 error keeps the original behavior: 2 models × 3 attempts = 6 calls.
+    const run = vi.fn(async () => {
+      throw new Error("transient_timeout");
+    });
+    const env = enabledEnv(run);
+    await runWorkersE2eTestGen(env, "system", "user", 1024);
+    expect(run).toHaveBeenCalledTimes(6);
+  });
 });