Fix SSE stream parser dropping tool calls on EOF#1412
Open
kanylbullen wants to merge 2 commits intorcourtman:mainfrom
Open
Fix SSE stream parser dropping tool calls on EOF#1412kanylbullen wants to merge 2 commits intorcourtman:mainfrom
kanylbullen wants to merge 2 commits intorcourtman:mainfrom
Conversation
The read loop in ChatStream breaks immediately on io.EOF without processing remaining buffered data. Per Go's io.Reader contract, Read may return both n > 0 and io.EOF simultaneously, so the final bytes (which may contain tool call deltas and [DONE]) are silently discarded. This causes the agentic loop to see tool_calls=0 even though the model correctly produced tool calls in the stream. Changes: - Process pendingData when EOF is received before breaking - Add fallback: emit accumulated tool calls if [DONE] was never reached (server closed connection early) Fixes rcourtman#1411 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes the OpenAI-compatible SSE streaming parser in ChatStream so it doesn’t drop buffered tool-call data when the underlying reader returns io.EOF (including the valid n > 0, err == io.EOF case), and adds a fallback to emit accumulated tool calls if the stream ends without a [DONE] event.
Changes:
- Process
pendingDataeven whenRead()returnsio.EOF, ensuring final SSE lines are parsed before exiting. - Append a trailing newline at EOF to force parsing of the last buffered line.
- Emit a fallback
"done"event with accumulated tool calls when the server closes the connection before[DONE]is observed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+857
to
+885
| // If we exited the loop without hitting [DONE] (e.g. server closed connection), | ||
| // still build and emit any accumulated tool calls so they aren't silently dropped. | ||
| if len(toolCallBuilders) > 0 && len(toolCalls) == 0 { | ||
| for _, builder := range toolCallBuilders { | ||
| var input map[string]interface{} | ||
| if err := json.Unmarshal([]byte(builder.args.String()), &input); err != nil { | ||
| input = map[string]interface{}{"raw": builder.args.String()} | ||
| } | ||
| toolCalls = append(toolCalls, ToolCall{ | ||
| ID: builder.id, | ||
| Name: builder.name, | ||
| Input: input, | ||
| }) | ||
| } | ||
| stopReason := finishReason | ||
| if len(toolCalls) > 0 { | ||
| stopReason = "tool_use" | ||
| } else if stopReason == "stop" || stopReason == "" { | ||
| stopReason = "end_turn" | ||
| } | ||
| callback(StreamEvent{ | ||
| Type: "done", | ||
| Data: DoneEvent{ | ||
| StopReason: stopReason, | ||
| ToolCalls: toolCalls, | ||
| InputTokens: inputTokens, | ||
| OutputTokens: outputTokens, | ||
| }, | ||
| }) |
Comment on lines
+717
to
734
| atEOF := false | ||
| for { | ||
| n, err := reader.Read(buf) | ||
| if n > 0 { | ||
| pendingData += string(buf[:n]) | ||
| } | ||
| if err != nil { | ||
| if err == io.EOF { | ||
| if err != io.EOF { | ||
| return fmt.Errorf("stream read error: %w", err) | ||
| } | ||
| atEOF = true | ||
| // At EOF, process any remaining pendingData then break | ||
| if pendingData == "" { | ||
| break | ||
| } | ||
| return fmt.Errorf("stream read error: %w", err) | ||
| // Ensure trailing data is processed by appending a newline | ||
| pendingData += "\n" | ||
| } |
Address review feedback:
- Extract shared tool-call finalization into emitFinalToolCalls closure
to eliminate duplication between [DONE] and EOF-fallback paths
- Build tool calls in deterministic index order (sorted)
- Normalize stopReason consistently in both paths
- Add unit tests:
- TestOpenAIClient_ChatStream_ToolCallWithSimultaneousEOF: verifies
tool calls are parsed when Read returns n>0 and io.EOF together
- TestOpenAIClient_ChatStream_ToolCallWithoutDONE: verifies fallback
emission when stream ends without [DONE]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
io.EOFis received instead of breaking immediately[DONE]eventRead()returns bothn > 0andio.EOFper Go's io.Reader contractProblem
The SSE read loop in
ChatStreambreaks onio.EOFbefore processingpendingData. If the server sends tool call chunks and[DONE]in the same TCP segment as the connection close, they are read into the buffer but never parsed. This causesChatStreamto returntool_calls: 0even though tool calls were received.Changes
internal/ai/providers/openai.go: Restructured the SSE read loop to processpendingDataat EOF before exitingdoneevent emission whentoolCallBuilderscontain data but[DONE]was never parsedTest plan
Fixes #1411
🤖 Generated with Claude Code