From 84dfc18f6bb2e028709775f1797c5dd5710880cc Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 26 Aug 2026 17:57:14 -0700 Subject: [PATCH 1/2] 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. --- 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 95a02b58824..8d1b17ca256 100644 --- a/tests/e2e/models.py +++ b/tests/e2e/models.py @@ -256,6 +256,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 b95801172ce9eb5356da95bccc87cb80b854677c Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Wed, 26 Aug 2026 18:57:38 -0700 Subject: [PATCH 2/2] 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, - 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);