Skip to content

Commit

Permalink
fix: Avoid infinite loop with rolling condensers and history truncati…
Browse files Browse the repository at this point in the history
…on (#6795)

Co-authored-by: openhands <[email protected]>
Co-authored-by: Calvin Smith <[email protected]>
  • Loading branch information
3 people authored Feb 19, 2025
1 parent cb72a06 commit 81f2b08
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
6 changes: 6 additions & 0 deletions openhands/memory/condenser/condenser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
44 changes: 44 additions & 0 deletions tests/unit/test_condenser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 81f2b08

Please sign in to comment.