Dispatch discriminated union fields to their variant class - #1860
Merged
Conversation
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
marked this pull request as ready for review
August 6, 2026 21:13
jar-stripe
requested review from
xavdid
and
a lite review from Copilot
and removed request for
a team
August 6, 2026 21:13
There was a problem hiding this comment.
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
TypedDictvariants. - Adds runtime tests for discriminator field presence/access on dict params and
StripeObjectinstances. - 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.
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
…ar/discriminated-unions
xavdid
reviewed
Aug 7, 2026
xavdid
left a comment
Member
There was a problem hiding this comment.
commented - but this isn't really testing types yet
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
…ar/discriminated-unions
jar-stripe
enabled auto-merge (squash)
August 27, 2026 17:46
Merged
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
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.
Why?
Generated union parents carry a
_inner_class_union_variant_typesmap that needs to be read to ensure we can properly read _field_encodings for the variant types.What?
_inner_class_union_variant_typesin_update_attributes, so the discriminator carried inside a union field's value selects the variant class, which then applies its own encodingsStripeObjectwhen 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 deserializestests/test_discriminated_unions.py— 24 tests across variant dispatch, unknown-variant fallback, inline unions,to_dictround-tripping, and request coercionSee Also