From 0863e6e5aba1b4b3884ab44a76306ec4961fc705 Mon Sep 17 00:00:00 2001 From: tangtaizhong666 Date: Sat, 13 Jun 2026 13:07:32 +0800 Subject: [PATCH] fix(memory): summarize full session tail during idle compaction (#4264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Idle compaction summarized only the dropped prefix, excluding the recent suffix it retains. On a finished conversation a late user correction or final result lands in that kept suffix, so it never reached the persisted summary and history kept the stale pre-correction conclusion — which, for idle sessions that are rarely resumed, is never fixed. Summarize over the full unconsolidated tail instead, while still removing (and raw-dumping on LLM failure) only the dropped messages. Adds an opt-in summary_context argument to Consolidator.archive so the summarization window and the archived set can differ without affecting other callers. --- nanobot/agent/memory.py | 22 ++++++++++-- tests/agent/test_consolidator.py | 58 ++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 9ba60bb3..263d77a9 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -799,15 +799,23 @@ class Consolidator: messages: list[dict], *, session_key: str | None = None, + summary_context: list[dict] | None = None, ) -> str | None: """Summarize messages via LLM and append 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_context``, when given, is fed to the summarizer in place of + ``messages`` so a caller can summarize over a wider window (e.g. the + full conversation tail) while still only archiving ``messages``. + Returns the summary text on success, None if nothing to archive. """ if not messages: return None + context = summary_context if summary_context is not None else messages try: - formatted = MemoryStore._format_messages(messages) + formatted = MemoryStore._format_messages(context) formatted = self._truncate_to_token_budget(formatted) response = await self.provider.chat_with_retry( model=self.model, @@ -990,7 +998,17 @@ class Consolidator: last_active = session.updated_at summary: str | None = "" if archive_msgs: - summary = await self.archive(archive_msgs, session_key=session_key) + # Summarize over the full unconsolidated tail — including the + # recent suffix we retain — not just the dropped prefix. Idle + # compaction usually runs on a finished conversation, so a late + # user correction or final result that landed in the kept suffix + # must still reach the persisted summary; otherwise history keeps + # the stale pre-correction conclusion that never gets fixed + # (#4264). Only archive_msgs are removed/raw-dumped; kept stays + # in the session. + summary = await self.archive( + archive_msgs, session_key=session_key, summary_context=tail + ) if summary and summary != "(nothing)": session.metadata["_last_summary"] = { diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 61ad0109..a87445a6 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -401,6 +401,56 @@ class TestCompactIdleSession: assert meta["text"] == "Summary of old conversation." assert "last_active" in meta + @pytest.mark.asyncio + async def test_summarizes_retained_suffix_not_just_dropped_prefix( + self, real_consolidator, mock_provider + ): + """idleCompact must summarize over the full unconsolidated tail, including + the recent suffix it retains. Otherwise a late user correction / final + result that lands in the kept suffix is excluded from the persisted + summary, leaving a stale wrong conclusion in history. Regression for #4264.""" + mock_provider.chat_with_retry.return_value = MagicMock( + content="Summary.", finish_reason="stop" + ) + sessions = real_consolidator.sessions + session = sessions.get_or_create("cli:correction") + for i in range(18): + session.add_message("user", f"user msg {i}") + session.add_message("assistant", f"assistant msg {i}") + # Final correction exchange lands inside the retained max_suffix window. + session.add_message("user", "no, that's wrong, use approach B") + session.add_message("assistant", "CORRECTED_FINAL_RESULT_alpha") + sessions.save(session) + + await real_consolidator.compact_idle_session("cli:correction", max_suffix=8) + + summarized = mock_provider.chat_with_retry.call_args.kwargs["messages"][1]["content"] + assert "CORRECTED_FINAL_RESULT_alpha" in summarized + + @pytest.mark.asyncio + async def test_raw_dumps_only_dropped_messages_on_llm_failure( + self, real_consolidator, mock_provider, store + ): + """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.""" + mock_provider.chat_with_retry.side_effect = RuntimeError("LLM unavailable") + sessions = real_consolidator.sessions + session = sessions.get_or_create("cli:rawdrop") + for i in range(18): + session.add_message("user", f"user msg {i}") + session.add_message("assistant", f"assistant msg {i}") + session.add_message("user", "final user follow-up") + session.add_message("assistant", "RETAINED_SUFFIX_marker") + sessions.save(session) + + await real_consolidator.compact_idle_session("cli:rawdrop", max_suffix=8) + + 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 + @pytest.mark.asyncio async def test_idle_compact_writes_session_key_to_history( self, @@ -537,11 +587,15 @@ class TestCompactIdleSession: "assistant-04", ] + # #4264: idle compaction now summarizes the full unconsolidated tail, so + # the dropped head (user-00), the non-contiguous dropped tail + # (assistant-09), and the retained suffix (user-14) are all summarized. + # Retention above still proves the non-contiguous suffix is handled. archived_call = mock_provider.chat_with_retry.call_args user_content = archived_call.kwargs["messages"][1]["content"] - assert "user-14" not in user_content - assert "assistant-00" not in user_content + assert "user-00" in user_content assert "assistant-09" in user_content + assert "user-14" in user_content @pytest.mark.asyncio async def test_acquires_consolidation_lock(self, real_consolidator, mock_provider):