feat: support AdCP 3.1 Beta 4#872
Merged
Merged
Conversation
37c539f to
4d6f0cd
Compare
There was a problem hiding this comment.
Approving. Schema regen to 3.1.0-beta.4 with the public-API surface kept additive and the wire-shape fix on Pagination going in the right direction.
Things I checked
- Public-API additivity.
src/adcp/__init__.pyandsrc/adcp/types/__init__.pyadd 14 names (FormatOptionReference, PackageSignalTargeting{,Group,Groups}, Placement, PlacementReference, ProductSignalTargetingOption, SignalListing, SignalRef, SignalTargeting{,Expression,Rules}, WebhookChallenge, WebhookChallengeResponse). No removals, no renames.tests/fixtures/public_api_snapshot.jsonconfirms diff is purely additive.feat:(notfeat!:) is the right semver signal. responses.py:614total→total_count. ThePaginationmodel carriesextra='forbid'; the upstream field has beentotal_countsince beta.3. The previous default emitted an unknown field that any conformance-checking adopter rejected. Fix, not regression.ad-tech-protocol-expertverified againstschemas/cache/3.1.0-beta.4/core/pagination-response.json._widen_media_buy_output_schema_for_version_compatatsrc/adcp/server/mcp_tools.py:1690-1735. Variant-match predicate (media_buy_idinrequiredANDstatus.const == "completed") targets only the sync-success arms ofcreate_media_buy/update_media_buy. Theerrorsarm has nostatusconst; thesubmittedtask-envelope arm lacksmedia_buy_id— both untouched.ad-tech-protocol-expertconfirms the upstreamoneOfdiscriminates branches viarequired-set + the submitted branch'snot(required: media_buy_id|packages), so wideningstatusdoes not collide with the envelope discriminator.- Generated-type regeneration audit.
src/adcp/types/generated_poc/**andsrc/adcp/types/_generated.pyare the output of running the codegen againstschemas/cache/3.1.0-beta.4/.ADCP_VERSIONis bumped in lockstep. ThePricingOption13/14/15/16 → 141/142/...and similar renumber churn inSCHEMA_DELTAS.mdis expected codegen behavior per the project README. - Type-layering preserved. New exports reach the surface through
_generated→__init__only — no source file outside the allowlist imports fromgenerated_poc/. signal_targeting_allowedprojection move. The field moved from_get_products_helpers.py's non-enum pass-through set into theGetProductsFieldenum, matching the upstream 3.1 product-projection contract. Test added attests/test_get_products_projection.py:174-189asserts buyers must explicitly request it.- Test-plan honesty. PR description's validation is all checked: 5422/5422 pass, both storyboard matrix rows pass (59/59 latest, 90/90 beta), pre-commit passes. No unchecked boxes.
Follow-ups (non-blocking — file as issues)
PackageSignalTargeting{Groups,Group}.operatoris a two-tier enum. Outer (Groups) is["all"]only; inner (Group) is["any","none"]only. Worth a one-line note in the public docstring so adopters don't pass"all"on an inner group and get rejected by the seller validator. (ad-tech-protocol-expertflag.)- MCP outputSchema widening has no regression test.
tests/test_server_dx.py:957-987exercises the post-widen shape, but nothing asserts the variant-match predicate stays narrow if Pydantic's union arm ordering reshuffles. Add a test that theerrorsandsubmittedvariants are not mutated. - Typed-Pydantic path for sync
create_media_buyis stricter than upstream.src/adcp/types/generated_poc/media_buy/create_media_buy_response.py:32pinsstatus: Literal["completed"], but the upstream schema makesstatusoptional and referencesMediaBuyStatus. The widening covers the MCP-advertisement layer; confirm the per-version response validator path actually short-circuits Pydantic.parse for 3.0 buyers, or 3.0 sellers' payloads will round-trip-fail through the typed path. (ad-tech-protocol-expertcaveat.)
Minor nits (non-blocking)
fix_deprecated_rootmodel_fieldsline-blanking is brittle.scripts/post_generate_fixes.py:1483-1487clears the entire line whenever it containsdeprecated=True. Today datamodel-code-generator emitsdeprecated=Trueon its own line for RootModel root fields, so it works. If a future emission packsField(deprecated=True, description=…, ge=…)onto one line (seebundled/signals/get_signals_request.py:813for an example of that layout elsewhere in the tree), the fix erases the wholeField(...)call and produces a syntax error. A targetedre.sub(r",?\s*deprecated\s*=\s*True\s*,?", "", lines[index])would be safer.- CI cache key is now a moving label.
.github/workflows/ci.yml:12switchedADCP_SDK_VERSIONfrom pinned8.1.0-beta.7to thebetadist-tag, and the npm cache key uses the tag directly. Cache contents will drift relative to the installed package over time. Not blocking —npm install -gstill runs every job — but a deterministic-version recording step would make storyboard reruns reproducible. The docstring you added already calls out the long-term solution (sticky per-minor tags upstream). _is_adcp_31_or_newerexception list.src/adcp/server/mcp_tools.py:52-60catchesAttributeError/TypeError; the only branch into the body operates on a string, so narrowing toValueErrorwould be tighter.
Safe to merge.
Contributor
Author
|
Nudging GitHub Actions after workflow-only CI gate fix did not create a run for the new head. |
Contributor
Author
|
Nudging GitHub Actions again after resolving the main-branch conflict; prior pushes only attached GitGuardian, not the required Actions suite. |
# Conflicts: # src/adcp/server/mcp_tools.py # tests/test_server_dx.py
33ef86e to
74e5529
Compare
This was referenced May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@adcp/sdkadcp-3.0andadcp-3.1dist-tagsValidation
PYTHONPATH=src uv run pytest tests/ -q- 5422 passed, 30 skipped, 10 deselected, 1 xfailedPYTHONPATH=src uv run ruff check src/adcp/server/mcp_tools.py src/adcp/server/responses.py examples/seller_agent.py tests/test_server_dx.pyADCP_SDK_VERSION=adcp-3.0 STORYBOARD_RESULT_PATH=.context/storyboard-result-adcp-3.0.json bash scripts/ci/run_storyboard_reference_seller.sh- 59 passed, 0 failedADCP_SDK_VERSION=adcp-3.1 ADCP_PORT=3002 STORYBOARD_RESULT_PATH=.context/storyboard-result-adcp-3.1-retry.json SELLER_LOG_PATH=/tmp/adcp-reference-seller-3002.log bash scripts/ci/run_storyboard_reference_seller.sh- 90 passed, 0 failednpm view @adcp/sdk dist-tags --json- confirmedadcp-3.0->7.11.0andadcp-3.1->8.1.0-beta.12