Repository navigation
FIX: Include matcher configuration in decoding scorer identity - #3075
Merged
Roman Lutz (romanlutz) merged 2 commits intoOct 11, 2026
Merged
Roman Lutz (romanlutz) merged 2 commits into
Roman Lutz (romanlutz) merged 2 commits into
Conversation
DecodingScorer._build_identifier recorded only the text matcher's class name, so the built-in matchers' case_sensitive, ignore_whitespace, threshold and n settings were absent from both the component hash and the evaluation hash. Two scorers returning opposite verdicts for the same input therefore shared an evaluation identity, and ScorerEvaluator._should_skip_evaluation could reuse metrics from a different matcher configuration. Pass the matcher's get_identifier_params() through, as microsoft#2979 did for the sibling SubStringScorer. Custom matchers that only implement is_match() remain supported through the same callable guard. Add the sibling's identity regressions, adapted: three parameterised cases over case sensitivity, threshold and n-gram size, each asserting that opposite verdicts no longer share an identity, plus equivalent-configuration stability for both built-in matchers.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Roman Lutz (romanlutz)
approved these changes
Oct 11, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 11, 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.
Description
Closes #3074 .
DecodingScorerinstances with different matcher settings could produce opposite verdicts but share bothhashandeval_hash, allowing cached evaluation metrics to be reused across different behavior. This is the same defect #2977 described forSubStringScorer, in the sibling class that #2979 did not touch: both hold aTextMatchinginstance at the same point of the same method, but onlySubStringScorer._build_identifier()passes itsget_identifier_params()through.The fix copies the two lines #2979 added to the sibling, so the built-in matchers' case sensitivity, whitespace handling, threshold, and n-gram size now reach the identity. Custom matchers that only implement
is_match()remain supported through the samecallable()guard. Built-in matcher hashes change intentionally, as they did forSubStringScorer; no shipped scorer metric registry referencesDecodingScorer, so no publishedeval_hashmoves.Scorer categories reach the emitted
score_categorybut are absent from both scorers' identifiers. #2979 left them out forSubStringScorertoo, so I kept this change symmetric with the sibling rather than widening it; happy to cover categories for both in a follow-up if you want them included.Tests and Documentation
tests/unit/score/test_decoding_scorer.py. After: 14 passed.tests/unit/score/suite reports the same 47 pre-existing failures before and after, with passes going from 3422 to 3427, which is exactly the five added cases. Those failures are confined totest_azure_content_filter.pyandtest_local_refusal_classifier_scorer.pyand are unrelated to this change.ruff checkandruff format --checkat the pinnedv0.16.10,git diff --check,check_async_suffix.pyandcheck_no_rest_roles.pyall pass.ty checkreports the same single pre-existing diagnostic on line 50 before and after, whichsubstring_scorer.pyalso reports. I ranpytestdirectly rather thanmake unit-test, and did not run the full repository suite.