From b95801172ce9eb5356da95bccc87cb80b854677c Mon Sep 17 00:00:00 2001
From: Yuneng Jiang <yuneng@berri.ai>
Date: Wed, 26 Aug 2026 18:57:38 -0700
Subject: [PATCH] fix(e2e): only the negative fallback assertion needs every
 replica to agree

litellm-e2e-ui 68 failed the test this PR was meant to stabilise: "fallback
never took effect", streak 4 of a required 5, 60s timeout. Requiring a
consecutive streak of 200s after the fallback is set was wrong. It asserts that
the fallback path succeeds five times running, which is a reliability claim the
test never intended to make, and the path is inherently retry-ish because the
broken primary is attempted first on every call. One intermittent non-200
resets the streak, so a mostly-working fallback never converges.

The two directions are not symmetric:

  before the write  proving NO replica serves it   -> needs every replica
  after the write   proving the fallback serves it -> one success is the claim

So the control keeps a multi-sample window and the success assertion goes back
to polling for a first sighting, on the wider 60s budget rather than the
original 30s that expired on litellm-e2e-ui 63.

Also drops the two local rebinds Greptile flagged against the repo's
no-reassignment convention: the streak counter is gone with the helper it lived
in, and the cache-round loop is now a lazy generator consumed by next().
---
 .../spend_tracking/test_cost_headers_e2e.py   |  7 +--
 .../ui/tests/settings/routerSettings.spec.ts  | 49 +++++++++----------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/tests/e2e/quota_management/spend_tracking/test_cost_headers_e2e.py b/tests/e2e/quota_management/spend_tracking/test_cost_headers_e2e.py
index a455f9f0db4..abc321ccde8 100644
--- a/tests/e2e/quota_management/spend_tracking/test_cost_headers_e2e.py
+++ b/tests/e2e/quota_management/spend_tracking/test_cost_headers_e2e.py
@@ -102,11 +102,8 @@ class TestCostHeaders:
                     return response
             return None
 
-        measured: StreamingResponse | None = None
-        for _ in range(CACHE_ATTEMPTS):
-            measured = prime_then_reread()
-            if measured is not None:
-                break
+        rounds = (prime_then_reread() for _ in range(CACHE_ATTEMPTS))
+        measured = next((response for response in rounds if response is not None), None)
         if measured is None:
             pytest.fail(
                 f"no cache read landed across {CACHE_ATTEMPTS} prime rounds of "
diff --git a/tests/e2e/ui/tests/settings/routerSettings.spec.ts b/tests/e2e/ui/tests/settings/routerSettings.spec.ts
index ada8e99e5c1..cd64e6e4453 100644
--- a/tests/e2e/ui/tests/settings/routerSettings.spec.ts
+++ b/tests/e2e/ui/tests/settings/routerSettings.spec.ts
@@ -139,24 +139,18 @@ async function patchRouterSettings(
 }
 
 /**
- * Requires a consecutive streak because a single reply only proves the one replica that
- * served it has reloaded, not the sibling still answering from the pre-update config.
+ * Spreads its samples across more than one reload cycle: a single reply only proves the one
+ * replica that served it has reloaded, not the sibling still on the pre-update config.
  */
-async function pollUntilSettled(
-  probe: () => Promise<number>,
-  matches: (status: number) => boolean,
-  message: string,
-): Promise<void> {
-  let streak = 0;
-  await expect
-    .poll(
-      async () => {
-        streak = matches(await probe()) ? streak + 1 : 0;
-        return streak;
-      },
-      { timeout: SETTLE_TIMEOUT_MS, intervals: [SETTLE_INTERVAL_MS], message },
-    )
-    .toBeGreaterThanOrEqual(SETTLE_PROBES);
+async function sampleStatuses(probe: () => Promise<number>): Promise<readonly number[]> {
+  return Array.from({ length: SETTLE_PROBES }).reduce<Promise<readonly number[]>>(
+    async (taken, _unused, index) => {
+      const sofar = await taken;
+      if (index > 0) await new Promise((resolve) => setTimeout(resolve, SETTLE_INTERVAL_MS));
+      return [...sofar, await probe()];
+    },
+    Promise.resolve([]),
+  );
 }
 
 test.describe("Router Settings - Loadbalancing", () => {
@@ -289,19 +283,24 @@ test.describe("Router Settings - Fallbacks serve the request", () => {
         })
       ).status();
 
-    // The control: it proves the reply below could only have come from the fallback.
-    await pollUntilSettled(
-      chatStatus,
-      (status) => status >= 400,
-      "broken primary unexpectedly succeeded on its own",
-    );
+    // The control: every replica must reject, or the reply below could have come from one
+    // that was still serving a fallback left behind by an earlier attempt.
+    await expect
+      .poll(async () => (await sampleStatuses(chatStatus)).every((status) => status >= 400), {
+        timeout: SETTLE_TIMEOUT_MS,
+        message: "broken primary unexpectedly succeeded on its own",
+      })
+      .toBe(true);
 
     await patchRouterSettings(request, {
       fallbacks: [{ [BROKEN_PRIMARY]: [PRIMARY] }],
     } as Partial<NonNullable<ConfigYAML["router_settings"]>>);
 
-    // Same call now succeeds, served by the fallback model.
-    await pollUntilSettled(chatStatus, (status) => status === 200, "fallback never took effect");
+    // One success is the whole claim here, so this waits for a first sighting rather than
+    // for every replica: demanding a streak would also assert a fallback hit rate.
+    await expect
+      .poll(chatStatus, { timeout: SETTLE_TIMEOUT_MS, message: "fallback never took effect" })
+      .toBe(200);
 
     // And the playground renders a reply for a model whose own upstream is down.
     await openPlayground(page);