mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-09-04 02:01:48 +03:00
fix(session): preserve complete transcripts
This commit is contained in:
chengyongru
chengyongru
@@ -171,12 +171,6 @@ class TestSessionTTLConfig:
|
||||
data = defaults.model_dump(mode="json", by_alias=True)
|
||||
assert data["idleCompactCheckIntervalSeconds"] == 10
|
||||
|
||||
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
|
||||
|
||||
|
||||
class TestIdleScanThrottling:
|
||||
"""Test scheduling of full idle-session scans."""
|
||||
|
||||
@@ -255,53 +249,7 @@ class TestAgentLoopTTLParam:
|
||||
kwargs = session.get_history.call_args.kwargs
|
||||
assert isinstance(kwargs.get("max_tokens"), int)
|
||||
assert kwargs["max_tokens"] > 0
|
||||
assert set(kwargs) == {"max_messages", "max_tokens", "extend_to_user"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_file_cap_archives_and_trims_old_messages(self, tmp_path):
|
||||
loop = _make_loop(tmp_path)
|
||||
loop.context.memory.raw_archive = MagicMock()
|
||||
|
||||
for i in range(4):
|
||||
msg = InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="u1",
|
||||
chat_id="direct",
|
||||
content=f"hello {i}",
|
||||
)
|
||||
await loop._process_message(msg)
|
||||
|
||||
session = loop.sessions.get_or_create("cli:direct")
|
||||
from nanobot.session.manager import FILE_MAX_MESSAGES
|
||||
assert len(session.messages) <= FILE_MAX_MESSAGES
|
||||
|
||||
def test_session_enforce_file_cap_skips_archive_when_dropped_prefix_already_consolidated(self, tmp_path):
|
||||
from nanobot.session.manager import Session
|
||||
archive_fn = MagicMock()
|
||||
session = Session(key="cli:direct")
|
||||
for i in range(8):
|
||||
session.add_message("user", f"u{i}")
|
||||
session.last_consolidated = 6
|
||||
|
||||
session.enforce_file_cap(on_archive=archive_fn, limit=4)
|
||||
|
||||
assert len(session.messages) <= 4
|
||||
archive_fn.assert_not_called()
|
||||
|
||||
def test_session_enforce_file_cap_archives_only_unconsolidated_dropped_prefix(self, tmp_path):
|
||||
from nanobot.session.manager import Session
|
||||
archive_fn = MagicMock()
|
||||
session = Session(key="cli:direct")
|
||||
for i in range(8):
|
||||
session.add_message("user", f"u{i}")
|
||||
session.last_consolidated = 2
|
||||
|
||||
session.enforce_file_cap(on_archive=archive_fn, limit=4)
|
||||
|
||||
assert len(session.messages) <= 4
|
||||
archive_fn.assert_called_once()
|
||||
archived = archive_fn.call_args.args[0]
|
||||
assert [m["content"] for m in archived] == ["u2", "u3"]
|
||||
assert set(kwargs) == {"max_tokens", "extend_to_user"}
|
||||
|
||||
|
||||
class TestAutoCompact:
|
||||
|
||||
@@ -363,7 +363,7 @@ class TestConsolidatorTokenBudget:
|
||||
await consolidator.maybe_consolidate_by_tokens(session, runtime=runtime)
|
||||
|
||||
async def test_estimate_uses_full_unconsolidated_tail(self, consolidator, runtime):
|
||||
"""Consolidation pressure must see messages hidden by the replay window."""
|
||||
"""Consolidation pressure must account for the full unarchived tail."""
|
||||
session = Session(key="test:full-tail")
|
||||
for i in range(160):
|
||||
session.add_message("user", f"msg-{i}")
|
||||
@@ -400,107 +400,6 @@ class TestConsolidatorTokenBudget:
|
||||
assert len(captured["history"]) == 8
|
||||
assert captured["history"][0]["content"] == "msg-2"
|
||||
|
||||
async def test_replay_window_overflow_is_archived_even_under_token_budget(
|
||||
self,
|
||||
consolidator,
|
||||
runtime,
|
||||
):
|
||||
"""Old messages that cannot be replayed should be materialized first."""
|
||||
consolidator._SAFETY_BUFFER = 0
|
||||
session = Session(key="test:replay-overflow")
|
||||
session.provider_state = _provider_state()
|
||||
for i in range(10):
|
||||
session.add_message("user", f"u{i}")
|
||||
session.add_message("assistant", f"a{i}")
|
||||
|
||||
consolidator.sessions._session_cache[session.key] = session
|
||||
consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(100, "tiktoken"))
|
||||
consolidator.archive_session = AsyncMock(return_value="old conversation summary")
|
||||
|
||||
await consolidator.maybe_consolidate_by_tokens(
|
||||
session,
|
||||
runtime=runtime,
|
||||
replay_max_messages=6,
|
||||
)
|
||||
|
||||
archive_end = consolidator.archive_session.await_args.kwargs["archive_end"]
|
||||
archived_chunk = session.messages[:archive_end]
|
||||
assert archived_chunk[0]["content"] == "u0"
|
||||
assert archived_chunk[-1]["content"] == "a6"
|
||||
assert session.last_consolidated == 14
|
||||
assert session.metadata["_last_summary"]["text"] == "old conversation summary"
|
||||
assert session.provider_state is None
|
||||
consolidator.sessions.save.assert_called()
|
||||
|
||||
async def test_replay_window_overflow_extends_to_long_recent_user_turn(
|
||||
self,
|
||||
consolidator,
|
||||
runtime,
|
||||
):
|
||||
"""Replay-window consolidation must not cut into the latest user turn."""
|
||||
session = Session(key="test:replay-tool-boundary")
|
||||
session.add_message("user", "old")
|
||||
session.add_message("assistant", "old answer")
|
||||
session.add_message("user", "record this")
|
||||
for i in range(4):
|
||||
session.messages.extend(_tool_round(f"call-{i}"))
|
||||
session.add_message("assistant", "final answer")
|
||||
|
||||
consolidator.sessions._session_cache[session.key] = session
|
||||
consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(100, "tiktoken"))
|
||||
consolidator.archive_session = AsyncMock(return_value="tool turn summary")
|
||||
|
||||
await consolidator.maybe_consolidate_by_tokens(
|
||||
session,
|
||||
runtime=runtime,
|
||||
replay_max_messages=4,
|
||||
)
|
||||
|
||||
archive_end = consolidator.archive_session.await_args.kwargs["archive_end"]
|
||||
archived_chunk = session.messages[:archive_end]
|
||||
assert [m["content"] for m in archived_chunk] == ["old", "old answer"]
|
||||
assert session.last_consolidated == 2
|
||||
|
||||
history = session.get_history(max_messages=4, extend_to_user=True)
|
||||
assert len(history) > 4
|
||||
assert history[0]["content"] == "record this"
|
||||
assert history[-1]["content"] == "final answer"
|
||||
|
||||
async def test_replay_window_overflow_uses_newer_user_inside_window(
|
||||
self,
|
||||
consolidator,
|
||||
runtime,
|
||||
):
|
||||
"""Do not extend to an older long turn when the hard window has a newer user."""
|
||||
session = Session(key="test:replay-newer-user")
|
||||
session.add_message("user", "old")
|
||||
session.add_message("assistant", "old answer")
|
||||
session.add_message("user", "long older turn")
|
||||
for i in range(8):
|
||||
session.messages.extend(_tool_round(f"older-{i}"))
|
||||
session.add_message("assistant", "older final")
|
||||
session.add_message("user", "new question")
|
||||
session.add_message("assistant", "new answer")
|
||||
|
||||
consolidator.sessions._session_cache[session.key] = session
|
||||
consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(100, "tiktoken"))
|
||||
consolidator.archive_session = AsyncMock(return_value="older turn summary")
|
||||
|
||||
await consolidator.maybe_consolidate_by_tokens(
|
||||
session,
|
||||
runtime=runtime,
|
||||
replay_max_messages=6,
|
||||
)
|
||||
|
||||
archive_end = consolidator.archive_session.await_args.kwargs["archive_end"]
|
||||
archived_chunk = session.messages[:archive_end]
|
||||
assert archived_chunk[2]["content"] == "long older turn"
|
||||
assert archived_chunk[-1]["content"] == "older final"
|
||||
assert session.last_consolidated == len(session.messages) - 2
|
||||
|
||||
history = session.get_history(max_messages=6, extend_to_user=True)
|
||||
assert [m["content"] for m in history] == ["new question", "new answer"]
|
||||
|
||||
async def test_token_overflow_appends_prompt_to_replay_prefix(
|
||||
self,
|
||||
consolidator,
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
"""Tests for token-bounded session history replay."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
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.providers.base import LLMResponse
|
||||
from nanobot.session.manager import Session
|
||||
|
||||
|
||||
def _make_loop(tmp_path: Path, context_window_tokens: int = 200_000) -> AgentLoop:
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.generation.max_tokens = 4096
|
||||
return AgentLoop(
|
||||
bus=MessageBus(),
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
model="test-model",
|
||||
context_window_tokens=context_window_tokens,
|
||||
)
|
||||
|
||||
|
||||
def _populated_session(turns: int) -> Session:
|
||||
session = Session(key="test:populated")
|
||||
for index in range(turns):
|
||||
session.add_message("user", f"msg-{index}")
|
||||
session.add_message("assistant", f"reply-{index}")
|
||||
return session
|
||||
|
||||
|
||||
def _tool_round(call_id: str) -> list[dict]:
|
||||
return [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [
|
||||
{"id": call_id, "type": "function", "function": {"name": "x", "arguments": "{}"}}
|
||||
],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": call_id, "name": "x", "content": "ok"},
|
||||
]
|
||||
|
||||
|
||||
def test_default_history_has_no_message_count_limit() -> None:
|
||||
session = _populated_session(1_001)
|
||||
|
||||
history = session.get_history()
|
||||
|
||||
assert len(history) == 2_002
|
||||
assert history[0]["content"] == "msg-0"
|
||||
assert history[-1]["content"] == "reply-1000"
|
||||
|
||||
|
||||
def test_explicit_message_limit_still_starts_at_user_turn() -> None:
|
||||
history = _populated_session(30).get_history(max_messages=25)
|
||||
|
||||
assert len(history) <= 25
|
||||
assert history[0]["role"] == "user"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_message_replays_with_token_budget_only(tmp_path: Path) -> None:
|
||||
loop = _make_loop(tmp_path, context_window_tokens=32_768)
|
||||
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 get_history:
|
||||
result = await loop._process_message(
|
||||
InboundMessage(channel="cli", sender_id="user", chat_id="test", content="hello")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert get_history.call_args.kwargs == {
|
||||
"max_tokens": loop._replay_token_budget(loop.llm_runtime()),
|
||||
"extend_to_user": False,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_budget_keeps_current_user_as_replay_boundary(tmp_path: Path) -> None:
|
||||
loop = _make_loop(tmp_path, context_window_tokens=8_000)
|
||||
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")
|
||||
session.add_message("user", "old")
|
||||
session.add_message("assistant", "old answer")
|
||||
session.add_message("user", "long older turn")
|
||||
for index in range(70):
|
||||
session.messages.extend(_tool_round(f"older-{index}"))
|
||||
session.add_message("assistant", "older final")
|
||||
|
||||
result = await loop._process_message(
|
||||
InboundMessage(
|
||||
channel="cli",
|
||||
sender_id="user",
|
||||
chat_id="test",
|
||||
content="new question",
|
||||
)
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
sent_messages = loop.provider.chat_with_retry.await_args.kwargs["messages"]
|
||||
sent_text = "\n".join(str(message.get("content")) for message in sent_messages)
|
||||
assert "new question" in sent_text
|
||||
assert "long older turn" not in sent_text
|
||||
@@ -6,7 +6,6 @@ import nanobot.agent.memory as memory_module
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.providers.base import LLMResponse
|
||||
from nanobot.session.manager import replay_max_messages_for_context
|
||||
|
||||
|
||||
def _make_loop(tmp_path, *, estimated_tokens: int, context_window_tokens: int) -> AgentLoop:
|
||||
@@ -227,7 +226,6 @@ async def test_preflight_consolidation_receives_pending_summary(tmp_path) -> Non
|
||||
loop.consolidator.maybe_consolidate_by_tokens.assert_any_await(
|
||||
session,
|
||||
runtime=runtime,
|
||||
replay_max_messages=replay_max_messages_for_context(runtime.context_window_tokens),
|
||||
)
|
||||
assert len(loop.consolidator.maybe_consolidate_by_tokens.call_args_list) == 2
|
||||
assert all(
|
||||
|
||||
@@ -1,221 +0,0 @@
|
||||
"""Tests for the internal max_messages replay cap."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import replace
|
||||
from pathlib import Path
|
||||
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.providers.base import LLMResponse
|
||||
from nanobot.providers.factory import ProviderSnapshot
|
||||
from nanobot.session.manager import (
|
||||
FILE_MAX_MESSAGES,
|
||||
Session,
|
||||
replay_max_messages_for_context,
|
||||
)
|
||||
|
||||
|
||||
def _make_loop(
|
||||
tmp_path: Path,
|
||||
context_window_tokens: int = 200_000,
|
||||
) -> AgentLoop:
|
||||
provider = MagicMock()
|
||||
provider.get_default_model.return_value = "test-model"
|
||||
provider.generation.max_tokens = 4096
|
||||
return AgentLoop(
|
||||
bus=MessageBus(),
|
||||
provider=provider,
|
||||
workspace=tmp_path,
|
||||
model="test-model",
|
||||
context_window_tokens=context_window_tokens,
|
||||
)
|
||||
|
||||
|
||||
def _populated_session(n: int) -> Session:
|
||||
"""Create a session with *n* user/assistant turn pairs."""
|
||||
session = Session(key="test:populated")
|
||||
for i in range(n):
|
||||
session.add_message("user", f"msg-{i}")
|
||||
session.add_message("assistant", f"reply-{i}")
|
||||
return session
|
||||
|
||||
|
||||
def _tool_round(call_id: str) -> list[dict]:
|
||||
return [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": None,
|
||||
"tool_calls": [
|
||||
{"id": call_id, "type": "function", "function": {"name": "x", "arguments": "{}"}}
|
||||
],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": call_id, "name": "x", "content": "ok"},
|
||||
]
|
||||
|
||||
|
||||
class TestMaxMessagesInit:
|
||||
"""Verify AgentLoop derives the internal replay cap correctly."""
|
||||
|
||||
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) == FILE_MAX_MESSAGES
|
||||
|
||||
def test_default_for_200k_context_reaches_file_cap(self, tmp_path: Path) -> None:
|
||||
loop = _make_loop(tmp_path)
|
||||
runtime = loop.runtime_resolver.runtime
|
||||
assert replay_max_messages_for_context(runtime.context_window_tokens) == 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)
|
||||
runtime = loop.runtime_resolver.runtime
|
||||
assert replay_max_messages_for_context(runtime.context_window_tokens) == 327
|
||||
|
||||
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"
|
||||
old_provider.generation.max_tokens = 4096
|
||||
new_provider = MagicMock()
|
||||
new_provider.generation.max_tokens = 4096
|
||||
loop = AgentLoop(
|
||||
bus=MessageBus(),
|
||||
provider=old_provider,
|
||||
workspace=tmp_path,
|
||||
model="old-model",
|
||||
context_window_tokens=32_768,
|
||||
provider_snapshot_loader=lambda: ProviderSnapshot(
|
||||
provider=new_provider,
|
||||
model="new-model",
|
||||
context_window_tokens=200_000,
|
||||
signature=("new-model",),
|
||||
),
|
||||
)
|
||||
|
||||
initial = loop.runtime_resolver.runtime
|
||||
assert replay_max_messages_for_context(initial.context_window_tokens) == 327
|
||||
loop.runtime_resolver.invalidate()
|
||||
refreshed = loop.llm_runtime()
|
||||
assert replay_max_messages_for_context(refreshed.context_window_tokens) == FILE_MAX_MESSAGES
|
||||
|
||||
|
||||
class TestGetHistoryWithMaxMessages:
|
||||
"""Verify get_history respects max_messages parameter."""
|
||||
|
||||
def test_default_uses_builtin_limit(self) -> None:
|
||||
session = _populated_session(80)
|
||||
history = session.get_history()
|
||||
assert len(history) <= FILE_MAX_MESSAGES
|
||||
|
||||
def test_explicit_max_messages_limits_output(self) -> None:
|
||||
session = _populated_session(40) # 80 messages total
|
||||
history = session.get_history(max_messages=20)
|
||||
assert len(history) <= 20
|
||||
|
||||
def test_max_messages_starts_at_user_turn(self) -> None:
|
||||
"""Sliced history should start with a user message, not mid-turn."""
|
||||
session = _populated_session(30) # 60 messages
|
||||
history = session.get_history(max_messages=25)
|
||||
assert history[0]["role"] == "user"
|
||||
|
||||
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) <= FILE_MAX_MESSAGES
|
||||
|
||||
def test_small_session_unaffected(self) -> None:
|
||||
"""When session has fewer messages than max_messages, all are returned."""
|
||||
session = _populated_session(5) # 10 messages
|
||||
history = session.get_history(max_messages=25)
|
||||
assert len(history) == 10
|
||||
|
||||
|
||||
class TestMaxMessagesIntegration:
|
||||
"""Verify AgentLoop passes the replay cap into get_history calls."""
|
||||
|
||||
@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)
|
||||
runtime = replace(loop.llm_runtime(), context_window_tokens=32_768)
|
||||
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:
|
||||
result = await loop._process_message(
|
||||
InboundMessage(channel="cli", sender_id="user", chat_id="test", content="hello"),
|
||||
runtime=runtime,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert mock_hist.call_count == 1
|
||||
assert mock_hist.call_args.kwargs["max_messages"] == 327
|
||||
assert mock_hist.call_args.kwargs["extend_to_user"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
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={})
|
||||
)
|
||||
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:
|
||||
result = await loop._process_message(
|
||||
InboundMessage(channel="cli", sender_id="user", chat_id="test", content="hello")
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
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
|
||||
async def test_process_message_uses_current_user_as_replay_boundary(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""A live user turn should not extend history to an older long tool turn."""
|
||||
loop = _make_loop(tmp_path, context_window_tokens=8_000)
|
||||
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")
|
||||
session.add_message("user", "old")
|
||||
session.add_message("assistant", "old answer")
|
||||
session.add_message("user", "long older turn")
|
||||
for i in range(70):
|
||||
session.messages.extend(_tool_round(f"older-{i}"))
|
||||
session.add_message("assistant", "older final")
|
||||
|
||||
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="new question",
|
||||
)
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert mock_hist.call_args.kwargs["extend_to_user"] is False
|
||||
sent_messages = loop.provider.chat_with_retry.await_args.kwargs["messages"]
|
||||
sent_text = "\n".join(str(message.get("content")) for message in sent_messages)
|
||||
assert "new question" in sent_text
|
||||
assert "long older turn" not in sent_text
|
||||
@@ -83,7 +83,6 @@ def test_loop_has_no_mutable_runtime_mirrors_or_legacy_snapshot_api(tmp_path: Pa
|
||||
}.isdisjoint(loop.__dict__)
|
||||
assert not hasattr(loop, "_apply_provider_snapshot")
|
||||
assert not hasattr(loop, "_build_model_preset_snapshot")
|
||||
assert not hasattr(loop, "_sync_replay_max_messages")
|
||||
|
||||
|
||||
def test_llm_runtime_refreshes_provider_snapshot(tmp_path: Path) -> None:
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
import pytest
|
||||
|
||||
from nanobot.providers.base import ProviderConversationState
|
||||
from nanobot.runtime_context import (
|
||||
RUNTIME_CONTEXT_HISTORY_META,
|
||||
@@ -832,9 +830,6 @@ def test_get_history_extend_to_user_keeps_newer_user_inside_window():
|
||||
_assert_no_orphans(history)
|
||||
|
||||
|
||||
# --- enforce_file_cap archive correctness (issue #4128) ---
|
||||
|
||||
|
||||
def test_retain_recent_legal_suffix_returns_dropped_messages():
|
||||
"""retain_recent_legal_suffix returns the actually-dropped messages."""
|
||||
session = Session(
|
||||
@@ -894,125 +889,6 @@ def test_retain_recent_legal_suffix_returns_all_on_zero():
|
||||
assert session.messages == []
|
||||
|
||||
|
||||
def test_enforce_file_cap_no_duplicate_archive_in_else_branch():
|
||||
"""When the tail is assistant-only, enforce_file_cap must not archive
|
||||
messages that are also retained (the bug from issue #4128)."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
session = Session(key="test:else-archive")
|
||||
# Build: 15 user messages, then 10 assistant messages (no user in tail)
|
||||
for i in range(15):
|
||||
session.messages.append({"role": "user", "content": f"u{i}"})
|
||||
for i in range(10):
|
||||
session.messages.append({"role": "assistant", "content": f"a{i}"})
|
||||
|
||||
archive_fn = MagicMock()
|
||||
session.enforce_file_cap(on_archive=archive_fn, limit=6)
|
||||
|
||||
assert len(session.messages) <= 6
|
||||
|
||||
# Verify archived messages have NO overlap with retained
|
||||
if archive_fn.called:
|
||||
archived = archive_fn.call_args.args[0]
|
||||
archived_ids = set(id(m) for m in archived)
|
||||
retained_ids = set(id(m) for m in session.messages)
|
||||
assert not archived_ids & retained_ids, (
|
||||
f"Duplicate messages in archive and retained: "
|
||||
f"overlap contents = {[m['content'] for m in archived if id(m) in retained_ids]}"
|
||||
)
|
||||
|
||||
|
||||
def test_enforce_file_cap_no_message_loss_in_else_branch():
|
||||
"""In the else branch, no messages should silently disappear — every
|
||||
message must be either retained or archived."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
session = Session(key="test:else-no-loss")
|
||||
all_messages = []
|
||||
for i in range(15):
|
||||
msg = {"role": "user", "content": f"u{i}"}
|
||||
session.messages.append(msg)
|
||||
all_messages.append(msg)
|
||||
for i in range(10):
|
||||
msg = {"role": "assistant", "content": f"a{i}"}
|
||||
session.messages.append(msg)
|
||||
all_messages.append(msg)
|
||||
|
||||
archive_fn = MagicMock()
|
||||
session.enforce_file_cap(on_archive=archive_fn, limit=6)
|
||||
|
||||
# Collect all messages accounted for (retained + archived)
|
||||
accounted = set(id(m) for m in session.messages)
|
||||
if archive_fn.called:
|
||||
for m in archive_fn.call_args.args[0]:
|
||||
accounted.add(id(m))
|
||||
|
||||
all_ids = set(id(m) for m in all_messages)
|
||||
missing = all_ids - accounted
|
||||
assert not missing, (
|
||||
f"Lost {len(missing)} message(s) — neither retained nor archived"
|
||||
)
|
||||
|
||||
|
||||
def test_enforce_file_cap_correct_archive_with_last_consolidated_in_else_branch():
|
||||
"""When last_consolidated > 0 and the else branch fires, only the
|
||||
unconsolidated dropped messages should be raw-archived. Messages in the
|
||||
consolidated prefix that are dropped do NOT need raw archiving."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
session = Session(key="test:else-lc-archive")
|
||||
# 20 messages total: u0..u9 (user), a0..a9 (assistant)
|
||||
for i in range(10):
|
||||
session.messages.append({"role": "user", "content": f"u{i}"})
|
||||
for i in range(10):
|
||||
session.messages.append({"role": "assistant", "content": f"a{i}"})
|
||||
# First 8 messages already consolidated
|
||||
session.last_consolidated = 8
|
||||
|
||||
archive_fn = MagicMock()
|
||||
session.enforce_file_cap(on_archive=archive_fn, limit=4)
|
||||
|
||||
if archive_fn.called:
|
||||
archived = archive_fn.call_args.args[0]
|
||||
# Archived messages should NOT include any from the consolidated prefix
|
||||
# (u0..u7). They should only be unconsolidated dropped messages.
|
||||
archived_contents = [m["content"] for m in archived]
|
||||
for c in archived_contents:
|
||||
assert c not in [f"u{i}" for i in range(8)], (
|
||||
f"Consolidated message {c!r} should not be raw-archived"
|
||||
)
|
||||
|
||||
|
||||
def test_enforce_file_cap_restores_session_when_archive_fails():
|
||||
state = ProviderConversationState(
|
||||
kind="openai_responses",
|
||||
provider="openai:test",
|
||||
model="test-model",
|
||||
version=1,
|
||||
payload={"items": []},
|
||||
)
|
||||
session = Session(key="test:archive-failure", provider_state=state)
|
||||
for i in range(8):
|
||||
session.messages.append({"role": "user", "content": f"msg{i}"})
|
||||
original_messages = session.messages
|
||||
original_updated_at = session.updated_at
|
||||
session.last_consolidated = 2
|
||||
|
||||
def fail_archive(_messages):
|
||||
raise RuntimeError("history unavailable")
|
||||
|
||||
with pytest.raises(RuntimeError, match="history unavailable"):
|
||||
session.enforce_file_cap(on_archive=fail_archive, limit=4)
|
||||
|
||||
assert session.messages is original_messages
|
||||
assert [message["content"] for message in session.messages] == [
|
||||
f"msg{i}" for i in range(8)
|
||||
]
|
||||
assert session.last_consolidated == 2
|
||||
assert session.provider_state is state
|
||||
assert session.updated_at == original_updated_at
|
||||
|
||||
|
||||
def test_retain_recent_legal_suffix_last_consolidated_correct_in_else_branch():
|
||||
"""last_consolidated after retain_recent_legal_suffix should reflect how
|
||||
many retained messages were inside the old consolidated prefix."""
|
||||
|
||||
@@ -167,40 +167,6 @@ def test_retain_drops_delivery_not_adjacent_to_anchor_user():
|
||||
assert _contents(session.messages) == ["ok", "great"]
|
||||
|
||||
|
||||
# --- Delivery preservation through the production entry points ---
|
||||
|
||||
|
||||
def test_enforce_file_cap_keeps_delivery_in_session():
|
||||
session = Session(key="test:cap-delivery")
|
||||
session.messages.append({"role": "user", "content": "setup"})
|
||||
session.messages.append(_delivery("Remember to drink water"))
|
||||
session.messages.append({"role": "user", "content": "ok"})
|
||||
session.messages.append({"role": "assistant", "content": "great"})
|
||||
|
||||
archived: list[list[dict]] = []
|
||||
session.enforce_file_cap(on_archive=archived.append, limit=3)
|
||||
|
||||
archived_flat = [m for chunk in archived for m in chunk]
|
||||
assert _has_delivery(session.messages)
|
||||
assert not any(m.get("_channel_delivery") for m in archived_flat)
|
||||
|
||||
|
||||
def test_enforce_file_cap_archives_only_prefix():
|
||||
session = Session(key="test:cap-prefix")
|
||||
session.messages.append({"role": "user", "content": "setup"})
|
||||
session.messages.append({"role": "assistant", "content": "first reply"})
|
||||
session.messages.append(_delivery("Remember to drink water"))
|
||||
session.messages.append({"role": "user", "content": "ok"})
|
||||
session.messages.append({"role": "assistant", "content": "great"})
|
||||
|
||||
archived: list[list[dict]] = []
|
||||
session.enforce_file_cap(on_archive=archived.append, limit=3)
|
||||
|
||||
archived_flat = [m for chunk in archived for m in chunk]
|
||||
assert _has_delivery(session.messages)
|
||||
assert _contents(archived_flat) == ["setup", "first reply"]
|
||||
|
||||
|
||||
def test_compact_probe_keeps_delivery_in_visible_suffix():
|
||||
"""compact_idle_session() trims a probe copy with extend_to_user=True; the
|
||||
visible suffix it keeps must still contain the delivery message."""
|
||||
|
||||
Reference in New Issue
Block a user