fix: make maxPrefixPenalty actually cap the prefix penalty [patch] - #83
Merged
Merged
Conversation
The prefix had two independent penalty sources. PenalizeNonPatternCharacters applied a capped cost when the first pattern codepoint matched, but the generic per-codepoint unmatchedLetterPenalty in the scoring loop had already charged every prefix codepoint, uncapped, and the two stacked. maxPrefixPenalty bounded only one of them, so the score for pattern "y" kept falling roughly 1 per extra prefix character without limit rather than flattening at -5. Track what the loop charged before the first match and refund it when that match lands, leaving PenalizeNonPatternCharacters as the sole, capped source of prefix cost. A subject that never matches keeps its per-codepoint penalties, so its score still reflects how much was skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FyZyutu7Xna2FUQK6o8KAC
|
This was referenced Sep 22, 2026
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
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.



Fixes #82
The bug
The unmatched prefix was charged twice, and only one of the two charges was capped:
PenalizeNonPatternCharactersappliesMath.Max(n * unmatchedPrefixLetterPenalty, maxPrefixPenalty)when the first pattern codepoint matches — correctly capped at-5.unmatchedLetterPenaltyin the scoring loop had already charged every one of those same prefix codepoints, uncapped.So the cap bounded one source while the other kept running. Matching pattern
"y"against ever-longer junk prefixes:yxyxxxyxxxxxyxxxxxxyxxxxxxxxxxxxyx×100 +yThe score now falls 1 per prefix character and flattens at the documented
-5, instead of falling without bound. (The10at zero prefix is the separator bonus a match at the start of the string earns — a separate effect, not prefix penalty.)For real use — ranking matches in long strings or file paths — a match behind a longer irrelevant prefix is now deprioritized rather than punished indefinitely, which is what the constant's name, its doc comment, and
PenalizeNonPatternCharacters' ownMath.Maxall intended.The fix
This takes the first of the two options the issue offers: make
PenalizeNonPatternCharactersthe sole, capped source of prefix cost.Rather than skipping the generic penalty in the prefix region outright, the loop tracks what it charged before the first match (
prefixPenaltyCharged) and refunds it at the moment that match lands, just before the capped penalty is applied. This keeps one behaviour that a plain skip would have lost: a subject that never matches the pattern at all keeps its per-codepoint penalties, so its score still reflects how much was skipped, asContains'outScoredocuments ("the score reflects how close the match was").PenalizeNonPatternCharactersitself is unchanged — it was always correct in isolation, which is why its own unit tests never caught this. The double-counting was in the caller.The doc comment on
maxPrefixPenaltynow states what the cap is for.Tests
Four new tests in
FuzzyTests.cs, exercising the fullCalculateScorepipeline rather thanPenalizeNonPatternCharactersin isolation:CalculateScore_LongUnmatchedPrefix_PenaltyFlattensAtTheCap— prefixes of 5, 12 and 100 characters all score identically. This is the issue's acceptance criterion.CalculateScore_PrefixPenalty_NeverExceedsTheDocumentedCap— across prefix lengths 1-20, the score never drops belowmaxPrefixPenalty.CalculateScore_LongerPrefix_NeverScoresHigherThanAShorterOne— the penalty is monotonic, so flattening never becomes a reward.CalculateScore_NonMatchingSubject_StillReflectsHowMuchWasSkipped— pins the behaviour the refund deliberately preserves.Verified by reverting only the
Fuzzy.cschange and re-running: the two cap tests fail, 49 pass. With the fix: 51 passed, 0 failed — including all 49 pre-existing tests, untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01FyZyutu7Xna2FUQK6o8KAC
Generated by Claude Code