From fd13c913877e7434184ca8edde0bee243d160cdc Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Wed, 3 Dec 2025 00:24:25 +0700 Subject: [PATCH] fix(backend): apply user-defined condenser_max_size in new v1 conversations (#11862) --- .../app_conversation_service_base.py | 39 +++++ .../live_status_app_conversation_service.py | 17 +- .../test_app_conversation_service_base.py | 150 +++++++++++++++++- ...st_live_status_app_conversation_service.py | 41 +++-- .../experiments/test_experiment_manager.py | 1 + 5 files changed, 229 insertions(+), 19 deletions(-) diff --git a/openhands/app_server/app_conversation/app_conversation_service_base.py b/openhands/app_server/app_conversation/app_conversation_service_base.py index 15b952e0be..f524167524 100644 --- a/openhands/app_server/app_conversation/app_conversation_service_base.py +++ b/openhands/app_server/app_conversation/app_conversation_service_base.py @@ -9,6 +9,7 @@ from typing import AsyncGenerator import base62 from openhands.app_server.app_conversation.app_conversation_models import ( + AgentType, AppConversationStartTask, AppConversationStartTaskStatus, ) @@ -25,7 +26,9 @@ from openhands.app_server.sandbox.sandbox_models import SandboxInfo from openhands.app_server.user.user_context import UserContext from openhands.sdk import Agent from openhands.sdk.context.agent_context import AgentContext +from openhands.sdk.context.condenser import LLMSummarizingCondenser from openhands.sdk.context.skills import load_user_skills +from openhands.sdk.llm import LLM from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace _logger = logging.getLogger(__name__) @@ -340,3 +343,39 @@ class AppConversationServiceBase(AppConversationService, ABC): return _logger.info('Git pre-commit hook installed successfully') + + def _create_condenser( + self, + llm: LLM, + agent_type: AgentType, + condenser_max_size: int | None, + ) -> LLMSummarizingCondenser: + """Create a condenser based on user settings and agent type. + + Args: + llm: The LLM instance to use for condensation + agent_type: Type of agent (PLAN or DEFAULT) + condenser_max_size: condenser_max_size setting + + Returns: + Configured LLMSummarizingCondenser instance + """ + # LLMSummarizingCondenser has defaults: max_size=120, keep_first=4 + condenser_kwargs = { + 'llm': llm.model_copy( + update={ + 'usage_id': ( + 'condenser' + if agent_type == AgentType.DEFAULT + else 'planning_condenser' + ) + } + ), + } + # Only override max_size if user has a custom value + if condenser_max_size is not None: + condenser_kwargs['max_size'] = condenser_max_size + + condenser = LLMSummarizingCondenser(**condenser_kwargs) + + return condenser diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index fff762a7dc..070640c907 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -76,12 +76,10 @@ from openhands.sdk.security.confirmation_policy import AlwaysConfirm from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace from openhands.server.types import AppMode from openhands.tools.preset.default import ( - get_default_condenser, get_default_tools, ) from openhands.tools.preset.planning import ( format_plan_structure, - get_planning_condenser, get_planning_tools, ) @@ -643,6 +641,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): agent_type: AgentType, system_message_suffix: str | None, mcp_config: dict, + condenser_max_size: int | None, ) -> Agent: """Create an agent with appropriate tools and context based on agent type. @@ -651,10 +650,14 @@ class LiveStatusAppConversationService(AppConversationServiceBase): agent_type: Type of agent to create (PLAN or DEFAULT) system_message_suffix: Optional suffix for system messages mcp_config: MCP configuration dictionary + condenser_max_size: condenser_max_size setting Returns: Configured Agent instance with context """ + # Create condenser with user's settings + condenser = self._create_condenser(llm, agent_type, condenser_max_size) + # Create agent based on type if agent_type == AgentType.PLAN: agent = Agent( @@ -662,9 +665,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): tools=get_planning_tools(), system_prompt_filename='system_prompt_planning.j2', system_prompt_kwargs={'plan_structure': format_plan_structure()}, - condenser=get_planning_condenser( - llm=llm.model_copy(update={'usage_id': 'planning_condenser'}) - ), + condenser=condenser, security_analyzer=None, mcp_config=mcp_config, ) @@ -673,9 +674,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): llm=llm, tools=get_default_tools(enable_browser=True), system_prompt_kwargs={'cli_mode': False}, - condenser=get_default_condenser( - llm=llm.model_copy(update={'usage_id': 'condenser'}) - ), + condenser=condenser, mcp_config=mcp_config, ) @@ -777,7 +776,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase): # Create agent with context agent = self._create_agent_with_context( - llm, agent_type, system_message_suffix, mcp_config + llm, agent_type, system_message_suffix, mcp_config, user.condenser_max_size ) # Finalize and return the conversation request diff --git a/tests/unit/app_server/test_app_conversation_service_base.py b/tests/unit/app_server/test_app_conversation_service_base.py index 2bc794e3c7..a179a11c24 100644 --- a/tests/unit/app_server/test_app_conversation_service_base.py +++ b/tests/unit/app_server/test_app_conversation_service_base.py @@ -5,13 +5,15 @@ and the recent bug fixes for git checkout operations. """ import subprocess -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest +from openhands.app_server.app_conversation.app_conversation_models import AgentType from openhands.app_server.app_conversation.app_conversation_service_base import ( AppConversationServiceBase, ) +from openhands.app_server.user.user_context import UserContext class MockUserInfo: @@ -286,6 +288,152 @@ async def test_clone_or_init_git_repo_custom_timeout(service): ) +@patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.LLMSummarizingCondenser' +) +def test_create_condenser_default_agent_with_none_max_size(mock_condenser_class): + """Test _create_condenser for DEFAULT agent with condenser_max_size = None uses default.""" + # Arrange + mock_user_context = Mock(spec=UserContext) + with patch.object( + AppConversationServiceBase, + '__abstractmethods__', + set(), + ): + service = AppConversationServiceBase( + init_git_in_empty_workspace=True, + user_context=mock_user_context, + ) + mock_llm = MagicMock() + mock_llm_copy = MagicMock() + mock_llm_copy.usage_id = 'condenser' + mock_llm.model_copy.return_value = mock_llm_copy + mock_condenser_instance = MagicMock() + mock_condenser_class.return_value = mock_condenser_instance + + # Act + service._create_condenser(mock_llm, AgentType.DEFAULT, None) + + # Assert + mock_condenser_class.assert_called_once() + call_kwargs = mock_condenser_class.call_args[1] + # When condenser_max_size is None, max_size should not be passed (uses SDK default of 120) + assert 'max_size' not in call_kwargs + # keep_first is never passed (uses SDK default of 4) + assert 'keep_first' not in call_kwargs + assert call_kwargs['llm'].usage_id == 'condenser' + mock_llm.model_copy.assert_called_once() + + +@patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.LLMSummarizingCondenser' +) +def test_create_condenser_default_agent_with_custom_max_size(mock_condenser_class): + """Test _create_condenser for DEFAULT agent with custom condenser_max_size.""" + # Arrange + mock_user_context = Mock(spec=UserContext) + with patch.object( + AppConversationServiceBase, + '__abstractmethods__', + set(), + ): + service = AppConversationServiceBase( + init_git_in_empty_workspace=True, + user_context=mock_user_context, + ) + mock_llm = MagicMock() + mock_llm_copy = MagicMock() + mock_llm_copy.usage_id = 'condenser' + mock_llm.model_copy.return_value = mock_llm_copy + mock_condenser_instance = MagicMock() + mock_condenser_class.return_value = mock_condenser_instance + + # Act + service._create_condenser(mock_llm, AgentType.DEFAULT, 150) + + # Assert + mock_condenser_class.assert_called_once() + call_kwargs = mock_condenser_class.call_args[1] + assert call_kwargs['max_size'] == 150 # Custom value should be used + # keep_first is never passed (uses SDK default of 4) + assert 'keep_first' not in call_kwargs + assert call_kwargs['llm'].usage_id == 'condenser' + mock_llm.model_copy.assert_called_once() + + +@patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.LLMSummarizingCondenser' +) +def test_create_condenser_plan_agent_with_none_max_size(mock_condenser_class): + """Test _create_condenser for PLAN agent with condenser_max_size = None uses default.""" + # Arrange + mock_user_context = Mock(spec=UserContext) + with patch.object( + AppConversationServiceBase, + '__abstractmethods__', + set(), + ): + service = AppConversationServiceBase( + init_git_in_empty_workspace=True, + user_context=mock_user_context, + ) + mock_llm = MagicMock() + mock_llm_copy = MagicMock() + mock_llm_copy.usage_id = 'planning_condenser' + mock_llm.model_copy.return_value = mock_llm_copy + mock_condenser_instance = MagicMock() + mock_condenser_class.return_value = mock_condenser_instance + + # Act + service._create_condenser(mock_llm, AgentType.PLAN, None) + + # Assert + mock_condenser_class.assert_called_once() + call_kwargs = mock_condenser_class.call_args[1] + # When condenser_max_size is None, max_size should not be passed (uses SDK default of 120) + assert 'max_size' not in call_kwargs + # keep_first is never passed (uses SDK default of 4) + assert 'keep_first' not in call_kwargs + assert call_kwargs['llm'].usage_id == 'planning_condenser' + mock_llm.model_copy.assert_called_once() + + +@patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.LLMSummarizingCondenser' +) +def test_create_condenser_plan_agent_with_custom_max_size(mock_condenser_class): + """Test _create_condenser for PLAN agent with custom condenser_max_size.""" + # Arrange + mock_user_context = Mock(spec=UserContext) + with patch.object( + AppConversationServiceBase, + '__abstractmethods__', + set(), + ): + service = AppConversationServiceBase( + init_git_in_empty_workspace=True, + user_context=mock_user_context, + ) + mock_llm = MagicMock() + mock_llm_copy = MagicMock() + mock_llm_copy.usage_id = 'planning_condenser' + mock_llm.model_copy.return_value = mock_llm_copy + mock_condenser_instance = MagicMock() + mock_condenser_class.return_value = mock_condenser_instance + + # Act + service._create_condenser(mock_llm, AgentType.PLAN, 200) + + # Assert + mock_condenser_class.assert_called_once() + call_kwargs = mock_condenser_class.call_args[1] + assert call_kwargs['max_size'] == 200 # Custom value should be used + # keep_first is never passed (uses SDK default of 4) + assert 'keep_first' not in call_kwargs + assert call_kwargs['llm'].usage_id == 'planning_condenser' + mock_llm.model_copy.assert_called_once() + + # ============================================================================= # Tests for _configure_git_user_settings # ============================================================================= diff --git a/tests/unit/app_server/test_live_status_app_conversation_service.py b/tests/unit/app_server/test_live_status_app_conversation_service.py index 0f61c161b2..00d80abf64 100644 --- a/tests/unit/app_server/test_live_status_app_conversation_service.py +++ b/tests/unit/app_server/test_live_status_app_conversation_service.py @@ -63,6 +63,7 @@ class TestLiveStatusAppConversationService: self.mock_user.llm_api_key = 'test_api_key' self.mock_user.confirmation_mode = False self.mock_user.search_api_key = None # Default to None + self.mock_user.condenser_max_size = None # Default to None # Mock sandbox self.mock_sandbox = Mock(spec=SandboxInfo) @@ -421,20 +422,21 @@ class TestLiveStatusAppConversationService: 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_tools' ) @patch( - 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_condenser' + 'openhands.app_server.app_conversation.app_conversation_service_base.AppConversationServiceBase._create_condenser' ) @patch( 'openhands.app_server.app_conversation.live_status_app_conversation_service.format_plan_structure' ) def test_create_agent_with_context_planning_agent( - self, mock_format_plan, mock_get_condenser, mock_get_tools + self, mock_format_plan, mock_create_condenser, mock_get_tools ): """Test _create_agent_with_context for planning agent type.""" # Arrange mock_llm = Mock(spec=LLM) mock_llm.model_copy.return_value = mock_llm mock_get_tools.return_value = [] - mock_get_condenser.return_value = Mock() + mock_condenser = Mock() + mock_create_condenser.return_value = mock_condenser mock_format_plan.return_value = 'test_plan_structure' mcp_config = {'default': {'url': 'test'}} system_message_suffix = 'Test suffix' @@ -448,7 +450,11 @@ class TestLiveStatusAppConversationService: mock_agent_class.return_value = mock_agent_instance self.service._create_agent_with_context( - mock_llm, AgentType.PLAN, system_message_suffix, mcp_config + mock_llm, + AgentType.PLAN, + system_message_suffix, + mcp_config, + self.mock_user.condenser_max_size, ) # Assert @@ -462,22 +468,27 @@ class TestLiveStatusAppConversationService: ) assert call_kwargs['mcp_config'] == mcp_config assert call_kwargs['security_analyzer'] is None + assert call_kwargs['condenser'] == mock_condenser + mock_create_condenser.assert_called_once_with( + mock_llm, AgentType.PLAN, self.mock_user.condenser_max_size + ) @patch( 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools' ) @patch( - 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_condenser' + 'openhands.app_server.app_conversation.app_conversation_service_base.AppConversationServiceBase._create_condenser' ) def test_create_agent_with_context_default_agent( - self, mock_get_condenser, mock_get_tools + self, mock_create_condenser, mock_get_tools ): """Test _create_agent_with_context for default agent type.""" # Arrange mock_llm = Mock(spec=LLM) mock_llm.model_copy.return_value = mock_llm mock_get_tools.return_value = [] - mock_get_condenser.return_value = Mock() + mock_condenser = Mock() + mock_create_condenser.return_value = mock_condenser mcp_config = {'default': {'url': 'test'}} # Act @@ -489,7 +500,11 @@ class TestLiveStatusAppConversationService: mock_agent_class.return_value = mock_agent_instance self.service._create_agent_with_context( - mock_llm, AgentType.DEFAULT, None, mcp_config + mock_llm, + AgentType.DEFAULT, + None, + mcp_config, + self.mock_user.condenser_max_size, ) # Assert @@ -498,7 +513,11 @@ class TestLiveStatusAppConversationService: assert call_kwargs['llm'] == mock_llm assert call_kwargs['system_prompt_kwargs']['cli_mode'] is False assert call_kwargs['mcp_config'] == mcp_config + assert call_kwargs['condenser'] == mock_condenser mock_get_tools.assert_called_once_with(enable_browser=True) + mock_create_condenser.assert_called_once_with( + mock_llm, AgentType.DEFAULT, self.mock_user.condenser_max_size + ) @pytest.mark.asyncio @patch( @@ -693,6 +712,10 @@ class TestLiveStatusAppConversationService: self.mock_user, 'gpt-4' ) self.service._create_agent_with_context.assert_called_once_with( - mock_llm, AgentType.DEFAULT, 'Test suffix', mock_mcp_config + mock_llm, + AgentType.DEFAULT, + 'Test suffix', + mock_mcp_config, + self.mock_user.condenser_max_size, ) self.service._finalize_conversation_request.assert_called_once() diff --git a/tests/unit/experiments/test_experiment_manager.py b/tests/unit/experiments/test_experiment_manager.py index 00435c597c..a21943ace2 100644 --- a/tests/unit/experiments/test_experiment_manager.py +++ b/tests/unit/experiments/test_experiment_manager.py @@ -152,6 +152,7 @@ class TestExperimentManagerIntegration: llm_base_url=None, llm_api_key=None, confirmation_mode=False, + condenser_max_size=None, ) async def get_secrets(self):