Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 116 additions & 0 deletions docs/design/0255-lsp-find-references.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# Design Doc: Optional LSP-backed find-references for trace-up precision

> **Status:** Accepted
> **Date:** 2026-07-14
> **Author:** Claude (direct implementation, no worker — user directive)
> **Related issues:** #255
> **Related PRs:** (this PR)

---

## Problem

Gap-analysis vs Serena MCP: Serena's find-references uses the language
server (`textDocument/references`) — a real AST/symbol table, high precision,
no missed references. CodeLens's caller/reference discovery
(`context --check trace --direction up`) uses a home-grown call graph that is
an *approximation*. Evidence: a run of ref-count/trace edge-case bugs were
found and fixed across the project (#210, #219, #222, #223 module-level
callers). The graph will always have edge cases; LSP `textDocument/references`
does not.

CodeLens already had the LSP capability: `lsp_client.py:350`
`find_references(file, line, character)` issues `textDocument/references`, and
`hybrid_engine.py` already used it internally to *verify* dead-code and
enhance impact (`_filter_external_references`). But that precision was never
exposed as a navigation path for agents.

## Goal

When `--deep` is active **and** an LSP server is available,
`context --check trace --direction up` (and `--direction both`) uses LSP
`textDocument/references` as the precision source for callers, annotating the
result `trace_source: "lsp"`. Without `--deep`, or without a live LSP server,
or when the symbol can't be resolved/located — the existing graph path is used
unchanged (`trace_source: "graph"`). Zero-config keeps working with no
regression and no LSP dependency.

## Changes

### Modified Files
- `scripts/hybrid_engine.py` — new
`HybridEngine.find_references_for_symbol(symbol_name)`. Reuses existing
machinery only: `_find_symbol_definition` (registry lookup) to resolve the
symbol → `(file, line)`, `_find_symbol_char` to locate the column, then
`lsp_client.find_references(..., include_declaration=False)`, then
`_filter_external_references` to drop the definition site. Converts LSP
0-indexed lines to 1-indexed. Returns `None` (not `[]`) when LSP is
inactive or the symbol can't be resolved, so the caller can distinguish
"no LSP path" from "LSP ran, found zero references". Never raises.
- `scripts/commands/trace.py`:
- `execute()` — after the graph `trace_symbol` call, when `args.deep` is
truthy and `direction in ("up", "both")`, calls the new
`_apply_lsp_trace_up`; otherwise annotates `trace_source: "graph"`.
- `_apply_lsp_trace_up(name, workspace, result)` — creates a hybrid engine
with `deep=True`, and **only if `engine.lsp_active`** replaces
`result["chains"]["up"]` with LSP-derived caller entries
(`source: "lsp"`), sets `trace_source: "lsp"`, and records
`graph_callers_found` / `lsp_callers_found` for A/B comparison. On engine
creation failure, inactive LSP, or `None` refs, it leaves the graph
chains untouched and annotates `trace_source: "graph"`. Always calls
`engine.cleanup()`.

### No new LSP infrastructure
Per the issue constraint, this reuses `lsp_client.find_references` and the
existing `hybrid_engine` resolution/filter helpers. `find_references_for_symbol`
is orchestration over those, not new LSP plumbing. LSP is never made a hard
dependency — the graph path is the default and the fallback.

### Placement rationale
The precision upgrade lives at the command boundary (`commands/trace.py`),
not in `trace_engine.py`. `trace_engine` stays a pure graph/flat backend with
an unchanged output shape; the opt-in LSP overlay is applied on top only when
`--deep` + LSP are present. This keeps the zero-config trace path completely
untouched and easy to reason about.

## Testing

`tests/test_issue255_lsp_references.py` (8 tests):

**Graceful degradation — live (real scan + CLI-equivalent trace):**
- no `--deep` → `trace_source: "graph"`, callers still found, LSP path never
touched (no `lsp_available` key).
- `--deep` on a real scanned workspace → well-formed output, `status: ok`,
no crash, no hang, `trace_source` in `{graph, lsp}`.

**LSP happy path — mocked** (`create_hybrid_engine` / `find_references`
mocked, mirroring #253):
- LSP active + refs → chains.up rewritten to LSP entries, `trace_source: lsp`,
stats + `graph/lsp_callers_found` updated, `cleanup()` called.
- LSP inactive → graph retained, `lsp_available: false`.
- refs `None` (symbol unresolved) → graph retained.
- engine creation raises → graph retained.
- `find_references_for_symbol` resolves def site, excludes it, converts
0→1-indexed; returns `None` when LSP inactive.

**Live verification (real CLI, this environment):**
- `codelens context <ws> --check trace --name helper --direction up` →
`trace_source: graph`, callers found — zero-config unaffected.
- same with `--deep` → `lsp_available: true`, but the live server did not
return usable references for the symbol, so it degraded to
`trace_source: graph` — no hang, no error, exit 0.

**LSP happy-path live limitation (honest):** the LSP happy path
(`trace_source: lsp` with real references) could **not** be verified against a
live server in the dev environment — rust-analyzer (the only installed
server) does not respond to `initialize` within 60s (pre-existing, same
limitation documented in #253). The happy path is covered by the mocked tests
above; only the graph fallback and graceful-degradation paths are
live-verified.

## Backward compatibility

Zero-config (`context --check trace ...` without `--deep`) is byte-for-byte
unchanged — the graph result only gains a `trace_source: "graph"` annotation.
No behavior change to `trace_engine.py`. `--direction down` is never touched
by this feature (callees are not references).
61 changes: 61 additions & 0 deletions scripts/commands/trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
)


def execute(args, workspace):

Check failure on line 56 in scripts/commands/trace.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=Wolfvin_CodeLens&issues=AZ9gg5OSg304P0QleNcF&open=AZ9gg5OSg304P0QleNcF&pullRequest=288
"""Execute the trace command."""
use_graph = getattr(args, "use_graph", True)
result = trace_symbol(
Expand All @@ -64,6 +64,16 @@
max_results=args.max_results,
use_graph=use_graph,
)
# Issue #255: opt-in LSP-backed find-references for trace-up precision.
# Only when --deep is active AND an LSP server is available AND we are
# tracing callers (up/both). Otherwise the graph path above is used
# unchanged (zero-config, no regression, no hang).
if isinstance(result, dict) and getattr(args, "deep", False) \
and args.direction in ("up", "both"):
_apply_lsp_trace_up(args.name, workspace, result)
else:
if isinstance(result, dict):
result.setdefault("trace_source", "graph")
# Apply pagination to chains.up and chains.down (issue #17).
if isinstance(result, dict) and isinstance(result.get("chains"), dict):
chains = result["chains"]
Expand All @@ -81,4 +91,55 @@
result["limit"] = limit
return result

def _apply_lsp_trace_up(name, workspace, result):
"""Replace ``result['chains']['up']`` with LSP-derived references when a
language server is available (issue #255).

Annotates ``result['trace_source']`` as ``"lsp"`` on success or ``"graph"``
when LSP is unavailable / cannot resolve the symbol, so consumers know the
precision source. Falls back to the graph chains (leaves them untouched) on
any failure — LSP is a precision enhancement, never a hard dependency.
"""
graph_up = result.get("chains", {}).get("up", []) if isinstance(result.get("chains"), dict) else []
try:
from hybrid_engine import create_hybrid_engine
engine = create_hybrid_engine(workspace, deep=True)
except Exception:
result["trace_source"] = "graph"
return
try:
if not engine.lsp_active:
result["trace_source"] = "graph"
result.setdefault("lsp_available", False)
return
refs = engine.find_references_for_symbol(name)
finally:
engine.cleanup()

result["lsp_available"] = True
if refs is None:
# LSP active but symbol unresolved / no references list — keep graph.
result["trace_source"] = "graph"
return

lsp_up = []
for ref in refs:
lsp_up.append({
"fn": "",
"file": ref.get("file", ""),
"line": ref.get("line", 0),
"depth": 1,
"source": "lsp",
})
if isinstance(result.get("chains"), dict):
result["chains"]["up"] = lsp_up
else:
result["chains"] = {"up": lsp_up, "down": []}
result["trace_source"] = "lsp"
result["graph_callers_found"] = len(graph_up)
result["lsp_callers_found"] = len(lsp_up)
stats = result.setdefault("stats", {})
stats["callers_found"] = len(lsp_up)


# Issue #199: deprecated "trace" alias registration removed; this module is now an implementation module imported by the "context" umbrella command.
50 changes: 50 additions & 0 deletions scripts/hybrid_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,56 @@ def _find_symbol_definition(self, symbol_name: str) -> Tuple[Optional[str], Opti
pass
return None, None

def find_references_for_symbol(self, symbol_name: str) -> Optional[List[Dict]]:
"""Resolve ``symbol_name`` to its definition, then ask the LSP server
for its references (issue #255 — LSP-backed trace-up precision).

Reuses the existing ``lsp_client.find_references`` +
``_find_symbol_definition`` + ``_find_symbol_char`` machinery — no new
LSP infrastructure. Returns a list of reference dicts::

{"file": <abs path>, "line": <1-indexed>, "character": <int>}

excluding the definition site itself (the caller wants callers, not the
declaration). Returns ``None`` when LSP is not active or the symbol
cannot be resolved/located, so the caller can distinguish "no LSP path"
from "LSP ran and found zero references" (empty list). Never raises.
"""
if not self.lsp_active:
return None
def_file, def_line = self._find_symbol_definition(symbol_name)
if not def_file or not def_line:
return None
abs_def = def_file if os.path.isabs(def_file) else os.path.join(self.workspace, def_file)
if not os.path.exists(abs_def):
return None
self.open_file_for_lsp(abs_def)
client = self.get_lsp_client(abs_def)
if not client:
return None
char = self._find_symbol_char(abs_def, def_line, symbol_name)
if char is None:
char = 0
lsp_line = max(0, def_line - 1)
try:
raw = client.find_references(abs_def, lsp_line, char, include_declaration=False)
except Exception:
return None
if raw is None:
return None
external = self._filter_external_references(raw, abs_def, lsp_line, char)
out: List[Dict] = []
for ref in external:
ref_uri = ref.get("uri", "")
ref_path = _uri_to_path(ref_uri) if ref_uri else ""
start = ref.get("range", {}).get("start", {})
out.append({
"file": ref_path,
"line": start.get("line", 0) + 1, # LSP 0-indexed -> report 1-indexed
"character": start.get("character", 0),
})
return out


def _paths_match(path_a: str, path_b: str) -> bool:
"""Compare two file paths for equality using normalized absolute paths.
Expand Down
Loading
Loading