From ff674144d66ed4e13c7d8f9422c74cce31dcd546 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Wed, 19 Aug 2026 16:30:34 +0800 Subject: [PATCH] fix(memory): reject truncated consolidation output --- nanobot/agent/memory.py | 7 +++++-- tests/agent/test_consolidator.py | 26 +++++++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 9c61887cc..eba81a36f 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -1007,8 +1007,11 @@ class Consolidator: logger.warning("Consolidation provider call failed, raw-dumping to history") self.store.raw_archive(messages, session_key=session_key) return None - if response.finish_reason == "error": - logger.warning("Consolidation provider returned an error, raw-dumping to history") + if response.finish_reason in {"error", "length"}: + logger.warning( + "Consolidation provider did not complete ({}), raw-dumping to history", + response.finish_reason, + ) self.store.raw_archive(messages, session_key=session_key) return None if response.has_tool_calls is True: diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 7d603147f..7c9f1cfd8 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -242,18 +242,26 @@ class TestConsolidatorPromptContract: assert "Do not mark something [skip] merely because it might already exist" in prompt 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 + """archive() must fall back when the LLM does not complete its overview. + + Error responses include overloaded / quota failures from #3244; length + responses contain a partial overview that is likewise unsafe to persist. """ - async def test_archive_falls_back_on_error_finish_reason( - self, consolidator, mock_provider, store, runtime + @pytest.mark.parametrize("finish_reason", ["error", "length"]) + async def test_archive_falls_back_on_incomplete_finish_reason( + self, + consolidator, + mock_provider, + store, + runtime, + finish_reason: str, ): - """LLM returning finish_reason='error' should trigger raw_archive, not write error text.""" + """Incomplete LLM output should trigger raw_archive, not persist partial text.""" + invalid_output = f"INVALID_{finish_reason.upper()}_OUTPUT" mock_provider.chat_with_retry.return_value = MagicMock( - content="Error: {'type': 'error', 'error': {'type': 'overloaded_error', 'message': 'overloaded_error (529)'}}", - finish_reason="error", + content=invalid_output, + finish_reason=finish_reason, ) messages = [ {"role": "user", "content": "fix the auth bug"}, @@ -264,7 +272,7 @@ class TestConsolidatorArchiveErrorHandling: 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"] + assert invalid_output not in entries[0]["content"] async def test_archive_preserves_summary_on_success( self, consolidator, mock_provider, store, runtime