Skip to content

feat(codegen)!: compile what a schema root states, and route every arm it declares - #1522

Open
KonstantinMirin wants to merge 3 commits into
adcontextprotocol:mainfrom
KonstantinMirin:integration/all-of-ours
Open

KonstantinMirin wants to merge 3 commits into
adcontextprotocol:mainfrom
KonstantinMirin:integration/all-of-ours

Conversation

@KonstantinMirin

@KonstantinMirin KonstantinMirin commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1517
Closes #1486

Carries the canonical-format and codegen work developed on the sales-agent
integration branch onto main as one change, plus four smaller fixes in the same
surface. The generated tree here is REGENERATED from the generator in this
commit, not copied from the branch it came from.

Why this is one change rather than a series

The generator and the types that consume it were co-developed. Grouping the
source commits by subject gives eight themes, but they are interleaved rather
than sequential: of 85 commits carrying reviewable content, the constraint
reader/compiler theme spans positions 0..80 and the split-root theme spans
6..82, in 60 contiguous runs. Thirty-three files appear in more than one theme,
three of them (scripts/post_generate_fixes.py, types/_forward_compat.py,
types/canonical_creative.py) in five. Any thematic split would reorder commits
that depend on each other, so the themes describe the work without being able to
schedule it.

The review burden is far smaller than the file count suggests, because most of
it is the generator:

hand-written src/   60 files
scripts/            13 files
generated output   250 files   (follows from the generator; see below)

What it does

Compile a schema root's constraint content instead of a third of it: one reader
for what a root states, a vocabulary the compiled constraints are written in, and
a compiler that names an arm and plans a branch class. Split roots then get a
shared public base (<Root>Shape), one dispatch hook mixed in rather than a copy
per root, and construction dispatched as well as validation, so a public name
resolves to the branch a document satisfies.

Around that: error.field carries the JSONPath-lite the spec requires, taken
from a located issue and resolved against the document the peer actually sent,
never from the document root; recovery text is derived from the code the pin
registers, deleting seventy literals that restated it; narrowing stops giving up
on the inputs that most need it; and pin gaps and compatibility annotations are
declared in the schema at the field rather than patched onto classes.

The four smaller fixes

  • Format references compare every parameter they declare. core/format-id.json
    declares four parameters beyond the (agent_url, id) identity, and three
    helpers each named three of them in a hand-written tuple, so pixel_ratio was
    never compared: a product requiring 2x accepted a creative claiming no density,
    and accepted one claiming a conflicting density, while the legacy-size upgrade
    discarded a density it was handed. The set is now derived from model_fields.
    pixel_ratio keeps one deliberate rule, because the schema gives it one —
    omitting it selects the backward-compatible 1x variant, so an absent density
    reads as 1x rather than as unspecified.

  • Open asset unions are generated, and route by a derived vocabulary. The
    callable discriminator read a hand-written frozenset of 14 individual and 12
    group types beside generated unions of 21 and 21, and it read that frozenset
    before pydantic reached the arms — so seven individual and nine group asset
    types the pinned schema declares parsed as the unknown fallback with their
    generated classes unused. The vocabulary now comes from the arms' own literals.

  • A declaration's v1 refs are read in either spelling. The generated
    ProductFormatDeclaration arms declare v1_format_ref; canonical_creative.Format
    exposes legacy_format_refs over private state. Five functions read the second
    and raised AttributeError on the first, which made the v2-to-v1 product
    projection unusable against the generated types, and client.py read it behind
    a getattr(..., ()) default that answered "asserts no v1 refs" with no error.
    All six reads go through one accessor that refuses a type declaring neither.

  • Four Legacy* names are re-exported from adcp.types.legacy.
    LegacyProposalTotalBudgetGuidance and the three
    LegacyUpdateMediaBuy{Success,Error,Submitted}Response names were reachable
    only from adcp.types.aliases, so an adopter following the migration module's
    own docstring could not reach them.

Note the first two are the same defect: a hand-written list beside a generated
truth, where the hand-written one was short. They were found independently.

Two of these tests were reconciled, not the refusal narrowed

test_canonical_boundary_serialization.py::...[parent] and
test_canonical_creatives_rc3.py::test_primary_dump_cannot_emit_legacy_identity_from_private_state_or_extras
are from the branch this work came from, and both failed here. Each builds its
fixture with model_construct -- deliberately, since the subject is what a DUMP
can emit and the fixture is a document validation refuses by design -- and then
reached its last assertion through Envelope(catalog=...) / Container(declaration=...),
which validates that fixture and is refused by the wrap validator on the
canonical boundary base.

Measured before changing anything, and measured twice independently by two
people: the refusal fires on that exact document at TOP LEVEL
(Format.model_validate, Catalog.model_validate) as well as nested. That is a
fact about production behaviour rather than a preference about what the test
should say, which is what makes editing our own test reviewable -- a reader who
doubts it can re-run those two calls. Since the refusal is correct at top level
it is not over-firing on re-validation, so narrowing it is out; and the document
is never legal in the first place, because model_construct bypasses validation
entirely. The expectation was always wrong and the older serialization path
simply never reached the check.

test_canonical_creatives_rc3 was checked separately rather than assumed to
share the cause. It does, and more plainly: that test's own first assertion is
pytest.raises(ValidationError, match="legacy creative identity") on a normal
construction, so it proves validation refuses this shape and then asks
validation to accept it.

No production code was changed on either side -- not the refusal, not
#1521's serialization path. Only the two tests' plumbing: each now builds its
containing model with model_construct, like the four sibling parameters that
already passed. [parent] is kept rather than deleted, because it is the only
case covering "the serializer does not add a fourth pass over the full tree when
nested", and its assertions are untouched -- roots == 3 still fails if the
strip stops running once per root, verified by breaking it to 99 and watching
all five parameters fail.

Not included, deliberately

  • No spec-version move. ADCP_VERSION stays at main's 3.2.1 and no file
    under schemas/ or _compliance/ is touched. The branch this came from also
    carried a 3.2.3 adoption; that overlaps fix(protocol): adopt signed AdCP 3.2.4 on SDK 9 beta #1479 (signed 3.2.4) on 1230 files and
    does not cherry-pick onto main, so it is left to that PR.
  • No interpreter or lockfile policy. The source branch pinned
    .python-version to 3.12 and committed a 3240-line uv.lock. Main has
    neither and its CI tests 3.10/3.11/3.13, so carrying an interpreter-pin
    proposal inside this change would invite rejection for a reason unrelated to
    the work. Excluded as a decision, not an oversight; it remains available as a
    separate proposal.

Verification

  • generate_versioned_stubs.py, generate_types.py, generate_versioned_bases.py
    all pass --check after regeneration, so the 250 generated files follow from
    the generator in this commit rather than from the tree they were copied out of.
  • ruff check src/ clean; mypy src/adcp/ clean over 1538 modules.
  • Formatting: this branch leaves 7 files unformatted, all of which are
    unformatted on main too; it formats the two of main's nine that it touches.
  • Every public symbol and model shape a downstream consumer resolves is unchanged
    against main: 323 of 323 imported names resolve, and all 164 pydantic models
    carry an identical (field, required) set.

Verification

base                 bde770b76 (upstream/main)

suite  main          9 failed / 16887 passed / 2641 skipped
suite  this branch   2 failed / 17451 passed / 2649 skipped
       new failures  0
       on both       2   pre-existing (agent_resolution_33, reporting_inline_source)
       on main only  7   node/JS-interop set; environment-dependent, nothing claimed from it

generators           generate_types / generate_versioned_stubs / generate_versioned_bases
                     --check, all clean
ruff check src/      clean
mypy src/adcp/       clean, 1538 modules

The generated tree is regenerated from the generator in this commit rather than copied.
Regeneration is what surfaced that types/response_dispatch.py and types/_ergonomic.py
are correctly deleted — main's generated tree still imported the former.

Known failing checks, with reasons

Two checks fail deliberately rather than being papered over. Both are filed.

Test Python 3.12 — one [assert-type] error in tests/type_checks/creative_asset_binding.py.
The readback manifest type binds one arm of a split root, where the pin
(core/creative-manifest.json, required: ['assets']) makes format_kind optional on the
root. Binding the root instead fixes this error and breaks
test_delivery_manifest_cannot_bypass_strict_input — measured in both directions. One type is
carrying two incompatible obligations, tolerant readback and strict input; the fix is two
types, filed separately rather than buried in a change this size.

Postgres conformance (Postgres 16, core) — one test,
test_core_reconciliation_refuses_a_managed_delivery_ledger. It injects destination_ref into
a healthy period, which core/reporting-obligation.json's allOf/4/then/required then requires
materialization counts alongside, so the read refuses with STATUS_READ_FAILED before the tier
check runs. Supplying those counts is not available either: a Core ledger has none, and
fabricating them makes the fixture assert something false. The fixture needs a redesign, not a
patch. The Postgres conformance (Postgres 16) row is the aggregator and mirrors this one.

Three other failures in this lane were real and are fixed: the ReportingObligation helper
declared health="healthy", production_status="published" while passing revisions=(), so
revision_count derived to 0 against a schema minimum of 1. Verified against the pin with
jsonschema independently of this branch's compiler. The core lane is 2766 passed, up from 2763.

BREAKING CHANGE: adcp.types.response_dispatch is removed, and with it ResponseArmDispatchMixin. The generated response classes now inherit BranchDispatchMixin directly, so a consumer needs no replacement import.

@KonstantinMirin
KonstantinMirin force-pushed the integration/all-of-ours branch 2 times, most recently from f341c4a to 42e4cd0 Compare October 17, 2026 01:48
…m it declares

Carries the canonical-format and codegen work developed on the sales-agent
integration branch onto main as one change, plus four smaller fixes in the same
surface. The generated tree here is REGENERATED from the generator in this
commit, not copied from the branch it came from.

## Why this is one change rather than a series

The generator and the types that consume it were co-developed. Grouping the
source commits by subject gives eight themes, but they are interleaved rather
than sequential: of 85 commits carrying reviewable content, the constraint
reader/compiler theme spans positions 0..80 and the split-root theme spans
6..82, in 60 contiguous runs. Thirty-three files appear in more than one theme,
three of them (`scripts/post_generate_fixes.py`, `types/_forward_compat.py`,
`types/canonical_creative.py`) in five. Any thematic split would reorder commits
that depend on each other, so the themes describe the work without being able to
schedule it.

The review burden is far smaller than the file count suggests, because most of
it is the generator:

    hand-written src/   60 files
    scripts/            13 files
    generated output   250 files   (follows from the generator; see below)

## What it does

Compile a schema root's constraint content instead of a third of it: one reader
for what a root states, a vocabulary the compiled constraints are written in, and
a compiler that names an arm and plans a branch class. Split roots then get a
shared public base (`<Root>Shape`), one dispatch hook mixed in rather than a copy
per root, and construction dispatched as well as validation, so a public name
resolves to the branch a document satisfies.

Around that: `error.field` carries the JSONPath-lite the spec requires, taken
from a located issue and resolved against the document the peer actually sent,
never from the document root; recovery text is derived from the code the pin
registers, deleting seventy literals that restated it; narrowing stops giving up
on the inputs that most need it; and pin gaps and compatibility annotations are
declared in the schema at the field rather than patched onto classes.

## The four smaller fixes

- **Format references compare every parameter they declare.** `core/format-id.json`
  declares four parameters beyond the `(agent_url, id)` identity, and three
  helpers each named three of them in a hand-written tuple, so `pixel_ratio` was
  never compared: a product requiring 2x accepted a creative claiming no density,
  and accepted one claiming a conflicting density, while the legacy-size upgrade
  discarded a density it was handed. The set is now derived from `model_fields`.
  `pixel_ratio` keeps one deliberate rule, because the schema gives it one —
  omitting it selects the backward-compatible 1x variant, so an absent density
  reads as 1x rather than as unspecified.

- **Open asset unions are generated, and route by a derived vocabulary.** The
  callable discriminator read a hand-written frozenset of 14 individual and 12
  group types beside generated unions of 21 and 21, and it read that frozenset
  before pydantic reached the arms — so seven individual and nine group asset
  types the pinned schema declares parsed as the unknown fallback with their
  generated classes unused. The vocabulary now comes from the arms' own literals.

- **A declaration's v1 refs are read in either spelling.** The generated
  `ProductFormatDeclaration` arms declare `v1_format_ref`; `canonical_creative.Format`
  exposes `legacy_format_refs` over private state. Five functions read the second
  and raised `AttributeError` on the first, which made the v2-to-v1 product
  projection unusable against the generated types, and `client.py` read it behind
  a `getattr(..., ())` default that answered "asserts no v1 refs" with no error.
  All six reads go through one accessor that refuses a type declaring neither.

- **Four `Legacy*` names are re-exported from `adcp.types.legacy`.**
  `LegacyProposalTotalBudgetGuidance` and the three
  `LegacyUpdateMediaBuy{Success,Error,Submitted}Response` names were reachable
  only from `adcp.types.aliases`, so an adopter following the migration module's
  own docstring could not reach them.

Note the first two are the same defect: a hand-written list beside a generated
truth, where the hand-written one was short. They were found independently.

## Two of these tests were reconciled, not the refusal narrowed

`test_canonical_boundary_serialization.py::...[parent]` and
`test_canonical_creatives_rc3.py::test_primary_dump_cannot_emit_legacy_identity_from_private_state_or_extras`
are from the branch this work came from, and both failed here. Each builds its
fixture with `model_construct` -- deliberately, since the subject is what a DUMP
can emit and the fixture is a document validation refuses by design -- and then
reached its last assertion through `Envelope(catalog=...)` / `Container(declaration=...)`,
which validates that fixture and is refused by the wrap validator on the
canonical boundary base.

Measured before changing anything, and measured twice independently by two
people: the refusal fires on that exact document at TOP LEVEL
(`Format.model_validate`, `Catalog.model_validate`) as well as nested. That is a
fact about production behaviour rather than a preference about what the test
should say, which is what makes editing our own test reviewable -- a reader who
doubts it can re-run those two calls. Since the refusal is correct at top level
it is not over-firing on re-validation, so narrowing it is out; and the document
is never legal in the first place, because `model_construct` bypasses validation
entirely. The expectation was always wrong and the older serialization path
simply never reached the check.

`test_canonical_creatives_rc3` was checked separately rather than assumed to
share the cause. It does, and more plainly: that test's own first assertion is
`pytest.raises(ValidationError, match="legacy creative identity")` on a normal
construction, so it proves validation refuses this shape and then asks
validation to accept it.

**No production code was changed on either side** -- not the refusal, not
`adcontextprotocol#1521`'s serialization path. Only the two tests' plumbing: each now builds its
containing model with `model_construct`, like the four sibling parameters that
already passed. `[parent]` is kept rather than deleted, because it is the only
case covering "the serializer does not add a fourth pass over the full tree when
nested", and its assertions are untouched -- `roots == 3` still fails if the
strip stops running once per root, verified by breaking it to 99 and watching
all five parameters fail.

## Not included, deliberately

- **No spec-version move.** `ADCP_VERSION` stays at main's 3.2.1 and no file
  under `schemas/` or `_compliance/` is touched. The branch this came from also
  carried a 3.2.3 adoption; that overlaps adcontextprotocol#1479 (signed 3.2.4) on 1230 files and
  does not cherry-pick onto main, so it is left to that PR.
- **No interpreter or lockfile policy.** The source branch pinned
  `.python-version` to 3.12 and committed a 3240-line `uv.lock`. Main has
  neither and its CI tests 3.10/3.11/3.13, so carrying an interpreter-pin
  proposal inside this change would invite rejection for a reason unrelated to
  the work. Excluded as a decision, not an oversight; it remains available as a
  separate proposal.

## Verification

- `generate_versioned_stubs.py`, `generate_types.py`, `generate_versioned_bases.py`
  all pass `--check` after regeneration, so the 250 generated files follow from
  the generator in this commit rather than from the tree they were copied out of.
- `ruff check src/` clean; `mypy src/adcp/` clean over 1538 modules.
- Formatting: this branch leaves 7 files unformatted, all of which are
  unformatted on main too; it formats the two of main's nine that it touches.
- Every public symbol and model shape a downstream consumer resolves is unchanged
  against main: 323 of 323 imported names resolve, and all 164 pydantic models
  carry an identical (field, required) set.

BREAKING CHANGE: `adcp.types.response_dispatch` is removed, and with it
`ResponseArmDispatchMixin`. A per-root copy of the dispatch hook is replaced by
one mixed-in hook, `adcp.types._branch_dispatch.BranchDispatchMixin`, which the
generated response classes now inherit directly -- so a consumer importing
`from adcp.types.response_dispatch import ResponseArmDispatchMixin` must stop;
nothing needs to be imported in its place, because the generated classes carry
the behaviour themselves. `adcp.types._ergonomic` is removed in the same way,
with the one coercion that was live declared at its field instead.
@KonstantinMirin
KonstantinMirin force-pushed the integration/all-of-ours branch from 42e4cd0 to 7de831d Compare October 17, 2026 01:52
… not one arm

``_DeliveryCreativeManifest`` refined
``CreativeManifestCanonicalFormatManifest`` -- oneOf[1] of the manifest root --
while the public ``CreativeManifest`` refines the root itself. Both are
reached through the SAME ``$ref`` to ``core/creative-manifest.json``:
``core/creative-variant.json`` for the input manifest,
``core/get-creative-delivery-response.json`` for the readback one. One schema
node refined twice, so the two owe a document one verdict, and they gave two.

The root's ``required`` is ``['assets']``; oneOf[1] additionally requires
``format_kind``. The disagreement showed up in two places, neither of them the
format-kind vocabulary:

* statically, ``format_kind`` read ``str`` on the readback manifest and
  ``str | None`` on the input one. That is the ``mypy --strict`` error
  ``tests/type_checks/creative_asset_binding.py:82`` carried, and the only
  reason the ``Test Python 3.12`` leg was red.
* in the refusal of ``{"assets": {}}``: the root-bound class names the root's
  combinator (``type='oneOf'``), the arm-bound one reported a ``missing``
  field and never reached the combinator. ``core/error.json`` requires an
  issue to carry a JSON Schema keyword and ``missing`` is not one.

Binding the root relaxes nothing at runtime. The root's ``oneOf`` is enforced
by the root's own conjunct validator, not by ``BranchDispatchMixin``'s branch
union -- the mixin hands the union only to the class that DECLARES
``_branch_models``, and a refinement declared in ``canonical_creative`` does
not. Measured against the input manifest on all three documents: canonical arm
accepted by both, neither-arm refused ``oneOf`` by both, both-arms refused
``value_error`` by both.

Note what the previous comments on this binding claimed and what is actually
true. They said binding the root "would make it validate as the root's UNION";
it does not, for the reason above. The real cost is narrower: a root-bound
instance is an instance of the root and of NEITHER arm, so revalidating the
INSTANCE through the generated root refuses it as ``model_type`` where it used
to be taken. That is the obligation adcontextprotocol#1241 asked for, and it now holds on all
four (response model x input class) pairs instead of three -- so
``test_delivery_manifest_cannot_bypass_strict_input``'s conditional collapsed
in the strengthening direction. Its own docstring already called the accepted
case "the instance-revalidation bypass that the subclassing opened".

Two new tests grade the fix rather than leaving it asserted only by mypy:
``test_readback_manifest_grades_a_document_exactly_as_the_input_manifest``
(behavioural, three documents) and
``test_readback_manifest_refines_the_manifest_root_not_one_arm``
(structural). Re-pointing the class at the arm turns 3 of those 4 cases red --
verified by doing it.

The shrink-only ceiling in ``tests/test_type_check_fixtures.py`` held exactly
this one error and is removed, not lowered: the residual is fixed, and the CI
step it mirrors tolerates nothing either, so a ceiling could only hide
something that reddens there.
…tion

``test_core_reconciliation_refuses_a_managed_delivery_ledger`` pinned
``destination_ref`` onto a HEALTHY period and expected
``MANAGED_DELIVERY_NOT_ENABLED``. It got ``STATUS_READ_FAILED``, because
``core/reporting-obligation.json``'s ``allOf[4]`` fires on the CONJUNCTION
``if: {properties: {health: {enum: [healthy, complete]}}, required: [health,
destination_ref]}`` and then requires ``materialization_count >= 1``,
``successful_materialization_count >= 1`` and ``resource_retained_until``. The
seller's payload was invalid against the pin, ``load_reporting_ledger``
refused it first, and the tier check under test never ran -- so the assertion
graded the read, not the tier.

Supplying those counts was tried and reverted, correctly: a Core ledger HAS no
materializations, so a fixture reporting one asserts something false about the
very tier under test. The pin says so in ``destination_ref``'s own
description -- "Core (API-delivered) obligations omit every destination and
materialization field".

The schema-valid, Core-legal state is to falsify ``allOf[4]``'s HEALTH
conjunct instead. The obligation is now ``delayed``: the source is left
unscripted, the obligation commits when the period closes, no revision exists,
and the read boundary sits after ``expected_at`` but inside the declared
automated recovery window -- step 8 of this module's own lifecycle narrative.
Nothing is fabricated. No materialization field is required or supplied,
``allOf[8]`` wants ``issues`` non-empty and an overdue period already carries
``REPORT_OVERDUE``, and ``destination_ref`` is left as the ONE thing in the
payload the pin forbids for Core.

Validated against the PIN rather than against this branch's compiler --
``jsonschema`` draft-07 over ``schemas/cache/3.2/`` with the whole cache as a
``referencing`` registry:

    delayed + destination_ref     PIN-VALID    <- the new fixture
    healthy + destination_ref     PIN-INVALID  <- the old fixture, failing on
                                                 exactly allOf[4]'s three
                                                 required materialization keys
    delayed, no destination_ref   PIN-VALID    <- control

Two guards against a fixture that passes for the wrong reason: an assertion
that the read boundary is inside the recovery window, so the obligation cannot
drift to ``action_required`` unnoticed, and a reconcile of the SAME ledger
through the unmodified handler, which must succeed and report
``destination_ref=None`` with ``health='delayed'``. Without the second, a
fixture that broke the read for any unrelated reason would still raise and
still be graded green. Removing the ``destination_ref`` injection now fails
with DID NOT RAISE -- verified by doing it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant