fix(incremental): bugbot review + CI test failures

Bugbot (PR #1479):
- Medium: pruneCache was exported but never called -> cache grew
  unbounded. Wire pruneCache into run-analyze before saveParseCache,
  using a transient usedKeys Set on ParseCache that the parse phase
  populates as it processes chunks.
- Low: willTryIncremental (pre-pipeline) and isIncremental
  (post-pipeline) could desync, silently dropping embeddings on
  mispredicted runs. Removed the prediction; the embedding cache
  now loads unconditionally when shouldLoadCache is true. The
  re-insert step gates on the actual isIncremental value to avoid
  PK-conflicts when the incremental-writeback path keeps DB rows.

CI test failures:
- cli-e2e #1169 + run-analyze.test.ts #1233: my dirty-tree gate on
  the lastCommit==HEAD early-return saw GitNexus's own auto-generated
  outputs (.claude/, .cursor/, AGENTS.md, CLAUDE.md) as dirty,
  perpetually defeating the up-to-date fast path. Extended the
  pathspec exclusion to cover all auto-gen outputs, not just
  .gitnexus/.
- ruby field-type disambig: my chunk-stability sort exposed a
  pre-existing order-dependency in Ruby cross-file resolution
  (`user.address.save -> Address#save` only resolves correctly when
  user.rb parses before address.rb in some configurations). Removed
  the sort. Filesystem ordering is stable enough in practice that
  the parse cache still hits the common case; the pre-existing
  fragility is left for a separate fix.
- pipeline-graph-golden: regenerated. Seeded Leiden RNG produces a
  partition different from the previous Math.random snapshot.
- staleness `parallel calls` was a CI timing flake; passes locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
abhigyanpatwari 2026-05-10 18:10:52 +05:30
parent 812e828362
commit 5eb05977e1
3 changed files with 94 additions and 32 deletions

View file

@ -163,13 +163,14 @@ export async function runChunkedParseAndResolve(
);
}
// Sort by path so chunk membership is stable across runs even when
// the filesystem returns scan order non-deterministically. Without
// this, the parse cache misses on every run because chunk boundaries
// shift even when no source file content has changed. Sort is
// ascending alphabetical — the comparator works for both POSIX and
// Windows path separators since both are `string` in JS.
parseableScanned.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0));
// We previously sorted parseableScanned alphabetically here for stable
// chunk membership across runs (so the parse cache wouldn't miss when
// filesystem-scan order varied). Removed because it surfaced a
// pre-existing order-dependency in Ruby cross-file resolution
// (`user.address.save → Address#save` resolution depends on file
// processing order — a separate bug to fix). Filesystem order on most
// platforms is stable enough in practice that the cache still hits the
// common case; runs where it doesn't simply pay a cold-parse cost.
const totalParseable = parseableScanned.length;
@ -337,6 +338,11 @@ export async function runChunkedParseAndResolve(
let chunkWorkerData: WorkerExtractedData | null;
const cachedRaw = chunkHash ? parseCache!.entries.get(chunkHash) : undefined;
// Track every chunk hash we touched so the orchestrator can
// prune stale entries (chunks whose composition no longer
// corresponds to a live chunk in the current scan) before saving.
if (parseCache && chunkHash) parseCache.usedKeys.add(chunkHash);
if (cachedRaw && cachedRaw.length > 0) {
// Cache hit: replay the cached worker output through the same
// merge logic the live worker path uses.
@ -526,6 +532,12 @@ export async function runChunkedParseAndResolve(
astCache.clear();
}
if (isDev && parseCache && (chunkCacheHits > 0 || chunkCacheMisses > 0)) {
logger.info(
`📦 parse-cache summary: ${chunkCacheHits} chunk hit(s), ${chunkCacheMisses} miss(es) across ${numChunks} chunk(s)`,
);
}
const fullWorkerHeritageMap =
deferredWorkerHeritage.length > 0
? buildHeritageMap(deferredWorkerHeritage, ctx, getHeritageStrategyForLanguage)

View file

@ -36,7 +36,7 @@ import {
} from '../storage/repo-manager.js';
import { computeFileHashes, diffFileHashes } from '../storage/file-hash.js';
import { extractChangedSubgraph } from './incremental/subgraph-extract.js';
import { loadParseCache, saveParseCache } from '../storage/parse-cache.js';
import { loadParseCache, saveParseCache, pruneCache } from '../storage/parse-cache.js';
import {
getCurrentCommit,
getRemoteUrl,
@ -206,13 +206,38 @@ export async function runFullAnalysis(
// may have uncommitted changes. Only short-circuit when the working
// tree is also clean — otherwise fall through to the incremental
// path which will hash-diff and update only changed files.
//
// We exclude paths that GitNexus itself writes during analyze:
// .gitnexus/ — db / parse cache / meta.json
// .claude/, .cursor/ — auto-generated agent skill files
// AGENTS.md, CLAUDE.md — auto-updated stats blocks
// Counting them as dirty would perpetually defeat the up-to-date
// fast path because the previous analyze just wrote them
// (regression vs PR #1233 behavior).
const dirty = (() => {
try {
const out = execFileSync('git', ['status', '--porcelain'], {
cwd: repoPath,
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
});
const out = execFileSync(
'git',
[
'status',
'--porcelain',
'--',
'.',
':(exclude).gitnexus',
':(exclude).gitnexus/**',
':(exclude).claude',
':(exclude).claude/**',
':(exclude).cursor',
':(exclude).cursor/**',
':(exclude)AGENTS.md',
':(exclude)CLAUDE.md',
],
{
cwd: repoPath,
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
},
);
return out.trim().length > 0;
} catch {
return true; // conservative on git failure
@ -281,18 +306,15 @@ export async function runFullAnalysis(
);
}
// Predict whether this run will use the incremental DB-writeback path.
// Used to suppress the embedding cache+restore cycle in the incremental
// case (embeddings stay in the DB; re-inserting them would PK-conflict).
const willTryIncremental =
!options.force &&
!!existingMeta &&
existingMeta.schemaVersion === INCREMENTAL_SCHEMA_VERSION &&
!!existingMeta.fileHashes &&
Object.keys(existingMeta.fileHashes).length > 0 &&
repoHasGit;
if (shouldLoadCache && existingMeta && !willTryIncremental) {
// We *always* load the embedding cache when one is requested (regardless
// of the predicted `willTryIncremental`). The post-pipeline branch may
// disagree with the prediction (e.g. when the pipeline produces zero
// File nodes, `isIncremental` flips false and the full-rebuild path
// wipes the DB) — loading unconditionally is cheap insurance against
// silently dropping embeddings on a mispredicted run. The re-insert
// step gates itself on the actual `isIncremental` value to avoid
// PK-conflicts when the incremental writeback path keeps the rows.
if (shouldLoadCache && existingMeta) {
try {
progress('embeddings', 0, 'Caching embeddings...');
await initLbug(lbugPath);
@ -356,12 +378,18 @@ export async function runFullAnalysis(
const newFileHashes = await computeFileHashes(repoPath, allFilePaths);
// Decide incremental vs full at THIS point (post-pipeline, pre-DB).
// willTryIncremental was the *prediction* used to skip the embedding
// cache cycle; here we re-evaluate against the actual pipeline output.
// All eligibility conditions are checked here against the actual
// pipeline output — no separate pre-pipeline prediction to desync from
// (Bugbot review on PR #1479: a prediction that flipped post-pipeline
// could skip the embedding cache load and then take the full-rebuild
// path, silently losing embeddings).
const isIncremental =
willTryIncremental &&
existingMeta !== null &&
!options.force &&
!!existingMeta &&
existingMeta.schemaVersion === INCREMENTAL_SCHEMA_VERSION &&
!!existingMeta.fileHashes &&
Object.keys(existingMeta.fileHashes).length > 0 &&
repoHasGit &&
allFilePaths.length > 0;
const hashDiff = isIncremental
@ -449,7 +477,12 @@ export async function runFullAnalysis(
progress('fts', 90, 'Search indexes ready');
// ── Phase 3.5: Re-insert cached embeddings ────────────────────────
if (cachedEmbeddings.length > 0) {
// Skipped on the incremental path because that path keeps the
// existing DB rows in place (re-inserting cached vectors over
// surviving rows would PK-conflict). On the full-rebuild path,
// the DB was wiped, so re-inserting the cache is the mechanism
// that preserves embeddings across the rebuild.
if (cachedEmbeddings.length > 0 && !isIncremental) {
const cachedDims = cachedEmbeddings[0].embedding.length;
const { EMBEDDING_DIMS } = await import('./lbug/schema.js');
if (cachedDims !== EMBEDDING_DIMS) {
@ -642,8 +675,16 @@ export async function runFullAnalysis(
// Persist the incremental parse cache for the next run. Wraps in
// try/catch so a cache-write failure never breaks an otherwise
// successful indexing run.
// successful indexing run. Prune stale chunk-hash entries first so
// the cache file size stays bounded across runs (chunks whose
// composition no longer matches anything in the current scan are
// dead weight; the parse phase populates `usedKeys` as it processes
// chunks).
try {
const pruned = pruneCache(parseCache, parseCache.usedKeys);
if (pruned > 0) {
log(`Parse cache: pruned ${pruned} stale chunk entries`);
}
await saveParseCache(storagePath, parseCache);
} catch (e) {
log(`Warning: could not save parse cache (${(e as Error).message}); continuing.`);

View file

@ -43,6 +43,14 @@ interface ParseCacheFile {
export interface ParseCache {
version: number;
entries: Map<string, ParseWorkerResult[]>;
/**
* Hashes referenced (hit OR miss-and-stored) by the current run.
* The parse phase populates this as it processes chunks; the orchestrator
* uses it as input to `pruneCache` before saving so entries that no
* longer correspond to any chunk in the current scan are discarded.
* Transient never serialized to disk.
*/
usedKeys: Set<string>;
}
/** SHA-256 hex of a single string or buffer. */
@ -118,7 +126,7 @@ export const loadParseCache = async (storagePath: string): Promise<ParseCache> =
for (const [k, v] of Object.entries(data.entries)) {
if (Array.isArray(v)) entries.set(k, v as ParseWorkerResult[]);
}
return { version: PARSE_CACHE_VERSION, entries };
return { version: PARSE_CACHE_VERSION, entries, usedKeys: new Set<string>() };
} catch {
return emptyCache();
}
@ -161,4 +169,5 @@ export const pruneCache = (cache: ParseCache, usedHashes: ReadonlySet<string>):
const emptyCache = (): ParseCache => ({
version: PARSE_CACHE_VERSION,
entries: new Map<string, ParseWorkerResult[]>(),
usedKeys: new Set<string>(),
});