Select optimal Levenshtein paths in EditDistance

[ESultanik]

Summary

EditDistance._best_match chose the wrong predecessor cell in the Levenshtein matrix, so graphtage reported edit costs above the optimum. Two separate defects caused it.

Defect 1: the comparison omitted transition costs. The method compared only the accumulated costs already stored in the three neighboring cells. Canonical Wagner-Fischer compares costs[r-1][c-1] + substitution_cost, costs[r][c-1] + removal_cost, and costs[r-1][c] + insertion_cost. "Cheapest predecessor" is not the same as "cheapest path" whenever the transition costs differ, which is almost always.

Defect 2: the bounds guard could not fire on a tie. Accepting the diagonal required its bounds to be strictly less than both border edits. Range.__lt__ orders lexicographically on (upper_bound, lower_bound), so two equal definitive ranges compare False in both directions. On a tie, control fell through to a branch that never compared the insertion against the removal, so the cheaper diagonal was discarded and the worse of the two borders could win.

Together these meant a substitution was never selected for strings. string_edit_distance uses insert_remove_penalty=0, so a Match between two distinct single-character StringNodes costs 1 and always ties with the insertion and the removal. Graphtage's string distance was the indel (LCS) distance, not the Levenshtein distance it documents.

Before and after

The reproducer from #89:

from graphtage import json
json.build_tree(["de", 1]).diff(json.build_tree([2])).edited_cost()

	before	after
["de", 1] -> [2]	4	3
"abcdef" -> "azced"	5	3
"abcdefg" -> "abhijfg"	6	3
"kitten" -> "sitting"	5	3

Across 200 random short-string pairs, 150 previously returned a cost above the optimum. A new property test now checks string_edit_distance against the levenshtein_distance reference already in the module.

The rendered output changes accordingly. For "abcdefg" -> "abhijfg":

before: "ab++hij++~~cde~~fg"
after:  "ab~~cde~~++hij++fg"

Canonical tie-break rule

The three candidates are scored on (path cost, number of edits along the path). The secondary key keeps the behavior added in a24764c and prefers one substitution over an insertion paired with a removal of equal cost.

Ties on both keys are broken by direction, in this fixed order:

1. diagonal (substitution)
2. border insertion
3. border removal

Reconstruction walks the matrix backwards, so preferring the insertion during traversal places the removal earlier in the forward edit sequence, which matches the convention of listing deletions before additions. Over 600 random pairs, exactly one alignment differs between this order and the diagonal-removal-insertion order, and it differs only in that ordering.

This rule is documented in the _best_match docstring. It is part of the output contract: changing it changes the edit sequence for inputs with several optimal alignments.

Re-baselined expectations

test	expectation	old	new
test_string_diff_printing	"abcdef" -> "azced" cost	5	3
test_string_diff_printing	rendered ANSI	3 removals, 2 insertions	2 substitutions, 1 removal, 1 insertion
test_string_diff_substitution_run	rendered ANSI for "abcdefg" -> "abhijfg"	3 insertions then 3 removals	3 substitutions
test_small_diff	"test": "foo" -> "bar" key/value pair cost	6	3
test_list_diff	[0, 1, 2, 3, 4, 5] -> [1, 2, 3, 4, 5]	one removal of 0	unchanged
test_ordered_list_reorder_costs	[1, 2, 3] -> [3, 2, 1] cost	4	2

test_string_diff_remove_insert_reordering is renamed to test_string_diff_substitution_run. Its edit sequence no longer contains any Insert or Remove edit, so the old name no longer described what it exercises. Its cost is now asserted as well.

test_ordered_list_reorder_costs arrived with #131 after this branch was cut and recorded the old cost. Reversing a three-element ordered list previously cost 4, as two insertions and two removals with the middle element matched. Two substitutions at 1 each make the optimum 2, so the sequence is now three matches: 1 -> 3, 2 -> 2, 3 -> 1. The test still contrasts with test_reorder_is_free, which asserts 0 for an UnorderedListNode.

test_list_diff is unchanged because the shared-suffix optimization reduces that pair to a single removal before any matrix is built. The test now asserts that outcome deliberately: exactly one removal, and a total cost of 1.

Other changes

make_distinct is dropped from _best_match. Instrumentation found the diagonal edit already definitive at all 45 call sites of the reproducer, and both border edits are always ConstantCostEdits whose tighten_bounds() is a permanent no-op. It is also insufficient for the new comparison, which needs exact costs rather than non-overlapping intervals. A _exact_cost helper replaces it: it tightens each candidate to a definitive bound and raises ValueError if it cannot, which guarantees the matrix never accumulates an Infinity upper bound into its numpy uint64 cost array. make_distinct itself stays in graphtage.bounds, where graphtage.matching still uses it.

Two adjacent defects surfaced by the new property test:

• levenshtein_distance returned the last cell its loops happened to visit. With an empty target string the inner loops never ran and it returned dist[0][0], which is 0. LeafNode.edits uses this value as a Match cost, so matching any node against an empty string looked free.
• EditDistance.bounds() never became definitive when the shared prefix and suffix consumed both sequences, for example when comparing two identical strings. is_complete() stays False because the 1x1 matrix holds only the None origin cell, and tighten_bounds() returns False immediately, so the bounds stayed at [0, 2n] and violated the tightening protocol.

Neither was reachable through TreeNode.diff, which short-circuits equal leaves, but both are reachable through the public levenshtein_distance and string_edit_distance entry points.

Verification

• Rebased onto 06e0417 with no conflicts. Full suite passes on Python 3.13 (121 passed, 88s), Python 3.8 (121 passed, 38s), and Python 3.14 (121 passed, 30s).
• Add --ignore-list-order #131's new formatting tests compare UnorderedListNode rendering against ListNode rendering and do not diff anything, so they are unaffected and pass. Keep forced color when stdout is redirected #130's change to colorama.init() does not reach Printer(ansi_color=True, out_stream=StringIO()), and the re-baselined ANSI byte sequences are byte-identical on the rebased tree.
• flake8 graphtage test --select=E9,F63,F7,F82 is clean. ruff check reports no new findings on the changed files.
• sphinx-build succeeds, with the same 5 pre-existing warnings as on master.
• No new tightening-protocol warnings: 300 random tree and string diffs with logging at WARNING produce zero records, the same as master.
• Performance improves rather than regresses. On a fixed-seed benchmark of 60 random string pairs and 60 random JSON trees: strings 1.60s -> 0.66s, trees 0.16s -> 0.11s. Fewer edits mean less downstream work.

Closes #89

🤖 Generated with Claude Code

[ESultanik]

Note on CI: an earlier run's build (3.12) job appeared to hang. It was the pre-existing issue #134, where test/test_formatting.py generates random documents without a fixed seed and occasionally hits a pathological case. The same job on master did the same thing in run 34137044499, where test_formatting.py alone took 2 hours 35 minutes and the run still passed, while every other Python version finished in about 2 minutes. That test never calls diff(), so it does not exercise the code this PR changes.

Resolved: after rebasing onto 06e0417, all checks pass, including build (3.12).