diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 91deccbab..74e23c6f9 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -202,7 +202,7 @@ class AgentLoop: timezone: str | None = None, session_ttl_minutes: int = 0, consolidation_ratio: float = 0.5, - max_messages: int = 0, + max_messages: int = 120, hooks: list[AgentHook] | None = None, unified_session: bool = False, disabled_skills: list[str] | None = None, @@ -260,7 +260,7 @@ class AgentLoop: disabled_skills=disabled_skills, ) self._unified_session = unified_session - self._max_messages = max_messages if max_messages > 0 else 0 + self._max_messages = max_messages if max_messages > 0 else 120 self._running = False self._mcp_servers = mcp_servers or {} self._mcp_stacks: dict[str, AsyncExitStack] = {} @@ -887,11 +887,10 @@ class AgentLoop: msg.metadata, session_key=key, ) _hist_kwargs: dict[str, Any] = { + "max_messages": self._max_messages, "max_tokens": self._replay_token_budget(), "include_timestamps": True, } - if self._max_messages > 0: - _hist_kwargs["max_messages"] = self._max_messages history = session.get_history(**_hist_kwargs) current_role = "assistant" if is_subagent else "user" @@ -977,11 +976,10 @@ class AgentLoop: message_tool.start_turn() _hist_kwargs: dict[str, Any] = { + "max_messages": self._max_messages, "max_tokens": self._replay_token_budget(), "include_timestamps": True, } - if self._max_messages > 0: - _hist_kwargs["max_messages"] = self._max_messages history = session.get_history(**_hist_kwargs) pending_ask_id = pending_ask_user_id(history) diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index 9eb8a4649..f507b3d80 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -91,9 +91,9 @@ class AgentDefaults(Base): serialization_alias="idleCompactAfterMinutes", ) # Auto-compact idle threshold in minutes (0 = disabled) max_messages: int = Field( - default=0, + default=120, ge=0, - ) # Max messages to replay from session history (0 = use default, respects token budget) + ) # Max messages to replay from session history (0 = use default 120, respects token budget) consolidation_ratio: float = Field( default=0.5, ge=0.1, diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index 095b3f550..de97135d4 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -12,15 +12,13 @@ from loguru import logger from nanobot.config.paths import get_legacy_sessions_dir from nanobot.utils.helpers import ( - estimate_message_tokens, ensure_dir, + estimate_message_tokens, find_legal_message_start, image_placeholder_text, safe_filename, ) - -HISTORY_MAX_MESSAGES = 120 FILE_MAX_MESSAGES = 2000 @@ -74,7 +72,7 @@ class Session: def get_history( self, - max_messages: int = HISTORY_MAX_MESSAGES, + max_messages: int = 120, *, max_tokens: int = 0, include_timestamps: bool = False, @@ -85,6 +83,7 @@ class Session: token budget from the tail (``max_tokens``) when provided. """ unconsolidated = self.messages[self.last_consolidated:] + max_messages = max_messages if max_messages > 0 else 120 sliced = unconsolidated[-max_messages:] # Avoid starting mid-turn when possible, except for proactive diff --git a/tests/agent/test_auto_compact.py b/tests/agent/test_auto_compact.py index 12578223b..ecef55044 100644 --- a/tests/agent/test_auto_compact.py +++ b/tests/agent/test_auto_compact.py @@ -10,8 +10,8 @@ import pytest from nanobot.agent.loop import AgentLoop from nanobot.bus.events import InboundMessage from nanobot.bus.queue import MessageBus -from nanobot.config.schema import AgentDefaults from nanobot.command import CommandContext +from nanobot.config.schema import AgentDefaults from nanobot.providers.base import LLMResponse @@ -75,10 +75,9 @@ class TestSessionTTLConfig: assert data["idleCompactAfterMinutes"] == 30 assert "sessionTtlMinutes" not in data - def test_session_history_and_file_cap_are_internal_constants(self): - """Session history/file cap should be internal constants, not config fields.""" - from nanobot.session.manager import HISTORY_MAX_MESSAGES, FILE_MAX_MESSAGES - assert HISTORY_MAX_MESSAGES == 120 + def test_session_file_cap_is_internal_constant(self): + """Session file cap should remain an internal constant, not a config field.""" + from nanobot.session.manager import FILE_MAX_MESSAGES assert FILE_MAX_MESSAGES == 2000 @@ -265,7 +264,6 @@ class TestAutoCompact: async def test_auto_compact_empty_session(self, tmp_path): """_archive on empty session should not archive.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) - session = loop.sessions.get_or_create("cli:test") archive_called = False diff --git a/tests/agent/test_max_messages_config.py b/tests/agent/test_max_messages_config.py index bf9f096bf..491ff16ec 100644 --- a/tests/agent/test_max_messages_config.py +++ b/tests/agent/test_max_messages_config.py @@ -3,17 +3,20 @@ from __future__ import annotations from pathlib import Path -from typing import Any -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from nanobot.agent.loop import AgentLoop +from nanobot.bus.events import InboundMessage from nanobot.bus.queue import MessageBus -from nanobot.session.manager import HISTORY_MAX_MESSAGES, Session +from nanobot.providers.base import LLMResponse +from nanobot.session.manager import Session + +DEFAULT_MAX_MESSAGES = 120 -def _make_loop(tmp_path: Path, max_messages: int = 0) -> AgentLoop: +def _make_loop(tmp_path: Path, max_messages: int = DEFAULT_MAX_MESSAGES) -> AgentLoop: provider = MagicMock() provider.get_default_model.return_value = "test-model" return AgentLoop( @@ -37,33 +40,31 @@ def _populated_session(n: int) -> Session: class TestMaxMessagesInit: """Verify AgentLoop stores the config value correctly.""" - def test_default_is_zero(self, tmp_path: Path) -> None: + def test_default_is_builtin_limit(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path) - assert loop._max_messages == 0 + assert loop._max_messages == DEFAULT_MAX_MESSAGES def test_positive_value_stored(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path, max_messages=25) assert loop._max_messages == 25 - def test_zero_means_unlimited(self, tmp_path: Path) -> None: - """max_messages=0 should not constrain get_history (uses default).""" + def test_zero_uses_builtin_limit(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path, max_messages=0) - assert loop._max_messages == 0 + assert loop._max_messages == DEFAULT_MAX_MESSAGES - def test_negative_treated_as_zero(self, tmp_path: Path) -> None: + def test_negative_treated_as_builtin_limit(self, tmp_path: Path) -> None: """Negative values should not produce negative slicing.""" loop = _make_loop(tmp_path, max_messages=-5) - assert loop._max_messages == 0 + assert loop._max_messages == DEFAULT_MAX_MESSAGES class TestGetHistoryWithMaxMessages: """Verify get_history respects max_messages parameter.""" - def test_default_uses_constant(self) -> None: + def test_default_uses_builtin_limit(self) -> None: session = _populated_session(80) history = session.get_history() - # Default HISTORY_MAX_MESSAGES=120, 80 pairs = 160 msgs, sliced to 120 - assert len(history) <= HISTORY_MAX_MESSAGES + assert len(history) <= DEFAULT_MAX_MESSAGES def test_explicit_max_messages_limits_output(self) -> None: session = _populated_session(40) # 80 messages total @@ -76,13 +77,10 @@ class TestGetHistoryWithMaxMessages: history = session.get_history(max_messages=25) assert history[0]["role"] == "user" - def test_max_messages_zero_returns_all(self) -> None: - """max_messages=0 with the default constant returns up to the constant.""" - session = _populated_session(10) # 20 messages - # When we pass 0 explicitly, unconsolidated[-0:] returns everything - # but the default is HISTORY_MAX_MESSAGES so this tests the default path - history = session.get_history() - assert len(history) == 20 + def test_max_messages_zero_uses_builtin_limit(self) -> None: + session = _populated_session(80) # 160 messages total + history = session.get_history(max_messages=0) + assert len(history) <= DEFAULT_MAX_MESSAGES def test_small_session_unaffected(self) -> None: """When session has fewer messages than max_messages, all are returned.""" @@ -94,41 +92,43 @@ class TestGetHistoryWithMaxMessages: class TestMaxMessagesIntegration: """Verify the config flows from AgentLoop into get_history calls.""" - def test_config_wired_to_history_call(self, tmp_path: Path) -> None: - """When max_messages > 0, get_history should receive it.""" + @pytest.mark.asyncio + async def test_process_message_passes_config_to_history_call(self, tmp_path: Path) -> None: + """The real message path should pass max_messages into session history replay.""" loop = _make_loop(tmp_path, max_messages=25) - session = _populated_session(40) # 80 messages + loop.provider.chat_with_retry = AsyncMock( + return_value=LLMResponse(content="ok", tool_calls=[], usage={}) + ) + loop.tools.get_definitions = MagicMock(return_value=[]) + loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign] + session = loop.sessions.get_or_create("cli:test") with patch.object(session, "get_history", wraps=session.get_history) as mock_hist: - # Call the internal method that builds history kwargs - kwargs: dict[str, Any] = { - "max_tokens": loop._replay_token_budget(), - "include_timestamps": True, - } - if loop._max_messages > 0: - kwargs["max_messages"] = loop._max_messages - session.get_history(**kwargs) + result = await loop._process_message( + InboundMessage(channel="cli", sender_id="user", chat_id="test", content="hello") + ) - assert mock_hist.call_count == 1 - call_kwargs = mock_hist.call_args - # max_messages is positional arg (first) or keyword - if call_kwargs.args: - assert call_kwargs.args[0] == 25 - else: - assert call_kwargs.kwargs.get("max_messages") == 25 + assert result is not None + assert mock_hist.call_count == 1 + assert mock_hist.call_args.kwargs["max_messages"] == 25 - def test_zero_config_omits_max_messages_kwarg(self, tmp_path: Path) -> None: - """When max_messages=0, get_history should use its default.""" + @pytest.mark.asyncio + async def test_zero_config_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path, max_messages=0) + loop.provider.chat_with_retry = AsyncMock( + return_value=LLMResponse(content="ok", tool_calls=[], usage={}) + ) + loop.tools.get_definitions = MagicMock(return_value=[]) + loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign] - kwargs: dict[str, Any] = { - "max_tokens": loop._replay_token_budget(), - "include_timestamps": True, - } - if loop._max_messages > 0: - kwargs["max_messages"] = loop._max_messages + session = loop.sessions.get_or_create("cli:test") + with patch.object(session, "get_history", wraps=session.get_history) as mock_hist: + result = await loop._process_message( + InboundMessage(channel="cli", sender_id="user", chat_id="test", content="hello") + ) - assert "max_messages" not in kwargs + assert result is not None + assert mock_hist.call_args.kwargs["max_messages"] == DEFAULT_MAX_MESSAGES class TestSchemaConfig: @@ -138,6 +138,12 @@ class TestSchemaConfig: from nanobot.config.schema import AgentDefaults defaults = AgentDefaults() + assert defaults.max_messages == DEFAULT_MAX_MESSAGES + + def test_schema_accepts_zero_as_builtin_limit(self) -> None: + from nanobot.config.schema import AgentDefaults + + defaults = AgentDefaults(max_messages=0) assert defaults.max_messages == 0 def test_schema_accepts_positive(self) -> None: