Repository navigation
fix(proxy): reserve the larger declared output budget for TPM limits - #37001
Conversation
|
|
Greptile SummaryThe PR updates TPM reservation estimates to use the larger successfully parsed output-token budget when both supported fields are declared
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/hooks/parallel_request_limiter_v3.py | Coerces supported budget representations and reserves against the largest declared output limit |
| tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py | Adds focused reservation and TPM enforcement tests while correcting the previously reported broad typing |
Reviews (3): Last reviewed commit: "fix(proxy): reserve the larger declared ..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
bugbot run |
10af552 to
47e1ab5
Compare
|
@greptileai please review the current head 47e1ab5, the test helper now returns Dict[str, Dict[str, object]] instead of Any |
The TPM pre-call reservation read `max_tokens or max_completion_tokens`, so a request declaring both was charged for whichever field came first. A caller sending `max_tokens=1` with `max_completion_tokens=10000` reserved 2 tokens and was then free to consume ten thousand, since the provider honours the modern field and litellm's own param mapping drops the legacy one for the gpt-5 and o-series families. Reserve against the larger of the declared budgets instead. Over-reserving is the safe direction for a limiter: post-call reconciliation refunds the difference between the reservation and actual usage, while under-reserving lets the window be exceeded before anything notices.
47e1ab5 to
3397a39
Compare
|
@greptileai please review the current head 3397a39, it fixes the bugbot Medium on float and numeric-string budgets skipping reservation |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3397a39. Configure here.
TLDR
Problem this solves:
max_tokens=1plusmax_completion_tokens=10000reserved 2 tokensHow it solves it:
User Flow
Before: a caller holding a key capped at 100 TPM gets a full-size generation admitted by declaring a tiny
max_tokensalongside a largemax_completion_tokens{"tpm_limit": 100}via POST https://litellm-domain/key/generate{"max_tokens": 1, "max_completion_tokens": 10000}{"max_completion_tokens": 10000}is refused with HTTP 429, so the cap is enforced only against callers who declare their budget onceAfter: the same request is refused, and declaring the budget twice buys nothing
{"tpm_limit": 100}via POST https://litellm-domain/key/generate{"max_tokens": 1, "max_completion_tokens": 10000}Limit type: tokens{"max_completion_tokens": 10000}is refused identically, so both spellings now cost the sameRelevant issues
Found while reviewing #36857, which makes
azure/gpt-5-chattranslatemax_tokensintomax_completion_tokensand so joins the families where the legacy field is dropped before the provider sees it. The under-reservation predates that change and already applied toazure/gpt-5,azure/o3-mini,openai/gpt-5andopenai/o3-mini, which is why the fix belongs here in the limiter rather than in any one provider's param mappingLinear ticket
Pre-Submission checklist
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Live proxy on 127.0.0.1:45559 backed by its own Postgres on 127.0.0.1:55490, with one Azure deployment pointed at a local HTTP server on 127.0.0.1:45560 that records the request body. The rate limiter is the real one, the key is a real virtual key carrying
tpm_limit: 100, and each leg uses a fresh key so no leg inherits another's window.--detailed_debugis on so the limiter prints the figure it reservedEvery leg runs the same three requests: the conflicting one, then two controls that declare the budget once
Before, at 40a4184 (this branch's base):
After, at 10af552 (this PR):
The reserved figure for the identical request moves from 2 to 10001, and case A moves from admitted to refused while both controls stay exactly where they were. Control B is the load-bearing one: it shows the harness can produce a 429 at this limit before the fix, so case A being admitted was the conflicting pair winning rather than the cap being unreachable. The HTTP 400 on case A before, and on control C in both runs, is the separate Azure defect in #36857, which this branch does not carry; it is what a request looks like when the limiter lets it through to the provider
Type
🐛 Bug Fix
Caveats (if any)
maxis right even whenmax_tokensis the larger declared valueFinal Attestation
Note
Cursor Bugbot is generating a summary for commit 10af552. Configure here.