fix(analyzer): remove a non-language code and a duplicate entry from default_recognizers.yaml - #2236
Conversation
…default_recognizers.yaml The five Korean entries listed `kr` next to `ko`. `ko` is the ISO 639-1 language code; `kr` is the ISO 3166-1 country code, which the entries already carry in `country_code`. The loader builds one recognizer per listed language, so `kr` either logged a "language is not supported" warning on every load, or - for a registry configured with it - registered a second copy of all five Korean recognizers that no NLP engine can serve. UkPostcodeRecognizer was listed twice: data-privacy-stack#1858 added it, and data-privacy-stack#1857 added it again and merged later. The registry built two identical instances. Generated-by: Claude Opus 5
… non-language codes Both tests fail on main and pass with the config fix. The language guard keeps an explicit set of ISO 639-1 codes so that adding a language is a deliberate edit rather than a side effect of a new entry. Generated-by: Claude Opus 5
data-privacy-stack#2170 parametrised this over ["ko", "kr"] to match the four sibling Kr* entries. `kr` is the ISO 3166-1 country code the entry already carries in `country_code`, and no NLP engine can serve it, so the `kr` case asserted the defect rather than the contract. Generated-by: Claude Opus 5
|
A correction to my own description — I did the archaeology after opening this, and the
Where I think the argument still holds is that the alias was then copied onto recognizers that never had a
For the bottom three, So, how would you like this shaped?
I am happy with any of them; 2 is the conservative one and I will push it if you prefer. The Sorry for the incomplete first framing. #1742 made a deliberate choice and my description read as though nobody had considered it. |
Change Description
Two data problems in
presidio-analyzer/presidio_analyzer/conf/default_recognizers.yaml, with a regression test for each.1.
kris not a language codeThe five Korean entries listed
kralongsidekoundersupported_languages.kois the ISO 639-1 language code;kris the ISO 3166-1 country code, which those same entries already carry incountry_code. Korea is the only country in the file where the country code leaked into the language list — Canada, the closest analogue, correctly pairscountry_code: cawithsupported_languages: [en, fr].RecognizerListLoader._get_recognizer_languagesbuilds one recognizer per listed language, so whatkrcosts depends on how the registry is configured:supported_languages["ko"]["en", "ko"]The second copy can never match anything: there is no spaCy
krmodel, so no NLP engine can serve that language while its regexes still run.ConfigurationValidator.validate_language_codesdoes not catch this —kris a well-formed two-letter code, just not the right one.This also touches one existing test. #2170 parametrised
test_loads_from_default_recognizers_yamlover["ko", "kr"], following the convention already in the file (krgoes back to #1675). That made thekrcase assert the defect rather than the contract, so this PR drops it and keepsko.2.
UkPostcodeRecognizerwas listed twice#1858 added it; #1857 added it again and merged later, so the file carried two byte-identical blocks and the registry built two identical instances.
To be precise about severity: analyzer output is unaffected, because identical spans are merged downstream. The cost is a duplicate regex pass and a config file that misstates itself.
Issue reference
No existing issue — I searched and found none for either finding. Happy to open one first if you would rather have that order.
Verification
mainand pass with the config fix:duplicate recognizer entries: ['UkPostcodeRecognizer']non-language codes in supported_languages: {'KrBrnRecognizer': ['kr'], 'KrRrnRecognizer': ['kr'], ...}— all fivepresidio-analyzersuite, before and after: the set of failing and erroring tests is byte-identical, 36 items, all pre-existing and caused by optional dependencies missing in my environment (langextract, nlp_engine_provider, azure, slim_spacy).diffof the twoFAILED/ERRORlists is empty.registry.recognizersfor each language configuration.ruff checkandruff format --checkon both touched test files report exactly what they report onmain(one pre-existing finding intest_recognizer_registry.py, seven intest_kr_passport_recognizer.py). My lines add none, and I left the pre-existing ones alone rather than widen the diff.A note on the language guard
test_default_recognizers_yaml_declares_languages_not_countrieskeeps an explicit set of ISO 639-1 codes, so that adding a language becomes a deliberate edit rather than a side effect of a new entry. If you would rather not carry that list, I am glad to drop the test and keep the data fix, or reshape it however you prefer.AI use
Claude Opus 5 assisted with the investigation, the edits, and running the checks above; each commit carries a
Generated-by:trailer. Every claim here was verified against a local checkout before it was written.Checklist