From 57f08d0f34c8ad95b98a6dd4caded321a1b5953c Mon Sep 17 00:00:00 2001 From: overgoy <32526203+overgoy@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:57:07 +0200 Subject: [PATCH] fix: let the shipped HuggingFaceNerRecognizer entry be enabled (#2222) default_recognizers.yaml ships HuggingFaceNerRecognizer with enabled: false and no model_name. EntityRecognizer.__init__ calls load(), and load() raised when model_name was missing, so flipping the shipped entry to true aborted the construction of the whole registry rather than adding one recognizer. The switch existed but could never be turned on. load() now leaves the recognizer inactive and logs why, so the registry builds. analyze() raises with the same actionable message, so a recognizer without a model is still reported rather than quietly returning no entities, which for a PII library would read as "this text is clean". The guard in analyze() is on model_name rather than on a missing pipeline, so lazy loading through load() keeps working as before. HuggingFaceNerRecognizer therefore leaves NOT_LOADABLE_FROM_SHIPPED_ENTRY, which the contract test kept it in with a comment calling it a pre-existing defect in the entry. The set stays in place, and empty, so the next entry that cannot load as shipped is visible there instead of silently untested. --- .../ner/huggingface_ner_recognizer.py | 28 ++++++++++++++++--- .../tests/test_huggingface_ner_recognizer.py | 8 ++++-- .../tests/test_recognizers_loader_utils.py | 15 ++++------ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/huggingface_ner_recognizer.py b/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/huggingface_ner_recognizer.py index f91fcb6473..7b7685fe92 100644 --- a/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/huggingface_ner_recognizer.py +++ b/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/huggingface_ner_recognizer.py @@ -258,16 +258,25 @@ def load(self) -> None: 1. Hardware acceleration setup (CUDA validation and fallback) 2. Lazy-loading of the heavyweight ML pipeline. - :raises ValueError: If model_name is not set + Without ``model_name`` the pipeline cannot be built, and this returns + without one rather than raising: ``EntityRecognizer.__init__`` calls + ``load()``, so raising here aborted the construction of the whole + registry for anyone who enabled the shipped + ``default_recognizers.yaml`` entry, which carries no ``model_name``. + The recognizer is then registered but inactive, and ``analyze()`` + raises with the same actionable message, so a missing model is still + reported rather than silently returning no entities. """ if self.ner_pipeline is not None: return if not self.model_name: - raise ValueError( - "model_name must be set before calling load(). " - "Pass it to __init__() or set it directly." + logger.warning( + "%s has no model_name and stays inactive. Set model_name to " + "use it, either in __init__() or on the recognizer entry.", + self.name, ) + return # Device validation and fallback device = self.device @@ -430,6 +439,17 @@ def analyze( # Defensive guard for entities input entities = entities or [] + if not self.model_name: + # load() leaves the recognizer inactive in this case, so that the + # shipped default_recognizers.yaml entry can be enabled without + # aborting the construction of the registry. Raising here rather + # than returning [] keeps a misconfigured recognizer from reading + # as "this text contains no PII". + raise ValueError( + "model_name must be set before calling analyze(). " + "Pass it to __init__() or set it directly." + ) + if not self.ner_pipeline: self.load() diff --git a/presidio-analyzer/tests/test_huggingface_ner_recognizer.py b/presidio-analyzer/tests/test_huggingface_ner_recognizer.py index 40368d93cc..e765a6b905 100644 --- a/presidio-analyzer/tests/test_huggingface_ner_recognizer.py +++ b/presidio-analyzer/tests/test_huggingface_ner_recognizer.py @@ -256,10 +256,14 @@ def test_hf_recognizer_load_errors(): with pytest.raises(ImportError): HuggingFaceNerRecognizer(model_name="test") - # 2. Test ValueError when model_name is missing + # 2. Without model_name the recognizer builds but stays inactive, so that + # enabling the shipped default_recognizers.yaml entry does not abort the + # construction of the registry. The missing model is reported on use. with patch(path, new=MagicMock()): + recognizer = HuggingFaceNerRecognizer(model_name=None) + assert recognizer.ner_pipeline is None with pytest.raises(ValueError, match="model_name must be set"): - HuggingFaceNerRecognizer(model_name=None) + recognizer.analyze("Katherine lives in Seoul", entities=["PERSON"]) @pytest.mark.usefixtures("mock_torch_installed") diff --git a/presidio-analyzer/tests/test_recognizers_loader_utils.py b/presidio-analyzer/tests/test_recognizers_loader_utils.py index 987b5c04d3..9926f6fb45 100644 --- a/presidio-analyzer/tests/test_recognizers_loader_utils.py +++ b/presidio-analyzer/tests/test_recognizers_loader_utils.py @@ -5,7 +5,7 @@ import inspect import re from pathlib import Path -from typing import Dict, List +from typing import Dict, List, Set import presidio_analyzer.predefined_recognizers as predefined import pytest @@ -607,15 +607,10 @@ def test_yaml_country_code_blank_value_raises(): LOADER_KWARGS = ("name", "supported_language") # Entries that cannot load from their shipped configuration even with every -# dependency installed, so the load test below cannot cover them. -# -# ``HuggingFaceNerRecognizer``: ``EntityRecognizer.__init__`` calls ``load()`` -# unconditionally and ``load()`` requires ``model_name``, which the shipped -# entry does not supply -- it raises ValueError once ``transformers`` and -# ``torch`` are present. That is a pre-existing defect in the entry, not -# something this contract can assert away, and adding ``model_name`` here would -# make the test download a model. It stays covered by the resolve test. -NOT_LOADABLE_FROM_SHIPPED_ENTRY = {"HuggingFaceNerRecognizer"} +# dependency installed, so the load test below cannot cover them. Empty: an +# entry that cannot be loaded as shipped is a defect in the entry, and keeping +# the set makes the next one visible here instead of silently untested. +NOT_LOADABLE_FROM_SHIPPED_ENTRY: Set[str] = set() # Entries gated behind an optional dependency, for which refusing to load with an # actionable ImportError is the intended behavior. The skip is scoped to these