Skip to content

swap: fix list element order on revert#197

Open
gaoflow wants to merge 1 commit into
inveniosoftware:masterfrom
gaoflow:fix-revert-list-order
Open

swap: fix list element order on revert#197
gaoflow wants to merge 1 commit into
inveniosoftware:masterfrom
gaoflow:fix-revert-list-order

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 20, 2026

Copy link
Copy Markdown

Summary

revert() restores list elements in the wrong order when three or more elements are removed from a list, silently corrupting data and breaking the documented revert(diff(a, b), b) == a round-trip:

from dictdiffer import diff, revert
revert(diff({'x': ['p', 'q', 'r']}, {'x': []}), {'x': []})
# -> {'x': ['p', 'r', 'q']}   (expected {'x': ['p', 'q', 'r']})

It only manifests with ≥3 removed elements (with ≤2 the order happens to coincide), which is why the existing test_revert (2 elements) doesn't catch it.

Cause

diff emits list removals in descending index order so that patch's del dest[key] stays valid as the list shrinks. swap turns those remove entries into add entries, which revert/patch replay with dest.insert(key, value) — but inserting at descending indices into a growing list scrambles the order; insert-replay needs ascending order.

swap's add handler already reverses its changes (list(reversed(changes))); the remove handler omitted the symmetric reversal. That asymmetry is the bug.

Fix

Make swap's remove handler symmetric with add:

def remove(node, changes):
    return ADD, node, list(reversed(changes))

Harmless for dict removals (assignment order is irrelevant) and sets (single entry); only list inserts depend on order.

Verified: the minimal case and a 2000-case revert/patch round-trip fuzz now report 0 mismatches; dict and set reverts unchanged. Added a regression test (full removal and partial removal of ≥3 elements) that fails before the fix. Full unit suite: 88 passed, 2 skipped; the swap doctest passes. The changed line is pycodestyle-clean.

This pull request was prepared with the assistance of AI, under my direction and review.

* Reverses the changes produced by swap's remove handler so that
  reverting a diff which removed three or more list elements restores
  their original order.  diff emits list removals in descending index
  order, but revert replays them as inserts (which need ascending
  order); the add handler already reverses its changes, the remove
  handler did not.

Signed-off-by: Vincent Gao <gaobing1230@gmail.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.

1 participant