From 81f2b08a890c1f3d6a08f038570e1caf7e6a4f94 Mon Sep 17 00:00:00 2001 From: Calvin Smith Date: Wed, 19 Feb 2025 07:45:33 -0700 Subject: [PATCH] fix: Avoid infinite loop with rolling condensers and history truncation (#6795) Co-authored-by: openhands Co-authored-by: Calvin Smith --- openhands/memory/condenser/condenser.py | 6 ++++ tests/unit/test_condenser.py | 44 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/openhands/memory/condenser/condenser.py b/openhands/memory/condenser/condenser.py index 878c27aec140..411ed39386f6 100644 --- a/openhands/memory/condenser/condenser.py +++ b/openhands/memory/condenser/condenser.py @@ -161,6 +161,12 @@ def __init__(self) -> None: @override def condensed_history(self, state: State) -> list[Event]: + # The history should grow monotonically -- if it doesn't, something has + # truncated the history and we need to reset our tracking. + if len(state.history) < self._last_history_length: + self._condensation = [] + self._last_history_length = 0 + new_events = state.history[self._last_history_length :] with self.metadata_batch(state): diff --git a/tests/unit/test_condenser.py b/tests/unit/test_condenser.py index e8c5af60afbb..fd1e922a103a 100644 --- a/tests/unit/test_condenser.py +++ b/tests/unit/test_condenser.py @@ -19,6 +19,7 @@ from openhands.events.observation.observation import Observation from openhands.llm import LLM from openhands.memory.condenser import Condenser +from openhands.memory.condenser.condenser import RollingCondenser from openhands.memory.condenser.impl import ( AmortizedForgettingCondenser, ImportantEventSelection, @@ -452,6 +453,49 @@ def test_llm_attention_condenser_invalid_config(): pytest.raises(ValueError, LLMAttentionCondenser.from_config, config) +def test_rolling_condenser_handles_truncation(mock_state: State): + """Test that RollingCondenser correctly handles history truncation.""" + + class TestRollingCondenser(RollingCondenser): + """Test implementation of RollingCondenser that just returns all events.""" + + def condense(self, events: list[Event]) -> list[Event]: + return events + + condenser = TestRollingCondenser() + + # Initial history with 3 events + events = [ + create_test_event('Event 1', id=1), + create_test_event('Event 2', id=2), + create_test_event('Event 3', id=3), + ] + mock_state.history = events + + # First condensation - should return all events + results = condenser.condensed_history(mock_state) + assert len(results) == 3 + assert [e._id for e in results] == [1, 2, 3] + + # Simulate truncation - history is now shorter, and the condensation should + # just include the truncated history + mock_state.history = mock_state.history[-1:] + + results = condenser.condensed_history(mock_state) + assert len(results) == 1 + assert results[0]._id == 3 + + # Adding more events and condensing should "rebase" us from the truncated history + mock_state.history += [ + create_test_event('Event 4', id=4), + create_test_event('Event 5', id=5), + ] + + results = condenser.condensed_history(mock_state) + assert len(results) == 3 + assert [e._id for e in results] == [3, 4, 5] + + def test_llm_attention_condenser_keeps_first_events(mock_llm, mock_state): """Test that the LLMAttentionCondenser keeps the right number of initial events when forgetting.""" max_size = 4