[ty] Initial support for workspace diagnostics #18939

dhruvmanila · 2025-06-25T16:30:25Z

Summary

This PR adds initial support for workspace diagnostics in the ty server.

Reference spec: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_diagnostic

This is currently implemented via the pull diagnostics method which was added in the current version (3.17) and the server advertises it via the diagnosticProvider.workspaceDiagnostics server capability.

Note: This might be a bit confusing but a workspace diagnostics is not for a single workspace but for all the workspaces that the server handles. These are the ones that the server received during initialization. Currently, the ty server doesn't support multiple workspaces so this capability is also limited to provide diagnostics only for a single workspace (the first one if the client provided multiple).

A new ty.diagnosticMode server setting is added which can be either workspace (for workspace diagnostics) or openFilesOnly (for checking only open files) (default). This is same as python.analysis.diagnosticMode that Pyright / Pylance utilizes. In the future, we could use the value under python.* namespace as fallback to improve the experience on user side to avoid setting the value multiple times.

Part of: astral-sh/ty#81

Test Plan

This capability was introduced in the current LSP version (~3 years) and the way it's implemented by various clients are a bit different. I've provided notes on what I've noticed and what would need to be done on our side to further improve the experience.

VS Code

VS Code sends the workspace/diagnostic requests every ~2 second:

[Trace - 12:12:32 PM] Sending request 'workspace/diagnostic - (403)'.
[Trace - 12:12:32 PM] Received response 'workspace/diagnostic - (403)' in 2ms.
[Trace - 12:12:34 PM] Sending request 'workspace/diagnostic - (404)'.
[Trace - 12:12:34 PM] Received response 'workspace/diagnostic - (404)' in 2ms.
[Trace - 12:12:36 PM] Sending request 'workspace/diagnostic - (405)'.
[Trace - 12:12:36 PM] Received response 'workspace/diagnostic - (405)' in 2ms.
[Trace - 12:12:38 PM] Sending request 'workspace/diagnostic - (406)'.
[Trace - 12:12:38 PM] Received response 'workspace/diagnostic - (406)' in 3ms.
[Trace - 12:12:40 PM] Sending request 'workspace/diagnostic - (407)'.
[Trace - 12:12:40 PM] Received response 'workspace/diagnostic - (407)' in 2ms.
...

I couldn't really find any resource that explains this behavior. But, this does mean that we'd need to implement the caching layer via the previous result ids sooner. This will allow the server to avoid sending all the diagnostics on every request and instead just send a response stating that the diagnostics hasn't changed yet. This could possibly be achieved by using the salsa ID.

If we switch from workspace diagnostics to open-files diagnostics, the server would send the diagnostics only via the textDocument/diagnostic endpoint. Here, when a document containing the diagnostic is closed, the server would send a publish diagnostics notification with an empty list of diagnostics to clear the diagnostics from that document. The issue is the VS Code doesn't seem to be clearing the diagnostics in this case even though it receives the notification. (I'm going to open an issue on VS Code side for this today.)

Screen.Recording.2025-06-27.at.19.50.33.mov

Zed

Zed's implementation works by refreshing the workspace diagnostics whenever the content of the documents are changed. This seems like a very reasonable behavior and I was a bit surprised that VS Code didn't use this heuristic.

Screen.Recording.2025-06-27.at.19.53.27.mov

Neovim

Neovim only recently added support for workspace diagnostics (neovim/neovim#34262, merged ~3 weeks ago) so it's only available on nightly versions.

The initial support is limited and requires fetching the workspace diagnostics manually as demonstrated in the video. It doesn't support refreshing the workspace diagnostics either, so that would need to be done manually as well. I'm assuming that these are just a temporary limitation and will be implemented before the stable release.

Screen.Recording.2025-06-30.at.12.07.38.mov

github-actions · 2025-06-25T16:33:38Z

`mypy_primer` results

Changes were detected when running on open source projects

operator (https://github.com/canonical/operator)
- TOTAL MEMORY USAGE: ~97MB
+ TOTAL MEMORY USAGE: ~106MB
-     memo fields = ~80MB
+     memo fields = ~88MB

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- TOTAL MEMORY USAGE: ~80MB
+ TOTAL MEMORY USAGE: ~88MB

discord.py (https://github.com/Rapptz/discord.py)
-     memo fields = ~189MB
+     memo fields = ~207MB

mkdocs (https://github.com/mkdocs/mkdocs)
- TOTAL MEMORY USAGE: ~117MB
+ TOTAL MEMORY USAGE: ~129MB

cwltool (https://github.com/common-workflow-language/cwltool)
-     memo fields = ~207MB
+     memo fields = ~228MB

MichaReiser · 2025-06-25T16:36:20Z

crates/ty_project/src/db.rs

+    /// Checks all files in the workspace, ignoring open files setting.
+    ///
+    /// This is used to power the workspace diagnostics capability in the language server.
+    pub fn check_all_files(&self) -> Vec<Diagnostic> {


I think I'd prefer not to introduce this new method and instead:

a) Remove the behavior that open files has today and make check always check all files (the LSP can call check_file if it only wants diagnostics for open files). The downside of this is that we may need another way to surface setting diagnostics
b) Add a CheckMode field

I actually thought of changing the current logic something like (b). I don't think we should do (a) as the playground checks only the files that are marked open. The LSP already uses check_file for pull diagnostic.

I also realised that it's a bit more complicated because of virtual files... we can never discover them by walking the file try. That means we'll need to check the open files when calling check or we'll miss those files.

Oh good point. I think that'll require astral-sh/ty#619 because currently the server uses the check_file method directly but to include the virtual files in the workspace diagnostics, we need to know the set of virtual files that are available. After #619, we can get the open_files set and filter based on virtual path.

Or, we could just ask the server index to get all the open virtual files.

Or, we could just ask the server index to get all the open virtual files.

I think that would work for workspace diagnostics but not necessarily how we use open_files in check_file_impl to determine if the file is open.

Something else we would have to be careful about when moving it to System is that we can't use it in any salsa query because the result can change from the outside.

codspeed-hq · 2025-06-25T16:40:40Z

CodSpeed WallTime Performance Report

Merging #18939 will not alter performance

_{Comparing dhruv/workspace-diagnostics (5cc62b0) with main (a95c18a)}

Summary

✅ 8 untouched benchmarks

github-actions · 2025-06-27T11:52:34Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

BurntSushi

Nice! This looks sensible to me.

BurntSushi · 2025-07-02T16:40:22Z

crates/ty_server/src/server/api/notifications/did_close.rs

+            // The server needs to clear the diagnostics regardless of whether the client supports
+            // pull diagnostics or not. This is because the client only has the capability to fetch
+            // the diagnostics but does not automatically clear them when a document is closed.
+            clear_diagnostics(&key, client);


How come this doesn't need to be done when in workspace diagnostic mode?

Because in workspace mode all diagnostics should be visible regardless of whether a document is open or close in an editor. So, we shouldn't clear the diagnostics when a user closes a file in the editor.

Ahhhh makes sense of course.

## Summary This PR adds support for `ty.diagnosticMode` for astral-sh/ruff#18939. ## Test Plan Refer to the test plain in the above PR.

MichaReiser · 2025-07-07T13:31:17Z

This is great. We may want to iterate on how we handle the different check modes in ty_project but what you have here is a good solution to start from.

dhruvmanila added server Related to the LSP server ty Multi-file analysis & type inference labels Jun 25, 2025

MichaReiser reviewed Jun 25, 2025

View reviewed changes

dhruvmanila force-pushed the dhruv/workspace-diagnostics branch 3 times, most recently from c8e8013 to 788f426 Compare June 27, 2025 11:32

dhruvmanila changed the title ~~[ty] [WIP] Support workspace diagnostics~~ [ty] Initial support for workspace diagnostics Jun 27, 2025

dhruvmanila force-pushed the dhruv/workspace-diagnostics branch from 788f426 to 2f806da Compare June 27, 2025 11:37

dhruvmanila mentioned this pull request Jun 27, 2025

Add support for ty.diagnosticMode astral-sh/ty-vscode#64

Merged

dhruvmanila changed the base branch from main to dhruv/background-request-handler June 30, 2025 05:44

dhruvmanila force-pushed the dhruv/workspace-diagnostics branch from 4900d71 to c109086 Compare June 30, 2025 05:45

dhruvmanila marked this pull request as ready for review June 30, 2025 06:44

dhruvmanila requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners June 30, 2025 06:44

AlexWaygood removed their request for review June 30, 2025 06:57

dhruvmanila requested review from BurntSushi and MichaReiser and removed request for carljm, dcreager and sharkdp June 30, 2025 13:49

BurntSushi approved these changes Jul 2, 2025

View reviewed changes

dhruvmanila mentioned this pull request Jul 3, 2025

[ty] Add background request task support #19041

Merged

dhruvmanila force-pushed the dhruv/background-request-handler branch from 0e72155 to 1252436 Compare July 3, 2025 10:57

Base automatically changed from dhruv/background-request-handler to main July 3, 2025 11:01

[ty] Support workspace diagnostics

5cc62b0

dhruvmanila force-pushed the dhruv/workspace-diagnostics branch from 7e52e98 to 5cc62b0 Compare July 3, 2025 11:01

dhruvmanila mentioned this pull request Jul 3, 2025

Workspace diagnostics astral-sh/ty#81

Closed

5 tasks

dhruvmanila enabled auto-merge (squash) July 3, 2025 11:03

dhruvmanila added a commit to astral-sh/ty-vscode that referenced this pull request Jul 3, 2025

Add support for ty.diagnosticMode config (#64)

49e3fb8

## Summary This PR adds support for `ty.diagnosticMode` for astral-sh/ruff#18939. ## Test Plan Refer to the test plain in the above PR.

dhruvmanila merged commit dc56c33 into main Jul 3, 2025
35 checks passed

dhruvmanila deleted the dhruv/workspace-diagnostics branch July 3, 2025 11:04

[ty] Initial support for workspace diagnostics #18939

[ty] Initial support for workspace diagnostics #18939

Uh oh!

Conversation

dhruvmanila commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

VS Code

Zed

Neovim

Uh oh!

github-actions bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #18939 will not alter performance

Summary

Uh oh!

github-actions bot commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

BurntSushi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MichaReiser commented Jul 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dhruvmanila commented Jun 25, 2025 •

edited

Loading

github-actions bot commented Jun 25, 2025 •

edited

Loading

`mypy_primer` results

dhruvmanila Jun 26, 2025 •

edited

Loading

codspeed-hq bot commented Jun 25, 2025 •

edited

Loading

github-actions bot commented Jun 27, 2025 •

edited

Loading

`ruff-ecosystem` results