mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-06 17:38:35 +00:00
fix(config): retire max messages setting
This commit is contained in:
parent
c8638dee46
commit
dacc699293
@ -57,7 +57,7 @@ from nanobot.session.goal_state import (
|
|||||||
sustained_goal_active,
|
sustained_goal_active,
|
||||||
)
|
)
|
||||||
from nanobot.session.keys import UNIFIED_SESSION_KEY, session_key_for_channel
|
from nanobot.session.keys import UNIFIED_SESSION_KEY, session_key_for_channel
|
||||||
from nanobot.session.manager import Session, SessionManager
|
from nanobot.session.manager import DEFAULT_REPLAY_MAX_MESSAGES, Session, SessionManager
|
||||||
from nanobot.utils.document import extract_documents, reference_non_image_attachments
|
from nanobot.utils.document import extract_documents, reference_non_image_attachments
|
||||||
from nanobot.utils.helpers import image_placeholder_text
|
from nanobot.utils.helpers import image_placeholder_text
|
||||||
from nanobot.utils.helpers import truncate_text as truncate_text_fn
|
from nanobot.utils.helpers import truncate_text as truncate_text_fn
|
||||||
@ -201,7 +201,7 @@ 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 = 500,
|
max_messages: int = DEFAULT_REPLAY_MAX_MESSAGES,
|
||||||
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,
|
||||||
@ -292,7 +292,9 @@ 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._max_messages = max_messages if max_messages > 0 else 500
|
self._max_messages = (
|
||||||
|
max_messages if max_messages > 0 else DEFAULT_REPLAY_MAX_MESSAGES
|
||||||
|
)
|
||||||
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] = {}
|
||||||
@ -390,7 +392,6 @@ class AgentLoop:
|
|||||||
disabled_skills=defaults.disabled_skills,
|
disabled_skills=defaults.disabled_skills,
|
||||||
session_ttl_minutes=defaults.session_ttl_minutes,
|
session_ttl_minutes=defaults.session_ttl_minutes,
|
||||||
consolidation_ratio=defaults.consolidation_ratio,
|
consolidation_ratio=defaults.consolidation_ratio,
|
||||||
max_messages=defaults.max_messages,
|
|
||||||
tools_config=config.tools,
|
tools_config=config.tools,
|
||||||
model_presets=preset_helpers.configured_model_presets(config),
|
model_presets=preset_helpers.configured_model_presets(config),
|
||||||
model_preset=defaults.model_preset,
|
model_preset=defaults.model_preset,
|
||||||
|
|||||||
@ -7,6 +7,7 @@ from pathlib import Path
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import pydantic
|
import pydantic
|
||||||
|
from loguru import logger
|
||||||
from pydantic import BaseModel
|
from pydantic import BaseModel
|
||||||
|
|
||||||
from nanobot.config.schema import Config, _resolve_tool_config_refs
|
from nanobot.config.schema import Config, _resolve_tool_config_refs
|
||||||
@ -152,6 +153,23 @@ def _env_replace(match: re.Match[str]) -> str:
|
|||||||
|
|
||||||
def _migrate_config(data: dict) -> dict:
|
def _migrate_config(data: dict) -> dict:
|
||||||
"""Migrate old config formats to current."""
|
"""Migrate old config formats to current."""
|
||||||
|
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)
|
||||||
|
# TODO(next version): Remove this legacy cleanup branch; the schema
|
||||||
|
# will silently ignore this field once the warning grace period ends.
|
||||||
|
logger.warning(
|
||||||
|
"agents.defaults.maxMessages/max_messages is legacy and ignored; "
|
||||||
|
"replay max messages is now an internal safety cap. Remove it from "
|
||||||
|
"config. This compatibility warning will be removed in the next version."
|
||||||
|
)
|
||||||
|
|
||||||
# Move tools.exec.restrictToWorkspace → tools.restrictToWorkspace
|
# Move tools.exec.restrictToWorkspace → tools.restrictToWorkspace
|
||||||
tools = data.get("tools", {})
|
tools = data.get("tools", {})
|
||||||
exec_cfg = tools.get("exec", {})
|
exec_cfg = tools.get("exec", {})
|
||||||
|
|||||||
@ -154,10 +154,6 @@ class AgentDefaults(Base):
|
|||||||
validation_alias=AliasChoices("idleCompactAfterMinutes", "sessionTtlMinutes"),
|
validation_alias=AliasChoices("idleCompactAfterMinutes", "sessionTtlMinutes"),
|
||||||
serialization_alias="idleCompactAfterMinutes",
|
serialization_alias="idleCompactAfterMinutes",
|
||||||
) # Auto-compact idle threshold in minutes (0 = disabled)
|
) # Auto-compact idle threshold in minutes (0 = disabled)
|
||||||
max_messages: int = Field(
|
|
||||||
default=500,
|
|
||||||
ge=0,
|
|
||||||
) # Last-resort max messages to replay from session history (0 = use default 500)
|
|
||||||
consolidation_ratio: float = Field(
|
consolidation_ratio: float = Field(
|
||||||
default=0.5,
|
default=0.5,
|
||||||
ge=0.1,
|
ge=0.1,
|
||||||
|
|||||||
@ -27,6 +27,7 @@ from nanobot.utils.helpers import (
|
|||||||
from nanobot.utils.subagent_channel_display import scrub_subagent_announce_body
|
from nanobot.utils.subagent_channel_display import scrub_subagent_announce_body
|
||||||
|
|
||||||
FILE_MAX_MESSAGES = 2000
|
FILE_MAX_MESSAGES = 2000
|
||||||
|
DEFAULT_REPLAY_MAX_MESSAGES = 500
|
||||||
_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*$')
|
||||||
@ -132,7 +133,7 @@ class Session:
|
|||||||
|
|
||||||
def get_history(
|
def get_history(
|
||||||
self,
|
self,
|
||||||
max_messages: int = 500,
|
max_messages: int = DEFAULT_REPLAY_MAX_MESSAGES,
|
||||||
*,
|
*,
|
||||||
max_tokens: int = 0,
|
max_tokens: int = 0,
|
||||||
extend_to_user: bool = False,
|
extend_to_user: bool = False,
|
||||||
@ -143,7 +144,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 500
|
max_messages = max_messages if max_messages > 0 else DEFAULT_REPLAY_MAX_MESSAGES
|
||||||
start_idx = recent_message_start_index(
|
start_idx = recent_message_start_index(
|
||||||
unconsolidated,
|
unconsolidated,
|
||||||
max_messages,
|
max_messages,
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
"""Tests for max_messages config wiring into session history replay."""
|
"""Tests for the internal max_messages replay cap."""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
@ -11,9 +11,9 @@ from nanobot.agent.loop import AgentLoop
|
|||||||
from nanobot.bus.events import InboundMessage
|
from nanobot.bus.events import InboundMessage
|
||||||
from nanobot.bus.queue import MessageBus
|
from nanobot.bus.queue import MessageBus
|
||||||
from nanobot.providers.base import LLMResponse
|
from nanobot.providers.base import LLMResponse
|
||||||
from nanobot.session.manager import Session
|
from nanobot.session.manager import DEFAULT_REPLAY_MAX_MESSAGES, Session
|
||||||
|
|
||||||
DEFAULT_MAX_MESSAGES = 500
|
DEFAULT_MAX_MESSAGES = DEFAULT_REPLAY_MAX_MESSAGES
|
||||||
|
|
||||||
|
|
||||||
def _make_loop(tmp_path: Path, max_messages: int = DEFAULT_MAX_MESSAGES) -> AgentLoop:
|
def _make_loop(tmp_path: Path, max_messages: int = DEFAULT_MAX_MESSAGES) -> AgentLoop:
|
||||||
@ -103,10 +103,10 @@ class TestGetHistoryWithMaxMessages:
|
|||||||
|
|
||||||
|
|
||||||
class TestMaxMessagesIntegration:
|
class TestMaxMessagesIntegration:
|
||||||
"""Verify the config flows from AgentLoop into get_history calls."""
|
"""Verify AgentLoop passes the replay cap into get_history calls."""
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_process_message_passes_config_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, max_messages=25)
|
||||||
loop.provider.chat_with_retry = AsyncMock(
|
loop.provider.chat_with_retry = AsyncMock(
|
||||||
@ -127,7 +127,7 @@ 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_config_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None:
|
async def test_zero_limit_passes_builtin_limit_to_history_call(self, tmp_path: Path) -> None:
|
||||||
loop = _make_loop(tmp_path, max_messages=0)
|
loop = _make_loop(tmp_path, max_messages=0)
|
||||||
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={})
|
||||||
@ -182,31 +182,3 @@ class TestMaxMessagesIntegration:
|
|||||||
sent_text = "\n".join(str(message.get("content")) for message in sent_messages)
|
sent_text = "\n".join(str(message.get("content")) for message in sent_messages)
|
||||||
assert "new question" in sent_text
|
assert "new question" in sent_text
|
||||||
assert "long older turn" not in sent_text
|
assert "long older turn" not in sent_text
|
||||||
|
|
||||||
|
|
||||||
class TestSchemaConfig:
|
|
||||||
"""Verify the config schema accepts max_messages."""
|
|
||||||
|
|
||||||
def test_schema_default(self) -> None:
|
|
||||||
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:
|
|
||||||
from nanobot.config.schema import AgentDefaults
|
|
||||||
|
|
||||||
defaults = AgentDefaults(max_messages=25)
|
|
||||||
assert defaults.max_messages == 25
|
|
||||||
|
|
||||||
def test_schema_rejects_negative(self) -> None:
|
|
||||||
from nanobot.config.schema import AgentDefaults
|
|
||||||
|
|
||||||
with pytest.raises(Exception): # Pydantic validation error
|
|
||||||
AgentDefaults(max_messages=-1)
|
|
||||||
|
|||||||
@ -2,6 +2,8 @@ import json
|
|||||||
import socket
|
import socket
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from nanobot.config.loader import load_config, save_config
|
from nanobot.config.loader import load_config, save_config
|
||||||
from nanobot.security.network import validate_url_target
|
from nanobot.security.network import validate_url_target
|
||||||
|
|
||||||
@ -93,6 +95,41 @@ def test_onboard_does_not_crash_with_legacy_memory_window(tmp_path, monkeypatch)
|
|||||||
assert result.exit_code == 0
|
assert result.exit_code == 0
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("field_name", ["maxMessages", "max_messages"])
|
||||||
|
def test_load_config_warns_and_ignores_legacy_max_messages(tmp_path, field_name) -> None:
|
||||||
|
config_path = tmp_path / "config.json"
|
||||||
|
config_path.write_text(
|
||||||
|
json.dumps({"agents": {"defaults": {field_name: 25, "maxTokens": 1234}}}),
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("nanobot.config.loader.logger.warning") as warning:
|
||||||
|
config = load_config(config_path)
|
||||||
|
|
||||||
|
assert config.agents.defaults.max_tokens == 1234
|
||||||
|
assert not hasattr(config.agents.defaults, "max_messages")
|
||||||
|
warning.assert_called_once()
|
||||||
|
message = warning.call_args.args[0]
|
||||||
|
assert "legacy and ignored" in message
|
||||||
|
assert "next version" in message
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_config_drops_legacy_max_messages(tmp_path) -> None:
|
||||||
|
config_path = tmp_path / "config.json"
|
||||||
|
config_path.write_text(
|
||||||
|
json.dumps({"agents": {"defaults": {"maxMessages": 25}}}),
|
||||||
|
encoding="utf-8",
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("nanobot.config.loader.logger.warning"):
|
||||||
|
config = load_config(config_path)
|
||||||
|
save_config(config, config_path)
|
||||||
|
saved = json.loads(config_path.read_text(encoding="utf-8"))
|
||||||
|
|
||||||
|
assert "maxMessages" not in saved["agents"]["defaults"]
|
||||||
|
assert "max_messages" not in saved["agents"]["defaults"]
|
||||||
|
|
||||||
|
|
||||||
def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
|
def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user