fix(build): native incremental rebuild perpetual full-rebuild loop#928
fix(build): native incremental rebuild perpetual full-rebuild loop#928carlos-alm wants to merge 2 commits intomainfrom
Conversation
…ead of 200ms
Root cause: version mismatch between npm package (3.9.3) and native addon
(3.9.2) caused perpetual full rebuilds. The Rust check_version_mismatch
read codegraph_version from build_meta (overwritten by JS to "3.9.3") and
compared it to CARGO_PKG_VERSION ("3.9.2") — always mismatching.
Fix (three-pronged):
- Rust: check engine_version instead of codegraph_version for rebuild
decisions — engine_version is consistently the addon's own version
- JS pipeline: stop overwriting Rust-written build_meta fields after native
orchestrator completes; skip codegraph_version mismatch check when native
engine is available (Rust handles its own version check)
- JS finalize: write actual engine version (ctx.engineVersion) to
engine_version instead of always using CODEGRAPH_VERSION (npm version)
Impact: 4 functions changed, 10 affected
|
Claude finished @carlos-alm's task —— View job Reviewing this PR for native incremental rebuild fix... Review Progress
Phase 1: Root Cause Analysis ✅Problem Being Solved: Version mismatch between npm package (3.9.3) and native Rust addon (3.9.2) causing perpetual full rebuilds instead of incremental builds. The Rust Root Cause is Sound: This is fixing a real performance issue, not hiding symptoms. The 5.8s → 214ms improvement is substantial and addresses actual broken incremental builds. Why This Approach: Three-pronged fix is logical:
Phase 2: Configuration Change Detection ✅No High-Risk Configuration Changes: This PR contains no linting rule relaxation, test timeout increases, or build configuration modifications. Changes are purely logic fixes to version comparison behavior. Phase 3: Test Integrity Verification ✅Test Changes Are Sound: The updated assertions in Phase 4: Code Quality Review ✅Strong Implementation:
No Security/Performance Concerns: Changes are purely metadata handling with no external inputs or performance regressions. Phase 5: Backlog Compliance Assessment ✅Zero-dep: ✓ — No new runtime dependencies added Tier: 1 (zero-dep ✓ + foundation-aligned ✓) Root Cause AnalysisProblem Being Solved: Version mismatch between npm package and native addon causing perpetual 5.8s full rebuilds instead of 214ms incremental builds Backlog Compliance
Final Recommendation
|
Greptile SummaryThis PR fixes a perpetual full-rebuild loop where the JS post-build step overwrote Confidence Score: 5/5Safe to merge — root cause is correctly identified and fixed, all three change sites are consistent, and no correctness regressions were found. All findings are P2. The test regex missing No files require special attention; the minor test assertion at Important Files Changed
Sequence DiagramsequenceDiagram
participant JS as JS Pipeline
participant Rust as Rust Orchestrator
participant DB as SQLite (build_meta)
note over JS,DB: Before fix — perpetual full-rebuild loop
JS->>Rust: buildGraph(...)
Rust->>DB: write engine_version = CARGO_PKG_VERSION (3.9.2)
Rust->>DB: write codegraph_version = CARGO_PKG_VERSION (3.9.2)
Rust-->>JS: result
JS->>DB: overwrite codegraph_version = CODEGRAPH_VERSION (3.9.3)
note right of DB: codegraph_version = 3.9.3
JS->>Rust: buildGraph(...) [next build]
Rust->>DB: read codegraph_version → 3.9.3
note right of Rust: 3.9.3 ≠ CARGO_PKG_VERSION 3.9.2 → force full rebuild!
Rust-->>JS: full rebuild (5.8s)
note over JS,DB: After fix — fast incremental path
JS->>JS: checkEngineSchemaMismatch skip version check nativeAvailable=true
JS->>Rust: buildGraph(...)
Rust->>DB: write engine_version = CARGO_PKG_VERSION (3.9.3)
Rust->>DB: write codegraph_version = CARGO_PKG_VERSION (3.9.3)
Rust-->>JS: result
JS->>DB: write only schema_version + built_at
JS->>Rust: buildGraph(...) [next build]
Rust->>DB: read engine_version → 3.9.3
note right of Rust: 3.9.3 == CARGO_PKG_VERSION 3.9.3 → early exit!
Rust-->>JS: early-exit (214ms)
Reviews (1): Last reviewed commit: "fix(test): remove unused pkg variable fr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(meta.engine_version).toMatch(/^\d+\.\d+\.\d+/); | ||
|
|
||
| // codegraph_version tracks the npm package version when the JS pipeline | ||
| // finalizes, or the addon version when the Rust orchestrator runs. | ||
| // Either is valid — the key invariant is it's a valid semver string. | ||
| expect(meta.codegraph_version).toMatch(/^\d+\.\d+\.\d+/); |
There was a problem hiding this comment.
Both regexes match any string that starts with a semver prefix — e.g. "3.9.3-corrupted" or "3.9.3\nunexpected" would pass. Adding $ prevents false positives and better documents the invariant.
| expect(meta.engine_version).toMatch(/^\d+\.\d+\.\d+/); | |
| // codegraph_version tracks the npm package version when the JS pipeline | |
| // finalizes, or the addon version when the Rust orchestrator runs. | |
| // Either is valid — the key invariant is it's a valid semver string. | |
| expect(meta.codegraph_version).toMatch(/^\d+\.\d+\.\d+/); | |
| expect(meta.engine_version).toMatch(/^\d+\.\d+\.\d+$/); | |
| // codegraph_version tracks the npm package version when the JS pipeline | |
| // finalizes, or the addon version when the Rust orchestrator runs. | |
| // Either is valid — the key invariant is it's a valid semver string. | |
| expect(meta.codegraph_version).toMatch(/^\d+\.\d+\.\d+$/); |
| // When the native orchestrator is available, it handles its own version | ||
| // check (engine_version vs CARGO_PKG_VERSION). The JS-side codegraph_version | ||
| // may differ from the Rust addon version (npm 3.9.3 vs addon 3.9.2), so | ||
| // checking it here would force unnecessary full rebuilds and prevent the | ||
| // native orchestrator from ever running its fast incremental path (#928). | ||
| // Only check codegraph_version when falling through to the JS pipeline. | ||
| if (!ctx.nativeAvailable) { | ||
| const prevVersion = meta('codegraph_version'); | ||
| if (prevVersion && prevVersion !== CODEGRAPH_VERSION) { | ||
| info( | ||
| `Codegraph version changed (${prevVersion} → ${CODEGRAPH_VERSION}), promoting to full rebuild.`, | ||
| ); | ||
| ctx.forceFullRebuild = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
checkEngineSchemaMismatch skips the codegraph_version check whenever ctx.nativeAvailable is true, even though the native orchestrator may subsequently fail and fall through to the JS pipeline. In that narrow scenario — native available, version bumped, orchestrator throws — the JS pipeline runs in whatever mode was already determined (potentially incremental) without the version-change promotion to full rebuild that the old code provided.
The comment says "the Rust orchestrator handles its own version check", which is true when the orchestrator succeeds. Given the warn + fall-through error handling in buildGraph, the JS pipeline does proceed without the version gate if the orchestrator throws after this check runs.
Codegraph Impact Analysis4 functions changed → 10 callers affected across 7 files
|
1 participant
Summary
check_version_mismatchto force a full rebuild on every build. The JS post-processing overwrotecodegraph_versioninbuild_metawith the npm version ("3.9.3"), but the Rust addon compared it againstCARGO_PKG_VERSION("3.9.2") — always mismatching.engine_versioninstead ofcodegraph_version; JS stops overwriting Rust-written metadata after native orchestrator; JS skips its own version check when native engine handles it.Before / After
Test plan
tests/integration/build.test.ts— build metadata assertions updatedtests/integration/incremental-parity.test.ts— all 26 tests passtests/builder/— all builder unit tests passcargo checkpasses