test(e2e): assert the long budget window keeps blocking after the short window resets - #33832
Conversation
Greptile SummaryAdds the "long-window still blocks after short-window resets" direction to the multi-window budget e2e suite, one new test per entity type (key and team). Both previous review findings are addressed:
Confidence Score: 5/5Safe to merge — this PR is additive e2e tests only, with no production code changes. The change is entirely test-only: new pydantic models, a refactored client helper, and two new live e2e test cases. The refactor of three No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/models.py | Adds BudgetWindowState (extends BudgetWindow with a pydantic-typed `datetime |
| tests/e2e/quota_management/budgets/budget_client.py | Adds TeamInfoRow + team_info field on TeamInfoResponse, the pure window_reset_at helper, key_budget_windows/team_budget_windows methods, and consolidates three duplicate /team/info HTTP calls behind a private _team_info method; refactor is behavior-preserving and well-structured. |
| tests/e2e/quota_management/budgets/test_multi_window_budget_e2e.py | Extracts _drive_to_block into a local helper, adds test_long_window_blocks_after_short_window_resets asserting the 1d cap holds after the 30s window provably rolls; both previous findings (non-None assert on long-window reset_at, 429 status pin) are addressed. |
| tests/e2e/quota_management/budgets/test_team_multi_window_budget_e2e.py | Team-side analog of the key test: extracts _drive_to_block, adds test_team_long_window_blocks_after_short_window_resets with the same polling-reset pattern and identical non-None diagnostic guards; no issues found. |
Reviews (6): Last reviewed commit: "docs(e2e): number the long-window team t..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@greptileai re review |
…rt window resets
The multi-window budget tests proved the tight window blocks and self-heals
but never asserted the other direction: a long (1d) window whose cap the
accumulated spend already crossed must keep refusing calls even inside a
fresh short window. Adds one test per file (key and team) that drives spend
to a block, waits for the short window's reset_at to strictly advance (the
reset job zeroes that window's counter in the same pass), then polls until
the refusal is attributed to the 1d window ("over 1d budget"), failing
immediately if any call succeeds or a non-budget error leaks. Harness gains
per-window reset_at readback: BudgetWindowState in models.py and
key_window_reset_at / team_window_reset_at on BudgetClient.
drive_to_block and as_datetime existed as five and four per-file copies in the budgets suite; both move to budget_client with each file keeping a thin delegating wrapper so call sites and per-file pacing stay unchanged. The three /team/info readers in budget_client now share a private _team_info. Also guard the long-window reset_at snapshots with explicit non-None asserts so the midnight-roll diagnostic cannot misreport when the window is missing from the info response (greptile P2s).
e538c04 to
f4f8af5
Compare
|
@greptileai re review |
…itellm_e2e_multi_window_long_cap
…ow readers BudgetWindowState.reset_at becomes a pydantic-parsed datetime, so the multi-window tests compare real datetimes instead of hand-parsing strings. The duration-keyed accessors are replaced by two plain readers, key_budget_windows and team_budget_windows, with the pure window_reset_at lookup exported; the client no longer encodes one test's access pattern.
|
@greptileai re review |
drive_to_block now returns the blocking response so a test body can assert on its shape; the two long-window tests assert status 429 explicitly, which also pins the multi-window enforcement path's HTTP mapping (the enforcement suite only covers the single-budget path). Other callers ignore the return and are unchanged.
|
@greptileai re review |
…-suite hoist The helper hoist rewrote four unrelated budget test files (reset, reset_advances, team_member_reset, user_across_keys) to pull drive_to_block and as_datetime out of budget_client, which is refactor churn beyond this PR's multi-window scope. This restores those four to their pre-PR state and gives the two multi-window tests their own inline drive-to-block loop again, so the PR touches only the multi-window feature: its two tests plus the budget_client window readers and the reset_at datetime typing they actually use. The suite-wide helper dedup can land on its own PR
|
@greptileai re review |
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
All proof captured at commit 95c915f against the live
tests/e2e/docker-compose.ymlstack (publishedmain-latestimage, throwaway postgres/redis, real provider calls costing real $). Local-only runtime bend, reverted before commit: this machine only has a Groq key, so the composelitellm_paramsforgpt-5.5andclaude-haiku-4-5were pointed atgroq/llama-3.3-70b-versatilefor the runs (same recipe as PR #33632), and the jaeger host port was remapped to 16687 to dodge a locally running jaegerThe customer-visible walkthrough, exactly what the new key-side test asserts:
Output:
The 30s window provably rolled (reset_at moved 18:22:30 to 18:23:00) and the very next refusal is attributed to the 1d window: the long cap enforces on its own schedule
Five consecutive runs of both files against the live stack, 60s cooldowns, zero failures (20/20 test passes):
each run being
An intermediate commit hoisted the suite's duplicated
drive_to_blockandas_datetimehelpers intobudget_client; a full confirmation pass of every file that commit touched came back green (17 passed in 652.12s: both multi-window files, test_budget_reset_e2e.py's five-case diagonal, all six test_budget_reset_advances_e2e.py rungs, test_team_member_budget_reset_e2e.py, and test_user_budget_across_keys_e2e.py). That hoist was then reverted to keep this PR scoped to the multi-window feature, so the four unrelated budget files are restored to their staging state verbatim and the two multi-window tests carry their own inline drive-to-block loop again; the un-hoist changes no test's behavior and type-checks clean (basedpyright tests/e2e: 0 errors)The branch was then rebased onto the ProxyClient rename (#33750), re-hashing the commits (proofs above were captured at pre-rebase hashes 95c915f and 1966dde); a post-rebase run of both multi-window files at f4f8af5 came back 4 passed in 315.09s. After merging latest staging and refining the readback (datetime-typed reset_at plus plain window readers, commit 0e793aa), one more live run of both files came back 4 passed in 312.01s; the compose stack for that run was restored from git history since #33837 removed tests/e2e/docker-compose.yml upstream. Note the current staging tip fails
basedpyright tests/e2eon its own: the load suite from #33748 still imports the module #33750 renamed, so the e2e type gate shows ~35 errors in tests/e2e/load/ on any PR until that is fixed separately; this branch's own files type-check cleanType
✅ Test
Changes
The multi-window budget e2e tests proved the tight window blocks and self-heals but never asserted the other direction: a long window whose cap the accumulated spend already crossed must keep refusing calls even inside a fresh short window. This PR adds exactly that direction, one test per existing file (key and team), and nothing else
Each new test mints its entity with
budget_limitsof a 30s window at 1e-9 and a 1d window at 5e-7 (both far below one real call's cost, so the first burn crosses both caps), drives spend to a 429budget_exceeded, then records the 30s window'sreset_atfrom/key/info//team/infoand waits for it to advance strictly past that value. The reset job advances a window'sreset_atin the same pass that zeroes its spend counter, so once it moved the short window is provably clean. The test then polls until the refusal is attributed to the 1d window (ExceededBudget: ... over 1d budget), failing immediately if any call returns 200 (the long cap failed to hold; the failure message diagnoses the rare case where the 1d window itself rolled at midnight UTC mid-test) or if a non-budget error leaks. Recordingreset_atafter the block rather than at mint closes a false-pass race where a boundary rolls before the burn's spend lands; the final phase polls rather than single-shots because enforcement reads a cached auth object whose view of the rolled window lags the DB write (run 1 of QA caught this live: the block stayed attributed to the 30s window for a few seconds afterreset_atadvanced)Harness support, modelled only to the fields read:
BudgetWindowState(aBudgetWindowplus a pydantic-parseddatetimereset_at) intests/e2e/models.py,budget_limitsonKeyInfo, and two plain readers onBudgetClient(key_budget_windows/team_budget_windows) with a purewindow_reset_atlookup, so the client exposes state rather than one test's access pattern and the tests compare real datetimes with no hand parsingEach file's drive-to-block loop returns the blocking response and both long-window tests assert its status is 429 in the test body, which also pins the multi-window enforcement path's HTTP mapping (the enforcement suite's 429 assert covers only the single-budget path); the observed 429s in the proof runs above are the live evidence for that assert
This PR stays scoped to the multi-window feature and does not touch the rest of the suite: the two new tests keep their own inline drive-to-block loop rather than sharing one, so no unrelated budget test file is modified. Both Greptile P2 findings are still addressed: explicit non-None asserts on the long-window
reset_atsnapshots so the midnight-roll diagnostic cannot misreport when the window is absent from the info response, and the three/team/inforeaders insidebudget_clientshare a private_team_info. Pulling the duplicateddrive_to_blockandas_datetimehelpers intobudget_clientis left for its own PRBoth new tests intentionally share their file's existing registry cell (
quota_management.budget.key_multi_window.blocks_then_resetsandquota_management.budget.team_multi_window.blocks_then_resets): this direction is the unasserted half of those same cells, so no new denominator rows. The collector runs clean with--strictOne adversarial-review finding is accepted rather than fixed: a mutation that wipes every window's Redis counter (long included) while advancing only the expired window's
reset_atwould not fail these tests, because enforcement re-derives an authoritative floor for the long window from spend logs and blocks anyway. That redundancy is invisible at the customer boundary, and the customer promise (the long cap holds) is exactly what the tests assert; mutations that break the promise itself (long enforcement removed, reset job advancing every window's boundary) are caughtQA runbook
Environment prerequisites: the tests/e2e docker-compose stack with provider keys for gpt-5.5 (OpenAI) and claude-haiku-4-5 (Anthropic) in tests/e2e/.env, or the Groq swap described in the proof section. The compose stack already sets proxy_budget_rescheduler_min_time/max_time to 5/10s; without that, the reset wait exceeds the tests' deadlines
Final Attestation