merge: resolve conflict with main in transcription.py
Keep _post_transcription_with_retry from PR branch, drop inline httpx calls that were replaced by the shared retry helper.
This commit is contained in:
@@ -130,11 +130,44 @@ class TestToolEventProgress:
|
||||
assert finish["result"] == "file.txt"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bus_progress_streams_provider_deltas_for_codex_style_provider(
|
||||
async def test_non_streaming_channel_does_not_publish_codex_progress_deltas(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Providers that opt in can stream content deltas through _progress messages."""
|
||||
"""Non-streaming channels should get one final reply, not token progress spam."""
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
provider.get_default_model.return_value = "openai-codex/gpt-5.5"
|
||||
provider.chat_with_retry = AsyncMock(return_value=LLMResponse(content="Hello", tool_calls=[]))
|
||||
provider.chat_stream_with_retry = AsyncMock()
|
||||
loop = AgentLoop(bus=bus, provider=provider, workspace=tmp_path, model="openai-codex/gpt-5.5")
|
||||
loop.tools.get_definitions = MagicMock(return_value=[])
|
||||
loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign]
|
||||
|
||||
await loop._dispatch(InboundMessage(
|
||||
channel="whatsapp",
|
||||
sender_id="u1",
|
||||
chat_id="chat1",
|
||||
content="say hello",
|
||||
))
|
||||
|
||||
outbound = []
|
||||
while bus.outbound_size > 0:
|
||||
outbound.append(await bus.consume_outbound())
|
||||
|
||||
assert [m.content for m in outbound] == ["Hello"]
|
||||
assert not any(m.metadata.get("_progress") for m in outbound)
|
||||
assert not any(m.metadata.get("_streamed") for m in outbound)
|
||||
provider.chat_stream_with_retry.assert_not_awaited()
|
||||
provider.chat_with_retry.assert_awaited_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_streaming_channel_streams_provider_deltas_for_codex_style_provider(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Streaming channels still receive provider deltas through _stream_delta messages."""
|
||||
bus = MessageBus()
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
@@ -156,18 +189,27 @@ class TestToolEventProgress:
|
||||
sender_id="u1",
|
||||
chat_id="chat1",
|
||||
content="say hello",
|
||||
metadata={"_wants_stream": True},
|
||||
))
|
||||
|
||||
outbound = []
|
||||
while bus.outbound_size > 0:
|
||||
outbound.append(await bus.consume_outbound())
|
||||
|
||||
progress = [m for m in outbound if m.metadata.get("_progress")]
|
||||
final = [m for m in outbound if not m.metadata.get("_progress")]
|
||||
deltas = [m for m in outbound if m.metadata.get("_stream_delta")]
|
||||
stream_end = [m for m in outbound if m.metadata.get("_stream_end")]
|
||||
final = [
|
||||
m for m in outbound
|
||||
if not m.metadata.get("_stream_delta")
|
||||
and not m.metadata.get("_stream_end")
|
||||
and not m.metadata.get("_turn_end")
|
||||
]
|
||||
|
||||
assert [m.content for m in progress] == ["Hel", "lo"]
|
||||
assert final[-2].content == "Hello"
|
||||
assert (final[-1].metadata or {}).get("_turn_end") is True
|
||||
assert [m.content for m in deltas] == ["Hel", "lo"]
|
||||
assert len(stream_end) == 1
|
||||
assert final[-1].content == "Hello"
|
||||
assert final[-1].metadata.get("_streamed") is True
|
||||
assert outbound[-1].metadata.get("_turn_end") is True
|
||||
provider.chat_with_retry.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -197,8 +239,12 @@ class TestToolEventProgress:
|
||||
loop.tools.prepare_call = MagicMock(return_value=(None, {"path": "foo.txt"}, None))
|
||||
loop.tools.execute = AsyncMock(return_value="ok")
|
||||
|
||||
streamed: list[str] = []
|
||||
progress: list[tuple[str, bool, list[dict] | None]] = []
|
||||
|
||||
async def on_stream(delta: str) -> None:
|
||||
streamed.append(delta)
|
||||
|
||||
async def on_progress(
|
||||
content: str,
|
||||
*,
|
||||
@@ -207,14 +253,15 @@ class TestToolEventProgress:
|
||||
) -> None:
|
||||
progress.append((content, tool_hint, tool_events))
|
||||
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop([], on_progress=on_progress)
|
||||
final_content, _, _, _, _ = await loop._run_agent_loop(
|
||||
[],
|
||||
on_progress=on_progress,
|
||||
on_stream=on_stream,
|
||||
)
|
||||
|
||||
assert final_content == "Done"
|
||||
assert [item[0] for item in progress[:3]] == [
|
||||
"I will",
|
||||
" inspect it.",
|
||||
'custom_tool("foo.txt")',
|
||||
]
|
||||
assert streamed == ["I will", " inspect it."]
|
||||
assert progress[0][0] == 'custom_tool("foo.txt")'
|
||||
assert all(item[0] != "I will inspect it." for item in progress)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -643,7 +643,7 @@ def test_persist_tool_result_logs_cleanup_failures(monkeypatch, tmp_path):
|
||||
lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("busy")),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.utils.helpers.logger.warning",
|
||||
"nanobot.utils.helpers.logger.exception",
|
||||
lambda message, *args: warnings.append(message.format(*args)),
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
"""Tests for provider progress delta routing in the shared runner."""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from nanobot.agent.runner import AgentRunner, AgentRunSpec
|
||||
from nanobot.config.schema import AgentDefaults
|
||||
from nanobot.providers.base import LLMResponse
|
||||
|
||||
_MAX_TOOL_RESULT_CHARS = AgentDefaults().max_tool_result_chars
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_can_disable_provider_progress_delta_streaming():
|
||||
"""AgentLoop disables token progress streaming for non-streaming channels."""
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
provider.chat_with_retry = AsyncMock(
|
||||
return_value=LLMResponse(content="done", tool_calls=[], usage={})
|
||||
)
|
||||
provider.chat_stream_with_retry = AsyncMock()
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
progress_cb = AsyncMock()
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[
|
||||
{"role": "system", "content": "system"},
|
||||
{"role": "user", "content": "hi"},
|
||||
],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=1,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
progress_callback=progress_cb,
|
||||
stream_progress_deltas=False,
|
||||
))
|
||||
|
||||
assert result.final_content == "done"
|
||||
provider.chat_with_retry.assert_awaited_once()
|
||||
provider.chat_stream_with_retry.assert_not_awaited()
|
||||
progress_cb.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_streams_provider_progress_deltas_by_default():
|
||||
"""Direct runner users keep the existing opt-in provider progress behavior."""
|
||||
provider = MagicMock()
|
||||
provider.supports_progress_deltas = True
|
||||
|
||||
async def chat_stream_with_retry(*, on_content_delta, **kwargs):
|
||||
await on_content_delta("he")
|
||||
await on_content_delta("llo")
|
||||
return LLMResponse(content="hello", tool_calls=[], usage={})
|
||||
|
||||
provider.chat_stream_with_retry = chat_stream_with_retry
|
||||
provider.chat_with_retry = AsyncMock()
|
||||
tools = MagicMock()
|
||||
tools.get_definitions.return_value = []
|
||||
progress_cb = AsyncMock()
|
||||
|
||||
runner = AgentRunner(provider)
|
||||
result = await runner.run(AgentRunSpec(
|
||||
initial_messages=[
|
||||
{"role": "system", "content": "system"},
|
||||
{"role": "user", "content": "hi"},
|
||||
],
|
||||
tools=tools,
|
||||
model="test-model",
|
||||
max_iterations=1,
|
||||
max_tool_result_chars=_MAX_TOOL_RESULT_CHARS,
|
||||
progress_callback=progress_cb,
|
||||
))
|
||||
|
||||
assert result.final_content == "hello"
|
||||
assert [call.args[0] for call in progress_cb.await_args_list] == ["he", "llo"]
|
||||
provider.chat_with_retry.assert_not_awaited()
|
||||
@@ -8,9 +8,9 @@ def _tc(name: str, args) -> ToolCallRequest:
|
||||
return ToolCallRequest(id="c1", name=name, arguments=args)
|
||||
|
||||
|
||||
def _hint(calls):
|
||||
def _hint(calls, max_length=40):
|
||||
"""Shortcut for format_tool_hints."""
|
||||
return format_tool_hints(calls)
|
||||
return format_tool_hints(calls, max_length=max_length)
|
||||
|
||||
|
||||
class TestToolHintKnownTools:
|
||||
@@ -254,3 +254,59 @@ class TestToolHintMixedFolding:
|
||||
assert "\u00d7" not in result
|
||||
parts = result.split(", ")
|
||||
assert len(parts) == 5
|
||||
|
||||
|
||||
class TestToolHintMaxLength:
|
||||
"""Test max_length parameter controls truncation of tool hints."""
|
||||
|
||||
def test_exec_default_truncates_at_40(self):
|
||||
cmd = "cd /very/long/path/to/some/project && npm run build && npm test"
|
||||
result = _hint([_tc("exec", {"command": cmd})], max_length=40)
|
||||
assert len(result) <= 50 # "$ " prefix + 40 + ellipsis
|
||||
assert "\u2026" in result
|
||||
|
||||
def test_exec_larger_max_length_shows_more(self):
|
||||
cmd = "cd /very/long/path/to/some/project && npm run build && npm test"
|
||||
short = _hint([_tc("exec", {"command": cmd})], max_length=40)
|
||||
long = _hint([_tc("exec", {"command": cmd})], max_length=120)
|
||||
assert len(long) > len(short)
|
||||
assert "npm test" in long
|
||||
|
||||
def test_exec_max_length_120_shows_full_command(self):
|
||||
cmd = "cd /home/user/project && npm install && npm run build"
|
||||
result = _hint([_tc("exec", {"command": cmd})], max_length=120)
|
||||
assert "npm run build" in result
|
||||
|
||||
def test_fallback_respects_max_length(self):
|
||||
long_val = "a" * 100
|
||||
result = _hint([_tc("custom_tool", {"data": long_val})], max_length=60)
|
||||
assert "\u2026" in result
|
||||
result_40 = _hint([_tc("custom_tool", {"data": long_val})], max_length=40)
|
||||
assert len(result) > len(result_40)
|
||||
|
||||
def test_mcp_respects_max_length(self):
|
||||
long_url = "https://example.com/very/long/path/to/resource"
|
||||
result = _hint([_tc("mcp_github__fetch", {"url": long_url})], max_length=80)
|
||||
result_40 = _hint([_tc("mcp_github__fetch", {"url": long_url})], max_length=40)
|
||||
assert len(result) >= len(result_40)
|
||||
|
||||
def test_path_type_respects_max_length(self):
|
||||
"""Path-type tools (read_file, write_file, etc.) should honor max_length."""
|
||||
long_path = "/home/user/.local/share/uv/tools/nanobot/agent/loop.py"
|
||||
short = _hint([_tc("read_file", {"path": long_path})], max_length=40)
|
||||
long = _hint([_tc("read_file", {"path": long_path})], max_length=120)
|
||||
assert len(long) > len(short)
|
||||
|
||||
def test_edit_path_respects_max_length(self):
|
||||
"""edit (is_path=True) should honor max_length, not stay hardcoded at 40."""
|
||||
long_path = "/home/user/projects/nanobot/src/agent/loop.py"
|
||||
short = _hint([_tc("edit", {"file_path": long_path})], max_length=40)
|
||||
long = _hint([_tc("edit", {"file_path": long_path})], max_length=120)
|
||||
assert len(long) > len(short)
|
||||
|
||||
def test_list_dir_path_respects_max_length(self):
|
||||
"""list_dir (is_path=True) should honor max_length."""
|
||||
long_path = "/home/user/.local/share/uv/tools/nanobot/"
|
||||
short = _hint([_tc("list_dir", {"path": long_path})], max_length=40)
|
||||
long = _hint([_tc("list_dir", {"path": long_path})], max_length=120)
|
||||
assert len(long) > len(short)
|
||||
|
||||
@@ -306,17 +306,19 @@ async def test_on_error_logs_network_issues_as_warning(monkeypatch) -> None:
|
||||
recorded: list[tuple[str, str]] = []
|
||||
|
||||
monkeypatch.setattr(
|
||||
"nanobot.channels.telegram.logger.warning",
|
||||
channel.logger,
|
||||
"warning",
|
||||
lambda message, error: recorded.append(("warning", message.format(error))),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.channels.telegram.logger.error",
|
||||
channel.logger,
|
||||
"error",
|
||||
lambda message, error: recorded.append(("error", message.format(error))),
|
||||
)
|
||||
|
||||
await channel._on_error(object(), SimpleNamespace(error=NetworkError("proxy disconnected")))
|
||||
|
||||
assert recorded == [("warning", "Telegram network issue: proxy disconnected")]
|
||||
assert recorded == [("warning", "network issue: proxy disconnected")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -330,13 +332,14 @@ async def test_on_error_summarizes_empty_network_error(monkeypatch) -> None:
|
||||
recorded: list[tuple[str, str]] = []
|
||||
|
||||
monkeypatch.setattr(
|
||||
"nanobot.channels.telegram.logger.warning",
|
||||
channel.logger,
|
||||
"warning",
|
||||
lambda message, error: recorded.append(("warning", message.format(error))),
|
||||
)
|
||||
|
||||
await channel._on_error(object(), SimpleNamespace(error=NetworkError("")))
|
||||
|
||||
assert recorded == [("warning", "Telegram network issue: NetworkError")]
|
||||
assert recorded == [("warning", "network issue: NetworkError")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -348,17 +351,19 @@ async def test_on_error_keeps_non_network_exceptions_as_error(monkeypatch) -> No
|
||||
recorded: list[tuple[str, str]] = []
|
||||
|
||||
monkeypatch.setattr(
|
||||
"nanobot.channels.telegram.logger.warning",
|
||||
channel.logger,
|
||||
"warning",
|
||||
lambda message, error: recorded.append(("warning", message.format(error))),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.channels.telegram.logger.error",
|
||||
channel.logger,
|
||||
"error",
|
||||
lambda message, error: recorded.append(("error", message.format(error))),
|
||||
)
|
||||
|
||||
await channel._on_error(object(), SimpleNamespace(error=RuntimeError("boom")))
|
||||
|
||||
assert recorded == [("error", "Telegram error: boom")]
|
||||
assert recorded == [("error", "error: boom")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -835,7 +835,7 @@ async def test_start_logs_install_hint_when_pyjwt_missing(make_channel, monkeypa
|
||||
ch = make_channel()
|
||||
errors = []
|
||||
monkeypatch.setattr(msteams_module, "MSTEAMS_AVAILABLE", False)
|
||||
monkeypatch.setattr(msteams_module.logger, "error", lambda message, *args: errors.append(message.format(*args)))
|
||||
monkeypatch.setattr(ch.logger, "error", lambda message, *args: errors.append(message.format(*args)))
|
||||
|
||||
await ch.start()
|
||||
|
||||
|
||||
@@ -467,7 +467,7 @@ async def test_connect_mcp_servers_logs_stdio_pollution_hint(
|
||||
yield # pragma: no cover
|
||||
|
||||
monkeypatch.setattr(sys.modules["mcp.client.stdio"], "stdio_client", _broken_stdio_client)
|
||||
monkeypatch.setattr("nanobot.agent.tools.mcp.logger.error", _error)
|
||||
monkeypatch.setattr("nanobot.agent.tools.mcp.logger.exception", _error)
|
||||
|
||||
registry = ToolRegistry()
|
||||
stacks = await connect_mcp_servers({"gh": MCPServerConfig(command="github-mcp")}, registry)
|
||||
|
||||
Reference in New Issue
Block a user