From aabc3d50170c64182b25815e0fa95d9d4ca4ffa4 Mon Sep 17 00:00:00 2001 From: Cheng Yongru Date: Fri, 17 Apr 2026 17:33:18 +0800 Subject: [PATCH] fix(memory): fall back to raw_archive on LLM error response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When chat_with_retry returns an error response (finish_reason='error') instead of raising an exception, archive() previously treated the error message as a valid summary and wrote it to history.jsonl, while the original session data was already cleared by /new — causing irreversible data loss. Fix: check finish_reason after the LLM call and raise RuntimeError on error responses, which naturally falls through to the existing raw_archive fallback. This preserves the original messages in history.jsonl instead of losing them. Fixes #3244 --- nanobot/agent/memory.py | 2 ++ tests/agent/test_consolidator.py | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index d80b43d1e..5f235e3aa 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -457,6 +457,8 @@ class Consolidator: tools=None, tool_choice=None, ) + if response.finish_reason == "error": + raise RuntimeError(f"LLM returned error: {response.content}") summary = response.content or "[no summary]" self.store.append_history(summary) return summary diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 28587e1b4..985bc6ade 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -65,6 +65,46 @@ class TestConsolidatorSummarize: assert result is None +class TestConsolidatorArchiveErrorHandling: + """archive() must fall back to raw_archive when the LLM returns an error + response (finish_reason == 'error'), e.g. overloaded / quota exceeded. + See https://github.com/HKUDS/nanobot/issues/3244 + """ + + async def test_archive_falls_back_on_error_finish_reason(self, consolidator, mock_provider, store): + """LLM returning finish_reason='error' should trigger raw_archive, not write error text.""" + mock_provider.chat_with_retry.return_value = MagicMock( + content="Error: {'type': 'error', 'error': {'type': 'overloaded_error', 'message': 'overloaded_error (529)'}}", + finish_reason="error", + ) + messages = [ + {"role": "user", "content": "fix the auth bug"}, + {"role": "assistant", "content": "Done, fixed the race condition."}, + ] + result = await consolidator.archive(messages) + assert result is None + entries = store.read_unprocessed_history(since_cursor=0) + assert len(entries) == 1 + assert "[RAW]" in entries[0]["content"] + assert "Error:" not in entries[0]["content"] + + async def test_archive_preserves_summary_on_success(self, consolidator, mock_provider, store): + """Normal LLM response should still produce a proper summary entry.""" + mock_provider.chat_with_retry.return_value = MagicMock( + content="User fixed a bug in the auth module.", + finish_reason="stop", + ) + messages = [ + {"role": "user", "content": "fix the auth bug"}, + {"role": "assistant", "content": "Done."}, + ] + result = await consolidator.archive(messages) + assert result == "User fixed a bug in the auth module." + entries = store.read_unprocessed_history(since_cursor=0) + assert len(entries) == 1 + assert "[RAW]" not in entries[0]["content"] + + class TestConsolidatorTokenBudget: async def test_prompt_below_threshold_does_not_consolidate(self, consolidator): """No consolidation when tokens are within budget."""