fix: preserve multi-type type union when schema has subschemas (#954)#1001
Open
ndreno wants to merge 1 commit intooxidecomputer:mainfrom
Open
fix: preserve multi-type type union when schema has subschemas (#954)#1001ndreno wants to merge 1 commit intooxidecomputer:mainfrom
type union when schema has subschemas (#954)#1001ndreno wants to merge 1 commit intooxidecomputer:mainfrom
Conversation
…decomputer#954) When a schema object had both `type: [a, b, c, ...]` and one of `oneOf`/`anyOf`/`allOf`/`not` on the same level, convert_schema_object matched an earlier arm that wildcarded the `instance_type` field (flagged by a pre-existing TODO: "we probably shouldn't ignore this"). The subschema branches were then converted in isolation, silently discarding the outer type union. For the schema in issue oxidecomputer#954: { "type": ["string", "number", "boolean", "array"], "oneOf": [ { "items": { "type": "string" } }, { "items": { "type": "number" } }, { "items": { "type": "boolean" } } ] } typify produced an enum with only three array variants, losing the string/number/boolean alternatives entirely. The fix tightens the match pattern to `None | Some(Single(_))`, letting `Some(Vec(_))` fall through to the existing merge arm which already folds the outer schema into each subschema branch via `try_merge_with_subschemas`. Single-type + subschema behaviour is preserved, so tolerant handling of schemas whose outer type conflicts with a branch (e.g. the rust-collisions fixture) is unchanged. Adds a fixture with eight cases covering the affected code path: - `type:[...]` combined with oneOf, anyOf, allOf, and not - explicit `type: array` in every oneOf branch (primitives pruned) - partially and fully unsatisfiable oneOf - simultaneous oneOf + allOf - singleton-type regression guard (rust-collisions pattern) No existing workspace fixture exercised `type: [...]` alongside subschemas on the same object, so this fix had zero pre-existing regression coverage.
ahl
requested changes
Apr 24, 2026
Collaborator
ahl
left a comment
ahl
left a comment
There was a problem hiding this comment.
Thanks for this. At least one test case is generating the wrong type.
For the $comments I'd ask for a little more brevity (are these LLM-written?).
| #[serde(untagged)] | ||
| pub enum SingleTypeOneOfArrayBranch { | ||
| Object { kind: ::std::string::String }, | ||
| Array([::std::string::String; 2usize]), |
Comment on lines
+467
to
+472
| // Subschemas with no body validation and either no type or a | ||
| // single type. A multi-type (`Vec`) instance_type is deliberately | ||
| // excluded so that it flows through the merge arm below, which | ||
| // folds the type union into each subschema branch. Preserving the | ||
| // earlier behaviour for `None` / `Single` keeps existing tolerant | ||
| // handling of schemas whose outer type may conflict with a branch. |
Collaborator
There was a problem hiding this comment.
Suggested change
| // Subschemas with no body validation and either no type or a | |
| // single type. A multi-type (`Vec`) instance_type is deliberately | |
| // excluded so that it flows through the merge arm below, which | |
| // folds the type union into each subschema branch. Preserving the | |
| // earlier behaviour for `None` / `Single` keeps existing tolerant | |
| // handling of schemas whose outer type may conflict with a branch. | |
| // Subschemas with no additional validation and either no type or a | |
| // single type. A multi-type (`Vec`) instance_type is deliberately | |
| // excluded so that it flows through the merge arm below. |
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
2 participants
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.
Re-submission of #1000 rebased directly on main. The earlier branch sat on top of other unrelated fork changes that altered doc-comment JSON field ordering, so the committed
.rsfixture didn't match upstream codegen andcargo testfailed after cherry-pick.This branch contains a single commit against current main:
typify-impl/src/convert.rs: narrow the Subschemas match arm frominstance_type: _toNone | Some(SingleOrVec::Single(_))so a multi-typeVec(_)instance_type falls through to the existing merge arm (try_merge_with_subschemas), which folds the outer type union into each subschema branch.typify/tests/schemas/type-array-with-subschemas.{json,rs}: new fixture generated against upstream codegen, 8 cases (oneOf/anyOf/allOf/not+ the singleton-type / rust-collisions regression guard + partially / fully unsatisfiableoneOf+ simultaneousoneOf+allOf).cargo test --workspace,cargo fmt --check,cargo clippy --all-targetsall green locally.Context on the fix itself is in #1000's description.
Closes #954.