Skip to content

Dispatch discriminated union fields to their variant class - #1860

Merged
jar-stripe merged 10 commits into
masterfrom
jar/discriminated-unions
Aug 28, 2026
Merged

Dispatch discriminated union fields to their variant class#1860
jar-stripe merged 10 commits into
masterfrom
jar/discriminated-unions

Conversation

@jar-stripe

@jar-stripe jar-stripe commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Why?

Generated union parents carry a _inner_class_union_variant_types map that needs to be read to ensure we can properly read _field_encodings for the variant types.

What?

  • reads _inner_class_union_variant_types in _update_attributes, so the discriminator carried inside a union field's value selects the variant class, which then applies its own encodings
  • falls back to a plain StripeObject when the discriminator is absent, is not a string, or names a variant this release does not know, so a variant added to the API later still deserializes
  • adds tests/test_discriminated_unions.py — 24 tests across variant dispatch, unknown-variant fallback, inline unions, to_dict round-tripping, and request coercion

See Also

Tests discriminated union type shapes for both request-side (TypedDict
params with Literal discriminator) and response-side (StripeObject
deserialization), covering standalone and inline variants.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude
@jar-stripe
jar-stripe marked this pull request as ready for review August 6, 2026 21:13
@jar-stripe
jar-stripe requested a review from a team as a code owner August 6, 2026 21:13
@jar-stripe
jar-stripe requested review from xavdid and a lite review from Copilot and removed request for a team August 6, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new test module intended to validate “discriminated union” payload shapes, covering both request-style dict params (TypedDict + Literal discriminator) and response-style StripeObject construct/to_dict round-trips.

Changes:

  • Introduces standalone and inline discriminator “union” examples via TypedDict variants.
  • Adds runtime tests for discriminator field presence/access on dict params and StripeObject instances.
  • Adds round-trip checks ensuring discriminator fields survive construct_from(...).to_dict().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_discriminated_unions.py Outdated
Comment thread tests/test_discriminated_unions.py
jar-stripe and others added 2 commits August 7, 2026 13:11
The module docstring now explicitly states these tests exercise runtime
semantics (dict construction, field access, round-trip), not static type
narrowing. The dict() comment explains what it's actually testing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude

@xavdid xavdid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented - but this isn't really testing types yet

jar-stripe and others added 5 commits August 7, 2026 18:33
Inline union tests now use the flattened TypedDict pattern (discriminator
and per-variant payload fields on the parent) rather than the incorrect
per-variant TypedDicts-with-type-field pattern that was there before.

Request-side tests now exercise `_api_encode` so they verify real SDK
encoding behavior (bracket notation, nested dicts) rather than just dict
construction and key lookup.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude
A discriminated union field arrived as a dict with no class attached, so it
became a bare StripeObject. That object carries no `_field_encodings`, so an
int64 or decimal field inside a variant stayed a string — `luminance` came
back as "1500" rather than 1500.

Codegen already emits `_inner_class_union_variant_types` on the parent
(`{"color": ("model", {"rgb": RgbColor, ...})}`); nothing read it. Consume it
in `_update_attributes` so the discriminator inside the value selects the
variant class, which then applies its own encodings.

Mirrors stripe-ruby#1923. Falls back to a plain StripeObject when the
discriminator is absent, is not a string, or names a variant this release
does not know, so a variant the API adds later still deserializes.

Rewrites tests/test_discriminated_unions.py, which could not detect any of
this: every response-side test ran `StripeObject.construct_from` on the base
class with no variant map, so all seven passed identically against `{"foo":
1}`. The fixtures now mirror the generated shape — two color variants with
*different* encodings — so identical wire bytes hydrate differently based
only on the discriminator. Seven of the 24 tests fail with the dispatch line
reverted.

The request side moves from `_api_encode` to `_coerce_v2_params`.
`_api_encode` is v1 form encoding, which treats any dict identically and so
asserted nothing about unions; v2 requests coerce through the method-level
schema. One test pins the generator's deliberate flattening of variants into
one field-name-keyed map.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Committed-By-Agent: claude
@jar-stripe jar-stripe changed the title Add discriminated union serialization tests Dispatch discriminated union fields to their variant class Aug 27, 2026
@jar-stripe
jar-stripe enabled auto-merge (squash) August 27, 2026 17:46
@jar-stripe
jar-stripe requested a review from xavdid August 27, 2026 17:46
xavdid
xavdid approved these changes Aug 27, 2026
@jar-stripe
jar-stripe merged commit f7823b4 into master Aug 28, 2026
19 checks passed
@jar-stripe
jar-stripe deleted the jar/discriminated-unions branch August 28, 2026 16:43
@jar-stripe jar-stripe mentioned this pull request Aug 31, 2026
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.

3 participants