Implement workspace/diagnostics - #64130
Draft
Ellen Agarwal (eagarwal-notion) wants to merge 12 commits into
Draft
Conversation
Author
|
@microsoft-github-policy-service agree company="Notion" |
Ellen Agarwal (eagarwal-notion)
force-pushed
the
eagarwal-workspace-diagnostics
branch
7 times, most recently
from
September 2, 2026 22:22
2a2a665 to
dad9e99
Compare
Nothing reads them yet. - experimental.workspaceDiagnostics.scope, which decides whether the editor reports on projects the user has not opened - experimentalWorkspaceDiagnosticsExclude, the paths a project does not want reported. Dependencies reached by module resolution are excluded by the compiler's own predicates, and this defaults to node_modules, which cannot be inferred that way because locally installed typings are program roots. tsconfig's own exclude cannot serve here: it filters root discovery, so imported files enter the program regardless. How much of the machine a pull uses is not settable: it checks projects at the concurrency a build uses for --builders, and each project with the checkers a build of it would use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Protocol and language service plumbing, with no handler wired up yet: - WorkspaceDiagnosticPartialResultParams, since the generated ProgressParams narrows $/progress values to work done progress - ProvideDiagnosticsForFile, extracted from ProvideDiagnostics so a caller holding a source file need not re-resolve it by URI - WorkspaceDiagnosticFiles, the set of files a project owns for reporting, which is where experimentalWorkspaceDiagnosticsExclude is applied - A 'Checking workspace' progress message Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- OpenProjects and ReferencedProjectPaths, to resolve the scopes that report on the open projects and the projects referencing them - ReleaseIdleDiagnosticsChecker, to hand back a checker that has just swept a whole program instead of waiting out the idle timeout; a project with an open file keeps its checker so that file's next pull stays warm Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three files, since the request has three separable concerns: - workspacediagnosticscache.go decides what need not be checked. It remembers which program version produced the result id a client holds, so an unchanged generation answers a whole project without checking it. Settings are compared as whole preference structs rather than a list of the ones known to matter: the auto-import registry can list its own because a stale entry there costs a missing completion, whereas here it would leave errors in the problem list that no longer exist. - workspacediagnosticsscope.go decides what to check: which projects the scope admits, which project reports each file, and how many run at once. - workspacediagnostics.go runs it, streaming reports through a partial result token and reporting progress. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The capability is absent from the initialize response and registered only once the scope setting asks for it, so a client does not start pulling the workspace until someone opts in. Workspace support is a property of a diagnostic provider rather than a method of its own, so it is offered by registering one; the document selector is empty because document diagnostics are already served by the provider advertised at initialize. Withdrawing matters as much as offering: a client holding the capability re-pulls on a timer, so the registration is dropped when the scope goes back to off or validation is disabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Program builds its own checker pool, but the project system supplies one, so it has no way to check a program the way the command line does: with the program's `checkers` count, the compiler's own partition of files across them, and one task per checker rather than per file. CheckingPool is that capability and NewCheckingPool hands back the compiler's implementation of it. Choosing grouped iteration now asks the pool whether it can do it, rather than checking for the compiler's own type, which is what the comment there already described. No behavior change: the compiler's pool is still the only implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pull checked a whole project through the one checker individual requests share, however many checkers a build of the same project would use. Which checker sees a file is part of how a program is checked, so the two could disagree, and one checker had to do all the work. The pool now answers compiler.CheckingPool by building the compiler's own pool on first use, so a whole-program check gets that count and that partition, and the compiler's grouped iteration drives it rather than anything here. Requests for a single file are untouched: they still share one checker, keep their request affinity, and never contend with a sweep. Those checkers are dropped once a swept project is no longer open, along with the global diagnostics they found, since nothing else collects those. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sweep asked for each file's diagnostics separately, which meant it, rather than the program, decided how checking was spread over checkers. WorkspaceDiagnosticsForProject checks the program once and keys the result by file, so the program splits the work as a build would and the sweep only formats what comes back. A project now reports when it is done rather than streaming as each of its files finishes; projects still stream as they finish. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The checkers a whole-program check uses hold the types of every file in the program, which is the largest thing a pull creates, and they were kept until the program was replaced. On a workspace of any size that is most of the memory a pull costs, held indefinitely. Nothing was buying it: a later pull that finds the project unchanged answers from the result ids the client already holds without checking anything, and one that finds it changed needs new checkers regardless. Measured on a 300 file project, releasing takes retained heap from 60 MiB to 26 MiB with the sweep and the repeat pull unchanged at 130ms and 1.1ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pull re-checked every file of every project in scope, however little had changed since the last one. Watch mode already solves this, so a pull now asks through the same thing it uses: incremental.Program tracks which files changed against the previous program and which files those changes reach, re-checks only those, and answers for the rest from what it cached. That brings its declaration signature comparison with it, so an edit that leaves a file's public shape alone stops at that file rather than travelling to its importers. Kept per project and rebuilt only when the project's program is, chained to the previous one so it can work out the change set. Suggestion diagnostics are no longer reported for files a pull covers: nothing caches them, so asking would re-check every file and undo all of this. A file the editor has open still reports them, since it is pulled directly. Measured on a chain of 300 files, editing the file everything depends on: 154ms to check the project, 20ms to answer the next pull. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A pull leaves out the documents the client has open, because a client that also pulls per document keeps each provider's results in its own collection and reconciles only within one, so anything reported by both shows up twice. That reasoning does not hold for a client that only pulls the workspace: there is nothing to collide with, and leaving open documents out means it never hears about the files being edited. experimental.workspaceDiagnostics. serverDiagnosticsDeDuplication turns the omission off for those clients. On unless set, so the editor keeps working as it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Asking for a project tree is what loads projects nothing has opened, so a pull asks for one. But getSnapshot treats any project tree request as a reason to build a new snapshot, without checking whether there is anything to load, and a client pulls every couple of seconds. Each pull therefore replaced the session's snapshot, and with it the programs a pull had just finished with. A pull now asks for the tree only when something could have changed since the last one that did: pending changes, or a snapshot the pull has not already loaded a tree for. Measured on a project pulled eight times with nothing changing between, snapshots built goes from nine to two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ellen Agarwal (eagarwal-notion)
force-pushed
the
eagarwal-workspace-diagnostics
branch
from
September 2, 2026 23:15
56ed071 to
3aad46c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Implement the workspace/diagnostics feature - see #63784.
This lets users typecheck their entire repository quickly while also getting the remaining feature set of the LSP (unlike watch mode). It also uses less memory than running the LSP alongside watch mode or the CLI. Finally, it provides errors to the user more directly - in their IDE.
Interface
This PR implements the workspace/diagnostic method in the LSP spec.
However, vscode will poll this method every 2 seconds by default. This is likely to cause issues for a large repository so we want to gate this behind a feature flag.
To do this, we don't advertise this feature as available during server startup. We dynamically enable this when the user enables/disables the relevant setting. This way it won't be enabled by default in Vscode.
We can see this working here:
Demo.mov
(TODO: fix the bug where two diagnostics appear simultaneously (this is due to the dynamic registration for workspace/diagnostic conflicting with the static registration for textDocument/diagnostic))
Another approach we could take is to enable this feature by default (and remove the option to disable it via config). That way we can use the initial capabilities to enable/disable this feature. To disable this in vscode, we could make this an option the extension parses. The extension can then modify the initialize message to disable support for this feature based on user configuration.
A third way would be to just report no errors when the feature is disabled. I'm not sure if that would cause any issues for editors.
Implementation
This approach is pretty naive - we check every file in the configured projects (see below). I'd love suggestions on how to implement this better - happy to do this in this PR or in follow ups.
To control memory/cpu usage, this has 4 modes:
Todo
Future/Potential improvements:
Please verify that:
Backlogmilestone (required)mainbranchnpx hereby testnpx hereby lintnpx hereby check:format