fix(builder): do not commit a file's hash when its parse produced no data

crates/codegraph-core/src/domain/graph/builder/pipeline.rs

@@ -719,7 +719,7 @@ pub fn run_pipeline(
     // than a "successful" build with missing data (#1827).
     let t0 = Instant::now();
     let insert_batches = build_insert_batches(&file_symbols);
-    let file_hashes = build_file_hash_entries(&parse_changes);
+    let file_hashes = build_file_hash_entries(&parse_changes, &file_symbols);
     crate::domain::graph::builder::stages::insert_nodes::do_insert_nodes(
         conn,
         &insert_batches,
@@ -1356,11 +1356,26 @@ fn build_insert_batches(
 /// For full builds, `detect_changes` returns `hash: None` because it skips
 /// reading file content. In that case we read and hash each file here so
 /// that `file_hashes` is populated for subsequent incremental builds.
+///
+/// A changed file with no entry in `file_symbols` means extraction failed
+/// outright (worker panic recovery, unreadable, unsupported/missing
+/// grammar) — as opposed to a file that parsed successfully but
+/// legitimately produced zero symbols, which DOES get an entry (with empty
+/// `definitions`/`exports`; `parse_files_parallel`'s `filter_map` only
+/// drops files where parsing itself never produced a tree). Committing a
+/// hash for the former would mark it "up to date" relative to graph data
+/// that was never written, permanently hiding the loss from every later
+/// incremental build (issue #2441) — skip it instead, so the next build
+/// still sees it as changed and reprocesses it. Mirrors
+/// `iterFileHashRecords`'s `parsedRelPaths` check in
+/// `src/domain/graph/builder/stages/insert-nodes.ts`.
 fn build_file_hash_entries(
     changed: &[&detect_changes::ChangedFile],
+    file_symbols: &BTreeMap<String, FileSymbols>,
 ) -> Vec<crate::domain::graph::builder::stages::insert_nodes::FileHashEntry> {
     changed
         .iter()
+        .filter(|c| file_symbols.contains_key(&c.rel_path))
         .filter_map(|c| {
             let hash = match c.hash.as_ref() {
                 Some(h) => h.clone(),
@@ -3054,4 +3069,58 @@ mod tests {
             Some("User")
         );
     }
+
+    fn changed_file(rel_path: &str, hash: &str) -> detect_changes::ChangedFile {
+        detect_changes::ChangedFile {
+            abs_path: format!("/repo/{rel_path}"),
+            rel_path: rel_path.to_string(),
+            content: None,
+            hash: Some(hash.to_string()),
+            mtime: 1000,
+            size: 10,
+            metadata_only: false,
+            reverse_dep_only: false,
+        }
+    }
+
+    // Issue #2441: a changed file whose extraction failed outright (worker
+    // panic recovery, unreadable, unsupported/missing grammar) has no entry
+    // in file_symbols at all — must not get a committed hash, or the next
+    // incremental build wrongly believes its (missing) graph data is up to
+    // date with the file's new content, permanently hiding the loss.
+    #[test]
+    fn skips_a_changed_file_with_no_file_symbols_entry() {
+        let ok = changed_file("a.js", "hash-a");
+        let failed = changed_file("b.js", "hash-b");
+        let changed: Vec<&detect_changes::ChangedFile> = vec![&ok, &failed];
+
+        let mut file_symbols = BTreeMap::new();
+        file_symbols.insert("a.js".to_string(), FileSymbols::new("a.js".to_string()));
+        // "b.js" intentionally has no entry — simulates a parse failure.
+
+        let entries = build_file_hash_entries(&changed, &file_symbols);
+        let files: Vec<&str> = entries.iter().map(|e| e.file.as_str()).collect();
+        assert_eq!(files, vec!["a.js"]);
+    }
+
+    // A file that parsed successfully but legitimately produced zero symbols
+    // (empty file, parser no-op) DOES get a file_symbols entry (with empty
+    // definitions/exports) — it must still get a committed hash, or the
+    // no-op fast-skip pre-flight on the next rebuild would reject it as
+    // "missing from file_hashes" and force a full rebuild.
+    #[test]
+    fn still_includes_a_changed_file_that_parsed_with_zero_symbols() {
+        let empty = changed_file("empty.js", "hash-empty");
+        let changed: Vec<&detect_changes::ChangedFile> = vec![&empty];
+
+        let mut file_symbols = BTreeMap::new();
+        file_symbols.insert(
+            "empty.js".to_string(),
+            FileSymbols::new("empty.js".to_string()),
+        );
+
+        let entries = build_file_hash_entries(&changed, &file_symbols);
+        assert_eq!(entries.len(), 1);
+        assert_eq!(entries[0].file, "empty.js");
+    }
 }

src/domain/graph/builder/stages/insert-nodes.ts

@@ -165,6 +165,7 @@ function* iterFileHashRecords(
   metadataUpdates: MetadataUpdate[],
   rootDir: string,
   caller: string,
+  parsedRelPaths?: ReadonlySet<string>,
 ): Generator<FileHashRecord> {
   const seen = new Set<string>();
 
@@ -176,6 +177,19 @@ function* iterFileHashRecords(
     const precomputed = precomputedData.get(relPath);
     if (precomputed?._reverseDepOnly) continue;
 
+    // #2441: a file that was actually scheduled for parsing (not a
+    // reverse-dep-only refresh, handled above) but produced no entry in
+    // fileSymbols means extraction failed outright — a crash, an unreadable
+    // file, or an unsupported/missing grammar — as opposed to a file that
+    // parsed successfully but legitimately produced zero symbols (#1068),
+    // which DOES get a fileSymbols entry (with empty definitions/exports).
+    // Committing this file's hash would mark it "up to date" relative to
+    // content whose graph data was never written, permanently hiding the
+    // loss from every later incremental build. Only enforced when the
+    // caller actually tracked parse outcomes (`parsedRelPaths` provided) —
+    // direct/synthetic callers keep the original unconditional behavior.
+    if (parsedRelPaths && !parsedRelPaths.has(relPath)) continue;
+
     const record = resolveHashFromPrecomputed(
       relPath,
       precomputed ?? ({} as PrecomputedFileData),
@@ -204,13 +218,21 @@ function* iterFileHashRecords(
  * `file_hashes`, which permanently breaks the JS-side fast-skip pre-flight on
  * any subsequent no-op rebuild (#1068).
  *
+ * `parsedRelPaths`, when provided, additionally excludes a file that was
+ * scheduled for parsing but has no entry there at all — a genuine parse
+ * failure rather than a legitimate empty result (#2441; see the check inside
+ * `iterFileHashRecords`). Omitted by direct/synthetic callers (e.g. this
+ * function's own unit tests), which don't track parse outcomes and keep the
+ * original unconditional #1068 behavior.
+ *
  * Exported for unit testing.
  */
 export function buildFileHashes(
   filesToParse: FileToParse[],
   precomputedData: Map<string, PrecomputedFileData>,
   metadataUpdates: MetadataUpdate[],
   rootDir: string,
+  parsedRelPaths?: ReadonlySet<string>,
 ): FileHashRecord[] {
   return [
     ...iterFileHashRecords(
@@ -219,6 +241,7 @@ export function buildFileHashes(
       metadataUpdates,
       rootDir,
       'buildFileHashes',
+      parsedRelPaths,
     ),
   ];
 }
@@ -478,13 +501,19 @@ export async function insertNodes(ctx: PipelineContext): Promise<void> {
  * correctly detects the file as still needing (re)processing.
  */
 export function commitFileHashes(ctx: PipelineContext): void {
-  const { filesToParse, metadataUpdates, rootDir } = ctx;
+  const { filesToParse, metadataUpdates, rootDir, fileSymbols } = ctx;
 
   const precomputedData = new Map<string, PrecomputedFileData>();
   for (const item of filesToParse) {
     if (item.relPath) precomputedData.set(item.relPath, item as PrecomputedFileData);
   }
-  const fileHashes = buildFileHashes(filesToParse, precomputedData, metadataUpdates, rootDir);
+  const fileHashes = buildFileHashes(
+    filesToParse,
+    precomputedData,
+    metadataUpdates,
+    rootDir,
+    new Set(fileSymbols.keys()),
+  );
   if (fileHashes.length === 0) return;
 
   if (ctx.engineName === 'native' && ctx.nativeDb?.healFileMetadata) {

tests/integration/issue-2441-scoped-build-failed-parse-data-loss.test.ts

@@ -0,0 +1,199 @@
+/**
+ * Regression test for #2441: a scoped incremental build (`codegraph build
+ * --incremental` with an explicit `scope`) that fails to PARSE a changed
+ * file still committed a `file_hashes` row matching that file's new on-disk
+ * content — permanently hiding the data loss from every later incremental
+ * build. Same failure mode as #2435 (a `file_hashes` row that outlives the
+ * data it describes), but pre-existing and independent of that fix, which
+ * only protects `rebuildFile` (the `codegraph watch` path).
+ *
+ * Root cause: `handleScopedBuild` purges a changed file's nodes/edges
+ * BEFORE parsing runs — but deliberately leaves its OLD `file_hashes` row
+ * alone (deferred-commit design from #1731: a hash only ever advances once
+ * the data it describes has been rebuilt to match). `parseFiles`
+ * (`domain/graph/builder/stages/parse-files.ts`) then parses the changed
+ * set; a file whose extraction fails outright (worker crash, unreadable,
+ * unsupported/missing grammar) simply gets no entry in
+ * `ctx.allSymbols`/`ctx.fileSymbols` — but `commitFileHashes`
+ * (`domain/graph/builder/stages/insert-nodes.ts`) built its hash list from
+ * `ctx.filesToParse` directly, with no check that the file actually
+ * produced data, so the file's hash still got overwritten with a value
+ * matching its NEW on-disk content, even though the (missing) new state was
+ * never written.
+ *
+ * Unlike #2435, the purge itself is NOT deferred here (a much larger change
+ * given the purge and the parse live in different pipeline stages) — the
+ * fix only withholds the file_hashes commit, so the OLD (pre-edit) hash
+ * survives the failed build. Since it no longer matches the file's actual
+ * current content, the next incremental build correctly detects the file as
+ * still changed and reprocesses it, recovering the data. The read-failure
+ * half of this bug class doesn't apply on the scoped-build path: change
+ * detection reads each candidate up front and drops unreadable files from
+ * the changed set before the purge ever runs.
+ */
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import Database from 'better-sqlite3';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { buildGraph } from '../../src/domain/graph/builder.js';
+import { getWasmWorkerPool } from '../../src/domain/wasm-worker-pool.js';
+
+function writeProject(dir: string, { callsTarget = true }: { callsTarget?: boolean } = {}) {
+  fs.mkdirSync(dir, { recursive: true });
+  fs.writeFileSync(path.join(dir, 'target.js'), 'export function target() { return 1; }\n');
+  fs.writeFileSync(
+    path.join(dir, 'caller.js'),
+    callsTarget
+      ? "import { target } from './target.js';\nexport function run() { return target(); }\n"
+      : 'export function run() { return 0; }\n',
+  );
+}
+
+function withDb<T>(dbPath: string, fn: (db: Database.Database) => T): T {
+  const db = new Database(dbPath, { readonly: true });
+  try {
+    return fn(db);
+  } finally {
+    db.close();
+  }
+}
+
+function readSymbolNames(dbPath: string, file: string): string[] {
+  return withDb(dbPath, (db) =>
+    (
+      db
+        .prepare("SELECT name FROM nodes WHERE file = ? AND kind != 'file' ORDER BY name")
+        .all(file) as Array<{ name: string }>
+    ).map((r) => r.name),
+  );
+}
+
+function readIncomingCalls(dbPath: string, targetFile: string, targetName: string): string[] {
+  return withDb(dbPath, (db) =>
+    (
+      db
+        .prepare(
+          `SELECT n_src.file AS file, n_src.name AS name 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 = ? AND n_tgt.name = ?
+           ORDER BY n_src.file, n_src.name`,
+        )
+        .all(targetFile, targetName) as Array<{ file: string; name: string }>
+    ).map((r) => `${r.file}:${r.name}`),
+  );
+}
+
+function hasFileHashRow(dbPath: string, file: string): boolean {
+  return withDb(
+    dbPath,
+    (db) => db.prepare('SELECT 1 FROM file_hashes WHERE file = ?').get(file) !== undefined,
+  );
+}
+
+function readFileHash(dbPath: string, file: string): string | undefined {
+  return withDb(
+    dbPath,
+    (db) =>
+      (
+        db.prepare('SELECT hash FROM file_hashes WHERE file = ?').get(file) as
+          | { hash: string }
+          | undefined
+      )?.hash,
+  );
+}
+
+describe('Issue #2441: a scoped build that fails to parse a changed file', () => {
+  let tmpDir: string;
+  let dbPath: string;
+
+  beforeEach(async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-2441-'));
+    writeProject(tmpDir);
+    await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine: 'wasm' });
+    dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
+
+    expect(readSymbolNames(dbPath, 'caller.js')).toContain('run');
+    expect(readIncomingCalls(dbPath, 'target.js', 'target')).toEqual(['caller.js:run']);
+    expect(hasFileHashRow(dbPath, 'caller.js')).toBe(true);
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it('does not commit a hash for the file, so a later scoped build recovers it', async () => {
+    const originalHash = readFileHash(dbPath, 'caller.js');
+    expect(originalHash).toBeDefined();
+
+    const callerAbs = path.join(tmpDir, 'caller.js');
+    // Change the file's on-disk content so change detection classifies it as
+    // changed, then force ITS parse specifically to fail (worker crash /
+    // soft error) — `WasmWorkerPool.parse` returning null is exactly what
+    // `parseFilesWasm` treats as "this file produced no output", the same
+    // signal a real crash or unsupported grammar would produce.
+    fs.writeFileSync(
+      callerAbs,
+      "import { target } from './target.js';\nexport function run() { return target() + 1; }\n",
+    );
+
+    const pool = getWasmWorkerPool();
+    const actualParse = pool.parse.bind(pool);
+    vi.spyOn(pool, 'parse').mockImplementation(async (filePath, code, opts) => {
+      if (filePath === callerAbs) return null;
+      return actualParse(filePath, code, opts);
+    });
+
+    await buildGraph(tmpDir, {
+      incremental: true,
+      skipRegistry: true,
+      engine: 'wasm',
+      scope: ['caller.js'],
+    });
+
+    vi.restoreAllMocks();
+
+    // The purge already ran before the failed parse — caller.js's old data
+    // is gone. That half of the current behavior is unchanged by this fix
+    // (the issue's own "stronger" lazy-purge option is a separate, larger
+    // change) — what matters is that the file is NOT silently marked up to
+    // date with nothing to show for it: the row survives unchanged (#1731's
+    // deferred-commit design left it alone before the purge ever ran), so it
+    // no longer matches the file's actual current content.
+    expect(readSymbolNames(dbPath, 'caller.js')).toEqual([]);
+    expect(readFileHash(dbPath, 'caller.js')).toBe(originalHash);
+
+    // A subsequent scoped build, with parsing succeeding normally this
+    // time, must actually reprocess the file rather than fast-skipping it
+    // as "unchanged" — proving the data loss is recoverable, not permanent.
+    await buildGraph(tmpDir, {
+      incremental: true,
+      skipRegistry: true,
+      engine: 'wasm',
+      scope: ['caller.js'],
+    });
+
+    expect(readSymbolNames(dbPath, 'caller.js')).toContain('run');
+    expect(readIncomingCalls(dbPath, 'target.js', 'target')).toEqual(['caller.js:run']);
+    expect(readFileHash(dbPath, 'caller.js')).not.toBe(originalHash);
+  });
+
+  it('still commits the hash on a successful scoped build', async () => {
+    // Counterpart sanity check: the fix must not withhold the hash from a
+    // file that parsed fine — only from one that genuinely produced nothing.
+    writeProject(tmpDir, { callsTarget: false });
+
+    await buildGraph(tmpDir, {
+      incremental: true,
+      skipRegistry: true,
+      engine: 'wasm',
+      scope: ['caller.js'],
+    });
+
+    expect(readSymbolNames(dbPath, 'caller.js')).toContain('run');
+    expect(readIncomingCalls(dbPath, 'target.js', 'target')).toEqual([]);
+    expect(hasFileHashRow(dbPath, 'caller.js')).toBe(true);
+  });
+});