fix(convert_dict_to_response): handle empty choices list without raising 500 APIError - #40294
Conversation
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Fetch the pull request head and its merge base | ||
| id: revisions | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| run: | | ||
| merge_base="$(gh api "repos/${GITHUB_REPOSITORY}/compare/${BASE_SHA}...${HEAD_SHA}" --jq '.merge_base_commit.sha')" | ||
| git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA" | ||
| echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" | ||
| - name: Set up uv |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
04696d5 to
fe955b7
Compare
…ing 500 APIError (Fixes BerriAI#40276)
fe955b7 to
c1132d0
Compare
ParBproject
left a comment
ParBproject
left a comment
There was a problem hiding this comment.
One malformed-response regression seems to come with the new Iterable check. choices is an OpenAI-style list, but values like {}, "", or () are also Iterable; because they are empty, the loop simply produces choices=[] and the response is now accepted as if it were the intended safety-block case. Before this change those falsy malformed shapes were rejected by not response_object.get("choices"). That can hide an upstream/provider contract failure as a legitimate empty completion. Could this narrow the accepted empty case to an actual list (or at least a sequence of choice mappings, explicitly excluding mappings/strings), and add a regression test such as choices={} still raising APIError?
…itellm_mat228_pr40294
… keep /v1/messages alive on an empty one Narrows the no-choices guard so a dict, string, or None still raises the APIError while an empty list passes through, guards the non-stream Anthropic bridge against indexing an empty choices list, and repairs test_completion_missing_role, whose raw-response mock was patched in as the create() callable itself so the handler only ever saw a MagicMock
Greptile SummaryThis PR treats
Confidence Score: 5/5The PR appears safe to merge with no outstanding actionable findings. The empty-choice paths are guarded without weakening validation for malformed values, and the cache-stream and Anthropic bridge cases have focused regression coverage. All previous review threads are resolved, including the line-length finding that Greptile explicitly conceded as incorrect.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/llm_response_utils/convert_dict_to_response.py | Accepts empty choice lists while consistently rejecting missing and non-list values. |
| litellm/litellm_core_utils/streaming_handler.py | Safely handles cached response chunks with no choices and permits normal stream termination. |
| litellm/llms/anthropic/experimental_pass_through/adapters/transformation.py | Converts an empty OpenAI completion into an empty Anthropic end-turn response. |
| tests/llm_translation/test_llm_response_utils/test_convert_dict_to_chat_completion.py | Verifies empty-list acceptance and precise rejection of null choices. |
| tests/test_litellm/litellm_core_utils/llm_response_utils/test_convert_dict_to_response.py | Covers synchronous and asynchronous empty, missing, and invalid choices conversion. |
| tests/test_litellm/litellm_core_utils/test_streaming_handler.py | Covers cached empty chunks and their synthesized terminal stop chunk. |
| tests/test_litellm/llms/anthropic/experimental_pass_through/adapters/test_anthropic_experimental_pass_through_adapters_transformation.py | Verifies Anthropic response shape and usage preservation for empty choices. |
| tests/test_litellm/test_main.py | Corrects the mocked SDK call so the fixture returns the intended raw response. |
Reviews (5): Last reviewed commit: "test(convert_dict_to_response): expect t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… and comment-free
|
bugbot run |
… an empty stream A stream cache hit on an entry stored with choices == [] indexed choices[0] in the cached_response branch and failed with IndexError, so the streaming converters' empty chunk had no working consumer. The branch now treats a chunk without choices as empty and lets the wrapper close the stream with its usual finish_reason stop chunk
|
bugbot run |
…converter error When a provider returns choices as null, an object, a string or a number, the converter said the response had no 'choices' even though the key was present in the raw keys it listed. A shared message now keeps the old wording for a missing key and names the offending type otherwise. The cached-stream regression test also pins the chunk count so a leaked extra chunk fails it.
|
bugbot run |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 21e6c6e. Configure here.
mateo-berri
left a comment
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
b7dad8b
into
BerriAI:litellm_internal_staging
5 participants
TLDR
Problem this solves:
choices: [](such as Gemini/Vertex safety blocks) trigger a 500APIError: Response was missing 'choices' key./v1/messagesbridge turn that empty list into a 500 the client cannot retry aroundHow it solves it:
"choices"is a real list rather than relying on boolean truthiness, so{},"", andNonestill raisechoices: []to map cleanly toModelResponse(choices=[])./v1/messagesanswers an empty completion with an empty assistant message andstop_reason: end_turnfinish_reason: stopchunk instead of failing onchoices[0]in the streaming wrapperchoicesis present but not a list (null, an object, a string, a number), the error names that type instead of claiming the key is missing; the old wording stays for a response that really has nochoiceskeyWhy
end_turnfor a completion with no choice: the Anthropic Messages API allows an emptycontentlist withstop_reason: end_turnand the Anthropic SDK'sMessagemodel accepts that shape, so an Anthropic client already handles it.refusalwould claim a safety refusal the provider did not report (Gemini'sNO_IMAGEmeans the requested modality was not produced),max_tokenswould be a guess: the reproduced case has zero output tokens, while a thinking model that spends its whole budget on thoughts is dropped the same way (#36881, with fix PRs #36870, #38301 and #40252 open) and arrives with nonzerooutput_tokensbecause the usage mapper folds reasoning tokens intocompletion_tokens, so the adapter cannot tell the two apart from usage and reports the one reason the SDK treats as a normal end. Raising would put the 500 back on the cache-hit path this PR fixes. The provider's own reason never reaches this layer: the Gemini transformation drops a candidate that has nocontentbefore buildingchoices(_process_candidates, unchanged here), so surfacing it is LIT-7413 (tracked publicly as #40477), a change in that transformation rather than in the converterThe same empty list is what Gemini's
MALFORMED_FUNCTION_CALLandOTHERcandidates become, because_process_candidatesdrops any candidate without acontentkey whatever itsfinishReason. So on/v1/messagesa botched tool call is now an emptyend_turnanswer where before it was a 500 after the gateway's own provider retries (two by default), the one path a nondeterministic failed call could have recovered through, and that trade holds until #40477 turns the dropped candidate into a choice with its finish reason. The truncated thinking model of #36881 is the same trade with a different cost: it ends as an emptyend_turnrather thanmax_tokens, so a client that would have continued the turn onmax_tokenswill not. A Gemini safety block is not one of these cases: the transformation turns a blocked prompt or a flaggedfinishReasoninto acontent_filterchoice, so the producer reproduced below is a candidate with nocontentkey. Thetests/test_litellm/test_main.pychange is required by the narrowed check: the patchedcreateleft aMagicMockwherechoicesgoes, which the oldIterabletest accepted and iterated to nothingWhy the converter rather than a Gemini-only change: the converter is the one place every non-stream answer, every cache hit and the
/v1/messagesbridge pass through, so fixing it there makes the first send and the cached repeat agree. The alternatives were a per-provider allowance (the converter has no provider at hand, so scoping it means threading one through every caller, and the cache-hit path would still 500 for any other provider that returns an empty list) and keeping the error (which keeps the customer's 500 and spends the gateway's retries on an answer that is empty every time). The cost is that every OpenAI-shaped upstream (OpenAI, Azure, Mistral, the rust bridge, any OpenAI-compatible server) now returnschoices: []as a 200 that router retries and fallbacks do not fire on, listed under Caveats. This also reverses thechoices: []case of #29492 (itstest_convert_to_model_response_object_empty_choices_raises_api_errorpinned the empty list as an error next to the missing key); that PR's missing-key tests stay as they wereUser Flow
Before: a Claude-compatible client pointed at a Gemini model through the gateway gets a 500 whenever Gemini answers with no candidate content
"model": "gemini-2.5-flash-image","max_tokens": 256, and a text-only question Gemini declines to answer with content{"type":"error","error":{"type":"api_error","message":"litellm.APIError: LiteLLM: provider returned a response with no 'choices'. Raw keys: ['id', 'created', 'model', 'object', 'system_fingerprint', 'choices', ...]"}}, an error that listschoicesamong the keys it says are missing. The bridge fails on the empty answer, the gateway retries, and the retry is served from the response cache the first attempt just filled"choices": []After: the same requests come back HTTP 200 as an empty assistant turn every time
"model": "gemini-2.5-flash-image","max_tokens": 256, and a text-only question Gemini declines to answer with content{"type":"message","role":"assistant","content":[],"stop_reason":"end_turn","usage":{"input_tokens":19,"output_tokens":0}}"choices": []"choices": [], served from the response cache, and a repeated/v1/messagessend comes back from the cache the same wayRelevant issues
Fixes #40276 (the 500 and the message that named
choicesas missing). The finish reason the issue also asks to see is #40477, a change in the Gemini transformationLinear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Both legs ran a 2-worker proxy with no database, booted from this branch's worktree at the named commit, with a local Redis response cache flushed and the proxy restarted between legs. Every request went to the real Gemini API. Requests 1 to 4 were each sent twice so the second send is a cache hit, requests 5 and 6 (
/v1/responses) once, and request 9 repeats those two the same way. Requests 7 and 7b copy one cached entry over another in Redis, which is the shape a semantic cache serves to a stream request, and restart the proxy so the answer comes from Redis rather than a worker's in-memory copy. Requests 8 to 8c do the same for/v1/messages: the Messages endpoint keeps its own copy of a streamed answer's events, so after the swap that copy is dropped, which makes the next stream fall through to the completion-level entry, the path in the reporter's traceback. Bearer values below are the proxy's own master key from the environment, and long response ids are shortenedconfig.yaml:Boot and shared shell setup (an image-only Gemini model asked a text-only question is a real upstream reply with a candidate but no content, which is how LiteLLM ends up with
choices: []):Before (096984b)
1. /v1/chat/completions, first send (cache miss)
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/chat/completions ${H[@]} -d '{"model":"gemini-2.5-flash-image","messages":[{"role":"user","content":"'"$Q"'"}]}'1b. /v1/chat/completions, same request 3 seconds later (cache hit)
2. /v1/chat/completions, stream=true, first send
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/chat/completions ${H[@]} -d '{"model":"gemini-2.5-flash-image","stream":true,"messages":[{"role":"user","content":"'"$Q"'"}]}'2b. /v1/chat/completions, stream=true, same request 3 seconds later (cache hit on the stream entry)
3. /v1/messages, first send (cache miss)
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/messages ${H[@]} -d '{"model":"gemini-2.5-flash-image","max_tokens":256,"messages":[{"role":"user","content":"'"$Q"'"}]}'3b. /v1/messages, same request 3 seconds later (cache hit)
4. /v1/messages, stream=true, first send
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/messages ${H[@]} -d '{"model":"gemini-2.5-flash-image","max_tokens":256,"stream":true,"messages":[{"role":"user","content":"'"$Q"'"}]}'4b. /v1/messages, stream=true, same request 3 seconds later
5. /v1/responses
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/responses ${H[@]} -d '{"model":"gemini-2.5-flash-image","input":"'"$Q"'"}'6. /v1/responses, stream=true
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/responses ${H[@]} -d '{"model":"gemini-2.5-flash-image","stream":true,"input":"'"$Q"'"}'7. Stream cache hit on an entry stored with
choices: [](the shape a semantic cache serves)redis-cli -p 21556 --raw GET $EMPTY_KEY | cut -c1-220(a cached non-stream answer withchoices: [], in this leg the one request 5 stored, since/v1/responsescaches its underlying chat completion too):redis-cli -p 21556 --raw GET $STREAM_KEY | cut -c1-220(request 2's cached answer):redis-cli -p 21556 SET $STREAM_KEY "$(redis-cli -p 21556 --raw GET $EMPTY_KEY)":7b. /v1/chat/completions, stream=true, sent again after the restart (served from the copied entry)
8. Stream cache hit on
/v1/messageson an entry stored withchoices: [](the path in the reporter's traceback)The cache is flushed first, then requests 3 and 4 are sent once each, the completion-level stream entry is overwritten with the empty non-stream entry, and the Messages endpoint's own copy of the streamed events is dropped so the next stream falls through to the completion-level cache
8a. /v1/messages, first send after the flush
8b. /v1/messages, stream=true, first send after the flush
redis-cli -p 21556 --raw GET $EMPTY_KEY | cut -c1-220(request 8a's cached answer):redis-cli -p 21556 --raw GET $STREAM_KEY | cut -c1-220(request 8b's cached answer):redis-cli -p 21556 SET $STREAM_KEY "$(redis-cli -p 21556 --raw GET $EMPTY_KEY)":redis-cli -p 21556 --raw GET $SSE_KEY | cut -c1-160(the Messages endpoint's own copy of request 8b's events):redis-cli -p 21556 DEL $SSE_KEY:8c. /v1/messages, stream=true, sent again after the restart (served from the copied entry)
9. /v1/responses sent twice, non-stream and stream, after emptying the cache
redis-cli -p 21556 FLUSHALL, then each/v1/responsesrequest is sent twice, 3 seconds apart9a. /v1/responses, first send after the flush (cache miss)
9b. /v1/responses, same request 3 seconds later (cache hit)
9c. /v1/responses, stream=true, first send after the flush
9d. /v1/responses, stream=true, same request 3 seconds later (cache hit on the stream entry)
After (21e6c6e)
1. /v1/chat/completions, first send (cache miss)
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/chat/completions ${H[@]} -d '{"model":"gemini-2.5-flash-image","messages":[{"role":"user","content":"'"$Q"'"}]}'1b. /v1/chat/completions, same request 3 seconds later (cache hit)
2. /v1/chat/completions, stream=true, first send
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/chat/completions ${H[@]} -d '{"model":"gemini-2.5-flash-image","stream":true,"messages":[{"role":"user","content":"'"$Q"'"}]}'2b. /v1/chat/completions, stream=true, same request 3 seconds later (cache hit on the stream entry)
3. /v1/messages, first send (cache miss)
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/messages ${H[@]} -d '{"model":"gemini-2.5-flash-image","max_tokens":256,"messages":[{"role":"user","content":"'"$Q"'"}]}'3b. /v1/messages, same request 3 seconds later (cache hit)
4. /v1/messages, stream=true, first send
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/messages ${H[@]} -d '{"model":"gemini-2.5-flash-image","max_tokens":256,"stream":true,"messages":[{"role":"user","content":"'"$Q"'"}]}'4b. /v1/messages, stream=true, same request 3 seconds later
5. /v1/responses
curl -s -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/responses ${H[@]} -d '{"model":"gemini-2.5-flash-image","input":"'"$Q"'"}'6. /v1/responses, stream=true
curl -sN -w '\nHTTP %{http_code}\n' http://127.0.0.1:$PORT/v1/responses ${H[@]} -d '{"model":"gemini-2.5-flash-image","stream":true,"input":"'"$Q"'"}'7. Stream cache hit on an entry stored with
choices: [](the shape a semantic cache serves)redis-cli -p 21556 --raw GET $EMPTY_KEY | cut -c1-220(request 1's cached answer):redis-cli -p 21556 --raw GET $STREAM_KEY | cut -c1-220(request 2's cached answer):redis-cli -p 21556 SET $STREAM_KEY "$(redis-cli -p 21556 --raw GET $EMPTY_KEY)":7b. /v1/chat/completions, stream=true, sent again after the restart (served from the copied entry)
8. Stream cache hit on
/v1/messageson an entry stored withchoices: [](the path in the reporter's traceback)The cache is flushed first, then requests 3 and 4 are sent once each, the completion-level stream entry is overwritten with the empty non-stream entry, and the Messages endpoint's own copy of the streamed events is dropped so the next stream falls through to the completion-level cache
8a. /v1/messages, first send after the flush
8b. /v1/messages, stream=true, first send after the flush
redis-cli -p 21556 --raw GET $EMPTY_KEY | cut -c1-220(request 8a's cached answer):redis-cli -p 21556 --raw GET $STREAM_KEY | cut -c1-220(request 8b's cached answer):redis-cli -p 21556 SET $STREAM_KEY "$(redis-cli -p 21556 --raw GET $EMPTY_KEY)":redis-cli -p 21556 --raw GET $SSE_KEY | cut -c1-160(the Messages endpoint's own copy of request 8b's events):redis-cli -p 21556 DEL $SSE_KEY:8c. /v1/messages, stream=true, sent again after the restart (served from the copied entry)
9. /v1/responses sent twice, non-stream and stream, after emptying the cache
redis-cli -p 21556 FLUSHALL, then each/v1/responsesrequest is sent twice, 3 seconds apart9a. /v1/responses, first send after the flush (cache miss)
9b. /v1/responses, same request 3 seconds later (cache hit)
9c. /v1/responses, stream=true, first send after the flush
9d. /v1/responses, stream=true, same request 3 seconds later (cache hit on the stream entry)
Observations from the run:
/v1/responsesalready returnedoutput: []before, unchanged/v1/messagessend (3b) is a cache hit. Before the fix the first send (3) fails with the same message because the router retries the bridge's error into the entry the first attempt filled/v1/messagesstream (4b) is served from the Messages endpoint's own copy of the events and never reaches the completion-level entry, so 8 drops that copy after the swap; 8c is then the reporter's trace, HTTP 500 before and an emptyend_turnstream after/v1/responsessend (9b) is HTTP 200 at both commits because the endpoint caches its own Responses object next to the underlying chat completion and replays that copy, so the repeat never reaches the converter; the stream repeat (9d) is served from the stream entry, which carries a choiceNO_IMAGEfinish reason is not surfaced, unchanged ([Bug]: Gemini candidate with a finishReason but no content is dropped, so the reason never reaches the client #40477, LIT-7413)Type
🐛 Bug Fix
Caveats (if any)
Medium
convert_to_model_response_object(OpenAI, Azure, Mistral, the rust bridge, any OpenAI-compatible server) now returnschoices: []as a 200, so router retries and fallbacks do not fire on it, clients that indexchoices[0]see an empty list, and with a response cache on that empty 200 is served for the entry's TTL after the upstream has recoveredMALFORMED_FUNCTION_CALL,OTHER) and a thinking model that spends its whole output budget on thoughts (MAX_TOKENSwith nocontent, Vertex/Gemini: content-less candidate (thinking model at MAX_TOKENS) yields empty choices -> IndexError on choices[0] #36881) now end a non-stream/v1/messagesturn as an emptyend_turninstead of a 500 after the gateway's own provider retries, the one path a nondeterministic failure could have recovered through; the truncated case arrives with nonzerooutput_tokens, and a client that continues a turn onmax_tokenswill notchoices; [Bug]: Gemini candidate with a finishReason but no content is dropped, so the reason never reaches the client #40477 (LIT-7413) surfaces it, and fix(vertex_ai): return a truncated choice when a Gemini candidate has no content #36870, fix(vertex_ai): return a truncated choice for content-less Gemini candidates #38301 and fix(vertex_ai): return an empty choice for content-less Gemini candidates #40252 are the open fixes for the truncationchoices[0]still turn the empty answer into a 500 one step later, the same as at the merge base: presidio withoutput_parse_pii(turning it off is the workaround), the content filter's own image-description call, prompt injection detection, and/v1/completionswith a listprompt(LIT-7414)stream, so only a semantic cache, a presetcache_key, or an expired Messages-level copy serves that entry to a stream request. The issue does not name the client, so the proof is curlLow
choicesasnull, an object or a string is covered by unit tests only; no real provider returns that shape on demandend_turnturn was not driven live; the Anthropic SDK parses that shapeosv-scanis not a required check and fails on a base lockfile advisory that PR fix(ui): bump smol-toml to 1.8.0 to clear GHSA-7w5x-hrqm-74c2 in osv-scan #40442 clearsFinal Attestation
/v1/messagesbridge by 3, 3b, 4, 4b and 8c, the Responses API cache replay by 9b and 9d (both carry a choice); the non-listchoicesmessage is covered by unit tests only, since no real provider returns that shape on demand; the guardrail block reply inproxy_server.py, the other producer of acached_responsestream, always carries a choice and was not driven liveNote
Medium Risk
Changes global completion conversion semantics so empty
choicesreturns HTTP 200 instead of 500, which may surprise clients that assume at least one choice; cache and bridge behavior shifts accordingly.Overview
Fixes 500 errors when providers return
choices: [](e.g. Gemini/Vertex candidates with no content), including response-cache replays that previously failed after a successful first request.Response conversion now requires
"choices"to be a list (isinstance(..., list)) instead of treating an empty list as missing.choices: []maps to an emptyModelResponse/ streaming chunk; missing or non-list values still raiseAPIErrorvia shared_invalid_choices_message(clearer type-specific messages).Downstream paths are aligned: the cached_response stream handler no longer indexes
choices[0]on empty lists; the Anthropic/v1/messagesadapter defaults finish reason when there are no choices (emptycontent+stop_reason: end_turn). Tests cover empty lists, invalid types, cache streaming, and the adapter.Reviewed by Cursor Bugbot for commit 21e6c6e. Bugbot is set up for automated code reviews on this repo. Configure here.