fix(agentic loop): return a real stream when code-interpreter interception converts the request - #37657
Draft
vineethsaivs wants to merge 2 commits into
Conversation
…e request
With code_interpreter_interception enabled, a `stream: true` request is
converted to a non-streaming call, and the dispatcher hands the result back
in streamed form. It returned a bare ModelResponseStream, which is a single
chunk object and not iterable, so the caller's `async for` failed with:
TypeError: 'async for' requires an object with __aiter__ method,
got ModelResponseStream
A plain assistant reply was enough to hit it, because the no-tool-call path
converts the response the same way. The chunk-shaped object also reached the
response cache, so a later request reading that entry raised KeyError: 'message'
in convert_to_model_response_object.
Wrap the response in a CustomStreamWrapper over MockResponseIterator, the
pattern the guardrail hooks already use to replay an assembled response as a
stream. main.py's `isinstance(response, CustomStreamWrapper)` branch now
recognises the result, and the existing cast on the return value becomes true.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
6 tasks
The strict gate flagged the new `Any` import as TID251 ("Use a concrete type").
It was only there for `cast(Any, logging_obj)`, so import `Logging` under
TYPE_CHECKING and cast to it by name. The file types `logging_obj` as `object`
throughout and narrows with `hasattr`, which this keeps, and the cast now says
what the value actually is rather than switching the checker off.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title
fix(agentic loop): return a real stream when code-interpreter interception converts the request
Relevant issues
Fixes #37652
Pre-Submission checklist
tests/directory: two cases intests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.pyType
🐛 Bug Fix
Root cause
code_interpreter_interceptionconverts astream: truechat request into a non-streaming call, marks it with_code_interpreter_interception_converted_stream, and then asks the agentic-loop dispatcher to hand the result back in streamed form._wrap_response_as_fake_stream(litellm/litellm_core_utils/chat_completion_agentic_loop.py) did that withconvert_model_response_to_streaming, which returns a singleModelResponseStreamobject. That is one chunk, not a stream: it has no__aiter__. The caller iterates it, so:Two consequences, both of which the issue reports:
litellm/main.pyguards the result withif isinstance(response, CustomStreamWrapper), and aModelResponseStreamfails that check, so the object is returned to the proxy untouched and blows up on iteration. This fires on both call sites, which is why a plain assistant reply with no tool call is enough: the no-follow-up path at the end ofmaybe_run_chat_completion_agentic_loopconverts the original response the same way.deltawheremessageis expected, hence the reportedKeyError: 'message'insideconvert_to_model_response_object.The existing
cast("ModelResponse | CustomStreamWrapper", ...)on the return value was simply not true.Fix
Wrap the response in a
CustomStreamWrapperoverMockResponseIterator, which is the pattern already used elsewhere in the repo for exactly this (replaying an assembled response as a stream): seebedrock_guardrails.py,tool_permission.py,model_armor.pyand six other guardrail hooks.MockResponseIteratorcalls the sameconvert_model_response_to_streaminginternally, so the chunk content is unchanged; it is now delivered through an object that is actually iterable, andmain.py'sisinstancebranch recognises it._wrap_response_as_fake_streamneedsmodel,custom_llm_providerandlogging_objto build the wrapper. All three are already in scope at both call sites. Iflogging_objis not a real logging object the helper returns the response unchanged, so this can never turn a working call into a new exception.Not folded in: the interception path still cannot deliver token-by-token output, since the underlying call really is non-streaming. This makes the response a well-formed single-chunk SSE stream rather than a 500, which is option (a) in the issue at the granularity the current design allows.
Testing
Two new tests in the existing
tests/test_litellm/litellm_core_utils/test_chat_completion_agentic_loop.py, one per affected path (after a tool-call follow-up, and with no tool call at all). Both assert the result is aCustomStreamWrapperand then actuallyasync forover it and check the reassembled text.The same file against unpatched
chat_completion_agentic_loop.py:Removing the
isinstanceassertion and going straight to the iteration reproduces the reported error verbatim on unpatched source:Regression control, the whole surrounding area, branch versus a pristine tree:
Exactly the two new tests, no other movement. The 3 failures are pre-existing on the base branch (
test_bedrock_converse_messages_pt_document_various_formats,TestIsBlockedIp::test_blocks_ietf_protocol_assignments_old_oracle_metadata,test_logfire_logger_accepts_env_vars_for_base_url) and unrelated.ruff checkreports the identical 31 pre-existing findings on both files before and after, none in the changed lines;ruff format --checkreports both files already formatted.To be explicit about what I did not do: I validated this through the dispatcher and the test suite, not against a live proxy with a real sandboxed code-interpreter run, so the end-to-end SSE delivery to a client is not something I can claim to have exercised. I also could not reproduce the cache half of the issue directly; the fix addresses it by never producing the chunk-shaped object in the first place, which is the object the reporter traced the
KeyError: 'message'to.