From 89e1a5a1d44344127aedf8955db560edcead8c86 Mon Sep 17 00:00:00 2001 From: Tiago Barbosa Date: Wed, 3 Jun 2026 16:56:18 +0100 Subject: [PATCH] Normalize geo codes consistently (Copilot review on #26) 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 --- src/yieldagent/integrations/linkedin/mapping.py | 2 +- src/yieldagent/integrations/linkedin/targeting.py | 3 ++- tests/integrations/test_linkedin_mapping.py | 13 ++++++++++++- tests/integrations/test_linkedin_targeting.py | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/yieldagent/integrations/linkedin/mapping.py b/src/yieldagent/integrations/linkedin/mapping.py index 2532c3c..798d53a 100644 --- a/src/yieldagent/integrations/linkedin/mapping.py +++ b/src/yieldagent/integrations/linkedin/mapping.py @@ -73,7 +73,7 @@ def line_item_locale(audience: Audience) -> dict[str, str]: Derived from the first audience geo if it is a valid ISO 3166-1 alpha-2 code; defaults to en/US otherwise. """ - country = audience.geos[0].upper() if audience.geos else "US" + country = audience.geos[0].strip().upper() if audience.geos else "US" if pycountry.countries.get(alpha_2=country) is None: country = "US" return {"country": country, "language": "en"} diff --git a/src/yieldagent/integrations/linkedin/targeting.py b/src/yieldagent/integrations/linkedin/targeting.py index c2b1297..79ad08e 100644 --- a/src/yieldagent/integrations/linkedin/targeting.py +++ b/src/yieldagent/integrations/linkedin/targeting.py @@ -193,7 +193,8 @@ async def _resolve_geos(self, audience: Audience) -> tuple[list[str], list[str]] """ urns: list[str] = [] unresolved: list[str] = [] - for code in audience.geos: + for raw in audience.geos: + code = raw.strip().upper() name = _country_name(code) urn = None if name: diff --git a/tests/integrations/test_linkedin_mapping.py b/tests/integrations/test_linkedin_mapping.py index b5dfaa7..aac2465 100644 --- a/tests/integrations/test_linkedin_mapping.py +++ b/tests/integrations/test_linkedin_mapping.py @@ -4,11 +4,12 @@ from datetime import date -from yieldagent.domain import CreativeAsset, Flight +from yieldagent.domain import Audience, CreativeAsset, Flight from yieldagent.integrations.linkedin.mapping import ( campaign_run_schedule, creative_content_reference, flight_to_run_schedule, + line_item_locale, post_article_content, ) @@ -74,3 +75,13 @@ def test_post_article_content_defaults_source_when_no_landing_url() -> None: def test_creative_content_reference_wraps_post_urn() -> None: assert creative_content_reference("urn:li:share:123") == {"reference": "urn:li:share:123"} + + +def test_line_item_locale_strips_and_uppercases_geo() -> None: + # A padded/lowercase code must still produce a valid locale, not fall back to US. + locale = line_item_locale(Audience(description="x", geos=["pt "])) + assert locale == {"country": "PT", "language": "en"} + + +def test_line_item_locale_defaults_to_us_for_unknown_code() -> None: + assert line_item_locale(Audience(description="x", geos=["ZZ"]))["country"] == "US" diff --git a/tests/integrations/test_linkedin_targeting.py b/tests/integrations/test_linkedin_targeting.py index 04d9c6e..bc41ed7 100644 --- a/tests/integrations/test_linkedin_targeting.py +++ b/tests/integrations/test_linkedin_targeting.py @@ -105,6 +105,21 @@ async def test_unknown_geo_code_is_unresolved_not_guessed() -> None: assert resolved.unresolved == {"geos": ["ZZ"]} +async def test_geo_codes_are_normalized_before_lookup() -> None: + resolver = TargetingResolver(_FakeTargetingClient()) + resolved = await resolver.resolve(Audience(description="x", geos=[" us ", "Pt"])) + # Whitespace/case variations still resolve to the same URNs. + assert _clause_facets(resolved.criteria) == {FACET_LOCATIONS: [_US_GEO, _PT_GEO]} + assert resolved.unresolved == {} + + +async def test_unresolved_geo_is_reported_normalized() -> None: + resolver = TargetingResolver(_FakeTargetingClient()) + resolved = await resolver.resolve(Audience(description="x", geos=["zz "])) + # Unresolved output is the normalized code, not the raw padded input. + assert resolved.unresolved == {"geos": ["ZZ"]} + + async def test_enum_facets_resolve_and_surface_misses() -> None: resolver = TargetingResolver(_FakeTargetingClient()) resolved = await resolver.resolve(