fix(compression): strip whitespace from tool_call_id in _sanitize_tool_pairs
_sanitize_tool_pairs() in ContextCompressor compared raw tool_call_id strings without stripping whitespace, the same bugfa3ab2ffdjust fixed in agent_runtime_helpers.py / run_agent.py (_get_tool_call_id_static + sanitize_api_messages). ContextCompressor has its own near-identical reimplementation of the pair-repair logic that was left unpatched. When assistant-side and result-side IDs diverge only in surrounding whitespace, the compressor misclassifies valid results as orphaned and replaces them with [Result unavailable] stubs — silent data loss on every compression cycle that touches such pairs. Apply the same .strip() fix to all three sites: - _get_tool_call_id (extracts IDs from assistant tool_calls) - result_call_ids accumulation loop - orphaned_results filter predicate Closes the sibling gap offa3ab2ffd/ #42405.
This commit is contained in:
@@ -6045,8 +6045,8 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
SimpleNamespace), for logging/display only. Matching logic must use
|
||||
:meth:`_tool_call_id_variants` instead — see its docstring."""
|
||||
if isinstance(tc, dict):
|
||||
return tc.get("call_id", "") or tc.get("id", "") or ""
|
||||
return getattr(tc, "call_id", "") or getattr(tc, "id", "") or ""
|
||||
return (tc.get("call_id", "") or tc.get("id", "") or "").strip()
|
||||
return (getattr(tc, "call_id", "") or getattr(tc, "id", "") or "").strip()
|
||||
|
||||
@staticmethod
|
||||
def _tool_call_id_variants(tc) -> set:
|
||||
@@ -6092,7 +6092,7 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
result_call_ids: set = set()
|
||||
for msg in messages:
|
||||
if msg.get("role") == "tool":
|
||||
cid = msg.get("tool_call_id")
|
||||
cid = (msg.get("tool_call_id") or "").strip()
|
||||
if cid:
|
||||
# Expand alias spellings on the RESULT side too — a
|
||||
# composite ``call|item`` tool_call_id must match a
|
||||
|
||||
@@ -3619,3 +3619,60 @@ class TestPreLlmFeasibilityCheck:
|
||||
feasibility_skip=compressor._last_feasibility_skip,
|
||||
)
|
||||
assert compressor._fallback_compression_streak == 1
|
||||
|
||||
|
||||
class TestSanitizeToolPairsWhitespace:
|
||||
"""_sanitize_tool_pairs must strip whitespace from tool_call_id before
|
||||
comparing, matching the fix applied to agent_runtime_helpers.py in
|
||||
commit fa3ab2ffd. Without stripping, a valid tool result whose
|
||||
tool_call_id has surrounding whitespace is misclassified as orphaned
|
||||
and silently replaced with a [Result unavailable] stub.
|
||||
"""
|
||||
|
||||
def _make(self):
|
||||
with patch("agent.context_compressor.get_model_context_length", return_value=100000):
|
||||
return ContextCompressor(model="test/model", quiet_mode=True,
|
||||
protect_first_n=2, protect_last_n=2)
|
||||
|
||||
def _assistant(self, call_id):
|
||||
return {
|
||||
"role": "assistant", "content": "",
|
||||
"tool_calls": [{"id": call_id, "type": "function",
|
||||
"function": {"name": "f", "arguments": "{}"}}],
|
||||
}
|
||||
|
||||
def test_leading_whitespace_on_result_id_preserved(self):
|
||||
c = self._make()
|
||||
msgs = [
|
||||
self._assistant("call_abc"),
|
||||
{"role": "tool", "tool_call_id": " call_abc", "content": "ok"},
|
||||
]
|
||||
out = c._sanitize_tool_pairs(msgs)
|
||||
tool_msgs = [m for m in out if m.get("role") == "tool"]
|
||||
assert len(tool_msgs) == 1
|
||||
assert tool_msgs[0]["content"] == "ok", "valid result must not be treated as orphaned"
|
||||
|
||||
def test_trailing_whitespace_on_result_id_preserved(self):
|
||||
c = self._make()
|
||||
msgs = [
|
||||
self._assistant("call_xyz"),
|
||||
{"role": "tool", "tool_call_id": "call_xyz ", "content": "data"},
|
||||
]
|
||||
out = c._sanitize_tool_pairs(msgs)
|
||||
tool_msgs = [m for m in out if m.get("role") == "tool"]
|
||||
assert len(tool_msgs) == 1
|
||||
assert tool_msgs[0]["content"] == "data"
|
||||
|
||||
def test_truly_orphaned_still_removed(self):
|
||||
"""Whitespace-trimmed ID that still has no match must be removed.
|
||||
The assistant's call_real has no matching result, so a stub is
|
||||
inserted in its place — the original orphaned entry must be gone."""
|
||||
c = self._make()
|
||||
msgs = [
|
||||
self._assistant("call_real"),
|
||||
{"role": "tool", "tool_call_id": " call_orphan ", "content": "stale"},
|
||||
]
|
||||
out = c._sanitize_tool_pairs(msgs)
|
||||
tool_call_ids = [m.get("tool_call_id") for m in out if m.get("role") == "tool"]
|
||||
assert "call_orphan" not in tool_call_ids, "genuinely orphaned result must be removed"
|
||||
assert " call_orphan " not in tool_call_ids, "original whitespace form must also be gone"
|
||||
|
||||
Reference in New Issue
Block a user