diff/patch: refactor

src/python/src/lib.rs

@@ -499,6 +499,11 @@ impl PyLinkMLValue {
         }
     }
 
+    #[getter]
+    fn node_id(&self) -> u64 {
+        self.value.node_id()
+    }
+
     #[getter]
     fn slot_definition(&self) -> Option<SlotDefinition> {
         match &self.value {
@@ -758,15 +763,25 @@ fn py_patch(
     py: Python<'_>,
     source: &PyLinkMLValue,
     deltas: &Bound<'_, PyAny>,
-) -> PyResult<PyLinkMLValue> {
+) -> 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 = patch_internal(&source.value, &deltas_vec, rust_sv);
-    Ok(PyLinkMLValue::new(new_value, source.sv.clone_ref(py)))
+    let (new_value, trace) = patch_internal(&source.value, &deltas_vec, rust_sv)
+        .map_err(|e| PyException::new_err(e.to_string()))?;
+    let trace_json = serde_json::json!({
+        "added": trace.added,
+        "deleted": trace.deleted,
+        "updated": trace.updated,
+    });
+    let py_val = PyLinkMLValue::new(new_value, source.sv.clone_ref(py));
+    let dict = pyo3::types::PyDict::new(py);
+    dict.set_item("value", Py::new(py, py_val)?)?;
+    dict.set_item("trace", json_value_to_py(py, &trace_json))?;
+    Ok(dict.into_any().unbind())
 }
 
 #[pyfunction(name = "to_turtle", signature = (value, skolem=None))]

src/runtime/src/diff.rs

@@ -1,7 +1,7 @@
-use crate::{load_json_str, LinkMLValue};
+use crate::{LResult, LinkMLError, LinkMLValue, NodeId};
 use linkml_schemaview::schemaview::{SchemaView, SlotView};
 use serde::{Deserialize, Serialize};
-use serde_json::{Map, Value as JsonValue};
+use serde_json::Value as JsonValue;
 
 const IGNORE_ANNOTATION: &str = "diff.linkml.io/ignore";
 
@@ -211,84 +211,240 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, treat_missing_as_null: b
     out
 }
 
-pub fn patch(source: &LinkMLValue, deltas: &[Delta], sv: &SchemaView) -> LinkMLValue {
-    let mut json = source.to_json();
+#[derive(Debug, Clone, Default)]
+pub struct PatchTrace {
+    pub added: Vec<NodeId>,
+    pub deleted: Vec<NodeId>,
+    pub updated: Vec<NodeId>,
+}
+
+pub fn patch(
+    source: &LinkMLValue,
+    deltas: &[Delta],
+    sv: &SchemaView,
+) -> LResult<(LinkMLValue, PatchTrace)> {
+    let mut out = source.clone();
+    let mut trace = PatchTrace::default();
     for d in deltas {
-        apply_delta(&mut json, d);
+        apply_delta_linkml(&mut out, &d.path, &d.new, sv, &mut trace)?;
     }
-    let json_str = serde_json::to_string(&json).unwrap();
-    let conv = sv.converter();
-    match source {
-        LinkMLValue::Object { class: ref c, .. } => load_json_str(&json_str, sv, c, &conv).unwrap(),
-        _ => panic!("patching non-map values is not supported here"),
+    Ok((out, trace))
+}
+
+fn collect_all_ids(value: &LinkMLValue, ids: &mut Vec<NodeId>) {
+    ids.push(value.node_id());
+    match value {
+        LinkMLValue::Scalar { .. } => {}
+        LinkMLValue::Null { .. } => {}
+        LinkMLValue::List { values, .. } => {
+            for v in values {
+                collect_all_ids(v, ids);
+            }
+        }
+        LinkMLValue::Mapping { values, .. } | LinkMLValue::Object { values, .. } => {
+            for v in values.values() {
+                collect_all_ids(v, ids);
+            }
+        }
     }
 }
 
-fn apply_delta(value: &mut JsonValue, delta: &Delta) {
-    apply_delta_inner(value, &delta.path, &delta.new);
+fn mark_added_subtree(v: &LinkMLValue, trace: &mut PatchTrace) {
+    collect_all_ids(v, &mut trace.added);
 }
 
-fn apply_delta_inner(value: &mut JsonValue, path: &[String], newv: &Option<JsonValue>) {
+fn mark_deleted_subtree(v: &LinkMLValue, trace: &mut PatchTrace) {
+    collect_all_ids(v, &mut trace.deleted);
+}
+
+// Removed thin wrappers; call LinkMLValue builders directly at call sites.
+
+fn apply_delta_linkml(
+    current: &mut LinkMLValue,
+    path: &[String],
+    newv: &Option<serde_json::Value>,
+    sv: &SchemaView,
+    trace: &mut PatchTrace,
+) -> LResult<()> {
     if path.is_empty() {
         if let Some(v) = newv {
-            *value = v.clone();
+            let (class_opt, slot_opt) = match current {
+                LinkMLValue::Object { class, .. } => (Some(class.clone()), None),
+                LinkMLValue::List { class, slot, .. } => (class.clone(), Some(slot.clone())),
+                LinkMLValue::Mapping { class, slot, .. } => (class.clone(), Some(slot.clone())),
+                LinkMLValue::Scalar { class, slot, .. } => (class.clone(), Some(slot.clone())),
+                LinkMLValue::Null { class, slot, .. } => (class.clone(), Some(slot.clone())),
+            };
+            let conv = sv.converter();
+            if let Some(cls) = class_opt {
+                let new_node = LinkMLValue::from_json(v.clone(), cls, slot_opt, sv, &conv, false)?;
+                mark_deleted_subtree(current, trace);
+                mark_added_subtree(&new_node, trace);
+                *current = new_node;
+            }
         }
-        return;
+        return Ok(());
     }
-    match value {
-        JsonValue::Object(map) => {
+
+    match current {
+        LinkMLValue::Object { values, class, .. } => {
             let key = &path[0];
             if path.len() == 1 {
                 match newv {
                     Some(v) => {
-                        map.insert(key.clone(), v.clone());
+                        if let Some(old_child) = values.get_mut(key) {
+                            if let LinkMLValue::Scalar { value, .. } = old_child {
+                                if !v.is_object() && !v.is_array() {
+                                    *value = v.clone();
+                                    trace.updated.push(old_child.node_id());
+                                    return Ok(());
+                                }
+                            }
+                            let conv = sv.converter();
+                            let slot = class.slots().iter().find(|s| s.name == *key).cloned();
+                            let new_child = LinkMLValue::from_json(
+                                v.clone(),
+                                class.clone(),
+                                slot,
+                                sv,
+                                &conv,
+                                false,
+                            )?;
+                            let old_snapshot = std::mem::replace(old_child, new_child);
+                            mark_deleted_subtree(&old_snapshot, trace);
+                            mark_added_subtree(old_child, trace);
+                            trace.updated.push(current.node_id());
+                        } else {
+                            let conv = sv.converter();
+                            let slot = class.slots().iter().find(|s| s.name == *key).cloned();
+                            let new_child = LinkMLValue::from_json(
+                                v.clone(),
+                                class.clone(),
+                                slot,
+                                sv,
+                                &conv,
+                                false,
+                            )?;
+                            // mark before insert
+                            mark_added_subtree(&new_child, trace);
+                            values.insert(key.clone(), new_child);
+                            trace.updated.push(current.node_id());
+                        }
                     }
                     None => {
-                        map.remove(key);
+                        if let Some(old_child) = values.remove(key) {
+                            mark_deleted_subtree(&old_child, trace);
+                            trace.updated.push(current.node_id());
+                        }
                     }
                 }
-            } else {
-                let entry = map
-                    .entry(key.clone())
-                    .or_insert(JsonValue::Object(Map::new()));
-                apply_delta_inner(entry, &path[1..], newv);
+            } else if let Some(child) = values.get_mut(key) {
+                apply_delta_linkml(child, &path[1..], newv, sv, trace)?;
             }
         }
-        JsonValue::Array(arr) => {
-            let idx: usize = path[0].parse().unwrap();
+        LinkMLValue::Mapping { values, slot, .. } => {
+            let key = &path[0];
             if path.len() == 1 {
                 match newv {
                     Some(v) => {
-                        if idx < arr.len() {
-                            arr[idx] = v.clone();
-                        } else if idx == arr.len() {
-                            arr.push(v.clone());
-                        } else {
-                            while arr.len() < idx {
-                                arr.push(JsonValue::Null);
-                            }
-                            arr.push(v.clone());
+                        let conv = sv.converter();
+                        let new_child = LinkMLValue::build_mapping_entry_for_slot(
+                            slot,
+                            v.clone(),
+                            sv,
+                            &conv,
+                            Vec::new(),
+                        )?;
+                        if let Some(old_child) = values.get(key) {
+                            mark_deleted_subtree(old_child, trace);
                         }
+                        // mark before insert
+                        mark_added_subtree(&new_child, trace);
+                        values.insert(key.clone(), new_child);
+                        trace.updated.push(current.node_id());
                     }
                     None => {
-                        if idx < arr.len() {
-                            arr.remove(idx);
+                        if let Some(old_child) = values.remove(key) {
+                            mark_deleted_subtree(&old_child, trace);
+                            trace.updated.push(current.node_id());
                         }
                     }
                 }
-            } else {
-                if idx >= arr.len() {
-                    arr.resize(idx + 1, JsonValue::Null);
-                }
-                apply_delta_inner(&mut arr[idx], &path[1..], newv);
+            } else if let Some(child) = values.get_mut(key) {
+                apply_delta_linkml(child, &path[1..], newv, sv, trace)?;
             }
         }
-        _ => {
-            if path.is_empty() {
-                if let Some(v) = newv {
-                    *value = v.clone();
+        LinkMLValue::List {
+            values,
+            slot,
+            class,
+            ..
+        } => {
+            let idx: usize = path[0].parse().map_err(|_| {
+                LinkMLError(format!("invalid list index '{}' in delta path", path[0]))
+            })?;
+            if path.len() == 1 {
+                match newv {
+                    Some(v) => {
+                        if idx < values.len() {
+                            let is_scalar_target =
+                                matches!(values[idx], LinkMLValue::Scalar { .. });
+                            if is_scalar_target && !v.is_object() && !v.is_array() {
+                                if let LinkMLValue::Scalar { value, .. } = &mut values[idx] {
+                                    *value = v.clone();
+                                    trace.updated.push(values[idx].node_id());
+                                }
+                            } else {
+                                let conv = sv.converter();
+                                let new_child = LinkMLValue::build_list_item_for_slot(
+                                    slot,
+                                    class.as_ref(),
+                                    v.clone(),
+                                    sv,
+                                    &conv,
+                                    Vec::new(),
+                                )?;
+                                let old = std::mem::replace(&mut values[idx], new_child);
+                                mark_deleted_subtree(&old, trace);
+                                mark_added_subtree(&values[idx], trace);
+                                trace.updated.push(current.node_id());
+                            }
+                        } else if idx == values.len() {
+                            let conv = sv.converter();
+                            let new_child = LinkMLValue::build_list_item_for_slot(
+                                slot,
+                                class.as_ref(),
+                                v.clone(),
+                                sv,
+                                &conv,
+                                Vec::new(),
+                            )?;
+                            // mark before push
+                            mark_added_subtree(&new_child, trace);
+                            values.push(new_child);
+                            trace.updated.push(current.node_id());
+                        } else {
+                            return Err(LinkMLError(format!(
+                                "list index out of bounds in add: {} > {}",
+                                idx,
+                                values.len()
+                            )));
+                        }
+                    }
+                    None => {
+                        if idx < values.len() {
+                            let old = values.remove(idx);
+                            mark_deleted_subtree(&old, trace);
+                            trace.updated.push(current.node_id());
+                        }
+                    }
                 }
+            } else if idx < values.len() {
+                apply_delta_linkml(&mut values[idx], &path[1..], newv, sv, trace)?;
             }
         }
+        LinkMLValue::Scalar { .. } => {}
+        LinkMLValue::Null { .. } => {}
     }
+    Ok(())
 }