From 8331afa759aa73f0f73298cf0c19bc65ff47a13e Mon Sep 17 00:00:00 2001 From: huyengiang101086-spec Date: Thu, 5 Nov 2026 14:07:15 +0800 Subject: [PATCH] fix(agents): pair null call IDs with empty result IDs (#7837) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): pair null call IDs with empty result IDs * fix(agents): treat producer sentinel IDs as absent when pairing Middleware producers spell a missing provider ID three ways: "" is canonical, str(tool_call.get("id", "")) yields "None" once the key is present with a null value, and the artifact/deferred/knowledge middlewares fall back to "missing_tool_call_id". Only "" reached the pairing pass, so a result carrying either sentinel was dropped as an orphan and the middleware's guidance was replaced by the interrupted placeholder — the exact outcome this PR set out to avoid, just through a different spelling. Route every absent spelling through one canonicalisation used by the pairing gate, the candidate comparison and the turn-completeness count. --------- Co-authored-by: huyengiang101086-spec <270684046+huyengiang101086-spec@users.noreply.github.com> Co-authored-by: Willem Jiang --- README.md | 4 + .../dangling_tool_call_middleware.py | 33 ++++++- backend/tests/AGENTS.md | 3 + .../tests/test_null_tool_result_pairing.py | 94 +++++++++++++++++++ 4 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 backend/tests/test_null_tool_result_pairing.py diff --git a/README.md b/README.md index b3cfc5aa54..4ea9fa4e5d 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,10 @@ For Google's official Gemini OpenAI-compatible endpoint, use the For MindIE XML tool calls, see the [argument parsing and newline compatibility guide](backend/docs/CONFIGURATION.md#mindie-xml-tool-arguments). +Tool-result recovery treats null and empty call IDs alike — including the +`"None"` and `"missing_tool_call_id"` spellings middleware producers use when a +provider call had no ID — preserving middleware guidance when a result can be +uniquely paired; ambiguous partial results remain rejected. Both synchronous and asynchronous streams retain this compatibility: tool-enabled streams simulate chunks from a non-streaming response, while no-tool streams stay native. Newline compatibility preserves literal escapes inside backtick and tilde code diff --git a/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py index 4f50ebebcc..94dcfefcad 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py @@ -39,6 +39,28 @@ _UNKNOWN_TOOL_NAME = "unknown_tool" _EMPTY_TOOL_NAME_ERROR = "Tool call could not be executed because its name was missing or empty." _SYNTHETIC_TOOL_CALL_ID_PREFIX = "deerflow_synthetic_tool_call_" +# Middleware producers spell an absent provider ID differently: "" is the canonical +# spelling, but ``str(tool_call.get("id", ""))`` yields "None" once the key is present +# with a null value, and the artifact/deferred/knowledge middlewares fall back to +# "missing_tool_call_id". They all answer the same call, so pairing treats them alike. +_ABSENT_TOOL_CALL_ID_SPELLINGS = frozenset({"None", "missing_tool_call_id"}) + + +def _canonical_tool_call_id(tool_call_id: object) -> object: + """Return one value for every spelling middleware uses for "this call had no ID".""" + return "" if _absent_tool_call_id(tool_call_id) else tool_call_id + + +def _absent_tool_call_id(tool_call_id: object) -> bool: + """Whether an ID is missing in any spelling: null, blank, or a producer sentinel.""" + if not _valid_tool_call_id(tool_call_id): + return True + return isinstance(tool_call_id, str) and tool_call_id in _ABSENT_TOOL_CALL_ID_SPELLINGS + + +def _same_tool_call_id(left: object, right: object) -> bool: + return _canonical_tool_call_id(left) == _canonical_tool_call_id(right) + def _valid_tool_name(name: object) -> bool: return isinstance(name, str) and bool(name.strip()) @@ -93,12 +115,12 @@ def _relabel_tool_call_ids(tool_calls: list, msg_index: int, source: str) -> tup def _turn_malformed_result_count(messages: list, start: int) -> int: - """Count the malformed results issued by the turn opened at ``start``.""" + """Count the unpaired results issued by the turn opened at ``start``.""" count = 0 for msg in messages[start + 1 :]: if getattr(msg, "type", None) == "ai": break - if isinstance(msg, ToolMessage) and not _valid_tool_call_id(msg.tool_call_id): + if isinstance(msg, ToolMessage) and _absent_tool_call_id(msg.tool_call_id): count += 1 return count @@ -123,7 +145,7 @@ def _claim_synthetic_id(open_calls: list[dict], result: ToolMessage, positional: Returning ``None`` leaves the result malformed for the orphan pass to drop, which is what an unattributable result gets today — better than inventing a pairing. """ - candidates = [position for position, entry in enumerate(open_calls) if entry["original"] == result.tool_call_id and _names_can_pair(entry["name"], result.name)] + candidates = [position for position, entry in enumerate(open_calls) if _same_tool_call_id(entry["original"], result.tool_call_id) and _names_can_pair(entry["name"], result.name)] if not candidates or (len(candidates) > 1 and not positional): return None return open_calls.pop(candidates[0])["synthetic"] @@ -415,8 +437,9 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): continue # Re-point an already-paired result at its call's new id so the pairing - # below keeps it instead of dropping it as an orphan. - if not isinstance(msg, ToolMessage) or _valid_tool_call_id(msg.tool_call_id): + # below keeps it instead of dropping it as an orphan. Results carrying a + # producer sentinel are still unpaired — the sentinel is not the call's id. + if not isinstance(msg, ToolMessage) or not _absent_tool_call_id(msg.tool_call_id): continue synthetic = _claim_synthetic_id(open_calls, msg, positional) if synthetic is not None: diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 5b6eeb8d91..df71580db6 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -1,5 +1,8 @@ # Backend Tests +`test_null_tool_result_pairing.py` pins null/empty/sentinel ID equivalence, provider-wire +pairing, and ambiguity rejection for incomplete parallel results. + `test_lark_broker_image.py` stages Python files from the broker Dockerfile's final-stage COPY instructions. Run shim installation and the health endpoint under `-I -S`, adding only the staged script directory to the import path; diff --git a/backend/tests/test_null_tool_result_pairing.py b/backend/tests/test_null_tool_result_pairing.py new file mode 100644 index 0000000000..fb7b2a8b3b --- /dev/null +++ b/backend/tests/test_null_tool_result_pairing.py @@ -0,0 +1,94 @@ +"""Missing call IDs must not swallow middleware-generated tool guidance.""" + +import pytest +from langchain_core.messages import AIMessage, ToolMessage +from langchain_openai.chat_models.base import _convert_message_to_dict + +from deerflow.agents.middlewares.dangling_tool_call_middleware import DanglingToolCallMiddleware + + +@pytest.mark.parametrize("call_id,result_id", [(None, ""), ("", None), (None, None), ("", "")]) +def test_missing_ids_preserve_result_and_provider_pairing(call_id, result_id): + call = AIMessage(content="", tool_calls=[{"name": "write_file", "id": call_id, "args": {"path": "a.md"}}]) + # Normal middleware replies use ""; model_construct also covers old None-ID history. + constructor = ToolMessage.model_construct if result_id is None else ToolMessage + result = constructor(content="Error: read the file before writing it", tool_call_id=result_id, name="write_file", status="error") + patched = DanglingToolCallMiddleware()._build_patched_messages([call, result]) + + assert patched is not None + assert len(patched) == 2 + assert patched[1].content == result.content + assert patched[1].status == "error" + wire_call, wire_result = map(_convert_message_to_dict, patched) + assert wire_call["tool_calls"][0]["id"] == wire_result["tool_call_id"] + assert wire_result["tool_call_id"] + assert call.tool_calls[0]["id"] == call_id + assert result.tool_call_id == result_id + + +def test_mixed_missing_ids_keep_partial_same_name_results_ambiguous(): + call = AIMessage(content="", tool_calls=[{"name": "bash", "id": call_id, "args": {}} for call_id in (None, "")]) + result = ToolMessage(content="UNATTRIBUTABLE", tool_call_id="", name="bash") + patched = DanglingToolCallMiddleware()._build_patched_messages([call, result]) + + assert patched is not None + assert all(message.content != "UNATTRIBUTABLE" for message in patched[1:]) + assert len(patched[1:]) == 2 + assert all(message.status == "error" for message in patched[1:]) + + +def test_missing_id_does_not_override_a_different_tool_name(): + call = AIMessage(content="", tool_calls=[{"name": "write_file", "id": None, "args": {}}]) + result = ToolMessage(content="UNRELATED", tool_call_id="", name="bash") + patched = DanglingToolCallMiddleware()._build_patched_messages([call, result]) + + assert patched is not None + assert patched[1].content != "UNRELATED" + assert patched[1].status == "error" + + +def test_mixed_missing_ids_complete_parallel_turn_keeps_input_order(): + call = AIMessage(content="", tool_calls=[{"name": "bash", "id": call_id, "args": {}} for call_id in (None, "")]) + replies = [ToolMessage(content=text, tool_call_id="", name="bash") for text in ("FIRST", "SECOND")] + patched = DanglingToolCallMiddleware()._build_patched_messages([call, *replies]) + + assert patched is not None + assert [message.content for message in patched[1:]] == ["FIRST", "SECOND"] + assert [message.tool_call_id for message in patched[1:]] == [tc["id"] for tc in patched[0].tool_calls] + + +@pytest.mark.parametrize("sentinel", ["None", "missing_tool_call_id"]) +def test_producer_sentinel_ids_pair_with_a_null_call_id(sentinel): + """Producers spell a missing ID as "None" or "missing_tool_call_id", not "".""" + call = AIMessage(content="", tool_calls=[{"name": "write_file", "id": None, "args": {"path": "a.md"}}]) + result = ToolMessage(content="Error: read the file before writing it", tool_call_id=sentinel, name="write_file", status="error") + patched = DanglingToolCallMiddleware()._build_patched_messages([call, result]) + + assert patched is not None + assert len(patched) == 2 + assert patched[1].content == result.content + assert patched[1].status == "error" + wire_call, wire_result = map(_convert_message_to_dict, patched) + assert wire_call["tool_calls"][0]["id"] == wire_result["tool_call_id"] + + +def test_producer_sentinel_pairing_does_not_invent_one_for_a_valid_id(): + """A sentinel result must not steal the slot of a call that does have an ID.""" + call = AIMessage(content="", tool_calls=[{"name": "write_file", "id": "call_1", "args": {}}]) + result = ToolMessage(content="UNRELATED", tool_call_id="missing_tool_call_id", name="write_file") + patched = DanglingToolCallMiddleware()._build_patched_messages([call, result]) + + assert patched is not None + assert patched[1].content != "UNRELATED" + assert patched[1].status == "error" + + +def test_producer_sentinels_complete_a_parallel_turn_in_input_order(): + """Mixed call spellings plus sentinel results: order still identifies the calls.""" + call = AIMessage(content="", tool_calls=[{"name": "bash", "id": call_id, "args": {}} for call_id in (None, "")]) + replies = [ToolMessage(content=text, tool_call_id="missing_tool_call_id", name="bash") for text in ("FIRST", "SECOND")] + patched = DanglingToolCallMiddleware()._build_patched_messages([call, *replies]) + + assert patched is not None + assert [message.content for message in patched[1:]] == ["FIRST", "SECOND"] + assert [message.tool_call_id for message in patched[1:]] == [tc["id"] for tc in patched[0].tool_calls]