feat(lsp): add agent-callable LSP query tools - #55532
Conversation
…-references, hover, document-symbols, workspace-symbols) Implements the active query phase of Issue NousResearch#516 (Phase 3), building on the existing passive diagnostic pipeline (Phase 2, PR NousResearch#24168). Adds 5 new LSP query methods to LSPClient: - go_to_definition() - find_references() - hover() - document_symbols() - workspace_symbols() Each uses the existing _send_request_with_retry() infrastructure and requires no new dependencies. Adds LSPService.query_lsp_sync() as the synchronous bridge from the agent-tool layer into the async LSP client, including automatic file opening via open_file() when the file isn't already open on the server. Registers 5 new Hermes agent tools under the 'coding' toolset: - lsp_go_to_definition - lsp_find_references - lsp_hover - lsp_document_symbols - lsp_workspace_symbols Also adds these tools to hermes-acp and hermes-api-server toolsets. All tools return structured JSON with 'error' and 'result' fields for robust model consumption. Graceful degradation when LSP is unavailable. Tests: 14 new tests (159 total LSP tests, all passing).
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
New LSP query tools (6 files, +1360 additions). Adds agent-callable LSP methods: go_to_definition, find_references, hover, document_symbols, workspace_symbols. Well-structured with proper timeout handling and result formatting.
Looks Good
- Clean API design with consistent parameter patterns (path, line, character)
- Proper timeout handling via
_send_request_with_retrywithLSP_QUERY_TIMEOUT - Good result formatting:
_format_locationand_format_symbolhandle both Location and LocationLink formats - DocumentSymbol tree flattening is correct (recursive with indent)
- Symbol kind names cover all LSP standard kinds
- Export includes
LSP_QUERY_TIMEOUTin__all__
Note
- The
_SYMBOL_KIND_NAMESdict could be made aClassVaror module constant for slightly cleaner access, but this is minor
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the existing LSP transport rather than adding a dependency. Current main still exposes LSP only through post-write diagnostics (website/docs/user-guide/features/lsp.md:9-15), so the feature remains needed.
Problems
tools/lsp_tools.py:389passes a directory toquery_lsp_sync. That path is gated byLSPService.enabled_for()(agent/lsp/manager.py:248-279), which selects a server with extension/basename matching (agent/lsp/servers.py:150-188). A workspace directory selects no server, solsp_workspace_symbolsreturns an empty success result. The test attests/agent/lsp/test_query_tools.py:292-302does not catch this because it only assertserror == "".tools/lsp_tools.py:177-181ignorestargetUriandtargetRange, althoughLSPClient.go_to_definition()accepts LocationLink results (agent/lsp/client.py:900-919). Those valid responses render as?rather than the definition target.
Suggested changes
- Select a concrete server for workspace-symbol queries, then assert a returned mock symbol in an end-to-end test.
- Format both Location and LocationLink results, with a LocationLink regression test.
Automated hermes-sweeper review.
| # workspace/symbol doesn't need a file path; pass cwd for workspace detection | ||
| cwd = __import__("os").getcwd() | ||
| result = svc.query_lsp_sync( | ||
| cwd, |
There was a problem hiding this comment.
query_lsp_sync() first calls enabled_for(file_path), which selects a server via extension/basename matching. cwd is normally a directory, so it has no matching server and this always becomes an empty successful result. Pass a representative source path/language or add workspace-level client selection, and assert the mock symbol result in this handler test.
| for loc in locations: | ||
| from agent.lsp.client import uri_to_path | ||
|
|
||
| uri = loc.get("uri", "") |
There was a problem hiding this comment.
go_to_definition() permits compliant LocationLink results, but those use targetUri/targetRange, not uri/range; this formatter will emit ? at 1:1. Handle both response shapes (or reuse a shared formatter) and add a LocationLink regression test.
Summary
Implements Phase 3 of Issue #516 — active LSP code intelligence tools, building on the existing passive diagnostic pipeline (Phase 2, PR #24168).
Adds 5 new agent-callable tools to the
codingtoolset (andhermes-acp/hermes-api-server):lsp_go_to_definition— find where a symbol is definedlsp_find_references— find all usages of a symbollsp_hover— get type information and documentationlsp_document_symbols— list all symbols in a filelsp_workspace_symbols— search symbols across the projectArchitecture
The implementation follows the existing LSP infrastructure patterns:
agent/lsp/client.pygo_to_definition(),find_references(),hover(),document_symbols(),workspace_symbols()) using_send_request_with_retry()— no new dependenciesagent/lsp/manager.pyquery_lsp_sync()wraps_query_async()through the existing_BackgroundLoop; auto-opens files on the server when neededtools/lsp_tools.pyregistry.register(), each returning structured JSON outputtoolsets.pycoding,hermes-acp, andhermes-api-servertoolsetstests/agent/lsp/test_query_tools.pyDesign decisions
_send_request_with_retry()query_lsp_sync()opens the file on the server if not already open, so single-shot queries work without a preceding write_query_async()maps LSP method names to the matchingLSPClientmethod, with a fallback to_send_request_with_retry()for arbitrary methodsworkspaceSymboldoesn't need a file path; the file_path parameter is used for workspace detectionTesting