merge reasoning and content messages for openai chat completions#331
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd829a8bc
ℹ️ 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 (@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 (@codex) address that feedback".
|
|
||
| let choices: Vec<Value> = resp | ||
| .messages | ||
| let response_messages = merge_reasoning_response_messages( |
There was a problem hiding this comment.
Preserve choice boundaries when merging reasoning
When this Chat Completions adapter is the target for a response whose universal messages are alternatives rather than ordered parts (for example, Google candidates, where GoogleAdapter::response_to_universal pushes one Message per candidate), this unconditional merge crosses choice boundaries. A first candidate containing only thought/reasoning followed by a second candidate with visible text is serialized as one Chat Completions choice, changing n/candidate semantics and moving another alternative's answer into choices[0]. Gate this merge to Responses-origin output items or preserve source grouping before merging.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
gonna move the merge to responses side
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c3a251a44
ℹ️ 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 (@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 (@codex) address that feedback".
| merged.push(Message::Assistant { | ||
| content: AssistantContent::Array(combined_parts), | ||
| id: next_id.or(reasoning_id), |
There was a problem hiding this comment.
Preserve both Responses item IDs when merging
When a Responses response has separate reasoning and message output items and a caller parses it through response_to_universal before emitting Responses again, this merged message can carry only next_id; the Responses serializer then assigns that single ID to the first regenerated output item (the reasoning item) and gives the visible message a placeholder. That drops or reassigns the original rs_*/msg_* item IDs, so follow-up item_reference requests can point at the wrong output item.
Useful? React with 👍 / 👎.
Main's reasoning/content merge for OpenAI chat completions (#331) changed the chat-completions → responses transform; refresh the case snapshot to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fix non-streaming Chat Completions responses transformed from OpenAI Responses when the upstream output contains
reasoning before visible text.
Previously, Responses output[] items were converted into separate Chat Completions choices, so a reasoning item could
become choices[0] while the visible text landed in choices[1]. For Chat Completions, choices are completion
alternatives, not ordered response parts. With the default n = 1, OpenAI-compatible callers expect the generated
assistant message at choices[0].message.content, so this produced silently empty outputs.
This change merges a leading reasoning-only assistant message into the following assistant message when serializing a
universal response back to Chat Completions. The transformed response now returns one choice containing both
message.reasoning and message.content.
Also added a regression test for Responses reasoning followed by message output, and updated payload transform
snapshots.