mirror/nanobot
1
0
Fork 0
mirror of https://github.com/HKUDS/nanobot.git synced 2026-06-15 07:14:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

style: trim orphan toolcall comments

Browse Source 
maintainer edit: keep the invariant comments in production code but remove repeated issue background from tests.
This commit is contained in:
chengyongru 2026-06-12 11:36:18 +08:00 committed by Xubin Ren
parent 33e6da14d8
commit d72d0102d9
4 changed files with 5 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
nanobot/agent/loop.py
View File

@ -1593,9 +1593,7 @@ class AgentLoop:
if role == "tool":
tool_call_id = entry.get("tool_call_id")
if not tool_call_id or str(tool_call_id) not in declared_tool_call_ids:
# A tool result without a declared call violates the
# OpenAI/Anthropic pairing contract and would poison
# every future request built from this session (#4006).
# Undeclared tool results corrupt future provider requests.
logger.warning(
"Dropping orphaned tool result {} from session {} during persistence",
tool_call_id or "(missing id)",
@ -1607,8 +1605,7 @@ class AgentLoop:
elif isinstance(content, list):
filtered = self._sanitize_persisted_blocks(content, should_truncate_text=True)
if not filtered:
# Dropping the message would leave its assistant
# tool_call without a result; keep a placeholder.
# Preserve the tool_call/result pair after block filtering.
filtered = [
{"type": "text", "text": "[tool result omitted during persistence]"}
]

7
nanobot/session/turn_continuation.py
View File

@ -182,11 +182,8 @@ def _save_skip_for_turn(
"""Return the persisted-message append boundary for this turn."""
if internal_continuation_inbound(message_metadata):
return initial_message_count
# ``build_messages`` merges the current message into the last history
# entry when both share a role, so the prompt prefix is not always
# ``2 + history_count``. Runner-appended messages always start at
# ``initial_message_count``; only step back when the current message
# exists as a standalone entry that was not persisted early.
# build_messages may merge the current message into a same-role history tail.
# Runner-appended messages start at initial_message_count in either shape.
has_standalone_current = initial_message_count > 1 + history_count
if has_standalone_current and not user_persisted_early:
return initial_message_count - 1

13
tests/agent/test_loop_save_turn.py
View File

@ -1293,11 +1293,6 @@ async def test_system_subagent_followup_uses_thread_session_and_slack_metadata(t
@pytest.mark.asyncio
async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path) -> None:
"""Regression for #4006: when history ends with a user message,
``build_messages`` merges the new user message into it, shrinking the
prompt prefix by one. The save boundary must follow, or the first
new-turn assistant message (carrying tool_calls) is cut from
persistence and its tool results are orphaned."""
loop = _make_full_loop(tmp_path)
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign]
@ -1306,8 +1301,6 @@ async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path
loop.sessions.save(session)
async def fake_run_agent_loop(initial_messages, **_kwargs):
# The merge branch collapsed the current user message into the
# history tail: the prompt prefix is [system, merged-user].
assert [m["role"] for m in initial_messages] == ["system", "user"]
return (
"done",
@ -1359,8 +1352,6 @@ async def test_turn_after_unanswered_user_keeps_tool_call_pairing(tmp_path: Path
def test_save_turn_keeps_placeholder_for_empty_tool_result_blocks() -> None:
# Dropping the whole tool message would leave the assistant tool_call
# without a result, which strict APIs reject as firmly as orphans.
loop = _mk_loop()
session = Session(key="test:empty-tool-blocks")
@ -1388,8 +1379,6 @@ def test_save_turn_keeps_placeholder_for_empty_tool_result_blocks() -> None:
def test_save_turn_drops_orphaned_tool_results() -> None:
# Defense in depth for #4006: whatever upstream bug produces a tool
# result whose call was never declared, it must not reach history.
loop = _mk_loop()
session = Session(key="test:orphan-guard")
session.add_message("user", "hi")
@ -1424,8 +1413,6 @@ def test_save_turn_drops_tool_results_without_tool_call_id() -> None:
def test_save_turn_keeps_tool_results_declared_in_prior_history() -> None:
# Declarations may live in already-persisted history (e.g. a restored
# runtime checkpoint), not only in the new-turn slice.
loop = _mk_loop()
session = Session(key="test:prior-declared")
session.add_message(

8
tests/session/test_turn_continuation.py
View File

@ -142,11 +142,6 @@ def test_internal_continuation_requires_budget_boundary_and_queue():
def test_save_skip_matches_prefix_when_current_message_merged():
# ``build_messages`` merges the current user message into a same-role
# history tail, so the prompt prefix is ``1 + history_count`` instead of
# ``2 + history_count``. The old boundary then cut the first new-turn
# assistant message (carrying tool_calls) from persistence, leaving its
# tool results orphaned in session history (#4006).
skip = _save_skip_for_turn(
message_metadata=None,
initial_message_count=2, # [system, merged user]
@ -157,14 +152,13 @@ def test_save_skip_matches_prefix_when_current_message_merged():
def test_save_skip_unchanged_for_standalone_current_message():
# [system, history user, current user] — early-persisted current message.
# [system, history user, current user] with the current user already saved.
assert _save_skip_for_turn(
message_metadata=None,
initial_message_count=3,
history_count=1,
user_persisted_early=True,
) == 3
# Same prefix, current message not persisted early: re-save it.
assert _save_skip_for_turn(
message_metadata=None,
initial_message_count=3,