From 14fa2b001a2e9e1ecf24ee9704b4e15ada677787 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sun, 2 Aug 2026 10:08:49 +0200 Subject: [PATCH 1/5] Add test: find_symbol with file kept open in LS and external modification --- test/serena/test_ls_file_sync.py | 59 +++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/test/serena/test_ls_file_sync.py b/test/serena/test_ls_file_sync.py index 4902ffa7..618559b3 100644 --- a/test/serena/test_ls_file_sync.py +++ b/test/serena/test_ls_file_sync.py @@ -13,7 +13,7 @@ import pytest from serena.agent import SerenaAgent from serena.project import Project -from serena.tools import FindReferencingSymbolsTool +from serena.tools import FindReferencingSymbolsTool, FindSymbolTool from solidlsp.ls_config import LanguageServerId from test.conftest import agent_for_project_context, get_repo_path @@ -132,3 +132,60 @@ def test_ls_low_level_find_references_with_explicit_sync(tmp_path): Tests that requesting references directly from the LS works if synchronisation is explicitly requested after external file changes. """ FileSystemSyncTestCase(use_serena_tool=False).run(tmp_path) + + +class SymbolPositionStaleAfterExternalEditTestCase: + """ + Tests that a Changed (not Created/Deleted) external edit to a file that already has an open + buffer in the language server session is reflected in symbol positions returned by + FindSymbolTool. This exercises SolidLanguageServer.resync_open_buffer, which is needed in + addition to workspace/didChangeWatchedFiles: once a document is open, a server may treat the + client (not the filesystem) as authoritative for its content and silently ignore watched-file + notifications for it (observed with pyright). + """ + + _TARGET_FILE = os.path.join("test_repo", "services.py") + _TARGET_SYMBOL = "create_user" + _PAD_LINES = 5 + + def _find(self, tool: FindSymbolTool) -> dict: + with tool.symbol_dict_grouper.disabled_context(): + response = tool.apply(name_path_pattern=self._TARGET_SYMBOL, relative_path=self._TARGET_FILE) + symbols = json.loads(response) + assert len(symbols) == 1, f"expected exactly one match for {self._TARGET_SYMBOL}, got {symbols}" + return symbols[0] + + def run(self, tmp_path): + repo_root = tmp_path / "repo" + shutil.copytree(get_repo_path(LanguageServerId.PYTHON), repo_root) + target_abs = repo_root / self._TARGET_FILE + + with agent_for_project_context(LanguageServerId.PYTHON, str(repo_root)) as agent: + project = agent.get_active_project_or_raise() + ls = next(iter(project.language_server_manager.iter_language_servers())) + tool = agent.get_tool(FindSymbolTool) + + # Hold the file's buffer open across the external edit, mirroring the state left + # behind by a multi-step editing sequence that keeps a buffer open between calls. + with ls.open_file(self._TARGET_FILE): + baseline_start_line = self._find(tool)["body_location"]["start_line"] + + original = target_abs.read_text(encoding="utf-8") + target_abs.write_text(("# pad\n" * self._PAD_LINES) + original, encoding="utf-8") + + after_start_line = self._find(tool)["body_location"]["start_line"] + + assert after_start_line == baseline_start_line + self._PAD_LINES, ( + f"expected {self._TARGET_SYMBOL} to have shifted by {self._PAD_LINES} lines after the " + f"external edit ({baseline_start_line} -> {baseline_start_line + self._PAD_LINES}), " + f"but FindSymbolTool reported {after_start_line}" + ) + + +def test_find_symbol_tool_reflects_external_change_to_open_buffer(tmp_path): + """ + Regression test for oraios/serena#1593: find_symbol returned a stale position for a symbol + in a file that was already open in the language server session and was then edited outside + of Serena's own edit tools. + """ + SymbolPositionStaleAfterExternalEditTestCase().run(tmp_path) From d179e2ba59ef2fd9ed7da6e58a4a5e225d5451e9 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 2 Aug 2026 10:57:07 +0200 Subject: [PATCH 2/5] Consistently configure the log level in tests --- test/conftest.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 9110b68e..c75e12db 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,4 +1,3 @@ -import logging import os import platform import re @@ -11,7 +10,7 @@ from typing import Any import pytest from _pytest.mark import Mark, MarkDecorator -from sensai.util.logging import configure +from sensai.util import logging from serena.agent import SerenaAgent from serena.config.serena_config import SerenaConfig, SerenaPaths @@ -26,7 +25,9 @@ from .solidlsp.clojure import is_clojure_cli_available from .solidlsp.elixir import EXPERT_UNAVAILABLE from .solidlsp.erlang import ERLANG_LS_UNAVAILABLE -configure(level=logging.INFO) +PYTEST_LOG_LEVEL = logging.DEBUG + +logging.configure(level=PYTEST_LOG_LEVEL) log = logging.getLogger(__name__) @@ -131,7 +132,7 @@ def start_default_ls_context(ls_id: LanguageServerId) -> Iterator[SolidLanguageS def create_default_serena_config(): - return SerenaConfig().with_headless_mode_overrides() + return SerenaConfig(log_level=PYTEST_LOG_LEVEL).with_headless_mode_overrides() def _create_default_project(ls_id: LanguageServerId, repo_root_override: str | None = None) -> Project: From fc7a8ec7324631ca4de67ffda2d1d9fa25b07706 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 2 Aug 2026 12:36:59 +0200 Subject: [PATCH 3/5] Fix: LSPFileBuffer: a stale content hash could be returned if files are kept open and file contents were not read before trying to retrieve the hash value --- CHANGELOG.md | 2 ++ src/solidlsp/ls.py | 23 +++++++++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf014908..dc03e18d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,8 @@ Status of the `main` branch. Changes prior to the next official version change w - Fix: F#'s `module ` declarations reported a `selectionRange` pointing at the `module` keyword instead of at ``, so looking up hover/references from a module symbol's position returned the keyword's own docs instead of the module's #925 + - Fix: `LSPFileBuffer`: a stale content hash could be returned if files are kept open + and file contents were not read before trying to retrieve the hash value * JetBrains: - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 1d117dba..6d64d76b 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -149,21 +149,27 @@ class LSPFileBuffer: """Ensure that the file is opened in the language server.""" self._open_in_ls() + def _invalidate_cached_data(self, mtime: float | None = None) -> float | None: + """ + Invalidates cached data (file contents, hash) if the file was modified since it was read + + :param: the current modification time if it was already read + """ + if self._read_file_modified_date is not None: + if mtime is None: + mtime = self.abs_path.stat().st_mtime + if mtime > self._read_file_modified_date: + self._contents = None + self._content_hash = None + @property def contents(self) -> str: file_modified_date = self.abs_path.stat().st_mtime - - # if contents are cached, check if they are stale (file modification since last read) and invalidate if so - if self._contents is not None: - assert self._read_file_modified_date is not None - if file_modified_date > self._read_file_modified_date: - self._contents = None - + self._invalidate_cached_data(file_modified_date) if self._contents is None: self._read_file_modified_date = file_modified_date self._contents = FileUtils.read_file(str(self.abs_path), self.encoding) self._content_hash = None - return self._contents @contents.setter @@ -179,6 +185,7 @@ class LSPFileBuffer: @property def content_hash(self) -> str: + self._invalidate_cached_data() if self._content_hash is None: self._content_hash = hashlib.md5(self.contents.encode(self.encoding)).hexdigest() return self._content_hash From e5c86a34223a1279ce6b58a4d29266e61936e9eb Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 2 Aug 2026 12:41:03 +0200 Subject: [PATCH 4/5] request_document_symbols: Improve logging --- src/solidlsp/ls.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 6d64d76b..c2cae169 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -1911,16 +1911,13 @@ class SolidLanguageServer(ABC): file_hash_and_result = self._document_symbols_cache.get(cache_key) if file_hash_and_result is None: log.debug("No cache hit for document symbols in %s", relative_file_path) - log.debug("perf: document_symbols_cache MISS path=%s", relative_file_path) else: file_hash, document_symbols = file_hash_and_result if file_hash == file_data.content_hash: - log.debug("Returning cached document symbols for %s", relative_file_path) - log.debug("perf: document_symbols_cache HIT path=%s", relative_file_path) + log.debug("Returning cached document symbols for %s (hash=%s)", relative_file_path, file_hash) return document_symbols - log.debug("Cached document symbol content for %s has changed", relative_file_path) - log.debug("perf: document_symbols_cache STALE path=%s", relative_file_path) + log.debug("Cached document symbol content for %s has changed (old hash=%s)", relative_file_path, file_hash) # no cached result: request the root symbols from the language server root_symbols = self._request_document_symbols(relative_file_path, file_data) @@ -2015,8 +2012,9 @@ class SolidLanguageServer(ABC): document_symbols = DocumentSymbols(unified_root_symbols) # update cache - log.debug("Updating cached document symbols for %s", relative_file_path) - self._document_symbols_cache[cache_key] = (file_data.content_hash, document_symbols) + content_hash = file_data.content_hash + log.debug("Updating cached document symbols for %s (hash=%s)", relative_file_path, content_hash) + self._document_symbols_cache[cache_key] = (content_hash, document_symbols) self._document_symbols_cache_is_modified = True return document_symbols From 9eb4df2e7bc4d129240f9a1c85a5ed9f39e5b3be Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 2 Aug 2026 12:55:12 +0200 Subject: [PATCH 5/5] Fix: Change the semantics of file opening (open_file) in the language server Instead of sending only the didOpen notification if the file is not already open, the method now ensures that the language server has the (current) contents of the file, sending the didChange notification in cases where the file is already open but has changed on disk. This better reflects the actual intention of calling the method. This affects the Vue and Svelte language servers, which unlike all other server implementations, kept files open, which could result in the servers reporting stale information in the absence of update notifications (external file changes). --- CHANGELOG.md | 5 ++++ src/solidlsp/ls.py | 58 +++++++++++++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc03e18d..7a895d15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,11 @@ Status of the `main` branch. Changes prior to the next official version change w returned the keyword's own docs instead of the module's #925 - Fix: `LSPFileBuffer`: a stale content hash could be returned if files are kept open and file contents were not read before trying to retrieve the hash value + - Fix: Change semantics of file opening (`open_file`) in the language server from "open file (if not already open)" + to "ensure that the language server has the (current) contents of the file" (by sending `textDocument/didOpen` + or `textDocument/didChange`), as this is always the intention of calling the method. + If files were kept open in the language server (which the Svelte and Vue language servers did), + the language server was not necessarily informed about updated contents. * JetBrains: - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index c2cae169..c9881c60 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -107,6 +107,7 @@ class LSPFileBuffer: self.language_server = language_server self.uri = uri self._read_file_modified_date: float | None = None + self._read_file_modified_date_passed_to_ls: float | None = None self._contents: str | None = None self.version = version self.language_id = language_id @@ -120,20 +121,40 @@ class LSPFileBuffer: def _open_in_ls(self) -> None: """ Open the file in the language server if it is not already open. + If it is already open, make sure the language server has the latest contents of the file. """ - if self._is_open_in_ls: - return - self._is_open_in_ls = True - self.language_server.server.notify.did_open_text_document( - { # ty: ignore[invalid-argument-type] # dict built from LSPConstants keys; shape matches the TypedDict - LSPConstants.TEXT_DOCUMENT: { - LSPConstants.URI: self.uri, - LSPConstants.LANGUAGE_ID: self.language_id, - LSPConstants.VERSION: 0, - LSPConstants.TEXT: self.contents, + if not self._is_open_in_ls: + self._is_open_in_ls = True + current_contents = self.contents + self._read_file_modified_date_passed_to_ls = self._read_file_modified_date + self.language_server.server.notify.did_open_text_document( + { # ty: ignore[invalid-argument-type] # dict built from LSPConstants keys; shape matches the TypedDict + LSPConstants.TEXT_DOCUMENT: { + LSPConstants.URI: self.uri, + LSPConstants.LANGUAGE_ID: self.language_id, + LSPConstants.VERSION: self.version, + LSPConstants.TEXT: current_contents, + } } - } - ) + ) + else: + # file already open: check if contents have changed and notify if so + current_contents = self.contents + if self._read_file_modified_date != self._read_file_modified_date_passed_to_ls: + self._read_file_modified_date_passed_to_ls = self._read_file_modified_date + self.language_server.server.notify.did_change_text_document( + { # ty: ignore[invalid-argument-type] # dict built from LSPConstants keys; shape matches the TypedDict + LSPConstants.TEXT_DOCUMENT: { + LSPConstants.URI: self.uri, + LSPConstants.VERSION: self.version, + }, + LSPConstants.CONTENT_CHANGES: [ + { + LSPConstants.TEXT: current_contents, + } + ], + } + ) def close(self) -> None: if self._is_open_in_ls: @@ -146,7 +167,11 @@ class LSPFileBuffer: ) def ensure_open_in_ls(self) -> None: - """Ensure that the file is opened in the language server.""" + """ + Ensure that the file is opened in the language server (or, if it is already open, + that the language server is made aware of the file's updated contents in case it + has changed on disk). + """ self._open_in_ls() def _invalidate_cached_data(self, mtime: float | None = None) -> float | None: @@ -1254,10 +1279,15 @@ class SolidLanguageServer(ABC): @contextmanager def open_file(self, relative_file_path: str, open_in_ls: bool = True) -> Iterator[LSPFileBuffer]: """ - Open a file in the Language Server. This is required before making any requests to the Language Server. + Opens a file. + + Note: Opening a file in the language server is typically a precondition for further requests + pertaining to the respective file. :param relative_file_path: The relative path of the file to open. :param open_in_ls: whether to open the file in the language server, sending the didOpen notification. + If the file is already open but file contents has changed since the original notification, + an update notification is sent instead. Set this to False to read the local file buffer without notifying the LS; the file can be opened in the LS later by calling the `ensure_open_in_ls` method on the returned LSPFileBuffer. """