From 0fa45247dc4aba106a712722683294f1943d3615 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:43:19 +0200 Subject: [PATCH 01/18] lsp: allow GORTEX_LSP_MAX_PARALLEL to override the spec cap Per-spec MaxParallel is hardcoded in the registry, so an operator whose server multiplexes better than the conservative default has no way to try a higher cap without editing source. Env wins over the spec value at spawn, same shape as GORTEX_LSP_SWEEP; unusable values fall through. --- docs/lsp.md | 4 ++ internal/semantic/lsp/maxparallel_env_test.go | 39 +++++++++++++++++++ internal/semantic/lsp/provider.go | 11 ++++++ 3 files changed, 54 insertions(+) create mode 100644 internal/semantic/lsp/maxparallel_env_test.go diff --git a/docs/lsp.md b/docs/lsp.md index 7e75eb2e..e978fdf1 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -464,3 +464,7 @@ 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). + `GORTEX_LSP_MAX_PARALLEL` overrides the spec's cap for every spawned + server — an operator-experiment knob for a machine whose servers + multiplex better than the conservative default assumes. Ignored when + zero, negative, or unparseable. diff --git a/internal/semantic/lsp/maxparallel_env_test.go b/internal/semantic/lsp/maxparallel_env_test.go new file mode 100644 index 00000000..0ad21335 --- /dev/null +++ b/internal/semantic/lsp/maxparallel_env_test.go @@ -0,0 +1,39 @@ +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) +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 845d4333..bf786c78 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -218,6 +218,14 @@ 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" + // 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 @@ -239,6 +247,9 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { } } maxParallel := spec.MaxParallel + if v, err := strconv.Atoi(strings.TrimSpace(os.Getenv(MaxParallelEnv))); err == nil && v > 0 { + maxParallel = v + } if maxParallel <= 0 { maxParallel = 10 } From 3a5dcdf382ca4e1971adb060ac341bad81e71f79 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:48:28 +0200 Subject: [PATCH 02/18] lsp: thread the max-parallel cap through config semantic.lsp_max_parallel overrides every spec's registry default when positive, threaded router-to-provider the same way lsp_sweep is; the GORTEX_LSP_MAX_PARALLEL env override keeps the last word, spec default as fallback - resolveMaxParallel mirrors resolveSweepMode's precedence. --- internal/config/config.go | 5 ++++ internal/semantic/config.go | 4 +++ internal/semantic/lsp/maxparallel_env_test.go | 16 ++++++++++++ internal/semantic/lsp/provider.go | 26 ++++++++++++++----- internal/semantic/lsp/router.go | 16 ++++++++++++ internal/serverstack/shared_server.go | 4 ++- 6 files changed, 63 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9e9661ea..910d8e1e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -236,6 +236,11 @@ 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"` + // 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..a6dd6733 100644 --- a/internal/semantic/config.go +++ b/internal/semantic/config.go @@ -21,6 +21,10 @@ 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"` + // 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/maxparallel_env_test.go b/internal/semantic/lsp/maxparallel_env_test.go index 0ad21335..45c9460f 100644 --- a/internal/semantic/lsp/maxparallel_env_test.go +++ b/internal/semantic/lsp/maxparallel_env_test.go @@ -37,3 +37,19 @@ func TestNewProviderFromSpec_MaxParallelEnvOverride(t *testing.T) { 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 bf786c78..c6312e65 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -226,6 +226,24 @@ func NewProvider(command string, args []string, languages []string, daemon bool, // 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 @@ -246,13 +264,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { } } } - maxParallel := spec.MaxParallel - if v, err := strconv.Atoi(strings.TrimSpace(os.Getenv(MaxParallelEnv))); err == nil && v > 0 { - maxParallel = v - } - if maxParallel <= 0 { - maxParallel = 10 - } + maxParallel := resolveMaxParallel(0, spec.MaxParallel) p := &Provider{ command: cmd, args: args, diff --git a/internal/semantic/lsp/router.go b/internal/semantic/lsp/router.go index c9158923..3b9c8b26 100644 --- a/internal/semantic/lsp/router.go +++ b/internal/semantic/lsp/router.go @@ -119,6 +119,12 @@ type Router struct { // env override wins over it at enrichment time. enrichSweepMode 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 +418,15 @@ func (r *Router) WithEnrichSweepMode(mode string) *Router { 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 +596,7 @@ func (r *Router) forSpecWorkspace(spec *ServerSpec, workspace string, pin bool) p.workspaceFolders = r.additionalWorkspaceFolders p.excludeGlobs = r.enrichExcludeGlobs p.sweepMode = r.enrichSweepMode + 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/serverstack/shared_server.go b/internal/serverstack/shared_server.go index b84ae485..294d5fb6 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -317,6 +317,7 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { RefuteUnconfirmed: conf.Semantic.RefuteUnconfirmed, ExcludeGlobs: conf.Semantic.ExcludeGlobs, LSPSweep: conf.Semantic.LSPSweep, + LSPMaxParallel: conf.Semantic.LSPMaxParallel, EagerLSP: eagerLSPEnabled(conf.Semantic), } for _, pc := range conf.Semantic.Providers { @@ -382,7 +383,8 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { WithMaxAlive(6). WithAdditionalWorkspaceFolders(conf.Semantic.AdditionalWorkspaceFolders). WithEnrichExcludeGlobs(conf.Semantic.ExcludeGlobs). - WithEnrichSweepMode(semCfg.LSPSweep) + WithEnrichSweepMode(semCfg.LSPSweep). + WithEnrichMaxParallel(semCfg.LSPMaxParallel) semMgr.SetLSPRouter(lspRouter) for _, pc := range semCfg.Providers { From 4af127e3cced00dfe086794b65e58c83ce7b8d28 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:49:16 +0200 Subject: [PATCH 03/18] docs: document the config-first max-parallel knob --- docs/lsp.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/lsp.md b/docs/lsp.md index e978fdf1..fbff0a41 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -464,7 +464,8 @@ 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). - `GORTEX_LSP_MAX_PARALLEL` overrides the spec's cap for every spawned - server — an operator-experiment knob for a machine whose servers - multiplex better than the conservative default assumes. Ignored when - zero, negative, or unparseable. + `semantic.lsp_max_parallel` in config overrides the spec's cap for + every 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. Non-positive or unparseable values fall through. From d5f4a22d046a13bd6ff490432250e41adca1ec8b Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:48:15 +0200 Subject: [PATCH 04/18] lsp: skip the didOpen lifecycle for servers that serve unopened files csharp-ls schedules read-only requests concurrently but treats every didOpen / didClose as an exclusive write that drains all in-flight reads, so an enrichment pass that interleaves opens serializes to single-request throughput regardless of maxParallel. Roslyn answers hover / references / call hierarchy for never-opened files, so a spec that sets NoDidOpen now runs the whole pass without the document lifecycle: the shared docSession degrades to a bounded content cache and sends no notifications. GORTEX_LSP_OPEN_DOCS overrides both ways. --- docs/lsp.md | 17 +++ internal/semantic/lsp/doc_session.go | 41 ++++--- internal/semantic/lsp/enrich_no_open_test.go | 118 +++++++++++++++++++ internal/semantic/lsp/provider.go | 8 ++ internal/semantic/lsp/registry.go | 18 +++ internal/semantic/lsp/sweep.go | 25 ++++ 6 files changed, 213 insertions(+), 14 deletions(-) create mode 100644 internal/semantic/lsp/enrich_no_open_test.go diff --git a/docs/lsp.md b/docs/lsp.md index 7e75eb2e..728292b8 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -235,6 +235,23 @@ 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. + ### Sweep modes The per-file sweep (phase 5) is gated by a **sweep mode**: diff --git a/internal/semantic/lsp/doc_session.go b/internal/semantic/lsp/doc_session.go index c74d7755..82af6d3f 100644 --- a/internal/semantic/lsp/doc_session.go +++ b/internal/semantic/lsp/doc_session.go @@ -23,6 +23,12 @@ 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 honestly reports zero. + sendOpens bool mu sync.Mutex perClient map[*Client]*clientDocs @@ -65,6 +71,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 +122,25 @@ 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-- - s.evictions++ + if s.sendOpens { + _ = s.p.enrichCloseDoc(c, evPath) + s.curOpen-- + s.evictions++ + } } - 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 +173,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/enrich_no_open_test.go b/internal/semantic/lsp/enrich_no_open_test.go new file mode 100644 index 00000000..4c9d6e08 --- /dev/null +++ b/internal/semantic/lsp/enrich_no_open_test.go @@ -0,0 +1,118 @@ +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 > 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: 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 is silent") + + t.Setenv(OpenDocsEnv, "0") + assert.False(t, resolveOpensDocs(nil), "env 0 skips the lifecycle for every server") + + t.Setenv(OpenDocsEnv, "false") + assert.False(t, resolveOpensDocs(&ServerSpec{Name: "x"}), "env false spelling") + + t.Setenv(OpenDocsEnv, "1") + assert.True(t, resolveOpensDocs(optOut), "env 1 is the kill switch: lifecycle forced on over the opt-out") +} + +// 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, closes := server.stats() + assert.Greater(t, opens, 0, "default pass still opens documents") + assert.Equal(t, opens, closes, "opens and closes stay paired") +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 845d4333..bd66feae 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -47,6 +47,12 @@ 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 // 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 +213,7 @@ func NewProvider(command string, args []string, languages []string, daemon bool, languages: languages, daemon: daemon, maxParallel: maxParallel, + opensDocs: resolveOpensDocs(nil), logger: logger, docVersions: map[string]int{}, openDocs: map[string]bool{}, @@ -249,6 +256,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { languages: spec.Languages, daemon: spec.Daemon, maxParallel: maxParallel, + opensDocs: resolveOpensDocs(spec), logger: logger, spec: spec, docVersions: map[string]int{}, diff --git a/internal/semantic/lsp/registry.go b/internal/semantic/lsp/registry.go index af0844a8..0be5b1b2 100644 --- a/internal/semantic/lsp/registry.go +++ b/internal/semantic/lsp/registry.go @@ -102,6 +102,18 @@ 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 // 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 +472,12 @@ 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 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/sweep.go b/internal/semantic/lsp/sweep.go index cdafd593..61053177 100644 --- a/internal/semantic/lsp/sweep.go +++ b/internal/semantic/lsp/sweep.go @@ -109,3 +109,28 @@ 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" + +// 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 spec's NoDidOpen, which +// wins over the open-by-default fallback. An unrecognised env value is +// ignored (falls through) rather than failing the pass. +func resolveOpensDocs(spec *ServerSpec) bool { + switch strings.ToLower(strings.TrimSpace(os.Getenv(OpenDocsEnv))) { + case "1", "true": + return true + case "0", "false": + return false + } + if spec != nil && spec.NoDidOpen { + return false + } + return true +} From d19bd92319917d6c400bd063e50d5a9a4e53c288 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:01:07 +0200 Subject: [PATCH 05/18] lsp: config knob for the didOpen lifecycle (semantic.lsp_open_docs) Same precedence shape as lsp_sweep: env > config > spec > default. Empty (the default) lets each spec's NoDidOpen decide; "on" forces the lifecycle for every server, "off" skips it everywhere. --- internal/config/config.go | 10 ++++++ internal/semantic/config.go | 5 +++ internal/semantic/lsp/enrich_no_open_test.go | 28 ++++++++++++----- internal/semantic/lsp/provider.go | 4 +-- internal/semantic/lsp/router.go | 16 ++++++++++ internal/semantic/lsp/sweep.go | 33 ++++++++++++++------ internal/serverstack/shared_server.go | 4 ++- 7 files changed, 81 insertions(+), 19 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9e9661ea..412e45e3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -236,6 +236,16 @@ 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"` // 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..f6df62c6 100644 --- a/internal/semantic/config.go +++ b/internal/semantic/config.go @@ -21,6 +21,11 @@ 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"` // 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/enrich_no_open_test.go b/internal/semantic/lsp/enrich_no_open_test.go index 4c9d6e08..b1baf47a 100644 --- a/internal/semantic/lsp/enrich_no_open_test.go +++ b/internal/semantic/lsp/enrich_no_open_test.go @@ -24,23 +24,37 @@ import ( // that opts in via ServerSpec.NoDidOpen gets a pass that sends no document // lifecycle at all. -// resolveOpensDocs precedence: env override > spec opt-out > open-by-default. +// 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: 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 is silent") + 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("", 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") + assert.False(t, resolveOpensDocs("", &ServerSpec{Name: "x"}), "env false spelling") t.Setenv(OpenDocsEnv, "1") - assert.True(t, resolveOpensDocs(optOut), "env 1 is the kill switch: lifecycle forced on over the opt-out") + 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 — diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index bd66feae..8f385b37 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -213,7 +213,7 @@ func NewProvider(command string, args []string, languages []string, daemon bool, languages: languages, daemon: daemon, maxParallel: maxParallel, - opensDocs: resolveOpensDocs(nil), + opensDocs: resolveOpensDocs("", nil), logger: logger, docVersions: map[string]int{}, openDocs: map[string]bool{}, @@ -256,7 +256,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { languages: spec.Languages, daemon: spec.Daemon, maxParallel: maxParallel, - opensDocs: resolveOpensDocs(spec), + opensDocs: resolveOpensDocs("", spec), logger: logger, spec: spec, docVersions: map[string]int{}, diff --git a/internal/semantic/lsp/router.go b/internal/semantic/lsp/router.go index c9158923..7c664ec0 100644 --- a/internal/semantic/lsp/router.go +++ b/internal/semantic/lsp/router.go @@ -119,6 +119,12 @@ 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 + 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 +418,15 @@ 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 +} + // 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 +596,7 @@ 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) // 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 61053177..fa20cc3d 100644 --- a/internal/semantic/lsp/sweep.go +++ b/internal/semantic/lsp/sweep.go @@ -117,17 +117,32 @@ func sweepFile(mode string, demand int, dispatch bool) bool { // server. Empty falls through to the spec's NoDidOpen. const OpenDocsEnv = "GORTEX_LSP_OPEN_DOCS" +// normalizeOpenDocs canonicalises an open-docs override to "on" / "off". +// An empty or unrecognised value returns "" so the caller falls through to +// the next precedence source. +func normalizeOpenDocs(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 spec's NoDidOpen, which -// wins over the open-by-default fallback. An unrecognised env value is -// ignored (falls through) rather than failing the pass. -func resolveOpensDocs(spec *ServerSpec) bool { - switch strings.ToLower(strings.TrimSpace(os.Getenv(OpenDocsEnv))) { - case "1", "true": - return true - case "0", "false": - return false +// 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 := normalizeOpenDocs(os.Getenv(OpenDocsEnv)); env != "" { + return env == "on" + } + if cfg := normalizeOpenDocs(configured); cfg != "" { + return cfg == "on" } if spec != nil && spec.NoDidOpen { return false diff --git a/internal/serverstack/shared_server.go b/internal/serverstack/shared_server.go index b84ae485..6a7414a8 100644 --- a/internal/serverstack/shared_server.go +++ b/internal/serverstack/shared_server.go @@ -317,6 +317,7 @@ func NewSharedServer(cfg SharedServerConfig) (*SharedServer, error) { RefuteUnconfirmed: conf.Semantic.RefuteUnconfirmed, ExcludeGlobs: conf.Semantic.ExcludeGlobs, LSPSweep: conf.Semantic.LSPSweep, + LSPOpenDocs: conf.Semantic.LSPOpenDocs, EagerLSP: eagerLSPEnabled(conf.Semantic), } for _, pc := range conf.Semantic.Providers { @@ -382,7 +383,8 @@ 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) semMgr.SetLSPRouter(lspRouter) for _, pc := range semCfg.Providers { From b76457d8ba10c839d8c4c036d35fc5551ba97532 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:05:40 +0200 Subject: [PATCH 06/18] lsp: wait for the paired didClose in the lifecycle control test closeAll's didClose is a fire-and-forget notification; asserting the pairing immediately races the pipe. --- internal/semantic/lsp/enrich_no_open_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/semantic/lsp/enrich_no_open_test.go b/internal/semantic/lsp/enrich_no_open_test.go index b1baf47a..de5c34b2 100644 --- a/internal/semantic/lsp/enrich_no_open_test.go +++ b/internal/semantic/lsp/enrich_no_open_test.go @@ -126,7 +126,12 @@ func TestLSP_Enrich_DefaultStillOpensDocuments(t *testing.T) { require.NoError(t, runEnrich(t, p, g, repoRoot, 10*time.Second)) - _, opens, closes := server.stats() + _, opens, _ := server.stats() assert.Greater(t, opens, 0, "default pass still opens documents") - assert.Equal(t, opens, closes, "opens and closes stay paired") + // 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") } From f50c893c453041c654160a064386bc6e0b84074a Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:35:07 +0200 Subject: [PATCH 07/18] lsp: confirm edges through definition for servers that leak on FindReferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit csharp-ls leaks per request on Roslyn's FindReferences machinery — ~10MB and a set of OS handles per textDocument/references round trip, ~0.7MB per callHierarchy/incomingCalls — and nothing is released short of a server restart, so a barrier-free enrichment pass collapses the process tens of GB deep before it completes. Position requests are clean (hover and definition both measure ~1KB/request, flat over hundreds of thousands of requests). ServerSpec.NoHeavyRequests (set for csharp-ls/omnisharp) now disables both request classes. Ambiguous edges route straight to the existing definition pass at their call sites, which reaches the same confirm / rebind verdicts from the cheap direction — ask the site what it binds to instead of asking the callee who references it. Two verdict shapes the references path used to paper over are handled explicitly: - definition on `new T(...)` answers the constructor's line; a type named for the target whose span contains that line confirms the instantiates edge (findEnclosingTypeNamed). - definition on a dispatched call answers the DECLARED member. When the stored edge targets one of its concrete impls, the declared- member edge is added at LSP grade and the impl edge — a devirtualization inference the compiler cannot vouch for — is left untouched at its heuristic tier; impl reachability flows through the interface-dispatch synthesis. An unrelated misbind still rebinds as before (implementsDeclaredMember separates the two). Measured on a 120-edge C# probe corpus: definition reproduces every edge the references path stored — 108 exact, 4 ctor-shape, 8 interface-shape — at ~1,100 requests/s where references sustained 2-10/s with an unbounded ramp. --- .../semantic/lsp/enrich_defconfirm_test.go | 267 ++++++++++++++++++ internal/semantic/lsp/graph_batch.go | 70 +++++ internal/semantic/lsp/provider.go | 66 ++++- internal/semantic/lsp/registry.go | 15 + 4 files changed, 413 insertions(+), 5 deletions(-) create mode 100644 internal/semantic/lsp/enrich_defconfirm_test.go diff --git a/internal/semantic/lsp/enrich_defconfirm_test.go b/internal/semantic/lsp/enrich_defconfirm_test.go new file mode 100644 index 00000000..5e124726 --- /dev/null +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -0,0 +1,267 @@ +package lsp + +import ( + "encoding/json" + "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") +} + +// 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() + 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 := graph.New() + 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 := newFakeLSPServer() + 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 }) + + 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") +} 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/provider.go b/internal/semantic/lsp/provider.go index 8f385b37..f416280c 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -53,6 +53,12 @@ type Provider struct { // 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. @@ -257,6 +263,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { daemon: spec.Daemon, maxParallel: maxParallel, opensDocs: resolveOpensDocs("", spec), + noHeavyRequests: spec.NoHeavyRequests, logger: logger, spec: spec, docVersions: map[string]int{}, @@ -511,6 +518,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 } @@ -1055,7 +1070,21 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre 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. + // Targets without a recorded site line are left at their heuristic + // tier: there is no site to ask definition at. + for _, grp := range confirmGroups { + for _, t := range grp.targets { + if t.edge.Line > 0 { + fallback = append(fallback, t) + } + } + } + } else { sem := make(chan struct{}, p.maxParallel) var wg sync.WaitGroup for _, grp := range confirmGroups { @@ -1227,6 +1256,26 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre fallbackMutations.stagePersist(t.edge) rmu.Unlock() result.EdgesConfirmed++ + 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) + rmu.Lock() + if fallbackMutations.stageAdd(view, declared) { + result.EdgesAdded++ + } + rmu.Unlock() + } 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 @@ -1241,7 +1290,8 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre } } releaseSite() - if len(fallbackMutations.reindexes) > 0 || len(fallbackMutations.persists) > 0 { + if len(fallbackMutations.reindexes) > 0 || len(fallbackMutations.persists) > 0 || + len(fallbackMutations.adds) > 0 { rmu.Lock() fallbackMutations.apply(g, nil) rmu.Unlock() @@ -1285,7 +1335,8 @@ 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) } @@ -1585,8 +1636,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 { diff --git a/internal/semantic/lsp/registry.go b/internal/semantic/lsp/registry.go index 0be5b1b2..ff392a36 100644 --- a/internal/semantic/lsp/registry.go +++ b/internal/semantic/lsp/registry.go @@ -114,6 +114,16 @@ type ServerSpec struct { // 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. + 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 @@ -478,6 +488,11 @@ var Servers = []ServerSpec{ // 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 From 0fb81c42477839f2ec3120e3f37b1337a3a9a2a9 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 20:55:42 +0200 Subject: [PATCH 08/18] lsp: env override for the heavy-request opt-out (GORTEX_LSP_HEAVY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NoHeavyRequests spec flag guards stock csharp-ls builds, but the leak it works around has a known upstream fix — an operator running a patched server build wants references / incomingCalls back without rebuilding gortex. GORTEX_LSP_HEAVY wins over the spec flag both ways, mirroring GORTEX_LSP_OPEN_DOCS: "on" restores the heavy legs for an opted-out spec, "off" disables them for every server, anything else falls through to the spec. --- .../semantic/lsp/enrich_defconfirm_test.go | 36 +++++++++++++++++++ internal/semantic/lsp/provider.go | 3 +- internal/semantic/lsp/registry.go | 5 ++- internal/semantic/lsp/sweep.go | 20 +++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/internal/semantic/lsp/enrich_defconfirm_test.go b/internal/semantic/lsp/enrich_defconfirm_test.go index 5e124726..035cde5c 100644 --- a/internal/semantic/lsp/enrich_defconfirm_test.go +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -265,3 +265,39 @@ func TestLSP_Enrich_DefConfirm_DispatchedCallAddsDeclaredMemberEdge(t *testing.T 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) + }) +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index f416280c..26e78e3e 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -220,6 +220,7 @@ func NewProvider(command string, args []string, languages []string, daemon bool, daemon: daemon, maxParallel: maxParallel, opensDocs: resolveOpensDocs("", nil), + noHeavyRequests: resolveNoHeavyRequests(nil), logger: logger, docVersions: map[string]int{}, openDocs: map[string]bool{}, @@ -263,7 +264,7 @@ func NewProviderFromSpec(spec *ServerSpec, logger *zap.Logger) *Provider { daemon: spec.Daemon, maxParallel: maxParallel, opensDocs: resolveOpensDocs("", spec), - noHeavyRequests: spec.NoHeavyRequests, + noHeavyRequests: resolveNoHeavyRequests(spec), logger: logger, spec: spec, docVersions: map[string]int{}, diff --git a/internal/semantic/lsp/registry.go b/internal/semantic/lsp/registry.go index ff392a36..e3ca52de 100644 --- a/internal/semantic/lsp/registry.go +++ b/internal/semantic/lsp/registry.go @@ -122,7 +122,10 @@ type ServerSpec struct { // 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. + // 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. diff --git a/internal/semantic/lsp/sweep.go b/internal/semantic/lsp/sweep.go index fa20cc3d..e607c7dd 100644 --- a/internal/semantic/lsp/sweep.go +++ b/internal/semantic/lsp/sweep.go @@ -149,3 +149,23 @@ func resolveOpensDocs(configured string, spec *ServerSpec) bool { } 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 := normalizeOpenDocs(os.Getenv(HeavyRequestsEnv)); env != "" { + return env == "off" + } + return spec != nil && spec.NoHeavyRequests +} From 0db62d9dd0442b4850e5b21c731411ecba93518d Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Wed, 19 Aug 2026 23:16:53 +0200 Subject: [PATCH 09/18] lsp: fan the definition pass out across call-site files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definition-rebind loop ran serially — a didOpen-era constraint that kept call-site document opens from overlapping goroutines. With the lifecycle gone for NoDidOpen servers, and the heavy opt-out routing the whole confirm load through this pass, the serialization became the sweep's longest pole: a large C# solution answers tens of thousands of definitions one at a time while the server sits mostly idle. Group the sorted targets by call-site file — one session acquire and one site cache per group, both group-local by construction — and fan the groups out across maxParallel, the same shape the reference-confirm sweep already uses. The verdict switch (which reads the view's edge indexes that staged mutations update) moves entirely under rmu; the definition round trip stays outside it. --- .../semantic/lsp/enrich_defconfirm_test.go | 81 +++++++ internal/semantic/lsp/provider.go | 199 ++++++++++-------- 2 files changed, 188 insertions(+), 92 deletions(-) diff --git a/internal/semantic/lsp/enrich_defconfirm_test.go b/internal/semantic/lsp/enrich_defconfirm_test.go index 035cde5c..6db07b61 100644 --- a/internal/semantic/lsp/enrich_defconfirm_test.go +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -2,6 +2,7 @@ package lsp import ( "encoding/json" + "fmt" "os" "path/filepath" "sync/atomic" @@ -301,3 +302,83 @@ func TestResolveNoHeavyRequests_Precedence(t *testing.T) { 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") +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index cd67ce91..47d4de1c 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -1205,115 +1205,130 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre rmu.Unlock() } - // 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}) } - 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++ - 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) - rmu.Lock() - if fallbackMutations.stageAdd(view, declared) { - result.EdgesAdded++ - } - rmu.Unlock() + 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 } - 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.EdgesConfirmed++ + 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, 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: + semantic.ConfirmEdge(t.edge, p.Name()) + fallbackMutations.stagePersist(t.edge) + result.EdgesConfirmed++ + 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++ + } + } + 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.EdgesConfirmed++ + } + rmu.Unlock() + } + }(&defGroups[i]) } + wg.Wait() } - releaseSite() if len(fallbackMutations.reindexes) > 0 || len(fallbackMutations.persists) > 0 || len(fallbackMutations.adds) > 0 { rmu.Lock() From ee213f885b88db48a69ab738e1104d51327a7267 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:49:04 +0200 Subject: [PATCH 10/18] lsp: break enrich wall time out per phase in the completion log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 241k requests took the same 26 minutes at maxParallel 6 and 12, so the pole is not slot starvation — but one aggregate duration cannot say which pass owns the minutes. Stamp each phase boundary (setup, impls, confirm, definitions, refs-add, sweep) and emit phase_*_ms alongside the request counters. --- .../semantic/lsp/enrich_phase_timing_test.go | 48 +++++++++++++++++++ internal/semantic/lsp/provider.go | 15 ++++++ 2 files changed, 63 insertions(+) create mode 100644 internal/semantic/lsp/enrich_phase_timing_test.go 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..a9f57692 --- /dev/null +++ b/internal/semantic/lsp/enrich_phase_timing_test.go @@ -0,0 +1,48 @@ +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. The Fleet +// forensics hit a wall without this: 241k requests took the same 26 minutes at +// maxParallel 6 and 12, so the pole is 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/provider.go b/internal/semantic/lsp/provider.go index 47d4de1c..63cf4ede 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -996,6 +996,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 @@ -1071,6 +1076,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 @@ -1204,6 +1210,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre mutations.apply(g, nil) rmu.Unlock() } + confirmDone := time.Now() // 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 @@ -1335,6 +1342,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre 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 @@ -1378,6 +1386,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre 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 @@ -1865,6 +1874,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()), From c6313b6d9eb4084207351f71d750f1756d310961 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:35:01 +0200 Subject: [PATCH 11/18] lsp: feed the targeted breaker from the definition pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under the heavy opt-out the definition pass carries the whole confirm load, but nothing in it ever observed the failure-streak breaker — a server that errors every request would grind through every sited target at one timeout each instead of aborting after the streak limit. Observe each definition round trip; cache hits, identifier misses, and answered-but-empty responses never touch the breaker. --- .../lsp/enrich_defconfirm_breaker_test.go | 104 ++++++++++++++++++ internal/semantic/lsp/provider.go | 14 ++- 2 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 internal/semantic/lsp/enrich_defconfirm_breaker_test.go 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..26c912fc --- /dev/null +++ b/internal/semantic/lsp/enrich_defconfirm_breaker_test.go @@ -0,0 +1,104 @@ +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") +} diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 63cf4ede..3e5c11e0 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -517,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 } @@ -532,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 } @@ -1287,7 +1297,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre if toNode == nil { continue } - cand, ok := p.definitionNodeAtSite(view, repoPrefix, absRoot, grp.rel, t.edge.Line, toNode.Name, content, cache) + 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 — From 6d6e74bd4a8a9a9d4acf7cfd418af222bf2540e3 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 12:03:48 +0200 Subject: [PATCH 12/18] =?UTF-8?q?lsp:=20review=20round=20=E2=80=94=20yield?= =?UTF-8?q?,=20on-demand=20gate,=20fallback=20recall,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from a fresh-eye review of the branch: - The definition pass fed no usefulYield, so the productivity checkpoint read a warm-graph no-heavy pass (interface adds ~0, def pass the only confirm source until hover) as zero-yield and cancelled it at the first tick after request volume flowed. Feed it from all three productive arms; pinned by a checkpoint-survival test that reproduces the cut. - ConfirmSymbolRefs still issued callHierarchy/incomingCalls for NoHeavyRequests servers — the on-demand find_usages path would accumulate the FindReferences leak one query at a time in a long-lived daemon. Gated. - The no-heavy fallback was built from confirmGroups, inheriting referent-side drops (no position, unserved referent file) that exist only because findReferences opens the referent's file. Build it from the raw sited-target list; a misbound edge with an unusable stored target is exactly what the rebind arm is for. - docs/lsp.md overstated the config knob's reach (router-spawned providers only). --- docs/lsp.md | 8 +- .../lsp/enrich_defconfirm_breaker_test.go | 79 +++++++++++++++++++ .../semantic/lsp/enrich_defconfirm_test.go | 51 ++++++++++++ .../semantic/lsp/enrich_phase_timing_test.go | 13 +-- internal/semantic/lsp/provider.go | 33 ++++++-- 5 files changed, 168 insertions(+), 16 deletions(-) diff --git a/docs/lsp.md b/docs/lsp.md index a2f6ac17..1b06c46a 100644 --- a/docs/lsp.md +++ b/docs/lsp.md @@ -482,7 +482,9 @@ for repositories you trust. 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 spawned server — the durable knob for a machine whose servers - multiplex better than the conservative default assumes. The + 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. Non-positive or unparseable values fall through. + 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/semantic/lsp/enrich_defconfirm_breaker_test.go b/internal/semantic/lsp/enrich_defconfirm_breaker_test.go index 26c912fc..880b88b4 100644 --- a/internal/semantic/lsp/enrich_defconfirm_breaker_test.go +++ b/internal/semantic/lsp/enrich_defconfirm_breaker_test.go @@ -102,3 +102,82 @@ func TestLSP_Enrich_DefinitionPassEmptyAnswersDoNotTrip(t *testing.T) { 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 index 6db07b61..efd6fe04 100644 --- a/internal/semantic/lsp/enrich_defconfirm_test.go +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -382,3 +382,54 @@ func TestLSP_Enrich_DefinitionPassRunsSiteFilesInParallel(t *testing.T) { } 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_phase_timing_test.go b/internal/semantic/lsp/enrich_phase_timing_test.go index a9f57692..7cbc9aa1 100644 --- a/internal/semantic/lsp/enrich_phase_timing_test.go +++ b/internal/semantic/lsp/enrich_phase_timing_test.go @@ -12,12 +12,13 @@ import ( "github.com/zzet/gortex/internal/graph" ) -// The completion log must break the enrich wall time out per pass. The Fleet -// forensics hit a wall without this: 241k requests took the same 26 minutes at -// maxParallel 6 and 12, so the pole is 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. +// 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() diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 3e5c11e0..b80f7617 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -1115,13 +1115,17 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // — 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. - // Targets without a recorded site line are left at their heuristic - // tier: there is no site to ask definition at. - for _, grp := range confirmGroups { - for _, t := range grp.targets { - if t.edge.Line > 0 { - fallback = append(fallback, t) - } + // 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 { @@ -1309,9 +1313,15 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // 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 @@ -1328,6 +1338,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre 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): @@ -1339,6 +1350,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre t.edge.Meta["rebound_from"] = oldTo fallbackMutations.stageReindex(view, t.edge, oldTo) result.EdgesConfirmed++ + usefulYield.Add(1) } rmu.Unlock() } @@ -2933,6 +2945,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 } From 44d1e65f434c1baef37363f7314c3766ee0e88d1 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 22:45:19 +0200 Subject: [PATCH 13/18] lsp: pin the declared-member arm on the heavy default path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The add-not-rebind verdict for dispatched call sites lives in the shared definition-rebind pass, so it changes edge topology for every server, not just the heavy-opt-out ones — but every behavior test for the arm set noHeavyRequests. Cover the default path: references sweep runs and answers empty, definition answers the declared member, and the site must keep its devirtualization guess while gaining the compiler-proven edge (the old behavior rebound the impl edge instead). The dispatched-call fixture moves into a shared helper for both tests. --- .../semantic/lsp/enrich_defconfirm_test.go | 74 ++++++++++++++++--- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/internal/semantic/lsp/enrich_defconfirm_test.go b/internal/semantic/lsp/enrich_defconfirm_test.go index efd6fe04..81004f39 100644 --- a/internal/semantic/lsp/enrich_defconfirm_test.go +++ b/internal/semantic/lsp/enrich_defconfirm_test.go @@ -201,19 +201,18 @@ func TestLSP_Enrich_DefConfirm_CtorHitConfirmsTypeTarget(t *testing.T) { assert.Equal(t, "dom.go::Crate", edge.To, "the edge keeps its type target — no retarget to the ctor") } -// 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() +// 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 := graph.New() 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", @@ -234,7 +233,6 @@ func TestLSP_Enrich_DefConfirm_DispatchedCallAddsDeclaredMemberEdge(t *testing.T } g.AddEdge(implEdge) - server := newFakeLSPServer() server.handle("textDocument/definition", func(json.RawMessage) (any, *jsonRPCError) { // The compiler's answer: the interface's declared member (0-based 2). return []Location{{ @@ -243,6 +241,19 @@ func TestLSP_Enrich_DefConfirm_DispatchedCallAddsDeclaredMemberEdge(t *testing.T }}, 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() @@ -267,6 +278,51 @@ func TestLSP_Enrich_DefConfirm_DispatchedCallAddsDeclaredMemberEdge(t *testing.T 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 From 3c3ef0e3baac50f3e3df1851b7de1922420470fe Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 22:53:55 +0200 Subject: [PATCH 14/18] docs: cover the heavy-request opt-out and GORTEX_LSP_HEAVY The mode that changes what C# enrichment produces was the one knob with no docs presence. Add a section to the enrichment cost model beside its open-docs sibling: what NoHeavyRequests skips (references confirm, incomingCalls, references-add, on-demand find_usages/get_callers confirmation), what replaces it (definition-at-call-site confirms, the dispatch synthesizer), the GORTEX_LSP_HEAVY override and why it is env-only, and the warning that heavy=on is only safe on a csharp-ls build carrying the upstream FindReferences leak fix. Also bring the definition-rebind phase description up to date with the declared-member verdict: add-not-rebind for devirtualization guesses. --- docs/lsp.md | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/docs/lsp.md b/docs/lsp.md index 74c4346f..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 @@ -256,6 +262,48 @@ 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**: From fd681fe561346a647caa94fca2cbdc097cba6baf Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 22:54:39 +0200 Subject: [PATCH 15/18] lsp: fix a comment describing the definition pass as serial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definition-rebind fallback fans out across maxParallel grouped by call-site file — the serial loop the comment described is what this branch replaced. --- internal/semantic/lsp/provider.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 948cfbdd..4b4d102f 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -1103,9 +1103,10 @@ 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. + // 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. confirmGroups := p.groupConfirmTargets(view.nodesByID, targets, degradedSkipFile) var confirmMu sync.Mutex confirmPromotions := make(map[*graph.Edge]struct{}) From 9b185c2bd639735da0c34a6dcd064b54ee28b391 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 22:59:56 +0200 Subject: [PATCH 16/18] =?UTF-8?q?lsp:=20review=20polish=20=E2=80=94=20grou?= =?UTF-8?q?p=20only=20on=20the=20sweep=20path,=20rename=20shared=20normali?= =?UTF-8?q?zer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit confirmGroups was computed unconditionally but the noHeavy path never reads it (that path deliberately builds from the raw target list); move the grouping into the sweep branch. And normalizeOpenDocs stopped being open-docs-specific the moment the heavy override borrowed it — rename to normalizeOnOff for the shared on/off vocabulary. --- internal/semantic/lsp/provider.go | 2 +- internal/semantic/lsp/sweep.go | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/semantic/lsp/provider.go b/internal/semantic/lsp/provider.go index 4b4d102f..bccc77ed 100644 --- a/internal/semantic/lsp/provider.go +++ b/internal/semantic/lsp/provider.go @@ -1107,7 +1107,6 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre // 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. - confirmGroups := p.groupConfirmTargets(view.nodesByID, targets, degradedSkipFile) var confirmMu sync.Mutex confirmPromotions := make(map[*graph.Edge]struct{}) var fallback []enrichTarget @@ -1130,6 +1129,7 @@ func (p *Provider) EnrichRepoContext(ctx context.Context, g graph.Store, repoPre } } } else { + confirmGroups := p.groupConfirmTargets(view.nodesByID, targets, degradedSkipFile) sem := make(chan struct{}, p.maxParallel) var wg sync.WaitGroup for _, grp := range confirmGroups { diff --git a/internal/semantic/lsp/sweep.go b/internal/semantic/lsp/sweep.go index e607c7dd..6c079b4e 100644 --- a/internal/semantic/lsp/sweep.go +++ b/internal/semantic/lsp/sweep.go @@ -117,10 +117,11 @@ func sweepFile(mode string, demand int, dispatch bool) bool { // server. Empty falls through to the spec's NoDidOpen. const OpenDocsEnv = "GORTEX_LSP_OPEN_DOCS" -// normalizeOpenDocs canonicalises an open-docs override to "on" / "off". -// An empty or unrecognised value returns "" so the caller falls through to -// the next precedence source. -func normalizeOpenDocs(v string) string { +// 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" @@ -138,10 +139,10 @@ func normalizeOpenDocs(v string) string { // 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 := normalizeOpenDocs(os.Getenv(OpenDocsEnv)); env != "" { + if env := normalizeOnOff(os.Getenv(OpenDocsEnv)); env != "" { return env == "on" } - if cfg := normalizeOpenDocs(configured); cfg != "" { + if cfg := normalizeOnOff(configured); cfg != "" { return cfg == "on" } if spec != nil && spec.NoDidOpen { @@ -164,7 +165,7 @@ const HeavyRequestsEnv = "GORTEX_LSP_HEAVY" // 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 := normalizeOpenDocs(os.Getenv(HeavyRequestsEnv)); env != "" { + if env := normalizeOnOff(os.Getenv(HeavyRequestsEnv)); env != "" { return env == "off" } return spec != nil && spec.NoHeavyRequests From 9f8e9e0563030a1639810dc4b9eb4ac617a0e148 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:08:54 +0200 Subject: [PATCH 17/18] lsp: count content-cache evictions under the lifecycle opt-out too A sendOpens=false session still evicts to keep the cache bounded, but the counter sat inside the lifecycle gate, so doc_evictions read zero exactly where cache churn is the only signal left. Count the eviction where it happens; didOpens / peak stay honestly zero for a lifecycle-off pass. --- internal/semantic/lsp/doc_session.go | 9 ++++-- internal/semantic/lsp/doc_session_test.go | 36 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/internal/semantic/lsp/doc_session.go b/internal/semantic/lsp/doc_session.go index 82af6d3f..496d709e 100644 --- a/internal/semantic/lsp/doc_session.go +++ b/internal/semantic/lsp/doc_session.go @@ -27,7 +27,9 @@ type docSession struct { // 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 honestly reports zero. + // 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 @@ -122,10 +124,13 @@ func (s *docSession) acquire(c *Client, absPath string) ([]byte, func(), error) evPath := front.Value.(string) cd.lru.Remove(front) delete(cd.open, evPath) + // 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-- - s.evictions++ } } 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) { From 5dccca620b6dd3c920384ff9e550dd60f642d630 Mon Sep 17 00:00:00 2001 From: pbednarcik <29061766+pbednarcik@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:23:44 +0200 Subject: [PATCH 18/18] chore: retrigger CI (lint runner failed downloading its config schema)