Skip to content

fix: keep finish_reason in create_stream when the finish chunk also carries usage - #8278

Open
Lucas Kim (kimnamu) wants to merge 1 commit into
microsoft:mainfrom
kimnamu:fix/bedrock-gpt-6-stream-finish-reason
Open

Lucas Kim (kimnamu) wants to merge 1 commit into
microsoft:mainfrom
kimnamu:fix/bedrock-gpt-6-stream-finish-reason

Conversation

@kimnamu

Copy link
Copy Markdown

Why are these changes needed?

Thanks for keeping create_stream working across so many OpenAI-compatible servers; the usage-chunk handling from #3972 is what this builds on.

create_stream reads finish_reason only from chunks with usage is None (_openai_client.py L948). The Amazon Bedrock runtime endpoint /openai/v1/chat/completions attaches usage to the GPT-6 finish chunk, so the streamed CreateResult.finish_reason is always "unknown", even for length and tool calls, while create() on the same request returns the real value.

This drops the chunk.usage is None condition. The stop_reason is None check that stays already keeps an earlier finish reason when a LiteLLM-style usage chunk follows. The Azure AI client already reads finish_reason whenever it is set.

Before After
Bedrock runtime us.openai.gpt-6-sol / us.openai.gpt-6-luna (and global.) stream: text, truncated, tool call¹ ❌ unknown ×3 each ✅ stop / length / function_calls (same as create())
Bedrock runtime us.openai.gpt-6-astra / global.openai.gpt-6-astra stream: text, truncated ❌ unknown ×2 each ✅ stop / length (same as create())
openai.gpt-oss-120b-1:0, Mantle openai.gpt-6-sol / openai.gpt-6-astra (usage not on finish chunk) ✅ ✅ unchanged
Streamed usage (GPT-6 Sol text) (8, 5) ✅ (8, 5) unchanged
LiteLLM-style usage chunk after stop (existing tests) ✅ ✅ unchanged
Public API, arguments, chunk order — ✅ unchanged

¹ with reasoning_effort="none", which Bedrock chat completions needs for Sol/Luna function tools.

Test RED/GREEN, checks, live runs

New test test_openai_chat_completion_client_create_stream_finish_reason_with_usage (mocked: content chunk → finish chunk with usage → choices=[] usage chunk, stop and length):

# source reverted to main
E       AssertionError: assert 'unknown' == 'stop'
E       AssertionError: assert 'unknown' == 'length'
2 failed, 86 deselected in 2.68s
# with fix
2 passed, 86 deselected in 2.41s

tests/models/test_openai_model_client.py: main 2 failed / 53 passed (the new test), fix 55 passed, 33 skipped. The agentchat tests that use this client (test_assistant_agent.py, test_group_chat.py, test_declarative_components.py): 177 passed, 3 skipped. ruff format --check and ruff check clean, pyright 0 errors, mypy no issues on both files.

Live, base_url=https://bedrock-runtime.us-east-1.amazonaws.com/openai/v1, max_completion_tokens=5:

# main
us.openai.gpt-6-sol: create=length create_stream=unknown
us.openai.gpt-6-luna: create=length create_stream=unknown
us.openai.gpt-6-astra: create=length create_stream=unknown
# this branch
us.openai.gpt-6-sol: create=length create_stream=length
us.openai.gpt-6-luna: create=length create_stream=length
us.openai.gpt-6-astra: create=length create_stream=length

Astra has no tool-call row: Bedrock's chat completions endpoint returns 400 for function tools with Astra ("To use function tools, use /v1/responses"), before and after this change.

Related issue number

Fixes #8277

Checks

…arries usage

OpenAIChatCompletionClient.create_stream only read choice.finish_reason
from chunks without usage. Servers that attach usage to the finish chunk
(e.g. the Amazon Bedrock runtime /openai/v1 endpoint with GPT-6 models)
always ended up with finish_reason "unknown", including length truncation
and tool calls. The existing `stop_reason is None` guard already keeps an
earlier finish_reason from being overwritten by a trailing usage chunk.

Co-Authored-By: Claude <noreply@anthropic.com>

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

1 participant