From 56e4e4cfb391e53b3a486c0153a2d9353fe4d54a Mon Sep 17 00:00:00 2001 From: Mohit Kalra Date: Tue, 21 Jul 2026 14:19:19 -0700 Subject: [PATCH] fix(analyzer): fix GLiNER multi-instance entity leakage and simplify config (#1760) Consolidates and builds on two other open PRs against this issue, crediting both explicitly: - Auto-derives a unique recognizer name from class_name + model_name when name is omitted on a class_name: GLiNERRecognizer YAML entry, so multiple instances no longer each require an explicit unique name. Approach credited to #2018 (ynachiket). - Adds an include_requested_entities_as_labels option (default true) to GLiNERRecognizer. When multiple instances are configured with different entity_mapping/threshold values, GLiNER's ad-hoc-label support previously let a requested entity type "leak" into an instance that wasn't configured for it, evaluated against the wrong threshold. Setting this to false per-instance closes the leak by restricting that instance to only ever request labels it's configured for. Design credited to #2154 (ultramancode), reproducing hexsm's original report. - Updates the GLiNER sample docs and recognizer_registry_provider.md to document both, replacing the earlier docs-only PR's caveat about the leakage bug with the actual fix. Closes #1760 Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 4 + docs/analyzer/recognizer_registry_provider.md | 3 +- docs/samples/python/gliner.md | 30 +++++ .../yaml_recognizer_models.py | 42 +++++++ .../ner/gliner_recognizer.py | 18 +++ .../tests/test_gliner_recognizer.py | 107 ++++++++++++++++++ .../test_recognizer_registry_provider.py | 62 ++++++++++ .../tests/test_yaml_recognizer_models.py | 53 +++++++++ 8 files changed, 318 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd3c11d975..705ab3c701 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ All notable changes to this project will be documented in this file. - `PhoneRecognizer.DEFAULT_SUPPORTED_REGIONS` used `"UK"`, which is not a valid `phonenumbers` (libphonenumber) region code — region codes are ISO 3166-1 alpha-2, where the United Kingdom is `"GB"`. The `"UK"` entry was a no-op, so UK numbers in national/local format (e.g. `020 7946 0958`) were never detected by default; only international-format `+44 …` numbers matched, because they carry the country code and match under any region. Replaced `"UK"` with `"GB"`. - Language model recognizers (`BasicLangExtractRecognizer`, `AzureOpenAILangExtractRecognizer`) configured in a recognizer registry YAML now honour `config_path` (and other recognizer-specific kwargs). Previously these entries were validated by the strict `PredefinedRecognizerConfig` schema, which has no `config_path` field and does not allow extra keys, so `config_path` was silently dropped and the recognizer fell back to its bundled default model configuration. Added a `LangExtractRecognizerConfig` model (`extra="allow"`) and registered both recognizer class names in `CONFIG_MODEL_MAP`. - `BasicLangExtractRecognizer` now honours values under `langextract.model.provider.language_model_params` (including `timeout` and `num_ctx`). Previously these were silently dropped because `langextract.extract()` ignores its `language_model_params` argument when a pre-built `ModelConfig` is passed via `config=`, causing Ollama-backed recognizers to fall back to langextract's 120s default regardless of the configured timeout. The recognizer now merges `language_model_params` into `ModelConfig.provider_kwargs`, which is the path that reaches the provider constructor. Explicit entries under `provider.kwargs:` still take precedence. Also fixed a `TypeError` when `kwargs:` or `language_model_params:` is `null` in the YAML. (#1943, Thanks @lsternlicht) +- `GLiNERRecognizer` instances configured side by side via YAML with different `entity_mapping`/`threshold` values could return entity types outside their own `entity_mapping`, evaluated against the wrong instance's threshold — because requested entity types were always appended as ad-hoc GLiNER labels regardless of which instance they belonged to. Added an `include_requested_entities_as_labels` constructor/YAML option (default `true`, preserving existing behavior); set to `false` per-instance to restrict that instance to only ever return entity types it's explicitly configured for. (#1760) + +#### Changed +- Configuring multiple `GLiNERRecognizer` instances via YAML with `class_name: GLiNERRecognizer` no longer requires an explicit unique `name` per entry — when omitted, a name is derived automatically from `class_name` and `model_name`. (#1760) ### Anonymizer ### General diff --git a/docs/analyzer/recognizer_registry_provider.md b/docs/analyzer/recognizer_registry_provider.md index 1e260b46ef..e85f7c0adf 100644 --- a/docs/analyzer/recognizer_registry_provider.md +++ b/docs/analyzer/recognizer_registry_provider.md @@ -104,7 +104,8 @@ The recognizer list comprises of both the predefined and custom recognizers, for - `supported_languages`: A list of supported languages that the analyzer will support. In case this field is missing, a recognizer will be created for each supported language provided to the `AnalyzerEngine`. In addition to the language code, this field also contains a list of context words, which increases confidence in the detection in case it is found in the surroundings of a detected entity (as seen in the credit card example above). - `type`: this could be either predefined or custom. As this is optional, if not stated otherwise, the default type is custom. - - `name`: Different per the type of the recognizer. For predefined recognizers, this is the class name as defined in presidio, while for custom recognizers, it will be set as the name of the recognizer. + - `name`: Different per the type of the recognizer. For predefined recognizers, this is the class name as defined in presidio (unless `class_name` is also provided, see below), while for custom recognizers, it will be set as the name of the recognizer. + - `class_name`: Optional. Only relevant for predefined recognizers. Explicitly sets the Python class to instantiate, while `name` becomes the recognizer's instance name (e.g. as it appears in `analysis_explanation.recognizer` on results). This is what allows configuring **multiple instances of the same recognizer class** in the same YAML file, each with its own parameters (e.g. two `GLiNERRecognizer` instances using different models/thresholds, see the [GLiNER sample](../samples/python/gliner.md#configuring-multiple-gliner-recognizers-via-yaml)). For `GLiNERRecognizer` specifically, `name` can also be omitted entirely — a unique instance name is derived automatically from `class_name` and `model_name`. - `patterns`: a list of objects of type `Pattern` that contains a name, score and regex that define matching patterns. - `enabled`: enables or disables the recognizer. - `supported_entity`: the detected entity associated by the recognizer. diff --git a/docs/samples/python/gliner.md b/docs/samples/python/gliner.md index 13dfe11bc7..d8ba14d7df 100644 --- a/docs/samples/python/gliner.md +++ b/docs/samples/python/gliner.md @@ -102,3 +102,33 @@ gliner_recognizer = GLiNERRecognizer( **Note:** Make sure `onnxruntime` is installed when using this feature. It's included in the `gliner` extra dependencies. +## Configuring multiple GLiNER recognizers via YAML + +`GLiNERRecognizer` can also be configured through the [recognizer registry YAML configuration](../../analyzer/recognizer_registry_provider.md). You can define **multiple** `GLiNERRecognizer` instances in the same YAML file (e.g. to use different models, entity mappings or thresholds side by side) by pointing `class_name` at `GLiNERRecognizer` for each entry: + +```yaml +recognizers: + - class_name: GLiNERRecognizer + type: predefined + supported_language: en + model_name: "urchade/gliner_multi_pii-v1" + threshold: 0.4 + entity_mapping: + person: PERSON + organization: ORGANIZATION + include_requested_entities_as_labels: false + + - class_name: GLiNERRecognizer + type: predefined + supported_language: en + model_name: "gliner-community/gliner_small-v2.5" + threshold: 0.25 + entity_mapping: + location: LOCATION + include_requested_entities_as_labels: false +``` + +- `class_name` tells the loader which Python class to instantiate. You no longer need to supply an explicit `name` for each entry: when omitted, a unique instance name is derived automatically from `class_name` and `model_name` (e.g. `GLiNERRecognizer_urchade_gliner_multi_pii_v1`) so multiple instances don't collide. Set `name` explicitly if you want a specific value (e.g. as it appears in `analysis_explanation.recognizer` on results). +- Any additional `GLiNERRecognizer` constructor argument (`model_name`, `threshold`, `entity_mapping`, `flat_ner`, `multi_label`, `map_location`, `load_onnx_model`, `onnx_model_file`, ...) can be set per entry. +- **`include_requested_entities_as_labels`** (default `true`): GLiNER supports ad-hoc labels, so by default each instance also considers entity types requested on the `analyze()` call even if they aren't in its own `entity_mapping`. When running multiple instances with *different* `entity_mapping`/`threshold` values side by side, set this to `false` on each instance so a requested entity type can't "leak" into an instance that wasn't configured for it and get evaluated against the wrong threshold. The example above sets it explicitly for this reason — omit it (or set `true`) for a single general-purpose instance where that behavior is desired. + diff --git a/presidio-analyzer/presidio_analyzer/input_validation/yaml_recognizer_models.py b/presidio-analyzer/presidio_analyzer/input_validation/yaml_recognizer_models.py index 05f8b6cef0..79d7222d7c 100644 --- a/presidio-analyzer/presidio_analyzer/input_validation/yaml_recognizer_models.py +++ b/presidio-analyzer/presidio_analyzer/input_validation/yaml_recognizer_models.py @@ -1,5 +1,6 @@ """Pydantic models for YAML recognizer configurations.""" +import re from typing import Any, Dict, List, Optional, Type, Union from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator @@ -195,6 +196,17 @@ class GLiNERRecognizerConfig(PredefinedRecognizerConfig): model_config = ConfigDict(extra="allow") + name: Optional[str] = Field( + default=None, + description=( + "Instance name for the recognizer. Optional for GLiNERRecognizer " + "entries that specify class_name: if omitted, a name is derived " + "automatically from class_name and model_name, so multiple " + "GLiNERRecognizer instances (e.g. different models/thresholds) " + "can be configured side by side without each requiring an " + "explicit unique name." + ), + ) model_name: Optional[str] = Field(None, description="GLiNER model name") flat_ner: Optional[bool] = Field(None, description="Use flat NER") multi_label: Optional[bool] = Field( @@ -205,6 +217,16 @@ class GLiNERRecognizerConfig(PredefinedRecognizerConfig): load_onnx_model: Optional[bool] = Field(None, description="Load ONNX model") onnx_model_file: Optional[str] = Field(None, description="ONNX model file name") entity_mapping: Optional[Dict[str, str]] = Field(None, description="Entity mapping") + include_requested_entities_as_labels: Optional[bool] = Field( + None, + description=( + "Whether to append analyze()-requested entity types as ad-hoc " + "GLiNER labels. Set to false to restrict this instance to only " + "return entity types declared in its own entity_mapping when " + "configuring multiple GLiNERRecognizer instances with different " + "entity_mapping/threshold values." + ), + ) @model_validator(mode="after") def validate_entity_mapping_and_supported_entities(self): @@ -217,6 +239,26 @@ def validate_entity_mapping_and_supported_entities(self): ) return self + @model_validator(mode="after") + def default_name_from_class_and_model(self): + """Derive a unique instance name when 'name' is omitted. + + Without an explicit name, multiple GLiNERRecognizer entries sharing + class_name: GLiNERRecognizer would otherwise all default to the same + instance name. Deriving from model_name keeps entries distinct in the + common case (different models per instance), which is the actual + motivating multi-instance use case; entries that also share the same + model_name still need an explicit 'name' to disambiguate. + """ + if not self.name: + base = self.class_name or "GLiNERRecognizer" + if self.model_name: + suffix = re.sub(r"[^A-Za-z0-9]+", "_", self.model_name).strip("_") + self.name = f"{base}_{suffix}" + else: + self.name = base + return self + def model_dump(self, *args, **kwargs) -> Dict[str, Any]: """Serialize the config without None values by default. diff --git a/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/gliner_recognizer.py b/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/gliner_recognizer.py index 4e12f7e634..e2d6f9b90f 100644 --- a/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/gliner_recognizer.py +++ b/presidio-analyzer/presidio_analyzer/predefined_recognizers/ner/gliner_recognizer.py @@ -42,6 +42,7 @@ def __init__( text_chunker: Optional[BaseTextChunker] = None, load_onnx_model: bool = False, onnx_model_file: str = "model.onnx", + include_requested_entities_as_labels: bool = True, **model_kwargs, ): """GLiNER model based entity recognizer. @@ -76,6 +77,20 @@ def __init__( :param model_kwargs: Additional keyword arguments to pass to GLiNER.from_pretrained(). This allows passing future parameters to the GLiNER model without explicit support in this recognizer. + :param include_requested_entities_as_labels: GLiNER supports ad-hoc + labels, so by default this recognizer appends any entity types + requested on the `analyze()` call (via `AnalyzerEngine.analyze`) + to the labels sent to the model, even if they aren't in this + recognizer's own `entity_mapping`/`supported_entities`. This is + useful for a single, general-purpose GLiNER instance, but when + multiple `GLiNERRecognizer` instances are configured side by side + with different `entity_mapping`s and thresholds, a globally + requested entity type can "leak" into an instance that wasn't + configured for it and get evaluated against that instance's + threshold instead of the intended one. Set this to `False` to + restrict this instance to only ever return entity types declared + in its own `entity_mapping`/`supported_entities`. Defaults to + `True` to preserve existing behavior. """ @@ -116,6 +131,7 @@ def __init__( self.threshold = threshold self.load_onnx_model = load_onnx_model self.onnx_model_file = onnx_model_file + self.include_requested_entities_as_labels = include_requested_entities_as_labels self.model_kwargs = model_kwargs # Use provided chunker or default to in-house character-based chunker @@ -219,6 +235,8 @@ def predict_func(text: str) -> List[RecognizerResult]: def __create_input_labels(self, entities): """Append the entities requested by the user to the list of labels if it's not there.""" # noqa: E501 labels = list(self.gliner_labels) + if not self.include_requested_entities_as_labels: + return labels for entity in entities: if ( entity not in self.model_to_presidio_entity_mapping.values() diff --git a/presidio-analyzer/tests/test_gliner_recognizer.py b/presidio-analyzer/tests/test_gliner_recognizer.py index 528634441d..7dc72f75af 100644 --- a/presidio-analyzer/tests/test_gliner_recognizer.py +++ b/presidio-analyzer/tests/test_gliner_recognizer.py @@ -323,3 +323,110 @@ def test_when_model_kwargs_then_passes_to_from_pretrained(): assert call_kwargs["custom_param2"] == 42 +# --------------------------------------------------------------------------- +# Tests for include_requested_entities_as_labels (issue #1760 comment thread: +# entity/threshold leakage across GLiNERRecognizer instances with different +# entity_mapping values) +# --------------------------------------------------------------------------- + + +def _labels_scoped_predict_entities(text, labels, **kwargs): + """Simulate GLiNER faithfully: only ever return a label it was asked for. + + The real bug reproduction depends on this — a mock that returns a fixed + value regardless of the ``labels`` argument wouldn't actually exercise + the fix, since the fix works by controlling which labels are sent to + GLiNER in the first place, not by filtering results after the fact. + """ + if "ORGANIZATION" in labels: + return [{"label": "ORGANIZATION", "start": 0, "end": 5, "score": 0.7}] + return [] + + +def test_default_true_reproduces_cross_instance_leakage(mock_gliner): + """Regression test for the bug reported in issue #1760: with the + default (include_requested_entities_as_labels=True), a recognizer + configured only for ADDRESS can still return an ORGANIZATION result + when ORGANIZATION is requested globally, because it gets appended as + an ad-hoc GLiNER label. + """ + if sys.version_info < (3, 10): + pytest.skip("gliner requires Python >= 3.10") + + mock_gliner.predict_entities.side_effect = _labels_scoped_predict_entities + + address_recognizer = GLiNERRecognizer( + entity_mapping={"Address": "ADDRESS"}, + threshold=0.65, + ) + address_recognizer.gliner = mock_gliner + + results = address_recognizer.analyze( + "Apple delivers the iPhone to London.", + entities=["ORGANIZATION", "PRODUCT", "ADDRESS"], + ) + + assert len(results) == 1 + assert results[0].entity_type == "ORGANIZATION" + + +def test_include_requested_entities_as_labels_false_prevents_leakage(mock_gliner): + """With the opt-out flag set, this instance never asks GLiNER to + consider entity types outside its own entity_mapping, so the leak + from the test above cannot occur. + """ + if sys.version_info < (3, 10): + pytest.skip("gliner requires Python >= 3.10") + + mock_gliner.predict_entities.side_effect = _labels_scoped_predict_entities + + address_recognizer = GLiNERRecognizer( + entity_mapping={"Address": "ADDRESS"}, + threshold=0.65, + include_requested_entities_as_labels=False, + ) + address_recognizer.gliner = mock_gliner + + results = address_recognizer.analyze( + "Apple delivers the iPhone to London.", + entities=["ORGANIZATION", "PRODUCT", "ADDRESS"], + ) + + assert results == [] + + +def test_include_requested_entities_as_labels_false_still_returns_own_entities( + mock_gliner, +): + """The opt-out flag must not suppress this instance's own entity type.""" + if sys.version_info < (3, 10): + pytest.skip("gliner requires Python >= 3.10") + + def predict_entities(text, labels, **kwargs): + if "Address" in labels: + return [{"label": "Address", "start": 0, "end": 6, "score": 0.9}] + return [] + + mock_gliner.predict_entities.side_effect = predict_entities + + address_recognizer = GLiNERRecognizer( + entity_mapping={"Address": "ADDRESS"}, + threshold=0.65, + include_requested_entities_as_labels=False, + ) + address_recognizer.gliner = mock_gliner + + results = address_recognizer.analyze( + "London delivers packages daily.", + entities=["ORGANIZATION", "PRODUCT", "ADDRESS"], + ) + + assert len(results) == 1 + assert results[0].entity_type == "ADDRESS" + + +def test_include_requested_entities_as_labels_defaults_to_true(): + recognizer = GLiNERRecognizer(entity_mapping={"Address": "ADDRESS"}) + assert recognizer.include_requested_entities_as_labels is True + + diff --git a/presidio-analyzer/tests/test_recognizer_registry_provider.py b/presidio-analyzer/tests/test_recognizer_registry_provider.py index bc294c7f43..a59e81e642 100644 --- a/presidio-analyzer/tests/test_recognizer_registry_provider.py +++ b/presidio-analyzer/tests/test_recognizer_registry_provider.py @@ -319,3 +319,65 @@ def test_direct_validation_with_missing_global_regex_flags(): # Verify default value and successful creation assert validated["global_regex_flags"] == 26 assert validated["supported_languages"] == ["en"] + + +def test_multiple_gliner_instances_without_explicit_name_get_distinct_ids(monkeypatch): + """End-to-end regression test for issue #1760. + + Two ``class_name: GLiNERRecognizer`` entries with different + ``model_name``/``entity_mapping``/``threshold`` values and no explicit + ``name`` should each load as a distinct recognizer instance (auto-derived + names), with ``include_requested_entities_as_labels: false`` set so + neither instance can return entity types outside its own mapping. + + The underlying ``GLiNER`` model class is mocked so this test doesn't + require the (heavy, optional) ``gliner`` extra to be installed. + """ + from unittest.mock import MagicMock + + from presidio_analyzer.predefined_recognizers.ner import ( + gliner_recognizer as gliner_recognizer_module, + ) + + monkeypatch.setattr(gliner_recognizer_module, "GLiNER", MagicMock()) + + provider = RecognizerRegistryProvider( + registry_configuration={ + "supported_languages": ["en"], + "recognizers": [ + { + "class_name": "GLiNERRecognizer", + "type": "predefined", + "supported_language": "en", + "model_name": "urchade/gliner_multi_pii-v1", + "threshold": 0.4, + "entity_mapping": {"person": "PERSON"}, + "include_requested_entities_as_labels": False, + }, + { + "class_name": "GLiNERRecognizer", + "type": "predefined", + "supported_language": "en", + "model_name": "gliner-community/gliner_small-v2.5", + "threshold": 0.25, + "entity_mapping": {"location": "LOCATION"}, + "include_requested_entities_as_labels": False, + }, + ], + } + ) + + recognizers = provider.create_recognizer_registry().recognizers + gliner_recognizers = { + r.name: r for r in recognizers if r.name.startswith("GLiNERRecognizer") + } + + assert len(gliner_recognizers) == 2 + assert gliner_recognizers["GLiNERRecognizer_urchade_gliner_multi_pii_v1"].threshold == 0.4 + assert ( + gliner_recognizers["GLiNERRecognizer_gliner_community_gliner_small_v2_5"].threshold + == 0.25 + ) + assert not any( + r.include_requested_entities_as_labels for r in gliner_recognizers.values() + ) diff --git a/presidio-analyzer/tests/test_yaml_recognizer_models.py b/presidio-analyzer/tests/test_yaml_recognizer_models.py index 3e092d676e..7489643a4b 100644 --- a/presidio-analyzer/tests/test_yaml_recognizer_models.py +++ b/presidio-analyzer/tests/test_yaml_recognizer_models.py @@ -5,6 +5,7 @@ from presidio_analyzer.input_validation.yaml_recognizer_models import ( BaseRecognizerConfig, CustomRecognizerConfig, + GLiNERRecognizerConfig, LangExtractRecognizerConfig, LanguageContextConfig, PredefinedRecognizerConfig, @@ -308,6 +309,58 @@ def test_configuration_validator_uses_recognizer_specific_dump_rules(): assert predefined_recognizer["supported_language"] is None +def test_gliner_config_name_required_before_this_change_now_optional(): + """class_name + model_name alone is enough; name is auto-derived.""" + config = GLiNERRecognizerConfig( + class_name="GLiNERRecognizer", + model_name="urchade/gliner_multi_pii-v1", + type="predefined", + ) + assert config.name == "GLiNERRecognizer_urchade_gliner_multi_pii_v1" + + +def test_gliner_config_two_instances_omitting_name_get_distinct_names(): + """The actual multi-instance motivation: different models -> different names.""" + first = GLiNERRecognizerConfig( + class_name="GLiNERRecognizer", + model_name="urchade/gliner_multi_pii-v1", + type="predefined", + ) + second = GLiNERRecognizerConfig( + class_name="GLiNERRecognizer", + model_name="gliner-community/gliner_small-v2.5", + type="predefined", + ) + assert first.name != second.name + + +def test_gliner_config_explicit_name_not_overridden(): + config = GLiNERRecognizerConfig( + name="MyCustomGliner", + class_name="GLiNERRecognizer", + model_name="urchade/gliner_multi_pii-v1", + type="predefined", + ) + assert config.name == "MyCustomGliner" + + +def test_gliner_config_no_name_no_model_name_falls_back_to_class_name(): + config = GLiNERRecognizerConfig(class_name="GLiNERRecognizer", type="predefined") + assert config.name == "GLiNERRecognizer" + + +def test_gliner_config_include_requested_entities_as_labels_preserved_on_dump(): + config = GLiNERRecognizerConfig( + class_name="GLiNERRecognizer", + model_name="urchade/gliner_multi_pii-v1", + entity_mapping={"Address": "ADDRESS"}, + include_requested_entities_as_labels=False, + type="predefined", + ) + dumped = config.model_dump() + assert dumped["include_requested_entities_as_labels"] is False + + def test_langextract_config_preserves_config_path(): """A BasicLangExtractRecognizer YAML entry must keep ``config_path``.