Skip to content

feat(types): regenerate schemas + inject Literal-discriminator defaults#254

Merged
bokelley merged 1 commit intomainfrom
bokelley/assess-upstream-breakers
Apr 22, 2026
Merged

feat(types): regenerate schemas + inject Literal-discriminator defaults#254
bokelley merged 1 commit intomainfrom
bokelley/assess-upstream-breakers

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Upstream spec added a cluster of breaking changes — most Asset classes went from "discriminator is implicit" to "required tag field with single-valued Literal[...] type". Pydantic honors required literally, breaking ergonomic construction:

# Before upstream change:
TextContent(content="hello")

# After upstream change, pre-fix:
TextContent(asset_type="text", content="hello")  # must repeat the tag

Our v3→v4 migration already handles the <Type>Asset → <Type>Content rename; this new breaker is separate (required tag on the renamed class) and hits 15 test failures across test_client, test_preview_html, and test_discriminated_unions.

The fix: pattern-based default injection

inject_literal_discriminator_defaults() in scripts/post_generate_fixes.py. Walks the AST of each generated Pydantic model; for any AnnAssign whose annotation is Literal['x'] (bare or Annotated-wrapped) and whose value is None (no default), appends = 'x' on the same line via a line-based edit.

Touched 687 fields across 615 classes in 105 files — not just Asset types. Pricing options, webhook types, catalog types, and every other spec discriminator with a single-value Literal all get defaults.

Why this is safe

  • Wire consumption is unchanged. Dicts with the tag present validate exactly as before.
  • Validation strength is preserved. The Literal type still rejects any other explicit value — defaulting doesn't weaken the tag check.
  • Pattern-based, not value-specific. Robust to spec-value churn (e.g., if upstream renames asset_type values) as long as the single-value-Literal shape holds.

What's NOT fixed

The 3 WebhookAsset / CatalogAsset / VAST residual failures I flagged in the assessment turned out to also be Literal discriminators — all 15 failures are resolved by this one change. Future genuine-non-discriminator breakers (where a field became required with no canonical default) would need a separate mitigation (BeforeValidator coercion, better error messages) — this PR doesn't add that infrastructure because we don't need it yet.

Tested

  • +16 tests in tests/test_literal_discriminator_defaults.py:
    • Asset-content defaults (5 types × asset_type check)
    • VAST/DAAST dual-discriminator defaults (3 shapes)
    • Validation-strength preservation (wrong explicit tag still raises ValidationError)
    • Wire-format compatibility (dict → model with and without tag)
    • Injector meta-tests (skips multi-value Literals, extracts single-value, skips non-Literal)
  • Full suite: 2102 passed, 22 skipped (was 2086 on previous green; 15 new cleared + 16 test additions - 15 removals).
  • ruff, mypy clean across 678 source files.

Also in this PR (upstream pulls)

Test plan

  • CI green across Python 3.10–3.13
  • Review scripts/post_generate_fixes.py:inject_literal_discriminator_defaults — the AST walk + line edit is the risky part. Edit order is sorted descending so earlier edits don't shift later line indices.
  • Review a sampling of the regenerated Pydantic classes — confirm the = 'text' suffix is well-formatted and syntactically valid (easy to spot-check on src/adcp/types/generated_poc/core/assets/text_asset.py).
  • Optional: consider whether to surface the injection count in the regen summary output (currently just logged as a count line — quiet, can go unnoticed).

🤖 Generated with Claude Code

Upstream spec added a cluster of breaking changes — most Asset classes
went from 'discriminator is implicit' to 'asset_type is required'.
Pydantic honors 'required' literally, breaking ergonomic construction:

  # Before upstream change:    TextContent(content='hello')

  # After upstream change, pre-fix:
    TextContent(asset_type='text', content='hello')  # must repeat tag

Fix: inject_literal_discriminator_defaults() in post_generate_fixes.py.
Pattern-matches AnnAssign nodes whose annotation is Literal['x'] (bare
or Annotated-wrapped) with no default, appends ` = 'x'` on the same
line. Touched 687 fields across 615 classes in 105 files — Asset
types, pricing options, webhook types, catalog types, all spec
discriminator fields.

Wire consumption unchanged. Literal type still rejects other values,
so validation strength preserved. Wire dicts with the tag present
validate as before; dicts without the tag now also work (fall through
to default) — useful for minimal clients.

Pattern-based, not value-specific: robust to spec-value churn as long
as the single-value-Literal discriminator shape holds.

Also pulls upstream PRs #250 (design doc) and #251 (A2A contextId,
+22 integration tests). All green.

+16 new tests in tests/test_literal_discriminator_defaults.py. Covers
Asset defaults, VAST/DAAST dual-discriminator defaults, validation-
strength preservation, wire-format compatibility, injector meta-tests
(skips multi-value literals, extracts single-value, skips non-Literal).

2102 passing (was 2086), mypy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 27468aa into main Apr 22, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant