fix(responses): preserve combined assistant tool turns - #4775
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c39bf30407
ℹ️ 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".
| if lastMessage.Get("role").String() == "assistant" && !lastMessage.Get("tool_calls").Exists() { | ||
| assistantMessage, _ := sjson.SetBytes(messages[lastIndex], "tool_calls", pendingToolCalls) |
There was a problem hiding this comment.
When the Chat Completions upstream emits mixed choices—such as choice 0 returning text while choice 1 returns a tool call—the response converter intentionally serializes them as distinct output items and output indexes (openai_openai-responses_response_test.go:338-380). If that output is replayed as Responses input, this role-only check merges choice 1's tool call into choice 0's assistant text, corrupting the alternatives into a single assistant turn. The merge needs provenance indicating that the message and call came from the same choice/turn rather than merely being adjacent.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bff4993f9
ℹ️ 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".
| lastMessage := gjson.ParseBytes(messages[lastIndex]) | ||
| if lastMessage.Get("role").String() == "assistant" && | ||
| !lastMessage.Get("tool_calls").Exists() && | ||
| messageProvenances[lastIndex] == pendingToolCallProvenance { |
There was a problem hiding this comment.
When the response items do not use the new synthetic ID shape (for example native msg_1/fc_1 histories, or streaming chat chunks whose response id is empty so emitted IDs become unparsable like msg__0/fc__1_0), openAIResponsesOutputItemProvenance returns "" for both items. The fresh evidence in this revision is that this equality check treats two unknown provenances as the same turn, so a mixed-n replay can still merge choice 1's tool call into choice 0's assistant text instead of keeping the alternatives separate; unknown provenance needs to be distinguished from matching provenance, or synthesized before these IDs are emitted.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 023f59adb4
ℹ️ 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".
| } | ||
| item := []byte(`{"id":"","type":"function_call","status":"completed","arguments":"","call_id":"","name":""}`) | ||
| item, _ = sjson.SetBytes(item, "id", fmt.Sprintf("fc_%s", callID)) | ||
| item, _ = sjson.SetBytes(item, "id", openAIResponsesNonStreamToolItemID(id, choice.Get("index").Int(), tcIndex.Int(), false)) |
There was a problem hiding this comment.
When a non-streaming chat completion returns multiple tool_calls in the same choice, this builds the Responses item ID from tcIndex.Int(). In gjson ForEach array iteration does not provide the array index as the key, so tcIndex.Int() is 0 for every element; two distinct upstream calls therefore both get an ID like fc_<response>_<choice>_0 (the custom branch above has the same issue). That leaves duplicate response.output[].id values for parallel tool calls even when the upstream call_ids are unique, which breaks clients/history code that keys items by item_id; use an explicit counter or iterate over tcs.Array() with an index.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
What changed
Why
The Responses request translator currently reconstructs this normalized output sequence:
as two adjacent Chat Completions assistant messages:
That changes the original assistant turn semantics and can cause an upstream model to treat the first message as a completed reply instead of continuing the tool-use turn. The converter now reconstructs the sequence atomically as:
Validation
go test ./internal/translator/openai/openai/responsesgo test ./internal/runtime/executor -run '^TestKimiNativeAssistantMessageSurvivesResponsesRoundTrip$'go build -o <temporary-path>/cli-proxy-api ./cmd/servergit diff --check origin/dev...HEADFixes #4676