mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(models): omit Codex tool results with blank call IDs (#6234)
* fix(models): omit Codex tool results with blank call IDs * fix(models): validate Codex call and result IDs symmetrically Use one strip-based validity check for assistant calls and tool results while preserving non-blank IDs verbatim. Add paired regressions for ordinary and invalid tool calls, plus non-blank correlation controls. Validation: 126 focused tests passed; 210 blocking-I/O tests passed; backend lint and format checks passed. Paired regression fails on the previous implementation for whitespace-only IDs. * fix(models): log dropped Codex tool results --------- Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
e59f6e5e92
commit
73a305cd87
@ -58,6 +58,17 @@ the same policy first so run metadata reports the effective values. Design note:
|
||||
`tests/test_reasoning_contract.py`, the contract section of
|
||||
`tests/test_model_factory.py`, `tests/test_models_router_reasoning.py`.
|
||||
|
||||
### Codex tool-call/result serialization (`packages/harness/deerflow/models/openai_codex_provider.py`)
|
||||
|
||||
`_convert_messages` uses `_is_valid_call_id` for both assistant tool calls
|
||||
(including `invalid_tool_calls`) and tool results, omitting empty or
|
||||
whitespace-only call IDs even when direct provider use bypasses middleware repair.
|
||||
Non-blank IDs remain byte-for-byte unchanged so calls and results keep their
|
||||
correlation; serialization does not trim IDs or rewrite the input messages.
|
||||
Each omitted tool result emits a warning with its normalized content length;
|
||||
the warning never includes the result content.
|
||||
Coverage: `tests/test_codex_provider.py`.
|
||||
|
||||
### Codex SSE termination (`packages/harness/deerflow/models/openai_codex_provider.py`)
|
||||
|
||||
`response.completed` ends stream consumption immediately, before transport EOF;
|
||||
|
||||
@ -29,6 +29,11 @@ logger = logging.getLogger(__name__)
|
||||
CODEX_BASE_URL = "https://chatgpt.com/backend-api/codex"
|
||||
|
||||
|
||||
def _is_valid_call_id(call_id: Any) -> bool:
|
||||
"""Check call/result correlation IDs without rewriting non-blank values."""
|
||||
return isinstance(call_id, str) and bool(call_id.strip())
|
||||
|
||||
|
||||
def _build_usage_metadata(oai_usage: dict) -> dict:
|
||||
"""Convert Codex/Responses API usage dict to LangChain usage_metadata format.
|
||||
|
||||
@ -170,7 +175,7 @@ class CodexChatModel(BaseChatModel):
|
||||
call_id = tc.get("id")
|
||||
if not (isinstance(name, str) and name):
|
||||
continue
|
||||
if not (isinstance(call_id, str) and call_id):
|
||||
if not _is_valid_call_id(call_id):
|
||||
continue
|
||||
args = tc.get("args")
|
||||
input_items.append(
|
||||
@ -182,11 +187,16 @@ class CodexChatModel(BaseChatModel):
|
||||
}
|
||||
)
|
||||
elif isinstance(msg, ToolMessage):
|
||||
content = self._normalize_content(msg.content)
|
||||
# A blank ID cannot identify the call this result answers.
|
||||
if not _is_valid_call_id(msg.tool_call_id):
|
||||
logger.warning("Dropping tool result with blank call_id (content %d chars)", len(content))
|
||||
continue
|
||||
input_items.append(
|
||||
{
|
||||
"type": "function_call_output",
|
||||
"call_id": msg.tool_call_id,
|
||||
"output": self._normalize_content(msg.content),
|
||||
"output": content,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@ -11,8 +11,10 @@ Covers:
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from langchain_core.messages import AIMessage, HumanMessage, SystemMessage, ToolMessage
|
||||
|
||||
from deerflow.models.credential_loader import CodexCliCredential
|
||||
@ -202,13 +204,85 @@ def test_convert_messages_ai_with_tool_calls():
|
||||
assert any(item.get("type") == "function_call" and item["name"] == "search" for item in items)
|
||||
|
||||
|
||||
def test_convert_messages_tool_message():
|
||||
@pytest.mark.parametrize("call_id", ["tc1", "0", " tc1 "])
|
||||
def test_convert_messages_tool_message(call_id, caplog):
|
||||
model = _make_model()
|
||||
tool_msg = ToolMessage(content="result data", tool_call_id="tc1")
|
||||
_, items = model._convert_messages([tool_msg])
|
||||
tool_msg = ToolMessage(content="result data", tool_call_id=call_id)
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.models.openai_codex_provider"):
|
||||
_, items = model._convert_messages([tool_msg])
|
||||
assert items[0]["type"] == "function_call_output"
|
||||
assert items[0]["call_id"] == "tc1"
|
||||
assert items[0]["call_id"] == call_id
|
||||
assert items[0]["output"] == "result data"
|
||||
assert caplog.record_tuples == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("call_id", ["tc1", "0", " tc1 "])
|
||||
def test_convert_messages_preserves_paired_call_ids(call_id):
|
||||
model = _make_model()
|
||||
ai_msg = AIMessage(content="", tool_calls=[{"name": "search", "args": {"q": "foo"}, "id": call_id}])
|
||||
tool_msg = ToolMessage(content="result data", tool_call_id=call_id)
|
||||
|
||||
_, items = model._convert_messages([ai_msg, tool_msg])
|
||||
|
||||
assert items == [
|
||||
{"type": "function_call", "name": "search", "arguments": '{"q": "foo"}', "call_id": call_id},
|
||||
{"type": "function_call_output", "call_id": call_id, "output": "result data"},
|
||||
]
|
||||
assert ai_msg.tool_calls[0]["id"] == call_id
|
||||
assert tool_msg.tool_call_id == call_id
|
||||
|
||||
|
||||
@pytest.mark.parametrize("blank_id", ["", " ", "\t\r\n"])
|
||||
@pytest.mark.parametrize("call_field", ["tool_calls", "invalid_tool_calls"])
|
||||
def test_convert_messages_omits_paired_calls_and_results_with_blank_ids(blank_id, call_field):
|
||||
model = _make_model()
|
||||
args = {"q": "foo"} if call_field == "tool_calls" else '{"q":'
|
||||
ai_msg = AIMessage(content="Searching.", **{call_field: [{"name": "search", "args": args, "id": blank_id}]})
|
||||
tool_msg = ToolMessage(content="result data", tool_call_id=blank_id)
|
||||
|
||||
_, items = model._convert_messages([ai_msg, tool_msg])
|
||||
|
||||
assert items == [{"role": "assistant", "content": "Searching."}]
|
||||
assert getattr(ai_msg, call_field)[0]["id"] == blank_id
|
||||
assert tool_msg.tool_call_id == blank_id
|
||||
|
||||
|
||||
@pytest.mark.parametrize("blank_id", ["", " ", "\t\r\n"])
|
||||
@pytest.mark.parametrize(
|
||||
("content", "content_length"),
|
||||
[
|
||||
("private result", 14),
|
||||
([{"type": "text", "text": "private"}, {"type": "text", "text": "result"}], 14),
|
||||
("", 0),
|
||||
],
|
||||
)
|
||||
def test_convert_messages_omits_tool_results_with_blank_call_ids(blank_id, content, content_length, caplog):
|
||||
model = _make_model()
|
||||
orphaned_result = ToolMessage(content=content, tool_call_id=blank_id)
|
||||
original_result = orphaned_result.model_dump()
|
||||
messages = [
|
||||
SystemMessage(content="Follow the instructions."),
|
||||
HumanMessage(content="Search for foo."),
|
||||
AIMessage(content="", tool_calls=[{"name": "search", "args": {"q": "foo"}, "id": "tc1"}]),
|
||||
orphaned_result,
|
||||
ToolMessage(content=[{"type": "text", "text": "result data"}], tool_call_id="tc1"),
|
||||
AIMessage(content="Done."),
|
||||
]
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="deerflow.models.openai_codex_provider"):
|
||||
instructions, items = model._convert_messages(messages)
|
||||
|
||||
assert instructions == "Follow the instructions."
|
||||
assert items == [
|
||||
{"role": "user", "content": "Search for foo."},
|
||||
{"type": "function_call", "name": "search", "arguments": '{"q": "foo"}', "call_id": "tc1"},
|
||||
{"type": "function_call_output", "call_id": "tc1", "output": "result data"},
|
||||
{"role": "assistant", "content": "Done."},
|
||||
]
|
||||
assert orphaned_result.model_dump() == original_result
|
||||
assert caplog.record_tuples == [
|
||||
("deerflow.models.openai_codex_provider", logging.WARNING, f"Dropping tool result with blank call_id (content {content_length} chars)"),
|
||||
]
|
||||
|
||||
|
||||
def test_convert_messages_keeps_placeholder_result_paired_with_invalid_tool_call():
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user