fix(solidlsp): gracefully degrade when a language server lacks textDocument/documentSymbol

[mariadb-KyleHutchinson]

Problem

get_diagnostics_for_file crashes outright (ToolCallError / SolidLSPException: Unhandled method textDocument/documentSymbol (-32601)) for any Ansible YAML file that has at least one real ansible-lint diagnostic to report.

The tool groups each diagnostic under the symbol that owns it via find_diagnostic_owner_symbol, which requests textDocument/documentSymbol from the language server. Its docstring already promises graceful degradation for this: "If a diagnostic cannot be mapped to a symbol, it is grouped under the special name path <file>." But that fallback was never implemented for the case where the server doesn't support documentSymbol at all — the request just throws, and the exception propagates all the way up through symbol.py's find_diagnostic_owner_symbol and symbol_tools.py's diagnostics grouping, failing the whole tool call.

This isn't a one-off or a config issue: ansible-language-server (the standard, actively-maintained Ansible LSP, now folded into vscode-ansible) does not implement textDocument/documentSymbol and the maintainers have explicitly declined to add it — see ansible/vscode-ansible#601, closed NOT_PLANNED in Jan 2024. So this reliably breaks get_diagnostics_for_file for every Ansible role/task/playbook file that has lint findings.

Example from a real session, before the fix:

ERROR ... SolidLSPException: Error processing request textDocument/documentSymbol with params:
{'textDocument': {'uri': 'file:///.../roles/columnstore_setup/tasks/multi_node.yml'}}
(caused by Unhandled method textDocument/documentSymbol (-32601))
...
serena.tools.tools_base.ToolCallError: SolidLSPException: Error processing request textDocument/documentSymbol...

Fix

Updated approach (per review): the fix is scoped to AnsibleLanguageServer rather than the shared base class. src/solidlsp/language_servers/ansible_language_server.py now overrides _request_document_symbols to unconditionally return None, since ansible-language-server never implements textDocument/documentSymbol — there's no point sending a request known to always fail. This matches the existing per-server override pattern already used by eclipse_jdtls.py and angular_language_server.py for other server-specific quirks.

src/solidlsp/ls.py is unchanged from main: the original version of this PR added a general try/except SolidLSPException around the document_symbol call in the shared get_raw_document_symbols, but review (see discussion below) pointed out this widened the blast radius to every language server and risked masking LanguageServerTerminatedException — the exception tools_base.py relies on to trigger language-server restart-and-retry — for three tools beyond just diagnostics (GetSymbolsOverviewTool, FindReferencingSymbolsTool). Scoping the fix to Ansible avoids all of that: no exception is ever thrown or caught, because the request is never sent.

I traced the full call chain before picking this spot:

• request_document_symbols (the caller of _request_document_symbols) already tolerates a None return — it logs a warning and returns an empty DocumentSymbols([]).
• _get_document_symbols_with_locations and _request_symbol_at_location iterate over that (now-empty) result without special-casing.
• find_diagnostic_owner_symbol (serena/symbol.py) already handles None from request_symbol_at_location.

So every layer above _request_document_symbols was already None/empty-safe; the only gap was that Ansible had no override telling it to skip the doomed request in the first place.

Verification

Confirmed against a real ce-tools repo with Ansible + Terraform files, before/after:

Before: get_diagnostics_for_file on roles/columnstore_setup/tasks/multi_node.yml threw ToolCallError, failing the task after ~2s.

After: same file, same request — the log now shows WARNING ... Received None response from the Language Server for document symbols in .../multi_node.yml ... Returning empty list, and the tool call completes normally with the diagnostics grouped under <file>:

{"multi_node.yml": {"Error": {"<file>": [{"message": "Trailing spaces", "code": "yaml[trailing-spaces]", "source": "ansible-lint"}, ...]}}}

Added test/solidlsp/ansible/test_ansible_basic.py::test_request_document_symbols_returns_none_without_contacting_server, a fast unit test asserting the override short-circuits without needing a running language server, node, or npm — closing the gap that the original fix only had manual verification for the crash itself.

Also ran the existing test suite (uv run pytest) against:

• test/serena/test_serena_agent.py -k test_get_diagnostics_for_file
• test/solidlsp/ansible/
• test/solidlsp/python/test_symbol_retrieval.py
• test/solidlsp/toml/test_toml_symbol_retrieval.py

94 passed, 1 pre-existing xfail (test_bare_symbol_names, ansible), 1 skipped, 1 failed due to a missing Go binary in the test environment (unrelated to this change, not a regression).

[AmirF194]

Nice fix, and the reasoning for a plain try/except over a pre-emptive capability check (since a missing documentSymbol handler returns a normal -32601 here instead of crashing the process, unlike the Julia case) makes sense.

One thing worth checking before merge: except SolidLSPException in get_raw_document_symbols is broad enough to also catch a language server that has terminated mid-request. SolidLSPException.cause can be a LanguageServerTerminatedException (ls_process.py), and tools_base.py's _apply relies on that exact exception reaching it to restart the language server and retry the tool call (checks e.is_language_server_terminated(), then calls restart_language_server and retries). With the new catch sitting inside get_raw_document_symbols, a termination during a document_symbol request would be swallowed as "no symbols" instead of propagating to that restart path, for every caller that funnels through here: get_diagnostics_for_file, GetSymbolsOverviewTool, and FindReferencingSymbolsTool, per your own PR description.

The existing pull-diagnostics fallback a few hundred lines up has the identical shape, so this isn't new to your change, but your diff explicitly widens it to three more tools. Is it worth narrowing the catch (skip the swallow when ex.is_language_server_terminated() and let it propagate), or is a dead server already handled some other way before it gets this far?

[opcode81]

We don't generally want to gloss over exceptions in this manner.
Instead, fix this at the source of the problem, i.e. in the Ansible language server: Simply override get_raw_document_symbols there with a plain return None.

[mariadb-KyleHutchinson]

Good call, done. Pushed a commit that:

• Reverts the try/except in ls.py entirely — get_raw_document_symbols is back to the single-line document_symbol call, unchanged from main.
• Adds AnsibleLanguageServer._request_document_symbols, overriding the base class to unconditionally return None with a comment pointing at Support vscode outline ansible/vscode-ansible#601. This matches the existing pattern in eclipse_jdtls.py / angular_language_server.py of overriding _request_document_symbols per-server rather than teaching the base class about server quirks.
• Added a unit test (test_request_document_symbols_returns_none_without_contacting_server) that asserts the override short-circuits without needing a running language server, node, or npm.

No change to any other language server's behavior.

[opcode81]

Great. Please add the change to the changelog, then we can merge this.

[mariadb-KyleHutchinson]

I just added the change to the changelog. Thank you for your guidance.

[mariadb-KyleHutchinson]

@AmirF194 Good catch. With opcode81's suggested approach, this is moot for Ansible specifically: AnsibleLanguageServer._request_document_symbols now overrides the base method entirely and returns None before any request is sent, so there's no SolidLSPException — from a terminated server or otherwise — to catch or swallow at this call site. The restart-on-termination path in tools_base.py is untouched by this fix.

You're right that the identical shape exists already in the pull-diagnostics fallback a few hundred lines up (_supports_pull_diagnostics / text_document_diagnostic), and that one does still risk swallowing LanguageServerTerminatedException. That's a pre-existing issue independent of this PR though, so I'd rather keep this PR scoped to the Ansible fix — I've filed a separate issue for the pull-diagnostics catch instead of widening the diff here.

[opcode81]

Thanks for your contribution!