Skip to content

Keyless-list diff: gap-based opcodes instead of positional matching - #103

Open
SirHephaistos wants to merge 8 commits into
mainfrom
fix/wi193-repeat-table-delta-lcs
Open

Keyless-list diff: gap-based opcodes instead of positional matching#103
SirHephaistos wants to merge 8 commits into
mainfrom
fix/wi193-repeat-table-delta-lcs

Conversation

@SirHephaistos

Copy link
Copy Markdown
Collaborator

Keyless-list diff: gap-based opcodes instead of positional matching

Diffing a multivalued inlined list whose items have no key/identifier matched
elements by index. Deleting or inserting a row shifted the list, so the diff
mis-attributed the change: surviving rows became Updates and the wrong row was
removed; a delete combined with an add read as an edit. (Downstream this broke
asset360 repeat-table row coloring + the removed-rows summary — work item WI193.)

Change

Rewrites the keyless branch of fn diff (src/runtime/src/diff.rs) into an
LCS gap-diff:

  • LCS over structural equality (equals) gives matched anchors; the runs between
    anchors are gaps.
  • Per gap, positions are paired as replaces (recurse → field-level Update)
    only when the two rows are similar (rows_similar: objects sharing ≥ half
    their fields; scalars always). Dissimilar pairs and leftover rows become
    Remove (source) + Add (target). This distinguishes a real field edit from a
    delete+add of unrelated rows.
  • Round-trip-safe. The patcher overwrites a list slot by index or appends and
    applies index Removes in descending order — it has no insert-at-index. So clean
    opcodes are emitted only when every insert appends (clean_safe); a mid-list
    insert falls back to the previous positional overwrite+append encoding. Removes
    are emitted last (adds append first) so every index stays valid on apply.

The keyed-list branch is unchanged (still matches by identifier).

Behavior delta (keyless lists)

Case Before After
delete first/middle row shifted Updates + wrong Remove clean Remove(s)
delete + add (unrelated) positional Updates Remove + Add
field edit (row keeps most fields) Update Update (unchanged)
reorder swap two Updates move (Remove + Add)
single-field row replace Update Remove + Add (edit/replace indistinguishable without a key)
append / mid-insert unchanged unchanged

Tests

src/runtime/tests/diff.rs extends diff_and_patch_keyless_object_list_shifts
with delete-first, duplicate, in-place edit, mid-insert (positional fallback),
combined delete+append, and same-position dissimilar replace — each asserts the
opcode shape and round-trips through patch. cargo test -p linkml_runtime,
clippy -D warnings, fmt, and stub_gen --check all clean.

Alexander and others added 6 commits June 19, 2026 10:34
Positional by-index diff of keyless inlined lists turned a row delete
into shifted Updates + a wrong-row Remove (WI193). Replace the else
branch with an LCS over structural equality: unchanged rows match by
subsequence, leftovers pair (in-place field Updates) then Remove/Add.
Keyed branch and patch-side index handling unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers WI193: delete-first-row -> single Remove (survivor not flagged),
duplicate rows -> one Remove by multiplicity, in-place edit -> single
field-level Update. Each round-trips through patch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…etion

The index-based patcher can overwrite-by-index or append but has no
insert-at-index, so emitting a clean Add for a mid-list insert would not
round-trip. Restrict the LCS path to pure deletions (target is an ordered
subsequence of source) where clean Removes round-trip via descending-index
application; everything else (insert/reorder/edit/mixed) keeps the
positional overwrite+append encoding the patcher can replay. Fixes WI193
(row delete no longer flags survivors) without regressing insert round-trips.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the pure-deletion-only hybrid with a full LCS gap diff: per gap
between matched anchors, pair rows as replaces (field Updates), excess
source rows as Removes, excess target rows as Adds. A combined
delete+add (e.g. [P,Q]->[Q,R]) now yields remove P + add R instead of
mis-attributed positional Updates. Stays round-trip-safe: clean opcodes
only when inserts append (suffix); mid-list inserts fall back to
positional overwrite+append.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A gap pairing two leftover rows was always a field-level Update, so a
deleted row replaced in place by an unrelated new row read as an edit
(WI193 combined case). Pair as an Update only when the rows are similar
(>= half their fields unchanged); dissimilar rows become Remove + Add.
Such an Add must append, so a dissimilar pair outside the trailing gap
falls back to the positional encoding for round-trip safety.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@SirHephaistos
SirHephaistos marked this pull request as ready for review June 19, 2026 13:15
@SirHephaistos
SirHephaistos requested a review from Copilot June 19, 2026 13:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates linkml_runtime’s semantic diffing for keyless inlined-object lists (lists whose items have no key/identifier slot) to avoid index-based “shift” misattribution by switching to an LCS (longest common subsequence) gap-based diff strategy, while preserving patch round-trip correctness via a positional fallback when mid-list inserts would be required.

Changes:

  • Implement LCS-based matching for keyless lists, emitting gap-derived Update/Add/Remove opcodes when patch-safe.
  • Add a rows_similar heuristic to decide when a gap pair should be treated as an in-place edit (field-level Update) vs Remove+Add.
  • Extend runtime diff/patch regression tests for keyless object lists (WI193) to cover deletes, duplicates, edits, mid-inserts, and dissimilar replacements.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/runtime/src/diff.rs Rewrites keyless list diffing to use LCS + gap opcode emission with patch-safety fallback; adds row similarity heuristic.
src/runtime/tests/diff.rs Adds/extends regression tests validating opcode shape and diff→patch round-trips for keyless object list scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/runtime/src/diff.rs Outdated
Comment thread src/runtime/src/diff.rs Outdated
Address PR review:
- rows_similar now counts a null-vs-missing field as equal when
  treat_missing_as_null, matching LinkMLInstance::equals, so that
  difference no longer flips a field edit into Remove+Add.
- Cap the O(n*m) LCS table (flat usize grid, ~8MB) and fall back to the
  linear positional diff for lists above the cap, bounding memory/time.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@SirHephaistos

Copy link
Copy Markdown
Collaborator Author

TLDR

Repeat-table rows have no unique key, so the diff matched rows by position — deleting or adding a row shifted everything, making the diff flag the wrong rows as edited/removed and lose the "Removed"
notification (WI193). We replaced the by-index matching with a sequence diff (LCS) that matches rows by content and tells a real edit from a delete+add, in both the Rust diff engine (persisted
changeset) and the Angular frontend (row coloring), plus tracked deletions directly from the user's click so the Removed + Restore list is always correct. This makes adding/deleting/editing
repeat-table rows show the right colors and the right removed entries instead of corrupting which row changed.

@SirHephaistos

Copy link
Copy Markdown
Collaborator Author

Both Copilot points are addressed in a1aaa1d:

  • rows_similar now counts a null-vs-missing field as equal when treat_missing_as_null (matches LinkMLInstance::equals), so it no longer flips a field edit into Remove+Add.
  • The LCS table is now a flat Vec<usize> capped at ~8 MB; lists above the cap fall back to the linear positional diff.

The two inline comments above are re-posts of the original review against the pre-fix revision.

Edit-vs-replace is a UI concern, not the diff engine's: deciding whether a
same-position content change is an in-place edit or a delete+add can't be
done from row content without a key. Remove the rows_similar guess — the
engine now reports a same-position change as a field-level Update; consumers
resolve add/remove intent from tracked ops. LCS stays (correct deletes),
clean opcodes only when inserts append (else positional). Tests updated:
same-position replace is an Update; pure delete/append/reorder/combined
unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants