refactor(e2e): bucket rate limits, budgets, and spend tracking under quota_management - #32928
Conversation
Greptile SummaryThis PR refactors the e2e coverage registry by introducing a
Confidence Score: 5/5Safe to merge — all changes are confined to the e2e test tree (registry metadata, test markers, and a test-helper lint fix); no proxy behaviour is touched. The diff is entirely within tests/e2e/: YAML registry definitions, additive @pytest.mark.covers markers on existing tests, a new QuotaCell schema type, and two small correctness fixes in a test helper. None of these paths affect runtime proxy logic. No files require special attention; the most substantive changes are schema.py and quota_management.yaml, both validated by the collector tests and basedpyright gate reported in the PR description.
|
| Filename | Overview |
|---|---|
| tests/e2e/coverage_registry/schema.py | Adds QuotaCell to the discriminated union, registers the quota_management prefix rollup and Loki label, and updates MODULE_ORDER; all additions are consistent with existing patterns. |
| tests/e2e/coverage_registry/quota_management.yaml | New file with 33 registry cells across ratelimit, budget, and spend_tracking behaviors; source corrections applied (parallel_request_limiter_v3.py for key-scope RPM/TPM enforcement), and spend_calculate exercised_on fixed to use management route name per prior review feedback. |
| tests/e2e/coverage_registry/reliability.yaml | Removes four ratelimit rows that have moved to quota_management.yaml; remaining rows are unaffected. |
| tests/e2e/logging/logging_client.py | Fixes two basedpyright gate errors: switches LangfuseListParams to serialization_alias (correct for a request-param model), and replaces raw json.loads + dict check in completion_response_id with a typed pydantic model that handles both parse errors and structural mismatches via a single ValidationError catch. |
| tests/e2e/budgets/test_budget_enforcement_e2e.py | Wraps each parametrized budget-enforcement case in pytest.param with a covers marker; no assertion changes. |
| tests/e2e/spend_tracking/test_spend_tracking_e2e.py | Adds @pytest.mark.covers markers to all existing spend-tracking tests; no assertion or logic changes. |
| tests/e2e/CLAUDE.md | Documents the new quota_management module with its naming grammar; removes ratelimits from the router/ description since those tests now belong to quota_management. |
| tests/e2e/budgets/test_budget_reset_advances_e2e.py | Adds covers markers to four test functions covering key spend blocking, window resets, multi-window independence, and team-member reset; no assertion changes. |
Reviews (2): Last reviewed commit: "fix(e2e): keep logging_client within the..." | Re-trigger Greptile
| - {id: quota_management.spend_tracking.spend_calculate.returns_cost, module: quota_management, tier: P2, behavior: spend_tracking, variant: spend_calculate, assertions: [returns_cost], exercised_on: [chat_completions], source: "proxy/spend_tracking/spend_management_endpoints.py", rationale: "/spend/calculate prices a hypothetical request at nonzero cost"} | ||
| - {id: quota_management.spend_tracking.pagination.keeps_total, module: quota_management, tier: P2, behavior: spend_tracking, variant: pagination, assertions: [keeps_total], exercised_on: [chat_completions], source: "proxy/spend_tracking/spend_management_endpoints.py", rationale: "Spend-logs v2 pagination caps page size without losing the total"} |
There was a problem hiding this comment.
exercised_on metadata inaccurate for management-API cells
Both spend_calculate.returns_cost and pagination.keeps_total list exercised_on: [chat_completions], but neither test exercises a chat-completions LLM endpoint — test_spend_calculate_returns_nonzero_cost calls /spend/calculate directly and test_spend_logs_v2_pagination_caps_pages_and_keeps_total calls /spend/logs. If the collector or a future dashboard slice filters cells by exercised_on, these rows will appear in the wrong bucket. The field should reflect the actual proxy endpoint exercised (e.g. a dedicated literal, or the management surface pattern used by mgmt.* cells).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
0ad1f7b to
d9bb776
Compare
|
@greptileai The spend_calculate row's exercised_on now names the management route its covering test actually drives (spend_calculate, following the non-LLM vocabulary the mcp_operations row in guardrail.yaml already uses); fixed in bff4ad2. The pagination row keeps chat_completions on purpose: that test drives two real chat calls to seed the spend rows it paginates, so the LLM endpoint genuinely is exercised |
|
Both calls look right. The The |
bff4ad2 to
0d1a789
Compare
|
@greptileai rebased onto latest litellm_internal_staging (which brings the new zero-error e2e basedpyright gate) and fixed the 5 gate errors in logging_client.py that the staging tip itself carries, so lint is green here |
|
@greptileai budgets/ and spend_tracking/ suites now live under tests/e2e/quota_management/ (one subfolder per registry behavior); the conftest spend-cleanup path follows the move |
|
The restructuring looks clean. The - spend_dir = str(Path(__file__).parent / "spend_tracking")
+ spend_dir = str(Path(__file__).parent / "quota_management" / "spend_tracking")One thing worth noting: |
67e90ca to
d37daf0
Compare
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
This PR only changes the e2e coverage registry (the denominator YAMLs, their schema, and
coversmarkers on existing tests); it adds no proxy behavior, so the proof is the registry tooling itself running against the tree at commit d37daf0.--strictperforms a full pytest collect-only pass over tests/e2e, so it proves every touched suite still imports and every marker (including the 27 newly wired ones) resolves to a registry cell:Type
🧹 Refactoring
✅ Test
Changes
This introduces a
quota_managementmodule in the e2e coverage registry, bucketing rate limits, budgets, and spend tracking into one dashboard category (agreed with the team). Rate limiting previously sat underreliability.*while budgets and spend tracking had no behavior rows at all; their suites predate the registry and carried nocoversmarkers, so their coverage was invisible to the dashboardschema.pygains aQuotaCelltype (behavior: ratelimit | budget | spend_tracking) in the discriminated union, plus theQuota Managementdashboard rollup and itsquota_managementLoki label. The six ratelimit rows move fromreliability.yamlinto the newquota_management.yamlwith ids renamed fromreliability.ratelimit.*toquota_management.ratelimit.*; thesourceon the two enforcement rows is corrected fromdynamic_rate_limiter_v3.pytoparallel_request_limiter_v3.py, where key-scope rpm/tpm enforcement actually lives, and two ratelimit cells are added for window reset and pacing headers (their covering tests land in the stacked PR #32914)The rest of the bucket is seeded from the existing suites: 15 budget rows (entity enforcement for key, internal user, end user, organization, and team member; tag budgets; per-model model_max_budget isolation; soft budget; key and team multi-window blocks-then-resets; key and team-member window resets; budget fallback rerouting; the spend-counter reseed guard) and 12 spend_tracking rows (nonzero cost rows for chat, streaming, and embeddings; zero-cost cache hits; key rollup vs sum of logs; the concurrent-burst lost-spend guard; tag and end-user attribution; per-model rows; failure rows; /spend/calculate; spend-logs v2 pagination). Every seeded row is claimed by an existing test via
@pytest.mark.covers, and the two budget CRUD tests also claim their existingmgmt.budget.*cells. Following review feedback, the spend_calculate row'sexercised_onnames the management route its covering test actually drives (spend_calculate) rather thanchat_completionsAn earlier revision carried a drive-by fix for
tests/e2e/logging/logging_client.py, which was red on the zero-error e2e basedpyright gate at the then-staging tip; latest litellm_internal_staging ships its own fix for that file, so the rebase drops the commit and this PR is back to registry changes onlyThe suite folders follow the registry:
tests/e2e/budgets/andtests/e2e/spend_tracking/move undertests/e2e/quota_management/as one subfolder per behavior (their files are otherwise untouched apart from thecoversmarkers; the session-finish spend-log cleanup path intests/e2e/conftest.pyfollows the move), the naming grammar intests/e2e/CLAUDE.mdand the registry README document the new module, and therouter/suite-folder description drops "rate limits" since those tests now belong to quota management (PR #32914 adds them inquota_management/ratelimit/). That stacked PR covers the remaining ratelimit cells