Skip to content

Fix pydantic unions that annotate the None member - #4557

Open
davidpavlovschi wants to merge 1 commit into
strawberry-graphql:mainfrom
davidpavlovschi:fix/pydantic-skipjsonschema-union
Open

Fix pydantic unions that annotate the None member#4557
davidpavlovschi wants to merge 1 commit into
strawberry-graphql:mainfrom
davidpavlovschi:fix/pydantic-skipjsonschema-union

Conversation

@davidpavlovschi

@davidpavlovschi davidpavlovschi commented Aug 1, 2026

Copy link
Copy Markdown

Repro

A pydantic model that annotates the None member of a union kills schema construction:

from typing import Union

from pydantic import BaseModel
from pydantic.json_schema import SkipJsonSchema

import strawberry


class ModelA(BaseModel):
    field_a: Union[str, SkipJsonSchema[None]] = None


@strawberry.experimental.pydantic.type(model=ModelA, all_fields=True)
class TypeA:
    pass


@strawberry.type
class Query:
    a: TypeA


schema = strawberry.Schema(query=Query)
strawberry.exceptions.invalid_union_type.InvalidUnionTypeError: Type `str` cannot be used in a GraphQL Union

I hit this on main at 34e9707, pydantic 2.13.4, Python 3.13.

Root cause

replace_types_recursively in strawberry/experimental/pydantic/fields.py rebuilds the union member by member. Each member keeps its Annotated wrapper, so Union[str, Annotated[None, SkipJsonSchema()]] comes out the other side unchanged. Python does not collapse that to Optional[str], because the union holds no bare NoneType.

StrawberryAnnotation._is_optional looks for that bare NoneType. It does not find one, so strawberry classifies the field as a real GraphQL union and hands str to from_union. Scalars are not valid union members, so you get InvalidUnionTypeError.

Nothing here is specific to SkipJsonSchema. Any Annotated[None, ...] union member breaks the same way.

Fix

I unwrap union members that are None behind an Annotated, right where the union gets rebuilt:

def _unwrap_annotated_none(type_: Any) -> Any:
    if get_origin(type_) is Annotated and get_args(type_)[0] is type(None):
        return type(None)

    return type_

The union branch now also catches typing.Union, not only PEP 604 UnionType. That matters because int | Annotated[None, "m"] evaluates to typing.Union at runtime, not types.UnionType, so the old isinstance(replaced_type, UnionType) branch never saw the broken case. The typing.Union path used to fall through to copy_with, which is Union[args] under the hood, so the rebuild itself behaves the same.

What it does not change

Only the None member loses its metadata. Metadata on None says nothing about the GraphQL type. Metadata on any other member can, and it survives untouched:

  • strawberry.lazy puts a StrawberryLazyReference in Annotated metadata, read by StrawberryAnnotation._get_type_with_args.
  • strawberry.union puts a StrawberryUnion in Annotated metadata, which names the union in the schema.

I added a test for the second one: a strawberry.union("AnimalUnion") inside an optional field still prints as AnimalUnion.

Optional detection past this point is unchanged. create_optional still filters NoneType and UNSET and rebuilds the child type the same way.

Test evidence

New tests, all failing before the fix and passing after:

  • tests/experimental/pydantic/test_basic.py::test_annotated_none_in_union covers a plain Annotated[None, "metadata"] member, both a two member and a three member union.
  • tests/experimental/pydantic/test_basic.py::test_skip_json_schema_none_in_union covers SkipJsonSchema[None], scalar and list.
  • tests/experimental/pydantic/test_basic.py::test_annotated_union_member_keeps_metadata pins the metadata that must survive.
  • tests/experimental/pydantic/schema/test_basic.py::test_basic_type_with_skip_json_schema_none_union builds the schema from the issue and asserts the printed SDL plus a query result.

Counts from my runs:

  • tests/experimental/pydantic on pydantic 2.13.4, before: 4 failed, 154 passed, 11 skipped, 1 xfailed. After: 158 passed, 11 skipped, 1 xfailed.
  • tests/experimental/pydantic on pydantic 1.10.26, before: 2 failed, 167 passed, 10 skipped, 1 xfailed. After: 169 passed, 10 skipped, 1 xfailed. The two SkipJsonSchema tests skip on v1.
  • Union and optional and annotated tests repo wide: 330 passed, 3 skipped, 1 xfailed, 5315 deselected.
  • Full suite minus the mypy, pyright and cli dirs: 4928 passed, 339 skipped, 207 deselected, 112 xfailed, 23 xpassed, 1 failed. The one failure is tests/typecheckers/test_pydantic.py::test_pydantic_type, which raises FileNotFoundError: [Errno 2] No such file or directory: 'ty' on my machine. It fails identically with my change reverted.
  • uv run mypy strawberry/experimental/pydantic/fields.py: no issues.
  • uv run ruff check and ruff format --check on the changed files: clean.

Fixes #3992

I used AI assistance on this change and reviewed everything.

Summary by Sourcery

Normalize Annotated None union members in Pydantic integration so Strawberry correctly treats these fields as optional while preserving metadata on other union members, and document the change with accompanying tests and release notes.

Bug Fixes:

  • Handle Pydantic union fields that annotate the None member as optional GraphQL fields instead of invalid unions

Enhancements:

  • Preserve Annotated metadata for non-None union members while normalizing None to support optional detection
  • Support both typing.Union and PEP 604 unions in the recursive type replacement logic

Documentation:

  • Add release notes describing the fix for Pydantic unions that annotate the None member and its impact on schema generation

Tests:

  • Add tests covering Annotated[None] in unions, SkipJsonSchema[None] in unions, and ensuring Annotated union members retain metadata and correct SDL/query behavior

A pydantic field like Union[str, SkipJsonSchema[None]] raised
InvalidUnionTypeError at schema construction. replace_types_recursively kept
the Annotated wrapper on the None member, so the union never collapsed to
Optional[str] and strawberry tried to build a GraphQL union out of str.

Unwrap union members that are None behind an Annotated. Metadata on other
members stays untouched, so strawberry.lazy references and strawberry.union
names keep resolving.
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for adding the RELEASE.md file!

Below is the changelog that will be used for the release.


This release fixes Pydantic fields that annotate the None member of a union.

Write field_a: Union[str, SkipJsonSchema[None]] in your model and Strawberry
now gives you a nullable String. Before, the Annotated wrapper hid the
NoneType that marks a field optional, so Strawberry read the field as a real
GraphQL union and raised InvalidUnionTypeError, telling you that str cannot
be used in a GraphQL union. Strawberry unwraps that None member now, and
leaves the metadata on every other member alone, so strawberry.lazy references and
strawberry.union names keep working.

This release was contributed by @davidpavlovschi in #4557

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In _unwrap_annotated_none, consider guarding get_args(type_)[0] with a length check to avoid a possible IndexError on malformed Annotated types, and perhaps clarify in the docstring that you return type(None) rather than the singleton None.
  • The _unwrap_annotated_none helper only handles Annotated[None, ...] at the top level; if nested constructs like Annotated[Union[str, None], ...] or multiple layers of Annotated are expected, you may want to make the unwrapping logic recursive or more general.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `_unwrap_annotated_none`, consider guarding `get_args(type_)[0]` with a length check to avoid a possible `IndexError` on malformed `Annotated` types, and perhaps clarify in the docstring that you return `type(None)` rather than the singleton `None`.
- The `_unwrap_annotated_none` helper only handles `Annotated[None, ...]` at the top level; if nested constructs like `Annotated[Union[str, None], ...]` or multiple layers of `Annotated` are expected, you may want to make the unwrapping logic recursive or more general.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Pydantic schema conversion for unions whose None member is wrapped in Annotated, allowing Strawberry to recognize them as optional fields.

  • Normalizes Annotated[None, ...] to bare NoneType while rebuilding both typing.Union and PEP 604 unions.
  • Preserves metadata on non-None union members, including named Strawberry unions.
  • Adds Pydantic v1/v2-compatible conversion tests and an end-to-end schema/query test.
  • Adds patch-release documentation for the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the union normalization narrowly scoped and covered at both type-conversion and schema-execution levels.

The changed conversion preserves non-None annotation metadata, correctly handles both supported union representations, and introduces no accepted functional, security, or quality issues.

Important Files Changed

Filename Overview
strawberry/experimental/pydantic/fields.py Extends recursive union reconstruction to normalize annotated None members without altering non-None metadata.
tests/experimental/pydantic/test_basic.py Adds focused coverage for plain annotated None, Pydantic SkipJsonSchema, multi-member optionals, and named-union metadata.
tests/experimental/pydantic/schema/test_basic.py Adds an end-to-end Pydantic v2 test confirming nullable SDL generation and successful query execution.
RELEASE.md Documents the corrected optional-field behavior and its motivating schema-construction failure.

Reviews (1): Last reviewed commit: "Fix pydantic unions that annotate the No..." | Re-trigger Greptile

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.

Pydantic Union with SkipJsonSchema breaks schema generation

1 participant