diff --git a/src/domain/graph/builder/incremental.ts b/src/domain/graph/builder/incremental.ts index 9dbff8be..c1f44379 100644 --- a/src/domain/graph/builder/incremental.ts +++ b/src/domain/graph/builder/incremental.ts @@ -11,7 +11,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { bulkNodeIdsByFile, purgeFileData } from '../../../db/index.js'; import { debug, warn } from '../../../infrastructure/logger.js'; -import { normalizePath } from '../../../shared/constants.js'; +import { normalizePath, TS_NATIVE_CONFIDENCE_FLOOR } from '../../../shared/constants.js'; import type { BetterSqlite3Database, EngineOpts, @@ -786,6 +786,66 @@ function buildCallEdges( return edgesAdded; } +// ── technique backfill (#1744) ────────────────────────────────────────── + +/** + * Backfill `technique = 'ts-native'` on `calls` edges just written by + * `buildCallEdges`/`emitIncrementalCallEdges` above, which insert edges via + * `stmts.insertEdge.run(...)` without ever setting `technique`, leaving it + * NULL. + * + * `'ts-native'` is not an engine marker — it is the resolution-technique + * label the full-build paths apply to every directly name/type-resolved + * `calls` edge, in both the WASM/JS pipeline (`emitDirectCallEdgesForCall` in + * stages/build-edges.ts) and the native pipeline (`buildCallEdgesNative`, + * same file), as opposed to `'points-to'` (alias/pts fallback) or `'cha'` / + * `'super-dispatch'` (virtual-dispatch expansion). `incremental.ts`'s call + * resolution (`resolveCallTargets` + the same-class/defineProperty + * fallbacks in `applyThisReceiverFallbacks`) implements only that same + * direct-resolution cascade — it has no pts or CHA/RTA post-pass of its own + * — so every edge it emits is the direct-resolution case and always + * belongs under `'ts-native'`, regardless of which engine parsed the file. + * + * Mirrors `applyEdgeTechniquesAfterNativeInsert` (full-build JS pipeline, + * stages/build-edges.ts) and `backfillEdgeTechniquesAfterNativeOrchestrator` + * (stages/native-orchestrator.ts): scope to the just-rebuilt files' source + * nodes, backfill NULL technique to 'ts-native', then lift any resulting + * ts-native edge below the confidence floor up to it — so a `calls` edge's + * `technique` (and, transitively, its confidence floor) no longer depends on + * whether it was last touched by a full build or a single-file watch rebuild. + * + * Scoped to `touchedFiles` (the rebuilt file + any reverse-dep cascade + * files), not a full-table scan. Chunked to stay within SQLite's + * SQLITE_LIMIT_VARIABLE_NUMBER (999 on older builds). + */ +function backfillIncrementalEdgeTechniques( + db: BetterSqlite3Database, + touchedFiles: readonly string[], +): void { + if (touchedFiles.length === 0) return; + const CHUNK_SIZE = 500; + const tx = db.transaction(() => { + for (let i = 0; i < touchedFiles.length; i += CHUNK_SIZE) { + const chunk = touchedFiles.slice(i, i + CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + db.prepare( + `UPDATE edges SET technique = 'ts-native' + WHERE kind = 'calls' AND technique IS NULL + AND source_id IN (SELECT id FROM nodes WHERE file IN (${placeholders}))`, + ).run(...chunk); + // Lift resolved ts-native edges below the confidence floor for this + // chunk, matching the floor lift the full-build native paths apply. + db.prepare( + `UPDATE edges SET confidence = ? + WHERE kind = 'calls' AND technique = 'ts-native' + AND confidence > 0 AND confidence < ? + AND source_id IN (SELECT id FROM nodes WHERE file IN (${placeholders}))`, + ).run(TS_NATIVE_CONFIDENCE_FLOOR, TS_NATIVE_CONFIDENCE_FLOOR, ...chunk); + } + }); + tx(); +} + // ── Main entry point ──────────────────────────────────────────────────── /** Build the "this file was deleted" result returned by `rebuildFile`. */ @@ -1008,6 +1068,13 @@ export async function rebuildFile( cache, ); edgesAdded += cascadeEdges; + + // Backfill technique='ts-native' (and the confidence floor) for this + // rebuild's calls edges — buildCallEdges above inserts edges without a + // technique value, unlike a full rebuild of either engine, which always + // tags directly-resolved calls edges 'ts-native' (#1744). + backfillIncrementalEdgeTechniques(db, [relPath, ...reverseDeps]); + // Include pre-deletion edge counts from reverse deps so the net delta // (edgesAdded - edgesBefore) is correct even when the cascade re-inserts // their edges unchanged. diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index edb4fe98..6a39f1db 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -1834,6 +1834,43 @@ async function backfillNativeDroppedFiles( cleanupThisDispatchWasmTrees(wasmResults); } +/** + * One-hop reverse dependents of `files`: files with a `calls` edge whose + * TARGET is a node inside one of `files`. The Rust incremental pipeline's own + * reverse-dep cascade (`reconnect_reverse_dep_edges` in detect_changes.rs) + * re-creates edges FROM these files INTO the changed files whenever the + * changed files' nodes are purged + reinserted with new IDs — without ever + * writing `technique` — so `backfillEdgeTechniquesAfterNativeOrchestrator` + * must reach them too, not just `changedFiles` itself (#1744). Mirrors the + * one-hop `findReverseDeps` query in `builder/incremental.ts`. + * + * Scoped to `e.kind = 'calls'` since that's the only edge kind the caller's + * backfill UPDATE touches — including other edge kinds (`imports`, + * `reexports`, `inherits`, ...) would only widen `scopeFiles` with files that + * have nothing for the UPDATE to change. + * + * Chunked to stay within SQLite's SQLITE_LIMIT_VARIABLE_NUMBER (999 on older + * builds). + */ +function findOneHopReverseDepFiles(db: BetterSqlite3Database, files: readonly string[]): string[] { + const found = new Set(); + const CHUNK_SIZE = 500; + for (let i = 0; i < files.length; i += CHUNK_SIZE) { + const chunk = files.slice(i, i + CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const rows = db + .prepare( + `SELECT DISTINCT n_src.file AS file FROM edges e + JOIN nodes n_src ON e.source_id = n_src.id + JOIN nodes n_tgt ON e.target_id = n_tgt.id + WHERE e.kind = 'calls' AND n_tgt.file IN (${placeholders}) AND n_src.kind != 'directory'`, + ) + .all(...chunk) as Array<{ file: string }>; + for (const r of rows) found.add(r.file); + } + return [...found]; +} + /** * Backfill the `technique` column on `calls` edges written by the native Rust * orchestrator, which does not write the column itself. Also lifts any @@ -1842,8 +1879,9 @@ async function backfillNativeDroppedFiles( * reflected in the call-confidence metric. * * For full builds, all `calls` edges in the DB are new so a global UPDATE is - * correct. For incremental builds, only changed-file source nodes are updated - * to avoid overwriting previously-set technique values on unchanged edges. + * correct. For incremental builds, only changed-file source nodes — plus + * their one-hop reverse dependents (#1744) — are updated to avoid overwriting + * previously-set technique values on unchanged edges. */ function backfillEdgeTechniquesAfterNativeOrchestrator( db: BetterSqlite3Database, @@ -1868,12 +1906,18 @@ function backfillEdgeTechniquesAfterNativeOrchestrator( ).run(TS_NATIVE_CONFIDENCE_FLOOR, TS_NATIVE_CONFIDENCE_FLOOR); return; } - // Incremental: scope to source nodes whose file is one of the changed files. + // Incremental: scope to source nodes whose file is one of the changed files, + // plus one-hop reverse dependents — callers whose outgoing edges into a + // changed file were reconnected by Rust's own cascade and so also carry a + // fresh, untagged technique value (#1744). + const scopeFiles = [ + ...new Set([...changedFiles, ...findOneHopReverseDepFiles(db, changedFiles)]), + ]; // Chunk to stay within SQLite's SQLITE_LIMIT_VARIABLE_NUMBER (999 on older builds). const CHUNK_SIZE = 500; const tx = db.transaction(() => { - for (let i = 0; i < changedFiles.length; i += CHUNK_SIZE) { - const chunk = changedFiles.slice(i, i + CHUNK_SIZE); + for (let i = 0; i < scopeFiles.length; i += CHUNK_SIZE) { + const chunk = scopeFiles.slice(i, i + CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(','); db.prepare( `UPDATE edges SET technique = 'ts-native' diff --git a/src/shared/constants.ts b/src/shared/constants.ts index bff1cab4..8e75b1b7 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -56,9 +56,10 @@ export const EXTENSIONS: ArrayCompatSet = withArrayCompat(new Set(SUPPOR * (confidence = 0.0) are intentionally excluded and must remain at 0.0 so * they stay below DEFAULT_MIN_CONFIDENCE and never surface in normal queries. * - * Used in `build-edges.ts` (in-memory + `applyEdgeTechniquesAfterNativeInsert`) - * and `native-orchestrator.ts` (`backfillEdgeTechniquesAfterNativeOrchestrator`). - * Centralised here so all three insertion paths apply the same value. + * Used in `build-edges.ts` (in-memory + `applyEdgeTechniquesAfterNativeInsert`), + * `native-orchestrator.ts` (`backfillEdgeTechniquesAfterNativeOrchestrator`), + * and `incremental.ts` (`backfillIncrementalEdgeTechniques`, #1744). + * Centralised here so all insertion paths apply the same value. */ export const TS_NATIVE_CONFIDENCE_FLOOR = 0.5; diff --git a/tests/integration/issue-1744-incremental-edge-technique.test.ts b/tests/integration/issue-1744-incremental-edge-technique.test.ts new file mode 100644 index 00000000..c3b69ee3 --- /dev/null +++ b/tests/integration/issue-1744-incremental-edge-technique.test.ts @@ -0,0 +1,227 @@ +/** + * Regression test for #1744: a `calls` edge's `technique` DB column differed + * depending on whether it was inserted via a full build or an incremental + * rebuild of the same final source state. + * + * Full builds always tag directly-resolved `calls` edges `technique = + * 'ts-native'` — a resolution-technique label applied by BOTH engines (not a + * native-engine marker): `emitDirectCallEdgesForCall` for the WASM/JS + * pipeline and `buildCallEdgesNative` for the native pipeline, both in + * `stages/build-edges.ts`. WASM/JS writes it inline via `batchInsertEdges`; + * native writes it via a post-insert backfill, since neither the native bulk + * `insertEdge` FFI nor the Rust orchestrator write `technique` at insert + * time. + * + * Two independent incremental-rebuild code paths had the same gap — the + * backfill never ran, or ran with an incomplete scope — both fixed here: + * + * 1. `codegraph watch` -> `rebuildFile` -> `buildCallEdges` -> + * `emitIncrementalCallEdges` in `src/domain/graph/builder/incremental.ts` + * inserted `calls` edges via `stmts.insertEdge.run(...)` without ever + * setting `technique`, for either engine. Fixed by + * `backfillIncrementalEdgeTechniques`, called unconditionally at the end + * of `rebuildFile`. + * + * 2. `codegraph build` (default incremental mode, native engine) -> + * `tryNativeOrchestrator`'s own backfill + * (`backfillEdgeTechniquesAfterNativeOrchestrator` in + * `stages/native-orchestrator.ts`) scoped its UPDATE to only the + * directly-changed files the Rust pipeline reports — missing one-hop + * reverse dependents (e.g. `callerA.js`/`callerB.js`) whose outgoing + * edges into the changed file are reconnected by Rust's own + * reverse-dep cascade (their target node IDs shifted when the changed + * file's nodes were purged + reinserted) and so also carry a fresh, + * untagged `technique`. Fixed by `findOneHopReverseDepFiles`, which + * expands the backfill's scope to include them. + * + * Fixture: callerA.js/callerB.js import and call `callee()` from callee.js. + * Editing callee.js (the file with *incoming* call edges, not a caller + * itself) is the scenario that exercises the reverse-dep cascade for gap (2) + * above — editing a caller file directly would already have worked even + * before this fix, since a changed file's own outgoing edges are always + * freshly (re)computed as part of its own changed-file processing. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { getNodeId as getNodeIdQuery, initSchema, openDb } from '../../src/db/index.js'; +import { rebuildFile } from '../../src/domain/graph/builder/incremental.js'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { EngineMode } from '../../src/types.js'; + +function writeFixture(dir: string, calleeMarker: string): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'callee.js'), + `export function callee() {\n ${calleeMarker}\n return 1;\n}\n`, + ); + fs.writeFileSync( + path.join(dir, 'callerA.js'), + `import { callee } from './callee.js';\nexport function callerA() {\n return callee();\n}\n`, + ); + fs.writeFileSync( + path.join(dir, 'callerB.js'), + `import { callee } from './callee.js';\nexport function callerB() {\n return callee();\n}\n`, + ); +} + +interface CallEdgeRow { + srcFile: string; + src: string; + tgt: string; + technique: string | null; + confidence: number; +} + +function readCallEdges(dbPath: string): CallEdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.file AS srcFile, n1.name AS src, n2.name AS tgt, e.technique, e.confidence + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.file, n1.name, n2.name`, + ) + .all() as CallEdgeRow[]; + } finally { + db.close(); + } +} + +function makeStmts(db: ReturnType) { + return { + insertNode: db.prepare( + 'INSERT OR IGNORE INTO nodes (name, kind, file, line, end_line) VALUES (?, ?, ?, ?, ?)', + ), + getNodeId: { + get: (name: string, kind: string, file: string, line: number) => { + const id = getNodeIdQuery(db, name, kind, file, line); + return id != null ? { id } : undefined; + }, + }, + insertEdge: db.prepare( + 'INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) VALUES (?, ?, ?, ?, ?)', + ), + countNodes: db.prepare('SELECT COUNT(*) as c FROM nodes WHERE file = ?'), + countEdges: db.prepare( + 'SELECT COUNT(*) as c FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)', + ), + findNodeInFile: db.prepare( + "SELECT id, kind, file FROM nodes WHERE name = ? AND kind IN ('function', 'method', 'class', 'interface', 'type', 'struct', 'enum', 'trait', 'record', 'module', 'constant') AND file = ?", + ), + findNodeByName: db.prepare( + "SELECT id, file, kind FROM nodes WHERE name = ? AND kind IN ('function', 'method', 'class', 'interface', 'type', 'struct', 'enum', 'trait', 'record', 'module', 'constant')", + ), + listSymbols: db.prepare("SELECT name, kind, line FROM nodes WHERE file = ? AND kind != 'file'"), + upsertFileHash: db.prepare( + 'INSERT OR REPLACE INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)', + ), + deleteFileHash: db.prepare('DELETE FROM file_hashes WHERE file = ?'), + }; +} + +// ── Suite 1: codegraph build --engine native (native orchestrator's own +// incremental path, stages/native-orchestrator.ts) ────────────────────── + +describe.skipIf(!isNativeAvailable())( + 'codegraph build --engine native: incremental rebuild technique matches full rebuild (#1744)', + () => { + let projDir: string; + let refDir: string; + + beforeAll(async () => { + projDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1744-buildcli-')); + writeFixture(projDir, '// v1'); + await buildGraph(projDir, { engine: 'native', incremental: false, skipRegistry: true }); + + // Edit ONLY callee.js — callerA.js/callerB.js are reverse deps whose + // outgoing edges get reconnected by the Rust incremental cascade. + writeFixture(projDir, '// v2 edited'); + await buildGraph(projDir, { engine: 'native', skipRegistry: true }); // incremental (default) + + refDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1744-buildcli-ref-')); + writeFixture(refDir, '// v2 edited'); + await buildGraph(refDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(projDir, { recursive: true, force: true }); + fs.rmSync(refDir, { recursive: true, force: true }); + }); + + it('tags reverse-dependent calls edges technique=ts-native after an incremental rebuild, matching a full rebuild', () => { + const incremental = readCallEdges(path.join(projDir, '.codegraph', 'graph.db')); + const reference = readCallEdges(path.join(refDir, '.codegraph', 'graph.db')); + + expect(incremental.length).toBeGreaterThan(0); + expect(incremental).toEqual(reference); + for (const edge of incremental) { + expect( + edge.technique, + `${edge.srcFile}: ${edge.src} -> ${edge.tgt} should be technique='ts-native', not ${edge.technique}`, + ).toBe('ts-native'); + } + }); + }, +); + +// ── Suite 2: codegraph watch (rebuildFile, builder/incremental.ts) ───────── + +function runWatchScenario(engine: EngineMode): void { + describe(`codegraph watch (rebuildFile): incremental rebuild technique matches full rebuild (#1744) — ${engine}`, () => { + let projDir: string; + let refDir: string; + + beforeAll(async () => { + projDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1744-watch-${engine}-`)); + writeFixture(projDir, '// v1'); + await buildGraph(projDir, { engine, incremental: false, skipRegistry: true }); + + // Edit ONLY callee.js via the watcher's single-file rebuild path. + writeFixture(projDir, '// v2 edited'); + const dbPath = path.join(projDir, '.codegraph', 'graph.db'); + const db = openDb(dbPath); + try { + initSchema(db); + const stmts = makeStmts(db); + await rebuildFile(db, projDir, path.join(projDir, 'callee.js'), stmts, { engine }, null); + } finally { + db.close(); + } + + refDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1744-watch-ref-${engine}-`)); + writeFixture(refDir, '// v2 edited'); + await buildGraph(refDir, { engine, incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(projDir, { recursive: true, force: true }); + fs.rmSync(refDir, { recursive: true, force: true }); + }); + + it('tags reverse-dependent calls edges technique=ts-native after rebuildFile, matching a full rebuild', () => { + const incremental = readCallEdges(path.join(projDir, '.codegraph', 'graph.db')); + const reference = readCallEdges(path.join(refDir, '.codegraph', 'graph.db')); + + expect(incremental.length).toBeGreaterThan(0); + expect(incremental).toEqual(reference); + for (const edge of incremental) { + expect( + edge.technique, + `${edge.srcFile}: ${edge.src} -> ${edge.tgt} should be technique='ts-native', not ${edge.technique}`, + ).toBe('ts-native'); + } + }); + }); +} + +runWatchScenario('wasm'); +describe.skipIf(!isNativeAvailable())('native engine coverage', () => { + runWatchScenario('native'); +});