Skip to content

Fix hash diffing algorithm #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 24, 2025
Merged

Fix hash diffing algorithm #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac6d934
Write tests for hash operation tree builder
jas14 Jun 1, 2025
dcaa00b
Rewrite hash diffing using key LCS
jas14 Oct 24, 2025
5b9e7d5
Add this PR to the changelog
jas14 Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Add official Ruby 3.4 support. [#289](https://github.com/splitwise/super_diff/pull/289) by [@olleolleolle](https://github.com/olleolleolle)

### Bug fixes

- Fix hash diffing algorithm. [#293](https://github.com/splitwise/super_diff/pull/293)

### Other changes

- Fix bundler gem caching in CI. [#289](https://github.com/splitwise/super_diff/pull/289) by [@olleolleolle](https://github.com/olleolleolle)
Expand Down
246 changes: 73 additions & 173 deletions lib/super_diff/basic/operation_tree_builders/hash.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,195 +21,95 @@ def build_operation_tree
private

def unary_operations_using_variant_of_patience_algorithm
operations = []
aks = actual.keys
eks = expected.keys
previous_ei = nil
ei = 0
ai = 0

# When diffing a hash, we're more interested in the 'actual' version
# than the 'expected' version, because that's the ultimate truth.
# Therefore, the diff is presented from the perspective of the 'actual'
# hash, and we start off by looping over it.
while ai < aks.size
ak = aks[ai]
av = actual[ak]
ev = expected[ak]
# While we iterate over 'actual' in order, we jump all over
# 'expected', trying to match up its keys with the keys in 'actual' as
# much as possible.
ei = eks.index(ak)

if should_add_noop_operation?(ak)
# (If we're here, it probably means that the key we're pointing to
# in the 'actual' and 'expected' hashes have the same value.)

if ei && previous_ei && (ei - previous_ei) > 1
# If we've jumped from one operation in the 'expected' hash to
# another operation later in 'expected' (due to the fact that the
# 'expected' hash is in a different order than 'actual'), collect
# any delete operations in between and add them to our operations
# array as deletes before adding the noop. If we don't do this
# now, then those deletes will disappear. (Again, we are mainly
# iterating over 'actual', so this is the only way to catch all of
# the keys in 'expected'.)
(previous_ei + 1).upto(ei - 1) do |ei2|
ek = eks[ei2]
ev2 = expected[ek]
av2 = actual[ek]

next unless
(!actual.include?(ek) || ev2 != av2) &&
operations.none? do |operation|
%i[delete noop].include?(operation.name) &&
operation.key == ek
end

operations << Core::UnaryOperation.new(
name: :delete,
collection: expected,
key: ek,
value: ev2,
index: ei2
)
end
end

operations = []
lcs_callbacks = LCSCallbacks.new(
operations: operations,
expected: expected,
actual: actual,
should_add_noop_operation: method(:should_add_noop_operation?)
)
Diff::LCS.traverse_sequences(eks, aks, lcs_callbacks)

operations
end

class LCSCallbacks
extend AttrExtras.mixin

pattr_initialize %i[operations! expected! actual! should_add_noop_operation!]
public :operations

def discard_a(event)
# This key is in `expected`, but may also be in `actual`.
key = event.old_element

# We want the diff to match the key order of `actual` as much as
# possible, so if this is also a key in `actual` that's just being
# reordered, don't add any operations now. The change or noop will
# be added later.
return if actual.include?(key)

operations << Core::UnaryOperation.new(
name: :delete,
collection: expected,
key: key,
value: expected[key],
index: event.old_position
)
end

def discard_b(event)
# This key is in `actual`, but may also be in `expected`.

key = event.new_element
handle_operation_on_key_in_actual(key, event)
end

def match(event)
# This key is part of the longest common subsequence.

key = event.old_element
handle_operation_on_key_in_actual(key, event)
end

private

def handle_operation_on_key_in_actual(key, event)
if should_add_noop_operation.call(key)
operations << Core::UnaryOperation.new(
name: :noop,
collection: actual,
key: ak,
value: av,
index: ai
key: key,
value: actual[key],
index: event.new_position
)
else
# (If we're here, it probably means that the key in 'actual' isn't
# present in 'expected' or the values don't match.)

if (operations.empty? || operations.last.name == :noop) &&
(ai.zero? || eks.include?(aks[ai - 1]))

# If we go from a match in the last iteration to a missing or
# extra key in this one, or we're at the first key in 'actual' and
# it's missing or extra, look for deletes in the 'expected' hash
# and add them to our list of operations before we add the
# inserts. In most cases we will accomplish this by backtracking a
# bit to the key in 'expected' that matched the key in 'actual' we
# processed in the previous iteration (or just the first key in
# 'expected' if this is the first key in 'actual'), and then
# iterating from there through 'expected' until we reach the end
# or we hit some other condition (see below).

start_index =
if ai.positive?
eks.index(aks[ai - 1]) + 1
else
0
end

start_index.upto(eks.size - 1) do |ei2|
ek = eks[ei2]
ev = expected[ek]
av2 = actual[ek]

if actual.include?(ek) && ev == av2
# If the key in 'expected' we've landed on happens to be a
# match in 'actual', then stop, because it's going to be
# handled in some future iteration of the 'actual' loop.
break
elsif aks[ai + 1..].any? do |k| # rubocop:disable Lint/DuplicateBranch for clarity
expected.include?(k) && expected[k] != actual[k]
end

# While we backtracked a bit to iterate over 'expected', we
# now have to look ahead. If we will end up encountering a
# insert that matches this delete later, stop and go back to
# iterating over 'actual'. This is because the delete we would
# have added now will be added later when we encounter the
# associated insert, so we don't want to add it twice.
break
else
operations << Core::UnaryOperation.new(
name: :delete,
collection: expected,
key: ek,
value: ev,
index: ei2
)
end

next unless ek == ak && ev != av

# If we're pointing to the same key in 'expected' as in
# 'actual', but with different values, go ahead and add an
# insert now to accompany the delete added above. That way
# they appear together, which will be easier to read.
operations << Core::UnaryOperation.new(
name: :insert,
collection: actual,
key: ak,
value: av,
index: ai
)
end
end

if expected.include?(ak) && ev != av &&
operations.none? do |op|
op.name == :delete && op.key == ak
end

# If we're here, it means that we didn't encounter any delete
# operations above for whatever reason and so we need to add a
# delete to represent the fact that the value for this key has
# changed.
# If the key is present in both `actual` and `expected`
# but the values don't match, create a `delete` operation
# just before the `insert`.
# (This condition will always be true for `match` events.)
if expected.include?(key)
operations << Core::UnaryOperation.new(
name: :delete,
collection: expected,
key: ak,
value: expected[ak],
index: ei
key: key,
value: expected[key],
index: event.old_position
)
end

if operations.none? { |op| op.name == :insert && op.key == ak }
# If we're here, it means that we didn't encounter any insert
# operations above. Since we already handled delete, the only
# alternative is that this key must not exist in 'expected', so
# we need to add an insert.
operations << Core::UnaryOperation.new(
name: :insert,
collection: actual,
key: ak,
value: av,
index: ai
)
end
operations << Core::UnaryOperation.new(
name: :insert,
collection: actual,
key: key,
value: actual[key],
index: event.new_position
)
end

ai += 1
previous_ei = ei
end

# The last thing to do is this: if there are keys in 'expected' that
# aren't in 'actual', and they aren't associated with any inserts to
# where they would have been added above, tack those deletes onto the
# end of our operations array.
(eks - aks - operations.map(&:key)).each do |ek|
ei = eks.index(ek)
ev = expected[ek]

operations << Core::UnaryOperation.new(
name: :delete,
collection: expected,
key: ek,
value: ev,
index: ei
)
end

operations
end

def should_add_noop_operation?(key)
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/rspec/include_matcher_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,8 @@
plain_line ' {'
plain_line ' number: 42,'
plain_line %( city: "Burbank",)
plain_line %( zip: "90210")
expected_line %(- state: "CA")
plain_line %( zip: "90210")
plain_line ' }'
end
)
Expand Down
Loading