feat(protocol): support AdCP 3.1.10 schemas - #1012
Conversation
|
The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final This is an automated message from the Argus AI review workflow. |
There was a problem hiding this comment.
Clean protocol bump. Schemas, regenerated models, codegen repairs, docs, and tests all move together — the version pin and the wire shape stay in lockstep, which is the whole point of the generated-type layering.
Things I checked
- ADCP_VERSION is the load-bearing pin, and it moved.
src/adcp/ADCP_VERSION3.1.8 → 3.1.10; README,canonical_formats/registry.py, andcanonical_formats/v1_to_v2.pydocstrings all track it.get_adcp_spec_version()and the registry loader (bundle_keycollapses to3.1) resolve consistently. No doc/version drift. - The TMPX shape change is regenerated end-to-end, not hand-edited.
identity_match_response.pyTmpxProviders.macros→chunks, roottmpx_macrosdropped,IdentityMatchResponserenamed toIdentityMatchResponseRouterPublisher. Newtmpx_chunk.py,provider_identity_match_response.py,publisher_tmpx_config.py, andprovider_registration.py(tmpx_macros→tmpx_slots) match the schema set.schemas/cache/3.1/trusted-match/tmpx-chunk.jsonandpublisher-tmpx-config.jsonare both added — every$refresolves, no dangling refs. - The alias layer survives the rename.
identity_match_response.pyappendsIdentityMatchResponse = IdentityMatchResponseRouterPublisherand retains a deprecatedTmpxMacrocompat model (post_generate_fixes.py:2011,restore_trusted_match_compatibility_aliases). Public imports throughaliases.pydon't break at import time. - Real codegen bug caught.
fix_publisher_tmpx_mapping_key_constraints(scripts/post_generate_fixes.py) rewrites datamodel-codegen's bareStringConstraints(...)outer map key toAnnotated[str, StringConstraints(...)]— without it Pydantic treats the constraint instance as a dataclass type and can't build the model. The new test importingpublisher_tmpx_configexercises exactly that build path. - Registry change is additive.
registries/v1-canonical-mapping.json1.2.0 → 1.3.0, +353 lines ofdisplay_*_image_2xRetina entries plus the new normative "Image rendition-set exception" rule — gated onrequired_pixel_ratios, which only the new entries carry, so existing-format projection is unchanged. - Test coverage is present, contrary to first glance.
test_trusted_match_validation.pymoves assertions to{slot_id, value}chunks, addstest_provider_identity_match_response_accepts_tmpx_chunks, and the provider-id rejection now asserts Pydantic'spropertyNamespattern error. Plustest_canonical_formats_registry.py/_v1_to_v2.pyfor Retina. ad-tech-protocol-expert: sound — the macros→slots split moves ad-server destination names off the wire into publisher-owned config, closing the cross-provider macro-name-hijack surface;not/anyOfroot guards are the correct draft-07 forbid idiom and are satisfiable.code-reviewer: no blockers.- The
adagents.py/brand/__init__.pyregen is pure timestamp-and-$schema-example-string churn (3.1.8 → 3.1.10 in embedded examples) — noted and discarded per the codegen-churn rule.
Follow-ups (non-blocking — file as issues)
IdentityMatchResponseshape changed under a preserved name. The alias keeps the import working, but the model no longer accepts roottmpx_macrosandTmpxProvidersnow wantschunks, notmacros— code deserializing the old #5689 shape intoIdentityMatchResponsewill nowValidationError. The schema self-documents this as sanctioned byx-status: experimental, and both experts signed off on breaking an experimental surface without a version hatch, so this ships. But the commit isfeat(protocol):with no!/BREAKING CHANGE:footer. Experimental-surface latitude is why this isn't a block — add a one-line migration note to the changelog (macros→chunks,IdentityMatchResponseis now the router→publisher shape) so adopters on the experimental path aren't surprised.
Minor nits (non-blocking)
- Mid-module
import re.restore_trusted_match_compatibility_aliases/ the validator patcher inject_PROVIDER_ID_PATTERNand imports below the module top in the generatedidentity_match_response.py— an E402-shaped artifact. Harmless as long asgenerated_poc/stays lint-excluded (it is, per CLAUDE.md); worth a glance thatmake lintstill skips it after the new file lands.
Approving on the strength of the regenerated-model/schema lockstep plus the preserved alias layer. Follow-up noted.
There was a problem hiding this comment.
Regenerates the Trusted Match/TMPX surface from AdCP 3.1.10 and lands the seller-Retina precedence fix. The wire-shape churn is a real break, but it sits entirely on an x-status: experimental surface and upstream shipped it as a patch increment — the feat: bump holds.
Things I checked
- Breaking-change classification.
TmpxProviders(extra='forbid') renamesmacros: list[TmpxMacro]→chunks: list[TmpxChunk]with a{name,value}→{slot_id,value}shape flip, roottmpx_macrosis removed, andprovider_registration.tmpx_macros→tmpx_slots. Old-shape payloads now raiseValidationError.ad-tech-protocol-expert: breaking at the wire level, unambiguously — butx-status: experimentalis confirmed present on every touched schema (schemas/cache/3.1/trusted-match/identity-match-response.json:14,provider-registration.json:5, and the three new files), and upstream itself shipped this as 3.1.8→3.1.10 patch, which is only legitimate under the experimental carve-out.feat:without!is defensible. Not a block. - Import-level compat preserved.
IdentityMatchResponse = IdentityMatchResponseRouterPublisherand the deprecatedTmpxMacroclasses are retained (unreferenced) bypost_generate_fixes.py. Verifiedaliases.py:1998,2001still resolves itsTmpxMacro as IdentityMatchTmpxMacro/ProviderRegistrationTmpxMacrore-exports against the retained classes — the alias layer does not break. - Codegen post-processing, not hand-edits. The two new repair functions run inside
post_generate_fixes.py:main()in the correct order —fix_trusted_match_runtime_validators(appends the indented validator into the class) beforerestore_trusted_match_compatibility_aliases(appends the module-levelIdentityMatchResponse = ...); reversed, that's a syntax error.code-reviewerconfirmed all three repair paths are idempotent (thebroken/fixedneedles are non-overlapping, the validator early-branches on_validate_tmpx_provider_ids in source). - Seller precedence (
v1_to_v2.py)._SELLER_AUTHORITATIVE_PARAM_KEYS = {pixel_ratios, required_pixel_ratios, slots}strips exactly the pixel-density/slot contracts fromregistry_paramson the seller-annotation path while width/height survive.code-reviewerenumerated the full registry param key-space ({aspect_ratio, daast_version, duration_ms_exact, height, pixel_ratios, slots, vast_version, width}) — the strip-set is correct and complete for 3.1.10.ad-tech-protocol-expert: correct reading ofadcontextprotocol/adcp#6217. - Registry.
v1-canonical-mapping.jsonat v1.3.0 matches the wire namingdisplay_WxH_image_2x/display_WxH_image_1x_2xacross all seven catalog sizes; test counts move 29→43 mappings, 22→36 literals, 7 structural held._generated.pyimports and__all__are consistent for all five new names.
Follow-ups (non-blocking — file as issues)
- Mark the TMPX surface experimental in the release notes. The
feat:bump is sound because the surface is experimental, but that status lives only in schema JSON and field descriptions —src/adcp/types/registry.pyexposes no programmaticx-statusmarker. Confirm the CHANGELOG/release entry names Trusted Match/TMPX as experimental so adopters relying on the old shape are on notice. This is the one thing that makes the minor bump defensible rather than a surprise. _SELLER_AUTHORITATIVE_PARAM_KEYSis a denylist. Correct today, but a future registry that adds a new slot/pixel-density parameter leaks it into seller-authored declarations. A governance test over the registry param key-space (fail if a new key isn't classified neutral-vs-authoritative) would close the gap.- Root-field prohibition not enforced in Python. Both response schemas forbid
tmpx_macros/tmpx_chunksat the root vianot:{anyOf:[...]}; datamodel-codegen dropsnotand the models useextra='allow', so a stray roottmpx_macrosis silently absorbed into extras rather than rejected. The SDK is more lenient than the schema on a privacy-boundary constraint —fix_trusted_match_runtime_validatorsalready injects a provider_id validator and could close this the same way.
Minor nits (non-blocking)
- Dead validator in
identity_match_response.py.tmpx_providers' dict key now carriesStringConstraints(pattern=r'^[A-Za-z0-9_]+$', min_length=1, max_length=64), which is exactly equivalent to_PROVIDER_ID_PATTERNand fires first — the injected_validate_tmpx_provider_idscan never raise. The test even had to move itsmatch=from"tmpx_providers keys"to"String should match pattern"to follow the change. Harmless, redundant. fix_publisher_tmpx_mapping_key_constraintsfails silently on codegen drift. Thebrokenneedle hardcodes the 12-space indent andpattern, min_length, max_lengtharg order; if datamodel-codegen changes either, the guard no-ops and ships an un-AnnotatedStringConstraintsouter key that Pydantic can't build. CI catches it (the import intest_trusted_match_validation.pyfails loudly), but araiseon the miss beatsprint(... already fixed).
Approving on the strength of the experimental-status confirmation plus faithful regen against the 3.1.10 wire shape. Follow-ups noted — the CHANGELOG experimental marker is the one worth doing before the GA cut.
Updates the SDK protocol pin and bundled schemas from AdCP 3.1.8 to the signed 3.1.10 release, including the expanded Retina canonical-format registry and refreshed documentation/signing provenance. Regenerates Python models for the Trusted Match TMPX slot contract while preserving prior public helper aliases for compatibility. Experimental Trusted Match migration: provider-supplied tmpx_macros/macros are replaced by publisher-owned tmpx_slots/chunks, and IdentityMatchResponse now represents the router-to-publisher response shape. Adds codegen repairs for the renamed response model and nested publisher mapping key constraints, plus Retina projection coverage across all seven catalog sizes. Aligns projection precedence with adcontextprotocol/adcp#6217: seller-authored canonical declarations retain neutral registry dimensions but do not inherit registry slots or pixel-density constraints. Validated with the full suite (6,216 passed, 41 skipped, 9 deselected, 1 expected xfail) and all pre-commit gates.