Skip to content

fix(llm): preserve explicit false stream override - #1531

Open
Rapeter wants to merge 1 commit into
alibaba:mainfrom
Rapeter:codex/fix-extra-body-stream-false
Open

Rapeter wants to merge 1 commit into
alibaba:mainfrom
Rapeter:codex/fix-extra-body-stream-false

Conversation

@Rapeter

@Rapeter Rapeter commented Sep 22, 2026

Copy link
Copy Markdown

Summary

  • Forward an explicit extra_body.stream: false for non-streaming OpenAI Chat Completions, Anthropic Messages, and OpenAI Responses requests.
  • Continue dropping stream: true, non-boolean stream values, and OpenAI stream_options where they are not valid.
  • Add wire-level regression coverage for all three clients.

Fixes #1527

Testing

  • go test -count=1 -run 'TestOpenAIClient_NonStreamingRequestDropsStreamField|TestAnthropicClient_ExtraBodyStreamDropped|TestAnthropicClient_ExtraBodyStreamFalseForwarded|TestOpenAIResponsesClient_ExtraBodyStreamDropped|TestOpenAIResponsesClient_ExtraBodyStreamFalseForwarded' ./internal/llm
  • Full non-race package suite: passed.
  • go vet and git diff --check: passed.
  • ocr review --audience agent --background ...: attempted but could not run because no LLM endpoint is configured in the local environment.
  • The repository's race-enabled make test could not run on this Windows environment because of the existing shell/runtime compatibility issue.

AI disclosure

Prepared with OpenAI Codex (GPT-5). I reviewed every changed line and understand the behavior and test coverage.

@CLAassistant

CLAassistant commented Sep 22, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 2 issue(s) in this PR.

  • ✅ Successfully posted inline: 2 comment(s)

Comment thread internal/llm/client.go
Comment on lines 1305 to 1310
if k == "stream" {
continue
stream, isBool := v.(bool)
if !isBool || stream {
continue
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · low
The comment on lines 1301-1304 says "Drop the key rather than forward it", but the code now forwards stream: false to the API. The comment should be updated to reflect the new conditional behavior, e.g.: "Drop the key when it would trigger SSE (non-boolean or true); forward explicit false so callers can override a default."

Comment on lines 126 to 131
if k == "stream" {
continue
stream, isBool := v.(bool)
if !isBool || stream {
continue
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

documentation · low
Same stale comment issue as in the Anthropic client: the comment says "Drop the key rather than forward it" but the code now conditionally forwards stream: false. Update the comment to match the new behavior.

@lizhengfeng101

Copy link
Copy Markdown
Contributor

@Rapeter Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extra_body.stream=false is silently dropped, breaking non-streaming OpenAI-compatible clients against gateways that stream by default

3 participants