Skip to content

fix(json_schema_to_type): add allOf and oneOf handling#4019

Closed
Will-hxw wants to merge 1 commit into
PrefectHQ:mainfrom
Will-hxw:fix-3839-allof-oneof
Closed

fix(json_schema_to_type): add allOf and oneOf handling#4019
Will-hxw wants to merge 1 commit into
PrefectHQ:mainfrom
Will-hxw:fix-3839-allof-oneof

Conversation

@Will-hxw
Copy link
Copy Markdown

Summary

  • Add allOf and oneOf handling to json_schema_to_type() function: oneOf returns Union type, allOf attempts to merge object types into a single dataclass

Why

Issue #3839: json_schema_to_type() silently returns Any for both allOf and oneOf schemas, causing type information loss.

Validation

  • Syntax check passed
  • Commit pushed to fork

Handle allOf (intersection) and oneOf (exclusive union) schemas in
json_schema_to_type(). Previously both silently returned Any.

oneOf: produces a Union type (similar to anyOf)
allOf: tries to merge object types, falls back to Union if not possible

Fixes: #3839
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. openapi Related to OpenAPI integration, parsing, or code generation features. labels Apr 22, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a02b6165fb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +416 to +419
if dc_field and dc_field.default is not DataclassField.MISSING:
merged_fields[field_name] = (field_type, field(default=dc_field.default))
elif dc_field and dc_field.default_factory is not DataclassField.MISSING:
merged_fields[field_name] = (field_type, field(default_factory=dc_field.default_factory))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use dataclasses.MISSING for merged field defaults

Checking dc_field.default and dc_field.default_factory against DataclassField.MISSING raises AttributeError because Field has no MISSING attribute. Any allOf schema that reaches _merge_object_types() with dataclass-backed object branches will crash during type generation instead of returning a merged type.

Useful? React with 👍 / 👎.

Comment on lines +528 to +532
types = [t for t in types if t is not type(None)]
if len(types) == 0:
return type(None)
elif len(types) == 1:
return types[0]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve null option when converting oneOf to a union

The oneOf branch drops type(None) and never reintroduces it, so schemas like {"oneOf": [{"type": "string"}, {"type": "null"}]} are converted to str instead of str | None. This rejects valid null values and produces an inaccurate Python type for nullable oneOf schemas.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

jlowin commented Apr 22, 2026

Thanks for the contribution! This overlaps with #3840, which addresses the same gap in json_schema_to_type() for allOf/oneOf. Closing this as a duplicate — feel free to follow along or contribute there instead.


Generated by Claude Code

@Will-hxw
Copy link
Copy Markdown
Author

Thanks for the review. I've addressed both P1 and P2 in a separate PR #4020 which supersedes this one. The fixes are:

  • P1: Changed to (aliased from )
  • P2: Added null preservation for both oneOf and allOf branches

This PR can be closed in favor of #4020.

@jlowin
Copy link
Copy Markdown
Member

jlowin commented Apr 22, 2026

Sorry, this was meant to be closed. the P1/P2 are not the issue (and, while a PR is open, should always be pushed to the original, not a new PR)

@jlowin jlowin closed this Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. openapi Related to OpenAPI integration, parsing, or code generation features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants