From a7c8ab3e3254f150b01a29bf1edda2f14d07f36f Mon Sep 17 00:00:00 2001 From: Serwan Asaad Date: Mon, 8 Dec 2025 21:59:04 +0100 Subject: [PATCH 1/5] Fix recursive_update_dict to add new keys from project config Previously, recursive_update_dict would skip keys that didn't exist in the base config, preventing project-specific configuration from adding new keys. This caused project overrides to be ignored when they introduced new configuration keys. Example bug scenario: - Base config: [quam] has version=3, no state_path - Project config: [quam] adds state_path="/project/path" - Result (before fix): state_path was SKIPPED - Result (after fix): state_path is ADDED The fix changes line 12 from "continue" to "to_update[k] = v; continue", ensuring new keys are added while preserving the existing recursive merge behavior for nested mappings. --- qualibrate_config/core/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qualibrate_config/core/utils.py b/qualibrate_config/core/utils.py index cab5c17..2e3a492 100644 --- a/qualibrate_config/core/utils.py +++ b/qualibrate_config/core/utils.py @@ -10,6 +10,7 @@ def recursive_update_dict( ) -> RawConfigType: for k, v in updates.items(): if k not in to_update: + to_update[k] = v continue if isinstance(v, Mapping): to_update[k] = recursive_update_dict(to_update.get(k, {}), v) From 298c824ce804632aaa922acf95110ddfe6896706 Mon Sep 17 00:00:00 2001 From: Serwan Asaad Date: Mon, 8 Dec 2025 22:02:48 +0100 Subject: [PATCH 2/5] Add unit tests for recursive_update_dict Tests cover the specific bug scenario where project configs add new keys that don't exist in the base config. The key test case is test_recursive_update_dict_project_config_use_case which verifies the real-world scenario: - Base config has [quam] section with version, no state_path - Project config adds [quam].state_path - Result should include both base and project values Additional test cases cover: - Adding new keys at various nesting levels - Updating existing keys - Adding new top-level sections - Nested merging with mixed new/existing keys - Edge cases (empty updates, non-dict overrides) All tests pass with the fix in recursive_update_dict. --- tests/unit/test_core/test_utils.py | 155 +++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 tests/unit/test_core/test_utils.py diff --git a/tests/unit/test_core/test_utils.py b/tests/unit/test_core/test_utils.py new file mode 100644 index 0000000..ab71b92 --- /dev/null +++ b/tests/unit/test_core/test_utils.py @@ -0,0 +1,155 @@ +import pytest + +from qualibrate_config.core.utils import recursive_update_dict + + +def test_recursive_update_dict_add_new_key(): + """Test that new keys from updates are added to the base dict.""" + base = {"section": {"existing": "value"}} + updates = {"section": {"new_key": "new_value"}} + + result = recursive_update_dict(base, updates) + + assert result["section"]["existing"] == "value" + assert result["section"]["new_key"] == "new_value" + + +def test_recursive_update_dict_update_existing_key(): + """Test that existing keys are properly updated.""" + base = {"section": {"key": "old_value"}} + updates = {"section": {"key": "new_value"}} + + result = recursive_update_dict(base, updates) + + assert result["section"]["key"] == "new_value" + + +def test_recursive_update_dict_add_new_top_level_section(): + """Test that completely new sections are added.""" + base = {"existing_section": {"key": "value"}} + updates = {"new_section": {"key": "value"}} + + result = recursive_update_dict(base, updates) + + assert "existing_section" in result + assert "new_section" in result + assert result["new_section"]["key"] == "value" + + +def test_recursive_update_dict_nested_merge_with_new_keys(): + """Test recursive merge preserves existing nested values while adding new ones.""" + base = { + "quam": { + "version": 3, + "serialization": {"include_defaults": True} + } + } + updates = { + "quam": { + "state_path": "/new/path", + "serialization": {"other_option": False} + } + } + + result = recursive_update_dict(base, updates) + + # Existing values preserved + assert result["quam"]["version"] == 3 + assert result["quam"]["serialization"]["include_defaults"] is True + + # New values added + assert result["quam"]["state_path"] == "/new/path" + assert result["quam"]["serialization"]["other_option"] is False + + +def test_recursive_update_dict_project_config_use_case(): + """ + Test the real-world use case: project config adding state_path + when base config doesn't have it. + + This was the bug scenario: + - Base config: [quam] has version=3, no state_path + - Project config: [quam] adds state_path="/project/path" + - Expected: state_path should be merged in + - Bug (before fix): state_path was skipped + """ + # Base config from ~/.qualibrate/config.toml + base = { + "quam": { + "raise_error_missing_reference": False, + "version": 3, + "serialization": {"include_defaults": True} + }, + "qualibrate": { + "version": 5, + "project": "CS_1" + } + } + + # Project config from ~/.qualibrate/projects/CS_1/config.toml + updates = { + "quam": { + "state_path": "/path/to/project/quam_state" + }, + "qualibrate": { + "storage": {"location": "/project/storage"} + } + } + + result = recursive_update_dict(base, updates) + + # Verify all base values preserved + assert result["quam"]["version"] == 3 + assert result["quam"]["raise_error_missing_reference"] is False + assert result["quam"]["serialization"]["include_defaults"] is True + assert result["qualibrate"]["version"] == 5 + assert result["qualibrate"]["project"] == "CS_1" + + # Verify new project-specific values added (this was the bug!) + assert result["quam"]["state_path"] == "/path/to/project/quam_state" + assert result["qualibrate"]["storage"]["location"] == "/project/storage" + + +def test_recursive_update_dict_empty_updates(): + """Test that empty updates dict doesn't break anything.""" + base = {"section": {"key": "value"}} + updates = {} + + result = recursive_update_dict(base, updates) + + assert result == base + + +def test_recursive_update_dict_non_dict_value_override(): + """Test that non-dict values properly override nested dicts.""" + base = {"section": {"key": {"nested": "value"}}} + updates = {"section": {"key": "simple_value"}} + + result = recursive_update_dict(base, updates) + + assert result["section"]["key"] == "simple_value" + + +def test_recursive_update_dict_multiple_new_keys_at_different_levels(): + """Test adding new keys at multiple nesting levels.""" + base = { + "level1": { + "level2": { + "existing": "value" + } + } + } + updates = { + "level1": { + "new_at_level2": "value2", + "level2": { + "new_at_level3": "value3" + } + } + } + + result = recursive_update_dict(base, updates) + + assert result["level1"]["level2"]["existing"] == "value" + assert result["level1"]["new_at_level2"] == "value2" + assert result["level1"]["level2"]["new_at_level3"] == "value3" From 2c7ad08513842274bf92170215691fc422b390b4 Mon Sep 17 00:00:00 2001 From: Serwan Asaad Date: Mon, 8 Dec 2025 22:04:59 +0100 Subject: [PATCH 3/5] fix lint issues --- tests/unit/test_core/test_utils.py | 40 +++++++----------------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/tests/unit/test_core/test_utils.py b/tests/unit/test_core/test_utils.py index ab71b92..2998b3f 100644 --- a/tests/unit/test_core/test_utils.py +++ b/tests/unit/test_core/test_utils.py @@ -1,5 +1,3 @@ -import pytest - from qualibrate_config.core.utils import recursive_update_dict @@ -37,17 +35,12 @@ def test_recursive_update_dict_add_new_top_level_section(): def test_recursive_update_dict_nested_merge_with_new_keys(): - """Test recursive merge preserves existing nested values while adding new ones.""" - base = { - "quam": { - "version": 3, - "serialization": {"include_defaults": True} - } - } + """Test recursive merge preserves existing values while adding new ones.""" + base = {"quam": {"version": 3, "serialization": {"include_defaults": True}}} updates = { "quam": { "state_path": "/new/path", - "serialization": {"other_option": False} + "serialization": {"other_option": False}, } } @@ -78,22 +71,15 @@ def test_recursive_update_dict_project_config_use_case(): "quam": { "raise_error_missing_reference": False, "version": 3, - "serialization": {"include_defaults": True} + "serialization": {"include_defaults": True}, }, - "qualibrate": { - "version": 5, - "project": "CS_1" - } + "qualibrate": {"version": 5, "project": "CS_1"}, } # Project config from ~/.qualibrate/projects/CS_1/config.toml updates = { - "quam": { - "state_path": "/path/to/project/quam_state" - }, - "qualibrate": { - "storage": {"location": "/project/storage"} - } + "quam": {"state_path": "/path/to/project/quam_state"}, + "qualibrate": {"storage": {"location": "/project/storage"}}, } result = recursive_update_dict(base, updates) @@ -132,19 +118,11 @@ def test_recursive_update_dict_non_dict_value_override(): def test_recursive_update_dict_multiple_new_keys_at_different_levels(): """Test adding new keys at multiple nesting levels.""" - base = { - "level1": { - "level2": { - "existing": "value" - } - } - } + base = {"level1": {"level2": {"existing": "value"}}} updates = { "level1": { "new_at_level2": "value2", - "level2": { - "new_at_level3": "value3" - } + "level2": {"new_at_level3": "value3"}, } } From d4d5900f706f8f8d3bcd3452e6dd675fc599be49 Mon Sep 17 00:00:00 2001 From: Serwan Asaad Date: Thu, 11 Dec 2025 09:56:35 +0100 Subject: [PATCH 4/5] remove reference to bug --- tests/unit/test_core/test_utils.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_core/test_utils.py b/tests/unit/test_core/test_utils.py index 2998b3f..a678776 100644 --- a/tests/unit/test_core/test_utils.py +++ b/tests/unit/test_core/test_utils.py @@ -57,14 +57,12 @@ def test_recursive_update_dict_nested_merge_with_new_keys(): def test_recursive_update_dict_project_config_use_case(): """ - Test the real-world use case: project config adding state_path - when base config doesn't have it. + Test adding new keys to project config when base config doesn't have them. - This was the bug scenario: + This test verifies that when merging configurations: - Base config: [quam] has version=3, no state_path - Project config: [quam] adds state_path="/project/path" - - Expected: state_path should be merged in - - Bug (before fix): state_path was skipped + - Expected: state_path should be merged in while preserving existing values """ # Base config from ~/.qualibrate/config.toml base = { @@ -91,7 +89,7 @@ def test_recursive_update_dict_project_config_use_case(): assert result["qualibrate"]["version"] == 5 assert result["qualibrate"]["project"] == "CS_1" - # Verify new project-specific values added (this was the bug!) + # Verify new project-specific values added assert result["quam"]["state_path"] == "/path/to/project/quam_state" assert result["qualibrate"]["storage"]["location"] == "/project/storage" From 9977aeb206fc8cda8a8c7d7884114c2d8418446a Mon Sep 17 00:00:00 2001 From: Serwan Asaad Date: Thu, 11 Dec 2025 10:02:44 +0100 Subject: [PATCH 5/5] Added docstring to recursive_update_dict --- qualibrate_config/core/utils.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/qualibrate_config/core/utils.py b/qualibrate_config/core/utils.py index 2e3a492..36366d6 100644 --- a/qualibrate_config/core/utils.py +++ b/qualibrate_config/core/utils.py @@ -8,6 +8,20 @@ def recursive_update_dict( to_update: RawConfigType, updates: Mapping[str, Any], ) -> RawConfigType: + """ + Recursively merge nested dictionaries, updating values and adding new keys. + + Used to merge project-specific config overrides on top of base config. + New keys are added, existing nested dicts are merged recursively, and + non-dict values are replaced. Modifies ``to_update`` in-place. + + Args: + to_update: Base dictionary to update (modified in-place). + updates: Dictionary with updates to apply recursively. + + Returns: + The updated dictionary (same reference as ``to_update``). + """ for k, v in updates.items(): if k not in to_update: to_update[k] = v