feat(creative)!: make canonical models primary in v7 by bokelley · Pull Request #994 · adcontextprotocol/adcp-client-python

bokelley · 2026-07-29T10:30:50Z

Summary

make canonical creative declarations the primary Python 7 SDK contract across root imports, models, clients, servers, callbacks, helpers, and response builders
isolate named-format compatibility behind explicit Legacy* APIs and make list_creative_formats legacy-only
implement TypeScript 13.0.0-rc.3 projection/downgrade parity, including deterministic option IDs, route precedence, collision rejection, and process-boundary safety
vendor the complete tagged RC3 product fixtures and transition-test corpus byte-for-byte, with SHA-256 integrity checks
add runtime Pydantic schema and serialization invariants that prevent primary models from exposing or emitting legacy creative identity
make modern server/framework capability construction advertise canonical creatives automatically while suppressing the feature for explicit AdCP 3.0
configure release-please to cut the v7 RC; keep the feature branch version aligned with the 6.6.0 release manifest until release automation updates both

Downstream blocker audit

AdCP 3.2 is no longer advertised until a real validator bundle ships; a regression requires request and response validators for every advertised native version
AdCP 3.1 dual emission treats canonical evidence as authoritative, and ignores format-shaped values in opaque context/ext bags
framework-generated 3.1/current capability responses default canonical_creatives: true; explicit 3.0 removes the field; explicit legacy-only compatibility fixtures remain supported
both actual seller storyboard CI tags pass locally: latest 93/93 selected steps and adcp-3.0 59/59
PR title and lead commit use feat(creative)!: and the lead commit has a concrete BREAKING CHANGE: footer

Compatibility behavior

exact owner plus ID resolution wins
bare-ID fallback is allowed only for a unique bundled AAO match
the original legacy tuple is retained for legacy delivery
custom converters override compatibility inference
collisions, unsafe owners, and contradictory selectors are rejected
canonical declarations are never reverse-guessed after a process boundary

Validation

pytest -q: 6,016 passed, 41 skipped, 9 deselected, 1 expected failure
@adcp/sdk@latest seller storyboard: 93 selected steps passed
@adcp/sdk@adcp-3.0 seller storyboard: 59 steps passed
all pre-commit gates pass against current origin/main
mypy and strict adopter type-check fixtures pass
Ruff, Black, Bandit, JSON/YAML, whitespace, conflict, case, large-file, and private-key checks pass
runtime release-surface probe confirms canonical root Format, unavailable root FormatId, explicit LegacyFormatId, and recursively clean primary schemas

Closes #972

aao-ipr-bot · 2026-07-29T11:49:34Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T11:53:53Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T11:59:20Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T12:09:40Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot

Request changes: breaking diff shipped under a minor-bump conventional-commit signal. The architecture is right — canonical-first with an explicit Legacy* boundary is the correct shape, and the credential and layering invariants hold — but the semver signal has to match the diff before this can merge.

MUST FIX (blocking)

1. Breaking public-API change under feat: without ! or a BREAKING CHANGE: footer. pyproject.toml:7 bumps 6.6.0 → 7.0.0rc1 (major), but the lead commit is feat(creative): make canonical models primary in v7 — no !, and every commit body is empty, so no BREAKING CHANGE: footer either. The diff is unambiguously breaking on the wire:

FormatId removed from the adcp.* surface (src/adcp/__init__.py __all__ + _LAZY_MODULES; confirmed gone in tests/fixtures/public_api_snapshot.json).
BuildCreativeRequest / BuildCreativeResponse / PreviewCreativeRequest / PreviewCreativeResponse (+ *Interactive* / *Static* / numbered variants) renamed to Legacy* in src/adcp/__init__.py.
ADCPClient.build_creative / preview_creative renamed to *_legacy in src/adcp/client.py, and build_creative / list_creative_formats now hard-rejected by execute_task's legacy_only_tasks gate.

This repo runs release-please + conventional commits: feat: computes a MINOR bump. Failure mode in one sentence: release-please reads feat(creative):, cuts 6.7.0, and every v6 adopter doing pip install -U then from adcp import FormatId / BuildCreativeRequest — or calling client.build_creative(...) — breaks on what the changelog calls a non-breaking minor. Fix: retitle to feat(creative)!: and add a BREAKING CHANGE: footer enumerating the removed/renamed public API. The migration note itself is already here (MIGRATION_v6_to_v7.md), so this is purely the commit-message signal — but the signal is the contract release-please acts on, not the prose. The version string and the commit currently disagree about what kind of release this is.

Things I checked

Security is clean. security-reviewer: no High/Medium. dispatch.py is capability-slug renames only — _build_request_context and _CREDENTIAL_SHAPED_KEY_SUFFIXES untouched, so the no-credentials-in-RequestContext.metadata contract holds. The PR actually adds an echo-path control: Format's before-validator rejects credential-shaped keys in params/extras (src/adcp/types/canonical_creative.py), tested in tests/test_canonical_projection_scope.py.
No committed secret. projection.py:60 hmac.new(b"", …) is an empty-key content hash for RC3 byte-parity, not an auth primitive; process-boundary safety does not rely on it being secret (resolve_legacy_format_refs at projection.py:721 refuses to reverse-guess without an explicit durable resolver regardless).
Wire shape holds. ad-tech-protocol-expert: sound-with-caveats, no cited divergence. Option-ID hashing handles the two parity traps correctly — JS integer normalization (float.is_integer() → int) and separators=(",",":")/ensure_ascii=False to mirror JSON.stringify — and LegacyFormatId.agent_url is narrowed to str (src/adcp/types/legacy.py:139-151) so Pydantic AnyUrl can't append a trailing slash and mutate the hashed tuple. Route precedence (exact owner+id wins, bare-id fallback only on unique bundled-AAO match), collision-poison-to-None, and unsafe-owner fail-closed (projection.py:83-104) are consistent with canonical-format semantics.
Layering not breached at the test level. tests/test_import_layering.py adds legacy.py and canonical_creative.py to the allowlist in lockstep with their generated_poc imports, so the enforced contract passes.
Imports resolve. Format/Product rebind to canonical_creative in _eager.py; FormatId is removed everywhere, not left dangling.

Follow-ups (non-blocking — file as issues)

CLAUDE.md layering list is now stale. The "Import Architecture for Generated Types" section still names only the old allow-set; legacy.py and canonical_creative.py now import from generated_poc and were added to test_import_layering.py but not to the doc. test_claude_md_documents_the_rule only regex-checks loosely, so it won't catch the drift. (code-reviewer)
Golden fixture is forward-only and narrow. tests/fixtures/canonical/typescript-13.0.0-rc.3-golden.json is 17 image/audio/generative cases from 3 owners with no downgrade golden, despite the PR title claiming "downgrade parity," and no video/custom/publisher-scope/non-ASCII coverage. Parity is pinned to a hand-authored fixture, not an automated TS↔Python conformance run. (ad-tech-protocol-expert)
Fail-closed product omission couples correctness to catalog freshness. project_legacy_product returns product=None when no format maps (projection.py:610-625); a v7 buyer against a live seller emitting named IDs not in the vendored rc3-aao-additions.json, with no legacy_format_converter, silently drops products. Defensible, but worth a documented refresh/versioning story for the vendored catalog.

Minor nits (non-blocking)

Unreachable diagnostic arm. projection.py:600-608: raw.pop(\"format_ids\", None) runs just above, so not raw.get(\"format_ids\") is always True and a non-empty-but-unprojectable list is mislabeled legacy_format_list_empty. Base the branch on had_legacy + the original list length. (code-reviewer)
FormatId removal raises a bare AttributeError. Unlike the _REMOVED_IN_V4 map (__init__.py:56-86), from adcp import FormatId now gives no migration hint. Add it to an informative-error map pointing at LegacyFormatId.
hmac framing. projection.py:60 empty-key HMAC is a determinism hash, not integrity — a one-line comment stops a future reader from treating format_option_id as a trust boundary.

Flip to approve once the lead commit carries feat(creative)!: + a BREAKING CHANGE: footer listing the removed/renamed public API. Everything else here is a follow-up.

BREAKING CHANGE: FormatId is removed from the normal root API in favor of LegacyFormatId and explicit legacy modules. BuildCreativeRequest/Response and PreviewCreativeRequest/Response variants are renamed to Legacy*; ADCPClient build_creative/preview_creative and list_creative_formats move to *_legacy methods. Product and creative filters no longer expose format_ids, and generic primary execution rejects legacy-only creative tasks.

aao-ipr-bot · 2026-07-29T12:26:37Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T12:32:53Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T12:39:18Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T13:03:26Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

aao-ipr-bot · 2026-07-29T13:07:23Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

Reject unsupported validator bundles, make capability routing deterministic, preserve opaque request bags, enforce safe one-to-one durable routes, and give validated publisher/community snapshots precedence over bundled AAO fallbacks. Vendor and pin the complete TypeScript 13.0.0-rc.3 transition corpus.

aao-ipr-bot · 2026-07-29T15:10:23Z

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

_{This is an automated message from the Argus AI review workflow.}

bokelley · 2026-07-29T15:12:32Z

Final head c23f87f8c is green across all 25 checks, including both seller storyboards, Python 3.10–3.13, schema drift, CodeQL, and security. The prior Argus change request only asked for the breaking-change title/footer; both are fixed. The fresh Argus run completed its check successfully but failed to post a review after 59 tool-permission denials, then left the stale CHANGES_REQUESTED in place: https://github.com/adcontextprotocol/adcp-client-python/actions/runs/30464071244. The bot was explicitly re-requested as a reviewer as well, but GitHub did not retain that request. Human re-review/dismissal of the stale review is now the only merge blocker.

bokelley marked this pull request as ready for review July 29, 2026 10:31

github-code-quality Bot found potential problems Jul 29, 2026

View reviewed changes

Comment thread src/adcp/types/aliases.py Fixed

Comment thread src/adcp/canonical_formats/projection.py Fixed

github-code-quality Bot found potential problems Jul 29, 2026

View reviewed changes

Comment thread src/adcp/types/aliases.py Fixed

bokelley force-pushed the python-972-rc3-parity branch from b0e8dbe to 8741518 Compare July 29, 2026 10:57

bokelley added this to the 7.0.0-rc milestone Jul 29, 2026

aao-ipr-bot Bot requested changes Jul 29, 2026

View reviewed changes

bokelley added 9 commits July 29, 2026 08:21

fix(creative): preserve canonical capability evidence

eb4bf08

fix(creative): enforce explicit legacy boundaries

e1301b5

ci(storyboard): allow canonical schema cold start

4d5a787

fix(example): convert storyboard legacy formats

0374ff9

fix(decisioning): forward creative compatibility adapters

46fb74f

fix(example): map reference creative catalog

f95f9bb

fix(decisioning): retain canonical route sidecars

79052e7

fix(creative): clarify migration diagnostics

fbd70f5

bokelley changed the title ~~feat(creative): make canonical models primary in v7~~ feat(creative)!: make canonical models primary in v7 Jul 29, 2026

bokelley force-pushed the python-972-rc3-parity branch from 7c5474c to fbd70f5 Compare July 29, 2026 12:21

bokelley added 2 commits July 29, 2026 08:57

fix(creative): address downstream release blockers

39e6e89

test(release): support Python 3.10 configuration checks

41add3a

Uh oh!

Conversation

bokelley commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Downstream blocker audit

Compatibility behavior

Validation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

MUST FIX (blocking)

Things I checked

Follow-ups (non-blocking — file as issues)

Minor nits (non-blocking)

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

aao-ipr-bot Bot commented Jul 29, 2026

Uh oh!

bokelley commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bokelley commented Jul 29, 2026 •

edited

Loading