fix: address PR review comments - rename _dedup to _dedupe and fix ID storage consistency
This commit is contained in:
@@ -373,7 +373,7 @@ class AgentRunner:
|
|||||||
# may repair or compact historical messages for the model, but
|
# may repair or compact historical messages for the model, but
|
||||||
# those synthetic edits must not shift the append boundary used
|
# those synthetic edits must not shift the append boundary used
|
||||||
# later when the caller saves only the new turn.
|
# later when the caller saves only the new turn.
|
||||||
messages_for_model = self._dedup_tool_calls(messages)
|
messages_for_model = self._dedupe_tool_calls(messages)
|
||||||
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
||||||
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
||||||
messages_for_model = self._microcompact(messages_for_model)
|
messages_for_model = self._microcompact(messages_for_model)
|
||||||
@@ -389,7 +389,7 @@ class AgentRunner:
|
|||||||
spec.session_key or "default",
|
spec.session_key or "default",
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
messages_for_model = self._dedup_tool_calls(messages)
|
messages_for_model = self._dedupe_tool_calls(messages)
|
||||||
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
messages_for_model = self._drop_orphan_tool_results(messages_for_model)
|
||||||
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
messages_for_model = self._backfill_missing_tool_results(messages_for_model)
|
||||||
except Exception:
|
except Exception:
|
||||||
@@ -1358,7 +1358,7 @@ class AgentRunner:
|
|||||||
return content
|
return content
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _dedup_tool_calls(
|
def _dedupe_tool_calls(
|
||||||
messages: list[dict[str, Any]],
|
messages: list[dict[str, Any]],
|
||||||
) -> list[dict[str, Any]]:
|
) -> list[dict[str, Any]]:
|
||||||
"""Remove duplicate tool_call / tool_result ids from the history.
|
"""Remove duplicate tool_call / tool_result ids from the history.
|
||||||
@@ -1383,11 +1383,11 @@ class AgentRunner:
|
|||||||
changed = False
|
changed = False
|
||||||
for tc in msg.get("tool_calls") or []:
|
for tc in msg.get("tool_calls") or []:
|
||||||
tid = tc.get("id") if isinstance(tc, dict) else None
|
tid = tc.get("id") if isinstance(tc, dict) else None
|
||||||
if tid and tid in seen_call_ids:
|
if tid and str(tid) in seen_call_ids:
|
||||||
changed = True
|
changed = True
|
||||||
continue
|
continue
|
||||||
if tid:
|
if tid:
|
||||||
seen_call_ids.add(tid)
|
seen_call_ids.add(str(tid))
|
||||||
kept.append(tc)
|
kept.append(tc)
|
||||||
if changed:
|
if changed:
|
||||||
replacement = dict(msg)
|
replacement = dict(msg)
|
||||||
|
|||||||
@@ -184,7 +184,7 @@ async def test_backfill_missing_tool_results_inserts_error():
|
|||||||
assert backfilled[0]["name"] == "read_file"
|
assert backfilled[0]["name"] == "read_file"
|
||||||
|
|
||||||
|
|
||||||
def test_dedup_tool_calls_removes_duplicate_ids():
|
def test_dedupe_tool_calls_removes_duplicate_ids():
|
||||||
from nanobot.agent.runner import AgentRunner
|
from nanobot.agent.runner import AgentRunner
|
||||||
|
|
||||||
messages = [
|
messages = [
|
||||||
@@ -205,7 +205,7 @@ def test_dedup_tool_calls_removes_duplicate_ids():
|
|||||||
{"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-dup"},
|
{"role": "tool", "tool_call_id": "b", "name": "y", "content": "rb-dup"},
|
||||||
]
|
]
|
||||||
|
|
||||||
cleaned = AgentRunner._dedup_tool_calls(messages)
|
cleaned = AgentRunner._dedupe_tool_calls(messages)
|
||||||
|
|
||||||
assert cleaned == [
|
assert cleaned == [
|
||||||
{"role": "user", "content": "hi"},
|
{"role": "user", "content": "hi"},
|
||||||
@@ -222,7 +222,7 @@ def test_dedup_tool_calls_removes_duplicate_ids():
|
|||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
def test_dedup_tool_calls_noop_when_unique():
|
def test_dedupe_tool_calls_noop_when_unique():
|
||||||
from nanobot.agent.runner import AgentRunner
|
from nanobot.agent.runner import AgentRunner
|
||||||
|
|
||||||
messages = [
|
messages = [
|
||||||
@@ -238,7 +238,7 @@ def test_dedup_tool_calls_noop_when_unique():
|
|||||||
]
|
]
|
||||||
|
|
||||||
# No duplicates -> identical list object returned (cheap no-op path).
|
# No duplicates -> identical list object returned (cheap no-op path).
|
||||||
assert AgentRunner._dedup_tool_calls(messages) is messages
|
assert AgentRunner._dedupe_tool_calls(messages) is messages
|
||||||
|
|
||||||
|
|
||||||
def test_drop_orphan_tool_results_removes_unmatched_tool_messages():
|
def test_drop_orphan_tool_results_removes_unmatched_tool_messages():
|
||||||
|
|||||||
Reference in New Issue
Block a user