From 73a305cd87bbacfd3018ee4520a88b85e22e81bf Mon Sep 17 00:00:00 2001 From: destire-mio Date: Sun, 4 Oct 2026 12:44:05 +0800 Subject: [PATCH] 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 --- .../harness/deerflow/models/AGENTS.md | 11 +++ .../deerflow/models/openai_codex_provider.py | 14 +++- backend/tests/test_codex_provider.py | 82 ++++++++++++++++++- 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/backend/packages/harness/deerflow/models/AGENTS.md b/backend/packages/harness/deerflow/models/AGENTS.md index 1aa0c30f95..5b96772db6 100644 --- a/backend/packages/harness/deerflow/models/AGENTS.md +++ b/backend/packages/harness/deerflow/models/AGENTS.md @@ -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; diff --git a/backend/packages/harness/deerflow/models/openai_codex_provider.py b/backend/packages/harness/deerflow/models/openai_codex_provider.py index 27f5591ad7..99b3a13e62 100644 --- a/backend/packages/harness/deerflow/models/openai_codex_provider.py +++ b/backend/packages/harness/deerflow/models/openai_codex_provider.py @@ -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, } ) diff --git a/backend/tests/test_codex_provider.py b/backend/tests/test_codex_provider.py index 2744b407d8..4e9b64d7cd 100644 --- a/backend/tests/test_codex_provider.py +++ b/backend/tests/test_codex_provider.py @@ -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():