Skip to content

fix(native): keep nativeDb open through finalize for correct build_meta#784

Merged
carlos-alm merged 3 commits intomainfrom
fix/751-build-meta-version-engine
Apr 3, 2026
Merged

fix(native): keep nativeDb open through finalize for correct build_meta#784
carlos-alm merged 3 commits intomainfrom
fix/751-build-meta-version-engine

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

@carlos-alm carlos-alm commented Apr 3, 2026

Summary

Fixes #751

Two bugs fixed:

  1. Stale WAL page cache on metadata write (commit f45389e): closeNativeDb(ctx, 'post-analyses') closed the native DB before finalize(), forcing persistBuildMetadata to fall back to the JS (better-sqlite3) path which had a stale WAL page cache after native WAL truncation — causing setBuildMeta writes to silently fail. Fixed by keeping nativeDb alive through finalize and calling refreshJsDb beforehand.

  2. engine_version used Rust crate version (commit 5b67d11): engine_version was set to ctx.engineVersion which comes from CARGO_PKG_VERSION via native.engineVersion(). When the Rust crate version drifts from the npm package version (Cargo.toml not bumped during release), build_meta stores the wrong version. Fixed by using CODEGRAPH_VERSION (npm package.json version) for engine_version in both nativeDb and JS DB write paths.

Test plan

  • New test build_meta reflects actual engine and version after build (#751) — asserts codegraph_version, engine_version, engine, built_at, and counts
  • engine_version assertion verifies it equals the npm package version (not the Rust crate version)
  • Existing version/engine mismatch rebuild tests pass
  • Pipeline unit tests pass (4/4)
  • Type-check passes (tsc --noEmit)

…ta (#751)

nativeDb was closed before finalize(), making persistBuildMetadata fall
back to the JS (better-sqlite3) path. After native WAL truncation the JS
connection's page cache was stale, causing the setBuildMeta write to
silently fail and leaving stale version/engine metadata. This broke
incremental builds by triggering a spurious "version changed" full
rebuild on every subsequent run.

Keep nativeDb alive through finalize so all metadata writes, advisory
checks, and count queries use the native path. Refresh the JS DB
beforehand so it has a valid page cache if finalize falls back to JS.
@claude
Copy link
Copy Markdown

claude bot commented Apr 3, 2026

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


PR Review Complete

Reviewing PR #784: fix(native): keep nativeDb open through finalize for correct build_meta

Review Checklist

  • Phase 1: Root Cause Analysis - understand the problem being solved
  • Phase 2: Configuration Change Detection - check for high-risk changes
  • Phase 3: Test Integrity - verify test quality and coverage
  • Phase 4: Code Quality Review - examine implementation details
  • Phase 5: Backlog Compliance - assess against project principles
  • Final Assessment: Rating and recommendation

Root Cause Analysis

Problem Being Solved: Issue #751 where build_meta table contained stale version/engine data, causing spurious "version changed" rebuilds on every codegraph build.

Root Cause:

  • closeNativeDb(ctx, 'post-analyses') closed the native DB connection before finalize() ran
  • This forced persistBuildMetadata() to fall back to the JS (better-sqlite3) path
  • However, the JS connection had a stale WAL page cache after native WAL truncation
  • Result: setBuildMeta writes silently failed, leaving stale metadata from previous builds

Why This Approach: The fix keeps nativeDb alive through finalize so metadata writes use the native path (which works correctly), while adding refreshJsDb() as a safety net if finalize falls back to JS paths. The closeDbPair inside finalize handles both connections properly.

Risk Assessment: Very low risk - this is a targeted fix for a specific timing issue with proper fallback handling.

Critical Concerns

None identified. This is a clean fix addressing a real database consistency bug.

Test Integrity ✅

New Test Added: The PR adds a comprehensive test 'build_meta reflects actual engine and version after build (#751)' that:

  • Rebuilds the graph and reads fresh metadata
  • Validates codegraph_version matches package.json
  • Ensures engine is either 'native' or 'wasm' (not stale)
  • Checks engine_version is a valid semver when native
  • Verifies built_at is a valid recent timestamp
  • Confirms node/edge counts are positive

Test Quality: The test is robust and directly validates the fix. No existing tests were weakened or removed.

Code Quality Review ✅

Implementation: The change is minimal and surgical:

  • Replaces premature closeNativeDb(ctx, 'post-analyses') with conditional refreshJsDb(ctx)
  • Keeps native connection alive for finalize to use native metadata paths
  • Maintains existing error handling and fallback patterns
  • Well-documented with clear comments explaining the WAL cache issue

No Security Issues: No command injection, SQL injection, or credential exposure risks.

Performance: Actually improves performance by avoiding unnecessary connection close/reopen cycles.

Backlog Compliance

  • Zero-dep: ✅ — no new runtime dependencies added
  • Foundation-aligned: ✅ — fully compliant with all 8 core principles:
    • P1 (always current): Fixes rebuild performance regression
    • P2 (native speed): Preserves native engine efficiency
    • P4 (zero-cost core): No cloud APIs, pure local fix
    • P8 (honest scope): Pure bug fix, no scope creep
  • Problem-fit: 4/5 — directly improves agent accuracy by preventing spurious rebuilds
  • Breaking: No — purely internal fix, no API changes

Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a high-quality fix for a real database consistency bug that was causing performance regression. The implementation is surgical, well-tested, and maintains all fallback patterns. The new integration test provides excellent coverage. No concerns identified across all review dimensions.

The fix directly addresses the stated problem (silent metadata write failures) with a proper solution (keeping native connection alive during finalize) rather than working around symptoms. Code quality is excellent with clear documentation of the WAL cache issue.


engine_version was set to ctx.engineVersion which comes from the Rust
crate's CARGO_PKG_VERSION via native.engineVersion(). When the Rust
crate version drifts from the npm package version (e.g. Cargo.toml not
bumped during release), build_meta stores the wrong version. Use
CODEGRAPH_VERSION (the npm package.json version) for both
codegraph_version and engine_version since the native engine is
distributed as part of the npm package.

Also strengthens the #751 regression test to assert engine_version
equals the npm version and uses a full rebuild to ensure metadata is
written for the small test fixture.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR fixes a silent build_meta write failure (#751) in the JS pipeline path. closeNativeDb was called before finalize(), causing persistBuildMetadata to fall back to the better-sqlite3 path with a stale WAL page cache (invalidated by the preceding rusqlite PRAGMA wal_checkpoint(TRUNCATE)), making setBuildMeta silently no-op and leaving stale engine/version metadata. The fix removes the premature close, calls refreshJsDb to give better-sqlite3 a clean page cache, and lets closeDbPair inside finalize handle both connections. A regression test verifying all metadata keys are correct after build is added.

Confidence Score: 5/5

This PR is safe to merge; the fix is targeted, well-understood, and the regression test exercises the exact failure path.

The change is a minimal 3-line deletion + 4-line addition in the JS pipeline path. The root cause analysis is accurate: the WAL truncation inside closeNativeDb invalidated better-sqlite3's page cache, causing the subsequent setBuildMeta write to silently fail. Keeping nativeDb open and calling refreshJsDb before finalize directly addresses both failure modes. The native orchestrator fast path is unaffected (it writes metadata before any WAL handoff). The new test correctly asserts all build_meta keys in the scenario where the bug was triggered. No P0/P1 issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/domain/graph/builder/pipeline.ts Removes premature closeNativeDb call before finalize() and adds a guarded refreshJsDb call; fix is minimal, correctly scoped to the JS pipeline path only.
tests/integration/build.test.ts Adds a new integration test that verifies build_meta keys (codegraph_version, engine, engine_version, built_at, node_count, edge_count) are correctly written after a full rebuild; correctly placed in the mismatch-promotion suite so it benefits from the prior engine-mismatch full rebuild.

Sequence Diagram

sequenceDiagram
    participant P as pipeline.ts (runPipelineStages)
    participant N as nativeDb (rusqlite)
    participant J as ctx.db (better-sqlite3)
    participant F as finalize.ts

    Note over P: Before fix
    P->>N: runAnalyses (suspend/resume WAL pattern)
    P->>N: closeNativeDb → PRAGMA wal_checkpoint(TRUNCATE)
    Note over J: WAL index cache invalidated ⚠️
    N-->>P: ctx.nativeDb = undefined
    P->>F: finalize()
    F->>J: persistBuildMetadata (falls back to JS path)
    Note over J: Stale WAL cache → setBuildMeta silently fails ❌

    Note over P: After fix (this PR)
    P->>N: runAnalyses (suspend/resume WAL pattern)
    P->>J: refreshJsDb() → close + reopen better-sqlite3
    Note over J: Fresh page cache ✅
    P->>F: finalize() with nativeDb still open
    F->>N: persistBuildMetadata (native path via ctx.nativeDb)
    Note over N: setBuildMeta written correctly ✅
    F->>N: closeDbPair → close nativeDb
    F->>J: closeDbPair → close ctx.db
Loading

Reviews (1): Last reviewed commit: "fix(native): use npm version for engine_..." | Re-trigger Greptile

@carlos-alm carlos-alm merged commit 1101d24 into main Apr 3, 2026
11 of 12 checks passed
@carlos-alm carlos-alm deleted the fix/751-build-meta-version-engine branch April 3, 2026 04:22
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(native): build_meta stores wrong version/engine after native build

1 participant