Jelajahi Sumber

CG-35: give the sync-convergence suite teeth against the rebind pass

The suite passed unchanged with `CODEGRAPH_NO_REBIND=1`, so the larger half
of CG-33 — the rebind pass — had no coverage at all.

The cause was the ground truth, not the cases: `rebuildEdgeSet` called
`indexAll()` on the live handle. That is not a rebuild. Every file hashes
identical, so the store writes nothing (`nodesCreated: 0`), no reference is
re-created, and every edge survives — the comparison read the synced index
against itself and could never fail. It now goes through `CodeGraph.recreate`,
which deletes the database file the way the CLI's `index` command does.

With a real rebuild, three existing cases fail under the kill switch. Adds two
more for the rules that carry the risk:

- an edge with no `refName` stamp (older engine) and a synthesized
  (`provenance='heuristic'`) edge are never deleted — both planted directly,
  and each verified load-bearing by mutation;
- a name over the 500-edge ceiling is declined losslessly rather than
  rebound in part, with a rare name in the same sync as the control that
  proves the pass ran.

The per-file-vs-batch-wide delta rule is likewise confirmed by mutation: a
batch-wide name set fails its case.

CODEGRAPH_NO_REBIND=1 now fails 4 cases; unset is green; full suite green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Colby McHenry 1 bulan lalu
induk
melakukan
02ee151e46
1 mengubah file dengan 171 tambahan dan 2 penghapusan
  1. 171 2
      __tests__/sync-rebuild-convergence.test.ts

+ 171 - 2
__tests__/sync-rebuild-convergence.test.ts

@@ -18,6 +18,21 @@
  * The assertions here compare the whole edge SET, never counts: the divergence
  * is bidirectional and nets out of a total (raw rows differed by 0.7% while
  * 4.3% of edges were wrong), so a count check passes on a broken index.
+ *
+ * ---
+ *
+ * THIS SUITE MUST FAIL WITH `CODEGRAPH_NO_REBIND=1` (CG-35).
+ *
+ * That environment variable is the kill switch on the rebind half of the fix
+ * (`src/index.ts`, guarding `resurrectStaleResolutionEdges`). The convergence
+ * cases below are the only coverage that half has, so the check is the suite's
+ * own regression test:
+ *
+ *     CODEGRAPH_NO_REBIND=1 npx vitest run __tests__/sync-rebuild-convergence.test.ts
+ *
+ * must report failures, and an unset run must be green. If you change a case
+ * here, re-run both. A version of this suite passed under the kill switch
+ * because `rebuildEdgeSet` was not rebuilding anything — see the note there.
  */
 
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
@@ -57,6 +72,21 @@ describe('Incremental sync converges to a full rebuild (CG-33)', () => {
     }
   };
 
+  /**
+   * Run `fn` against a second, WRITABLE connection to the same database. Used
+   * by the two rule tests below to plant edge shapes the extractor cannot
+   * produce on demand — an edge from an engine older than the refName stamp,
+   * and a synthesized dispatch edge.
+   */
+  const withDb = <T>(fn: (db: ReturnType<typeof createDatabase>['db']) => T): T => {
+    const { db } = createDatabase(path.join(testDir, '.codegraph', 'codegraph.db'));
+    try {
+      return fn(db);
+    } finally {
+      db.close();
+    }
+  };
+
   /** Human-readable diff, so a failure names the edges instead of just a count. */
   const describeDiff = (synced: Set<string>, rebuilt: Set<string>): string => {
     const missing = [...rebuilt].filter((e) => !synced.has(e));
@@ -66,10 +96,21 @@ describe('Incremental sync converges to a full rebuild (CG-33)', () => {
 
   /**
    * Rebuild the index from scratch over the CURRENT tree and return its edge
-   * set. `indexAll` recreates the database file, so this is the same ground
-   * truth a user gets from `codegraph index`.
+   * set — the ground truth a user gets from `codegraph index`.
+   *
+   * It must go through `CodeGraph.recreate`, which is what the CLI's `index`
+   * command does: it DELETES the database file and builds an empty one. Calling
+   * `indexAll` on the live handle instead is not a rebuild at all — every file
+   * hashes identical, so the store writes nothing (`nodesCreated: 0`), no
+   * reference is re-created, and every existing edge survives untouched. The
+   * comparison then reads the synced index against ITSELF and can never fail,
+   * which is exactly how this suite passed with `CODEGRAPH_NO_REBIND=1` (CG-35).
    */
   const rebuildEdgeSet = async (): Promise<Set<string>> => {
+    // Close the live handle first: `recreate` unlinks the database file, and a
+    // held handle makes that EBUSY on Windows.
+    cg.destroy();
+    cg = await CodeGraph.recreate(testDir);
     await cg.indexAll();
     return edgeSet();
   };
@@ -206,6 +247,134 @@ describe('Incremental sync converges to a full rebuild (CG-33)', () => {
     expect(describeDiff(synced, rebuilt)).toBe('missing from synced: 0, stale in synced: 0');
   });
 
+  /**
+   * The rebind pass DELETES an edge and re-inserts the reference behind it, so
+   * it may only touch edges it can reconstruct. Two shapes it must leave alone,
+   * both of which it would otherwise destroy permanently:
+   *
+   * - an edge with no `metadata.refName` — written by an engine older than the
+   *   stamp. Rebuilding a reference from the target's plain name would strip the
+   *   receiver context the original text carried (`h.greet` → `greet`);
+   * - a synthesized dispatch edge (`provenance='heuristic'`), which is not
+   *   resolution output at all: nothing would re-create it, and the synthesizer
+   *   that wired it does not run again on this sync.
+   *
+   * Both are planted directly, since extraction cannot be asked to emit them.
+   * The sync then changes the answer for `pct`, which is exactly the condition
+   * that makes the pass want to re-open every edge targeting `pct`.
+   */
+  it('never deletes an edge it cannot reconstruct — no refName stamp, or synthesized', async () => {
+    write('src/caller.ts', `export function run(): number {\n  return pct(1);\n}\n`);
+    write('src/other.ts', `export function other(): number {\n  return 0;\n}\n`);
+    write('src/zeta.ts', `export function pct(n: number): number {\n  return n;\n}\n`);
+    cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } });
+    await cg.indexAll();
+
+    const planted = withDb((db) => {
+      const pct = db.prepare("SELECT id FROM nodes WHERE name = 'pct'").get() as { id: string };
+      const other = db.prepare("SELECT id FROM nodes WHERE name = 'other'").get() as { id: string };
+
+      // 1. Strip the stamp off the real edge, leaving the rest of its metadata
+      //    intact — the shape an index built before the stamp existed has.
+      db.prepare(
+        `UPDATE edges SET metadata = json_remove(metadata, '$.refName')
+         WHERE target = ? AND kind = 'calls'`
+      ).run(pct.id);
+
+      // 2. A synthesized edge that DOES carry a stamp, so only the provenance
+      //    rule can save it.
+      db.prepare(
+        `INSERT INTO edges (source, target, kind, metadata, line, col, provenance)
+         VALUES (?, ?, 'calls', ?, 1, 0, 'heuristic')`
+      ).run(other.id, pct.id, JSON.stringify({ refName: 'pct', synthesizedBy: 'cg35-test' }));
+
+      return {
+        unstamped: `${(db.prepare("SELECT source FROM edges WHERE target = ? AND provenance IS NULL AND kind = 'calls'").get(pct.id) as { source: string }).source}|${pct.id}|calls`,
+        synthesized: `${other.id}|${pct.id}|calls`,
+      };
+    });
+
+    const before = edgeSet();
+    expect(before.has(planted.unstamped)).toBe(true);
+    expect(before.has(planted.synthesized)).toBe(true);
+
+    write('src/alpha.ts', `export function pct(n: number): number {\n  return n * 2;\n}\n`);
+    const result = await cg.sync();
+    expect(result.definitionDelta).toContain('pct');
+
+    // Both survive: the pass considered them (their target is `pct`) and
+    // declined. Drift is the acceptable outcome here; an edge that no pass can
+    // ever restore is not.
+    const after = edgeSet();
+    expect(after.has(planted.unstamped)).toBe(true);
+    expect(after.has(planted.synthesized)).toBe(true);
+  });
+
+  /**
+   * The per-name ceiling in `getResolutionEdgesByTargetName` (500 by default).
+   * Above it a name is generic — `push`, `get`, `join` — one new definition
+   * won't flip most of its references, and rebinding an arbitrary subset would
+   * manufacture wrong edges while costing the most work. It must DECLINE the
+   * name outright, and declining must be lossless.
+   *
+   * The rare name in the same sync is the control: it proves the pass ran and
+   * that the ceiling is what spared the generic one, not a dead rebind pass.
+   */
+  it('declines a name over the per-name ceiling instead of rebinding an arbitrary subset', async () => {
+    // Must exceed the 500 default in getResolutionEdgesByTargetName.
+    const OVER_CEILING = 501;
+    const callers = Array.from(
+      { length: OVER_CEILING },
+      (_, i) => `export function hot${i}(): number {\n  return push(${i});\n}\n`
+    ).join('');
+    write('src/hot.ts', callers);
+    write('src/rare.ts', `export function rare(): number {\n  return tug(1);\n}\n`);
+    write(
+      'src/zzz_defs.ts',
+      `export function push(n: number): number {\n  return n;\n}\nexport function tug(n: number): number {\n  return n;\n}\n`
+    );
+    cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } });
+    await cg.indexAll();
+
+    const targetsOf = (name: string): string[] =>
+      withDb((db) =>
+        (
+          db
+            .prepare(
+              `SELECT t.file_path AS file FROM edges e
+               JOIN nodes t ON t.id = e.target
+               JOIN nodes s ON s.id = e.source
+               WHERE t.name = ? AND e.kind = 'calls'`
+            )
+            .all(name) as Array<{ file: string }>
+        ).map((r) => r.file)
+      );
+
+    expect(targetsOf('push')).toHaveLength(OVER_CEILING);
+    expect(new Set(targetsOf('push'))).toEqual(new Set(['src/zzz_defs.ts']));
+    expect(targetsOf('tug')).toEqual(['src/zzz_defs.ts']);
+
+    // One sync adds a competing definition of BOTH names, in a file that sorts
+    // first and is therefore the rebuild's answer for each.
+    write(
+      'src/aaa.ts',
+      `export function push(n: number): number {\n  return n * 2;\n}\nexport function tug(n: number): number {\n  return n * 2;\n}\n`
+    );
+    const result = await cg.sync();
+    expect(result.definitionDelta).toContain('push');
+    expect(result.definitionDelta).toContain('tug');
+
+    // `push` is untouched — every edge still there, still on the old target.
+    // This is knowingly divergent from a rebuild; see "Don't chase the
+    // residual" in docs/benchmarks/index-drift-cg33.md.
+    const pushTargets = targetsOf('push');
+    expect(pushTargets).toHaveLength(OVER_CEILING);
+    expect(new Set(pushTargets)).toEqual(new Set(['src/zzz_defs.ts']));
+
+    // `tug` — the control — rebound.
+    expect(targetsOf('tug')).toEqual(['src/aaa.ts']);
+  });
+
   /**
    * Guards the escape hatch itself: with the rebind pass off, the same sequence
    * must still produce a structurally sound index (no lost or orphaned edges) —