mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
Merge pull request #58350 from kshitijk4poor/salvage/dedup-tool-call-id
fix(agent): deduplicate tool_call_id across pre-API sanitizers (#58327)
This commit is contained in:
commit
7203898ce4
2 changed files with 139 additions and 0 deletions
|
|
@ -506,6 +506,12 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int:
|
|||
tc_id = msg.get("tool_call_id")
|
||||
if tc_id and tc_id in known_tool_ids:
|
||||
filtered.append(msg)
|
||||
# Consume the id so a SECOND tool result carrying the same
|
||||
# tool_call_id (duplicate from a retry/crash/session-resume
|
||||
# glitch) falls into the drop branch below instead of being
|
||||
# replayed — strict providers (DeepSeek) reject a duplicate
|
||||
# tool_call_id with HTTP 400 (#58327). Credit: #55436.
|
||||
known_tool_ids.discard(tc_id)
|
||||
else:
|
||||
repairs += 1
|
||||
else:
|
||||
|
|
@ -2478,6 +2484,50 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]
|
|||
"Pre-call sanitizer: added %d stub tool result(s)",
|
||||
len(missing_results),
|
||||
)
|
||||
|
||||
# 3. Deduplicate tool_call_ids. Strict providers (DeepSeek) reject a
|
||||
# payload where the same tool_call_id appears more than once with HTTP 400
|
||||
# "Duplicate value for 'tool_call_id'" (#58327). Duplicates can arise from
|
||||
# retries, crash/resume glitches, or a compression window that re-emits a
|
||||
# tool result. This is the final pre-API chokepoint, so dedup defensively
|
||||
# here even though repair_message_sequence also consumes matched ids.
|
||||
# (a) collapse duplicate tool_calls WITHIN an assistant message
|
||||
# (b) drop later tool result messages reusing an already-seen id
|
||||
seen_assistant_call_ids: set = set()
|
||||
seen_result_call_ids: set = set()
|
||||
deduped: List[Dict[str, Any]] = []
|
||||
removed_dupes = 0
|
||||
for msg in messages:
|
||||
role = msg.get("role")
|
||||
if role == "assistant" and msg.get("tool_calls"):
|
||||
kept_tcs = []
|
||||
for tc in msg.get("tool_calls") or []:
|
||||
cid = _ra().AIAgent._get_tool_call_id_static(tc)
|
||||
if cid and cid in seen_assistant_call_ids:
|
||||
removed_dupes += 1
|
||||
continue
|
||||
if cid:
|
||||
seen_assistant_call_ids.add(cid)
|
||||
kept_tcs.append(tc)
|
||||
if len(kept_tcs) != len(msg.get("tool_calls") or []):
|
||||
msg = {**msg, "tool_calls": kept_tcs}
|
||||
deduped.append(msg)
|
||||
elif role == "tool":
|
||||
cid = (msg.get("tool_call_id") or "").strip()
|
||||
if cid and cid in seen_result_call_ids:
|
||||
removed_dupes += 1
|
||||
continue
|
||||
if cid:
|
||||
seen_result_call_ids.add(cid)
|
||||
deduped.append(msg)
|
||||
else:
|
||||
deduped.append(msg)
|
||||
if removed_dupes:
|
||||
messages = deduped
|
||||
_ra().logger.debug(
|
||||
"Pre-call sanitizer: removed %d duplicate tool_call_id reference(s)",
|
||||
removed_dupes,
|
||||
)
|
||||
return messages
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -621,3 +621,92 @@ def test_repair_does_NOT_merge_codex_interim_assistants():
|
|||
assert len(interim) == 2
|
||||
encs = [m["codex_reasoning_items"][0]["encrypted_content"] for m in interim]
|
||||
assert "enc_first" in encs and "enc_second" in encs
|
||||
|
||||
|
||||
# ── tool_call_id de-duplication (#58327) ────────────────────────────────────
|
||||
# Strict providers (DeepSeek) reject a payload where the same tool_call_id
|
||||
# appears more than once with HTTP 400 "Duplicate value for 'tool_call_id'".
|
||||
|
||||
|
||||
def test_repair_deduplicates_duplicate_tool_results():
|
||||
"""A second tool result reusing an already-matched tool_call_id is dropped.
|
||||
|
||||
repair_message_sequence consumes the id from known_tool_ids on first match
|
||||
so the duplicate falls into the repair/drop branch (#58327, kernel #55436).
|
||||
"""
|
||||
from agent.agent_runtime_helpers import repair_message_sequence
|
||||
|
||||
agent = _bare_agent()
|
||||
messages = [
|
||||
{"role": "user", "content": "run the tool"},
|
||||
{"role": "assistant", "content": "",
|
||||
"tool_calls": [{"id": "call_1", "type": "function",
|
||||
"function": {"name": "test", "arguments": "{}"}}]},
|
||||
{"role": "tool", "tool_call_id": "call_1", "content": "res1"},
|
||||
{"role": "tool", "tool_call_id": "call_1", "content": "res1 duplicate"},
|
||||
]
|
||||
repairs = repair_message_sequence(agent, messages)
|
||||
assert repairs == 1
|
||||
tool_msgs = [m for m in messages if m.get("role") == "tool"]
|
||||
assert len(tool_msgs) == 1
|
||||
assert tool_msgs[0]["content"] == "res1"
|
||||
|
||||
|
||||
def test_sanitize_deduplicates_duplicate_tool_results():
|
||||
"""sanitize_api_messages (final pre-API chokepoint) drops duplicate tool
|
||||
results sharing a tool_call_id."""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
messages = [
|
||||
{"role": "user", "content": "hi"},
|
||||
{"role": "assistant", "content": None,
|
||||
"tool_calls": [{"id": "call_X", "type": "function",
|
||||
"function": {"name": "foo", "arguments": "{}"}}]},
|
||||
{"role": "tool", "tool_call_id": "call_X", "content": "A"},
|
||||
{"role": "tool", "tool_call_id": "call_X", "content": "B (duplicate)"},
|
||||
{"role": "assistant", "content": "done"},
|
||||
]
|
||||
out = sanitize_api_messages(list(messages))
|
||||
tool_ids = [m["tool_call_id"] for m in out if m.get("role") == "tool"]
|
||||
assert tool_ids == ["call_X"] # exactly one survives
|
||||
|
||||
|
||||
def test_sanitize_deduplicates_duplicate_assistant_tool_call_ids():
|
||||
"""sanitize_api_messages collapses duplicate tool_calls sharing an id
|
||||
WITHIN a single assistant message (the message[6] shape from #58327)."""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
messages = [
|
||||
{"role": "assistant", "content": None, "tool_calls": [
|
||||
{"id": "call_Y", "type": "function",
|
||||
"function": {"name": "foo", "arguments": "{}"}},
|
||||
{"id": "call_Y", "type": "function",
|
||||
"function": {"name": "bar", "arguments": "{}"}},
|
||||
]},
|
||||
{"role": "tool", "tool_call_id": "call_Y", "content": "r"},
|
||||
]
|
||||
out = sanitize_api_messages(list(messages))
|
||||
assistant = [m for m in out if m.get("role") == "assistant"][0]
|
||||
ids = [tc["id"] for tc in assistant["tool_calls"]]
|
||||
assert ids == ["call_Y"] # duplicate collapsed
|
||||
|
||||
|
||||
def test_sanitize_preserves_distinct_tool_call_ids():
|
||||
"""Negative control: legitimate DISTINCT tool_call_ids must NOT be dropped
|
||||
(guards against over-dedup)."""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
messages = [
|
||||
{"role": "assistant", "content": None, "tool_calls": [
|
||||
{"id": "call_A", "type": "function",
|
||||
"function": {"name": "a", "arguments": "{}"}},
|
||||
{"id": "call_B", "type": "function",
|
||||
"function": {"name": "b", "arguments": "{}"}},
|
||||
]},
|
||||
{"role": "tool", "tool_call_id": "call_A", "content": "ra"},
|
||||
{"role": "tool", "tool_call_id": "call_B", "content": "rb"},
|
||||
]
|
||||
out = sanitize_api_messages(list(messages))
|
||||
assistant = [m for m in out if m.get("role") == "assistant"][0]
|
||||
assert [tc["id"] for tc in assistant["tool_calls"]] == ["call_A", "call_B"]
|
||||
assert sorted(m["tool_call_id"] for m in out if m.get("role") == "tool") == ["call_A", "call_B"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue