Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- Add integration tests under `src/runtime/tests/` when changing CLI/runtime behavior.
- Prefer `assert_cmd` for CLI and `predicates` for output checks. Keep fixtures in `src/runtime/tests/data/`.
- Run `cargo test --workspace` locally; ensure tests don’t rely on network input.
- Prefer modifying existing tests over adding new ones for new code paths. Extend current scenarios with extra assertions/fixtures to avoid redundant tests proliferating. For example, if adding null-handling in diff/patch, enhance the existing diff tests rather than introducing separate "basic diff works" tests that become redundant.

## Commit & Pull Request Guidelines
- Commits: short, imperative summary (e.g., “Add __repr__ for LinkMLValue”); group related changes.
Expand Down
15 changes: 12 additions & 3 deletions src/python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ impl PyLinkMLValue {
match &self.value {
LinkMLValue::Scalar { slot, .. } => Some(slot.name.clone()),
LinkMLValue::List { slot, .. } => Some(slot.name.clone()),
LinkMLValue::Null { slot, .. } => Some(slot.name.clone()),
_ => None,
}
}
Expand All @@ -491,6 +492,7 @@ impl PyLinkMLValue {
fn kind(&self) -> String {
match &self.value {
LinkMLValue::Scalar { .. } => "scalar".to_string(),
LinkMLValue::Null { .. } => "null".to_string(),
LinkMLValue::List { .. } => "list".to_string(),
LinkMLValue::Mapping { .. } => "mapping".to_string(),
LinkMLValue::Object { .. } => "object".to_string(),
Expand All @@ -502,6 +504,7 @@ impl PyLinkMLValue {
match &self.value {
LinkMLValue::Scalar { slot, .. } => Some(slot.definition().clone()),
LinkMLValue::List { slot, .. } => Some(slot.definition().clone()),
LinkMLValue::Null { slot, .. } => Some(slot.definition().clone()),
_ => None,
}
}
Expand All @@ -512,6 +515,7 @@ impl PyLinkMLValue {
LinkMLValue::Object { class, .. } => Some(class.def().clone()),
LinkMLValue::Scalar { class: Some(c), .. } => Some(c.def().clone()),
LinkMLValue::List { class: Some(c), .. } => Some(c.def().clone()),
LinkMLValue::Null { class: Some(c), .. } => Some(c.def().clone()),
_ => None,
}
}
Expand All @@ -522,13 +526,15 @@ impl PyLinkMLValue {
LinkMLValue::Object { class, .. } => Some(class.def().name.clone()),
LinkMLValue::Scalar { class: Some(c), .. } => Some(c.def().name.clone()),
LinkMLValue::List { class: Some(c), .. } => Some(c.def().name.clone()),
LinkMLValue::Null { class: Some(c), .. } => Some(c.def().name.clone()),
_ => None,
}
}

fn __len__(&self) -> PyResult<usize> {
Ok(match &self.value {
LinkMLValue::Scalar { .. } => 0,
LinkMLValue::Null { .. } => 0,
LinkMLValue::List { values, .. } => values.len(),
LinkMLValue::Mapping { values, .. } => values.len(),
LinkMLValue::Object { values, .. } => values.len(),
Expand Down Expand Up @@ -645,6 +651,9 @@ impl PyLinkMLValue {
LinkMLValue::Scalar { value, slot, .. } => {
format!("LinkMLValue.Scalar(slot='{}', value={})", slot.name, value)
}
LinkMLValue::Null { slot, .. } => {
format!("LinkMLValue.Null(slot='{}')", slot.name)
}
LinkMLValue::List { values, slot, .. } => {
format!(
"LinkMLValue.List(slot='{}', len={})",
Expand Down Expand Up @@ -725,17 +734,17 @@ fn load_json(
Ok(PyLinkMLValue::new(v, sv))
}

#[pyfunction(name = "diff", signature = (source, target, ignore_missing_target=None))]
#[pyfunction(name = "diff", signature = (source, target, treat_missing_as_null=None))]
fn py_diff(
py: Python<'_>,
source: &PyLinkMLValue,
target: &PyLinkMLValue,
ignore_missing_target: Option<bool>,
treat_missing_as_null: Option<bool>,
) -> PyResult<PyObject> {
let deltas = diff_internal(
&source.value,
&target.value,
ignore_missing_target.unwrap_or(false),
treat_missing_as_null.unwrap_or(false),
);
let vals: Vec<JsonValue> = deltas
.iter()
Expand Down
66 changes: 48 additions & 18 deletions src/runtime/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ impl LinkMLValue {
pub fn to_json(&self) -> JsonValue {
match self {
LinkMLValue::Scalar { value, .. } => value.clone(),
LinkMLValue::Null { .. } => JsonValue::Null,
LinkMLValue::List { values, .. } => {
JsonValue::Array(values.iter().map(|v| v.to_json()).collect())
}
Expand All @@ -46,13 +47,20 @@ impl LinkMLValue {
}
}

pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: bool) -> Vec<Delta> {
/// Compute a semantic diff between two LinkMLValue trees.
///
/// Semantics of nulls and missing values:
/// - X -> null: update to null (old = X, new = null).
/// - null -> X: update from null (old = null, new = X).
/// - missing -> X: add (old = None, new = X).
/// - X -> missing: ignored by default; if `treat_missing_as_null` is true, update to null (old = X, new = null).
pub fn diff(source: &LinkMLValue, target: &LinkMLValue, treat_missing_as_null: bool) -> Vec<Delta> {
fn inner(
path: &mut Vec<String>,
slot: Option<&SlotView>,
s: &LinkMLValue,
t: &LinkMLValue,
ignore_missing: bool,
treat_missing_as_null: bool,
out: &mut Vec<Delta>,
) {
if let Some(sl) = slot {
Expand Down Expand Up @@ -81,14 +89,17 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: b
.or_else(|| tc.slots().iter().find(|s| s.name == *k));
path.push(k.clone());
match tm.get(k) {
Some(tv) => inner(path, slot_view, sv, tv, ignore_missing, out),
Some(tv) => inner(path, slot_view, sv, tv, treat_missing_as_null, out),
None => {
if !ignore_missing && !slot_view.is_some_and(slot_is_ignored) {
out.push(Delta {
path: path.clone(),
old: Some(sv.to_json()),
new: None,
});
if !slot_view.is_some_and(slot_is_ignored) {
// Missing target slot: either ignore (default) or treat as update to null
if treat_missing_as_null {
out.push(Delta {
path: path.clone(),
old: Some(sv.to_json()),
new: Some(JsonValue::Null),
});
}
}
}
}
Expand Down Expand Up @@ -118,7 +129,9 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: b
for i in 0..max_len {
path.push(i.to_string());
match (sl.get(i), tl.get(i)) {
(Some(sv), Some(tv)) => inner(path, None, sv, tv, ignore_missing, out),
(Some(sv), Some(tv)) => {
inner(path, None, sv, tv, treat_missing_as_null, out)
}
(Some(sv), None) => out.push(Delta {
path: path.clone(),
old: Some(sv.to_json()),
Expand All @@ -140,7 +153,9 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: b
for k in keys {
path.push(k.clone());
match (sm.get(&k), tm.get(&k)) {
(Some(sv), Some(tv)) => inner(path, None, sv, tv, ignore_missing, out),
(Some(sv), Some(tv)) => {
inner(path, None, sv, tv, treat_missing_as_null, out)
}
(Some(sv), None) => out.push(Delta {
path: path.clone(),
old: Some(sv.to_json()),
Expand All @@ -156,14 +171,29 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: b
path.pop();
}
}
_ => {
let sv = s.to_json();
let tv = t.to_json();
if sv != tv {
(LinkMLValue::Null { .. }, LinkMLValue::Null { .. }) => {}
(LinkMLValue::Null { .. }, tv) => {
out.push(Delta {
path: path.clone(),
old: Some(JsonValue::Null),
new: Some(tv.to_json()),
});
}
(sv, LinkMLValue::Null { .. }) => {
out.push(Delta {
path: path.clone(),
old: Some(sv.to_json()),
new: Some(JsonValue::Null),
});
}
(sv, tv) => {
let sj = sv.to_json();
let tj = tv.to_json();
if sj != tj {
out.push(Delta {
path: path.clone(),
old: Some(sv),
new: Some(tv),
old: Some(sj),
new: Some(tj),
});
}
}
Expand All @@ -175,7 +205,7 @@ pub fn diff(source: &LinkMLValue, target: &LinkMLValue, ignore_missing_target: b
None,
source,
target,
ignore_missing_target,
treat_missing_as_null,
&mut out,
);
out
Expand Down
40 changes: 34 additions & 6 deletions src/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ pub enum LinkMLValue {
class: Option<ClassView>,
sv: SchemaView,
},
Null {
slot: SlotView,
class: Option<ClassView>,
sv: SchemaView,
},
List {
values: Vec<LinkMLValue>,
slot: SlotView,
Expand Down Expand Up @@ -104,6 +109,7 @@ impl LinkMLValue {
current = values.get(key)?;
}
LinkMLValue::Scalar { .. } => return None,
LinkMLValue::Null { .. } => return None,
}
}
Some(current)
Expand Down Expand Up @@ -253,6 +259,12 @@ impl LinkMLValue {
sv: sv.clone(),
})
}
// Preserve explicit null as a Null value for list-valued slot
(false, JsonValue::Null) => Ok(LinkMLValue::Null {
slot: sl.clone(),
class: Some(class.clone()),
sv: sv.clone(),
}),
(false, other) => Err(LinkMLError(format!(
"expected list for slot `{}`, found {:?} at {}",
sl.name,
Expand Down Expand Up @@ -371,6 +383,12 @@ impl LinkMLValue {
sv: sv.clone(),
})
}
// Preserve explicit null as a Null value for mapping-valued slot
JsonValue::Null => Ok(LinkMLValue::Null {
slot: sl.clone(),
class: class.clone(),
sv: sv.clone(),
}),
other => Err(LinkMLError(format!(
"expected mapping for slot `{}`, found {:?} at {}",
sl.name,
Expand Down Expand Up @@ -483,12 +501,20 @@ impl LinkMLValue {
classview_name
))
})?;
Ok(LinkMLValue::Scalar {
value,
slot: sl,
class: Some(class.clone()),
sv: sv.clone(),
})
if value.is_null() {
Ok(LinkMLValue::Null {
slot: sl,
class: Some(class.clone()),
sv: sv.clone(),
})
} else {
Ok(LinkMLValue::Scalar {
value,
slot: sl,
class: Some(class.clone()),
sv: sv.clone(),
})
}
}

fn from_json_internal(
Expand Down Expand Up @@ -616,6 +642,7 @@ fn validate_inner(value: &LinkMLValue) -> std::result::Result<(), String> {
}
Ok(())
}
LinkMLValue::Null { .. } => Ok(()),
LinkMLValue::List { values, .. } => {
for v in values {
validate_inner(v)?;
Expand Down Expand Up @@ -647,6 +674,7 @@ pub fn validate(value: &LinkMLValue) -> std::result::Result<(), String> {
fn validate_collect(value: &LinkMLValue, errors: &mut Vec<String>) {
match value {
LinkMLValue::Scalar { .. } => {}
LinkMLValue::Null { .. } => {}
LinkMLValue::List { values, .. } => {
for v in values {
validate_collect(v, errors);
Expand Down
14 changes: 14 additions & 0 deletions src/runtime/src/turtle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ fn serialize_map<W: Write>(
}
}
}
LinkMLValue::Null { .. } => {
// Null is treated as absent; emit nothing
}
LinkMLValue::Object { values, class, .. } => {
let class_ref = &class;
let (obj, child_id) =
Expand Down Expand Up @@ -288,6 +291,9 @@ fn serialize_map<W: Write>(
}
}
}
LinkMLValue::Null { .. } => {
// Skip null items
}
LinkMLValue::Object {
values: mv, class, ..
} => {
Expand Down Expand Up @@ -364,6 +370,9 @@ fn serialize_map<W: Write>(
}
}
}
LinkMLValue::Null { .. } => {
// nothing
}
LinkMLValue::Object {
values: mv, class, ..
} => {
Expand Down Expand Up @@ -523,6 +532,7 @@ pub fn write_turtle<W: Write>(
formatter.serialize_triple(triple.as_ref())?;
}
}
LinkMLValue::Null { .. } => {}
LinkMLValue::List { .. } => {}
LinkMLValue::Mapping { .. } => {}
}
Expand Down Expand Up @@ -578,12 +588,16 @@ pub fn write_turtle<W: Write>(
formatter.serialize_triple(triple.as_ref())?;
}
}
LinkMLValue::Null { .. } => {
// nothing
}
LinkMLValue::List { .. } => {}
LinkMLValue::Mapping { .. } => {}
}
}
}
LinkMLValue::Scalar { .. } => {}
LinkMLValue::Null { .. } => {}
}
let out_buf = formatter.finish()?;
let mut out = String::from_utf8(out_buf).unwrap_or_default();
Expand Down
11 changes: 11 additions & 0 deletions src/runtime/tests/data/example_personinfo_data_nulls.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
objects:
- id: P:100
objecttype: https://w3id.org/linkml/examples/personinfo/Person
name: Null Collections Person
# multivalued scalar list
aliases: null
# inlined-as-list of class instances
has_employment_history: null
# inlined-as-dict of class instances
has_familial_relationships: null

Loading
Loading