From ba7cdffc025fc030ba7582e8fb1ef790e412005a Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Tue, 21 Jul 2026 12:33:41 +0200 Subject: [PATCH] FindReferencingSymbolsTool: Limit file system sync to the necessary case (i.e. no full symbol search), adding tests that specifically test the tool path --- src/serena/tools/symbol_tools.py | 4 +- test/conftest.py | 17 ++- test/serena/test_ls_file_sync.py | 172 ++++++++++++++++++++----------- 3 files changed, 131 insertions(+), 62 deletions(-) diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 308ab0cb..69d24d81 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -272,7 +272,9 @@ class FindReferencingSymbolsTool(Tool, ToolMarkerSymbolicRead): :param max_answer_chars: max result length; -1 for default :return: a list of JSON objects with the symbols referencing the requested symbol """ - self.project.ls_sync_file_system_changes() + # file system sync needed for case where symbol finder does not perform a global search, updating everything + if relative_path: + self.project.ls_sync_file_system_changes() include_body = False # It is probably never a good idea to include the body of the referencing symbols parsed_include_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in include_kinds] if include_kinds else None diff --git a/test/conftest.py b/test/conftest.py index 6b25dec3..626cdb78 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -13,6 +13,7 @@ import pytest from _pytest.mark import Mark, MarkDecorator from sensai.util.logging import configure +from serena.agent import SerenaAgent from serena.config.serena_config import SerenaConfig, SerenaPaths from serena.constants import SERENA_MANAGED_DIR_NAME from serena.project import Project @@ -129,7 +130,7 @@ def start_default_ls_context(language: Language) -> Iterator[SolidLanguageServer def create_default_serena_config(): - return SerenaConfig(gui_log_window=False, web_dashboard=False) + return SerenaConfig().with_headless_mode_overrides() def _create_default_project(language: Language, repo_root_override: str | None = None) -> Project: @@ -238,6 +239,20 @@ def project_with_ls_context(language: Language, repo_root_override: str | None = yield project +@contextmanager +def agent_for_project_context(language: Language, repo_root_override: str | None = None) -> Iterator[SerenaAgent]: + project_root = str(get_repo_path(language)) if repo_root_override is None else repo_root_override + agent = SerenaAgent(project=project_root, serena_config=create_default_serena_config()) + + # wait for agent to be ready + agent.execute_task(lambda: None) + + try: + yield agent + finally: + agent.on_shutdown() + + @pytest.fixture(scope="module") def project_with_ls(request: LanguageParamRequest) -> Iterator[Project]: if not hasattr(request, "param"): diff --git a/test/serena/test_ls_file_sync.py b/test/serena/test_ls_file_sync.py index babe4bab..feeacc8c 100644 --- a/test/serena/test_ls_file_sync.py +++ b/test/serena/test_ls_file_sync.py @@ -5,84 +5,136 @@ Exercises all three ``FileChangeType`` branches against a real pyright backend: Created (new caller file), Changed (append a second caller), Deleted (remove the caller file). """ +import json import os import shutil import pytest -from solidlsp import SolidLanguageServer +from serena.agent import SerenaAgent +from serena.project import Project +from serena.tools import FindReferencingSymbolsTool from solidlsp.ls_config import Language -from test.conftest import get_repo_path, project_with_ls_context +from test.conftest import agent_for_project_context, get_repo_path pytestmark = pytest.mark.python -# A method that already has a call site in the fixture, so its baseline reference set is non-empty. -_TARGET_FILE = os.path.join("test_repo", "services.py") -_TARGET_SYMBOL = "create_user" -_CALLER_REL_PATH = os.path.join("test_repo", "external_caller.py") -_CALLER_ONE = "external_caller_one" -_CALLER_TWO = "external_caller_two" +class FileSystemSyncTestCase: + """ + Tests the language server file system synchronisation behaviour, using a reference search + as an example of a feature that generally depends on the synchronisation. + + It supports two modes: + * using Serena's tools (where an explicit sync is not needed because it builds on find_symbol) + * a low-level mode that uses LS methods directly and therefore needs the sync + """ + + # A method that already has a call site in the fixture, so its baseline reference set is non-empty. + _TARGET_FILE = os.path.join("test_repo", "services.py") + _TARGET_SYMBOL = "create_user" + + _CALLER_REL_PATH = os.path.join("test_repo", "external_caller.py") + _CALLER_ONE = "external_caller_one" + _CALLER_TWO = "external_caller_two" + + def __init__(self, use_serena_tool: bool, tool_use_relative_path: bool = True): + """ + :param use_serena_tool: whether to use the Serena tool, which does not need an explicit FS sync + """ + self._use_serena_tool = use_serena_tool + self._tool_use_relative_path = tool_use_relative_path + + def _caller_source(self, *function_names: str) -> str: + body = "\n\n".join( + f"def {name}() -> User:\n return UserService().{self._TARGET_SYMBOL}('x', 'y', 'z@example.com')" for name in function_names + ) + return "from test_repo.models import User\nfrom test_repo.services import UserService\n\n\n" + body + "\n" + + def _referencing_symbol_names(self, agent: SerenaAgent) -> list[str]: + """Names of the symbols that reference ``_TARGET_SYMBOL``, per the warm language server.""" + if self._use_serena_tool: + tool = agent.get_tool(FindReferencingSymbolsTool) + with tool.symbol_dict_grouper.disabled_context(): + relative_path = self._TARGET_FILE if self._tool_use_relative_path else "" + response = tool.apply(name_path=self._TARGET_SYMBOL, relative_path=relative_path) + ref_symbols = json.loads(response) + symbol_names = [ref["name_path"].split("/")[-1] for ref in ref_symbols] + return symbol_names + else: + ls = next(iter(agent.get_active_project_or_raise().language_server_manager.iter_language_servers())) + document_symbols = ls.request_document_symbols(self._TARGET_FILE).get_all_symbols_and_roots() + target = next((s for s in document_symbols[0] if s.get("name") == self._TARGET_SYMBOL), None) + assert target is not None and "selectionRange" in target, f"{self._TARGET_SYMBOL} not found in {self._TARGET_FILE}" + start = target["selectionRange"]["start"] + references = ls.request_referencing_symbols(self._TARGET_FILE, start["line"], start["character"]) + return [ref.symbol["name"] for ref in references] + + def run(self, tmp_path): + # Work on an isolated copy so we can freely create/edit/delete files under the project root. + repo_root = tmp_path / "repo" + shutil.copytree(get_repo_path(Language.PYTHON), repo_root) + caller_abs = repo_root / self._CALLER_REL_PATH + + with agent_for_project_context(Language.PYTHON, str(repo_root)) as agent: + project = agent.get_active_project_or_raise() + + # Warm the reference index, then establish the freshness baseline (first poll never notifies). + self._referencing_symbol_names(agent) + self._sync_fs(project, 0, "") + + # --- Created ------------------------------------------------------------------------------- + caller_abs.write_text(self._caller_source(self._CALLER_ONE), encoding="utf-8") + + if not self._use_serena_tool: + # Adversarial: without the poll, the warm server has never seen the new file -> stale. + assert self._CALLER_ONE not in self._referencing_symbol_names(agent), "expected the new caller to be invisible before poll" + + self._sync_fs(project, 1, "Created") + assert self._CALLER_ONE in self._referencing_symbol_names(agent), "new caller must be visible after poll" + + # --- Changed ------------------------------------------------------------------------------- + caller_abs.write_text(self._caller_source(self._CALLER_ONE, self._CALLER_TWO), encoding="utf-8") + self._sync_fs(project, 1, "Changed") + names_after_change = self._referencing_symbol_names(agent) + assert self._CALLER_TWO in names_after_change, "appended caller must be visible after poll" + assert self._CALLER_ONE in names_after_change + + # --- Deleted ------------------------------------------------------------------------------- + caller_abs.unlink() + self._sync_fs(project, 1, "Deleted") + names_after_delete = self._referencing_symbol_names(agent) + assert self._CALLER_ONE not in names_after_delete, "deleted caller must disappear after poll" + assert self._CALLER_TWO not in names_after_delete + + def _sync_fs(self, project: Project, num_expected_events: int, event_type: str): + # apply sync only if Serena's tool is not being used + if not self._use_serena_tool: + assert project.ls_sync_file_system_changes() == num_expected_events, f"expected {num_expected_events} {event_type} event(s)" -def _caller_source(*function_names: str) -> str: - body = "\n\n".join( - f"def {name}() -> User:\n return UserService().{_TARGET_SYMBOL}('x', 'y', 'z@example.com')" for name in function_names - ) - return "from test_repo.models import User\nfrom test_repo.services import UserService\n\n\n" + body + "\n" +def test_find_referencing_symbol_tool_handles_sync_with_relative_path(tmp_path): + """ + Tests that the FindReferencingSymbolsTool handles file system synchronisation correctly when passed a relative path. + """ + FileSystemSyncTestCase(use_serena_tool=True, tool_use_relative_path=True).run(tmp_path) -def _referencing_symbol_names(ls: SolidLanguageServer) -> list[str]: - """Names of the symbols that reference ``_TARGET_SYMBOL``, per the warm language server.""" - document_symbols = ls.request_document_symbols(_TARGET_FILE).get_all_symbols_and_roots() - target = next((s for s in document_symbols[0] if s.get("name") == _TARGET_SYMBOL), None) - assert target is not None and "selectionRange" in target, f"{_TARGET_SYMBOL} not found in {_TARGET_FILE}" - start = target["selectionRange"]["start"] - references = ls.request_referencing_symbols(_TARGET_FILE, start["line"], start["character"]) - return [ref.symbol["name"] for ref in references] +def test_find_referencing_symbol_tool_handles_sync_without_relative_path(tmp_path): + """ + Tests that the FindReferencingSymbolsTool handles file system synchronisation correctly when no relative path is provided. + """ + FileSystemSyncTestCase(use_serena_tool=True, tool_use_relative_path=False).run(tmp_path) -def test_external_changes_become_visible_after_sync(tmp_path): - # Work on an isolated copy so we can freely create/edit/delete files under the project root. - repo_root = tmp_path / "repo" - shutil.copytree(get_repo_path(Language.PYTHON), repo_root) - caller_abs = repo_root / _CALLER_REL_PATH - - with project_with_ls_context(Language.PYTHON, str(repo_root)) as project: - language_server = next(iter(project.language_server_manager.iter_language_servers())) - - def sync_fs() -> int: - return project.language_server_manager.sync_file_system_changes() - - # Warm the reference index, then establish the freshness baseline (first poll never notifies). - _referencing_symbol_names(language_server) - assert sync_fs() == 0, "first poll must only establish the baseline" - - # --- Created ------------------------------------------------------------------------------- - caller_abs.write_text(_caller_source(_CALLER_ONE), encoding="utf-8") - - # Adversarial: without the poll, the warm server has never seen the new file -> stale. - assert _CALLER_ONE not in _referencing_symbol_names(language_server), "expected the new caller to be invisible before poll" - - assert sync_fs() == 1, "one Created event expected" - assert _CALLER_ONE in _referencing_symbol_names(language_server), "new caller must be visible after poll" - - # --- Changed ------------------------------------------------------------------------------- - caller_abs.write_text(_caller_source(_CALLER_ONE, _CALLER_TWO), encoding="utf-8") - assert sync_fs() == 1, "one Changed event expected" - names_after_change = _referencing_symbol_names(language_server) - assert _CALLER_TWO in names_after_change, "appended caller must be visible after poll" - assert _CALLER_ONE in names_after_change - - # --- Deleted ------------------------------------------------------------------------------- - caller_abs.unlink() - assert sync_fs() == 1, "one Deleted event expected" - names_after_delete = _referencing_symbol_names(language_server) - assert _CALLER_ONE not in names_after_delete, "deleted caller must disappear after poll" - assert _CALLER_TWO not in names_after_delete +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) -def test_first_poll_is_baseline_only(tmp_path): +def test_sync_first_poll_is_baseline_only(tmp_path): """A newly-loaded project must not report its entire pre-existing file set as changed.""" repo_root = tmp_path / "repo" shutil.copytree(get_repo_path(Language.PYTHON), repo_root)