Skip to content

Use .get() for discriminator access in generated union encoders#176

Merged
darshkpatel merged 1 commit intomainfrom
fix-codegen-pyright-not-required
Mar 5, 2026
Merged

Use .get() for discriminator access in generated union encoders#176
darshkpatel merged 1 commit intomainfrom
fix-codegen-pyright-not-required

Conversation

@darshkpatel
Copy link
Contributor

Why

Follow-up to #175. The discriminator field in a discriminated union may be NotRequired in the TypedDict. Direct key access (x["shapeType"]) triggers pyright's reportTypedDictNotRequiredAccess error.

This broke the pid2 codegen CI when the scribe schema added discriminated union variants where the discriminator field is optional.

What changed

Use x.get("key") instead of x["key"] for discriminator checks in the generated ternary chain. This is safe because a missing key returns None, which won't match any discriminator value and falls through to the next branch.

Test plan

  • All 64 tests pass
  • Updated snapshot for test_unknown_enum
  • Tested end-to-end against the pid2 schema from ai-infra — codegen, mypy, and pyright all pass

~ written by Zerg 👾

The discriminator field may be NotRequired in the TypedDict, so direct
key access (x["key"]) triggers pyright's reportTypedDictNotRequiredAccess.
Using .get() is safe because a missing key returns None, which won't match
any discriminator value and falls through to the next branch.
@darshkpatel darshkpatel marked this pull request as ready for review March 5, 2026 21:49
@darshkpatel darshkpatel requested a review from a team as a code owner March 5, 2026 21:49
@darshkpatel darshkpatel requested review from jackyzha0 and poorvapotnis and removed request for a team March 5, 2026 21:49
@darshkpatel darshkpatel enabled auto-merge (squash) March 5, 2026 21:50
@darshkpatel darshkpatel merged commit c2138d8 into main Mar 5, 2026
3 checks passed
@darshkpatel darshkpatel deleted the fix-codegen-pyright-not-required branch March 5, 2026 21:50
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.

2 participants