Skip to content

Do not coalesce LSP responses as mutations - #4502

Open
heejaechang wants to merge 3 commits into
facebook:mainfrom
heejaechang:heejaechang-fix-didopen-queue-race
Open

Do not coalesce LSP responses as mutations#4502
heejaechang wants to merge 3 commits into
facebook:mainfrom
heejaechang:heejaechang-fix-didopen-queue-race

Conversation

@heejaechang

@heejaechang heejaechang commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

LSP responses are processed on the normal FIFO queue, but they were classified as state mutations. That classification also updates the queue's last_mutation marker, which tells earlier event handlers that a later mutation is waiting and that expensive work such as validation can be deferred.

This can lose diagnostics during startup:

  1. The server receives textDocument/didOpen.
  2. The client immediately replies to a server-initiated request such as workspace/configuration.
  3. Because the response is marked as a mutation, dequeuing didOpen reports subsequent_mutation = true.
  4. didOpen skips open-file validation under the assumption that the later mutation will make that work redundant.
  5. A configuration response may be a no-op and does not necessarily trigger validation, so nothing compensates for the skipped work.

The server can then remain healthy and responsive while never publishing diagnostics for the opened file until some unrelated later event triggers validation.

This change classifies LspResponse with normal-priority query events instead. Responses still preserve FIFO ordering and are handled exactly as before, but they no longer claim that work triggered by preceding document mutations is redundant. This matches the queue's coalescing invariant: only events that can safely supersede prior mutation work should update last_mutation.

A deterministic regression enqueues didOpen followed by a configuration-shaped response and verifies that:

  • didOpen is not reported as having a subsequent mutation, so validation is not suppressed.
  • The response remains queued and is delivered in FIFO order.

Test Plan

  • cargo test lsp::non_wasm::queue::tests --quiet
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema

heejaechang and others added 3 commits August 10, 2026 16:49
Client responses are FIFO messages, but they do not guarantee a validation that can replace work skipped by an earlier document mutation. Treating them as subsequent mutations could therefore suppress didOpen diagnostics permanently. Classifying responses as non-mutations preserves FIFO handling while ensuring didOpen performs its required validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document event kinds in terms of whether later events make earlier mutation work redundant, and keep the classification match focused on classification rather than the motivating bug.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the existing Query event-kind vocabulary while documenting that queries and responses are normal-priority events that do not count as mutations for coalescing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@meta-codesync

meta-codesync Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants