fix(analyzer): keep recognizer score thresholds when config omits them - #2210
fix(analyzer): keep recognizer score thresholds when config omits them#2210omri374 wants to merge 3 commits into
Conversation
RecognizerListLoader assigned the normalized score_thresholds value unconditionally after instantiating a recognizer. normalize_score_thresholds maps a missing key to an empty mapping, so a configuration entry that said nothing about thresholds silently overwrote whatever the recognizer class had set for itself. Every other configuration key behaves the opposite way: absent keys are never passed to the constructor, so class defaults stand. Thresholds are now applied only when the key is present. An explicit empty mapping still clears them. No predefined recognizer currently declares its own thresholds, so this is a no-op for the existing suite, but it blocks any recognizer that wants a threshold as part of its own definition. The same unconditional assignment in the custom-recognizer branch and in add_pattern_recognizer_from_dict is fixed for consistency.
Coverage report (presidio-structured)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Coverage report (presidio-cli)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
Fixed various issues with recognizer configurations and behavior, including score thresholds, region codes, and language model parameters.
Coverage report (presidio-anonymizer)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Fixes a configuration-loading bug in presidio-analyzer where a recognizer’s class-defined score_thresholds were unintentionally overwritten with {} when the registry config omitted the score_thresholds key (because normalize_score_thresholds(None) returns {} and the loader assigned it unconditionally).
Changes:
- Update
RecognizerListLoader.get()to only assignrecognizer.score_thresholdswhen the config key is present (predefined + custom branches). - Update
RecognizerRegistry.add_pattern_recognizer_from_dict()to apply the same “only if key present” rule. - Add regression tests covering omission vs explicit override vs explicit empty mapping behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| presidio-analyzer/presidio_analyzer/recognizer_registry/recognizers_loader_utils.py | Preserve class-defined thresholds unless configuration explicitly provides score_thresholds. |
| presidio-analyzer/presidio_analyzer/recognizer_registry/recognizer_registry.py | Apply the same omission semantics for the dict-based recognizer loading path. |
| presidio-analyzer/tests/test_recognizer_registry.py | Add regression tests ensuring omission keeps class defaults while explicit values override/clear. |
| CHANGELOG.md | Adds a changelog entry for this fix (but see PR comment re: repo policy). |
Coverage report (presidio-image-redactor)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
Coverage report (presidio-analyzer)Click to see where and how coverage changed
The report is truncated to 25 files out of 79. To see the full report, please visit the workflow summary page. This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (4)
presidio-analyzer/presidio_analyzer/recognizer_registry/recognizers_loader_utils.py:446
- Same issue as the predefined branch: using key membership treats
score_thresholds: None(often produced by config model dumping) as “present”, which can still clear a recognizer’s class-defined thresholds.
has_score_thresholds = "score_thresholds" in recognizer_conf
presidio-analyzer/presidio_analyzer/recognizer_registry/recognizers_loader_utils.py:410
has_score_thresholds = "score_thresholds" in recognizer_confwill still be true when config validation/model_dump includesscore_thresholds: Nonefor an omitted key, so the class default can still be overwritten to{}. Use a value-based check so omission (ornull) doesn’t count as an override, while{}still clears thresholds.
This issue also appears on line 446 of the same file.
has_score_thresholds = "score_thresholds" in recognizer_conf
presidio-analyzer/presidio_analyzer/recognizer_registry/recognizer_registry.py:347
has_score_thresholds = "score_thresholds" in recognizer_configtreats an explicitNonevalue as an override, which would still clear any class-defined thresholds afternormalize_score_thresholds(None)->{}. To match the “omission keeps defaults” rule, basehas_score_thresholdson the popped value being non-None.
recognizer_config = recognizer_dict.copy()
has_score_thresholds = "score_thresholds" in recognizer_config
score_thresholds = normalize_score_thresholds(
recognizer_config.pop("score_thresholds", None)
)
presidio-analyzer/tests/test_recognizer_registry.py:266
- These regression tests exercise
RecognizerListLoader.get(...)directly, but the primary production path goes throughRecognizerRegistryProvider+ConfigurationValidator, which can reintroducescore_thresholds: Nonefor omitted keys viamodel_dump()and mask this bug. Consider adding a test which constructs aRecognizerRegistryProvider(registry_configuration=...)with the key omitted and asserts the class-defined thresholds are preserved end-to-end.
def _load_predefined(conf):
return RecognizerListLoader.get(
recognizers=[conf],
global_regex_flags=re.DOTALL | re.MULTILINE | re.IGNORECASE,
supported_languages=["en"],
)
Change Description
Recognizer score thresholds set by a recognizer class are discarded when the registry configuration omits
score_thresholds.RecognizerListLoader.get()excludesscore_thresholdsfrom the constructor kwargs and instead assigns it after instantiation:normalize_score_thresholds(None)returns{}, so an entry that says nothing about thresholds overwrites whatever the class set for itself.This is the opposite of how every other configuration key behaves. Keys such as
contextare only passed when present, socontext = context if context else self.CONTEXTkeeps the class default. Omission means "say nothing", not "reset to empty".Fix
Apply the configured value only when the key is present. An explicit empty mapping remains a deliberate reset.
Applied in three places: the predefined branch and the custom branch of
RecognizerListLoader.get(), andRecognizerRegistry.add_pattern_recognizer_from_dict.Impact
No predefined recognizer currently declares its own thresholds, so the assignment has always been
{}over{}and the bug has never been observable. It becomes observable the moment a recognizer class defines a threshold as part of its own definition, which #2159 is the first to do: all six of its new recognizers load withscore_thresholds={}and emit pattern-only matches that their tests assert are suppressed.Tests
Four regression tests using a recognizer that declares its own thresholds:
The first and last fail without the fix.
Issue reference
Follow-up to #2116. Blocks #2159.
Checklist