feat(lsp): implement LspRuntime get_hover and get_definitions methods #1243

Closed
opened 2026-04-01 14:52:30 +00:00 by hamza.khyari · 3 comments
Member

Metadata

  • Commit Message: feat(lsp): implement get_hover and get_definitions for LSP runtime
  • Branch: feature/lsp-epic-completion
  • Type: Feature
  • Priority: Medium
  • MoSCoW: Should have
  • Points: 5
  • Milestone: v3.6.0

Background and Context

Epic #824 (LSP Functional Runtime) requires LspRuntime to support all LSP operations defined in the LspCapability enum. Currently, get_diagnostics() and get_completions() are implemented, but get_hover() and get_definitions() are missing. The tool adapter maps all 11 capabilities but only 2 dispatch to the runtime — the remaining 9 fall through to raise LspNotAvailableError.

This issue adds get_hover and get_definitions to both LspClient and LspRuntime, and wires them into the tool adapter, bringing the working capability count from 2 to 4.

Expected Behavior

  • LspClient.get_hover(uri, line, character) sends textDocument/hover and returns the Hover result dict or None
  • LspClient.get_definitions(uri, line, character) 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 coordinate conversion
  • Tool adapter dispatches HOVER and DEFINITIONS to runtime instead of raising error
  • did_close() is always called via try/finally (safety improvement over existing pattern)

Acceptance Criteria

  • LspClient.get_hover() sends textDocument/hover and returns dict | None
  • LspClient.get_definitions() sends textDocument/definition and returns list[dict]
  • LspClient.get_definitions() handles single Location, Location[], LocationLink[], and null responses
  • LspRuntime.get_hover() validates inputs (name, file_path, line >= 1, column >= 1)
  • LspRuntime.get_definitions() validates inputs
  • Both runtime methods use try/finally for did_close() safety
  • Tool adapter HOVER capability dispatches to runtime.get_hover()
  • Tool adapter DEFINITIONS capability dispatches to runtime.get_definitions()
  • Existing lsp_tool_adapter_coverage test updated (HOVER -> REFERENCES for unimplemented capability test)

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Subtasks

  • Implement get_hover() on LspClient (textDocument/hover)
  • Implement get_definitions() on LspClient (textDocument/definition)
  • Implement get_hover() on LspRuntime with validation + coordinate conversion
  • Implement get_definitions() on LspRuntime with validation + coordinate conversion
  • Wire HOVER and DEFINITIONS in tool adapter _make_runtime_handler()
  • Update lsp_tool_adapter_coverage.feature (HOVER -> REFERENCES for unimplemented test)
  • Tests (Behave): 21 scenarios covering all response paths and input validation
  • Verify coverage >= 97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors
## Metadata - **Commit Message**: `feat(lsp): implement get_hover and get_definitions for LSP runtime` - **Branch**: `feature/lsp-epic-completion` - **Type**: Feature - **Priority**: Medium - **MoSCoW**: Should have - **Points**: 5 - **Milestone**: v3.6.0 ## Background and Context Epic #824 (LSP Functional Runtime) requires `LspRuntime` to support all LSP operations defined in the `LspCapability` enum. Currently, `get_diagnostics()` and `get_completions()` are implemented, but `get_hover()` and `get_definitions()` are missing. The tool adapter maps all 11 capabilities but only 2 dispatch to the runtime — the remaining 9 fall through to `raise LspNotAvailableError`. This issue adds `get_hover` and `get_definitions` to both `LspClient` and `LspRuntime`, and wires them into the tool adapter, bringing the working capability count from 2 to 4. ## Expected Behavior - `LspClient.get_hover(uri, line, character)` sends `textDocument/hover` and returns the Hover result dict or None - `LspClient.get_definitions(uri, line, character)` 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 coordinate conversion - Tool adapter dispatches HOVER and DEFINITIONS to runtime instead of raising error - `did_close()` is always called via `try/finally` (safety improvement over existing pattern) ## Acceptance Criteria - [ ] `LspClient.get_hover()` sends `textDocument/hover` and returns `dict | None` - [ ] `LspClient.get_definitions()` sends `textDocument/definition` and returns `list[dict]` - [ ] `LspClient.get_definitions()` handles single Location, Location[], LocationLink[], and null responses - [ ] `LspRuntime.get_hover()` validates inputs (name, file_path, line >= 1, column >= 1) - [ ] `LspRuntime.get_definitions()` validates inputs - [ ] Both runtime methods use `try/finally` for `did_close()` safety - [ ] Tool adapter HOVER capability dispatches to `runtime.get_hover()` - [ ] Tool adapter DEFINITIONS capability dispatches to `runtime.get_definitions()` - [ ] Existing `lsp_tool_adapter_coverage` test updated (HOVER -> REFERENCES for unimplemented capability test) ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. ## Subtasks - [ ] Implement `get_hover()` on `LspClient` (textDocument/hover) - [ ] Implement `get_definitions()` on `LspClient` (textDocument/definition) - [ ] Implement `get_hover()` on `LspRuntime` with validation + coordinate conversion - [ ] Implement `get_definitions()` on `LspRuntime` with validation + coordinate conversion - [ ] Wire HOVER and DEFINITIONS in tool adapter `_make_runtime_handler()` - [ ] Update `lsp_tool_adapter_coverage.feature` (HOVER -> REFERENCES for unimplemented test) - [ ] Tests (Behave): 21 scenarios covering all response paths and input validation - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors
hamza.khyari added this to the v3.6.0 milestone 2026-04-01 14:52:30 +00:00
freemo self-assigned this 2026-04-02 06:13:58 +00:00
Owner

PR #1240 Review Complete

Code quality: Approved — Implementation is spec-aligned, well-structured, and thoroughly tested (21 Behave scenarios).

Merge status: ⚠️ Blocked by conflictCHANGELOG.md has a textual merge conflict (both the PR branch and master added entries at the top of ## Unreleased). All source code files have no conflicts.

Action needed: Rebase feature/lsp-epic-completion onto current master to resolve the CHANGELOG.md conflict. Once rebased, the PR is ready to merge.

## PR #1240 Review Complete **Code quality: ✅ Approved** — Implementation is spec-aligned, well-structured, and thoroughly tested (21 Behave scenarios). **Merge status: ⚠️ Blocked by conflict** — `CHANGELOG.md` has a textual merge conflict (both the PR branch and master added entries at the top of `## Unreleased`). All source code files have no conflicts. **Action needed:** Rebase `feature/lsp-epic-completion` onto current `master` to resolve the CHANGELOG.md conflict. Once rebased, the PR is ready to merge.
Owner

PR #1240 reviewed, approved, and merged.

The branch was rebased onto current master to resolve a CHANGELOG.md conflict (both master and the branch had added entries to the ## Unreleased section). All source code was conflict-free. The implementation of get_hover and get_definitions for LspClient, LspRuntime, and the tool adapter has been merged with 21 Behave BDD scenarios.

PR #1240 reviewed, approved, and merged. The branch was rebased onto current master to resolve a CHANGELOG.md conflict (both master and the branch had added entries to the `## Unreleased` section). All source code was conflict-free. The implementation of `get_hover` and `get_definitions` for `LspClient`, `LspRuntime`, and the tool adapter has been merged with 21 Behave BDD scenarios.
freemo 2026-04-02 16:50:27 +00:00
Owner

PR #1240 reviewed, approved, and merged via squash.

Review summary: Implementation correctly adds get_hover and get_definitions to LspClient and LspRuntime, wires them into the tool adapter (4 of 11 capabilities now working), and includes 21 Behave BDD scenarios with full path coverage. Code is spec-aligned, well-tested, and follows established patterns with an improvement (try/finally for did_close() safety).

PR #1240 reviewed, approved, and merged via squash. **Review summary:** Implementation correctly adds `get_hover` and `get_definitions` to `LspClient` and `LspRuntime`, wires them into the tool adapter (4 of 11 capabilities now working), and includes 21 Behave BDD scenarios with full path coverage. Code is spec-aligned, well-tested, and follows established patterns with an improvement (`try/finally` for `did_close()` safety).
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#824 Epic: LSP Functional Runtime
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#1243
No description provided.