Repository navigation
Conversation
How a variant appears on the wire follows from its shape in serde's data model (unit, newtype, tuple or struct) and its enum's representation. OpenAPI, Python and the shared layout each worked out the shape from `Fields` themselves; #210 added a fourth copy for Python's JSON schemas. `codegen/schema/shape.rs` defines `Shape::of(&Fields)` once: - Unit: no fields, `"A"` when externally tagged - Newtype: one unnamed field, written as that field's value - Tuple: any other number of unnamed fields, written as an array; `A()` is an empty tuple, not a unit - Struct: named fields, written as an object; `A {}` is an empty struct Now read from it: - OpenAPI's `variant_to_schema`, for every representation - the layout's variants of an internally tagged enum - Python's externally tagged enum renderer, including the `(wire_name, shape)` entries #210 passes to the runtime's JSON schema helper, and the unit-only check for a flattened externally tagged enum Generated output is unchanged: the demo client (Python, TypeScript, Rust) and the codegen coverage, enum compatibility and field presence fixtures are byte-identical to main, and no snapshot changed. OpenAPI's empty externally tagged variants (`B {}`, `C()`) still leave their key out of `required`, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ
|
📖 Documentation Preview: https://reflectapi-docs-preview-pr-212.partly.workers.dev Updated automatically from commit 09d54d4 |
The rest of Python's codegen that decides a shape now matches on the shared Shape instead of on Fields: - the adjacently tagged, internally tagged and untagged enum renderers - the variants of a struct's flattened internally tagged enum, and variant_flattened_groups - render_enum's check for complex variants, which keeps counting an empty tuple variant `A()` as a unit - enum_unit_tag_set - the newtype-struct checks in render_struct and type_ref_to_python_type What remains matches Fields only to visit every field whatever its shape (type variables, reference validation, type parameter renames, generic inference) or to build one, so Shape adds nothing there. `Struct::is_tuple() && fields.len() == 1` and `Shape::Newtype` agree on any schema whose unnamed fields are named by index, which reflection always produces. Generated output is unchanged: the demo client and the codegen coverage, enum compatibility and field presence fixtures are byte-identical to main, and no snapshot changed. A new test pins the error for an internally tagged tuple variant, which passes on main too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ
hardbyte
marked this pull request as ready for review
October 30, 2026 02:55
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
Author
|
@claude review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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.
Another step towards #129, following #211. It picks up the review comment on #210:
How a variant appears on the wire follows from its shape in serde's data model and its enum's representation. OpenAPI, Python and the shared layout each worked out the shape from
Fieldsthemselves, and #210 added another copy for Python's JSON schemas. This PR defines it once, and every place in OpenAPI and Python that decides a shape now reads it.The shape
codegen/schema/shape.rs, next tolayout.rs:ShapeUnit"A"Newtype(&Field){"A": T}Tuple(&[Field])A(){"A": [T, U]}/{"A": []}Struct(&[Field])A {}{"A": {...}}/{"A": {}}Shape::of(&Fields)classifies;Shape::fields()lists the fields in order.Now read from it
OpenAPI.
variant_to_schemamatches onShapefor every representation (external, adjacent, internal, untagged), replacing itsFieldsmatches andfields.len() == 1 && !fields[0].is_named()checks.Layout. The variants of an internally tagged enum.
Python (second commit for everything after the first item):
(wire_name, shape)entries fix(python): describe the wire shape in JSON schemas of tagged and flattened enums #210 passes to the runtime'sexternally_tagged_json_schemacome fromShapethrough one mapping,json_schema_shape, instead of a string written in each branch.variant_flattened_groupsrender_enum's complex-variant check, which keeps counting an empty tuple variantA()as a unit (see below).enum_unit_tag_set.render_structandtype_ref_to_python_type.Struct::is_tuple() && fields.len() == 1andShape::Newtypeagree on any schema whose unnamed fields are named by index, which reflection always produces.What still matches
Fieldsin Python only visits every field whatever its shape, or builds one:collect_type_vars_from_typevalidate_type_referencesrename_type_params_in_typeinfer_enum_generic_paramsconcrete_struct_from_type_refmaterialize_nested_policy_flattenShapeadds nothing there;fields.iter()would be the tidy-up.Output
Generated output is unchanged. Two existing quirks are kept on purpose, because fixing either changes output and belongs in its own PR:
B {},C()) leaves its key out ofrequired, although serde always writes it.render_enumcountsA()as a unit, soenum E { A, B() }becomes a string enum withB = "B". Serde writesB()as{"B": []}, so the generated client can't read that variant.Testing
53f50d1) and on this branch, after each commit. All 60 files are byte-identical, including fix(python): describe the wire shape in JSON schemas of tagged and flattened enums #210's serde samples.tuple[()]androot: Nonefor untagged enumsShape: a newtype vs a tuple (includingA()), empty and non-empty structs, field ordercargo fmt --check, clippy-D warnings --all-targets, andcargo test --workspace --all-features: 368 passedNot changed
Fieldsitself, in about 13 places; separate PR.openapi.rs, and in the runtime'sserde.py, which rebuilds it from Pydantic's core schema. Removing that duplication would mean codegen emitting the schema fragments itself, which is a separate decision.🤖 Generated with Claude Code
https://claude.ai/code/session_01VjkNsfHSd5osRevK95McpQ