From 5239482268ebd15145c9ce0b7ff584b4fbf38929 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 03:07:22 +0000 Subject: [PATCH] Fill in blank tool-call ids so strict providers accept the history Some models return a tool call with an empty or missing id. The paired tool result then carries a blank tool_call_id, which strict providers reject with a 400 on every replay, permanently killing the agent. Treat blank ids like duplicates: assign a fresh call_ and pair the output with it, both in the replayed history and in the streamed turn. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- strix/config/tool_call_ids.py | 59 ++++++++++---- tests/test_tool_call_ids.py | 149 ++++++++++++++++++++++++++++++++-- 2 files changed, 184 insertions(+), 24 deletions(-) diff --git a/strix/config/tool_call_ids.py b/strix/config/tool_call_ids.py index 31f78850..b3af8eb9 100644 --- a/strix/config/tool_call_ids.py +++ b/strix/config/tool_call_ids.py @@ -1,12 +1,17 @@ -"""Keep tool-call ids unique within a conversation. +"""Keep tool-call ids present and unique within a conversation. Some providers return per-turn tool-call ids (``exec_command:0``, ``exec_command:1``, ...) whose counter restarts on every turn. Once the same id appears twice in one conversation, the request payload has two assistant tool calls sharing an id and strict providers reject the whole turn, which permanently kills the agent because the malformed history is replayed on -every retry. Rewriting duplicates to fresh unique ids keeps the history -valid for any provider. +every retry. + +Others omit the id altogether, or return it as an empty string. That turns +the paired tool result into a ``tool`` message with an empty +``tool_call_id``, which strict providers reject the same way and with the +same permanent outcome. Rewriting both blank and duplicate ids to fresh +unique ones keeps the history valid for any provider. """ from __future__ import annotations @@ -22,23 +27,29 @@ def new_call_id() -> str: return f"call_{uuid4().hex}" +def _pairing_key(call_id: Any) -> str: + """Bucket an id for call/output pairing; all blank ids share one bucket.""" + return call_id if isinstance(call_id, str) else "" + + def collect_call_ids(items: list[Any]) -> set[str]: used: set[str] = set() for item in items: if isinstance(item, dict): call_id = item.get("call_id") - if isinstance(call_id, str): + if isinstance(call_id, str) and call_id: used.add(call_id) - elif isinstance(item, ResponseFunctionToolCall): + elif isinstance(item, ResponseFunctionToolCall) and item.call_id: used.add(item.call_id) return used def dedupe_history_call_ids(items: list[Any]) -> tuple[list[Any], bool]: - """Rewrite duplicate call ids in a conversation history. + """Rewrite blank and duplicate call ids in a conversation history. Outputs are paired with their call by order, so parallel calls that share - an id keep answering the right call after the rewrite. + an id — or are all missing one — keep answering the right call after the + rewrite. """ used: set[str] = set() pending: dict[str, deque[str]] = defaultdict(deque) @@ -49,22 +60,24 @@ def dedupe_history_call_ids(items: list[Any]) -> tuple[list[Any], bool]: if not isinstance(item, dict): rebuilt.append(item) continue - call_id = item.get("call_id") - if not isinstance(call_id, str): + + kind = item.get("type") + if kind not in ("function_call", "function_call_output"): rebuilt.append(item) continue - kind = item.get("type") + call_id = item.get("call_id") + key = _pairing_key(call_id) if kind == "function_call": - effective = call_id - if call_id in used: + effective = key + if not effective or effective in used: effective = new_call_id() item = {**item, "call_id": effective} # noqa: PLW2901 changed = True used.add(effective) - pending[call_id].append(effective) - elif kind == "function_call_output": - queue = pending.get(call_id) + pending[key].append(effective) + else: + queue = pending.get(key) if queue: effective = queue.popleft() if effective != call_id: @@ -83,7 +96,7 @@ def dedupe_input(model_input: str | list[Any]) -> str | list[Any]: class TurnCallIdRewriter: - """Rewrite a single turn's tool-call ids that collide with the history. + """Rewrite a single turn's tool-call ids that are blank or collide with the history. A turn's items surface several times (streamed item events, then the completed response), so the same original id must always map to the same @@ -93,12 +106,26 @@ class TurnCallIdRewriter: def __init__(self, model_input: str | list[Any]) -> None: self._used = set() if isinstance(model_input, str) else collect_call_ids(model_input) self._remap: dict[str, str] = {} + self._blank_remap: dict[str, str] = {} self._settled: set[str] = set() + def _rewrite_blank(self, item: ResponseFunctionToolCall) -> ResponseFunctionToolCall: + """Give a call with no id one, keyed by item id so parallel calls differ.""" + key = item.id or "" + replacement = self._blank_remap.get(key) + if replacement is None: + replacement = new_call_id() + self._blank_remap[key] = replacement + self._used.add(replacement) + self._settled.add(replacement) + return item.model_copy(update={"call_id": replacement}) + def rewrite_item(self, item: Any) -> Any: if not isinstance(item, ResponseFunctionToolCall): return item original = item.call_id + if not original: + return self._rewrite_blank(item) if original in self._settled: return item replacement = self._remap.get(original) diff --git a/tests/test_tool_call_ids.py b/tests/test_tool_call_ids.py index bfb03598..2647315f 100644 --- a/tests/test_tool_call_ids.py +++ b/tests/test_tool_call_ids.py @@ -1,11 +1,12 @@ -"""Tests for tool-call id uniqueness. +"""Tests for tool-call id presence and uniqueness. Providers that number tool calls per turn (``exec_command:0``, ``:1``, ...) restart the counter on every turn, so the same id eventually appears twice in -one conversation. Strict providers then reject the whole request, and because -the history is replayed on every retry the agent can never recover. A gateway -that validates id uniqueness the way those providers do proves both the -failure and the fix. +one conversation. Others hand back a tool call with no id at all, leaving the +paired tool message with a blank ``tool_call_id``. Strict providers reject the +whole request either way, and because the history is replayed on every retry +the agent can never recover. Gateways that validate ids the way those +providers do prove both failures and their fixes. """ from __future__ import annotations @@ -125,10 +126,51 @@ class _StrictHandler(BaseHTTPRequestHandler): self.wfile.write(encoded) -@pytest.fixture -def strict_gateway() -> Iterator[str]: +class _BlankIdHandler(BaseHTTPRequestHandler): + """Gateway that hands out an id-less tool call and rejects blank ids, like GLM does.""" + + def log_message(self, *args: Any) -> None: + pass + + def do_POST(self) -> None: + length = int(self.headers.get("Content-Length", 0)) + body = json.loads(self.rfile.read(length) or b"{}") + messages = body.get("messages", []) + _REQUESTS.append(messages) + + for index, message in enumerate(messages): + if message.get("role") == "tool" and not message.get("tool_call_id"): + self._respond( + 400, + { + "error": { + "message": ( + f"messages[{index}]: tool messages must include " + "a non-empty string tool_call_id" + ), + "code": 400, + } + }, + ) + return + + if len(_REQUESTS) == 1: + self._respond(200, _tool_call_completion("")) + else: + self._respond(200, _text_completion("all done")) + + def _respond(self, status: int, payload: dict[str, Any]) -> None: + encoded = json.dumps(payload).encode() + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(encoded))) + self.end_headers() + self.wfile.write(encoded) + + +def _serve(handler: type[BaseHTTPRequestHandler]) -> Iterator[str]: _REQUESTS.clear() - server = HTTPServer(("127.0.0.1", 0), _StrictHandler) + server = HTTPServer(("127.0.0.1", 0), handler) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() try: @@ -138,6 +180,16 @@ def strict_gateway() -> Iterator[str]: server.server_close() +@pytest.fixture +def strict_gateway() -> Iterator[str]: + yield from _serve(_StrictHandler) + + +@pytest.fixture +def blank_id_gateway() -> Iterator[str]: + yield from _serve(_BlankIdHandler) + + def _model(base_url: str) -> Model: # The gateway answers plain JSON, so the run loop's streamed turns are # served non-streamed; the ids on the wire are the same either way. @@ -222,6 +274,61 @@ def test_history_dedupe_pairs_parallel_calls_by_order() -> None: assert rebuilt[1]["call_id"] != "dup" +@pytest.mark.asyncio +async def test_blank_call_id_is_rejected_by_the_provider_without_the_wrapper( + blank_id_gateway: str, +) -> None: + # Repro: the model answers with a tool call carrying no id, so the paired + # tool result goes back as a ``tool`` message with a blank ``tool_call_id`` + # and the provider rejects the whole replayed history with a 400. + with pytest.raises(Exception, match="non-empty string tool_call_id"): + await _run_agent(blank_id_gateway, wrap=False) + + +@pytest.mark.asyncio +async def test_blank_call_id_is_filled_in_so_the_history_stays_valid( + blank_id_gateway: str, +) -> None: + result = await _run_agent(blank_id_gateway, wrap=True) + + assert result.final_output == "all done" + call_ids = _assistant_call_ids(_REQUESTS[-1]) + assert len(call_ids) == 1 + assert call_ids[0].startswith("call_") + assert _tool_results(_REQUESTS[-1]) == ["did 1"] + + +def test_history_dedupe_fills_in_blank_ids_and_keeps_outputs_paired() -> None: + items = [ + {"type": "function_call", "call_id": "", "name": "a", "arguments": "{}"}, + {"type": "function_call", "call_id": "", "name": "b", "arguments": "{}"}, + {"type": "function_call_output", "call_id": "", "output": "for-a"}, + {"type": "function_call_output", "call_id": "", "output": "for-b"}, + ] + + rebuilt, changed = dedupe_history_call_ids(items) + + assert changed + ids = [item["call_id"] for item in rebuilt] + assert all(call_id.startswith("call_") for call_id in ids) + assert ids[0] == ids[2] + assert ids[1] == ids[3] + assert ids[0] != ids[1] + + +def test_history_dedupe_fills_in_a_missing_call_id_key() -> None: + items = [ + {"type": "function_call", "name": "a", "arguments": "{}"}, + {"type": "function_call_output", "output": "x"}, + ] + + rebuilt, changed = dedupe_history_call_ids(items) + + assert changed + assert rebuilt[0]["call_id"] == rebuilt[1]["call_id"] + assert rebuilt[0]["call_id"].startswith("call_") + + def test_history_dedupe_leaves_unique_ids_alone() -> None: items = [ {"type": "function_call", "call_id": "call_a", "name": "a", "arguments": "{}"}, @@ -247,3 +354,29 @@ def test_turn_rewriter_is_stable_across_repeated_sightings() -> None: assert first.call_id != "exec_command:0" assert second.call_id == first.call_id + + +def test_turn_rewriter_fills_in_a_blank_id_stably() -> None: + rewriter = TurnCallIdRewriter([]) + call = ResponseFunctionToolCall( + id="fc_1", call_id="", name="a", arguments="{}", type="function_call" + ) + + first = rewriter.rewrite_item(call) + second = rewriter.rewrite_item(call) + + assert first.call_id.startswith("call_") + assert second.call_id == first.call_id + assert rewriter.rewrite_item(first).call_id == first.call_id + + +def test_turn_rewriter_gives_parallel_blank_calls_distinct_ids() -> None: + rewriter = TurnCallIdRewriter([]) + a = ResponseFunctionToolCall( + id="fc_1", call_id="", name="a", arguments="{}", type="function_call" + ) + b = ResponseFunctionToolCall( + id="fc_2", call_id="", name="b", arguments="{}", type="function_call" + ) + + assert rewriter.rewrite_item(a).call_id != rewriter.rewrite_item(b).call_id