fix(agent): keep tool results when a server reuses one tool_call_id
The #58327 dedup passes treat a repeated tool_call_id as garbage from a
retry/crash/resume glitch and drop it. That assumes tool_call_id is
globally unique, which it is not: llama.cpp emits a single constant id
for every tool call it ever returns (verified — three separate
completions from one server all carried the same id).
Under a seen-once-drop-forever rule, the SECOND legitimate tool result
of such a session looks like a duplicate and is deleted. From the second
tool call onward the model never sees any result: it announces its next
action, the turn ends, and the task is left unfinished. Bisected to
dba585c17 over a 2258-commit range; reproduced live on v0.19.0 (1/6 runs
completed a 4-step file task, vs 20/20 on the last release before that
commit, same model and server).
Key off OUTSTANDING calls instead of every id ever seen. Both original
protections are preserved: a replayed result still answers no pending
call and is still dropped, and duplicate tool_calls sharing an id within
one assistant message are still collapsed. A genuine new call that
reuses the id re-arms it first.
repair_message_sequence needs no change — it already resets its id set
per assistant message, so only the final pre-API pass mis-fires.
Live result after the fix: 8/8 runs complete, 17-26s each (was 1/6 with
runs hitting a 150s ceiling).
This commit is contained in:
committed by
Teknium
parent
046a868b7f
commit
0b8fd04bea
@@ -3678,9 +3678,21 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]
|
||||
# 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
|
||||
# (b) drop tool results that answer no OUTSTANDING tool call
|
||||
#
|
||||
# (b) tracks outstanding calls rather than every id ever seen, because
|
||||
# ``tool_call_id`` is NOT globally unique in practice: llama.cpp emits a
|
||||
# single constant id for every tool call it ever returns (verified: three
|
||||
# separate completions from one server all carry the same id). A
|
||||
# seen-once-drop-forever rule reads the SECOND legitimate tool result of
|
||||
# such a session as a duplicate and deletes it, so from the second tool
|
||||
# call onward the model never sees any result — it announces its next
|
||||
# action and the turn dies with the work unfinished. Outstanding-call
|
||||
# semantics keep both protections intact: a re-emitted result still
|
||||
# answers no pending call and is still dropped, while a genuine new call
|
||||
# that reuses the id re-arms that id first.
|
||||
seen_assistant_call_ids: set = set()
|
||||
seen_result_call_ids: set = set()
|
||||
outstanding_call_ids: set = set()
|
||||
deduped: List[Dict[str, Any]] = []
|
||||
removed_dupes = 0
|
||||
for msg in messages:
|
||||
@@ -3694,6 +3706,7 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]
|
||||
continue
|
||||
if cid:
|
||||
seen_assistant_call_ids.add(cid)
|
||||
outstanding_call_ids.add(cid)
|
||||
kept_tcs.append(tc)
|
||||
if kept_tcs:
|
||||
msg = {**msg, "tool_calls": kept_tcs}
|
||||
@@ -3702,11 +3715,15 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]
|
||||
deduped.append(msg)
|
||||
elif role == "tool":
|
||||
cid = (msg.get("tool_call_id") or "").strip()
|
||||
if cid and cid in seen_result_call_ids:
|
||||
if cid and cid not in outstanding_call_ids:
|
||||
removed_dupes += 1
|
||||
continue
|
||||
if cid:
|
||||
seen_result_call_ids.add(cid)
|
||||
# Answered: this id is no longer outstanding, so a second
|
||||
# result replaying it is still caught above.
|
||||
outstanding_call_ids.discard(cid)
|
||||
# A reused id must be re-armable by the next assistant call.
|
||||
seen_assistant_call_ids.discard(cid)
|
||||
deduped.append(msg)
|
||||
else:
|
||||
deduped.append(msg)
|
||||
|
||||
@@ -336,6 +336,75 @@ def test_sanitize_preserves_distinct_tool_call_ids():
|
||||
assert sorted(m["tool_call_id"] for m in out if m.get("role") == "tool") == ["call_A", "call_B"]
|
||||
|
||||
|
||||
# ── tool_call_id reuse by local servers (#70724) ────────────────────────────
|
||||
# llama.cpp emits ONE constant tool_call_id for every tool call it returns, so
|
||||
# ``tool_call_id`` is not globally unique in practice. The #58327 dedup pass
|
||||
# must key off outstanding calls, not "seen at any point", or every result
|
||||
# after the first is deleted and the agent stops mid-task.
|
||||
|
||||
|
||||
CONSTANT_ID = "ZsSt4SkIFMRz0HtqT7MTlimNvzlKM896"
|
||||
|
||||
|
||||
def _call(cid, name="terminal"):
|
||||
return {"role": "assistant", "content": None,
|
||||
"tool_calls": [{"id": cid, "type": "function",
|
||||
"function": {"name": name, "arguments": "{}"}}]}
|
||||
|
||||
|
||||
def _result(cid, content):
|
||||
return {"role": "tool", "tool_call_id": cid, "name": "terminal",
|
||||
"content": content}
|
||||
|
||||
|
||||
def test_sanitize_keeps_results_when_server_reuses_one_tool_call_id():
|
||||
"""Every answered call survives even when all of them share one id.
|
||||
|
||||
Contract: a tool result is dropped for being unanswerable, never for
|
||||
reusing an id that an earlier call already retired.
|
||||
"""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
messages = [{"role": "user", "content": "do three steps"}]
|
||||
for i in range(3):
|
||||
messages.append(_call(CONSTANT_ID))
|
||||
messages.append(_result(CONSTANT_ID, f"step {i} output"))
|
||||
|
||||
out = sanitize_api_messages(list(messages))
|
||||
results = [m for m in out if m.get("role") == "tool"]
|
||||
assert [m["content"] for m in results] == [
|
||||
"step 0 output", "step 1 output", "step 2 output",
|
||||
]
|
||||
calls = [m for m in out if m.get("role") == "assistant" and m.get("tool_calls")]
|
||||
assert len(calls) == 3
|
||||
|
||||
|
||||
def test_sanitize_still_drops_replayed_result_for_retired_call():
|
||||
"""The #58327 protection holds: a second result for an already-answered
|
||||
call answers nothing outstanding and is still dropped."""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
messages = [
|
||||
{"role": "user", "content": "hi"},
|
||||
_call(CONSTANT_ID),
|
||||
_result(CONSTANT_ID, "real"),
|
||||
_result(CONSTANT_ID, "replayed by a retry/resume glitch"),
|
||||
]
|
||||
out = sanitize_api_messages(list(messages))
|
||||
assert [m["content"] for m in out if m.get("role") == "tool"] == ["real"]
|
||||
|
||||
|
||||
def test_sanitize_drops_result_with_no_preceding_call():
|
||||
"""A tool result that never had a call is an orphan regardless of id."""
|
||||
from agent.agent_runtime_helpers import sanitize_api_messages
|
||||
|
||||
out = sanitize_api_messages([
|
||||
{"role": "user", "content": "hi"},
|
||||
_result("id_never_requested", "orphan"),
|
||||
])
|
||||
assert [m for m in out if m.get("role") == "tool"] == []
|
||||
|
||||
|
||||
def test_sanitize_drops_empty_tool_calls_array():
|
||||
"""sanitize_api_messages strips ``tool_calls: []`` from assistant messages.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user