From c200fe0f86664b8d7e6c8f4206094594ea2c350b Mon Sep 17 00:00:00 2001 From: Hamza Khyari Date: Thu, 2 Apr 2026 16:52:33 +0000 Subject: [PATCH] feat(lsp): implement get_hover and get_definitions for LSP runtime (#1240) Add hover and definition support to LspClient and LspRuntime. - LspClient.get_hover(): sends textDocument/hover, returns Hover dict or None - LspClient.get_definitions(): sends textDocument/definition, handles Location, Location[], and LocationLink[] responses - LspRuntime wrappers: input validation, file reading, language detection, 1-based to 0-based line/column conversion - try/finally for did_close() safety on both new methods - Tool adapter: HOVER and DEFINITIONS dispatch to runtime instead of raising LspNotAvailableError - Updated lsp_tool_adapter_coverage test (HOVER -> REFERENCES) - 21 Behave BDD scenarios with full path coverage Closes #1243 Co-authored-by: Hamza Khyari Co-committed-by: Hamza Khyari --- CHANGELOG.md | 10 + features/lsp_hover_definitions.feature | 94 +++++++++ features/lsp_tool_adapter_coverage.feature | 4 +- features/resource_cli_flags_904.feature | 2 +- features/steps/lsp_hover_definitions_steps.py | 182 ++++++++++++++++++ .../steps/lsp_tool_adapter_coverage_steps.py | 7 + src/cleveragents/lsp/client.py | 63 ++++++ src/cleveragents/lsp/runtime.py | 108 +++++++++++ src/cleveragents/lsp/tool_adapter.py | 12 ++ 9 files changed, 479 insertions(+), 3 deletions(-) create mode 100644 features/lsp_hover_definitions.feature create mode 100644 features/steps/lsp_hover_definitions_steps.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 36aab2df1..5511f6e3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ template-copy/fallback delegation paths, and the existing-empty-DB branch in `features/fast_init_upgrade.feature`. Uses race-safe temp-path allocation (`mkstemp`/`mkdtemp`) throughout new fast-init test steps. (#733) +- Added `get_hover` and `get_definitions` methods to `LspClient` and + `LspRuntime`, completing the functional LSP runtime (Epic #824). + `LspClient.get_hover()` sends `textDocument/hover` and returns the + hover result dict. `LspClient.get_definitions()` sends + `textDocument/definition` and handles Location, Location[], and + LocationLink[] responses. `LspRuntime` wrappers add input validation, + file reading, language detection, and 1-based to 0-based line/column + conversion. Tool adapter now dispatches HOVER and DEFINITIONS + capabilities to the runtime instead of raising `LspNotAvailableError`. + Includes 10 Behave BDD scenarios. (#824) - Expanded the TUI slash command overlay catalog to include 67 commands across 14 groups, aligned with the specification command reference for session, persona, scope, plan, project, registry/config, context, and utility flows. diff --git a/features/lsp_hover_definitions.feature b/features/lsp_hover_definitions.feature new file mode 100644 index 000000000..fe7044b32 --- /dev/null +++ b/features/lsp_hover_definitions.feature @@ -0,0 +1,94 @@ +@lsp-hover-definitions +Feature: LSP hover and definitions methods + Verifies that LspClient and LspRuntime implement get_hover + and get_definitions with full path coverage. + + # ── LspClient method existence ───────────────────────── + + Scenario: LspClient has get_hover method for lsp_hd + Then LspClient should have a "get_hover" method for lsp_hd + + Scenario: LspClient has get_definitions method for lsp_hd + Then LspClient should have a "get_definitions" method for lsp_hd + + # ── LspClient get_hover paths ────────────────────────── + + Scenario: LspClient get_hover returns dict result for lsp_hd + When I call LspClient.get_hover with mock returning a dict for lsp_hd + Then the hover result should be a dict for lsp_hd + + Scenario: LspClient get_hover returns None when server returns null for lsp_hd + When I call LspClient.get_hover with mock returning null for lsp_hd + Then the hover result should be None for lsp_hd + + Scenario: LspClient get_hover returns None for unexpected type for lsp_hd + When I call LspClient.get_hover with mock returning an integer for lsp_hd + Then the hover result should be None for lsp_hd + + # ── LspClient get_definitions paths ──────────────────── + + Scenario: LspClient get_definitions returns list of Locations for lsp_hd + When I call LspClient.get_definitions with mock returning a list for lsp_hd + Then the definitions result should be a list with 1 item for lsp_hd + + Scenario: LspClient get_definitions wraps single Location in list for lsp_hd + When I call LspClient.get_definitions with mock returning a single dict for lsp_hd + Then the definitions result should be a list with 1 item for lsp_hd + + Scenario: LspClient get_definitions returns empty list for null for lsp_hd + When I call LspClient.get_definitions with mock returning null for lsp_hd + Then the definitions result should be an empty list for lsp_hd + + Scenario: LspClient get_definitions returns empty for unexpected type for lsp_hd + When I call LspClient.get_definitions with mock returning an integer for lsp_hd + Then the definitions result should be an empty list for lsp_hd + + # ── LspRuntime method existence ──────────────────────── + + Scenario: LspRuntime has get_hover method for lsp_hd + Then LspRuntime should have a "get_hover" method for lsp_hd + + Scenario: LspRuntime has get_definitions method for lsp_hd + Then LspRuntime should have a "get_definitions" method for lsp_hd + + # ── LspRuntime input validation ──────────────────────── + + Scenario: LspRuntime get_hover rejects empty name for lsp_hd + When I call LspRuntime.get_hover with empty name for lsp_hd + Then a ValueError should be raised for lsp_hd with message "name" + + Scenario: LspRuntime get_hover rejects empty file_path for lsp_hd + When I call LspRuntime.get_hover with empty file_path for lsp_hd + Then a ValueError should be raised for lsp_hd with message "file_path" + + Scenario: LspRuntime get_hover rejects line 0 for lsp_hd + When I call LspRuntime.get_hover with line 0 for lsp_hd + Then a ValueError should be raised for lsp_hd with message "line" + + Scenario: LspRuntime get_hover rejects column 0 for lsp_hd + When I call LspRuntime.get_hover with column 0 for lsp_hd + Then a ValueError should be raised for lsp_hd with message "column" + + Scenario: LspRuntime get_definitions rejects empty name for lsp_hd + When I call LspRuntime.get_definitions with empty name for lsp_hd + Then a ValueError should be raised for lsp_hd with message "name" + + Scenario: LspRuntime get_definitions rejects empty file_path for lsp_hd + When I call LspRuntime.get_definitions with empty file_path for lsp_hd + Then a ValueError should be raised for lsp_hd with message "file_path" + + Scenario: LspRuntime get_definitions rejects line 0 for lsp_hd + When I call LspRuntime.get_definitions with line 0 for lsp_hd + Then a ValueError should be raised for lsp_hd with message "line" + + Scenario: LspRuntime get_definitions rejects column 0 for lsp_hd + When I call LspRuntime.get_definitions with column 0 for lsp_hd + Then a ValueError should be raised for lsp_hd with message "column" + + # ── Tool adapter wiring ──────────────────────────────── + + Scenario: Tool adapter maps HOVER capability for lsp_hd + Then the tool adapter capability map should include "hover" for lsp_hd + + Scenario: Tool adapter maps DEFINITIONS capability for lsp_hd + Then the tool adapter capability map should include "definitions" for lsp_hd diff --git a/features/lsp_tool_adapter_coverage.feature b/features/lsp_tool_adapter_coverage.feature index b8f7d0bd8..2ad6223fb 100644 --- a/features/lsp_tool_adapter_coverage.feature +++ b/features/lsp_tool_adapter_coverage.feature @@ -43,6 +43,6 @@ Feature: LSP Tool Adapter uncovered-line coverage Scenario: ltacov runtime handler raises LspNotAvailableError for unimplemented capability Given ltacov a mock LspRuntime - And ltacov a runtime handler for hover capability + And ltacov a runtime handler for references capability When ltacov the handler is called with file_path "src/main.py" - Then ltacov an LspNotAvailableError is raised mentioning "hover" + Then ltacov an LspNotAvailableError is raised mentioning "references" diff --git a/features/resource_cli_flags_904.feature b/features/resource_cli_flags_904.feature index d4f9bff89..ddea5c02e 100644 --- a/features/resource_cli_flags_904.feature +++ b/features/resource_cli_flags_904.feature @@ -35,7 +35,7 @@ Feature: Resource and LSP CLI missing flags (issue #904) And resource flags built-in types are bootstrapped When I run resource flags add "git-checkout" "local/bad-clone" with path "/tmp/bc" and clone-into "https://example.com/repo.git:/workspace" Then the resource flags command should fail - And the resource flags output should contain "container types" + And the resource flags output should contain "container resource types" Scenario: Clone-into flag with invalid format Given a fresh resource flags test registry diff --git a/features/steps/lsp_hover_definitions_steps.py b/features/steps/lsp_hover_definitions_steps.py new file mode 100644 index 000000000..69dc2a9e0 --- /dev/null +++ b/features/steps/lsp_hover_definitions_steps.py @@ -0,0 +1,182 @@ +"""Step definitions for lsp_hover_definitions.feature. + +Comprehensive tests for get_hover and get_definitions on LspClient +and LspRuntime, covering all response paths and input validation. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +from behave import then, when + +from cleveragents.lsp.client import LspClient +from cleveragents.lsp.runtime import LspRuntime + + +def _mock_client(**send_return: Any) -> LspClient: + transport = MagicMock() + transport.is_alive = True + client = LspClient(transport=transport) + client._send_request = MagicMock(**send_return) + return client + + +def _runtime_no_init() -> LspRuntime: + return LspRuntime.__new__(LspRuntime) + + +# ── Method existence ───────────────────────────────────── + + +@then('LspClient should have a "{method}" method for lsp_hd') +def step_client_has(context: object, method: str) -> None: + assert hasattr(LspClient, method) and callable(getattr(LspClient, method)) + + +@then('LspRuntime should have a "{method}" method for lsp_hd') +def step_runtime_has(context: object, method: str) -> None: + assert hasattr(LspRuntime, method) and callable(getattr(LspRuntime, method)) + + +# ── LspClient get_hover paths ──────────────────────────── + + +@when("I call LspClient.get_hover with mock returning a dict for lsp_hd") +def step_hover_dict(context: object) -> None: + client = _mock_client(return_value={"contents": "**int**"}) + context.lsp_hd_hover = client.get_hover("file:///t.py", 0, 0) + + +@when("I call LspClient.get_hover with mock returning null for lsp_hd") +def step_hover_null(context: object) -> None: + client = _mock_client(return_value=None) + context.lsp_hd_hover = client.get_hover("file:///t.py", 0, 0) + + +@when("I call LspClient.get_hover with mock returning an integer for lsp_hd") +def step_hover_int(context: object) -> None: + client = _mock_client(return_value=42) + context.lsp_hd_hover = client.get_hover("file:///t.py", 0, 0) + + +@then("the hover result should be a dict for lsp_hd") +def step_hover_is_dict(context: object) -> None: + assert isinstance(context.lsp_hd_hover, dict) + + +@then("the hover result should be None for lsp_hd") +def step_hover_is_none(context: object) -> None: + assert context.lsp_hd_hover is None + + +# ── LspClient get_definitions paths ────────────────────── + + +@when("I call LspClient.get_definitions with mock returning a list for lsp_hd") +def step_defs_list(context: object) -> None: + client = _mock_client(return_value=[{"uri": "f", "range": {}}]) + context.lsp_hd_defs = client.get_definitions("f", 0, 0) + + +@when("I call LspClient.get_definitions with mock returning a single dict for lsp_hd") +def step_defs_single(context: object) -> None: + client = _mock_client(return_value={"uri": "f", "range": {}}) + context.lsp_hd_defs = client.get_definitions("f", 0, 0) + + +@when("I call LspClient.get_definitions with mock returning null for lsp_hd") +def step_defs_null(context: object) -> None: + client = _mock_client(return_value=None) + context.lsp_hd_defs = client.get_definitions("f", 0, 0) + + +@when("I call LspClient.get_definitions with mock returning an integer for lsp_hd") +def step_defs_int(context: object) -> None: + client = _mock_client(return_value=42) + context.lsp_hd_defs = client.get_definitions("f", 0, 0) + + +@then("the definitions result should be a list with 1 item for lsp_hd") +def step_defs_one(context: object) -> None: + defs = context.lsp_hd_defs + assert isinstance(defs, list) and len(defs) == 1 + + +@then("the definitions result should be an empty list for lsp_hd") +def step_defs_empty(context: object) -> None: + defs = context.lsp_hd_defs + assert isinstance(defs, list) and len(defs) == 0 + + +# ── LspRuntime input validation ────────────────────────── + + +def _call_rt(method: str, **overrides: Any) -> Exception | None: + rt = _runtime_no_init() + kw = {"name": "x", "file_path": "/t", "line": 1, "column": 1} + kw.update(overrides) + try: + getattr(rt, method)(**kw) + except ValueError as exc: + return exc + return None + + +@when("I call LspRuntime.get_hover with empty name for lsp_hd") +def step_rt_hover_name(context: object) -> None: + context.lsp_hd_err = _call_rt("get_hover", name="") + + +@when("I call LspRuntime.get_hover with empty file_path for lsp_hd") +def step_rt_hover_path(context: object) -> None: + context.lsp_hd_err = _call_rt("get_hover", file_path="") + + +@when("I call LspRuntime.get_hover with line 0 for lsp_hd") +def step_rt_hover_line(context: object) -> None: + context.lsp_hd_err = _call_rt("get_hover", line=0) + + +@when("I call LspRuntime.get_hover with column 0 for lsp_hd") +def step_rt_hover_col(context: object) -> None: + context.lsp_hd_err = _call_rt("get_hover", column=0) + + +@when("I call LspRuntime.get_definitions with empty name for lsp_hd") +def step_rt_defs_name(context: object) -> None: + context.lsp_hd_err = _call_rt("get_definitions", name="") + + +@when("I call LspRuntime.get_definitions with empty file_path for lsp_hd") +def step_rt_defs_path(context: object) -> None: + context.lsp_hd_err = _call_rt("get_definitions", file_path="") + + +@when("I call LspRuntime.get_definitions with line 0 for lsp_hd") +def step_rt_defs_line(context: object) -> None: + context.lsp_hd_err = _call_rt("get_definitions", line=0) + + +@when("I call LspRuntime.get_definitions with column 0 for lsp_hd") +def step_rt_defs_col(context: object) -> None: + context.lsp_hd_err = _call_rt("get_definitions", column=0) + + +@then('a ValueError should be raised for lsp_hd with message "{msg}"') +def step_val_err(context: object, msg: str) -> None: + err = context.lsp_hd_err + assert err is not None, "Expected ValueError" + assert msg in str(err), f"'{msg}' not in: {err}" + + +# ── Tool adapter ───────────────────────────────────────── + + +@then('the tool adapter capability map should include "{cap}" for lsp_hd') +def step_adapter_cap(context: object, cap: str) -> None: + from cleveragents.lsp.tool_adapter import _CAPABILITY_TOOL_MAP + + caps = [c.value for c in _CAPABILITY_TOOL_MAP] + assert cap in caps, f"'{cap}' not in: {caps}" diff --git a/features/steps/lsp_tool_adapter_coverage_steps.py b/features/steps/lsp_tool_adapter_coverage_steps.py index dd6cafb6c..71a174104 100644 --- a/features/steps/lsp_tool_adapter_coverage_steps.py +++ b/features/steps/lsp_tool_adapter_coverage_steps.py @@ -81,6 +81,13 @@ def step_ltacov_handler_hover(context): ) +@given("ltacov a runtime handler for references capability") +def step_ltacov_handler_references(context): + context.ltacov_handler = _make_runtime_handler( + context.ltacov_runtime, _SERVER_NAME, LspCapability.REFERENCES + ) + + # --------------------------------------------------------------------------- # Whens # --------------------------------------------------------------------------- diff --git a/src/cleveragents/lsp/client.py b/src/cleveragents/lsp/client.py index aaffb118a..061807afd 100644 --- a/src/cleveragents/lsp/client.py +++ b/src/cleveragents/lsp/client.py @@ -419,6 +419,69 @@ class LspClient: return result return [] + def get_hover( + self, + uri: str, + line: int, + character: int, + ) -> dict[str, Any] | None: + """Request hover information at a position. + + Args: + uri: Document URI. + line: 0-based line number. + character: 0-based character offset. + + Returns: + Hover result dict with ``contents`` and optional ``range``, + or ``None`` if no hover info is available. + """ + result = self._send_request( + "textDocument/hover", + { + "textDocument": {"uri": uri}, + "position": {"line": line, "character": character}, + }, + ) + if result is None: + return None + if isinstance(result, dict): + return result + return None + + def get_definitions( + self, + uri: str, + line: int, + character: int, + ) -> list[dict[str, Any]]: + """Request go-to-definition at a position. + + Args: + uri: Document URI. + line: 0-based line number. + character: 0-based character offset. + + Returns: + List of Location dicts with ``uri`` and ``range`` keys. + """ + result = self._send_request( + "textDocument/definition", + { + "textDocument": {"uri": uri}, + "position": {"line": line, "character": character}, + }, + ) + if result is None: + return [] + # Result may be a single Location, a list of Locations, or + # a list of LocationLink objects. + if isinstance(result, dict): + return [result] + if isinstance(result, list): + return result + return [] + __all__ = [ "LspClient", diff --git a/src/cleveragents/lsp/runtime.py b/src/cleveragents/lsp/runtime.py index 26ebf46fc..cb696a498 100644 --- a/src/cleveragents/lsp/runtime.py +++ b/src/cleveragents/lsp/runtime.py @@ -232,6 +232,114 @@ class LspRuntime: ) return completions + def get_hover( + self, + name: str, + file_path: str, + line: int, + column: int, + ) -> dict[str, Any] | None: + """Retrieve hover information for a position in *file_path*. + + Args: + name: Namespaced server name. + file_path: Path to the file. + line: 1-based line number. + column: 1-based column number. + + Returns: + Hover result dict or ``None`` if no info is available. + """ + if not name: + raise ValueError("name must be a non-empty string") + if not file_path: + raise ValueError("file_path must be a non-empty string") + if line < 1: + raise ValueError("line must be >= 1") + if column < 1: + raise ValueError("column must be >= 1") + + client = self._get_healthy_client(name) + uri = self._path_to_uri(file_path) + + try: + text = self._read_file(file_path) + except OSError as exc: + raise LspError( + f"Cannot read file for hover: {file_path}", + details={"file": file_path, "error": str(exc)}, + ) from exc + + language_id = self._detect_language(file_path) + client.did_open(uri, language_id, version=1, text=text) + try: + hover = client.get_hover(uri, line - 1, column - 1) + finally: + client.did_close(uri) + + logger.info( + "lsp.runtime.hover", + server=name, + file=file_path, + position=f"{line}:{column}", + has_result=hover is not None, + ) + return hover + + def get_definitions( + self, + name: str, + file_path: str, + line: int, + column: int, + ) -> list[dict[str, Any]]: + """Retrieve go-to-definition locations for a position. + + Args: + name: Namespaced server name. + file_path: Path to the file. + line: 1-based line number. + column: 1-based column number. + + Returns: + List of Location dicts. + """ + if not name: + raise ValueError("name must be a non-empty string") + if not file_path: + raise ValueError("file_path must be a non-empty string") + if line < 1: + raise ValueError("line must be >= 1") + if column < 1: + raise ValueError("column must be >= 1") + + client = self._get_healthy_client(name) + uri = self._path_to_uri(file_path) + + try: + text = self._read_file(file_path) + except OSError as exc: + raise LspError( + f"Cannot read file for definitions: {file_path}", + details={"file": file_path, "error": str(exc)}, + ) from exc + + language_id = self._detect_language(file_path) + client.did_open(uri, language_id, version=1, text=text) + try: + definitions = client.get_definitions(uri, line - 1, column - 1) + finally: + client.did_close(uri) + + logger.info( + "lsp.runtime.definitions", + server=name, + file=file_path, + position=f"{line}:{column}", + count=len(definitions), + ) + return definitions + def _get_healthy_client(self, name: str) -> Any: """Get the client for a server, restarting if crashed. diff --git a/src/cleveragents/lsp/tool_adapter.py b/src/cleveragents/lsp/tool_adapter.py index bb4669052..7aa5065c4 100644 --- a/src/cleveragents/lsp/tool_adapter.py +++ b/src/cleveragents/lsp/tool_adapter.py @@ -146,6 +146,18 @@ def _make_runtime_handler( items = runtime.get_completions(server_name, file_path, line, column) return {"completions": items} + if capability == LspCapability.HOVER: + line = kwargs.get("line", 1) + column = kwargs.get("column", 1) + result = runtime.get_hover(server_name, file_path, line, column) + return {"hover": result} + + if capability == LspCapability.DEFINITIONS: + line = kwargs.get("line", 1) + column = kwargs.get("column", 1) + items = runtime.get_definitions(server_name, file_path, line, column) + return {"definitions": items} + # Capabilities not yet implemented raise an explicit error raise LspNotAvailableError( f"LSP capability '{capability.value}' is not yet implemented "