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

[AmirF194]

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.

[chatgpt-codex-connector]

💡 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".

[chatgpt-codex-connector]

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 👍 / 👎.

[AmirF194]

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.

[felmonon]

Reviewed the current head against current main and the Responses streaming event contract. The reasoning payload is emitted as a separate ResponseReasoningItem / output slot, so it should affect output_index, not the assistant message’s content_index. Removing only the reasoning increment is the right boundary, and the regression test verifies the text and refusal indexes against the completed message.

I also checked the refusal-before-text Codex note: that ordering mismatch exists independently on main; this focused change does not introduce it. The branch merges cleanly with current main, and all required checks are green. I don’t see a blocker from my side.

[AmirF194]

Thanks for the careful pass. Agreed: the reasoning payload rides output_index, so scoping the removal to just the reasoning increment leaves the assistant message's content_index intact. And yes, the refusal-before-text ordering is a separate pre-existing issue on main, so I kept this change narrow to keep the two untangled.

[AmirF194]

No rush on this, just following up. felmonon reviewed it a few days ago and didn't flag a blocker; happy to adjust anything else that would help it move.

[github-actions]

This PR is stale because it has been open for 10 days with no activity.

[PranavMishra28]

This looks right to me, and main already contains the argument for it: OutputLayout.assistant_message_output_index (chatcmpl_stream_handler.py:140-149) is what accounts for the reasoning item via _reasoning_output_count, so bumping content_index as well was double counting. The newer Bedrock content-filter refusal site added since this branch already sets refusal_index = 0 unconditionally, with a comment stating exactly the rule this PR applies.

Two notes.

The branch predates that third refusal site and needs a rebase. Main has grown a refusal path this PR does not touch; it is already correct, so nothing to change there, but the diff will not apply cleanly as is.

The same desync still exists for refusal-before-text, which this PR does not cover. Final assembly always appends text then refusal (chatcmpl_stream_handler.py:1043-1047), but the streaming indices are first-arrival-wins, so when a refusal delta arrives first it takes content_index 0 while ending up at content[1]. I ran the handler with refusal first on this branch:

streamed content_index : {'ResponseOutputRefusal': 0, 'ResponseOutputText': 1}
final message position : {'ResponseOutputText': 0, 'ResponseOutputRefusal': 1}

test_stream_handler_places_text_after_existing_refusal_part (tests/models/test_openai_chatcompletions_stream.py:770-804) currently asserts both halves of that: text_part_added.content_index == 1 alongside isinstance(assistant_item.content[0], ResponseOutputText). A consumer applying response.output_text.delta events by content_index into the accumulated message writes text into the refusal slot.

Ordering the final assembly by the recorded content_index rather than text-then-refusal would resolve it, or documenting that content_index is arrival order and not a position in message.content. Happy to see that left out of scope here if you would rather keep this PR to the reasoning fix; it is a separate bug and this one is a strict improvement either way.

[AmirF194]

Thanks for the detailed check, especially for running the refusal-first case and pulling the exact test that documents the ordering assumption. Agreed on scope: that desync is real but predates this branch and is a separate bug, so I'll leave it out of this PR.

On the rebase: GitHub still reports this as mergeable against current main (no conflicts on the files this PR touches), so I'll hold off rebasing unless a maintainer asks for it. If the new refusal site changes once this actually gets reviewed, happy to redo it then.

[seratch]

Since this PR was opened, the same fix has landed on main in #4177. That change removes the reasoning offset from both the text and refusal paths and adds coverage showing that assistant content remains at content_index=0 while reasoning occupies a separate output slot. Current main is green, so there is no remaining code to carry over from this branch.

I'm going to close this PR as superseded by #4177. Thank you again for surfacing the issue and providing the focused analysis and regression case.

[AmirF194]

Good to see #4177 covers both the text and refusal paths, glad it landed cleanly on main. Thanks for the update and for closing the loop here.