diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index debbf799..56c996ed 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -205,7 +205,6 @@ class AgentLoop: timezone: str | None = None, session_ttl_minutes: int = 0, consolidation_ratio: float = 0.5, - max_messages: int = 0, hooks: list[AgentHook] | None = None, unified_session: bool = False, disabled_skills: list[str] | None = None, @@ -296,12 +295,7 @@ class AgentLoop: llm_wall_timeout_for_session=lambda sk: runner_wall_llm_timeout_s(self.sessions, sk), ) self._unified_session = unified_session - self._explicit_max_messages = max_messages > 0 - self._max_messages = ( - max_messages - if self._explicit_max_messages - else replay_max_messages_for_context(self.context_window_tokens) - ) + self._max_messages = replay_max_messages_for_context(self.context_window_tokens) self._running = False self._mcp_servers = mcp_servers or {} self._mcp_stacks: dict[str, AsyncExitStack] = {} @@ -444,8 +438,7 @@ class AgentLoop: logger.info("Runtime model switched for next turn: {} -> {}", old_model, model) def _sync_replay_max_messages(self) -> None: - if not self._explicit_max_messages: - self._max_messages = replay_max_messages_for_context(self.context_window_tokens) + self._max_messages = replay_max_messages_for_context(self.context_window_tokens) def _refresh_provider_snapshot(self) -> None: if self._provider_snapshot_loader is None: diff --git a/nanobot/config/loader.py b/nanobot/config/loader.py index 78436507..0addc842 100644 --- a/nanobot/config/loader.py +++ b/nanobot/config/loader.py @@ -156,12 +156,12 @@ def _migrate_config(data: dict) -> dict: agents = data.get("agents", {}) defaults = agents.get("defaults", {}) if isinstance(agents, dict) else {} if isinstance(defaults, dict): - legacy_max_message_keys = [ - key for key in ("maxMessages", "max_messages") if key in defaults - ] - if legacy_max_message_keys: - for key in legacy_max_message_keys: - defaults.pop(key, None) + had_legacy_max_messages = ( + "maxMessages" in defaults or "max_messages" in defaults + ) + defaults.pop("maxMessages", None) + defaults.pop("max_messages", None) + if had_legacy_max_messages: # TODO(next version): Remove this legacy cleanup branch; the schema # will silently ignore this field once the warning grace period ends. logger.warning( diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index e7f93f1c..fc90fea0 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -29,7 +29,6 @@ from nanobot.utils.subagent_channel_display import scrub_subagent_announce_body FILE_MAX_MESSAGES = 2000 MIN_REPLAY_MAX_MESSAGES = 120 REPLAY_TOKENS_PER_MESSAGE = 100 -DEFAULT_REPLAY_MAX_MESSAGES = FILE_MAX_MESSAGES _MESSAGE_TIME_PREFIX_RE = re.compile(r"^\[Message Time: [^\]]+\]\n?") _LOCAL_IMAGE_BREADCRUMB_RE = re.compile(r"^\[image: (?:/|~)[^\]]+\]\s*$") _TOOL_CALL_ECHO_RE = re.compile(r'^\s*(?:generate_image|message)\([^)]*\)\s*$') @@ -48,7 +47,7 @@ _FORK_VOLATILE_METADATA_KEYS = { def replay_max_messages_for_context(context_window_tokens: int | None) -> int: if not context_window_tokens or context_window_tokens <= 0: - return DEFAULT_REPLAY_MAX_MESSAGES + return FILE_MAX_MESSAGES return min( FILE_MAX_MESSAGES, max(MIN_REPLAY_MAX_MESSAGES, context_window_tokens // REPLAY_TOKENS_PER_MESSAGE), @@ -144,7 +143,7 @@ class Session: def get_history( self, - max_messages: int = DEFAULT_REPLAY_MAX_MESSAGES, + max_messages: int = FILE_MAX_MESSAGES, *, max_tokens: int = 0, extend_to_user: bool = False, @@ -155,7 +154,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 DEFAULT_REPLAY_MAX_MESSAGES + max_messages = max_messages if max_messages > 0 else FILE_MAX_MESSAGES start_idx = recent_message_start_index( unconsolidated, max_messages, diff --git a/tests/agent/conftest.py b/tests/agent/conftest.py index 57f678aa..c239733b 100644 --- a/tests/agent/conftest.py +++ b/tests/agent/conftest.py @@ -38,7 +38,6 @@ def make_loop( model: str = "test-model", context_window_tokens: int = 128_000, session_ttl_minutes: int = 0, - max_messages: int = 120, unified_session: bool = False, mcp_servers: dict | None = None, tools_config=None, @@ -64,7 +63,6 @@ def make_loop( model=model, context_window_tokens=context_window_tokens, session_ttl_minutes=session_ttl_minutes, - max_messages=max_messages, unified_session=unified_session, ) if mcp_servers is not None: @@ -79,8 +77,8 @@ def make_loop( if patch_deps: with patch("nanobot.agent.loop.ContextBuilder"), \ patch("nanobot.agent.loop.SessionManager"), \ - patch("nanobot.agent.loop.SubagentManager") as MockSubMgr: - MockSubMgr.return_value.cancel_by_session = AsyncMock(return_value=0) + patch("nanobot.agent.loop.SubagentManager") as mock_sub_mgr: + mock_sub_mgr.return_value.cancel_by_session = AsyncMock(return_value=0) return AgentLoop(**kwargs) return AgentLoop(**kwargs) diff --git a/tests/agent/test_max_messages_config.py b/tests/agent/test_max_messages_config.py index f4fe153e..58101138 100644 --- a/tests/agent/test_max_messages_config.py +++ b/tests/agent/test_max_messages_config.py @@ -13,17 +13,14 @@ from nanobot.bus.queue import MessageBus from nanobot.providers.base import LLMResponse from nanobot.providers.factory import ProviderSnapshot from nanobot.session.manager import ( - DEFAULT_REPLAY_MAX_MESSAGES, + FILE_MAX_MESSAGES, Session, replay_max_messages_for_context, ) -DEFAULT_MAX_MESSAGES = DEFAULT_REPLAY_MAX_MESSAGES - def _make_loop( tmp_path: Path, - max_messages: int = 0, context_window_tokens: int = 200_000, ) -> AgentLoop: provider = MagicMock() @@ -34,7 +31,6 @@ def _make_loop( provider=provider, workspace=tmp_path, model="test-model", - max_messages=max_messages, context_window_tokens=context_window_tokens, ) @@ -67,29 +63,16 @@ class TestMaxMessagesInit: def test_context_formula(self) -> None: assert replay_max_messages_for_context(8_000) == 120 assert replay_max_messages_for_context(32_768) == 327 - assert replay_max_messages_for_context(200_000) == DEFAULT_MAX_MESSAGES + assert replay_max_messages_for_context(200_000) == FILE_MAX_MESSAGES def test_default_for_200k_context_reaches_file_cap(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path) - assert loop._max_messages == DEFAULT_MAX_MESSAGES + assert loop._max_messages == FILE_MAX_MESSAGES def test_default_scales_with_context_window(self, tmp_path: Path) -> None: loop = _make_loop(tmp_path, context_window_tokens=32_768) assert loop._max_messages == 327 - 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_uses_context_derived_limit(self, tmp_path: Path) -> None: - loop = _make_loop(tmp_path, max_messages=0) - assert loop._max_messages == DEFAULT_MAX_MESSAGES - - 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 == DEFAULT_MAX_MESSAGES - def test_provider_refresh_resyncs_context_derived_limit(self, tmp_path: Path) -> None: old_provider = MagicMock() old_provider.get_default_model.return_value = "old-model" @@ -112,7 +95,7 @@ class TestMaxMessagesInit: assert loop._max_messages == 327 loop._refresh_provider_snapshot() - assert loop._max_messages == DEFAULT_MAX_MESSAGES + assert loop._max_messages == FILE_MAX_MESSAGES class TestGetHistoryWithMaxMessages: @@ -121,7 +104,7 @@ class TestGetHistoryWithMaxMessages: def test_default_uses_builtin_limit(self) -> None: session = _populated_session(80) history = session.get_history() - assert len(history) <= DEFAULT_MAX_MESSAGES + assert len(history) <= FILE_MAX_MESSAGES def test_explicit_max_messages_limits_output(self) -> None: session = _populated_session(40) # 80 messages total @@ -137,7 +120,7 @@ class TestGetHistoryWithMaxMessages: 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 + assert len(history) <= FILE_MAX_MESSAGES def test_small_session_unaffected(self) -> None: """When session has fewer messages than max_messages, all are returned.""" @@ -152,7 +135,8 @@ class TestMaxMessagesIntegration: @pytest.mark.asyncio async def test_process_message_passes_limit_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) + loop = _make_loop(tmp_path) + loop._max_messages = 25 loop.provider.chat_with_retry = AsyncMock( return_value=LLMResponse(content="ok", tool_calls=[], usage={}) ) @@ -171,8 +155,11 @@ class TestMaxMessagesIntegration: assert mock_hist.call_args.kwargs["extend_to_user"] is False @pytest.mark.asyncio - async def test_zero_limit_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None: - loop = _make_loop(tmp_path, max_messages=0) + async def test_default_limit_passes_context_derived_limit_to_history_call( + self, + tmp_path: Path, + ) -> None: + loop = _make_loop(tmp_path) loop.provider.chat_with_retry = AsyncMock( return_value=LLMResponse(content="ok", tool_calls=[], usage={}) ) @@ -186,7 +173,7 @@ class TestMaxMessagesIntegration: ) assert result is not None - assert mock_hist.call_args.kwargs["max_messages"] == DEFAULT_MAX_MESSAGES + assert mock_hist.call_args.kwargs["max_messages"] == FILE_MAX_MESSAGES assert mock_hist.call_args.kwargs["extend_to_user"] is False @pytest.mark.asyncio @@ -195,7 +182,8 @@ class TestMaxMessagesIntegration: tmp_path: Path, ) -> None: """A live user turn should not extend history to an older long tool turn.""" - loop = _make_loop(tmp_path, max_messages=6) + loop = _make_loop(tmp_path) + loop._max_messages = 6 loop.provider.chat_with_retry = AsyncMock( return_value=LLMResponse(content="ok", tool_calls=[], usage={}) )