From 474c84ed8d209cf38b0f9d55ebc3c22c2d050366 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:18:34 -0700 Subject: [PATCH] fix(agent): uniquify duplicate tool-call ids to keep call/result pairing lossless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port from openclaw/openclaw#110518 / #110956: some models reuse one call id for different tool calls in a single batch (native Kimi Responses replays, Ollama-compatible endpoints, degraded models at long context). Hermes kept both calls but the pre-API sanitizer then dropped the later call/result pair per id (#58327), so the second call's output silently vanished from every replayed payload — the model never saw it and confabulated. _uniquify_tool_call_ids renames later collisions to a deterministic _d suffix at ingestion, before validation/dispatch/history build, so both pairs survive. Composite Responses ids collide on the call half and keep their response-item half. Deterministic suffixes preserve prompt-cache prefix stability (no random UUIDs). --- agent/conversation_loop.py | 8 +++ run_agent.py | 77 +++++++++++++++++++++ tests/run_agent/test_agent_guardrails.py | 88 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 1f72873edc10..9eb3fdad4b04 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -5469,6 +5469,14 @@ def run_conversation( args_preview = raw_args[:200] if isinstance(raw_args, str) else repr(raw_args)[:200] logging.debug("Tool call: %s with args: %s...", tc.function.name, args_preview) + # Uniquify duplicate tool-call ids BEFORE any downstream + # consumer (validation error paths, dispatch, history build, + # Responses item-id derivation). Models that reuse one id for + # different calls in a batch otherwise lose the later call's + # result: the pre-API sanitizer keeps only the first + # call/result pair per id. See _uniquify_tool_call_ids. + agent._uniquify_tool_call_ids(assistant_message.tool_calls) + # Validate tool call names - detect model hallucinations # Repair mismatched tool names before validating for tc in assistant_message.tool_calls: diff --git a/run_agent.py b/run_agent.py index 40a1cdfe700f..21e550386106 100644 --- a/run_agent.py +++ b/run_agent.py @@ -4224,6 +4224,83 @@ class AIAgent: logger.warning("Removed duplicate tool call: %s", tc.function.name) return unique if len(unique) < len(tool_calls) else tool_calls + @staticmethod + def _uniquify_tool_call_ids(tool_calls: list) -> list: + """Ensure every tool call in a single assistant turn has a distinct id. + + Some models/providers reuse one call id across different calls in a + single batch (observed with native Kimi Responses replays, Ollama- + compatible endpoints, and degraded models at long context; same bug + class as openclaw/openclaw#110518 / #110956). Duplicate ids are lossy + downstream: the pre-API sanitizer keeps only the first call/result + pair per id (#58327), so the later call's result silently vanishes + from every replayed payload, and strict providers (Anthropic + tool_use, DeepSeek) reject duplicate ids outright. + + The first occurrence keeps its id; later collisions get a + deterministic ``_d`` suffix — never a random UUID, which would + break prompt-cache prefix stability across replays. Mutates the + entries in place (SDK models / SimpleNamespace / dicts) and returns + the same list. Blank/missing ids are left for the deterministic + fallback in ``build_assistant_message``. + """ + seen: set = set() + for tc in tool_calls or []: + if isinstance(tc, dict): + raw = tc.get("call_id") or tc.get("id") or "" + else: + raw = getattr(tc, "call_id", None) or getattr(tc, "id", None) or "" + raw = raw.strip() if isinstance(raw, str) else "" + if not raw: + continue + # Composite Responses ids ("call_x|fc_y") collide on the call + # half — that's the pairing key providers enforce per turn. + cid = raw.split("|", 1)[0] + if not cid: + continue + if cid not in seen: + seen.add(cid) + continue + n = 2 + new_id = f"{cid}_d{n}" + while new_id in seen: + n += 1 + new_id = f"{cid}_d{n}" + seen.add(new_id) + + def _renamed(value): + # Preserve a composite id's response-item half so the + # provider's real fc_/item id survives the rename. + if isinstance(value, str) and "|" in value: + return f"{new_id}|{value.split('|', 1)[1]}" + return new_id + + try: + if isinstance(tc, dict): + if tc.get("id"): + tc["id"] = _renamed(tc["id"]) + else: + tc["id"] = new_id + if tc.get("call_id"): + tc["call_id"] = new_id + else: + tc.id = _renamed(getattr(tc, "id", None)) + if getattr(tc, "call_id", None): + tc.call_id = new_id + except Exception: + logger.warning( + "Could not uniquify duplicate tool call id %s", cid + ) + continue + _fn = tc.get("function") if isinstance(tc, dict) else getattr(tc, "function", None) + _fn_name = (_fn.get("name") if isinstance(_fn, dict) else getattr(_fn, "name", None)) or "?" + logger.warning( + "Model reused tool call id %s within one turn; renamed the " + "duplicate to %s (tool=%s) to keep call/result pairing " + "lossless.", cid, new_id, _fn_name, + ) + return tool_calls + def _repair_tool_call(self, tool_name: str) -> str | None: """Forwarder — see ``agent.agent_runtime_helpers.repair_tool_call``.""" from agent.agent_runtime_helpers import repair_tool_call diff --git a/tests/run_agent/test_agent_guardrails.py b/tests/run_agent/test_agent_guardrails.py index bcec4e3d7c8f..9bf565fd6308 100644 --- a/tests/run_agent/test_agent_guardrails.py +++ b/tests/run_agent/test_agent_guardrails.py @@ -4,6 +4,7 @@ Covers three static methods on AIAgent (inspired by PR #1321 — @alireza78a): - _sanitize_api_messages() — Phase 1: orphaned tool pair repair - _cap_delegate_task_calls() — Phase 2a: subagent concurrency limit - _deduplicate_tool_calls() — Phase 2b: identical call deduplication + - _uniquify_tool_call_ids() — Phase 2c: duplicate-id repair (lossless pairing) """ import types @@ -277,6 +278,93 @@ class TestDeduplicateToolCalls: assert len(tcs) == original_len +# --------------------------------------------------------------------------- +# Phase 2c — _uniquify_tool_call_ids +# --------------------------------------------------------------------------- + +def make_tc_id(id_: str, name: str, arguments: str = "{}", call_id=None) -> types.SimpleNamespace: + tc = types.SimpleNamespace() + tc.id = id_ + if call_id is not None: + tc.call_id = call_id + tc.function = types.SimpleNamespace(name=name, arguments=arguments) + return tc + + +class TestUniquifyToolCallIds: + + def test_distinct_calls_sharing_id_get_unique_ids(self): + tcs = [ + make_tc_id("call_1", "read_file", '{"path":"a.txt"}'), + make_tc_id("call_1", "read_file", '{"path":"b.txt"}'), + ] + out = AIAgent._uniquify_tool_call_ids(tcs) + assert out is tcs + assert tcs[0].id == "call_1" # first occurrence untouched + assert tcs[1].id == "call_1_d2" # deterministic suffix + assert len({tc.id for tc in tcs}) == 2 + + def test_three_way_collision(self): + tcs = [make_tc_id("x", "t", '{"a":1}'), + make_tc_id("x", "t", '{"a":2}'), + make_tc_id("x", "t", '{"a":3}')] + AIAgent._uniquify_tool_call_ids(tcs) + assert [tc.id for tc in tcs] == ["x", "x_d2", "x_d3"] + + def test_suffix_collision_with_existing_id_avoided(self): + # A real id "x_d2" already exists — the rename must skip past it. + tcs = [make_tc_id("x", "t", '{"a":1}'), + make_tc_id("x_d2", "t", '{"a":2}'), + make_tc_id("x", "t", '{"a":3}')] + AIAgent._uniquify_tool_call_ids(tcs) + assert [tc.id for tc in tcs] == ["x", "x_d2", "x_d3"] + + def test_unique_ids_untouched(self): + tcs = [make_tc_id("a", "t1"), make_tc_id("b", "t2")] + AIAgent._uniquify_tool_call_ids(tcs) + assert [tc.id for tc in tcs] == ["a", "b"] + + def test_blank_and_missing_ids_left_for_fallback(self): + tc_blank = make_tc_id("", "t1") + tc_none = make_tc_id(None, "t2") + tc_noattr = make_tc("t3") # no .id attribute at all + AIAgent._uniquify_tool_call_ids([tc_blank, tc_none, tc_noattr]) + assert tc_blank.id == "" + assert tc_none.id is None + assert not hasattr(tc_noattr, "id") + + def test_call_id_sibling_kept_consistent(self): + # Responses-path objects carry call_id; build_assistant_message + # prefers it, so the rename must update both. + tcs = [make_tc_id("call_1", "t", '{"a":1}', call_id="call_1"), + make_tc_id("call_1", "t", '{"a":2}', call_id="call_1")] + AIAgent._uniquify_tool_call_ids(tcs) + assert tcs[1].id == "call_1_d2" + assert tcs[1].call_id == "call_1_d2" + assert tcs[0].call_id == "call_1" + + def test_dict_entries_supported(self): + tcs = [{"id": "c", "function": {"name": "t", "arguments": '{"a":1}'}}, + {"id": "c", "call_id": "c", "function": {"name": "t", "arguments": '{"a":2}'}}] + AIAgent._uniquify_tool_call_ids(tcs) + assert tcs[0]["id"] == "c" + assert tcs[1]["id"] == "c_d2" + assert tcs[1]["call_id"] == "c_d2" + + def test_empty_and_none_safe(self): + assert AIAgent._uniquify_tool_call_ids([]) == [] + assert AIAgent._uniquify_tool_call_ids(None) is None + + def test_composite_responses_ids_collide_on_call_half(self): + # "call_x|fc_y" composites pair on the call half; the rename must + # keep the provider's response-item half intact. + tcs = [make_tc_id("call_x|fc_1", "t", '{"a":1}'), + make_tc_id("call_x|fc_2", "t", '{"a":2}')] + AIAgent._uniquify_tool_call_ids(tcs) + assert tcs[0].id == "call_x|fc_1" + assert tcs[1].id == "call_x_d2|fc_2" + + # --------------------------------------------------------------------------- # _get_tool_call_id_static # ---------------------------------------------------------------------------