mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-04 08:28:36 +00:00
511c764f45
When an LLM response arrives with finish_reason='length' and has_tool_calls but blank text content (e.g. the model spent its whole output budget on a tool call whose closing tag was truncated), the runner dropped the tool calls and then misrouted the blank response into the empty-response retry branch. Retrying the same prompt cannot recover from output-budget exhaustion, so every retry hit the same length ceiling and the turn ended in the generic apology. The length-recovery branch was gated on 'finish_reason == length and not is_blank_text(clean)', so a blank-but-truncated turn could never reach it. - The empty-response retry branch now excludes finish_reason == 'length' (in addition to 'error'). - The length-recovery branch no longer requires non-blank content, so a blank-but-truncated turn enters recovery and appends build_length_recovery_message (which handles a blank tail safely). Adds a regression test asserting the length-recovery path is taken; it fails on the unfixed code and passes with the fix. Fixes #5133
360 lines
12 KiB
Python
360 lines
12 KiB
Python
"""Tests for AgentRunner error handling: tool errors, LLM errors,
|
|
session message isolation, and tool result preservation."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, MagicMock
|
|
|
|
import pytest
|
|
|
|
from agent.runner_helpers import make_run_spec
|
|
from nanobot.config.schema import AgentDefaults
|
|
from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest
|
|
|
|
_MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_runner_returns_structured_tool_error():
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
|
content="working",
|
|
tool_calls=[ToolCallRequest(id="call_1", name="list_dir", arguments={})],
|
|
))
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
tools.execute = AsyncMock(side_effect=RuntimeError("boom"))
|
|
|
|
runner = AgentRunner()
|
|
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=2,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
fail_on_tool_error=True,
|
|
))
|
|
|
|
assert result.stop_reason == "tool_error"
|
|
assert result.error == "Error: RuntimeError: boom"
|
|
assert result.tool_events == [
|
|
{"name": "list_dir", "status": "error", "detail": "boom"}
|
|
]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
@pytest.mark.parametrize("control_error", [KeyboardInterrupt, SystemExit])
|
|
async def test_runner_propagates_tool_control_flow_exceptions(control_error: type[BaseException]):
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
|
|
async def execute(_name, _args):
|
|
raise control_error("stop")
|
|
|
|
tools = SimpleNamespace(
|
|
get_definitions=lambda: [],
|
|
execute=execute,
|
|
)
|
|
runner = AgentRunner()
|
|
spec = make_run_spec(
|
|
provider,
|
|
initial_messages=[],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=1,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
)
|
|
|
|
with pytest.raises(control_error):
|
|
await runner._run_tool(
|
|
spec,
|
|
ToolCallRequest(id="call_1", name="list_dir", arguments={}),
|
|
external_lookup_counts={},
|
|
workspace_violation_counts={},
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_llm_error_not_appended_to_session_messages():
|
|
"""When LLM returns finish_reason='error', the error content must NOT be
|
|
appended to the messages list (prevents polluting session history)."""
|
|
from nanobot.agent.runner import (
|
|
_PERSISTED_MODEL_ERROR_PLACEHOLDER,
|
|
AgentRunner,
|
|
)
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
|
content="429 rate limit exceeded", finish_reason="error", tool_calls=[], usage={},
|
|
))
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "hello"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=5,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
))
|
|
|
|
assert result.stop_reason == "error"
|
|
assert result.final_content == "429 rate limit exceeded"
|
|
assistant_msgs = [m for m in result.messages if m.get("role") == "assistant"]
|
|
assert all("429" not in (m.get("content") or "") for m in assistant_msgs), \
|
|
"Error content should not appear in session messages"
|
|
assert assistant_msgs[-1]["content"] == _PERSISTED_MODEL_ERROR_PLACEHOLDER
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_llm_arrearage_error_surfaces_clear_message():
|
|
"""Arrearage errors yield a clear user-facing message, not a raw dump (#3006)."""
|
|
from nanobot.agent.runner import _ARREARAGE_ERROR_MESSAGE, AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
|
content="HTTP 402 insufficient_quota", finish_reason="error", error_status_code=402,
|
|
))
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "hello"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=5,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
))
|
|
|
|
assert result.stop_reason == "error"
|
|
assert result.final_content == _ARREARAGE_ERROR_MESSAGE
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
@pytest.mark.parametrize(
|
|
("finish_reason", "expected_stop_reason"),
|
|
[
|
|
("refusal", "completed"),
|
|
("content_filter", "completed"),
|
|
("error", "error"),
|
|
],
|
|
)
|
|
async def test_runner_ignores_tool_calls_when_finish_reason_blocks_execution(
|
|
finish_reason: str,
|
|
expected_stop_reason: str,
|
|
):
|
|
"""Provider/gateway-injected tool calls under terminal block reasons must not run."""
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(
|
|
content="Request blocked by provider policy.",
|
|
finish_reason=finish_reason,
|
|
tool_calls=[ToolCallRequest(id="call_1", name="exec", arguments={"command": "echo nope"})],
|
|
usage={},
|
|
))
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
tools.execute = AsyncMock(return_value="should not run")
|
|
|
|
result = await AgentRunner().run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "run a command"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=2,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
))
|
|
|
|
tools.execute.assert_not_awaited()
|
|
assert result.stop_reason == expected_stop_reason
|
|
assert result.tools_used == []
|
|
assert result.final_content == "Request blocked by provider policy."
|
|
assert not any(msg.get("role") == "tool" for msg in result.messages)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_runner_tool_error_sets_final_content():
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
|
|
async def chat_with_retry(*, messages, **kwargs):
|
|
return LLMResponse(
|
|
content="working",
|
|
tool_calls=[ToolCallRequest(id="call_1", name="read_file", arguments={"path": "x"})],
|
|
usage={},
|
|
)
|
|
|
|
provider.chat_with_retry = chat_with_retry
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
tools.execute = AsyncMock(side_effect=RuntimeError("boom"))
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "do task"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=1,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
fail_on_tool_error=True,
|
|
))
|
|
|
|
assert result.final_content == "Error: RuntimeError: boom"
|
|
assert result.stop_reason == "tool_error"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_runner_preserves_successful_exec_output_that_starts_with_error():
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
|
|
async def chat_with_retry(*, messages, **kwargs):
|
|
if not any(msg.get("role") == "tool" for msg in messages):
|
|
return LLMResponse(
|
|
content="working",
|
|
tool_calls=[
|
|
ToolCallRequest(id="call_1", name="exec", arguments={"command": "report"})
|
|
],
|
|
usage={},
|
|
)
|
|
return LLMResponse(content="done", usage={})
|
|
|
|
provider.chat_with_retry = chat_with_retry
|
|
output = "Error: generated report successfully\n\nExit code: 0"
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
tools.execute = AsyncMock(return_value=output)
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "run report"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=2,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
fail_on_tool_error=True,
|
|
))
|
|
|
|
assert result.final_content == "done"
|
|
assert result.stop_reason == "completed"
|
|
assert result.tool_events == [
|
|
{"name": "exec", "status": "ok", "detail": "Error: generated report successfully Exit code: 0"}
|
|
]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_runner_tool_error_preserves_tool_results_in_messages():
|
|
"""When a tool raises a fatal error, its results must still be appended
|
|
to messages so the session never contains orphan tool_calls (#2943)."""
|
|
from nanobot.agent.runner import AgentRunner
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
|
|
async def chat_with_retry(*, messages, **kwargs):
|
|
return LLMResponse(
|
|
content=None,
|
|
tool_calls=[
|
|
ToolCallRequest(id="tc1", name="read_file", arguments={"path": "a"}),
|
|
ToolCallRequest(id="tc2", name="exec", arguments={"cmd": "bad"}),
|
|
],
|
|
usage={},
|
|
)
|
|
|
|
provider.chat_with_retry = chat_with_retry
|
|
provider.chat_stream_with_retry = chat_with_retry
|
|
|
|
call_idx = 0
|
|
|
|
async def fake_execute(name, args, **kw):
|
|
nonlocal call_idx
|
|
call_idx += 1
|
|
if call_idx == 2:
|
|
raise RuntimeError("boom")
|
|
return "file content"
|
|
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
tools.execute = AsyncMock(side_effect=fake_execute)
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "do stuff"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=1,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
fail_on_tool_error=True,
|
|
))
|
|
|
|
assert result.stop_reason == "tool_error"
|
|
# Both tool results must be in messages even though tc2 had a fatal error.
|
|
tool_msgs = [m for m in result.messages if m.get("role") == "tool"]
|
|
assert len(tool_msgs) == 2
|
|
assert tool_msgs[0]["tool_call_id"] == "tc1"
|
|
assert tool_msgs[1]["tool_call_id"] == "tc2"
|
|
# The assistant message with tool_calls must precede the tool results.
|
|
asst_tc_idx = next(
|
|
i for i, m in enumerate(result.messages)
|
|
if m.get("role") == "assistant" and m.get("tool_calls")
|
|
)
|
|
tool_indices = [
|
|
i for i, m in enumerate(result.messages) if m.get("role") == "tool"
|
|
]
|
|
assert all(ti > asst_tc_idx for ti in tool_indices)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_length_finish_with_blank_content_routes_to_length_recovery():
|
|
"""Regression test for #5133.
|
|
|
|
A response with finish_reason='length' and blank content (e.g. the model
|
|
spent its whole output budget on a tool call whose closing tag was
|
|
truncated) must take the length-recovery path, not the empty-response
|
|
retry path. Retrying the same prompt cannot recover from output-budget
|
|
exhaustion.
|
|
"""
|
|
from nanobot.agent.runner import AgentRunner
|
|
from nanobot.utils.runtime import LENGTH_RECOVERY_PROMPT
|
|
|
|
provider = MagicMock(spec=LLMProvider)
|
|
# First call: truncated (length) with blank content and a dropped tool call.
|
|
# Second call: normal completion so the loop can terminate.
|
|
provider.chat_with_retry = AsyncMock(side_effect=[
|
|
LLMResponse(
|
|
content="",
|
|
finish_reason="length",
|
|
tool_calls=[ToolCallRequest(id="call_1", name="exec", arguments={})],
|
|
usage={},
|
|
),
|
|
LLMResponse(content="done", finish_reason="stop", tool_calls=[], usage={}),
|
|
])
|
|
tools = MagicMock()
|
|
tools.get_definitions.return_value = []
|
|
|
|
runner = AgentRunner()
|
|
result = await runner.run(make_run_spec(provider,
|
|
initial_messages=[{"role": "user", "content": "do a long task"}],
|
|
tools=tools,
|
|
model="test-model",
|
|
max_iterations=5,
|
|
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
|
))
|
|
|
|
# The runner must have injected a length-recovery prompt and continued,
|
|
# rather than exhausting empty-response retries into a generic apology.
|
|
user_msgs = [m.get("content") or "" for m in result.messages if m.get("role") == "user"]
|
|
assert any(LENGTH_RECOVERY_PROMPT in c for c in user_msgs), (
|
|
"expected a length-recovery message to be appended for a "
|
|
"finish_reason='length' response with blank content"
|
|
)
|
|
assert result.final_content == "done"
|