feat(python): Add better undiscriminated union request types#16189
feat(python): Add better undiscriminated union request types#16189amckinney wants to merge 2 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for endpoint in service.endpoints: | ||
| for path_parameter in endpoint.all_path_parameters: | ||
| add(path_parameter.value_type) | ||
| for query_parameter in endpoint.query_parameters: | ||
| add(query_parameter.value_type) | ||
| for header in endpoint.response_headers or []: |
There was a problem hiding this comment.
🔴 Missing endpoint request headers in _collect_endpoint_referenced_ids causes incorrect file skipping
The _collect_endpoint_referenced_ids method at pydantic_generator_context_impl.py:331-336 iterates endpoint.all_path_parameters, endpoint.query_parameters, and endpoint.response_headers, but does not iterate endpoint.headers (endpoint-level request headers). By contrast, the SDK's endpoint function generator at endpoint_function_generator.py:456 uses service.headers + endpoint.headers, showing that endpoint.headers is a real, populated field.
If an enum type is used only as an endpoint-specific request header AND as a member of an undiscriminated union, it will not appear in referenced_elsewhere and will be incorrectly added to the inline-eligible set. Its file will be skipped (should_inline_away_type returns True), but it's still referenced from the endpoint signature — producing a broken import in the generated code.
| for endpoint in service.endpoints: | |
| for path_parameter in endpoint.all_path_parameters: | |
| add(path_parameter.value_type) | |
| for query_parameter in endpoint.query_parameters: | |
| add(query_parameter.value_type) | |
| for header in endpoint.response_headers or []: | |
| for endpoint in service.endpoints: | |
| for path_parameter in endpoint.all_path_parameters: | |
| add(path_parameter.value_type) | |
| for query_parameter in endpoint.query_parameters: | |
| add(query_parameter.value_type) | |
| for header in endpoint.headers: | |
| add(header.value_type) | |
| for header in endpoint.response_headers or []: | |
| add(header.value_type) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| union_member_ids.update(self._direct_named_ids(member.type)) | ||
| elif shape.type == "object": | ||
| for extension in shape.extends: |
There was a problem hiding this comment.
🔴 Container-wrapped enum members are marked eligible for inlining but never actually inlined, causing missing files
In _get_inline_eligible_enum_ids (pydantic_generator_context_impl.py:383-385), union_member_ids is populated using _direct_named_ids(member.type), which traverses into containers (optional, list, set, map, nullable). For example, a union member optional<SomeEnum> adds SomeEnum to union_member_ids. If SomeEnum isn't referenced elsewhere, should_inline_away_type(SomeEnum) returns True and its file is skipped.
However, _literal_value_reprs_for_member (pydantic_generator_context_impl.py:440-452) only handles direct named references and direct container/literal types — it returns None for optional<SomeEnum>. So the member falls through to the get_member_hint path which emits a type reference to SomeEnum by name, but the file was skipped → broken import in generated code.
Root cause: mismatch between eligibility detection and inlining logic
The eligibility check descends into containers to find named IDs, but the inlining check only handles top-level named/literal references. The fix should restrict union_member_ids collection to only add type IDs that would actually be inlined — i.e., direct named references at the member's top-level type.
Prompt for agents
The problem is in _get_inline_eligible_enum_ids in pydantic_generator_context_impl.py. When building union_member_ids for undiscriminated unions, _direct_named_ids is used which traverses through containers (optional, list, set, etc.). But _literal_value_reprs_for_member only handles direct named references and direct literal containers - it does NOT handle container-wrapped named types like optional<SomeEnum>.
This means an enum referenced as optional<SomeEnum> in an undiscriminated union gets added to union_member_ids (via _direct_named_ids), but when the union is actually generated, _literal_value_reprs_for_member returns None for the optional<SomeEnum> member, so SomeEnum is referenced by name in the generated code. However, SomeEnum's file was skipped because should_inline_away_type returned True.
The fix should restrict the union_member_ids collection to only include types that _literal_value_reprs_for_member would actually inline. For the undiscriminatedUnion case in the loop, instead of using _direct_named_ids(member.type), check member.type.get_as_union().type == 'named' and only add that type_id directly. This ensures only directly-referenced enums (not container-wrapped ones) are considered for inlining.
Was this helpful? React with 👍 or 👎 to provide feedback.
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
1 participant
Description
This adds the
inline_undiscriminated_union_request_paramsconfiguration option to the Python SDK generator. This significantly improves the generated method signature for inlined request types that contain undiscriminated unions of literal types.In general, these undiscriminated unions are forward compatible, such that they accept any value on the response side. The problem is that this makes the request less specific than it can be, which yields a less than ideal UX. For example, consider the following before/after.
Before
After
In particular, the experience before allowed the user to specify anything and their code would compile. This more accurately represents what the user is expected to specify, which guides them through the experience.
Testing