fix: iterate barrel re-parse discovery to stop dropping chained-barrel edges#1179
Conversation
…l edges Closes #1174 Stage 6b's barrel candidate discovery was single-pass: it only looked at the originally changed files' imports. When a hybrid barrel (file with ≥1 reexport + many local defs, e.g. src/domain/parser.ts) was re-parsed, its outgoing edges were wiped — but the barrel-through edges from that barrel to leaf files (via a second-level barrel like src/extractors/ index.ts) could not be re-emitted because the second barrel was never loaded into file_symbols. Result: 32 imports edges (native) / 37 (WASM) silently dropped per incremental rebuild, and they never came back without --no-incremental. The Rust orchestrator and the JS WASM-fallback pipeline now iterate the barrel-candidate discovery until file_symbols is stable. JS-side, three related defects were exposed once parser.ts started being processed correctly: * reparseBarrelFiles was marking every re-parsed file as barrel-only even when it was a hybrid (the isBarrelFile heuristic returns true for reexports >= ownDefs); only mark the actual barrel-only files. * build-edges' lazy fallback queried kind != 'file', broader than the upfront load's specific definition kinds, leaking parameters and properties into call resolution. * resolve-imports' delete-outgoing-edges had no kind filter, wiping contains/parameter_of that insertNodes only emits for changed files — aligned with the Rust orchestrator's filter. Verification on the dogfooded repo: full=1371, incremental=1371 imports edges on both engines (was -32 / -37 on main). Every edge kind is identical between full and incremental on the native engine; only minor imports-type / contains stragglers remain on WASM (separate concerns). Regression test parameterized over both engines: tests/integration/issue-1174-chained-barrel-incremental.test.ts.
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisReviewing PR #1179: "fix: iterate barrel re-parse discovery to stop dropping chained-barrel edges" Todo List
Root Cause AnalysisProblem Being Solved: Incremental rebuilds were silently dropping 32-37 import edges whenever an unrelated file was touched. The issue occurred with chained barrel files (e.g., Why This Approach: The iterative solution is architecturally sound. Barrel resolution inherently requires fixpoint iteration since barrels can import other barrels in chains. The implementation mirrors the theoretical requirement perfectly - iterate until Risk Assessment: Very low risk. The changes are surgical, well-tested, and maintain strict parity between native Rust and WASM engines. Backlog Compliance
Critical ConcernsNone identified. This is a high-quality fix that addresses a real production bug with comprehensive testing. Code Quality AssessmentPositive aspects:
Technical improvements:
Test IntegrityThe new test
Final Recommendation
|
Greptile SummaryThis PR fixes incremental rebuilds silently dropping import edges whenever a file in a chained-barrel structure was touched, by converting Stage 6b's single-pass barrel candidate discovery into an iterative loop in both the Rust (native) and TypeScript (WASM) pipelines. Three related JS-side defects are fixed in the same commit: hybrid barrels were unconditionally marked
Confidence Score: 5/5Safe to merge; both engines now produce identical edge counts between full and incremental builds, verified across multiple edit/revert cycles. The Rust and JS implementations are structurally parallel and both preserve the convergence guarantee. The two findings are a redundant DB query per loop iteration in the JS path and a gap in test coverage for the contains/parameter_of edge-kind fix — neither affects correctness of the shipped behaviour. resolve-imports.ts (allBarrelFiles query hoisting) and the integration test (missing contains/parameter_of assertion). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Stage 6b entry"] --> B["Compute barrel_files_in_db (once)"]
B --> C["Seed: collect_imported_barrel_candidates"]
C --> D["Seed: collect_reexport_from_barrels (first pass only)"]
D --> E{"barrel_paths_to_parse empty?"}
E -- yes --> Z["Done — fileSymbols stable"]
E -- no --> F["sort + dedup + take"]
F --> G["parse_files_parallel / parseFilesAuto"]
G --> H["delete edges NOT IN contains/parameter_of\nmerge into fileSymbols\nisBarrelFile → barrelOnlyFiles"]
H --> I["collect_imported_barrel_candidates from newly_added"]
I --> E
Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/1174-increm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| while (true) { | ||
| const before = fileSymbols.size; | ||
| const barrelCandidates = findBarrelCandidates(ctx); | ||
| await reparseBarrelFiles(ctx, barrelCandidates); | ||
| if (fileSymbols.size === before) break; | ||
| } |
There was a problem hiding this comment.
reparseBarrelFiles are unused
reparseBarrelFiles was refactored to collect and return newly-added paths, and its JSDoc says "Returns the relative paths of newly-merged files so the caller can scan them for the next level of barrel candidates." However, resolveImports discards the return value entirely — convergence is detected via fileSymbols.size === before instead.
As a consequence, every iteration of the loop calls findBarrelCandidates with the full fileSymbols keyset, which (in the small-files path) runs the reexportSourceStmt DB query for every file in fileSymbols — including all previously-added barrels. The Rust implementation avoids this by only calling collect_reexport_from_barrels once for the initial seed and using the newly_added slice for subsequent collect_imported_barrel_candidates calls. The returned string[] is dead API today; if the intent is to scope future iterations, the value needs to be wired in.
There was a problem hiding this comment.
Fixed in b36a09d. reparseBarrelFiles now returns the relative paths of newly-merged files and the caller uses that array as the frontier for the next iteration of findBarrelCandidates, matching the Rust orchestrator's &newly_added slice. A firstPass flag also gates the reexportSourceStmt DB query so subsequent passes only walk newly-merged barrels' imports, avoiding the previous O(fileSymbols) re-query on every loop iteration.
Codegraph Impact Analysis19 functions changed → 11 callers affected across 8 files
|
Iteration 2+ of the JS barrel-discovery loop was re-querying the DB for every key in fileSymbols rather than just barrels newly merged in the previous pass, ballooning the work each iteration. The Rust orchestrator already does this correctly via the `&newly_added` slice. Wire the `string[]` returned by reparseBarrelFiles through the loop so each pass only walks newly-merged paths' imports, and gate the reexport-from DB query behind a `firstPass` flag — re-parsed barrels haven't changed content, so they can't surface new reexport-from candidates anyway. Matches the Rust seed-only `collect_reexport_from_barrels` call. No behavior change beyond perf; the regression test for #1174 (issue-1174-chained-barrel-incremental.test.ts) still passes on both engines and full=1371 / incremental=1371 imports edges on the dogfooded repo. Addresses Greptile feedback on #1179.
1 participant
Summary
Closes #1174.
Incremental rebuilds were silently dropping 32 imports edges on native (37 on WASM) whenever an unrelated file was touched. The lost edges never came back without
--no-incremental.Root cause: Stage 6b's barrel candidate discovery was single-pass. When a hybrid barrel (a file with ≥1 reexport + many local defs, e.g.
src/domain/parser.ts) was re-parsed, its outgoing edges were wiped — but the barrel-through edges from that barrel to leaf files (via a second-level barrel likesrc/extractors/index.ts) could not be re-emitted, because the second barrel was never loaded intofile_symbols.Changes
crates/codegraph-core/src/build_pipeline.rs— Stage 6b now iterates barrel discovery: after each batch of re-parses, scan the newly merged files' imports for further barrel candidates. Loop untilfile_symbolsstabilises. Convergence is guaranteed since the set of barrel files is bounded.src/domain/graph/builder/stages/resolve-imports.ts— Same iterative loop in the JS WASM-fallback pipeline. Three related JS-side defects fixed in the same commit because they were exposed once parser.ts started being processed correctly:reparseBarrelFileswas marking every re-parsed file as barrel-only, including hybrids. Now onlyisBarrelFile()-true files get marked.kind NOT IN ('contains', 'parameter_of')so thecontains/parameter_ofedges thatinsertNodesonly emits for changed files survive the re-parse.src/domain/graph/builder/stages/build-edges.ts— Scoped-load lazy fallback was filtering bykind != 'file'(broader than the upfront load's specific definition kinds), leaking parameter/property nodes into call resolution. Aligned both queries to the sameNODE_KIND_FILTER_SQLconstant.Verification
Dogfooded repo, both engines:
Native: every edge kind matches between full and incremental. WASM: imports/calls/parameter_of/contains/etc. all match (minor
imports-typestraggler is a separate concern, out of scope for #1174).New parameterized regression test in
tests/integration/issue-1174-chained-barrel-incremental.test.tscovers both engines on a fixture that mirrors the failing repo shape:app.js → parser.js (hybrid barrel) → extractors/index.js (pure barrel) → extractors/{alpha,beta,gamma,delta}.js.Test plan
full=1371,incremental=1371imports on both engines, across multiple edit/revert cyclesnpm run typecheckclean