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