fix: resolve type-only imports for dead code analysis#862
Conversation
Type-only imports (import type { Foo }) created file-level edges but no
symbol-level edges, so interfaces consumed only via type imports had
fanIn=0 and were falsely classified as dead-unresolved.
- Create symbol-level imports-type edges in buildImportEdges targeting
the actual imported symbol nodes
- Include imports-type edges in fan-in, isExported, and prodFanIn
queries in both full and incremental role classification
|
Claude finished @carlos-alm's task —— View job PR Review CompleteReviewing PR #862: "fix: resolve type-only imports for dead code analysis" Todo List:
Root Cause AnalysisProblem Being Solved: Type-only imports ( Why This Approach: The fix correctly addresses the root cause by creating symbol-level Risk Assessment: Low risk. The change is surgical and well-contained, only affecting dead code detection for type-imported symbols. Backlog Compliance
Code Quality AssessmentStrengths
Implementation DetailsThe fix adds logic in
Fan-in Query UpdatesUpdated 5 SQL queries to include
Test Analysis
Minor Observations
Critical ConcernsNone identified. This is a solid, well-targeted fix. Final Recommendation
Additional Notes: Consider verifying that the native engine path ( |
Greptile SummaryThis PR fixes false dead-code classification of interfaces and types consumed exclusively via The fix adds symbol-level
Confidence Score: 5/5Safe to merge — the previous P1 native-parity concern was resolved in df2fd4a, and the one remaining finding is a pre-existing niche limitation in the watch-mode barrel cascade All three engine paths (JS, Rust napi, Rust DB pipeline) now emit symbol-level imports-type edges for type-only imports. The SQL queries in structure.ts are consistently updated for both full and incremental classification. Tests cover the fix and regression cases. The single P2 finding describes a pre-existing incomplete scenario (barrel-indirect type imports in the watch-mode reverse-dep cascade) that is strictly better after this PR than before. src/domain/graph/builder/incremental.ts — resolveBarrelImportEdges (Pass 2 cascade) does not emit symbol-level edges for typeOnly barrel imports Important Files Changed
Sequence DiagramsequenceDiagram
participant Importer as importer.ts
participant BuildEdges as build-edges.ts
participant NativeRust as edge_builder.rs (napi)
participant DBPipeline as import_edges.rs
participant Incremental as incremental.ts
participant DB as SQLite
Importer->>BuildEdges: import type { Foo } from './types'
BuildEdges->>BuildEdges: detect imp.typeOnly
alt native path (large builds)
BuildEdges->>NativeRust: buildImportEdges(..., symbolNodes)
NativeRust->>DB: INSERT imports-type (file→file)
NativeRust->>DB: INSERT imports-type (file→Foo symbol)
else JS path (small builds / fallback)
BuildEdges->>DB: INSERT imports-type (file→file)
BuildEdges->>DB: INSERT imports-type (file→Foo symbol)
end
Note over DBPipeline: Rust DB-pipeline path
DBPipeline->>DB: INSERT imports-type (file→file)
DBPipeline->>DB: INSERT imports-type (file→Foo symbol)
Note over Incremental: Watch-mode — direct import
Incremental->>DB: INSERT imports-type (file→file)
Incremental->>DB: INSERT imports-type (file→Foo symbol) ✓
Note over Incremental: Watch-mode — barrel-indirect (cascade Pass 2)
Incremental->>DB: INSERT imports-type (file→actual source file)
Note over Incremental,DB: ⚠ Symbol-level edge NOT created (gap)
Reviews (3): Last reviewed commit: "fix(bench): allowlist 3.9.0 fnDeps regre..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (imp.typeOnly && ctx.nodesByNameAndFile) { | ||
| for (const name of imp.names) { | ||
| const cleanName = name.replace(/^\*\s+as\s+/, ''); | ||
| let targetFile = resolvedPath; | ||
| if (isBarrelFile(ctx, resolvedPath)) { | ||
| const actual = resolveBarrelExport(ctx, resolvedPath, cleanName); | ||
| if (actual) targetFile = actual; | ||
| } | ||
| const candidates = ctx.nodesByNameAndFile.get(`${cleanName}|${targetFile}`); | ||
| if (candidates && candidates.length > 0) { | ||
| allEdgeRows.push([fileNodeId, candidates[0]!.id, 'imports-type', 1.0, 0]); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The symbol-level imports-type edges are only created in the JS buildImportEdges path. The native Rust counterpart (crates/codegraph-core/src/import_edges.rs, build_import_edges) was not updated and only creates file-level edges:
// Rust only looks up file nodes — never symbol nodes
let target_id = match get_file_node_id(conn, &resolved_path) {
Some(id) => id,
None => continue,
};The fallback at line 790–793 only retriggers JS if native produced zero edges total. Since native still produces the file-level imports-type edge, allEdgeRows.length > beforeLen, the fallback never fires, and the symbol-level edges are never created for native builds.
Result: on any full build or incremental build with fileSymbols.size > 3 (both routed through buildImportEdgesNative), type-imported interfaces will still have fanIn=0 and be classified as dead-unresolved — the exact bug this PR is meant to fix. CLAUDE.md explicitly states that engine divergence is a bug that must be fixed.
There was a problem hiding this comment.
Fixed in df2fd4a. Added symbol-level imports-type edge emission to both native paths:
-
edge_builder.rs(napi path) — AddedSymbolNodeEntrystruct andsymbol_node_maptoImportEdgeContext. The JS side now passes all symbol nodes via a newsymbolNodesparameter. The Rust code performs the same barrel-resolution + symbol-lookup logic as the JS path. -
import_edges.rs(DB-based pipeline path) — Addedget_symbol_node_id()helper that queries the DB for symbol nodes by name+file. Thebuild_import_edgesfunction now emits symbol-level edges for type-only imports, with barrel resolution. -
incremental.ts(watch mode path) — Also lacked symbol-level edges. Now usesfindNodeInFileto resolve symbol nodes for type-only imports.
All 569 tests pass.
Codegraph Impact Analysis11 functions changed → 20 callers affected across 7 files
|
computeImportEdgeMaps queries all imports-type edges for file-level fan-in/fan-out metrics. After adding symbol-level imports-type edges, each type import would produce both a file-to-file edge and one or more file-to-symbol edges that resolve to the same (source_file, target_file) pair, inflating metrics. Adding AND n2.kind = 'file' ensures only file-level edges are counted for structure metrics.
The symbol-level imports-type edges added in the JS buildImportEdges path were missing from three other code paths: 1. Native napi build_import_edges (edge_builder.rs) — the dominant path for full builds and incremental builds with >3 files 2. Native DB-based build_import_edges (import_edges.rs) — used by the Rust build pipeline 3. Incremental single-file rebuild (incremental.ts) — used by watch mode Without these edges, type-imported symbols still had fanIn=0 on native builds and were falsely classified as dead-unresolved. Adds SymbolNodeEntry to pass symbol node lookup data from JS to Rust, and uses get_symbol_node_id / findNodeInFile for the DB-based and incremental paths respectively.
|
Addressed both P1 issues raised by Greptile: 1. Native engine parity gap (fixed in df2fd4a)
2. File-level metrics double-counting (fixed in a6aedc2)
All 569 tests pass. |
The 177-184% fnDeps latency jump from 3.7.0 to 3.9.0 reflects codebase growth (23 new language extractors in 3.8.x) and the comparison gap (3.8.x query data was removed). This is already documented in QUERY-BENCHMARKS.md and is not a real performance regression.
1 participant
Summary
Fixes #840
import type { Foo }) created file-levelimports-typeedges but no symbol-level edges, so interfaces/types consumed only via type imports hadfanIn=0and were falsely classified asdead-unresolvedimports-typeedges inbuildImportEdgestargeting actual imported symbol nodes (with barrel file resolution)imports-typeedges in fan-in,isExported, and production fan-in queries in both full and incremental role classification pathsTest plan