diff --git a/openhands-sdk/openhands/sdk/agent/agent.py b/openhands-sdk/openhands/sdk/agent/agent.py index 2806677ade..893321f702 100644 --- a/openhands-sdk/openhands/sdk/agent/agent.py +++ b/openhands-sdk/openhands/sdk/agent/agent.py @@ -27,7 +27,10 @@ TokenEvent, UserRejectObservation, ) -from openhands.sdk.event.condenser import Condensation, CondensationRequest +from openhands.sdk.event.condenser import ( + Condensation, + CondensationRequest, +) from openhands.sdk.llm import ( LLMResponse, Message, diff --git a/openhands-sdk/openhands/sdk/context/condenser/__init__.py b/openhands-sdk/openhands/sdk/context/condenser/__init__.py index 155f745f64..09c7ad09b6 100644 --- a/openhands-sdk/openhands/sdk/context/condenser/__init__.py +++ b/openhands-sdk/openhands/sdk/context/condenser/__init__.py @@ -1,5 +1,6 @@ from openhands.sdk.context.condenser.base import ( CondenserBase, + NoCondensationAvailableException, RollingCondenser, ) from openhands.sdk.context.condenser.llm_summarizing_condenser import ( @@ -15,4 +16,5 @@ "NoOpCondenser", "PipelineCondenser", "LLMSummarizingCondenser", + "NoCondensationAvailableException", ] diff --git a/openhands-sdk/openhands/sdk/context/condenser/base.py b/openhands-sdk/openhands/sdk/context/condenser/base.py index 938b3495d9..28741f0c2a 100644 --- a/openhands-sdk/openhands/sdk/context/condenser/base.py +++ b/openhands-sdk/openhands/sdk/context/condenser/base.py @@ -1,4 +1,5 @@ from abc import ABC, abstractmethod +from enum import Enum from logging import getLogger from openhands.sdk.context.view import View @@ -66,6 +67,29 @@ class PipelinableCondenserBase(CondenserBase): condenser should not nest another pipeline condenser)""" +class NoCondensationAvailableException(Exception): + """Raised when a condenser is asked to provide a condensation but none is available. + + This can happen if the condenser's `should_condense` method returns True, but due to + API constraints no condensation can be generated. + + When this exception is raised from a rolling condenser's `get_condensation` method, + the agent will fall back to using the uncondensed view for the next agent step. + """ + + +class CondensationRequirement(Enum): + """The type of condensation required by a rolling condenser.""" + + HARD = "hard" + """Indicates that a condensation is required right now, and the agent cannot proceed + without it. + """ + + SOFT = "soft" + """Indicates that a condensation is desired but not strictly required.""" + + class RollingCondenser(PipelinableCondenserBase, ABC): """Base class for a specialized condenser strategy that applies condensation to a rolling history. @@ -73,15 +97,27 @@ class RollingCondenser(PipelinableCondenserBase, ABC): The rolling history is generated by `View.from_events`, which analyzes all events in the history and produces a `View` object representing what will be sent to the LLM. - If `should_condense` says so, the condenser is then responsible for generating a - `Condensation` object from the `View` object. This will be added to the event - history which should -- when given to `get_view` -- produce the condensed `View` to - be passed to the LLM. + If `condensation_requirement` says so, the condenser is then responsible for + generating a `Condensation` object from the `View` object. This will be added to the + event history which should -- when given to `get_view` -- produce the condensed + `View` to be passed to the LLM. """ @abstractmethod - def should_condense(self, view: View, agent_llm: LLM | None = None) -> bool: - """Determine if a view should be condensed.""" + def condensation_requirement( + self, view: View, agent_llm: LLM | None = None + ) -> CondensationRequirement | None: + """Determine how a view should be condensed. + + Args: + view: The current view of the conversation history. + agent_llm: LLM instance used by the agent. Condensers use this for token + counting purposes. Defaults to None. + + Returns: + CondensationRequirement | None: The type of condensation required, or None + if no condensation is needed. + """ @abstractmethod def get_condensation( @@ -92,8 +128,23 @@ def get_condensation( def condense(self, view: View, agent_llm: LLM | None = None) -> View | Condensation: # If we trigger the condenser-specific condensation threshold, compute and # return the condensation. - if self.should_condense(view, agent_llm=agent_llm): - return self.get_condensation(view, agent_llm=agent_llm) + request = self.condensation_requirement(view, agent_llm=agent_llm) + if request is not None: + try: + return self.get_condensation(view, agent_llm=agent_llm) + + except NoCondensationAvailableException as e: + logger.debug(f"No condensation available: {e}") + + if request == CondensationRequirement.SOFT: + # For soft requests, we can just return the uncondensed view. This + # request will _eventually_ be handled, but it's not critical that + # we do so immediately. + return view + + # Otherwise re-raise the exception. + else: + raise e # Otherwise we're safe to just return the view. else: diff --git a/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py b/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py index ad0d3c6fe6..8cbdd01df9 100644 --- a/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py +++ b/openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py @@ -4,7 +4,11 @@ from pydantic import Field, model_validator -from openhands.sdk.context.condenser.base import RollingCondenser +from openhands.sdk.context.condenser.base import ( + CondensationRequirement, + NoCondensationAvailableException, + RollingCondenser, +) from openhands.sdk.context.condenser.utils import ( get_suffix_length_for_token_reduction, get_total_token_count, @@ -84,9 +88,30 @@ def get_condensation_reasons( return reasons - def should_condense(self, view: View, agent_llm: LLM | None = None) -> bool: + def condensation_requirement( + self, view: View, agent_llm: LLM | None = None + ) -> CondensationRequirement | None: reasons = self.get_condensation_reasons(view, agent_llm) - return reasons != set() + + # No reasons => no condensation needed. + if reasons == set(): + return None + + # If the reasons are for resource constraints, we can treat it as a soft + # requirement. We want to condense when we can, but there's still space in the + # context window or we'd also see Reason.REQUEST. That means we can delay the + # condensation if there isn't one available (based on the view's manipulation + # indices). + resource_reasons = {Reason.TOKENS, Reason.EVENTS} + if reasons.issubset(resource_reasons): + return CondensationRequirement.SOFT + + # Requests -- whether they come from the user or the agent -- are always hard + # requirements. We need to condense now because: + # 1. the user expects it + # 2. the agent has no more room in the context window and can't continue + if Reason.REQUEST in reasons: + return CondensationRequirement.HARD def _get_summary_event_content(self, view: View) -> str: """Extract the text content from the summary event in the view, if any. @@ -124,12 +149,7 @@ def _generate_condensation( Raises: ValueError: If forgotten_events is empty (0 events to condense). """ - if len(forgotten_events) == 0: - raise ValueError( - "Cannot condense 0 events. This typically occurs when a tool loop " - "spans almost the entire view, leaving no valid range for forgetting " - "events. Consider adjusting keep_first or max_size parameters." - ) + assert len(forgotten_events) > 0, "No events to condense." # Convert events to strings for the template event_strings = [str(forgotten_event) for forgotten_event in forgotten_events] @@ -236,11 +256,19 @@ def get_condensation( ) -> Condensation: # The condensation is dependent on the events we want to drop and the previous # summary. - summary_event_content = self._get_summary_event_content(view) forgotten_events, summary_offset = self._get_forgotten_events( view, agent_llm=agent_llm ) + if not forgotten_events: + raise NoCondensationAvailableException( + "Cannot condense 0 events. This typically occurs when a tool loop " + "spans almost the entire view, leaving no valid range for forgetting " + "events. Consider adjusting keep_first or max_size parameters." + ) + + summary_event_content = self._get_summary_event_content(view) + return self._generate_condensation( summary_event_content=summary_event_content, forgotten_events=forgotten_events, diff --git a/openhands-sdk/openhands/sdk/context/view.py b/openhands-sdk/openhands/sdk/context/view.py index 0b6b594a98..c80739e1ac 100644 --- a/openhands-sdk/openhands/sdk/context/view.py +++ b/openhands-sdk/openhands/sdk/context/view.py @@ -489,9 +489,11 @@ def from_events(events: Sequence[Event]) -> View: # Check for an unhandled condensation request -- these are events closer to the # end of the list than any condensation action. unhandled_condensation_request = False + for event in reversed(events): if isinstance(event, Condensation): break + if isinstance(event, CondensationRequest): unhandled_condensation_request = True break diff --git a/tests/integration/tests/t09_token_condenser.py b/tests/integration/tests/t09_token_condenser.py index dec8cf5715..193a94df73 100644 --- a/tests/integration/tests/t09_token_condenser.py +++ b/tests/integration/tests/t09_token_condenser.py @@ -40,7 +40,7 @@ class TokenCondenserTest(BaseIntegrationTest): def __init__(self, *args, **kwargs): """Initialize test with tracking variables.""" - self.condensation_count = 0 + self.condensations: list[Condensation] = [] super().__init__(*args, **kwargs) # Some models explicitly disallow long, repetitive tool loops for cost/safety. @@ -87,25 +87,26 @@ def conversation_callback(self, event): super().conversation_callback(event) if isinstance(event, Condensation): - if self.condensation_count >= 1: + if len(self.condensations) >= 1: logger.info("2nd condensation detected! Stopping test early.") self.conversation.pause() # We allow the first condensation request to test if # thinking block + condensation will work together - self.condensation_count += 1 + self.condensations.append(event) def setup(self) -> None: logger.info(f"Token condenser test: max_tokens={self.condenser.max_tokens}") def verify_result(self) -> TestResult: """Verify that condensation was triggered based on token count.""" - if self.condensation_count == 0: + if len(self.condensations) == 0: return TestResult( success=False, reason="Condensation not triggered. Token counting may not work.", ) + events_summarized = len(self.condensations[0].forgotten_event_ids) return TestResult( success=True, - reason="Condensation triggered. Token counting works correctly.", + reason=f"Condensation triggered, summarizing {events_summarized} events.", ) diff --git a/tests/sdk/context/condenser/test_llm_summarizing_condenser.py b/tests/sdk/context/condenser/test_llm_summarizing_condenser.py index ebb463e9b8..ea91347330 100644 --- a/tests/sdk/context/condenser/test_llm_summarizing_condenser.py +++ b/tests/sdk/context/condenser/test_llm_summarizing_condenser.py @@ -1,9 +1,10 @@ from typing import Any, cast -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from litellm.types.utils import ModelResponse +from openhands.sdk.context.condenser.base import CondensationRequirement from openhands.sdk.context.condenser.llm_summarizing_condenser import ( LLMSummarizingCondenser, Reason, @@ -108,13 +109,15 @@ def test_should_condense(mock_llm: LLM) -> None: small_events = [message_event(f"Event {i}") for i in range(max_size)] small_view = View.from_events(small_events) - assert not condenser.should_condense(small_view) + assert condenser.condensation_requirement(small_view) is None - # Create events above the threshold + # Create events above the threshold (triggers EVENTS reason -> SOFT requirement) large_events = [message_event(f"Event {i}") for i in range(max_size + 1)] large_view = View.from_events(large_events) - assert condenser.should_condense(large_view) + assert ( + condenser.condensation_requirement(large_view) == CondensationRequirement.SOFT + ) def test_condense_returns_view_when_no_condensation_needed(mock_llm: LLM) -> None: @@ -554,7 +557,7 @@ def test_most_aggressive_condensation_chosen(mock_llm: LLM) -> None: def test_generate_condensation_raises_on_zero_events(mock_llm: LLM) -> None: - """Test that _generate_condensation raises ValueError when given 0 events. + """Test that _generate_condensation raises AssertionError when given 0 events. This prevents the LLM from being called with an empty event list, which would produce a confusing summary like "I don't see any events provided to summarize." @@ -562,7 +565,7 @@ def test_generate_condensation_raises_on_zero_events(mock_llm: LLM) -> None: """ condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) - with pytest.raises(ValueError, match="Cannot condense 0 events"): + with pytest.raises(AssertionError, match="No events to condense"): condenser._generate_condensation( summary_event_content="", forgotten_events=[], @@ -571,3 +574,133 @@ def test_generate_condensation_raises_on_zero_events(mock_llm: LLM) -> None: # Verify the LLM was never called cast(MagicMock, mock_llm.completion).assert_not_called() + + +@pytest.mark.parametrize( + "reasons", + [set()], +) +def test_condensation_requirement_returns_none( + mock_llm: LLM, reasons: set[Reason] +) -> None: + """Test that condensation_requirement returns None when appropriate. + + Mocks get_condensation_reasons to test different reason combinations. + """ + condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) + events: list[Event] = [message_event(f"Event {i}") for i in range(10)] + view = View.from_events(events) + + with patch.object( + LLMSummarizingCondenser, "get_condensation_reasons", return_value=reasons + ): + result = condenser.condensation_requirement(view) + assert result is None + + +@pytest.mark.parametrize( + "reasons", + [ + {Reason.TOKENS}, + {Reason.EVENTS}, + {Reason.TOKENS, Reason.EVENTS}, + ], +) +def test_condensation_requirement_returns_soft( + mock_llm: LLM, reasons: set[Reason] +) -> None: + """Test that condensation_requirement returns SOFT for resource constraints. + + Mocks get_condensation_reasons to test different resource reason combinations. + """ + condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) + events: list[Event] = [message_event(f"Event {i}") for i in range(10)] + view = View.from_events(events) + + with patch.object( + LLMSummarizingCondenser, "get_condensation_reasons", return_value=reasons + ): + result = condenser.condensation_requirement(view) + assert result == CondensationRequirement.SOFT + + +@pytest.mark.parametrize( + "reasons", + [ + {Reason.REQUEST}, + {Reason.REQUEST, Reason.TOKENS}, + {Reason.REQUEST, Reason.EVENTS}, + {Reason.REQUEST, Reason.TOKENS, Reason.EVENTS}, + ], +) +def test_condensation_requirement_returns_hard( + mock_llm: LLM, reasons: set[Reason] +) -> None: + """Test that condensation_requirement returns HARD when REQUEST is present. + + Mocks get_condensation_reasons to test different combinations with REQUEST. + """ + condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) + events: list[Event] = [message_event(f"Event {i}") for i in range(10)] + view = View.from_events(events) + + with patch.object( + LLMSummarizingCondenser, "get_condensation_reasons", return_value=reasons + ): + result = condenser.condensation_requirement(view) + assert result == CondensationRequirement.HARD + + +def test_condense_with_hard_requirement_and_no_condensation_available( + mock_llm: LLM, +) -> None: + """Test that condense raises error with hard requirement but no condensation. + + When there's a hard requirement but no valid condensation range available + (e.g., entire view is a single atomic unit), should raise an exception. + """ + from openhands.sdk.context.condenser.base import NoCondensationAvailableException + + condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) + events: list[Event] = [message_event(f"Event {i}") for i in range(10)] + view = View.from_events(events) + + # Mock to return HARD requirement but no events to condense + with ( + patch.object( + LLMSummarizingCondenser, + "get_condensation_reasons", + return_value={Reason.REQUEST}, + ), + patch.object(condenser, "_get_forgotten_events", return_value=([], 0)), + ): + with pytest.raises(NoCondensationAvailableException): + condenser.condense(view) + + +def test_condense_with_soft_requirement_and_no_condensation_available( + mock_llm: LLM, +) -> None: + """Test that condense returns view with soft requirement but no condensation. + + When there's a soft requirement but no valid condensation range available, + should return the original view unchanged. + """ + condenser = LLMSummarizingCondenser(llm=mock_llm, max_size=100, keep_first=2) + events: list[Event] = [message_event(f"Event {i}") for i in range(10)] + view = View.from_events(events) + + # Mock to return SOFT requirement but no events to condense + with ( + patch.object( + LLMSummarizingCondenser, + "get_condensation_reasons", + return_value={Reason.EVENTS}, + ), + patch.object(condenser, "_get_forgotten_events", return_value=([], 0)), + ): + result = condenser.condense(view) + assert isinstance(result, View) + assert result == view + # LLM should not be called + cast(MagicMock, mock_llm.completion).assert_not_called() diff --git a/tests/sdk/context/condenser/test_rolling_condenser.py b/tests/sdk/context/condenser/test_rolling_condenser.py new file mode 100644 index 0000000000..d7df33506e --- /dev/null +++ b/tests/sdk/context/condenser/test_rolling_condenser.py @@ -0,0 +1,157 @@ +from unittest.mock import MagicMock + +import pytest + +from openhands.sdk.context.condenser.base import ( + CondensationRequirement, + NoCondensationAvailableException, + RollingCondenser, +) +from openhands.sdk.context.view import View +from openhands.sdk.event.base import Event +from openhands.sdk.event.condenser import Condensation +from openhands.sdk.event.llm_convertible import MessageEvent +from openhands.sdk.llm import LLM, Message, TextContent + + +def message_event(content: str) -> MessageEvent: + return MessageEvent( + llm_message=Message(role="user", content=[TextContent(text=content)]), + source="user", + ) + + +class MockRollingCondenser(RollingCondenser): + """Mock implementation of RollingCondenser for testing.""" + + def __init__( + self, + condensation_requirement_value: CondensationRequirement | None = None, + raise_exception: bool = False, + ): + self._condensation_requirement_value = condensation_requirement_value + self._raise_exception = raise_exception + + def condensation_requirement( + self, view: View, agent_llm: LLM | None = None + ) -> CondensationRequirement | None: + return self._condensation_requirement_value + + def get_condensation( + self, view: View, agent_llm: LLM | None = None + ) -> Condensation: + if self._raise_exception: + raise NoCondensationAvailableException( + "No condensation available due to API constraints" + ) + # Return a simple condensation for successful case + return Condensation( + forgotten_event_ids=[view.events[0].id], + summary="Mock summary", + summary_offset=0, + llm_response_id="mock-response-id", + ) + + +def test_rolling_condenser_returns_view_when_no_condensation_needed() -> None: + """Test that RollingCondenser returns the original view when + condensation_requirement returns None. + """ + condenser = MockRollingCondenser(condensation_requirement_value=None) + + events: list[Event] = [ + message_event("Event 1"), + message_event("Event 2"), + message_event("Event 3"), + ] + view = View.from_events(events) + + result = condenser.condense(view) + + assert isinstance(result, View) + assert result == view + + +def test_rolling_condenser_returns_condensation_when_needed() -> None: + """Test that RollingCondenser returns a Condensation when condensation_requirement + returns HARD. + """ + condenser = MockRollingCondenser( + condensation_requirement_value=CondensationRequirement.HARD, + raise_exception=False, + ) + + events: list[Event] = [ + message_event("Event 1"), + message_event("Event 2"), + message_event("Event 3"), + ] + view = View.from_events(events) + + result = condenser.condense(view) + + assert isinstance(result, Condensation) + assert result.summary == "Mock summary" + + +def test_rolling_condenser_returns_view_on_no_condensation_available_exception() -> ( + None +): + """Test that RollingCondenser returns the original view when + NoCondensationAvailableException is raised with SOFT requirement. + + This tests the exception handling for SOFT condensation requirements which catches + NoCondensationAvailableException from get_condensation() and returns the + original view as a fallback. + """ + condenser = MockRollingCondenser( + condensation_requirement_value=CondensationRequirement.SOFT, + raise_exception=True, + ) + + events: list[Event] = [ + message_event("Event 1"), + message_event("Event 2"), + message_event("Event 3"), + ] + view = View.from_events(events) + + # Even though condensation_requirement returns SOFT, the exception should be + # caught and the original view should be returned + result = condenser.condense(view) + + assert isinstance(result, View) + assert result == view + assert result.events == events + + +def test_rolling_condenser_with_agent_llm() -> None: + """Test that RollingCondenser works with optional agent_llm parameter.""" + condenser = MockRollingCondenser( + condensation_requirement_value=CondensationRequirement.HARD, + raise_exception=False, + ) + + events: list[Event] = [ + message_event("Event 1"), + message_event("Event 2"), + message_event("Event 3"), + ] + view = View.from_events(events) + + # Create a mock LLM + mock_llm = MagicMock(spec=LLM) + + # Condense with agent_llm parameter + result = condenser.condense(view, agent_llm=mock_llm) + + assert isinstance(result, Condensation) + assert result.summary == "Mock summary" + + +def test_no_condensation_available_exception_message() -> None: + """Test that NoCondensationAvailableException raisable with custom message.""" + exception_message = "Custom error message about API constraints" + + with pytest.raises(NoCondensationAvailableException, match=exception_message): + raise NoCondensationAvailableException(exception_message)