Skip to content

Compare characters by value in DamerauLevenshteinDistance - #770

Merged
garydgregory merged 1 commit into
apache:masterfrom
Weiki886:fix-damerau-levenshtein-non-ascii
Sep 3, 2026
Merged

Compare characters by value in DamerauLevenshteinDistance#770
garydgregory merged 1 commit into
apache:masterfrom
Weiki886:fix-damerau-levenshtein-non-ascii

Conversation

@Weiki886

@Weiki886 Weiki886 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

calculateCost compared the boxed Character values returned by
SimilarityInput.at with ==, so it only recognized equal characters
that Character.valueOf caches. That cache covers values up to 127,
so every non-ASCII character compared unequal to itself: applying the
distance to "caf\u00e9" and "caf\u00e9" reported 1 instead of 0,
and a two-character CJK input compared against itself reported 2. The
adjacent-transposition branch never fired for non-ASCII input either,
so transposing two accented characters cost 2 instead of 1. The
limited variant was affected the same way and could return -1 for
inputs whose real distance is within the threshold.

Compare with Objects.equals instead, matching every other class in
this package: LevenshteinDistance, LevenshteinDetailedDistance,
JaroWinklerSimilarity and HammingDistance all use equals on the
values from at.


  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Codex (OpenAI GPT-5) was used to discover the bug via automated property-based testing, propose the fix, write the regression tests, and craft this description. Every change was reviewed and verified by a human contributor.
  • Run a successful build using the default Maven goal with mvn; that is mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

calculateCost compared the boxed Character values returned by
SimilarityInput.at with ==, so it only recognized equal characters
that Character.valueOf caches. That cache covers values up to 127,
so every non-ASCII character compared unequal to itself: applying the
distance to "caf\u00e9" and "caf\u00e9" reported 1 instead of 0, and a
two-character CJK input compared against itself reported 2. The
adjacent-transposition branch never fired for non-ASCII input either,
so transposing two accented characters cost 2 instead of 1. The
limited variant was affected the same way and could return -1 for
inputs whose real distance is within the threshold.

Compare with Objects.equals instead, matching every other class in
this package: LevenshteinDistance, LevenshteinDetailedDistance,
JaroWinklerSimilarity and HammingDistance all use equals on the
values from at.
@garydgregory
garydgregory merged commit 1465758 into apache:master Sep 3, 2026
15 of 17 checks passed
@garydgregory

Copy link
Copy Markdown
Member

@Weiki886 LGTM, merged 🚀
Thank you!

@eekyrus

eekyrus commented Sep 4, 2026

Copy link
Copy Markdown

Hi, any ETA when new release can be expected? Without this fix the Damerau-Levenshtein algorithm in unusable.

@garydgregory

garydgregory commented Sep 4, 2026

Copy link
Copy Markdown
Member

@Weiki886
Thank you for your query. I will put this component on my to do list to release but it might be a few weeks since there other bug fixes to deal with in Commons... In the meantime, please point your configuration to our Maven snapshot repository where you'll find 1.15.1-SNAPSHOT builds to test.

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.

3 participants