diff --git a/CHANGELOG.md b/CHANGELOG.md index b20e237c84..c7eedc92c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/__tests__/dominant-file-cache.test.ts b/__tests__/dominant-file-cache.test.ts new file mode 100644 index 0000000000..f9ba57eef5 --- /dev/null +++ b/__tests__/dominant-file-cache.test.ts @@ -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 { + 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'); + }); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index 9666a256f5..86215d061b 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -251,6 +251,13 @@ export class QueryBuilder { private nodeCache: Map = 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 diff --git a/src/db/sqlite-adapter.ts b/src/db/sqlite-adapter.ts index e94b587b95..39b2d8ba0f 100644 --- a/src/db/sqlite-adapter.ts +++ b/src/db/sqlite-adapter.ts @@ -32,6 +32,8 @@ export interface SqliteDatabase { transaction(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; }