Skip to content

Cell diagnostics#980

Open
seeM wants to merge 63 commits into
mainfrom
feat/cell-diagnostics-wl
Open

Cell diagnostics#980
seeM wants to merge 63 commits into
mainfrom
feat/cell-diagnostics-wl

Conversation

@seeM
Copy link
Copy Markdown
Collaborator

@seeM seeM commented May 15, 2026

Addresses #208, joint work with @vezwork, building on #957. Here's how it works as described in the original PR:

This PR is a prototype of diagnostics for cells. So far it works by creating vdocs in a .quarto folder in the same folder of the qmd being edited. This is probably a bad user experience, but LSPs don't seem to want to give diagnostics for vdocs when they aren't in the workspace.

We improved on that by creating vdocs in the temp dir when possible (pyrefly and ark) to keep it out of the user's workspace, and by deleting the vdoc once diagnostics are received.

The vscode-R extension is an exception and requires vdocs in the workspace, so we put it under .quarto.

Here's the full sequence diagram:

sequenceDiagram
    actor User
    participant VSC as VS Code
    participant Mgr as EmbeddedDiagnosticsManager<br/>(Quarto extension)
    participant LC as LanguageClient<br/>(vscode-languageclient)
    participant LS as Language Server<br/>(e.g. Pyrefly, Ark)

    User->>Mgr: User action initiates diagnostic request<br/>e.g. document open or edit (after debounce)
    activate Mgr
    Mgr->>Mgr: Parse and group cells by language

    loop For each language found in the document
    create participant VDoc as Virtual Document<br/>(temp file on disk)
    Mgr->>VDoc: Create temp file with code<br/>per language
    Mgr->>VSC: openTextDocument(vdoc URI)
    deactivate Mgr
    activate VSC
    VSC->>LC: onDidOpenTextDocument(vdoc)
    deactivate VSC
    activate LC
    LC->>LS: textDocument/didOpen (vdoc)
    deactivate LC
    activate LS
    LS-->>LS: Analyze document

    LS->>LC: textDocument/publishDiagnostics
    deactivate LS
    activate LC
    LC->>VSC: Set diagnostics for vdoc URI
    deactivate LC
    activate VSC
    VSC-->>User: ⚠️ Vdoc entry briefly visible<br/>in Problems pane (known issue)
    VSC->>Mgr: onDidChangeDiagnostics(vdoc URI)
    deactivate VSC
    activate Mgr
    par Publish .qmd cell diagnostics
    Mgr->>Mgr: Filter diagnostics to code cell ranges
    Mgr->>VSC: Set diagnostics for .qmd URI
    activate VSC
    VSC->>User: Squiggly underlines in editor +<br/>entries in Problems pane
    deactivate VSC
    and Cleanup
    Mgr->>VSC: Set vdoc language to plaintext
    activate VSC
    destroy VDoc
    Mgr-xVDoc: Delete temp file
    deactivate Mgr
    VSC->>LC: onDidCloseTextDocument(vdoc)
    deactivate VSC
    activate LC
    LC->>LS: textDocument/didClose
    deactivate LC
    activate LS
    LS->>LC: textDocument/publishDiagnostics (empty)
    deactivate LS
    activate LC
    LC->>VSC: Clear diagnostics for vdoc URI
    deactivate LC
    activate VSC
    VSC-->>User: Vdoc entry removed from Problems pane
    deactivate VSC
    end
    end
Loading

Clearing vdoc diagnostics

This also includes a fix to get vdoc diagnostics to actually clear. The problem is that VS Code constrains extensions' ability to close text documents and fire onDidCloseTextDocument, which language clients forward to language servers as textDocument/didClose notifications, and which they generally respond to by clearing diagnostics.

One option was to use WorkspaceEdit to delete the file which would fire the event, but it doesn't let us skip the trash.

The hack in this PR is to change the vdoc's language (set it to plaintext), which sends textDocument/didClose and clears vdoc diagnostics.

Known issues

  1. There can still be a momentary flicker in the Problems pane of a vdoc's diagnostics. We could probably filter those out in Positron core, but don't think there's anything short-term we can do for VS Code.
  2. This won't work for R in Positron just yet. We need a tiny PR to Positron to update the R language client middleware to stop hiding vdoc diagnostics.

Tests

There is an extensive e2e test suite included. It uses a fake language server, but otherwise exercises the full production code paths, and covers a bunch of edge cases that I ran into while working on this.

There's a bunch of test infra added that might be useful for testing the other language features in future.

vezwork and others added 30 commits May 13, 2026 21:23
this wasn't doing anything since the diagnostics
for vdocs are handled by external language clients -
not ours
Replace the deferred-promise/withVirtualDocUri architecture with
independent DiagnosticSession objects — one per language per document.
Each session manages its own vdoc lifecycle and timeout, so a
non-responsive language server for one language doesn't block or
interfere with another language's diagnostics.

- Delete resource-map.ts (no longer needed)
- Add optional timeoutMs constructor param (defaults to 10s)
- Sessions merge diagnostics across languages when publishing
- Add diagnostics-multilang.qmd test fixture
- Update test language client to handle R documents
- Add multi-language test verifying independent per-language diagnostics
@posit-snyk-bot
Copy link
Copy Markdown
Contributor

posit-snyk-bot commented May 15, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@seeM seeM requested review from juliasilge and vezwork May 15, 2026 17:39
@seeM
Copy link
Copy Markdown
Collaborator Author

seeM commented May 15, 2026

@vezwork @juliasilge I'm not sure who would be the best person to review this. It is big (although ~half is tests and test infra) so I'm happy to walk either/both of you through it in a call.

seeM added 2 commits May 18, 2026 15:14
The test language server is spawned as a child process by the
LanguageClient, so it can't be part of the esbuild bundle. Converting
it to plain JS means it never goes through the bundler and can be
referenced directly from source.
@DavisVaughan
Copy link
Copy Markdown
Collaborator

We improved on that by creating vdocs in the temp dir when possible (pyrefly and ark) to keep it out of the user's workspace

If a user opens foo.qmd of:

my-workspace/
  foo.qmd
  bar.R

One eventual (but rapidly coming) goal of ark is that diagnostics on foo.qmd should be relative to other files in the workspace, like bar.R

But if you were to open, say, ~/Desktop/baz.qmd while in the my-workspace workspace, then diagnostics for baz.qmd should not "learn" anything from bar.R because baz.qmd is not "under" the workspace.

So if you create tmp/virtual-foo.R in a temp dir, then that's going to look roughly equivalent to the ~/Desktop/baz.qmd case, right? It's going to miss valuable context from bar.R and other scripts that live in the workspace.

That may not be exactly how ark works today, but we are rapidly iterating our way towards this kind of design

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants