Skip to content

Commit 566797c

Browse files
authored
fix: skip native build orchestrator for addon ≤3.8.0 and fix path bug (#758)
* fix: skip native build orchestrator for addon ≤3.8.0 and fix path bug The native build orchestrator in addon 3.8.0 has a path normalization bug: file_symbols keys use absolute paths but known_files uses relative paths, causing zero import/call edges. Additionally, the orchestrator ran even when engine: 'wasm' was explicitly requested. JS-side fixes (immediate): - Skip orchestrator when engine version is 3.8.0 (falls back to JS) - Skip orchestrator when engine is not 'native' (respects engine option) - Skip orchestrator on forceFullRebuild (version/schema mismatch) - Add 'No changes detected' logging on native earlyExit - Add 'Incremental: N changed' logging from native pipeline results - Write build_meta after native build for cross-engine consistency Rust-side fix (takes effect in next native addon release): - Use relative_path() instead of normalize_path() when building file_symbols map, and update sym.file to relative path - Add changedCount/removedCount/isFullBuild to BuildPipelineResult so JS side can emit incremental status logging * fix: use semver comparison for orchestrator version check Replace hardcoded `=== '3.8.0'` with `semverCompare(version, '3.8.0') <= 0` so the check automatically passes for 3.9.0+ without manual updates. * style: format orchestrator version check on single line * fix: address review comments on native pipeline guard (#758) - Fix comment: say "3.8.0" (exact match) instead of misleading "≤3.8.0" - Log which condition triggered forceJs bypass for easier debugging - Document why JS overwrites Rust-written codegraph_version in build_meta * fix: include built_at in native orchestrator setBuildMeta (#758) The native orchestrator path omitted built_at from setBuildMeta, leaving the build timestamp stale after native runs. The JS finalize stage already writes it — match that behavior so codegraph info shows the correct build time regardless of which engine path ran. * fix(native): normalize batch key separators for Windows import edge resolution The Rust build_import_edges function constructs lookup keys as format!("{}/{}", root_dir, file), mixing backslashes in root_dir with forward slashes. On Windows, the JS batch map keys use path.join() which produces all-backslash paths, causing key mismatches and silently dropping 5 import edges. Fix: - Normalize batch map keys to forward slashes (resolve.ts, resolve-imports.ts) - Normalize lookup keys in getResolved and buildImportEdgesNative - Normalize root_dir in Rust key construction (edge_builder.rs) - Skip native import-edge builder for addon 3.8.0 (pre-compiled addon still has the bug; Rust fix takes effect in 3.8.1+) * fix: normalize batch key lookups in resolution tests for Windows Tests that look up resolveImportsBatch results must use normalizePath(fromFile) to match the normalized keys in the batch map.
1 parent a058615 commit 566797c

8 files changed

Lines changed: 89 additions & 16 deletions

File tree

crates/codegraph-core/src/build_pipeline.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ pub struct BuildPipelineResult {
5757
pub edge_count: i64,
5858
pub file_count: usize,
5959
pub early_exit: bool,
60+
pub changed_count: usize,
61+
pub removed_count: usize,
62+
pub is_full_build: bool,
6063
}
6164

6265
/// Normalize path to forward slashes.
@@ -191,6 +194,9 @@ pub fn run_pipeline(
191194
edge_count: 0,
192195
file_count: collect_result.files.len(),
193196
early_exit: true,
197+
changed_count: 0,
198+
removed_count: 0,
199+
is_full_build: false,
194200
});
195201
}
196202

@@ -247,8 +253,9 @@ pub fn run_pipeline(
247253

248254
// Build file symbols map (relative path → FileSymbols)
249255
let mut file_symbols: HashMap<String, FileSymbols> = HashMap::new();
250-
for sym in parsed {
251-
let rel = normalize_path(&sym.file);
256+
for mut sym in parsed {
257+
let rel = relative_path(root_dir, &sym.file);
258+
sym.file = rel.clone();
252259
file_symbols.insert(rel, sym);
253260
}
254261
timing.parse_ms = t0.elapsed().as_secs_f64() * 1000.0;
@@ -426,6 +433,9 @@ pub fn run_pipeline(
426433
edge_count,
427434
file_count: collect_result.files.len(),
428435
early_exit: false,
436+
changed_count: parse_changes.len(),
437+
removed_count: change_result.removed.len(),
438+
is_full_build: change_result.is_full_build,
429439
})
430440
}
431441

crates/codegraph-core/src/edge_builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ pub fn build_import_edges(
541541
let mut edges = Vec::new();
542542

543543
for file_input in &files {
544-
let abs_file = format!("{}/{}", root_dir, file_input.file);
544+
let abs_file = format!("{}/{}", root_dir.replace('\\', "/"), file_input.file);
545545

546546
for imp in &file_input.imports {
547547
// Barrel-only files: only emit reexport edges

src/domain/graph/builder/pipeline.ts

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,18 @@
66
*/
77
import path from 'node:path';
88
import { performance } from 'node:perf_hooks';
9-
import { closeDbPair, getBuildMeta, initSchema, MIGRATIONS, openDb } from '../../../db/index.js';
9+
import {
10+
closeDbPair,
11+
getBuildMeta,
12+
initSchema,
13+
MIGRATIONS,
14+
openDb,
15+
setBuildMeta,
16+
} from '../../../db/index.js';
1017
import { detectWorkspaces, loadConfig } from '../../../infrastructure/config.js';
11-
import { info, warn } from '../../../infrastructure/logger.js';
18+
import { debug, info, warn } from '../../../infrastructure/logger.js';
1219
import { loadNative } from '../../../infrastructure/native.js';
20+
import { semverCompare } from '../../../infrastructure/update-check.js';
1321
import { CODEGRAPH_VERSION } from '../../../shared/version.js';
1422
import type { BuildGraphOpts, BuildResult } from '../../../types.js';
1523
import { getActiveEngine } from '../../parser.js';
@@ -338,7 +346,27 @@ export async function buildGraph(
338346
// When available, run the entire build pipeline in Rust with zero
339347
// napi crossings (eliminates WAL dual-connection dance). Falls back
340348
// to the JS pipeline on failure or when native is unavailable.
341-
const forceJs = process.env.CODEGRAPH_FORCE_JS_PIPELINE === '1';
349+
//
350+
// Native addon 3.8.0 has a path bug: file_symbols keys are absolute
351+
// paths but known_files are relative, causing zero import/call edges.
352+
// Skip the orchestrator for affected versions (fixed in 3.9.0+).
353+
const orchestratorBuggy = !!ctx.engineVersion && semverCompare(ctx.engineVersion, '3.8.0') <= 0;
354+
const forceJs =
355+
process.env.CODEGRAPH_FORCE_JS_PIPELINE === '1' ||
356+
ctx.forceFullRebuild ||
357+
orchestratorBuggy ||
358+
ctx.engineName !== 'native';
359+
if (forceJs) {
360+
const reason =
361+
process.env.CODEGRAPH_FORCE_JS_PIPELINE === '1'
362+
? 'CODEGRAPH_FORCE_JS_PIPELINE=1'
363+
: ctx.forceFullRebuild
364+
? 'forceFullRebuild'
365+
: orchestratorBuggy
366+
? `buggy addon ${ctx.engineVersion}`
367+
: `engine=${ctx.engineName}`;
368+
debug(`Skipping native orchestrator: ${reason}`);
369+
}
342370
if (!forceJs && ctx.nativeDb?.buildGraph) {
343371
try {
344372
const resultJson = ctx.nativeDb.buildGraph(
@@ -353,17 +381,44 @@ export async function buildGraph(
353381
nodeCount?: number;
354382
edgeCount?: number;
355383
fileCount?: number;
384+
changedCount?: number;
385+
removedCount?: number;
386+
isFullBuild?: boolean;
356387
};
357388

358389
if (result.earlyExit) {
390+
info('No changes detected');
359391
closeDbPair({ db: ctx.db, nativeDb: ctx.nativeDb });
360392
return;
361393
}
362394

395+
// Log incremental status to match JS pipeline output
396+
const changed = result.changedCount ?? 0;
397+
const removed = result.removedCount ?? 0;
398+
if (!result.isFullBuild && (changed > 0 || removed > 0)) {
399+
info(`Incremental: ${changed} changed, ${removed} removed`);
400+
}
401+
363402
// Map Rust timing fields to the JS BuildResult format.
364403
// Rust handles collect+detect+parse+insert+resolve+edges+structure+roles.
365-
// AST/complexity/CFG/dataflow analyses are not yet ported to Rust.
366404
const p = result.phases;
405+
406+
// Sync build_meta so JS-side version/engine checks work on next build.
407+
// Note: the Rust orchestrator also writes codegraph_version (using
408+
// CARGO_PKG_VERSION). We intentionally overwrite it here with the npm
409+
// package version so that the JS-side "version changed → full rebuild"
410+
// detection (line ~97) compares against the authoritative JS version.
411+
// The two versions are kept in lockstep by the release process.
412+
setBuildMeta(ctx.db, {
413+
engine: ctx.engineName,
414+
engine_version: ctx.engineVersion || '',
415+
codegraph_version: CODEGRAPH_VERSION,
416+
schema_version: String(ctx.schemaVersion),
417+
built_at: new Date().toISOString(),
418+
node_count: String(result.nodeCount ?? 0),
419+
edge_count: String(result.edgeCount ?? 0),
420+
});
421+
367422
closeDbPair({ db: ctx.db, nativeDb: ctx.nativeDb });
368423
info(
369424
`Native build orchestrator completed: ${result.nodeCount ?? 0} nodes, ${result.edgeCount ?? 0} edges, ${result.fileCount ?? 0} files`,

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { performance } from 'node:perf_hooks';
99
import { getNodeId } from '../../../../db/index.js';
1010
import { debug } from '../../../../infrastructure/logger.js';
1111
import { loadNative } from '../../../../infrastructure/native.js';
12+
import { normalizePath } from '../../../../shared/constants.js';
1213
import type {
1314
BetterSqlite3Database,
1415
Call,
@@ -219,7 +220,7 @@ function buildImportEdgesNative(
219220
addFileNodeId(resolvedPath);
220221

221222
// Check if this resolution is in batchResolved; if not, add supplemental
222-
const resolveKey = `${path.join(rootDir, relPath)}|${imp.source}`;
223+
const resolveKey = `${normalizePath(path.join(rootDir, relPath))}|${imp.source}`;
223224
if (!ctx.batchResolved?.has(resolveKey)) {
224225
supplementalResolved.push({ key: resolveKey, resolvedPath });
225226
}
@@ -743,10 +744,12 @@ export async function buildEdges(ctx: PipelineContext): Promise<void> {
743744
}
744745
}
745746

746-
// Skip native import-edge path for small incremental builds (≤3 files):
747-
// napi-rs marshaling overhead exceeds computation savings.
747+
// Skip native import-edge path for small incremental builds (≤3 files)
748+
// and for addon 3.8.0 which has a Windows path-separator bug in key
749+
// construction (format!("{}/{}", root_dir, file) vs path.join backslashes).
750+
const importEdgeBuggy = ctx.engineVersion === '3.8.0';
748751
const useNativeImportEdges =
749-
native?.buildImportEdges && (ctx.isFullBuild || ctx.fileSymbols.size > 3);
752+
native?.buildImportEdges && !importEdgeBuggy && (ctx.isFullBuild || ctx.fileSymbols.size > 3);
750753
if (useNativeImportEdges) {
751754
buildImportEdgesNative(ctx, getNodeIdStmt, allEdgeRows, native!);
752755
} else {

src/domain/graph/builder/stages/resolve-imports.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import path from 'node:path';
22
import { performance } from 'node:perf_hooks';
33
import { debug } from '../../../../infrastructure/logger.js';
4+
import { normalizePath } from '../../../../shared/constants.js';
45
import type { Import } from '../../../../types.js';
56
import { parseFilesAuto } from '../../../parser.js';
67
import { resolveImportPath, resolveImportsBatch } from '../../resolve.js';
@@ -71,7 +72,9 @@ export async function resolveImports(ctx: PipelineContext): Promise<void> {
7172
const symbols = fileSymbols.get(relPath);
7273
if (!symbols) continue;
7374
for (const imp of symbols.imports) {
74-
const resolved = ctx.batchResolved?.get(`${path.join(rootDir, relPath)}|${imp.source}`);
75+
const resolved = ctx.batchResolved?.get(
76+
`${normalizePath(path.join(rootDir, relPath))}|${imp.source}`,
77+
);
7578
const target =
7679
resolved ??
7780
resolveImportPath(path.join(rootDir, relPath), imp.source, rootDir, aliases);
@@ -142,7 +145,7 @@ export async function resolveImports(ctx: PipelineContext): Promise<void> {
142145

143146
export function getResolved(ctx: PipelineContext, absFile: string, importSource: string): string {
144147
if (ctx.batchResolved) {
145-
const key = `${absFile}|${importSource}`;
148+
const key = `${normalizePath(absFile)}|${importSource}`;
146149
const hit = ctx.batchResolved.get(key);
147150
if (hit !== undefined) return hit;
148151
}

src/domain/graph/resolve.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ export function resolveImportsBatch(
565565
// Native resolver's .js → .ts remap fails on unnormalized paths —
566566
// apply JS-side fallback (same fix as resolveImportPath).
567567
const resolved = remapJsToTs(normalized, rootDir);
568-
map.set(`${r.fromFile}|${r.importSource}`, resolved);
568+
map.set(`${normalizePath(r.fromFile)}|${r.importSource}`, resolved);
569569
}
570570
return map;
571571
} catch (e) {

tests/resolution/parity.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
resolveImportsBatch,
1616
} from '../../src/domain/graph/resolve.js';
1717
import { isNativeAvailable, loadNative } from '../../src/infrastructure/native.js';
18+
import { normalizePath } from '../../src/shared/constants.js';
1819

1920
const hasNative = isNativeAvailable();
2021

@@ -257,7 +258,7 @@ describe.skipIf(!hasNative)('Batch import resolution', () => {
257258

258259
for (const { fromFile, importSource } of inputs) {
259260
const individual = resolveImportPathJS(fromFile, importSource, rootDir, noAliases);
260-
const batchKey = `${fromFile}|${importSource}`;
261+
const batchKey = `${normalizePath(fromFile)}|${importSource}`;
261262
expect(batchResult.get(batchKey)).toBe(individual);
262263
}
263264
});

tests/unit/resolve.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
resolveViaWorkspace,
2424
setWorkspaces,
2525
} from '../../src/domain/graph/resolve.js';
26+
import { normalizePath } from '../../src/shared/constants.js';
2627

2728
// ─── Temp project setup ──────────────────────────────────────────────
2829

@@ -236,7 +237,7 @@ describe('resolveImportsBatch', () => {
236237
const result = resolveImportsBatch([{ fromFile, importSource: './math.js' }], tmpDir, null);
237238
// Skip when native addon is not available
238239
if (result === null) return;
239-
const key = `${fromFile}|./math.js`;
240+
const key = `${normalizePath(fromFile)}|./math.js`;
240241
const resolved = result.get(key);
241242
expect(resolved).toBeDefined();
242243
expect(resolved).toMatch(/math\.ts$/);

0 commit comments

Comments
 (0)