diff --git a/docs/lsp.md b/docs/lsp.md index 693936ce..61b6b490 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -209,7 +209,13 @@ ambiguous. A pass runs up to five phases: An answer that agrees with the heuristic target counts as `edges_confirmed`; an answer naming a different same-name declaration rewrites the edge (tagged `rebound_from`) and counts as `edges_rebound` — - a correction of the heuristic graph, not a confirmation of it. + a correction of the heuristic graph, not a confirmation of it. One + exception: an answer naming the *declared* dispatch member (interface / + abstract base) while the stored target is one of its concrete + implementations does not rewrite anything — the stored edge is a + devirtualization guess definition cannot vouch for, so the compiler-proven + edge to the declared member is added (`edges_added`) and the guess keeps + its heuristic tier. 4. **References-add pass** — only for servers that expose references but not a call hierarchy; recovers the caller edges a declaration's references imply. 5. **Per-file sweep** — the whole-repo hover / hierarchy phase. Per function or @@ -239,6 +245,65 @@ This matters most for `clangd` without a compilation database: every `didOpen` triggers a full fallback-preamble + AST rebuild, so reopening the same file across phases multiplies that cost. +### Servers that skip the lifecycle entirely + +A server whose spec sets `NoDidOpen` answers position requests (hover, +references, call hierarchy) for files it loaded from its own workspace, +without any `didOpen` — and for one server family that is worth far more +than the saved notification. csharp-ls schedules read-only requests +concurrently but treats every `didOpen` / `didClose` as an exclusive write: +it waits for all in-flight reads to retire and blocks every read queued +behind it. A pass that interleaves opens with its queries therefore +serializes to single-request throughput no matter how many requests it +keeps in flight — measured on a real C# monorepo as ~8 req/s with the +lifecycle against ~1,000 req/s without it. With `NoDidOpen` the document +session degrades to a pure content cache (file bytes still read from disk +once per file) and the pass sends zero document notifications. The +`GORTEX_LSP_OPEN_DOCS` env var overrides in both directions: `1` forces +the lifecycle back on, `0` skips it for every server. + +### Servers that skip the heavy request classes + +A server whose spec sets `NoHeavyRequests` never receives the two request +classes that ride its find-references machinery: `textDocument/references` +and `callHierarchy/incomingCalls`. For the one spec that sets it today — C# +(`omnisharp`, including its `csharp-ls` fallback command) — the reason is a +memory leak, not throughput: released csharp-ls builds (≤ 0.26) hold ~10MB +plus a set of OS handles per references round trip and ~0.7MB per +incomingCalls, released only at process exit. A full enrichment pass over a +large repo pushes tens of GB through that leak, and a long-lived daemon +answering usage queries accumulates it one request at a time. + +What changes under the opt-out: + +- **Edge confirmation moves to definition.** The references confirm pass + (phase 2) is skipped and the definition pass (phase 3) becomes the primary + confirm source, covering every sited ambiguous edge — the same verdicts at + position-request cost. Edge tiers and recall are unaffected. +- **`callHierarchy/incomingCalls` is never sent.** Dispatch fan-out stays + with the graph-side interface-dispatch synthesizer; the outgoing side of + the call hierarchy still runs. +- **The references-add pass (phase 4) is skipped** — it exists for servers + without a call hierarchy and is references-driven by definition. +- **On-demand confirmation is disabled.** The query-time path that upgrades + `find_usages` / `get_callers` answers with a live LSP round trip returns + immediately for these servers, so C# usage queries answer from the stored + graph tiers alone. + +The `GORTEX_LSP_HEAVY` env var overrides the spec in both directions, with +the same value vocabulary as `GORTEX_LSP_OPEN_DOCS`: `on` / `1` / `true` +restores references and incomingCalls for an opted-out server, `off` / `0` +/ `false` disables them for every server, and an empty or unrecognised +value falls through to the spec. The knob is deliberately env-only: it +exists to match a specific server *build*, not a workspace, and a durable +config key would outlive the build it was set for. + +> **Warning:** set `GORTEX_LSP_HEAVY=on` for C# only on a csharp-ls build +> that carries the FindReferences leak fix +> ([razzmatazz/csharp-language-server#410](https://github.com/razzmatazz/csharp-language-server/pull/410)). +> On any released build up to 0.26, a full heavy pass over a large repo can +> push ~30GB through the leak and OOM the server mid-pass. + ### Sweep modes The per-file sweep (phase 5) is gated by a **sweep mode**: @@ -471,3 +536,10 @@ for repositories you trust. - One `*lsp.Provider` per spec, regardless of how many MCP sessions hit it. Concurrency is bounded by `ServerSpec.MaxParallel` (6-10 inflight requests per server depending on the spec). + `semantic.lsp_max_parallel` in config overrides the spec's cap for + every router-spawned server — the durable knob for a machine whose + servers multiplex better than the conservative default assumes. The + `GORTEX_LSP_MAX_PARALLEL` env override wins over both for one-run + experiments (and also reaches resolver-pool providers, which take env + + spec only; legacy config-declared providers keep their explicit + setting). Non-positive or unparseable values fall through. diff --git a/internal/config/config.go b/internal/config/config.go index 9e9661ea..f7f1e3eb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -236,6 +236,21 @@ type SemanticConfig struct { // passes still run. // The GORTEX_LSP_SWEEP env override wins over this setting. LSPSweep string `mapstructure:"lsp_sweep" yaml:"lsp_sweep,omitempty"` + // LSPOpenDocs overrides whether the LSP enrichment pass sends the + // textDocument/didOpen / didClose document lifecycle: + // - "" (DEFAULT): each server spec decides — a Roslyn server that + // answers position requests for never-opened files skips the + // lifecycle (its scheduler treats every didOpen as an exclusive + // write that serializes the pass), everything else keeps it. + // - "on": force the lifecycle for every server (kill switch). + // - "off": skip it for every server (experiment switch). + // The GORTEX_LSP_OPEN_DOCS env override wins over this setting. + LSPOpenDocs string `mapstructure:"lsp_open_docs" yaml:"lsp_open_docs,omitempty"` + // LSPMaxParallel caps concurrent LSP requests per spawned server, + // overriding each spec's registry default (6-10) when positive. A + // machine whose servers multiplex well can raise it without editing + // source. The GORTEX_LSP_MAX_PARALLEL env override wins over this. + LSPMaxParallel int `mapstructure:"lsp_max_parallel" yaml:"lsp_max_parallel,omitempty"` // EagerLSP runs the subprocess LSP servers during synchronous enrichment. // Default false: LSP is the slowest part of a cold index and the in-process // tiers (go-types, tree-sitter floor) cover the fast baseline, so LSP is diff --git a/internal/semantic/config.go b/internal/semantic/config.go index 5f55d283..6a62e240 100644 --- a/internal/semantic/config.go +++ b/internal/semantic/config.go @@ -21,6 +21,15 @@ type Config struct { // each spawned LSP provider via the router's WithEnrichSweepMode. The // GORTEX_LSP_SWEEP env override wins over it at enrichment time. LSPSweep string `mapstructure:"lsp_sweep" yaml:"lsp_sweep,omitempty"` + // LSPOpenDocs mirrors config.SemanticConfig.LSPOpenDocs — the + // didOpen-lifecycle override ("" spec-decides / "on" / "off"). Threaded + // to each spawned LSP provider via the router's WithEnrichOpenDocs. The + // GORTEX_LSP_OPEN_DOCS env override wins over it at enrichment time. + LSPOpenDocs string `mapstructure:"lsp_open_docs" yaml:"lsp_open_docs,omitempty"` + // LSPMaxParallel mirrors config.SemanticConfig.LSPMaxParallel — the + // concurrent-request cap for spawned LSP servers. Zero keeps each + // spec's own default; GORTEX_LSP_MAX_PARALLEL wins over both. + LSPMaxParallel int `mapstructure:"lsp_max_parallel" yaml:"lsp_max_parallel,omitempty"` // EagerLSP runs the subprocess LSP servers during the synchronous // enrichment pass. Default false: LSP is the slowest part of a cold index // (a full gopls/tsserver/rust-analyzer/pyright sweep can run for minutes to diff --git a/internal/semantic/lsp/doc_session.go b/internal/semantic/lsp/doc_session.go index c74d7755..496d709e 100644 --- a/internal/semantic/lsp/doc_session.go +++ b/internal/semantic/lsp/doc_session.go @@ -23,6 +23,14 @@ import ( type docSession struct { p *Provider cap int // simultaneously-open ceiling per client + // sendOpens gates the server-side lifecycle. When false (the provider's + // opensDocs resolved off — see ServerSpec.NoDidOpen) the session keeps + // its entry / LRU machinery purely as a bounded content cache: acquire + // still reads and caches file bytes, but no didOpen / didClose is ever + // sent, and the open-lifecycle telemetry (didOpens, curOpen, peakOpen) + // honestly reports zero. Evictions still count — they track the cache + // churn either way. + sendOpens bool mu sync.Mutex perClient map[*Client]*clientDocs @@ -65,6 +73,7 @@ func newDocSession(p *Provider) *docSession { return &docSession{ p: p, cap: cp, + sendOpens: p.opensDocs, perClient: map[*Client]*clientDocs{}, openCounts: map[string]int{}, } @@ -115,21 +124,28 @@ func (s *docSession) acquire(c *Client, absPath string) ([]byte, func(), error) evPath := front.Value.(string) cd.lru.Remove(front) delete(cd.open, evPath) - _ = s.p.enrichCloseDoc(c, evPath) - s.curOpen-- + // Evictions count the cache churn, not the didClose traffic — a + // lifecycle-off session evicts entries all the same and hiding that + // would blind the content-cache telemetry. s.evictions++ + if s.sendOpens { + _ = s.p.enrichCloseDoc(c, evPath) + s.curOpen-- + } } - if err := s.p.enrichOpenDoc(c, absPath, content); err != nil { - return nil, func() {}, err + if s.sendOpens { + if err := s.p.enrichOpenDoc(c, absPath, content); err != nil { + return nil, func() {}, err + } + s.didOpens++ + s.openCounts[absPath]++ + s.curOpen++ + if s.curOpen > s.peakOpen { + s.peakOpen = s.curOpen + } } cd.open[absPath] = &docEntry{refs: 1, content: content} - s.didOpens++ - s.openCounts[absPath]++ - s.curOpen++ - if s.curOpen > s.peakOpen { - s.peakOpen = s.curOpen - } return content, s.releaseFunc(c, absPath), nil } @@ -162,9 +178,11 @@ func (s *docSession) closeAll() { s.mu.Lock() defer s.mu.Unlock() for c, cd := range s.perClient { - for path := range cd.open { - _ = s.p.enrichCloseDoc(c, path) - s.curOpen-- + if s.sendOpens { + for path := range cd.open { + _ = s.p.enrichCloseDoc(c, path) + s.curOpen-- + } } cd.open = map[string]*docEntry{} cd.lru.Init() diff --git a/internal/semantic/lsp/doc_session_test.go b/internal/semantic/lsp/doc_session_test.go index a21dad43..186eaea1 100644 --- a/internal/semantic/lsp/doc_session_test.go +++ b/internal/semantic/lsp/doc_session_test.go @@ -197,6 +197,42 @@ func TestDocSession_LRUEvictsPairedAndBounded(t *testing.T) { assert.Equal(t, 3, opens, "each of the three files was opened once") } +// A session with the lifecycle off (sendOpens=false) still evicts to keep +// the content cache bounded — and the evictions counter must record that +// churn. Eviction telemetry tracks the cache, not the didClose traffic; +// the lifecycle counters (didOpens, peak) stay honestly zero. +func TestDocSession_NoSendOpens_EvictionsStillCounted(t *testing.T) { + repoRoot := t.TempDir() + files := []string{"a.go", "b.go", "c.go"} + for _, f := range files { + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, f), []byte("package main\n"), 0o644)) + } + + server := newInstrumentedServer() + p, cleanup := providerWithInstrumentedServer(t, server, []string{"go"}, 1) + defer cleanup() + + session := newDocSession(p) + session.cap = 2 + session.sendOpens = false + + for _, f := range files { + _, release, err := session.acquire(p.client, filepath.Join(repoRoot, f)) + require.NoError(t, err) + release() + } + + didOpens, _, evictions, peakOpen := session.stats() + assert.Zero(t, didOpens, "no lifecycle — didOpen telemetry stays zero") + assert.Zero(t, peakOpen) + assert.Equal(t, 1, evictions, + "the third acquire evicted the oldest cache entry and must be counted") + + _, opens, closes := server.stats() + assert.Zero(t, opens, "no didOpen is ever sent") + assert.Zero(t, closes, "no didClose is ever sent") +} + // Pinned entries are never evicted: holding refs on cap files and acquiring one // more overshoots cap (no didClose) rather than closing a pinned document. func TestDocSession_PinnedNeverEvicted(t *testing.T) { diff --git a/internal/semantic/lsp/enrich_defconfirm_breaker_test.go b/internal/semantic/lsp/enrich_defconfirm_breaker_test.go new file mode 100644 index 00000000..880b88b4 --- /dev/null +++ b/internal/semantic/lsp/enrich_defconfirm_breaker_test.go @@ -0,0 +1,183 @@ +package lsp + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" +) + +// breakerFixture seeds nSites caller files, each with one sited ambiguous +// edge to the same declaration, so the definition pass has one group per +// file. The definition handler is the test's to wire. +func breakerFixture(t *testing.T, repoRoot string, g graph.Store, nSites int) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "def.go"), + []byte("package p\nfunc target() {}\n"), 0o644)) + g.AddNode(&graph.Node{ID: "def.go::target", Kind: graph.KindFunction, Name: "target", + FilePath: "def.go", StartLine: 2, EndLine: 2, Language: "go"}) + for i := 0; i < nSites; i++ { + rel := fmt.Sprintf("call%03d.go", i) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, rel), + []byte("package p\nfunc caller() { target() }\n"), 0o644)) + callerID := rel + "::caller" + g.AddNode(&graph.Node{ID: callerID, Kind: graph.KindFunction, Name: "caller", + FilePath: rel, StartLine: 2, EndLine: 2, Language: "go"}) + g.AddEdge(&graph.Edge{ + From: callerID, To: "def.go::target", Kind: graph.EdgeCalls, + FilePath: rel, Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginTextMatched, + }) + } +} + +// Under the heavy opt-out the definition pass carries the whole confirm load, +// so it must feed the targeted failure-streak breaker: a server that errors +// every definition request has to abort the pass after the streak limit, not +// grind through every sited target at one timeout each. +func TestLSP_Enrich_DefinitionPassTripsBreakerOnDeadServer(t *testing.T) { + t.Setenv(SweepEnv, "full") + t.Setenv("GORTEX_LSP_BREAKER", "5") + const nSites = 40 + const maxParallel = 2 + + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + breakerFixture(t, repoRoot, g, nSites) + + var defs atomic.Int64 + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + defs.Add(1) + return nil, &jsonRPCError{Code: -32603, Message: "boom"} + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithFakeServerParallel(t, server, []string{"go"}, maxParallel) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) + + // Streak limit plus the requests already in flight across maxParallel + // goroutines when the breaker trips. + assert.LessOrEqual(t, defs.Load(), int64(5+maxParallel), + "a dead server must trip the targeted breaker, not answer every site") +} + +// The trip must come from transport failures only. A server that ANSWERS +// every definition request with an empty result is healthy — no verdict for +// the edge, but the breaker stays disarmed and every site gets asked. +func TestLSP_Enrich_DefinitionPassEmptyAnswersDoNotTrip(t *testing.T) { + t.Setenv(SweepEnv, "full") + t.Setenv("GORTEX_LSP_BREAKER", "5") + const nSites = 40 + + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + breakerFixture(t, repoRoot, g, nSites) + + var defs atomic.Int64 + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + defs.Add(1) + return []Location{}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithFakeServerParallel(t, server, []string{"go"}, 2) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) + + assert.Equal(t, int64(nSites), defs.Load(), + "answered-but-empty is not a failure; every site must still be asked") +} + +// A PRODUCTIVE definition pass must survive the productivity checkpoint. +// Under the heavy opt-out the def pass carries the whole confirm load, and +// on a warm graph it is the only yield source until the hover phase — if its +// confirm/add/rebind arms do not feed usefulYield, the checkpoint reads a +// pass that is settling hundreds of edges as zero-yield and cancels it at +// the first tick after request volume flows. +func TestLSP_Enrich_DefinitionPassYieldSurvivesProductivityCheckpoint(t *testing.T) { + t.Setenv(SweepEnv, "full") + t.Setenv("GORTEX_LSP_PRODUCTIVITY_WINDOW", "150ms") + const nSites = 300 + + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + breakerFixture(t, repoRoot, g, nSites) + + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + // Slow enough that the pass spans several checkpoint windows while + // confirming at ~30 edges per window — far above the 10-per-window + // floor a productive pass must sustain. + time.Sleep(5 * time.Millisecond) + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "def.go")), + Range: Range{Start: Position{Line: 1, Character: 5}, End: Position{Line: 1, Character: 11}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithFakeServerParallel(t, server, []string{"go"}, 2) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 60*time.Second)) + + confirmed := 0 + for _, e := range g.AllEdges() { + if e.Kind == graph.EdgeCalls && e.Confidence == 1.0 { + confirmed++ + } + } + assert.Equal(t, nSites, confirmed, + "a pass confirming hundreds of edges must not be cut by the productivity checkpoint") +} + +// The heavy opt-out must also cover the on-demand path: ConfirmSymbolRefs +// rides callHierarchy/incomingCalls, the same FindReferences machinery the +// sweep skips. A long-lived daemon answering find_usages against a leaky +// server would otherwise accumulate the leak one query at a time. +func TestLSP_ConfirmSymbolRefs_NoHeavySkipsIncoming(t *testing.T) { + repoRoot := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "svc.go"), + []byte("package p\nfunc served() {}\n"), 0o644)) + g := graph.New() + n := &graph.Node{ID: "svc.go::served", Kind: graph.KindFunction, Name: "served", + FilePath: "svc.go", StartLine: 2, EndLine: 2, Language: "go"} + g.AddNode(n) + + server := newFakeLSPServer() + var prepares, incomings atomic.Int64 + server.handle("textDocument/prepareCallHierarchy", func(json.RawMessage) (any, *jsonRPCError) { + prepares.Add(1) + return []CallHierarchyItem{{Name: "served"}}, nil + }) + server.handle("callHierarchy/incomingCalls", func(json.RawMessage) (any, *jsonRPCError) { + incomings.Add(1) + return []CallHierarchyIncomingCall{}, nil + }) + + p, cleanup := providerWithFakeServerParallel(t, server, []string{"go"}, 2) + defer cleanup() + p.noHeavyRequests = true + + confirmed, err := p.ConfirmSymbolRefs(g, repoRoot, n) + require.NoError(t, err) + assert.Zero(t, confirmed) + assert.Zero(t, incomings.Load(), + "incomingCalls must never be issued under the heavy opt-out") +} diff --git a/internal/semantic/lsp/enrich_defconfirm_test.go b/internal/semantic/lsp/enrich_defconfirm_test.go new file mode 100644 index 00000000..81004f39 --- /dev/null +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -0,0 +1,491 @@ +package lsp + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" +) + +// The csharp-ls / omnisharp spec opts out of the heavy request classes: +// textDocument/references and callHierarchy/incomingCalls both ride Roslyn's +// FindReferences machinery, which leaks unboundedly per request in csharp-ls. +// Edge confirmation goes through textDocument/definition at the call site +// instead — same verdicts, position-request cost. +func TestCSharpSpecOptsOutOfHeavyRequests(t *testing.T) { + spec := SpecByName("omnisharp") + require.NotNil(t, spec) + assert.True(t, spec.NoHeavyRequests, "csharp spec must skip references/incomingCalls") + + p := NewProviderFromSpec(spec, nil) + assert.True(t, p.noHeavyRequests, "spec opt-out must reach the provider") + + def := NewProvider("fake-lsp", nil, []string{"go"}, false, 2, nil) + assert.False(t, def.noHeavyRequests, "a spec-less provider keeps the heavy legs") +} + +// defConfirmFixture seeds one ambiguous call edge whose site line names the +// target, plus fake-server handlers for references (counting), definition +// (answering the target's declaration), and hover. The returned edge is the +// live store object — ConfirmEdge mutates it in place. +func defConfirmFixture(t *testing.T, repoRoot string, g graph.Store, server *fakeLSPServer) (edge *graph.Edge, refs, defs *atomic.Int64) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "def.go"), + []byte("package p\nfunc target() {}\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "call.go"), + []byte("package p\nfunc caller() { target() }\n"), 0o644)) + + g.AddNode(&graph.Node{ID: "def.go::target", Kind: graph.KindFunction, Name: "target", + FilePath: "def.go", StartLine: 2, EndLine: 2, Language: "go"}) + g.AddNode(&graph.Node{ID: "call.go::caller", Kind: graph.KindFunction, Name: "caller", + FilePath: "call.go", StartLine: 2, EndLine: 2, Language: "go"}) + edge = &graph.Edge{ + From: "call.go::caller", To: "def.go::target", Kind: graph.EdgeCalls, + FilePath: "call.go", Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginTextMatched, + } + g.AddEdge(edge) + + refs, defs = &atomic.Int64{}, &atomic.Int64{} + // References would confirm the site (the location matches the edge) — the + // point of the heavy opt-out is that it must never be asked. + server.handle("textDocument/references", func(json.RawMessage) (any, *jsonRPCError) { + refs.Add(1) + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "call.go")), + Range: Range{Start: Position{Line: 1, Character: 16}, End: Position{Line: 1, Character: 22}}, + }}, nil + }) + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + defs.Add(1) + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "def.go")), + Range: Range{Start: Position{Line: 1, Character: 5}, End: Position{Line: 1, Character: 11}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + return edge, refs, defs +} + +// With the heavy opt-out, an ambiguous edge is confirmed through definition at +// its call site — and the references round trip never happens. +func TestLSP_Enrich_NoHeavy_DefinitionConfirmsWithoutReferences(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + edge, refs, defs := defConfirmFixture(t, repoRoot, g, server) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Zero(t, refs.Load(), "references must never be requested under the heavy opt-out") + assert.Positive(t, defs.Load(), "the confirm verdict must come from definition") + assert.Equal(t, graph.OriginLSPResolved, edge.Origin, "the edge must be promoted") + assert.Equal(t, 1.0, edge.Confidence) +} + +// Without the opt-out nothing changes: the references confirm sweep still runs +// first — this pins that gopls-shaped providers are untouched. +func TestLSP_Enrich_HeavyDefault_StillIssuesReferences(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + edge, refs, _ := defConfirmFixture(t, repoRoot, g, server) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Positive(t, refs.Load(), "the default path still confirms through references") + assert.Equal(t, graph.OriginLSPResolved, edge.Origin) +} + +// The heavy opt-out also silences callHierarchy/incomingCalls — even for a +// dispatch-relevant method that the demand default would interrogate. The +// outgoing side stays: it is free today and lights up if the server ever +// implements it. +func TestLSP_Enrich_NoHeavy_SkipsIncomingEvenForDispatchMethod(t *testing.T) { + t.Setenv(SweepEnv, "") // demand default + + repoRoot := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "svc.go"), + []byte("package p\n\ntype Shape interface{ Area() float64 }\n\ntype Circle struct{}\n\nfunc (c Circle) Area() float64 { return 0 }\n"), 0o644)) + + server := newFakeLSPServer() + prepare, outgoing, incoming := callHierarchyCounters(server, repoRoot) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + p.noHeavyRequests = true + + g := graph.New() + g.AddNode(&graph.Node{ID: "svc.go::Shape", Kind: graph.KindInterface, Name: "Shape", + FilePath: "svc.go", StartLine: 3, EndLine: 3, Language: "go"}) + g.AddNode(&graph.Node{ID: "svc.go::Circle", Kind: graph.KindType, Name: "Circle", + FilePath: "svc.go", StartLine: 5, EndLine: 5, Language: "go"}) + g.AddNode(&graph.Node{ID: "svc.go::Circle.Area", Kind: graph.KindMethod, Name: "Area", + FilePath: "svc.go", StartLine: 7, EndLine: 7, Language: "go"}) + g.AddEdge(&graph.Edge{From: "svc.go::Circle.Area", To: "svc.go::Circle", Kind: graph.EdgeMemberOf}) + g.AddEdge(&graph.Edge{From: "svc.go::Circle", To: "svc.go::Shape", Kind: graph.EdgeImplements}) + + require.NoError(t, runEnrich(t, p, g, repoRoot, 3*time.Second)) + + assert.Positive(t, prepare.Load()) + assert.Positive(t, outgoing.Load(), "outgoing stays on — it is not a heavy request") + assert.Zero(t, incoming.Load(), "incoming rides FindReferences and must be skipped") +} + +// Definition on `new T(...)` answers the constructor's declaration line, not +// the type's. An instantiates edge targeting the type must still count that +// as an exact confirm: the constructor is a member of the very type the edge +// names. +func TestLSP_Enrich_DefConfirm_CtorHitConfirmsTypeTarget(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "dom.go"), + []byte("package p\ntype Crate struct{\n\tn int\n}\nfunc ctor() {}\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "use.go"), + []byte("package p\nfunc maker() { _ = Crate{} }\n"), 0o644)) + + g := graph.New() + g.AddNode(&graph.Node{ID: "dom.go::Crate", Kind: graph.KindType, Name: "Crate", + FilePath: "dom.go", StartLine: 2, EndLine: 5, Language: "go"}) + // The constructor: a differently-named member on a line inside the type's + // span — the shape a C# `Crate.` node has. + ctor := &graph.Node{ID: "dom.go::Crate.", Kind: graph.KindMethod, Name: "Crate.", + FilePath: "dom.go", StartLine: 5, EndLine: 5, Language: "go"} + g.AddNode(ctor) + g.AddEdge(&graph.Edge{From: ctor.ID, To: "dom.go::Crate", Kind: graph.EdgeMemberOf}) + g.AddNode(&graph.Node{ID: "use.go::maker", Kind: graph.KindFunction, Name: "maker", + FilePath: "use.go", StartLine: 2, EndLine: 2, Language: "go"}) + edge := &graph.Edge{ + From: "use.go::maker", To: "dom.go::Crate", Kind: graph.EdgeInstantiates, + FilePath: "use.go", Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginTextMatched, + } + g.AddEdge(edge) + + server := newFakeLSPServer() + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + // The ctor's declaration line (0-based 4), inside Crate's span. + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "dom.go")), + Range: Range{Start: Position{Line: 4, Character: 5}, End: Position{Line: 4, Character: 9}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Equal(t, graph.OriginLSPResolved, edge.Origin, + "a definition landing on the ctor confirms the containing-type target") + assert.Equal(t, 1.0, edge.Confidence) + assert.Equal(t, "dom.go::Crate", edge.To, "the edge keeps its type target — no retarget to the ctor") +} + +// dispatchedCallFixture seeds a dispatched call site: an interface with a +// declared member, one concrete impl, and a stored AST-inferred call edge +// targeting the impl. The fake server's definition answers the interface's +// declared member — the compiler vouching for the declaration, not the +// devirtualization guess. Returns the live impl edge. +func dispatchedCallFixture(t *testing.T, repoRoot string, g graph.Store, server *fakeLSPServer) *graph.Edge { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "scales.go"), + []byte("package p\ntype IScale interface {\n\tWeigh() int\n}\n\ntype DrumScale struct{\n\tWeighImpl int\n}\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "station.go"), + []byte("package p\nfunc station() { Weigh() }\n"), 0o644)) + + g.AddNode(&graph.Node{ID: "scales.go::IScale", Kind: graph.KindInterface, Name: "IScale", + FilePath: "scales.go", StartLine: 2, EndLine: 4, Language: "go"}) + g.AddNode(&graph.Node{ID: "scales.go::IScale.Weigh", Kind: graph.KindMethod, Name: "Weigh", + FilePath: "scales.go", StartLine: 3, EndLine: 3, Language: "go"}) + g.AddEdge(&graph.Edge{From: "scales.go::IScale.Weigh", To: "scales.go::IScale", Kind: graph.EdgeMemberOf}) + g.AddNode(&graph.Node{ID: "scales.go::DrumScale", Kind: graph.KindType, Name: "DrumScale", + FilePath: "scales.go", StartLine: 6, EndLine: 8, Language: "go"}) + g.AddNode(&graph.Node{ID: "scales.go::DrumScale.Weigh", Kind: graph.KindMethod, Name: "Weigh", + FilePath: "scales.go", StartLine: 7, EndLine: 7, Language: "go"}) + g.AddEdge(&graph.Edge{From: "scales.go::DrumScale.Weigh", To: "scales.go::DrumScale", Kind: graph.EdgeMemberOf}) + g.AddEdge(&graph.Edge{From: "scales.go::DrumScale", To: "scales.go::IScale", Kind: graph.EdgeImplements}) + g.AddNode(&graph.Node{ID: "station.go::station", Kind: graph.KindFunction, Name: "station", + FilePath: "station.go", StartLine: 2, EndLine: 2, Language: "go"}) + implEdge := &graph.Edge{ + From: "station.go::station", To: "scales.go::DrumScale.Weigh", Kind: graph.EdgeCalls, + FilePath: "station.go", Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginASTInferred, + } + g.AddEdge(implEdge) + + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + // The compiler's answer: the interface's declared member (0-based 2). + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "scales.go")), + Range: Range{Start: Position{Line: 2, Character: 1}, End: Position{Line: 2, Character: 6}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + return implEdge +} + +// A dispatched call site: definition answers the interface's declared member +// while the stored edge targets one concrete impl. The declared-member edge is +// added at LSP grade; the impl edge — an inference the compiler cannot vouch +// for — is left exactly as it was: not retargeted, not demoted, not promoted. +func TestLSP_Enrich_DefConfirm_DispatchedCallAddsDeclaredMemberEdge(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + implEdge := dispatchedCallFixture(t, repoRoot, g, server) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Equal(t, "scales.go::DrumScale.Weigh", implEdge.To, + "the impl edge keeps its target — the devirtualization guess is not destroyed") + assert.Equal(t, graph.OriginASTInferred, implEdge.Origin, + "the impl edge keeps its origin — an inference stays labeled as one") + assert.Equal(t, 0.7, implEdge.Confidence) + + var declared *graph.Edge + for _, e := range g.GetOutEdges("station.go::station") { + if e.To == "scales.go::IScale.Weigh" && e.Kind == graph.EdgeCalls { + declared = e + } + } + require.NotNil(t, declared, "the compiler-proven edge to the declared member must be added") + assert.Equal(t, graph.OriginLSPResolved, declared.Origin) + assert.Equal(t, 2, declared.Line, "the added edge carries the call site") +} + +// The declared-member arm lives in the shared definition-rebind pass, so it is +// default-path behavior, not part of the heavy opt-out. With the heavy legs ON +// and references answering empty, the unconfirmed site falls through to the +// definition pass — and the verdict must be the same add-not-rebind: previously +// this path REBOUND the impl edge to the declared member, so a gopls/tsserver/ +// pyright repo now keeps the devirtualization guess AND gains the declared edge. +func TestLSP_Enrich_HeavyDefault_DispatchedCallAddsDeclaredMemberEdge(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + implEdge := dispatchedCallFixture(t, repoRoot, g, server) + + // The references sweep runs first and must yield no verdict, leaving the + // site to the definition pass. + refs := &atomic.Int64{} + server.handle("textDocument/references", func(json.RawMessage) (any, *jsonRPCError) { + refs.Add(1) + return []Location{}, nil + }) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + // noHeavyRequests stays false — this is every spec-less server's path. + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Positive(t, refs.Load(), "the default path must still run its references sweep") + assert.Equal(t, "scales.go::DrumScale.Weigh", implEdge.To, + "the impl edge keeps its target — the devirtualization guess is not destroyed") + assert.Equal(t, graph.OriginASTInferred, implEdge.Origin, + "the impl edge keeps its origin — an inference stays labeled as one") + assert.Equal(t, 0.7, implEdge.Confidence) + + var declared *graph.Edge + for _, e := range g.GetOutEdges("station.go::station") { + if e.To == "scales.go::IScale.Weigh" && e.Kind == graph.EdgeCalls { + declared = e + } + } + require.NotNil(t, declared, "the compiler-proven edge must be added on the default path too") + assert.Equal(t, graph.OriginLSPResolved, declared.Origin) + assert.Equal(t, 2, declared.Line, "the added edge carries the call site") +} + +// GORTEX_LSP_HEAVY overrides the heavy-request opt-out in both directions: +// "on" restores references / incomingCalls for a spec that opted out (the +// operator runs a patched server without the leak), "off" force-disables +// them for every server. Empty or unrecognised falls through to the spec. +func TestResolveNoHeavyRequests_Precedence(t *testing.T) { + csharp := SpecByName("omnisharp") + require.NotNil(t, csharp) + + t.Run("default follows the spec", func(t *testing.T) { + t.Setenv(HeavyRequestsEnv, "") + assert.False(t, resolveNoHeavyRequests(nil), "spec-less providers keep the heavy legs") + assert.True(t, resolveNoHeavyRequests(csharp)) + }) + t.Run("env on re-enables heavies for an opted-out spec", func(t *testing.T) { + t.Setenv(HeavyRequestsEnv, "on") + assert.False(t, resolveNoHeavyRequests(csharp)) + }) + t.Run("env off disables heavies for every server", func(t *testing.T) { + t.Setenv(HeavyRequestsEnv, "off") + assert.True(t, resolveNoHeavyRequests(nil)) + }) + t.Run("unrecognised value falls through to the spec", func(t *testing.T) { + t.Setenv(HeavyRequestsEnv, "garbage") + assert.True(t, resolveNoHeavyRequests(csharp)) + assert.False(t, resolveNoHeavyRequests(nil)) + }) + t.Run("constructors plumb the override", func(t *testing.T) { + t.Setenv(HeavyRequestsEnv, "on") + p := NewProviderFromSpec(csharp, nil) + assert.False(t, p.noHeavyRequests, "a patched server's operator can restore references/incoming") + t.Setenv(HeavyRequestsEnv, "off") + def := NewProvider("fake-lsp", nil, []string{"go"}, false, 2, nil) + assert.True(t, def.noHeavyRequests) + }) +} + +// The definition pass fans out across site files. The serial loop predates +// the NoDidOpen lifecycle — it existed to keep document opens from +// overlapping — but with no didOpen to serialize, a heavy-opt-out server +// answering 46k definitions one at a time is the pass's longest pole. Six +// site files with a slow definition handler must overlap. +func TestLSP_Enrich_DefinitionPassRunsSiteFilesInParallel(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + + const n = 6 + for i := 0; i < n; i++ { + defFile := fmt.Sprintf("def%d.go", i) + callFile := fmt.Sprintf("call%d.go", i) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, defFile), + []byte(fmt.Sprintf("package p\nfunc target%d() {}\n", i)), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, callFile), + []byte(fmt.Sprintf("package p\nfunc caller%d() { target%d() }\n", i, i)), 0o644)) + g.AddNode(&graph.Node{ID: fmt.Sprintf("%s::target%d", defFile, i), Kind: graph.KindFunction, + Name: fmt.Sprintf("target%d", i), FilePath: defFile, StartLine: 2, EndLine: 2, Language: "go"}) + g.AddNode(&graph.Node{ID: fmt.Sprintf("%s::caller%d", callFile, i), Kind: graph.KindFunction, + Name: fmt.Sprintf("caller%d", i), FilePath: callFile, StartLine: 2, EndLine: 2, Language: "go"}) + g.AddEdge(&graph.Edge{ + From: fmt.Sprintf("%s::caller%d", callFile, i), To: fmt.Sprintf("%s::target%d", defFile, i), + Kind: graph.EdgeCalls, FilePath: callFile, Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginTextMatched, + }) + } + + // The instrumented rig dispatches each request in its own goroutine, so + // concurrency is genuinely observable (fakeLSPServer answers inline in + // its read loop and would pin in-flight at 1 regardless of the client). + server := newInstrumentedServer() + var inflight, maxInflight atomic.Int64 + server.handle("textDocument/definition", func(params json.RawMessage) (any, *jsonRPCError) { + cur := inflight.Add(1) + for { + prev := maxInflight.Load() + if cur <= prev || maxInflight.CompareAndSwap(prev, cur) { + break + } + } + time.Sleep(40 * time.Millisecond) + inflight.Add(-1) + var req struct { + TextDocument struct { + URI string `json:"uri"` + } `json:"textDocument"` + } + _ = json.Unmarshal(params, &req) + var idx int + if _, err := fmt.Sscanf(filepath.Base(uriToAbsPath(req.TextDocument.URI)), "call%d.go", &idx); err != nil { + return []Location{}, nil + } + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, fmt.Sprintf("def%d.go", idx))), + Range: Range{Start: Position{Line: 1, Character: 5}, End: Position{Line: 1, Character: 12}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithInstrumentedServer(t, server, []string{"go"}, 4) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) + + assert.GreaterOrEqual(t, maxInflight.Load(), int64(2), + "definition requests for distinct site files must overlap") + confirmed := 0 + for i := 0; i < n; i++ { + for _, e := range g.GetOutEdges(fmt.Sprintf("call%d.go::caller%d", i, i)) { + if e.Kind == graph.EdgeCalls && e.Origin == graph.OriginLSPResolved { + confirmed++ + } + } + } + assert.Equal(t, n, confirmed, "parallelism must not cost any confirm verdict") +} + +// The no-heavy fallback must include every SITED target, not just those the +// references sweep could serve: groupConfirmTargets drops targets on +// referent-side conditions (no position, unserved referent file) that exist +// only because findReferences opens the referent's file. The definition pass +// opens the call-site file, so a misbound edge whose stored target has no +// usable position must still reach the pass — and get rebound to the real +// declaration. +func TestLSP_Enrich_NoHeavy_RebindsSitedEdgeWithPositionlessTarget(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "def.go"), + []byte("package p\nfunc target() {}\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, "call.go"), + []byte("package p\nfunc caller() { target() }\n"), 0o644)) + + g := graph.New() + g.AddNode(&graph.Node{ID: "def.go::target", Kind: graph.KindFunction, Name: "target", + FilePath: "def.go", StartLine: 2, EndLine: 2, Language: "go"}) + // The heuristically-bound target: same name, NO position — the exact + // shape groupConfirmTargets cannot serve. + g.AddNode(&graph.Node{ID: "ghost.go::target", Kind: graph.KindFunction, Name: "target", + FilePath: "ghost.go", Language: "go"}) + g.AddNode(&graph.Node{ID: "call.go::caller", Kind: graph.KindFunction, Name: "caller", + FilePath: "call.go", StartLine: 2, EndLine: 2, Language: "go"}) + edge := &graph.Edge{ + From: "call.go::caller", To: "ghost.go::target", Kind: graph.EdgeCalls, + FilePath: "call.go", Line: 2, + Confidence: 0.7, ConfidenceLabel: "INFERRED", Origin: graph.OriginTextMatched, + } + g.AddEdge(edge) + + server := newFakeLSPServer() + server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { + return []Location{{ + URI: pathToURI(filepath.Join(repoRoot, "def.go")), + Range: Range{Start: Position{Line: 1, Character: 5}, End: Position{Line: 1, Character: 11}}, + }}, nil + }) + server.handle("textDocument/hover", func(json.RawMessage) (any, *jsonRPCError) { return nil, nil }) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + p.noHeavyRequests = true + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + assert.Equal(t, "def.go::target", edge.To, + "the sited edge must be rebound to the declaration the compiler names") + assert.Equal(t, 1.0, edge.Confidence) +} diff --git a/internal/semantic/lsp/enrich_no_open_test.go b/internal/semantic/lsp/enrich_no_open_test.go new file mode 100644 index 00000000..de5c34b2 --- /dev/null +++ b/internal/semantic/lsp/enrich_no_open_test.go @@ -0,0 +1,137 @@ +package lsp + +import ( + "encoding/json" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.uber.org/zap" +) + +// The didOpen-free enrichment pass. csharp-ls (and any server with the same +// reader-writer request scheduler) runs read-only requests concurrently but +// treats didOpen / didClose as exclusive writes: each one waits for every +// in-flight read to retire and blocks every read queued behind it. A sweep +// that interleaves opens with its queries therefore serializes to +// single-request throughput regardless of maxParallel — measured on a real +// C# monorepo as 7.8 req/s against 1,017 req/s for the same request stream +// with the lifecycle removed. Roslyn answers hover / references / call +// hierarchy for never-opened files from its own workspace load, so a server +// that opts in via ServerSpec.NoDidOpen gets a pass that sends no document +// lifecycle at all. + +// resolveOpensDocs precedence: env override > configured value (yaml +// `semantic.lsp_open_docs`) > spec opt-out > open-by-default. +func TestResolveOpensDocs_Precedence(t *testing.T) { + optOut := &ServerSpec{Name: "x", NoDidOpen: true} + + t.Setenv(OpenDocsEnv, "") + assert.True(t, resolveOpensDocs("", nil), "no spec, no env, no config: lifecycle stays on") + assert.True(t, resolveOpensDocs("", &ServerSpec{Name: "x"}), "spec without opt-out: lifecycle stays on") + assert.False(t, resolveOpensDocs("", optOut), "spec opt-out wins when env and config are silent") + + assert.False(t, resolveOpensDocs("off", nil), "config off skips the lifecycle") + assert.True(t, resolveOpensDocs("on", optOut), "config on overrides the spec opt-out") + assert.False(t, resolveOpensDocs("garbage", optOut), "unrecognised config falls through to the spec") + + t.Setenv(OpenDocsEnv, "0") + assert.False(t, resolveOpensDocs("", nil), "env 0 skips the lifecycle for every server") + assert.False(t, resolveOpensDocs("on", nil), "env wins over config") + + t.Setenv(OpenDocsEnv, "false") + assert.False(t, resolveOpensDocs("", &ServerSpec{Name: "x"}), "env false spelling") + + t.Setenv(OpenDocsEnv, "1") + assert.True(t, resolveOpensDocs("off", optOut), "env 1 is the kill switch: lifecycle forced on over config and spec") +} + +// The yaml knob must reach the provider through the router's spawn path, +// exactly like lsp_sweep does. +func TestRouterWithEnrichOpenDocs_PlumbsToProvider(t *testing.T) { + t.Setenv(OpenDocsEnv, "") + r := NewRouter("", zap.NewNop()).WithEnrichOpenDocs("off") + assert.Equal(t, "off", r.enrichOpenDocs) +} + +// The spec flag must reach the provider on the FromSpec construction path — +// the one the runtime router uses. +func TestNewProviderFromSpec_OpensDocs(t *testing.T) { + t.Setenv(OpenDocsEnv, "") + optOut := &ServerSpec{Name: "x", Command: "definitely-not-a-binary-zz", + Languages: []string{"csharp"}, NoDidOpen: true} + assert.False(t, NewProviderFromSpec(optOut, zap.NewNop()).opensDocs) + + plain := &ServerSpec{Name: "y", Command: "definitely-not-a-binary-zz", + Languages: []string{"csharp"}} + assert.True(t, NewProviderFromSpec(plain, zap.NewNop()).opensDocs) +} + +// csharp-ls is the measured case; its spec opts out. (The spec is shared +// with omnisharp as the alternative-command fallback — both are Roslyn +// workspaces that serve unopened files.) +func TestCSharpSpecOptsOutOfDidOpen(t *testing.T) { + spec := SpecByName("omnisharp") + require.NotNil(t, spec) + assert.True(t, spec.NoDidOpen, "csharp-ls / omnisharp spec must opt out of the didOpen lifecycle") +} + +// A pass with opensDocs=false must send ZERO document notifications while +// still hovering every node: the docSession degrades to a pure content +// cache. The instrumented server counts didOpen / didClose it receives. +func TestLSP_Enrich_NoDidOpen_SendsNoDocLifecycle(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot, g := seedRepo(t, 6) + + server := newInstrumentedServer() + var hovers atomic.Int64 + server.handle("textDocument/hover", func(params json.RawMessage) (any, *jsonRPCError) { + hovers.Add(1) + return map[string]any{ + "contents": map[string]any{"kind": "plaintext", "value": "func F() string"}, + }, nil + }) + + p, cleanup := providerWithInstrumentedServer(t, server, []string{"go"}, 4) + defer cleanup() + p.opensDocs = false + + require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) + + peak, opens, closes := server.stats() + assert.Zero(t, opens, "no didOpen may be sent in a NoDidOpen pass") + assert.Zero(t, closes, "no didClose either — nothing was opened") + assert.Zero(t, peak) + assert.GreaterOrEqual(t, hovers.Load(), int64(6), "every node still hovered without the lifecycle") +} + +// Control pin: a provider that has not opted out keeps the exact didOpen / +// didClose pairing it has today. +func TestLSP_Enrich_DefaultStillOpensDocuments(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot, g := seedRepo(t, 3) + + server := newInstrumentedServer() + server.handle("textDocument/hover", func(params json.RawMessage) (any, *jsonRPCError) { + return map[string]any{ + "contents": map[string]any{"kind": "plaintext", "value": "func F() string"}, + }, nil + }) + + p, cleanup := providerWithInstrumentedServer(t, server, []string{"go"}, 4) + defer cleanup() + + require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) + + _, opens, _ := server.stats() + assert.Greater(t, opens, 0, "default pass still opens documents") + // closeAll's didClose is a fire-and-forget notification; wait for it to + // land rather than racing the pipe. + assert.Eventually(t, func() bool { + _, o, c := server.stats() + return o > 0 && c == o + }, 2*time.Second, 10*time.Millisecond, "opens and closes stay paired") +} diff --git a/internal/semantic/lsp/enrich_phase_timing_test.go b/internal/semantic/lsp/enrich_phase_timing_test.go new file mode 100644 index 00000000..7cbc9aa1 --- /dev/null +++ b/internal/semantic/lsp/enrich_phase_timing_test.go @@ -0,0 +1,49 @@ +package lsp + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + + "github.com/zzet/gortex/internal/graph" +) + +// The completion log must break the enrich wall time out per pass. Speed +// forensics on a production C# monorepo hit a wall without this: 241k +// requests took the same 26 minutes at maxParallel 6 and 12, so the pole was +// NOT slot starvation — but the single aggregate duration cannot say which +// pass owns the minutes. Every phase boundary already exists in +// EnrichRepoContext; this pins that their wall times ride the log the +// forensics scripts scrape. +func TestLSP_Enrich_CompletionLogCarriesPhaseTimings(t *testing.T) { + t.Setenv(SweepEnv, "full") + repoRoot := t.TempDir() + g := graph.New() + server := newFakeLSPServer() + defConfirmFixture(t, repoRoot, g, server) + + p, cleanup := providerWithFakeServer(t, server, []string{"go"}) + defer cleanup() + core, logs := observer.New(zap.InfoLevel) + p.logger = zap.New(core) + + require.NoError(t, runEnrich(t, p, g, repoRoot, 5*time.Second)) + + entries := logs.FilterMessage("LSP enrich: hover phase complete").All() + require.Len(t, entries, 1, "completion log must be emitted exactly once") + fields := entries[0].ContextMap() + for _, k := range []string{ + "phase_setup_ms", "phase_impls_ms", "phase_confirm_ms", + "phase_definitions_ms", "phase_refs_add_ms", "phase_sweep_ms", + } { + v, ok := fields[k] + require.Truef(t, ok, "completion log must carry %s", k) + ms, isInt := v.(int64) + require.Truef(t, isInt, "%s must be an int64 millisecond count, got %T", k, v) + assert.GreaterOrEqual(t, ms, int64(0), k) + } +} diff --git a/internal/semantic/lsp/graph_batch.go b/internal/semantic/lsp/graph_batch.go index 35f20d5d..84e25391 100644 --- a/internal/semantic/lsp/graph_batch.go +++ b/internal/semantic/lsp/graph_batch.go @@ -205,6 +205,76 @@ func (v *lspGraphView) findDeclarationNode(filePath string, oneBasedLine int, na return near } +// findEnclosingTypeNamed returns the type / interface declaration named name +// whose span contains oneBasedLine in filePath. This is the ctor shape: +// definition on `new T(...)` answers the constructor's line — a member with +// its own name — but the line sits inside the declaration of the very type +// the site instantiates. +func (v *lspGraphView) findEnclosingTypeNamed(filePath string, oneBasedLine int, name string) *graph.Node { + for _, n := range v.nodesByFile[viewPathKey(filePath)] { + if n == nil || n.Name != name { + continue + } + if n.Kind != graph.KindType && n.Kind != graph.KindInterface { + continue + } + if n.StartLine <= oneBasedLine && oneBasedLine <= n.EndLine { + return n + } + } + return nil +} + +// declaredDispatchMember reports whether n is a declared dispatch target — a +// member of an interface, or an abstract-marked member. A definition landing +// on one means the call site's static receiver is the declared surface, not +// any concrete impl. +func (v *lspGraphView) declaredDispatchMember(n *graph.Node) bool { + if n == nil { + return false + } + if isAbstractMarked(n) { + return true + } + parent := v.memberParentType(n) + return parent != nil && parent.Kind == graph.KindInterface +} + +// implementsDeclaredMember reports whether impl is a concrete implementation +// of the declared member decl: an explicit overrides edge, or membership in a +// type that implements / extends decl's declaring type. +func (v *lspGraphView) implementsDeclaredMember(impl, decl *graph.Node) bool { + if impl == nil || decl == nil { + return false + } + for _, e := range v.outByID[impl.ID] { + if e.Kind == graph.EdgeOverrides && e.To == decl.ID { + return true + } + } + implParent := v.memberParentType(impl) + declParent := v.memberParentType(decl) + if implParent == nil || declParent == nil { + return false + } + for _, e := range v.outByID[implParent.ID] { + if (e.Kind == graph.EdgeImplements || e.Kind == graph.EdgeExtends) && e.To == declParent.ID { + return true + } + } + return false +} + +// memberParentType resolves n's declaring type through its member_of edge. +func (v *lspGraphView) memberParentType(n *graph.Node) *graph.Node { + for _, e := range v.outByID[n.ID] { + if e.Kind == graph.EdgeMemberOf { + return v.nodesByID[e.To] + } + } + return nil +} + func (v *lspGraphView) findMatchingEdge(from, to string, kind graph.EdgeKind) *graph.Edge { for _, e := range v.outByID[from] { if e.To == to && e.Kind == kind { diff --git a/internal/semantic/lsp/maxparallel_env_test.go b/internal/semantic/lsp/maxparallel_env_test.go new file mode 100644 index 00000000..45c9460f --- /dev/null +++ b/internal/semantic/lsp/maxparallel_env_test.go @@ -0,0 +1,55 @@ +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +// GORTEX_LSP_MAX_PARALLEL lets an operator raise a server's concurrent +// request cap without editing the registry — the same experiment knob +// shape as GORTEX_LSP_SWEEP. Env wins over the spec default; an unusable +// value falls through rather than failing the spawn. +func TestNewProviderFromSpec_MaxParallelEnvOverride(t *testing.T) { + spec := &ServerSpec{Name: "csharp-ls", Command: "csharp-ls", + Languages: []string{"csharp"}, MaxParallel: 6} + + t.Setenv(MaxParallelEnv, "") + p := NewProviderFromSpec(spec, zap.NewNop()) + assert.Equal(t, 6, p.maxParallel, "spec default when the env is unset") + + t.Setenv(MaxParallelEnv, "12") + p = NewProviderFromSpec(spec, zap.NewNop()) + assert.Equal(t, 12, p.maxParallel, "env override wins over the spec default") + + for _, junk := range []string{"0", "-3", "garbage"} { + t.Setenv(MaxParallelEnv, junk) + p = NewProviderFromSpec(spec, zap.NewNop()) + assert.Equal(t, 6, p.maxParallel, "unusable value %q falls through to the spec", junk) + } + + // A spec with no cap of its own keeps the package default unless the + // env names one. + bare := &ServerSpec{Name: "x", Command: "x", Languages: []string{"go"}} + t.Setenv(MaxParallelEnv, "") + assert.Equal(t, 10, NewProviderFromSpec(bare, zap.NewNop()).maxParallel) + t.Setenv(MaxParallelEnv, "4") + assert.Equal(t, 4, NewProviderFromSpec(bare, zap.NewNop()).maxParallel) +} + +// The durable operator home is config (`semantic.lsp_max_parallel`), with +// the same precedence shape as the sweep mode: env override wins over the +// configured value, which wins over the spec default. +func TestResolveMaxParallel_Precedence(t *testing.T) { + t.Setenv(MaxParallelEnv, "") + assert.Equal(t, 6, resolveMaxParallel(0, 6), "spec default when env + config are unset") + assert.Equal(t, 12, resolveMaxParallel(12, 6), "configured value wins over the spec default") + assert.Equal(t, 6, resolveMaxParallel(-3, 6), "non-positive config is ignored") + assert.Equal(t, 10, resolveMaxParallel(0, 0), "package default when nothing names a cap") + + t.Setenv(MaxParallelEnv, "16") + assert.Equal(t, 16, resolveMaxParallel(12, 6), "env wins over config and spec") + t.Setenv(MaxParallelEnv, "junk") + assert.Equal(t, 12, resolveMaxParallel(12, 6), "unusable env falls through to config") +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index ff40bce0..bccc77ed 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -47,6 +47,18 @@ type Provider struct { // router from config when the provider is spawned. An empty value means // the demand-gated default; the GORTEX_LSP_SWEEP env override wins over it. sweepMode string + // opensDocs reports whether the enrichment pass sends the + // textDocument/didOpen / didClose lifecycle before querying a file. + // Resolved at construction from OpenDocsEnv and the spec's NoDidOpen; + // true for every server that has not opted out. See ServerSpec.NoDidOpen + // for why a barrier-scheduling server wants this off. + opensDocs bool + // noHeavyRequests disables the textDocument/references and + // callHierarchy/incomingCalls legs of the enrichment pass. Set from + // ServerSpec.NoHeavyRequests for servers whose FindReferences machinery + // leaks per request (csharp-ls); ambiguous edges are confirmed through + // the definition pass at their call sites instead. + noHeavyRequests bool // spec is the ServerSpec this provider was built from (when the // caller used NewProviderFromSpec). nil for legacy NewProvider // invocations — those fall back to single-language routing. @@ -207,6 +219,8 @@ func NewProvider(command string, args []string, languages []string, daemon bool, languages: languages, daemon: daemon, maxParallel: maxParallel, + opensDocs: resolveOpensDocs("", nil), + noHeavyRequests: resolveNoHeavyRequests(nil), logger: logger, docVersions: map[string]int{}, openDocs: map[string]bool{}, @@ -218,6 +232,32 @@ func NewProvider(command string, args []string, languages []string, daemon bool, } } +// MaxParallelEnv overrides every spawned server's concurrent-request cap, +// winning over the spec default — the same operator-experiment shape as +// GORTEX_LSP_SWEEP. The probe-measured levers differ per machine (a server +// that multiplexes well takes a higher cap than its conservative spec +// default), so the knob lets an operator try a value for one run without +// editing the registry. Zero, negative, or unparseable values are ignored. +const MaxParallelEnv = "GORTEX_LSP_MAX_PARALLEL" + +// resolveMaxParallel picks the effective concurrent-request cap by the +// sweep-mode precedence: the GORTEX_LSP_MAX_PARALLEL env override wins +// over the operator-configured value (`semantic.lsp_max_parallel`), which +// wins over the spec default; 10 is the package fallback. Non-positive or +// unparseable values at any level fall through to the next. +func resolveMaxParallel(configured, specDefault int) int { + if v, err := strconv.Atoi(strings.TrimSpace(os.Getenv(MaxParallelEnv))); err == nil && v > 0 { + return v + } + if configured > 0 { + return configured + } + if specDefault > 0 { + return specDefault + } + return 10 +} + // NewProviderFromSpec builds a Provider directly from a ServerSpec. // Mostly equivalent to NewProvider but lets the runtime router resolve // the right `languageId` per file extension and pick the first @@ -238,10 +278,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { } } } - maxParallel := spec.MaxParallel - if maxParallel <= 0 { - maxParallel = 10 - } + maxParallel := resolveMaxParallel(0, spec.MaxParallel) p := &Provider{ command: cmd, args: args, @@ -249,6 +286,8 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { languages: spec.Languages, daemon: spec.Daemon, maxParallel: maxParallel, + opensDocs: resolveOpensDocs("", spec), + noHeavyRequests: resolveNoHeavyRequests(spec), logger: logger, spec: spec, docVersions: map[string]int{}, @@ -478,7 +517,14 @@ func edgeExistsAt(view *lspGraphView, from, to string, kind graph.EdgeKind, line // from disk by the caller (via the shared document session). A nil content // yields a no-verdict, matching the pre-session behaviour when the site // file could not be opened. -func (p *Provider) definitionNodeAtSite(view *lspGraphView, repoPrefix, absRoot, siteRel string, siteLine int, name string, content []byte, cache map[string]*graph.Node) (*graph.Node, bool) { +// +// breaker (nilable) observes each definition round trip: under the heavy +// opt-out this pass carries the whole confirm load, so a server that +// errors every request must trip the failure streak instead of grinding +// through every sited target at one timeout each. Only transport results +// feed it — a cache hit or an identifier miss never reaches the server, +// and an answered-but-empty response counts as success. +func (p *Provider) definitionNodeAtSite(view *lspGraphView, repoPrefix, absRoot, siteRel string, siteLine int, name string, content []byte, breaker *phaseBreaker, cache map[string]*graph.Node) (*graph.Node, bool) { if siteRel == "" || siteLine <= 0 || name == "" { return nil, false } @@ -493,6 +539,9 @@ func (p *Provider) definitionNodeAtSite(view *lspGraphView, repoPrefix, absRoot, return nil, false } locs, err := p.FindDefinition(absRoot, siteRel, siteLine-1, col, lspCallTimeout()) + if breaker != nil { + breaker.observe(err == nil) + } if err != nil || len(locs) == 0 { return nil, false } @@ -503,6 +552,14 @@ func (p *Provider) definitionNodeAtSite(view *lspGraphView, repoPrefix, absRoot, return nil, true } node := findDeclarationNode(view, scopedPath(repoPrefix, defPath), locs[0].Range.Start.Line+1, name) + if node == nil { + // A definition can land on a member declaration INSIDE the target + // type — `new T(...)` answers the constructor's line, not the type's, + // and the ctor node carries its own name. A type declaration named + // exactly `name` whose span contains the answered line is the same + // verdict: the site binds to that type. + node = view.findEnclosingTypeNamed(scopedPath(repoPrefix, defPath), locs[0].Range.Start.Line+1, name) + } cache[key] = node return node, true } @@ -949,6 +1006,11 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre targetedBreaker := newPhaseBreaker(lspPhaseFailureStreakLimit(), p.logger, "targeted", repoPrefix) hoverBreaker := newPhaseBreaker(lspPhaseFailureStreakLimit(), p.logger, "hover", repoPrefix) + // Phase boundary timestamps: the completion log breaks the pass wall time + // out per phase (phase_*_ms) — an aggregate duration cannot say which pass + // owns the minutes when a run needs speed forensics. + setupDone := time.Now() + // Query implementations for interface nodes. A degraded pass skips this: // the query opens each interface's file, and a database-less clangd cannot // resolve implementations across translation units regardless. Graph @@ -1024,6 +1086,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre interfaceMutations.apply(g, nil) rmu.Unlock() } + implsDone := time.Now() // Query references for AMBIGUOUS edges to confirm/refute. Promotion // to the lsp tier is identity-anchored: the server's evidence must @@ -1040,14 +1103,33 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // so each file is opened once (per-goroutine, via enrichOpenDoc) and // serves every target sharing it — turning the ~7 edges/s sequential // round-trip loop into maxParallel-wide throughput. The definition-rebind - // fallback opens arbitrary call-site files, so it runs serially afterward - // over the targets the sweep left unconfirmed, keeping document open/close - // from overlapping across goroutines. - confirmGroups := p.groupConfirmTargets(view.nodesByID, targets, degradedSkipFile) + // fallback that follows fans out the same way over the targets the sweep + // left unconfirmed, grouped by call-site file — each site file is owned + // by exactly one goroutine, so document open/close never overlaps on the + // same document. var confirmMu sync.Mutex confirmPromotions := make(map[*graph.Edge]struct{}) var fallback []enrichTarget - { + if p.noHeavyRequests { + // This server leaks per references request (ServerSpec.NoHeavyRequests) + // — the reference sweep never runs. Every sited target goes straight + // to the definition pass below, which reaches the same confirm / + // rebind verdicts from the call site at position-request cost. + // Built from the raw target list, not confirmGroups: the grouping + // drops targets on referent-side conditions (no position, unserved + // referent file) that exist only because findReferences opens the + // REFERENT's file, and a misbound edge with an unusable stored + // target is exactly what the rebind arm is for. The definition pass + // applies its own call-site filters when it groups. Targets without + // a recorded site line are left at their heuristic tier: there is + // no site to ask definition at. + for _, t := range targets { + if t.edge.Line > 0 { + fallback = append(fallback, t) + } + } + } else { + confirmGroups := p.groupConfirmTargets(view.nodesByID, targets, degradedSkipFile) sem := make(chan struct{}, p.maxParallel) var wg sync.WaitGroup for _, grp := range confirmGroups { @@ -1143,107 +1225,147 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre mutations.apply(g, nil) rmu.Unlock() } + confirmDone := time.Now() - // Serial definition-rebind fallback: for a site the reference sweep did - // not tie to the edge's target, ask the server what the site actually - // resolves to (textDocument/definition at the site identifier). When the - // definition lands back on the target we confirm anyway (reference lists - // can be incomplete); when it names a DIFFERENT known declaration we - // rebind the edge to the correct target instead of leaving a - // misattribution behind. Runs after the parallel sweep so arbitrary - // call-site document opens never overlap across goroutines. + // Definition-rebind pass: for a site the reference sweep did not tie to + // the edge's target, ask the server what the site actually resolves to + // (textDocument/definition at the site identifier). When the definition + // lands back on the target we confirm anyway (reference lists can be + // incomplete); when it names a DIFFERENT known declaration we rebind the + // edge to the correct target instead of leaving a misattribution behind. // - // Sites are ordered by their call-site file so one session acquire - // serves every site in a file — a caller with many misbound sites in - // one file is opened once, not once per site. Stable so sites keep - // their relative order within a file. + // Sites are grouped by their call-site file — one session acquire and + // one definition cache serve every site in the file — and the groups fan + // out across maxParallel. The serial loop this replaces existed to keep + // call-site document opens from overlapping across goroutines; for a + // heavy-opt-out server this pass carries the whole confirm load (tens of + // thousands of definitions on a large solution), and a position request + // needs no such serialization. Stable sort so sites keep their relative + // order within a file. sort.SliceStable(fallback, func(i, j int) bool { return edgeSiteRelPath(fallback[i].edge, repoPrefix, nodeRelPath(fallback[i].node)) < edgeSiteRelPath(fallback[j].edge, repoPrefix, nodeRelPath(fallback[j].node)) }) - defSiteCache := map[string]*graph.Node{} - var ( - curSiteRel string - curContent []byte - relSite func() - siteOpen bool - ) - releaseSite := func() { - if siteOpen { - relSite() - siteOpen = false - } - curContent = nil + type defSiteGroup struct { + rel string + targets []enrichTarget } - fallbackMutations := newLSPMutationBatch() + var defGroups []defSiteGroup for _, t := range fallback { - if targetedCtx.Err() != nil || targetedBreaker.isTripped() { - break - } - toNode := view.nodesByID[t.edge.To] - if toNode == nil { - continue - } - callerRel := nodeRelPath(t.node) - siteRel := edgeSiteRelPath(t.edge, repoPrefix, callerRel) + siteRel := edgeSiteRelPath(t.edge, repoPrefix, nodeRelPath(t.node)) if !p.servesFile(siteRel) { continue // never open a call-site file this server can't compile } if degradedSkipFile != nil && degradedSkipFile(siteRel) { continue // degraded mode: never open this call-site (e.g. a header) } - // Hold one open document per run of same-file sites: acquire on the - // first site of a file, release when the file changes (and once more - // after the loop). A failed acquire yields nil content, which - // definitionNodeAtSite treats as a no-verdict — the same outcome the - // per-site openDocument failure produced before. - if siteRel != curSiteRel { - releaseSite() - curSiteRel = siteRel - content, release, err := session.acquire(p.client, filepath.Join(absRoot, siteRel)) - if err == nil { - curContent = content - relSite = release - siteOpen = true + if len(defGroups) == 0 || defGroups[len(defGroups)-1].rel != siteRel { + defGroups = append(defGroups, defSiteGroup{rel: siteRel}) + } + defGroups[len(defGroups)-1].targets = append(defGroups[len(defGroups)-1].targets, t) + } + fallbackMutations := newLSPMutationBatch() + { + sem := make(chan struct{}, p.maxParallel) + var wg sync.WaitGroup + for i := range defGroups { + if targetedCtx.Err() != nil || targetedBreaker.isTripped() { + break } + wg.Add(1) + sem <- struct{}{} + go func(grp *defSiteGroup) { + defer func() { + <-sem + wg.Done() + }() + if targetedCtx.Err() != nil || targetedBreaker.isTripped() { + return + } + // The file is unique to this group, so no two goroutines + // open it. A failed acquire yields nil content, which + // definitionNodeAtSite treats as a no-verdict — the same + // outcome the per-site openDocument failure produced before. + var content []byte + if c, release, err := session.acquire(p.client, filepath.Join(absRoot, grp.rel)); err == nil { + content = c + defer release() + } + // Cache keys carry the site file, so the cache is + // group-local by construction — no sharing to guard. + cache := map[string]*graph.Node{} + for _, t := range grp.targets { + if targetedCtx.Err() != nil || targetedBreaker.isTripped() { + return + } + toNode := view.nodesByID[t.edge.To] + if toNode == nil { + continue + } + cand, ok := p.definitionNodeAtSite(view, repoPrefix, absRoot, grp.rel, t.edge.Line, toNode.Name, content, targetedBreaker, cache) + // The verdict switch reads the view's edge indexes + // (edgeExistsAt, the dispatch-member helpers) that the + // staged mutations also update, so the whole switch — + // counters included — runs under rmu. The expensive + // part, the definition round trip above, stays outside. + rmu.Lock() + switch { + case !ok || cand == nil: + // No verdict — leave the edge at its heuristic tier + // so min_tier filtering excludes it. + case cand.ID == toNode.ID: + // Yield feeds the productivity checkpoint: under the + // heavy opt-out this pass is the only confirm source + // until the hover phase, and without these the + // checkpoint reads a pass that settles thousands of + // edges here as zero-yield and cancels it. + semantic.ConfirmEdge(t.edge, p.Name()) + fallbackMutations.stagePersist(t.edge) + result.EdgesConfirmed++ + usefulYield.Add(1) + case view.declaredDispatchMember(cand) && view.implementsDeclaredMember(toNode, cand): + // The compiler answers the DECLARED member — the site's static + // receiver is the interface / abstract base, so the stored edge + // to one of its concrete impls is a devirtualization inference + // definition cannot vouch for. Add the compiler-proven edge to + // the declared member and leave the inference exactly as it was + // (not retargeted, not promoted): impl reachability flows + // through the declared member's dispatch fan-out, and the + // heuristic tier keeps the guess honestly labeled. A target + // UNRELATED to the declared member is not this case — that is a + // plain misbind and falls through to the rebind below. + if !edgeExistsAt(view, t.edge.From, cand.ID, t.edge.Kind, t.edge.Line) { + declared := newLSPResolvedEdge(t.edge.From, cand.ID, t.edge.Kind, + t.edge.FilePath, t.edge.Line, p.Name(), graph.OriginLSPResolved) + if fallbackMutations.stageAdd(view, declared) { + result.EdgesAdded++ + usefulYield.Add(1) + } + } + case rebindTargetAcceptable(cand.Kind) && !edgeExistsAt(view, t.edge.From, cand.ID, t.edge.Kind, t.edge.Line): + // Mutate the full edge state before staging the set-oriented + // reindex so the backend persists the complete confirmed payload. + oldTo := t.edge.To + t.edge.To = cand.ID + semantic.ConfirmEdge(t.edge, p.Name()) + t.edge.Meta["rebound_from"] = oldTo + fallbackMutations.stageReindex(view, t.edge, oldTo) + result.EdgesRebound++ + usefulYield.Add(1) + } + rmu.Unlock() + } + }(&defGroups[i]) } - siteLine := t.edge.Line - cand, ok := p.definitionNodeAtSite(view, repoPrefix, absRoot, siteRel, siteLine, toNode.Name, curContent, defSiteCache) - switch { - case !ok || cand == nil: - // No verdict — leave the edge at its heuristic tier so - // min_tier filtering excludes it. - case cand.ID == toNode.ID: - rmu.Lock() - semantic.ConfirmEdge(t.edge, p.Name()) - fallbackMutations.stagePersist(t.edge) - rmu.Unlock() - result.EdgesConfirmed++ - // Both fallback arms are yield the productivity checkpoint must - // see: on a degraded pass this loop can be the ONLY source of - // progress, and without these the checkpoint reads a pass that - // settles thousands of edges here as zero-yield and cancels it. - usefulYield.Add(1) - case rebindTargetAcceptable(cand.Kind) && !edgeExistsAt(view, t.edge.From, cand.ID, t.edge.Kind, t.edge.Line): - rmu.Lock() - // Mutate the full edge state before staging the set-oriented - // reindex so the backend persists the complete confirmed payload. - oldTo := t.edge.To - t.edge.To = cand.ID - semantic.ConfirmEdge(t.edge, p.Name()) - t.edge.Meta["rebound_from"] = oldTo - fallbackMutations.stageReindex(view, t.edge, oldTo) - rmu.Unlock() - result.EdgesRebound++ - usefulYield.Add(1) - } - } - releaseSite() - if len(fallbackMutations.reindexes) > 0 || len(fallbackMutations.persists) > 0 { + wg.Wait() + } + if len(fallbackMutations.reindexes) > 0 || len(fallbackMutations.persists) > 0 || + len(fallbackMutations.adds) > 0 { rmu.Lock() fallbackMutations.apply(g, nil) rmu.Unlock() } + defsDone := time.Now() // Degraded finalisation: the interface pass, the references-add pass, and // the per-file hover / hierarchy sweep are all skipped when a needed @@ -1284,9 +1406,11 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // never ADDED. Ask textDocument/references per declaration and mint those // call edges. Runs under the targeted budget (before the hover sweep) so // a deadline cut sheds hover work, not the recall-bearing add. - if p.Supports("textDocument/references") && !p.Supports("textDocument/prepareCallHierarchy") { + if !p.noHeavyRequests && + p.Supports("textDocument/references") && !p.Supports("textDocument/prepareCallHierarchy") { p.referencesAddPass(targetedCtx, g, view, repoPrefix, absRoot, langNodes, rmu, session, result) } + refsAddDone := time.Now() // Per-file document lifecycle + bounded concurrency. The original // implementation bulk-opened every target file up front and closed @@ -1584,8 +1708,13 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // (or under a full sweep). Demand-first file ordering // sweeps the demand-bearing callers before any deadline // cut, so a skipped incoming costs no reachable edge. - wantIncoming := sweepMode == sweepModeFull || - nodeDispatch[n.ID] || nodeDemand[n.ID] + // incomingCalls rides the same FindReferences + // machinery as references — a NoHeavyRequests + // server never gets the round trip, whatever the + // sweep mode says. + wantIncoming := !p.noHeavyRequests && + (sweepMode == sweepModeFull || + nodeDispatch[n.ID] || nodeDemand[n.ID]) for _, item := range items { if outs, oerr := p.outgoingCalls(item); oerr == nil { for _, oc := range outs { @@ -1769,6 +1898,12 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre zap.Int("reopened_files", reopenedFiles), zap.Int("doc_evictions", docEvictions), zap.Int("peak_open_docs", peakOpenDocs), + zap.Int64("phase_setup_ms", setupDone.Sub(start).Milliseconds()), + zap.Int64("phase_impls_ms", implsDone.Sub(setupDone).Milliseconds()), + zap.Int64("phase_confirm_ms", confirmDone.Sub(implsDone).Milliseconds()), + zap.Int64("phase_definitions_ms", defsDone.Sub(confirmDone).Milliseconds()), + zap.Int64("phase_refs_add_ms", refsAddDone.Sub(defsDone).Milliseconds()), + zap.Int64("phase_sweep_ms", time.Since(refsAddDone).Milliseconds()), zap.Int64("req_references", p.reqStats.references.Load()), zap.Int64("req_implementations", p.reqStats.implementations.Load()), zap.Int64("req_definitions", p.reqStats.definitions.Load()), @@ -2814,6 +2949,13 @@ func (p *Provider) ConfirmSymbolRefs(g graph.Store, repoRoot string, n *graph.No if n == nil || (n.Kind != graph.KindFunction && n.Kind != graph.KindMethod) { return 0, nil } + if p.noHeavyRequests { + // incomingCalls rides the same FindReferences machinery the sweep + // skips for this server (ServerSpec.NoHeavyRequests) — a long-lived + // daemon answering find_usages would accumulate the leak one query + // at a time. + return 0, nil + } if !p.Supports("textDocument/prepareCallHierarchy") { return 0, nil } diff --git a/internal/semantic/lsp/registry.go b/internal/semantic/lsp/registry.go index af0844a8..e3ca52de 100644 --- a/internal/semantic/lsp/registry.go +++ b/internal/semantic/lsp/registry.go @@ -102,6 +102,31 @@ type ServerSpec struct { // std-library types the graph never indexes, so its recall lives in // the full sweep rather than in cheaper static confirmation. DefaultSweepMode string + // NoDidOpen marks a server that answers position requests (hover, + // references, call hierarchy) for files that were never opened with + // `textDocument/didOpen`, reading them from its own workspace load + // instead. For such a server the enrichment pass skips the didOpen / + // didClose document lifecycle entirely. The point is throughput, not + // tidiness: a scheduler that runs read-only requests concurrently but + // treats document notifications as exclusive writes (csharp-ls) drains + // every in-flight read on each didOpen, so a sweep that interleaves + // opens with its queries serializes to single-request throughput no + // matter how many requests it keeps in flight. The + // GORTEX_LSP_OPEN_DOCS env override wins over this flag both ways. + NoDidOpen bool + // NoHeavyRequests marks a server whose FindReferences machinery leaks + // memory per request until process exit — csharp-ls holds ~10MB and a + // set of OS handles per textDocument/references round trip and ~0.7MB + // per callHierarchy/incomingCalls, with nothing released short of a + // restart, so a full sweep collapses the server long before it + // completes. For such a server the enrichment pass skips both request + // classes: ambiguous edges are confirmed through textDocument/definition + // at their call sites (a clean position request) and dispatch fan-out + // stays with the graph-side interface-dispatch synthesis. The + // GORTEX_LSP_HEAVY env override wins over this flag both ways — an + // operator running a server build without the leak sets it "on" to + // restore the heavy legs. + NoHeavyRequests bool // ProjectReady, when non-nil, reports whether a workspace has the // project setup this server needs to resolve anything at all — e.g. // node_modules for tsserver, whose every cross-file / import lookup @@ -460,6 +485,17 @@ var Servers = []ServerSpec{ Priority: 5, Daemon: true, MaxParallel: 6, + // Both Roslyn workspaces answer position requests for files that + // were never didOpen'd, and csharp-ls's request scheduler treats + // every didOpen / didClose as an exclusive write that drains all + // in-flight reads — an enrichment pass that interleaves opens + // serializes to single-request throughput. Skip the lifecycle. + NoDidOpen: true, + // csharp-ls leaks per-request on the FindReferences path + // (references, incomingCalls) until process exit; confirm through + // definition at call sites instead and leave dispatch fan-out to + // the interface-dispatch synthesizer. + NoHeavyRequests: true, // csharp-ls is a Roslyn stdio LSP (`dotnet tool install csharp-ls`) // that speaks plain LSP with no args. Current versions discover // solutions on their own (recursive .sln/.slnx glob, most-projects diff --git a/internal/semantic/lsp/router.go b/internal/semantic/lsp/router.go index c9158923..d6bb5d4b 100644 --- a/internal/semantic/lsp/router.go +++ b/internal/semantic/lsp/router.go @@ -119,6 +119,17 @@ type Router struct { // env override wins over it at enrichment time. enrichSweepMode string + // enrichOpenDocs is the configured didOpen-lifecycle override ("on" / + // "off"), propagated to every spawned provider. Empty lets each spec's + // NoDidOpen decide; the GORTEX_LSP_OPEN_DOCS env override wins over it + // at enrichment time. + enrichOpenDocs string + // enrichMaxParallel is the operator-configured concurrent-request cap + // (`semantic.lsp_max_parallel`), propagated to every spawned provider. + // Zero keeps each spec's own default; the GORTEX_LSP_MAX_PARALLEL env + // override wins over both. + enrichMaxParallel int + mu sync.Mutex providers map[providerKey]*routedProvider // (spec.Name, workspace) → cached provider enabled map[string]*ServerSpec // spec.Name → spec marked enabled by config (no spawn until For/ForSpec) @@ -412,6 +423,24 @@ func (r *Router) WithEnrichSweepMode(mode string) *Router { return r } +// WithEnrichOpenDocs sets the configured didOpen-lifecycle override ("on" / +// "off") propagated to every spawned provider. An empty value lets each +// spec's NoDidOpen decide; the GORTEX_LSP_OPEN_DOCS env override still wins +// over whatever is set here. Builder-style. +func (r *Router) WithEnrichOpenDocs(v string) *Router { + r.enrichOpenDocs = v + return r +} + +// WithEnrichMaxParallel sets the operator-configured concurrent-request +// cap propagated to every spawned provider. Zero keeps each spec's own +// default; the GORTEX_LSP_MAX_PARALLEL env override still wins over +// whatever is set here. Builder-style. +func (r *Router) WithEnrichMaxParallel(n int) *Router { + r.enrichMaxParallel = n + return r +} + // WithReaperInterval starts a background reaper that calls Reap() at // the given cadence. Idempotent — calling twice replaces the previous // reaper. A zero duration disables reaping. @@ -581,6 +610,8 @@ func (r *Router) forSpecWorkspace(spec *ServerSpec, workspace string, pin bool) p.workspaceFolders = r.additionalWorkspaceFolders p.excludeGlobs = r.enrichExcludeGlobs p.sweepMode = r.enrichSweepMode + p.opensDocs = resolveOpensDocs(r.enrichOpenDocs, spec) + p.maxParallel = resolveMaxParallel(r.enrichMaxParallel, spec.MaxParallel) // ruby-lsp (and any spec opting in) runs a `bundle install` for a composed // bundle on spawn unless BUNDLE_GEMFILE is set; point it at the workspace's // own Gemfile when present so enrichment skips that install. diff --git a/internal/semantic/lsp/sweep.go b/internal/semantic/lsp/sweep.go index cdafd593..6c079b4e 100644 --- a/internal/semantic/lsp/sweep.go +++ b/internal/semantic/lsp/sweep.go @@ -109,3 +109,64 @@ func sweepFile(mode string, demand int, dispatch bool) bool { return demand > 0 || dispatch } } + +// OpenDocsEnv is the environment variable that overrides whether the +// enrichment pass sends the textDocument/didOpen / didClose document +// lifecycle before querying a file. "1" / "true" forces the lifecycle on +// even for a server whose spec opts out; "0" / "false" skips it for every +// server. Empty falls through to the spec's NoDidOpen. +const OpenDocsEnv = "GORTEX_LSP_OPEN_DOCS" + +// normalizeOnOff canonicalises an on/off override value ("on" / "1" / +// "true", "off" / "0" / "false") — the shared vocabulary of the open-docs +// and heavy-requests overrides. An empty or unrecognised value returns "" +// so the caller falls through to the next precedence source. +func normalizeOnOff(v string) string { + switch strings.ToLower(strings.TrimSpace(v)) { + case "on", "1", "true": + return "on" + case "off", "0", "false": + return "off" + default: + return "" + } +} + +// resolveOpensDocs reports whether the enrichment pass should send the +// didOpen / didClose lifecycle for this server, by precedence: the +// GORTEX_LSP_OPEN_DOCS env override wins over the operator-configured +// value (`semantic.lsp_open_docs`), which wins over the spec's NoDidOpen, +// which wins over the open-by-default fallback. An unrecognised value at +// any level is ignored (falls through) rather than failing the pass. +func resolveOpensDocs(configured string, spec *ServerSpec) bool { + if env := normalizeOnOff(os.Getenv(OpenDocsEnv)); env != "" { + return env == "on" + } + if cfg := normalizeOnOff(configured); cfg != "" { + return cfg == "on" + } + if spec != nil && spec.NoDidOpen { + return false + } + return true +} + +// HeavyRequestsEnv is the environment variable that overrides the +// heavy-request opt-out (ServerSpec.NoHeavyRequests) in both directions: +// "on" / "1" / "true" restores textDocument/references and +// callHierarchy/incomingCalls for a server whose spec opts out — the +// operator runs a build without the FindReferences leak — while "off" / +// "0" / "false" disables them for every server. Empty falls through to +// the spec. +const HeavyRequestsEnv = "GORTEX_LSP_HEAVY" + +// resolveNoHeavyRequests reports whether the enrichment pass must skip the +// heavy request classes for this server: the GORTEX_LSP_HEAVY env override +// wins over the spec's NoHeavyRequests, which wins over the allow-by-default +// fallback. Shares the on/off vocabulary of the open-docs override. +func resolveNoHeavyRequests(spec *ServerSpec) bool { + if env := normalizeOnOff(os.Getenv(HeavyRequestsEnv)); env != "" { + return env == "off" + } + return spec != nil && spec.NoHeavyRequests +} diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index b84ae485..29eb2be3 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -317,6 +317,8 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { RefuteUnconfirmed: conf.Semantic.RefuteUnconfirmed, ExcludeGlobs: conf.Semantic.ExcludeGlobs, LSPSweep: conf.Semantic.LSPSweep, + LSPOpenDocs: conf.Semantic.LSPOpenDocs, + LSPMaxParallel: conf.Semantic.LSPMaxParallel, EagerLSP: eagerLSPEnabled(conf.Semantic), } for _, pc := range conf.Semantic.Providers { @@ -382,7 +384,9 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { WithMaxAlive(6). WithAdditionalWorkspaceFolders(conf.Semantic.AdditionalWorkspaceFolders). WithEnrichExcludeGlobs(conf.Semantic.ExcludeGlobs). - WithEnrichSweepMode(semCfg.LSPSweep) + WithEnrichSweepMode(semCfg.LSPSweep). + WithEnrichOpenDocs(semCfg.LSPOpenDocs). + WithEnrichMaxParallel(semCfg.LSPMaxParallel) semMgr.SetLSPRouter(lspRouter) for _, pc := range semCfg.Providers {