diff --git a/openhands/cli/settings.py b/openhands/cli/settings.py index afa611b1af..fe737d5cff 100644 --- a/openhands/cli/settings.py +++ b/openhands/cli/settings.py @@ -27,7 +27,7 @@ from openhands.core.config.condenser_config import ( CondenserPipelineConfig, ConversationWindowCondenserConfig, ) -from openhands.core.config.utils import OH_DEFAULT_AGENT +from openhands.core.config.config_utils import OH_DEFAULT_AGENT from openhands.memory.condenser.impl.llm_summarizing_condenser import ( LLMSummarizingCondenserConfig, ) diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index a8555b5178..7191a92e6f 100644 --- a/openhands/core/config/utils.py +++ b/openhands/core/config/utils.py @@ -20,10 +20,6 @@ from openhands.core.config.condenser_config import ( condenser_config_from_toml_section, create_condenser_config, ) -from openhands.core.config.config_utils import ( - OH_DEFAULT_AGENT, - OH_MAX_ITERATIONS, -) from openhands.core.config.extended_config import ExtendedConfig from openhands.core.config.kubernetes_config import KubernetesConfig from openhands.core.config.llm_config import LLMConfig @@ -712,14 +708,14 @@ def get_parser() -> argparse.ArgumentParser: parser.add_argument( '-c', '--agent-cls', - default=OH_DEFAULT_AGENT, + default=None, type=str, help='Name of the default agent to use', ) parser.add_argument( '-i', '--max-iterations', - default=OH_MAX_ITERATIONS, + default=None, type=int, help='The maximum number of iterations to run the agent', ) diff --git a/tests/unit/test_arg_parser.py b/tests/unit/test_arg_parser.py index 6b9a4b434c..6858c858a4 100644 --- a/tests/unit/test_arg_parser.py +++ b/tests/unit/test_arg_parser.py @@ -1,6 +1,6 @@ import pytest -from openhands.core.config import OH_DEFAULT_AGENT, OH_MAX_ITERATIONS, get_parser +from openhands.core.config import get_parser def test_parser_default_values(): @@ -10,8 +10,8 @@ def test_parser_default_values(): assert args.directory is None assert args.task == '' assert args.file is None - assert args.agent_cls == OH_DEFAULT_AGENT - assert args.max_iterations == OH_MAX_ITERATIONS + assert args.agent_cls is None + assert args.max_iterations is None assert args.max_budget_per_task is None assert args.eval_output_dir == 'evaluation/evaluation_outputs/outputs' assert args.eval_n_limit is None diff --git a/tests/unit/test_config_precedence.py b/tests/unit/test_config_precedence.py index 97b38b5c11..3881472ea4 100644 --- a/tests/unit/test_config_precedence.py +++ b/tests/unit/test_config_precedence.py @@ -3,6 +3,8 @@ from unittest.mock import MagicMock, patch import pytest from openhands.core.config import ( + OH_DEFAULT_AGENT, + OH_MAX_ITERATIONS, OpenHandsConfig, get_llm_config_arg, setup_config_from_args, @@ -308,3 +310,74 @@ def test_cli_settings_json_not_override_config_toml( # Verify that settings.json did not override config.toml assert test_llm_config.model == 'config-toml-model' assert test_llm_config.api_key == 'config-toml-api-key' + + +def test_default_values_applied_when_none(): + """Test that default values are applied when config values are None.""" + + # Create mock args with None values for agent_cls and max_iterations + mock_args = MagicMock() + mock_args.config_file = None + mock_args.llm_config = None + mock_args.agent_cls = None + mock_args.max_iterations = None + + # Load config + with patch( + 'openhands.core.config.utils.load_openhands_config', + return_value=OpenHandsConfig(), + ): + config = setup_config_from_args(mock_args) + + # Verify they match the expected defaults + assert config.default_agent == OH_DEFAULT_AGENT + assert config.max_iterations == OH_MAX_ITERATIONS + + +def test_cli_args_override_defaults(): + """Test that CLI arguments override default values.""" + + # Create mock args with custom values + mock_args = MagicMock() + mock_args.config_file = None + mock_args.llm_config = None + mock_args.agent_cls = 'CustomAgent' + mock_args.max_iterations = 50 + + # Load config + with patch( + 'openhands.core.config.utils.load_openhands_config', + return_value=OpenHandsConfig(), + ): + config = setup_config_from_args(mock_args) + + # Verify custom values are used instead of defaults + assert config.default_agent == 'CustomAgent' + assert config.max_iterations == 50 + + +def test_cli_args_none_uses_config_toml_values(): + """Test that when CLI args agent_cls and max_iterations are None, config.toml values are used.""" + + # Create mock args with None values for agent_cls and max_iterations + mock_args = MagicMock() + mock_args.config_file = None + mock_args.llm_config = None + mock_args.agent_cls = None + mock_args.max_iterations = None + + # Create a config with specific values from config.toml + config_from_toml = OpenHandsConfig() + config_from_toml.default_agent = 'ConfigTomlAgent' + config_from_toml.max_iterations = 100 + + # Load config + with patch( + 'openhands.core.config.utils.load_openhands_config', + return_value=config_from_toml, + ): + config = setup_config_from_args(mock_args) + + # Verify config.toml values are preserved when CLI args are None + assert config.default_agent == 'ConfigTomlAgent' + assert config.max_iterations == 100