fix(types): derive response arms from schemas

[bokelley]

Summary

This replaces the hand-written post-generate response-arm stubs with schema-derived generation so numbered SDK response classes stay aligned with the JSON schemas.
It regenerates the affected response modules, including full sync_creatives Creative fields, BuildCreative submitted/error numbering, submitted aliases/guards, and refreshed public exports/snapshot.
The root cause was post_generate_fixes.py silently overwriting datamodel-codegen's spec-faithful output with non-spec manual payload shapes.

Testing

• uv run --extra dev python -m pytest -q
• uv run --extra dev mypy src/adcp
• uv run --extra dev mypy --strict tests/type_checks
• pre-commit hooks during commit: black, ruff, mypy, adopter type-checks, bandit, whitespace/EOF, JSON and merge-conflict checks

[aao-ipr-bot]

Substance is right — this kills the post_generate_fixes.py stubs that were silently overwriting spec-faithful codegen output with status: Any = None payloads. Holding for one semver question before I approve: a public response model flipped from all-optional to required, shipped under fix:.

ad-tech-protocol-expert: sound. The 22 emitted arms match the 3.1.0-rc.9 bundle the SDK pins. The build_creative submitted arm is genuinely in the spec now, so flipping test_build_creative_response_has_no_submitted_arm → ..._includes_submitted_arm retires the adcp#3392 tripwire correctly rather than papering over it. names: dict[str,Any] → list[dict[str,str]] and house: str → House{domain,name} are corrections, not regressions.

python-expert: sound-with-caveats. Emitter is correct and idempotent-by-construction for the schemas it consumes; every finding is a latent gap, not a live bug.

code-reviewer: no blockers. Its "non-reproducible build_creative_response.py" Major is a false positive — it read the pre-PR on-disk file. The committed submitted arms all carry status: Literal[task_status_1.TaskStatus.submitted] (build_creative_response.py BuildCreativeResponse6, create_media_buy CreateMediaBuyResponse3); the emitter emits the plain TaskStatus form and inject_literal_discriminator_defaults — reordered to run after restore_response_variant_aliases in main() — rewrites it. Full pipeline reproduces.

The question (what flips this to approve)

fix(types): cuts a patch under release-please. Two changes in here read as contract changes, not patches:

1. BuildCreativeSuccessResponse (= BuildCreativeResponse1) flipped a field from optional to required. Old stub was status: Any = None; new arm requires creative_manifest (build_creative_response.py:130). BuildCreativeSuccessResponse.model_validate({...}) on a payload that omits creative_manifest — previously accepted — now raises ValidationError. Real wire deserialization of conformant responses is unaffected or improved, and the public export set is purely additive (public_api_snapshot.json is +10/-0), so this is narrow — but it is an optional→required flip on a name reachable from adcp.*.

2. use_enum_values=True removed from the submitted media-buy arms (create_media_buy_response.py CreateMediaBuyResponse3, and the new sync arms). response.status is now a TaskStatus member, not a plain str. Guards are safe (TaskStatus is a StrEnum, so == \"submitted\" holds — verified task_status.py:10), and JSON model_dump is identical, but python-mode model_dump() now yields the enum member. The PR body documents neither change.

Flip to approve on either: (a) confirm no adopter depends on the old stub shapes / pre-enum dump, so patch semver is intended, or (b) re-cut as fix!: with a one-line migration note. Maintainer's call on adopter reality — I can't see it from the diff.

Things I checked

• Arm reorder (0,4,1,2,3,5) keeps the public numbering load-bearing: Response1=success, Response2=error, Response6=submitted — matches BuildCreativeErrorResponse = BuildCreativeResponse2 (aliases.py:476) and BuildCreativeSubmittedResponse = BuildCreativeResponse6. test_type_aliases.py asserts all three.
• public_api_snapshot.json +10/-0 — additive only. No public export removed or renamed despite the _generated.py flat-namespace churn (ArtifactRecord, Fallback, FontRole, OpentypeFeature, WeightRangeItem dropped from the internal consolidation but not from the curated public surface).
• New is_*_submitted guards short-circuit before success/error (guards.py:248-271), and is_build_creative_success/is_build_creative_error now exclude the submitted envelope. Exported from adcp.types and re-checked in test_type_guards.py.
• Union parse safety: _validate_union_type is left-to-right first-match, and extra='allow' waives only extra keys, not required-field validation — a submitted payload can't match an earlier success/error arm and vice versa (ad-tech-protocol-expert confirmed against response_parser.py).
• Type-system layering intact: no new non-internal importer of generated_poc/** (guards.py:106-110 imports the numbered arms, but guards.py is internal).

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

• BuildCreativeResponse4 partial-failure ambiguity. The creatives arm declares optional errors; is_adcp_error returns True on any non-empty errors, so a Response4 success carrying per-item errors is classified as error by is_build_creative_error and excluded from is_build_creative_success. Newly-surfaced arm — worth a comment or a dedicated guard branch.
• The (0,4,1,2,3,5) reorder is gated on len(arms)==6. If the schema gains or drops a build_creative arm the reorder silently disables and BuildCreativeResponse2 stops being the error arm, breaking the alias with no test failure. Assert the arm count or key the reorder on arm shape.
• Emitter robustness gaps (python-expert), none triggered by current inputs: no identifier sanitization (a future response field named after a keyword or with a hyphen emits a SyntaxError); _merge_all_of silently widens allOf-with-$ref to Any; _generated_class_name falls back to class_names[-1], which guesses the wrong class if a module's primary class diverges from both its title and stem; a self-referential local #/ ref recurses unbounded in ref_type. Prefer failing loud over the [-1] guess.
• The post-generate idempotency test monkeypatches OUTPUT_DIR to an empty tmp dir, so cross-module ref resolution takes the title-pascal fallback, not the real read-the-generated-module path. It passes only because the two coincide today — it won't catch a class_names[-1] mis-binding.
• Add a _normalize_legacy_status round-trip conformance test (ad-tech-protocol-expert): a raw {status:'active'} with no media_buy_status should land as status='completed', media_buy_status='active'. That validator is the load-bearing path for the adcp#4906 deprecation window and the success arm's status: Literal['completed'] override depends on it.

Minor nits (non-blocking)

1. server/responses.py:680 docstring now says "List matches BuildCreativeResponse3 (multi-format)" — correct for the new numbering, and the function returns a dict so behavior is unchanged. Fine; noting it tracks the renumber.
2. Interesting that the third drift-correction pass (restore_signal_catalog_type_alias, restore_format_asset_numbered_aliases, restore_response_variant_aliases) had to move ahead of inject_literal_discriminator_defaults in main() to land the Literal rewrite — the pass ordering is now itself load-bearing. Worth a comment at the main() list so the next reorder doesn't quietly undo it.

Held for the semver confirmation above — answer either way and I'll approve.

[aao-ipr-bot]

Schema-derived arms are the right architecture, but the emitter drops nullability on required-nullable fields and ships a create_media_buy arm that can't parse a spec-valid response. One concrete file:line regression blocks; the rest is sound.

The fix itself is the right shape — restore_response_variant_aliases() deriving arms from the JSON schemas instead of hand-written status: Any stubs is exactly the way to stop the post-generate layer from silently overwriting codegen with non-spec payloads. ad-tech-protocol-expert: sound — the BuildCreativeResponse reorder (0,4,1,2,3,5) faithfully maps the schema oneOf (Success/MultiSuccess/Variant/Estimate/Error/Submitted) onto the stable public numbering, keeping Response1=success / Response2=error so adopters don't shift; the submitted arms are genuinely in the upstream schemas; extra='allow' is preserved on every arm so forward-compat is intact.

MUST FIX (blocking)

1. Required-nullable fields lose | None — create_media_buy success arm can't deserialize a provisional buy. Emitter.type_for collapses "type": ["string","null"] to "string" (non_null = [item for item in schema_type if item != "null"]) and discards the null arm; emit_response_class then emits required fields with no | None. Result: src/adcp/types/generated_poc/media_buy/create_media_buy_response.py CreateMediaBuyResponse1.confirmed_at: AwareDatetime — required, non-nullable. But schemas/cache/3.1.0-rc.9/media-buy/create-media-buy-response.json:42-46,161 types confirmed_at as ["string","null"] and lists it in required, with the description spelling it out: "May be null in deferred or manual-approval flows… provisional buys with confirmed_at: null cannot be active." What breaks for adopters: a spec-compliant response {"media_buy_id": …, "revision": 1, "confirmed_at": null, "status": "completed", "packages": […]} raises ValidationError. The prior hand-written stub had confirmed_at: AwareDatetime | None and parsed it. Corroborated: the AdCP storyboard runner — sales-proposal-mode (proposal_finalize) check is FAILURE on this PR (create_media_buy step: ✗ Response matches create-media-buy-response.json schema) and passes on main.

   Fix: thread nullability through the list-type branch — when "null" is present in a type list, append | None to the emitted type even when the field is required. Then audit every other required + […, "null"] field across the 22 regenerated arms the same way — confirmed_at is the one the storyboard caught, not necessarily the only one. Reconfirm proposal_finalize goes green before re-pushing.

Things I checked

• confirmed_at schema shape and required-membership against the packaged ADCP_VERSION (3.1.0-rc.9) — ["string","null"], required. Bug confirmed.
• BuildCreative arm reorder + src/adcp/server/responses.py:680 docstring correctly updated BuildCreativeResponse2 → BuildCreativeResponse3 for multi-format. Consistent.
• Submitted arms dropping use_enum_values=True and the _normalize_submitted_status before-validator: safe — TaskStatus is a StrEnum (src/adcp/types/generated_poc/enums/task_status.py:10), so Pydantic accepts the raw "submitted" against Literal[TaskStatus.submitted] and response.status == "submitted" holds for the guards. The new test_*_accepts_submitted_response / guard tests exercise exactly this and the Py3.13 job passed.
• Type guards (guards.py): is_*_submitted short-circuits before success/error; BuildCreativeSuccessBranches unions the four non-error/non-submitted arms. No gap.
• main() reordering of the three restore_* fns: restore_response_variant_aliases is now fully self-contained (reads schema, rewrites whole file), no broken ordering dependency.
• Public-API surface: new exports are additive (*SubmittedResponse, SyncCreativesResponse3, new guards); removed _generated names (FontRole, Style, OpentypeFeature, WeightRangeItem, ArtifactRecord, Fallback) are not in tests/fixtures/public_api_snapshot.json — internal generated names, not a public break. fix(types): semver is correct for the additive/conformance surface.
• Import layering: only _generated.py / aliases.py reach into generated_poc/. No breach.

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

• BuildCreativeResponse2 / CreateMediaBuyResponse2 construction tightening. The error arms went from loose stubs to errors: Field(min_length=1). An adopter constructing the error arm with an empty errors list now fails validation. ad-tech-protocol-expert flagged this as defensible under fix: (the old shape was non-spec, the arm was already "error") — agreed, but note it in the changelog prose so adopters aren't surprised.

Minor nits (non-blocking)

1. Header timestamp. Regenerated arms carry # timestamp: preserved-by-post-generate-fixes with a double space after the colon (codegen emits a single space). Cosmetic; harmless.

A PR whose stated purpose is to stop the SDK drifting from protocol shape, landing one | None short of protocol shape, is a notable place to land. Fix the nullability, re-green the storyboard, and this is ready to ship.

[aao-ipr-bot]

Block on one concrete deserialization regression. The schema-derived emitter is the right architecture — single code path, spec-faithful arms, no more hand-written stubs drifting from the wire — but the [type, "null"] collapse drops nullability on required fields, and that breaks a currently-working path.

MUST FIX

CreateMediaBuySuccessResponse.confirmed_at regresses required-nullable → required-non-null.

• Schema schemas/cache/3.1.0-rc.9/media-buy/create-media-buy-response.json:43-48 declares confirmed_at as type: ["string","null"], listed in required at :161, with the description "May be null in deferred or manual-approval flows until seller commitment occurs." The schema even carries an if confirmed_at is null conditional that depends on null being legal.
• The old stub had it right: confirmed_at: AwareDatetime | None (required key, nullable value). The new emitter produces confirmed_at: AwareDatetime (non-null) — src/adcp/types/generated_poc/media_buy/create_media_buy_response.py, CreateMediaBuyResponse1.
• What breaks for adopters: a spec-conformant seller create_media_buy response in a deferred / manual-approval flow carrying "confirmed_at": null now raises ValidationError. That payload parsed before this PR. The PR's stated goal is spec-faithfulness; for this field it moves away from spec.

Root cause is in the load-bearing file, scripts/post_generate_fixes.py: Emitter.type_for collapses ["string","null"] to the single non-null type and discards the null arm, then emit_response_class emits a bare field: T for anything in required — no | None. The emitter has no way to express "present-but-nullable." Fix is narrow: when collapsing a [T, "null"] type array, union None into the result even for required fields, so the field lands as confirmed_at: AwareDatetime | None (required key, no default). Re-run codegen after.

ad-tech-protocol-expert: sound-with-caveats — every other arm I had checked (build_creative 6-arm, sync_creatives, get_brand_identity, acquire_rights) is faithful; this is the one field that diverges. update_media_buy.implementation_date has the same [string,null] shape but is not required, so it lands on | None via the optionality path and masks how systemic the bug is — any required-nullable field hits it.

Semver

Flagged by both experts and worth resolving in the same pass. The union widening (new BuildCreativeResponse6, SyncCreativesResponse3, etc.) is additive. But the success-arm tightenings — GetBrandIdentityResponse1.house str → House object, names dict → list[dict[str,str]], and the confirmed_at change once corrected to required-nullable — are breaking for adopters who hand-construct or deserialize these arms. house: str → object is a genuine spec-correction (the old type couldn't deserialize a conformant payload), so I'm not blocking on the tightenings themselves. But fix: understates the impact. Re-tag as fix!: with a BREAKING note in the body, or split the additive arms from the breaking shape changes.

Things I checked

• TaskStatus is a StrEnum (enums/task_status.py → _str_enum.py), so dropping use_enum_values=True from the submitted arms is safe: .status is now the enum member but TaskStatus.submitted == "submitted" still holds, and the is_*_submitted guards (guards.py) pass. One caveat not exercised by tests: model_dump() now emits the enum member, not a plain "submitted" string — confirm no serializer / idempotency-cache path does a strict JSON-type check on dumped status.
• BuildCreative arm reorder (0,4,1,2,3,5) in Emitter.render matches the schema oneOf order (success / error / submitted) and the aliases (Success=R1, Error=R2, Submitted=R6, success-branches R1|R3|R4|R5).
• Public API snapshot diff is additions-only (BuildCreativeSubmittedResponse, Sync*SubmittedResponse, SyncCreativesResponse3) — no public name removed. The generated_poc nested renames (ArtifactRecord→Artifact, PreviewInput→Input, Style/Fallback/OpentypeFeature/WeightRangeItem dropped) are internal-only; the import-layering allowlist keeps them off the public surface.
• Inverting test_build_creative_response_has_no_submitted_arm → ..._includes_submitted_arm is correct: build_creative is async-capable in rc.9 (the schema's own arm 6 is the submitted envelope), and the original guard's docstring prescribed exactly this update when adcp#3392 landed.
• _normalize_legacy_status validators on the media-buy success arms are preserved and only touch status/media_buy_status, not confirmed_at.

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

• build_creative success/error guards misclassify the capped variant arm. is_build_creative_success/is_build_creative_error (guards.py) still key off is_adcp_error (non-empty errors). BuildCreativeResponse4 carries advisory errors[] alongside budget_status: "capped" — a successful-but-capped multi response reads as is_build_creative_error → True. Gate the build-creative guards on the discriminating fields (creatives / creative_manifest(s) / estimate) rather than errors presence.
• Idempotence is reasoned-stable but under-tested. test_post_generate_sync_creatives_response_arms_match_schema_creative_fields runs restore_response_variant_aliases() twice in isolation for one module. The main() reorder now runs the three restore_* passes before inject_literal_discriminator_defaults and friends, so on a real run those passes mutate freshly-emitted arms. Worth a test that runs the relevant main() slice twice and asserts byte-stability.

Minor nits (non-blocking)

1. Stray "SyncEventSourcesResponse" in the guards __all__. It's inserted among guard-function names, between "is_sync_catalogs_submitted" and the content-standards comment (guards.py). It's a defined TypeAlias so import * doesn't break, but every other union alias in that module is deliberately unexported. Drop it, or export them uniformly.
2. BuildCreative reorder is hard-gated on len(arms) == 6. If a future spec adds or removes a branch, the reorder silently no-ops while aliases.py still hard-codes BuildCreativeResponse3/4/5/6 — a silent mismapping rather than a failure. Assert the arm count, or derive the error/submitted index by title / status.const instead of the positional tuple.

Fix confirmed_at, re-run codegen, re-tag the breaking scope, and this is ready.

[aao-ipr-bot]

Right fix. The root cause is named correctly: post_generate_fixes.py was overwriting datamodel-codegen's spec-faithful output with hand-written status: Any = None stubs, and those stubs silently drifted from the protocol. Deriving the arms from the JSON schema makes the compatibility layer unable to lie about wire shape — that's the right shape for this layer.

Verified the load-bearing claim: ad-tech-protocol-expert confirms the 6-arm regeneration of build_creative_response.py matches schemas/cache/3.1.0-rc.9/media-buy/build-creative-response.json exactly (single-success, error, multi-format, per-creative, estimate, submitted), and that the historical-numbering reorder (0,4,1,2,3,5) produces the right Response1..6. adcp#3392 has effectively landed between rc.2 and rc.9 — replacing test_build_creative_response_has_no_submitted_arm with test_build_creative_response_includes_submitted_arm is correct, not a regression.

Things I checked

• Public surface is strictly additive. tests/fixtures/public_api_snapshot.json has only + lines — no removals. The dropped internal names (Fallback, FontRole, Style, WeightRangeItem, ArtifactRecord, OpentypeFeature) were never in the public snapshot, so this is internal generated-name churn, not a public break.
• Submitted-arm simplification is safe on the wire. Dropping use_enum_values=True and _normalize_submitted_status while keeping status: Literal[TaskStatus.submitted] = TaskStatus.submitted + validate_default=True still parses raw {\"status\": \"submitted\", \"task_id\": ...} because TaskStatus is a real str subclass (_str_enum.py) and pydantic v2 coerces the matching string to the member. server/responses.py:180 dumps mode=\"json\", which emits the bare string. New tests in test_code_generation.py and test_type_guards.py exercise raw-string parsing across build_creative/sync_creatives/sync_catalogs — they genuinely cover the path.
• schema_loader.py date-time FormatChecker is correct. start-timing.json is oneOf: [{const: \"asap\"}, {format: \"date-time\"}] with no date-only branch, so rejecting \"2026-04-01\" and accepting \"asap\" is spec-faithful. jsonschema treats format as a non-validating annotation by default — registering the checker is necessary for oneOf branch selection. Non-string → True early return matches JSON Schema format semantics.
• _merge_all_of fails open to Any on $ref/oneOf/anyOf/non-object parts — widens rather than emitting a wrong type. extra='allow' on every arm means a future 7th upstream variant still deserializes leniently.

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

• The len(arms) == 6 reorder in restore_response_variant_aliases is a time-bomb. arms = [arms[index] for index in (0, 4, 1, 2, 3, 5)] is gated on arm count, and aliases.py:477,479 hard-map BuildCreativeErrorResponse=Response2 / BuildCreativeSubmittedResponse=Response6 on those positions. When upstream adds a 7th build_creative arm, the gate goes False, arms fall back to raw schema order, and Response2 silently becomes a success branch — miscompiling the error/submitted aliases and is_build_creative_* guards with no failure at codegen time. Harden it: detect the error arm by required errors and the submitted arm by status.const == \"submitted\", or raise when the arm count/shape changes so the next regeneration fails loud instead of producing wrong types. This is the one thing to fix before the next upstream schema bump.
• Construction-time break for adopters. BuildCreativeResponse1 now requires creative_manifest (was a status: Any = None stub); CreateMediaBuyResponse1 now requires confirmed_at/revision/packages. Deserialization is unaffected (real payloads carry these; extra='allow'), but anyone hand-constructing the old stub shape now hits ValidationError. Shipping under fix(types): is defensible since the prior shape was never spec-valid — but call it out in the release notes so it isn't a surprise patch-bump.
• Lock the wire contract with one serialization assertion. The new tests cover guards and round-trip validation but never assert BuildCreativeSubmittedResponse(...).model_dump(mode=\"json\")[\"status\"] == \"submitted\". Cheap insurance against a future use_enum_values reintroduction breaking the dumped shape.

Minor nits (non-blocking)

1. Stray __all__ entry. guards.py adds \"SyncEventSourcesResponse\" to __all__ under the # Catalog guards comment (diff L4548). It's defined (guards.py:139), so import * won't break — but every other entry there is an is_* guard, and no other response union is exported from guards. Copy/paste slip; drop it.
2. datetime.fromisoformat is stricter than the regex on 3.10. The _RFC3339_DATE_TIME regex already validates structure; the secondary fromisoformat check rejects some RFC3339-valid offset/fractional forms on the 3.10 CI leg. Low impact for AdCP payloads, but the regex alone is the safer gate.

The generator was lying about the wire shape; now it can't. LGTM. Follow-ups noted below.