Add the semantic navigation queries, with source context by lshrinivas · Pull Request #3 · martint/henka

lshrinivas · 2026-08-27T14:08:12Z

Fills in the semantic navigation queries SPEC.md §8 has always promised but nothing implemented, adds two the language servers already answer, and changes what a navigation result contains.

The operations

Spec first (docs/spec-history/0002-navigation-queries.md), then one commit per operation:

Operation	Target	Answers
`go-to-definition`	position	where is this declared?
`find-implementations`	position	which concrete overrides exist?
`describe-symbol`	position	what type / signature / docs does this have?
`file-outline`	file	what is in this file?
`prepare-call-hierarchy`	position	which declaration does this position name?
`incoming-calls`	project + item	who calls this?
`outgoing-calls`	project + item	what does this call?

Each is a read-only query offered by all three LSP-backed providers — Java, Rust, TypeScript/JavaScript — with the conversion shared in henka-lsp so every provider returns one shape. Providers also start advertising the client capabilities their new requests need.

Operations are named for the caller's intent rather than the LSP gesture, following find-usages: LSP's hover is describe-symbol, documentSymbol is file-outline.

Results now quote the source they point at

A bare coordinate leaves an agent no better off than a text search — it still opens the file to see what is there, where grep had already shown it. Measured against Claude Code, returning the source alongside the location increased accuracy significantly and cut follow-up file reads from 15.2 to 3.2 per episode.

So every location a query returns carries text, the line at that coordinate, with an optional context_lines window. find-usages and symbol-search are retrofitted in the first commit of the stack, so the catalog never shows two conventions. The text is read from the working copy the request was answered against — the overlay included — so it always matches the coordinate beside it.

This is a deliberate departure from LSP, whose clients are editors that already have the buffer open.

Notes

The call hierarchy is three operations, not one. It follows LSP's two-phase protocol: prepare-call-hierarchy resolves a position to items, and both directions take an item back. The item carries an opaque handle — the server's own CallHierarchyItem, passed through untouched, because jdtls attaches private data it requires on the follow-up. Resolution happens once, and overloads stay distinct.
Empty is never an error. No definition, no implementations, no callers, no symbols: all count: 0. An unreadable file costs an entry its text, not its place in the result.
Compile- and unit-tested only. Exercising these end to end needs jdtls / rust-analyzer / tsserver running (cargo test -- --ignored).
Rebase note: Carry a code-action kind on operation descriptors in LSP proxy for Claude Code ↔ Henka #1 adds a field to every OperationDescriptor literal. This branch adds a good many more of them; whichever lands second needs the field reconciled across them — mechanical, but worth knowing.

🤖 Generated with Claude Code

martint

Reviewed against merge base fda72534 (single commit). Verified locally: cargo check --workspace --all-targets on the branch tip is clean, no warnings. (No rustfmt component on the 1.98.0 toolchain here, so formatting is unverified.)

Inline comments below; two points that don't belong on a line:

🔴 must-do — the docs that advertise the operation menu weren't updated

docs/SPEC.md §8 already lists symbol-search under "Semantic queries", so the spec is fine — it described this as intended catalog all along. Two other places are now wrong or incomplete:

README.md:71 enumerates the concrete per-language menu: "Java offers rename, find-usages, change-signature, …; Rust offers …; TypeScript/JavaScript offers …". All three lists are now stale — this PR adds symbol-search to every one of them. A reader takes that sentence as the shipped menu, and it's a one-word edit in each clause.
crates/henka-server/skills/refactoring.md is the agent-facing doc, and it's where this change actually pays off. The "prefer a semantic query over text search" paragraph currently argues the case with find-usages only. But the motivation in your PR description — "a caller holding only a symbol's name has to grep for it and guess which hit is the declaration" — is precisely the gap symbol-search closes, and the skill never names it. The "Operations come in two kinds" section does mention "symbol search" in the query list, but that predates this PR and reads as aspirational alongside call/type hierarchy, which still don't exist.

The second one isn't bookkeeping. An op that only surfaces through list_operations gets reached for when the agent already thought to look; the skill is what makes it a first reach rather than a fallback after grep — which is the entire premise of the change.

💡 suggestion — the three `SymbolSearchOp` impls are byte-identical apart from two tokens

Diffing them, the only differences across java/rust/ts are languages (vec![Language::Java] / vec![Language::Rust] / languages()) and the session accessor (jdtls(ctx) / ra(ctx) / ts(ctx)). The descriptor, the schema, the param extraction, the ensure_indexed → workspace/symbol → symbols_to_query body: all three copies are the same 50 lines.

Per-crate duplication is the house style and I wouldn't push on it for RenameOp or FindUsagesOp — those genuinely diverge (rust passes newName directly, java's CodeActionOp has its own title-matching, ts wraps a different session). This one doesn't diverge at all, and won't: workspace/symbol takes one string and returns one shape regardless of language.

henka-lsp is already the place where the shared LSP mapping lives (symbols_to_query landed there for exactly this reason). A pub async fn symbol_search(client: &LspClient, root: &Path, query: &str) -> Result<Value> there would leave each crate with a descriptor plus a three-line run, and the fix for anything raised in the inline comments — the empty-query default, a limit, kind naming — would then be one edit instead of three kept-in-sync ones.

Worth doing now while there's exactly one caller shape to extract, rather than after the three copies have drifted.

martint · 2026-09-03T11:34:49Z

+        let query = req
+            .params
+            .get("query")
+            .and_then(Value::as_str)
+            .unwrap_or("");


🔴 must-do — The descriptor declares "required": ["query"] but the run silently substitutes "" when it's absent. Two problems with that:

It contradicts the sibling ops. RenameOp (operations.rs:70) does .ok_or_else(|| CoreError::InvalidTarget("`new_name` is required".into()))? for exactly this situation. A caller that omits query should get that error, not a request.

"" isn't a degenerate no-op at the LSP layer, it's a wildcard. jdtls turns the query into a query + "*" search pattern, so an empty query matches the entire workspace index and dumps it into the caller's context. rust-analyzer returns nothing for the same input, so the "forgot the param" behavior also silently differs by language.

Suggest mirroring RenameOp and erroring instead. Same in the rust (operations.rs:310-314) and ts (operations.rs:135-139) copies.

Fixed — missing query now errors via InvalidTarget, matching RenameOp's new_name handling, in all three crates.

martint · 2026-09-03T11:34:49Z

+            params_schema: json!({
+                "type": "object",
+                "required": ["query"],
+                "properties": {
+                    "query": {
+                        "type": "string",
+                        "description": "Partial or full symbol name to search for."
+                    }
+                }
+            }),
+        }
+    }
+


💡 suggestion — Consider a limit parameter (with a default) alongside query.

Unlike find-usages, which is anchored to one resolved symbol, this is a prefix/fuzzy query — a two- or three-character query on a large project is a normal thing for a caller to try, and the result goes straight into an agent's context window. The servers don't protect you consistently either: rust-analyzer caps its workspace symbol search internally, jdtls doesn't cap at all.

Truncating with the count reported honestly ("count" = total matched, plus a truncated flag, rather than silently returning fewer) also gives the caller the signal it needs to refine the query instead of guessing.

Added an optional limit param (default 200); the response now reports the total count plus truncated: true when capped.

martint · 2026-09-03T11:34:49Z

+    if value.is_null() {
+        return Ok(json!({ "count": 0, "symbols": [] }));
+    }
+    let items: Vec<LspSymbolInfo> = serde_json::from_value(value)?;


❓ question — This deserializes strictly into Vec<LspSymbolInfo>, which requires location.range on every entry. LSP 3.17 widened the workspace/symbol result to SymbolInformation[] | WorkspaceSymbol[] | null, and a WorkspaceSymbol's location is allowed to be { uri } with no range (the client resolves it later via workspaceSymbol/resolve).

Henka doesn't advertise workspace.symbol.resolveSupport in any of the three initialize payloads, so a spec-abiding server shouldn't send the reduced form — but if one does, this doesn't degrade, it fails: serde_json::from_value errors and the whole call comes back as an opaque backend error with no hint about what the server returned.

Did you check what jdtls / rust-analyzer / tsserver actually return on the versions you're pinning? If the answer is "SymbolInformation everywhere, today", that's fine and worth a line of comment here recording it. If it's uncertain, making range optional and skipping (or emitting without coordinates) the entries that lack one keeps one odd server from taking down the whole result.

Good catch — range is now optional on the wire type, and a symbol missing one is dropped instead of failing the whole deserialize.

martint · 2026-09-03T11:34:49Z

+                .to_string();
+            let mut obj = json!({
+                "name": s.name,
+                "kind": s.kind,


💡 suggestion — kind goes out as the raw LSP SymbolKind integer, so a result reads "kind": 5 and the caller has to know the enum to make anything of it.

That matters more here than it would elsewhere: the PR's whole premise is that a caller holding only a name can't tell which hit is the declaration it wants. Given ["Foo", "Foo", "Foo"] in three files, "class vs. constructor vs. field" is exactly the disambiguator, and a bare 5 doesn't supply it — the agent on the other end has to have the LSP spec memorized to use the field at all.

Suggest mapping to a lowercase name ("class", "method", "field", …) with a fallback for unknown values, so the output is self-describing the way the rest of the query results are.

Done — kind is now a lowercase name ("class", "method", …) via a SymbolKind mapping, with a numeric-string fallback for unmapped values.

martint · 2026-09-03T11:34:49Z

+
+#[derive(Debug, Deserialize)]
+struct LspSymbolInfo {
+    name: String,
+    kind: u32,
+    location: LspLocation,
+    #[serde(rename = "containerName", default)]
+    container_name: Option<String>,
+}


💡 suggestion — Every other Lsp* wire type in this file — LspPosition, LspRange, LspTextEdit, LspTextDocument, LspDocumentChange, LspWorkspaceEdit, LspLocation — is declared in the block at the top, above the first pub fn. This one sits in the middle of the file, after the function that consumes it. Move it up with the others.

Moved LspSymbolInfo and its location type up next to the other Lsp* wire types.

martint · 2026-09-03T11:34:49Z

+/// Convert an LSP `SymbolInformation[]` response (from `workspace/symbol`) into
+/// a structured symbol-search result, with paths expressed relative to `root`
+/// where possible.
+pub fn symbols_to_query(value: Value, root: &Path) -> Result<Value> {


💡 suggestion — The PR description says "compile- and unit-tested only", but no test came with it — mod tests at the bottom is unchanged, and symbols_to_query is the one piece of this change that's testable without a language server running.

It has four behaviors worth pinning, all cheap:

Value::Null → {"count": 0, "symbols": []} (mirroring null_edit_is_empty)

a URI under root comes back as a relative path

a URI outside root falls through strip_prefix's unwrap_or and comes back absolute — the branch most likely to surprise someone later

containerName present → container_name in the output; absent → key omitted entirely, not null

That last one is a shape contract for every consumer of this operation, and right now nothing holds it in place.

Added unit tests for symbols_to_query: null input, path relative/absolute, container_name presence, missing-range filtering, and truncation.

lshrinivas · 2026-09-08T18:43:57Z

Addressed the two non-line points too: added symbol-search to the per-language menus in README.md and named it explicitly in the skill's semantic-query paragraph in refactoring.md; and extracted the shared request+normalize logic into LspSession::symbol_search in henka-lsp, so each provider's SymbolSearchOp::run is now just descriptor + param extraction.

Henka's edit side is broad while its query side is two operations wide, and three of the semantic queries SPEC.md already promises — definition, implementations, call hierarchy — have nothing behind them. Every navigation question a query can't answer falls back to reading files, which is slower, burns context, and answers from text what the compiler already knows. Write down the whole set before implementing any of it: what each operation targets, what it returns, why the call hierarchy ships as three operations rather than one, and how LSP's several response shapes per request collapse to the one shape a caller reads. Measuring LSP navigation against grep for Claude Code showed a bare coordinate leaves the agent no better off — it still opens the file to see what is there, where grep had already shown it. Returning the source alongside the location moved pass@1 on rename tasks from 0.67 to 0.83 and cut follow-up reads from 15.2 to 3.2 per episode, so require it of every location these queries return, and of the two that already shipped without it.

Every operation so far is targeted by a coordinate, so a caller holding only a symbol's name has to grep for it and guess which hit is the declaration — the text-search guesswork these tools exist to replace. Exposing each language server's workspace/symbol request closes the gap: a name resolves to a file and range the position-targeted operations can then act on, over the same MCP surface and with no editor involved. The result is normalized the way find-usages already normalizes locations — paths relative to the project root, range fields flattened — so a caller reads one shape and never handles a mount path or an LSP type.

A bare coordinate leaves a caller no better off than a text search: it still has to open the file to see what is there, which is the read these queries exist to replace. find-usages and symbol-search now carry the source line at each location, with an optional context_lines window, read from the working copy the request was answered against so the text always matches the coordinate beside it.

go-to-definition answers the question an agent otherwise guesses at from imports and naming, and answers it with the declaring line in hand. The result is always a list: a definition can legitimately be several places, and a shape that varies by case is a shape callers get wrong.

An override and a call site are different questions, and only the override says where the behavior an agent is about to change lives. Rust and TypeScript also start advertising the capability the request needs, which they had no reason to before.

describe-symbol answers "what is this?" without reading the declaration — the resolved generic type, the selected overload, the doc comment on a dependency whose source isn't even in the tree. LSP calls it hover, a gesture name for a caller that has no cursor; the three content shapes servers still send collapse into one markdown string.

file-outline answers "what is in here?" for a fraction of a file's length, and hands back the coordinates a rename or find-usages needs. Symbols stay nested because containment is part of the answer, and each is addressed and quoted by its name rather than its body — a class's range is the whole file.

The first half of a call hierarchy walk: a position names one or more declarations, and each item comes back with the server's own opaque handle to it alongside readable fields. Resolution happens once, so the walk that follows asks about a fixed declaration rather than re-resolving a coordinate — which also keeps overloads distinct.

Answers "who calls this?" from the item prepare-call-hierarchy resolved, returning each caller with its own handle so the walk can continue, and each call site with the line it is written on — which is how a caller tells the overload it cares about from the one it does not.

The other direction of the walk. Its call sites live in the queried item's own file rather than the callee's, so they are quoted from there — the line a call is written on is what says whether the walk needs to keep going.

martint reviewed Sep 3, 2026

View reviewed changes

lshrinivas force-pushed the symbol-search branch from ef4c64f to aba0a93 Compare September 8, 2026 17:37

lshrinivas added 10 commits September 8, 2026 18:38

Find what a call hierarchy item calls

1e4e13d

The other direction of the walk. Its call sites live in the queried item's own file rather than the callee's, so they are quoted from there — the line a call is written on is what says whether the walk needs to keep going.

lshrinivas force-pushed the symbol-search branch from aba0a93 to 1e4e13d Compare September 9, 2026 00:57

lshrinivas requested a review from martint September 9, 2026 00:57

lshrinivas changed the title ~~Let callers reach a symbol without knowing its file~~ Add the semantic navigation queries, with source context Sep 9, 2026

Conversation

lshrinivas commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The operations

Results now quote the source they point at

Notes

Uh oh!

martint left a comment

Choose a reason for hiding this comment

🔴 must-do — the docs that advertise the operation menu weren't updated

💡 suggestion — the three SymbolSearchOp impls are byte-identical apart from two tokens

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lshrinivas commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lshrinivas commented Aug 27, 2026 •

edited

Loading

💡 suggestion — the three `SymbolSearchOp` impls are byte-identical apart from two tokens