Skip to content

Make diff() account for member order in ordered_json objects - #5465

Open
nlohmann wants to merge 3 commits into
developfrom
fix/ordered-json-diff-order
Open

nlohmann wants to merge 3 commits into
developfrom
fix/ordered-json-diff-order

Conversation

@nlohmann

@nlohmann nlohmann commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

basic_json::diff() walks the source object, looks each key up in the target, recurses if present, and emits remove/add for keys missing on either side — but it never considers member order. For nlohmann::json (std::map-backed) that's fine, because operator== is order-insensitive and std::map's iteration order is a pure function of the key set (always sorted by key), so two objects with the same keys always iterate in the same relative order regardless of insertion history.

For nlohmann::ordered_json (ordered_map-backed, insertion-ordered), operator== is order-sensitive (documented in operator==.md), but diff() never emitted patch ops to fix order. As a result, diff() could return a patch that, applied via patch(), does not reproduce a target that only differs in member order — violating the documented round-trip guarantee in diff.md:

source.patch(diff(source, target)) == target;

Fix

In the object case of diff(), detect whether the common keys already appear in the same relative order in source and target, and whether any new key would need to land somewhere other than the end. When both hold (always true for plain json), the original minimal per-key recursive diff is used, unchanged. When they don't (only reachable for ordered_json), every common key is removed and every key (common + new) is re-added, in target's order, with its final value — relying on patch()'s object "add" (which uses operator[]) appending at the end of an ordered_map when the key doesn't already exist, which is exactly what's needed to fix its position.

This is a no-op for plain json (the "reorder" branch is provably unreachable there), so there's no behavior or performance change for the common case.

Test plan

  • Added tests/src/unit-ordered_json.cpp regression tests covering: pure reorder, new key inserted at the front, reorder + value change, reorder + delete, reorder + nested recursive diff, 3+ key shuffle, and a minimality check confirming the fast path is unchanged for plain json and same-order ordered_json.
  • Property-tested with 20,000 randomly generated ordered_json source/target pairs (shuffled/overlapping key sets, nested values) asserting source.patch(diff(source, target)) == target — 0 failures.
  • Ran the existing tests/src/unit-json_patch.cpp suite offline to confirm all inline json::diff/patch assertions on plain json still pass.
  • Regenerated single_include/nlohmann/json.hpp via make amalgamate.

Breaking change?

No breaking changes to the public API. ordered_json::diff() now produces different (larger, but correct) patches in the specific case where the target only differs from source in member order — previously it produced an incomplete/incorrect patch there, so this is a bug fix to already-broken behavior, not a change to any documented contract. json::diff() (plain, std::map-backed) is provably unaffected.

🤖 Generated with Claude Code

@nlohmann
nlohmann changed the base branch from develop to fix/swap-diagnostics-parents September 5, 2026 14:53
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from 065dd4d to 221dcf8 Compare September 5, 2026 14:57
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Sep 5, 2026
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from 7ddf826 to 450fc8d Compare September 5, 2026 20:01
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from 450fc8d to 70125db Compare September 9, 2026 11:17
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from 70125db to 319e939 Compare September 11, 2026 08:22
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch 2 times, most recently from 7eea0c4 to ce5de56 Compare September 14, 2026 07:16
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from ce5de56 to 2a48970 Compare September 14, 2026 19:21
Base automatically changed from fix/swap-diagnostics-parents to develop September 15, 2026 18:46
diff() compared source/target objects purely by key set, ignoring
relative member order. For ordered_json (insertion-ordered, vector-
backed object_t), two objects that differ only in member order are
unequal via operator==, but diff() never emitted any patch operation
to fix the order, so source.patch(diff(source, target)) == target
could fail to hold.

Fix by detecting when common keys appear in a different relative
order in source vs. target (or when a new key would need to land
somewhere other than the end), and in that case removing and
re-adding the affected keys in target's order, which relies on
patch()'s "add" op appending new keys at the end of an ordered_map.
For plain json (std::map-backed, always key-sorted iteration) this
is a no-op and the original minimal per-key diff path is unchanged.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The previous fix for ordered_json member order re-derived common-key
order and suffix information with extra target.find()/source.find()
calls layered on top of the pre-existing removed/added-key passes,
instead of reusing those same passes. This roughly tripled the number
of map lookups per diff() call for every object, including plain
`json`, where the reordering path is never taken.

Piggyback the order tracking (and the "add" op construction for new
keys) onto the two passes the algorithm already needs to detect
removed/added keys, and walk the fast path's recursion in lockstep
with the precomputed common-key list instead of re-querying `target`.
This restores diff() to its pre-existing lookup count; benchmarked at
n=1000 keys, ordered_json::diff() was roughly 2x slower than baseline
before this change and is back within noise of baseline after it.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Splitting removed-key detection and common-key recursion into separate
passes (for the earlier lookup-count fix) changed the emitted patch's
op order: all "remove" ops now came before all recursive per-key diffs,
instead of interleaved in source's iteration order as the original
implementation did. This broke docs/mkdocs/docs/examples/diff.output's
exact-match CI check (ci_test_examples) even though the patch was still
semantically correct.

Defer "remove" emission into the same walk that does the recursive
diffs, so common keys and deleted keys are interleaved in source order
again, matching historical output.

While restructuring that walk, the reordering ("slow path") branch was
only emitting "remove" for keys common to both objects, never for keys
present in source but genuinely absent from target -- a key deleted
alongside an actual reorder would silently survive the patch. Fixed by
removing every source key in the slow path (both deleted and common
keys need removing there; common keys are then re-added in target's
order). Verified with a targeted reorder+deletion case and a fresh
20,000-case round-trip fuzz run (0 failures).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann force-pushed the fix/ordered-json-diff-order branch from 2a48970 to 86386ec Compare September 15, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L review needed It would be great if someone could review the proposed changes. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant