Skip to content

Fix make_patch TypeError for numeric-string dict key moves#182

Open
gaoflow wants to merge 1 commit into
stefankoegl:masterfrom
gaoflow:fix-item-added-numeric-str-key
Open

Fix make_patch TypeError for numeric-string dict key moves#182
gaoflow wants to merge 1 commit into
stefankoegl:masterfrom
gaoflow:fix-item-added-numeric-str-key

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 20, 2026

Copy link
Copy Markdown

Summary

make_patch raises an uncaught TypeError instead of producing a valid patch when the diff moves a list element while a numeric-string dict key is also removed:

import jsonpatch
jsonpatch.make_patch({'0': [], 'd': [], 'a': [-1, {'a': ['s']}]},
                     {'d': [0, []], 'c': ['s2']})
# TypeError: '>=' not supported between instances of 'str' and 'int'

Cause

DiffBuilder._item_added guarded its index-shifting undo loop with type(op.key) == int and type(key) == int. But PatchOperation.key casts numeric-string dict keys (e.g. "0") to int, so a dict-key removal at /0 passes the int check and wrongly enters the loop. The loop then calls _on_undo_remove on sibling ops, and a RemoveOperation with a genuine string key ('a') hits if self.key >= key:, comparing 'a' >= 0TypeError.

The sibling _item_removed already fixed this exact bug class for issues #119/#120 by checking the affected container's type instead of trusting op.key's type; _item_added was left with the old heuristic.

Fix

Mirror the _item_removed fix: check whether the removed item's container in src_doc is a list (using op.pointer.to_last(self.src_doc)[0]) rather than relying on the int-cast key. The destination key type is irrelevant to whether the source remove-index needs shifting, so — like _item_removed — the redundant type(key) == int check is dropped.

make_patch now returns a valid, round-tripping patch ([remove /a, add /c, add /d/0, move /0→/d/1]) for these inputs.

Verification

  • Added a regression test (symmetric to test_issue119/test_issue120) that fails before the fix with the TypeError.
  • Full suite: 117 tests OK.
  • A 40k-pair make_patch/apply_patch round-trip fuzz, run identically against the previous code and this change: this fix is a strict subset of the prior failures (it introduces zero new failures) and additionally repairs round-trip cases where a list element moves into a dict key. The few remaining fuzz failures are distinct, pre-existing issues (move/add ordering) left out to keep this PR focused.

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

DiffBuilder._item_added used `type(op.key) == int` to decide whether a
matched remove was a list-element move needing index shifting. But
PatchOperation.key casts numeric-string dict keys (e.g. "0") to int, so
removing such a dict key wrongly entered the shifting loop, which then
compared a real string key against an int and raised
"TypeError: '>=' not supported between instances of 'str' and 'int'".

Check the removed item's container type in src_doc instead, mirroring the
existing _item_removed fix (issues 119/120). make_patch now returns a
valid, round-tripping patch for these cases.
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