Harden and speed up the LSP; add workspace-wide rename

Reliability
- Isolate diagnostics: a checker exception clears that file's diagnostics
  and self-heals instead of downing the connection.
- Startup indexing: per-file guard + finally so one unreadable file (or an
  early throw) cannot hang the client forever.
- Fix false "multi-line statement" errors from parens inside string literals.
- Fill findReferences generic-arg gaps for global functions/vars.

Performance (all version-gated, invalidated on index change)
- Memoize getClassHierarchyOrdered (the hottest shared primitive).
- O(1) findFunctionOverloads via a lazy method index.
- Access-modifier checks skip per-token resolution unless the member is
  actually private/protected.
- Precompute a block-comment mask, removing the O(n^2) backward scan.
- Reverse index (symbolsByUri) makes per-keystroke index removal O(edited
  file) instead of O(whole workspace).
- Drop redundant docCache scans covered by globalSymbolIndex; memoize the
  comment-stripped text.

Feature: workspace-wide, identity-checked rename
- renameSymbol lexes each workspace file and resolves every occurrence,
  rewriting call sites / member accesses / type refs (not just decls) while
  never touching a same-named symbol on an unrelated class.
- Guards: skip files whose cached text is out of sync with their AST, skip
  occurrences inside comments/strings, and only rename symbols defined in
  workspace (not read-only include-path) files.

Simplification
- Remove dead modules/handlers (printer, scopes, types stubs, documents,
  includePaths), unused RuleContext plumbing, and duplicated helpers;
  extract shared diagnostics dispatch into revalidate.ts.

Tests: 306 passing (jest); typecheck clean. handoffs/ documents the deferred
larger refactors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: DaemonF0rge

handoffs/01-callsite-parser-ast.md

@@ -0,0 +1,90 @@
+# Hand-off 1 — Emit `CallSite` nodes from the parser; retire the regex re-tokenizers
+
+**Effort:** L **Risk:** medium **Prerequisite for:** hand-off 2, phase B
+
+## Problem
+
+The parser (`server/src/analysis/ast/parser.ts`) stops at declaration level. Per
+function body it emits only:
+- `bodyTypeRefs: TypeNode[]` — static-call targets (`ClassName.` where ClassName
+  is PascalCase), **deduped per body** (first occurrence only).
+- `bodyIdentifierRefs: BodyIdentifierRef[]` — standalone identifiers **not**
+  preceded by `.` and **not** followed by `(`, also **deduped per body**.
+
+Neither captures call sites, member accesses, or every occurrence. So the
+diagnostics checkers re-derive call structure from **raw text with regex** on the
+per-keystroke path:
+
+- `checkFunctionCallArgs` (`graph.ts:6365`, ~350 lines) — regex-matches calls,
+  then uses brittle heuristics to decide decl-vs-call (`/(?:void|int|float…)\s+$/`
+  look-behind, backward `[` walks, `endsWith('new')`). These guards are the top
+  false-positive source.
+- `checkTypeMismatches` (`graph.ts:5378`, ~200 lines) — same shape.
+- `parseCallArguments` (`graph.ts:5954`) — manually splits argument text on
+  top-level commas (handles nested parens/brackets/strings/templates by hand).
+
+Cost: O(fileLength) regex passes on every keystroke, plus a maintenance maze.
+
+## Goal
+
+Have the parser emit a minimal, **complete** (non-deduped) `CallSite[]` for each
+function body, and migrate the two checkers to consume it — deleting the
+decl-vs-call heuristics and `parseCallArguments`.
+
+```ts
+interface CallSite {
+  calleeName: string;
+  calleeStart: Position;      // start of the callee identifier
+  calleeEnd: Position;
+  argRanges: { start: number; end: number }[]; // char offsets of each top-level arg
+  isDeclaration: boolean;     // true for `Type name(...)`-style decls the parser already knows aren't calls
+}
+```
+
+Add `callSites: CallSite[]` to `FunctionDeclNode` (parser.ts interface ~173).
+
+## Where to emit it
+
+The body loop already tracks `parenDepth` (parser.ts:702) and walks tokens with
+`prevPrev`/`prev`/`t`. When `prev` is an identifier and `t.value === '('` at the
+statement level, record a `CallSite`: `calleeName = prev.value`, positions from
+`prev.start/end`, and `argRanges` by scanning to the matching `)` and splitting on
+commas at the call's paren depth. `isDeclaration` is derivable from the same
+context the parser already has (a preceding type token / modifier run).
+
+Keep the emission O(n) — it must not regress the hot parse path.
+
+## Plan (incremental — one checker at a time)
+
+1. Add the `CallSite` interface + `callSites` field + emission in the body loop.
+   Add **parser unit tests** for tricky inputs: nested calls `f(g(x), y)`, string
+   args with commas `f("a,b", c)`, `new X()`, array args `f({1,2})`, chained
+   `a.b().c()`. Assert `calleeName`, `argRanges`, `isDeclaration`.
+2. Migrate `checkFunctionCallArgs` to iterate `func.callSites` instead of its
+   regex call-detection. **Diff diagnostics output on a corpus** — ranges must be
+   byte-for-byte identical (the 303 tests assert exact ranges). Commit only when
+   the diff is empty (or the deltas are verified improvements).
+3. Migrate `checkTypeMismatches` the same way.
+4. Delete `parseCallArguments` once no caller remains.
+
+## Do NOT touch
+
+`parseExpressionChainBackward`, `parseChainMembers`, `resolveChainReturnType`
+(the completion/hover path). They run on **incomplete cursor text** where the
+parser has no `CallSite`. Retiring those is hand-off 2, phase B.
+
+## Risks
+
+- **Exact ranges.** The parser-derived offsets must reproduce current highlight
+  ranges exactly. Golden-diff before every commit.
+- **Hot path.** Emission runs on every keystroke parse — keep it O(n), no
+  backtracking.
+- **`isDeclaration` correctness.** Getting this wrong flips real diagnostics;
+  cover it heavily in the parser unit tests.
+
+## Done when
+
+- `checkFunctionCallArgs` and `checkTypeMismatches` no longer regex-scan for calls.
+- `parseCallArguments` is deleted.
+- Parser emits `callSites`; unit tests cover the tricky cases.
+- Diagnostics are byte-identical on the corpus; `npm test` green.

handoffs/02-chain-resolver-unification.md

@@ -0,0 +1,69 @@
+# Hand-off 2 — Unify the two chain resolvers
+
+**Effort:** M (phase A) / L (phase B) **Risk:** medium **Phase B depends on:** hand-off 1
+
+## Problem
+
+Expression-chain resolution is implemented twice:
+
+- **Completion/hover path** (works on incomplete cursor text): two steppers,
+  `resolveChainSteps` (`graph.ts:2172`) and `resolveChainStepsWithIndexing`
+  (`graph.ts:2259`). The former is a **strict subset** of the latter — same walk,
+  minus array-indexing support.
+- **Diagnostics path** (works on full expression text): `resolveChainReturnType`
+  (`graph.ts:2320`), `resolveVariableChainType` (`graph.ts:2425`), plus the text
+  helpers `parseChainMembers` (`graph.ts:2093`) and `countIndexingLevels`
+  (`graph.ts:1581`).
+
+## Phase A — collapse the two steppers (contained, do this first)
+
+`resolveChainSteps` ⊂ `resolveChainStepsWithIndexing`. Delete the former; route
+its callers through the latter by mapping their string arrays to segments:
+`calls.map(name => ({ name, isIndexed: false }))`.
+
+**The one behavioral divergence to reconcile:** on `Cast` → `Class`,
+`resolveChainSteps` preserves `templateMap` (via a `continue`) while
+`resolveChainStepsWithIndexing` resets it. Pick the **preserve** behavior as
+canonical, but implement it **without a literal `continue`** — a bare `continue`
+in the indexing stepper would skip the `isIndexed` dereference and regress
+`.Cast()[0]`. Guard the typedef/genericArgs rebuild block so `Cast` skips it while
+still falling through to the `isIndexed` deref.
+
+- Callers of `resolveChainSteps` to update: `graph.ts:2221`, `2264`, `2278`,
+  `5969` (verify with a fresh grep — line numbers drift).
+- Do **not** fold `resolveMethodCallWithTemplates` in blindly: its extends-clause
+  generic-arg handling (its `~1250-1257` block) is not present in the stepper and
+  would need explicit preservation + a generic-inheritance test.
+
+**Test before touching:** add completion + type-mismatch tests exercising a
+`Cast()` chain and an indexed chain (`x.Cast()[0]`, `arr.Get(i).Field`). Then run
+all 303. Net ≈ 45 lines deleted, one drift point removed.
+
+## Phase B — retire the diagnostics-side text resolvers (gated on hand-off 1)
+
+Once the parser emits expression structure (`CallSite`, and ideally chain
+segments) from hand-off 1, the diagnostics callers can stop text-parsing.
+
+Why it's gated: those callers resolve the chain **root** via `getVarTypeAtLine`
+(line-scoped locals) rather than `resolveVariableType(doc, pos)`, and they consume
+full, non-dot-terminated expression text. Unifying requires a forward segment
+parser that reproduces the `isCall` / `isIndexed` / indexing-level semantics that
+user-visible type-mismatch / return-type / cast diagnostics depend on **exactly**.
+
+Do it **one caller at a time**, each guarded by golden-diffing diagnostics, only
+after hand-off 1 lands. Retire `parseChainMembers` and `countIndexingLevels` when
+their last caller is migrated.
+
+## Risks
+
+- Phase A: the `Cast` reconciliation is subtle — wrong and you regress
+  `.Cast()[0]`. The added completion/type-mismatch tests are the guard.
+- Phase B: the diagnostics resolvers back user-visible diagnostics with exact
+  ranges; reproduce their semantics precisely, incrementally, with diffing.
+
+## Done when
+
+- Phase A: `resolveChainSteps` deleted, one canonical stepper, Cast + indexing
+  chains verified, 303 green.
+- Phase B: `resolveChainReturnType` / `resolveVariableChainType` /
+  `parseChainMembers` / `countIndexingLevels` gone, diagnostics byte-identical.

handoffs/03-globalsymbolindex-targeted-removal.md

@@ -0,0 +1,78 @@
+# Hand-off 3 — Targeted removal for `globalSymbolIndex` (the second per-keystroke full scan)
+
+**Effort:** S–M **Risk:** low–medium **Start here** — cheapest, reuses a landed pattern.
+
+## Problem
+
+The review's reverse-index work made `removeIndexEntriesForUri` (`graph.ts`)
+touch only the edited file's symbols via a per-URI record, `symbolsByUri` —
+O(edited file) instead of O(all workspace symbols) per keystroke.
+
+But its sibling, `updateGlobalSymbolIndex` (`graph.ts:626`), still does the old
+**full scan** of `globalSymbolIndex` on every re-parse to drop the URI's old
+entries:
+
+```ts
+for (const [name, entry] of this.globalSymbolIndex) {
+    if (entry.uri === uri) this.globalSymbolIndex.delete(name);
+}
+```
+
+That's the same O(all-symbols)-per-keystroke cost, left unoptimized only because
+`globalSymbolIndex` also holds `VarDecl` (global variable) names, which
+`symbolsByUri` does not currently record (`updateAllIndexes` handles ClassDecl /
+EnumDecl / FunctionDecl / Typedef, not VarDecl — vars live only in
+`globalSymbolIndex`).
+
+> Note: this is the genuinely-valuable half of the deferred "merge
+> `updateGlobalSymbolIndex` + `updateAllIndexes`" item. A full method merge is
+> **not** needed and adds little; the win is the targeted removal below.
+
+## Goal
+
+Make `updateGlobalSymbolIndex`'s removal targeted via the reverse index,
+eliminating the second per-keystroke full scan.
+
+## Plan
+
+1. **Record vars in the reverse index.** Extend the `symbolsByUri` value type with
+   `vars: string[]`. Add a `VarDecl` branch to `updateAllIndexes` that pushes the
+   var name into `contributed.vars` (vars don't go into the five name-keyed maps —
+   this record is purely so `globalSymbolIndex` removal can be targeted).
+2. **Targeted removal in `updateGlobalSymbolIndex`.** Replace the full-scan loop
+   with: read `symbolsByUri.get(uri)` and, for each recorded name across
+   `classes ∪ enums ∪ functions ∪ typedefs ∪ vars`, delete it from
+   `globalSymbolIndex` **only if** `globalSymbolIndex.get(name)?.uri === uri`
+   (single-entry, last-writer-wins semantics — same as the enum/typedef handling
+   in the landed `removeIndexEntriesForUri`). Fall back to the existing full scan
+   when no record exists (first index of the URI / defensive).
+
+### Ordering (verify, it's the crux)
+
+`ensure()` calls `updateGlobalSymbolIndex(uri, ast)` **then**
+`updateAllIndexes(uri, ast)`. Inside `updateAllIndexes`, `removeIndexEntriesForUri`
+deletes `symbolsByUri[uri]` and then `updateAllIndexes` re-sets it with the new
+record. So when `updateGlobalSymbolIndex` runs (first), `symbolsByUri[uri]` still
+holds the **previous** run's complete record (including vars) — exactly what the
+removal needs. Confirm this ordering hasn't changed before relying on it.
+
+## Risks
+
+`globalSymbolIndex` is load-bearing: completions, `getWorkspaceSymbols`, and —
+since the review — `checkUnknownSymbols` / `typeExists` all rely on it. A stale
+entry surfaces as a **phantom symbol**. The landed reverse-index test only checks
+`classIndex`, so it would not catch a `globalSymbolIndex` leak.
+
+## Test strategy (required)
+
+Extend the `index maintenance (reverse-index cleanup)` describe in
+`test/features.test.ts`: after re-indexing a file that **drops a global function
+and a global var**, assert they no longer appear via a public path —
+`analyzer.getWorkspaceSymbols('<name>')` returns empty (and/or a completion query
+no longer offers them). Keep the existing class assertions.
+
+## Done when
+
+- `updateGlobalSymbolIndex` no longer full-scans on the per-keystroke path.
+- `symbolsByUri` records vars; ordering invariant confirmed.
+- New staleness test (global function + var removal) passes; all tests green.

handoffs/README.md

@@ -0,0 +1,40 @@
+# LSP refactor hand-offs
+
+Deferred follow-ups from the LSP review. Each file is a self-contained spec:
+context, goal, exact locations, an incremental plan, risks, a test strategy, and
+"done" criteria. They are independent — pick up any one on its own.
+
+Baseline when these were written: `server/` typechecks clean, `npm test` = 303
+tests passing. The diagnostics checkers assert **exact** diagnostic ranges, so
+any change touching offsets must be validated by diffing diagnostics output, not
+just by the suite passing.
+
+| # | Hand-off | Effort | Risk | Why it's worth doing |
+|---|----------|--------|------|----------------------|
+| 1 | [CallSite parser AST](01-callsite-parser-ast.md) | L | med | Deletes ~550 lines of per-keystroke regex re-tokenizing and its false positives |
+| 2 | [Chain-resolver unification](02-chain-resolver-unification.md) | M (phase A) / L (phase B) | med | Removes two parallel chain resolvers; phase B gated on #1 |
+| 3 | [globalSymbolIndex targeted removal](03-globalsymbolindex-targeted-removal.md) | S–M | low–med | Kills the *second* per-keystroke O(all-symbols) index scan (sibling of one already fixed) |
+
+Start with **#3** — it's the cheapest, lowest-risk, and reuses a pattern already
+landed in the codebase. **#1** unlocks the second phase of **#2**.
+
+## Explicitly NOT recommended
+
+- **Extracting the ~3,900-line diagnostics cluster into a separate "provider"
+  class.** Pure reorganization: it nets *more* code (a ~25-member facade to hand
+  the provider the analyzer's internals), cuts zero CPU, and gains no real
+  testability (the provider still needs a fully-populated analyzer). If the only
+  goal is navigability, split the diagnostics methods into a `graph.diagnostics.ts`
+  partial via TypeScript declaration merging / a mixin — same file-size relief,
+  near-zero risk, no new interface. Don't build the provider.
+
+## Intentionally skipped micro-items (not worth a hand-off)
+
+- **`checkTypeCompatibility` pair-memo** — dominated by the `getClassHierarchyOrdered`
+  memo that already landed; its only unique gain is skipping a free early-exit.
+- **`getHover` double chain-resolve** — hover fires on mouse dwell, not per
+  keystroke; the second resolve is negligible.
+- **`_sourceUri` / `_containerClassName` / `_containerIsModded` typing** — moving
+  these synthetic fields onto `SymbolNodeBase` to drop the `as any` casts is pure
+  churn across ~24 sites with no runtime change. Only worth doing if all three are
+  done together in one pass while touching that code for another reason.

server/src/analysis/ast/parser.ts

@@ -1020,17 +1020,18 @@ export function parse(
         const vars: VarDeclNode[] = [];
         let sawDefault = false;
         while (!eof()) {
-            const typeNode = structuredClone(baseTypeNode);
-
             // Support trailing `T name[]`
             if (peek().value === '[') {
 
                 // Prevent additional [] after identifier if already declared in type
-                if (typeNode.arrayDims.length !== 0) {
+                if (baseTypeNode.arrayDims.length !== 0) {
                     throwErr(peek(), "not another [");
                 }
 
-                parseArrayDims(doc, typeNode);
+                // Consume the trailing '[..]' tokens into a throwaway scratch node.
+                // The parsed dims are unused (each var's type is baseTypeNode below),
+                // so a shallow copy avoids a per-declaration structuredClone.
+                parseArrayDims(doc, { ...baseTypeNode, arrayDims: [] });
             }
 
             // value initialization (skip for now)

server/src/analysis/ast/printer.ts

@@ -1,6 +1,6 @@
 import { Diagnostic, DiagnosticSeverity } from 'vscode-languageserver';
-import { DiagnosticRule, RuleContext } from './rules';
-import { File, ClassDeclNode, FunctionDeclNode, VarDeclNode } from '../ast/parser';
+import { DiagnosticRule } from './rules';
+import { File, ClassDeclNode, FunctionDeclNode } from '../ast/parser';
 
 /**
  * Pluggable Diagnostic Rule Engine
@@ -28,11 +28,11 @@ export class DiagnosticEngine {
     }
 
     /** Run all registered rules against an AST file */
-    run(ast: File, context: RuleContext): Diagnostic[] {
+    run(ast: File): Diagnostic[] {
         const diagnostics: Diagnostic[] = [];
         for (const rule of this.rules) {
             try {
-                const ruleDiags = rule.check(ast, context);
+                const ruleDiags = rule.check(ast);
                 diagnostics.push(...ruleDiags);
             } catch (err) {
                 // Don't let a single rule failure break all diagnostics
@@ -55,7 +55,7 @@ class ConflictingModifiersRule implements DiagnosticRule {
     name = 'Conflicting Modifiers';
     severity = DiagnosticSeverity.Error;
 
-    check(ast: File, _context: RuleContext): Diagnostic[] {
+    check(ast: File): Diagnostic[] {
         const diags: Diagnostic[] = [];
 
         const conflicts: [string, string, string][] = [
@@ -114,19 +114,18 @@ class StrongRefParameterRule implements DiagnosticRule {
 
     private static readonly STRONG_REF_MODIFIERS = ['autoptr', 'ref'];
 
-    check(ast: File, _context: RuleContext): Diagnostic[] {
+    check(ast: File): Diagnostic[] {
         const diags: Diagnostic[] = [];
 
         const checkParams = (func: FunctionDeclNode) => {
             for (const param of func.parameters) {
-                for (const mod of StrongRefParameterRule.STRONG_REF_MODIFIERS) {
-                    if (param.modifiers?.includes(mod)) {
-                        diags.push({
-                            message: `Method argument '${param.name}' can't be a strong reference. Remove '${mod}' from the parameter.`,
-                            range: { start: param.nameStart, end: param.nameEnd },
-                            severity: this.severity
-                        });
-                    }
+                const mod = param.modifiers?.find(m => StrongRefParameterRule.STRONG_REF_MODIFIERS.includes(m));
+                if (mod) {
+                    diags.push({
+                        message: `Method argument '${param.name}' can't be a strong reference. Remove '${mod}' from the parameter.`,
+                        range: { start: param.nameStart, end: param.nameEnd },
+                        severity: this.severity
+                    });
                 }
             }
         };

server/src/analysis/diagnostics/engine.ts

@@ -1,5 +1,5 @@
 import { Diagnostic, DiagnosticSeverity } from 'vscode-languageserver';
-import { ClassDeclNode, File } from '../ast/parser';
+import { File } from '../ast/parser';
 
 /**
  * Base interface for all diagnostic rules.
@@ -13,18 +13,5 @@ export interface DiagnosticRule {
   /** Severity of diagnostics produced by this rule */
   severity: DiagnosticSeverity;
   /** Run the rule against an AST and return diagnostics */
-  check(ast: File, context: RuleContext): Diagnostic[];
-}
-
-/**
- * Context object passed to diagnostic rules, providing
- * access to the analyzer's indexes and resolution methods.
- */
-export interface RuleContext {
-  /** Look up a class by name */
-  findClassByName(name: string): ClassDeclNode | null;
-  /** Get all classes in inheritance hierarchy */
-  getClassHierarchy(className: string): ClassDeclNode[];
-  /** Get the number of indexed files (for threshold checks) */
-  indexedFileCount: number;
+  check(ast: File): Diagnostic[];
 }