Repository navigation
Improve list diffing with sequence alignment to avoid cascading changes - #206
Merged
Merged
Conversation
make_patch compared list items by index, so inserting or removing an item made every item after it look changed. Move detection undid some of that for scalars, but lists of objects or arrays were rebuilt field by field: inserting one object at the front of a list of 2000 gave 4003 operations instead of one 'add'. Lists are now aligned before their items are compared. A common prefix and suffix are skipped, and difflib.SequenceMatcher aligns the rest. SequenceMatcher ignores items that occur often in long lists, so its alignment is only used if it changes fewer items than comparing them by index. Items are matched by a key that follows the existing notion of equality: leaves are compared by their JSON dump, so 1 and true stay different, and the key order of objects does not matter. This re-enables test_should_just_add_new_item_not_rebuild_all_list, which was disabled when the DiffBuilder was introduced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R62pG28eF1nRucQY4KsxoJ
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Alignment can generate incorrect patches for JSON-distinct containers, and its runtime is unbounded quadratic.
2 open findings
What changed in this PR
Improves list diff generation by aligning unchanged items to avoid cascading patch operations.
Changes:
- Adds content-based item keys and sequence alignment.
- Refactors recursive item comparison and expands list-diff tests.
| File | Description |
|---|---|
jsonpatch.py |
Implements aligned list diffing. |
tests.py |
Adds insertion, removal, repetition, and ordering cases. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Move detection stored values by (value, type(value)) and compared unhashable ones with ==, so [1] and [true] were considered equal and one was moved where the other belongs. Aligning lists made this reachable from more inputs, e.g. [[1], [2], [3]] -> [[2], [3], [true]]. Values are now stored by the same JSON-aware key that aligns list items (#180). Values that dumps cannot serialize get a key equal only to themselves, so they can still be added, removed and moved as before. SequenceMatcher compares each item with the equal items of the other list, which took 22 seconds for a list of 100,000 repeated numbers whose first and last items were replaced. Aligning is now skipped if that takes more comparisons than comparing by index would take steps, which is quadratic in the number of items changed by index, as the diff turns them into moves, with a floor for lists that differ in few items. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R62pG28eF1nRucQY4KsxoJ
Master now matches up list items itself (_differing_runs, #78), which replaces the alignment of this branch: it also estimates operations recursively and matches up all items of shorter lists. What this branch adds on top of it: - Move detection keys values like _differing_runs does, so that values that are equal only in Python (e.g. [1] and [true]) are no longer moved where the other belongs. Values that cannot be serialized are only moved where they are added unchanged. - For long lists, matching up is skipped if SequenceMatcher would take more comparisons than max(_EXACT_MATCH_LIMIT, operations by position squared). It took 22 s for 100,000 repeated numbers whose first and last items were replaced, and takes 0.8 s now. - The test from #83 is enabled, and tests of this branch that master's tests for #78 cover are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R62pG28eF1nRucQY4KsxoJ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves the JSON patch generation algorithm for lists by implementing intelligent sequence alignment. Instead of comparing list items by index position, the algorithm now identifies which items have actually moved or changed, preventing cascading modifications when items are inserted or removed.
Key Changes
_changed_blocksfunction): Usesdifflib.SequenceMatcherto intelligently align source and destination lists, identifying blocks of changes rather than comparing by index alone_item_keymethod): Creates hashable keys for complex items (dicts, lists) to enable proper item matching across list transformations, with keys based on content rather than identity_compare_listsmethod): Completely rewrote to use the new alignment algorithm, which maps items by their content and only reports actual changes_compare_itemsmethod): Separated the recursive comparison logic into its own method for better code organizationtest_should_just_add_new_item_not_rebuild_all_listis now enabled and passing, demonstrating the fix for issue Fix optimizing list insertion/deletion diffs #83Notable Implementation Details
_changed_itemshelper function evaluates the cost of different alignment strategies to choose the optimal oneTest Coverage
Added comprehensive test cases covering:
https://claude.ai/code/session_01R62pG28eF1nRucQY4KsxoJ
fixes #83