Skip to content

PhoneRecognizer: analysis_explanation names the previous match's region when an international number precedes a national one #2269

Description

@feiiiiii5

What happens

PhoneRecognizer.analyze rebinds the region variable it is iterating over to the region detected for the current match, so that value leaks into every later match in the same pass. The result is an analysis_explanation that names a region which never matched the number.

This reproduces with the default recognizer — no supported_regions, no custom configuration, no registry:

from presidio_analyzer.predefined_recognizers.generic.phone_recognizer import PhoneRecognizer

text = "My international number is +44 1234 567890, and my US one is (415) 555-0132"
for res in PhoneRecognizer().analyze(text, ["PHONE_NUMBER"]):
    print(res.start, res.end, res.score, res.analysis_explanation.textual_explanation)

Measured on main @ f251c513, Python 3.11:

27 42 0.4 Recognized as GB region phone number, using PhoneRecognizer     <- +44 1234 567890, correct
61 75 0.4 Recognized as GB region phone number, using PhoneRecognizer     <- (415) 555-0132, wrong

The US number is explained as a GB number. Swap the two numbers in the text and both labels are correct:

 5 19 … Recognized as US region phone number    (415) 555-0132
25 40 … Recognized as GB region phone number    +44 1234 567890

So the bug is order-dependent, which is also why it is invisible today: every multi-number case in tests/test_phone_recognizer.py::test_when_phone_with_textual_explanation_then_succeed puts the national-format number first and the international one second.

Why

for region in self.supported_regions:                      # outer loop variable
    for match in phonenumbers.PhoneNumberMatcher(text, region, ...):
        try:
            parsed_number = phonenumbers.parse(text[match.start : match.end])
            region = phonenumbers.region_code_for_number(parsed_number)   # rebinds it
            results += [self._get_recognizer_result(match, text, region, nlp_artifacts)]
        except NumberParseException:
            results += [self._get_recognizer_result(match, text, region, nlp_artifacts)]

phonenumbers.parse() is called with no default_region, so a national-format number always raises NumberParseException and takes the except branch — which uses whatever region currently holds. After any international number in the same pass, that is the previous match's region rather than the region being iterated.

What is affected

Detection itself is not: spans, scores and entity types are identical before and after (verified on both orders above). Only analysis_explanation.textual_explanation is wrong. That still matters for anything that surfaces or keys on the explanation — the field exists precisely to say which region matched — and this class has needed an explanation fix before (#1330 fixed a wrong analysis_explanation here).

Expected

Each result reports the region it was actually matched under: the +44 number as GB, the (415) number as US, regardless of the order they appear in the text.

Suggested fix is a per-match matched_region so the loop variable is never overwritten; I have it ready with a regression case added to the existing parametrized test, Patch with regression test: #2270

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions