fix(streaming): detect text-only stream drops with no finish_reason (#32086) - #67616
Closed
ajzrva-sys wants to merge 1 commit into
Closed
fix(streaming): detect text-only stream drops with no finish_reason (#32086)#67616ajzrva-sys wants to merge 1 commit into
ajzrva-sys wants to merge 1 commit into
Conversation
…ousResearch#32086) When a streaming response ends cleanly (HTTP 200) with no finish_reason after delivering text but no tool calls, the chunk collector silently stamps finish_reason='stop' and the conversation loop presents truncated text as a complete response. Three stream-drop paths now exist after chunk collection: 1. Zero-chunk → EmptyStreamError, retried (existing) 2. Tool-call in progress, no finish_reason → partial-stream-stub (existing) 3. Text-only, no finish_reason → partial-stream-stub (NEW — this fix) Path 3 routes through the same PARTIAL_STREAM_STUB_ID + FINISH_REASON_LENGTH machinery as path 2. The conversation loop shows 'Stream interrupted — requesting continuation' and injects a continue prompt, giving the model a chance to resume where the stream dropped. Observed with DeepSeek provider where CloudFront drops SSE streams mid-response after delivering partial text.
Collaborator
|
Merged via #67923 — your fix was cherry-picked with authorship preserved, plus a follow-up refactor extracting a shared Thanks for the clear writeup and the well-targeted fix! |
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.
Summary
When a streaming response ends cleanly (HTTP 200, no error) with no finish_reason after delivering text but no tool calls, the chunk collector silently stamps
finish_reason='stop'and the conversation loop presents truncated text as a complete response. The model's intended next step is lost, and the user sees a partial sentence with no indication anything went wrong.Current State
Three stream-drop paths exist in
_interruptible_streaming_api_call's chunk collector:Fix
Adds Path 3 detection: when
finish_reason is Noneand we received text content but no tool calls, route through the samePARTIAL_STREAM_STUB_ID+FINISH_REASON_LENGTHmachinery as Path 2. The conversation loop already handles this — it shows⚠️ Stream interrupted by network errorthen↻ Stream interrupted — requesting continuation (1/4)...and injects a continue prompt.Changes
agent/chat_completion_helpers.py: add text-only stream-drop guard between the tool-call drop check and the effective_finish_reason fallthroughtests/run_agent/test_partial_stream_finish_reason.py: new testtest_no_finish_reason_text_only_routes_to_stub— clean stream end with text only, no finish_reason, verifies stub routingValidation
Fixes #32086