refactor: extract shared apply_patches_impl, fix ops::apply_patches for complex values\n\n- Extract batch-flush logic into pub(crate) apply_patches_impl in document.rs\n- ops::apply_patches now delegates to shared impl (fixes complex Replace via _core API)\n- Add tests: root-level replace, indentation depths 0/2, _core.apply_patches with dict/list\n- 183 tests pass"

Author: nathanjmcdougall

src/document.rs

@@ -137,73 +137,69 @@ impl PyDocument {
     }
 
     /// Apply patches to this document and return a new document.
-    /// NOTE: Similar patch-application logic exists in ops::apply_patches (returns String).
     fn apply_patches(&self, patches: Vec<PyPatch>) -> PyResult<Self> {
-        let mut current_doc = self.inner.clone();
-        let mut batch: Vec<usize> = Vec::new();
-
-        for (idx, patch) in patches.iter().enumerate() {
-            let is_complex_replace = matches!(
-                &patch.operation.inner,
-                yamlpatch::Op::Replace(v) if matches!(v, serde_yaml::Value::Mapping(_) | serde_yaml::Value::Sequence(_))
-            );
-
-            if is_complex_replace {
-                // Flush any pending yamlpatch batch first
-                if !batch.is_empty() {
-                    let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
-                        .iter()
-                        .map(|&i| yamlpatch::Patch {
-                            route: patches[i].route.to_yamlpath_route(),
-                            operation: patches[i].operation.inner.clone(),
-                        })
-                        .collect();
-                    current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches)
-                        .map_err(|e| {
-                            PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!(
-                                "Patch failed: {e}"
-                            ))
-                        })?;
-                    batch.clear();
-                }
+        let doc = apply_patches_impl(&self.inner, &patches).map_err(|e| {
+            PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!("Patch failed: {e}"))
+        })?;
+        Ok(Self { inner: doc })
+    }
+}
 
-                // Apply the complex replace directly
-                let route = patch.route.to_yamlpath_route();
-                let value = match &patch.operation.inner {
-                    yamlpatch::Op::Replace(v) => v,
-                    _ => unreachable!(),
-                };
-                current_doc =
-                    apply_complex_replace(&current_doc, &route, value).map_err(|e| {
-                        PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!(
-                            "Patch failed: {e}"
-                        ))
-                    })?;
-            } else {
-                batch.push(idx);
+/// Shared patch-application logic used by both PyDocument::apply_patches and ops::apply_patches.
+pub(crate) fn apply_patches_impl(
+    doc: &yamlpath::Document,
+    patches: &[PyPatch],
+) -> Result<yamlpath::Document, String> {
+    let mut current_doc = doc.clone();
+    let mut batch: Vec<usize> = Vec::new();
+
+    for (idx, patch) in patches.iter().enumerate() {
+        let is_complex_replace = matches!(
+            &patch.operation.inner,
+            yamlpatch::Op::Replace(v) if matches!(v, serde_yaml::Value::Mapping(_) | serde_yaml::Value::Sequence(_))
+        );
+
+        if is_complex_replace {
+            // Flush any pending yamlpatch batch first
+            if !batch.is_empty() {
+                let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
+                    .iter()
+                    .map(|&i| yamlpatch::Patch {
+                        route: patches[i].route.to_yamlpath_route(),
+                        operation: patches[i].operation.inner.clone(),
+                    })
+                    .collect();
+                current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches)
+                    .map_err(|e| e.to_string())?;
+                batch.clear();
             }
-        }
 
-        // Flush remaining batch
-        if !batch.is_empty() {
-            let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
-                .iter()
-                .map(|&i| yamlpatch::Patch {
-                    route: patches[i].route.to_yamlpath_route(),
-                    operation: patches[i].operation.inner.clone(),
-                })
-                .collect();
-            current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches).map_err(
-                |e| {
-                    PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!(
-                        "Patch failed: {e}"
-                    ))
-                },
-            )?;
+            // Apply the complex replace directly
+            let route = patch.route.to_yamlpath_route();
+            let value = match &patch.operation.inner {
+                yamlpatch::Op::Replace(v) => v,
+                _ => unreachable!(),
+            };
+            current_doc = apply_complex_replace(&current_doc, &route, value)?;
+        } else {
+            batch.push(idx);
         }
+    }
 
-        Ok(Self { inner: current_doc })
+    // Flush remaining batch
+    if !batch.is_empty() {
+        let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
+            .iter()
+            .map(|&i| yamlpatch::Patch {
+                route: patches[i].route.to_yamlpath_route(),
+                operation: patches[i].operation.inner.clone(),
+            })
+            .collect();
+        current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches)
+            .map_err(|e| e.to_string())?;
     }
+
+    Ok(current_doc)
 }
 
 fn apply_complex_replace(

src/ops.rs

@@ -157,22 +157,13 @@ impl PyPatch {
 }
 
 /// Apply a list of patches to a YAML source string.
-/// NOTE: Similar patch-application logic exists in PyDocument::apply_patches (returns Document).
 #[pyfunction]
 pub fn apply_patches(source: &str, patches: Vec<PyPatch>) -> PyResult<String> {
     let document = yamlpath::Document::new(source).map_err(|e| {
         PyErr::new::<pyo3::exceptions::PyValueError, _>(format!("Invalid YAML: {e}"))
     })?;
 
-    let yaml_patches: Vec<yamlpatch::Patch<'_>> = patches
-        .iter()
-        .map(|p| yamlpatch::Patch {
-            route: p.route.to_yamlpath_route(),
-            operation: p.operation.inner.clone(),
-        })
-        .collect();
-
-    let result = yamlpatch::apply_yaml_patches(&document, &yaml_patches).map_err(|e| {
+    let result = crate::document::apply_patches_impl(&document, &patches).map_err(|e| {
         PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!("Patch failed: {e}"))
     })?;
 

tests/test_core_ops.py

@@ -49,6 +49,21 @@ def test_append_to_sequence(self):
         assert "- c" in result
         assert "- a" in result
 
+    def test_replace_with_dict(self):
+        source = "config:\n  key: value\n"
+        patches = [Patch(route=Route(["config"]), operation=Op.replace({"a": 1}))]
+        result = apply_patches(source, patches)
+        assert "a: 1" in result
+        assert "key: value" not in result
+
+    def test_replace_with_list(self):
+        source = "repos: []\n"
+        patches = [
+            Patch(route=Route(["repos"]), operation=Op.replace([{"repo": "local"}]))
+        ]
+        result = apply_patches(source, patches)
+        assert "repo: local" in result
+
     def test_preserves_comments(self):
         source = "# top comment\nname: foo  # inline"
         patches = [Patch(route=Route(["name"]), operation=Op.replace("bar"))]

tests/test_document.py

@@ -248,6 +248,27 @@ def test_replace_comment_relocation(self):
         result = doc2["repos"]
         assert result == [{"repo": "local"}]
 
+    def test_replace_root_level_with_dict(self):
+        doc = Document("key: value\n")
+        doc2 = doc.replace(value={"new_key": "new_val", "another": 42})
+        assert doc2["new_key"] == "new_val"
+        assert doc2["another"] == 42
+
+    def test_replace_top_level_key_with_dict(self):
+        """Indentation depth 0: top-level key gets value indented at 2 spaces."""
+        doc = Document("config: old\n")
+        doc2 = doc.replace("config", value={"a": 1})
+        assert doc2["config"] == {"a": 1}
+        # Value should be indented at 2 spaces (base_indent=0 + 2)
+        assert "  a: 1" in doc2.source
+
+    def test_replace_depth2_key_with_dict(self):
+        """Indentation depth 2: nested key gets value indented at 4 spaces."""
+        doc = Document("outer:\n  config: old\n")
+        doc2 = doc.replace("outer", "config", value={"a": 1})
+        assert doc2["outer", "config"] == {"a": 1}
+        assert "    a: 1" in doc2.source
+
 
 class TestDocumentAdd:
     def test_add_key(self):