perf(search): seek the name index for exact-name lookups

CHANGELOG.md

@@ -21,6 +21,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Looking a symbol up by name no longer reads the whole graph. Every search made one full pass over all indexed symbols for each word you typed, and a question that named several symbols made two more passes per name — including for a word that matches nothing, which is the common case. The cost therefore grew with the size of the project, and it was paid again on every message when the prompt hook is enabled. These lookups now go through the name index instead. Results are identical; only the time to get them changes, and it no longer grows with the project.
+
 - C, C++, Objective-C and Rust unions are now indexed as first-class `union` nodes. A `union` declaration previously produced no symbol at all, so it never appeared in search or `codegraph_explore`, and anything attached to it disappeared with it — in Rust, every `impl SomeTrait for MyUnion` lost its edge, the methods from that impl were left pointing at a type the graph did not contain, and asking which types implement a trait quietly skipped the union ones. A union-shaped dispatch table in C now resolves its function pointers like a struct-shaped one. A `typedef union { … } Name;` in C keeps the typedef's name and is no longer mistaken for a plain type alias. Thanks @ctype-lab. Re-index after upgrading to pick up unions in existing projects. (#1515)
 
 - A long-lived index no longer drifts away from what a fresh `codegraph index` would produce. When a file gained or lost a symbol, references to that name in files the sync never touched kept pointing at the definition that was correct before the change, and — because nothing distinguished two same-named definitions — the winner could come down to the order files happened to be written, which differs between a full index and a sync. On this project's own repository, replaying 80 commits through `sync` left 5.7% of connections wrong; it is now 1.3%, and the wrong-answers-still-being-asserted half drops by 99.7%. Since call edges are what flow questions follow and what `codegraph_explore` ranks files by, this quietly degraded answers as an index aged, with nothing to indicate it. Syncing is unchanged in speed, and an edit that only changes a function's body does no extra work at all. Set `CODEGRAPH_NO_REBIND=1` to opt out.

__tests__/name-lookup-index.test.ts

@@ -0,0 +1,206 @@
+/**
+ * Exact-name lookups must seek `idx_nodes_lower_name`
+ *
+ * `nodes` carries two name indexes and neither one can serve
+ * `WHERE name = ? COLLATE NOCASE`:
+ *
+ *   - `idx_nodes_name` is BINARY-collated, so NOCASE equality can't use it;
+ *   - `idx_nodes_lower_name` is an expression index on `lower(name)`, and the
+ *     planner only matches it against the same expression.
+ *
+ * So every exact-name lookup written that way degrades to a full table scan.
+ * The `LIMIT`s on those queries do not save them: SQLite can only stop early
+ * once it has produced `LIMIT` rows, and the common cases — a query term that
+ * is not a symbol at all, or a name with only a handful of definitions — never
+ * reach it and scan the whole table.
+ *
+ * These tests read the planner's own verdict rather than a wall-clock number,
+ * so they are deterministic and fail loudly if a lookup regresses to a scan.
+ * `lower(name) = lower(?)` (not a JS-side `.toLowerCase()`) is the required
+ * form — see the folding-parity test at the bottom for why.
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import * as fs from 'fs';
+import * as path from 'path';
+import * as os from 'os';
+import { DatabaseConnection } from '../src/db';
+import { QueryBuilder } from '../src/db/queries';
+import { SqliteDatabase } from '../src/db/sqlite-adapter';
+import { Node } from '../src/types';
+
+function makeNode(id: string, name: string, filePath = 'src/a.ts'): Node {
+  return {
+    id,
+    kind: 'function',
+    name,
+    qualifiedName: name,
+    filePath,
+    language: 'typescript',
+    startLine: 1,
+    endLine: 2,
+    startColumn: 0,
+    endColumn: 0,
+    updatedAt: Date.now(),
+  };
+}
+
+/** Wraps a db so every `prepare()` is recorded, then delegates unchanged. */
+function recordingDb(raw: SqliteDatabase): { db: SqliteDatabase; sqls: string[] } {
+  const sqls: string[] = [];
+  const db: SqliteDatabase = {
+    prepare(sql: string) {
+      sqls.push(sql);
+      return raw.prepare(sql);
+    },
+    exec: (sql: string) => raw.exec(sql),
+    pragma: (str: string, options?: { simple?: boolean }) => raw.pragma(str, options),
+    transaction: <T>(fn: (...args: any[]) => T) => raw.transaction(fn),
+    close: () => raw.close(),
+    get open() {
+      return raw.open;
+    },
+  };
+  return { db, sqls };
+}
+
+/** SQL that filters `nodes` on whole-name equality, in either spelling. */
+function exactNameLookups(sqls: string[]): string[] {
+  return sqls.filter(
+    (s) =>
+      /\bFROM\s+nodes\b/i.test(s) &&
+      (/\bname\s*(COLLATE\s+NOCASE\s*)?=\s*\?(\s*COLLATE\s+NOCASE)?/i.test(s) ||
+        /\blower\(name\)\s*=/i.test(s))
+  );
+}
+
+/** The planner's access path for the `nodes` table in a statement. */
+function nodesAccessPath(raw: SqliteDatabase, sql: string): string {
+  const args = new Array((sql.match(/\?/g) ?? []).length).fill('x');
+  const rows = raw.prepare(`EXPLAIN QUERY PLAN ${sql}`).all(...args) as { detail: string }[];
+  const detail = rows.map((r) => r.detail).find((d) => /\bnodes\b/.test(d));
+  return detail ?? rows.map((r) => r.detail).join(' | ');
+}
+
+describe('exact-name lookups seek idx_nodes_lower_name', () => {
+  let dir: string;
+  let conn: DatabaseConnection;
+  let raw: SqliteDatabase;
+
+  beforeAll(() => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'name-lookup-index-'));
+    conn = DatabaseConnection.initialize(path.join(dir, 'test.db'));
+    raw = conn.getDb();
+    const seed = new QueryBuilder(raw);
+
+    // A corpus wide enough that a scan and a seek can't accidentally agree on
+    // ordering, with `handleRequest` deliberately rare (2 nodes) — the shape
+    // the LIMITs never short-circuit on.
+    const nodes: Node[] = [];
+    for (let i = 0; i < 300; i++) {
+      nodes.push(makeNode(`filler-${i}`, `filler${i}Symbol`, `src/pkg${i % 7}/f${i}.ts`));
+    }
+    nodes.push(makeNode('hr-1', 'handleRequest', 'src/server/router.ts'));
+    nodes.push(makeNode('hr-2', 'HandleRequest', 'src/server/legacy.ts'));
+    for (const n of nodes) seed.insertNode(n);
+  });
+
+  afterAll(() => {
+    conn.close();
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('searchNodes issues its exact-name supplement as an index seek', () => {
+    const { db, sqls } = recordingDb(raw);
+    const q = new QueryBuilder(db);
+
+    const results = q.searchNodes('handleRequest');
+    expect(results.length).toBeGreaterThan(0);
+
+    const lookups = exactNameLookups(sqls);
+    // Guard against a vacuous pass: the supplement must actually have run.
+    expect(lookups.length).toBeGreaterThan(0);
+
+    for (const sql of lookups) {
+      expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
+    }
+  });
+
+  it('findNodesByExactName issues both of its passes as index seeks', () => {
+    const { db, sqls } = recordingDb(raw);
+    const q = new QueryBuilder(db);
+
+    const results = q.findNodesByExactName(['handleRequest']);
+    expect(results.length).toBeGreaterThan(0);
+
+    const lookups = exactNameLookups(sqls);
+    // Two passes: the file_path probe and the row fetch.
+    expect(lookups.length).toBeGreaterThanOrEqual(2);
+
+    for (const sql of lookups) {
+      expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
+    }
+  });
+
+  it('getNodesByLowerName seeks the index and does not depend on the caller lowering', () => {
+    const { db, sqls } = recordingDb(raw);
+    const q = new QueryBuilder(db);
+
+    // Previously this took an already-lowered string on trust: anything with an
+    // uppercase letter in it silently returned nothing.
+    expect(q.getNodesByLowerName('handlerequest').map((n) => n.id).sort()).toEqual([
+      'hr-1',
+      'hr-2',
+    ]);
+    expect(q.getNodesByLowerName('HandleRequest').map((n) => n.id).sort()).toEqual([
+      'hr-1',
+      'hr-2',
+    ]);
+    expect(q.getNodesByLowerName('HANDLEREQUEST').map((n) => n.id).sort()).toEqual([
+      'hr-1',
+      'hr-2',
+    ]);
+
+    const lookups = exactNameLookups(sqls);
+    expect(lookups.length).toBeGreaterThan(0);
+    for (const sql of lookups) {
+      expect(nodesAccessPath(raw, sql)).toMatch(/SEARCH nodes USING .*idx_nodes_lower_name/);
+    }
+  });
+
+  it('still matches case-insensitively across both call sites', () => {
+    const q = new QueryBuilder(raw);
+
+    const exact = q.findNodesByExactName(['HANDLEREQUEST']);
+    expect(exact.map((r) => r.node.id).sort()).toEqual(['hr-1', 'hr-2']);
+
+    const searched = q.searchNodes('HandleRequest');
+    const ids = new Set(searched.map((r) => r.node.id));
+    expect(ids.has('hr-1')).toBe(true);
+    expect(ids.has('hr-2')).toBe(true);
+  });
+
+  it('folds exactly what COLLATE NOCASE folded — ASCII only', () => {
+    // SQLite's NOCASE and its `lower()` are both ASCII-only. JavaScript's
+    // `.toLowerCase()` is not, so lowering the parameter in JS and comparing
+    // against `lower(name)` would silently stop matching non-ASCII names that
+    // NOCASE used to match. `lower(?)` keeps both sides on SQLite's rules.
+    const probe = new QueryBuilder(raw);
+    probe.insertNode(makeNode('uni-1', 'Ünïcode', 'src/i18n/a.ts'));
+
+    const found = probe.findNodesByExactName(['Ünïcode']);
+    expect(found.map((r) => r.node.id)).toContain('uni-1');
+
+    // The mixed-ASCII half still folds, as NOCASE did.
+    probe.insertNode(makeNode('uni-2', 'Ünïcodeloader', 'src/i18n/b.ts'));
+    const folded = probe.findNodesByExactName(['ÜnïcodeLOADER']);
+    expect(folded.map((r) => r.node.id)).toContain('uni-2');
+
+    // Same rule for the fuzzy-match lookup. Note what this does NOT claim: a
+    // caller that lowers in JavaScript first still hands over `ünïcode`, which
+    // is not what SQLite's `lower()` makes of `Ünïcode`, so the gap stays open
+    // on that side.
+    expect(probe.getNodesByLowerName('Ünïcode').map((n) => n.id)).toContain('uni-1');
+    expect(probe.getNodesByLowerName('ÜnïcodeLOADER').map((n) => n.id)).toContain('uni-2');
+  });
+});

src/db/queries.ts

@@ -1168,15 +1168,26 @@ export class QueryBuilder {
   }
 
   /**
-   * Get nodes by lowercase name match (uses idx_nodes_lower_name expression index)
+   * Get nodes by name, case-insensitively (seeks the idx_nodes_lower_name
+   * expression index).
+   *
+   * The parameter is lowered in SQL rather than trusted to arrive lowered, so
+   * the lookup means the same thing whatever casing a caller hands it. Written
+   * as a bare `lower(name) = ?` it silently returned nothing for any input
+   * carrying an uppercase letter, and — because SQLite's `lower()` folds ASCII
+   * only while JavaScript's `.toLowerCase()` folds Unicode — a caller that
+   * pre-lowered in JavaScript could not match a non-ASCII name at all.
+   *
+   * Note this hardens the query, not its one caller: `matchFuzzy` still lowers
+   * in JavaScript before calling, so the non-ASCII gap remains open there.
    */
-  getNodesByLowerName(lowerName: string): Node[] {
+  getNodesByLowerName(name: string): Node[] {
     if (!this.stmts.getNodesByLowerName) {
       this.stmts.getNodesByLowerName = this.db.prepare(
-        'SELECT * FROM nodes WHERE lower(name) = ?'
+        'SELECT * FROM nodes WHERE lower(name) = lower(?)'
       );
     }
-    const rows = this.stmts.getNodesByLowerName.all(lowerName) as NodeRow[];
+    const rows = this.stmts.getNodesByLowerName.all(name) as NodeRow[];
     return rows.map(rowToNode);
   }
 
@@ -1242,12 +1253,25 @@ export class QueryBuilder {
     // pushing them past the FTS fetch limit before post-hoc scoring can help.
     // Use the max BM25 score as the base so the nameMatchBonus (exact=30 vs
     // prefix=20) actually differentiates them after rescoring.
+    //
+    // Whole-name equality MUST be written as `lower(name) = lower(?)` so it
+    // seeks `idx_nodes_lower_name`. The equivalent `name = ? COLLATE NOCASE`
+    // matches no index — `idx_nodes_name` is BINARY-collated and the expression
+    // index only matches the same expression — and degrades to a full table
+    // scan. The `LIMIT 20` does not rescue it: SQLite can only stop early once
+    // it has produced 20 rows, and this runs once per query term, most of which
+    // name nothing in the corpus. Measured per term on an unmatched term:
+    // 0.08ms on gin (2.5k nodes), 0.39ms on excalidraw (11k), 2.4ms on django
+    // (62k) — and growing with the corpus, where the seek is flat at ~0.002ms.
+    // Lowering the parameter in SQL rather than in JS is deliberate: SQLite's
+    // `lower()` and NOCASE both fold ASCII only, while JS `.toLowerCase()`
+    // folds Unicode, which would silently stop matching non-ASCII names.
     if (results.length > 0 && query) {
       const existingIds = new Set(results.map(r => r.node.id));
       const maxFtsScore = Math.max(...results.map(r => r.score));
       const terms = query.split(/\s+/).filter(t => t.length >= 2);
       for (const term of terms) {
-        let sql = 'SELECT * FROM nodes WHERE name = ? COLLATE NOCASE';
+        let sql = 'SELECT * FROM nodes WHERE lower(name) = lower(?)';
         const params: (string | number)[] = [term];
         if (kinds && kinds.length > 0) {
           sql += ` AND kind IN (${kinds.map(() => '?').join(',')})`;
@@ -1547,9 +1571,16 @@ export class QueryBuilder {
     // Pass 2: Query each name, boosting results that co-locate with distinctive symbols.
 
     // Pass 1: Find files containing each queried name, identify distinctive names
+    //
+    // Both passes spell whole-name equality as `lower(name) = lower(?)` so they
+    // seek `idx_nodes_lower_name` — see the note in `searchNodes` for why the
+    // `name = ? COLLATE NOCASE` form full-scans instead. This path is the one
+    // that hurts most: it runs both passes for every symbol extracted from the
+    // query, and extraction is generous, so most of those names are absent from
+    // the corpus and never reach either LIMIT.
     const nameToFiles = new Map<string, Set<string>>();
     for (const name of names) {
-      let sql = 'SELECT DISTINCT file_path FROM nodes WHERE name COLLATE NOCASE = ?';
+      let sql = 'SELECT DISTINCT file_path FROM nodes WHERE lower(name) = lower(?)';
       const params: (string | number)[] = [name];
       if (kinds && kinds.length > 0) {
         sql += ` AND kind IN (${kinds.map(() => '?').join(',')})`;
@@ -1577,7 +1608,7 @@ export class QueryBuilder {
       let sql = `
         SELECT nodes.*, 1.0 as score
         FROM nodes
-        WHERE name COLLATE NOCASE = ?
+        WHERE lower(name) = lower(?)
       `;
       const params: (string | number)[] = [name];