runtime: introduce PatchOptions {ignore_no_ops, treat_missing_as_null} with defaults; make no-op skipping configurable; wire CLI --ignore-noop/--treat-missing-as-null; update Python patch signature; update tests

Frank Dekervel · Frank Dekervel · commit 4b34691e19bd · 2025-09-11T15:29:24.000+02:00
diff --git a/src/python/src/lib.rs b/src/python/src/lib.rs
@@ -764,22 +764,30 @@ fn py_diff(
     Ok(json_value_to_py(py, &JsonValue::Array(vals)))
 }
 
-#[pyfunction(name = "patch", signature = (source, deltas, treat_missing_as_null = false))]
+#[pyfunction(name = "patch", signature = (source, deltas, treat_missing_as_null = true, ignore_no_ops = true))]
 fn py_patch(
     py: Python<'_>,
     source: &PyLinkMLValue,
     deltas: &Bound<'_, PyAny>,
     treat_missing_as_null: bool,
+    ignore_no_ops: bool,
 ) -> PyResult<PyObject> {
     let json_mod = PyModule::import(py, "json")?;
     let deltas_str: String = json_mod.call_method1("dumps", (deltas,))?.extract()?;
     let deltas_vec: Vec<Delta> =
         serde_json::from_str(&deltas_str).map_err(|e| PyException::new_err(e.to_string()))?;
     let sv_ref = source.sv.bind(py).borrow();
     let rust_sv = sv_ref.as_rust();
-    let (new_value, trace) =
-        patch_internal(&source.value, &deltas_vec, rust_sv, treat_missing_as_null)
-            .map_err(|e| PyException::new_err(e.to_string()))?;
+    let (new_value, trace) = patch_internal(
+        &source.value,
+        &deltas_vec,
+        rust_sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops,
+            treat_missing_as_null,
+        },
+    )
+    .map_err(|e| PyException::new_err(e.to_string()))?;
     let trace_json = serde_json::json!({
         "added": trace.added,
         "deleted": trace.deleted,
diff --git a/src/runtime/src/diff.rs b/src/runtime/src/diff.rs
@@ -248,23 +248,31 @@ pub struct PatchTrace {
     pub updated: Vec<NodeId>,
 }
 
+#[derive(Debug, Clone, Copy)]
+pub struct PatchOptions {
+    pub ignore_no_ops: bool,
+    pub treat_missing_as_null: bool,
+}
+
+impl Default for PatchOptions {
+    fn default() -> Self {
+        Self {
+            ignore_no_ops: true,
+            treat_missing_as_null: true,
+        }
+    }
+}
+
 pub fn patch(
     source: &LinkMLValue,
     deltas: &[Delta],
     sv: &SchemaView,
-    treat_missing_as_null: bool,
+    opts: PatchOptions,
 ) -> LResult<(LinkMLValue, PatchTrace)> {
     let mut out = source.clone();
     let mut trace = PatchTrace::default();
     for d in deltas {
-        apply_delta_linkml(
-            &mut out,
-            &d.path,
-            &d.new,
-            sv,
-            &mut trace,
-            treat_missing_as_null,
-        )?;
+        apply_delta_linkml(&mut out, &d.path, &d.new, sv, &mut trace, opts)?;
     }
     Ok((out, trace))
 }
@@ -303,7 +311,7 @@ fn apply_delta_linkml(
     newv: &Option<serde_json::Value>,
     sv: &SchemaView,
     trace: &mut PatchTrace,
-    treat_missing_as_null: bool,
+    opts: PatchOptions,
 ) -> LResult<()> {
     if path.is_empty() {
         if let Some(v) = newv {
@@ -317,7 +325,7 @@ fn apply_delta_linkml(
             let conv = sv.converter();
             if let Some(cls) = class_opt {
                 let new_node = LinkMLValue::from_json(v.clone(), cls, slot_opt, sv, &conv, false)?;
-                if current.equals(&new_node, treat_missing_as_null) {
+                if opts.ignore_no_ops && current.equals(&new_node, opts.treat_missing_as_null) {
                     // No-op delta; skip to preserve node IDs
                     return Ok(());
                 }
@@ -346,7 +354,9 @@ fn apply_delta_linkml(
                             false,
                         )?;
                         if let Some(old_child) = values.get_mut(key) {
-                            if old_child.equals(&new_child, treat_missing_as_null) {
+                            if opts.ignore_no_ops
+                                && old_child.equals(&new_child, opts.treat_missing_as_null)
+                            {
                                 // no-op; skip
                                 return Ok(());
                             }
@@ -368,7 +378,8 @@ fn apply_delta_linkml(
                             }
                         } else {
                             // adding a Null assignment may be a no-op when treating missing as null
-                            if treat_missing_as_null
+                            if opts.ignore_no_ops
+                                && opts.treat_missing_as_null
                                 && matches!(new_child, LinkMLValue::Null { .. })
                             {
                                 return Ok(());
@@ -381,7 +392,8 @@ fn apply_delta_linkml(
                     }
                     None => {
                         if let Some(old_child) = values.get(key) {
-                            if treat_missing_as_null
+                            if opts.ignore_no_ops
+                                && opts.treat_missing_as_null
                                 && matches!(old_child, LinkMLValue::Null { .. })
                             {
                                 // deleting a Null assignment: no-op
@@ -395,7 +407,7 @@ fn apply_delta_linkml(
                     }
                 }
             } else if let Some(child) = values.get_mut(key) {
-                apply_delta_linkml(child, &path[1..], newv, sv, trace, treat_missing_as_null)?;
+                apply_delta_linkml(child, &path[1..], newv, sv, trace, opts)?;
             }
         }
         LinkMLValue::Mapping { values, slot, .. } => {
@@ -412,7 +424,9 @@ fn apply_delta_linkml(
                             Vec::new(),
                         )?;
                         if let Some(old_child) = values.get(key) {
-                            if old_child.equals(&new_child, treat_missing_as_null) {
+                            if opts.ignore_no_ops
+                                && old_child.equals(&new_child, opts.treat_missing_as_null)
+                            {
                                 return Ok(());
                             }
                             mark_deleted_subtree(old_child, trace);
@@ -430,7 +444,7 @@ fn apply_delta_linkml(
                     }
                 }
             } else if let Some(child) = values.get_mut(key) {
-                apply_delta_linkml(child, &path[1..], newv, sv, trace, treat_missing_as_null)?;
+                apply_delta_linkml(child, &path[1..], newv, sv, trace, opts)?;
             }
         }
         LinkMLValue::List {
@@ -455,7 +469,9 @@ fn apply_delta_linkml(
                                 &conv,
                                 Vec::new(),
                             )?;
-                            if values[idx].equals(&new_child, treat_missing_as_null) {
+                            if opts.ignore_no_ops
+                                && values[idx].equals(&new_child, opts.treat_missing_as_null)
+                            {
                                 return Ok(());
                             }
                             match (&mut values[idx], &new_child) {
@@ -504,14 +520,7 @@ fn apply_delta_linkml(
                     }
                 }
             } else if idx < values.len() {
-                apply_delta_linkml(
-                    &mut values[idx],
-                    &path[1..],
-                    newv,
-                    sv,
-                    trace,
-                    treat_missing_as_null,
-                )?;
+                apply_delta_linkml(&mut values[idx], &path[1..], newv, sv, trace, opts)?;
             }
         }
         LinkMLValue::Scalar { .. } => {}
diff --git a/src/runtime/tests/diff.rs b/src/runtime/tests/diff.rs
@@ -57,7 +57,16 @@ fn diff_and_patch_person() {
         }
     }
 
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     let patched_json = patched.to_json();
     let target_json = tgt.to_json();
     let src_json = src.to_json();
@@ -93,7 +102,16 @@ fn diff_ignore_missing_target() {
 
     let deltas = diff(&src, &tgt, false);
     assert!(deltas.is_empty());
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     let patched_json = patched.to_json();
     let src_json = src.to_json();
     assert_eq!(patched_json, src_json);
@@ -135,7 +153,16 @@ fn diff_and_patch_personinfo() {
             assert!(tgt.navigate_path(&d.path).is_some());
         }
     }
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert_eq!(patched.to_json(), tgt.to_json());
 }
 
diff --git a/src/runtime/tests/diff_identifier.rs b/src/runtime/tests/diff_identifier.rs
@@ -76,7 +76,16 @@ fn single_inlined_object_identifier_change_is_replacement() {
     assert!(d.old.is_some() && d.new.is_some());
 
     // Patch should yield target
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert_eq!(patched.to_json(), tgt.to_json());
 }
 
@@ -145,7 +154,16 @@ fn single_inlined_object_non_identifier_change_is_field_delta() {
             "diagnosis".to_string()
         ]));
 
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert_eq!(patched.to_json(), tgt.to_json());
 }
 
@@ -194,7 +212,16 @@ fn list_inlined_object_identifier_change_is_replacement() {
         .iter()
         .any(|d| d.path == vec!["objects".to_string(), "2".to_string(), "id".to_string()]));
 
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert_eq!(patched.to_json(), tgt.to_json());
 }
 
@@ -241,6 +268,15 @@ fn mapping_inlined_identifier_change_is_add_delete() {
         .iter()
         .any(|d| d.path == vec!["things".to_string(), "alpha".to_string(), "key".to_string()]));
 
-    let (patched, _trace) = patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, _trace) = patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert_eq!(patched.to_json(), tgt.to_json());
 }
diff --git a/src/runtime/tests/trace.rs b/src/runtime/tests/trace.rs
@@ -70,7 +70,16 @@ fn node_ids_preserved_scalar_update() {
     .unwrap();
 
     let deltas = diff(&src, &tgt, false);
-    let (patched, trace) = linkml_runtime::patch(&src, &deltas, &sv, false).unwrap();
+    let (patched, trace) = linkml_runtime::patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
 
     assert!(trace.added.is_empty());
     assert!(trace.deleted.is_empty());
@@ -123,7 +132,16 @@ fn patch_trace_add_in_list() {
     let deltas = diff(&base, &target, false);
     let mut pre = Vec::new();
     collect_ids(&base, &mut pre);
-    let (patched, trace) = linkml_runtime::patch(&base, &deltas, &sv, false).unwrap();
+    let (patched, trace) = linkml_runtime::patch(
+        &base,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     let mut post = Vec::new();
     collect_ids(&patched, &mut post);
 
@@ -164,7 +182,16 @@ fn patch_missing_to_null_semantics() {
 
     // treat_missing_as_null = true => no-op; no trace changes, no node id changes
     let pre_id = src.node_id();
-    let (patched_same, trace_same) = linkml_runtime::patch(&src, &deltas, &sv, true).unwrap();
+    let (patched_same, trace_same) = linkml_runtime::patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: true,
+        },
+    )
+    .unwrap();
     assert!(
         trace_same.added.is_empty()
             && trace_same.deleted.is_empty()
@@ -179,7 +206,16 @@ fn patch_missing_to_null_semantics() {
     }
 
     // treat_missing_as_null = false => apply explicit null
-    let (patched_null, trace_applied) = linkml_runtime::patch(&src, &deltas, &sv, false).unwrap();
+    let (patched_null, trace_applied) = linkml_runtime::patch(
+        &src,
+        &deltas,
+        &sv,
+        linkml_runtime::diff::PatchOptions {
+            ignore_no_ops: true,
+            treat_missing_as_null: false,
+        },
+    )
+    .unwrap();
     assert!(trace_applied.updated.contains(&patched_null.node_id()));
     // age present as Null
     if let LinkMLValue::Object { values, .. } = &patched_null {
diff --git a/src/tools/src/bin/linkml_patch.rs b/src/tools/src/bin/linkml_patch.rs
