diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 753f9c954..bdcd83534 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -807,7 +807,7 @@ _HISTORY_ENTRY_HARD_CAP = 64_000 # emergency cap in append_history class Consolidator: - """Lightweight consolidation: summarizes evicted messages into history.jsonl.""" + """Summarize compacted messages into history.jsonl.""" _MAX_CONSOLIDATION_ROUNDS = 5 @@ -998,14 +998,9 @@ class Consolidator: session_key: str | None = None, summary_messages: list[dict[str, Any]] | None = None, ) -> str | None: - """Summarize messages via LLM and append to history.jsonl. + """Summarize messages and append the result to history.jsonl. - ``messages`` are the messages being archived (removed from the live - session); they are what gets raw-dumped if the LLM call fails. - ``summary_messages``, when given, lets callers include retained - messages in the summary without archiving them. - - Returns the summary text on success, None if nothing to archive. + ``summary_messages`` adds context but is excluded from raw fallback. """ if not messages: return None @@ -1166,13 +1161,7 @@ class Consolidator: runtime: LLMRuntime, max_suffix: int = 8, ) -> str | None: - """Hard-truncate an idle session under the consolidation lock. - - Used by AutoCompact so all session mutation goes through a single - lock-protected path. Returns the summary text on success, ``None`` - if the LLM failed (raw_archive fallback), or ``""`` if there was - nothing to archive. - """ + """Archive an idle prefix and hide it from replay without deleting it.""" lock = self.get_lock(session_key) async with lock: self.sessions.invalidate(session_key) @@ -1192,24 +1181,21 @@ class Consolidator: last_consolidated=0, ) result = probe.retain_recent_legal_suffix(max_suffix, extend_to_user=True) - messages_to_keep = probe.messages - messages_to_remove = result.dropped[result.already_consolidated_count:] + visible_suffix = probe.messages + messages_to_remove = result.dropped - if not messages_to_remove and not messages_to_keep: + if not messages_to_remove: self.sessions.save(session) return "" last_active = session.updated_at - summary: str | None = "" - if messages_to_remove: - # Summarize the retained suffix too, but only remove/raw-dump - # the messages that are no longer kept in the live session. - summary = await self.archive( - messages_to_remove, - runtime=runtime, - session_key=session_key, - summary_messages=messages_to_summarize, - ) + # The visible suffix informs the summary but stays out of raw fallback. + summary = await self.archive( + messages_to_remove, + runtime=runtime, + session_key=session_key, + summary_messages=messages_to_summarize, + ) if summary and summary != "(nothing)": session.metadata["_last_summary"] = { @@ -1217,17 +1203,17 @@ class Consolidator: "last_active": last_active.isoformat(), } - session.messages = messages_to_keep - session.last_consolidated = 0 + # Preserve history and advance only the replay boundary. + session.last_consolidated = len(session.messages) - len(visible_suffix) self.sessions.save(session) - if messages_to_remove: - logger.info( - "Idle-session compact for {}: archived={}, kept={}, summary={}", - session_key, - len(messages_to_remove), - len(messages_to_keep), - bool(summary), - ) + logger.info( + "Idle-session compact for {}: archived={}, visible={}, retained={}, summary={}", + session_key, + len(messages_to_remove), + len(visible_suffix), + len(session.messages), + bool(summary), + ) return summary diff --git a/tests/agent/test_auto_compact.py b/tests/agent/test_auto_compact.py index 957d04989..4a53a711c 100644 --- a/tests/agent/test_auto_compact.py +++ b/tests/agent/test_auto_compact.py @@ -80,7 +80,6 @@ def _make_fake_compact( track_archived: list | None = None, track_count: bool = False, ): - """Return a fake compact_idle_session that mirrors the real method's session mutation.""" from nanobot.session.manager import Session as _Session state = {"count": 0} @@ -106,21 +105,20 @@ def _make_fake_compact( max_suffix, extend_to_user=True, ) - kept = probe.messages - archive_msgs = result.dropped[result.already_consolidated_count:] + visible_suffix = probe.messages + archive_msgs = result.dropped - if not archive_msgs and not kept: + if not archive_msgs: loop.sessions.save(session) return "" last_active = session.updated_at s = summary - if archive_msgs: - if on_archive: - result = on_archive(archive_msgs) - s = result if isinstance(result, str) else summary - if track_archived is not None: - track_archived.extend(archive_msgs) + if on_archive: + result = on_archive(archive_msgs) + s = result if isinstance(result, str) else summary + if track_archived is not None: + track_archived.extend(archive_msgs) if s and s != "(nothing)": session.metadata["_last_summary"] = { @@ -128,8 +126,7 @@ def _make_fake_compact( "last_active": last_active.isoformat(), } - session.messages = kept - session.last_consolidated = 0 + session.last_consolidated = len(session.messages) - len(visible_suffix) loop.sessions.save(session) return s @@ -368,8 +365,7 @@ class TestAutoCompact: await loop.close_mcp() @pytest.mark.asyncio - async def test_auto_compact_archives_prefix_and_keeps_recent_suffix(self, tmp_path): - """_archive should summarize the old prefix and keep a recent legal suffix.""" + async def test_auto_compact_archives_prefix_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) @@ -384,9 +380,12 @@ class TestAutoCompact: assert len(archived_messages) == 4 session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES - assert session_after.messages[0]["content"] == "msg user 2" - assert session_after.messages[-1]["content"] == "msg assistant 5" + assert len(session_after.messages) == 12 + assert session_after.messages[0]["content"] == "msg user 0" + visible = session_after.get_history(max_messages=12) + assert len(visible) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert visible[0]["content"] == "msg user 2" + assert visible[-1]["content"] == "msg assistant 5" await loop.close_mcp() @pytest.mark.asyncio @@ -403,17 +402,19 @@ class TestAutoCompact: await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime()) session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) > loop.auto_compact._RECENT_SUFFIX_MESSAGES - assert session_after.messages[0]["content"] == "record this" - assert session_after.messages[-1]["content"] == "done" + assert session_after.messages[0]["content"] == "old user 0" + visible = session_after.get_history(max_messages=len(session_after.messages)) + assert len(visible) > loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert visible[0]["content"] == "record this" + assert visible[-1]["content"] == "done" tool_results = { m.get("tool_call_id") - for m in session_after.messages + for m in visible if m.get("role") == "tool" } assert all( tc["id"] in tool_results - for m in session_after.messages + for m in visible for tc in (m.get("tool_calls") or []) ) await loop.close_mcp() @@ -436,7 +437,10 @@ class TestAutoCompact: assert entry is not None assert entry[0] == "User said hello." session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(session_after.messages) == 12 + assert len(session_after.get_history(max_messages=12)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) await loop.close_mcp() @pytest.mark.asyncio @@ -474,11 +478,10 @@ class TestAutoCompact: class TestAutoCompactIdleDetection: - """Test idle detection triggers auto-new in _process_message.""" + """Idle detection tests.""" @pytest.mark.asyncio async def test_no_auto_compact_when_ttl_disabled(self, tmp_path): - """No auto-new should happen when TTL is 0 (disabled).""" loop = _make_loop(tmp_path, session_ttl_minutes=0) session = loop.sessions.get_or_create("cli:test") session.add_message("user", "old message") @@ -494,7 +497,6 @@ class TestAutoCompactIdleDetection: @pytest.mark.asyncio async def test_auto_compact_triggers_on_idle(self, tmp_path): - """Proactive auto-new archives expired session; _process_message reloads it.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") _add_turns(session, 6, prefix="old") @@ -514,13 +516,16 @@ class TestAutoCompactIdleDetection: session_after = loop.sessions.get_or_create("cli:test") assert len(archived_messages) == 4 - assert not any(m["content"] == "old user 0" for m in session_after.messages) + assert any(m["content"] == "old user 0" for m in session_after.messages) + assert not any( + m["content"] == "old user 0" + for m in session_after.get_history(max_messages=len(session_after.messages)) + ) assert any(m["content"] == "new msg" for m in session_after.messages) await loop.close_mcp() @pytest.mark.asyncio async def test_no_auto_compact_when_active(self, tmp_path): - """No auto-new should happen when session is recently active.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") session.add_message("user", "recent message") @@ -558,7 +563,6 @@ class TestAutoCompactIdleDetection: @pytest.mark.asyncio async def test_auto_compact_with_slash_new(self, tmp_path): - """Auto-new fires before /new dispatches; session is cleared twice but idempotent.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") for i in range(4): @@ -576,7 +580,6 @@ class TestAutoCompactIdleDetection: assert "new session started" in response.content.lower() session_after = loop.sessions.get_or_create("cli:test") - # Session is empty (auto-new archived and cleared, /new cleared again) assert len(session_after.messages) == 0 await loop.close_mcp() @@ -617,11 +620,10 @@ class TestAutoCompactIdleDetection: class TestAutoCompactSystemMessages: - """Test that auto-new also works for system messages.""" + """System-message idle compaction tests.""" @pytest.mark.asyncio async def test_auto_compact_triggers_for_system_messages(self, tmp_path): - """Proactive auto-new archives expired session; system messages reload it.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") _add_turns(session, 6, prefix="old") @@ -640,9 +642,10 @@ class TestAutoCompactSystemMessages: await loop._process_message(msg) session_after = loop.sessions.get_or_create("cli:test") + assert any(m["content"] == "old user 0" for m in session_after.messages) assert not any( m["content"] == "old user 0" - for m in session_after.messages + for m in session_after.get_history(max_messages=len(session_after.messages)) ) await loop.close_mcp() @@ -652,7 +655,6 @@ class TestAutoCompactEdgeCases: @pytest.mark.asyncio async def test_auto_compact_with_nothing_summary(self, tmp_path): - """Auto-new should not inject when archive produces '(nothing)'.""" loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") _add_turns(session, 6, prefix="thanks") @@ -666,15 +668,17 @@ class TestAutoCompactEdgeCases: await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime()) session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(session_after.messages) == 12 + assert len(session_after.get_history(max_messages=12)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) # "(nothing)" summary should not be stored assert "cli:test" not in loop.auto_compact._summaries await loop.close_mcp() @pytest.mark.asyncio - async def test_auto_compact_archive_failure_still_keeps_recent_suffix(self, tmp_path): - """Auto-new should keep the recent suffix even if LLM archive falls back to raw dump.""" + async def test_auto_compact_archive_failure_preserves_raw_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, prefix="important") @@ -687,7 +691,10 @@ class TestAutoCompactEdgeCases: await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime()) session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(session_after.messages) == 12 + assert len(session_after.get_history(max_messages=12)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) await loop.close_mcp() @@ -725,13 +732,10 @@ class TestAutoCompactEdgeCases: class TestAutoCompactIntegration: - """End-to-end test of auto session new feature.""" + """Idle compaction integration tests.""" @pytest.mark.asyncio async def test_full_lifecycle(self, tmp_path): - """ - Full lifecycle: messages -> idle -> auto-new -> archive -> clear -> summary injected as runtime context. - """ loop = _make_loop(tmp_path, session_ttl_minutes=15) session = loop.sessions.get_or_create("cli:test") @@ -759,6 +763,7 @@ class TestAutoCompactIntegration: tool_calls=[], ) ) + await loop.auto_compact._archive("cli:test", runtime=loop.llm_runtime()) msg = InboundMessage( channel="cli", sender_id="user", chat_id="test", @@ -769,9 +774,13 @@ class TestAutoCompactIntegration: # Phase 4: Verify session_after = loop.sessions.get_or_create("cli:test") - # The oldest messages should be trimmed from live session history + assert any( + "past tense is used" in str(m.get("content", "")).lower() + for m in session_after.messages + ) assert not any( - "past tense is used" in str(m.get("content", "")) for m in session_after.messages + "past tense is used" in str(m.get("content", "")).lower() + for m in session_after.get_history(max_messages=len(session_after.messages)) ) # Summary should NOT be persisted in session (ephemeral, one-shot) @@ -821,7 +830,7 @@ class TestAutoCompactIntegration: class TestProactiveAutoCompact: - """Test proactive auto-new on idle ticks (TimeoutError path in run loop).""" + """Proactive idle compaction tests.""" @staticmethod async def _run_check_expired(loop, active_session_keys=()): @@ -899,7 +908,10 @@ class TestProactiveAutoCompact: await self._run_check_expired(loop) session_after = loop.sessions.get_or_create("cli:test") - assert len(session_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(session_after.messages) == 10 + assert len(session_after.get_history(max_messages=10)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) assert len(archived_messages) == 2 entry = loop.auto_compact._summaries.get("cli:test") assert entry is not None @@ -1082,7 +1094,10 @@ class TestProactiveAutoCompact: assert _fake_compact.state["count"] == 1 s1_after = loop.sessions.get_or_create("cli:expired_idle") - assert len(s1_after.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(s1_after.messages) == 12 + assert len(s1_after.get_history(max_messages=12)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) s2_after = loop.sessions.get_or_create("cli:expired_active") assert len(s2_after.messages) == 12 # Preserved s3_after = loop.sessions.get_or_create("cli:recent") @@ -1211,7 +1226,10 @@ class TestSummaryPersistence: # prepare_session should recover summary from metadata reloaded = loop.sessions.get_or_create("cli:test") - assert len(reloaded.messages) == loop.auto_compact._RECENT_SUFFIX_MESSAGES + assert len(reloaded.messages) == 12 + assert len(reloaded.get_history(max_messages=12)) == ( + loop.auto_compact._RECENT_SUFFIX_MESSAGES + ) _, summary = loop.auto_compact.prepare_session(reloaded, "cli:test") assert summary is not None diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 9f17285c9..51ff4744d 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -586,7 +586,7 @@ class TestConsolidatorTokenBudget: class TestCompactIdleSession: - """Tests for Consolidator.compact_idle_session — lock-protected idle truncation.""" + """Idle compaction tests.""" @pytest.fixture def real_consolidator(self, store, mock_provider): @@ -602,11 +602,9 @@ class TestCompactIdleSession: ) @pytest.mark.asyncio - async def test_archives_prefix_keeps_suffix( + async def test_archives_prefix_preserves_messages_and_hides_prefix( self, real_consolidator, mock_provider, runtime ): - """20 user/assistant turns → compact with max_suffix=8 → messages ≤ 8, - last_consolidated=0, _last_summary stored.""" mock_provider.chat_with_retry.return_value = MagicMock( content="Summary of old conversation.", finish_reason="stop" ) @@ -624,9 +622,15 @@ class TestCompactIdleSession: ) assert result == "Summary of old conversation." + sessions.invalidate("cli:test") reloaded = sessions.get_or_create("cli:test") - assert len(reloaded.messages) <= 8 - assert reloaded.last_consolidated == 0 + assert len(reloaded.messages) == 40 + assert reloaded.messages[0]["content"] == "user msg 0" + assert reloaded.last_consolidated == 32 + visible = reloaded.get_history(max_messages=40) + assert len(visible) == 8 + assert visible[0]["content"] == "user msg 16" + assert visible[-1]["content"] == "assistant msg 19" meta = reloaded.metadata.get("_last_summary") assert meta is not None assert meta["text"] == "Summary of old conversation." @@ -665,9 +669,7 @@ class TestCompactIdleSession: async def test_raw_dumps_only_dropped_messages_on_llm_failure( self, real_consolidator, mock_provider, store, runtime ): - """Summarizing over the full tail must not widen what gets raw-dumped on - LLM failure: the breadcrumb should contain only the removed prefix, not - the retained suffix that stays live in the session. Regression for #4264.""" + """Extra summary context must not enter raw fallback. Regression for #4264.""" mock_provider.chat_with_retry.side_effect = RuntimeError("LLM unavailable") sessions = real_consolidator.sessions session = sessions.get_or_create("cli:rawdrop") @@ -684,8 +686,11 @@ 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 # removed prefix is the breadcrumb - assert "RETAINED_SUFFIX_marker" not in raw # retained suffix not dumped + assert "user msg 0" in raw + assert "RETAINED_SUFFIX_marker" not in raw + reloaded = sessions.get_or_create("cli:rawdrop") + assert len(reloaded.messages) == 38 + assert reloaded.messages[-1]["content"] == "RETAINED_SUFFIX_marker" @pytest.mark.asyncio async def test_idle_compact_writes_session_key_to_history( @@ -757,10 +762,9 @@ class TestCompactIdleSession: assert "_last_summary" not in reloaded.metadata @pytest.mark.asyncio - async def test_llm_failure_still_truncates( + async def test_llm_failure_preserves_history_but_advances_replay_boundary( self, real_consolidator, mock_provider, store, runtime ): - """LLM raises RuntimeError → raw_archive fires, session still truncated, returns None.""" mock_provider.chat_with_retry.side_effect = RuntimeError("LLM unavailable") sessions = real_consolidator.sessions session = sessions.get_or_create("cli:fail") @@ -778,9 +782,16 @@ class TestCompactIdleSession: entries = store.read_unprocessed_history(since_cursor=0) assert any("[RAW]" in e["content"] for e in entries) - # Session should still be truncated reloaded = sessions.get_or_create("cli:fail") - assert len(reloaded.messages) <= 4 + assert len(reloaded.messages) == 20 + assert reloaded.messages[0]["content"] == "u0" + assert reloaded.last_consolidated == 16 + assert [m["content"] for m in reloaded.get_history(max_messages=20)] == [ + "u8", + "a8", + "u9", + "a9", + ] @pytest.mark.asyncio async def test_respects_last_consolidated( @@ -802,6 +813,9 @@ class TestCompactIdleSession: "cli:offset", runtime=runtime, max_suffix=4 ) assert result == "Tail summary." + reloaded = sessions.get_or_create("cli:offset") + assert len(reloaded.messages) == 60 + assert reloaded.last_consolidated == 56 # Verify only the unconsolidated tail was processed: # 10 unconsolidated messages (50-59), keep suffix of 4 → archive 6 @@ -812,14 +826,12 @@ class TestCompactIdleSession: assert "u25" in user_content or "a25" in user_content @pytest.mark.asyncio - async def test_non_contiguous_suffix_archives_actual_dropped_messages( + async def test_extended_suffix_archives_only_hidden_prefix( self, real_consolidator, mock_provider, runtime, ): - """Assistant-only tails extend back to the latest user turn, so archive - the actual dropped messages rather than a computed prefix.""" mock_provider.chat_with_retry.return_value = MagicMock( content="Tail summary.", finish_reason="stop" ) @@ -837,7 +849,9 @@ class TestCompactIdleSession: assert result == "Tail summary." reloaded = sessions.get_or_create("cli:noncontiguous") - assert [m["content"] for m in reloaded.messages] == [ + assert len(reloaded.messages) == 25 + assert reloaded.last_consolidated == 14 + assert [m["content"] for m in reloaded.get_history(max_messages=25)] == [ "user-14", "assistant-00", "assistant-01", @@ -987,23 +1001,21 @@ class TestConsolidatorSessionRefresh: # Simulate: background consolidation captures old reference old_ref = session - # AutoCompact runs first and truncates to 8 await consolidator.compact_idle_session( "cli:test", runtime=runtime, max_suffix=8, ) - # Background consolidation runs with stale reference — - # should detect the session was replaced and not undo the compact. await consolidator.maybe_consolidate_by_tokens( old_ref, runtime=runtime, ) session_after = sessions.get_or_create("cli:test") - # Messages should still be truncated (not restored to 40) - assert len(session_after.messages) <= 8 + assert len(session_after.messages) == 40 + assert session_after.last_consolidated == 32 + assert len(session_after.get_history(max_messages=40)) == 8 class TestRawArchiveTruncation: