Skip to content

fix(models): don't offset streamed content_index by the reasoning item#3757

Open
AmirF194 wants to merge 1 commit into
openai:mainfrom
AmirF194:fix/stream-content-index-reasoning
Open

fix(models): don't offset streamed content_index by the reasoning item#3757
AmirF194 wants to merge 1 commit into
openai:mainfrom
AmirF194:fix/stream-content-index-reasoning

Conversation

@AmirF194

@AmirF194 AmirF194 commented Jul 8, 2026

Copy link
Copy Markdown

Problem

The Chat Completions stream handler builds each assistant content part's content_index by counting the reasoning item. But reasoning is emitted as a separate output item (its own output_index, 0), not a content part of the assistant message: _reasoning_output_count / assistant_message_output_index already place the message at a later output_index because of it.

So when a stream contains reasoning followed by assistant text (and/or a refusal), the message's first content part is streamed with content_index=1, even though it lands at message.content[0] in the final response.completed message, and even though the OpenAI Responses API emits content_index=0 for the same output. content_index is the part's position within that item's content array; output_index is what distinguishes items.

This affects reasoning models routed through the Chat Completions adapter (for example deepseek-reasoner, and OpenAI-compatible reasoning models) that also return visible text or a refusal in the same turn. A consumer that reconstructs the message from the raw streamed events via message.content[event.content_index] writes to the wrong slot (or hits an IndexError). The SDK's high-level RunResult is unaffected because it reads the final response.completed item; the impact is on consumers of the raw streamed events (raw_response_event).

Root cause

In chatcmpl_stream_handler.py, the text and refusal branches both did:

content_index = 0
if state.reasoning_content_index_and_output:   # reasoning is a separate output item, not a content part
    content_index += 1
if state.refusal_content_index_and_output:
    content_index += 1

The text/refusal cross-increment is correct (both are content parts of the same message). The reasoning increment is the bug.

Fix

Drop the reasoning increment in both branches. The final message assembly appends parts in order and never used these content_index values, so only the streamed events change; the final response.completed message is byte-identical.

Verification

Clean Docker container (python:3.12-slim, uv sync --frozen):

  • New regression test test_stream_handler_content_index_excludes_reasoning_item fails on main (streamed content_index=1) and passes with the fix.
  • tests/models/test_openai_chatcompletions_stream.py and tests/models/test_reasoning_content.py: 62 passed. The existing content_index == 1 test (refusal-then-text, no reasoning) still passes, via the refusal increment.
  • ruff check, ruff format --check, and mypy on the changed files: clean.

No public API change (parameter/field order and import paths are untouched). This corrects the streamed content_index value so it matches the final message and the Responses API.

The Chat Completions stream handler computed each assistant content part's
content_index by counting the reasoning item, but reasoning is emitted as a
separate output item (its own output_index), not a content part of the
assistant message. So when a response streams reasoning followed by text (or
a refusal) — e.g. reasoning models routed through the Chat Completions adapter
such as deepseek-reasoner — the text part was streamed with content_index=1
while it lands at message.content[0] in the final message and in what the
OpenAI Responses API emits for the same output. A consumer reconstructing the
message via message.content[event.content_index] writes to the wrong slot.

Drop the reasoning increment in both the text and refusal branches. The
text<->refusal cross-increment (both are real content parts of the message)
is unchanged, and the final message assembly appends parts in order without
using these indices, so only the streamed events change.

Regression test fails on main (content_index=1) and passes with the fix; the
stream + reasoning suites stay green (62 passed).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e4d6e02fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +759 to 761
# The reasoning item is a separate output item, so it must not shift
# this content index (see the text branch above).
refusal_index = 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep refusal indexes aligned when text arrives later

When the stream order is reasoning -> refusal -> text, removing the reasoning offset here makes the refusal part emit at content_index=0; the later text branch then emits text at content_index=1 because a refusal already exists. However response.completed is still assembled with text before refusal (assistant_msg.content.append(text) then append(refusal)), so raw stream consumers reconstruct the parts swapped relative to the final message. This affects exactly the refusal-before-text shape the handler already supports, now with a preceding reasoning item.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks, I checked this and the text/refusal swap is pre-existing, not introduced here. This PR only removes the reasoning-item offset; the text <-> refusal cross-increment in both branches is untouched. In the reasoning -> refusal -> text order: on main refusal streams at content_index=1 and text at 2; on this branch refusal at 0 and text at 1. In both cases the streamed order is refusal-then-text while the final message assembles text-then-refusal (lines 1044-1047 append text before refusal unconditionally), so the parts are swapped relative to the stream either way. This change only shifts the absolute indices down by one; it doesn't create or widen that gap.

It's worth fixing, but it's a separate concern (the final-assembly order vs arrival order), so I've kept this PR to the reasoning offset. Happy to open a follow-up to align the both-text-and-refusal case if you'd like.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants