perf(context): compute the dominant file once per index state (#1864) (#2086)

* perf(context): compute the dominant file once per index state (#1864) getDominantFile() backs context ranking's core-directory boost. Its answer depends only on the graph, but the whole-graph aggregation behind it (edges joined to both endpoints' nodes, grouped by file) ran on every generic codegraph_explore - seconds per call on a large index. Memoize it in QueryBuilder against a database change stamp: total_changes() (rows this connection wrote) plus PRAGMA data_version (commits by any other connection or process). Both are O(1), so no write path has to remember to invalidate, and a sync made by another process is seen on the next call. Any write forces a recompute; the heuristic's result is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(context): never keep a dominant file read inside a transaction (#1864) total_changes() counts writes that are later rolled back, so a result computed inside a transaction could survive the ROLLBACK under an unchanged stamp and describe edges that no longer exist. getDominantFile now skips the memo while a transaction is open (and on a runtime without isTransaction). Ported from the maintainer's hardening of this fix in #1919. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(db): drop the dominant-file memo when a QueryBuilder is rebound (#1864) The memo's change stamp (total_changes + data_version) is per connection, and fresh read-only connections to two different databases report the same one ("0:2"). A pool worker rebound onto a rebuilt index kept serving the old database's dominant file until something wrote through the new connection. A test rebinds onto a second project and fails without the reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): credit @danusha2345 for #1864 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: danusha2345 <danusha2345@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-18 13:53:01 +08:00 · 2026-09-29 04:37:41 +00:00
parent b44550e328
commit 09b9646b7e
4 changed files with 200 additions and 0 deletions
@@ -174,6 +174,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - On Windows, the shared MCP daemon now waits longer for another program — an antivirus scan, an indexer, or another session reading its lock file — to let go of that file, so it starts instead of leaving the session to fall back to a slower in-process server. (#1773)
 - A file over the size limit is no longer read before it is skipped. Large committed video and blob fixtures used to be loaded into memory in full, once per indexing pass, only to be discarded; indexing, change detection and the viewer now go by the file's size instead. Thanks @danusha2345. (#1910)
 - An MPEG transport stream video that happens to be named `.ts` (golden fixtures under `testdata/`, e2e clips) is now recognised from its first bytes and skipped instead of being fed to the TypeScript parser, which spent a long time on each clip for no symbols. Real TypeScript files are still indexed. Thanks @danusha2345. (#1910)
+- Repeated `codegraph_explore` calls from a running MCP server no longer re-rank the whole graph each time to find the project's core file; the answer is reused until the index changes. Thanks @danusha2345. (#1864)
 - File watching no longer drops the full re-scan a removed directory asks for when that sync fails, so the deleted files leave the index instead of lingering. (#1964)
 - Running `codegraph index` while the MCP server is running no longer leaves live auto-sync writing into the old, replaced index: the server now switches to the rebuilt index before its next update and catches up on everything it would otherwise have missed. Thanks @danusha2345. (#1902)
 - Daemon startup and cleanup now preserve live legacy PID-only locks while still reclaiming dead or identity-disproved records, preventing two writers from serving the same project.
@@ -0,0 +1,140 @@
+/**
+ * getDominantFile() is computed once per database state (#1864).
+ *
+ * The dominant-file heuristic (context ranking's core-directory boost) is a
+ * whole-graph aggregation whose answer does not depend on the query, yet it
+ * ran on every generic explore — seconds per call on a large index. It is now
+ * memoized against a database change stamp. These tests pin both halves:
+ * repeated explores reuse the answer, and any write — by this connection or
+ * by another process's sync — makes the next call see the new graph.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import * as fs from 'fs';
+import * as path from 'path';
+import * as os from 'os';
+import CodeGraph from '../src';
+import { QueryBuilder } from '../src/db/queries';
+import { DatabaseConnection, getDatabasePath } from '../src/db';
+
+/** A file whose functions call each other in a chain: `n` in-file call edges. */
+function chain(prefix: string, n: number): string {
+  const lines: string[] = [];
+  for (let i = 0; i < n; i++) {
+    const next = i + 1 < n ? `${prefix}${i + 1}();` : '';
+    lines.push(`export function ${prefix}${i}(): void { ${next} }`);
+  }
+  return lines.join('\n') + '\n';
+}
+
+function queriesOf(cg: CodeGraph): QueryBuilder {
+  return (cg as unknown as { queries: QueryBuilder }).queries;
+}
+
+function spyCompute(cg: CodeGraph) {
+  return vi.spyOn(queriesOf(cg) as unknown as { computeDominantFile: () => unknown }, 'computeDominantFile');
+}
+
+describe('dominant file — computed once per index state (#1864)', () => {
+  let dir: string;
+  const open: CodeGraph[] = [];
+
+  afterEach(() => {
+    for (const cg of open.splice(0)) cg.close();
+    vi.restoreAllMocks();
+    if (dir) fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  async function setup(): Promise<CodeGraph> {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dominant-'));
+    fs.mkdirSync(path.join(dir, 'core'));
+    fs.mkdirSync(path.join(dir, 'ext'));
+    fs.writeFileSync(path.join(dir, 'core', 'engine.ts'), chain('engineStep', 40));
+    fs.writeFileSync(path.join(dir, 'ext', 'plugin.ts'), chain('pluginStep', 5));
+    const cg = await CodeGraph.init(dir, { index: true });
+    open.push(cg);
+    return cg;
+  }
+
+  it('reuses the answer across explores while the database is unchanged', async () => {
+    const cg = await setup();
+    const compute = spyCompute(cg);
+
+    for (const q of ['engine step', 'plugin step', 'how does the engine run']) {
+      await cg.findRelevantContext(q);
+    }
+    expect(compute).toHaveBeenCalledTimes(1);
+    expect(queriesOf(cg).getDominantFile()?.filePath).toBe('core/engine.ts');
+    expect(compute).toHaveBeenCalledTimes(1);
+  });
+
+  it('sees a new dominant file after a sync changes the graph', async () => {
+    const cg = await setup();
+    expect(queriesOf(cg).getDominantFile()?.filePath).toBe('core/engine.ts');
+
+    fs.writeFileSync(path.join(dir, 'ext', 'plugin.ts'), chain('pluginStep', 120));
+    await cg.sync();
+
+    const compute = spyCompute(cg);
+    expect(queriesOf(cg).getDominantFile()?.filePath).toBe('ext/plugin.ts');
+    expect(compute).toHaveBeenCalledTimes(1);
+  });
+
+  it('sees a sync made through another connection (another process)', async () => {
+    const writer = await setup();
+    const reader = await CodeGraph.open(dir);
+    open.push(reader);
+    expect(queriesOf(reader).getDominantFile()?.filePath).toBe('core/engine.ts');
+
+    fs.writeFileSync(path.join(dir, 'ext', 'plugin.ts'), chain('pluginStep', 120));
+    await writer.sync();
+
+    expect(queriesOf(reader).getDominantFile()?.filePath).toBe('ext/plugin.ts');
+  });
+
+  it('does not keep a result read inside a transaction that is rolled back', async () => {
+    const cg = await setup();
+    const q = queriesOf(cg);
+    const before = q.getDominantFile();
+    const db = (cg as any).db.getDb();
+    db.exec('BEGIN');
+    try {
+      db.exec('DELETE FROM edges');
+      expect(q.getDominantFile()).toBeNull();
+    } finally {
+      db.exec('ROLLBACK');
+    }
+    expect(q.getDominantFile()).toEqual(before);
+  });
+
+  it('forgets the answer when rebound to another connection (a pool worker following a rebuilt index)', async () => {
+    await setup();
+    const other = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dominant-other-'));
+    const conns: DatabaseConnection[] = [];
+    try {
+      fs.mkdirSync(path.join(other, 'ext'));
+      fs.writeFileSync(path.join(other, 'ext', 'plugin.ts'), chain('pluginStep', 120));
+      (await CodeGraph.init(other, { index: true })).close();
+      conns.push(DatabaseConnection.open(getDatabasePath(dir)), DatabaseConnection.open(getDatabasePath(other)));
+      const q = new QueryBuilder(conns[0]!.getDb());
+      expect(q.getDominantFile()?.filePath).toBe('core/engine.ts');
+      // Two fresh connections to different databases can report the same
+      // change stamp, so only dropping the memo on rebind keeps this honest.
+      q.rebind(conns[1]!.getDb());
+      expect(q.getDominantFile()?.filePath).toBe('ext/plugin.ts');
+    } finally {
+      for (const c of conns) c.close();
+      fs.rmSync(other, { recursive: true, force: true });
+    }
+  });
+
+  it.runIf(process.platform !== 'win32')('sees a database rebuilt and reopened under it', async () => {
+    const reader = await setup();
+    expect(queriesOf(reader).getDominantFile()?.filePath).toBe('core/engine.ts');
+    fs.writeFileSync(path.join(dir, 'ext', 'plugin.ts'), chain('pluginStep', 120));
+    const rebuilt = await CodeGraph.recreate(dir);
+    try { await rebuilt.indexAll(); } finally { rebuilt.close(); }
+    expect(reader.reopenIfReplaced()).toBe(true);
+    expect(queriesOf(reader).getDominantFile()?.filePath).toBe('ext/plugin.ts');
+  });
+});
@@ -251,6 +251,13 @@ export class QueryBuilder {
  private nodeCache: Map<string, Node> = new Map();
  private readonly maxCacheSize = 1000;

+  // getDominantFile()'s answer, tagged with the database change stamp it was
+  // computed under (see getChangeStamp). Query-independent, so one value
+  // serves every explore until the database changes (#1864).
+  private dominantFileMemo:
+    | { stamp: string; value: { filePath: string; edgeCount: number; nextEdgeCount: number } | null }
+    | undefined;
+
  // Prepared statements (lazily initialized)
  private stmts: {
    insertNode?: SqliteStatement;
@@ -294,6 +301,7 @@ export class QueryBuilder {
    getAllFilePaths?: SqliteStatement;
    getAllNodeNames?: SqliteStatement;
    getDominantFile?: SqliteStatement;
+    getChangeStamp?: SqliteStatement;
    getTopRouteFile?: SqliteStatement;
    getRoutingManifest?: SqliteStatement;
    insertNameSegment?: SqliteStatement;
@@ -379,6 +387,10 @@ export class QueryBuilder {
    this.stmts = {};
    this.batchStmts.clear();
    this.edgeKindStmts.clear();
+    // The change stamp is per connection, and fresh connections to two
+    // different databases report the same one — the memo goes with the old
+    // connection, or a worker following a rebuilt index keeps its answer (#1864).
+    this.dominantFileMemo = undefined;
  }

  private edgeKindStmt(sql: string): SqliteStatement {
@@ -1055,8 +1067,49 @@ export class QueryBuilder {
   * Excludes test/spec files from candidacy via path-pattern. The agent's
   * typical question is "how does X work", not "how is X tested", so
   * boosting a test file's directory would be a misfire.
+   *
+   * The answer depends only on the graph, never on the query, and the
+   * aggregation behind it scans every edge — seconds on a large index, paid
+   * by every generic explore (#1864). So it is memoized per database change
+   * stamp: recomputed only after something wrote to the database.
   */
  getDominantFile(): { filePath: string; edgeCount: number; nextEdgeCount: number } | null {
+    // total_changes() counts writes that are later rolled back, so a result
+    // read inside a transaction could outlive a ROLLBACK under an unchanged
+    // stamp. Never keep one; `undefined` (a runtime without the getter) is
+    // treated the same way.
+    if (this.db.inTransaction !== false) {
+      this.dominantFileMemo = undefined;
+      return this.computeDominantFile();
+    }
+    const stamp = this.getChangeStamp();
+    if (this.dominantFileMemo?.stamp === stamp) return this.dominantFileMemo.value;
+    const value = this.computeDominantFile();
+    this.dominantFileMemo = { stamp, value };
+    return value;
+  }
+
+  /**
+   * A value that differs whenever the database content may have changed
+   * since the last call, whoever changed it: `total_changes()` counts the
+   * rows this connection inserted, updated or deleted, and
+   * `PRAGMA data_version` moves when any OTHER connection — another process's
+   * sync, a CLI `codegraph index` beside a running MCP server — commits. Both
+   * are O(1), so no write path has to remember to invalidate anything.
+   * Coarse on purpose: any write, not just one to nodes/edges, forces a
+   * recompute, which only costs time, never a stale answer.
+   */
+  private getChangeStamp(): string {
+    if (!this.stmts.getChangeStamp) {
+      this.stmts.getChangeStamp = this.db.prepare(
+        'SELECT total_changes() AS changes, (SELECT data_version FROM pragma_data_version) AS version'
+      );
+    }
+    const row = this.stmts.getChangeStamp.get() as { changes: number; version: number };
+    return `${row.changes}:${row.version}`;
+  }
+
+  private computeDominantFile(): { filePath: string; edgeCount: number; nextEdgeCount: number } | null {
    if (!this.stmts.getDominantFile) {
      // Pull top 20 candidates; we then filter out test/generated files
      // in code (regex-grade matching that SQL LIKE can't express). The
@@ -32,6 +32,8 @@ export interface SqliteDatabase {
  transaction<T>(fn: (...args: any[]) => T): (...args: any[]) => T;
  close(): void;
  readonly open: boolean;
+  /** Undefined on a runtime without `isTransaction`; callers must then not memoize. */
+  readonly inTransaction?: boolean;
 }

 /**
@@ -73,6 +75,10 @@ class NodeSqliteAdapter implements SqliteDatabase {
    return toWslSharedIndexError(err, this._dbPath) ?? err;
  }

+  get inTransaction(): boolean | undefined {
+    return this._db.isTransaction;
+  }
+
  get open(): boolean {
    return this._db.isOpen;
  }