From 61bb76281aac901719ed87e820731b7e3ba5bbc7 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 26 May 2026 00:47:02 +0000 Subject: [PATCH] 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. --- .../core/inference/context_compaction.py | 38 ++++++++++--------- .../tests/test_context_compaction_edge.py | 24 ++++++++++++ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/studio/backend/core/inference/context_compaction.py b/studio/backend/core/inference/context_compaction.py index 96d091d2a3..4572e702b4 100644 --- a/studio/backend/core/inference/context_compaction.py +++ b/studio/backend/core/inference/context_compaction.py @@ -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): diff --git a/studio/backend/tests/test_context_compaction_edge.py b/studio/backend/tests/test_context_compaction_edge.py index e7b367573b..601c96b544 100644 --- a/studio/backend/tests/test_context_compaction_edge.py +++ b/studio/backend/tests/test_context_compaction_edge.py @@ -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)