mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-07 21:08:34 +03:00
fix(memory): archive short idle sessions for Dream
This commit is contained in:
@@ -80,8 +80,6 @@ def _make_fake_compact(
|
||||
track_archived: list | None = None,
|
||||
track_count: bool = False,
|
||||
):
|
||||
from nanobot.session.manager import Session as _Session
|
||||
|
||||
state = {"count": 0}
|
||||
|
||||
async def _fake_compact(key: str, *, runtime, max_suffix: int = 8) -> str:
|
||||
@@ -92,25 +90,8 @@ def _make_fake_compact(
|
||||
if not tail:
|
||||
loop.sessions.save(session)
|
||||
return ""
|
||||
|
||||
probe = _Session(
|
||||
key=session.key,
|
||||
messages=tail.copy(),
|
||||
created_at=session.created_at,
|
||||
updated_at=session.updated_at,
|
||||
metadata={},
|
||||
last_consolidated=0,
|
||||
)
|
||||
result = probe.retain_recent_legal_suffix(
|
||||
max_suffix,
|
||||
extend_to_user=True,
|
||||
)
|
||||
visible_suffix = probe.messages
|
||||
archive_msgs = result.dropped
|
||||
|
||||
if not archive_msgs:
|
||||
loop.sessions.save(session)
|
||||
return ""
|
||||
archive_end = session.last_consolidated + len(tail)
|
||||
archive_msgs = tail
|
||||
|
||||
last_active = session.updated_at
|
||||
s = summary
|
||||
@@ -126,7 +107,7 @@ def _make_fake_compact(
|
||||
"last_active": last_active.isoformat(),
|
||||
}
|
||||
|
||||
session.last_consolidated = len(session.messages) - len(visible_suffix)
|
||||
session.last_consolidated = archive_end
|
||||
loop.sessions.save(session)
|
||||
return s
|
||||
|
||||
@@ -365,7 +346,7 @@ class TestAutoCompact:
|
||||
await loop.close_mcp()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_compact_archives_prefix_without_deleting_history(self, tmp_path):
|
||||
async def test_auto_compact_archives_full_tail_without_deleting_history(self, tmp_path):
|
||||
loop = _make_loop(tmp_path, session_ttl_minutes=15)
|
||||
session = loop.sessions.get_or_create("cli:test")
|
||||
_add_turns(session, 6)
|
||||
@@ -378,7 +359,7 @@ class TestAutoCompact:
|
||||
|
||||
await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime())
|
||||
|
||||
assert len(archived_messages) == 4
|
||||
assert len(archived_messages) == 12
|
||||
session_after = loop.sessions.get_or_create("cli:test")
|
||||
assert len(session_after.messages) == 12
|
||||
assert session_after.messages[0]["content"] == "msg user 0"
|
||||
@@ -473,7 +454,7 @@ class TestAutoCompact:
|
||||
|
||||
await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime())
|
||||
|
||||
assert len(archived_messages) == 2
|
||||
assert len(archived_messages) == 10
|
||||
await loop.close_mcp()
|
||||
|
||||
|
||||
@@ -515,7 +496,7 @@ class TestAutoCompactIdleDetection:
|
||||
await loop._process_message(msg)
|
||||
|
||||
session_after = loop.sessions.get_or_create("cli:test")
|
||||
assert len(archived_messages) == 4
|
||||
assert len(archived_messages) == 12
|
||||
assert any(m["content"] == "old user 0" for m in session_after.messages)
|
||||
assert not any(
|
||||
m["content"] == "old user 0"
|
||||
@@ -724,7 +705,7 @@ class TestAutoCompactEdgeCases:
|
||||
await loop._process_message(msg)
|
||||
|
||||
session_after = loop.sessions.get_or_create("cli:test")
|
||||
assert archived_messages == []
|
||||
assert [message["content"] for message in archived_messages] == ["previous message"]
|
||||
assert any(m["content"] == "previous message" for m in session_after.messages)
|
||||
assert any(m["content"] == "interrupted response" for m in session_after.messages)
|
||||
|
||||
@@ -912,7 +893,7 @@ class TestProactiveAutoCompact:
|
||||
assert len(session_after.get_history(max_messages=10)) == (
|
||||
loop.auto_compact._RECENT_SUFFIX_MESSAGES
|
||||
)
|
||||
assert len(archived_messages) == 2
|
||||
assert len(archived_messages) == 10
|
||||
entry = loop.auto_compact._summaries.get("cli:test")
|
||||
assert entry is not None
|
||||
assert entry[0] == "User chatted about old things."
|
||||
|
||||
@@ -405,13 +405,37 @@ class TestCheckExpired:
|
||||
scheduler.assert_not_called()
|
||||
assert "dream:20260602-155256" not in ac._archiving
|
||||
|
||||
def test_already_trimmed_session_skips(self):
|
||||
"""Expired session with no removable tail should not be re-scheduled."""
|
||||
def test_short_unarchived_session_schedules(self):
|
||||
"""A short idle session still needs an archive entry for Dream."""
|
||||
ac = _make_autocompact(ttl=15)
|
||||
mock_sm = MagicMock(spec=SessionManager)
|
||||
last_active = datetime(2026, 1, 1, 10, 0, 0)
|
||||
session = _make_session("cli:short", updated_at=last_active)
|
||||
_add_turns(session, 2)
|
||||
mock_sm.list_sessions.return_value = [
|
||||
{"key": "cli:short", "updated_at": last_active.isoformat()},
|
||||
]
|
||||
mock_sm.get_or_create.return_value = session
|
||||
ac.sessions = mock_sm
|
||||
|
||||
scheduled = []
|
||||
|
||||
def scheduler(coro):
|
||||
scheduled.append(coro)
|
||||
coro.close()
|
||||
|
||||
ac.check_expired(scheduler, _runtime)
|
||||
|
||||
assert len(scheduled) == 1
|
||||
assert ac._archiving == {"cli:short"}
|
||||
|
||||
def test_fully_archived_session_skips(self):
|
||||
ac = _make_autocompact(ttl=15)
|
||||
mock_sm = MagicMock(spec=SessionManager)
|
||||
last_active = datetime(2026, 1, 1, 10, 0, 0)
|
||||
session = _make_session("cli:done", updated_at=last_active)
|
||||
_add_turns(session, 2)
|
||||
session.last_consolidated = len(session.messages)
|
||||
mock_sm.list_sessions.return_value = [
|
||||
{"key": "cli:done", "updated_at": last_active.isoformat()},
|
||||
]
|
||||
|
||||
@@ -391,6 +391,25 @@ class TestConsolidatorTokenBudget:
|
||||
assert len(captured["history"]) == 160
|
||||
assert captured["history"][0]["content"].endswith("msg-0")
|
||||
|
||||
async def test_estimate_includes_recent_archived_replay(self, consolidator, runtime):
|
||||
session = Session(key="test:archived-replay")
|
||||
for i in range(10):
|
||||
session.add_message("user", f"msg-{i}")
|
||||
session.last_consolidated = len(session.messages)
|
||||
|
||||
captured: dict[str, list[dict]] = {}
|
||||
|
||||
def build_messages(**kwargs):
|
||||
captured["history"] = kwargs["history"]
|
||||
return kwargs["history"]
|
||||
|
||||
consolidator._build_messages = build_messages
|
||||
|
||||
consolidator.estimate_session_prompt_tokens(session, runtime=runtime)
|
||||
|
||||
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,
|
||||
@@ -620,7 +639,7 @@ class TestCompactIdleSession:
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_archives_prefix_preserves_messages_and_hides_prefix(
|
||||
async def test_archives_full_tail_preserves_messages_and_replays_recent_suffix(
|
||||
self, real_consolidator, mock_provider, runtime
|
||||
):
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(
|
||||
@@ -645,7 +664,7 @@ class TestCompactIdleSession:
|
||||
reloaded = sessions.get_or_create("cli:test")
|
||||
assert len(reloaded.messages) == 40
|
||||
assert reloaded.messages[0]["content"] == "user msg 0"
|
||||
assert reloaded.last_consolidated == 32
|
||||
assert reloaded.last_consolidated == 40
|
||||
assert reloaded.provider_state is None
|
||||
visible = reloaded.get_history(max_messages=40)
|
||||
assert len(visible) == 8
|
||||
@@ -657,6 +676,82 @@ class TestCompactIdleSession:
|
||||
assert "last_active" in meta
|
||||
assert reloaded.updated_at == old_ts
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_short_idle_session_archives_once(
|
||||
self, real_consolidator, mock_provider, store, runtime
|
||||
):
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(
|
||||
content="Short summary.", finish_reason="stop"
|
||||
)
|
||||
sessions = real_consolidator.sessions
|
||||
session = sessions.get_or_create("cli:short")
|
||||
session.add_message("user", "hello")
|
||||
session.add_message("assistant", "hi")
|
||||
sessions.save(session)
|
||||
|
||||
first = await real_consolidator.compact_idle_session("cli:short", runtime=runtime)
|
||||
second = await real_consolidator.compact_idle_session("cli:short", runtime=runtime)
|
||||
|
||||
assert first == "Short summary."
|
||||
assert second == ""
|
||||
mock_provider.chat_with_retry.assert_awaited_once()
|
||||
assert len(store.read_unprocessed_history(since_cursor=0)) == 1
|
||||
reloaded = sessions.get_or_create("cli:short")
|
||||
assert reloaded.last_consolidated == 2
|
||||
assert [message["content"] for message in reloaded.get_history()] == ["hello", "hi"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_new_messages_advance_existing_archive_progress(
|
||||
self, real_consolidator, mock_provider, runtime
|
||||
):
|
||||
mock_provider.chat_with_retry.return_value = MagicMock(
|
||||
content="Summary.", finish_reason="stop"
|
||||
)
|
||||
sessions = real_consolidator.sessions
|
||||
session = sessions.get_or_create("cli:incremental")
|
||||
session.add_message("user", "first user")
|
||||
session.add_message("assistant", "first assistant")
|
||||
sessions.save(session)
|
||||
|
||||
await real_consolidator.compact_idle_session("cli:incremental", runtime=runtime)
|
||||
current = sessions.get_or_create("cli:incremental")
|
||||
current.add_message("user", "second user")
|
||||
current.add_message("assistant", "second assistant")
|
||||
sessions.save(current)
|
||||
await real_consolidator.compact_idle_session("cli:incremental", runtime=runtime)
|
||||
|
||||
assert mock_provider.chat_with_retry.await_count == 2
|
||||
latest_prompt = mock_provider.chat_with_retry.await_args_list[-1].kwargs["messages"][1][
|
||||
"content"
|
||||
]
|
||||
assert "second user" in latest_prompt
|
||||
assert "first user" not in latest_prompt
|
||||
assert sessions.get_or_create("cli:incremental").last_consolidated == 4
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_append_remains_unarchived(
|
||||
self, real_consolidator, mock_provider, runtime
|
||||
):
|
||||
sessions = real_consolidator.sessions
|
||||
session = sessions.get_or_create("cli:concurrent")
|
||||
session.add_message("user", "captured user")
|
||||
session.add_message("assistant", "captured assistant")
|
||||
sessions.save(session)
|
||||
|
||||
async def append_during_archive(**_kwargs):
|
||||
current = sessions.get_or_create("cli:concurrent")
|
||||
current.add_message("user", "late user")
|
||||
current.add_message("assistant", "late assistant")
|
||||
return LLMResponse(content="Summary.", finish_reason="stop")
|
||||
|
||||
mock_provider.chat_with_retry.side_effect = append_during_archive
|
||||
|
||||
await real_consolidator.compact_idle_session("cli:concurrent", runtime=runtime)
|
||||
|
||||
reloaded = sessions.get_or_create("cli:concurrent")
|
||||
assert len(reloaded.messages) == 4
|
||||
assert reloaded.last_consolidated == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_summarizes_retained_suffix_not_just_dropped_prefix(
|
||||
self, real_consolidator, mock_provider, runtime
|
||||
@@ -686,10 +781,10 @@ class TestCompactIdleSession:
|
||||
assert "CORRECTED_FINAL_RESULT_alpha" in summarized
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_raw_dumps_only_dropped_messages_on_llm_failure(
|
||||
async def test_raw_dumps_full_archive_batch_on_llm_failure(
|
||||
self, real_consolidator, mock_provider, store, runtime
|
||||
):
|
||||
"""Extra summary context must not enter raw fallback. Regression for #4264."""
|
||||
"""The fallback covers the same full range as successful idle archival."""
|
||||
mock_provider.chat_with_retry.side_effect = RuntimeError("LLM unavailable")
|
||||
sessions = real_consolidator.sessions
|
||||
session = sessions.get_or_create("cli:rawdrop")
|
||||
@@ -707,7 +802,7 @@ class TestCompactIdleSession:
|
||||
raw = "\n".join(e["content"] for e in store.read_unprocessed_history(since_cursor=0))
|
||||
assert "[RAW]" in raw
|
||||
assert "user msg 0" in raw
|
||||
assert "RETAINED_SUFFIX_marker" not in raw
|
||||
assert "RETAINED_SUFFIX_marker" in raw
|
||||
reloaded = sessions.get_or_create("cli:rawdrop")
|
||||
assert len(reloaded.messages) == 38
|
||||
assert reloaded.messages[-1]["content"] == "RETAINED_SUFFIX_marker"
|
||||
@@ -805,8 +900,12 @@ class TestCompactIdleSession:
|
||||
reloaded = sessions.get_or_create("cli:fail")
|
||||
assert len(reloaded.messages) == 20
|
||||
assert reloaded.messages[0]["content"] == "u0"
|
||||
assert reloaded.last_consolidated == 16
|
||||
assert reloaded.last_consolidated == 20
|
||||
assert [m["content"] for m in reloaded.get_history(max_messages=20)] == [
|
||||
"u6",
|
||||
"a6",
|
||||
"u7",
|
||||
"a7",
|
||||
"u8",
|
||||
"a8",
|
||||
"u9",
|
||||
@@ -835,10 +934,10 @@ class TestCompactIdleSession:
|
||||
assert result == "Tail summary."
|
||||
reloaded = sessions.get_or_create("cli:offset")
|
||||
assert len(reloaded.messages) == 60
|
||||
assert reloaded.last_consolidated == 56
|
||||
assert reloaded.last_consolidated == 60
|
||||
|
||||
# Verify only the unconsolidated tail was processed:
|
||||
# 10 unconsolidated messages (50-59), keep suffix of 4 → archive 6
|
||||
# All 10 unconsolidated messages (50-59) are archived exactly once.
|
||||
archived_call = mock_provider.chat_with_retry.call_args
|
||||
user_content = archived_call.kwargs["messages"][1]["content"]
|
||||
# Should contain only tail messages, not early ones
|
||||
@@ -846,7 +945,7 @@ class TestCompactIdleSession:
|
||||
assert "u25" in user_content or "a25" in user_content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_extended_suffix_archives_only_hidden_prefix(
|
||||
async def test_full_archive_keeps_extended_legal_replay_suffix(
|
||||
self,
|
||||
real_consolidator,
|
||||
mock_provider,
|
||||
@@ -870,7 +969,7 @@ class TestCompactIdleSession:
|
||||
|
||||
reloaded = sessions.get_or_create("cli:noncontiguous")
|
||||
assert len(reloaded.messages) == 25
|
||||
assert reloaded.last_consolidated == 14
|
||||
assert reloaded.last_consolidated == 25
|
||||
assert [m["content"] for m in reloaded.get_history(max_messages=25)] == [
|
||||
"user-14",
|
||||
"assistant-00",
|
||||
@@ -1034,7 +1133,7 @@ class TestConsolidatorSessionRefresh:
|
||||
|
||||
session_after = sessions.get_or_create("cli:test")
|
||||
assert len(session_after.messages) == 40
|
||||
assert session_after.last_consolidated == 32
|
||||
assert session_after.last_consolidated == 40
|
||||
assert len(session_after.get_history(max_messages=40)) == 8
|
||||
|
||||
|
||||
|
||||
@@ -208,6 +208,71 @@ def test_orphan_trim_with_last_consolidated():
|
||||
assert all(m.get("role") != "tool" or m["tool_call_id"].startswith("new_") for m in history)
|
||||
|
||||
|
||||
def test_get_history_replays_recent_messages_after_full_archive():
|
||||
session = Session(key="test:fully-archived")
|
||||
for i in range(10):
|
||||
session.messages.append({"role": "user", "content": f"u{i}"})
|
||||
session.messages.append({"role": "assistant", "content": f"a{i}"})
|
||||
session.last_consolidated = len(session.messages)
|
||||
|
||||
history = session.get_history(max_messages=100)
|
||||
|
||||
assert [message["content"] for message in history] == [
|
||||
"u6",
|
||||
"a6",
|
||||
"u7",
|
||||
"a7",
|
||||
"u8",
|
||||
"a8",
|
||||
"u9",
|
||||
"a9",
|
||||
]
|
||||
|
||||
|
||||
def test_get_history_extends_compacted_replay_to_preceding_user():
|
||||
session = Session(key="test:compacted-tool-turn")
|
||||
session.messages.extend(
|
||||
[
|
||||
{"role": "user", "content": "old"},
|
||||
{"role": "assistant", "content": "old answer"},
|
||||
{"role": "user", "content": "run tools"},
|
||||
*_tool_turn("keep", 0),
|
||||
*_tool_turn("keep", 1),
|
||||
*_tool_turn("keep", 2),
|
||||
{"role": "assistant", "content": "done"},
|
||||
]
|
||||
)
|
||||
session.last_consolidated = len(session.messages)
|
||||
|
||||
history = session.get_history(max_messages=100)
|
||||
|
||||
assert history[0]["content"] == "run tools"
|
||||
assert history[-1]["content"] == "done"
|
||||
_assert_no_orphans(history)
|
||||
|
||||
|
||||
def test_compacted_tool_turn_can_extend_past_message_cap():
|
||||
session = Session(key="test:long-compacted-tool-turn")
|
||||
session.messages.extend(
|
||||
[
|
||||
{"role": "user", "content": "old"},
|
||||
{"role": "assistant", "content": "old answer"},
|
||||
{"role": "user", "content": "run many tools"},
|
||||
]
|
||||
)
|
||||
for i in range(50):
|
||||
session.messages.extend(_tool_turn("keep", i))
|
||||
session.messages.append({"role": "assistant", "content": "done"})
|
||||
session.last_consolidated = len(session.messages)
|
||||
|
||||
history = session.get_history(max_messages=120)
|
||||
|
||||
assert len(history) > 120
|
||||
assert history[0]["content"] == "run many tools"
|
||||
assert history[-1]["content"] == "done"
|
||||
_assert_no_orphans(history)
|
||||
|
||||
|
||||
# --- Edge: no tool messages at all ---
|
||||
|
||||
def test_no_tool_messages_unchanged():
|
||||
|
||||
@@ -57,7 +57,7 @@ def test_loaded_corrupt_offset_keeps_messages(tmp_path: Path):
|
||||
def test_valid_offset_is_preserved():
|
||||
session = _session(10, 4)
|
||||
assert session.last_consolidated == 4
|
||||
assert len(session.get_history()) == 6
|
||||
assert len(session.get_history()) == 8
|
||||
|
||||
|
||||
def test_loaded_null_metadata_becomes_empty_dict(tmp_path: Path):
|
||||
|
||||
Reference in New Issue
Block a user