fix: refactor dotted key expansion to handle nested objects recursively

[hamzahalq]

Summary by CodeRabbit

• Bug Fixes
  • Improved JSON key handling to properly expand dotted notation recursively throughout the entire rendered output structure, rather than only at the top level.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR modifies ScribanJsonHelper.cs to expand dotted JSON keys recursively throughout nested structures instead of only at the top level. A new ExpandDottedKeys helper walks JToken trees and applies dotted-to-nested conversion at every depth, replacing the prior top-level-only transformation in the Render method.

Changes

Recursive dotted-key expansion in JSON rendering

Layer / File(s)	Summary
Recursive expansion helper and integration SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs	A new ExpandDottedKeys private method recursively traverses JObject and JArray structures to expand dotted keys at all nesting levels. The Render method now calls this helper instead of applying top-level-only dotted-key transformation, enabling nested objects to have their dotted properties expanded as well.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Dots within dots, nested deep,
Now expand through structures we keep,
From surface to root, all paths unwind,
Recursive magic, one change aligned! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: refactoring dotted key expansion to recursively handle nested objects, which aligns with the code changes that added recursive traversal of JObject and JArray structures.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hamza/fix/mapper-issues

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs`:
- Around line 143-145: The loop that calls SetByPath for each prop after
ExpandDottedKeys can silently overwrite when dotted paths collide (e.g.,
existing "a" vs incoming "a.b"); modify the logic to detect such collisions and
throw instead of overwriting: either add a pre-check in the caller before
SetByPath that inspects the current result for existing nodes along the dotted
path (using prop.Name and the structure returned by ExpandDottedKeys) or harden
SetByPath itself to validate each path segment (in SetByPath, when
traversing/creating nodes, throw an exception if a scalar value exists where an
object is required or an object exists where a scalar is being set). Ensure the
thrown exception includes the conflicting path and the names of the existing and
incoming node types so callers can fail fast on collisions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0c42f40-aede-40db-aaaa-951e934d2e91

📥 Commits

Reviewing files that changed from the base of the PR and between ca88acf and 4f58883.

📒 Files selected for processing (1)

• SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Detect and block dotted-path collisions to prevent silent data loss.

Line 144 now applies SetByPath recursively at every depth. If an object has both a and a.b, one value is overwritten based on property order. Please fail fast on collision instead of replacing existing nodes.

💡 Proposed fix

 private static void SetByPath(JObject root, string path, JToken value)
 {
     var parts = path.Split('.');
     JObject current = root;
     for (int i = 0; i < parts.Length - 1; i++)
     {
         var part = parts[i];
-        if (current[part] is not JObject child)
-        {
-            child = new JObject();
-            current[part] = child;
-        }
-        current = child;
+        var existing = current[part];
+        if (existing is JObject child)
+        {
+            current = child;
+            continue;
+        }
+
+        if (existing != null)
+            throw new InvalidOperationException(
+                $"Cannot expand dotted path '{path}': segment '{part}' is already a non-object value.");
+
+        var newChild = new JObject();
+        current[part] = newChild;
+        current = newChild;
     }
-    current[parts[^1]] = value;
+    var leaf = parts[^1];
+    if (current[leaf] is JObject && value is not JObject)
+        throw new InvalidOperationException(
+            $"Cannot set path '{path}': destination is already an object.");
+
+    current[leaf] = value;
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SW.Bitween.NativeAdapters/JsonMapper/ScribanJsonHelper.cs` around lines 143 -
145, The loop that calls SetByPath for each prop after ExpandDottedKeys can
silently overwrite when dotted paths collide (e.g., existing "a" vs incoming
"a.b"); modify the logic to detect such collisions and throw instead of
overwriting: either add a pre-check in the caller before SetByPath that inspects
the current result for existing nodes along the dotted path (using prop.Name and
the structure returned by ExpandDottedKeys) or harden SetByPath itself to
validate each path segment (in SetByPath, when traversing/creating nodes, throw
an exception if a scalar value exists where an object is required or an object
exists where a scalar is being set). Ensure the thrown exception includes the
conflicting path and the names of the existing and incoming node types so
callers can fail fast on collisions.