test(e2e): pin regional cost-map uplift and rate-limit attribution - #34652
Conversation
Greptile SummaryAdds live end-to-end coverage for two customer-visible attribution paths.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking test-integrity gap in the rate-limit scenario. The registry and regional-pricing additions are consistent with the harness, while the rate-limit test should verify that its first provider request succeeds so it cannot validate gateway attribution after an unrelated initial failure. Files Needing Attention: tests/e2e/logging/test_prometheus_ratelimit_failure_e2e.py
|
| Filename | Overview |
|---|---|
| tests/e2e/llm_translation/test_regional_pricing_e2e.py | Adds a well-scoped model-management test that checks exact global and regional cost-map rates and defers cleanup using the created model IDs. |
| tests/e2e/logging/test_prometheus_ratelimit_failure_e2e.py | Adds Prometheus attribution coverage, but does not establish that a provider request succeeds before the gateway rate limit is exercised. |
| tests/e2e/coverage_registry/logging.yaml | Adds a valid logging coverage cell matching the new Prometheus test marker. |
| tests/e2e/coverage_registry/mgmt.yaml | Adds a valid management coverage cell matching the new regional-pricing test marker. |
Reviews (1): Last reviewed commit: "test(e2e): pin regional cost-map uplift ..." | Re-trigger Greptile
| f"driving {CALLS} calls against an rpm_limit of {RPM_LIMIT} produced no 429; " | ||
| f"statuses were {statuses}, so the limiter never rejected and there is " | ||
| f"nothing for the metric to attribute" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Successful request is never established
The status assertion only requires one 429, so the test can validate the gateway labels even when the initial provider request fails. Assert that the first response succeeds before checking that later requests exceed the key's RPM limit; otherwise the advertised end-to-end provider scenario remains unverified.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Two shipped regressions that had no live coverage, both about a number a customer reconciles against something external. Bedrock's regional Claude variants are priced above the global variant, and the cost-map mappings regressed so a regional deployment resolved the global rate (LIT-3912), silently under-billing every regional call. The test registers the global, us and eu variants of one model and pins each resolved rate from /model/info, asserting the exact expected uplift rather than regional > global: a mapping that landed on some other model's rate would satisfy an inequality while being just as wrong. It reads pricing only, so it needs no AWS credentials and spends nothing. A 429 is actionable only if it says who produced it, because a vendor rejection means back off or fail over while a gateway rejection means the key's own limit is too low. exception_class is what separates them, HTTPException for the gateway's limiter against a provider error class for the vendor, and api_provider is what tells a multi-provider deployment which upstream was involved (PR BerriAI#27687). The test drives a key past its own RPM limit, which is unambiguously gateway-side, and requires the resulting failure series to be labelled that way and to name a provider. Both were mutation-checked against the live proxy: claiming no uplift, claiming the gateway 429 carries a vendor error class, scraping for an alias that made no calls, and requiring an empty provider each fail the relevant test, and both pass restored.
c2b5489 to
9e27a9a
Compare
TLDR
Problem this solves:
How it solves it:
/model/infoRelevant 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)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Both captured against a local proxy on port 4000 at this branch's commit
c2b54892fa, on deployments and keys the run created through the management API exactly the way the tests do. This is a test-only PR, so the proof is that the behavior each test asserts is real and observable over HTTP, not that apytestrun went green1. Regional variants resolve their own uplifted cost-map rate (LIT-3912). Three deployments of the same model differing only in region prefix, each read back through
/model/infowith no pricing override configured. The regional rates are the 10% uplift over global, which is the mapping that regressedNo provider call is made, so this needs no AWS credentials and spends nothing. That is deliberate: the regression is in rate resolution, and resolution is fully observable before any token is billed
2. A gateway 429 is attributed to the gateway and names a provider (PR #27687). A key limited to one request a minute, driven three times
exception_class=HTTPExceptionis the gateway's own limiter, as against a vendor rejection which surfaces its provider error class. For contrast, a real vendor 429 observed on the same proxy during this work looked likeexception_class="Openai.RateLimitError" exception_status="429" api_provider="openai", which is the distinction the test pinsFour mutations were run against the live proxy and each failed the relevant test: claiming no regional uplift, claiming the gateway 429 carries a vendor error class, scraping for a key alias that made no calls, and requiring an empty
api_provider. Both tests pass again once reverted, which is the evidence they would have failed before the fixes rather than passing vacuouslyType
✅ Test
Changes
Two independent regression tests plus their registry cells,
mgmt.model.info.reports_regional_upliftandlogging.prometheus.failure.attributes_rate_limit_source. No product code and no harness plumbing changesThe pricing test asserts the exact expected uplift rather than an inequality.
regional > globalwould pass on a mapping that resolved the regional variant to some unrelated model's higher rate, which is the same class of bug wearing a different number. Rates are compared with a relative tolerance because the uplift arithmetic is not exact in binary, andpytest.approxis untyped under the suite's basedpyright gateThe rate-limit test asserts on
exception_classbecause that is the only label that separates a gateway rejection from a vendor one;exception_statusis429either way. It also requiresapi_providerto be non-empty, since a multi-provider deployment cannot act on a 429 that does not say which upstream was involvedQA runbook
tests/e2e/llm_translation/test_regional_pricing_e2e.py::TestRegionalUpliftPricing::test_regional_variants_price_above_global - a regional bedrock deployment bills at its own uplifted rate, not the global one
anthropic.claude-opus-4-7, one each for theglobal.,us.andeu.prefixes, with noinput_cost_per_tokenoverride:curl -X POST http://localhost:4000/model/new -H "Authorization: Bearer sk-1234" -d '{"model_name":"rp-us","litellm_params":{"model":"bedrock/us.anthropic.claude-opus-4-7","aws_region_name":"us-east-1"}}'(needsstore_model_in_db: true; no AWS credentials required)/model/infoand read each row'smodel_infoblock, which is the rate the proxy resolved rather than anything configuredinput=5e-06/output=2.5e-05, and bothus.andeu.at exactly 1.1x those,input=5.5e-06/output=2.75e-05tests/e2e/logging/test_prometheus_ratelimit_failure_e2e.py::TestPrometheusRateLimitAttribution::test_gateway_rate_limit_is_labelled_with_source_and_provider - a 429 raised by the gateway's own limiter is labelled as the gateway's, and names the provider
curl -X POST http://localhost:4000/key/generate -H "Authorization: Bearer sk-1234" -d '{"key_alias":"rl-manual","models":["gemini-2.5-flash"],"rpm_limit":1}'(needsprometheusinlitellm_settings.callbacks)/v1/chat/completionsrequests with that key inside one minute and expect the first to return 200 and the rest 429/metrics(follow the 307 withcurl -L) and findlitellm_proxy_failed_requests_metric_totalforapi_key_alias=rl-manualexception_status="429",exception_class="HTTPException"(the gateway's limiter, not a vendor class) and a non-emptyapi_providerFinal Attestation