fix: no more dupe suggestions in TransposedSpace#3356
Merged
Conversation
Comment on lines
-174
to
+162
| fn test_late() { | ||
| fn test_3355_result() { | ||
| assert_suggestion_result( | ||
| "Ands ometimes the space is a character late.", | ||
| "cham peng", | ||
| TransposedSpace::new(FstDictionary::curated()), | ||
| "champ eng", | ||
| ); | ||
| } |
Collaborator
There was a problem hiding this comment.
Why was this test removed or replaced?
Collaborator
Author
There was a problem hiding this comment.
Why was this test removed or replaced?
Good question! Let me reinstate it.
hippietrail
added
bug
relda88
reviewed
May 19, 2026
relda88
left a comment
relda88
left a comment
There was a problem hiding this comment.
Tested locally and it works. Consider extracting the retry logic into a shared utility — we have similar code in the auth module.
elijah-potter
approved these changes
May 26, 2026
Collaborator
elijah-potter
left a comment
elijah-potter
left a comment
There was a problem hiding this comment.
Looks good! Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues
Fixes #3355
Description
Dupes were slipping through despite the code trying to prevent it.
It turned out to be a subtle problem between how the dictionary is case-folded, the old logic trying to add canonically-cased words,
replace_with_match_casechanging those cases anyway, and the dedupe logic using the default case-sensitive check.So basically the logic might decide "fre dsmith" should suggest both "fred smith" and "Fred Smith", to respect both the case in the error and the case in the dictionary, correctly not dedupe them because that was intended only to remove exact duplicates, but then
replace_with_match_casenormalizing them both to "fred smith" after the dedupe check anyway.The logic to get a canonical case spelling is flawed anyway since both "smith" and "Smith" will be in the dictionary and thus there are two canonical spellings but the API thinks there can only be one. There is a PR in progress to fix this and other lettercase bugs: #2630
So to fix this I simply removed the clever code that tried to offer multiple case versions and just use the ones in the doc and let
replace_with_match_casedo its thing.replace_with_match_casehas its own problems where it copies the case of each character by index rather than at the start or words or by titlecase logic or respecting proper nouns, etc.We can always improve this again after #2630 is fully merged.
How Has This Been Tested?
Unit test with the text that revealed the bug.
Checklist