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