Skip to content

fix BaseModel.model_copy() seems to lose intersection data #3113#3547

Draft
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3113
Draft

fix BaseModel.model_copy() seems to lose intersection data #3113#3547
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3113

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

Summary

Fixes #3113

Test Plan

@github-actions
Copy link
Copy Markdown

Diff from mypy_primer, showing the effect of this PR on open source code:

steam.py (https://github.com/Gobot1234/steam.py)
+ ERROR steam/ext/commands/commands.py:782:44-46: Argument `Command[CogT] & GroupMixin` is not assignable to parameter `self` with type `Command & GroupMixin` in function `GroupMixin.remove_all_commands` [bad-argument-type]

strawberry (https://github.com/strawberry-graphql/strawberry)
- ERROR strawberry/relay/types.py:239:27-33: Argument `(self: LazyType[Unknown, Unknown]) -> type[Any] | type[Any]` is not assignable to parameter `cls` with type `type` in function `issubclass` [bad-argument-type]
+ ERROR strawberry/relay/types.py:239:27-33: Argument `(self: LazyType[Unknown, Unknown] & type[Any]) -> type[Any] | type[Any]` is not assignable to parameter `cls` with type `type` in function `issubclass` [bad-argument-type]
- ERROR strawberry/relay/types.py:244:16-22: Returned type `(self: LazyType[Unknown, Unknown]) -> type[Any] | type[Any]` is not assignable to declared return type `type[Node]` [bad-return]
+ ERROR strawberry/relay/types.py:244:16-22: Returned type `(self: LazyType[Unknown, Unknown] & type[Any]) -> type[Any] | type[Any]` is not assignable to declared return type `type[Node]` [bad-return]

static-frame (https://github.com/static-frame/static-frame)
+ ERROR static_frame/core/frame.py:9340:35-41: Argument `Frame & Self@Frame` is not assignable to parameter `self` with type `Frame & Self@Frame` in function `Frame.rename` [bad-argument-type]

mypy (https://github.com/python/mypy)
- ERROR mypy/checkmember.py:1523:12-1528:6: Returned type `CallableType` is not assignable to declared return type `F` [bad-return]

discord.py (https://github.com/Rapptz/discord.py)
+ ERROR discord/ext/commands/core.py:1355:56-58: Argument `Command[CogT, Ellipsis, Any] & GroupMixin[Unknown]` is not assignable to parameter `self` with type `Command[Unknown, Ellipsis, Any] & GroupMixin[Unknown]` in function `GroupMixin.recursively_remove_all_commands` [bad-argument-type]
+ ERROR discord/ext/commands/core.py:1455:49-51: Argument `Command[CogT, Ellipsis, Any] & GroupMixin[Unknown]` is not assignable to parameter `self` with type `Command[Unknown, Ellipsis, Any] & GroupMixin[Unknown]` in function `GroupMixin.walk_commands` [bad-argument-type]

artigraph (https://github.com/artigraph/artigraph)
- ERROR src/arti/types/pydantic.py:27:16-23: Returned type `Type | _NamedMixin` is not assignable to declared return type `Type` [bad-return]

@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 22, 2026 09:07
Copilot AI review requested due to automatic review settings May 22, 2026 09:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

❌ 3 regression(s) | ✅ 2 improvement(s) | ➖ 1 neutral | 6 project(s) total | +6, -4 errors

3 regression(s) across steam.py, static-frame, discord.py. error kinds: bad-argument-type. caused by as_attribute_base1(), lookup_attr_from_attribute_base1_with_self_type(), remove_all_commands(). 2 improvement(s) across mypy, artigraph.

Project Verdict Changes Error Kinds Root Cause
steam.py ❌ Regression +1 bad-argument-type remove_all_commands()
strawberry ➖ Neutral +2, -2 bad-argument-type, bad-return
static-frame ❌ Regression +1 bad-argument-type as_attribute_base1()
mypy ✅ Improvement -1 bad-return lookup_attr_from_attribute_base1_with_self_type()
discord.py ❌ Regression +2 bad-argument-type lookup_attr_from_attribute_base1_with_self_type()
artigraph ✅ Improvement -1 bad-return as_attribute_base1()
Detailed analysis

❌ Regression (3)

steam.py (+1)

The error is a false positive introduced by the PR's changes to intersection type self-type propagation. The narrowed type Command[CogT] & GroupMixin should be assignable to the self parameter of GroupMixin.remove_all_commands(), which expects GroupMixin[CogT]. The mismatch between Command[CogT] & GroupMixin and Command & GroupMixin in the error message suggests the self-type resolution is dropping generic parameters during intersection handling.
Attribution: The PR changes intersection type handling in pyrefly/lib/alt/attr.rs by adding a self_type field to AttributeBase1::Intersect and using lookup_attr_from_attribute_base1_with_self_type / get_instance_attribute_with_self_type / get_enum_or_instance_attribute_with_self_type in pyrefly/lib/alt/class/class_field.rs and pyrefly/lib/alt/class/enums.rs. The new self_type propagation through intersection types is causing the self parameter of remove_all_commands() to be resolved as Command & GroupMixin (without the CogT parameter) instead of properly matching the narrowed intersection type Command[CogT] & GroupMixin.

static-frame (+1)

The error message shows Frame & Self@Frame is not assignable to Frame & Self@Frame — the same type on both sides. This is clearly a false positive caused by the PR's new intersection-aware Self resolution creating a type that fails an internal identity/assignability check. The code is correct, and neither mypy nor pyright flag it.
Attribution: The change to as_attribute_base1() in pyrefly/lib/alt/attr.rs now wraps intersection types with Some(self_type) in AttributeBase1::Intersect, and the new lookup_attr_from_attribute_base1_with_self_type() method resolves attributes using get_instance_attribute_with_self_type() / get_enum_or_instance_attribute_with_self_type(). This new self-type-aware lookup path appears to produce a type that, while textually identical (Frame & Self@Frame), fails pyrefly's internal assignability check — likely because the intersection type created during the lookup is structurally different from the one expected by the parameter.

discord.py (+2)

The PR's intersection self-type handling incorrectly computes the expected self parameter type for methods called on intersection types. The expected type shows Command[Unknown, ...] where it should show Command[CogT, ...], causing a false bad-argument-type error. Both errors are pyrefly-only, confirming these are regressions.
Attribution: The change to lookup_attr_from_attribute_base1_with_self_type() in pyrefly/lib/alt/attr.rs and the addition of get_instance_attribute_with_self_type() in pyrefly/lib/alt/class/class_field.rs introduced intersection self-type propagation. When resolving method calls on intersection types like Command[CogT, ..., Any] & GroupMixin[Unknown], the new code passes the intersection as the self_type via Instance::of_protocol(cls, self_type), which appears to incorrectly resolve the CogT type parameter to Unknown in the expected self parameter type, creating a spurious CogT vs Unknown mismatch.

✅ Improvement (2)

mypy (-1)

This was a false positive removal. The function bind_self_fast correctly handles both Overloaded (via recursion + cast on line 1515) and CallableType (via copy_modified on line 1523) cases. After the assert isinstance(method, CallableType) narrowing, returning CallableType is correct. Mypy's own codebase doesn't flag this, confirming it's a false positive.
Attribution: The PR changes to pyrefly/lib/alt/attr.rs modified AttributeBase1::Intersect to carry a self_type and added lookup_attr_from_attribute_base1_with_self_type(). The changes to type resolution for intersection types likely improved how pyrefly handles generic method return types when Self types are involved, which indirectly fixed this false positive about CallableType not being assignable to F.

artigraph (-1)

This is a clear improvement. The removed error was a false positive caused by pyrefly incorrectly handling Self-returning methods on intersection types. On line 24, subtype has type Type (the return type of to_artigraph). After the isinstance(subtype, _NamedMixin) check on line 25, inside the if-block subtype is narrowed to Type & _NamedMixin (an intersection type). Calling model_copy() on this intersection type should return Self, which should resolve to Type & _NamedMixin — still a subtype of Type. After the if-block at line 27, the type of subtype is the join of the two branches: the else-branch where it's Type, and the if-branch where it's the result of model_copy(). If model_copy() correctly returns Type & _NamedMixin, the join would be Type (since the intersection is a subtype of Type), which is assignable to the declared return type Type. However, pyrefly was incorrectly resolving Self on the intersection type, producing Type | _NamedMixin (a union of the intersection's components) instead of Type & _NamedMixin. This caused the join at line 27 to be Type | _NamedMixin, which is not assignable to Type (since _NamedMixin alone is not necessarily a Type). The PR fixes this by correctly propagating the self type through intersection attribute lookups, so model_copy() on Type & _NamedMixin correctly returns Type & _NamedMixin, making the overall return type Type.
Attribution: The fix is in pyrefly/lib/alt/attr.rs. The AttributeBase1::Intersect variant now carries an Option<Type> representing the self type. In as_attribute_base1() for Type::Intersect, the self type is now stored as Some(self_type). The new method lookup_attr_from_attribute_base1_with_self_type() uses this self type when resolving attributes on intersection components, which correctly propagates the intersection type as the Self type for methods like model_copy(). The companion changes in pyrefly/lib/alt/class/class_field.rs (get_instance_attribute_with_self_type()) and pyrefly/lib/alt/class/enums.rs (get_enum_or_instance_attribute_with_self_type()) provide the underlying attribute resolution with the correct self type. The test case test_self_returning_method_on_typevar_intersection in pyrefly/lib/test/narrow.rs directly demonstrates this fix.

➖ Neutral (1)

strawberry (+2, -2)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.

Suggested fixes

Summary: The PR's new intersection self-type propagation via Instance::of_protocol(cls, self_type) drops generic parameters from individual intersection components, causing false positive bad-argument-type errors in 3 projects.

1. In get_instance_attribute_with_self_type() in pyrefly/lib/alt/class/class_field.rs, the method uses Instance::of_protocol(cls, self_type) to create the instance for attribute resolution. The of_protocol constructor appears to replace the generic parameters of cls with those derived from the protocol self_type, but when cls is a generic class like Command[CogT] that is part of an intersection Command[CogT] & GroupMixin, the generic parameter CogT gets lost (replaced with Unknown or dropped entirely). The fix should preserve the original generic parameters of cls while still using self_type for Self-type resolution. Specifically, instead of Instance::of_protocol(cls, self_type), the code should use a mechanism that sets the self-type for Self resolution but keeps the original class's type parameters intact — e.g., something like Instance::of_class_with_self_type(cls, self_type) or conditionally use of_protocol only when the method actually returns Self (has a Self type in its signature). As a simpler immediate fix: when constructing the Instance, check if cls already has concrete/bound type parameters, and if so, use Instance::of_class(cls) as the base and only apply the self_type substitution to Self-typed positions in the resolved attribute, rather than passing it through of_protocol which appears to affect all type parameter resolution.

Files: pyrefly/lib/alt/class/class_field.rs, pyrefly/lib/alt/class/enums.rs
Confidence: high
Affected projects: steam.py, static-frame, discord.py
Fixes: bad-argument-type
All 3 regression projects (steam.py, static-frame, discord.py) show the same root cause: when resolving methods on intersection types, the generic type parameters of individual intersection components are being lost or replaced with Unknown. The error messages confirm this: steam.py shows Command[CogT] & GroupMixin becoming Command & GroupMixin (parameters dropped), discord.py shows CogT becoming Unknown, and static-frame shows Frame & Self@Frame failing assignability to itself (structural identity issue). All 4 regression errors are pyrefly-only (0/4 mypy, 0/4 pyright). The Instance::of_protocol(cls, self_type) call in get_instance_attribute_with_self_type() is the common code path for all these cases — it's used by both lookup_attr_from_attribute_base1_with_self_type for ClassInstance and Quantified/SelfType bases. The of_protocol constructor is designed for protocol matching where the self_type overrides type resolution, but here it's being repurposed for intersection self-type propagation, which has the side effect of corrupting the original class's generic parameters. This fix would eliminate all 4 regression errors across the 3 affected projects while preserving the improvements in artigraph and mypy (which work because they involve Self-returning methods where the self_type propagation is correct).


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (1 heuristic, 5 LLM)

@asukaminato0721 asukaminato0721 marked this pull request as draft May 22, 2026 09:13
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.

BaseModel.model_copy() seems to lose intersection data

2 participants