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.
This commit is contained in:
+2
-84
@@ -5,7 +5,6 @@ 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
|
||||||
@@ -374,8 +373,7 @@ class AgentRunner:
|
|||||||
# may repair or compact historical messages for the model, but
|
# may repair or compact historical messages for the model, but
|
||||||
# those synthetic edits must not shift the append boundary used
|
# those synthetic edits must not shift the append boundary used
|
||||||
# later when the caller saves only the new turn.
|
# 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)
|
||||||
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
|
||||||
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
||||||
messages_for_model = self._microcompact(messages_for_model)
|
messages_for_model = self._microcompact(messages_for_model)
|
||||||
messages_for_model = self._apply_tool_result_budget(spec, 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",
|
spec.session_key or "default",
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
messages_for_model = self._dedupe_tool_calls(messages)
|
messages_for_model = self._drop_orphan_tool_results(messages)
|
||||||
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
|
||||||
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
||||||
except Exception:
|
except Exception:
|
||||||
messages_for_model = messages
|
messages_for_model = messages
|
||||||
@@ -1358,85 +1355,6 @@ class AgentRunner:
|
|||||||
return truncate_text(content, spec.max_tool_result_chars)
|
return truncate_text(content, spec.max_tool_result_chars)
|
||||||
return content
|
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
|
@staticmethod
|
||||||
def _drop_orphan_tool_results(
|
def _drop_orphan_tool_results(
|
||||||
messages: list[dict[str, Any]],
|
messages: list[dict[str, Any]],
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import hashlib
|
|||||||
import re
|
import re
|
||||||
import secrets
|
import secrets
|
||||||
import string
|
import string
|
||||||
|
from collections import deque
|
||||||
from collections.abc import Awaitable, Callable
|
from collections.abc import Awaitable, Callable
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
@@ -156,6 +157,42 @@ class AnthropicProvider(LLMProvider):
|
|||||||
"""Return ``(system, anthropic_messages)``."""
|
"""Return ``(system, anthropic_messages)``."""
|
||||||
system: str | list[dict[str, Any]] = ""
|
system: str | list[dict[str, Any]] = ""
|
||||||
raw: 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:
|
for msg in messages:
|
||||||
role = msg.get("role", "")
|
role = msg.get("role", "")
|
||||||
@@ -166,7 +203,7 @@ class AnthropicProvider(LLMProvider):
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
if role == "tool":
|
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":
|
if raw and raw[-1]["role"] == "user":
|
||||||
prev_c = raw[-1]["content"]
|
prev_c = raw[-1]["content"]
|
||||||
if isinstance(prev_c, list):
|
if isinstance(prev_c, list):
|
||||||
@@ -180,7 +217,10 @@ class AnthropicProvider(LLMProvider):
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
if role == "assistant":
|
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
|
continue
|
||||||
|
|
||||||
if role == "user":
|
if role == "user":
|
||||||
@@ -193,11 +233,20 @@ class AnthropicProvider(LLMProvider):
|
|||||||
return system, self._merge_consecutive(raw)
|
return system, self._merge_consecutive(raw)
|
||||||
|
|
||||||
@staticmethod
|
@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")
|
content = msg.get("content")
|
||||||
|
tool_call_id = msg.get("tool_call_id", "")
|
||||||
block: dict[str, Any] = {
|
block: dict[str, Any] = {
|
||||||
"type": "tool_result",
|
"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):
|
if isinstance(content, list):
|
||||||
block["content"] = AnthropicProvider._convert_user_content(content)
|
block["content"] = AnthropicProvider._convert_user_content(content)
|
||||||
@@ -208,7 +257,11 @@ class AnthropicProvider(LLMProvider):
|
|||||||
return block
|
return block
|
||||||
|
|
||||||
@staticmethod
|
@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]] = []
|
blocks: list[dict[str, Any]] = []
|
||||||
content = msg.get("content")
|
content = msg.get("content")
|
||||||
|
|
||||||
@@ -231,9 +284,10 @@ class AnthropicProvider(LLMProvider):
|
|||||||
continue
|
continue
|
||||||
func = tc.get("function", {})
|
func = tc.get("function", {})
|
||||||
args = func.get("arguments", "{}")
|
args = func.get("arguments", "{}")
|
||||||
|
raw_id = tc.get("id") or _gen_tool_id()
|
||||||
blocks.append({
|
blocks.append({
|
||||||
"type": "tool_use",
|
"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", ""),
|
"name": func.get("name", ""),
|
||||||
"input": tool_arguments_object_for_replay(args),
|
"input": tool_arguments_object_for_replay(args),
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -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
|
from nanobot.providers.base import LLMResponse, ToolCallRequest
|
||||||
|
|
||||||
_MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars
|
_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"
|
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():
|
def test_drop_orphan_tool_results_removes_unmatched_tool_messages():
|
||||||
from nanobot.agent.runner import AgentRunner
|
from nanobot.agent.runner import AgentRunner
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
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():
|
def test_anthropic_parse_response_remaps_duplicate_tool_use_ids():
|
||||||
response = SimpleNamespace(
|
response = SimpleNamespace(
|
||||||
content=[
|
content=[
|
||||||
|
|||||||
Reference in New Issue
Block a user