Merge pull request #1075 from oraios/fix/batch_hover_optimization

Fix: the request_info for a batch of symbols was actually never optimized

Author: MischaPanch

src/serena/symbol.py

@@ -11,6 +11,7 @@
 
 import serena.jetbrains.jetbrains_types as jb
 from solidlsp import SolidLanguageServer
+from solidlsp.ls import LSPFileBuffer
 from solidlsp.ls import ReferenceInSymbol as LSPReferenceInSymbol
 from solidlsp.ls_types import Position, SymbolKind, UnifiedSymbolInformation
 
@@ -549,14 +550,14 @@ def __init__(self, ls: SolidLanguageServer | LanguageServerManager, agent: Union
         self._ls_manager: LanguageServerManager = ls_manager
         self.agent = agent
 
-    def _request_info(self, relative_file_path: str, line: int, column: int) -> str | None:
+    def _request_info(self, relative_file_path: str, line: int, column: int, file_buffer: LSPFileBuffer | None = None) -> str | None:
         """Retrieves information (in a sanitized format) about the symbol at the desired location,
         typically containing the docstring and signature.
 
         Returns None if no information is available.
         """
         lang_server = self.get_language_server(relative_file_path)
-        hover_info = lang_server.request_hover(relative_file_path=relative_file_path, line=line, column=column)
+        hover_info = lang_server.request_hover(relative_file_path=relative_file_path, line=line, column=column, file_buffer=file_buffer)
         if hover_info is None:
             return None
 
@@ -651,26 +652,28 @@ def request_info_for_symbol_batch(
             t0_file = perf_counter() if debug_enabled else 0.0
             file_hover_lookups = 0
 
-            for sym in file_symbols:
-                # Check budget before starting a new hover request
-                # symbol_info_budget_seconds=0 disables the budget mechanism (the first inequality)
-                if 0 < symbol_info_budget_seconds <= hover_spent_seconds:
-                    skipped_due_to_budget += 1
-                    info = None
-                    # log once when budget exceeded
-                    if skipped_due_to_budget == 1:
-                        log.debug("Skipping further hover operations due to budget exceeded")
-                else:
-                    line = sym.line
-                    column = sym.column
-                    assert line is not None and column is not None  # for mypy, we filtered invalid symbols above
-                    t0_hover = perf_counter()
-                    info = self._request_info(file_path, line, column)
-                    hover_spent_seconds += perf_counter() - t0_hover
-                    file_hover_lookups += 1
-                    total_hover_lookups += 1
-
-                info_by_symbol[sym] = info
+            ls = self.get_language_server(file_path)
+            with ls.open_file(file_path) as file_buffer:
+                for sym in file_symbols:
+                    # Check budget before starting a new hover request
+                    # symbol_info_budget_seconds=0 disables the budget mechanism (the first inequality)
+                    if 0 < symbol_info_budget_seconds <= hover_spent_seconds:
+                        skipped_due_to_budget += 1
+                        info = None
+                        # log once when budget exceeded
+                        if skipped_due_to_budget == 1:
+                            log.debug("Skipping further hover operations due to budget exceeded")
+                    else:
+                        line = sym.line
+                        column = sym.column
+                        assert line is not None and column is not None  # for mypy, we filtered invalid symbols above
+                        t0_hover = perf_counter()
+                        info = self._request_info(file_path, line, column, file_buffer=file_buffer)
+                        hover_spent_seconds += perf_counter() - t0_hover
+                        file_hover_lookups += 1
+                        total_hover_lookups += 1
+
+                    info_by_symbol[sym] = info
 
             if debug_enabled:
                 file_elapsed_ms = (perf_counter() - t0_file) * 1000

src/solidlsp/language_servers/al_language_server.py

@@ -1055,7 +1055,9 @@ def normalize_name(symbol: UnifiedSymbolInformation) -> None:
         return document_symbols
 
     @override
-    def request_hover(self, relative_file_path: str, line: int, column: int) -> ls_types.Hover | None:
+    def request_hover(
+        self, relative_file_path: str, line: int, column: int, file_buffer: LSPFileBuffer | None = None
+    ) -> ls_types.Hover | None:
         """
         Override to inject original AL object name (with type and ID) into hover responses.
 
@@ -1065,7 +1067,7 @@ def request_hover(self, relative_file_path: str, line: int, column: int) -> ls_t
         # Normalize path separators for cross-platform compatibility (backslash → forward slash)
         relative_file_path = self._normalize_path(relative_file_path)
 
-        hover = super().request_hover(relative_file_path, line, column)
+        hover = super().request_hover(relative_file_path, line, column, file_buffer=file_buffer)
 
         if hover is None:
             return None

src/solidlsp/language_servers/csharp_language_server.py

@@ -15,7 +15,7 @@
 
 from overrides import override
 
-from solidlsp.ls import DocumentSymbols, LanguageServerDependencyProvider, SolidLanguageServer
+from solidlsp.ls import DocumentSymbols, LanguageServerDependencyProvider, LSPFileBuffer, SolidLanguageServer
 from solidlsp.ls_config import LanguageServerConfig
 from solidlsp.ls_exceptions import SolidLSPException
 from solidlsp.ls_types import Hover, UnifiedSymbolInformation
@@ -206,14 +206,14 @@ def request_document_symbols(self, relative_file_path: str, file_buffer: Any = N
         return symbols
 
     @override
-    def request_hover(self, relative_file_path: str, line: int, column: int) -> Hover | None:
+    def request_hover(self, relative_file_path: str, line: int, column: int, file_buffer: LSPFileBuffer | None = None) -> Hover | None:
         """
         Override to inject original Roslyn symbol names (with type annotations) into hover responses.
 
         When hovering over a symbol whose name was normalized, we prepend the original
         full name (e.g., 'Add(int, int) : int') to the hover content.
         """
-        hover = super().request_hover(relative_file_path, line, column)
+        hover = super().request_hover(relative_file_path, line, column, file_buffer=file_buffer)
 
         if hover is None:
             return None

src/solidlsp/language_servers/eclipse_jdtls.py

@@ -849,7 +849,7 @@ def do_nothing(params: dict) -> None:
         log.info("Startup complete")
 
     @override
-    def _request_hover(self, uri: str, line: int, column: int) -> ls_types.Hover | None:
+    def _request_hover(self, file_buffer: LSPFileBuffer, line: int, column: int) -> ls_types.Hover | None:
         # Eclipse JDTLS lazily loads javadocs on first hover request, then caches them.
         # This means the first request often returns incomplete info (just the signature),
         # while subsequent requests return the full javadoc.
@@ -879,12 +879,12 @@ def content_score(result: ls_types.Hover | None) -> tuple[int, int]:
                 return (1, len(contents))
 
         max_retries = 5
-        best_result = super()._request_hover(uri, line, column)
+        best_result = super()._request_hover(file_buffer, line, column)
         best_score = content_score(best_result)
 
         for _ in range(max_retries):
             sleep(0.05)
-            new_result = super()._request_hover(uri, line, column)
+            new_result = super()._request_hover(file_buffer, line, column)
             new_score = content_score(new_result)
             if new_score > best_score:
                 best_result = new_result

src/solidlsp/ls.py

@@ -756,6 +756,8 @@ def _open_file_context(
             be opened in the LS later by calling the `ensure_open_in_ls` method on the returned LSPFileBuffer.
         """
         if file_buffer is not None:
+            expected_uri = pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri()
+            assert file_buffer.uri == expected_uri, f"Inconsistency between provided {file_buffer.uri=} and {expected_uri=}"
             if open_in_ls:
                 file_buffer.ensure_open_in_ls()
             yield file_buffer
@@ -1555,32 +1557,29 @@ def request_overview(self, within_relative_path: str) -> dict[str, list[UnifiedS
         else:
             return self.request_dir_overview(within_relative_path)
 
-    def request_hover(self, relative_file_path: str, line: int, column: int) -> ls_types.Hover | None:
+    def request_hover(
+        self, relative_file_path: str, line: int, column: int, file_buffer: LSPFileBuffer | None = None
+    ) -> ls_types.Hover | None:
         """
         Raise a [textDocument/hover](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_hover) request to the Language Server
         to find the hover information at the given line and column in the given file. Wait for the response and return the result.
 
         :param relative_file_path: The relative path of the file that has the hover information
         :param line: The line number of the symbol
         :param column: The column number of the symbol
+        :param file_buffer: The file buffer to use for the request. If not provided, the file will be read from disk.
+            Can be used for optimizing number of file reads in downstream code
         """
-        with self.open_file(relative_file_path):
-            uri = pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri()
-            return self._request_hover(uri, line, column)
+        with self._open_file_context(relative_file_path, file_buffer=file_buffer) as fb:
+            return self._request_hover(fb, line, column)
 
-    def _request_hover(self, uri: str, line: int, column: int) -> ls_types.Hover | None:
+    def _request_hover(self, file_buffer: LSPFileBuffer, line: int, column: int) -> ls_types.Hover | None:
         """
-        Internal method that performs the actual hover request.
-        The file must already be open when calling this method.
-        Subclasses can override this to customize hover behavior (e.g., retries).
-
-        :param uri: The URI of the file
-        :param line: The line number of the symbol
-        :param column: The column number of the symbol
+        Performs the actual hover request.
         """
         response = self.server.send.hover(
             {
-                "textDocument": {"uri": uri},
+                "textDocument": {"uri": file_buffer.uri},
                 "position": {
                     "line": line,
                     "character": column,