pyrefly add version support to textDocument/publishDiagnostics

close #794

PS: I know little about LSP or this project

Oct 25 '25 03:10 dearfl

Thanks for contributing! A few tests for this behavior would make us confident it works really well.

Could you add some tests in the lsp_interaction folder where each version for a diagnostics response is correct?

Thanks!!

I added a small test for this, however I can't figure out why it won't work, might need a little help here. : ( It seems Server::did_change happens after Interation::Client::expect_message? I don't know why, is there a way to synchronize? Or maybe I'm using it wrong?

Oct 28 '25 07:10 dearfl

could you paste the test failure? it should show the messages back/forth.

running 1 test
client--->server {"id":1,"method":"initialize","params":{"capabilities":{"textDocument":{"publishDiagnostics":{"codeDescriptionSupport":true,"dataSupport":true,"relatedInformation":true,"tagSupport":{"valueSet":[1,2]},"versionSupport":true}},"workspace":{"configuration":true}},"clientInfo":{"name":"debug"},"processId":64851,"rootPath":"/","trace":"verbose"}}
client<---server {"id":1,"result":{"capabilities":{"codeActionProvider":{"codeActionKinds":["quickfix"]},"completionProvider":{"triggerCharacters":["."]},"definitionProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"hoverProvider":true,"inlayHintProvider":true,"positionEncoding":"utf-16","signatureHelpProvider":{"triggerCharacters":["(",","]},"textDocumentSync":2,"typeDefinitionProvider":true,"workspace":{"fileOperations":{"willRename":{"filters":[{"pattern":{"glob":"**/*.{py,pyi}","matches":"file"},"scheme":"file"}]}},"workspaceFolders":{"changeNotifications":true,"supported":true}},"workspaceSymbolProvider":true},"serverInfo":{"name":"pyrefly-lsp","version":"pyrefly-lsp-test-version"}}}
client--->server {"method":"initialized","params":{}}
Reading messages
DEBUG Running with 3 threads (5 MiB stack size)
client<---server {"id":1,"method":"workspace/configuration","params":{"items":[{"section":"python"}]}}
client--->server {"id":1}
client--->server {"method":"textDocument/didOpen","params":{"textDocument":{"languageId":"python","text":"# Copyright (c) Meta Platforms, Inc. and affiliates.\n#\n# This source code is licensed under the MIT license found in the\n# LICENSE file in the root directory of this source tree.\n\nprint(\"Did you catch the change?\")\n","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}}
client--->server {"id":2,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
DEBUG Running epoch 1 of run 0
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
Handling non-canceled request textDocument/diagnostic (2)
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}
client--->server {"method":"textDocument/didChange","params":{"contentChanges":[{"text":"# test"}],"textDocument":{"languageId":"python","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}}
client--->server {"id":3,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
DEBUG Running epoch 1 of run 1
DEBUG Committing transaction
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}

thread 'test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics' panicked at pyrefly/lib/test/lsp/lsp_interaction/object_model.rs:531:17:
assertion failed: `(left == right)`: Response mismatch

Diff < left / right > :
<{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}
>{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Connection closed.
DEBUG Running epoch 1 of run 2
test test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics ... FAILED

failures:

failures:
    test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2554 filtered out; finished in 0.21s

error: test failed, to rerun pass `-p pyrefly --lib`

Oct 28 '25 16:10 dearfl

could you paste the test failure? it should show the messages back/forth.

running 1 test
client--->server {"id":1,"method":"initialize","params":{"capabilities":{"textDocument":{"publishDiagnostics":{"codeDescriptionSupport":true,"dataSupport":true,"relatedInformation":true,"tagSupport":{"valueSet":[1,2]},"versionSupport":true}},"workspace":{"configuration":true}},"clientInfo":{"name":"debug"},"processId":64851,"rootPath":"/","trace":"verbose"}}
client<---server {"id":1,"result":{"capabilities":{"codeActionProvider":{"codeActionKinds":["quickfix"]},"completionProvider":{"triggerCharacters":["."]},"definitionProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"hoverProvider":true,"inlayHintProvider":true,"positionEncoding":"utf-16","signatureHelpProvider":{"triggerCharacters":["(",","]},"textDocumentSync":2,"typeDefinitionProvider":true,"workspace":{"fileOperations":{"willRename":{"filters":[{"pattern":{"glob":"**/*.{py,pyi}","matches":"file"},"scheme":"file"}]}},"workspaceFolders":{"changeNotifications":true,"supported":true}},"workspaceSymbolProvider":true},"serverInfo":{"name":"pyrefly-lsp","version":"pyrefly-lsp-test-version"}}}
client--->server {"method":"initialized","params":{}}
Reading messages
DEBUG Running with 3 threads (5 MiB stack size)
client<---server {"id":1,"method":"workspace/configuration","params":{"items":[{"section":"python"}]}}
client--->server {"id":1}
client--->server {"method":"textDocument/didOpen","params":{"textDocument":{"languageId":"python","text":"# Copyright (c) Meta Platforms, Inc. and affiliates.\n#\n# This source code is licensed under the MIT license found in the\n# LICENSE file in the root directory of this source tree.\n\nprint(\"Did you catch the change?\")\n","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}}
client--->server {"id":2,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
DEBUG Running epoch 1 of run 0
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
Handling non-canceled request textDocument/diagnostic (2)
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}
client--->server {"method":"textDocument/didChange","params":{"contentChanges":[{"text":"# test"}],"textDocument":{"languageId":"python","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}}
client--->server {"id":3,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
DEBUG Running epoch 1 of run 1
DEBUG Committing transaction
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}

thread 'test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics' panicked at pyrefly/lib/test/lsp/lsp_interaction/object_model.rs:531:17:
assertion failed: `(left == right)`: Response mismatch

Diff < left / right > :
<{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}
>{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Connection closed.
DEBUG Running epoch 1 of run 2
test test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics ... FAILED

failures:

failures:
    test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2554 filtered out; finished in 0.21s

error: test failed, to rerun pass `-p pyrefly --lib`

hmm that test looks like the behavior you want. like the other comments say, you're looking for the publishDiagnostic notification, not the textDocument/diagnostic response. the first one happens because of the type error then the next one happens because of the didChange. you can remove the document diagnostic requests and make a similar method to expect_publish_diagnostics_error_count but with a version attached. this'll let you test the behavior correctly!

Oct 29 '25 20:10 kinto0

could you paste the test failure? it should show the messages back/forth.

running 1 test
client--->server {"id":1,"method":"initialize","params":{"capabilities":{"textDocument":{"publishDiagnostics":{"codeDescriptionSupport":true,"dataSupport":true,"relatedInformation":true,"tagSupport":{"valueSet":[1,2]},"versionSupport":true}},"workspace":{"configuration":true}},"clientInfo":{"name":"debug"},"processId":64851,"rootPath":"/","trace":"verbose"}}
client<---server {"id":1,"result":{"capabilities":{"codeActionProvider":{"codeActionKinds":["quickfix"]},"completionProvider":{"triggerCharacters":["."]},"definitionProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"hoverProvider":true,"inlayHintProvider":true,"positionEncoding":"utf-16","signatureHelpProvider":{"triggerCharacters":["(",","]},"textDocumentSync":2,"typeDefinitionProvider":true,"workspace":{"fileOperations":{"willRename":{"filters":[{"pattern":{"glob":"**/*.{py,pyi}","matches":"file"},"scheme":"file"}]}},"workspaceFolders":{"changeNotifications":true,"supported":true}},"workspaceSymbolProvider":true},"serverInfo":{"name":"pyrefly-lsp","version":"pyrefly-lsp-test-version"}}}
client--->server {"method":"initialized","params":{}}
Reading messages
DEBUG Running with 3 threads (5 MiB stack size)
client<---server {"id":1,"method":"workspace/configuration","params":{"items":[{"section":"python"}]}}
client--->server {"id":1}
client--->server {"method":"textDocument/didOpen","params":{"textDocument":{"languageId":"python","text":"# Copyright (c) Meta Platforms, Inc. and affiliates.\n#\n# This source code is licensed under the MIT license found in the\n# LICENSE file in the root directory of this source tree.\n\nprint(\"Did you catch the change?\")\n","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}}
client--->server {"id":2,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
DEBUG Running epoch 1 of run 0
 WARN While finding Python interpreter: Python environment (version, platform, or site-package-path) has value unset, but no Python interpreter could be found to query for values. Falling back to Pyrefly defaults for missing values.
Handling non-canceled request textDocument/diagnostic (2)
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}
client--->server {"method":"textDocument/didChange","params":{"contentChanges":[{"text":"# test"}],"textDocument":{"languageId":"python","uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}}
client--->server {"id":3,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py"}}}
DEBUG Running epoch 1 of run 1
DEBUG Committing transaction
client<---server {"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}

thread 'test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics' panicked at pyrefly/lib/test/lsp/lsp_interaction/object_model.rs:531:17:
assertion failed: `(left == right)`: Response mismatch

Diff < left / right > :
<{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":2}}
>{"method":"textDocument/publishDiagnostics","params":{"diagnostics":[],"uri":"file:///tmp/pyrefly_lsp_testiT0Ha4/text_document.py","version":1}}


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Connection closed.
DEBUG Running epoch 1 of run 2
test test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics ... FAILED

failures:

failures:
    test::lsp::lsp_interaction::diagnostic::test_version_support_publish_diagnostics

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2554 filtered out; finished in 0.21s

error: test failed, to rerun pass `-p pyrefly --lib`

hmm that test looks like the behavior you want. like the other comments say, you're looking for the publishDiagnostic notification, not the textDocument/diagnostic response. the first one happens because of the type error then the next one happens because of the didChange. you can remove the document diagnostic requests and make a similar method to expect_publish_diagnostics_error_count but with a version attached. this'll let you test the behavior correctly!

Now the test always fails with Timeout error, idk why.

Oct 30 '25 17:10 dearfl

bumping some old PRs: if you still want a review on this, please re-request review

Nov 20 '25 22:11 kinto0

@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D88799624.

Dec 10 '25 00:12 meta-codesync[bot]

@dearfl it looks like a bunch of tests are failing. would you be able to investigate this?

Dec 10 '25 22:12 kinto0

@dearfl it looks like a bunch of tests are failing. would you be able to investigate this?

Sorry, I can't figure out why this test fails, could use some help.

Dec 11 '25 11:12 dearfl

looks like a deadlock! I think we need to drop the lock on version_info in text_document_did_change before calling validate_in_memory_and_commit_if_possible. otherwise, both calls will be fighting for the lock.

Dec 12 '25 02:12 kinto0

looks like a deadlock! I think we need to drop the lock on version_info in text_document_did_change before calling validate_in_memory_and_commit_if_possible. otherwise, both calls will be fighting for the lock.

Okay, it works on my machine now.

Dec 12 '25 12:12 dearfl

@kinto0 merged this pull request in facebook/pyrefly@aacf5fe135174ca5218591e9b8b1a33315a46d77.

Dec 12 '25 21:12 meta-codesync[bot]