From ab1da05e5987816047f06f396288fee8d673db1c Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 26 Aug 2026 17:57:14 -0700 Subject: [PATCH 1/3] test(e2e): de-flake the cost-header cache read and the router fallback control Two e2e tests fail on timing rather than on litellm behaviour. Measured over the last ~35 litellm-e2e / litellm-e2e-ui runs: routerSettings.spec.ts:254 9/35 runs (7 flaky-on-retry, 2 hard failures) test_cost_headers_e2e.py 1/29 runs it appeared in Router fallback control ----------------------- The e2e stack runs replicaCount 2 with proxy_config_reload_interval_seconds 7, and every request is routed independently, so an observation of the new config only proves the replica that served it reloaded. patchRouterSettings returns as soon as /config/update returns, and clearBrokenFallback never waits at all, so a retry's one-shot control assertion could be answered by a sibling replica still holding the previous attempt's fallback. That is exactly the observed pair of errors: "fallback never took effect" on the first attempt and "broken primary unexpectedly succeeded on its own" on the retry. Both assertions now poll for a consecutive streak spanning more than one reload cycle, mirroring the PROPAGATION_TIMEOUT / settle_propagation doctrine the Python suite already applies in e2e_config.py. Cost-header cache read ---------------------- The prime and measure calls fired back to back with no gap, and each retry threw away the prefix it had just paid to prime in favour of a fresh one. OpenAI publishes a primed prefix asynchronously and routes cache lookups by prompt_cache_key, so the test was rerolling the least likely path to a hit. Each round now pins a prompt_cache_key and re-reads the same primed prefix up to CACHE_REREADS times before rotating, so a fresh prefix is spent only after the primed one has genuinely failed to become readable. No production code changes; prompt_cache_key is added to the e2e ChatBody model, which serializes exclude_none and so is inert for every other caller. (cherry picked from commit 84dfc18f6bb2e028709775f1797c5dd5710880cc) --- tests/e2e/models.py | 1 + .../spend_tracking/test_cost_headers_e2e.py | 41 +++++++++---- .../ui/tests/settings/routerSettings.spec.ts | 57 ++++++++++++++----- 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/tests/e2e/models.py b/tests/e2e/models.py index 5e2cb90958e..b54621857c9 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -229,6 +229,7 @@ class ChatBody(BaseModel): reasoning_effort: str | None = None thinking: ThinkingParam | None = None service_tier: str | None = None + prompt_cache_key: str | None = None tools: Sequence[ChatTool | McpChatTool] | None = None tool_choice: str | None = None guardrails: list[str] | None = None 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 203be611905..a455f9f0db4 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 @@ -12,11 +12,16 @@ header is exercised with a real nonzero value instead of passing vacuously. The backend is gpt-5.5 because it reports cached tokens on the second call; the gpt-5.6 line reports cache writes and never a read, which would leave the cache-read header at zero forever. The raw-transport send is used because the -typed chat client validates bodies and drops headers. OpenAI caching is -best-effort, so the prime+measure round retries with a fresh prefix before -failing. +typed chat client validates bodies and drops headers. + +OpenAI publishes a primed prefix asynchronously and routes lookups by +prompt_cache_key, so a measure fired the instant the prime returns can miss a +prefix that is about to become readable. Each round pins a cache key and re-reads +the prefix it already paid to prime before spending a fresh one. """ +import time + import pytest from cost_rows import approx_equal, cacheable_prefix, register_priced_model @@ -31,6 +36,8 @@ pytestmark = pytest.mark.e2e BACKEND = "openai/gpt-5.5" OPENAI_API_KEY = "os.environ/OPENAI_API_KEY" CACHE_ATTEMPTS = 3 +CACHE_REREADS = 3 +CACHE_SETTLE_SECONDS = 2.0 INPUT_RATE = 4e-05 OUTPUT_RATE = 8e-05 @@ -70,7 +77,7 @@ class TestCostHeaders: ), ) - def priced_call(content: str) -> StreamingResponse: + def priced_call(content: str, cache_key: str) -> StreamingResponse: response = client.proxy.transport.send( "/chat/completions", headers=client.proxy.transport.bearer(scoped_key), @@ -78,21 +85,33 @@ class TestCostHeaders: model=model, messages=[ChatMessage(role="user", content=content)], max_completion_tokens=4000, + prompt_cache_key=cache_key, ), ) assert response.ok, f"chat failed (status {response.status_code}): {response.body[:300]}" return response + def prime_then_reread() -> StreamingResponse | None: + marker = unique_marker() + prefix = cacheable_prefix(marker) + priced_call(f"{prefix}\nReply with the single word ready.", marker) + for _ in range(CACHE_REREADS): + time.sleep(CACHE_SETTLE_SECONDS) + response = priced_call(f"{prefix}\nReply with the single word measured.", marker) + if _header_cost(response, "x-litellm-response-cost-cache-read") > 0: + return response + return None + + measured: StreamingResponse | None = None for _ in range(CACHE_ATTEMPTS): - prefix = cacheable_prefix(unique_marker()) - priced_call(f"{prefix}\nReply with the single word ready.") - measured = priced_call(f"{prefix}\nReply with the single word measured.") - if _header_cost(measured, "x-litellm-response-cost-cache-read") > 0: + measured = prime_then_reread() + if measured is not None: break - else: + if measured is None: pytest.fail( - f"no cache read landed across {CACHE_ATTEMPTS} prime+measure rounds; " - "the cache-read cost header was never exercised with a nonzero value" + f"no cache read landed across {CACHE_ATTEMPTS} prime rounds of " + f"{CACHE_REREADS} re-reads each; the cache-read cost header was never " + "exercised with a nonzero value" ) total = measured.response_cost diff --git a/tests/e2e/ui/tests/settings/routerSettings.spec.ts b/tests/e2e/ui/tests/settings/routerSettings.spec.ts index 1188e8f201e..ada8e99e5c1 100644 --- a/tests/e2e/ui/tests/settings/routerSettings.spec.ts +++ b/tests/e2e/ui/tests/settings/routerSettings.spec.ts @@ -117,6 +117,11 @@ const ADMIN_AUTH = { Authorization: `Bearer ${users[Role.ProxyAdmin].password}`, }; +// Five probes 2s apart outlast the e2e stack's proxy_config_reload_interval_seconds of 7. +const SETTLE_INTERVAL_MS = 2_000; +const SETTLE_PROBES = 5; +const SETTLE_TIMEOUT_MS = 60_000; + /** * Apply a router_settings patch through the typed /config/update contract. The * server merges it over existing settings (request wins), so only the passed keys @@ -133,6 +138,27 @@ async function patchRouterSettings( expect(res.ok(), `seed /config/update failed: ${res.status()} ${await res.text()}`).toBeTruthy(); } +/** + * 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. + */ +async function pollUntilSettled( + probe: () => Promise, + matches: (status: number) => boolean, + message: string, +): Promise { + 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); +} + test.describe("Router Settings - Loadbalancing", () => { test.use({ storageState: ADMIN_STORAGE_PATH }); @@ -252,29 +278,30 @@ test.describe("Router Settings - Fallbacks serve the request", () => { }); test("a request to an unreachable model is answered by its fallback", async ({ page, request }) => { - const chat = async () => - request.post("/v1/chat/completions", { - headers: { ...ADMIN_AUTH, "Content-Type": "application/json" }, - data: { - model: BROKEN_PRIMARY, - messages: [{ role: "user", content: "fallback probe" }], - }, - }); + const chatStatus = async () => + ( + await request.post("/v1/chat/completions", { + headers: { ...ADMIN_AUTH, "Content-Type": "application/json" }, + data: { + model: BROKEN_PRIMARY, + messages: [{ role: "user", content: "fallback probe" }], + }, + }) + ).status(); // The control: it proves the reply below could only have come from the fallback. - expect((await chat()).status(), "broken primary unexpectedly succeeded on its own").toBeGreaterThanOrEqual(400); + await pollUntilSettled( + chatStatus, + (status) => status >= 400, + "broken primary unexpectedly succeeded on its own", + ); await patchRouterSettings(request, { fallbacks: [{ [BROKEN_PRIMARY]: [PRIMARY] }], } as Partial>); // Same call now succeeds, served by the fallback model. - await expect - .poll(async () => (await chat()).status(), { - timeout: 30_000, - message: "fallback never took effect", - }) - .toBe(200); + await pollUntilSettled(chatStatus, (status) => status === 200, "fallback never took effect"); // And the playground renders a reply for a model whose own upstream is down. await openPlayground(page); From a2aecfea7f2905ed569ccc8d733efcff13c3074c Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 26 Aug 2026 18:57:38 -0700 Subject: [PATCH 2/3] 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(). (cherry picked from commit b95801172ce9eb5356da95bccc87cb80b854677c) --- .../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, - matches: (status: number) => boolean, - message: string, -): Promise { - 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): Promise { + return Array.from({ length: SETTLE_PROBES }).reduce>( + 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>); - // 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); From da1d292dceeaf8f098ea9955eeaf7e7fb2a74a02 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 28 Aug 2026 00:15:02 -0700 Subject: [PATCH 3/3] test(e2e): measure the select popup after it settles instead of mid-flight Both anchoring tests read the trigger's box before the click and the popup's box the instant it turns visible. Base UI places the popup asynchronously and opening it can shift the trigger, so both boxes could be sampled before the layout settled. The run on 1eedaa3a43 missed by 4.2px (expected >= 446.015, got 441.799) on a tree with no UI changes at all, having passed on 21092d633b, which differs only in a deleted python test and a budget json. Each assertion now re-reads both boxes under expect.poll. The conditions themselves are unchanged: the popup must sit at or below the trigger's bottom edge in the first test and must not overlap it in the second. Polling cannot mask a genuinely misplaced popup, since one that never lands correctly still fails when the poll times out. (cherry picked from commit d8679508d4c1d1992923e75295df62188e4ed6b9) --- .../autoRouterTemplateSelect.spec.ts | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/tests/e2e/ui/tests/modelsPage/autoRouterTemplateSelect.spec.ts b/tests/e2e/ui/tests/modelsPage/autoRouterTemplateSelect.spec.ts index 98bd1b84f11..1d080ec82b8 100644 --- a/tests/e2e/ui/tests/modelsPage/autoRouterTemplateSelect.spec.ts +++ b/tests/e2e/ui/tests/modelsPage/autoRouterTemplateSelect.spec.ts @@ -1,4 +1,4 @@ -import { expect, test, type Page as PlaywrightPage } from "@playwright/test"; +import { expect, test, type Locator, type Page as PlaywrightPage } from "@playwright/test"; import { ADMIN_STORAGE_PATH } from "../../constants"; import { navigateToPage } from "../../helpers/navigation"; import { Page } from "../../fixtures/pages"; @@ -17,43 +17,49 @@ async function openTemplateSelect(page: PlaywrightPage) { return trigger; } +function pollPixelsBelowTrigger(trigger: Locator, popup: Locator) { + return expect.poll(async () => { + const triggerBox = await trigger.boundingBox(); + const popupBox = await popup.boundingBox(); + if (!triggerBox || !popupBox) return null; + return popupBox.y - (triggerBox.y + triggerBox.height); + }); +} + +function pollPopupOverlapsTrigger(trigger: Locator, popup: Locator) { + return expect.poll(async () => { + const triggerBox = await trigger.boundingBox(); + const popupBox = await popup.boundingBox(); + if (!triggerBox || !popupBox) return null; + return popupBox.y < triggerBox.y + triggerBox.height && popupBox.y + popupBox.height > triggerBox.y; + }); +} + test.describe("Auto Router template select anchoring", () => { test.use({ storageState: ADMIN_STORAGE_PATH }); test("opens the options below the trigger rather than over it", async ({ page }) => { await page.setViewportSize({ width: 1280, height: 900 }); const trigger = await openTemplateSelect(page); - const triggerBox = await trigger.boundingBox(); await trigger.click(); const popup = page.locator('[data-slot="select-content"]'); await expect(popup).toBeVisible(); - const popupBox = await popup.boundingBox(); - - expect(triggerBox).not.toBeNull(); - expect(popupBox).not.toBeNull(); // Item-aligned mode reports "none" and puts the active item over the trigger. await expect(popup).toHaveAttribute("data-side", "bottom"); - expect(popupBox!.y).toBeGreaterThanOrEqual(triggerBox!.y + triggerBox!.height); + await pollPixelsBelowTrigger(trigger, popup).toBeGreaterThanOrEqual(0); }); test("flips above the trigger instead of covering it when there is no room below", async ({ page }) => { await page.setViewportSize({ width: 1280, height: 560 }); const trigger = await openTemplateSelect(page); await trigger.scrollIntoViewIfNeeded(); - const triggerBox = await trigger.boundingBox(); await trigger.click(); const popup = page.locator('[data-slot="select-content"]'); await expect(popup).toBeVisible(); - const popupBox = await popup.boundingBox(); - expect(triggerBox).not.toBeNull(); - expect(popupBox).not.toBeNull(); - - const overlaps = - popupBox!.y < triggerBox!.y + triggerBox!.height && popupBox!.y + popupBox!.height > triggerBox!.y; - expect(overlaps).toBe(false); + await pollPopupOverlapsTrigger(trigger, popup).toBe(false); }); });