fix(perf): guard post-native passes on 1-file incremental rebuilds

[carlos-alm]

Native 1-file incremental rebuilds were 11–14x slower than WASM (1901ms vs 171ms) with ~1823ms unaccounted in phase timings. Two post-native passes ran unconditionally even when they had no work to do:

• backfillNativeDroppedFiles call gate: Previously called whenever changedCount > 0 (i.e. any incremental with changes), even when detectDroppedLanguageGap returned an empty gap. backfillNativeDroppedFiles already has an internal early-exit for empty gaps, but removing the spurious outer conditions (isFullBuild || removedCount > 0 || changedCount > 0) eliminates the wasted call entirely. Now only called when gap.missingAbs.length > 0 || gap.staleRel.length > 0.

• Node/edge COUNT(*) re-count: Ran unconditionally after all post-passes, even when none of them wrote any new edges. COUNT(*) over 50K+ edge tables is non-trivial, especially via the NativeDbProxy napi-rs round-trip. Now gated on postPassWroteData — true only when the backfill, CHA expansion, or this/super dispatch actually inserted edges. On a typical 1-file incremental (no class hierarchy, no WASM-only files), all three are zero — the scan is skipped entirely.

Combined with the related fixes already in this branch stack (phase timings visibility #1434, CHA incremental scoping #1441, symbolsOnly inline #1437, threshold #1435), native 1-file incremental should approach the expected ~43ms from v3.9.6.

Closes #1454

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review: fix(perf): guard post-native passes on 1-file incremental rebuilds

• Gather context
• Read changed file
• Root cause analysis
• Code quality review
• Backlog compliance assessment
• Post final review

---

Root Cause Analysis

Problem Being Solved: Native 1-file incremental rebuilds were 11–14x slower than WASM (~1901ms vs ~171ms), with ~1823ms unaccounted by phase timings. Two JS post-native passes executed unconditionally even when they had zero work:

1. backfillNativeDroppedFiles was called when isFullBuild || removedCount > 0 || changedCount > 0, which covers every meaningful incremental. The function already guards its body with an early return for empty gaps, but the outer call overhead (including going through NativeDbProxy) was still paid.
2. COUNT(*) FROM nodes + COUNT(*) FROM edges ran unconditionally after all post-passes — even when no post-pass wrote a single edge. On a 50K+ edge table via the napi-rs NativeDbProxy round-trip, this was non-trivial.

Why This Approach: The fix is the minimal correct change. For optimization (1): the redundant isFullBuild || removedCount > 0 || changedCount > 0 conditions never provided useful signal for gating the backfill — the real gate is whether gap.missingAbs.length > 0 || gap.staleRel.length > 0. The PR removes the false outer condition and gates directly on the gap content, which was already being detected unconditionally anyway. For optimization (2): a new postPassWroteData boolean correctly aggregates whether any of the three post-passes produced output, and the expensive scan is wrapped in that guard.

Risk Assessment: Low. Both changes are pure skips of work that was already no-ops in the common 1-file incremental case:

• The backfill's internal early-exit (if (missingAbs.length === 0 && staleRel.length === 0) return) was already correct; the outer gate just removes the wasted call overhead.
• The backfillHappened flag mirrors the gap.missingAbs.length > 0 || gap.staleRel.length > 0 expression already evaluated one line above — no new logic, just naming.
• For the COUNT gate: chaEdgeCount comes from runPostNativeCha's return value; thisDispatchTargetIds.size > 0 already existed and was used for role re-classification gating. Reusing both for the recount gate is consistent with existing semantics.

One subtle point: backfillEdgeTechniquesAfterNativeOrchestrator is NOT included in postPassWroteData. The technique backfill UPDATEs existing calls edges rather than INSERTing new ones, so it doesn't change counts — this is correct.

---

Code Quality

Change 1: backfill gate (native-orchestrator.ts:1535-1538)

Before:

if (result.isFullBuild || removedCount > 0 || changedCount > 0 || gap.missingAbs.length > 0 || gap.staleRel.length > 0) {
  await backfillNativeDroppedFiles(ctx, gap);
}

After:

if (gap.missingAbs.length > 0 || gap.staleRel.length > 0) {
  await backfillNativeDroppedFiles(ctx, gap);
}
const backfillHappened = gap.missingAbs.length > 0 || gap.staleRel.length > 0;

✓ Correct — the old conditions were purely redundant; the function always no-oped on empty gaps anyway.

⚠️ Minor: backfillHappened duplicates the condition already evaluated on the previous line. A micro-cleanup would be:

const backfillHappened = gap.missingAbs.length > 0 || gap.staleRel.length > 0;
if (backfillHappened) {
  await backfillNativeDroppedFiles(ctx, gap);
}

This avoids evaluating the same boolean expression twice. Not a bug — just a readability nit. Consistent with the code's style elsewhere (e.g. the earlyExit gap check at line 1432–1435 already does it in this order).

Change 2: COUNT(*) gate (native-orchestrator.ts:1624–1638)

const postPassWroteData = backfillHappened || chaEdgeCount > 0 || thisDispatchTargetIds.size > 0;
if (postPassWroteData) { ... }

✓ All three sources of new data are correctly tracked.
✓ The technique backfill (UPDATEs only) is correctly excluded from the gate.
✓ Consistent with the existing role re-classification gate at line 1578 which also uses chaEdgeCount > 0 || thisDispatchTargetIds.size > 0.
✓ Stale counts are NOT an issue: when postPassWroteData is false, the Rust orchestrator's counts (result.nodeCount, result.edgeCount) are authoritative since no JS pass touched the DB.

Removed variables (removedCount, changedCount): Cleanly removed from the diff — they were only used for the old gate condition. ✓

Comment quality: Updated comments accurately describe the new behavior and rationale, including the NativeDbProxy napi-rs round-trip cost explanation. ✓

---

Configuration Change Detection

No linting rules, test timeouts, coverage thresholds, or TypeScript flags were modified. ✓

Test Integrity

No test files changed. ✓ The behavior being gated (calling backfill or COUNT scan) is an internal optimization — existing integration tests covering full-build and incremental correctness still exercise the code paths when gaps exist.

---

Backlog Compliance

• Zero-dep: ✓ — no new npm dependencies
• Foundation-aligned: ✓ — pure local performance optimization, no cloud calls, no new infrastructure
• Problem-fit: 4/5 — Directly reduces AI agent round-trip latency for incremental graph queries. A 14x slowdown on 1-file incrementals is a real user-visible regression that makes codegraph feel broken for frequent use in watch-mode or hook-triggered rebuilds (exactly the agentic context this tool is built for).
• Breaking: No — purely internal; same DB state, same output, same API contract
• Tier: 1

---

Final Recommendation

• Rating: ⭐⭐⭐⭐ (4/5)
• Action: APPROVE (with nit)
• Reasoning: The fix is correct, minimal, well-reasoned, and addresses a real performance cliff. The only nit is the duplicated boolean expression for backfillHappened (see Change 1 above) — cosmetic, not a bug. The PR description accurately explains both fixes and their relationship to the broader branch stack. No rules relaxed, no tests weakened, no new deps.

Optional cleanup (not blocking): Hoist the backfillHappened expression to avoid the duplicated condition:

// native-orchestrator.ts ~L1535
const backfillHappened = gap.missingAbs.length > 0 || gap.staleRel.length > 0;
if (backfillHappened) {
  await backfillNativeDroppedFiles(ctx, gap);
}

[greptile-apps]

Greptile Summary

Guards two post-native passes that were running unconditionally on every incremental build — backfillNativeDroppedFiles and the node/edge COUNT(*) re-count — behind their respective preconditions, eliminating 1800+ ms of wasted work on typical 1-file incrementals. A follow-up commit also hoists backfillHappened before the if guard to avoid the duplicate boolean expression flagged in review.

• backfillNativeDroppedFiles gate: removed the broad isFullBuild || removedCount > 0 || changedCount > 0 outer condition; the function is now called only when gap.missingAbs.length > 0 || gap.staleRel.length > 0, which is the true precondition for any work to do.
• COUNT(*) re-count gate: introduced postPassWroteData (backfillHappened || chaEdgeCount > 0 || thisDispatchTargetIds.size > 0) so the expensive SQLite scan over 50K+ rows is skipped entirely when all three edge-writing post-passes were no-ops.

Confidence Score: 5/5

Safe to merge — both guards are structurally correct and the follow-up hoist resolves the previously flagged duplicate expression.

The backfill gate narrows a call that had a no-op internal early-exit into one that is never made at all when the gap is empty; no correctness is lost. The postPassWroteData condition covers all three edge-writing passes (backfill, CHA, this-dispatch) and correctly excludes the technique-column backfill which only updates existing rows and cannot affect COUNT(*). The backfillHappened hoist writes the boolean expression exactly once, eliminating the out-of-sync maintenance risk.

No files require special attention.

Important Files Changed

Filename	Overview
src/domain/graph/builder/stages/native-orchestrator.ts	Two call-site guards added around backfillNativeDroppedFiles and the COUNT(*) re-count; backfillHappened hoisted before its if block. Logic is correct — postPassWroteData covers all three edge-writing passes, and techniqueBackfill (column update only, no new rows) correctly remains outside the gate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[tryNativeOrchestrator] --> B[detectDroppedLanguageGap]
    B --> C{gap non-empty?\nmissingAbs > 0\nor staleRel > 0}
    C -- "backfillHappened = true" --> D[backfillNativeDroppedFiles]
    C -- "backfillHappened = false\n(skip)" --> E
    D --> E[runPostNativeCha\n→ chaEdgeCount]
    E --> F[runPostNativeThisDispatch\n→ thisDispatchTargetIds]
    F --> G[backfillEdgeTechniquesAfterNativeOrchestrator\nalways runs — column update only]
    G --> H{postPassWroteData?\nbackfillHappened OR\nchaEdgeCount > 0 OR\nthisDispatchTargetIds.size > 0}
    H -- yes --> I["COUNT(*) nodes + edges\nupdate build_meta if changed"]
    H -- "no (1-file quiet incremental)" --> J[use Rust orchestrator counts as-is]
    I --> K[info log + return result]
    J --> K

Loading

_{Reviews (3): Last reviewed commit: "refactor(perf): hoist backfillHappened b..." | Re-trigger Greptile}

[greptile-apps]

backfillHappened re-evaluates the same boolean expression that already guards the await on the line above. If the condition ever changes (e.g. a third field is added to gap), both sites must be updated in sync. Hoist the variable before the if so the condition is written exactly once.

Suggested change

	const gapDetectStart = performance.now();
	const gap = detectDroppedLanguageGap(ctx);
	if (
	result.isFullBuild ||
	removedCount > 0 ||
	changedCount > 0 ||
	gap.missingAbs.length > 0 ||
	gap.staleRel.length > 0
	) {
	if (gap.missingAbs.length > 0 || gap.staleRel.length > 0) {
	await backfillNativeDroppedFiles(ctx, gap);
	}
	const backfillHappened = gap.missingAbs.length > 0 || gap.staleRel.length > 0;
	const gapDetectMs = performance.now() - gapDetectStart;
	const gapDetectStart = performance.now();
	const gap = detectDroppedLanguageGap(ctx);
	const backfillHappened = gap.missingAbs.length > 0 || gap.staleRel.length > 0;
	if (backfillHappened) {
	await backfillNativeDroppedFiles(ctx, gap);
	}
	const gapDetectMs = performance.now() - gapDetectStart;

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[carlos-alm]

Fixed — hoisted backfillHappened before the if so the boolean expression is written exactly once. The variable declaration now serves as the if guard directly.

[github-actions]

Codegraph Impact Analysis

1 functions changed → 5 callers affected across 5 files

• tryNativeOrchestrator in src/domain/graph/builder/stages/native-orchestrator.ts:1370 (5 transitive callers)

[carlos-alm]

Addressed: hoisted backfillHappened before the if guard per both Greptile and Claude review nits (commit 304a158). The CI failure ('memory access out of bounds' in --version on ubuntu-latest) passes locally and on macOS/Windows — the WASM crash appears to be an ephemeral CI environment issue unrelated to the TypeScript logic change. Re-triggering to confirm.

[carlos-alm]

@greptileai