Skip to content

fix(analyzer): skip SpacyRecognizer for SlimSpacyNlpEngine#2168

Open
ultramancode wants to merge 4 commits into
data-privacy-stack:mainfrom
ultramancode:fix/slim-supported-entities-contract
Open

fix(analyzer): skip SpacyRecognizer for SlimSpacyNlpEngine#2168
ultramancode wants to merge 4 commits into
data-privacy-stack:mainfrom
ultramancode:fix/slim-supported-entities-contract

Conversation

@ultramancode

Copy link
Copy Markdown
Contributor

Change Description

SlimSpacyNlpEngine disables NER, but the recognizer registry treated it like a regular spaCy engine and added SpacyRecognizer.

When SpacyRecognizer received an empty supported-entities list from the slim engine, it fell back to its default entities. As a result, the registry reported NER entities that the slim engine could not produce.

When SlimSpacyNlpEngine is used, the registry no longer adds SpacyRecognizer automatically.
If SpacyRecognizer is explicitly configured, analyzer initialization fails because the slim engine does not produce the NER results it requires.

This does not affect tokenization, lemmatization, or context-based score enhancement provided by the slim engine.

Checklist

  • I have reviewed the contribution guidelines
  • I agree to follow this project's Code of Conduct
  • I confirm that I have the right to submit this contribution and that it does not knowingly contain proprietary or confidential code.
  • My code includes unit tests
  • All unit tests and lint checks pass locally
  • My PR contains documentation updates / additions if required

Comment thread presidio-analyzer/presidio_analyzer/recognizer_registry/recognizer_registry.py Outdated
Comment thread presidio-analyzer/presidio_analyzer/recognizer_registry/recognizer_registry.py Outdated
@ultramancode

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I've updated the PR based on your suggestions. The capability-based approach made the implementation cleaner and easier to maintain.

@property
def has_ner(self) -> bool:
"""Returns True if this engine performs Named Entity Recognition natively."""
return False

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. If someone has a custom nlp engine, it might stop working if this property is set to False. Better to keep it always True and False for the no-op and slim engines

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! has_ner now defaults to True, while SlimSpacyNlpEngine and NoOpNlpEngine explicitly return False. Tests and documentation have been updated accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants