diff --git a/CHANGELOG.md b/CHANGELOG.md index e7559656f7..3ec9ce6750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,6 +172,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### MCP / indexing - 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) +- 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. (#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) - 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. - Incremental sync now keeps edge rebinding crash-safe: replacing a resolved edge with its recovery reference commits atomically, so an interruption cannot permanently remove the relationship. diff --git a/__tests__/dominant-file-cache.test.ts b/__tests__/dominant-file-cache.test.ts new file mode 100644 index 0000000000..7e4f3a1f35 --- /dev/null +++ b/__tests__/dominant-file-cache.test.ts @@ -0,0 +1,118 @@ +/** + * 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 type { QueryBuilder } from '../src/db/queries'; + +/** 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.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 14a6643412..d4a60156e8 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; @@ -290,6 +297,7 @@ export class QueryBuilder { getAllFilePaths?: SqliteStatement; getAllNodeNames?: SqliteStatement; getDominantFile?: SqliteStatement; + getChangeStamp?: SqliteStatement; getTopRouteFile?: SqliteStatement; getRoutingManifest?: SqliteStatement; insertNameSegment?: SqliteStatement; @@ -995,8 +1003,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; }