Repository navigation
Conversation
A JSON Schema whose root is a scalar describes a value, not an object. datamodel-code-generator cannot express that and emits a RootModel wrapper, which is not a str anywhere Python treats one: str renders root='sports', equality against the wrapped string is False, and hashing raises TypeError. That last one escaped the typed validation path on geo include/exclude overlap and surfaced to buyers as INTERNAL_ERROR instead of a correctable INVALID_REQUEST. rewrite_scalar_rootmodels rewrites every scalar root to a str, int, or float subclass, so the fix covers all 224 in the generated tree and anything generated later rather than the 18 that happen to be exported. It runs last among the substantive fixers, because every earlier fixer can still emit or edit a RootModel wrapper around a scalar. The condition is checked on the generated AST, where refs are already resolved, and lives at the point of decision in _rootmodel_scalar and _scalar_root_spec. Roots that compose - unions, arrays, refs to objects, AnyUrl, date-times, Literal, dict - keep their RootModel, which is correct for them. bool roots keep it too; Python forbids subclassing bool. A scalar root the rewriter does not recognize is reported rather than rewritten. The bases carry the schema's constraints in a dict rather than a single hardcoded pattern, so the same mechanism serves minLength, maxLength and the numeric bounds, and serves int and float roots without a second mechanism. They also carry title, description and examples, which a naive rewrite would have dropped from the emitted JSON Schema. The rewrite happens in generated_poc itself, not in aliases.py: generated modules import these names directly from generated_poc, so a redefinition one layer up would leave two classes behind one name. 8.0.0 is one day old and has no known adopters, so the remedy for the break below is a clean major with 8.0.0 deprecated, not a compatibility layer that carries a defective public surface forward. Fixes adcontextprotocol#1277 BREAKING CHANGE: the 224 generated types whose JSON Schema root is a scalar, 18 of them exported from adcp, are str/int/float subclasses instead of RootModel subclasses. Two consequences for callers. issubclass(X, RootModel) is now False for those types, so an isinstance or issubclass check written against RootModel no longer matches them. X.model_dump, X.model_validate and X.model_json_schema no longer exist on them, because they are no longer Pydantic models. TypeAdapter is the replacement, and is the correct API for a non-model. What does NOT break: .root reads and X(root=...) construction both still work behind a DeprecationWarning, so no existing call site has to change. Values validate, serialize and compare exactly as before, and now also behave as the scalar the schema declares - including hashing, which previously raised. Migration: - use the value as a str, int or float directly - it already is one - replace x.root with x - replace X.model_dump(x) with TypeAdapter(X).dump_python(x), and likewise validate_python and json_schema
Output only, from scripts/generate_types.py against the pinned 3.2 bundle. generate_types.py --check passes, so the tree is exactly what a clean regeneration produces. RootModel[str] 189 -> 0, RootModel[int] 21 -> 0, RootModel[float] 14 -> 0, replaced by 224 scalar subclasses. The 241 RootModel classes that remain are the compositions: 142 unions, 62 arrays, 12 single refs to an object, 9 AnyUrl, 5 Literal, 5 Any, 4 dict, 2 AwareDatetime. Two uniqueness validators change with it. selected_product_ids and media_buy_ids pulled .root off each element because the RootModel was unhashable; both are now set over the values directly, matching the accepted_losses validator beside the first one. The rest of the diff is the regeneration timestamps of the files that changed and the regenerated SCHEMA_DELTAS.md report. Refs adcontextprotocol#1277
.root still works on a scalar schema behind a DeprecationWarning, which is what keeps every existing call site compiling. But the SDK was one of those call sites, so the library warned about its own internals on paths an adopter cannot fix: 14,701 warnings in one suite run against 554 at baseline. The hasattr and getattr probes for a root attribute are now isinstance checks against RootModel, which is what they always meant, and the reads on types that are now scalars are plain str calls so values and the error-message reprs are byte-identical. Found by reading the warning origins from a suite run rather than by grepping, which is how the getattr spelling in compat, reporting._consumer and reporting._reconcile turned up at all. media_buy_actions normalizes through plain_scalar: its _string_value and _normalized_value helpers fall into the non-RootModel branch now, and without the normalization they would hand a caller the subclass where the annotation promises a plain str. Measured identical to the pre-change RootModel path for both a string and a float root. _annotation_contains and _patch_equivalent_model_field take type instead of type[BaseModel]: they match a bundled clone by class identity, and two of those clones are now str subclasses. Suite warnings are back to 670, with no origin under src. Refs adcontextprotocol#1277
tests/test_scalar_roots.py holds the behaviour the wrappers lacked: is-a-str, equality against a bare str, hashing and set membership, the geo include/exclude intersection that produced INTERNAL_ERROR, construction validation for pattern and numeric bounds, wrong-scalar-type rejection, no-arg TypeError, model-field round trip and instance identity, attribute closure, copy and pickle, the JSON Schema compared against the vendored core/property-tag.json, and both deprecation paths. Two sweeps run over the whole exported scalar population rather than a sample: one asserts each is its Python scalar with no unrecognized schema keyword, the other constructs every examples value the schemas supply and checks it compares equal to the bare scalar. Three guards keep the fix from eroding. test_no_scalar_rootmodels_remain_in_ generated_tree fails on any leftover scalar RootModel, which is what turns a rewriter skip into a test failure instead of a quietly restored broken wrapper; verified by reintroducing one and watching it go red. test_composing_roots_keep_their_rootmodel pins the other half of the condition. test_every_scalar_root_has_exactly_one_class_behind_it holds the split-identity line: every public surface must expose the same class object that generated_poc defines. Four existing tests asserted the old shape and now assert the new contract rather than being deleted: the two issubclass assertions in test_type_aliases, the TmpxMacro model_fields check in test_collision_aliases, and the wire-schema comparison in test_composability_invariant, which now goes through TypeAdapter because a scalar schema has no model_json_schema. test_decisioning_property_list stubbed PropertyId as a MagicMock carrying a root attribute and now uses the real type. Refs adcontextprotocol#1277
unwrap_rootmodel_unions decided which RootModel wrappers to remove by
looking the class name up in _UNWRAP_TO_UNION, a hand-maintained set of 43
names. It now derives the decision from the root and carries the
generator's own metadata across.
A union of models becomes a union type alias annotated with the Field the
generator already emitted, so the description, the title and -- where the
schema declares one -- the discriminator survive the unwrap. The name-driven
unwrap dropped all three, and four of its 43 names came from
discriminator-declaring schemas.
A single-model root becomes a subclass of that model, which keeps the name
constructible and moves the root description to the class docstring.
A value root keeps its wrapper: str, AnyUrl, list[X], Literal[...] and
dict[str, Any] carry their constraints there and nowhere else. So do two
more, and the rule says why rather than naming them:
- a wrapper another union discriminates against. AssetVariant
discriminates 21 arms on asset_type and VastAsset, DaastAsset,
DisplayTagAsset and PublishedPostAsset each group shapes that share one
asset_type, so inlining one gives the parent two choices for one tag
and Pydantic refuses the module at import time.
- a wrapper another union in the same module names as an arm.
AdcpAgentsAuthorization2 groups 14 catalog shapes under one choice of
AdcpAgentsAuthorization; inlining it makes those shapes siblings of the
other arm and Pydantic then selects a different one.
expose_account_reference_union_fields is deleted. It rewrote every
annotation naming account_ref.AccountReference into its two arms; the rule
emits AccountReference AS those arms, so the annotations already name them.
Three fixes that inject onto a wrapper -- the two postal ones and the
adagents renderer constraint -- move ahead of this pass, so the predicate
sees the injected validator and keeps those wrappers with no name needed.
_class_end_offset reads a class's extent from the AST where two injection
sites used to assume the body ran up to the next class; a module now holds
type aliases between its classes. _generated_class_name resolves a module
alias as well as a class, because a composing root is now an alias and that
is the name a field references. remove_unused_pydantic_field_imports drops
an unused RootModel too, and this pass edits no imports at all, because
fixes between it and the end of the pipeline match exact generated import
lines.
BREAKING CHANGE: 114 generated classes stop being RootModel subclasses -- 107
union roots and 7 single-model roots. A caller that reaches a wrapper's inner
model through `.root`, or calls `TypeName.model_validate(...)` on a union root,
changes at those sites: use `TypeAdapter(TypeName).validate_python(payload)`,
which honors the discriminator, and read the inner value directly instead of
through `.root`. A single-model root keeps `TypeName.model_validate(...)`.
134 generated classes stop being RootModel subclasses: 127 union roots become union type aliases and 7 single-model roots become subclasses of their target. 331 remain, all of them value roots or wrappers another union needs.
Each of these read or validated through a wrapper the generator no longer emits. reporting_inspection, server/principal and webhook_sender hold a module-level TypeAdapter for the union they validate, which is this repo's existing idiom for an unwrapped union and honors the discriminator that X.model_validate on a union never could. decisioning/refine appends the arm it built instead of wrapping it. utils/preview_cache reads the render directly. feed_mirror's _apply_event binds the payload as Any -- the branches dispatch on an event_type string rather than the union's discriminator, so the payload was never typed there; the wrapper's __getattr__ supplied the Any before. utils/response_parser peels Annotated metadata before asking whether a response type is a union, so a published union root reaches the arm loop. types/_forward_compat rebuilds only McpWebhookPayload; that rebuild re-resolves AdcpAsyncResponseData, which is a union type alias. types/error_narrowing recognizes the asset-variant arm by its inline spelling (tagged-union[ImageAsset,...]) as well as by the wrapper class name, and returns the focused set when no variant name was matched -- falling back to the input there put the dropped sibling arm back. The module's own docstring named this fix: "Schema-level fix (Annotated[Union[...], Field(discriminator=...)]) avoids the issue entirely".
tests/test_code_generation.py grades the rule on one fixture per branch: a discriminated union root becomes an annotated alias with the Field intact, a value root keeps its wrapper, an arm of a discriminated union keeps its wrapper, a single-model root becomes a subclass carrying its description, and a root with a validator keeps its wrapper. test_discriminated_union_roots_report_one_tag_error asserts on the shipped models: Deployment and ListProductsResponse each report exactly one union_tag_invalid for an unknown tag, where ListProductsResponse reported five errors across both arms before. tests/type_checks/composing_root_subclassing.py is the adopter contract under mypy --strict with no type: ignore -- subclass an arm, subclass a single-model root, narrow Signal.deployments to a seller's deployment type, and validate a union root through a TypeAdapter. The narrowing is the line that fails against 8.0.0. The rest migrate call sites: a union validates through TypeAdapter, and an element of an unwrapped union is the arm rather than a wrapper. test_account_reference_request_fields looks for a RootModel subclass anywhere in the annotation instead of one class by name, which is the obligation the test was written for.
generate_types.py preserves the committed report when a regeneration is a no-op (DELTAS_FILE is left alone when before == after), so the file carried the delta of an intermediate regeneration and a clean re-run never corrected it. Rebuilt with diff_generated_types.format_diff over a snapshot of generated_poc at 6677169 against the tree this branch ships. The six entries under "Files removed" still exist on disk. The tool reports a file when no class in it declares a field, and each of those six now declares only the union type alias its wrapper became.
A `$ref` whose fragment is a JSON pointer into another file's `properties`
selects that property's type. datamodel-code-generator has no name for a
structural position, so it emitted a single-arm `RootModel` wrapper per
referencing site and typed the field with the wrapper.
`core/targeting-input.json` declares 27 of its 37 fields as "the type
`core/targeting.json` gives this property, or null" and hit the defect twice
over: the pointer hid the arm's `type` keyword, and a two-arm `anyOf` whose
non-null arm declares a `type` gets a wrapper of its own.
`TargetingOverlayInput` and `TargetingOverlay` ended up disagreeing on 27 of
their 37 field types, and reading a wrapped collection field returned
something no consumer can use: indexing and `len()` raise, and iterating
yields the `('root', [...])` field pair instead of the elements.
The wrapper also reaches the wire. Every MCP `inputSchema` comes from
`model_json_schema()`, and a wrapper carries the field's `description` onto
its own definition, so `create_media_buy` advertised 5 of 37 targeting
fields documented.
Resolve the pointer into the selected subschema before generation, and spell
"S or null" on a property as a nullable type list rather than a two-arm union.
Both run on the temporary schema tree; the published cache is unchanged.
`TargetingOverlayInput` now agrees with `TargetingOverlay` on 27 of the 37
fields instead of 10, with no per-position `RootModel` wrapper on either side,
and the advertised schema documents 28 of 37. `ProductPurchaseInput` goes from
19 of its 22 shared fields disagreeing with `ProductPurchase` to 2.
`CatalogSelection` gains six fields as plain types. The remaining 10
differences on the overlay are element classes that each module defines for
itself, plus one `list`-against-`Sequence` asymmetry, neither caused by the
pointer.
Eleven generated element classes are now defined in two modules each and are
identical in both, so `collision_allowlist.json` records them (net +11, -1).
BREAKING CHANGE: resolving a pointer ref to its target removes the intermediate
class the pointer previously generated, so `adcp.types._generated.__all__` goes
3010 -> 2964: 49 names removed, 3 added. The removed names are artifacts of the
defect — an import of `GeoCountries` was an import of a per-module `RootModel`
wrapper that existed only because the pointer went unresolved — and the
replacement is the shared type the field now points at. Four rules cover all
49: a wrapper over a pointed-to position becomes the type the pointer selects
(41 names); a numbered duplicate becomes the un-suffixed class in the same
module (`DoohMetrics1`, `Geometry3`, `GeoProximityItem2`, `Viewability1`,
`QualifierModel`, and `Qualifier3` -> `Qualifier5`); `Field0` was a
generator-invented base whose 11 fields are now declared on
`DeliveryMetricAggregate1` directly; and `Ext` loses only its export slot, the
class surviving as `brand_discovery.Ext` and `core.assets.asset_union.Ext`
with the fields that used it now carrying `core.ext.ExtensionObject`. The PR
body carries a per-name replacement table.
Added: ComplianceTaskCompletionData1, Fields1, Qualifier5.
Removed: AdcpMajorVersion, AdcpVersion, AgencyEstimateNumber, AudienceEvidencePins, AudienceExclude, AudienceInclude, AxeExcludeSegment, AxeIncludeSegment, Bidding, Browser, BrowserExclude, CatalogIds, CollectionListExclude, Context, DailyBudgetCap, DaypartTargets, DevicePlatformExclude, DeviceTypeExclude, DoohMetrics1, EndTime, Ext, Field0, FormatOptionRefs, GeoCountries, GeoCountriesExclude, GeoMetrosExclude, GeoPlaces, GeoPlacesExclude, GeoPostalAreas, GeoPostalAreasExclude, GeoProximityItem2, GeoRegions, GeoRegionsExclude, Geometry3, Gtins, Ids, MinSpendTarget, ObservedDocumentVastVersion, OptimizationGoals, PerformanceStandards, Pricing, PricingOptionId, PropertyListExclude, Qualifier3, QualifierModel, Query, StartTime, StoreCatchments, Viewability1.
No deprecated aliases are provided: keeping them alive preserves, in the public
namespace, names that no longer correspond to anything the schema declares.
…tors Resolved src/adcp/webhook_sender.py: the import line conflicted where main added BaseModel and the branch added TypeAdapter. Both are needed, so the line imports both.
|
Ladon cannot review this PR until merge conflicts are resolved. |
pm25coder
reviewed
Oct 3, 2026
pm25coder
left a comment
pm25coder
left a comment
There was a problem hiding this comment.
Read this against main — the two normalizations and the before/after counts make the mechanism easy to follow. One note on the residuals you classify as regressions: they look like a single class, and it is narrower than "every structural pointer".
Both are pointers whose target the generator already has a shared name for:
media-buy/product-fields.json#/items.product-fields.jsonis a root array, soproduct_fields.pyalready declaresProductResponseField(the 38-member enum). The diff dropsfrom . import product_fieldsand re-declares those 38 members locally inget_products_request.py(class Fields) — a copy where an import existed.core/targeting.json#/properties/age_restriction. That property is an object, sotargeting.pyalready declaresAgeRestriction, andTargetingOverlay.age_restrictionisAgeRestriction | None(checked atmain@56fefd2). Inlining givestargeting_input.pyits own copy.
The positions that need inlining are the ones the generator has no name for, and those are exactly the ones it wraps in a single-arm RootModel: GeoCountries(RootModel[list[GeoCountry]]), DevicePlatform, AxeIncludeSegment(RootModel[str]), Demographics. For an object or a bare enum it does have a name, and the pointer already resolved to it.
So the predicate that removes both regressions — without a position-keyed special case — is on the selected subschema rather than the pointer token: inline when the target is one the generator wraps (type: array, a constrained scalar, or a bare $ref node), leave it when the target is a named object or enum. That is the same criterion the docstring states ("mints a single-arm RootModel ... instead of the type the pointer selects"), applied to what the generator wraps rather than to every token outside $defs. It also lines up with the issue's criterion 2, which scopes the required agreement to targets that are a whole-file $ref, an enum, or a plain scalar/array — those either already have a shared name or need none.
The nine residual element-class duplications stay either way, since those targets are wrapped positions whose inner class is anonymous.
Worth narrowing the rule, or is the uniform inline the deliberate trade?
The eight media_buy response modules conflicted on the generator's timestamp comment alone. Their bodies merged cleanly: each carries adcontextprotocol#1351's `replayed: Literal[True] | None = None` alongside this branch's composing-root `Annotated[...]` alias. Re-running the generation pipeline over the merged tree produced no further change, so the merge result is the regenerated result.
refine_proposals_response.py and request_proposals_response.py conflicted on the generator's timestamp comment alone; their bodies merged cleanly, carrying adcontextprotocol#1351's `replayed: Literal[True] | None = None` alongside this branch's resolved pointer refs. Re-running the generation pipeline over the merged tree produced no further change, and __all__ still measures 2964.
One real conflict in scripts/post_generate_fixes.py: both sides appended a numbered item 8 to the module docstring's list of passes. Resolved as the union, numbered in execution order -- 8 is adcontextprotocol#1351's optional boolean const pass, which main registers after fix_unchanged_literal_defaults, and 9 is this branch's scalar RootModel rewrite, which stays last because every earlier fixer may still emit a RootModel[<scalar>] for it to consume. Both functions and both registrations merged cleanly outside the conflict; the two passes do not interact. The three media_buy response modules conflicted on the generator's timestamp comment alone. Re-running the generation pipeline over the merged tree produced no further change, and the output carries both intents: zero stale 'replayed: Literal[True] = True' anywhere in src/, and 679 scalar-subclass exports that are not RootModel subclasses.
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean codegen fix with no blocking or medium findings.
This PR (fix(codegen)!) resolves structural pointer $refs to the type they select. The reviewer found no Critical/High/Medium issues:
- The two new normalizations in
scripts/generate_types.py(the largest non-generated file) are cycle-terminating, correctly scoped, and use safe immutable defaults. - The 49 public
__all__removals are correctly flagged breaking with a full migration table and reach no live import; the breaking-change semver signal is satisfied (fix!:+ migration note). - Aliases / forward-compat layering holds; generated output is reproducible regeneration (not a hand-edit); CI gates stay armed.
- Self-disclosed residuals are runtime-equivalent and immaterial.
Decision-table walk: no critical/high (row 1 no); gated_paths: false (row 2 no); high_risk: false (rows 3/5 no); no medium findings (rows 4/8 no); no prior decision (row 6 no); no no-auto-approve team match (row 7 no). Falls through to row 9 → approve.
Note: review_decision is REVIEW_REQUIRED, but since gated_paths is false, row 2 does not fire and this is not an approval blocker.
# Conflicts: # SCHEMA_DELTAS.md # scripts/post_generate_fixes.py # src/adcp/types/generated_poc/account/list_account_changes_request.py # src/adcp/types/generated_poc/adagents.py # src/adcp/types/generated_poc/brand/acquire_rights_request.py # src/adcp/types/generated_poc/brand/verify_brand_claims_request.py # src/adcp/types/generated_poc/brand_discovery.py # src/adcp/types/generated_poc/bundled/protocol/get_adcp_capabilities_response.py # src/adcp/types/generated_poc/compliance/comply_test_controller_response.py # src/adcp/types/generated_poc/core/account_identity_change_preview.py # src/adcp/types/generated_poc/core/assets/asset_union.py # src/adcp/types/generated_poc/core/canonical_product.py # src/adcp/types/generated_poc/core/data_provider_signal_selector.py # src/adcp/types/generated_poc/core/macro_bearing_url.py # src/adcp/types/generated_poc/core/product.py # src/adcp/types/generated_poc/core/product_execution_requirement.py # src/adcp/types/generated_poc/core/product_filters.py # src/adcp/types/generated_poc/core/publisher_property_selector.py # src/adcp/types/generated_poc/core/registry_event.py # src/adcp/types/generated_poc/core/targeting.py # src/adcp/types/generated_poc/core/targeting_overlay_support.py # src/adcp/types/generated_poc/core/transformer.py # src/adcp/types/generated_poc/core/vendor_pricing_option.py # src/adcp/types/generated_poc/creative/list_transformers_request.py # src/adcp/types/generated_poc/creative/sync_creatives_request.py # src/adcp/types/generated_poc/error_details/vast_version_mismatch.py # src/adcp/types/generated_poc/governance/reported_outcome_error.py # src/adcp/types/generated_poc/media_buy/acceptance_policy_requirement.py # src/adcp/types/generated_poc/media_buy/build_creative_request.py # src/adcp/types/generated_poc/media_buy/get_products_response.py # src/adcp/types/generated_poc/media_buy/legacy_purchase_continuation_input.py # src/adcp/types/generated_poc/media_buy/product_purchase.py # src/adcp/types/generated_poc/media_buy/sync_audiences_request.py # src/adcp/types/generated_poc/media_buy/sync_event_sources_request.py # src/adcp/types/generated_poc/protocol/get_adcp_capabilities_response.py # src/adcp/types/generated_poc/signals/get_signals_request.py # src/adcp/types/generated_poc/trusted_match/provider_registration.py
# Conflicts: # SCHEMA_DELTAS.md # src/adcp/types/generated_poc/compliance/comply_test_controller_request.py # src/adcp/types/generated_poc/compliance/task_completion_data.py # src/adcp/types/generated_poc/core/catalog.py # src/adcp/types/generated_poc/core/committed_metric.py # src/adcp/types/generated_poc/core/delivery_metric_aggregate.py # src/adcp/types/generated_poc/core/product_offer_filters.py # src/adcp/types/generated_poc/core/targeting.py # src/adcp/types/generated_poc/core/version_envelope.py # src/adcp/types/generated_poc/error_details/vast_version_mismatch.py # src/adcp/types/generated_poc/media_buy/get_products_request.py # src/adcp/types/generated_poc/media_buy/product_purchase.py
…ries no field description
…port main's freezegun contract (adcontextprotocol#1339) forbids a module-level TypeAdapter: the cold-start scenario resolves lazy exports with datetime patched, and webhook_sender's adapter was built in that window.
# Conflicts: # docs/extending-types.md
adcontextprotocol#1341's ratchet reads a union root's arms with typing.get_args; a composing root is now Annotated[A | B, Field(...)] and the first arg is the union.
# Conflicts: # SCHEMA_DELTAS.md # src/adcp/types/generated_poc/account/list_account_changes_request.py # src/adcp/types/generated_poc/adagents.py # src/adcp/types/generated_poc/brand/acquire_rights_request.py # src/adcp/types/generated_poc/brand/verify_brand_claims_request.py # src/adcp/types/generated_poc/brand_discovery.py # src/adcp/types/generated_poc/bundled/protocol/get_adcp_capabilities_response.py # src/adcp/types/generated_poc/compliance/comply_test_controller_response.py # src/adcp/types/generated_poc/core/account_identity_change_preview.py # src/adcp/types/generated_poc/core/assets/asset_union.py # src/adcp/types/generated_poc/core/canonical_product.py # src/adcp/types/generated_poc/core/data_provider_signal_selector.py # src/adcp/types/generated_poc/core/macro_bearing_url.py # src/adcp/types/generated_poc/core/product.py # src/adcp/types/generated_poc/core/product_execution_requirement.py # src/adcp/types/generated_poc/core/product_filters.py # src/adcp/types/generated_poc/core/publisher_property_selector.py # src/adcp/types/generated_poc/core/registry_event.py # src/adcp/types/generated_poc/core/targeting.py # src/adcp/types/generated_poc/core/targeting_overlay_support.py # src/adcp/types/generated_poc/core/transformer.py # src/adcp/types/generated_poc/core/vendor_pricing_option.py # src/adcp/types/generated_poc/creative/list_transformers_request.py # src/adcp/types/generated_poc/creative/sync_creatives_request.py # src/adcp/types/generated_poc/error_details/vast_version_mismatch.py # src/adcp/types/generated_poc/governance/reported_outcome_error.py # src/adcp/types/generated_poc/media_buy/acceptance_policy_requirement.py # src/adcp/types/generated_poc/media_buy/build_creative_request.py # src/adcp/types/generated_poc/media_buy/get_products_response.py # src/adcp/types/generated_poc/media_buy/legacy_purchase_continuation_input.py # src/adcp/types/generated_poc/media_buy/product_purchase.py # src/adcp/types/generated_poc/media_buy/refine_proposals_response.py # src/adcp/types/generated_poc/media_buy/sync_audiences_request.py # src/adcp/types/generated_poc/media_buy/sync_event_sources_request.py # src/adcp/types/generated_poc/protocol/get_adcp_capabilities_response.py # src/adcp/types/generated_poc/signals/get_signals_request.py # src/adcp/types/generated_poc/trusted_match/provider_registration.py
# Conflicts: # SCHEMA_DELTAS.md
…ld union names it as an arm The same-module smart-union guard did not see a cross-module reference: a oneOf root another module's field names in a non-discriminated union (assets: LocalizedCreativeAsset | Assets) was unwrapped, and the field's union then scored the root's arms against its siblings directly, which can select a different arm for the same payload with no error. The guard now reads every module's field unions (two or more non-None arms). It skips a wrapper's own root annotation and union roots that compose other roots — async-response-data flattened its response unions before this pass existed, and re-wrapping them would take away unions adopters already use — and it exempts discriminated roots, which a nested union selects by tag rather than by score. On the pinned bundle two roots keep their wrapper as on main: LocalizedCreativeAsset and PlacementIdentity.
# Conflicts: # SCHEMA_DELTAS.md
# Conflicts: # SCHEMA_DELTAS.md # src/adcp/types/generated_poc/core/targeting.py # src/adcp/types/generated_poc/error_details/vast_version_mismatch.py # src/adcp/types/generated_poc/media_buy/product_purchase.py
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
The reviewer returned a clean pass with no critical/high/medium findings. Walking the decision table top-down:
- Row 1 (critical/high): no findings — skip.
- Row 2 (gated_paths + not APPROVED): gated_paths is false — skip.
- Row 3 (deleted high-risk): high_risk is false — skip.
- Row 4 (medium data-loss/schema/infra): no findings — skip.
- Row 5 (modified high-risk + medium): high_risk false — skip.
- Row 6 (sticky escalate): prior decision was approve — skip.
- Row 7 (no-auto-approve team): no team match — skip.
- Row 8 (3+ medium): zero findings — skip.
- Row 9: approve.
The substantive non-generated change is the _names_a_class guard in scripts/generate_types.py, covered by test_inline_structural_pointer_ref_leaves_a_class_shaped_selection_alone. The breaking surface carries the required fix(codegen)!: + BREAKING CHANGE semver signal with a migration table (SCHEMA_DELTAS.md), satisfying the mandatory public-surface semver rule. generated_poc/_generated.py/collision_allowlist are consistent regeneration output; import-layering, forward-compat, and credential handling are untouched. review_decision is REVIEW_REQUIRED but gated_paths is false, so the hard approval gate does not apply.
Leaving object and enum pointer selections to the generator keeps the shared DoohMetrics, VenueBreakdownItem, ViewedSecondsHistogramItem and ViewedSecondsPercentiles classes in one module, so they no longer collide.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
Clean pass. This is a subsequent review following a prior approve at head f15917e. The delta since that approval is a single file — scripts/collision_allowlist.json — which shrinks by four names (DoohMetrics, VenueBreakdownItem, ViewedSecondsHistogramItem, ViewedSecondsPercentiles) downstream of the PR's structural-pointer-ref resolution giving each type a single generated home. The shrink is fail-closed: test_allowlist_matches_current_collisions_exactly asserts the allowlist equals the real collision set, so a wrongly-shrunk allowlist fails CI rather than silently shadowing a class for adopters.
No critical/high/medium findings in this run. The remaining diff is regeneration output under src/adcp/types/generated_poc/** and _generated.py paired with the codegen script and schema-delta docs — consistent with the "generated code is not source" workflow (regeneration + allowlist/tests), not a hand-edit.
Gate checks:
- No no-auto-approve team match (HARD RULE not triggered).
gated_paths: false— row 2 does not fire even thoughreview_decisionis REVIEW_REQUIRED.high_risk: false— no deleted/modified high-risk trigger.- Prior decision was
approve, notescalate— row 6 n/a. - Zero medium findings — rows 4, 5, 8 n/a.
Rows 1–8 all pass through; row 9 applies → approve.
bokelley
pushed a commit
that referenced
this pull request
Oct 4, 2026
…hema (#1375) fix(validation)!: make the generated models agree with the bundled schema Addresses #1359. The differences that remain are enumerated below with their causes, and each is a decision rather than an omission. The SDK ships two validators per tool — the bundled JSON Schema reached through `adcp.validation`, and the generated request model in `adcp.types` — and they do not accept the same documents. Measured on the pinned `3.2` bundle across 39546 documents (a schema-valid payload per tool, one leaf at a time replaced with a value of a different JSON type), **5995 diverge**, and **17 of the 72 request models refuse a payload the bundled schema declares valid**. This closes the two causes that account for most of it, adds the suite that keeps them closed, and names the differences that remain. ## 1. The bundled validator now enforces every format the bundle uses `schema_loader.py` built each validator with a bare `FormatChecker()` plus one hand-registered check for `date-time`. A bare `FormatChecker()` carries only the checks whose optional dependencies import, and `uri` needs `rfc3987`, which this package does not depend on. `format: uri` is the most-used format in the 3.2 bundle — 8580 occurrences across its schema files against `date-time`'s 5552 — and not one occurrence was checked. `format: hostname` (35 occurrences) was not checked either. Both are formats the generated models enforce, so each was a divergence, and validation strength varied with whatever the adopter happened to have installed. `_build_format_checker()` is now the one builder every validator in the module uses, and it supplies `uri` by delegating to `TypeAdapter(AnyUrl)` — the same parser the generated models apply to a `format: uri` field, so both sides consult one predicate instead of two — plus `hostname` carrying the RFC 1123 grammar the generator emits as a `pattern`. The two construction sites that each hand-registered `date-time` separately now share it. Effect: divergences in the schema-accepts direction fall from 4604 to 1112 documents, and models refusing a schema-valid payload fall from 17 tools to 5. ## 2. The generated models use strict scalars JSON Schema types are exact: `type: boolean` admits `true` and `false`. Pydantic's lax mode is wider for three of them, measured against 2.13: ``` bool accepts 1, 0, "1", "0", "yes", "true" int accepts "1", True, 1.0 float accepts "1", True str coerces nothing ``` So `ListAccountsRequest.model_validate({"sandbox": "yes"}).sandbox` was `True` while the bundled schema for the same tool refused the payload — 641 divergent documents over 158 field sites in the boolean, integer and number classes. Whether a call was a dry run depended on which of the SDK's two validators the receiving side consulted. `scripts/generate_types.py` now passes `--strict-types bool int float`, so the generator derives the annotation from each schema's own declared type. `str` is deliberately excluded: it changes no verdict, and it perturbs `--reuse-model` dedup enough to expand `core/audience_evidence.py`'s subject field into a union of three `RootModel` arms that all carry the same discriminator values, which pydantic rejects outright. `AwareDatetime`, `AnyUrl`, `UUID` and `date` keep lax parsing — JSON carries those as strings and the string is the wire form. `StrictInt` alone overshoots. JSON Schema counts a number with a zero fractional part as an integer, so the bundled validator accepts `1.0` for an integer field and rejects `1.5`, while `StrictInt` rejects both — and the SDK's own reporting code passes integral floats into integer fields, which the schema permits. `SchemaInt` in `adcp.types.base` is `StrictInt` plus that narrowing: ```python SchemaInt = Annotated[StrictInt, BeforeValidator(_as_json_schema_integer)] ``` ``` 1 -> ACCEPT 1 1.5 -> refuse (int_type) 1.0 -> ACCEPT 1 True -> refuse (int_type) '1' -> refuse (int_type) ``` `point_integer_fields_at_the_schema_integer_type` rewrites the generator's `StrictInt` marker to it across 249 files. The derivation still comes from the schema — `--strict-types int` is what marks an integer field in the first place. Effect, over the 73 tools graded once the format fix lands: boolean, integer and number divergences fall from 1291 documents to 225, and the residual in those three classes is explicit `null` — 222 of the 225 — which is a separate cause (below). Strict scalars are `Annotated[bool, Strict()]` and friends, so every attribute keeps its declared type. `tests/type_checks/` passes unchanged under `mypy --strict` with no new suppressions. **Breaking:** a caller that relied on the coercion now gets a `ValidationError`. That is the point — the model's accepted set is the schema's accepted set. ## 3. `tests/conformance/test_schema_model_parity.py` `tests/test_validation_parity.py` already grades this boundary from `validation-parity-cases.json`: 8 cases, all on `get_signals`, request direction only, with 3 allowlisted divergences. That is 1 of 84 tools, which is why the figures above went unseen. This suite is the same idea driven off the bundle instead of a fixture list, and the two are complementary rather than overlapping — `get_signals` is one of the six tools whose schema this instantiator cannot satisfy, and the three divergences those fixtures allowlist are all conditionals, matching this suite's `conditional_subschema_is_not_generated` entry. The suite enumerates the bundle, so a schema added by a version bump is graded without an edit here. Per tool it instantiates the request schema, grows the payload with every optional property that keeps it valid, replaces each leaf with each of 18 values, and requires the two validators to return the same verdict. Differences the SDK still has are named in `_schema_parity.DECLARED`, each with the reason it is there. A divergence matching none of them fails the suite; a declared entry that stops matching fails it too, so the set only shrinks. Two companion sets work the same way: `UNREACHABLE` (six tools whose `required` sets sit inside composition branches the instantiator cannot satisfy) and `UNMODELLED` (six tools that ship a request schema and expose no request model). None of the three is keyed on a count. `86 passed`, about half a minute. The suite goes red when either defect returns: reverting `src/adcp/types/generated_poc` to its pre-change state gives `67 failed, 19 passed`; removing the `uri` and `hostname` checkers gives `28 failed, 58 passed`; replacing `SchemaInt` with a bare `StrictInt` goes red on the integral float at every integer field in the bundle. On its first run it caught a case not anticipated — `comply_test_controller/params/viewability/measurable_impressions` refused through an `if`/`then` subschema — which is now the `conditional_subschema_is_not_generated` entry. The rest of the suite is unchanged: `19 failed, 12811 passed` against the base commit's `19 failed, 12725 passed`, the same 19 node-ids, and the 86 added passes are this suite's own tests. ## 4. Two repairs the regeneration required Both are in the generator pipeline and both remove a spelling rather than adding one. `add_to_import()` and `drop_from_imports()` replace four separate implementations of "add a name to the `from pydantic import` line", three of which corrupted a parenthesized import. Strict annotations lengthen that line past the formatter's width, so `adagents.py` came out as `from pydantic import (, model_validator` and the next pass died in `ast.parse`. The helpers read whichever names are there, keep the statement's form, and raise when there is no import to rewrite. Four exact-literal `.replace()` sites that silently matched nothing now go through them too. `drop_from_imports` visits every matching statement rather than the first: a generated module can carry more than one import from the same module — the passes that inject a validator or serializer add their own — and stopping at the first renames a symbol the second still imports. `remove_imports_shadowed_by_a_local_class()` drops an import whose name a module-level class in the same module redefines. `--strict-types float` made `core/wholesale_feed_event.py` stop reusing `signal_listing.Range` — the two differ only in field descriptions — so the generator emitted the class locally and kept the import, binding the name twice and tripping mypy's `no-redef`. The pass is AST-driven with no name list; an AST scan of the tree found exactly one module in this state. ## What an adopter can do now Validate a request with either of the SDK's two validators and get the same answer for boolean, integer and number fields, and for every `format: uri` and `format: hostname` field in the protocol — rather than one answer from the client hook and a different one from the model the server parses with. And see the rest: the differences that remain are enumerated with their causes instead of being discovered one payload at a time. ## What remains, and why it is not here | class | documents | why | |---|---|---| | an optional field accepts explicit `null` | 948 | the generator renders an optional property as `X \| None`, conflating absent with null. The per-field answer already exists — `_schema_admits_null` in `types/versioned.py:216`, used to build `omit_none_fields` for serialization — and carrying it to the input path is a decision about whether AdCP intends `null` to be accepted. If it does, the schemas declare `["string","null"]` and the models are already right | | a `date-time` or `date` field reads a number as a Unix timestamp | 616 | no generator flag expresses it; needs a constrained annotation in the runtime | | a `format: uri` field drops a sibling `pattern` | not in this corpus | pydantic cannot apply a string constraint to `AnyUrl`; needs a constrained-URI type in the runtime | | the canonical model refuses a legacy creative identity the bundled schema retains | 1077 (4 tools) | `core/format-id.json` is deprecated and retained for 3.x compatibility while `CanonicalBoundaryModel` refuses it; which artifact is wrong is a design decision | | a union arm absorbs a type error as an extra field | 67 | separate defect in how a `oneOf` becomes a union of `extra="allow"` arms | | `Literal[True]` accepts the integer `1` | 2 | pydantic compares a literal by equality | | an `if`/`then` constraint is not generated | 1 | by design; `AdCPBaseModel`'s docstring states it | | the model requires a field the schema leaves optional | 33 (`creative_approval`) | needs a decision about which side is right | Each of these is a `DECLARED` entry except the dropped `pattern`, which this corpus does not reach — it needs a mutation that parses as a URL and violates a sibling constraint, and `"http://example.com"` against a `^https://` pattern is the obvious one to add. Declared entries cannot grow silently and must be deleted once fixed. BREAKING CHANGE: generated models validate `boolean`, `integer` and `number` fields strictly. A payload relying on pydantic's lax coercion — `"yes"` or `1` for a boolean, `"1"` or `true` for an integer, `"1"` for a number — now raises `ValidationError` instead of being coerced, matching the bundled JSON Schema for the same tool. An integral float for an integer field (`1.0`) is still accepted and narrowed to `int`, because JSON Schema counts it as an integer. The bundled schema validator additionally enforces `format: uri` and `format: hostname`, so a response carrying a malformed URL now fails response validation, which defaults to strict outside production. ## Integration notes (landing on top of #1286, #1353 and #1371) - **Scalar roots are strict too.** `--strict-types` spells an integer root `RootModel[StrictInt]` and a number root `RootModel[StrictFloat]`, which #1286's rewriter did not recognise, so `MajorVersion`, `ViewThreshold` and the other 35 int/float roots would have kept their wrapper. `adcp.types._scalar.SCALAR_BASES` now maps `StrictInt`/`SchemaInt`/`StrictFloat`/`StrictStr` as well; `ScalarInt` validates like `SchemaInt` (strict, with an integral float narrowed to `int`, via the shared `as_json_schema_integer`) and `ScalarFloat` is strict. `_VALUE_ROOT_NAMES` in `unwrap_rootmodel_unions` gained the strict spellings. - **A dead fix repaired.** `fix_product_publisher_property_model_coercion` matched the exact text of `core/product.py`'s pydantic import line; strict annotations changed that line, so on this branch the pass printed "import shape not found" and the publisher-property coercion on `Product` silently disappeared (the effect manifest in #1374 is what flagged it). It now goes through `ensure_pydantic_import`. - Regenerated from the combined generator; `SCHEMA_DELTAS.md` rebuilt as the `main`→head comparison.
KonstantinMirin
added a commit
to KonstantinMirin/adcp-client-python
that referenced
this pull request
Oct 5, 2026
… base Phase 2 made every canonical model a real subclass of the generated wire model it refines. _DeliveryCreativeManifest's own docstring says it is "deliberately not a subtype of the strict input", and after the conversion it is exactly that subtype -- the code contradicts its stated intent. The consequence is the weakening adcontextprotocol#1241 exists to prevent. Pydantic's default revalidate_instances="never" skips validation for an instance of any subclass, so WireCreativeManifest.model_validate(delivery_manifest) -> ACCEPTED handed back a manifest carrying a format_kind the pinned enum does not know, and the tolerance reached creative input. Measured directly: the instance path was accepted while the dump path was still rejected, which is why only the instance parametrizations failed. The strict base now revalidates SUBCLASS instances only. An exact-class instance still passes through untouched, so composing a model into a field keeps its identity (test_composability_invariant). The base is read off __bases__ rather than named, so it follows the declaration. 450 tests across the canonical, manifest, composability and subclass modules pass with it, including the composability grader. Phase 2 never ran tests/test_delivery_manifest_readback.py -- IMPL-CLONES section 7's three batches do not list it -- so this failed on that branch too, unmeasured. Also corrected to the new truth, each with the reason at the site: - the two 'assert not isinstance(x, input_model)' checks, false by construction now that the canonical models are subclasses; the pytest.raises calls carry the obligation and are what grade the bypass above - three fixtures that omitted a required field group adcontextprotocol#1368 now enforces: a CreativeManifest with neither format_id nor format_kind (core/creative-manifest.json root oneOf), a Placement with neither name nor publisher_domain, and a CreateMediaBuyRequest with no package group The four module-level TypeAdapter constructions adcontextprotocol#1353 and adcontextprotocol#1375 added are now @deferred_adapter, the repo's existing primitive for this. tests/test_freezegun_models.py forbids building a schema at import time and landed on main after both branches forked, so neither ever saw it. base.py and _get_products_helpers.py now reach generated types by their public adcp.types.domains path instead of adcp.types.generated_poc, which is what adcontextprotocol#1367's schema-mirroring tree is for. No allowlist grew: test_import_layering is keyed by file and both files were absent from it, so phase 1, phase 2 and the adcontextprotocol#1371 merge each failed that ratchet before this.
bokelley
added a commit
that referenced
this pull request
Oct 6, 2026
…e surface (#1392) * feat(types): add the AdcpRequest/AdcpResponse lattice and normalize the version envelope Hand-written field-less markers in adcp.types.base carry the accessors and the three MRO-derived field classifiers; a post-generation pass inserts them as the first base of every task message named by the pinned bundle's task registry; and a schema transform in generate_types gives the pointer-ref and wire-equivalent inline version declarations real AdcpVersionEnvelope ancestry. * test(types): record the forged-model refusal and the two predicates' asymmetry A field-presence check accepts any model declaring adcp_version and adcp_major_version; both markers refuse it. AdcpRequest is field-less by design, so inheriting it supplies nothing and a forger can inherit it and invent fields, while AdcpVersionEnvelope refuses that and accepts a model that is no task's request. The conjunction refuses both; neither predicate alone does. * refactor(types): give the task-message resolution one home, runnable against a wheel scripts/task_message_lattice.py now holds the registry walk, the title-to-class mangling, the provenance header map and the root-class rule. Three consumers share it instead of each carrying a copy: the marker-insertion pass, the ratchet test, and a --package measurement that reads the registry and the generated tree out of an installed adcp package, so the ancestry counts can be re-measured against a wheel rather than trusted from a report. * fix(types): refuse to normalize a schema whose union arm is a bare $ref Relocating the version stubs into a root allOf deletes the field from an arm that is a $ref elsewhere: dmcg merges a root properties into every arm it builds, but a root allOf base reaches only the arm built from the root object. Measured on the two pointer-ref proposals responses, RefineProposalsResponse2 went from 10 fields to 9 and RequestProposalsResponse4 from 16 to 15, both losing adcp_version under extra=forbid -- so a seller echoing the version on the submitted arm would have been rejected where it previously validated. Both schemas are refused and stay in the ratchet's residue by name; the all-arms ancestry is 70/77 either way, because that arm gains no ancestry from the rewrite. * refactor(types)!: make every canonical model a subclass, not a runtime copy The canonical creative layer built its 32 public classes with ``create_model``, copying ``model_fields`` off the generated wire model and inheriting nothing from it. That is why an injected generation-pass validator reaches the generated class and never the public one — the blocking finding on PR #1368. ARCH-ONE-SURFACE §6 refuses a second injection mechanism for the copy layer, so the copy goes instead: each public class is now ``class X(<generated>, CanonicalBoundaryModel)`` in real source. Four things the copy had to re-attach per class are declared once and inherited: * the envelope ancestry — ``_ENVELOPE_BASES`` and ``_canonical_clone_bases`` existed only because a copy loses it; it is transitive now, verified on all eleven canonical responses; * the wrap serializer that strips legacy creative identity; * the boundary's ``extra="allow"``, which still wins because ``CanonicalBoundaryModel`` is the last base; * the declared-field removal of legacy creative identity. This is the one thing inheritance cannot express, and it applies to 12 of the 32 sites (16 optional field declarations) through ``_field_definitions``' unconditional ``is_legacy_creative_identity_key`` filter — not through ``exclude=``, which no site used. It is now one ``__pydantic_init_subclass__`` rule on the boundary, reading the same strip predicate as the input validator, the schema sanitizer and the serializer. Also deleted: ``canonical_creative.pyi`` (253 lines, which hid 715 inherited fields across 31 classes and required four of them while inventing one), eight intermediate ``_*Base`` classes that existed only because ``create_model`` cannot carry a method body, the dead ``_canonical_enum`` helper, and two ``status`` redeclarations that restated what the generated arm already pins. Deleting the stub exposes the module to mypy for the first time; the errors it had been hiding are fixed rather than suppressed, and three assertions whose subject was the stub are corrected to the property that now carries the contract. BREAKING CHANGE: every public canonical creative model is now a subclass of the generated class it refines rather than a runtime copy of its fields. isinstance(x, GeneratedClass) and issubclass(CanonicalClass, GeneratedClass) are now True where they were False, so code using the negative to tell the two layers apart needs a different test. src/adcp/types/canonical_creative.pyi is deleted and the real source is what mypy reads, so previously hidden required fields are enforced at type-check time — ListCreativesResponse requires query_summary and pagination, and a construction that type-checked against the stub may now fail. Validators the generation pipeline injects into a generated class now reach the public canonical model by inheritance, so a public model refuses documents it previously accepted: CreativeManifest requires one of format_id or format_kind. * feat(types): add AdcpVersionEnvelope to the curated protocol surface `ProtocolEnvelope` is on `adcp.types.protocol`; `AdcpVersionEnvelope` was not, so a consumer assembling the pair had to take one from the curated surface and the other from the flat namespace. `adcp.types` already binds the name. This adds it to `adcp.types.protocol` as well — the `__all__` entry and the `TYPE_CHECKING` mirror, matching how `ProtocolEnvelope` is bound there — and grades what the pair is for. A consumer refusing a hand-written model that merely looks like a spec type needs both descent predicates, not either one: ```python issubclass(model, AdcpRequest) and issubclass(model, AdcpVersionEnvelope) ``` `AdcpRequest` is field-less, so inheriting it grants nothing and a class can declare it as a base and invent whatever fields it likes. `AdcpVersionEnvelope` refuses that, because inheriting it is what supplies the two spec fields — and it accepts a model that composes the envelope without being any task's message. The conjunction refuses all three constructed forgeries; either predicate alone refuses two. `test_the_version_envelope_has_a_public_name` asserts object identity between the public name and the generated class — a copy would make the conjunction refuse every real request — and then evaluates the conjunction through the public spelling only. * feat(codegen): derive a union arm's name from its schema node ARCH-ONE-SURFACE §3's rule, as a module that answers one question — what is this arm called — and does nothing else. Step zero classifies: an arm that introduces no property the base schema already declares is a VALIDATION arm and mints no class, because a public class for it would name something no buyer sends. Then the rungs, first one that holds: title, discriminator const, FULL required set, fail closed. Two findings the rule produced on its own, neither read off the bundle: * uniqueness is a property OF each rung, not a check bolted onto rung 3. ``core/audience-selector.json`` has three arms all pinning ``type: "signal"`` (binary, categorical, numeric), so the const rung names them identically and must fall through. The fail-closed collision refusal found it. * the classifier's ORDER matters. ``create-media-buy-request.json`` anyOf[2] declares only ``required`` and ``not`` — no ``type``, no ``properties`` — so a "nothing structural, therefore a scalar" fallback claims the purest validation arm in the tree. A scalar is recognized by what it positively declares. Measured over ``schemas/cache/3.2``, the bundle generation reads (``resolve_bundle_key("3.2.1")`` -> ``3.2``), excluding ``bundled/`` and ``mcp/``: 557 root arms = 278 named (169 title, 82 const, 27 required-set) + 278 minting no class (96 ref, 10 scalar, 172 validation) + 1 refused. The one refusal is ``content-standards/get-content-standards-response.json`` oneOf[0], exactly the single arm the design predicts, and the one upstream title the rule costs. The worked examples reproduce exactly: ``CreateMediaBuySuccess/Error/Submitted``, ``SignalRefProduct/DataProvider/SignalSource``, and all five distinct ``a2ui/bound-value.json`` names the set-difference would collapse. Counts are pinned by equality, the way the version-envelope residue is: a schema bump that moves one fails and is read. Nothing is permitted by appearing in them. * test(codegen): pin the two response arms the naming rule cannot name today The exact residue the surface move has to decide about. Of the 58 arm classes `restore_response_variant_aliases` numbers by position across 22 response modules, the rule names 56. Two it does not, for two different reasons: `get-content-standards-response.json` oneOf[0] is rung 4 and needs the one upstream title, and `get-creative-features-response.json` oneOf[0] is a bare $ref whose arm already names the shared class it selects — the emitter minting a new class for it is the defect, not the missing name. * fix(types): an inherited validator that reads a removed field reads None The one interaction between the two halves of the subclass conversion. `list-creatives-response.json` carries an injected `_validate_format_reference_xor` reading `self.format_id`; inheritance is the point of this layer, so it now runs on the canonical model, and with the field merely deleted it raised AttributeError instead of validating. Binding each removed name to None after class creation is the removal's own truth — pydantic never sees a field candidate, model_fields and the JSON schema and the wire are untouched — and the XOR reduces to exactly the invariant the canonical model should hold: format_kind must be set. Measured scope: one such validator across the 31 canonical models, found by `tests/test_manifest_response_readback.py::test_direct_listed_creative_kind_stays_strict`, which the subclass conversion had broken. Both directions of the reduced invariant are now graded. * fix(codegen): read a composing root's alias the way Annotated is defined The task-message marker pass resolves a union root through its type alias and walks the alias expression for the arm names. A composing root is emitted as `Annotated[A | B, Field(...)]`, so that walk collected `Annotated` and the `Field` call beside it as if they were arms, and the pass failed closed on the first one: RuntimeError: media-buy/list-products-response.json: union arm 'Annotated' of ListProductsResponse is not defined in list_products_response.py 91 generated aliases have that shape. _alias_arms_expr now strips the metadata to the first subscript element, which is what Annotated[T, ...] means, before the walk. A plain class root, a bare union alias and a nested alias are all unchanged, and an alias naming a class the module does not define still fails closed -- the fail-closed behavior is the point of the check and is kept. insert_task_message_markers now marks 203 task-message classes across 154 modules, and is recorded as firing in post_generation_manifest.json. * fix(types): stop a tolerant delivery model passing as its strict wire base Phase 2 made every canonical model a real subclass of the generated wire model it refines. _DeliveryCreativeManifest's own docstring says it is "deliberately not a subtype of the strict input", and after the conversion it is exactly that subtype -- the code contradicts its stated intent. The consequence is the weakening #1241 exists to prevent. Pydantic's default revalidate_instances="never" skips validation for an instance of any subclass, so WireCreativeManifest.model_validate(delivery_manifest) -> ACCEPTED handed back a manifest carrying a format_kind the pinned enum does not know, and the tolerance reached creative input. Measured directly: the instance path was accepted while the dump path was still rejected, which is why only the instance parametrizations failed. The strict base now revalidates SUBCLASS instances only. An exact-class instance still passes through untouched, so composing a model into a field keeps its identity (test_composability_invariant). The base is read off __bases__ rather than named, so it follows the declaration. 450 tests across the canonical, manifest, composability and subclass modules pass with it, including the composability grader. Phase 2 never ran tests/test_delivery_manifest_readback.py -- IMPL-CLONES section 7's three batches do not list it -- so this failed on that branch too, unmeasured. Also corrected to the new truth, each with the reason at the site: - the two 'assert not isinstance(x, input_model)' checks, false by construction now that the canonical models are subclasses; the pytest.raises calls carry the obligation and are what grade the bypass above - three fixtures that omitted a required field group #1368 now enforces: a CreativeManifest with neither format_id nor format_kind (core/creative-manifest.json root oneOf), a Placement with neither name nor publisher_domain, and a CreateMediaBuyRequest with no package group The four module-level TypeAdapter constructions #1353 and #1375 added are now @deferred_adapter, the repo's existing primitive for this. tests/test_freezegun_models.py forbids building a schema at import time and landed on main after both branches forked, so neither ever saw it. base.py and _get_products_helpers.py now reach generated types by their public adcp.types.domains path instead of adcp.types.generated_poc, which is what #1367's schema-mirroring tree is for. No allowlist grew: test_import_layering is keyed by file and both files were absent from it, so phase 1, phase 2 and the #1371 merge each failed that ratchet before this. * test(types): grade validator inheritance instead of clone carrying `tests/test_canonical_validator_parity.py` held one obligation — a document the generated class refuses, the public canonical name refuses too — over a mechanism that no longer exists. The canonical models were built with `create_model` over a copy of the source's fields, so the generated class was not in the MRO and every validator had to be re-decorated onto the copy, with any validator touching a dropped field skipped and recorded in `__adcp_canonical_validators_left_behind__`. The models are real subclasses now, so inheritance carries them and there is no carrying step, no skip list and no bookkeeping ClassVars to read. Every obligation is re-pointed at the new mechanism rather than dropped, and two are stronger for it: - `test_no_generated_validator_is_skipped` admits no exceptions, where the old test asserted only that a skip was justified by a dropped field. Nothing is skipped: the removal rule binds each removed name to `None` after class creation, so a rule written against `format_id` reduces instead of raising `AttributeError`. - `test_canonical_manifest_enforces_its_required_group_through_format_kind` asserts a refusal where the clone accepted the document. `core/creative-manifest.json` declares a `format_id | format_kind` root group; the clone skipped that validator because the canonical boundary has no `format_id`, so `{"assets": {}}` validated. The public name now refuses it and accepts it through `format_kind`. The two separate model-validator and field-validator tests are one test over both decorator kinds. Field validators are kept in scope although no generated ancestor declares one today, because an assertion covering only the kind that currently exists stops grading the moment a post-generation fix emits the other. The population is the 54 refinements in `canonical_creative`, which is broader than the 31 publicly bound models `tests/test_canonical_models_are_subclasses.py` parametrizes over: a validator dropped on a private `_Legacy*` refinement is just as silent. Verified by removing the generated ancestor from `CreativeManifest` on a scratch edit: the manifest test and the anti-vacuity floor both go red, naming the class. `tests/test_task_message_lattice.py` builds a `ValidateInputRequest` whose manifest now carries a `format_kind`, because `enforce_root_required_groups` enforces that group on the generated class. The accessors under test are indifferent to which arm satisfies it. * fix(types): take the domain mirror in the SDK's own decisioning helper `_get_products_helpers.py` imported two field enums out of `adcp.types.generated_poc`. Class names inside that tree renumber when the schema bundle moves, so an import that resolves today can name a different class after the next regeneration, and `tests/test_import_layering.py` forbids the import for exactly that reason. Both now take `adcp.types.domains.media_buy.*`, the mirror of the module that declares them. The mirror is a re-export, so each name is the same class object, and it is keyed on the schema path rather than on generator output order. No allowlist entry was added. `ALLOWED_FILES` in the layering test names facade and re-export layers, which this module is not. * docs: document the type surface and the generation pipeline The type layer has no architecture documentation. `docs/design/` holds one file, about a FastMCP registration investigation. `docs/extending-types.md` covers how a consumer subclasses a generated type — which base to pick, which variant alias to import, `Field(exclude=True)`, `@model_serializer` — and says nothing about what the surface guarantees or how it is produced. Nothing describes the generation pipeline at all: outside `CHANGELOG.md` and one unrelated proposal, `generate_types`, `post_generate_fixes` and `consolidate_exports` are named in no document in the repository, and `make regenerate-schemas` appears only in `schemas/patches/README.md`. Two documents, split by audience. `docs/type-surface.md` is a consumer's map: where to import from and when each spelling applies, what is deliberately not public and what the layering guard enforces, what `AdcpRequest` and `AdcpResponse` guarantee and what can be done with one before the tool is known, how envelope fields are told from payload fields, where the envelope ancestry is incomplete and why those residues are upstream's, and how response variations are named and dispatched. It defers to `extending-types.md` for subclassing and variant selection rather than restating either. `docs/design/type-generation.md` is for whoever changes the pipeline: every step in order with what it consumes and produces, which step touches the network, the ordering constraints inside the post-generation pass and why a fix must locate by structure rather than by spelling, the two derived guards that keep the export surface derived, how a class gets its name, the guards, how to add a schema feature, and what to do when the generator renders something wrong. It records the generator pin and the reason for it, which was documented nowhere: 0.72.0 and later cannot generate this tree, and 0.71.0 is rejected on measured non-equivalence, so there is no newer-generator lever for any future defect. Both state what the architecture refuses and why, because a reader who does not know what was rejected will propose it again. Two things the documents are careful about, since both are easy to get wrong when reading the tree rather than the history. The schema-derived union-arm naming rule is described as what it is — present, graded, and not wired — with the positional emitter that actually ships named beside it. And the staleness check is `generate_types.py --check`, which works today but which `make validate-generated` does not run, so the document says to run it yourself rather than implying the gate is automatic. `extending-types.md` said `AdcpVersionEnvelope` is not exported from `adcp.types`. It is, and it is now on the curated `adcp.types.protocol` surface too; the note is corrected and points at the new document for the message markers. * refactor(codegen)!: generate the domain tree as the public tree datamodel-code-generator wrote its models to `adcp.types.generated_poc`, a private tree, and `consolidate_exports.py` then emitted 1,059 one-line re-export modules under `adcp.types.domains` pointing at it. Two trees of ~1,086 files each, a copying step between them, and the private one was still the definition site — all of the cost of a public domain layout and none of the benefit. `OUTPUT_DIR` is now `src/adcp/types/domains`. The generator writes the public tree, so the module that DEFINES a class is the module an adopter imports it from. There is nothing to copy and nothing that can drift. What consolidation still writes are the namespaces DERIVED from that tree — `_generated.py`, each `<domain>/__init__.py`, the `DOMAINS` roster and `error_details.py` — never a second copy of a module. `generate_module_mirror` is deleted, and with it `restore_generation_dates` and the deferred-format parameter that existed only to make a thousand-file batch write date-stable. Four domains whose single schema sits at the bundle root generate as `domains/<domain>.py`, so that module already IS the domain root and gets no written root; a new post-generation pass gives each of the four an `__all__`, because a domain root is a public namespace and `export_resolution` reads `__all__` to record what one promises. The staging flow changes shape to match: the package copy is built before generation rather than after it, dmcg generates into `types/domains` inside that copy, consolidation writes the roots into the same directory, and the tree installs as one artifact instead of two. The models are untouched. Against the previous tree, exactly one non-`__init__` file differs — `enums/format_category.py`, the removed-type shim, whose message names the new path — and no module is missing. The domain roots bind the same names they bound before, to the same objects: `tests/fixtures/public_api_snapshot.json` is byte-identical, which it can be because `export_resolution` strips the generated prefix before keying a name, so `creative.list_creatives_response:Creative` is the key either way. `adcp.types.generated_poc` becomes a one-file tombstone that raises `ImportError` naming the prefix rename. Every module stem moved unchanged, so the migration is `adcp.types.generated_poc.` -> `adcp.types.domains.`; issue #1360 measured one production seller carrying 110 such imports, and fifteen lines turns a bare `ModuleNotFoundError` into that sentence. A working shim was refused: redirecting `__path__` would put a second module object in `sys.modules` for every module, which is two class objects per class and the exact identity break `test_export_surface_is_derived.py` exists to catch. `tests/test_import_layering.py` shrinks, which is the clearest measure of the change. The forbidden-prefix tuple goes from two entries to one — only `adcp.types._generated` resolves a name by something other than the schema — and the frozen violation baseline goes from eight files to one, because the eight were reaching into a private tree that no longer exists. A new assertion keeps the shrink deliberate: re-adding `adcp.types.domains` to the tuple would forbid the definition site, and now fails by name. `test_collision_guard.py`'s mirror-coverage test counted exports out of the generated tree and compared them to exports out of the same tree — two reads of one directory, so it passed whatever the public layout was. It is replaced by two that ask the runtime: every declared `(module, name)` pair must import from `adcp.types.domains.<module>`, and a real class must be DEFINED there. Adding a module-level re-export of a sibling class fails the second clause by name, which is the shape a mirror tree has. Two entries join `scripts/post_generation_manifest.json`, because #1374 landed #1374's effect manifest on main after both passes were written and it fails the build on a fix it does not know. `declare_root_schema_module_exports` is the pass this commit adds. `insert_task_message_markers` is this PR's own, from the lattice commit, which predates the manifest. Both fire, so both are declared `true`; `--update-fix-manifest` reclassified nothing else. One line inside the v3 codemod changes for a reason worth stating. `_replacement_is_identical` imports the module a flagged adopter import names, to check the proposed public replacement is the same object. The spelling the codemod DETECTS stays the v3 one; the module it IMPORTS is where those classes live now. The import sits inside `except (AttributeError, ImportError)`, so leaving it would have swallowed `ModuleNotFoundError` and silently degraded every per-symbol auto-rewrite to a flag. BREAKING CHANGE: adcp.types.generated_poc is removed. The generated models are defined at adcp.types.domains.<domain>.<schema>, with every module stem unchanged, so the migration is a prefix rename: adcp.types.generated_poc. -> adcp.types.domains. . Importing the old path raises ImportError carrying that instruction rather than ModuleNotFoundError, and the module is removed in v10. No public name changes what it resolves to: tests/fixtures/public_api_snapshot.json is byte-identical. adcp.types.domains is no longer an internal layer either - it is the definition site and a supported import for a name the flat adcp.types namespace cannot bind. * docs: the generated tree is the public tree, not an internal layer Every document that named `adcp.types.generated_poc` either described a path that no longer exists or, after the prefix rename, said `adcp.types.domains` is internal and must never be imported. That second reading is now false: the generator defines every public class there. The correction is the same everywhere, and it separates two things these documents had conflated. `adcp.types._generated` is internal because it binds a bare name several schemas declare to one sort-order winner, so a schema addition can repoint a name an adopter already imports. A domain path cannot move that way — it names the declaring schema. What a domain path does not protect you from is a NUMBERED class name: codegen numbers anonymous variant classes by traversal order, so `Assets162` can name a different shape after the next regeneration whatever path you reach it by, and `aliases.py` is what gives those a stable spelling. "Private module" and "unstable name" were one warning; they are two, and only the second applies to a domain path. `docs/type-surface.md` gains the statement the layout now supports: one tree, the definition site IS the public address, and the assertion to go with it (`Product.__module__ == "adcp.types.domains.core.product"`). `docs/design/type-generation.md` records the output path, the reordered staging steps, and why four domains have no written root. `CHANGELOG.md` and the five `MIGRATION_*.md` files are deliberately untouched: they describe releases in which that path existed and was private, which is still true of those releases. * test(types): grade the canonical field removal, and hide it from the checkers `CanonicalBoundaryModel.__pydantic_init_subclass__` removes the inherited legacy-identity fields through pydantic internals — deleting from `model_fields`, popping `__annotations__`, binding `None`, forcing a rebuild. That works on 2.13 and is not a supported API, so a minor pydantic release could change it silently. `tests/test_canonical_removed_fields.py` makes that loud. For every canonical model that inherits a legacy-identity field — 11 models, 15 declarations, derived from the generated ancestors and `is_legacy_creative_identity_key` rather than from a hand-written table — it asserts the name is absent from `model_fields`, from the JSON schema at every depth, and from `model_dump` / `model_dump_json` of an instance that really carries it, and that `model_validate` refuses a document holding it, for the boundary's stated reason. The same removal left a static disagreement: the annotation is inherited, so both mypy and pyright accepted `CreativeManifest(format_id=<correctly typed>)` and the runtime then raised. Each removal is now redeclared in a `TYPE_CHECKING` block with `Field(default=None, init=False)`, which both checkers read through pydantic's `dataclass_transform`, so the keyword is unavailable statically too. No suppression: four other spellings were measured and each needed one. The runtime surface of all 31 canonical models is byte-identical before and after. A guard test reads the module's AST and pins that every derived removal has such a redeclaration, with `init=False`, annotated exactly as its generated parent — and a second runs both checkers over a generated snippet, with the wire model as the control, so a refusal for the wrong reason cannot pass. `docs/types-9-migration.md` described a stub this branch deletes and a `_canonical_clone` helper it removes. Both paragraphs now describe the subclass model, with the adopter consequence our trial merge found: the runtime still coerces `pacing="even"`, the checkers now want the enum member or `model_validate`. Not a regression — main's stub did not declare the field at all, so the same call was refused there too. * feat(types)!: serve the pre-9.0 generated_poc path from the module it moved to The 9.0 tombstone raised `ImportError` for every `adcp.types.generated_poc.*` import. #1360 measured 110 of them in one production seller, so that breaks all of them at once, on upgrade, with no migration window. A finder at the front of `sys.meta_path` now serves each deprecated name from its canonical address and warns. The loader executes nothing: it imports `adcp.types.domains.<same stem>` and hands that object back, so `sys.modules` holds ONE module object under both keys and one class object per class. Redirecting a package's `__path__` instead would make the machinery load each module a second time under the old name, and an `isinstance` against the wrong copy fails for no visible reason. The machinery's only write to a module returned from `create_module` is `__spec__` — `_init_module_attrs` sets `__name__`, `__loader__`, `__package__`, `__path__` and `__file__` only when absent — and `exec_module` puts the canonical spec back, so the shared object carries no trace of the old name. `tests/test_generated_poc_alias.py` grades that over the root, all 26 domains and a derived sample of their schema modules: one object under both keys, every public name identical by `is`, no deprecated string on `__name__` / `__spec__` / `__package__` / `__file__`, the warning once per module naming the new path, and the finder declining every name outside the deprecated root. Reverting the loader to a second load turns 203 of them red. A real import is graded in subprocesses, where `sys.modules` is empty and the warning has not been consumed. `importlib.abc` is imported under `TYPE_CHECKING` only: it costs 9ms, on the import path whose cost `adcp.types`' lazy `__getattr__` exists to keep down. The alias module costs 189us. The deprecation has an expiry rather than becoming a permanent second surface: v10 removes it, stated in the warning, in the migration guide and in `REMOVED_IN_MAJOR`, and a test fails once either the declared or the installed major reaches it. BREAKING CHANGE: `adcp.types.generated_poc.*` is a deprecated alias of `adcp.types.domains.*` rather than the 9.0 tombstone that raised ImportError. Every import under it emits a DeprecationWarning naming its canonical path, and v10 removes the alias. Move to `adcp.types.domains.<same stem>` — the module stems are unchanged, so the migration is a prefix rename. * feat(types)!: make format_kind an open str the SDK never refuses `core/canonical-format-kind.json` declares a closed 16-member `enum` and, in the same file, requires a consumer SDK to retain an unknown `format_kind` and not fail the payload: "the producer-side enum stays closed; the consumer-side enum stays open for forward compatibility." The schema knows the rule is directional and then encodes it as one closed enum, which cannot carry that. A generated model that reproduces an incoherent schema faithfully does not inherit correctness from it; it propagates the incoherence into every consumer. This is a deliberate, documented override of the pin, with the upstream ask filed as adcontextprotocol/adcp#7929. One type, declared once. `OPEN_VOCABULARY_SCHEMAS` in scripts/generate_types.py names the schema and one transform in the existing chain replaces every `$ref` to it with `type: string`. Resolution goes through `_normalize_schema_ref_target`, the single owner of the canonical-URL, root-relative and file-relative spellings, so the transform cannot miss a spelling resolution understands. The schema file itself is untouched, so `CanonicalFormatKind` is still generated with its 16 members as the vocabulary and no public name is removed. The SDK refuses nothing, in either direction, and that is the decision. A seller supports some set of format kinds, and that set is the seller's, not this library's and not the pinned enum's. It can be larger than the sixteen — a kind promoted in a spec newer than the pin — or smaller. A pinned SDK cannot tell a kind a seller invented from a kind defined after its pin: both are simply not in its 16. Refusing the second to prevent the first would make this library's version a ceiling on what the protocol permits, which is the closed enum's defect with the enforcement moved into a validator. "I accept your request and then tell you I cannot process this creative" is a seller's answer, not a type error, and a pinned library cannot make it on the seller's behalf. The producer-side MUST is the seller's obligation; this library gives it the vocabulary to meet it. So the vocabulary is relocated, not discarded. `CanonicalFormatKind` stays a first-class export for comparison — it is a `StrEnum`, so `creative.format_kind == CanonicalFormatKind.image` holds against a plain string field and no adopter writes a literal — and `is_canonical_format_kind` answers membership. Its vocabulary is a PARAMETER defaulting to the pinned set, because only the caller knows which spec version its counterpart speaks and which kinds it actually supports. The SDK never calls it; its docstring says so, and says it is the sanctioned way to be strict. One function with a default argument: no registry type, no vocabulary class, no resolver, no configuration object. Five pieces of scaffolding existed only to reconcile the closed enum with the open requirement, and all five are gone: the `_OpenCanonicalFormatKind` alias, a second copy of it in `_forward_compat`, the `preserve_open_delivery_format_kind` post-generation fix that hand-patched one generated field, `_revalidate_subclass_instances_of_strict_base`, which flipped the wire bases to `revalidate_instances="subclass-instances"` so a subclass instance could not pass as validated output, and `_StrictFormatKind`, the per-direction base that refused a non-canonical kind on a request. With one open type everywhere, none of them has anything to do. `test_no_model_carries_a_vocabulary_validator` grades the absence structurally over every kind-bearing model, so a validator reintroduced under any name fails. The two read-back delivery classes survive, and the reason is measured rather than argued. Deleting them and typing `DeliveryCreative.variants` as `list[CreativeVariant]` turns the readback suites 6 red, of which exactly one is lost behaviour: the canonical models are real subclasses of the generated wire models they refine, so a delivery row carrying a manifest instance of the generated class is a PARENT instance, which pydantic refuses as `model_type`. Their `_normalize_readback` dumps it first. The other four failures grade the two-class split itself and would be restated. The classes are therefore about parent-instance acceptance and nothing about the vocabulary, which their docstrings now say. The cost is that the generated models no longer agree with the bundled schema for this field, where #1375 established that they should. It is declared as ONE named entry with its reason in tests/conformance/_schema_parity.py -- `format_kind_is_an_open_vocabulary_by_decision` -- and the parity rule is not relaxed: no predicate was widened and the entry deletes itself once it stops matching. Its property names are derived from the bundle rather than listed, because a hand-written pair covered only 21 of the 28 divergences. Removing the entry turns 4 tool parity cases red, the same 4 as before the strictness base was deleted, so the exemption's scope did not widen. The override is documented where a reader hits it: the module docstring quotes the schema's own normative text, and docs/types-9-migration.md gains a section with the `is` -> `==` migration, the membership recipe and what to delete when #7929 lands. BREAKING CHANGE: `format_kind` and `format_kinds` are `str`, not `CanonicalFormatKind`, on every generated and canonical model. Identity comparisons stop holding — replace `x.format_kind is CanonicalFormatKind.y` with `==`, which works against the member and the plain string. No model refuses a value any more, in either direction: a request model that raised `ValidationError` for a kind outside the sixteen now accepts it. Check the vocabulary yourself with `adcp.types.is_canonical_format_kind`, whose second argument is the vocabulary to check against. * fix(types): serve a split generated_poc module from both of its halves The pre-9.0 alias assumed every module stem moved unchanged, so a deprecated name resolved to one canonical module. One case has two: a root discovery schema, whose basename collides with a task-schema directory. brand.json used to generate into generated_poc/brand/__init__.py -- 140 classes including Brand -- and now generates into domains/brand_discovery.py, leaving domains/brand/__init__.py to the generated domain aggregator. So generated_poc.brand resolved and `from ... import Brand` raised an ImportError naming a module the adopter never wrote, which is worse than no alias at all. The second half is derived from the generator's own ROOT_DISCOVERY_SCHEMAS naming rather than listed here: discovery_half appends the _discovery suffix to a domain-level package and asks find_spec. A schema added to that dict is served without touching this module, and a test fails the build if the generator ever names one some other way. _SplitAliasModule serves one deprecated name over both halves, discovery first because that is the surface the pre-9.0 module had, and the two halves share 12 names. It defines no class, so every class reached through it is still the canonical class -- which is the property the one-module-object rule was protecting, and the only one isinstance reads. It borrows the aggregator's __path__ because _find_and_load reads the parent's before any meta-path finder runs. The alias docstring asserted the prefix rename and the single module object as unconditional facts. Both are now stated with the case that breaks them. The suite could not see this: it asserted over the names the CANONICAL module binds, never over the names the PRE-9.0 module exported. The new split family asserts the second direction over every public name of both halves, and takes its split set from ROOT_DISCOVERY_SCHEMAS so the next split is graded with no edit there. Three existing per-pair tests are parameterized over the unsplit pairs, derived by subtracting that same set. Closes #1402 * fix(types): make every Legacy name the generated class, one per response arm Two defects with one cause: the Legacy* set was curated by hand, so what it named drifted from what the generator emits. LegacyFormatId was a SUBCLASS of the generated FormatReferenceStructuredObject, and no public name on adcp.types resolved to the generated class at all. The subclass was written when agent_url was an AnyUrl that normalized the wire bytes; #1384 moved it to WireUrl -- StrictStr plus the absolute-URL validator plus the format: uri schema -- which is that subclass's entire purpose, and left every redeclaration a weakening of the parent it was written to narrow. Measured: the generated class accepts width: 300.0 through SchemaInt's JSON-schema-integer coercion and the public name for it refused the same payload, so the flat surface and the type the SDK's own model fields declare disagreed about one wire document. The parent already carries extra=allow, the same id pattern and pixel_ratio; its own _validate_agent_url duplicated _require_absolute_url and its model_dump override was a bare super call. So the fix is a deletion: the Legacy name is the generated class, as every other name in that module already is. The arm names had four holes: LegacyUpdateMediaBuy Success and Error shipped while Submitted and all three LegacyCreateMediaBuy arms did not, so a seller holding an arm read off the wire could only name it through adcp.types.domains.media_buy -- the private tree this release retires, because the bare public name is the canonical SUBCLASS and isinstance against it is False for that instance. Adding four names would have fixed four instances: the generator had already opened a fifth by adding a fourth preview_creative arm nobody named, and the derived test found it. The required set is now computed from the generated unions -- the arms come from typing.get_args of the union each module declares, so a schema that gains an arm changes the obligation with no edit -- and the surface is refused if it does not satisfy it. It is keyed on whether a bare public name reaches the generated arm, which is why sync_creatives needs no Legacy arm names: there, the generated class already IS the public one. The Legacy-name test asserts a round trip, not __module__: the class must be the attribute the module it claims actually binds under its own qualname. A probe proved why. __module__ alone is one line to fake, and it is a weak witness here because module_from_spec stamps the canonical spec onto a duplicate too; a shadows-a-same-named-base rule alone misses the shape this repo shipped, since the clone had a different class name. test_root_surface_is_canonical_and_legacy_is_explicit pinned the defect by name, asserting LegacyFormatId.__name__. Restated to the new obligation rather than deleted. Closes #1398 Closes #1399 * docs: name the one generated_poc stem that split, where an adopter reads it The migration guide stated the prefix rename unconditionally and promised the same module object for every deprecated name. Both are true of 1,087 of the 1,088 stems and false of brand, which is the one an adopter is most likely to have imported: the old generated_poc.brand WAS brand.json, and the new domains.brand is the generated domain aggregator, with brand.json's classes at domains.brand_discovery. A guide that tells an adopter a rename is mechanical when one case is not is how the ImportError in #1402 reaches them a second time, after the alias stops serving it in v10. States the split, the import to write, and the one property that deprecated name cannot keep -- module identity -- alongside the one it does, which is class identity and the only one isinstance reads. * fix(types): tell a split deprecated name's reader where its classes went The DeprecationWarning an adopter reads on importing generated_poc.brand said "import adcp.types.domains.brand instead; the module stems are unchanged, so this is a prefix rename". For that one name the advice sends them to a module that does not bind what they imported -- the same wrong answer the ImportError gave, restated as guidance, and guidance outlives the alias: it is what they act on in v10 when the alias stops serving them. A split name's warning now names both halves and says which is which, derived from discovery_half rather than branched on a stem. Every other name keeps the prefix-rename text verbatim. The codemod's own resolution helper restated the same claim. It fails safe today -- an unresolvable symbol takes the AttributeError branch and is flagged rather than rewritten, and neither symbol map holds a brand.json class -- so this corrects the comment and records where the fix belongs if one is ever added, which is the discovery half and not a wider except clause. * chore(codegen): move the unwired union-arm naming rule to #1409 scripts/union_arm_names.py and its tests renamed nothing in this PR: the rule is not wired into the pipeline, and arms are still named positionally by restore_response_variant_aliases. Per review, it lands later together with the renames that use it. The code is preserved on claude/issue-1409-union-arm-naming (this PR's head before this commit), and #1409 records the bundle measurements and the two surface-policy decisions wiring it needs. The design doc keeps the description of the positional emitter that ships and points at #1409 for the rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Brian O'Kelley <bokelley@scope3.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
3 participants
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.
fix(codegen)!: resolve structural pointer refs to the type they select
Addresses #1362. Criteria 1, 3 and 4 are met; criterion 2 is met for every
field except
geo_postal_areas_exclude, which the issue's second carve-out covers.Residuals, including two regressions, are listed below.
What was wrong
A
$refwhose fragment is a JSON pointer into another file'spropertiesselectsthat property's type. datamodel-code-generator has no name for a structural
position, so it minted a single-arm
RootModelwrapper per referencing site andtyped the field with the wrapper.
core/targeting-input.jsondeclares 27 of its 37 fields as "the typecore/targeting.jsongives this property, or null" (the other ten are 5 whole-file$refs and 5 inline type-lists), and those 27 hit the defect twice over:typekeyword, andanyOfwhose non-null arm declares atypegets a wrapper of its own.So
TargetingOverlayInputandTargetingOverlay— one wire document — disagreed on27 of their 37 field types, and reading a wrapped collection field returned
something no consumer can use: indexing and
len()raise, and iterating yields the('root', [...])field pair instead of the elements, with no error.keyword_targetsis the counterexample inside the same file. It spells nullabilityas a type list instead of a union and generates
list[KeywordTarget] | None.What changed
Two normalizations in
scripts/generate_types.py, both on the temporary schema treethat
flatten_schemasbuilds. The published schema cache is untouched.inline_structural_pointer_refsresolves a cross-file$refwhose fragmentaddresses a structural position (
/properties/<name>,/items,/oneOf/<n>) intothe subschema the pointer selects. A fragment whose first token is
$defsordefinitionsnames a reusable subschema that already generates one shared class, soit passes through untouched. Refs inside a selection are rebased against the file
that owns it, including same-document fragments, which address a document no longer
in scope once the selection moves. A pointer that is the only arm of an
allOfmerges into the enclosing schema:
core/catalog-selection.jsonaddsuniqueItemsthat way and
compliance/comply-test-controller-request.jsonadds adescription,and leaving the selection inside the
allOfloses theadditionalPropertiestheselection carries. Keywords at the referencing site win over the selection's.
collapse_nullable_unionsrewritesanyOf: [S, {"type": "null"}]on a namedproperty as
Swith a nullable type list. A$refarm already yieldsT | Noneandis left alone; so is a union of three or more arms. Only a named property is
normalized — a nullable type list in an
itemsposition makes the generator emitclass XItem5(Optional[XItem]), which neither imports nor runs, andformats/canonical/audio_hosted.jsondeclaresduration_ms_range.itemsthat way.The two compose: resolving the pointer exposes the arm's
type, and the collapsethen removes the wrapper rather than relocating it.
Result
TargetingOverlayInputandTargetingOverlaynow disagree on 10 of their 37 fieldtypes instead of 27. Eighteen fields that disagreed now agree, and no per-position
RootModelwrapper is left on either side:ProductPurchaseInputandProductPurchaseshare 22 field names, and the numberthat disagree drops from 19 to 2 —
targeting_overlay(class identity, #1181'ssubject) and a duplicated
CatalogId:CatalogSelectiongains the same six fields as plain types, and itstypefieldreaches the shared
CatalogTypeenum instead of a per-position wrapper:Across the whole generated tree, single-arm
RootModelclasses drop 324 -> 277 andthe field annotations referencing one drop 759 -> 602 (303 -> 256 and 716 -> 566
excluding
bundled/).Residuals, including two regressions
10 of the overlay's 37 fields still differ, from two causes, neither of them the
pointer:
targeting_input.GeoCountryagainsttargeting.GeoCountry, identical in shape. Apointed-to subschema that nests an anonymous object or constrained scalar has no
name to reference across files, so sharing those element classes is a separate ask.
geo_countriesis one of the nine.geo_postal_areas_exclude, islist[PostalArea]againstSequence[PostalArea]— the same shared class, differing only in the container,because
rewrite_response_list_to_sequenceapplies to response models. Three ofthe nine carry that asymmetry on top of their element-class difference.
age_restrictionregresses. It selects an inline object that the generator usedto name once in
targeting.pyand share, so the two overlays agreed on it before.Inlined, it is defined in both modules — identical in shape, and the only overlay
field that got worse.
GetProductsRequest.fieldsregresses in its annotation. This is the one#/itemspointer in the set (media-buy/product-fields.json#/items), and the oldcodegen already resolved it to the shared enum:
Runtime acceptance and serialization are identical — both enums are
StrEnumandvalidate by value, and the same member set reaches the wire.
ProductResponseFieldis reachable from no public module on either side (
adcp,adcp.types, and thepartial modules all raise
AttributeError), so only code importing fromgenerated_poc— whichadcp/types/__init__.pyinstructs against — sees the change.It is the same family as
age_restriction: a shared class becoming per-module.Inlining the
#/itemspointer could be skipped to avoid it, at the cost of a specialcase keyed on position.
Two fields keep a
RootModel, identically on both sides:placement_selectionand
collection_selectionreference whole-file schemas whose root is a union(
RootModel[PlacementSelection1 | PlacementSelection2]). That construct is #1077'sand #1277's subject, not a per-position wrapper.
What an adopter can now do
Read a request-side overlay with the same code that reads a response-side one.
That loop silently collected one
('root', [...])tuple before, then raisedAttributeErroron.root.Pass an input overlay's field to a function written against the output overlay.
Under
mypy --strict, on 8.0.0:Discover a documented tool schema.
adcp/server/mcp_tools.py:1902builds everyMCP
inputSchemafrommodel_json_schema(), and a wrapper carries the field'sdescriptiononto its own definition, leaving the field node bare. In the schemacreate_media_buyadvertises:Over the model as a whole, property subschemas identical to the output model's go
from 5 of 37 to 28 of 37, and fields routed through a generated
$defswrapper from32 to 10. The nine fields still undocumented are whole-file
$refs, where pydanticputs the description on the referenced definition — the same on both models.
Validation behaviour and
model_dump()were already identical on both classesbefore this change, and remain so. The defect is in the type and the published
schema, not in what either class accepts.
BREAKING CHANGE
Resolving a pointer
$refto its target removes the intermediate class the pointerpreviously generated.
src/adcp/types/_generated.py's__all__goes 3010 -> 2964:49 names out, 3 in (
ComplianceTaskCompletionData1,Fields1,Qualifier5).Every removed name is an artifact of the defect. An import of
GeoCountrieswas animport of a per-module
RootModelwrapper that existed only because the pointer wentunresolved; the replacement is the shared type the field now points at. Four rules
cover all 49, and the table below names the replacement for each:
selects, which is now the field's own annotation.
AudienceExclude->list[str];DaypartTargets->list[DaypartTarget].one.
DoohMetrics1->DoohMetrics,Geometry3->Geometry,GeoProximityItem2->GeoProximityItem,Viewability1->Viewability,QualifierModel->Qualifier,Qualifier3->Qualifier5.Field0was a base ofDeliveryMetricAggregate1, whose 11 fields are now declared on it directly.Extstill existsas
brand_discovery.Extandcore.assets.asset_union.Ext, both excluded frompublic consolidation (
scripts/consolidate_exports.py:188,194). The fields thatused it now carry
core.ext.ExtensionObject.AdcpMajorVersioncore.version_envelopeintAdcpVersioncore.version_envelopestrAgencyEstimateNumbermedia_buy.product_purchasestrAudienceEvidencePinsmedia_buy.product_purchaselist[core.audience_evidence_pin.AudienceEvidencePin]AudienceExcludecore.targetinglist[str]AudienceIncludecore.targetinglist[str]AxeExcludeSegmentcore.targetingstrAxeIncludeSegmentcore.targetingstrBiddingmedia_buy.product_purchasecore.bidding_policy.BiddingPolicyBrowsercore.targetinglist[enums.browser_family.BrowserFamily]BrowserExcludecore.targetinglist[enums.browser_family.BrowserFamily]CatalogIdsmedia_buy.product_purchaselist[media_buy.product_purchase_input.CatalogId]CollectionListExcludecore.targetingcore.collection_list_ref.CollectionListReferenceContextmedia_buy.product_purchasecore.context.ContextObjectDailyBudgetCapmedia_buy.product_purchasefloatDaypartTargetscore.targetinglist[core.daypart_target.DaypartTarget]DevicePlatformExcludecore.targetinglist[enums.device_platform.DevicePlatform]DeviceTypeExcludecore.targetinglist[enums.device_type.DeviceType]DoohMetrics1core.delivery_metricscore.delivery_metrics.DoohMetricsEndTimemedia_buy.product_purchaseAwareDatetimeExtmedia_buy.product_purchasecore.ext.ExtensionObject(the class survives asbrand_discovery.Extandcore.assets.asset_union.Ext; only the export slot is gone)Field0core.delivery_metric_aggregateDeliveryMetricAggregate1; its 11 fields are now declared onDeliveryMetricAggregate1itselfFormatOptionRefsmedia_buy.product_purchaselist[core.format_option_ref.FormatOptionReference]GeoCountriescore.targetinglist[core.targeting_input.GeoCountry]GeoCountriesExcludecore.targetinglist[core.targeting_input.GeoCountriesExcludeItem]GeoMetrosExcludecore.targetinglist[core.targeting_input.GeoMetrosExcludeItem]GeoPlacescore.targetinglist[core.geo_place_area.GeographicPlaceArea]GeoPlacesExcludecore.targetinglist[core.geo_place_area.GeographicPlaceArea]GeoPostalAreascore.targetinglist[core.postal_area.PostalArea]GeoPostalAreasExcludecore.targetinglist[core.postal_area.PostalArea]GeoProximityItem2core.targetingcore.targeting.GeoProximityItem/core.targeting_input.GeoProximityItemGeoRegionscore.targetinglist[core.targeting_input.GeoRegion]GeoRegionsExcludecore.targetinglist[core.targeting_input.GeoRegionsExcludeItem]Geometry3core.targetingcore.targeting.GeometryGtinscore.cataloglist[core.catalog_selection.Gtin]Idscore.cataloglist[str]MinSpendTargetmedia_buy.product_purchasefloatObservedDocumentVastVersionerror_details.vast_version_mismatchstrOptimizationGoalsmedia_buy.product_purchaselist[core.canonical_optimization_goal.CanonicalOptimizationGoal]PerformanceStandardsmedia_buy.product_purchaselist[core.canonical_performance_standard.CanonicalPerformanceStandard]Pricingmedia_buy.product_purchasecore.canonical_pricing_option.CanonicalPricingOptionPricingOptionIdmedia_buy.product_purchasestrPropertyListExcludecore.targetingcore.property_list_ref.PropertyListReferenceQualifier3core.delivery_metric_aggregatecore.delivery_metric_aggregate.Qualifier5QualifierModelcore.committed_metriccore.committed_metric.QualifierQuerycore.catalogstrStartTimemedia_buy.product_purchaseAwareDatetimeStoreCatchmentscore.targetinglist[core.targeting_input.StoreCatchment]Viewability1core.delivery_metricscore.delivery_metrics.ViewabilityModule paths are relative to
adcp.types.generated_poc.What does not break
tests/test_public_api.py's snapshot is unmodified, andadcp,adcp.typesand thetwelve partial modules export exactly the same names before and after. 48 of the 49
were reachable only from
adcp.types._generated, whichadcp/types/__init__.pydocuments as never to be imported; the 49th,
GeoPostalAreas, is public throughadcp.types.capabilitiesand resolves to a bundled class this change does not touch,so it stays reachable. No hand-written module or test in the repo references any of
the 49 for the class this change removes.
scripts/diff_generated_types.py'sfind_drift— which collapses generatedclass-name changes on purpose, because generator output renumbers with filesystem
iteration order — puts the semantic change at 10 files:
No deprecated aliases are added. Keeping them alive preserves, in the public
namespace, names that no longer correspond to anything the schema declares.
Tests
tests/type_checks/targeting_input_field_types.pyreads collection and scalarfields off both overlays under
mypy --strict, then passes each to a function typedwith the output overlay's field type. Against 8.0.0 it reports fourteen errors;
these name the defect directly:
tests/test_targeting_overlay_schema.py::test_advertised_targeting_fields_carry_the_description_the_schema_declarespins the advertised
create_media_buyschema: it names the nine fields that legitimatelycarry no description and fails on any other bare field. Against the pre-change
generated tree it reports 32.
tests/test_code_generation.pyadds nine cases over the two normalizations: pointerselection, ref rebasing (relative and same-document), the one-arm
allOfmerge,$defspointers passing through, the nullable collapse, and the three shapes thecollapse declines — a
$refarm, a three-arm union, and anitemsposition.tests/test_targeting_overlay_compat.py:99asserted the wrapper:Both arms now read the field the same way, so the branch collapses to one assertion.
Reproducibility
Re-running all six codegen steps from the committed state leaves
git status --porcelainempty, so the whole 1,899/929 generated diff is the productof the two generator rules and carries no unrelated churn.
make validate-generateddoes not establish this — it compiles the output — so the regeneration run is the
check that does.
SCHEMA_DELTAS.mdis rebuilt from a base-to-HEAD comparison rather than from aregeneration:
generate_types.pykeeps the committed report when a regeneration is ano-op, so for an already-landed change the pipeline cannot produce the delta. 42
report lines across 18 modules,
core/targeting.pylisting 27 removed classes.Gate
The nine failures reproduce with the pre-change generated tree checked out:
eight in
tests/test_version_scoped_models.pyfromsrc/adcp/types/versioned.py:486callingUnion.__getitem__on a tuple, whichPython 3.14 rejects, and one in
tests/test_reporting_interop_corpus.pyfrom a Nodefixture.
BREAKING CHANGE: resolving a pointer ref to its target removes the intermediate class the pointer previously generated, so
adcp.types._generated.__all__goes 3010 -> 2964: 49 names removed, 3 added. The removed names are artifacts of the defect, and the replacement is the shared type the field now points at. No deprecated aliases are provided. See the per-name replacement table above.Integration notes (landing on top of #1286 and #1353)
Two regressions this PR's body documented are fixed rather than declared:
age_restrictionis shared again andGetProductsRequest.fieldskeepsProductResponseField._names_a_class()inscripts/generate_types.pyleaves astructural pointer alone when the selection is an object, an enumeration or a
composition: the generator already resolves such a pointer to the one shared class
the owning module emits, and inlining it copied the class into the referencing
module (the copy is not structurally identical once nullability and local
annotations merge in). The overlay disagreement is 9 of 37 fields, all per-module
element classes of constrained scalars.
test_inline_structural_pointer_ref_leaves_a_class_shaped_selection_alonepins it, and
test_advertised_targeting_fields_carry_the_description_the_schema_declareslists
age_restrictionamong the fields that legitimately carry no description(a
$refputs the description on the definition).tests/type_checks/targeting_input_field_types.pyno longer reads.rootonGeoCountry: after feat(types)!: emit scalar schema roots as plain scalar subclasses #1286 a scalar root is thestr.Corrected accounting for
adcp.types._generated.__all__on the regenerated tree:44 names out, 0 in (not 49 out / 3 in).
DoohMetrics1,Viewability1,Qualifier3,QualifierModelandField0stay, because their pointers select objects and are nowleft to the generator;
ComplianceTaskCompletionData1,Fields1andQualifier5arenot introduced. The remaining 44 rows of the table above are unchanged. Public
surfaces (
adcp,adcp.types, the partial modules) are unchanged.