Skip to content

Commit a1f2c52

Browse files
authored
fix: run analysis phases after native Rust build orchestrator (#757)
* fix: run analysis phases after native Rust build orchestrator The native Rust orchestrator early-returned with hardcoded 0ms for AST, complexity, CFG, and dataflow phases — meaning these analyses were never computed during native builds. After the orchestrator completes, reconstruct a minimal fileSymbols map from the DB and run analyses via the JS engine (native standalone functions + WASM fallback) with proper WAL handoff. * fix: address review feedback for native analysis phases (#757) - Scope analysis SQL query to changed files for incremental builds by returning changedFiles from the Rust build pipeline result (P1) - Guard against openDb failure leaving ctx.db as a stale closed handle by nullifying before reopen (P1) - Clear ctx.engineOpts.nativeDb after analysis nativeDb cleanup, matching the pattern used in runPipelineStages (P2) * fix: add missing changed_files field to early-exit BuildPipelineResult (#757) The Rust struct gained a changed_files field but the early-exit return path was not updated, causing a compile error on all platforms. * fix: handle openDb failure after native build with clear error (#757) If openDb throws after the native build succeeds, return a partial result (with 0ms analysis timings) instead of falling through to the JS pipeline with ctx.db = null, which would cause a confusing secondary error. Addresses Greptile P2 feedback. * fix: guard against empty changedFiles array in SQL query (#757) An empty changedFiles array is truthy in JavaScript, so the previous `if (changedFiles)` guard would pass and generate invalid SQL `IN ()` which SQLite rejects. Check `.length > 0` as well so empty arrays fall through to the full-table scan, matching the correct behavior for deletion-only incremental builds.
1 parent bb8d059 commit a1f2c52

2 files changed

Lines changed: 163 additions & 6 deletions

File tree

crates/codegraph-core/src/build_pipeline.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ pub struct BuildPipelineResult {
5757
pub edge_count: i64,
5858
pub file_count: usize,
5959
pub early_exit: bool,
60+
/// Files that were parsed/changed in this build cycle.
61+
/// `None` for full builds (all files), `Some` for incremental builds.
62+
#[serde(skip_serializing_if = "Option::is_none")]
63+
pub changed_files: Option<Vec<String>>,
6064
pub changed_count: usize,
6165
pub removed_count: usize,
6266
pub is_full_build: bool,
@@ -194,6 +198,7 @@ pub fn run_pipeline(
194198
edge_count: 0,
195199
file_count: collect_result.files.len(),
196200
early_exit: true,
201+
changed_files: Some(vec![]),
197202
changed_count: 0,
198203
removed_count: 0,
199204
is_full_build: false,
@@ -342,6 +347,12 @@ pub fn run_pipeline(
342347
let t0 = Instant::now();
343348
let line_count_map = structure::build_line_count_map(&file_symbols, root_dir);
344349
let changed_files: Vec<String> = file_symbols.keys().cloned().collect();
350+
// Keep a copy for the result — changed_files is moved into roles classification below.
351+
let analysis_scope: Option<Vec<String>> = if change_result.is_full_build {
352+
None
353+
} else {
354+
Some(changed_files.clone())
355+
};
345356

346357
let existing_file_count = structure::get_existing_file_count(conn);
347358
// Use parse_changes.len() for the threshold — changed_files includes
@@ -433,6 +444,7 @@ pub fn run_pipeline(
433444
edge_count,
434445
file_count: collect_result.files.len(),
435446
early_exit: false,
447+
changed_files: analysis_scope,
436448
changed_count: parse_changes.len(),
437449
removed_count: change_result.removed.len(),
438450
is_full_build: change_result.is_full_build,

src/domain/graph/builder/pipeline.ts

Lines changed: 151 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { debug, info, warn } from '../../../infrastructure/logger.js';
1919
import { loadNative } from '../../../infrastructure/native.js';
2020
import { semverCompare } from '../../../infrastructure/update-check.js';
2121
import { CODEGRAPH_VERSION } from '../../../shared/version.js';
22-
import type { BuildGraphOpts, BuildResult } from '../../../types.js';
22+
import type { BuildGraphOpts, BuildResult, Definition, ExtractorOutput } from '../../../types.js';
2323
import { getActiveEngine } from '../../parser.js';
2424
import { setWorkspaces } from '../resolve.js';
2525
import { PipelineContext } from './context.js';
@@ -381,6 +381,7 @@ export async function buildGraph(
381381
nodeCount?: number;
382382
edgeCount?: number;
383383
fileCount?: number;
384+
changedFiles?: string[];
384385
changedCount?: number;
385386
removedCount?: number;
386387
isFullBuild?: boolean;
@@ -419,10 +420,154 @@ export async function buildGraph(
419420
edge_count: String(result.edgeCount ?? 0),
420421
});
421422

422-
closeDbPair({ db: ctx.db, nativeDb: ctx.nativeDb });
423423
info(
424424
`Native build orchestrator completed: ${result.nodeCount ?? 0} nodes, ${result.edgeCount ?? 0} edges, ${result.fileCount ?? 0} files`,
425425
);
426+
427+
// ── Run analysis phases (AST, complexity, CFG, dataflow) ──────
428+
// Not yet ported to Rust. After the native orchestrator finishes,
429+
// reconstruct a minimal fileSymbols map from the DB and run analyses
430+
// via the JS engine (native standalone functions + WASM fallback).
431+
let analysisTiming = { astMs: 0, complexityMs: 0, cfgMs: 0, dataflowMs: 0 };
432+
const needsAnalysis =
433+
opts.ast !== false ||
434+
opts.complexity !== false ||
435+
opts.cfg !== false ||
436+
opts.dataflow !== false;
437+
438+
if (needsAnalysis) {
439+
// WAL handoff: checkpoint through rusqlite, close nativeDb,
440+
// reopen better-sqlite3 with a fresh page cache (#715, #736).
441+
try {
442+
ctx.nativeDb!.exec('PRAGMA wal_checkpoint(TRUNCATE)');
443+
} catch {
444+
/* ignore checkpoint errors */
445+
}
446+
try {
447+
ctx.nativeDb!.close();
448+
} catch {
449+
/* ignore close errors */
450+
}
451+
ctx.nativeDb = undefined;
452+
try {
453+
ctx.db.close();
454+
} catch {
455+
/* ignore close errors */
456+
}
457+
ctx.db = null!; // avoid closeDbPair operating on a stale handle
458+
try {
459+
ctx.db = openDb(ctx.dbPath);
460+
} catch (reopenErr) {
461+
warn(
462+
`Failed to reopen DB for analysis after native build: ${(reopenErr as Error).message}`,
463+
);
464+
// Native build succeeded but we can't run analyses — return partial result
465+
return {
466+
phases: {
467+
setupMs: +((p.setupMs ?? 0) + (p.collectMs ?? 0) + (p.detectMs ?? 0)).toFixed(1),
468+
parseMs: +(p.parseMs ?? 0).toFixed(1),
469+
insertMs: +(p.insertMs ?? 0).toFixed(1),
470+
resolveMs: +(p.resolveMs ?? 0).toFixed(1),
471+
edgesMs: +(p.edgesMs ?? 0).toFixed(1),
472+
structureMs: +(p.structureMs ?? 0).toFixed(1),
473+
rolesMs: +(p.rolesMs ?? 0).toFixed(1),
474+
astMs: 0,
475+
complexityMs: 0,
476+
cfgMs: 0,
477+
dataflowMs: 0,
478+
finalizeMs: +(p.finalizeMs ?? 0).toFixed(1),
479+
},
480+
};
481+
}
482+
483+
// Reconstruct minimal fileSymbols from DB for analysis visitors.
484+
// Each entry needs definitions with name/kind/line/endLine so the
485+
// engine can match complexity/CFG results to the right functions.
486+
// For incremental builds, scope to only the files that were parsed
487+
// in this cycle (matching the JS pipeline's behaviour in run-analyses.ts).
488+
const changedFiles = result.changedFiles;
489+
let query =
490+
'SELECT file, name, kind, line, end_line as endLine FROM nodes WHERE file IS NOT NULL';
491+
const params: string[] = [];
492+
if (changedFiles && changedFiles.length > 0) {
493+
const placeholders = changedFiles.map(() => '?').join(',');
494+
query += ` AND file IN (${placeholders})`;
495+
params.push(...changedFiles);
496+
}
497+
query += ' ORDER BY file, line';
498+
const rows = ctx.db.prepare(query).all(...params) as {
499+
file: string;
500+
name: string;
501+
kind: string;
502+
line: number;
503+
endLine: number | null;
504+
}[];
505+
506+
const fileSymbols = new Map<string, ExtractorOutput>();
507+
for (const row of rows) {
508+
let entry = fileSymbols.get(row.file);
509+
if (!entry) {
510+
entry = {
511+
definitions: [],
512+
calls: [],
513+
imports: [],
514+
classes: [],
515+
exports: [],
516+
typeMap: new Map(),
517+
};
518+
fileSymbols.set(row.file, entry);
519+
}
520+
entry.definitions.push({
521+
name: row.name,
522+
kind: row.kind as Definition['kind'],
523+
line: row.line,
524+
endLine: row.endLine ?? undefined,
525+
});
526+
}
527+
528+
// Reopen nativeDb for analysis features (suspend/resume WAL pattern).
529+
const native = loadNative();
530+
if (native?.NativeDatabase) {
531+
try {
532+
ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath);
533+
if (ctx.engineOpts) ctx.engineOpts.nativeDb = ctx.nativeDb;
534+
} catch {
535+
ctx.nativeDb = undefined;
536+
if (ctx.engineOpts) ctx.engineOpts.nativeDb = undefined;
537+
}
538+
}
539+
540+
try {
541+
const { runAnalyses: runAnalysesFn } = await import('../../../ast-analysis/engine.js');
542+
analysisTiming = await runAnalysesFn(
543+
ctx.db,
544+
fileSymbols,
545+
ctx.rootDir,
546+
opts,
547+
ctx.engineOpts,
548+
);
549+
} catch (err) {
550+
warn(`Analysis phases failed after native build: ${(err as Error).message}`);
551+
}
552+
553+
// Close nativeDb after analyses
554+
if (ctx.nativeDb) {
555+
try {
556+
ctx.nativeDb.exec('PRAGMA wal_checkpoint(TRUNCATE)');
557+
} catch {
558+
/* ignore checkpoint errors */
559+
}
560+
try {
561+
ctx.nativeDb.close();
562+
} catch {
563+
/* ignore close errors */
564+
}
565+
ctx.nativeDb = undefined;
566+
if (ctx.engineOpts) ctx.engineOpts.nativeDb = undefined;
567+
}
568+
}
569+
570+
closeDbPair({ db: ctx.db, nativeDb: ctx.nativeDb });
426571
return {
427572
phases: {
428573
setupMs: +((p.setupMs ?? 0) + (p.collectMs ?? 0) + (p.detectMs ?? 0)).toFixed(1),
@@ -432,10 +577,10 @@ export async function buildGraph(
432577
edgesMs: +(p.edgesMs ?? 0).toFixed(1),
433578
structureMs: +(p.structureMs ?? 0).toFixed(1),
434579
rolesMs: +(p.rolesMs ?? 0).toFixed(1),
435-
astMs: 0,
436-
complexityMs: 0,
437-
cfgMs: 0,
438-
dataflowMs: 0,
580+
astMs: +(analysisTiming.astMs ?? 0).toFixed(1),
581+
complexityMs: +(analysisTiming.complexityMs ?? 0).toFixed(1),
582+
cfgMs: +(analysisTiming.cfgMs ?? 0).toFixed(1),
583+
dataflowMs: +(analysisTiming.dataflowMs ?? 0).toFixed(1),
439584
finalizeMs: +(p.finalizeMs ?? 0).toFixed(1),
440585
},
441586
};

0 commit comments

Comments
 (0)