Merge pull request #1110 from oraios/fix/markdown-heading-symbol

Fix: Markdown heading symbols were low-level symbols

src/solidlsp/language_servers/marksman.py

@@ -6,11 +6,19 @@ Contains various configurations and settings specific to Markdown.
 import logging
 import os
 import pathlib
+from collections.abc import Hashable
 
 from overrides import override
 
-from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer
+from solidlsp.ls import (
+    DocumentSymbols,
+    LanguageServerDependencyProvider,
+    LanguageServerDependencyProviderSinglePath,
+    LSPFileBuffer,
+    SolidLanguageServer,
+)
 from solidlsp.ls_config import LanguageServerConfig
+from solidlsp.ls_types import SymbolKind, UnifiedSymbolInformation
 from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
 from solidlsp.settings import SolidLSPSettings
 
@@ -106,6 +114,35 @@ class Marksman(SolidLanguageServer):
     def is_ignored_dirname(self, dirname: str) -> bool:
         return super().is_ignored_dirname(dirname) or dirname in ["node_modules", ".obsidian", ".vitepress", ".vuepress"]
 
+    def _document_symbols_cache_fingerprint(self) -> Hashable | None:
+        request_document_symbols_override_version = 1
+        return request_document_symbols_override_version
+
+    @override
+    def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols:
+        """Override to remap Marksman's heading symbol kinds from String to Namespace.
+
+        Marksman LSP returns all markdown headings (h1-h6) with SymbolKind.String (15).
+        This is problematic because String (15) >= Variable (13), so headings are
+        classified as "low-level" and filtered out of symbol overviews.
+        Remapping to Namespace (3) fixes this and is semantically appropriate
+        (headings are named sections containing other content).
+        """
+        document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer)
+
+        # NOTE: When changing this method, also update the cache fingerprint method above
+
+        def remap_heading_kinds(symbol: UnifiedSymbolInformation) -> None:
+            if symbol["kind"] == SymbolKind.String:
+                symbol["kind"] = SymbolKind.Namespace
+            for child in symbol.get("children", []):
+                remap_heading_kinds(child)
+
+        for sym in document_symbols.root_symbols:
+            remap_heading_kinds(sym)
+
+        return document_symbols
+
     @staticmethod
     def _get_initialize_params(repository_absolute_path: str) -> InitializeParams:
         """

test/solidlsp/markdown/test_markdown_basic.py

@@ -7,8 +7,10 @@ like request_document_symbols using the markdown test repository.
 
 import pytest
 
+from serena.symbol import LanguageServerSymbol
 from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
+from solidlsp.ls_types import SymbolKind
 
 
 @pytest.mark.markdown
@@ -24,17 +26,19 @@ class TestMarkdownLanguageServerBasics:
     @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True)
     def test_markdown_request_document_symbols(self, language_server: SolidLanguageServer) -> None:
         """Test request_document_symbols for markdown files."""
-        # Test getting symbols from README.md
         all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots()
 
-        # Extract heading symbols (LSP Symbol Kind 15 is String, but marksman uses kind 15 for headings)
-        # Note: Different markdown LSPs may use different symbol kinds for headings
-        # Marksman typically uses kind 15 (String) for markdown headings
         heading_names = [symbol["name"] for symbol in all_symbols]
 
         # Should detect headings from README.md
         assert "Test Repository" in heading_names or len(all_symbols) > 0, "Should find at least one heading"
 
+        # Verify that markdown headings are remapped from String to Namespace
+        for symbol in all_symbols:
+            assert (
+                symbol["kind"] == SymbolKind.Namespace
+            ), f"Heading '{symbol['name']}' should have kind Namespace, got {SymbolKind(symbol['kind']).name}"
+
     @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True)
     def test_markdown_request_symbols_from_guide(self, language_server: SolidLanguageServer) -> None:
         """Test symbol detection in guide.md file."""
@@ -54,7 +58,6 @@ class TestMarkdownLanguageServerBasics:
     @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True)
     def test_markdown_request_document_symbols_with_body(self, language_server: SolidLanguageServer) -> None:
         """Test request_document_symbols with body extraction."""
-        # Test with include_body=True
         all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots()
 
         # Should have found some symbols
@@ -63,3 +66,30 @@ class TestMarkdownLanguageServerBasics:
         # Note: Not all markdown LSPs provide body information for symbols
         # This test is more lenient and just verifies the API works
         assert all_symbols is not None, "Should return symbols even if body extraction is limited"
+
+    @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True)
+    def test_markdown_headings_not_low_level(self, language_server: SolidLanguageServer) -> None:
+        """Test that markdown headings are not classified as low-level symbols.
+
+        Verifies the fix for the issue where Marksman's SymbolKind.String (15)
+        caused all headings to be filtered out of get_symbols_overview.
+        """
+        all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots()
+        assert len(all_symbols) > 0, "Should find headings in README.md"
+
+        for symbol in all_symbols:
+            ls_symbol = LanguageServerSymbol(symbol)
+            assert (
+                not ls_symbol.is_low_level()
+            ), f"Heading '{symbol['name']}' should not be low-level (kind={SymbolKind(symbol['kind']).name})"
+
+    @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True)
+    def test_markdown_nested_headings_remapped(self, language_server: SolidLanguageServer) -> None:
+        """Test that nested headings (h1-h5) are all remapped from String to Namespace."""
+        all_symbols, _root_symbols = language_server.request_document_symbols("api.md").get_all_symbols_and_roots()
+
+        # api.md has deeply nested headings (h1 through h5)
+        assert len(all_symbols) > 5, "api.md should have many headings"
+
+        for symbol in all_symbols:
+            assert symbol["kind"] == SymbolKind.Namespace, f"Nested heading '{symbol['name']}' should be remapped to Namespace"