diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index aff921c0..d8f5434b 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -5,6 +5,7 @@ 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 @@ -1361,18 +1362,29 @@ class AgentRunner: def _dedupe_tool_calls( messages: list[dict[str, Any]], ) -> list[dict[str, Any]]: - """Remove duplicate tool_call / tool_result ids from the history. + """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, since - the bad message is re-sent on every turn. Keep the first occurrence of - each id across all assistant tool_calls, and likewise keep only the - first tool result per id so pairing stays 1:1. + 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() - seen_result_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 @@ -1383,11 +1395,20 @@ class AgentRunner: changed = False for tc in msg.get("tool_calls") or []: tid = tc.get("id") if isinstance(tc, dict) else None - if tid and str(tid) in seen_call_ids: - changed = True + if not tid: + kept.append(tc) continue - if tid: - seen_call_ids.add(str(tid)) + 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) @@ -1395,10 +1416,17 @@ class AgentRunner: elif role == "tool": tid = msg.get("tool_call_id") if tid: - if str(tid) in seen_result_ids: + raw_id = str(tid) + queue = pending_result_ids.get(raw_id) + if not queue: drop = True else: - seen_result_ids.add(str(tid)) + 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]] diff --git a/nanobot/providers/anthropic_provider.py b/nanobot/providers/anthropic_provider.py index 3d0c5322..97335afd 100644 --- a/nanobot/providers/anthropic_provider.py +++ b/nanobot/providers/anthropic_provider.py @@ -530,15 +530,19 @@ class AnthropicProvider(LLMProvider): if block.type == "text": content_parts.append(block.text) elif block.type == "tool_use": - # Anthropic requires every tool_use id to be unique. A mis-assembled - # stream can occasionally surface the same block twice; keep the first - # and drop the duplicate so it never poisons the persisted history. - if block.id in seen_tool_ids: - logger.warning("dropping duplicate tool_use id from response: {}", block.id) - continue - seen_tool_ids.add(block.id) + tool_id = str(block.id or _gen_tool_id()) + if tool_id in seen_tool_ids: + original_id = tool_id + while tool_id in seen_tool_ids: + tool_id = _gen_tool_id() + logger.warning( + "remapping duplicate tool_use id from response: {} -> {}", + original_id, + tool_id, + ) + seen_tool_ids.add(tool_id) tool_calls.append(ToolCallRequest( - id=block.id, + id=tool_id, name=block.name, arguments=block.input, )) diff --git a/tests/agent/test_runner_governance.py b/tests/agent/test_runner_governance.py index 1bcb6e87..5e8fc784 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, ToolCallRequest +from nanobot.providers.base import LLMResponse _MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars @@ -184,7 +184,7 @@ async def test_backfill_missing_tool_results_inserts_error(): assert backfilled[0]["name"] == "read_file" -def test_dedupe_tool_calls_removes_duplicate_ids(): +def test_dedupe_tool_calls_remaps_duplicate_ids(): from nanobot.agent.runner import AgentRunner messages = [ @@ -194,15 +194,14 @@ def test_dedupe_tool_calls_removes_duplicate_ids(): "content": "", "tool_calls": [ {"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}}, - {"id": "b", "type": "function", "function": {"name": "y", "arguments": "{}"}}, - # Duplicate of "b" — would trigger "tool_use ids must be unique". - {"id": "b", "type": "function", "function": {"name": "y", "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"}, - # Duplicate result for "b". - {"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-dup"}, + {"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-remapped"}, ] cleaned = AgentRunner._dedupe_tool_calls(messages) @@ -214,11 +213,13 @@ def test_dedupe_tool_calls_removes_duplicate_ids(): "content": "", "tool_calls": [ {"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}}, - {"id": "b", "type": "function", "function": {"name": "y", "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"}, ] diff --git a/tests/providers/test_anthropic_tool_result.py b/tests/providers/test_anthropic_tool_result.py index 0ae89723..eb284074 100644 --- a/tests/providers/test_anthropic_tool_result.py +++ b/tests/providers/test_anthropic_tool_result.py @@ -10,6 +10,8 @@ Also tests that bare dicts without a "type" field are coerced to text blocks, fixing Anthropic "content.0.type: Field required" rejections (#3993). """ +from types import SimpleNamespace + from nanobot.providers.anthropic_provider import AnthropicProvider @@ -132,3 +134,33 @@ def test_anthropic_sanitized_tool_ids_avoid_simple_collisions(): ids = [block["id"] for block in blocks if block["type"] == "tool_use"] assert len(ids) == len(set(ids)) == 2 assert all(all(ch.isalnum() or ch in "_-" for ch in tool_id) for tool_id in ids) + + +def test_anthropic_parse_response_remaps_duplicate_tool_use_ids(): + response = SimpleNamespace( + content=[ + SimpleNamespace( + type="tool_use", + id="toolu_same", + name="read_file", + input={"path": "a.txt"}, + ), + SimpleNamespace( + type="tool_use", + id="toolu_same", + name="read_file", + input={"path": "b.txt"}, + ), + ], + stop_reason="tool_use", + usage=None, + ) + + result = AnthropicProvider._parse_response(response) + + assert len(result.tool_calls) == 2 + assert result.tool_calls[0].id == "toolu_same" + assert result.tool_calls[0].arguments == {"path": "a.txt"} + assert result.tool_calls[1].id != "toolu_same" + assert result.tool_calls[1].id.startswith("toolu_") + assert result.tool_calls[1].arguments == {"path": "b.txt"}