Skip to content

Normalize geo codes consistently (Copilot review on #26) - #31

Merged
th0rz05 merged 1 commit into
mainfrom
fix/geo-code-normalization
Jun 3, 2026
Merged

Normalize geo codes consistently (Copilot review on #26)#31
th0rz05 merged 1 commit into
mainfrom
fix/geo-code-normalization

Conversation

@th0rz05

@th0rz05 th0rz05 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses both inline comments from Copilot's review of #26 — geo codes were normalized inconsistently across the two places that consume them:

  • line_item_locale (mapping.py) uppercased but did not strip whitespace. A brief geo like "PT " failed the pycountry lookup and silently fell back to US — even though the targeting resolver (which strips inside _country_name) resolved the same value correctly. So targeting and locale could disagree.
  • _resolve_geos (targeting.py) recorded unresolved entries using the raw input string (e.g. "pt ") rather than the normalized code.

Fix

Normalize each geo code once with strip().upper() and use that single form for the typeahead lookup, the locale country, and the unresolved report.

Tests

  • us/Pt (padded + mixed case) still resolve to the US/PT URNs.
  • An unknown padded code ("zz ") surfaces as the normalized "ZZ" in unresolved.
  • line_item_locale(["pt "]){"country": "PT", ...} (no longer falls back to US); unknown code still defaults to US.

47 tests pass; ruff clean.

Note: Copilot could only review #26 — its review of #27#30 hit the account's review quota limit, not a code issue.

Copilot flagged that geo handling normalized inconsistently:
  * line_item_locale uppercased but didn't strip, so a brief geo like "PT "
    failed the pycountry lookup and silently fell back to US, even though the
    targeting resolver (which strips) resolved it fine;
  * _resolve_geos recorded unresolved entries using the raw padded/lowercased
    input rather than the normalized code.

Normalize each geo code once with strip().upper() and use that form for the
lookup, the locale, and the unresolved report. Adds tests for whitespace/case
tolerance in both resolution and locale, and that unresolved codes surface
normalized.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 3, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@th0rz05
th0rz05 merged commit 58ae403 into main Jun 3, 2026
@th0rz05
th0rz05 deleted the fix/geo-code-normalization branch June 3, 2026 15:56
th0rz05 added a commit that referenced this pull request Jun 9, 2026
Copilot flagged that geo handling normalized inconsistently:
  * line_item_locale uppercased but didn't strip, so a brief geo like "PT "
    failed the pycountry lookup and silently fell back to US, even though the
    targeting resolver (which strips) resolved it fine;
  * _resolve_geos recorded unresolved entries using the raw padded/lowercased
    input rather than the normalized code.

Normalize each geo code once with strip().upper() and use that form for the
lookup, the locale, and the unresolved report. Adds tests for whitespace/case
tolerance in both resolution and locale, and that unresolved codes surface
normalized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants