Skip to content

fix Implement static checking for model_validate in pydantic #1123 - #3390

Open
asukaminato0721 wants to merge 3 commits into
meta:mainfrom
asukaminato0721:1123
Open

asukaminato0721 wants to merge 3 commits into
meta:mainfrom
asukaminato0721:1123

Conversation

@asukaminato0721

@asukaminato0721 asukaminato0721 commented May 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #1123

Added synthesized model_validate support for Pydantic models including nested model/list dict input shapes.

Wired the synthesized method into Pydantic dataclass fields.

Added call-time constraint checking for model_validate, including inline literals and same-scope literal initializers.

Test Plan

add test

@meta-cla meta-cla Bot added the cla signed label May 14, 2026
@github-actions github-actions Bot added size/xl and removed size/xl labels May 14, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added size/xl and removed size/xl labels May 14, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added size/xl and removed size/xl labels May 14, 2026
@github-actions

This comment has been minimized.

@asukaminato0721
asukaminato0721 marked this pull request as ready for review May 14, 2026 16:06
Copilot AI review requested due to automatic review settings May 14, 2026 16:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a synthesized model_validate classmethod to Pydantic dataclass-style models in pyrefly's static checker, and adds call-time constraint checking that walks dict literals (and same-scope literal initializers) passed to Model.model_validate(...) to flag values that violate field constraints such as Field(ge=...). It also extends emit_pydantic_argument_constraint to recurse through union types so each member is checked.

Changes:

  • Synthesize a model_validate classmethod on every Pydantic model whose own class doesn't already define one, with a typed-dict-shaped obj parameter that recursively expands nested models, lists, and dicts.
  • Add check_pydantic_model_validate_constraints invoked from the generic call path; it inspects dict literals (and traces a name's same-scope initializer) and the inferred typed-dict shape for ge/le/etc. violations on nested fields.
  • Add four new pydantic tests covering nested-field range checking, from_attributes, custom model_validate preservation, and a super().model_validate(...) flow.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
pyrefly/lib/alt/class/pydantic.rs Implements synthesized model_validate builder, dict-shape type construction, and the recursive expr/type constraint checkers; adds union recursion to emit_pydantic_argument_constraint.
pyrefly/lib/alt/class/dataclass.rs Wires the synthesized model_validate into Pydantic dataclass synthesized fields, suppressed only when the current class itself defines model_validate.
pyrefly/lib/alt/call.rs Invokes the new constraint checker on every call expression.
pyrefly/lib/test/pydantic/field.rs Adds tests for nested constraint detection, from_attributes, custom-method preservation, and super().model_validate(...).
Comments suppressed due to low confidence (5)

pyrefly/lib/alt/class/pydantic.rs:995

  • When expected is a Type::Union, this iterates each member and recursively checks the same actual against every member, emitting a constraint violation for any union member whose nested-model shape disagrees. For a value typed as int | str this is harmless because non-pydantic members are no-ops, but for ItemA | ItemB (both pydantic models) the same actual will be validated against both models and may produce duplicate or false-positive errors when the value is only intended to satisfy one branch of the union. Consider only emitting an error if all members fail (i.e. a "best-match" or "any-passes" semantics) for nested model unions.
            Type::Union(union) => {
                for expected in union.members.iter() {
                    self.check_pydantic_field_expr_constraints(
                        expected, actual, range, errors, seen,
                    );
                }
            }

pyrefly/lib/alt/class/pydantic.rs:1084

  • Same union false-positive concern as in check_pydantic_field_expr_constraints: when the expected type is a union of multiple pydantic models, the actual value is validated against every member and any failing branch will report a constraint error, even if another branch in the union accepts the value.
        match (expected, actual) {
            (Type::Union(expected), _) => {
                for expected in expected.members.iter() {
                    self.check_pydantic_field_type_constraints(
                        expected, actual, range, errors, seen,
                    );
                }
            }

pyrefly/lib/alt/class/pydantic.rs:832

  • When the user writes bad = {"items": [..., {"quantity": -3}]} followed by Inventory.model_validate(bad), this code recurses into the initializer expression of bad and emits a constraint violation at the literal's range — but the assignment bad = {...} is, by itself, not a pydantic call. The error then surfaces on the assignment line even when the user could plausibly use bad for unrelated purposes, and the diagnostic gives no indication that the violation comes from a downstream model_validate call. Consider either reporting the error at the model_validate call's range (with a note pointing back to the literal) or qualifying the message so it isn't confusing on the assignment line.
        if let Some(initializer) = self.pydantic_name_initializer(obj) {
            self.check_pydantic_model_validate_expr_constraints(
                &cls,
                dataclass,
                initializer,
                range,
                errors,
                &mut seen,
            );
            seen.clear();
        }

pyrefly/lib/alt/class/pydantic.rs:288

  • Recursion into nested container types only handles list and dict; nested pydantic models inside tuple, set, frozenset, Sequence, Mapping, etc. will not have their dict-shape input form added to obj_ty. This makes the accepted argument type for model_validate inconsistent with what Pydantic actually accepts at runtime (it coerces dict shapes through any iterable container, not just list/dict).
                } else if cls.class_object() == self.stdlib.list_object()
                    && let [elem] = cls.targs().as_slice()
                {
                    self.heap.mk_class_type(
                        self.stdlib
                            .list(self.pydantic_model_validate_type(elem.clone(), seen)),
                    )
                } else if cls.class_object() == self.stdlib.dict_object()
                    && let [key, value] = cls.targs().as_slice()
                {
                    self.heap.mk_class_type(self.stdlib.dict(
                        key.clone(),
                        self.pydantic_model_validate_type(value.clone(), seen),
                    ))
                } else {
                    Type::ClassType(cls)
                }
            }

pyrefly/lib/alt/class/pydantic.rs:881

  • pydantic_name_initializer only resolves Binding::Expr/Binding::NameAssign/Binding::AnnotatedType chains and ignores re-assignments. If the user writes bad = {"quantity": 5}; bad = {"quantity": -3}; Inventory.model_validate(bad), only one of these initializers will be inspected (whichever the binding key resolves to), and the analysis may either miss real violations or report them on a stale initializer that doesn't actually flow into the call. Consider documenting this best-effort behavior in a comment, or restricting it to bindings that are provably the unique reaching definition.
    fn pydantic_name_initializer<'b>(&'b self, actual: &Expr) -> Option<&'b Expr> {
        let Expr::Name(name) = actual else {
            return None;
        };
        let key = Key::BoundName(ShortIdentifier::expr_name(name));
        let idx = self.bindings().key_to_idx_hashed_opt(Hashed::new(&key))?;
        let mut idx = idx;
        let mut gas = Gas::new(100);
        while !gas.stop() {
            match self.bindings().get(idx) {
                Binding::Forward(forward)
                | Binding::PromoteForward(forward)
                | Binding::ForwardToFirstUse(forward) => idx = *forward,
                binding => return Self::pydantic_initializer_from_binding(binding),
            }
        }
        None
    }

    fn pydantic_initializer_from_binding(binding: &Binding) -> Option<&Expr> {
        match binding {
            Binding::Expr(_, expr) => Some(expr),
            Binding::NameAssign(assign) => Some(&assign.expr),
            Binding::AnnotatedType(_, inner) => Self::pydantic_initializer_from_binding(inner),
            _ => None,
        }
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +150 to +159
if metadata.is_pydantic_model()
&& !self
.get_class_fields(cls)
.is_some_and(|fields| fields.contains(&Name::new_static("model_validate")))
{
let root_model_type = self
.get_pydantic_root_model_type_via_mro(cls, &metadata)
.map(|(ty, _)| ty);
fields.insert(
Name::new_static("model_validate"),
Comment thread pyrefly/lib/alt/call.rs
Comment on lines +1893 to +1899
self.check_pydantic_model_validate_constraints(
ty,
&x.arguments.args,
&x.arguments.keywords,
x.arguments.range,
errors,
);
Comment on lines +250 to +256
ClassSynthesizedField::new(self.heap.mk_function(Function {
signature: Callable::list(
ParamList::new(params),
self.heap.mk_self_type(self.as_class_type_unchecked(cls)),
),
metadata,
}))
@github-actions

This comment has been minimized.

@meta-codesync

meta-codesync Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D114357524. (Because this pull request was imported automatically, there will not be any future comments.)

@codspeed

codspeed Bot commented Jul 31, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 35 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing asukaminato0721:1123 (aadd153) with main (1e1e325)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@stroxler stroxler 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.

Cool! We've got some issues that need to be addressed before we can merge this though:

  • Exponential synthesis cost: a 61-line nested-model fixture takes 27.3s versus 3.3s on the parent, without calling model_validate.
  • Union false positives: input valid for one model alternative is rejected for violating another alternative’s constraints.
  • Custom methods aren’t preserved: inherited overrides get shadowed; own overrides still receive inappropriate checks against their raw input.
  • API regression: the synthesized signature rejects Pydantic 2.12’s supported extra= argument.
  • Incorrect alias selection: alias precedence and call-time by_alias/by_name controls aren’t respected.
  • Dictionary-spread false positives: an overwritten literal remains subject to constraint checking.

More detailed AI review report:

1. P1 — Eager recursive input-shape synthesis grows exponentially

Location: pydantic.rs, lines 261–280, recursive expansion, lines 330–355.

Every model eagerly constructs a recursively expanded input type. Removing a model from seen after each visit means shared descendants are reconstructed for each field: a chain of models with two fields pointing to the preceding model produces exponential work.

from pydantic import BaseModel

class M0(BaseModel):
    x: int

class M1(BaseModel):
    left: M0
    right: M0

class M2(BaseModel):
    left: M1
    right: M1

# Repeat through M14. No model_validate call is needed.

Measured total checker wall times on the same Linux node and fixture generator:

  • Depth 6, 29 project lines: parent 3.258s; PR 3.352s.
  • Depth 10, 45 project lines: parent 3.282s; PR 4.755s.
  • Depth 14, 61 project lines: parent 3.296s; PR 27.289s.

This fixture has no range constraints. The extra work also does not provide strict argument validation: object and dict[str, Any] already accept the input, and union normalization promotes anonymous TypedDict branches to ordinary dictionaries, discarding their field structure.

Fix: remove the eager structural input expansion and retain the existing permissive signature. If structural typing is needed separately, use specialization-aware memoization and explicit complexity bounds. Constraint checking should be lazy and should exit immediately when no relevant constraint is reachable.

2. P1 — Union alternatives are treated as simultaneous requirements

Location: pydantic.rs, lines 1208–1214; the type walker repeats this at lines 1297–1303.

class Positive(BaseModel):
    x: int = Field(ge=0)

class Negative(BaseModel):
    x: int = Field(le=-1)

class Either(BaseModel):
    item: Positive | Negative

Either.model_validate({'item': {'x': -1}})

Pydantic accepts the input as Negative. The PR reports a violation of Positive.x's ge=0 constraint. Both walkers descend into every expected union member and emit each member's failures independently.

Fix: respect alternative semantics. Emit a definitive constraint error only when all viable alternatives fail, or check the discriminator-selected branch. Conservatively skipping unsupported ambiguous unions is preferable to false positives.

3. P1 — Custom model_validate methods are not actually preserved

Two independently reproduced failures need fixing:

  • dataclass.rs, lines 224–236: the synthesis guard checks only members defined on the current class, so it shadows an inherited custom method.
  • pydantic.rs, lines 1061–1072: call-time checking recognizes any method named model_validate on a Pydantic class, including an own custom override that ignores or transforms its input.

Inherited-method repro:

class Parent(BaseModel):
    x: int

    @classmethod
    def model_validate(cls, obj: object = None, *, strict=None,
                       from_attributes=None, context=None,
                       by_alias=None, by_name=None) -> Self:
        return cls(x=1)

class Child(Parent):
    pass

Child.model_validate()

The PR reports missing obj; both the parent checker and Pydantic accept it. Separately, an own override returning cls(x=1) on a model with x = Field(ge=0) still causes Model.model_validate({'x': -1}) to be rejected, although the override never validates that input.

Fix: inspect the visible non-synthesized member through the MRO, following the existing model_construct approach, and restrict built-in constraint checking to the actual Pydantic implementation or explicitly tagged synthesized functions—not a name match.

4. P1 — The synthesized signature removes Pydantic 2.12's extra= argument

Location: pydantic.rs, lines 285–318.

class Model(BaseModel):
    x: int

Model.model_validate({'x': 1}, extra='forbid')

With Pydantic 2.12.0 installed, runtime execution succeeds and the parent checker reports no errors. The PR reports Unexpected keyword argument extra. Replacing the library signature with a hard-coded parameter list loses supported version-specific parameters and will drift again as the API evolves.

Fix: preserve the installed library's method signature rather than reconstructing it. If synthesis is necessary, derive the signature from the real method and change only the intended component.

5. P2 — Alias resolution chooses the wrong input value

Location: pydantic.rs, lines 1123–1133, duplicated at lines 1253–1263.

class Aliased(BaseModel):
    model_config = ConfigDict(validate_by_name=True, validate_by_alias=True)
    x: int = Field(ge=0, alias='y')

Aliased.model_validate({'x': -1, 'y': 1})

Pydantic gives the alias priority and accepts y=1; the PR checks x=-1. The walkers also ignore the call's by_alias and by_name controls. On an alias-only model, passing {'x': 1, 'y': -1} with by_name=True, by_alias=False is accepted at runtime but rejected by the PR.

Fix: centralize effective input-key selection, apply call-time controls over model configuration, and preserve Pydantic's alias precedence. Do not emit a definite error when dynamic options make the selected value uncertain.

6. P2 — Unknown dictionary spreads leave overridden literals marked as certain

Location: pydantic.rs, lines 1160–1167.

class Model(BaseModel):
    x: int = Field(ge=0)

patch: dict[str, int] = {'x': 1}
Model.model_validate({'x': -1, **patch})

Pydantic accepts the final x=1; the PR reports a violation at the overwritten -1. The literal-field map drops opaque unpackings instead of accounting for their ability to overwrite earlier keys.

Fix: preserve dictionary update ordering and key provenance. After an unresolved spread, earlier potentially overwritten keys are no longer certain and must not independently produce a definitive constraint violation.

Recommended implementation direction

Keep the existing library method and signature. Add a small, tagged/implementation-aware constraint-checking path that uses shared field/alias metadata, avoids re-inferring arguments when no constraints apply, and handles alternatives and uncertain input provenance conservatively. The current three recursive walkers duplicate logic while disagreeing about containers, aliases, and generic specialization. Avoid expanding this patch into complete Pydantic schema interpretation.

@stroxler stroxler 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.

whoops meant to request changes

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement static checking for model_validate in pydantic

4 participants