Repository navigation
fix(models): pair Codex invalid tool calls with their tool results - #5509
WillemJiang merged 2 commits into
Conversation
`_parse_response` parks a function_call whose `arguments` are not valid JSON on `AIMessage.invalid_tool_calls`, keeping its id and name. `_convert_messages` serialized only `msg.tool_calls`, so the placeholder ToolMessage that `DanglingToolCallMiddleware` injects to answer that call was emitted as a `function_call_output` whose call_id had no matching `function_call` item in the same request. Responses requires that pairing, turning a recoverable malformed call into a hard provider error. Emit `invalid_tool_calls` alongside `tool_calls` as `function_call` items.
github-actions
Bot
added
area:backend
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed at head a108750. The core fix is correct and verified end-to-end: DanglingToolCallMiddleware is always in the agent chain and gives every invalid_tool_call a (possibly synthetic) call_id, a normalized name, and "{}" arguments plus a paired placeholder ToolMessage, so emitting function_call items for invalid_tool_calls exactly repairs the orphan function_call_output that the Responses endpoint rejects. The new test fails against main and passes at head (86/86 across test_codex_provider.py + test_dangling_tool_call_middleware.py), and ruff format/check are clean. One suggestion below about nullable InvalidToolCall fields on paths that reach _convert_messages without the middleware.
| "type": "function_call", | ||
| "name": tc["name"], | ||
| "arguments": json.dumps(tc["args"]) if isinstance(tc["args"], dict) else tc["args"], | ||
| "call_id": tc["id"], |
There was a problem hiding this comment.
InvalidToolCall fields are nullable — langchain's parser and this provider's own _parse_tool_call_arguments (via .get()) can produce name: None, id: None, or args: None. This loop then emits {"type": "function_call", "name": null, ..., "call_id": null}, which the Responses endpoint rejects as schema-invalid — turning a case the old serializer silently dropped into a full-request 400. In the agent path DanglingToolCallMiddleware relabels/normalizes these before the request, but _convert_messages is also reachable without the middleware (direct model.invoke callers). Consider skipping invalid calls whose name/id are not non-empty strings (defaulting args to "{}"); that cannot orphan a placeholder, since the middleware mints synthetic ids for exactly those calls before serialization.
There was a problem hiding this comment.
Confirmed and reproduced — that path does emit the nulls. 22a8b250 drops a call that is missing either field.
Driving _convert_messages without the middleware, for a malformed function_call item on each of those shapes:
no call_id -> {"type":"function_call","name":"write_file","arguments":"{\"a\":","call_id":null}
no name -> {"type":"function_call","name":null,"arguments":"{\"a\":","call_id":"call_x"}
neither -> {"type":"function_call","name":null,"arguments":"{\"a\":","call_id":null}
_parse_tool_call_arguments builds these with output_item.get("name") and .get("call_id"), so both really are None rather than absent. The same three inputs through DanglingToolCallMiddleware come out as unknown_tool / deerflow_synthetic_tool_call_1_invalid_0 with arguments: {}, and pair with their placeholder — which is what makes the skip safe: a call the middleware has not repaired is one it did not mint an id for, so no placeholder exists to orphan.
Arguments also fall back to "{}" when they are neither an object nor a string, since args is nullable on the same type. Two tests cover both directions: the three shapes are dropped without the middleware, and with it they are still serialized and still paired with their function_call_output. 88 passed across test_codex_provider.py and test_dangling_tool_call_middleware.py.
InvalidToolCall fields are nullable, and serializing every invalid call as a
function_call item sends name: null and call_id: null for one that is missing
them, which the Responses schema does not accept. For a caller that reaches
_convert_messages without DanglingToolCallMiddleware, that turned a call the
old serializer dropped into a rejected request.
A call missing either field is now skipped, and its arguments fall back to
"{}" when they are neither an object nor a string. Skipping cannot orphan the
placeholder ToolMessage that this branch pairs the call with: the middleware
mints a synthetic id and a fallback name for exactly these calls before
serialization, so a call still missing them here has no placeholder.
github-actions
Bot
added
size/M
- NOTES 补 bytedance/deer-flow#5509、agno-agi/agno#9948 两条中文说明（此前 cron 已把标题行写进贡献区但没有说明） - 缺陷表：deer-flow #5473 → 社区 PR #5494 已修复；openai-agents-python #5041 → 维护者未接受并关闭（需真实 provider wire 证据） - deer-flow star 数 82k → 83k
Why
A Codex model that emits a
function_callwhoseargumentsare not valid JSON does not fail the turn:CodexChatModel._parse_responseparks the call onAIMessage.invalid_tool_calls(keeping itscall_idand name), andDanglingToolCallMiddleware— always in the agent middleware chain atbackend/packages/harness/deerflow/agents/factory.py:251— answers it with a placeholderToolMessagecarrying the samecall_id, so the model sees a recoverable tool error.The serializer in
CodexChatModel._convert_messages(backend/packages/harness/deerflow/models/openai_codex_provider.py:157) then dropped that call. It emitted input items formsg.tool_callsonly, so the placeholder reachedPOST /backend-api/codex/responsesas afunction_call_outputwhosecall_idhad no matchingfunction_callitem in the same request. Responses requires the pair, so a malformed call — the exact case the middleware exists to recover from — ended in a provider error instead of a retry.Related context: #2890 reported this failure class on the OpenAI-compatible path, where the repair landed in the middleware. The Codex Responses path serializes its own request and was not covered by that fix.
What changed
invalid_tool_callsasfunction_callinput items next to the valid ones, so the placeholderfunction_call_outputthat answers them stays paired.invalid_tool_callspopulation), and the middleware are unchanged. Chat Completions providers already serializeinvalid_tool_callsthis way through LangChain's converter; this brings the Responses serializer in line with them.Surface area
frontend/backend/applanggraph.json, or prompt changedocker/or sandboxed executionskills/backend/pyproject.tomlorfrontend/package.json(say what it buys us)No box matches: the change is the Codex Responses request serializer in
backend/packages/harness/deerflow/models/openai_codex_provider.py, a harness model provider rather thanbackend/app, and no default is changed. The bug-fix verification below is the scope signal for review.Bug fix verification
backend/tests/test_codex_provider.py::test_convert_messages_keeps_placeholder_result_paired_with_invalid_tool_callmainand green on this branch? yesThe test drives the real chain rather than a hand-built message:
_parse_responseon a Codexfunction_callframe with truncated JSON, thenDanglingToolCallMiddleware._build_patched_messages, then_convert_messages. Onmainthe placeholder is dropped at the serialization boundary:Validation
AI assistance
Tool(s) used: Claude Code
How you used it: Claude Code traced the failure from the middleware placeholder back to the Responses serializer, wrote the regression test and the serializer change, and ran the red/green cycle plus the full backend suite. I reviewed every line of the diff and the test's assertions before committing.