fix(streaming): backfill response.completed output from output_item.done events - #31332
Conversation
Greptile SummaryThis PR fixes a streaming failure where certain providers (e.g. chatgpt.com's Codex backend) send
Confidence Score: 5/5The change is safe to merge: it adds opt-in backfill that only fires when output is empty and accumulated items exist, so existing providers that send authoritative output in response.completed are completely unaffected. The backfill path is tightly guarded (empty-output check + non-empty accumulator check), the list comprehension is inside the try/except so serialization failures degrade to a logged warning rather than crashing the stream, Pydantic and dict items are both handled, and all new code is thoroughly tested with 12 focused regression tests. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/responses/streaming_iterator.py | Core fix: adds _streamed_output_items/_streamed_text_only_items accumulation in init, backfill logic in _process_chunk, and _accumulate_streamed_output_item method. Backfill list comprehension is correctly inside the try/except. Both Pydantic and dict items are handled via hasattr(item, "model_dump") check. |
| litellm/responses/mcp/mcp_streaming_iterator.py | Defensive initialization of _streamed_output_items and _streamed_text_only_items for MCPEnhancedStreamingIterator, which bypasses super().init(). These are never populated by the MCP class itself (it delegates to an inner base_iterator), but prevent AttributeError if something inspects them. |
| litellm/router.py | Defensive initialization of the two new dicts on FallbackResponsesStreamWrapper, which also bypasses super().init(). The wrapper proxies already-processed chunks from inner iterators (where backfill already occurred), so these dicts are never populated. |
| tests/test_litellm/responses/test_streaming_iterator_output_recovery.py | New test file with 12 regression tests covering all significant code paths; uses only mocks and locally constructed httpx.Response objects — no real network calls. Well-structured helpers and comprehensive edge case coverage. |
| tests/test_litellm/llms/chatgpt/responses/test_chatgpt_responses_transformation.py | Only ruff reformatting changes (line wrapping in function arguments); no logical changes to assertions or test behavior. |
Reviews (5): Last reviewed commit: "fix(streaming): backfill response.comple..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a bug where
Confidence Score: 4/5Safe to merge once the model_dump() call is moved inside its guarding try-except; all other changes are formatting or straightforward attribute additions. The backfill logic in _process_chunk guards only the final output assignment inside a try-except, but the model_dump() list comprehension that builds the backfill payload sits just outside it. If any accumulated item lacks model_dump() — for example, a plain dict returned by a provider's transformation layer — the exception bypasses the warning handler and reaches the outer except block, which calls _handle_failure() and re-raises. That turns a best-effort recovery step into a hard stream failure. In the current chatgpt.com provider path the items are always BaseLiteLLMOpenAIResponseObject instances, so the happy path works, but the error boundary is incorrectly drawn for other providers. litellm/responses/streaming_iterator.py — specifically the backfill block inside _process_chunk around the model_dump() call
|
| Filename | Overview |
|---|---|
| litellm/responses/streaming_iterator.py | Adds OUTPUT_ITEM_DONE and OUTPUT_TEXT_DONE accumulation + response.completed backfill; the model_dump() call is incorrectly placed outside its guarding try-except, which can cause stream failure instead of graceful degradation |
| litellm/responses/mcp/mcp_streaming_iterator.py | Mirrors the two new accumulator dict attributes in MCPEnhancedStreamingIterator, which bypasses super().init(); straightforward and correct |
| litellm/router.py | Adds the two new accumulator dicts to FallbackResponsesStreamWrapper; uses bare dict type annotation instead of dict[int, BaseLiteLLMOpenAIResponseObject] — minor inconsistency, no runtime impact |
| litellm/llms/chatgpt/responses/transformation.py | Ruff-only reformatting; no logical changes |
| tests/test_litellm/responses/test_streaming_iterator_output_recovery.py | Seven new unit tests covering backfill, ordering, fallback precedence, incomplete handling, and dict-type contract; all mock-only, no real network calls |
| tests/test_litellm/llms/chatgpt/responses/test_chatgpt_responses_transformation.py | Ruff-only reformatting of existing tests; no assertion changes, no weakened coverage |
Reviews (1): Last reviewed commit: "fix(streaming): backfill response.comple..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ec55844 to
3b5c7d5
Compare
|
Validated this approach locally against Before the patch, {"message":"ChatgptException - Unknown items in responses API response: []"}After applying the streaming iterator accumulation/backfill approach from this PR locally:
So this PR matches the failure mode I hit and fixes it in practice. |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
Following up on my June 26 validation — confirmed still reproducible on
The non-streaming path hits the In agent frameworks like Manually applied the patch to
Config: |
|
Conflict resolution detail (for the rebase): The merge conflict in Resolution that worked against current
The |
3b5c7d5 to
23795ac
Compare
23795ac to
2f6b895
Compare
|
@ryan-crabbe-berri @mateo-berri would appreciate a review when you get a chance. Greptile is 5/5 and veria-ai shows no open security concerns. The three failing CI checks are all pre-existing (repo-wide lint, a flaky agentcore mock test, and the SSO test broken by #33261). |
2f6b895 to
0fd9e47
Compare
e475d8b to
d6c9829
Compare
|
All CI checks are now green. @ryan-crabbe-berri @mateo-berri this is ready for a final review and merge. |
29d7b20 to
5b256f6
Compare
735a6a0 to
2d66e86
Compare
|
This will fix chatgpt suscriptions not working, right ? What is blocking this important fix at this point ? |
|
Reproduced on released Non-stream: HTTP 500 This is the same empty |
|
As soon as this is fixed one wrapper help container to pipe through the chatgpt llms will not be needed anymore in kubernetes. |
|
Verified this also fixes the #25429 bridge failure: non-streaming /v1/chat/completions on chatgpt/gpt-5.4 returned 500 on staging and 200 on this head. |
5189a39 to
bd29c24
Compare
…one SSE events When streaming ChatGPT subscription responses, the terminal response.completed event carries an empty output: [] even though the model produced text. The streaming iterator now accumulates output_item.done and output_text.done events as they arrive and backfills them into the completed response object at the terminal event, so logging, spend tracking, and post-stream hooks see the real content instead of an empty list.
bd29c24 to
755a8b1
Compare
|
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 755a8b1. Configure here.
|
Please also consider the option to
|
|
If you do not want to wait for this PR to merge, you can try the fix now with an ephemeral https://github.com/safrano9999/litellm-database-chatgpt-reasoning#try-it-now For an existing Podman Quadlet setup, it is a drop-in image replacement; the only additional mount is one named volume to persist the ChatGPT authentication directory. The existing database configuration and environment stay unchanged. Immutable image: The The build also publishes OCI source/base/patch hashes, BuildKit provenance, an SPDX SBOM, and a GitHub artifact attestation bound to the image digest. This is now running reliably in my live setup: ChatGPT Responses output is recovered correctly and the models work end-to-end without an additional wrapper. Everything finally works cleanly. Based on that result, I recommend merging this fix into |
Out of scope for this PR i believe |
Relevant issues
Fixes #25429. Related to #26179. Supersedes #30934
Linear ticket
N/A
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit(201 pre-existing failures in prisma/database, vertex AI, and MCP semantic filter tests; none in touched files — 324 tests acrosstests/test_litellm/responses/andtests/test_litellm/llms/chatgpt/pass cleanly)@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
The
chatgpt/provider routes through chatgpt.com's Codex backend, which sendsresponse.completedwithoutput: []. The actual assistant content arrives via precedingresponse.output_item.doneSSE events. Without accumulation, the chat-completions bridge receives an empty output list and raises:The fix teaches
BaseResponsesAPIStreamingIterator._process_chunkto accumulateresponse.output_item.donepayloads as they stream in and backfill them into theresponse.completedchunk before it is stored ascompleted_response. Items are serialized to plain dicts viamodel_dump()so the downstream_handle_raw_dict_response_itemcallback in the transformation layer can process them. The existing_recover_output_items_from_raw_ssefallback inLiteLLMResponsesTransformationHandler.transform_responseis preserved as a second layer if the streaming path is ever bypassed.Before:
After:
Type
Bug Fix
Changes
BaseResponsesAPIStreamingIteratoraccumulatesresponse.output_item.doneitems into_streamed_output_itemsduring streaming. A secondary_streamed_text_only_itemsdict catches providers that emitresponse.output_text.donewithout a precedingresponse.output_item.done. Atresponse.completedorresponse.incomplete, ifoutputis empty and either dict is non-empty, the merged items are sorted byoutput_indexand backfilled onto the response object. Items are serialized viamodel_dump()when they are Pydantic instances and passed through unchanged when they are already plain dicts; both the serialization and the assignment sit inside atry/exceptso any failure degrades to a logged warning rather than crashing the stream.FallbackResponsesStreamWrapper(router.py) andMCPEnhancedStreamingIterator(mcp_streaming_iterator.py) both bypasssuper().__init__()and are updated to mirror the two new instance attributes.Twelve regression tests are added to
tests/test_litellm/responses/test_streaming_iterator_output_recovery.py, covering: the core backfill, multi-item index ordering, authoritative output preservation,response.incompletebackfill,response.failedno-backfill,output_text.donefallback,output_item.doneprecedence, replace-in-place content slots, gap padding, absentoutput_indexsequential fallback, exception swallowing, and the dict-type contract required by the downstream transformation layer.The bulk of the line-count change in
streaming_iterator.pyis ruff reformatting of pre-existing code required to passruff format; the logical additions are confined to the accumulation block in_process_chunkand the two new instance attributes in__init__.Note
Cursor Bugbot is generating a summary for commit 755a8b1. Configure here.