Skip to content

Fix Delta JSON round-trip crash when orjson is not installed#613

Open
chuenchen309 wants to merge 1 commit into
qlustered:devfrom
chuenchen309:fix/delta-json-opcode-roundtrip
Open

Fix Delta JSON round-trip crash when orjson is not installed#613
chuenchen309 wants to merge 1 commit into
qlustered:devfrom
chuenchen309:fix/delta-json-opcode-roundtrip

Conversation

@chuenchen309

Copy link
Copy Markdown

The bug

Delta's documented JSON round-trip crashes on a default install (one without the optional orjson extra) for any list diff that goes through difflib opcodes — i.e. reorders or duplicates:

from deepdiff import DeepDiff, Delta
from deepdiff.serialization import json_dumps, json_loads

t1 = ['a', 'b', 'c', 'd', 'b', 'e']
t2 = ['b', 'c', 'x', 'b', 'e', 'f']
diff = DeepDiff(t1, t2)

dump = Delta(diff, serializer=json_dumps).dumps()
Delta(dump, deserializer=json_loads)      # TypeError
# deepdiff.helper.Opcode() argument after ** must be a mapping, not list

This is the repo's own contract: tests/test_delta.py::TestDeltaCompareFunc::test_list_of_alphabet_and_its_delta already asserts exactly this round-trip. It passes in CI only because the dev/test environment installs orjson. On a plain pip install deepdiff that test fails, because orjson is optional ([project.dependencies] is just orderly-set; orjson lives in the optimize/dev/test extras).

Root cause

A serialize/deserialize asymmetry around the Opcode NamedTuple:

  • Serialize — with orjson, the encoder's default callback runs JSON_CONVERTOR[tuple] (serialization.py), turning each Opcode into a mapping via _asdict(). The stdlib json fallback (serialization.py, json_dumps when orjson is None) serializes any tuple/NamedTuple subclass natively as a JSON array and never invokes default, so an Opcode becomes a positional list like ["equal", 1, 3, 0, 2, null, null].
  • Deserializedelta.py unconditionally does Opcode(**op_code), assuming a mapping. On an orjson-less install the opcode is a list, so **list raises TypeError.

A serialize-side fix alone can't work: stdlib json never calls default for tuple subclasses, so the robust fix is on deserialization.

The fix

Accept both encodings when rebuilding opcodes — mapping → Opcode(**op_code), array → Opcode(*op_code) (the array is in NamedTuple field order, so positional reconstruction is exact).

Verification (re-runnable from the diff)

  • Without orjson, before the fix: test_list_of_alphabet_and_its_delta fails at delta.py with the TypeError above. After the fix it passes, and the full tests/test_delta.py goes from 25 failures to 1.
  • With orjson (the CI condition): tests/test_delta.py is 129 passed, 0 failed — the mapping path is untouched, so no regression.
  • Added test_delta_iterable_opcodes_json_roundtrip_builtin_json, which forces the stdlib-json path via force_use_builtin_json=True. It fails on the current code even with orjson installed, so the round-trip is now guarded regardless of the install, not just when orjson happens to be absent.

One thing left out of scope

The remaining no-orjson failure is test_delta_repr: it asserts orjson's compact separators ({"iterable_item_added":{...}}), while stdlib json emits spaces ({"iterable_item_added": {...}}). That's a separate repr/formatting assumption, unrelated to this crash — I left it alone to keep the diff focused, but happy to address it here or in a follow-up if you'd prefer.

Base branch is dev per AGENTS.md.


This PR was authored by an AI coding agent (Claude Code) running on this account: the AI found the bug, ran the repro, wrote the tests, and wrote this description. The human account holder reviews every change and is accountable for it. The verification above is real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.

Delta serializes each iterable Opcode NamedTuple, then rebuilds it in
the JSON deserializer with Opcode(**op_code), which assumes a mapping.
That only holds under orjson: its default callback runs JSON_CONVERTOR
and emits Opcodes as dicts. The stdlib json fallback (used on any
install without the optional orjson extra) encodes NamedTuples natively
as positional arrays, so Opcode(**op_code) raises TypeError and the
documented round-trip Delta(dump, deserializer=json_loads) crashes for
any list diff that goes through difflib opcodes (reorders / duplicates).

Accept both shapes on deserialization: mapping -> Opcode(**op_code),
array -> Opcode(*op_code) (fields are in NamedTuple order). Added a
regression test that forces the builtin-json path via
force_use_builtin_json=True so it guards the array shape even where
orjson is installed.
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