Skip to content

Commit 246bce3

Browse files
committed
fix: iterate barrel re-parse discovery to stop dropping chained-barrel 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.
1 parent 4d8df7b commit 246bce3

12 files changed

Lines changed: 370 additions & 65 deletions

File tree

crates/codegraph-core/src/build_pipeline.rs

Lines changed: 119 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,16 @@ fn collect_source_files(
600600
/// Barrel files (re-export-only index files) may not be in file_symbols because
601601
/// they weren't changed or reverse-deps. Without their symbols, barrel resolution
602602
/// in Stage 7 can't create transitive import edges.
603+
///
604+
/// Discovery is iterative: a barrel that imports another barrel (e.g.
605+
/// `parser.ts → extractors/index.ts → extractors/<lang>.ts`) needs both
606+
/// loaded so Stage 7 can emit the barrel-through edges from the first barrel
607+
/// to the leaf targets. Without the loop, only the first level of barrels
608+
/// gets merged into `file_symbols`; the deeper chain has no entry in
609+
/// `reexport_map`, so `resolve_barrel_export` returns `None` and the
610+
/// barrel-through edges are silently dropped on every incremental rebuild
611+
/// (#1174). Convergence is guaranteed because `file_symbols` grows
612+
/// monotonically and is bounded by the set of barrel files in the project.
603613
fn reparse_barrel_candidates(
604614
conn: &Connection,
605615
root_dir: &str,
@@ -624,67 +634,38 @@ fn reparse_barrel_candidates(
624634
rows.into_iter().collect()
625635
};
626636

627-
// Check which barrels are imported by parsed files but not in file_symbols
628-
let mut barrel_paths_to_parse: Vec<String> = Vec::new();
629-
for (rel_path, symbols) in file_symbols.iter() {
630-
for imp in &symbols.imports {
631-
let abs_file = Path::new(root_dir).join(rel_path);
632-
let fwd = abs_file.to_str().unwrap_or("").replace('\\', "/");
633-
let key = format!("{}|{}", fwd, imp.source);
634-
if let Some(resolved) = batch_resolved.get(&key) {
635-
if barrel_files_in_db.contains(resolved) && !file_symbols.contains_key(resolved)
636-
{
637-
let abs = Path::new(root_dir).join(resolved);
638-
if abs.exists() {
639-
barrel_paths_to_parse
640-
.push(abs.to_str().unwrap_or("").to_string());
641-
}
642-
}
643-
}
644-
}
645-
}
646-
647-
// Also find barrels that re-export FROM changed files
648-
{
649-
let changed_rel: Vec<&str> = file_symbols.keys().map(|s| s.as_str()).collect();
650-
if let Ok(mut stmt) = conn.prepare(
651-
"SELECT DISTINCT n1.file FROM edges e \
652-
JOIN nodes n1 ON e.source_id = n1.id \
653-
JOIN nodes n2 ON e.target_id = n2.id \
654-
WHERE e.kind = 'reexports' AND n1.kind = 'file' AND n2.file = ?1",
655-
) {
656-
for changed in &changed_rel {
657-
if let Ok(rows) = stmt.query_map(rusqlite::params![changed], |row| {
658-
row.get::<_, String>(0)
659-
}) {
660-
for row in rows.flatten() {
661-
if !file_symbols.contains_key(&row) {
662-
let abs = Path::new(root_dir).join(&row);
663-
if abs.exists() {
664-
barrel_paths_to_parse
665-
.push(abs.to_str().unwrap_or("").to_string());
666-
}
667-
}
668-
}
669-
}
670-
}
671-
}
672-
}
673-
674-
// Re-parse barrel files and merge into file_symbols
675-
if !barrel_paths_to_parse.is_empty() {
637+
// Seed: barrels imported by the initial file_symbols (= changed files),
638+
// plus barrels that re-export FROM any changed file. The reexport-from
639+
// seed only fires on the initial pass — re-parsed barrels haven't
640+
// changed in content, so they can't trigger new reexport-from candidates.
641+
let initial_files: Vec<String> = file_symbols.keys().cloned().collect();
642+
let mut barrel_paths_to_parse: Vec<String> = collect_imported_barrel_candidates(
643+
root_dir,
644+
&initial_files,
645+
batch_resolved,
646+
&barrel_files_in_db,
647+
file_symbols,
648+
);
649+
barrel_paths_to_parse.extend(collect_reexport_from_barrels(
650+
conn,
651+
root_dir,
652+
&initial_files,
653+
file_symbols,
654+
));
655+
656+
// Iterative re-parse: each pass merges the queued barrels into file_symbols,
657+
// then scans their imports for additional barrel candidates the previous
658+
// pass couldn't see.
659+
while !barrel_paths_to_parse.is_empty() {
676660
barrel_paths_to_parse.sort();
677661
barrel_paths_to_parse.dedup();
662+
let to_parse = std::mem::take(&mut barrel_paths_to_parse);
678663
// Re-parse barrel candidates — these may be hybrid barrels (reexports
679664
// AND local definitions / call sites, see #979). Dataflow/AST analysis
680665
// is skipped because the barrel is not itself a "changed" file; Stage 7
681666
// will reconstruct all outgoing edge kinds from the fresh parse.
682-
let barrel_parsed = parallel::parse_files_parallel(
683-
&barrel_paths_to_parse,
684-
root_dir,
685-
false,
686-
false,
687-
);
667+
let barrel_parsed = parallel::parse_files_parallel(&to_parse, root_dir, false, false);
668+
let mut newly_added: Vec<String> = Vec::with_capacity(barrel_parsed.len());
688669
for mut sym in barrel_parsed {
689670
let rel = relative_path(root_dir, &sym.file);
690671
sym.file = rel.clone();
@@ -727,9 +708,91 @@ fn reparse_barrel_candidates(
727708
batch_resolved.insert(key, r.resolved_path.clone());
728709
}
729710
}
730-
file_symbols.insert(rel, sym);
711+
file_symbols.insert(rel.clone(), sym);
712+
newly_added.push(rel);
713+
}
714+
715+
// Scan just-merged barrels for further barrel imports (next level of
716+
// the chain). batch_resolved is now up to date for these imports.
717+
barrel_paths_to_parse = collect_imported_barrel_candidates(
718+
root_dir,
719+
&newly_added,
720+
batch_resolved,
721+
&barrel_files_in_db,
722+
file_symbols,
723+
);
724+
}
725+
}
726+
727+
/// Walk the imports of `from_files` and return absolute paths of any barrel
728+
/// candidates (files in `barrel_files_in_db` not yet in `file_symbols`) that
729+
/// exist on disk.
730+
fn collect_imported_barrel_candidates(
731+
root_dir: &str,
732+
from_files: &[String],
733+
batch_resolved: &HashMap<String, String>,
734+
barrel_files_in_db: &HashSet<String>,
735+
file_symbols: &HashMap<String, FileSymbols>,
736+
) -> Vec<String> {
737+
let mut out = Vec::new();
738+
for rel_path in from_files {
739+
let symbols = match file_symbols.get(rel_path) {
740+
Some(s) => s,
741+
None => continue,
742+
};
743+
let abs_file = Path::new(root_dir).join(rel_path);
744+
let fwd = abs_file.to_str().unwrap_or("").replace('\\', "/");
745+
for imp in &symbols.imports {
746+
let key = format!("{}|{}", fwd, imp.source);
747+
if let Some(resolved) = batch_resolved.get(&key) {
748+
if barrel_files_in_db.contains(resolved)
749+
&& !file_symbols.contains_key(resolved)
750+
{
751+
let abs = Path::new(root_dir).join(resolved);
752+
if abs.exists() {
753+
out.push(abs.to_str().unwrap_or("").to_string());
754+
}
755+
}
756+
}
757+
}
758+
}
759+
out
760+
}
761+
762+
/// Find barrels that re-export from any of `changed_files`. Used as a seed
763+
/// for the iterative re-parse so a renamed/removed symbol in a changed file
764+
/// re-emits the affected barrel's outgoing edges.
765+
fn collect_reexport_from_barrels(
766+
conn: &Connection,
767+
root_dir: &str,
768+
changed_files: &[String],
769+
file_symbols: &HashMap<String, FileSymbols>,
770+
) -> Vec<String> {
771+
let mut out = Vec::new();
772+
let mut stmt = match conn.prepare(
773+
"SELECT DISTINCT n1.file FROM edges e \
774+
JOIN nodes n1 ON e.source_id = n1.id \
775+
JOIN nodes n2 ON e.target_id = n2.id \
776+
WHERE e.kind = 'reexports' AND n1.kind = 'file' AND n2.file = ?1",
777+
) {
778+
Ok(stmt) => stmt,
779+
Err(_) => return out,
780+
};
781+
for changed in changed_files {
782+
if let Ok(rows) =
783+
stmt.query_map(rusqlite::params![changed], |row| row.get::<_, String>(0))
784+
{
785+
for row in rows.flatten() {
786+
if !file_symbols.contains_key(&row) {
787+
let abs = Path::new(root_dir).join(&row);
788+
if abs.exists() {
789+
out.push(abs.to_str().unwrap_or("").to_string());
790+
}
791+
}
792+
}
731793
}
732794
}
795+
out
733796
}
734797

735798
/// Stage 9: Finalize build — persist metadata, write journal, return counts.

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,9 +770,11 @@ function reconnectReverseDepEdges(ctx: PipelineContext): void {
770770
* their import targets. Falls back to loading ALL nodes for full builds or
771771
* larger incremental changes.
772772
*/
773+
const NODE_KIND_FILTER_SQL = `kind IN ('function','method','class','interface','struct','type','module','enum','trait','record','constant')`;
774+
773775
function loadNodes(ctx: PipelineContext): { rows: QueryNodeRow[]; scoped: boolean } {
774776
const { db, fileSymbols, isFullBuild, batchResolved } = ctx;
775-
const nodeKindFilter = `kind IN ('function','method','class','interface','struct','type','module','enum','trait','record','constant')`;
777+
const nodeKindFilter = NODE_KIND_FILTER_SQL;
776778

777779
// Gate: only scope for small incremental on large codebases
778780
if (!isFullBuild && fileSymbols.size <= ctx.config.build.smallFilesThreshold) {
@@ -816,8 +818,13 @@ function loadNodes(ctx: PipelineContext): { rows: QueryNodeRow[]; scoped: boolea
816818
function addLazyFallback(ctx: PipelineContext, scopedLoad: boolean): void {
817819
if (!scopedLoad) return;
818820
const { db } = ctx;
821+
// Match the upfront kind filter exactly. Using `kind != 'file'` here lets
822+
// parameters, properties, and other non-definition kinds leak into call
823+
// resolution, producing bogus call edges like `parser.ts → <a parameter
824+
// with the same name>` (#1174 follow-up). Calls only ever target the
825+
// definition kinds, so the fallback's filter must agree with `loadNodes`.
819826
const fallbackStmt = db.prepare(
820-
`SELECT id, name, kind, file, line FROM nodes WHERE name = ? AND kind != 'file'`,
827+
`SELECT id, name, kind, file, line FROM nodes WHERE name = ? AND ${NODE_KIND_FILTER_SQL}`,
821828
);
822829
const originalGet = ctx.nodesByName.get.bind(ctx.nodesByName);
823830
ctx.nodesByName.get = (name: string) => {

src/domain/graph/builder/stages/resolve-imports.ts

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,22 @@ function findBarrelCandidates(ctx: PipelineContext): Array<{ file: string }> {
9595
.all() as Array<{ file: string }>;
9696
}
9797

98-
/** Re-parse barrel files and update fileSymbols/reexportMap with fresh data. */
98+
/**
99+
* Re-parse barrel files and update fileSymbols/reexportMap with fresh data.
100+
* Returns the relative paths of newly-merged files so the caller can scan
101+
* them for the next level of barrel candidates.
102+
*
103+
* A re-parsed file is marked `barrel-only` only when it really is one (the
104+
* `isBarrelFile` check — reexports >= ownDefs). The previous unconditional
105+
* `.add(relPath)` caused hybrid barrels with many local defs (e.g. a file
106+
* with one `export type ... from` and dozens of internal functions) to drop
107+
* all their non-reexport imports in build-edges, since the barrel-only branch
108+
* skips them (#1174).
109+
*/
99110
async function reparseBarrelFiles(
100111
ctx: PipelineContext,
101112
barrelCandidates: Array<{ file: string }>,
102-
): Promise<void> {
113+
): Promise<string[]> {
103114
const { db, fileSymbols, rootDir, engineOpts } = ctx;
104115

105116
const barrelPaths: string[] = [];
@@ -109,18 +120,27 @@ async function reparseBarrelFiles(
109120
}
110121
}
111122

112-
if (barrelPaths.length === 0) return;
123+
if (barrelPaths.length === 0) return [];
113124

125+
// Preserve `contains` and `parameter_of` — those are emitted by insertNodes,
126+
// which only runs on the original (changed + reverse-dep) fileSymbols. Barrel
127+
// candidates are merged here *after* insertNodes, so wiping those kinds
128+
// would permanently drop them (mirrors the Rust orchestrator's Stage 6b
129+
// delete in build_pipeline.rs).
114130
const deleteOutgoingEdges = db.prepare(
115-
'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)',
131+
`DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)
132+
AND kind NOT IN ('contains', 'parameter_of')`,
116133
);
117134

135+
const added: string[] = [];
118136
try {
119137
const barrelSymbols = await parseFilesAuto(barrelPaths, rootDir, engineOpts);
120138
for (const [relPath, fileSym] of barrelSymbols) {
121139
deleteOutgoingEdges.run(relPath);
122140
fileSymbols.set(relPath, fileSym);
123-
ctx.barrelOnlyFiles.add(relPath);
141+
if (isBarrelFile(ctx, relPath)) {
142+
ctx.barrelOnlyFiles.add(relPath);
143+
}
124144
const reexports = fileSym.imports.filter((imp: Import) => imp.reexport);
125145
if (reexports.length > 0) {
126146
ctx.reexportMap.set(
@@ -132,10 +152,12 @@ async function reparseBarrelFiles(
132152
})),
133153
);
134154
}
155+
added.push(relPath);
135156
}
136157
} catch (e: unknown) {
137158
debug(`Barrel re-parse failed (non-fatal): ${(e as Error).message}`);
138159
}
160+
return added;
139161
}
140162

141163
export async function resolveImports(ctx: PipelineContext): Promise<void> {
@@ -156,8 +178,23 @@ export async function resolveImports(ctx: PipelineContext): Promise<void> {
156178

157179
ctx.barrelOnlyFiles = new Set<string>();
158180
if (!isFullBuild) {
159-
const barrelCandidates = findBarrelCandidates(ctx);
160-
await reparseBarrelFiles(ctx, barrelCandidates);
181+
// Iteratively discover and re-parse barrel chains. A barrel that imports
182+
// another barrel (e.g. `parser.ts → extractors/index.ts → extractors/<lang>.ts`)
183+
// needs both loaded so build-edges can emit the barrel-through edges from
184+
// the first barrel to the leaf targets. Without iteration, only the first
185+
// level of barrels gets merged into fileSymbols; the deeper chain has no
186+
// entry in reexportMap and the resolver silently drops the affected edges
187+
// on every incremental rebuild (#1174).
188+
//
189+
// Convergence is guaranteed because fileSymbols grows monotonically and
190+
// is bounded by the set of barrel files in the project — each iteration
191+
// either adds a previously-unseen barrel or terminates.
192+
while (true) {
193+
const before = fileSymbols.size;
194+
const barrelCandidates = findBarrelCandidates(ctx);
195+
await reparseBarrelFiles(ctx, barrelCandidates);
196+
if (fileSymbols.size === before) break;
197+
}
161198
}
162199
}
163200

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { runParser } from './parser.js';
2+
3+
export function main(input) {
4+
return runParser(input);
5+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function extractAlpha(input) {
2+
return `alpha:${input}`;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function extractBeta(input) {
2+
return `beta:${input}`;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function extractDelta(input) {
2+
return `delta:${input}`;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function extractGamma(input) {
2+
return `gamma:${input}`;
3+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Pure barrel — re-exports only, no local definitions.
2+
// Two-level chain test: parser.js (hybrid barrel) → this barrel → leaf files.
3+
export { extractAlpha } from './alpha.js';
4+
export { extractBeta } from './beta.js';
5+
export { extractDelta } from './delta.js';
6+
export { extractGamma } from './gamma.js';
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Hybrid barrel: has one re-export AND many local definitions.
2+
// Mirrors src/domain/parser.ts in the dogfooded reproduction of #1174:
3+
// the file is flagged as a barrel candidate by the orchestrator (because
4+
// it has ≥1 reexports edge in the DB) yet is *not* barrel-only because
5+
// its local defs outnumber its reexports.
6+
export { Token } from './types/index.js';
7+
8+
import { extractAlpha, extractBeta, extractDelta, extractGamma } from './extractors/index.js';
9+
10+
export function runParser(input) {
11+
const alpha = extractAlpha(input);
12+
const beta = extractBeta(input);
13+
const gamma = extractGamma(input);
14+
const delta = extractDelta(input);
15+
return combineResults(alpha, beta, gamma, delta);
16+
}
17+
18+
export function combineResults(a, b, c, d) {
19+
return [a, b, c, d].join('|');
20+
}
21+
22+
export function describeParser() {
23+
return 'chained-barrel parser';
24+
}
25+
26+
export function resetParser() {
27+
return null;
28+
}

0 commit comments

Comments
 (0)