Ver Fonte

fix(extraction): decode kernel results in indexAll retry passes; self-heal wiped rows (#1541) (#1575)

The parse-pool workers return kernel-language extractions as an undecoded
buffer transport (nodes/edges EMPTY, tables in kernelBuffers). indexAll's
main loop decodes them (or hands the buffers to the store worker), but its
two retry passes — plain retry and the comments-stripped last resort —
stored the transport as-is: the storage gate passed via errors.length === 0,
zero nodes were inserted, and the files row was written with node_count = 0
while the original error was spliced out of the summary. Any worker
crash/timeout whose in-flight file was a kernel-routed language permanently
recorded that file as "(0 symbols)" — silently, and immune to later syncs
because the stored hash matches the on-disk bytes (#1541; v1.4.1 predates
the kernel path, which is why it was unaffected).

- Both retry passes now materialize kernel results before the gate, store,
  counters, and log lines.
- storeExtractionResult materializes at entry as defense-in-depth, so no
  storage path can persist an undecoded transport again.
- Zero-node rows on symbol-bearing languages (only the wipe produces these —
  every real extraction stores at least the file node) are dropped during
  full-reconcile sync and indexAll so already-affected files re-index
  automatically after upgrading. Scoped watcher syncs leave rows outside
  their scope untouched.
- The comments-stripped salvage now downgrades the failure to a visible
  warning instead of erasing it: the recovered result can be incomplete, and
  reporting clean success made a fresh index quietly disagree with a later
  per-file re-parse of the same bytes (#1565's init-vs-sync divergence).

Repro (released 1.5.0): CODEGRAPH_PARSE_TIMEOUT_MS=1 codegraph init on any
Python project → "Retry OK: <file> (0 nodes)" and permanent
"(python, 0 symbols)" rows. Fixed build stores real symbols under the same
forcing, and heals rows wiped by prior runs.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Colby Mchenry há 2 semanas atrás
pai
commit
26045b3159
3 ficheiros alterados com 207 adições e 4 exclusões
  1. 2 0
      CHANGELOG.md
  2. 150 0
      __tests__/kernel-retry-materialize.test.ts
  3. 55 4
      src/extraction/index.ts

+ 2 - 0
CHANGELOG.md

@@ -47,6 +47,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - When a `codegraph_explore` answer shows a file as excerpts, a large excerpt that no longer fit was dropped entirely instead of being shortened. If the file's first excerpt happened to be a trivial one — an import block, a one-line helper next to the code you asked about — the excerpt carrying the actual answer was the one thrown away, and the file came back with a quarter of the room it had been given. On real projects that meant the top-ranked file delivered a fraction of its share while a far less relevant file took the rest. Excerpts are now shortened to fit, whole method by whole method, and only dropped when what is left is too small to hold anything readable.
 - When you name a symbol in a `codegraph_explore` query, its definition now actually comes back. Two cases previously lost it. If the symbols you named don't call one another — sibling functions inside the same factory or module are the everyday example — CodeGraph stopped treating them as symbols you had asked for, and answered with whatever sat at the top of their file instead; on one 1,400-line file that meant a same-stem `QueuedMessage` interface on line 70 came back while the `queueMessage` function on line 1087 did not. And when an answer had to be trimmed to fit, it was trimmed from the bottom of the file down, so a symbol near the end of a long file was always the first thing cut. Trimming now protects the definitions you named wherever they sit in the file.
 - The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475)
+- Fixed a v1.5.0 regression where a perfectly valid file could be permanently recorded as having 0 symbols, with no error reported. When a file's first parse attempt was interrupted — a parsing worker crash or timeout, most likely on slow or heavily loaded machines — the automatic retry stored an empty result for any language on the native extraction path, so the file's functions and classes silently vanished from search, callers, and impact until the file was next edited. Retries now store the file's real symbols, and a file already recorded as symbol-free is detected and repaired automatically by the next sync or re-index after upgrading. Thanks @Baiae for the report. (#1541)
+- When indexing has to fall back to parsing a file with its comment lines stripped — a last-resort recovery after repeated parser crashes — the file is now flagged with a visible warning instead of being reported as cleanly indexed. The recovered result can be incomplete, and reporting success made a fresh index quietly disagree with a later re-parse of the same unchanged file. Thanks @jeremypetz for the precise init-versus-sync symbol accounting that exposed this. (#1565)
 
 ## [1.5.0] - 2026-07-21
 

+ 150 - 0
__tests__/kernel-retry-materialize.test.ts

@@ -0,0 +1,150 @@
+/**
+ * Kernel results must be DECODED before they are persisted (#1541).
+ *
+ * The bulk-index parse workers return kernel extractions as an undecoded
+ * buffer transport: `nodes`/`edges`/`unresolvedReferences` are EMPTY and the
+ * real tables ride in `kernelBuffers`. The main loop decodes (or hands the
+ * buffers to the store worker), but indexAll's retry passes used to store the
+ * transport as-is — the storage gate passed via `errors.length === 0`, zero
+ * nodes were inserted, and the file was permanently recorded as
+ * "(0 symbols)" with the retry counted as a success. Any worker
+ * crash/timeout whose in-flight file was a kernel-routed language silently
+ * wiped that file's symbols (issue #1541: v1.5.0 indexes a valid Python file
+ * as 0 symbols; v1.4.1, pre-kernel, indexed it correctly).
+ *
+ * This pins the store boundary: storeExtractionResult must materialize a
+ * buffer-transport result before persisting, so every caller — including the
+ * retry passes — stores the real nodes.
+ *
+ * Skips when no kernel binary is staged (same gating as the parity suites).
+ */
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import { CodeGraph } from '../src';
+import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars';
+import { tryKernelExtractRaw } from '../src/extraction/kernel';
+import type { ExtractionResult } from '../src/types';
+
+const KERNEL_PATH = path.join(
+  __dirname,
+  '..',
+  'codegraph-kernel',
+  'prebuilds',
+  `${process.platform}-${process.arch}`,
+  'codegraph-kernel.node'
+);
+const kernelBuilt = fs.existsSync(KERNEL_PATH);
+
+describe.skipIf(!kernelBuilt)('kernel buffer-transport storage (#1541)', () => {
+  let dir: string;
+  let cg: CodeGraph;
+
+  beforeEach(async () => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'kernel-retry-mat-'));
+    cg = await CodeGraph.init(dir);
+    await initGrammars();
+    await loadGrammarsForLanguages(['python']);
+  });
+
+  afterEach(() => {
+    cg.destroy();
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('storeExtractionResult persists the decoded nodes of a raw kernel result', async () => {
+    const source =
+      'def target_fn(root, mission_path):\n' +
+      '    return (root, mission_path)\n' +
+      '\n' +
+      'class Adapter:\n' +
+      '    def adapt(self):\n' +
+      '        return target_fn(1, 2)\n';
+    const filePath = 'adapter.py';
+    fs.writeFileSync(path.join(dir, filePath), source);
+
+    // A genuine undecoded transport, exactly as parse-worker builds it.
+    const raw = tryKernelExtractRaw(filePath, source, 'python');
+    expect(raw).not.toBeNull();
+    expect(raw!.counts.nodes).toBeGreaterThan(0);
+    const transport: ExtractionResult = {
+      nodes: [],
+      edges: [],
+      unresolvedReferences: [],
+      errors: raw!.errors,
+      durationMs: 0,
+      kernelBuffers: raw!.buffers,
+      kernelCounts: raw!.counts,
+    };
+
+    const stats = fs.statSync(path.join(dir, filePath));
+    const orchestrator = (cg as unknown as { orchestrator: { storeExtractionResult(f: string, c: string, l: string, s: fs.Stats, r: ExtractionResult): Promise<void> } }).orchestrator;
+    await orchestrator.storeExtractionResult(filePath, source, 'python', stats, transport);
+
+    // The files row must carry the real symbol count, not the transport's
+    // empty array — a 0 here is the #1541 "(python, 0 symbols)" wipe.
+    const file = cg.getFile(filePath);
+    expect(file).not.toBeNull();
+    expect(file!.nodeCount).toBe(raw!.counts.nodes);
+
+    // And the nodes themselves must be queryable.
+    const nodes = cg.getNodesInFile(filePath);
+    expect(nodes.length).toBe(raw!.counts.nodes);
+    expect(nodes.map((n) => n.name)).toContain('target_fn');
+    expect(nodes.map((n) => n.name)).toContain('Adapter');
+  });
+});
+
+/**
+ * Self-heal for rows the released bug already wiped: a files row recorded
+ * with zero nodes on a symbol-bearing language can only be a #1541 casualty
+ * (every real extraction stores at least the file node), and its content
+ * hash matches the on-disk bytes, so hash-based reconciles skip it forever.
+ * The full-reconcile sync and indexAll now drop such rows so the file
+ * re-indexes. Kernel-independent — the wipe is simulated at the DB.
+ */
+describe('zero-node row self-heal (#1541)', () => {
+  let dir: string;
+  let cg: CodeGraph;
+
+  beforeEach(async () => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'zero-node-heal-'));
+    fs.writeFileSync(
+      path.join(dir, 'adapter.py'),
+      'def target_fn(root, mission_path):\n' +
+        '    return (root, mission_path)\n' +
+        '\n' +
+        'class Adapter:\n' +
+        '    def adapt(self):\n' +
+        '        return target_fn(1, 2)\n'
+    );
+    cg = await CodeGraph.init(dir);
+    await cg.indexAll();
+  });
+
+  afterEach(() => {
+    cg.destroy();
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('sync repairs a wiped row even though the content hash is unchanged', async () => {
+    const before = cg.getFile('adapter.py');
+    expect(before).not.toBeNull();
+    expect(before!.nodeCount).toBeGreaterThan(0);
+
+    // Simulate the released-v1.5.0 wipe: nodes gone, row says 0 symbols,
+    // content hash still matching the file on disk.
+    const db = (cg as unknown as { db: { getDb(): { prepare(sql: string): { run(...args: unknown[]): unknown } } } }).db.getDb();
+    db.prepare('DELETE FROM nodes WHERE file_path = ?').run('adapter.py');
+    db.prepare('UPDATE files SET node_count = 0 WHERE path = ?').run('adapter.py');
+    expect(cg.getFile('adapter.py')!.nodeCount).toBe(0);
+
+    await cg.sync();
+
+    const after = cg.getFile('adapter.py');
+    expect(after).not.toBeNull();
+    expect(after!.nodeCount).toBe(before!.nodeCount);
+    expect(cg.getNodesInFile('adapter.py').map((n) => n.name)).toContain('target_fn');
+  });
+});

+ 55 - 4
src/extraction/index.ts

@@ -1577,6 +1577,11 @@ export class ExtractionOrchestrator {
     });
     if (process.env.CODEGRAPH_SYNTH_TIMINGS) console.error(`[phase-timing] scan: ${Date.now() - tScan}ms (${files.length} files)`);
 
+    // A re-index over an existing DB skips unchanged-hash files at the store,
+    // which would preserve wiped zero-node rows (#1541) — drop them first so
+    // this run stores their files fresh. No-op on a fresh DB.
+    this.healZeroNodeRows();
+
     // Detect frameworks once per indexAll run using the scanned file list.
     // Names are passed to each parse call so framework-specific extractors
     // (route nodes, middleware, etc.) run after the tree-sitter pass.
@@ -2025,8 +2030,16 @@ export class ExtractionOrchestrator {
           continue;
         }
 
+        // The pool hands kernel results back as an undecoded buffer transport
+        // (`nodes`/`edges` EMPTY, tables in kernelBuffers). The main loop
+        // decodes or forwards to the store worker; this path stores directly,
+        // so decode here — otherwise a kernel-language retry passes the gate
+        // below via `errors.length === 0`, stores nothing, and the file is
+        // permanently recorded as "(0 symbols)" with the error erased (#1541).
+        const language = detectLanguage(filePath, content, overrides);
+        result = materializeKernelResult(result, filePath, language);
+
         if (result.nodes.length > 0 || result.errors.length === 0) {
-          const language = detectLanguage(filePath, content, overrides);
           const stats = await fsp.stat(path.join(this.rootDir, filePath));
           await this.storeExtractionResult(filePath, content, language, stats, result, commitYield);
 
@@ -2075,13 +2088,21 @@ export class ExtractionOrchestrator {
             continue;
           }
 
+          // Same undecoded-transport hazard as the first retry pass (#1541).
+          const language = detectLanguage(filePath, fullContent, overrides);
+          result = materializeKernelResult(result, filePath, language);
+
           if (result.nodes.length > 0 || result.errors.length === 0) {
-            const language = detectLanguage(filePath, fullContent, overrides);
             const stats = await fsp.stat(path.join(this.rootDir, filePath));
             await this.storeExtractionResult(filePath, fullContent, language, stats, result, commitYield);
 
-            const idx = errors.indexOf(errEntry);
-            if (idx >= 0) errors.splice(idx, 1);
+            // Salvaged from comment-stripped source: keep a visible trace in
+            // the summary instead of erasing the failure outright — the
+            // stored result may be missing whatever the failing parse choked
+            // on, and a silently "clean" file here is how an index quietly
+            // disagrees with a later per-file sync of the same bytes (#1565).
+            errEntry.severity = 'warning';
+            errEntry.message = `Indexed from comment-stripped source after repeated parse failures (symbols may be incomplete until the file is re-indexed): ${errEntry.message}`;
             filesErrored--;
             filesIndexed++;
             totalNodes += result.nodes.length;
@@ -2267,6 +2288,26 @@ export class ExtractionOrchestrator {
   /**
    * Store extraction result in database
    */
+  /**
+   * Delete file rows recorded with ZERO nodes so their files re-index.
+   *
+   * No extraction path stores an empty, error-free result for a
+   * symbol-bearing language — even an empty file keeps its file node — so a
+   * zero-node row is a wiped one (#1541: an interrupted parse's retry stored
+   * an undecoded kernel transport). The wiped row's content hash matches the
+   * on-disk bytes, so every hash-based reconcile skips the file forever;
+   * deleting the row lets the normal add path repair it. File-level-only
+   * languages (yaml, twig, properties) are left alone. Deleting a zero-node
+   * row cascades nothing: it has no nodes, so no edges or refs either.
+   */
+  private healZeroNodeRows(): void {
+    for (const f of this.queries.getAllFiles()) {
+      if (f.nodeCount === 0 && !isFileLevelOnlyLanguage(f.language)) {
+        this.queries.deleteFile(f.path);
+      }
+    }
+  }
+
   private async storeExtractionResult(
     filePath: string,
     content: string,
@@ -2275,6 +2316,12 @@ export class ExtractionOrchestrator {
     result: ExtractionResult,
     onYield?: MaybeYield
   ): Promise<void> {
+    // A kernel result can arrive as an undecoded buffer transport (empty
+    // node/edge arrays, tables riding in kernelBuffers). Decode it before
+    // storing — persisting the transport as-is records the file as having no
+    // symbols at all (#1541). No-op for already-decoded results.
+    result = materializeKernelResult(result, filePath, language);
+
     // Bulk inserts run in bounded sub-transactions with a yield between, so a
     // giant generated file (tens of thousands of symbols) can't block the
     // event loop — and the #850 watchdog heartbeat — for the whole store.
@@ -2636,6 +2683,10 @@ export class ExtractionOrchestrator {
       if (process.env.CODEGRAPH_SYNTH_TIMINGS) console.error(`[phase-timing] sync-scan: ${Date.now() - tSyncScan}ms (${currentFiles.length} files)`);
       filesChecked = currentFiles.length;
 
+      // Full reconcile only (scoped syncs must not touch rows outside their
+      // scope): drop zero-node rows so the wiped files re-index as adds below.
+      this.healZeroNodeRows();
+
       const tTracked = Date.now();
       trackedFiles = this.queries.getAllFiles();
       if (process.env.CODEGRAPH_SYNTH_TIMINGS) console.error(`[phase-timing] sync-tracked-load: ${Date.now() - tTracked}ms (${trackedFiles.length} tracked)`);