From 3d38a105cfa63e883b00a3d1b58d6bdf95913df8 Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Mon, 3 Mar 2025 20:32:46 +0100 Subject: [PATCH] Add loading from toml for condensers (#6974) Co-authored-by: openhands Co-authored-by: Calvin Smith --- config.template.toml | 68 +++++ .../agenthub/codeact_agent/codeact_agent.py | 2 +- openhands/core/config/app_config.py | 1 + openhands/core/config/condenser_config.py | 146 ++++++++++- openhands/core/config/utils.py | 52 +++- openhands/memory/condenser/__init__.py | 2 +- tests/unit/test_config.py | 238 +++++++++++++++++- 7 files changed, 495 insertions(+), 14 deletions(-) diff --git a/config.template.toml b/config.template.toml index 331663c619..2bf7662bd2 100644 --- a/config.template.toml +++ b/config.template.toml @@ -95,6 +95,11 @@ workspace_base = "./workspace" # List of allowed file extensions for uploads #file_uploads_allowed_extensions = [".*"] +# Whether to enable the default LLM summarizing condenser when no condenser is specified in config +# When true, a LLMSummarizingCondenserConfig will be used as the default condenser +# When false, a NoOpCondenserConfig (no summarization) will be used +#enable_default_condenser = true + #################################### LLM ##################################### # Configuration for LLM models (group name starts with 'llm') # use 'llm' for the default LLM config @@ -294,6 +299,69 @@ llm_config = 'gpt3' # The security analyzer to use (For Headless / CLI only - In Web this is overridden by Session Init) #security_analyzer = "" +#################################### Condenser ################################# +# Condensers control how conversation history is managed and compressed when +# the context grows too large. Each agent uses one condenser configuration. +############################################################################## +[condenser] +# The type of condenser to use. Available options: +# - "noop": No condensing, keeps full history (default) +# - "observation_masking": Keeps full event structure but masks older observations +# - "recent": Keeps only recent events and discards older ones +# - "llm": Uses an LLM to summarize conversation history +# - "amortized": Intelligently forgets older events while preserving important context +# - "llm_attention": Uses an LLM to prioritize most relevant context +type = "noop" + +# Examples for each condenser type (uncomment and modify as needed): + +# 1. NoOp Condenser - No additional settings needed +#type = "noop" + +# 2. Observation Masking Condenser +#type = "observation_masking" +# Number of most-recent events where observations will not be masked +#attention_window = 100 + +# 3. Recent Events Condenser +#type = "recent" +# Number of initial events to always keep (typically includes task description) +#keep_first = 1 +# Maximum number of events to keep in history +#max_events = 100 + +# 4. LLM Summarizing Condenser +#type = "llm" +# Reference to an LLM config to use for summarization +#llm_config = "condenser" +# Number of initial events to always keep (typically includes task description) +#keep_first = 1 +# Maximum size of history before triggering summarization +#max_size = 100 + +# 5. Amortized Forgetting Condenser +#type = "amortized" +# Number of initial events to always keep (typically includes task description) +#keep_first = 1 +# Maximum size of history before triggering forgetting +#max_size = 100 + +# 6. LLM Attention Condenser +#type = "llm_attention" +# Reference to an LLM config to use for attention scoring +#llm_config = "condenser" +# Number of initial events to always keep (typically includes task description) +#keep_first = 1 +# Maximum size of history before triggering attention mechanism +#max_size = 100 + +# Example of a custom LLM configuration for condensers that require an LLM +# If not provided, it falls back to the default LLM +#[llm.condenser] +#model = "gpt-4o" +#temperature = 0.1 +#max_tokens = 1024 + #################################### Eval #################################### # Configuration for the evaluation, please refer to the specific evaluation # plugin for the available options diff --git a/openhands/agenthub/codeact_agent/codeact_agent.py b/openhands/agenthub/codeact_agent/codeact_agent.py index 027995c6a1..6266278dc2 100644 --- a/openhands/agenthub/codeact_agent/codeact_agent.py +++ b/openhands/agenthub/codeact_agent/codeact_agent.py @@ -91,7 +91,7 @@ class CodeActAgent(Agent): self.conversation_memory = ConversationMemory(self.prompt_manager) self.condenser = Condenser.from_config(self.config.condenser) - logger.debug(f'Using condenser: {self.condenser}') + logger.debug(f'Using condenser: {type(self.condenser)}') def reset(self) -> None: """Resets the CodeAct Agent.""" diff --git a/openhands/core/config/app_config.py b/openhands/core/config/app_config.py index 98bc5d0c27..3f12bb4c30 100644 --- a/openhands/core/config/app_config.py +++ b/openhands/core/config/app_config.py @@ -82,6 +82,7 @@ class AppConfig(BaseModel): daytona_target: str = Field(default='us') cli_multiline_input: bool = Field(default=False) conversation_max_age_seconds: int = Field(default=864000) # 10 days in seconds + enable_default_condenser: bool = Field(default=True) defaults_dict: ClassVar[dict] = {} diff --git a/openhands/core/config/condenser_config.py b/openhands/core/config/condenser_config.py index aca1d090d7..b2a3caccc0 100644 --- a/openhands/core/config/condenser_config.py +++ b/openhands/core/config/condenser_config.py @@ -1,7 +1,8 @@ -from typing import Literal +from typing import Literal, cast -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, ValidationError +from openhands.core import logger from openhands.core.config.llm_config import LLMConfig @@ -10,17 +11,21 @@ class NoOpCondenserConfig(BaseModel): type: Literal['noop'] = Field('noop') + model_config = {'extra': 'forbid'} + class ObservationMaskingCondenserConfig(BaseModel): """Configuration for ObservationMaskingCondenser.""" type: Literal['observation_masking'] = Field('observation_masking') attention_window: int = Field( - default=10, + default=100, description='The number of most-recent events where observations will not be masked.', ge=1, ) + model_config = {'extra': 'forbid'} + class RecentEventsCondenserConfig(BaseModel): """Configuration for RecentEventsCondenser.""" @@ -34,9 +39,11 @@ class RecentEventsCondenserConfig(BaseModel): ge=0, ) max_events: int = Field( - default=10, description='Maximum number of events to keep.', ge=1 + default=100, description='Maximum number of events to keep.', ge=1 ) + model_config = {'extra': 'forbid'} + class LLMSummarizingCondenserConfig(BaseModel): """Configuration for LLMCondenser.""" @@ -49,13 +56,17 @@ class LLMSummarizingCondenserConfig(BaseModel): # at least one event by default, because the best guess is that it's the user task keep_first: int = Field( default=1, - description='The number of initial events to condense.', + description='Number of initial events to always keep in history.', ge=0, ) max_size: int = Field( - default=10, description='Maximum number of events to keep.', ge=1 + default=100, + description='Maximum size of the condensed history before triggering forgetting.', + ge=2, ) + model_config = {'extra': 'forbid'} + class AmortizedForgettingCondenserConfig(BaseModel): """Configuration for AmortizedForgettingCondenser.""" @@ -74,6 +85,8 @@ class AmortizedForgettingCondenserConfig(BaseModel): ge=0, ) + model_config = {'extra': 'forbid'} + class LLMAttentionCondenserConfig(BaseModel): """Configuration for LLMAttentionCondenser.""" @@ -95,7 +108,10 @@ class LLMAttentionCondenserConfig(BaseModel): ge=0, ) + model_config = {'extra': 'forbid'} + +# Type alias for convenience CondenserConfig = ( NoOpCondenserConfig | ObservationMaskingCondenserConfig @@ -104,3 +120,121 @@ CondenserConfig = ( | AmortizedForgettingCondenserConfig | LLMAttentionCondenserConfig ) + + +def condenser_config_from_toml_section( + data: dict, llm_configs: dict | None = None +) -> dict[str, CondenserConfig]: + """ + Create a CondenserConfig instance from a toml dictionary representing the [condenser] section. + + For CondenserConfig, the handling is different since it's a union type. The type of condenser + is determined by the 'type' field in the section. + + Example: + Parse condenser config like: + [condenser] + type = "noop" + + For condensers that require an LLM config, you can specify the name of an LLM config: + [condenser] + type = "llm" + llm_config = "my_llm" # References [llm.my_llm] section + + Args: + data: The TOML dictionary representing the [condenser] section. + llm_configs: Optional dictionary of LLMConfig objects keyed by name. + + Returns: + dict[str, CondenserConfig]: A mapping where the key "condenser" corresponds to the configuration. + """ + + # Initialize the result mapping + condenser_mapping: dict[str, CondenserConfig] = {} + + # Process config + try: + # Determine which condenser type to use based on 'type' field + condenser_type = data.get('type', 'noop') + + # Handle LLM config reference if needed + if ( + condenser_type in ('llm', 'llm_attention') + and 'llm_config' in data + and isinstance(data['llm_config'], str) + ): + llm_config_name = data['llm_config'] + if llm_configs and llm_config_name in llm_configs: + # Replace the string reference with the actual LLMConfig object + data_copy = data.copy() + data_copy['llm_config'] = llm_configs[llm_config_name] + config = create_condenser_config(condenser_type, data_copy) + else: + logger.openhands_logger.warning( + f"LLM config '{llm_config_name}' not found for condenser. Using default LLMConfig." + ) + # Create a default LLMConfig if the referenced one doesn't exist + data_copy = data.copy() + # Try to use the fallback 'llm' config + if llm_configs is not None: + data_copy['llm_config'] = llm_configs.get('llm') + config = create_condenser_config(condenser_type, data_copy) + else: + config = create_condenser_config(condenser_type, data) + + condenser_mapping['condenser'] = config + except (ValidationError, ValueError) as e: + logger.openhands_logger.warning( + f'Invalid condenser configuration: {e}. Using NoOpCondenserConfig.' + ) + # Default to NoOpCondenserConfig if config fails + config = NoOpCondenserConfig() + condenser_mapping['condenser'] = config + + return condenser_mapping + + +# For backward compatibility +from_toml_section = condenser_config_from_toml_section + + +def create_condenser_config(condenser_type: str, data: dict) -> CondenserConfig: + """ + Create a CondenserConfig instance based on the specified type. + + Args: + condenser_type: The type of condenser to create. + data: The configuration data. + + Returns: + A CondenserConfig instance. + + Raises: + ValueError: If the condenser type is unknown. + ValidationError: If the provided data fails validation for the condenser type. + """ + # Mapping of condenser types to their config classes + condenser_classes = { + 'noop': NoOpCondenserConfig, + 'observation_masking': ObservationMaskingCondenserConfig, + 'recent': RecentEventsCondenserConfig, + 'llm': LLMSummarizingCondenserConfig, + 'amortized': AmortizedForgettingCondenserConfig, + 'llm_attention': LLMAttentionCondenserConfig, + } + + if condenser_type not in condenser_classes: + raise ValueError(f'Unknown condenser type: {condenser_type}') + + # Create and validate the config using direct instantiation + # Explicitly handle ValidationError to provide more context + try: + config_class = condenser_classes[condenser_type] + # Use type casting to help mypy understand the return type + return cast(CondenserConfig, config_class(**data)) + except ValidationError as e: + # Just re-raise with a more descriptive message, but don't try to pass the errors + # which can cause compatibility issues with different pydantic versions + raise ValueError( + f"Validation failed for condenser type '{condenser_type}': {e}" + ) diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index 69edc3695c..2e34eaa6a7 100644 --- a/openhands/core/config/utils.py +++ b/openhands/core/config/utils.py @@ -12,9 +12,11 @@ import toml from dotenv import load_dotenv from pydantic import BaseModel, SecretStr, ValidationError +from openhands import __version__ from openhands.core import logger from openhands.core.config.agent_config import AgentConfig from openhands.core.config.app_config import AppConfig +from openhands.core.config.condenser_config import condenser_config_from_toml_section from openhands.core.config.config_utils import ( OH_DEFAULT_AGENT, OH_MAX_ITERATIONS, @@ -193,6 +195,44 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml') -> None: # Re-raise ValueError from SandboxConfig.from_toml_section raise ValueError('Error in [sandbox] section in config.toml') + # Process condenser section if present + if 'condenser' in toml_config: + try: + # Pass the LLM configs to the condenser config parser + condenser_mapping = condenser_config_from_toml_section( + toml_config['condenser'], cfg.llms + ) + # Assign the default condenser configuration to the default agent configuration + if 'condenser' in condenser_mapping: + # Get the default agent config and assign the condenser config to it + default_agent_config = cfg.get_agent_config() + default_agent_config.condenser = condenser_mapping['condenser'] + logger.openhands_logger.debug( + 'Default condenser configuration loaded from config toml and assigned to default agent' + ) + except (TypeError, KeyError, ValidationError) as e: + logger.openhands_logger.warning( + f'Cannot parse [condenser] config from toml, values have not been applied.\nError: {e}' + ) + # If no condenser section is in toml but enable_default_condenser is True, + # set LLMSummarizingCondenserConfig as default + elif cfg.enable_default_condenser: + from openhands.core.config.condenser_config import LLMSummarizingCondenserConfig + + # Get default agent config + default_agent_config = cfg.get_agent_config() + + # Create default LLM summarizing condenser config + default_condenser = LLMSummarizingCondenserConfig( + llm_config=cfg.get_llm_config(), # Use default LLM config + ) + + # Set as default condenser + default_agent_config.condenser = default_condenser + logger.openhands_logger.debug( + 'Default LLM summarizing condenser assigned to default agent (no condenser in config)' + ) + # Process extended section if present if 'extended' in toml_config: try: @@ -203,7 +243,15 @@ def load_from_toml(cfg: AppConfig, toml_file: str = 'config.toml') -> None: ) # Check for unknown sections - known_sections = {'core', 'extended', 'agent', 'llm', 'security', 'sandbox'} + known_sections = { + 'core', + 'extended', + 'agent', + 'llm', + 'security', + 'sandbox', + 'condenser', + } for key in toml_config: if key.lower() not in known_sections: logger.openhands_logger.warning(f'Unknown section [{key}] in {toml_file}') @@ -492,8 +540,6 @@ def parse_arguments() -> argparse.Namespace: args = parser.parse_args() if args.version: - from openhands import __version__ - print(f'OpenHands version: {__version__}') sys.exit(0) diff --git a/openhands/memory/condenser/__init__.py b/openhands/memory/condenser/__init__.py index cdb0e543dc..b7bd0244ef 100644 --- a/openhands/memory/condenser/__init__.py +++ b/openhands/memory/condenser/__init__.py @@ -1,4 +1,4 @@ import openhands.memory.condenser.impl # noqa F401 (we import this to get the condensers registered) from openhands.memory.condenser.condenser import Condenser, get_condensation_metadata -__all__ = ['Condenser', 'get_condensation_metadata'] +__all__ = ['Condenser', 'get_condensation_metadata', 'CONDENSER_REGISTRY'] diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 5d8f122168..04a3e74b35 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -16,7 +16,9 @@ from openhands.core.config import ( load_from_toml, ) from openhands.core.config.condenser_config import ( + LLMSummarizingCondenserConfig, NoOpCondenserConfig, + RecentEventsCondenserConfig, ) from openhands.core.logger import openhands_logger @@ -566,13 +568,243 @@ def test_cache_dir_creation(default_config, tmpdir): assert os.path.exists(default_config.cache_dir) -def test_agent_config_condenser_default(): - """Test that default agent condenser is NoOpCondenser.""" - config = AppConfig() +def test_agent_config_condenser_with_no_enabled(): + """Test default agent condenser with enable_default_condenser=False.""" + config = AppConfig(enable_default_condenser=False) agent_config = config.get_agent_config() assert isinstance(agent_config.condenser, NoOpCondenserConfig) +def test_condenser_config_from_toml_basic(default_config, temp_toml_file): + """Test loading basic condenser configuration from TOML.""" + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[condenser] +type = "recent" +keep_first = 3 +max_events = 15 +""") + + load_from_toml(default_config, temp_toml_file) + + # Verify that the condenser config is correctly assigned to the default agent config + agent_config = default_config.get_agent_config() + assert isinstance(agent_config.condenser, RecentEventsCondenserConfig) + assert agent_config.condenser.keep_first == 3 + assert agent_config.condenser.max_events == 15 + + # We can also verify the function works directly + from openhands.core.config.condenser_config import ( + condenser_config_from_toml_section, + ) + + condenser_data = {'type': 'recent', 'keep_first': 3, 'max_events': 15} + condenser_mapping = condenser_config_from_toml_section(condenser_data) + + assert 'condenser' in condenser_mapping + assert isinstance(condenser_mapping['condenser'], RecentEventsCondenserConfig) + assert condenser_mapping['condenser'].keep_first == 3 + assert condenser_mapping['condenser'].max_events == 15 + + +def test_condenser_config_from_toml_with_llm_reference(default_config, temp_toml_file): + """Test loading condenser configuration with LLM reference from TOML.""" + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[llm.condenser_llm] +model = "gpt-4" +api_key = "test-key" + +[condenser] +type = "llm" +llm_config = "condenser_llm" +keep_first = 2 +max_size = 50 +""") + + load_from_toml(default_config, temp_toml_file) + + # Verify that the LLM config was loaded + assert 'condenser_llm' in default_config.llms + assert default_config.llms['condenser_llm'].model == 'gpt-4' + + # Verify that the condenser config is correctly assigned to the default agent config + agent_config = default_config.get_agent_config() + assert isinstance(agent_config.condenser, LLMSummarizingCondenserConfig) + assert agent_config.condenser.keep_first == 2 + assert agent_config.condenser.max_size == 50 + assert agent_config.condenser.llm_config.model == 'gpt-4' + + # Test the condenser config with the LLM reference + from openhands.core.config.condenser_config import ( + condenser_config_from_toml_section, + ) + + condenser_data = { + 'type': 'llm', + 'llm_config': 'condenser_llm', + 'keep_first': 2, + 'max_size': 50, + } + condenser_mapping = condenser_config_from_toml_section( + condenser_data, default_config.llms + ) + + assert 'condenser' in condenser_mapping + assert isinstance(condenser_mapping['condenser'], LLMSummarizingCondenserConfig) + assert condenser_mapping['condenser'].keep_first == 2 + assert condenser_mapping['condenser'].max_size == 50 + assert condenser_mapping['condenser'].llm_config.model == 'gpt-4' + + +def test_condenser_config_from_toml_with_missing_llm_reference( + default_config, temp_toml_file +): + """Test loading condenser configuration with missing LLM reference from TOML.""" + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[condenser] +type = "llm" +llm_config = "missing_llm" +keep_first = 2 +max_size = 50 +""") + + load_from_toml(default_config, temp_toml_file) + + # Test the condenser config with a missing LLM reference + from openhands.core.config.condenser_config import ( + condenser_config_from_toml_section, + ) + + condenser_data = { + 'type': 'llm', + 'llm_config': 'missing_llm', + 'keep_first': 2, + 'max_size': 50, + } + condenser_mapping = condenser_config_from_toml_section( + condenser_data, default_config.llms + ) + + assert 'condenser' in condenser_mapping + assert isinstance(condenser_mapping['condenser'], NoOpCondenserConfig) + # Should not have a default LLMConfig when the reference is missing + assert not hasattr(condenser_mapping['condenser'], 'llm_config') + + +def test_condenser_config_from_toml_with_invalid_config(default_config, temp_toml_file): + """Test loading invalid condenser configuration from TOML.""" + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[condenser] +type = "invalid_type" +""") + + load_from_toml(default_config, temp_toml_file) + + # Test the condenser config with an invalid type + from openhands.core.config.condenser_config import ( + condenser_config_from_toml_section, + ) + + condenser_data = {'type': 'invalid_type'} + condenser_mapping = condenser_config_from_toml_section(condenser_data) + + # Should default to NoOpCondenserConfig when the type is invalid + assert 'condenser' in condenser_mapping + assert isinstance(condenser_mapping['condenser'], NoOpCondenserConfig) + + +def test_condenser_config_from_toml_with_validation_error( + default_config, temp_toml_file +): + """Test loading condenser configuration with validation error from TOML.""" + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[condenser] +type = "recent" +keep_first = -1 # Invalid: must be >= 0 +max_events = 0 # Invalid: must be >= 1 +""") + + load_from_toml(default_config, temp_toml_file) + + # Test the condenser config with validation errors + from openhands.core.config.condenser_config import ( + condenser_config_from_toml_section, + ) + + condenser_data = {'type': 'recent', 'keep_first': -1, 'max_events': 0} + condenser_mapping = condenser_config_from_toml_section(condenser_data) + + # Should default to NoOpCondenserConfig when validation fails + assert 'condenser' in condenser_mapping + assert isinstance(condenser_mapping['condenser'], NoOpCondenserConfig) + + +def test_default_condenser_behavior_enabled(default_config, temp_toml_file): + """Test the default condenser behavior when enable_default_condenser is True.""" + # Create a minimal TOML file with no condenser section + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[core] +# Empty core section, no condenser section +""") + + # Set enable_default_condenser to True + default_config.enable_default_condenser = True + load_from_toml(default_config, temp_toml_file) + + # Verify the default agent config has LLMSummarizingCondenserConfig + agent_config = default_config.get_agent_config() + assert isinstance(agent_config.condenser, LLMSummarizingCondenserConfig) + assert agent_config.condenser.keep_first == 1 + assert agent_config.condenser.max_size == 100 + + +def test_default_condenser_behavior_disabled(default_config, temp_toml_file): + """Test the default condenser behavior when enable_default_condenser is False.""" + # Create a minimal TOML file with no condenser section + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[core] +# Empty core section, no condenser section +""") + + # Set enable_default_condenser to False + default_config.enable_default_condenser = False + load_from_toml(default_config, temp_toml_file) + + # Verify the agent config uses NoOpCondenserConfig + agent_config = default_config.get_agent_config() + assert isinstance(agent_config.condenser, NoOpCondenserConfig) + + +def test_default_condenser_explicit_toml_override(default_config, temp_toml_file): + """Test that explicit condenser in TOML takes precedence over the default.""" + # Set enable_default_condenser to True + default_config.enable_default_condenser = True + + # Create a TOML file with an explicit condenser section + with open(temp_toml_file, 'w', encoding='utf-8') as toml_file: + toml_file.write(""" +[condenser] +type = "recent" +keep_first = 3 +max_events = 15 +""") + + # Load the config + load_from_toml(default_config, temp_toml_file) + + # Verify the explicit condenser from TOML takes precedence + agent_config = default_config.get_agent_config() + assert isinstance(agent_config.condenser, RecentEventsCondenserConfig) + assert agent_config.condenser.keep_first == 3 + assert agent_config.condenser.max_events == 15 + + def test_api_keys_repr_str(): # Test LLMConfig llm_config = LLMConfig(