From 0e9861558a4c57a2147c2ddf9e2f3ed3a3d2a2e6 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Tue, 23 Jun 2026 23:52:47 +0800 Subject: [PATCH] fix: keep duplicate id repair in Anthropic provider maintainer edit: move duplicate tool_use history repair out of AgentRunner and into Anthropic message conversion, reusing the OpenAI-compatible queue-mapping approach locally without broadening the shared runner path. --- nanobot/agent/runner.py | 86 +------------------ nanobot/providers/anthropic_provider.py | 66 ++++++++++++-- tests/agent/test_runner_governance.py | 60 +------------ tests/providers/test_anthropic_tool_result.py | 46 ++++++++++ 4 files changed, 109 insertions(+), 149 deletions(-) diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index d8f5434b..e3398ecb 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -5,7 +5,6 @@ from __future__ import annotations import asyncio import inspect import os -from collections import deque from contextlib import suppress from copy import deepcopy from dataclasses import dataclass, field @@ -374,8 +373,7 @@ class AgentRunner: # may repair or compact historical messages for the model, but # those synthetic edits must not shift the append boundary used # later when the caller saves only the new turn. - messages_for_model = self._dedupe_tool_calls(messages) - messages_for_model = self._drop_orphan_tool_results(messages_for_model) + messages_for_model = self._drop_orphan_tool_results(messages) messages_for_model = self._backfill_missing_tool_results(messages_for_model) messages_for_model = self._microcompact(messages_for_model) messages_for_model = self._apply_tool_result_budget(spec, messages_for_model) @@ -390,8 +388,7 @@ class AgentRunner: spec.session_key or "default", ) try: - messages_for_model = self._dedupe_tool_calls(messages) - messages_for_model = self._drop_orphan_tool_results(messages_for_model) + messages_for_model = self._drop_orphan_tool_results(messages) messages_for_model = self._backfill_missing_tool_results(messages_for_model) except Exception: messages_for_model = messages @@ -1358,85 +1355,6 @@ class AgentRunner: return truncate_text(content, spec.max_tool_result_chars) return content - @staticmethod - def _dedupe_tool_calls( - messages: list[dict[str, Any]], - ) -> list[dict[str, Any]]: - """Make duplicate tool_call ids unique while keeping matching results paired. - - Anthropic rejects any request where two tool_use blocks share an id - ("tool_use ids must be unique"). An accidental duplicate (e.g. from a - mis-assembled stream) otherwise poisons the session permanently. Some - Anthropic-compatible providers also reuse ids for distinct parallel - calls, so preserve every call and rewrite later duplicates instead of - silently dropping work. Tool results are remapped in call order. - """ - seen_call_ids: set[str] = set() - duplicate_counts: dict[str, int] = {} - pending_result_ids: dict[str, deque[str]] = {} - updated: list[dict[str, Any]] | None = None - - def _unique_id(raw_id: str) -> str: - duplicate_counts[raw_id] = duplicate_counts.get(raw_id, 1) + 1 - suffix = duplicate_counts[raw_id] - while True: - candidate = f"{raw_id}__dedupe_{suffix}" - if candidate not in seen_call_ids: - return candidate - suffix += 1 - - for idx, msg in enumerate(messages): - role = msg.get("role") - replacement: dict[str, Any] | None = None - drop = False - - if role == "assistant" and msg.get("tool_calls"): - kept = [] - changed = False - for tc in msg.get("tool_calls") or []: - tid = tc.get("id") if isinstance(tc, dict) else None - if not tid: - kept.append(tc) - continue - raw_id = str(tid) - mapped_id = raw_id - if raw_id in seen_call_ids: - mapped_id = _unique_id(raw_id) - changed = True - seen_call_ids.add(mapped_id) - pending_result_ids.setdefault(raw_id, deque()).append(mapped_id) - if mapped_id != tid: - tc = dict(tc) - tc["id"] = mapped_id - changed = True - kept.append(tc) - if changed: - replacement = dict(msg) - replacement["tool_calls"] = kept - elif role == "tool": - tid = msg.get("tool_call_id") - if tid: - raw_id = str(tid) - queue = pending_result_ids.get(raw_id) - if not queue: - drop = True - else: - mapped_id = queue.popleft() - if not queue: - pending_result_ids.pop(raw_id, None) - if mapped_id != tid: - replacement = dict(msg) - replacement["tool_call_id"] = mapped_id - - if (replacement is not None or drop) and updated is None: - updated = [dict(m) for m in messages[:idx]] - if drop: - continue - if updated is not None: - updated.append(replacement if replacement is not None else dict(msg)) - - return messages if updated is None else updated - @staticmethod def _drop_orphan_tool_results( messages: list[dict[str, Any]], diff --git a/nanobot/providers/anthropic_provider.py b/nanobot/providers/anthropic_provider.py index 97335afd..9e3cf9ce 100644 --- a/nanobot/providers/anthropic_provider.py +++ b/nanobot/providers/anthropic_provider.py @@ -7,6 +7,7 @@ import hashlib import re import secrets import string +from collections import deque from collections.abc import Awaitable, Callable from typing import Any @@ -156,6 +157,42 @@ class AnthropicProvider(LLMProvider): """Return ``(system, anthropic_messages)``.""" system: str | list[dict[str, Any]] = "" raw: list[dict[str, Any]] = [] + seen_tool_ids: set[str] = set() + duplicate_counts: dict[str, int] = {} + pending_tool_ids: dict[str, deque[str]] = {} + + def unique_tool_id(value: Any) -> str: + raw_id = str(value) if value else _gen_tool_id() + mapped_id = _sanitize_tool_id(raw_id) + if mapped_id and mapped_id not in seen_tool_ids: + seen_tool_ids.add(mapped_id) + if value: + pending_tool_ids.setdefault(str(value), deque()).append(mapped_id) + return mapped_id + + seed = mapped_id or _gen_tool_id() + duplicate_counts[seed] = duplicate_counts.get(seed, 1) + 1 + suffix = duplicate_counts[seed] + while True: + candidate = f"{seed}__dedupe_{suffix}" + if candidate not in seen_tool_ids: + seen_tool_ids.add(candidate) + if value: + pending_tool_ids.setdefault(str(value), deque()).append(candidate) + return candidate + suffix += 1 + + def map_tool_result_id(value: Any) -> str: + if not value: + return _sanitize_tool_id(value or "") + raw_id = str(value) + queue = pending_tool_ids.get(raw_id) + if queue: + mapped_id = queue.popleft() + if not queue: + pending_tool_ids.pop(raw_id, None) + return mapped_id + return _sanitize_tool_id(raw_id) for msg in messages: role = msg.get("role", "") @@ -166,7 +203,7 @@ class AnthropicProvider(LLMProvider): continue if role == "tool": - block = self._tool_result_block(msg) + block = self._tool_result_block(msg, map_tool_result_id=map_tool_result_id) if raw and raw[-1]["role"] == "user": prev_c = raw[-1]["content"] if isinstance(prev_c, list): @@ -180,7 +217,10 @@ class AnthropicProvider(LLMProvider): continue if role == "assistant": - raw.append({"role": "assistant", "content": self._assistant_blocks(msg)}) + raw.append({ + "role": "assistant", + "content": self._assistant_blocks(msg, map_tool_id=unique_tool_id), + }) continue if role == "user": @@ -193,11 +233,20 @@ class AnthropicProvider(LLMProvider): return system, self._merge_consecutive(raw) @staticmethod - def _tool_result_block(msg: dict[str, Any]) -> dict[str, Any]: + def _tool_result_block( + msg: dict[str, Any], + *, + map_tool_result_id: Callable[[Any], str] | None = None, + ) -> dict[str, Any]: content = msg.get("content") + tool_call_id = msg.get("tool_call_id", "") block: dict[str, Any] = { "type": "tool_result", - "tool_use_id": _sanitize_tool_id(msg.get("tool_call_id", "")), + "tool_use_id": ( + map_tool_result_id(tool_call_id) + if map_tool_result_id is not None + else _sanitize_tool_id(tool_call_id) + ), } if isinstance(content, list): block["content"] = AnthropicProvider._convert_user_content(content) @@ -208,7 +257,11 @@ class AnthropicProvider(LLMProvider): return block @staticmethod - def _assistant_blocks(msg: dict[str, Any]) -> list[dict[str, Any]]: + def _assistant_blocks( + msg: dict[str, Any], + *, + map_tool_id: Callable[[Any], str] | None = None, + ) -> list[dict[str, Any]]: blocks: list[dict[str, Any]] = [] content = msg.get("content") @@ -231,9 +284,10 @@ class AnthropicProvider(LLMProvider): continue func = tc.get("function", {}) args = func.get("arguments", "{}") + raw_id = tc.get("id") or _gen_tool_id() blocks.append({ "type": "tool_use", - "id": _sanitize_tool_id(tc.get("id") or _gen_tool_id()), + "id": map_tool_id(raw_id) if map_tool_id is not None else _sanitize_tool_id(raw_id), "name": func.get("name", ""), "input": tool_arguments_object_for_replay(args), }) diff --git a/tests/agent/test_runner_governance.py b/tests/agent/test_runner_governance.py index 5e8fc784..901afc71 100644 --- a/tests/agent/test_runner_governance.py +++ b/tests/agent/test_runner_governance.py @@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from nanobot.config.schema import AgentDefaults -from nanobot.providers.base import LLMResponse +from nanobot.providers.base import LLMResponse, ToolCallRequest _MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars @@ -184,64 +184,6 @@ async def test_backfill_missing_tool_results_inserts_error(): assert backfilled[0]["name"] == "read_file" -def test_dedupe_tool_calls_remaps_duplicate_ids(): - from nanobot.agent.runner import AgentRunner - - messages = [ - {"role": "user", "content": "hi"}, - { - "role": "assistant", - "content": "", - "tool_calls": [ - {"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}}, - {"id": "b", "type": "function", "function": {"name": "y", "arguments": '{"path":"a.txt"}'}}, - # Duplicate id with different arguments should be preserved, not dropped. - {"id": "b", "type": "function", "function": {"name": "y", "arguments": '{"path":"b.txt"}'}}, - ], - }, - {"role": "tool", "tool_call_id": "a", "name": "x", "content": "ra"}, - {"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb"}, - {"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-remapped"}, - ] - - cleaned = AgentRunner._dedupe_tool_calls(messages) - - assert cleaned == [ - {"role": "user", "content": "hi"}, - { - "role": "assistant", - "content": "", - "tool_calls": [ - {"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}}, - {"id": "b", "type": "function", "function": {"name": "y", "arguments": '{"path":"a.txt"}'}}, - {"id": "b__dedupe_2", "type": "function", "function": {"name": "y", "arguments": '{"path":"b.txt"}'}}, - ], - }, - {"role": "tool", "tool_call_id": "a", "name": "x", "content": "ra"}, - {"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb"}, - {"role": "tool", "tool_call_id": "b__dedupe_2", "name": "y", "content": "rb-remapped"}, - ] - - -def test_dedupe_tool_calls_noop_when_unique(): - from nanobot.agent.runner import AgentRunner - - messages = [ - {"role": "user", "content": "hi"}, - { - "role": "assistant", - "content": "", - "tool_calls": [ - {"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}}, - ], - }, - {"role": "tool", "tool_call_id": "a", "name": "x", "content": "ra"}, - ] - - # No duplicates -> identical list object returned (cheap no-op path). - assert AgentRunner._dedupe_tool_calls(messages) is messages - - def test_drop_orphan_tool_results_removes_unmatched_tool_messages(): from nanobot.agent.runner import AgentRunner diff --git a/tests/providers/test_anthropic_tool_result.py b/tests/providers/test_anthropic_tool_result.py index eb284074..9cec6769 100644 --- a/tests/providers/test_anthropic_tool_result.py +++ b/tests/providers/test_anthropic_tool_result.py @@ -136,6 +136,52 @@ def test_anthropic_sanitized_tool_ids_avoid_simple_collisions(): assert all(all(ch.isalnum() or ch in "_-" for ch in tool_id) for tool_id in ids) +def test_anthropic_convert_messages_remaps_duplicate_history_tool_ids(): + provider = AnthropicProvider.__new__(AnthropicProvider) + + _system, messages = provider._convert_messages([ + {"role": "user", "content": "check both files"}, + { + "role": "assistant", + "content": "", + "tool_calls": [ + { + "id": "toolu_same", + "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"a.txt"}'}, + }, + { + "id": "toolu_same", + "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"b.txt"}'}, + }, + ], + }, + {"role": "tool", "tool_call_id": "toolu_same", "name": "read_file", "content": "a"}, + {"role": "tool", "tool_call_id": "toolu_same", "name": "read_file", "content": "b"}, + ]) + + tool_uses = [ + block + for block in messages[1]["content"] + if isinstance(block, dict) and block.get("type") == "tool_use" + ] + tool_results = [ + block + for block in messages[2]["content"] + if isinstance(block, dict) and block.get("type") == "tool_result" + ] + tool_use_ids = [block["id"] for block in tool_uses] + tool_result_ids = [block["tool_use_id"] for block in tool_results] + + assert len(tool_use_ids) == 2 + assert tool_use_ids[0] == "toolu_same" + assert tool_use_ids[1] == "toolu_same__dedupe_2" + assert tool_result_ids == tool_use_ids + assert tool_uses[0]["input"] == {"path": "a.txt"} + assert tool_uses[1]["input"] == {"path": "b.txt"} + + def test_anthropic_parse_response_remaps_duplicate_tool_use_ids(): response = SimpleNamespace( content=[