Recompute responded_ids after orphan-tool drop for PR #5710
Before, the final sweep computed `responded_ids` once before pruning orphan tools, so a tool message arriving before its assistant got dropped as orphan but the later assistant still matched `ids <= responded_ids` and kept its dangling `tool_calls` entry, violating the chat-template invariant the sweep exists to enforce. Split the sweep into two passes: drop orphan tools first, then recompute `responded_ids` from the surviving tools and mark assts whose tool_calls aren't all responded for strip.
This commit is contained in:
parent
ad6a34502e
commit
61bb76281a
2 changed files with 44 additions and 18 deletions
|
|
@ -314,18 +314,26 @@ class SlidingWindowCompact(CompactStrategy):
|
|||
else:
|
||||
rewrite_strip_tool_calls.add(asst_idx)
|
||||
|
||||
# Final invariant sweep: drop any surviving tool message whose
|
||||
# tool_call_id has no matching assistant ``tool_calls`` earlier
|
||||
# in the kept output. This catches orphans pair_map could not
|
||||
# link -- e.g. a tool that arrives after a user boundary and
|
||||
# references an assistant that the boundary-clearing logic in
|
||||
# ``_pair_linked_indices`` no longer treats as the pair root.
|
||||
# Anchored tool messages (multimodal content) stay regardless,
|
||||
# matching the existing leak-rather-than-violate-anchor rule.
|
||||
# Also collect assistant indices whose surviving tool_calls
|
||||
# entries lack a matching tool follow-up so we can strip the
|
||||
# orphan ids from a copy below (same reason: OpenAI 400s on
|
||||
# tool_calls without matching tool responses).
|
||||
# Final invariant sweep, two passes so the asst-strip decision
|
||||
# sees the post-orphan-drop tool set:
|
||||
# pass 1: drop tools whose tcid has no matching assistant
|
||||
# ``tool_calls`` earlier in the kept output (anchored tools
|
||||
# stay; same leak-rather-than-violate-anchor rule);
|
||||
# pass 2: recompute responded_ids from the surviving tools and
|
||||
# mark assts whose tool_calls aren't all responded for strip.
|
||||
# Computing responded_ids before pass 1 would let a stale tcid
|
||||
# of a just-dropped orphan tool satisfy `ids <= responded_ids`,
|
||||
# leaving the asst with dangling tool_calls (OpenAI 400).
|
||||
seen_ids: set[str] = set()
|
||||
for i, m in enumerate(messages):
|
||||
if i in dropped:
|
||||
continue
|
||||
if m.get("role") == "assistant":
|
||||
seen_ids |= _assistant_tool_call_ids(m)
|
||||
elif m.get("role") == "tool" and i not in anchor_idx:
|
||||
tcid = m.get("tool_call_id")
|
||||
if isinstance(tcid, str) and tcid and tcid not in seen_ids:
|
||||
dropped.add(i)
|
||||
responded_ids: set[str] = set()
|
||||
for i, m in enumerate(messages):
|
||||
if i in dropped:
|
||||
|
|
@ -334,19 +342,13 @@ class SlidingWindowCompact(CompactStrategy):
|
|||
tcid = m.get("tool_call_id")
|
||||
if isinstance(tcid, str) and tcid:
|
||||
responded_ids.add(tcid)
|
||||
seen_ids: set[str] = set()
|
||||
for i, m in enumerate(messages):
|
||||
if i in dropped:
|
||||
continue
|
||||
if m.get("role") == "assistant":
|
||||
ids = _assistant_tool_call_ids(m)
|
||||
seen_ids |= ids
|
||||
if ids and not (ids <= responded_ids):
|
||||
rewrite_strip_tool_calls.add(i)
|
||||
elif m.get("role") == "tool" and i not in anchor_idx:
|
||||
tcid = m.get("tool_call_id")
|
||||
if isinstance(tcid, str) and tcid and tcid not in seen_ids:
|
||||
dropped.add(i)
|
||||
|
||||
out: list[dict] = []
|
||||
for i, m in enumerate(messages):
|
||||
|
|
|
|||
|
|
@ -883,3 +883,27 @@ def test_partial_tool_drop_strips_orphan_tool_call_id():
|
|||
out = SlidingWindowCompact(keep_recent = 1).compact(msgs, budget_tokens = 200)
|
||||
asst_ids, tool_ids = _surviving_tool_ids(out)
|
||||
assert asst_ids == tool_ids, (asst_ids, tool_ids)
|
||||
|
||||
|
||||
def test_orphan_tool_before_asst_does_not_leave_dangling_tool_calls():
|
||||
"""Malformed input: a tool message arrives BEFORE the assistant
|
||||
that references its id. The final sweep must drop the orphan tool
|
||||
AND strip the now-dangling tool_call from the later assistant so
|
||||
the chat template stays valid. Earlier behavior computed
|
||||
responded_ids once before the orphan-tool drop, so the post-drop
|
||||
asst still matched ids <= responded_ids and kept its tool_calls.
|
||||
"""
|
||||
msgs = [
|
||||
_msg("system", "sys"),
|
||||
_msg("user", "first task"),
|
||||
_msg("tool", content = "early", tool_call_id = "call_a"),
|
||||
_msg(
|
||||
"assistant",
|
||||
content = "thinking",
|
||||
tool_calls = [_tool_call("call_a")],
|
||||
),
|
||||
_long("user", 50),
|
||||
]
|
||||
out = SlidingWindowCompact(keep_recent = 5).compact(msgs, budget_tokens = 1000)
|
||||
asst_ids, tool_ids = _surviving_tool_ids(out)
|
||||
assert asst_ids == tool_ids, (asst_ids, tool_ids)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue