-
Notifications
You must be signed in to change notification settings - Fork 6.4k
fix: preserve assistant/tool call ordering in recorded turns #7038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
commit cfcc87a changed how response items are batched and recorded, which caused tool call objects to be recorded before the assistant message content in some turns. This could result in a { role: "assistant", content: null, tool_calls: [...] } message being sent before the actual assistant message with text content, breaking ordering expectations downstream. This change reorders the items collected for a single turn so that the last non-empty assistant message is placed immediately before the first tool call object when both occur in the same turn, without perturbing prior history. A new unit test ensures that recorded items now appear in the expected order: assistant text, then tool call object, then the corresponding tool output input item.
|
Thanks for the contribution. Is there an open bug report associated with this PR? We ask that all bug fix PRs have a linked bug report (see contribution guidelines). It's not clear to me what problem this PR is solving. You mentioned that the recent change is "breaking ordering expectations downstream". Whose expectations, and how does that manifest? |
|
It only affects chat/completions endpoint with OpenAI compatible API servers. With the current codex: Test with the following. Run a test server (needs {
cat <<'EOF'
HTTP/1.1 200 OK
Content-Type: text/event-stream
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"content":"ASSISTANT MESSAGE HERE"},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"content":null,"tool_calls":[{"index":0,"id":"call_testid","type":"function","function":{"name":"shell","arguments":""}}]},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\"command\":[\"bash\",\"-lc\",\"uname\"],\"timeout_ms\":120000,\"workdir\":\"/tmp\"}"}}]},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]}
data: [DONE]
EOF
} | nc -l 8888 | sed '/^[^{]/d' | jq '.messages[2:]'
{
cat <<'EOF'
HTTP/1.1 200 OK
Content-Type: text/event-stream
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{"content":"CHECK WHAT I RECEIVED"},"finish_reason":null}]}
data: {"id":"chatcmpl-testid","object":"chat.completion.chunk","created":1763690000,"model":"gpt-test","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}
data: [DONE]
EOF
} | nc -l 8888 | sed '/^[^{]/d' | jq '.messages[2:]'From another terminal connect with codex The our You can see that the This commit moves assistant messages, if there's any, before the |
|
And I just saw a bug report. This PR should fix #7051 |
|
codex --version I'm using [email protected] version via |
|
I believe this issue is model-dependent. When the messages are arranged in the sequence: Some models return an error while others do not. For example, when I used glm-4.6 and sent requests in that order, no error occurred. When I submitted the same request to deepseek-chat, it returned an HTTP 400 response with the following error: But it indeed affect the GPT-series models. Below is the request log I captured from Codex (version: 0.63.0): {
"model": "glm-4.6",
"messages": [
{
"role": "system",
"content": "You are a coding agent running in the Codex CLI, a terminal-based coding assistant. Codex CLI is....."
},
{
"role": "user",
"content": "<environment_context>...</environment_context>"
},
{
"role": "user",
"content": "write foo into bar file and save in the current location"
},
{
"role": "assistant",
"content": null,
"tool_calls": [
{
"id": "call_c64a59b760fa422e8e603a2f",
"type": "function",
"function": {
"name": "exec_command",
"arguments": "{\"cmd\":\"echo \\\"foo\\\" > bar\"}"
}
}
]
},
{
"role": "assistant",
"content": "\nI'll create a file named \"bar\" with the content \"foo\" in the current location.\n"
},
{
"role": "tool",
"tool_call_id": "call_c64a59b760fa422e8e603a2f",
"content": "Chunk ID: 987afe\nWall time: 0.0261 seconds\nProcess exited with code 0\nOriginal token count: 0\nOutput:\n"
}
],
"stream": true,
"tools": [...]
}And the Codex can finish the above task with glm only. Below is my model config: [model_providers.glm_4_6]
name = "glm_4_6"
base_url = "https://open.bigmodel.cn/api/coding/paas/v4"
env_key = "GLM_API_KEY"
wire_api = "chat"
query_params = {}
[model_providers.deepseek-chat]
name = "DeepSeek"
base_url = "https://api.deepseek.com"
env_key = "DEEPSEEK_API_KEY"
wire_api = "chat"
query_params = {} |
commit cfcc87a changed how response items are batched and recorded, which caused tool call objects to be recorded before the assistant message content in some turns.
This could result in a { role: "assistant", content: null, tool_calls: [...] } message being sent before the actual assistant message with text content, breaking ordering expectations downstream.
This change reorders the items collected for a single turn so that the last non-empty assistant message is placed immediately before the first tool call object when both occur in the same turn, without perturbing prior history.
A new unit test ensures that recorded items now appear in the expected order: assistant text, then tool call object, then the corresponding tool output input item.