fix(perf): wire engine selection through openRepo to fix query benchmarks

[carlos-alm]

Summary

• openRepo() always tried native NAPI path first regardless of engine selection, causing both "wasm" and "native" benchmark workers to measure the same native rusqlite open/close overhead (~27ms vs ~10ms with direct better-sqlite3)
• This produced a false 177% regression in fnDeps query benchmarks (v3.7.0→v3.9.0), blocking CI on all open PRs
• Add engine param to openRepo() and check CODEGRAPH_ENGINE env var so query functions respect engine selection
• Set CODEGRAPH_ENGINE in benchmark fork workers
• Add v3.9.0 fnDeps depth 1/3/5 to KNOWN_REGRESSIONS (published data measured with broken routing)

Root cause

openRepo() in src/db/connection.ts had no engine selection — it unconditionally called isNativeAvailable() and used the native path when available. The benchmark fork (scripts/lib/fork-engine.ts) set __BENCH_ENGINE__ per worker to label engine runs, but openRepo never checked it. On CI where both engines are available, both workers measured identical native performance.

Test plan

• regression-guard.test.ts — 14/14 pass (fnDeps regressions now in KNOWN_REGRESSIONS)
• queries-unit.test.ts — 74/74 pass
• Integration tests — all query tests pass
• TypeScript compiles clean
• Lint clean on changed files

[claude]

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

---

PR Review: Engine Selection Fix

Todo List:

• Read and analyze changed files
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity review
• Phase 4: Code quality assessment
• Phase 5: Backlog compliance grading
• Provide final review recommendation

Root Cause Analysis

Problem Being Solved: The openRepo() function in src/db/connection.ts unconditionally tried the native NAPI path first regardless of explicit engine selection, causing both "wasm" and "native" benchmark workers to measure identical native rusqlite performance (~27ms vs ~10ms with better-sqlite3). This produced false 177% regressions in fnDeps query benchmarks (v3.7.0→v3.9.0), blocking CI on all open PRs.

Why This Approach: The fix correctly addresses the root cause by:

1. Adding engine parameter to openRepo() options
2. Checking process.env.CODEGRAPH_ENGINE environment variable
3. Setting CODEGRAPH_ENGINE in benchmark fork workers
4. Respecting explicit engine selection with proper fallback hierarchy

Risk Assessment: ✅ This fixes a real measurement bug, not hiding symptoms. The issue was in infrastructure—benchmark isolation was broken, not the measured code.

Configuration Change Detection

✅ No high-risk configuration changes detected:

• No biome/linting rule changes
• No test timeout or coverage threshold modifications
• No build configuration changes
• No TypeScript strict mode changes

Test Integrity Review

✅ Strong test integrity:

• The KNOWN_REGRESSIONS additions are appropriately documented with detailed explanations
• Regressions are marked as temporary with clear fix path (v3.10.0 will reflect corrected measurements)
• Root cause is explained: engine routing bug, not actual performance regression
• No test weakening—just acknowledging bad historical data due to measurement bug

Code Quality Assessment

src/db/connection.ts (openRepo changes):

• ✅ Clean parameter addition with proper TypeScript typing
• ✅ Correct fallback hierarchy: opts.engine > CODEGRAPH_ENGINE env > 'auto'
• ✅ Well-documented with clear comment explaining purpose
• ✅ Engine check properly guards native path: engine !== 'wasm' && isNativeAvailable()

scripts/lib/fork-engine.ts (benchmark isolation fix):

• ✅ Minimal, surgical change: added CODEGRAPH_ENGINE: workerName to forked env
• ✅ Proper placement in forkWorker function where all env setup happens
• ✅ Consistent with existing pattern ([envKey]: workerName)

tests/benchmarks/regression-guard.test.ts (known regressions):

• ✅ Detailed documentation explaining the root cause
• ✅ Clear indication this is published data with broken measurement
• ✅ Proper scoping: only v3.9.0 fnDeps metrics affected
• ✅ Forward-looking note about v3.10.0 corrected measurements

Backlog Compliance

• Zero-dep: ✓ — No new runtime dependencies added
• Foundation-aligned: ✓ — Maintains P1 (always-current) and P4 (zero-cost core) principles
• Problem-fit: 5 — Directly addresses measurement accuracy and CI reliability
• Breaking: No — Purely additive parameter, backward compatible API
• Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Critical Concerns

None. This is a clean infrastructure fix that:

1. Addresses real measurement bug with proper root cause analysis
2. Maintains backward compatibility (optional parameter with sensible defaults)
3. Properly documents known issues without weakening test standards
4. Follows established patterns in the codebase

Final Recommendation

• Rating: ⭐⭐⭐⭐⭐ (5/5)
• Action: APPROVE
• Reasoning: Excellent fix that properly addresses the root cause of false regression reports. Clean implementation, proper documentation, and strong test integrity. The KNOWN_REGRESSIONS additions are well-justified as they acknowledge bad historical measurement data rather than lowering standards. Critical for unblocking CI on all open PRs while maintaining benchmark integrity.

[greptile-apps]

Greptile Summary

This PR fixes a benchmark instrumentation bug where openRepo() ignored engine selection, causing both wasm and native benchmark workers to measure identical native rusqlite performance (~27 ms), producing a false 177% regression in fnDeps query benchmarks that blocked CI on all open PRs.

The fix is two-part: forkWorker in scripts/lib/fork-engine.ts now sets CODEGRAPH_ENGINE=<engine> in each worker's environment alongside the existing __BENCH_ENGINE__ label, and openRepo() in src/db/connection.ts now reads opts.engine || process.env.CODEGRAPH_ENGINE || 'auto' to route to the correct backend. openReadonlyWithNative() receives the same env-var check for consistency.

• scripts/lib/fork-engine.ts: one-line fix adding CODEGRAPH_ENGINE: workerName to the forked worker env — the minimal change needed to wire engine identity through fork()
• src/db/connection.ts: openRepo() gains an opts.engine param with priority opts.engine > CODEGRAPH_ENGINE env > auto; openReadonlyWithNative() gains the same env-var check; all existing callers remain backward-compatible since engine is optional
• tests/benchmarks/regression-guard.test.ts: three v3.9.0 fnDeps KNOWN_REGRESSIONS entries to unblock CI (measurements taken under broken routing) plus a new self-enforcing stale-exemption test that reads package.json and fails if any entry is more than 1 minor version behind the current release, ensuring bookkeeping cannot silently go stale

Confidence Score: 5/5

Safe to merge — root cause correctly identified and fixed with no P0/P1 issues remaining.

The fix is minimal, targeted, and correct. The two-part change (set CODEGRAPH_ENGINE in the fork + respect it in openRepo) fully addresses the false-regression root cause. Backward compatibility is preserved: opts.engine is optional and all existing callers continue to work unchanged. The stale-exemption test adds durable self-enforcement so KNOWN_REGRESSIONS bookkeeping cannot silently go stale. No logic bugs, security issues, or edge-case regressions found across any of the three changed files.

No files require special attention.

Important Files Changed

Filename	Overview
scripts/lib/fork-engine.ts	Adds CODEGRAPH_ENGINE env var to forked worker processes so each worker uses its intended engine; this is the primary bug fix.
src/db/connection.ts	Adds engine selection to openRepo() (opts.engine > CODEGRAPH_ENGINE env > auto) and makes openReadonlyWithNative() consistent, fixing native-path bypass.
tests/benchmarks/regression-guard.test.ts	Adds 3 KNOWN_REGRESSIONS exemptions for v3.9.0 fnDeps metrics measured with broken routing; adds stale-entry self-detection test keyed on package version.

Sequence Diagram

sequenceDiagram
    participant P as Parent (forkEngines)
    participant W1 as Worker: wasm
    participant W2 as Worker: native
    participant OR as openRepo()

    Note over P,OR: BEFORE fix — CODEGRAPH_ENGINE never set in worker env
    P->>W1: fork(env: __BENCH_ENGINE__=wasm)
    W1->>OR: openRepo()
    OR-->>W1: isNativeAvailable()=true → uses rusqlite
    P->>W2: fork(env: __BENCH_ENGINE__=native)
    W2->>OR: openRepo()
    OR-->>W2: isNativeAvailable()=true → uses rusqlite
    Note over W1,W2: Both workers measure native overhead — false regression!

    Note over P,OR: AFTER fix — CODEGRAPH_ENGINE wired through fork
    P->>W1: fork(env: __BENCH_ENGINE__=wasm, CODEGRAPH_ENGINE=wasm)
    W1->>OR: openRepo()
    OR-->>W1: engine=wasm → skips native → uses better-sqlite3
    P->>W2: fork(env: __BENCH_ENGINE__=native, CODEGRAPH_ENGINE=native)
    W2->>OR: openRepo()
    OR-->>W2: engine=native → uses rusqlite NativeRepository

Loading

_{Reviews (2): Last reviewed commit: "fix: add staleness check for KNOWN_REGRE..." | Re-trigger Greptile}

[greptile-apps]

KNOWN_REGRESSIONS entries have no expiry enforcement

The three 3.9.0:fnDeps depth {1,3,5} entries are documented in the inline comment above (lines 68–73) to be removed once v3.10.0 benchmark data is merged. There is no automated check that enforces this cleanup. If these entries are left in place after v3.10.0 data lands, the regression guard will silently pass genuine regressions in those three metrics for all future releases.

Consider adding a test-time assertion that checks the current package version and warns (or fails) when any KNOWN_REGRESSIONS entry is more than one minor version behind the running package — this would make the stale-exemption problem self-detecting rather than requiring manual bookkeeping.

[carlos-alm]

Fixed in b6e919a. Added a KNOWN_REGRESSIONS entries are not stale test that reads the current package version, computes the minor-version gap for each entry, and fails when any entry is more than 1 minor version behind. This makes stale exemptions self-detecting — the test will fail as soon as v3.10.0 is released if the entries aren't cleaned up.

[github-actions]

Codegraph Impact Analysis

3 functions changed → 41 callers affected across 33 files

• forkWorker in scripts/lib/fork-engine.ts:67 (5 transitive callers)
• openRepo in src/db/connection.ts:361 (29 transitive callers)
• openReadonlyWithNative in src/db/connection.ts:412 (8 transitive callers)

[carlos-alm]

Addressed the P1 openReadonlyWithNative engine-bypass gap (from the Greptile review). Fixed in 99b6e0f — added the same CODEGRAPH_ENGINE env-var guard to openReadonlyWithNative() so it skips the native path when engine === 'wasm', consistent with openRepo(). This closes the latent measurement gap for structure-query, dataflow, and module-map callers.

[carlos-alm]

@greptileai