fix(codegen): keep required nullable response fields nullable

[bokelley]

Problem

media-buy/create-media-buy-response.json types confirmed_at as ["string", "null"] and lists it in the success branch's required. In JSON Schema those are independent axes: the key must be present, and its value may be null. A buy still in pending_creatives has no seller commitment instant to report, so null is a real protocol state, not a theoretical one.

The custom response-arm emitter in scripts/post_generate_fixes.py (restore_response_variant_aliases) read required as "not Optional" and never consulted the declared type array, so it emitted:

confirmed_at: AwareDatetime          # cannot hold the null its own schema permits

CreateMediaBuyResponse1(media_buy_id="x", packages=[], confirmed_at=None, revision=1)
# pydantic_core.ValidationError: confirmed_at — Input should be a valid datetime

Notably, datamodel-code-generator (see core/media_buy.py, media_buy/get_media_buys_response.py) and scripts/generate_versioned_stubs.py (see src/adcp/types/v32.pyi) both already get this right — the custom emitter was the outlier, and its rule would narrow any future required-and-nullable field the same way.

Fix

scripts/post_generate_fixes.py

• New _schema_permits_null() — recognizes both spellings the pinned schemas use: the type: [..., "null"] array form and a oneOf/anyOf branch of {"type": "null"}.
• New _union_with_none() / _top_level_union_parts() — bracket-aware widening that will not duplicate an existing top-level None (so dict[str, str | None] widens correctly and str | None is left alone).
• Both the response-arm emitter and the nested-object emitter now widen a required property with | None when — and only when — the schema permits null, without adding a default. The field stays required on the wire; it just can hold the null the schema allows. Optional properties route through the same helper, which is a no-op for every property in the current tree.

The fix lives in the generator, so it survives re-runs; no generated file was hand-edited.

src/adcp/types/canonical_creative.pyi

The public alias adcp.types.CreateMediaBuySuccessResponse is a runtime _canonical_clone, so type checkers read it from this hand-maintained stub rather than from the model. The stub left confirmed_at undeclared as a field and typed the constructor parameter Any, so the contract was invisible statically even after the codegen fix (reading the attribute was an attr-defined error; passing None was unchecked). It now declares confirmed_at: datetime | None both as a field and as the constructor argument.

Regenerated artifact

Only src/adcp/types/generated_poc/media_buy/create_media_buy_response.py changed — one line:

-    confirmed_at: AwareDatetime
+    confirmed_at: AwareDatetime | None

A sweep of every schema the emitter owns found confirmed_at on the create_media_buy success arm to be the only required-and-nullable property in scope. core/registry-feed-response.json's cursor (the other case named in the issue) is emitted by datamodel-code-generator, which already produces UUID | None with no default — it needed no change.

Tests

• tests/test_code_generation.py — unit coverage for _schema_permits_null / _union_with_none; a regeneration test that runs the emitter into a tmp_path tree and asserts (via AST, against the pinned schema) that confirmed_at is AwareDatetime | None with no default while the required non-nullable sibling media_buy_id is untouched; plus a guard that the committed generated tree carries the fix.
• tests/test_create_media_buy_response_types.py — runtime: confirmed_at=None validates, the field is still required (omitting it raises), a committed timestamp still parses and a malformed one still fails, and the null round-trips through model_dump(exclude_none=False) → model_validate.
• tests/type_checks/required_nullable_response_fields.py — mypy --strict fixture, zero type: ignore, covering both adopter surfaces: the public adcp.types.CreateMediaBuySuccessResponse (field read, is None narrowing, and construction with both None and a real timestamp) and the generated LegacyCreateMediaBuyResponse1 behind it. Verified as negative controls: reverting the stub produces 7 mypy errors, reverting the generated file produces 3.

Out of scope

AdCPBaseModel.model_dump still defaults to exclude_none=True, so a serialized null confirmed_at is dropped unless the caller passes exclude_none=False. Reconciling that blanket default with required-and-nullable fields is the issue's second-order note — a breaking serialization change with a much wider blast radius — and is deliberately untouched here.

Checks run locally

make lint, make typecheck-all (mypy src/, mypy --strict tests/type_checks/, type-ignore contract), make validate-generated, and the full pytest tests/ suite: 7688 passed, 42 skipped, 1 xfailed. All pre-commit hooks pass.

Closes #1137

🤖 Generated with Claude Code

---

Open workspace in Conductor

[aao-secretariat]

Ladon verdict: Approve

Approve

Clean, well-tested codegen fix. The custom response-arm emitter now widens a required property with | None only when its schema permits null, so CreateMediaBuySuccess.confirmed_at deserializes the null its own schema allows while remaining required (no default). The one-line generated diff is regeneration output paired with the emitter change — not a hand-edit — consistent with the "generated code is not source" rule. The fix: prefix is correct: the change is corrective and makes deserialization strictly more permissive (does not remove/rename a public export, flip required↔optional in a break-inducing way, or change wire shape so existing buyers stop deserializing), so no breaking-semver signal is required. The pre-existing exclude_none=True serialization gap is explicitly and correctly scoped out.

No blocking or Medium findings. high_risk is false, gated_paths is false, no author team gate. Rows 1–8 do not fire → row 9 approve.

[aao-secretariat]

Ladon verdict: Approve

Approve.

Clean pass. This is a spec-conformance fix for #1137: the custom response emitter now keeps required-and-nullable fields nullable (confirmed_at: AwareDatetime | None, no default) via bracket-aware, idempotent widening helpers. The generated artifact and .pyi stub were updated to match, with thorough runtime and type-check coverage. The datetime -> datetime | None widening is fix-appropriate (deserialization broadens, requiredness is unchanged, migration note present in README and PR body), so the fix: prefix is correct — no breaking semver signal is required.

No Critical, High, or Medium findings. Not gated (gated_paths: false), not high-risk, no no-auto-approve team match. None of decision rows 1–8 fire, so this falls through to row 9 → approve. Consistent with the prior clean approve.