fix: preserve duplicate-id tool calls
maintainer edit: remap duplicate tool_use/tool_call ids instead of dropping later calls, so Anthropic-compatible providers that reuse ids for distinct parallel tool calls keep all requested work while still sending unique ids.
This commit is contained in:
+40
-12
@@ -5,6 +5,7 @@ from __future__ import annotations
|
|||||||
import asyncio
|
import asyncio
|
||||||
import inspect
|
import inspect
|
||||||
import os
|
import os
|
||||||
|
from collections import deque
|
||||||
from contextlib import suppress
|
from contextlib import suppress
|
||||||
from copy import deepcopy
|
from copy import deepcopy
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
@@ -1361,18 +1362,29 @@ class AgentRunner:
|
|||||||
def _dedupe_tool_calls(
|
def _dedupe_tool_calls(
|
||||||
messages: list[dict[str, Any]],
|
messages: list[dict[str, Any]],
|
||||||
) -> 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
|
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
|
("tool_use ids must be unique"). An accidental duplicate (e.g. from a
|
||||||
mis-assembled stream) otherwise poisons the session permanently, since
|
mis-assembled stream) otherwise poisons the session permanently. Some
|
||||||
the bad message is re-sent on every turn. Keep the first occurrence of
|
Anthropic-compatible providers also reuse ids for distinct parallel
|
||||||
each id across all assistant tool_calls, and likewise keep only the
|
calls, so preserve every call and rewrite later duplicates instead of
|
||||||
first tool result per id so pairing stays 1:1.
|
silently dropping work. Tool results are remapped in call order.
|
||||||
"""
|
"""
|
||||||
seen_call_ids: set[str] = set()
|
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
|
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):
|
for idx, msg in enumerate(messages):
|
||||||
role = msg.get("role")
|
role = msg.get("role")
|
||||||
replacement: dict[str, Any] | None = None
|
replacement: dict[str, Any] | None = None
|
||||||
@@ -1383,11 +1395,20 @@ class AgentRunner:
|
|||||||
changed = False
|
changed = False
|
||||||
for tc in msg.get("tool_calls") or []:
|
for tc in msg.get("tool_calls") or []:
|
||||||
tid = tc.get("id") if isinstance(tc, dict) else None
|
tid = tc.get("id") if isinstance(tc, dict) else None
|
||||||
if tid and str(tid) in seen_call_ids:
|
if not tid:
|
||||||
changed = True
|
kept.append(tc)
|
||||||
continue
|
continue
|
||||||
if tid:
|
raw_id = str(tid)
|
||||||
seen_call_ids.add(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)
|
kept.append(tc)
|
||||||
if changed:
|
if changed:
|
||||||
replacement = dict(msg)
|
replacement = dict(msg)
|
||||||
@@ -1395,10 +1416,17 @@ class AgentRunner:
|
|||||||
elif role == "tool":
|
elif role == "tool":
|
||||||
tid = msg.get("tool_call_id")
|
tid = msg.get("tool_call_id")
|
||||||
if tid:
|
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
|
drop = True
|
||||||
else:
|
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:
|
if (replacement is not None or drop) and updated is None:
|
||||||
updated = [dict(m) for m in messages[:idx]]
|
updated = [dict(m) for m in messages[:idx]]
|
||||||
|
|||||||
@@ -530,15 +530,19 @@ class AnthropicProvider(LLMProvider):
|
|||||||
if block.type == "text":
|
if block.type == "text":
|
||||||
content_parts.append(block.text)
|
content_parts.append(block.text)
|
||||||
elif block.type == "tool_use":
|
elif block.type == "tool_use":
|
||||||
# Anthropic requires every tool_use id to be unique. A mis-assembled
|
tool_id = str(block.id or _gen_tool_id())
|
||||||
# stream can occasionally surface the same block twice; keep the first
|
if tool_id in seen_tool_ids:
|
||||||
# and drop the duplicate so it never poisons the persisted history.
|
original_id = tool_id
|
||||||
if block.id in seen_tool_ids:
|
while tool_id in seen_tool_ids:
|
||||||
logger.warning("dropping duplicate tool_use id from response: {}", block.id)
|
tool_id = _gen_tool_id()
|
||||||
continue
|
logger.warning(
|
||||||
seen_tool_ids.add(block.id)
|
"remapping duplicate tool_use id from response: {} -> {}",
|
||||||
|
original_id,
|
||||||
|
tool_id,
|
||||||
|
)
|
||||||
|
seen_tool_ids.add(tool_id)
|
||||||
tool_calls.append(ToolCallRequest(
|
tool_calls.append(ToolCallRequest(
|
||||||
id=block.id,
|
id=tool_id,
|
||||||
name=block.name,
|
name=block.name,
|
||||||
arguments=block.input,
|
arguments=block.input,
|
||||||
))
|
))
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from nanobot.config.schema import AgentDefaults
|
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
|
_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"
|
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
|
from nanobot.agent.runner import AgentRunner
|
||||||
|
|
||||||
messages = [
|
messages = [
|
||||||
@@ -194,15 +194,14 @@ def test_dedupe_tool_calls_removes_duplicate_ids():
|
|||||||
"content": "",
|
"content": "",
|
||||||
"tool_calls": [
|
"tool_calls": [
|
||||||
{"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}},
|
{"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"}'}},
|
||||||
# Duplicate of "b" — would trigger "tool_use ids must be unique".
|
# Duplicate id with different arguments should be preserved, not dropped.
|
||||||
{"id": "b", "type": "function", "function": {"name": "y", "arguments": "{}"}},
|
{"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": "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"},
|
||||||
# Duplicate result for "b".
|
{"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-remapped"},
|
||||||
{"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-dup"},
|
|
||||||
]
|
]
|
||||||
|
|
||||||
cleaned = AgentRunner._dedupe_tool_calls(messages)
|
cleaned = AgentRunner._dedupe_tool_calls(messages)
|
||||||
@@ -214,11 +213,13 @@ def test_dedupe_tool_calls_removes_duplicate_ids():
|
|||||||
"content": "",
|
"content": "",
|
||||||
"tool_calls": [
|
"tool_calls": [
|
||||||
{"id": "a", "type": "function", "function": {"name": "x", "arguments": "{}"}},
|
{"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": "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"},
|
||||||
|
{"role": "tool", "tool_call_id": "b__dedupe_2", "name": "y", "content": "rb-remapped"},
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -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).
|
blocks, fixing Anthropic "content.0.type: Field required" rejections (#3993).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
from nanobot.providers.anthropic_provider import AnthropicProvider
|
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"]
|
ids = [block["id"] for block in blocks if block["type"] == "tool_use"]
|
||||||
assert len(ids) == len(set(ids)) == 2
|
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)
|
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"}
|
||||||
|
|||||||
Reference in New Issue
Block a user