From 39fff41dd4c072e07b544e52b148526962767895 Mon Sep 17 00:00:00 2001 From: Calvin Smith Date: Fri, 1 Aug 2025 10:35:40 -0600 Subject: [PATCH] Set default condenser to `ConversationWindowCondenser` (#10031) Co-authored-by: Calvin Smith Co-authored-by: openhands --- openhands/cli/settings.py | 22 +++++++++++++---- openhands/core/config/agent_config.py | 11 +++++++-- tests/unit/test_agents.py | 5 ++++ tests/unit/test_cli_settings.py | 35 ++++++++++++++++++++++----- tests/unit/test_config.py | 7 +++--- 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/openhands/cli/settings.py b/openhands/cli/settings.py index de2b4c5f58..21cb1f5d89 100644 --- a/openhands/cli/settings.py +++ b/openhands/cli/settings.py @@ -23,7 +23,10 @@ from openhands.cli.utils import ( ) from openhands.controller.agent import Agent from openhands.core.config import OpenHandsConfig -from openhands.core.config.condenser_config import NoOpCondenserConfig +from openhands.core.config.condenser_config import ( + CondenserPipelineConfig, + ConversationWindowCondenserConfig, +) from openhands.core.config.utils import OH_DEFAULT_AGENT from openhands.memory.condenser.impl.llm_summarizing_condenser import ( LLMSummarizingCondenserConfig, @@ -472,12 +475,21 @@ async def modify_llm_settings_advanced( agent_config = config.get_agent_config(config.default_agent) if enable_memory_condensation: - agent_config.condenser = LLMSummarizingCondenserConfig( - llm_config=llm_config, - type='llm', + agent_config.condenser = CondenserPipelineConfig( + type='pipeline', + condensers=[ + ConversationWindowCondenserConfig(type='conversation_window'), + # Use LLMSummarizingCondenserConfig with the custom llm_config + LLMSummarizingCondenserConfig( + llm_config=llm_config, type='llm', keep_first=4, max_size=120 + ), + ], ) + else: - agent_config.condenser = NoOpCondenserConfig(type='noop') + agent_config.condenser = ConversationWindowCondenserConfig( + type='conversation_window' + ) config.set_agent_config(agent_config) settings = await settings_store.load() diff --git a/openhands/core/config/agent_config.py b/openhands/core/config/agent_config.py index ef280ff965..53bee7b46b 100644 --- a/openhands/core/config/agent_config.py +++ b/openhands/core/config/agent_config.py @@ -2,7 +2,10 @@ from __future__ import annotations from pydantic import BaseModel, ConfigDict, Field, ValidationError -from openhands.core.config.condenser_config import CondenserConfig, NoOpCondenserConfig +from openhands.core.config.condenser_config import ( + CondenserConfig, + ConversationWindowCondenserConfig, +) from openhands.core.config.extended_config import ExtendedConfig from openhands.core.logger import openhands_logger as logger from openhands.utils.import_utils import get_impl @@ -44,7 +47,11 @@ class AgentConfig(BaseModel): enable_som_visual_browsing: bool = Field(default=True) """Whether to enable SoM (Set of Marks) visual browsing.""" condenser: CondenserConfig = Field( - default_factory=lambda: NoOpCondenserConfig(type='noop') + # The default condenser is set to the conversation window condenser -- if + # we use NoOp and the conversation hits the LLM context length limit, + # the agent will generate a condensation request which will never be + # handled. + default_factory=lambda: ConversationWindowCondenserConfig() ) extended: ExtendedConfig = Field(default_factory=lambda: ExtendedConfig({})) """Extended configuration for the agent.""" diff --git a/tests/unit/test_agents.py b/tests/unit/test_agents.py index ffd2ab3498..32ad833612 100644 --- a/tests/unit/test_agents.py +++ b/tests/unit/test_agents.py @@ -249,6 +249,7 @@ def test_step_with_no_pending_actions(mock_state: State): llm.completion = Mock(return_value=mock_response) llm.is_function_calling_active = Mock(return_value=True) # Enable function calling llm.is_caching_prompt_active = Mock(return_value=False) + llm.format_messages_for_llm = Mock(return_value=[]) # Mock message formatting # Create agent with mocked LLM config = AgentConfig() @@ -269,6 +270,10 @@ def test_step_with_no_pending_actions(mock_state: State): initial_user_message._source = EventSource.USER mock_state.history = [initial_user_message] + # Mock the view returned by condenser + mock_view = View(events=mock_state.history) + mock_state.view = mock_view + action = agent.step(mock_state) assert isinstance(action, MessageAction) assert action.content == 'Task completed' diff --git a/tests/unit/test_cli_settings.py b/tests/unit/test_cli_settings.py index e0d7f60de5..758c8d70f8 100644 --- a/tests/unit/test_cli_settings.py +++ b/tests/unit/test_cli_settings.py @@ -19,16 +19,24 @@ from openhands.storage.settings.file_settings_store import FileSettingsStore # Mock classes for condensers class MockLLMSummarizingCondenserConfig: - def __init__(self, llm_config, type): + def __init__(self, llm_config, type, keep_first=4, max_size=120): self.llm_config = llm_config self.type = type + self.keep_first = keep_first + self.max_size = max_size -class MockNoOpCondenserConfig: +class MockConversationWindowCondenserConfig: def __init__(self, type): self.type = type +class MockCondenserPipelineConfig: + def __init__(self, type, condensers): + self.type = type + self.condensers = condensers + + class TestDisplaySettings: @pytest.fixture def app_config(self): @@ -467,7 +475,13 @@ class TestModifyLLMSettingsAdvanced: 'openhands.cli.settings.LLMSummarizingCondenserConfig', MockLLMSummarizingCondenserConfig, ) - @patch('openhands.cli.settings.NoOpCondenserConfig', MockNoOpCondenserConfig) + @patch( + 'openhands.cli.settings.ConversationWindowCondenserConfig', + MockConversationWindowCondenserConfig, + ) + @patch( + 'openhands.cli.settings.CondenserPipelineConfig', MockCondenserPipelineConfig + ) async def test_modify_llm_settings_advanced_success( self, mock_confirm, mock_session, mock_list_agents, app_config, settings_store ): @@ -521,7 +535,10 @@ class TestModifyLLMSettingsAdvanced: 'openhands.cli.settings.LLMSummarizingCondenserConfig', MockLLMSummarizingCondenserConfig, ) - @patch('openhands.cli.settings.NoOpCondenserConfig', MockNoOpCondenserConfig) + @patch( + 'openhands.cli.settings.ConversationWindowCondenserConfig', + MockConversationWindowCondenserConfig, + ) async def test_modify_llm_settings_advanced_user_cancels( self, mock_confirm, mock_session, mock_list_agents, app_config, settings_store ): @@ -548,7 +565,10 @@ class TestModifyLLMSettingsAdvanced: 'openhands.cli.settings.LLMSummarizingCondenserConfig', MockLLMSummarizingCondenserConfig, ) - @patch('openhands.cli.settings.NoOpCondenserConfig', MockNoOpCondenserConfig) + @patch( + 'openhands.cli.settings.ConversationWindowCondenserConfig', + MockConversationWindowCondenserConfig, + ) async def test_modify_llm_settings_advanced_invalid_agent( self, mock_print, @@ -599,7 +619,10 @@ class TestModifyLLMSettingsAdvanced: 'openhands.cli.settings.LLMSummarizingCondenserConfig', MockLLMSummarizingCondenserConfig, ) - @patch('openhands.cli.settings.NoOpCondenserConfig', MockNoOpCondenserConfig) + @patch( + 'openhands.cli.settings.ConversationWindowCondenserConfig', + MockConversationWindowCondenserConfig, + ) async def test_modify_llm_settings_advanced_user_rejects_save( self, mock_confirm, mock_session, mock_list_agents, app_config, settings_store ): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index bb500dcacd..dacae26223 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -16,6 +16,7 @@ from openhands.core.config import ( load_openhands_config, ) from openhands.core.config.condenser_config import ( + ConversationWindowCondenserConfig, LLMSummarizingCondenserConfig, NoOpCondenserConfig, RecentEventsCondenserConfig, @@ -680,7 +681,7 @@ def test_agent_config_condenser_with_no_enabled(): """Test default agent condenser with enable_default_condenser=False.""" config = OpenHandsConfig(enable_default_condenser=False) agent_config = config.get_agent_config() - assert isinstance(agent_config.condenser, NoOpCondenserConfig) + assert isinstance(agent_config.condenser, ConversationWindowCondenserConfig) def test_sandbox_volumes_toml(default_config, temp_toml_file): @@ -907,9 +908,9 @@ def test_default_condenser_behavior_disabled(default_config, temp_toml_file): default_config.enable_default_condenser = False load_from_toml(default_config, temp_toml_file) - # Verify the agent config uses NoOpCondenserConfig + # Verify the agent config uses ConversationWindowCondenserConfig agent_config = default_config.get_agent_config() - assert isinstance(agent_config.condenser, NoOpCondenserConfig) + assert isinstance(agent_config.condenser, ConversationWindowCondenserConfig) def test_default_condenser_explicit_toml_override(default_config, temp_toml_file):