Conversation
`_wrap_response_as_fake_stream` returned the bare converted chunk, so a `code_interpreter_interception` request with `stream: true` failed with `TypeError: 'async for' requires an object with __aiter__ method, got ModelResponseStream`. The same bare chunk is `chat.completion.chunk`-shaped where a full `chat.completion` is expected, which is how a malformed entry reaches the response cache and later raises `KeyError: 'message'`. Wrap the response in a `CustomStreamWrapper` over a `MockResponseIterator` — the pattern the responses-transformation handler already uses — so the value is genuinely async- and sync-iterable. The `cast` at the call site already claimed `CustomStreamWrapper`; now that is true. Fixes BerriAI#37652 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR replaces a bare converted response chunk with a sync- and async-iterable
Confidence Score: 4/5The PR appears safe to merge, with only a non-blocking test-organization issue. The fake-stream implementation restores the expected iterable streaming contract, and no reachable runtime regression was established; the remaining concern is that its tests are separated from the mapped test file. Files Needing Attention: tests/test_litellm/litellm_core_utils/test_agentic_loop_fake_stream.py
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/chat_completion_agentic_loop.py | Wraps downgraded complete responses with the established stream abstraction; no blocking correctness issue was established. |
| tests/test_litellm/litellm_core_utils/test_agentic_loop_fake_stream.py | Adds focused regression coverage, but places it in a new module rather than the repository’s existing mapped test file. |
Reviews (1): Last reviewed commit: "style: apply ruff format to the new agen..." | Re-trigger Greptile
| @@ -0,0 +1,72 @@ | |||
| """A downgraded stream must come back as something `async for` can consume.""" | |||
There was a problem hiding this comment.
Regression tests use separate module
This bug fix places all five regression cases in a new module instead of extending the existing mapped test file for chat_completion_agentic_loop.py, fragmenting coverage and making the cases easier to omit from targeted test runs.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Fixes #37652.
Root cause
_wrap_response_as_fake_stream(litellm/litellm_core_utils/chat_completion_agentic_loop.py)returned the bare converted chunk:
convert_model_response_to_streamingreturns a singleModelResponseStream—one chunk, not a stream. Measured on a clean checkout:
That is both reported symptoms in one line:
stream: trueto a singlenon-streamed call so the agentic loop can run, then hands this value back to
the streaming path, which does
async forover it —TypeError: 'async for' requires an object with __aiter__ method, got ModelResponseStream.No tool call is needed to reach it; a plain assistant reply is enough.
chat.completion.chunk-shapedwhere a full
chat.completionis expected, so a cached copy carriesdeltainstead of
messageand a later read raisesKeyError: 'message'insideconvert_to_model_response_object. The reporter saw this in productionwithout an independent repro; the
objectfield above is the mechanism.The
castat one of the call sites already claimed"ModelResponse | CustomStreamWrapper", which the returned value neversatisfied.
The fix
Wrap the response in a
CustomStreamWrapperover aMockResponseIterator—the same construction
litellm/completion_extras/litellm_responses_transformation/handler.pyalready uses to present a non-streamed response as a stream. The result is
genuinely async- and sync-iterable, and the existing
castbecomes true.The helper needs
model,custom_llm_providerandlogging_objto build thewrapper; both call sites already have all three, so they are threaded through as
keyword-only arguments.
This takes option (a) from the issue — make the combination work — rather than
(b) reject it with a 4xx, since the downgrade machinery was clearly built to
support it and only the hand-back was wrong.
Verification
Five tests in
tests/test_litellm/litellm_core_utils/test_agentic_loop_fake_stream.py:the result exposes
__aiter__and is aCustomStreamWrapper— the reported crashit is sync-iterable too
iterating yields
chat.completion.chunkobjects that reassemble the originalcontent, so the downgrade is transparent to the client
an already-wrapped stream passes through unchanged (no double wrapping)
an object with no
choicesis returned untouchedtests/test_litellm/litellm_core_utils: 10 failures on this branch and theidentical 10 on a clean checkout (verified by diffing the failure lists), so
nothing here regresses; those are pre-existing.
ruff format --checkclean on both touched files;ruff checkreports thesame 5 pre-existing findings on the source file as base, none added.
Type-discipline checker: identical counts to base on every rule.
Note on the cache symptom
I fixed the shape at its source rather than adding a guard in the cache layer.
If you would also like a defensive check where the entry is written — rejecting
a
chat.completion.chunkwhere achat.completionis expected — I am happy toadd it, but it seemed better not to paper over the producer inside this fix.
On the red
code-qualitycheck: it is not from this PR. It fails on everyPR against
litellm_internal_stagingright now, including ones that touch noworkflow files, because three unit shards in
.github/workflows/test-unit.ymlcap the job below the startup-safety invariant. Fixed independently in #38046;
this PR needs no change for it.