|
| 1 | +# Design Doc: Optional LSP-backed find-references for trace-up precision |
| 2 | + |
| 3 | +> **Status:** Accepted |
| 4 | +> **Date:** 2026-07-14 |
| 5 | +> **Author:** Claude (direct implementation, no worker — user directive) |
| 6 | +> **Related issues:** #255 |
| 7 | +> **Related PRs:** (this PR) |
| 8 | +
|
| 9 | +--- |
| 10 | + |
| 11 | +## Problem |
| 12 | + |
| 13 | +Gap-analysis vs Serena MCP: Serena's find-references uses the language |
| 14 | +server (`textDocument/references`) — a real AST/symbol table, high precision, |
| 15 | +no missed references. CodeLens's caller/reference discovery |
| 16 | +(`context --check trace --direction up`) uses a home-grown call graph that is |
| 17 | +an *approximation*. Evidence: a run of ref-count/trace edge-case bugs were |
| 18 | +found and fixed across the project (#210, #219, #222, #223 module-level |
| 19 | +callers). The graph will always have edge cases; LSP `textDocument/references` |
| 20 | +does not. |
| 21 | + |
| 22 | +CodeLens already had the LSP capability: `lsp_client.py:350` |
| 23 | +`find_references(file, line, character)` issues `textDocument/references`, and |
| 24 | +`hybrid_engine.py` already used it internally to *verify* dead-code and |
| 25 | +enhance impact (`_filter_external_references`). But that precision was never |
| 26 | +exposed as a navigation path for agents. |
| 27 | + |
| 28 | +## Goal |
| 29 | + |
| 30 | +When `--deep` is active **and** an LSP server is available, |
| 31 | +`context --check trace --direction up` (and `--direction both`) uses LSP |
| 32 | +`textDocument/references` as the precision source for callers, annotating the |
| 33 | +result `trace_source: "lsp"`. Without `--deep`, or without a live LSP server, |
| 34 | +or when the symbol can't be resolved/located — the existing graph path is used |
| 35 | +unchanged (`trace_source: "graph"`). Zero-config keeps working with no |
| 36 | +regression and no LSP dependency. |
| 37 | + |
| 38 | +## Changes |
| 39 | + |
| 40 | +### Modified Files |
| 41 | +- `scripts/hybrid_engine.py` — new |
| 42 | + `HybridEngine.find_references_for_symbol(symbol_name)`. Reuses existing |
| 43 | + machinery only: `_find_symbol_definition` (registry lookup) to resolve the |
| 44 | + symbol → `(file, line)`, `_find_symbol_char` to locate the column, then |
| 45 | + `lsp_client.find_references(..., include_declaration=False)`, then |
| 46 | + `_filter_external_references` to drop the definition site. Converts LSP |
| 47 | + 0-indexed lines to 1-indexed. Returns `None` (not `[]`) when LSP is |
| 48 | + inactive or the symbol can't be resolved, so the caller can distinguish |
| 49 | + "no LSP path" from "LSP ran, found zero references". Never raises. |
| 50 | +- `scripts/commands/trace.py`: |
| 51 | + - `execute()` — after the graph `trace_symbol` call, when `args.deep` is |
| 52 | + truthy and `direction in ("up", "both")`, calls the new |
| 53 | + `_apply_lsp_trace_up`; otherwise annotates `trace_source: "graph"`. |
| 54 | + - `_apply_lsp_trace_up(name, workspace, result)` — creates a hybrid engine |
| 55 | + with `deep=True`, and **only if `engine.lsp_active`** replaces |
| 56 | + `result["chains"]["up"]` with LSP-derived caller entries |
| 57 | + (`source: "lsp"`), sets `trace_source: "lsp"`, and records |
| 58 | + `graph_callers_found` / `lsp_callers_found` for A/B comparison. On engine |
| 59 | + creation failure, inactive LSP, or `None` refs, it leaves the graph |
| 60 | + chains untouched and annotates `trace_source: "graph"`. Always calls |
| 61 | + `engine.cleanup()`. |
| 62 | + |
| 63 | +### No new LSP infrastructure |
| 64 | +Per the issue constraint, this reuses `lsp_client.find_references` and the |
| 65 | +existing `hybrid_engine` resolution/filter helpers. `find_references_for_symbol` |
| 66 | +is orchestration over those, not new LSP plumbing. LSP is never made a hard |
| 67 | +dependency — the graph path is the default and the fallback. |
| 68 | + |
| 69 | +### Placement rationale |
| 70 | +The precision upgrade lives at the command boundary (`commands/trace.py`), |
| 71 | +not in `trace_engine.py`. `trace_engine` stays a pure graph/flat backend with |
| 72 | +an unchanged output shape; the opt-in LSP overlay is applied on top only when |
| 73 | +`--deep` + LSP are present. This keeps the zero-config trace path completely |
| 74 | +untouched and easy to reason about. |
| 75 | + |
| 76 | +## Testing |
| 77 | + |
| 78 | +`tests/test_issue255_lsp_references.py` (8 tests): |
| 79 | + |
| 80 | +**Graceful degradation — live (real scan + CLI-equivalent trace):** |
| 81 | +- no `--deep` → `trace_source: "graph"`, callers still found, LSP path never |
| 82 | + touched (no `lsp_available` key). |
| 83 | +- `--deep` on a real scanned workspace → well-formed output, `status: ok`, |
| 84 | + no crash, no hang, `trace_source` in `{graph, lsp}`. |
| 85 | + |
| 86 | +**LSP happy path — mocked** (`create_hybrid_engine` / `find_references` |
| 87 | +mocked, mirroring #253): |
| 88 | +- LSP active + refs → chains.up rewritten to LSP entries, `trace_source: lsp`, |
| 89 | + stats + `graph/lsp_callers_found` updated, `cleanup()` called. |
| 90 | +- LSP inactive → graph retained, `lsp_available: false`. |
| 91 | +- refs `None` (symbol unresolved) → graph retained. |
| 92 | +- engine creation raises → graph retained. |
| 93 | +- `find_references_for_symbol` resolves def site, excludes it, converts |
| 94 | + 0→1-indexed; returns `None` when LSP inactive. |
| 95 | + |
| 96 | +**Live verification (real CLI, this environment):** |
| 97 | +- `codelens context <ws> --check trace --name helper --direction up` → |
| 98 | + `trace_source: graph`, callers found — zero-config unaffected. |
| 99 | +- same with `--deep` → `lsp_available: true`, but the live server did not |
| 100 | + return usable references for the symbol, so it degraded to |
| 101 | + `trace_source: graph` — no hang, no error, exit 0. |
| 102 | + |
| 103 | +**LSP happy-path live limitation (honest):** the LSP happy path |
| 104 | +(`trace_source: lsp` with real references) could **not** be verified against a |
| 105 | +live server in the dev environment — rust-analyzer (the only installed |
| 106 | +server) does not respond to `initialize` within 60s (pre-existing, same |
| 107 | +limitation documented in #253). The happy path is covered by the mocked tests |
| 108 | +above; only the graph fallback and graceful-degradation paths are |
| 109 | +live-verified. |
| 110 | + |
| 111 | +## Backward compatibility |
| 112 | + |
| 113 | +Zero-config (`context --check trace ...` without `--deep`) is byte-for-byte |
| 114 | +unchanged — the graph result only gains a `trace_source: "graph"` annotation. |
| 115 | +No behavior change to `trace_engine.py`. `--direction down` is never touched |
| 116 | +by this feature (callees are not references). |
0 commit comments