fix(rate-limits): keep the v3 limiter out of provider-facing metadata on responses routes - #35207
Conversation
…etadata on responses routes
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThe PR keeps rate-limiter state in the route-appropriate internal metadata bucket.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the prior test-coverage concern is resolved because the updated assertion passes the stash through
|
| Filename | Overview |
|---|---|
| litellm/proxy/hooks/parallel_request_limiter_v3.py | Routes limiter stash writes through the internal metadata helper and reads the route-owned metadata channel without exposing a new actionable defect. |
| tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py | Adds regression coverage for absent and caller-supplied provider metadata and now directly exercises lookup through litellm_params["litellm_metadata"]. |
Reviews (2): Last reviewed commit: "Update tests/test_litellm/proxy/hooks/te..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
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 68de15b. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM
Pretty bad pattern to have internal bookkeeping. I think it was really hard to understand the fact that we pop for security reasons, then we push the bookkeeping params, then we pop again with all_litellm_params and we use metadata for chat completions and litellm_metadata for responses (because responses standard has metadata). I am reworking this to be typed async safe contextvar here: #35278
…3.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
@greptileai review latest head |
TLDR
Problem this solves:
/v1/responsessendmetadataupstreammetadatareturned HTTP 400How it solves it:
litellm_metadatawhenever the route owns itmetadatais never created or touchedRelevant issues
Fixes #35197
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
Proof of fix posted as a PR comment: live proxy on localhost:4000, real provider calls, before/after commit hashes and the raw upstream request body captured from
--detailed_debugType
🐛 Bug Fix
Changes
_PROXY_MaxParallelRequestsHandlerstashed every rate-limit value into both metadata channels at once:For chat completions that is harmless, since
metadatais inall_litellm_paramsand never reaches the provider. For the routes inLITELLM_METADATA_ROUTES(Responses,/v1/messages, batches, files)litellm_metadatais the internal channel andmetadatais a real provider request parameter, so the branch above conjured a provider-visiblemetadataobject out of an internal bookkeeping write; OpenAI-compatible backends that don't implement Responses metadata answered 400. #32711 made this fire on every successful rate-limit check, so any key/user/team/project/org/agent RPM, TPM, or max-parallel limit was enough to trigger itThe write now goes through the existing
get_or_create_metadata_buckethelper, which resolves the single proxy-internal bucket the same way every other reader and writer of internal metadata does, so a Responses request only ever growslitellm_metadata, and a caller that supplies its ownmetadatagets it back untouched_lookup_stashed_valuealso learned to readlitellm_params["litellm_metadata"], not justlitellm_params["metadata"]; with the dual write gone, that is the channel the value lives in when a Responses caller supplied provider metadata of their ownFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/09dfb72721944792b432e6de150873fd
Note
Medium Risk
Changes the pre-call rate limiter’s request-data mutation on a hot proxy path; scope is narrow (metadata channel selection) but incorrect bucket choice could break TPM reconciliation or logging on mixed routes.
Overview
Fixes #35197: successful rate-limit checks on Responses-style routes were writing internal stash keys into
metadata, which is forwarded to the provider and caused HTTP 400 from backends that reject unknown Responsesmetadata.The v3 parallel request limiter no longer dual-writes to
metadataandlitellm_metadata. Stash values (TPM reservation, rate-limit descriptors, parallel slot markers, etc.) go only throughget_or_create_metadata_bucket, so internal state lands inlitellm_metadataon routes that own it and never creates or mutates callermetadata._lookup_stashed_valuenow resolves stash keys fromlitellm_params["litellm_metadata"]as well aslitellm_params["metadata"], matching the single-bucket write path when callers supply their own provider metadata.A regression test covers
aresponsespre-call behavior with and without caller-suppliedmetadata.Reviewed by Cursor Bugbot for commit 68de15b. Bugbot is set up for automated code reviews on this repo. Configure here.