Skip to content

Commit 6686019

Browse files
authored
fix(native): js-side fast-skip for incremental no-op rebuilds (#1054) (#1064)
* fix(native): js-side fast-skip for incremental no-op rebuilds (#1054) The Rust orchestrator's internal early-exit fires reliably locally but not in CI, where every native no-op rebuild was paying the full ~2s pipeline cost (parse, ast, cfg, dataflow, edges all re-running). WASM exits in ~20ms via detectChanges before any work happens. Mirror that behavior at the JS layer: a read-only Tier-0/Tier-1 (mtime+size) check before invoking the native orchestrator. When every collected file matches file_hashes, skip the orchestrator entirely. Tier-2 hashing stays on the native side — any mismatch falls through and lets Rust's detect_changes remain the source of truth. Benchmark on this repo (744 files): native noopRebuildMs: 2125ms → 22ms (matches WASM's 23ms) Closes #1054 * fix(detect-changes): guard fast-skip on empty CFG/dataflow tables (#1064) The Tier-0/Tier-1 fast-skip introduced by #1064 short-circuited builds based purely on mtime+size, missing the runPendingAnalysis guard that the existing JS early-exit path (detectChanges, line ~610) always runs. If CFG or dataflow analysis was enabled (or tables wiped) between builds and no source files changed, mtime/size would still match file_hashes, detectNoChanges would return true, and the pending analysis pass would silently never run — leaving cfg_blocks and dataflow empty indefinitely on no-op-rebuild repos. Add a conservative pending-analysis guard: when opts.cfg !== false and cfg_blocks is empty, return false; same for opts.dataflow / dataflow. The caller then falls through to the orchestrator (or JS pipeline), which populates the tables via the existing runPendingAnalysis path. Adds unit tests for detectNoChanges covering empty file_hashes, mtime+size match, deleted tracked file, mtime drift, and the new pending-analysis guards for both cfg and dataflow. Impact: 2 functions changed, 7 affected * fix(builder): avoid redundant collectFiles on fast-skip fallthrough (#1064) When the JS-side fast-skip pre-flight ran but detectNoChanges returned false, control fell through to tryNativeOrchestrator and, on native fallback, to runPipelineStages — which called collectFiles again at line 901, doubling the filesystem walk on the non-skip path and counteracting the PR's goal of eliminating overhead. Guard the collectFiles stage so it returns early when ctx.allFiles and ctx.discoveredDirs are already populated (and not in scoped mode). On pre-flight failure, the buildGraph catch block now resets these fields so the guard correctly falls through and re-collects under runPipelineStages's own engine state. Impact: 2 functions changed, 8 affected
1 parent 0dd948f commit 6686019

4 files changed

Lines changed: 272 additions & 2 deletions

File tree

src/domain/graph/builder/pipeline.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import path from 'node:path';
99
import { performance } from 'node:perf_hooks';
1010
import {
1111
acquireAdvisoryLock,
12+
closeDb,
1213
closeDbPair,
1314
getBuildMeta,
1415
initSchema,
@@ -39,6 +40,7 @@ import {
3940
getInstalledWasmExtensions,
4041
parseFilesWasmForBackfill,
4142
} from '../../parser.js';
43+
import { writeJournalHeader } from '../journal.js';
4244
import { setWorkspaces } from '../resolve.js';
4345
import { PipelineContext } from './context.js';
4446
import { batchInsertNodes, collectFiles as collectFilesUtil, loadPathAliases } from './helpers.js';
@@ -47,7 +49,7 @@ import { buildEdges } from './stages/build-edges.js';
4749
import { buildStructure } from './stages/build-structure.js';
4850
// Pipeline stages
4951
import { collectFiles } from './stages/collect-files.js';
50-
import { detectChanges } from './stages/detect-changes.js';
52+
import { detectChanges, detectNoChanges } from './stages/detect-changes.js';
5153
import { finalize } from './stages/finalize.js';
5254
import { insertNodes } from './stages/insert-nodes.js';
5355
import { parseFiles } from './stages/parse-files.js';
@@ -1000,6 +1002,42 @@ export async function buildGraph(
10001002
try {
10011003
setupPipeline(ctx);
10021004

1005+
// ── JS-side fast-skip for native incremental (#1054) ──────────────
1006+
// The Rust orchestrator's internal early-exit fires reliably locally
1007+
// but not in CI, where every no-op rebuild was paying the full ~2s
1008+
// pipeline cost. A read-only mtime+size check here matches WASM's
1009+
// ~20ms early-exit and skips the orchestrator entirely when no
1010+
// source files have changed. Tier-2 hashing is left to the native
1011+
// side: any mismatch falls through and lets Rust's detect_changes
1012+
// remain the source of truth.
1013+
if (
1014+
ctx.nativeAvailable &&
1015+
ctx.engineName === 'native' &&
1016+
ctx.incremental &&
1017+
!ctx.forceFullRebuild &&
1018+
!(ctx.opts as Record<string, unknown>).scope
1019+
) {
1020+
try {
1021+
await collectFiles(ctx);
1022+
if (
1023+
detectNoChanges(ctx.db, ctx.allFiles, ctx.rootDir, ctx.opts as Record<string, unknown>)
1024+
) {
1025+
info('No changes detected. Graph is up to date.');
1026+
writeJournalHeader(ctx.rootDir, Date.now());
1027+
closeDb(ctx.db);
1028+
return;
1029+
}
1030+
} catch (err) {
1031+
// Pre-flight is best-effort — any failure falls through to the
1032+
// orchestrator, which performs its own complete detection.
1033+
// Reset ctx.allFiles so runPipelineStages re-collects under its own
1034+
// engine state if we ended up partially populated before throwing.
1035+
ctx.allFiles = undefined as unknown as string[];
1036+
ctx.discoveredDirs = undefined as unknown as Set<string>;
1037+
debug(`native fast-skip pre-flight failed: ${toErrorMessage(err)}`);
1038+
}
1039+
}
1040+
10031041
// ── Rust orchestrator fast path (#695) ────────────────────────────
10041042
// When available, run the entire build pipeline in Rust with zero
10051043
// napi crossings (eliminates WAL dual-connection dance). Falls back

src/domain/graph/builder/stages/collect-files.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,15 @@ function tryFastCollect(
100100
export async function collectFiles(ctx: PipelineContext): Promise<void> {
101101
const { rootDir, config, opts } = ctx;
102102

103+
// Skip when the JS-side fast-skip pre-flight (#1054) already populated the
104+
// file list and changes were detected, causing fallthrough to the native
105+
// orchestrator and then to runPipelineStages. Avoids redoing the filesystem
106+
// walk on the non-skip path (~8ms on 473 files). On pre-flight failure the
107+
// caller resets ctx.allFiles so this guard correctly falls through.
108+
if (!opts.scope && ctx.allFiles?.length && ctx.discoveredDirs?.size) {
109+
return;
110+
}
111+
103112
if (opts.scope) {
104113
// Scoped rebuild: rebuild only specified files.
105114
//

src/domain/graph/builder/stages/detect-changes.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,94 @@ function handleIncrementalBuild(ctx: PipelineContext): void {
512512
purgeAndAddReverseDeps(ctx, changePaths, reverseDeps);
513513
}
514514

515+
/**
516+
* Read-only pre-flight check for the native orchestrator.
517+
*
518+
* Returns true iff every collected source file has matching mtime+size in
519+
* `file_hashes` and no DB-tracked file has been removed. When true, the
520+
* caller can short-circuit before invoking the native orchestrator —
521+
* matching WASM's ~20 ms early-exit path and avoiding the ~2s flat
522+
* per-call native rebuild overhead seen in CI (#1054).
523+
*
524+
* Intentionally Tier-0/Tier-1 only (journal + mtime/size). Tier-2 content
525+
* hashing is left to the native side: when this returns false the caller
526+
* falls through to the orchestrator, which performs its own complete
527+
* detection and is the source of truth.
528+
*
529+
* Conservatively returns false when CFG or dataflow analysis is enabled
530+
* but the corresponding tables are empty — otherwise the fast-skip would
531+
* silently suppress the pending-analysis pass that the JS path runs via
532+
* `runPendingAnalysis`, and CFG/dataflow data would never populate on
533+
* repos where source files don't change between builds.
534+
*
535+
* Pure read of `db` and the filesystem — never mutates either.
536+
*/
537+
export function detectNoChanges(
538+
db: BetterSqlite3Database,
539+
allFiles: string[],
540+
rootDir: string,
541+
opts?: Record<string, unknown>,
542+
): boolean {
543+
let hasTable = false;
544+
try {
545+
db.prepare('SELECT 1 FROM file_hashes LIMIT 1').get();
546+
hasTable = true;
547+
} catch {
548+
/* table missing — first build */
549+
}
550+
if (!hasTable) return false;
551+
552+
const rows = db.prepare('SELECT file, hash, mtime, size FROM file_hashes').all() as FileHashRow[];
553+
if (rows.length === 0) return false;
554+
const existing = new Map<string, FileHashRow>(rows.map((r) => [r.file, r]));
555+
556+
const currentFiles = new Set<string>();
557+
for (const file of allFiles) {
558+
currentFiles.add(normalizePath(path.relative(rootDir, file)));
559+
}
560+
for (const existingFile of existing.keys()) {
561+
if (!currentFiles.has(existingFile)) return false;
562+
}
563+
564+
for (const file of allFiles) {
565+
const relPath = normalizePath(path.relative(rootDir, file));
566+
const record = existing.get(relPath);
567+
if (!record) return false;
568+
const stat = fileStat(file) as FileStat | undefined;
569+
if (!stat) return false;
570+
const storedMtime = record.mtime || 0;
571+
const storedSize = record.size || 0;
572+
if (storedSize <= 0) return false;
573+
if (Math.floor(stat.mtimeMs) !== storedMtime || stat.size !== storedSize) return false;
574+
}
575+
576+
// Pending-analysis guard: if CFG/dataflow is enabled but the corresponding
577+
// table is empty (analysis newly enabled, or tables wiped between builds),
578+
// fall through so the orchestrator / JS pipeline can run runPendingAnalysis.
579+
// Mirrors the check at the top of runPendingAnalysis (see line ~244).
580+
if (opts) {
581+
if (opts.cfg !== false && hasEmptyAnalysisTable(db, 'cfg_blocks')) return false;
582+
if (opts.dataflow !== false && hasEmptyAnalysisTable(db, 'dataflow')) return false;
583+
}
584+
585+
return true;
586+
}
587+
588+
/**
589+
* Returns true if `table` exists and has zero rows, matching the empty-table
590+
* semantics of `runPendingAnalysis`. A missing table is treated as empty
591+
* (the conservative outcome), so the caller falls through to the orchestrator
592+
* which will create the schema and populate it.
593+
*/
594+
function hasEmptyAnalysisTable(db: BetterSqlite3Database, table: string): boolean {
595+
try {
596+
const row = db.prepare(`SELECT COUNT(*) as c FROM ${table}`).get() as { c: number } | undefined;
597+
return (row?.c ?? 0) === 0;
598+
} catch {
599+
return true;
600+
}
601+
}
602+
515603
export async function detectChanges(ctx: PipelineContext): Promise<void> {
516604
const start = performance.now();
517605
try {

tests/builder/detect-changes.test.ts

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import path from 'node:path';
77
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
88
import { closeDb, initSchema, openDb } from '../../src/db/index.js';
99
import { PipelineContext } from '../../src/domain/graph/builder/context.js';
10-
import { detectChanges } from '../../src/domain/graph/builder/stages/detect-changes.js';
10+
import {
11+
detectChanges,
12+
detectNoChanges,
13+
} from '../../src/domain/graph/builder/stages/detect-changes.js';
1114
import { writeJournalHeader } from '../../src/domain/graph/journal.js';
1215

1316
let tmpDir: string;
@@ -142,3 +145,135 @@ describe('detectChanges stage', () => {
142145
fs.rmSync(dir, { recursive: true, force: true });
143146
});
144147
});
148+
149+
describe('detectNoChanges fast-skip', () => {
150+
function seedFile(dir: string, name: string, content: string): string {
151+
const filePath = path.join(dir, name);
152+
fs.writeFileSync(filePath, content);
153+
return filePath;
154+
}
155+
156+
function seedHashRow(
157+
db: ReturnType<typeof openDb>,
158+
relPath: string,
159+
filePath: string,
160+
): { mtime: number; size: number } {
161+
const stat = fs.statSync(filePath);
162+
const mtime = Math.floor(stat.mtimeMs);
163+
db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
164+
relPath,
165+
'deadbeef',
166+
mtime,
167+
stat.size,
168+
);
169+
return { mtime, size: stat.size };
170+
}
171+
172+
it('returns false when file_hashes is empty (first build)', () => {
173+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-empty-'));
174+
const dbDir = path.join(dir, '.codegraph');
175+
fs.mkdirSync(dbDir, { recursive: true });
176+
const db = openDb(path.join(dbDir, 'graph.db'));
177+
initSchema(db);
178+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
179+
180+
expect(detectNoChanges(db, [file], dir)).toBe(false);
181+
182+
closeDb(db);
183+
fs.rmSync(dir, { recursive: true, force: true });
184+
});
185+
186+
it('returns true when mtime+size match seeded file_hashes', () => {
187+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-match-'));
188+
const dbDir = path.join(dir, '.codegraph');
189+
fs.mkdirSync(dbDir, { recursive: true });
190+
const db = openDb(path.join(dbDir, 'graph.db'));
191+
initSchema(db);
192+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
193+
seedHashRow(db, 'a.js', file);
194+
195+
expect(detectNoChanges(db, [file], dir)).toBe(true);
196+
197+
closeDb(db);
198+
fs.rmSync(dir, { recursive: true, force: true });
199+
});
200+
201+
it('returns false when a tracked file has been deleted', () => {
202+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-deleted-'));
203+
const dbDir = path.join(dir, '.codegraph');
204+
fs.mkdirSync(dbDir, { recursive: true });
205+
const db = openDb(path.join(dbDir, 'graph.db'));
206+
initSchema(db);
207+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
208+
seedHashRow(db, 'a.js', file);
209+
seedHashRow(db, 'gone.js', file); // tracked but no longer on disk
210+
211+
expect(detectNoChanges(db, [file], dir)).toBe(false);
212+
213+
closeDb(db);
214+
fs.rmSync(dir, { recursive: true, force: true });
215+
});
216+
217+
it('returns false when mtime differs from seeded value', () => {
218+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-mtime-'));
219+
const dbDir = path.join(dir, '.codegraph');
220+
fs.mkdirSync(dbDir, { recursive: true });
221+
const db = openDb(path.join(dbDir, 'graph.db'));
222+
initSchema(db);
223+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
224+
const stat = fs.statSync(file);
225+
db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
226+
'a.js',
227+
'deadbeef',
228+
Math.floor(stat.mtimeMs) + 1000, // skewed mtime
229+
stat.size,
230+
);
231+
232+
expect(detectNoChanges(db, [file], dir)).toBe(false);
233+
234+
closeDb(db);
235+
fs.rmSync(dir, { recursive: true, force: true });
236+
});
237+
238+
it('returns false when CFG analysis is enabled but cfg_blocks is empty (#1064)', () => {
239+
// Pending-analysis guard: even though mtime+size match, if cfg_blocks
240+
// is empty (analysis newly enabled), the caller must fall through so
241+
// runPendingAnalysis can populate the table.
242+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-pendingCfg-'));
243+
const dbDir = path.join(dir, '.codegraph');
244+
fs.mkdirSync(dbDir, { recursive: true });
245+
const db = openDb(path.join(dbDir, 'graph.db'));
246+
initSchema(db);
247+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
248+
seedHashRow(db, 'a.js', file);
249+
// cfg_blocks table is created empty by initSchema — that's the trigger.
250+
251+
// Without opts: legacy behaviour — fast-skip returns true.
252+
expect(detectNoChanges(db, [file], dir)).toBe(true);
253+
// With cfg enabled (cfg !== false) and cfg_blocks empty: must return false.
254+
expect(detectNoChanges(db, [file], dir, { cfg: true, dataflow: false })).toBe(false);
255+
// When cfg explicitly disabled (and dataflow disabled too so its guard
256+
// doesn't fire), the empty cfg table is irrelevant.
257+
expect(detectNoChanges(db, [file], dir, { cfg: false, dataflow: false })).toBe(true);
258+
259+
closeDb(db);
260+
fs.rmSync(dir, { recursive: true, force: true });
261+
});
262+
263+
it('returns false when dataflow is enabled but dataflow table is empty (#1064)', () => {
264+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-noChange-pendingDf-'));
265+
const dbDir = path.join(dir, '.codegraph');
266+
fs.mkdirSync(dbDir, { recursive: true });
267+
const db = openDb(path.join(dbDir, 'graph.db'));
268+
initSchema(db);
269+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
270+
seedHashRow(db, 'a.js', file);
271+
272+
// Disable cfg so only the dataflow guard is exercised.
273+
expect(detectNoChanges(db, [file], dir, { cfg: false, dataflow: true })).toBe(false);
274+
expect(detectNoChanges(db, [file], dir, { cfg: false, dataflow: false })).toBe(true);
275+
276+
closeDb(db);
277+
fs.rmSync(dir, { recursive: true, force: true });
278+
});
279+
});

0 commit comments

Comments
 (0)