Skip to content

impl Add support for Django reverse relationships #2071#2074

Open
asukaminato0721 wants to merge 8 commits into
facebook:mainfrom
asukaminato0721:2071
Open

impl Add support for Django reverse relationships #2071#2074
asukaminato0721 wants to merge 8 commits into
facebook:mainfrom
asukaminato0721:2071

Conversation

@asukaminato0721

@asukaminato0721 asukaminato0721 commented Jan 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2071

Implemented Django reverse relationship synthesis by scanning model field bindings in the module, inferring reverse accessors for ForeignKey/ManyToManyField (and OneToOneField), parsing related_name (including %(class)s/%(app_label)s), and adding a RelatedManager helper for reverse FK accessors.

Notes/limits:

Reverse accessors are synthesized only from relation fields declared in the same module and with literal related_name/None (dynamic names are skipped).

Cross-module reverse inference isn’t covered yet. supported.

Test Plan

Added assertions for reverse accessors

@meta-cla meta-cla Bot added the cla signed label Jan 12, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 12, 2026 12:12
Copilot AI review requested due to automatic review settings January 12, 2026 12:12

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 pull request implements support for Django reverse relationships, addressing issue #2071. The implementation synthesizes reverse accessors for ForeignKey, ManyToManyField, and OneToOneField by scanning model field bindings, inferring reverse accessors, parsing related_name (including template substitution for %(class)s and %(app_label)s), and adding a RelatedManager helper for reverse ForeignKey accessors.

Changes:

  • Added reverse relationship field synthesis for Django models
  • Implemented related_name template parsing with %(class)s and %(app_label)s support
  • Added RelatedManager type support for ForeignKey reverse accessors

Reviewed changes

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

File Description
pyrefly/lib/alt/class/django.rs Core implementation of reverse relationship synthesis including relation kind detection, related_name parsing with template substitution, and manager type resolution
pyrefly/lib/test/django/foreign_key.rs Added test assertion for ForeignKey reverse accessor (article_set) on Reporter model
pyrefly/lib/test/django/many_to_many.rs Added test assertion for ManyToManyField reverse accessor (books) on Author model

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

@github-actions

This comment has been minimized.

@migeed-z

Copy link
Copy Markdown
Contributor

Thank you so much! I will import and take a closer look.

@meta-codesync

meta-codesync Bot commented Jan 12, 2026

Copy link
Copy Markdown
Contributor

@migeed-z has imported this pull request. If you are a Meta employee, you can view this in D90513475.

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

Review automatically exported from Phabricator review in Meta.

@migeed-z

Copy link
Copy Markdown
Contributor

Hello! we got to review the diff in more details. Here are some open questions:

  • The implementation requires iterating over every single ClassField binding. Can we do better? this would be expensive for large django models. Can we store do one pass in the bindings phase and collect ForeignKey, ManyToManyField, and OneToOneField assignments and store the names of those fields in class metadata and go from there?
  • Cross module support seems important to me on realistic codebases and ultimately the goal. Is this approach going in the right direction towards that?
@asukaminato0721

Copy link
Copy Markdown
Contributor Author

Moved Django reverse-relationship synthesis to a module-level binding/index so we compute reverse fields once per module (instead of scanning every KeyClassField for each model).

The binder now records candidate relation fields during the class-body pass, and the solver builds a reverse-relation index keyed by target class that get_django_model_synthesized_fields can query.

@github-actions

This comment has been minimized.

@asukaminato0721 asukaminato0721 marked this pull request as draft January 15, 2026 03:12
@asukaminato0721

Copy link
Copy Markdown
Contributor Author

Started cross‑module support by building a global reverse‑relation index once per solve and caching it in the thread state. The index merges each module’s KeyDjangoRelations result, so reverse accessors now come from all modules loaded in the current transaction (not just the current module).

This required exporting KeyDjangoRelations and adding LookupAnswer::modules() so the solver can enumerate available modules and combine their per‑module indexes.

@asukaminato0721 asukaminato0721 marked this pull request as ready for review January 15, 2026 03:28
@github-actions

This comment has been minimized.

Comment thread pyrefly/lib/test/django/many_to_many.rs Outdated
@github-actions

This comment has been minimized.

@migeed-z

Copy link
Copy Markdown
Contributor

Thank you so much for the updates. We are currently making some changes to incremental checks and since this change will affect incremental, we need to hold off on merging till the changes are made. But I reviewed the diff in detail and the high level approach looks pretty good, so once those changes are made, we should be good to go :)

@yangdanny97

yangdanny97 commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

cc @kinto0 this involves exports changes

at a high level its how i would expect it to be solved, but will likely need modifications to work properly w/ incremental checking

we need to make sure that the reverse dependency edge actually gets added, and make sure that creating a bunch of small cycles doesn't kill perf

@asukaminato0721 we're in the middle of a fairly large change to the exports logic, so we'll give this a closer look after we finish that

@github-actions

This comment has been minimized.

@kinto0

kinto0 commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

cc @kinto0 this involves exports changes

at a high level its how i would expect it to be solved, but will likely need modifications to work properly w/ incremental checking

we need to make sure that the reverse dependency edge actually gets added, and make sure that creating a bunch of small cycles doesn't kill perf

@asukaminato0721 we're in the middle of a fairly large change to the exports logic, so we'll give this a closer look after we finish that

thanks for the tag. once my code lands, we will need to make sure that KeyDjangoRelations updates state correctly.

if you want to get a headstart (though no guarantee this will stay the same so it's probably worth waiting), you will need to do something similar to #2202 for this new key. and you'll need tests similar to #2203

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@SHxKM

SHxKM commented May 26, 2026

Copy link
Copy Markdown

This is great. Are there any plans to merge this?

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

Primer Diff Classification

✅ 1 improvement(s) | 1 project(s) total | +5, -15 errors

1 improvement(s) across zulip.

Project Verdict Changes Error Kinds Root Cause
zulip ✅ Improvement +5, -15 Removed missing-attribute on Django reverse relations solve_django_reverse_relations()
Detailed analysis

✅ Improvement (1)

zulip (+5, -15)

Removed missing-attribute on Django reverse relations: All 15 removed errors were false positives for Django reverse relationship attributes (attachment_set, submessage_set, direct_groups, etc.) that exist at runtime. The PR correctly synthesizes these. Clear improvement.
New missing-attribute on ScheduledMessage.attachment_set: 3 errors where code accesses attachment_set on Message | ScheduledMessage without narrowing. Pyright confirms these. ScheduledMessage likely lacks this reverse relation. Improvement - real type issue.
New missing-attribute on UserGroup.named_user_group: The NamedUserGroup model defines usergroup_ptr with related_name='named_user_group' pointing to UserGroup (groups.py line 53-63). This reverse relation should be synthesized but isn't. The code guards with hasattr() anyway. False positive - regression.
New bad-return on get_realm_domains: Django .values('domain', 'allow_subdomains') returns ValuesQuerySet which produces dict[str, Any], not RealmDomainDict TypedDict. This is a known Django typing limitation. Neither mypy nor pyright flag this. False positive - regression.

Overall: Net change: -15 false positives removed, +3 true positives added (ScheduledMessage.attachment_set confirmed by pyright), +2 false positives added (UserGroup.named_user_group reverse relation exists but not synthesized; bad-return from Django .values() typing). Overall net improvement of 13 fewer false positives.

Per-category reasoning:

  • Removed missing-attribute on Django reverse relations: All 15 removed errors were false positives for Django reverse relationship attributes (attachment_set, submessage_set, direct_groups, etc.) that exist at runtime. The PR correctly synthesizes these. Clear improvement.
  • New missing-attribute on ScheduledMessage.attachment_set: 3 errors where code accesses attachment_set on Message | ScheduledMessage without narrowing. Pyright confirms these. ScheduledMessage likely lacks this reverse relation. Improvement - real type issue.
  • New missing-attribute on UserGroup.named_user_group: The NamedUserGroup model defines usergroup_ptr with related_name='named_user_group' pointing to UserGroup (groups.py line 53-63). This reverse relation should be synthesized but isn't. The code guards with hasattr() anyway. False positive - regression.
  • New bad-return on get_realm_domains: Django .values('domain', 'allow_subdomains') returns ValuesQuerySet which produces dict[str, Any], not RealmDomainDict TypedDict. This is a known Django typing limitation. Neither mypy nor pyright flag this. False positive - regression.

Attribution: The changes in pyrefly/lib/alt/class/django.rs (new solve_django_reverse_relations(), get_related_manager_type(), django_related_name() methods) and pyrefly/lib/binding/class.rs (record_django_relation_field()) implement Django reverse relation synthesis. The KeyDjangoRelations binding in pyrefly/lib/binding/binding.rs and cross-module support in pyrefly/lib/alt/answers_solver.rs (django_reverse_relations_index()) enable the feature across modules. This correctly removes 15 false positives but introduces 2 false positives (UserGroup.named_user_group not being synthesized, and bad-return from .values() type inference change).


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (1 LLM)

@rchen152

Copy link
Copy Markdown
Contributor

This is great. Are there any plans to merge this?

Sorry for the lack of movement on this one. Yes, I do still intend to review and merge this; it's just a very large PR and we're rather backed up on reviews.

@github-actions

Copy link
Copy Markdown

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

zulip (https://github.com/zulip/zulip)
- ERROR corporate/views/support.py:951:13-42: Object of class `RemoteZulipServer` has no attribute `remoterealm_set` [missing-attribute]
- ERROR zerver/actions/create_user.py:809:26-52: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
+ ERROR zerver/actions/create_user.py:822:28-55: Object of class `UserGroup` has no attribute `named_user_group` [missing-attribute]
- ERROR zerver/actions/uploads.py:120:44-66: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:120:44-66: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/uploads.py:131:9-31: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:131:9-31: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/uploads.py:143:35-57: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:143:35-57: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/users.py:433:35-61: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/user_groups.py:791:17-43: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/user_groups.py:885:21-47: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/widget.py:153:12-34: Object of class `Message` has no attribute `submessage_set` [missing-attribute]
- ERROR zerver/management/commands/export_search.py:296:35-57: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/models/realms.py:1437:17-38: Object of class `Realm` has no attribute `realmdomain_set` [missing-attribute]
+ ERROR zerver/models/realms.py:1437:12-76: Returned type `list[dict[str, Any]]` is not assignable to declared return type `list[RealmDomainDict]` [bad-return]
- ERROR zerver/tests/test_custom_profile_data.py:473:26-66: Object of class `UserProfile` has no attribute `customprofilefieldvalue_set` [missing-attribute]
- ERROR zerver/tests/test_custom_profile_data.py:478:26-66: Object of class `UserProfile` has no attribute `customprofilefieldvalue_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5939:25-43: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5940:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5944:25-43: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5945:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5949:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
@github-actions

Copy link
Copy Markdown

Primer Diff Classification

✅ 1 improvement(s) | 1 project(s) total | +5, -15 errors

1 improvement(s) across zulip.

Project Verdict Changes Error Kinds Root Cause
zulip ✅ Improvement +5, -15 Removed missing-attribute on Django reverse relations solve_django_reverse_relations()
Detailed analysis

✅ Improvement (1)

zulip (+5, -15)

Removed missing-attribute on Django reverse relations: All 15 removed errors were false positives for Django reverse relationship attributes (e.g., attachment_set, submessage_set, direct_groups, realmdomain_set). These are automatically created by Django's ORM. Removing them is an improvement.
New missing-attribute on ScheduledMessage.attachment_set: 3 errors confirmed by pyright (3/3 cross-check). The function check_attachment_reference_change takes Message | ScheduledMessage but accesses attachment_set which exists on Message via a ManyToManyField reverse relation from Attachment. ScheduledMessage has a separate scheduled_message relation from Attachment, not attachment_set. These are real type issues being correctly caught.
New missing-attribute on UserGroup.named_user_group: This is a false positive. The access at line 822 (user_group.named_user_group) is safe because user_group iterates over named_user_groups (line 820), which was populated only with groups that passed the hasattr(group, 'named_user_group') check at line 815. The named_user_group attribute is a OneToOneField reverse relation from NamedUserGroup to UserGroup, which the PR now correctly synthesizes on UserGroup. However, the type checker cannot narrow the type through the hasattr-based list filtering pattern to know the attribute is guaranteed to exist in the loop. Pyright does NOT flag this (0/1 cross-check), confirming it's a false positive.
New bad-return on realms.py: list[dict[str, Any]] not assignable to list[RealmDomainDict]. This is pyrefly-only (0/1 in both mypy and pyright) and likely a pre-existing issue that became visible when realmdomain_set was resolved, allowing the type checker to analyze the return expression. TypedDict assignability from dict[str, Any] is debatable - dict[str, Any] should be assignable to a TypedDict in many practical scenarios, but strict type checkers may reject it.

Overall: Net result: 15 false positives removed, 3 real bugs caught (ScheduledMessage.attachment_set confirmed by pyright), 1 false positive (UserGroup.named_user_group guarded by hasattr and confirmed not flagged by pyright), and 1 questionable bad-return. The overall impact is strongly positive - the PR correctly implements Django reverse relationship synthesis.

Attribution: The changes in pyrefly/lib/alt/class/django.rs (new solve_django_reverse_relations(), get_related_manager_type(), django_related_name() methods) and pyrefly/lib/binding/class.rs (record_django_relation_field()) implement Django reverse relationship synthesis. The pyrefly/lib/state/state.rs changes add cross-module support via modules() and DjangoReverseRelationIndex. This resolved 15 false positives but exposed 3 real issues on ScheduledMessage and 2 edge cases.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (1 LLM)

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

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

zulip (https://github.com/zulip/zulip)
- ERROR corporate/views/support.py:951:13-42: Object of class `RemoteZulipServer` has no attribute `remoterealm_set` [missing-attribute]
- ERROR zerver/actions/create_user.py:809:26-52: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
+ ERROR zerver/actions/create_user.py:822:28-55: Object of class `UserGroup` has no attribute `named_user_group` [missing-attribute]
- ERROR zerver/actions/uploads.py:120:44-66: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:120:44-66: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/uploads.py:131:9-31: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:131:9-31: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/uploads.py:143:35-57: Object of class `Message` has no attribute `attachment_set`
+ ERROR zerver/actions/uploads.py:143:35-57: Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- Object of class `ScheduledMessage` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/actions/users.py:433:35-61: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/user_groups.py:791:17-43: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/user_groups.py:885:21-47: Object of class `UserProfile` has no attribute `direct_groups` [missing-attribute]
- ERROR zerver/lib/widget.py:153:12-34: Object of class `Message` has no attribute `submessage_set` [missing-attribute]
- ERROR zerver/management/commands/export_search.py:296:35-57: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/models/realms.py:1437:17-38: Object of class `Realm` has no attribute `realmdomain_set` [missing-attribute]
+ ERROR zerver/models/realms.py:1437:12-76: Returned type `list[dict[str, Any]]` is not assignable to declared return type `list[RealmDomainDict]` [bad-return]
- ERROR zerver/tests/test_custom_profile_data.py:473:26-66: Object of class `UserProfile` has no attribute `customprofilefieldvalue_set` [missing-attribute]
- ERROR zerver/tests/test_custom_profile_data.py:478:26-66: Object of class `UserProfile` has no attribute `customprofilefieldvalue_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5939:25-43: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5940:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5944:25-43: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5945:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
- ERROR zerver/tests/test_message_fetch.py:5949:26-44: Object of class `Message` has no attribute `attachment_set` [missing-attribute]
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Primer Diff Classification

✅ 1 improvement(s) | 1 project(s) total | +5, -15 errors

1 improvement(s) across zulip.

Project Verdict Changes Error Kinds Root Cause
zulip ✅ Improvement +5, -15 Removed missing-attribute on Django reverse accessors solve_django_reverse_relations()
Detailed analysis

✅ Improvement (1)

zulip (+5, -15)

Removed missing-attribute on Django reverse accessors: All 15 removed errors were false positives for legitimate Django reverse relationship attributes (attachment_set, submessage_set, direct_groups, customprofilefieldvalue_set, remoterealm_set, realmdomain_set). These are created by Django's ORM at runtime. Improvement.
New missing-attribute on ScheduledMessage.attachment_set: 3 errors where code accesses attachment_set on Message | ScheduledMessage without narrowing. Pyright confirms 3/3. The Attachment model's FK points to Message, not ScheduledMessage, so ScheduledMessage legitimately lacks this reverse accessor. This is a real type narrowing issue. Improvement.
New missing-attribute on UserGroup.named_user_group: 1 error where code accesses named_user_group after hasattr guard. Neither mypy nor pyright flag this. The NamedUserGroup model has a OneToOneField to UserGroup, so the reverse accessor should exist. This may be an incomplete synthesis or the hasattr guard not being respected. Likely a false positive. Regression.
New bad-return on RealmDomainDict: 1 error about list[dict[str, Any]] not assignable to list[RealmDomainDict]. Neither mypy nor pyright flag this. Appears unrelated to the Django reverse relations feature. Likely a pre-existing pyrefly strictness issue that surfaced. Regression.

Overall: Net improvement: 15 false positives removed, 3 new errors confirmed by pyright (likely real narrowing issues with ScheduledMessage), 2 pyrefly-only new errors (1 likely false positive due to hasattr guard not narrowing, 1 unrelated bad-return). The overall direction is strongly positive — the PR correctly implements Django reverse relationship synthesis, fixing a major class of false positives.

Per-category reasoning:

  • Removed missing-attribute on Django reverse accessors: All 15 removed errors were false positives for legitimate Django reverse relationship attributes (attachment_set, submessage_set, direct_groups, customprofilefieldvalue_set, remoterealm_set, realmdomain_set). These are created by Django's ORM at runtime. Improvement.
  • New missing-attribute on ScheduledMessage.attachment_set: 3 errors where code accesses attachment_set on Message | ScheduledMessage without narrowing. Pyright confirms 3/3. The Attachment model's FK points to Message, not ScheduledMessage, so ScheduledMessage legitimately lacks this reverse accessor. This is a real type narrowing issue. Improvement.
  • New missing-attribute on UserGroup.named_user_group: 1 error where code accesses named_user_group after hasattr guard. Neither mypy nor pyright flag this. The NamedUserGroup model has a OneToOneField to UserGroup, so the reverse accessor should exist. This may be an incomplete synthesis or the hasattr guard not being respected. Likely a false positive. Regression.
  • New bad-return on RealmDomainDict: 1 error about list[dict[str, Any]] not assignable to list[RealmDomainDict]. Neither mypy nor pyright flag this. Appears unrelated to the Django reverse relations feature. Likely a pre-existing pyrefly strictness issue that surfaced. Regression.

Attribution: The changes in pyrefly/lib/alt/class/django.rs (new solve_django_reverse_relations(), get_related_manager_type(), django_related_name(), etc.) and pyrefly/lib/binding/class.rs (record_django_relation_field()) implement Django reverse relationship synthesis. The pyrefly/lib/state/state.rs changes enable cross-module reverse relation lookup via modules() on LookupAnswer. This directly caused the 15 false positive removals. The 3 new ScheduledMessage errors arise because Message now has attachment_set synthesized but ScheduledMessage doesn't (different FK relationship). The UserGroup error may be an incomplete cross-module synthesis.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (1 LLM)

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