infrahub_sdk/utils.py (5)

151-153: List merge is correct but O(n^2); consider a fast path for empty lists.

Current membership checks make this quadratic. A small readability/perf win: early-return when either side is empty.

Apply this diff to short-circuit obvious cases:

-            elif isinstance(a_val, list) and isinstance(b_val, list):
+            elif isinstance(a_val, list) and isinstance(b_val, list):
                 # Merge lists
                 # Cannot use compare_list because list of dicts won't work (dict not hashable)
-                dicta[key] = [i for i in a_val if i not in b_val] + b_val
+                if not a_val:
+                    dicta[key] = b_val
+                elif not b_val:
+                    dicta[key] = a_val
+                else:
+                    dicta[key] = [i for i in a_val if i not in b_val] + b_val

---

154-157: None-aware merge resolves the “plain vs property” conflict (issue #7022).

• If a_val is None and b_val is not None → take b_val.
• If b_val is None → keep a_val.
• Equal scalars → no-op.
  This exactly handles cases like {"name": None} + {"name": {"value": None}} → {"name": {"value": None}}.

Consider documenting this behavior in the function docstring for future readers.

---

139-141: Document the new None-handling semantics in the docstring.

Clarify merge rules to prevent regressions and ease maintenance.

Apply this diff to enhance the docstring:

 def deep_merge_dict(dicta: dict, dictb: dict, path: list | None = None) -> dict:
-    """Deep Merge Dictionary B into Dictionary A.
-    Code is inspired by https://stackoverflow.com/a/7205107
-    """
+    """Deep merge Dictionary B into Dictionary A (mutates A).
+    - Dicts are merged recursively.
+    - Lists are merged by appending unique items from A that are not in B, preserving B's order.
+    - None-handling:
+      * If A has None and B has non-None, take B's value.
+      * If B has None and A has non-None, keep A's value.
+      * If values are equal, keep A.
+    - For differing non-None, non-mergeable scalars, a ValueError is raised (“Conflict at <path>”).
+    Code is inspired by https://stackoverflow.com/a/7205107
+    """

---

144-147: Minor readability improvement: iterate items to avoid repeated dict lookups.

Using items() removes double indexing and keeps b_val scoped consistently.

Apply this diff:

-    for key in dictb:
-        b_val = dictb[key]
+    for key, b_val in dictb.items():
         if key in dicta:
-            a_val = dicta[key]
+            a_val = dicta[key]

---

160-161: Use the local b_val for consistency and a tiny perf win.

Avoids re-indexing dictb.

Apply this diff:

-        else:
-            dicta[key] = dictb[key]
+        else:
+            dicta[key] = b_val

tests/unit/sdk/test_utils.py (1)

99-105: Good coverage for scalar None-vs-non-None; add nested dict vs None to reflect the reported issue.

To directly exercise the “name” vs “name__value” case that motivated this PR, add tests where one side is a dict and the other is None.

Apply this diff within the same test or as separate assertions:

     assert deep_merge_dict(d, e) == {"keyA": [10, 20, 30], "keyB": "foo"}
     assert deep_merge_dict(f, g) == {"keyA": "foo", "keyB": "bar"}
+
+    # None vs dict handling (motivating scenario)
+    h = {"name": None}
+    i = {"name": {"value": None}}
+    assert deep_merge_dict(h, i) == {"name": {"value": None}}
+    # Reverse direction should preserve the non-None dict
+    assert deep_merge_dict(i, h) == {"name": {"value": None}}

Alternatively, you can add a dedicated test function:

def test_deep_merge_dict_none_vs_dict():
    assert deep_merge_dict({"name": None}, {"name": {"value": None}}) == {"name": {"value": None}}
    assert deep_merge_dict({"name": {"value": None}}, {"name": None}) == {"name": {"value": None}}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro