Skip to content

fix(diff): match keyed lists by identifier instead of by position#92

Open
kervel wants to merge 2 commits intomainfrom
fix/diff-keyed-list-remove
Open

fix(diff): match keyed lists by identifier instead of by position#92
kervel wants to merge 2 commits intomainfrom
fix/diff-keyed-list-remove

Conversation

@kervel
Copy link
Copy Markdown

@kervel kervel commented Apr 24, 2026

Summary

  • inlined_as_list collections of keyed objects were diffed positionally, so a mid-list remove produced shifted Update deltas plus a trailing Remove. On patch, the label resolver picks the first matching id, so the Remove strikes the just-updated duplicate — a pure mid-list remove looks like a no-op, and removes combined with trailing edits lose the edits.
  • Fix: when every element on both sides carries an identifier, match items by id. Emit a clean Remove for items only in the source, Add for items only in the target, and recurse on matched pairs. Lists of scalars or unkeyed objects keep the positional path; mappings were already key-matched.

Test plan

  • cargo test -p linkml_runtime --test diff — new diff_and_patch_remove_from_keyed_list regression (remove-from-middle plus trailing field update) + 9 existing
  • cargo test --workspace
  • cargo fmt --all -- --check
  • cargo clippy -p schemaview -p linkml_runtime -p linkml_tools -p linkml_runtime_python -p linkml_wasm --all-targets --all-features -- -D warnings --no-deps
  • cargo run -p linkml_runtime_python --bin stub_gen --features stubgen -- --check

Frank Dekervel added 2 commits April 24, 2026 10:06
`inlined_as_list` collections of keyed objects were diffed positionally,
so a mid-list remove produced a chain of shifted `Update` deltas plus a
trailing `Remove`. On patch, the label resolver always picks the first
matching id, so the Remove strikes the just-updated duplicate and
silently clobbers any trailing change — a pure mid-list remove appears
as a no-op, and removes combined with edits on later items lose the
edits.

When every element on both sides carries an identifier, match items by
id: emit a clean `Remove` for items only in the source, an `Add` for
items only in the target, and recurse on matched pairs. Lists of
scalars or unkeyed objects keep the existing positional algorithm.
Mappings were already key-matched.

Regression test covers remove-from-middle combined with a trailing
field update on a keyed list.
@kervel kervel requested a review from tijlwillems April 24, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant