fix(builder): incremental rebuild EISDIR, edge parity, dev tarball, native version#241
fix(builder): incremental rebuild EISDIR, edge parity, dev tarball, native version#241carlos-alm merged 8 commits intomainfrom
Conversation
…ld docs - Add node/edge count drift detection after incremental builds (>20% warns to use --no-incremental), persisting counts in build_meta - Improve complexity command: when data is missing but graph exists, suggest --no-incremental instead of implying no graph exists - Add @huggingface/transformers as devDependency so embedding benchmarks work out of the box - Document dev build tarball installation in README (npm install <url> fails for optional platform deps) Impact: 3 functions changed, 5 affected
…provements Impact: 1 functions changed, 1 affected
- Add build.driftThreshold to config defaults (0.2) so the 20% incremental build drift threshold is user-configurable - Include actual drift percentages and threshold in warning message for easier debugging - Regenerate package-lock.json to include @huggingface/transformers devDependency (fixes License Compliance Scan CI failure) Impact: 1 functions changed, 1 affected
…ative version - filter directory nodes from reverse-deps query to prevent EISDIR (#235) - load unchanged barrel files into reexportMap so barrel-resolved edges aren't dropped during incremental rebuilds (#236) - add --strip flag to sync-native-versions.js removing platform optionalDependencies in dev builds, fixing npm install failures (#237) - sync Cargo.toml version with package.json and automate via version script - add barrel-project fixture and incremental-parity test Impact: 4 functions changed, 1 affected
|
Claude finished @carlos-alm's task —— View job PR Review CompleteReviewing PR #241: fix(builder): incremental rebuild EISDIR, edge parity, dev tarball, native version Todo List:
Phase 1: Root Cause Analysis ✅Problem Being Solved: Four distinct incremental build issues identified during v2.5.35-dev dogfooding:
Why This Approach: These are genuine architectural fixes addressing real issues:
Phase 2: Configuration Change Detection ✅LOW RISK: No risky configuration changes detected:
Phase 3: Test Integrity ✅EXCELLENT: New test coverage addresses the core issue:
Test Quality: The test directly validates the fix with proper isolation (temporary directories) and comprehensive verification (node count, edge count, node equality, edge equality). Phase 4: Code Quality ✅SOLID IMPLEMENTATION:
Architecture Quality: All fixes are minimally invasive, well-documented with comments, and follow existing patterns. No performance regressions or security concerns. Root Cause AnalysisProblem Being Solved: Four genuine incremental build bugs identified through dogfooding, not configuration workarounds Critical ConcernsNone. This is high-quality defensive programming addressing real architectural issues. Final Recommendation
|
Greptile SummaryThis PR comprehensively addresses four dogfooding issues with well-structured fixes:
The implementation includes comprehensive testing with a new All 1063 existing tests pass, and the changes integrate cleanly with the existing codebase architecture. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[Start Incremental Build] --> CheckBuild{Full Build?}
CheckBuild -->|Yes| ParseAll[Parse All Files]
CheckBuild -->|No| FindChanged[Find Changed Files]
FindChanged --> ReverseDeps[Query Reverse Dependencies]
ReverseDeps --> FilterDirs[Filter Directory Nodes<br/>Fix #235]
FilterDirs --> ParseChanged[Parse Changed + Reverse Dep Files]
ParseChanged --> LoadBarrels[Load Unchanged Barrel Files<br/>Fix #236]
LoadBarrels --> BuildReexportMap[Build reexportMap]
ParseAll --> BuildEdges[Build Edges Loop]
BuildReexportMap --> BuildEdges
BuildEdges --> SkipBarrelOnly{File in<br/>barrelOnlyFiles?}
SkipBarrelOnly -->|Yes| Skip[Skip - Edges Already in DB]
SkipBarrelOnly -->|No| CreateEdges[Create Import/Call Edges]
Skip --> NextFile{More Files?}
CreateEdges --> NextFile
NextFile -->|Yes| BuildEdges
NextFile -->|No| DriftCheck{Incremental?}
DriftCheck -->|Yes| VerifyDrift[Check Node/Edge Drift<br/>vs Previous Counts]
DriftCheck -->|No| SaveMetadata[Save Metadata]
VerifyDrift --> DriftThreshold{Drift ><br/>threshold?}
DriftThreshold -->|Yes| Warn[Warn: Rebuild Recommended]
DriftThreshold -->|No| SaveMetadata
Warn --> SaveMetadata
SaveMetadata --> StoreCount[Store node_count,<br/>edge_count in metadata]
StoreCount --> End[Build Complete]
Last reviewed commit: 1216acc |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }, | ||
| "devDependencies": { | ||
| "@biomejs/biome": "^2.4.4", | ||
| "@huggingface/transformers": "^3.8.1", |
There was a problem hiding this comment.
Why was @huggingface/transformers added to devDependencies? This change isn't mentioned in the PR description. If needed for testing, consider adding a note in the PR description.
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!
* docs: add dogfood report for v2.5.35-dev.26434e2 * docs: move dogfood report to correct path * docs: remove report from incorrect worktree path * docs: move v2.5.0 dogfood report to generated/dogfood/ * fix: incremental build verification, complexity warnings, and dev build docs - Add node/edge count drift detection after incremental builds (>20% warns to use --no-incremental), persisting counts in build_meta - Improve complexity command: when data is missing but graph exists, suggest --no-incremental instead of implying no graph exists - Add @huggingface/transformers as devDependency so embedding benchmarks work out of the box - Document dev build tarball installation in README (npm install <url> fails for optional platform deps) Impact: 3 functions changed, 5 affected * fix: configurable drift threshold, drift logging, and lock file sync - Add build.driftThreshold to config defaults (0.2) so the 20% incremental build drift threshold is user-configurable - Include actual drift percentages and threshold in warning message for easier debugging - Regenerate package-lock.json to include @huggingface/transformers devDependency (fixes License Compliance Scan CI failure) Impact: 1 functions changed, 1 affected * fix(builder): incremental rebuild EISDIR, edge parity, dev tarball, native version - filter directory nodes from reverse-deps query to prevent EISDIR (#235) - load unchanged barrel files into reexportMap so barrel-resolved edges aren't dropped during incremental rebuilds (#236) - add --strip flag to sync-native-versions.js removing platform optionalDependencies in dev builds, fixing npm install failures (#237) - sync Cargo.toml version with package.json and automate via version script - add barrel-project fixture and incremental-parity test Impact: 4 functions changed, 1 affected * docs: update 2.6.0 changelog with merged PRs #233 #239 #240 #241
1 participant
Summary
Fixes four issues found during v2.5.35-dev dogfooding:
directorynodes from reverse-deps SQL query so directories don't enter the incremental parse pipelinereexportMapduring incremental builds so barrel-resolved import/call edges aren't silently dropped--stripflag tosync-native-versions.jsthat removes platform@optave/codegraph-*entries fromoptionalDependenciesin dev buildsCargo.tomlversion to2.6.0and automate via the version sync scriptTest plan
incremental-parity.test.js— full build vs incremental build produces identical nodes and edges (barrel-project fixture)--stripflag verified: removes platform deps, keeps other optional depsCloses #235, closes #236, closes #237