From 6ec170cde5eaebef9108d0c8e2a8718e0e294aba Mon Sep 17 00:00:00 2001 From: Xun Date: Wed, 21 Jan 2026 09:33:20 +0800 Subject: [PATCH] fix: handle false values correctly in (#823) Fixes a critical bug in the from_runnable_config() method where falsy values (like False, 0, and empty strings) were being incorrectly filtered out, causing configuration fields to revert to their default values. The fix changes the filter condition from if v to if v is not None, ensuring only None values are skipped. --- src/config/configuration.py | 2 +- tests/unit/config/test_configuration.py | 57 ++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/config/configuration.py b/src/config/configuration.py index 34694ad..8414f1c 100644 --- a/src/config/configuration.py +++ b/src/config/configuration.py @@ -77,4 +77,4 @@ class Configuration: for f in fields(cls) if f.init } - return cls(**{k: v for k, v in values.items() if v}) + return cls(**{k: v for k, v in values.items() if v is not None}) diff --git a/tests/unit/config/test_configuration.py b/tests/unit/config/test_configuration.py index 8fa551d..8551d47 100644 --- a/tests/unit/config/test_configuration.py +++ b/tests/unit/config/test_configuration.py @@ -68,18 +68,20 @@ def test_from_runnable_config_with_env_override(monkeypatch): def test_from_runnable_config_with_none_and_falsy(monkeypatch): + """Test that None values are skipped but falsy values (0, False, empty string) are preserved.""" config_dict = { "configurable": { - "max_plan_iterations": None, - "max_step_num": 0, # falsy, should be skipped - "max_search_results": "", + "max_plan_iterations": None, # None should be skipped, use default + "max_step_num": 0, # 0 is valid, should be preserved + "max_search_results": "", # Empty string should be preserved } } config = Configuration.from_runnable_config(config_dict) - # Should fall back to defaults for skipped/falsy values + # None values should fall back to defaults assert config.max_plan_iterations == 1 - assert config.max_step_num == 3 - assert config.max_search_results == 3 + # Falsy but valid values should be preserved + assert config.max_step_num == 0 + assert config.max_search_results == "" def test_from_runnable_config_with_no_config(): @@ -91,6 +93,49 @@ def test_from_runnable_config_with_no_config(): assert config.mcp_settings is None +def test_from_runnable_config_with_boolean_false_values(): + """Test that boolean False values are correctly preserved and not filtered out. + + This is a regression test for the bug where False values were treated as falsy + and filtered out, causing fields to revert to their default values. + """ + config_dict = { + "configurable": { + "enable_web_search": False, # Should be preserved as False, not revert to True + "enable_deep_thinking": False, # Should be preserved as False + "enforce_web_search": False, # Should be preserved as False + "enforce_researcher_search": False, # Should be preserved as False + "max_plan_iterations": 5, # Control: non-falsy value + } + } + config = Configuration.from_runnable_config(config_dict) + + # Assert that False values are preserved + assert config.enable_web_search is False, "enable_web_search should be False, not default True" + assert config.enable_deep_thinking is False, "enable_deep_thinking should be False" + assert config.enforce_web_search is False, "enforce_web_search should be False" + assert config.enforce_researcher_search is False, "enforce_researcher_search should be False, not default True" + + # Control: verify non-falsy values still work + assert config.max_plan_iterations == 5 + + +def test_from_runnable_config_with_boolean_true_values(): + """Test that boolean True values work correctly (control test).""" + config_dict = { + "configurable": { + "enable_web_search": True, + "enable_deep_thinking": True, + "enforce_web_search": True, + } + } + config = Configuration.from_runnable_config(config_dict) + + assert config.enable_web_search is True + assert config.enable_deep_thinking is True + assert config.enforce_web_search is True + + def test_get_recursion_limit_default(): from src.config.configuration import get_recursion_limit