From 2bc224276579edccf8bee23ff2212a25a8b9ceb7 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Tue, 19 May 2026 22:57:33 -0400 Subject: [PATCH 1/7] Address impact and graph review findings --- REVIEW_PLAN.md | 55 ++++++++++++ src/graphs/symbol-graph-detailed.ts | 126 ++++++++++------------------ src/impact/analyzer.ts | 7 +- src/impact/context.ts | 49 +++++------ src/impact/hunks.ts | 94 +++++++++++++++++++++ src/impact/map.ts | 29 +------ src/impact/report-suggestions.ts | 59 ++----------- src/impact/report.ts | 14 +--- src/impact/streaming.ts | 21 ++++- src/impact/suggestions.ts | 20 +---- src/indexer/build-index.ts | 25 +++--- src/indexer/navigation.ts | 26 ++++-- 12 files changed, 281 insertions(+), 244 deletions(-) create mode 100644 REVIEW_PLAN.md create mode 100644 src/impact/hunks.ts diff --git a/REVIEW_PLAN.md b/REVIEW_PLAN.md new file mode 100644 index 00000000..7c5fbfab --- /dev/null +++ b/REVIEW_PLAN.md @@ -0,0 +1,55 @@ +# Code Review Follow-up Plan + +Scope: `src/graphs`, `src/impact`, and `src/indexer`. + +## Checklist + +- [x] Thread `maxRefs` through impact reference lookup. + - `src/impact/analyzer.ts` applies `maxRefs` after `findReferences()` returns, so hot symbols can still scan and contextualize more references than needed. + - Suggested fix: pass `maxReferences` to `findReferences()` with enough headroom for test and ignore filtering, then keep the existing post-filter cap. + - Tests: impact analysis with a hot symbol, ignored/test refs, and `refContext` enabled. + +- [x] Reuse parsed context for namespace member reference lookup. + - `src/indexer/navigation.ts` builds or retrieves scope for namespace imports, then `collectNamespaceMemberRefs()` reparses the same file with no cached context. + - Suggested fix: pass parsed context, or a tree/source/sup tuple, into namespace member collection. + - Tests: namespace reference lookup with `keepParsed` enabled and disabled. + +- [ ] Consolidate native and JS fallback import-binding conversion. + - `src/indexer/imports.ts` duplicates capture-to-`ImportBinding` conversion and per-language heuristics across native and JS fallback branches. + - Suggested fix: introduce a shared helper that accepts normalized captures and emits bindings. + - Tests: parity coverage for Java, C#, Go, Rust, Kotlin, Swift, Zig, C, and C++ import binding extraction. + +- [ ] Share module specifier parsing between graph and index extraction. + - `src/graphs/specifiers.ts` has its own parser path for Python, PHP, Kotlin, Rust, C#, JS/TS fallback, HTML-like, and stylesheet specifiers. + - Suggested fix: factor common statement/specifier parsing below both graph specifiers and index import binding extraction. + - Tests: graph/index parity tests for extracted raw specifiers and type-only behavior. + +- [x] Deduplicate member-chain resolution in detailed symbol graph. + - `src/graphs/symbol-graph-detailed.ts` contains two near-identical chain walkers for namespace/member usage. + - Suggested fix: extract a shared chain resolver that accepts the emitting callback and label. + - Tests: `membersOnly` and full detailed symbol graph cases for optional chaining, subscript/string keys, and namespace imports. + +- [x] Review manifest/index build edge merging for duplicate handling cost. + - `src/indexer/build-index.ts` only deduplicates workspace manifest edges through `appendUniqueGraphEdges`; per-file graph edges are appended directly. + - Suggested fix: decide whether duplicate edges are expected from per-file collectors, and centralize edge key generation if deduplication is required. + - Tests: duplicate import edges from mixed graph/index paths and workspace manifest edges. + +- [x] Extract shared diff hunk utilities for impact workflows. + - Hunk line parsing exists in `src/impact/map.ts`, `src/impact/suggestions.ts`, `src/impact/report.ts`, and `src/impact/report-suggestions.ts`. + - Suggested fix: create a small shared module for changed-line collection, added/removed line text extraction, removed-line mapping, and new-file hunk ranges. + - Tests: deletion-only hunks, mixed replacement hunks, EOF deletion, and report hunk range rendering. + +- [x] Reuse graph adjacency helpers in impact context and test candidate discovery. + - `src/impact/context.ts` rebuilds forward and reverse dependency maps locally instead of using `index.graphAdjacency` or `buildGraphAdjacency`. + - Suggested fix: route file-subgraph and candidate-test traversal through existing graph adjacency helpers while preserving edge metadata where needed. + - Tests: candidate test ranking and N-hop context traversal with duplicate and type-only edges. + +- [x] Cap reference scans in untested-change suggestions. + - `src/impact/report-suggestions.ts` calls `findReferences()` for every changed symbol while building test coverage suggestions, but does not pass `maxReferences`. + - Suggested fix: add a small cap or option-derived cap for this probe, since it only needs to know whether any reference is in a test file. + - Tests: exported hot symbol with many references and at least one test reference. + +- [x] Reduce expensive streaming impact item deduplication. + - `src/impact/streaming.ts` deduplicates emitted items by `JSON.stringify(item)`, which can become expensive when context refs are included in large partial updates. + - Suggested fix: use a stable structural key based on file, phase, reasons, symbols, severity/depth, and ref count instead of serializing full context payloads. + - Tests: streaming with repeated partial updates and `refContext` enabled. diff --git a/src/graphs/symbol-graph-detailed.ts b/src/graphs/symbol-graph-detailed.ts index eda59d1c..f79ac42a 100644 --- a/src/graphs/symbol-graph-detailed.ts +++ b/src/graphs/symbol-graph-detailed.ts @@ -286,6 +286,48 @@ export async function buildSymbolGraphDetailed( "optional_chain", sup.id === "python" ? "attribute" : "", ]); + const resolveMemberChainTarget = (chainNode: SyntaxNodeLike): SymbolDef | null => { + const names: string[] = []; + let current: SyntaxNodeLike | null = chainNode; + let base: SyntaxNodeLike | null = null; + const pushProp = (propNode: SyntaxNodeLike | null) => { + if (!propNode) return; + if (propertyIdentifierTypes.includes(propNode.type)) names.push(sliceText(propNode, src)); + else if (propNode.type === "string") names.push(unquote(sliceText(propNode, src))); + else if (propNode.type === "identifier") { + const keyName = sliceText(propNode, src); + const value = constStringOf.get(keyName); + if (typeof value === "string") names.push(value); + } + }; + while (current && optionalMemberTypes.has(current.type)) { + if (current.type === "subscript_expression") { + base = current.child(0) ?? base; + const indexNode = current.child(2); + pushProp(indexNode); + current = base; + } else if ( + current.type === memberExpressionType || + current.type === "optional_member_expression" || + current.type === "attribute" + ) { + base = current.child(0) ?? base; + const propNode = + current.childForFieldName?.("property") ?? current.child(2) ?? current.childForFieldName?.("attribute"); + pushProp(propNode); + current = base; + } else if (current.type === "optional_chain") { + current = current.child(0); + } else { + break; + } + } + if (!current || !isIdentifierType(sup, current.type)) return null; + const alias = sliceText(current, src); + const targetFile = aliasToTargetModule.get(alias); + if (!targetFile || !names.length) return null; + return resolveMemberPathFromModule(targetFile, names); + }; const walkCollect = (node: SyntaxNodeLike) => { if ( node.type === "function_declaration" || @@ -403,46 +445,7 @@ export async function buildSymbolGraphDetailed( node.child(0); const tryResolveChain = (node: SyntaxNodeLike, fromId?: string, label = "uses") => { - const names: string[] = []; - let current: SyntaxNodeLike | null = node; - let base: SyntaxNodeLike | null = null; - const pushProp = (propNode: SyntaxNodeLike | null) => { - if (!propNode) return; - if (propertyIdentifierTypes.includes(propNode.type)) names.push(sliceText(propNode, src)); - else if (propNode.type === "string") names.push(unquote(sliceText(propNode, src))); - else if (propNode.type === "identifier") { - const keyName = sliceText(propNode, src); - const value = constStringOf.get(keyName); - if (typeof value === "string") names.push(value); - } - }; - while (current && optionalMemberTypes.has(current.type)) { - if (current.type === "subscript_expression") { - base = current.child(0) ?? base; - const indexNode = current.child(2); - pushProp(indexNode); - current = base; - } else if ( - current.type === memberExpressionType || - current.type === "optional_member_expression" || - current.type === "attribute" - ) { - base = current.child(0) ?? base; - const propNode = - current.childForFieldName?.("property") ?? current.child(2) ?? current.childForFieldName?.("attribute"); - pushProp(propNode); - current = base; - } else if (current.type === "optional_chain") { - current = current.child(0); - } else { - break; - } - } - if (!current || !isIdentifierType(sup, current.type)) return false; - const alias = sliceText(current, src); - const targetFile = aliasToTargetModule.get(alias); - if (!targetFile || !names.length) return false; - const targetDef = resolveMemberPathFromModule(targetFile, names); + const targetDef = resolveMemberChainTarget(node); if (targetDef && fromId) { const toId = defNodeId(targetDef); if (!nodes.has(toId)) nodes.set(toId, nodeForDef(targetDef)); @@ -544,48 +547,7 @@ export async function buildSymbolGraphDetailed( const walkForMembers = (node: SyntaxNodeLike) => { const tryResolveChainLocal = (chainNode: SyntaxNodeLike) => { - const names: string[] = []; - let current: SyntaxNodeLike | null = chainNode; - let base: SyntaxNodeLike | null = null; - const pushProp = (propNode: SyntaxNodeLike | null) => { - if (!propNode) return; - if (propertyIdentifierTypes.includes(propNode.type)) names.push(sliceText(propNode, src)); - else if (propNode.type === "string") names.push(unquote(sliceText(propNode, src))); - else if (propNode.type === "identifier") { - const keyName = sliceText(propNode, src); - const value = constStringOf.get(keyName); - if (typeof value === "string") names.push(value); - } - }; - while (current && optionalMemberTypes.has(current.type)) { - if (current.type === "subscript_expression") { - base = current.child(0) ?? base; - const indexNode = current.child(2); - pushProp(indexNode); - current = base; - } else if ( - current.type === memberExpressionType || - current.type === "optional_member_expression" || - current.type === "attribute" - ) { - base = current.child(0) ?? base; - const propNode = - current.childForFieldName?.("property") ?? - current.child(2) ?? - current.childForFieldName?.("attribute"); - pushProp(propNode); - current = base; - } else if (current.type === "optional_chain") { - current = current.child(0); - } else { - break; - } - } - if (!current || !isIdentifierType(sup, current.type)) return; - const alias = sliceText(current, src); - const targetFile = aliasToTargetModule.get(alias); - if (!targetFile || !names.length) return; - const targetDef = resolveMemberPathFromModule(targetFile, names); + const targetDef = resolveMemberChainTarget(chainNode); if (targetDef) { const toId = defNodeId(targetDef); if (!nodes.has(toId)) nodes.set(toId, nodeForDef(targetDef)); diff --git a/src/impact/analyzer.ts b/src/impact/analyzer.ts index e2b186b6..2b1fbd49 100644 --- a/src/impact/analyzer.ts +++ b/src/impact/analyzer.ts @@ -57,6 +57,10 @@ const severityWeightKeys: ReadonlyArray = [ "depthDecay", ]; +function referenceScanLimitForKeptRefs(maxRefs: number): number { + return Math.max(maxRefs + 50, maxRefs * 4); +} + function normalizeSeverityWeights(weights: SeverityWeights): SeverityWeights { const normalized: SeverityWeights = { ...DEFAULT_SEVERITY_WEIGHTS }; const invalidEntries: string[] = []; @@ -205,8 +209,9 @@ export async function analyzeImpact( ...(refBlockMaxLines !== undefined && { blockMaxLines: refBlockMaxLines, }), + maxReferences: referenceScanLimitForKeptRefs(maxRefs), } - : undefined, + : { maxReferences: referenceScanLimitForKeptRefs(maxRefs) }, ); if (refs.status === "ok") { diff --git a/src/impact/context.ts b/src/impact/context.ts index d61204b7..dd65e5cf 100644 --- a/src/impact/context.ts +++ b/src/impact/context.ts @@ -2,6 +2,7 @@ import type { FileId } from "../types.js"; import type { ProjectIndex } from "../indexer.js"; import { buildSymbolGraphDetailed } from "../graphs.js"; import type { SymbolEdge } from "../graphs.js"; +import { buildGraphAdjacency, getForwardNeighbors, getReverseNeighbors } from "../graphs/adjacency.js"; import { createGraphFileResolver } from "./path.js"; import { compileTestPatterns, createIndexTestFileMatcher } from "./testPatterns.js"; @@ -67,6 +68,8 @@ function collectFileSubgraph( const edges: Array<{ from: FileId; to: FileId; typeOnly?: boolean }> = []; const visited = new Set(); const queue: Array<{ file: FileId; depth: number }> = []; + const adjacency = index.graphAdjacency ?? buildGraphAdjacency(index.graph); + const typeOnlyByEdge = new Map(); // Initialize with impacted files for (const file of impactedFiles) { @@ -75,21 +78,9 @@ function collectFileSubgraph( queue.push({ file, depth: 0 }); } - // Build forward and reverse dependency maps for efficient traversal - const forwardDeps = new Map(); // file -> files it depends on - const reverseDeps = new Map(); // file -> files that depend on it - for (const edge of index.graph.edges) { if (edge.to.type === "file") { - // Forward: A -> B means A depends on B - const deps = forwardDeps.get(edge.from) || []; - deps.push(edge.to.path); - forwardDeps.set(edge.from, deps); - - // Reverse: A -> B means B is depended on by A - const revDeps = reverseDeps.get(edge.to.path) || []; - revDeps.push(edge.from); - reverseDeps.set(edge.to.path, revDeps); + typeOnlyByEdge.set(`${edge.from}\0${edge.to.path}`, edge.typeOnly); } } @@ -100,31 +91,40 @@ function collectFileSubgraph( if (depth >= hops) continue; // Add forward dependencies (files this file depends on) - const deps = forwardDeps.get(file) || []; + const deps = getForwardNeighbors(adjacency, file); for (const dep of deps) { if (!visited.has(dep)) { visited.add(dep); nodes.add(dep); queue.push({ file: dep, depth: depth + 1 }); } - edges.push({ from: file, to: dep }); + edges.push(edgeFor(file, dep, typeOnlyByEdge)); } // Add reverse dependencies (files that depend on this file) - const revDeps = reverseDeps.get(file) || []; + const revDeps = getReverseNeighbors(adjacency, file); for (const revDep of revDeps) { if (!visited.has(revDep)) { visited.add(revDep); nodes.add(revDep); queue.push({ file: revDep, depth: depth + 1 }); } - edges.push({ from: revDep, to: file }); + edges.push(edgeFor(revDep, file, typeOnlyByEdge)); } } return { nodes, edges }; } +function edgeFor( + from: FileId, + to: FileId, + typeOnlyByEdge: ReadonlyMap, +): { from: FileId; to: FileId; typeOnly?: boolean } { + const typeOnly = typeOnlyByEdge.get(`${from}\0${to}`); + return typeOnly !== undefined ? { from, to, typeOnly } : { from, to }; +} + async function collectSymbolNeighbors( index: ProjectIndex, changedSymbolIds: string[], @@ -251,20 +251,11 @@ export function listCandidateTestFiles( const candidates = new Map(); const resolveGraphFile = createGraphFileResolver(index.graph.nodes); const resolvedChangedFiles = changedFiles.map((file) => resolveGraphFile(file)); + const adjacency = index.graphAdjacency ?? buildGraphAdjacency(index.graph); // Default test patterns (can be extended by caller) const allPatterns = compileTestPatterns(testPatterns); const isIndexTestFile = createIndexTestFileMatcher(index, allPatterns, projectRoot, resolvedChangedFiles); - // Build reverse dependency map: file -> files that depend on it - const reverseDeps = new Map(); - for (const edge of index.graph.edges) { - if (edge.to.type === "file") { - const deps = reverseDeps.get(edge.to.path) || []; - deps.push(edge.from); - reverseDeps.set(edge.to.path, deps); - } - } - // Find test files that import changed symbols directly const symbolFiles = new Set(); for (const symbolId of changedSymbolIds) { @@ -274,7 +265,7 @@ export function listCandidateTestFiles( } for (const file of symbolFiles) { - const dependents = reverseDeps.get(file) || []; + const dependents = getReverseNeighbors(adjacency, file); for (const dependent of dependents) { if (isIndexTestFile(dependent)) { candidates.set(dependent, { @@ -288,7 +279,7 @@ export function listCandidateTestFiles( // Find test files that depend on changed files (lower confidence) for (const changedFile of resolvedChangedFiles) { - const dependents = reverseDeps.get(changedFile) || []; + const dependents = getReverseNeighbors(adjacency, changedFile); for (const dependent of dependents) { if (isIndexTestFile(dependent) && !candidates.has(dependent)) { candidates.set(dependent, { diff --git a/src/impact/hunks.ts b/src/impact/hunks.ts new file mode 100644 index 00000000..bb2d6995 --- /dev/null +++ b/src/impact/hunks.ts @@ -0,0 +1,94 @@ +import type { FileChange } from "./types.js"; + +export type HunkLineText = { + added: string[]; + removed: string[]; + changed: string[]; +}; + +export function collectChangedLines(hunks: FileChange["hunks"]): Set { + const changedLines = new Set(); + for (const hunk of hunks) { + let oldLine = hunk.oldStart; + let newLine = hunk.newStart; + let deletionStreak = 0; + for (const line of hunk.lines) { + if (line.startsWith(" ")) { + oldLine += 1; + newLine += 1; + deletionStreak = 0; + } else if (line.startsWith("+")) { + changedLines.add(newLine); + newLine += 1; + deletionStreak = 0; + } else if (line.startsWith("-")) { + const mappedLine = newLine > 0 ? newLine + deletionStreak : oldLine; + changedLines.add(mappedLine); + oldLine += 1; + deletionStreak += 1; + } + } + } + return changedLines; +} + +export function collectRemovedLines(change: FileChange): Set { + const removed = new Set(); + for (const hunk of change.hunks) { + let oldLine = hunk.oldStart; + let newLine = hunk.newStart; + let deletionStreak = 0; + for (const line of hunk.lines) { + if (line.startsWith(" ")) { + oldLine += 1; + newLine += 1; + deletionStreak = 0; + continue; + } + if (line.startsWith("-")) { + const mapped = newLine > 0 ? newLine + deletionStreak : oldLine; + removed.add(mapped); + oldLine += 1; + deletionStreak += 1; + continue; + } + if (line.startsWith("+")) { + newLine += 1; + deletionStreak = 0; + } + } + } + return removed; +} + +export function collectHunkLineText(change: FileChange): HunkLineText { + const added: string[] = []; + const removed: string[] = []; + const changed: string[] = []; + for (const hunk of change.hunks) { + for (const line of hunk.lines) { + if (line.startsWith("+")) { + const text = line.slice(1); + added.push(text); + changed.push(text); + } else if (line.startsWith("-")) { + const text = line.slice(1); + removed.push(text); + changed.push(text); + } + } + } + return { added, removed, changed }; +} + +export function newFileRangeForHunk(hunk: FileChange["hunks"][number]): { start: number; end: number } { + let newLine = hunk.newStart; + let lastNewLine = newLine - 1; + for (const line of hunk.lines) { + if (line.startsWith(" ") || line.startsWith("+")) { + lastNewLine = newLine; + newLine += 1; + } + } + return { start: hunk.newStart, end: Math.max(hunk.newStart, lastNewLine) }; +} diff --git a/src/impact/map.ts b/src/impact/map.ts index 6d253f76..d0f32013 100644 --- a/src/impact/map.ts +++ b/src/impact/map.ts @@ -6,6 +6,9 @@ import { supportForFile } from "../languages.js"; import type { LanguageSupport } from "../languages.js"; import type { SyntaxNodeLike, SyntaxTreeLike } from "../languages/types.js"; import type { FileChange, ChangedSymbol } from "./types.js"; +import { collectChangedLines } from "./hunks.js"; + +export { collectChangedLines } from "./hunks.js"; function symbolHandleFromLocal(file: FileId, local: SymbolDef): string { const index = local.range.start.index ?? 0; @@ -225,32 +228,6 @@ function findNodesInLines(tree: SyntaxTreeLike, changedLines: Set): Synt return nodes; } -export function collectChangedLines(hunks: FileChange["hunks"]): Set { - const changedLines = new Set(); - for (const hunk of hunks) { - let oldLine = hunk.oldStart; - let newLine = hunk.newStart; - let deletionStreak = 0; - for (const line of hunk.lines) { - if (line.startsWith(" ")) { - oldLine++; - newLine++; - deletionStreak = 0; - } else if (line.startsWith("+")) { - changedLines.add(newLine); - newLine++; - deletionStreak = 0; - } else if (line.startsWith("-")) { - const mappedLine = newLine > 0 ? newLine + deletionStreak : oldLine; - changedLines.add(mappedLine); - oldLine++; - deletionStreak += 1; - } - } - } - return changedLines; -} - type NodeClassification = { type: "definition" | "import" | "export" | "callsite"; typeOnly?: boolean; diff --git a/src/impact/report-suggestions.ts b/src/impact/report-suggestions.ts index ec21111b..d547ddb1 100644 --- a/src/impact/report-suggestions.ts +++ b/src/impact/report-suggestions.ts @@ -3,76 +3,29 @@ import path from "node:path"; import { SymbolKind, type ProjectIndex, findReferences } from "../indexer.js"; import { mapLimit, resolveFilePathFromRoot } from "../util.js"; import { listCandidateTestFiles } from "./context.js"; +import { collectHunkLineText, collectRemovedLines } from "./hunks.js"; import { normalizeImpactFilePath } from "./path.js"; import { collectImpactSuggestions } from "./suggestions.js"; import { compileTestPatterns, createIndexTestFileMatcher } from "./testPatterns.js"; import type { ChangedSymbol, FileChange, ImpactOptions, ImpactSuggestion } from "./types.js"; const CONFIG_FILE_RE = /(^|\/)(?:tsconfig(?:\.[^./]+)?\.json|jsconfig\.json|vite\.config\.[cm]?[jt]s|webpack\.config\.[cm]?[jt]s|rollup\.config\.[cm]?[jt]s|esbuild\.config\.[cm]?[jt]s|babel\.config\.[cm]?[jt]s|\.eslintrc(?:\.[^./]+)?|prettier\.config\.[cm]?[jt]s|package\.json|pnpm-workspace\.ya?ml|lerna\.json|turbo\.json|nx\.json|\.env(?:\.[^/]*)?)$/i; - -function collectRemovedLines(change: FileChange): Set { - const removed = new Set(); - for (const hunk of change.hunks) { - let oldLine = hunk.oldStart; - let newLine = hunk.newStart; - let deletionStreak = 0; - for (const line of hunk.lines) { - if (line.startsWith(" ")) { - oldLine += 1; - newLine += 1; - deletionStreak = 0; - continue; - } - if (line.startsWith("-")) { - const mapped = newLine > 0 ? newLine + deletionStreak : oldLine; - removed.add(mapped); - oldLine += 1; - deletionStreak += 1; - continue; - } - if (line.startsWith("+")) { - newLine += 1; - deletionStreak = 0; - } - } - } - return removed; -} +const UNTESTED_CHANGE_REFERENCE_SCAN_LIMIT = 500; function matchesConfigSemantics(filePath: string): boolean { return CONFIG_FILE_RE.test(filePath); } function collectAddedLines(change: FileChange): string[] { - const added: string[] = []; - for (const hunk of change.hunks) { - for (const line of hunk.lines) { - if (line.startsWith("+")) added.push(line.slice(1)); - } - } - return added; + return collectHunkLineText(change).added; } function collectRemovedLinesText(change: FileChange): string[] { - const removed: string[] = []; - for (const hunk of change.hunks) { - for (const line of hunk.lines) { - if (line.startsWith("-")) removed.push(line.slice(1)); - } - } - return removed; + return collectHunkLineText(change).removed; } function collectRemovedAndAddedLines(change: FileChange): string[] { - const lines: string[] = []; - for (const hunk of change.hunks) { - for (const line of hunk.lines) { - if (line.startsWith("+") || line.startsWith("-")) { - lines.push(line.slice(1)); - } - } - } - return lines; + return collectHunkLineText(change).changed; } function collectTsconfigPathAliases(change: FileChange): string[] { @@ -457,6 +410,8 @@ async function collectUntestedChangeSuggestions( kind: symbol.kind, range: symbol.range, }, + }, { + maxReferences: UNTESTED_CHANGE_REFERENCE_SCAN_LIMIT, }); if (refs.status !== "ok") return undefined; diff --git a/src/impact/report.ts b/src/impact/report.ts index 11c3513a..60133791 100644 --- a/src/impact/report.ts +++ b/src/impact/report.ts @@ -23,19 +23,9 @@ import type { import { IMPACT_SCHEMA_VERSION } from "./types.js"; import { buildSymbolGraphDetailed, findDetailedCycles } from "../graphs.js"; import { discoverProjectFiles, normalizePath, resolveFilePathFromRoot } from "../util.js"; +import { newFileRangeForHunk } from "./hunks.js"; import { createGraphFileResolver, normalizeImpactFileChange, toImpactReportFilePath } from "./path.js"; - -export function newFileRangeForHunk(hunk: FileChange["hunks"][number]): { start: number; end: number } { - let newLine = hunk.newStart; - let lastNewLine = newLine - 1; - for (const line of hunk.lines) { - if (line.startsWith(" ") || line.startsWith("+")) { - lastNewLine = newLine; - newLine += 1; - } - } - return { start: hunk.newStart, end: Math.max(hunk.newStart, lastNewLine) }; -} +export { newFileRangeForHunk } from "./hunks.js"; export async function buildImpactReport( projectRoot: string, diff --git a/src/impact/streaming.ts b/src/impact/streaming.ts index f25eee58..509276d6 100644 --- a/src/impact/streaming.ts +++ b/src/impact/streaming.ts @@ -153,6 +153,24 @@ function buildLightStreamSummaryReport( }; } +function impactItemEmissionKey(item: ImpactItem, partial: boolean): string { + const symbols = item.symbols.slice().sort().join(","); + const reasons = item.reasons.slice().sort().join(","); + const refCount = item.refs?.length ?? 0; + const hintCount = item.explain?.hints?.length ?? 0; + return [ + item.file, + partial ? "partial" : "final", + symbols, + reasons, + item.severity.toFixed(6), + String(item.depth ?? 0), + String(item.confidence ?? 0), + String(refCount), + String(hintCount), + ].join("|"); +} + /** * Stream impact analysis results as they are discovered. * @@ -232,8 +250,7 @@ export async function* analyzeImpactStreaming( let impactedItems: ImpactItem[] = []; let impactError: string | null = null; const queueImpactItem = (item: ImpactItem, partial: boolean) => { - const signature = JSON.stringify(item); - const key = `${item.file}::${partial ? "partial" : "final"}::${signature}`; + const key = impactItemEmissionKey(item, partial); if (emittedSignatures.has(key)) return; emittedSignatures.add(key); impactQueue.push({ diff --git a/src/impact/suggestions.ts b/src/impact/suggestions.ts index aa44eca2..84e27250 100644 --- a/src/impact/suggestions.ts +++ b/src/impact/suggestions.ts @@ -4,6 +4,7 @@ import { goToDefinition, ensureParsedContext } from "../indexer.js"; import type { LanguageSupport } from "../languages.js"; import type { SyntaxNodeLike, SyntaxTreeLike } from "../languages/types.js"; import { normalizePath, resolveFilePathFromRoot, toProjectRelativePath } from "../util.js"; +import { collectChangedLines } from "./hunks.js"; import type { FileChange, ImpactOptions, ImpactSuggestion, ImpactSuggestionConfidence } from "./types.js"; type ReferenceCandidate = { @@ -345,25 +346,6 @@ function isInImportOrExport(node: SyntaxNodeLike): boolean { return false; } -function collectChangedLines(hunks: FileChange["hunks"]): Set { - const changedLines = new Set(); - for (const hunk of hunks) { - let newLine = hunk.newStart; - for (const line of hunk.lines) { - if (line.startsWith(" ")) { - newLine++; - } else if (line.startsWith("+")) { - changedLines.add(newLine); - newLine++; - } else if (line.startsWith("-")) { - const mappedLine = newLine; - changedLines.add(mappedLine); - } - } - } - return changedLines; -} - function resolveFilePath(projectRoot: string, file: FileId): FileId { return normalizePath(resolveFilePathFromRoot(projectRoot, file)); } diff --git a/src/indexer/build-index.ts b/src/indexer/build-index.ts index 640ee2e2..79b86dcf 100644 --- a/src/indexer/build-index.ts +++ b/src/indexer/build-index.ts @@ -475,6 +475,11 @@ function ensureJsonModule(modules: Map, filePath: string): }); } +function graphEdgeKey(edge: Edge): string { + const target = edge.to.type === "file" ? edge.to.path : edge.to.name; + return `${edge.from}::${target}::${edge.raw ?? ""}::${edge.typeOnly ? 1 : 0}`; +} + function expandStarImports(modules: Map): void { const importAlreadyPresent = (imports: ImportBinding[], candidate: ImportBinding): boolean => imports.some((existing) => { @@ -773,28 +778,20 @@ async function buildIndexFromFileListShared( }); if (timings) timings.parseMs = Math.round(performance.now() - parseStart); const graphStart = performance.now(); - const appendUniqueGraphEdges = (edges: Edge[]) => { + const seenGraphEdges = new Set(); + const appendUniqueGraphEdges = (edges: readonly Edge[]) => { if (!edges.length) return; - const seen = new Set( - graph.edges.map( - (edge) => - `${edge.from}::${edge.to.type === "file" ? edge.to.path : edge.to.name}::${edge.raw ?? ""}::${edge.typeOnly ? 1 : 0}`, - ), - ); for (const edge of edges) { - const key = `${edge.from}::${edge.to.type === "file" ? edge.to.path : edge.to.name}::${edge.raw ?? ""}::${edge.typeOnly ? 1 : 0}`; - if (seen.has(key)) continue; - seen.add(key); + const key = graphEdgeKey(edge); + if (seenGraphEdges.has(key)) continue; + seenGraphEdges.add(key); graph.edges.push(edge); if (edge.to.type === "file") graph.nodes.add(edge.to.path); } }; for (const [file, mod, edges] of fileResults) { modules.set(file, mod); - for (const edge of edges) { - graph.edges.push(edge); - if (edge.to.type === "file") graph.nodes.add(edge.to.path); - } + appendUniqueGraphEdges(edges); } const workspaceManifestEdges = await collectWorkspaceManifestDependencyEdges( projectRoot, diff --git a/src/indexer/navigation.ts b/src/indexer/navigation.ts index 6a9dd7eb..5f761be8 100644 --- a/src/indexer/navigation.ts +++ b/src/indexer/navigation.ts @@ -1,6 +1,6 @@ import { type LanguageSupport } from "../languages.js"; import type { SyntaxNodeLike, SyntaxTreeLike } from "../languages/types.js"; -import { ensureParsedContext } from "./parse-context.js"; +import { ensureParsedContext, type ParsedFileContext } from "./parse-context.js"; import { resolveMemberAccessDefinition } from "./navigation-goto.js"; import { findClosestBinding, @@ -271,10 +271,17 @@ export async function findReferences( if (!module) continue; let scopeIndex: ScopeIndex | null = null; + let candidateParsedContext: ParsedFileContext | null = null; + const ensureCandidateParsed = async (): Promise => { + if (!candidateParsedContext) { + const parsedEntry = index.parsed?.get(fileId); + candidateParsedContext = await ensureParsedContext(fileId, parsedEntry); + } + return candidateParsedContext; + }; const ensureScope = async (): Promise => { if (!scopeIndex) { - const parsedEntry = index.parsed?.get(fileId); - const parsed = await ensureParsedContext(fileId, parsedEntry); + const parsed = await ensureCandidateParsed(); scopeIndex = getCachedScope(index, fileId, module, parsed); } return scopeIndex; @@ -291,8 +298,8 @@ export async function findReferences( const hit = resolveExport(index, targetFile, exportedName); const matchesDef = hit?.kind === "resolved" ? sameDef(hit.def, def) : targetFile === definitionFile; if (!matchesDef) continue; - await ensureScope(); - const ranges = await collectNamespaceMemberRefs(fileId, imp.localNS, exportedName); + const parsed = await ensureCandidateParsed(); + const ranges = await collectNamespaceMemberRefs(fileId, imp.localNS, exportedName, parsed); for (const range of ranges) { if (hasReachedMaxReferences()) break; pushRef({ @@ -406,8 +413,13 @@ export async function findReferences( }; } -export async function collectNamespaceMemberRefs(file: string, ns: string, member: string): Promise { - const parsed = await ensureParsedContext(file, undefined); +export async function collectNamespaceMemberRefs( + file: string, + ns: string, + member: string, + parsedContext?: ParsedFileContext, +): Promise { + const parsed = parsedContext ?? (await ensureParsedContext(file, undefined)); const sup = parsed.sup; const source = parsed.source; const tree = parsed.tree; From 04b87f91df12712ed8367fede9a7d657d0cb2615 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Tue, 19 May 2026 23:04:01 -0400 Subject: [PATCH 2/7] Consolidate import extraction paths --- REVIEW_PLAN.md | 4 +- src/graphs/specifiers.ts | 15 +- src/indexer/imports.ts | 452 ++++++++++----------------------------- 3 files changed, 122 insertions(+), 349 deletions(-) diff --git a/REVIEW_PLAN.md b/REVIEW_PLAN.md index 7c5fbfab..51d77406 100644 --- a/REVIEW_PLAN.md +++ b/REVIEW_PLAN.md @@ -14,12 +14,12 @@ Scope: `src/graphs`, `src/impact`, and `src/indexer`. - Suggested fix: pass parsed context, or a tree/source/sup tuple, into namespace member collection. - Tests: namespace reference lookup with `keepParsed` enabled and disabled. -- [ ] Consolidate native and JS fallback import-binding conversion. +- [x] Consolidate native and JS fallback import-binding conversion. - `src/indexer/imports.ts` duplicates capture-to-`ImportBinding` conversion and per-language heuristics across native and JS fallback branches. - Suggested fix: introduce a shared helper that accepts normalized captures and emits bindings. - Tests: parity coverage for Java, C#, Go, Rust, Kotlin, Swift, Zig, C, and C++ import binding extraction. -- [ ] Share module specifier parsing between graph and index extraction. +- [x] Share module specifier parsing between graph and index extraction. - `src/graphs/specifiers.ts` has its own parser path for Python, PHP, Kotlin, Rust, C#, JS/TS fallback, HTML-like, and stylesheet specifiers. - Suggested fix: factor common statement/specifier parsing below both graph specifiers and index import binding extraction. - Tests: graph/index parity tests for extracted raw specifiers and type-only behavior. diff --git a/src/graphs/specifiers.ts b/src/graphs/specifiers.ts index 53aa3f18..7f43dc07 100644 --- a/src/graphs/specifiers.ts +++ b/src/graphs/specifiers.ts @@ -8,6 +8,7 @@ import { import { type LanguageSupport } from "../languages.js"; import { parseCsharpUsingDirective, + parseKotlinImportStatement, parsePhpImportStatement, parseRustImportStatement, } from "../languages/importStatementParsers.js"; @@ -60,12 +61,6 @@ function isHtmlLikeLanguage(languageId: string, filePath?: string): boolean { return !!filePath && filePath.toLowerCase().endsWith(".astro"); } -function extractKotlinImportSpecifier(statementText: string): string | null { - const match = statementText.match(/^\s*import\s+([A-Za-z_][\w.]*(?:\.\*)?)(?:\s+as\s+[A-Za-z_][\w]*)?\s*$/m); - if (!match?.[1]) return null; - return match[1].endsWith(".*") ? match[1].slice(0, -2) : match[1]; -} - function extractPhpQualifiedSpecifiersFromTree(source: string, tree: SyntaxTreeLike): ModuleSpecifier[] { const specifiers: ModuleSpecifier[] = []; const seen = new Set(); @@ -318,8 +313,8 @@ export function collectModuleSpecifiersFromSource( (support.id === "ts" || support.id === "tsx") && (/\b(import|export)\s+type\b/.test(stmtText) || /^\s*declare\s+module\s+["']/.test(stmtText)); if (support.id === "kotlin") { - const spec = extractKotlinImportSpecifier(stmtText); - if (spec) out.push({ spec, typeOnly: false }); + const parsed = parseKotlinImportStatement(stmtText); + if (parsed) out.push({ spec: parsed.from, typeOnly: false }); continue; } if (support.id === "rust") { @@ -405,8 +400,8 @@ export function collectModuleSpecifiersFromSource( (support.id === "ts" || support.id === "tsx") && (/\b(import|export)\s+type\b/.test(stmtText) || /^\s*declare\s+module\s+["']/.test(stmtText)); if (support.id === "kotlin") { - const spec = extractKotlinImportSpecifier(stmtText); - if (spec) out.push({ spec, typeOnly: false }); + const parsed = parseKotlinImportStatement(stmtText); + if (parsed) out.push({ spec: parsed.from, typeOnly: false }); continue; } if (support.id === "rust") { diff --git a/src/indexer/imports.ts b/src/indexer/imports.ts index 9cc1a44a..67b01a46 100644 --- a/src/indexer/imports.ts +++ b/src/indexer/imports.ts @@ -49,6 +49,8 @@ import type { LanguageSupport } from "../languages.js"; import type { JsLanguage } from "../languages/types.js"; import type { ImportBinding } from "./types.js"; +type ResolvedImportTarget = Exclude; + function parseObjectPatternBindings(patternText: string): Array<{ imported: string; local: string }> { const trimmed = patternText.trim(); if (!trimmed.startsWith("{") || !trimmed.endsWith("}")) return []; @@ -587,17 +589,105 @@ export async function collectImportsForFile( ? await loadNearestTsconfigFor(file, opts?.logLevel) : undefined; const workspaceConfig = await loadWorkspaceConfig(projectRoot); + const resolvedImportCache = new Map>(); - const resolveFrom = async (from: string, phpImportType?: "class" | "function" | "const") => { + const resolveFrom = async ( + from: string, + phpImportType?: "class" | "function" | "const", + ): Promise => { + const cacheKey = `${from}\0${phpImportType ?? ""}`; + const cached = resolvedImportCache.get(cacheKey); + if (cached) return await cached; const resolutionHints = opts?.graphOptions?.resolutionHints; - const resolved = await resolveImportSpecifier(projectRoot, file, from, resolvedSup.id, { - ...(tsCfg?.matchPath ? { matchPath: tsCfg.matchPath } : {}), - ...(workspaceConfig ? { workspaceConfig } : {}), - resolveNodeModules: !!opts?.graphOptions?.resolveNodeModules, - ...(resolutionHints ? { resolutionHints } : {}), - ...(phpImportType ? { phpImportType } : {}), - }); - return typeof resolved === "string" ? resolved.replace(/\\/g, "/") : resolved; + const resolved = (async (): Promise => { + const result = await resolveImportSpecifier(projectRoot, file, from, resolvedSup.id, { + ...(tsCfg?.matchPath ? { matchPath: tsCfg.matchPath } : {}), + ...(workspaceConfig ? { workspaceConfig } : {}), + resolveNodeModules: !!opts?.graphOptions?.resolveNodeModules, + ...(resolutionHints ? { resolutionHints } : {}), + ...(phpImportType ? { phpImportType } : {}), + }); + return typeof result === "string" ? result.replace(/\\/g, "/") : result; + })(); + resolvedImportCache.set(cacheKey, resolved); + return await resolved; + }; + const appendImplicitImportBinding = (args: { + from: string; + resolved: ResolvedImportTarget; + typeOnly: boolean; + stmtText: string; + alias?: string; + wildcard?: boolean; + }): void => { + const { from, resolved, typeOnly, stmtText, alias, wildcard } = args; + if (resolvedSup.id === "java") { + const parts = from.split("."); + const last = parts[parts.length - 1]; + if (last === "*") { + imports.push({ kind: "star", from, resolved, typeOnly }); + } else if (last && /^[A-Z]/.test(last)) { + imports.push({ kind: "named", local: last, imported: last, from, resolved, typeOnly }); + } + } else if (resolvedSup.id === "csharp") { + if (alias) { + const fromParts = from.split("."); + const imported = fromParts[fromParts.length - 1] ?? alias; + imports.push({ kind: "named", local: alias, imported, from, resolved, typeOnly }); + } else { + imports.push({ kind: "star", from, resolved, typeOnly }); + } + } else if (resolvedSup.id === "ruby") { + imports.push({ kind: "star", from, resolved }); + } else if (resolvedSup.id === "go") { + if (alias) { + if (alias === "_") return; + if (alias === ".") { + imports.push({ kind: "star", from, resolved }); + return; + } + imports.push({ kind: "namespace", localNS: alias, from, resolved }); + } else { + const parts = from.replace(/"/g, "").split("/"); + const last = parts[parts.length - 1]; + if (last) imports.push({ kind: "namespace", localNS: last, from, resolved }); + } + } else if (resolvedSup.id === "rust") { + if (stmtText.startsWith("mod ")) { + imports.push({ kind: "namespace", localNS: from, from, resolved }); + } else { + const parts = from.split("::"); + const last = parts[parts.length - 1]; + if (!last) return; + if (last === "*") { + imports.push({ kind: "star", from, resolved }); + } else { + imports.push({ kind: "named", local: last, imported: last, from, resolved }); + } + } + } else if (resolvedSup.id === "kotlin") { + if (wildcard || from.endsWith(".*")) { + imports.push({ kind: "star", from, resolved, typeOnly }); + } else { + const parts = from.split("."); + const imported = parts[parts.length - 1]; + if (imported) imports.push({ kind: "named", local: alias ?? imported, imported, from, resolved, typeOnly }); + } + } else if (resolvedSup.id === "swift") { + const parts = from.split("."); + const last = parts[parts.length - 1]; + if (!last) return; + if (parts.length === 1) { + imports.push({ kind: "namespace", localNS: last, from, resolved, typeOnly }); + imports.push({ kind: "star", from, resolved, typeOnly }); + } else { + imports.push({ kind: "named", local: last, imported: last, from, resolved, typeOnly }); + } + } else if (resolvedSup.id === "zig") { + if (alias) imports.push({ kind: "namespace", localNS: alias, from, resolved, typeOnly }); + } else if (resolvedSup.id === "c" || resolvedSup.id === "cpp") { + imports.push({ kind: "star", from, resolved, typeOnly }); + } }; const runFallback = async () => { @@ -807,149 +897,14 @@ export async function collectImportsForFile( } if (!caps["def"] && !caps["ns"] && !inames.length && !patterns.length) { - if (resolvedSup.id === "java") { - const parts = from.split("."); - const last = parts[parts.length - 1]; - if (last === "*") { - imports.push({ kind: "star", from, resolved, typeOnly }); - } else if (last && /^[A-Z]/.test(last)) { - imports.push({ - kind: "named", - local: last, - imported: last, - from, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "csharp") { - if (caps["alias"]) { - const alias = caps["alias"].text; - const fromParts = from.split("."); - const imported = fromParts[fromParts.length - 1] ?? alias; - imports.push({ - kind: "named", - local: alias, - imported, - from, - resolved, - typeOnly, - }); - } else { - imports.push({ kind: "star", from, resolved, typeOnly }); - } - } else if (resolvedSup.id === "ruby") { - imports.push({ kind: "star", from, resolved }); - } else if (resolvedSup.id === "go") { - if (caps["alias"]) { - const alias = caps["alias"].text; - if (alias === ".") { - imports.push({ - kind: "star", - from, - resolved, - }); - continue; - } - if (alias === "_") { - continue; - } - imports.push({ - kind: "namespace", - localNS: alias, - from, - resolved, - }); - } else { - const parts = from.replace(/"/g, "").split("/"); - const last = parts[parts.length - 1]; - if (last) { - imports.push({ - kind: "namespace", - localNS: last, - from, - resolved, - }); - } - } - } else if (resolvedSup.id === "rust") { - if (stmtText.startsWith("mod ")) { - imports.push({ - kind: "namespace", - localNS: from, - from, - resolved, - }); - } else { - const parts = from.split("::"); - const last = parts[parts.length - 1]; - if (!last) continue; - if (last === "*") { - imports.push({ kind: "star", from, resolved }); - } else { - imports.push({ - kind: "named", - local: last, - imported: last, - from, - resolved, - }); - } - } - } else if (resolvedSup.id === "kotlin") { - const wildcard = !!caps["wild"] || from.endsWith(".*"); - if (wildcard) { - imports.push({ kind: "star", from, resolved, typeOnly }); - } else { - const parts = from.split("."); - const imported = parts[parts.length - 1]; - if (!imported) continue; - imports.push({ - kind: "named", - local: caps["alias"]?.text ?? imported, - imported, - from, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "swift") { - const parts = from.split("."); - const last = parts[parts.length - 1]; - if (!last) continue; - if (parts.length === 1) { - imports.push({ - kind: "namespace", - localNS: last, - from, - resolved, - typeOnly, - }); - imports.push({ kind: "star", from, resolved, typeOnly }); - } else { - imports.push({ - kind: "named", - local: last, - imported: last, - from, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "zig") { - const alias = caps["alias"]?.text; - if (alias) { - imports.push({ - kind: "namespace", - localNS: alias, - from, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "c" || resolvedSup.id === "cpp") { - imports.push({ kind: "star", from, resolved, typeOnly }); - } + appendImplicitImportBinding({ + from, + resolved, + typeOnly, + stmtText, + ...(caps["alias"]?.text ? { alias: caps["alias"].text } : {}), + ...(caps["wild"] ? { wildcard: true } : {}), + }); } } await finalizeLanguageSpecificImports(); @@ -1105,191 +1060,14 @@ export async function collectImportsForFile( // Heuristics for languages where we captured @from but no explicit bindings if (fromValue && !caps["def"] && !caps["ns"] && !inames.length && !patterns.length) { - if (resolvedSup.id === "java") { - // import java.util.List; -> local "List" - const parts = fromValue.split("."); - const last = parts[parts.length - 1]; - if (last === "*") { - imports.push({ - kind: "star", - from: fromValue, - resolved, - typeOnly, - }); - } else if (last && /^[A-Z]/.test(last)) { - imports.push({ - kind: "named", - local: last, - imported: last, - from: fromValue, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "csharp") { - const aliasNode = caps["alias"]; - if (aliasNode) { - const alias = aliasNode.text; - // For "using Alias = Type.Path;", try to grab the last part as the imported name - let imported = alias; - const fromParts = fromValue.split("."); - if (fromParts.length) { - const candidate = fromParts[fromParts.length - 1]; - if (candidate) imported = candidate; - } - - imports.push({ - kind: "named", - local: alias, - imported, - from: fromValue, - resolved, - typeOnly, - }); - } else { - // implicit namespace import - treated as star to bring members into scope - imports.push({ - kind: "star", - from: fromValue, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "ruby") { - // require 'foo' -> star import to bring constants into scope - imports.push({ kind: "star", from: fromValue, resolved }); - } else if (resolvedSup.id === "go") { - // import "fmt" -> local "fmt" - // import "github.com/pkg/foo" -> local "foo" - const aliasNode = caps["alias"]; - if (aliasNode) { - const alias = aliasNode.text; - if (alias === ".") { - imports.push({ - kind: "star", - from: fromValue, - resolved, - }); - continue; - } - if (alias === "_") { - continue; - } - imports.push({ - kind: "namespace", - localNS: alias, - from: fromValue, - resolved, - }); - } else { - const parts = fromValue.replace(/"/g, "").split("/"); - const last = parts[parts.length - 1]; - if (!last) continue; - imports.push({ - kind: "namespace", - localNS: last, - from: fromValue, - resolved, - }); - } - } else if (resolvedSup.id === "rust") { - // mod utils; -> namespace (from="utils") - // use foo::bar; -> named (from="foo::bar") - if (stmtText.startsWith("mod ")) { - // treat 'mod name;' as namespace import pointing to name.rs / name/mod.rs - imports.push({ - kind: "namespace", - localNS: fromValue, - from: fromValue, - resolved, - }); - } else { - const parts = fromValue.split("::"); - const last = parts[parts.length - 1]; - if (!last) continue; - if (last === "*") { - imports.push({ kind: "star", from: fromValue, resolved }); - } else { - imports.push({ - kind: "named", - local: last, - imported: last, - from: fromValue, - resolved, - }); - } - } - } else if (resolvedSup.id === "kotlin") { - const aliasNode = caps["alias"]; - const wildcard = !!caps["wild"] || fromValue.endsWith(".*"); - if (wildcard) { - imports.push({ - kind: "star", - from: fromValue, - resolved, - typeOnly, - }); - } else { - const parts = fromValue.split("."); - const imported = parts[parts.length - 1]; - if (!imported) continue; - const local = aliasNode ? aliasNode.text : imported; - imports.push({ - kind: "named", - local, - imported, - from: fromValue, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "swift") { - const parts = fromValue.split("."); - const last = parts[parts.length - 1]; - if (!last) continue; - if (parts.length === 1) { - imports.push({ - kind: "namespace", - localNS: last, - from: fromValue, - resolved, - typeOnly, - }); - imports.push({ - kind: "star", - from: fromValue, - resolved, - typeOnly, - }); - } else { - imports.push({ - kind: "named", - local: last, - imported: last, - from: fromValue, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "zig") { - const alias = caps["alias"]?.text; - if (alias) { - imports.push({ - kind: "namespace", - localNS: alias, - from: fromValue, - resolved, - typeOnly, - }); - } - } else if (resolvedSup.id === "c" || resolvedSup.id === "cpp") { - imports.push({ - kind: "star", - from: fromValue, - resolved, - typeOnly, - }); - } + appendImplicitImportBinding({ + from: fromValue, + resolved, + typeOnly, + stmtText, + ...(caps["alias"]?.text ? { alias: caps["alias"].text } : {}), + ...(caps["wild"] ? { wildcard: true } : {}), + }); } } } catch (error) { From 3671bd8c4f2f3ae71f8b9ede333f67b08a49eb93 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Tue, 19 May 2026 23:18:04 -0400 Subject: [PATCH 3/7] Document second-pass complexity review --- REVIEW_PLAN.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/REVIEW_PLAN.md b/REVIEW_PLAN.md index 51d77406..85aa1b19 100644 --- a/REVIEW_PLAN.md +++ b/REVIEW_PLAN.md @@ -53,3 +53,39 @@ Scope: `src/graphs`, `src/impact`, and `src/indexer`. - `src/impact/streaming.ts` deduplicates emitted items by `JSON.stringify(item)`, which can become expensive when context refs are included in large partial updates. - Suggested fix: use a stable structural key based on file, phase, reasons, symbols, severity/depth, and ref count instead of serializing full context payloads. - Tests: streaming with repeated partial updates and `refContext` enabled. + +## Second-Pass Complexity Findings + +- [ ] Split the CLI dispatcher into shared command context plus focused command runners. + - `src/cli.ts` has a 1,188-line `runCliWithActiveRuntime()` block that mixes argument parsing, discovery setup, graph/index/review command execution, output formatting, report writing, and repeated build-option assembly. + - Suggested fix: extract a `CliCommandContext` builder, a `resolveCliScanPlan()` helper that returns files plus deleted/existing git state once, and move the remaining in-file command bodies into focused modules. Start with `graph`, `index`, `impact`, and `review` because they contain the most duplicated build/index/report plumbing. + - Tests: CLI regression coverage for `graph --sqlite`, `graph --symbols-detailed`, `index --json`, `impact --pretty`, `review --summary`, and include-root/git-diff combinations. + +- [ ] Factor import extraction into language-specific extractors with a shared binding sink. + - `src/indexer/imports.ts` still has a 1,018-line `collectImportsForFile()` orchestration block. It now shares implicit binding conversion, but graph-only handling, Python regex import extraction, statement overrides, native capture handling, and JS/CJS fallback remain coupled in one function. + - Suggested fix: introduce an `ImportExtractionContext` with `resolveFrom()`, `pushBinding()`, fallback reporting, and source/language metadata; move Python, JS/CJS fallback, graph-only, and native statement override logic into separate modules under `src/indexer/imports/`. + - Tests: import binding parity for TypeScript/JavaScript, Python multiline and alias forms, Java/Kotlin/C#/Go/Rust/PHP native and fallback paths, and graph-only module specifier behavior. + +- [ ] Decompose detailed symbol graph collection into reusable AST passes. + - `src/graphs/symbol-graph-detailed.ts` has a 690-line `buildSymbolGraphDetailed()` with nested walkers for definitions, aliases, member chains, calls, class inheritance, decorators, and Rust impls. + - Suggested fix: build per-file indexes (`localsByName`, imported alias maps, namespace maps), extract small collectors for `uses/calls`, member chains, decorators, class inheritance, and Rust impls, and run one merged traversal per function body where possible. + - Performance opportunity: current function-body processing walks the same subtree separately for alias uses, member uses, and calls; a merged visitor should reduce repeated AST traversal on large functions. + - Tests: detailed symbol graph edge cases for namespace members, `membersOnly`, decorator edges, Java/C# inheritance, Rust impls, optional chaining, and max-edge pruning. + +- [ ] Split review report generation into staged pipeline helpers. + - `src/review.ts` has a 485-line `buildReviewReport()` that handles change discovery, diff normalization, deleted snapshots, incremental indexing, changed-symbol mapping, reference lookups, file summaries, graph delta, candidate tests, SQL context, risk summary, and final report assembly. + - Suggested fix: extract `collectReviewChanges()`, `buildReviewIndex()`, `summarizeChangedFiles()`, `collectReviewGraphDelta()`, and `assembleReviewReport()` so each stage has explicit inputs/outputs and can be tested directly. + - Correctness opportunity: make deleted/missing/ignored file status transitions explicit in one data structure instead of spreading them across changed-file sets, diff maps, and final summary branches. + - Tests: raw diff, git `WORKTREE`, deleted files, missing explicit files, ignored diff files, include-symbol-details, SQL context, and candidate-test sorting. + +- [ ] Share full and incremental index-build state machines. + - `src/indexer/build-index.ts` has two large overlapping flows: `buildIndexFromFileListShared()` at 303 lines and `buildProjectIndexIncremental()` at 371 lines. Both initialize reports, graph options, file signatures, worker pools, parsed caches, bloom filters, JSON dependency modules, graph adjacency, manifest writes, and final `ProjectIndex` assembly. + - Suggested fix: extract reusable helpers for `IndexBuildRunState`, `prepareFileSignatures()`, `buildFileModules()`, `writeIndexManifest()`, `finalizeProjectIndex()`, and parsed-cache trimming. Keep full and incremental file-selection logic separate, but share execution and finalization. + - Performance opportunity: centralizing file signature and cached-edge reuse should make it easier to avoid recomputing graph edges or SQL fact caches when only module cache state changed. + - Tests: cache modes, incremental strict, manifest option mismatch, deleted tracked files, SQL corpus signatures, worker pool parity, and parsed-cache reuse. + +- [ ] Split multi-language resolution into per-language modules and unify project symbol indexes. + - `src/util/resolution.ts` is 1,872 lines and combines TS path config, generic specifier resolution, node_modules resolution, JVM package indexes, PHP composer/symbol scanning, Go module/workspace handling, and Python package resolution. + - Suggested fix: move language-specific resolvers into `src/util/resolution/{go,jvm,php,python,node}.ts` and keep `resolveImportSpecifier()` as a small dispatcher. Share a generic project-symbol-index builder for Java, Kotlin, and PHP while keeping PHP's multi-namespace/kind metadata. + - Correctness opportunity: PHP, Java, and Kotlin symbol indexers use regex/token scanners independent of native parsed symbols; consider reusing native/local export extraction where available so import resolution and indexed symbols stay aligned. + - Tests: resolution regression suites for Java, Kotlin, PHP Composer/autoload/classmap, Go workspaces, Python namespace packages, TS paths, node_modules exports, and cache clearing. From fa42adbd2ec7769f6a8de61de8b4f1e818742ecf Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Tue, 19 May 2026 23:22:04 -0400 Subject: [PATCH 4/7] Document third-pass complexity review --- REVIEW_PLAN.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/REVIEW_PLAN.md b/REVIEW_PLAN.md index 85aa1b19..00585b6b 100644 --- a/REVIEW_PLAN.md +++ b/REVIEW_PLAN.md @@ -89,3 +89,53 @@ Scope: `src/graphs`, `src/impact`, and `src/indexer`. - Suggested fix: move language-specific resolvers into `src/util/resolution/{go,jvm,php,python,node}.ts` and keep `resolveImportSpecifier()` as a small dispatcher. Share a generic project-symbol-index builder for Java, Kotlin, and PHP while keeping PHP's multi-namespace/kind metadata. - Correctness opportunity: PHP, Java, and Kotlin symbol indexers use regex/token scanners independent of native parsed symbols; consider reusing native/local export extraction where available so import resolution and indexed symbols stay aligned. - Tests: resolution regression suites for Java, Kotlin, PHP Composer/autoload/classmap, Go workspaces, Python namespace packages, TS paths, node_modules exports, and cache clearing. + +## Third-Pass Complexity Findings + +- [ ] Share declaration and export extraction infrastructure across locals, scope, and impact mapping. + - `src/indexer/locals-and-exports.ts` has a 598-line `collectLocalsAndExportsFromSource()` and `src/indexer/scope.ts` has a 263-line `buildScopeIndexFromSource()`; both classify declaration names, compute ranges, walk parameters, and interpret language-specific definition nodes. `src/impact/map.ts` also repeats declaration-name checks while locating changed symbols. + - Suggested fix: introduce a shared declaration walker that emits normalized declaration events (`name`, `kind`, `range`, `node`, `scopeRole`) and let locals/export extraction, scope indexing, and impact mapping consume those events. + - Correctness opportunity: this would keep language additions from updating locals but not scope, or scope but not impact symbol mapping. + - Tests: scope-quality, changed-line mapping, and cross-language local/export extraction for TypeScript, Python, Go, Rust, Kotlin, Swift, C/C++, PHP, and Ruby. + +- [ ] Split module specifier extraction from edge resolution and reuse fallback/query handling. + - `src/graphs/specifiers.ts` has a 330-line `collectModuleSpecifiersFromSource()` and `src/graph-edge-collector.ts` has a 109-complexity `collectEdgesForFile()`. Both contain language routing, fallback reporting, graph-only handling, HTML/style extras, and per-language resolution decisions. + - Suggested fix: make specifier extraction return normalized `ModuleSpecifier` values plus diagnostics only, then move edge target resolution into a table of language-specific resolver functions. Share helper code for native query execution, JS fallback recovery, HTML/style appended specifiers, and graph-only resolution config. + - Performance opportunity: the edge collector currently resolves all specifiers via `Promise.all`, which can burst filesystem work for large files; a bounded resolver queue would make graph builds smoother without changing results. + - Tests: fallback import extraction diagnostics, graph-only document links, CSS/SCSS/Less URL imports, HTML/Vue/Svelte/Astro imports, PHP qualified references, JVM package fan-out, and dynamic import heuristics. + +- [ ] Unify member-access parsing for goto, references, and detailed symbol graph. + - Member/object/property extraction appears in `src/indexer/navigation-goto.ts`, `src/indexer/navigation.ts` (`collectNamespaceMemberRefs()`), and `src/graphs/symbol-graph-detailed.ts`. Each handles Python/Ruby/Go/Java/C#/Kotlin/Swift member shapes with local variations. + - Suggested fix: create a `memberAccess.ts` helper that normalizes member nodes into `{ object, property, chain }` and supports language-specific node shapes. Use it in goto, namespace reference collection, and detailed symbol graph member/call collectors. + - Correctness opportunity: optional chaining, Kotlin/Swift navigation expressions, Ruby scope resolution, and Go qualified types should resolve consistently across goto, refs, and symbol graph edges. + - Tests: `goto`, `references`, and detailed-symbol edge cases for namespace members, optional/member chains, Ruby `::`, Go qualified types, Java static members, C# nested members, Kotlin aliases, and Swift static factories. + +- [ ] Extract shared SQL lexical helpers and reuse them across facts, navigation, and MCP query checks. + - `splitTopLevelCommaSeparated()` is duplicated in `src/sql/extractFacts.ts` and `src/sql/navigation.ts`; paren-depth logic is also duplicated. `src/mcp/server.ts` has its own SQL comment/literal stripper for resource checks. + - Suggested fix: create `src/sql/lex.ts` for top-level splitting, paren depth, comment/string masking, and bounded identifier scanning. Reuse it in fact extraction, SQL navigation, and MCP SQLite query validation. + - Correctness opportunity: SQL parsing edge cases such as quoted strings, comments, nested expressions, and CTE clauses should not diverge between review facts and navigation. + - Tests: SQL fact extraction, SQL navigation, SQL review context, and MCP `query_sqlite` resource-bound rejection tests. + +- [ ] Share native binding contracts between runtime and worker code. + - `NativeBinding` is declared separately in `src/native/treeSitterNative.ts` and `src/worker/nativeExtractWorker.ts`, with slightly different optional capabilities. + - Suggested fix: move the binding interface and result/fallback reason contracts into a shared native module imported by both runtime and worker code. + - Correctness opportunity: adding a native capability such as syntax-tree parsing or compact queries should update one contract instead of relying on duplicated structural types. + - Tests: native binding loader, native worker parity, native runtime mode, and native parser ownership tests. + +- [ ] Decompose project-file discovery parsers into manifest-specific helpers. + - `src/util/projectFiles.ts` combines discovery traversal with many manifest-name parsers (`package.json`, TOML, INI, setup.py, Maven, Gradle, .NET, Go, Gem, Swift) and a long `PROJECT_FILE_DEFINITIONS` registry. + - Suggested fix: move parser helpers and definitions into `src/util/projectFiles/definitions.ts` and `src/util/projectFiles/parsers.ts`, leaving traversal/path-safety logic in the main module. + - Correctness opportunity: parser tests can cover manifest name extraction directly, instead of only through full project discovery. + - Tests: project file discovery, workspace detection, language parity docs fixtures where project metadata affects external classification. + +- [ ] Split MCP server into tool registry, HTTP transport, file security, and SQL guard modules. + - `src/mcp/server.ts` is 1,286 lines and mixes MCP tool handlers, Streamable HTTP session handling, Host-header checks, JSON body parsing, file confinement, file-prefix reads, UTF-8 truncation, SQLite query guarding, and row/byte bounding. + - Suggested fix: extract `mcp/tools.ts`, `mcp/http.ts`, `mcp/security.ts`, and `mcp/sqliteGuard.ts`, keeping `server.ts` as composition glue. + - Reuse opportunity: UTF-8 truncation and path confinement are general utilities that could also support artifact/file-reading code paths. + - Tests: MCP server tests for tool handlers, HTTP Host validation, request size limits, artifact path confinement, file reads, SQLite row/byte limits, and unsupported SQL functions. + +- [ ] Centralize agent limit normalization and bounded output shaping. + - `src/agent/search.ts`, `src/agent/explain.ts`, and `src/agent-tools.ts` each normalize limits, bound results, and convert file/module references into agent-facing shapes. Handle formatting is shared, but bounded-list and output normalization policies are still scattered. + - Suggested fix: introduce `src/agent/bounds.ts` and `src/agent/normalize.ts` for limit clamping, bounded-list metadata, relative path normalization, and common follow-up command shaping. + - Correctness opportunity: agent search, explain, artifact questions, and tool wrappers should report omission counts and path shapes consistently. + - Tests: agent search, agent explain, agent tools, artifact build, and package metadata API-surface tests. From 7250151925c12f5410f2a449b65550bb09c8f5a0 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Tue, 19 May 2026 23:53:47 -0400 Subject: [PATCH 5/7] Document fourth-pass complexity review --- REVIEW_PLAN.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/REVIEW_PLAN.md b/REVIEW_PLAN.md index 00585b6b..8c96351a 100644 --- a/REVIEW_PLAN.md +++ b/REVIEW_PLAN.md @@ -139,3 +139,53 @@ Scope: `src/graphs`, `src/impact`, and `src/indexer`. - Suggested fix: introduce `src/agent/bounds.ts` and `src/agent/normalize.ts` for limit clamping, bounded-list metadata, relative path normalization, and common follow-up command shaping. - Correctness opportunity: agent search, explain, artifact questions, and tool wrappers should report omission counts and path shapes consistently. - Tests: agent search, agent explain, agent tools, artifact build, and package metadata API-surface tests. + +## Fourth-Pass Complexity Findings + +- [ ] Split native runtime orchestration from native query execution helpers. + - `src/native/treeSitterNative.ts` is a high fan-in hotspot and now owns binding loading, runtime-mode enforcement, normalized query caching, compact/full query execution, single-query execution, JS fallback bridging, and syntax-tree parsing. + - Suggested fix: extract native binding state/runtime-mode helpers, normalized query metadata, query execution wrappers, and JS fallback bridging into focused modules under `src/native/`. Keep the public runtime facade small and preserve existing exported entry points. + - Correctness opportunity: compact imports, full language queries, ad-hoc queries, and syntax-tree parsing repeat fallback reason/error shaping; one result-normalization helper would keep native-required failures and unsupported-language behavior consistent. + - Tests: native runtime mode, native query normalization, compact imports fallback, native parser ownership, native worker parity, and explicit native-required failure paths. + +- [ ] Decompose SQLite persistence into schema, write/update, and query modules. + - `src/sqlite.ts` is 920 lines and combines schema creation/migration, insert/delete helpers, full writes, incremental updates, canned graph queries, raw read-only query validation, and snapshot metadata. + - Suggested fix: split `sqlite/schema.ts`, `sqlite/write.ts`, `sqlite/update.ts`, `sqlite/query.ts`, and `sqlite/guards.ts`. Keep schema-version handling and migration helpers isolated so persistent storage changes have one upgrade path. + - Correctness opportunity: `ensureSchema()` creates tables and patches columns in the same file as query execution; separating schema upgrades would make older on-disk database regression tests easier to maintain. + - Tests: SQLite full write, incremental update/delete, schema migration from older fixtures, raw read-only guard behavior, artifact SQLite generation, and MCP SQLite query paths. + +- [ ] Extract impact report assembly into reusable compact/full report stages. + - `src/impact/report.ts` has a 265-line `buildCompactReport()` plus full-report assembly, re-export chain discovery, top impacts, clusters, cycles, and surface-area summaries in one module. + - Suggested fix: introduce staged helpers for display-file normalization, file-index construction, compact serializers, graph summary sections, re-export chains, top impacts, clusters, and surface area. Share the precomputed file/symbol indexes between compact and full formats. + - Performance opportunity: compact report construction repeatedly calls `displayFile()` and looks up file indexes across each section; a shared serializer context can avoid repeated normalization and make missing-index errors explicit. + - Tests: compact/full impact reports, project file metadata, re-export chains, graph cycles, clusters, surface area summaries, top impacts, and schema-version compatibility. + +- [ ] Split impact analyzer into direct-reference, transitive, and severity calculators. + - `src/impact/analyzer.ts` has a 221-line `analyzeImpact()` that handles option normalization, ignore/test matchers, bounded reference lookup, streaming emission, direct impact merging, file-level changes, and transitive propagation. The same module also owns severity scoring. + - Suggested fix: extract `impact/direct.ts`, `impact/transitive.ts`, and `impact/severity.ts` around a shared `ImpactAnalysisContext` containing matchers, dependency stats, diagnostics, and emit hooks. + - Correctness opportunity: include-test and ignore-glob policy is rebuilt across direct and transitive phases; one context would keep filtering and diagnostics consistent as new impact reasons are added. + - Tests: direct refs with `maxRefs`, ignored/test refs, file-level changes, transitive depth/type-only edges, diagnostics counters, streaming partial items, and severity weight overrides. + +- [ ] Centralize build-cache option normalization, manifest comparison, and reports. + - `src/indexer/build-cache.ts` is 807 lines and mixes workspace manifest edges, memory/disk module cache, file signatures, fallback extraction reports, manifest IO, build-option summaries, diffing, and graph-option equality. + - Suggested fix: split cache option normalization/equality into `indexer/build-cache/options.ts`, manifest IO/verification into `manifest.ts`, module cache read/write into `module-cache.ts`, and report shaping into `reports.ts`. + - Correctness opportunity: the same normalized option shapes drive manifest writes, manifest diffs, and graph-option equality; a single typed comparer would reduce false rebuilds and missed rebuilds when discovery or graph options change. + - Tests: cache invalidation, cache strict/off modes, discovery option normalization, graph option equality, fallback extraction report aggregation, disk cache reuse, and manifest mismatch messages. + +- [ ] Decompose graph-only document link extraction and chunking by format. + - `src/documentLinks.ts` is 692 lines and routes Markdown, MDX, Astro, Handlebars, reStructuredText, AsciiDoc, HTML attributes, inline scripts, link normalization, and Markdown parsing in one file. `src/chunking/chunkFile.ts` is another large format-sensitive flow for block extraction, splitting, merging, and gap filling. + - Suggested fix: move document extractors into `documentLinks/{markdown,html,rst,asciidoc,sfc}.ts` behind a small dispatcher, and split chunking into match collection, block classification, large-block splitting, merge, and gap-fill helpers. + - Correctness opportunity: document specifier normalization should match graph-only edge extraction, chunking, and source-style imports for mixed formats like MDX/Astro/SFC files. + - Tests: Markdown reference/inline links, MDX/Astro/Handlebars imports, RST toctrees/targets, AsciiDoc xref/include/link forms, HTML `srcset`/inline scripts, CSS-style URLs, and chunk splitting/merge behavior. + +- [ ] Split external dependency classification into manifest parsers, stdlib tables, and context lookup. + - `src/graphs/external-classifier.ts` is 743 lines and contains large stdlib tables, manifest parsers for many ecosystems, ancestor-boundary search, package-name matching, caches, and final external classification. + - Suggested fix: move ecosystem manifest readers into `graphs/external/manifests.ts`, stdlib/module tables into `stdlib.ts`, context/ancestor lookup into `context.ts`, and leave `classifyExternalSpecifier()` as a small coordinator. + - Correctness opportunity: manifest parsing overlaps with project-file discovery but uses separate parsing rules; extracting parser units makes it easier to align dependency detection with discovery fixtures and language parity claims. + - Tests: unresolved import classification for Node, Python, Ruby, Go, Rust, Zig, Java/Kotlin, .NET, C/C++, Swift, Composer, nested manifests, VCS boundaries, and cache reset/stats. + +- [ ] Separate graph query parsing and traversal execution from SQLite query dispatch. + - `src/query.ts`, `src/graphs/queries.ts`, `src/cli/graphQueries.ts`, and `src/sqlite.ts` each participate in graph-query parsing, graph traversal, result bounding, cycle detail construction, and canned SQL-backed query dispatch. + - Suggested fix: introduce a typed query AST/parser module and execution modules for in-memory graph queries and SQLite-backed canned queries. Reuse traversal helpers for neighbors, shortest paths, reverse dependencies, cycles, and unresolved imports. + - Performance opportunity: `querySymbolNeighbors()` builds incoming/outgoing maps per call and then scans all edges again to materialize results; reusable adjacency indexes would help interactive agents and CLI graph queries on large symbol graphs. + - Tests: text query parsing, symbol neighbor depth/direction/label filters, detailed cycle ordering/remediation hints, unresolved import classification, CLI graph query output, and SQLite canned query parity. From 90f9f0ff464534e4c71fdd51bda4e05c482a8944 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Wed, 20 May 2026 00:10:09 -0400 Subject: [PATCH 6/7] Address PR review comments --- src/impact/context.ts | 20 +++++++++++++------- src/indexer/build-index.ts | 2 +- tests/impact-context.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/impact/context.ts b/src/impact/context.ts index dd65e5cf..e4486d95 100644 --- a/src/impact/context.ts +++ b/src/impact/context.ts @@ -69,7 +69,7 @@ function collectFileSubgraph( const visited = new Set(); const queue: Array<{ file: FileId; depth: number }> = []; const adjacency = index.graphAdjacency ?? buildGraphAdjacency(index.graph); - const typeOnlyByEdge = new Map(); + const typeOnlyByPair = new Map(); // Initialize with impacted files for (const file of impactedFiles) { @@ -80,7 +80,11 @@ function collectFileSubgraph( for (const edge of index.graph.edges) { if (edge.to.type === "file") { - typeOnlyByEdge.set(`${edge.from}\0${edge.to.path}`, edge.typeOnly); + const key = `${edge.from}\0${edge.to.path}`; + const current = typeOnlyByPair.get(key) ?? { allTypeOnly: true, hasTypeOnlyMetadata: false }; + current.allTypeOnly = current.allTypeOnly && edge.typeOnly === true; + current.hasTypeOnlyMetadata = current.hasTypeOnlyMetadata || edge.typeOnly !== undefined; + typeOnlyByPair.set(key, current); } } @@ -98,7 +102,7 @@ function collectFileSubgraph( nodes.add(dep); queue.push({ file: dep, depth: depth + 1 }); } - edges.push(edgeFor(file, dep, typeOnlyByEdge)); + edges.push(edgeFor(file, dep, typeOnlyByPair)); } // Add reverse dependencies (files that depend on this file) @@ -109,7 +113,7 @@ function collectFileSubgraph( nodes.add(revDep); queue.push({ file: revDep, depth: depth + 1 }); } - edges.push(edgeFor(revDep, file, typeOnlyByEdge)); + edges.push(edgeFor(revDep, file, typeOnlyByPair)); } } @@ -119,10 +123,12 @@ function collectFileSubgraph( function edgeFor( from: FileId, to: FileId, - typeOnlyByEdge: ReadonlyMap, + typeOnlyByPair: ReadonlyMap, ): { from: FileId; to: FileId; typeOnly?: boolean } { - const typeOnly = typeOnlyByEdge.get(`${from}\0${to}`); - return typeOnly !== undefined ? { from, to, typeOnly } : { from, to }; + const typeOnly = typeOnlyByPair.get(`${from}\0${to}`); + if (!typeOnly) return { from, to }; + if (typeOnly.allTypeOnly) return { from, to, typeOnly: true }; + return typeOnly.hasTypeOnlyMetadata ? { from, to, typeOnly: false } : { from, to }; } async function collectSymbolNeighbors( diff --git a/src/indexer/build-index.ts b/src/indexer/build-index.ts index 79b86dcf..1d8982e7 100644 --- a/src/indexer/build-index.ts +++ b/src/indexer/build-index.ts @@ -476,7 +476,7 @@ function ensureJsonModule(modules: Map, filePath: string): } function graphEdgeKey(edge: Edge): string { - const target = edge.to.type === "file" ? edge.to.path : edge.to.name; + const target = edge.to.type === "file" ? `file:${edge.to.path}` : `external:${edge.to.name}`; return `${edge.from}::${target}::${edge.raw ?? ""}::${edge.typeOnly ? 1 : 0}`; } diff --git a/tests/impact-context.test.ts b/tests/impact-context.test.ts index a5a73efa..ae9e6fd8 100644 --- a/tests/impact-context.test.ts +++ b/tests/impact-context.test.ts @@ -6,6 +6,8 @@ import { collectImpactContext, listCandidateTestFiles } from "../src/impact/inde import { createTestIndex } from "./test-utils.js"; import { buildProjectIndex, buildProjectIndexFromFiles } from "../src/index.js"; import { normalizePath } from "../src/util.js"; +import type { ProjectIndex } from "../src/indexer.js"; +import type { Edge } from "../src/types.js"; describe("Impact Context Collection", () => { describe("collectImpactContext", () => { @@ -68,6 +70,30 @@ describe("Impact Context Collection", () => { expect(context.neighborFiles.size).toBe(0); }); + it("marks a file-pair edge as type-only only when all matching graph edges are type-only", async () => { + const sourceFile = "/repo/src/source.ts"; + const targetFile = "/repo/src/types.ts"; + const edges: Edge[] = [ + { from: sourceFile, to: { type: "file", path: targetFile }, raw: "./types", typeOnly: true }, + { from: sourceFile, to: { type: "file", path: targetFile }, raw: "./types-runtime", typeOnly: false }, + ]; + const index: ProjectIndex = { + graph: { + nodes: new Set([sourceFile, targetFile]), + edges, + }, + modules: new Map(), + byFile: new Map(), + exportCache: new Map(), + scopeCache: new Map(), + }; + + const context = await collectImpactContext(index, [sourceFile], [], 1); + const edge = context.fileSubgraph.edges.find((entry) => entry.from === sourceFile && entry.to === targetFile); + + expect(edge?.typeOnly).toBe(false); + }); + it("should collect symbol neighbors with correct relationships", async () => { const index = await createTestIndex("typescript"); From f84cc627360f5c02cbc837244fac52ac40255bb8 Mon Sep 17 00:00:00 2001 From: Luke Zehrung Date: Wed, 20 May 2026 01:33:52 -0400 Subject: [PATCH 7/7] Include type-only state in streaming dedupe --- src/impact/streaming.ts | 4 +++- tests/impact-streaming.test.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/impact/streaming.ts b/src/impact/streaming.ts index 509276d6..4ef70fc7 100644 --- a/src/impact/streaming.ts +++ b/src/impact/streaming.ts @@ -153,7 +153,7 @@ function buildLightStreamSummaryReport( }; } -function impactItemEmissionKey(item: ImpactItem, partial: boolean): string { +export function impactItemEmissionKey(item: ImpactItem, partial: boolean): string { const symbols = item.symbols.slice().sort().join(","); const reasons = item.reasons.slice().sort().join(","); const refCount = item.refs?.length ?? 0; @@ -166,6 +166,8 @@ function impactItemEmissionKey(item: ImpactItem, partial: boolean): string { item.severity.toFixed(6), String(item.depth ?? 0), String(item.confidence ?? 0), + String(item.typeOnly ?? ""), + String(item.explain?.typeOnly ?? ""), String(refCount), String(hintCount), ].join("|"); diff --git a/tests/impact-streaming.test.ts b/tests/impact-streaming.test.ts index e2c27e46..bb88b7b8 100644 --- a/tests/impact-streaming.test.ts +++ b/tests/impact-streaming.test.ts @@ -9,6 +9,7 @@ import { type ImpactStreamChunk, type ImpactStreamSummaryReport, } from "../src/impact/index.js"; +import { impactItemEmissionKey } from "../src/impact/streaming.js"; import { buildProjectIndex } from "../src/index.js"; async function mkTmpDir(prefix: string): Promise { @@ -39,6 +40,36 @@ async function firstStreamError(stream: AsyncGenerator): Prom } describe("Impact streaming", () => { + it("keeps type-only status in impact item emission dedupe keys", () => { + const baseItem = { + file: "src/consumer.ts", + symbols: ["helper"], + reasons: ["transitive" as const], + severity: 0.3, + depth: 1, + confidence: 0.5, + }; + + const runtimeKey = impactItemEmissionKey( + { + ...baseItem, + typeOnly: false, + explain: { reason: "transitive", depth: 1, typeOnly: false }, + }, + true, + ); + const typeOnlyKey = impactItemEmissionKey( + { + ...baseItem, + typeOnly: true, + explain: { reason: "transitive", depth: 1, typeOnly: true }, + }, + true, + ); + + expect(typeOnlyKey).not.toBe(runtimeKey); + }); + it("matches analyzeImpactFromDiff results", async () => { const root = path.resolve(process.cwd(), "tests", "samples", "typescript"); const index = await buildProjectIndex(root);