fix(server): project postal capabilities by AdCP version#952
Conversation
There was a problem hiding this comment.
Clean version-aware projection of geo_postal_areas. Right shape: one typed GeoPostalAreas declaration, the framework emits only what the caller negotiated, and it fail-closes to the legacy 3.0 booleans for unversioned callers.
Things I checked
- Wire shape, both endpoints.
ad-tech-protocol-expert: sound. Diffed the projection output against the cached schemas — 3.0 (schemas/cache/3.0/protocol/get-adcp-capabilities-response.json:266) is legacy-booleans-only withadditionalProperties: false, so dropping the alias-less native systems (BRcep, INpin, ZApostal_code) from 3.0 projections isn't just acceptable, it's the only schema-valid behavior — emitting them would fail validation. 3.1 (postal-area-support.json) blesses native+legacy coexistence, so normalizing a 3.1 caller's deprecated booleans into native shape is a valid choice, and the^[A-Z]{2}$passthrough for future country keys matches the schema'spropertyNamesanyOf. - Mapping completeness.
_NATIVE_TO_LEGACY_POSTALcovers all 11LegacyPostalCodeSystemmembers exactly and inverts bijectively.False/absent booleans treated as absent, so projection never invents support. - Version gate.
_is_native_geo_postal_version(projections.py):None→legacy, unparseable→legacy (ValueError caught),3.0→legacy,3.1/3.1.0-rc.13→native.normalize_to_release_precisionstrips the patch component first, sominor_rawis always single-digit — noint("1.0")blowup. Fail-closed throughout. - Handler dump-then-patch (
handler.py:1722-1746): readsgeo_postal_areasoff the typed model, mutates themodel_dumpdict, and the cascade pop (targeting→execution) only fires on genuinely empty dicts — sibling keys liketrusted_match/creative_specssurvive. Correct, and the empty-projection case is covered bytest_native_postal_capabilities_without_legacy_alias_are_omitted_for_30_callers. - Import layering:
projections.pypullsGeoPostalAreas/LegacyPostalCodeSystemfromadcp.types.capabilities(allowlisted), not_generated/generated_poc. Compliant. - Public surface:
project_geo_postal_areasis additive —__init__.py,__all__, andpublic_api_snapshot.jsonall updated consistently. Non-breaking, sofix:carries a valid semver signal. Schema-validation tests assert both endpoints against the bundled validator — the right guard.
Follow-ups (non-blocking — file as issues)
fix(server):vs the changelog### Featuresentry. The commit type and the changelog section disagree — andfeat:is the more honest type, since this is new auto-projection behavior, not a bug fix. Semver is non-breaking either way; just retitle or move the line so release-please notes read straight.- Handler-path forward-compat is narrower than the PR framing.
GeoPostalAreasisextra='ignore', so an unknown country key (NL) survives only on the public-helper raw-Mappingpath — through a real typed model in the handler it's dropped at validation before projection runs. Not a regression (consistent with the SDK's forward-compat posture), but the "preserves future native country keys" story only holds for the helper, not the handler.
Minor nits (non-blocking)
_NATIVE_POSTAL_COUNTRIESis dead code.projections.pydefines the 11-element tuple but nothing references it — country filtering is done entirely by_COUNTRY_KEY_RE. ruff won't catch a module-level unused constant, so it ships silently. Delete it.- Redundant inner
try/exceptin_is_native_geo_postal_version.normalize_to_release_precisionalready guarantees digit-onlymajor.minor, so theint()guard never trips. Harmless defensive code — leave it or drop it.
Approving on the strength of the schema-diff confirmation at both version endpoints plus the bijective mapping check. Follow-ups noted.
There was a problem hiding this comment.
Clean version-aware projection of geo_postal_areas with the right shape — one typed declaration, serialized to whatever the caller negotiated. The native↔legacy mapping is symmetric and schema-complete, and the layering holds.
Things I checked
- Mapping table is complete and inverse-consistent.
_NATIVE_TO_LEGACY_POSTAL(11 entries) matches the 11LegacyPostalCodeSystemmembers exactly;_LEGACY_TO_NATIVE_POSTALis its true inverse, so the native-path_LEGACY_TO_NATIVE_POSTAL[legacy.value]can never KeyError.ad-tech-protocol-expertcross-checked againstschemas/cache/3.0/.../get-adcp-capabilities-response.json:266-305(11 booleans,additionalProperties: false) and the 3.1core/postal-area-support.json. - Exactly-one-shape is correct — for 3.0 it's mandatory. The 3.0 schema forbids native country keys (
additionalProperties: false), so emitting both shapes to a 3.0 caller would be schema-invalid. BRcep/ INpin/ ZApostal_codehave no 3.0 boolean and no native representation in 3.0 at all — silent omission is the only valid option, an AdCP 3.0 expressiveness gap, not a PR defect. Falsebooleans never invent support. Handler passes the model;_geo_postal_payloaddumps withexclude_none=True; theis True/is not Trueguards treatFalseas absent. Projection is additive-only.- Handler cleanup block is non-destructive (
handler.py:1722-1742). Operates on a freshmodel_dumpcopy and only popstargeting/executiononce empty; siblingmedia_buy.*and otherexecution.*keys keep those dicts populated.context is Nonehandled. - No layering breach.
projections.pyimportsGeoPostalAreas/LegacyPostalCodeSystemfrom the allowlistedadcp.types.capabilities, not from_generated/generated_poc— so it doesn't need to be on thetest_import_layering.pyallowlist and won't trip it.capabilities.py(already allowlisted) is the only file adding a direct_generatedimport, which is permitted. Both names are stable, not codegen-numbered. - Public-API audit:
project_geo_postal_areasis a non-breaking addition —__all__, the snapshot fixture (public_api_snapshot.json), andCHANGELOG.mdall updated.fix(server)prefix is correct for an additive behavior fix. - Largest-file rule (
examples/v3_reference_seller/src/platform.py, 203 net): read it. Themodel_constructpath at ~L1487 is the one site worth flagging (follow-up below); the rest —_product_format_options,_normalize_echo_urls, thesandbox/valid_actionsadditions — is example-contract plumbing and safe.
Follow-ups (non-blocking — file as issues)
version is Nonedisagrees with the SDK's own unversioned convention._is_native_geo_postal_version(None)returnsFalse(→ deprecated 3.0 booleans), butsrc/adcp/server/responses.py:444projectsadcp_version is Noneas newest/3.1 (adcp_version is None or _is_adcp_31_or_newer(...)), andbase.py:132-137documents A2A-unversioned traffic as "served as the current SDK shape." Net effect on an A2A-unversioned response: 3.1media_buy_statusfromresponses.pysitting next to fused-booleangeo_postal_areasfrom this helper — internally inconsistent, and it silently drops BR/IN/ZA for native-capable unversioned callers. Not a hard break (the deprecated booleans stay valid under 3.1's schema, and MCP is pinned to3.0upstream of this code), which is why this is a follow-up, not a block. But two projection layers disagreeing on whatNonemeans is a wart.test_unversioned_postal_capabilities_fall_back_to_30_projectioncodifies the current behavior — ifNone→3.0is deliberate, justify it against theresponses.pyprecedent; otherwise align onNone→native.ad-tech-protocol-expert: sound-with-caveats, this is the caveat._NATIVE_POSTAL_COUNTRIESis dead code (projections.py). Defined, never referenced —_iter_native_postal_systemsgates on_COUNTRY_KEY_REinstead. It also misleads: it lists BR/IN/ZA as if special-cased when the regex accepts any 2-letter uppercase key. Delete it.project_geo_postal_areasis a new public export with noAGENTS.mdmention. Minor docs drift.
Minor nits (non-blocking)
- The
model_constructhack in the example reimplements an existing helper, wrong.examples/v3_reference_seller/src/platform.py:~1487buildsUpdateMediaBuySuccessResponseviamodel_construct(... status="completed", media_buy_status=...), bypassing validation and leavingaffected_packagesas raw unvalidated dicts on a field typed as list-of-models. The generated model already has amode="before"validator that maps legacystatus→media_buy_status, and the repo shipsupdate_media_buy_response()(src/adcp/server/responses.py:468) that does this correctly —examples/seller_agent.pyuses it. A reference example modelingmodel_constructover the validated path teaches the wrong pattern. Prefermodel_validateor the existing helper. - Post-validation
product.format_ids = ... # type: ignoremutation (platform.py:~935) works around codegen droppingProductFormatDeclarationdiscriminator fields. Safe (the data was validated as part ofproduct_payload), but tie the comment to the upstream codegen issue number so nobody "cleans it up."
Mapping is right, the wire shapes validate on both 3.0 and 3.1 paths, and the only real caveat ships valid bytes. Approving. The None-handling inconsistency is the one to chase — half this diff is example storyboard fixes riding under a postal-projection title, which is its own small adventure.
There was a problem hiding this comment.
Clean version-aware postal projection. Right shape: one typed GeoPostalAreas declaration, serialized to whatever shape the caller negotiated — sellers stop hand-forking 3.0 vs 3.1 capability responses.
Things I checked
- Mapping table is a complete bijection.
_NATIVE_TO_LEGACY_POSTAL(11 pairs) maps exactly ontoLegacyPostalCodeSystem's 11 members and the named native country properties incore/postal-area-support.json— US/GB/CA carry both sub-systems, DE/CH/AT/FR/AU one each. No missing or extra pair._LEGACY_TO_NATIVE_POSTAL[legacy.value]atprojections.py:652therefore can't KeyError. - Both wire shapes validate.
ad-tech-protocol-expert: sound — native{"US":["zip","zip_plus_four"],"BR":["cep"]}matches the 3.1postal-area-supportref; legacy only ever emits the 11 closed booleans, so it can't violate 3.0'sadditionalProperties: false. Tests assert both viavalidate_responsefor 3.0 and 3.1. - Projection never invents support.
payload.get(legacy.value) is True/is not TruetreatsFalseandNoneas absent. No-alias native systems (BRcep, INpin, ZApostal_code) drop from 3.0 — correct lossy down-projection, not a failure. - Handler mutation is safe.
handler.py:1722-1742projects off the typed model but rewrites the dumped dict, replacinggeo_postal_areaswholesale (no stale native+legacy mix), then pops emptytargeting/executionunderisinstance(..., dict)guards. Empty-projection (3.0 + BR-only) pops cleanly — covered bytest_native_postal_capabilities_without_legacy_alias_are_omitted_for_30_callers. - Version gate.
major > 3 or (major == 3 and minor >= 1),ValueError-guarded, unversioned → legacy.3.1.0-rc.13normalizes to release3.1→ native. Conservative default is right. - Security.
security-reviewer: none.geo_postal_areasis static class-level capability metadata, not request- or tenant-derived;resolved_adcp_versionis a boolean branch selector, never a sink; noctx_metadatacredential-echo path touched. - Public surface.
project_geo_postal_areasis additive — tracked inpublic_api_snapshot.json,__all__, andcapabilities.py'sLegacyPostalCodeSystemre-export. Layering respected:projections.pyimports fromadcp.types.capabilities, not fromgenerated_poc/_generated.
Follow-ups (non-blocking — file as issues)
- Open-union country keys are unreachable through the typed path. The 3.1 schema permits arbitrary ISO countries (NL
postal_code,custom) viaadditionalProperties, but the generatedGeoPostalAreasis closed — 11 named countries + 11 booleans.project_geo_postal_areashandles open-union keys correctly when fed a raw dict (that's whattest_public_postal_projection_preserves_future_native_country_keysexercises), but the handler reads off the typed model, so a seller can't actually declare NL today. Pre-existing codegen limitation, not introduced here. Closing it means a_forward_compat.pypatch onGeoPostalAreasanalogous toFormat.assets. - CHANGELOG hand-edit. The new line lands under
### Featuresof an already-released version section, and afix(server):commit filed under Features. release-please regenerates this from commit history — the manual edit will be clobbered or misfiled. Drop it and let the tooling generate it.
Minor nits (non-blocking)
update_media_buyswitched tomodel_construct.examples/v3_reference_seller/src/platform.py(update_media_buy) now doescast(Any, UpdateMediaBuySuccessResponse).model_construct(... status="completed", media_buy_status=MediaBuyStatus(response_status) ...). Functionally correct — it hand-replicates the model's legacy-status normalization — butmodel_constructbypasses validation, soaffected_packagesstays raw dicts and thecast(Any, ...)defeats field-name type checking. Example code, happy-path smoke test only. An adopter who copies the pattern loses the runtime guard. Worth a comment pointing at the codegen issue it works around.get_productspost-validation reassignment. Same file: afterProduct.model_validate(...),product.format_ids/product.format_optionsget reassigned with# type: ignore[assignment]to restore wire declarations the validator drops. Fine for a demo seller; leave a breadcrumb to the upstream codegen issue so it can be removed later.
Approving on the strength of the complete mapping table plus both projections validating against the bundled 3.0 and 3.1 schemas. Follow-ups noted below.
Summary
project_geo_postal_areashelper for AdCP version-aware postal capability serializationget_adcp_capabilitiespostal declarations to legacy fused booleans for unversioned / 3.0 callers and native country-keyed maps for 3.1+ callersExpert review
Validation
PYTHONPATH=src python3 -m pytest tests/test_decisioning_capabilities_projection.py tests/test_public_api.py tests/test_import_layering.pyPYTHONPATH=src mypy src/ruff check tests/test_decisioning_capabilities_projection.py src/adcp/types/projections.py src/adcp/decisioning/handler.py src/adcp/decisioning/platform.py src/adcp/types/__init__.py src/adcp/types/capabilities.pyblack --check tests/test_decisioning_capabilities_projection.py src/adcp/types/projections.py src/adcp/decisioning/handler.py src/adcp/decisioning/platform.py src/adcp/types/__init__.py src/adcp/types/capabilities.pyCloses #951