[ysheikh2]

Summary

In file mode and the language server, each file gets the config of its location. If a library with its own config and .venv is installed editable into an app's .venv, the library's imports resolve in its own .venv. The app then sees two copies of the same package and reports "DataFrame is not assignable to DataFrame". Project mode is not affected.

This PR checks such a library in the importer's environment:

• Handle has an optional, interned SitePackagePath of the importer's environment. It is set when a module is found under the importer's site-package path and its own config uses a different one. SysInfo and the stdlib are unchanged.
• State::get_config gives such a handle a copy of its location config with the importer's site-package path, so the library's imports resolve to the same modules as the app's.
• Some lookups from an app into the library build a handle without an import. They now use the handle the app's environment gives that module. These are class fields, class type parameters, go to definition on a method, go to type definition, and a class's docstring in hover.
• Find references, safe delete and incoming calls include every copy of a module. Workspace diagnostics and check --check-all report each file once, from its own environment.
• The docs describe the behaviour where they explain configuration selection in the IDE, configuration finding, the site package path and editable installs.

An open file keeps its own folder's environment. A library imported from two environments is checked once per environment, as in pyright and ty. A module in a .venv outside any config is checked in the importer's environment, and once more in the default config if it is opened directly. One case remains. A library found through search-path can use a type from its own environment. Looking up that type from the app can then load a second copy of its module.

Fixes #3556

• I am a human, and I fully understand the code I am submitting.

Prepared with an AI assistant (Claude) and reviewed by the author.

Test Plan

• New tests in pyrefly/lib/test/state.rs cover two apps sharing a library and a change of the library's environment. They also cover class fields through a function-local import, a .venv outside any config, IDE lookups and hover from an app, references, and --check-all errors. Each fails without the part of the change it covers.
• cargo test --profile bench and cargo clippy on Linux, the state tests on macOS, and the test.py lint. Not run on Windows.
• pyrefly check --threads 1, release build on Linux, median of 7 runs. The apps install the library editable into their own uv environments.

Setup	Instructions (billions)	Peak memory (MiB)	Errors
Project mode, PyTorch	107.67 → 108.33	568 → 571	529 → 529
Project mode, 200-module library with heavy dependencies	2.35 → 2.35	128 → 128	0 → 0
File mode, 1 app with that library	3.17 → 2.77	146 → 127	40 → 0
File mode, 4 apps with that library	5.77 → 7.04	223 → 266	160 → 0
File mode, 1 app, .venv outside any config	3.66 → 2.33	183 → 128	40 → 0
File mode, 4 apps, .venv outside any config	8.42 → 5.93	374 → 266	160 → 0
File mode, 2 apps with a 1000-module library	1.63 → 1.95	84 → 102	0 → 0
File mode, 8 apps with a 1000-module library	3.31 → 5.52	87 → 217	0 → 0

The extra cost comes from checking a library once per environment that imports it. mypy_primer runs single-environment project checks, so it doesn't exercise this change.

[meta-cla]

Hi @ysheikh2!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[kinto0]

Sorry about the delayed response. I think this breaks a lot of assumptions we have: in the IDE, which environment do we pick when a file is open? I won't have enough time to review this today or reply to the original issue unfortuately.

[ysheikh2]

Sorry about the delayed response. I think this breaks a lot of assumptions we have: in the IDE, which environment do we pick when a file is open? I won't have enough time to review this today or reply to the original issue unfortuately.

Hi, thanks for your feedback

I've never come across this being an issue in any other type checker for python, with basedpyright, pyright, mypy it's always just worked for these monorepo setups and/or multi-root VSCode workspaces. A lot of my team hits this with our monorepos, as does the reporter of #3556, and it also comes up in #4987.

The setup from #3556 is a VS Code workspace with two folders, each with its own interpreter:

app/   its own pyproject.toml and .venv, with lib as an editable dependency, which brings polars into that .venv
lib/   its own pyproject.toml and .venv, with polars as a dependency

Both .venvs therefore have the same polars version. app/main.py calls mylib.frames.width(pl.DataFrame()).

pyrefly picks a file's environment from where the file is, not from what imports it. polars in app/main.py comes from app/.venv, but polars in lib/mylib/frames.py comes from lib/.venv. The two DataFrame classes don't match, so pyrefly reports an error. pyrefly check app/main.py gives the same error without an editor.

To answer your question, an open file still uses its own folder's environment with this PR, so an open frames.py is checked with lib/.venv, as today. The PR adds a second check of frames.py with app/.venv, used only for app's results and never shown in the editor.

This is the same model the other type checkers use. pyright, basedpyright and ty use one environment per workspace folder, and every file a folder reaches, including a library installed as editable, is checked with that folder's environment. A file reached from two folders is analysed twice, once per folder. Eric Traut describes this in microsoft/pyright#4329 (comment). mypy uses one environment per run. This PR uses the importing environment instead of the workspace folder as the unit.

[meta-codesync]

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

[kinto0]

Hey, thanks again for the effort here. This makes sense and I do see the need for a change like this, specifically for editable installs / PEP 610 support.

Handle is the right place, but including this as part of sys_info clouds all other usage of sys_info like stdlib. Maybe we put it in handle directly. We also need to be careful this doesn't hurt performance much considering we are now checking many copies of dependencies.

Thanks for your patience: I'll have more time next week to go over this change, discuss with the team, and derisk it further.

[ysheikh2]

Thanks, that makes sense. I've moved it out of SysInfo into Handle.

• Handle now has an optional field with the importer's site-package path. It is set only when a module is found through the importer's site-package path and its own config uses a different one.
• An editable library with its own .venv is the usual case. State::get_config gives such a handle a copy of its location config with the importer's site-package path.
• SysInfo and the stdlib are unchanged, and in project mode every handle stays as it was.

Some further research and checks for how other libraries implement:

This matches ty. ty identifies a module by the file and the environment checking it, and resolves a library's imports in the importer's environment, so a library imported from two environments is checked twice (astral-sh/ty#819 (comment)). Two equal environments share one copy, as they do here. Pyright also takes site-packages from the importer, but takes extraPaths from the library file's own execution environment. Eric Traut has said he regrets execution environments (microsoft/pyright#7979 (comment)), and the ty maintainers cited that when they declined per-package settings (astral-sh/ty#693). That's why the site-package path here comes from the importer. The copy still uses the library's own search path, which I've left as it is.

On performance, I measured pyrefly check with a release build, median of 7 runs, before and after:

Setup	Instructions (billions)	Peak memory (MiB)
Project mode, 200-module library with heavy dependencies	2.25 → 2.25	129 → 129
File mode, 1 app with that library installed editable	2.65 → 2.26	148 → 129
File mode, 4 apps with that library	4.67 → 6.06	234 → 276
File mode, 8 apps with a 1000-module library	1.84 → 4.88	94 → 230

With one app the check takes 15% fewer instructions, because the library no longer loads a second copy of its dependencies from its own .venv. The cost grows when apps with different environments share a library, since the library is checked once per environment. ty and pyright pay the same cost. Deciding the handle and looking up its config take about 8% of the CPU samples in the 8-app case.

[kinto0]

Thanks for moving it into Handle, and for the numbers. Some feedback after going through it:

1. Why does the IDE need to change at all? I'd expect opening a library file to still check it as the library, in its own environment, and if someone wants to see it through the app's environment they can override the interpreter. Then hover/definition/call hierarchy, copies(), and merging references could all go away, and the change is just the Handle field, imported_handle, the derived configs in get_config, and caching Handles in imports.
2. Why the custom Hash on Handle? Leaving the path out puts every copy of a file in the same bucket, and Eq still compares each PathBuf. Handle is the key for all our hot maps, so I'd rather intern the site-package path so it's Copy, eq/hash are a pointer compare, and we can just derive Hash.
3. If we do keep the IDE changes: copies() scans every loaded handle on each get_rdeps/get_transitive_rdeps call, and find references calls those in a loop, so this gets quadratic on big workspaces. We'd want an index from the plain handle to its copies (or a benchmark showing it's fine).
4. Does the deferred_deps reuse in TransactionHandle::get_module show up in a benchmark? It's another lookup in a hot path, so I'd drop it unless it's measurable.
5. Can you add a test for a cross-module lookup that skips import resolution? e.g. the fields of a class returned through a function-local import (def f(): from mylib.b import B; return B()). That goes through get_class_fields, not import_handle.
6. What happens with a venv outside any config? Those modules get the default config, so every third-party module there gets a copy, and opening one directly (e.g. via go-to-definition) makes another one. A test or a note in the summary would help.

The workspace diagnostics filter in server.rs, the derived configs in invalidate_find_for_configs, and test_library_environment_change_rechecks_importer look good, let's keep those.

Not many bites on my discussion here and no issues bringing it up internally, so I think we're unblocked on landing this. Thanks again for your patience.

[ysheikh2]

Thanks for going through it, and for taking it to the team. I've merged the latest main and worked through the points. Benchmarks below are pyrefly check --threads 1 on Linux, release builds, median of 7 runs, in billions of instructions retired.

1. Why the IDE changes. An open library file is still checked as the library, in its own environment. The IDE changes are for the other direction, an app file looking up something in the library. The app's import gives the library's copy in the app's environment. A handle built with for_module from the app names the library's own module instead. That module isn't loaded unless the library file is open, so the lookup comes back empty.

I reverted each IDE change in turn and kept only the ones a test fails without:

Change	Test that fails without it
class_module and function_def_location use imported_handle	test_library_definitions_from_app: go to definition on a library method gives DefinitionNotFound, and go to type definition on a library function gives AnswersNotFound
copies() in get_rdeps and get_transitive_rdeps	test_library_references_from_each_environment and test_library_open_in_editor_references: find references and safe delete from the library miss the apps' uses
docstring_for_class_object_type in hover uses imported_handle	test_library_definitions_from_app: hover on k = mylib.K in the app shows no docstring for K

Keyword arguments, outgoing calls, merging incoming calls by file and the in-memory counterparts in find references gave the same results without their changes, so I've dropped those. An importer's copy of a library file is only a dependency, so it has no index for incoming calls to read.

2. Hash. The field is now an interned SitePackagePath, which is Copy, compares by pointer and hashes a precomputed hash. Handle derives Hash again. The full test suite passes with it, and PyTorch reports the same 529 errors as on main.

3. copies(). It now looks up one handle per interned site-package path (SitePackagePath::all()) instead of scanning every loaded handle. That's one lookup per Python environment, with no index to keep in sync.

4. The deferred_deps reuse. It is measurable. Without it, deciding the handle on every explicit-path lookup costs 11% in project mode.

Project mode	main	with reuse	without reuse
PyTorch	107.67	108.33 (+0.6%)	119.93 (+11.4%)
200-module library with heavy dependencies	2.35	2.35 (+0.3%)	2.41 (+2.6%)

I've rewritten it so it adds no lookup. The explicit-path branch reads the handle from the deferred_deps entry it already looks up to record the dependency.

5. Cross-module lookup without an import. Added test_library_class_fields_without_import. The library imports a class inside a function, and the app reads a field of an instance of it. It fails if the explicit-path lookup builds a plain handle. TransactionTypeEnv::class_tparams, added on main since, built a plain handle for the class's module in the same way, so it now uses the importer's handle too.

6. A venv outside any config. Added test_environment_outside_any_config. A module the app imports from such a venv is checked in the app's environment and resolves its own imports there. Opening that module directly, for example through go to definition, checks it a second time in the default config. With one app, this is cheaper than main, which already checked those modules through the default config and so loaded a second copy of their dependencies:

File mode, .venv outside any config	main	this PR
1 app	3.66, 40 errors	2.33, 0 errors
4 apps	8.42, 160 errors	5.93, 0 errors

I've added this to the summary.

One more fix from going over it again: pyrefly check --check-all reported a library's errors once per environment it was checked in. get_all_errors now reports each file once, from its own environment where it was checked there (test_library_all_errors_reported_once). I've also described the behaviour in the docs, in the sections on IDE configuration selection, configuration finding, the site package path and editable installs.