feat: unified AST analysis framework (Phase 3.1)#388
Conversation
Implement Phase 3.1 of the architectural refactoring roadmap. Replace 4 independent AST analysis passes (complexity, dataflow, AST-store, CFG) with a shared visitor framework that runs complexity, dataflow, and AST-store visitors in a single DFS walk per file. New framework files: - visitor.js: shared DFS walker with enterNode/exitNode/enterFunction/ exitFunction hooks, per-visitor skipChildren, nesting/scope tracking - engine.js: orchestrates all analyses in one coordinated pass, replaces 4 sequential buildXxx blocks + WASM pre-parse in builder.js - metrics.js: extracted Halstead derived math, LOC, MI from complexity.js - visitor-utils.js: extracted shared helpers from dataflow.js - visitors/complexity-visitor.js: cognitive/cyclomatic/nesting + Halstead - visitors/ast-store-visitor.js: new/throw/await/string/regex extraction - visitors/dataflow-visitor.js: scope stack + define-use chains - tests/unit/visitor.test.js: 9 tests for walker framework Modified modules: - builder.js: 4 sequential buildXxx blocks → single runAnalyses call - complexity.js: delegates walk to complexity visitor - dataflow.js: delegates walk to dataflow visitor - ast.js: delegates walk to ast-store visitor, removed dead code Bug fixes: - Fix Ruby dataflow parity: skip nested return keyword tokens inside return statements (tree-sitter Ruby grammar nests return/return) Hook fixes: - check-dead-exports.sh: exclude public API (index.js) and dynamic import() consumers from dead export detection - check-readme.sh: match ROADMAP.md at any path; allow commit when at least one doc is staged (developer reviewed which docs need updating) Docs: - ROADMAP.md: update 3.1 with completed items and remaining work - BACKLOG.md: add Tier 1h with dynamic import/re-export tracking (ID 71) - CLAUDE.md: add ast-analysis/ to architecture table Impact: 51 functions changed, 47 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
The hook correctly catches a real limitation — codegraph doesn't track dynamic import() consumers as graph edges. The proper fix is to add dynamic import edge tracking (backlog ID 81), not to exclude them from the hook. Reverting the workaround.
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisReviewing PR #388: unified AST analysis framework (Phase 3.1) Review Checklist
Root Cause Analysis ✅Problem Being Solved: This PR addresses the architectural inefficiency described in ROADMAP.md Phase 3.1. The codebase currently has 4 independent AST analysis engines (complexity, CFG, dataflow, AST-store) that each perform their own tree traversal, causing redundant work (~3 tree walks per file during build). Why This Approach: The solution introduces a shared pluggable visitor framework ( Risk Assessment: Low architectural risk. The approach preserves existing APIs and output formats by storing pre-computed results in the same fields that the original Configuration Change Detection ✅No concerning configuration relaxations found:
Test Integrity Verification ✅Strong test coverage for new functionality:
Code Quality Review ✅Architecture & Design ExcellenceStrong separation of concerns:
Bug Fixes & Improvements
Performance Impact
Code Organization
Critical ConcernsNone identified. This is a well-executed architectural improvement that:
Final Recommendation |
Greptile SummaryThis PR implements Phase 3.1 of the architectural roadmap: replacing 4 independent AST analysis passes in Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
B[builder.js\nrunAnalyses call] --> E[engine.js\nrunAnalyses]
E --> WP[WASM pre-parse\nfor CFG + dataflow files]
WP --> UW[Unified pre-walk\nwalkWithVisitors per file]
UW --> AST[ast-store-visitor\nextract new/throw/await/string/regex]
UW --> CX[complexity-visitor\ncognitive + cyclomatic + Halstead]
UW --> DF[dataflow-visitor\ndefine-use chains + arg flows]
AST --> SYM1[symbols.astNodes = results]
CX --> SYM2[def.complexity = metrics]
DF --> SYM3[symbols.dataflow = results]
E --> BA[buildAstNodes\nDB writes only if astNodes present]
E --> BC[buildComplexityMetrics\nDB writes + native fallback]
E --> BF[buildCFGData\nMode B: own statement traversal]
E --> BD[buildDataflowEdges\nDB writes only if dataflow present]
subgraph Visitor Framework
VW[visitor.js\nwalkWithVisitors DFS]
VW -->|enterNode / exitNode| V1[Visitor 1]
VW -->|enterFunction / exitFunction| V2[Visitor 2]
VW -->|per-visitor skipChildren| V3[Visitor N]
end
UW --> VW
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const _wasSkipping = halsteadSkip; | ||
| if (hRules) { |
There was a problem hiding this comment.
Unused variable _wasSkipping.
_wasSkipping is assigned but never read anywhere in the function. It appears to be a leftover from an earlier draft where it may have been used to restore halsteadSkip on exitNode. The exitNode hook already handles the restoration correctly by checking hRules?.skipTypes.has(node.type) directly. This declaration can be removed.
| const _wasSkipping = halsteadSkip; | |
| if (hRules) { | |
| if (hRules) { |
There was a problem hiding this comment.
Fixed — removed the unused _wasSkipping variable in the latest commit.
…ex, remove unused var Impact: 2 functions changed, 23 affected
|
Fixed both |
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
| if (hRules.skipTypes.has(type)) halsteadSkip = true; | ||
| if (!halsteadSkip) { | ||
| if (hRules.compoundOperators.has(type)) { | ||
| operators.set(type, (operators.get(type) || 0) + 1); | ||
| } | ||
| if (node.childCount === 0) { | ||
| if (hRules.operatorLeafTypes.has(type)) { | ||
| operators.set(type, (operators.get(type) || 0) + 1); | ||
| } else if (hRules.operandLeafTypes.has(type)) { | ||
| const text = node.text; | ||
| operands.set(text, (operands.get(text) || 0) + 1); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ── Complexity: track nesting depth ── | ||
| if (nestingLevel > maxNesting) maxNesting = nestingLevel; | ||
|
|
||
| // Handle logical operators in binary expressions | ||
| if (type === cRules.logicalNodeType) { | ||
| const op = node.child(1)?.type; | ||
| if (op && cRules.logicalOperators.has(op)) { | ||
| cyclomatic++; | ||
| const parent = node.parent; | ||
| let sameSequence = false; | ||
| if (parent && parent.type === cRules.logicalNodeType) { | ||
| const parentOp = parent.child(1)?.type; | ||
| if (parentOp === op) sameSequence = true; | ||
| } | ||
| if (!sameSequence) cognitive++; | ||
| // Don't skip children — walker handles recursion | ||
| } | ||
| } | ||
|
|
||
| // Handle optional chaining (cyclomatic only) | ||
| if (type === cRules.optionalChainType) { | ||
| cyclomatic++; | ||
| } | ||
|
|
||
| // Handle branch/control flow nodes (skip keyword leaf tokens) | ||
| if (cRules.branchNodes.has(type) && node.childCount > 0) { | ||
| // Pattern A: else clause wraps if (JS/C#/Rust) | ||
| if (cRules.elseNodeType && type === cRules.elseNodeType) { | ||
| const firstChild = node.namedChild(0); | ||
| if (firstChild && firstChild.type === cRules.ifNodeType) { | ||
| // else-if: the if_statement child handles its own increment | ||
| return; | ||
| } | ||
| cognitive++; | ||
| return; | ||
| } | ||
|
|
||
| // Pattern B: explicit elif node (Python/Ruby/PHP) | ||
| if (cRules.elifNodeType && type === cRules.elifNodeType) { | ||
| cognitive++; | ||
| cyclomatic++; | ||
| return; | ||
| } | ||
|
|
||
| // Detect else-if via Pattern A or C | ||
| let isElseIf = false; | ||
| if (type === cRules.ifNodeType) { | ||
| if (cRules.elseViaAlternative) { | ||
| isElseIf = | ||
| node.parent?.type === cRules.ifNodeType && | ||
| node.parent.childForFieldName('alternative')?.id === node.id; | ||
| } else if (cRules.elseNodeType) { | ||
| isElseIf = node.parent?.type === cRules.elseNodeType; | ||
| } | ||
| } | ||
|
|
||
| if (isElseIf) { | ||
| cognitive++; | ||
| cyclomatic++; | ||
| return; | ||
| } | ||
|
|
||
| // Regular branch node | ||
| cognitive += 1 + nestingLevel; | ||
| cyclomatic++; | ||
|
|
||
| if (cRules.switchLikeNodes?.has(type)) { | ||
| cyclomatic--; | ||
| } | ||
|
|
||
| // Nesting nodes are handled by the walker's nestingNodeTypes option | ||
| // But we still need them to count in complexity — they already do above | ||
| } | ||
|
|
||
| // Pattern C plain else: block that is the alternative of an if_statement (Go/Java) | ||
| if ( | ||
| cRules.elseViaAlternative && | ||
| type !== cRules.ifNodeType && | ||
| node.parent?.type === cRules.ifNodeType && | ||
| node.parent.childForFieldName('alternative')?.id === node.id | ||
| ) { | ||
| cognitive++; | ||
| } | ||
|
|
||
| // Handle case nodes (cyclomatic only, skip keyword leaves) | ||
| if (cRules.caseNodes.has(type) && node.childCount > 0) { | ||
| cyclomatic++; | ||
| } | ||
|
|
||
| // Handle nested function definitions (increase nesting) | ||
| // In file-level mode funcDepth handles this; in function-level mode the | ||
| // nestingNodeTypes option should include function nodes | ||
| }, | ||
|
|
||
| exitNode(node) { | ||
| // Restore halsteadSkip when leaving a skip-type subtree | ||
| if (hRules?.skipTypes.has(node.type)) { | ||
| halsteadSkip = false; |
There was a problem hiding this comment.
halsteadSkip is reset prematurely when skip-type nodes nest.
When a skip-type node A contains another skip-type node B:
enterNode(A)→halsteadSkip = trueenterNode(B)→halsteadSkip = true(already set)exitNode(B)→halsteadSkip = false← bug: still inside A's subtree- Remaining children of A process with
halsteadSkip = false, incorrectly counting their operators/operands
The original computeAllMetrics avoided this by propagating skipH as a recursive call parameter — once set, it remained true for the entire subtree. With TypeScript skipTypes = {type_annotation, type_parameters, return_type, implements_clause}, a construct like function foo(): Record<string, T> could have a return_type containing a type_annotation, triggering this bug.
Replace the boolean flag with a depth counter to mirror the original behavior:
| if (hRules.skipTypes.has(type)) halsteadSkip = true; | |
| if (!halsteadSkip) { | |
| if (hRules.compoundOperators.has(type)) { | |
| operators.set(type, (operators.get(type) || 0) + 1); | |
| } | |
| if (node.childCount === 0) { | |
| if (hRules.operatorLeafTypes.has(type)) { | |
| operators.set(type, (operators.get(type) || 0) + 1); | |
| } else if (hRules.operandLeafTypes.has(type)) { | |
| const text = node.text; | |
| operands.set(text, (operands.get(text) || 0) + 1); | |
| } | |
| } | |
| } | |
| } | |
| // ── Complexity: track nesting depth ── | |
| if (nestingLevel > maxNesting) maxNesting = nestingLevel; | |
| // Handle logical operators in binary expressions | |
| if (type === cRules.logicalNodeType) { | |
| const op = node.child(1)?.type; | |
| if (op && cRules.logicalOperators.has(op)) { | |
| cyclomatic++; | |
| const parent = node.parent; | |
| let sameSequence = false; | |
| if (parent && parent.type === cRules.logicalNodeType) { | |
| const parentOp = parent.child(1)?.type; | |
| if (parentOp === op) sameSequence = true; | |
| } | |
| if (!sameSequence) cognitive++; | |
| // Don't skip children — walker handles recursion | |
| } | |
| } | |
| // Handle optional chaining (cyclomatic only) | |
| if (type === cRules.optionalChainType) { | |
| cyclomatic++; | |
| } | |
| // Handle branch/control flow nodes (skip keyword leaf tokens) | |
| if (cRules.branchNodes.has(type) && node.childCount > 0) { | |
| // Pattern A: else clause wraps if (JS/C#/Rust) | |
| if (cRules.elseNodeType && type === cRules.elseNodeType) { | |
| const firstChild = node.namedChild(0); | |
| if (firstChild && firstChild.type === cRules.ifNodeType) { | |
| // else-if: the if_statement child handles its own increment | |
| return; | |
| } | |
| cognitive++; | |
| return; | |
| } | |
| // Pattern B: explicit elif node (Python/Ruby/PHP) | |
| if (cRules.elifNodeType && type === cRules.elifNodeType) { | |
| cognitive++; | |
| cyclomatic++; | |
| return; | |
| } | |
| // Detect else-if via Pattern A or C | |
| let isElseIf = false; | |
| if (type === cRules.ifNodeType) { | |
| if (cRules.elseViaAlternative) { | |
| isElseIf = | |
| node.parent?.type === cRules.ifNodeType && | |
| node.parent.childForFieldName('alternative')?.id === node.id; | |
| } else if (cRules.elseNodeType) { | |
| isElseIf = node.parent?.type === cRules.elseNodeType; | |
| } | |
| } | |
| if (isElseIf) { | |
| cognitive++; | |
| cyclomatic++; | |
| return; | |
| } | |
| // Regular branch node | |
| cognitive += 1 + nestingLevel; | |
| cyclomatic++; | |
| if (cRules.switchLikeNodes?.has(type)) { | |
| cyclomatic--; | |
| } | |
| // Nesting nodes are handled by the walker's nestingNodeTypes option | |
| // But we still need them to count in complexity — they already do above | |
| } | |
| // Pattern C plain else: block that is the alternative of an if_statement (Go/Java) | |
| if ( | |
| cRules.elseViaAlternative && | |
| type !== cRules.ifNodeType && | |
| node.parent?.type === cRules.ifNodeType && | |
| node.parent.childForFieldName('alternative')?.id === node.id | |
| ) { | |
| cognitive++; | |
| } | |
| // Handle case nodes (cyclomatic only, skip keyword leaves) | |
| if (cRules.caseNodes.has(type) && node.childCount > 0) { | |
| cyclomatic++; | |
| } | |
| // Handle nested function definitions (increase nesting) | |
| // In file-level mode funcDepth handles this; in function-level mode the | |
| // nestingNodeTypes option should include function nodes | |
| }, | |
| exitNode(node) { | |
| // Restore halsteadSkip when leaving a skip-type subtree | |
| if (hRules?.skipTypes.has(node.type)) { | |
| halsteadSkip = false; | |
| export function createComplexityVisitor(cRules, hRules, options = {}) { | |
| const { fileLevelWalk = false } = options; | |
| // Per-function accumulators | |
| let cognitive = 0; | |
| let cyclomatic = 1; | |
| let maxNesting = 0; | |
| let operators = hRules ? new Map() : null; | |
| let operands = hRules ? new Map() : null; | |
| let halsteadSkipDepth = 0; | |
| // ... (rest of function unchanged) | |
| return { | |
| // ... (name, functionNodeTypes, enterFunction, exitFunction, enterNode hooks unchanged) | |
| enterNode(node, context) { | |
| // In file-level mode, skip nodes outside any function | |
| if (fileLevelWalk && !activeFuncNode) return; | |
| const type = node.type; | |
| const nestingLevel = fileLevelWalk ? context.nestingLevel + funcDepth : context.nestingLevel; | |
| // ── Halstead classification ── | |
| if (hRules) { | |
| if (hRules.skipTypes.has(type)) halsteadSkipDepth++; | |
| const halsteadSkip = halsteadSkipDepth > 0; | |
| if (!halsteadSkip) { | |
| if (hRules.compoundOperators.has(type)) { | |
| operators.set(type, (operators.get(type) || 0) + 1); | |
| } | |
| if (node.childCount === 0) { | |
| if (hRules.operatorLeafTypes.has(type)) { | |
| operators.set(type, (operators.get(type) || 0) + 1); | |
| } else if (hRules.operandLeafTypes.has(type)) { | |
| const text = node.text; | |
| operands.set(text, (operands.get(text) || 0) + 1); | |
| } | |
| } | |
| } | |
| } | |
| // ── Complexity: track nesting depth ── | |
| if (nestingLevel > maxNesting) maxNesting = nestingLevel; | |
| // ... (rest of enterNode logic unchanged) | |
| }, | |
| exitNode(node) { | |
| // Decrement skip depth when leaving a skip-type node | |
| if (hRules?.skipTypes.has(node.type)) { | |
| halsteadSkipDepth--; | |
| } | |
| }, | |
| // ... (finish hook unchanged) | |
| }; | |
| } |
There was a problem hiding this comment.
Fixed in c196c8e — replaced the halsteadSkip boolean with a halsteadSkipDepth counter. enterNode increments on skip-type entry, exitNode decrements on leave, and Halstead counting is gated on halsteadSkipDepth === 0. The reset() function zeroes the counter. This matches the original recursive propagation behavior and correctly handles nested skip-type nodes like return_type containing type_annotation.
| if (doAst) { | ||
| const t0 = performance.now(); | ||
| try { | ||
| const { buildAstNodes } = await import('../ast.js'); | ||
| await buildAstNodes(db, fileSymbols, rootDir, _engineOpts); | ||
| } catch { | ||
| // Non-fatal | ||
| } | ||
| timing.astMs = performance.now() - t0; | ||
| } | ||
|
|
||
| if (doComplexity) { | ||
| const t0 = performance.now(); | ||
| try { | ||
| const { buildComplexityMetrics } = await import('../complexity.js'); | ||
| await buildComplexityMetrics(db, fileSymbols, rootDir, _engineOpts); | ||
| } catch { | ||
| // Non-fatal | ||
| } | ||
| timing.complexityMs = performance.now() - t0; | ||
| } | ||
|
|
||
| if (doCfg) { | ||
| const t0 = performance.now(); | ||
| try { | ||
| const { buildCFGData } = await import('../cfg.js'); | ||
| await buildCFGData(db, fileSymbols, rootDir, _engineOpts); | ||
| } catch { | ||
| // Non-fatal | ||
| } | ||
| timing.cfgMs = performance.now() - t0; | ||
| } | ||
|
|
||
| if (doDataflow) { | ||
| const t0 = performance.now(); | ||
| try { | ||
| const { buildDataflowEdges } = await import('../dataflow.js'); | ||
| await buildDataflowEdges(db, fileSymbols, rootDir, _engineOpts); | ||
| } catch { | ||
| // Non-fatal | ||
| } | ||
| timing.dataflowMs = performance.now() - t0; | ||
| } |
There was a problem hiding this comment.
All four analysis catch blocks are silent, swallowing errors completely without logging.
If any of buildAstNodes, buildComplexityMetrics, buildCFGData, or buildDataflowEdges fails — due to data corruption, an import error, or a logic regression — the graph will be built with silently missing metrics and no diagnostic trail. The previous builder.js code logged each failure via debug():
} catch (err) {
debug(`AST node extraction failed: ${err.message}`);
}Restore debug logging to each catch block so that failures are discoverable during --verbose builds:
| if (doAst) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildAstNodes } = await import('../ast.js'); | |
| await buildAstNodes(db, fileSymbols, rootDir, _engineOpts); | |
| } catch { | |
| // Non-fatal | |
| } | |
| timing.astMs = performance.now() - t0; | |
| } | |
| if (doComplexity) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildComplexityMetrics } = await import('../complexity.js'); | |
| await buildComplexityMetrics(db, fileSymbols, rootDir, _engineOpts); | |
| } catch { | |
| // Non-fatal | |
| } | |
| timing.complexityMs = performance.now() - t0; | |
| } | |
| if (doCfg) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildCFGData } = await import('../cfg.js'); | |
| await buildCFGData(db, fileSymbols, rootDir, _engineOpts); | |
| } catch { | |
| // Non-fatal | |
| } | |
| timing.cfgMs = performance.now() - t0; | |
| } | |
| if (doDataflow) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildDataflowEdges } = await import('../dataflow.js'); | |
| await buildDataflowEdges(db, fileSymbols, rootDir, _engineOpts); | |
| } catch { | |
| // Non-fatal | |
| } | |
| timing.dataflowMs = performance.now() - t0; | |
| } | |
| if (doAst) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildAstNodes } = await import('../ast.js'); | |
| await buildAstNodes(db, fileSymbols, rootDir, _engineOpts); | |
| } catch (err) { | |
| console.error(`[engine] buildAstNodes failed: ${err.message}`); | |
| } | |
| timing.astMs = performance.now() - t0; | |
| } | |
| if (doComplexity) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildComplexityMetrics } = await import('../complexity.js'); | |
| await buildComplexityMetrics(db, fileSymbols, rootDir, _engineOpts); | |
| } catch (err) { | |
| console.error(`[engine] buildComplexityMetrics failed: ${err.message}`); | |
| } | |
| timing.complexityMs = performance.now() - t0; | |
| } | |
| if (doCfg) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildCFGData } = await import('../cfg.js'); | |
| await buildCFGData(db, fileSymbols, rootDir, _engineOpts); | |
| } catch (err) { | |
| console.error(`[engine] buildCFGData failed: ${err.message}`); | |
| } | |
| timing.cfgMs = performance.now() - t0; | |
| } | |
| if (doDataflow) { | |
| const t0 = performance.now(); | |
| try { | |
| const { buildDataflowEdges } = await import('../dataflow.js'); | |
| await buildDataflowEdges(db, fileSymbols, rootDir, _engineOpts); | |
| } catch (err) { | |
| console.error(`[engine] buildDataflowEdges failed: ${err.message}`); | |
| } | |
| timing.dataflowMs = performance.now() - t0; | |
| } |
There was a problem hiding this comment.
Fixed in c196c8e — all 5 silent catch blocks now log via debug() from logger.js, matching the pattern used throughout the codebase. Failures will be visible with --verbose.
| import path from 'node:path'; | ||
| import { computeLOCMetrics, computeMaintainabilityIndex } from './metrics.js'; |
There was a problem hiding this comment.
performance is used as an implicit global at lines 114 and 250, but never imported. While performance is a global in Node.js 16+, the rest of the codebase explicitly imports it from 'node:perf_hooks' for clarity and forward compatibility (e.g., builder.js). Add the explicit import:
| import path from 'node:path'; | |
| import { computeLOCMetrics, computeMaintainabilityIndex } from './metrics.js'; | |
| import path from 'node:path'; | |
| import { performance } from 'node:perf_hooks'; | |
| import { computeLOCMetrics, computeMaintainabilityIndex } from './metrics.js'; |
There was a problem hiding this comment.
Fixed in c196c8e — added \ to match the convention in \ and the rest of the codebase.
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
…ing, perf import - Replace halsteadSkip boolean with halsteadSkipDepth counter to handle nested skip-type nodes correctly (e.g. return_type containing type_annotation in TypeScript) - Add debug() logging to all silent catch blocks in engine.js so failures are discoverable with --verbose - Explicitly import performance from node:perf_hooks for clarity Impact: 5 functions changed, 24 affected
…ysis-framework Impact: 6 functions changed, 6 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
src/ast-analysis/engine.js
Outdated
|
|
||
| // Merge nesting nodes for complexity tracking | ||
| for (const t of cRules.nestingNodes) walkerOpts.nestingNodeTypes.add(t); | ||
| for (const t of cRules.functionNodes) walkerOpts.nestingNodeTypes.add(t); |
There was a problem hiding this comment.
Function nodes in nestingNodeTypes inflates cognitive complexity
cRules.functionNodes are added to the walker's nestingNodeTypes (line 167). This means the DFS walker increments context.nestingLevel whenever it enters a function node. As a result, every child node inside any function body starts with context.nestingLevel = 1, not 0.
The complexity visitor then computes effective nesting as context.nestingLevel + funcDepth (line 109 in complexity-visitor.js). For a branch node inside a top-level function:
context.nestingLevel = 1(incremented by the function node being innestingNodeTypes)funcDepth = 0cognitive += 1 + 1 = 2
The original computeAllMetrics passed nesting = 0 for top-level functions, so the same branch correctly contributed 1 + 0 = 1. This is a systematic +1 offset applied to every nestingLevel value used in cognitive += 1 + nestingLevel.
funcDepth is already responsible for tracking function-level nesting (it starts at 0 for the active function and increments for nested ones), so adding functionNodes to nestingNodeTypes double-counts nested function boundaries. This only affects the WASM complexity path (when the native engine hasn't pre-populated def.complexity), which means Ruby, Python, and other languages on non-native builds would silently produce inflated cognitive complexity scores.
// Remove this line — function-level nesting is already tracked via funcDepth in the complexity visitor
for (const t of cRules.functionNodes) walkerOpts.nestingNodeTypes.add(t);There was a problem hiding this comment.
Fixed in a47eb47 — removed the \ line. Function-level nesting is already tracked via \ in the complexity visitor, so adding function nodes to the walker's \ was double-counting and inflating cognitive complexity by +1 inside every function body.
src/ast-analysis/engine.js
Outdated
| const complexityResults = results.complexity || []; | ||
| // Match results back to definitions by function start line | ||
| const resultByLine = new Map(); | ||
| for (const r of complexityResults) { | ||
| if (r.funcNode) { | ||
| const line = r.funcNode.startPosition.row + 1; | ||
| resultByLine.set(line, r.metrics); | ||
| } | ||
| } | ||
| for (const def of defs) { | ||
| if ((def.kind === 'function' || def.kind === 'method') && def.line && !def.complexity) { | ||
| const metrics = resultByLine.get(def.line); | ||
| if (metrics) { | ||
| // Compute LOC + MI from the actual function node text | ||
| const funcResult = complexityResults.find( | ||
| (r) => r.funcNode && r.funcNode.startPosition.row + 1 === def.line, | ||
| ); |
There was a problem hiding this comment.
Redundant O(n²) complexityResults.find() inside per-def loop
resultByLine is built right above (lines 208–213) specifically to provide O(1) lookup of metrics by line number. However, to retrieve the corresponding funcNode for LOC re-computation, complexityResults.find() is called again inside the same per-def loop (line 220). This is an O(n²) scan for files with many function definitions.
The fix is to store both metrics and funcNode in the map during construction:
const resultByLine = new Map();
for (const r of complexityResults) {
if (r.funcNode) {
const line = r.funcNode.startPosition.row + 1;
resultByLine.set(line, r); // store the full result, not just r.metrics
}
}
for (const def of defs) {
if (...) {
const funcResult = resultByLine.get(def.line); // O(1), contains both metrics and funcNode
if (funcResult) {
const { metrics, funcNode } = funcResult;
const loc = computeLOCMetrics(funcNode, langId);
...
}
}
}There was a problem hiding this comment.
Fixed in a47eb47 — resultByLine now stores the full result object ({ metrics, funcNode, funcName }) instead of just r.metrics, so the per-def loop uses resultByLine.get(def.line) for O(1) access to both metrics and funcNode. The redundant complexityResults.find() is eliminated.
…lookup Address two Greptile review comments on PR #388: 1. Function nodes in nestingNodeTypes inflates cognitive complexity by +1 for every function body — funcDepth already tracks function-level nesting, so adding functionNodes to the walker's nestingNodeTypes double-counts. 2. Replace O(n²) complexityResults.find() with O(1) resultByLine map lookup by storing the full result object (metrics + funcNode) instead of just metrics. Impact: 1 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review Checklist
Root Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
src/complexity.js
Outdated
| const nestingNodes = new Set(cRules.nestingNodes); | ||
| // Add function nodes as nesting nodes (nested functions increase nesting) | ||
| for (const t of cRules.functionNodes) nestingNodes.add(t); |
There was a problem hiding this comment.
computeAllMetrics inflates cognitive complexity by +1 via root function nesting
Adding cRules.functionNodes to nestingNodeTypes causes the root function node itself to increment context.nestingLevel from 0 → 1 before its body is walked. Every branch inside the function therefore sees nestingLevel = 1 instead of 0, producing cognitive += 1 + 1 = 2 where the original algorithm computed 1 + 0 = 1.
The file-level walk in engine.js correctly avoids this by not adding function nodes to nestingNodeTypes (funcDepth already tracks function-level nesting), but the function-level walk here has no equivalent guard. This affects fallback complexity computation and direct API callers of computeAllMetrics that don't use the engine's pre-computed metrics.
The fix is to not add function nodes to nestingNodeTypes for function-level mode:
| const nestingNodes = new Set(cRules.nestingNodes); | |
| // Add function nodes as nesting nodes (nested functions increase nesting) | |
| for (const t of cRules.functionNodes) nestingNodes.add(t); | |
| const nestingNodes = new Set(cRules.nestingNodes); | |
| // NOTE: do NOT add functionNodes here for function-level mode. | |
| // The root is the function under analysis (nesting starts at 0). |
There was a problem hiding this comment.
Fixed in 77b4516 — removed the for (const t of cRules.functionNodes) nestingNodes.add(t) line in computeAllMetrics. Same root cause as the engine.js fix (a47eb47): the walker increments context.nestingLevel for function nodes in nestingNodeTypes, but in function-level mode the root function node itself gets counted, inflating all cognitive scores by +1.
| function collectResult(funcNode) { | ||
| const halstead = | ||
| hRules && operators && operands ? computeHalsteadDerived(operators, operands) : null; | ||
| const loc = computeLOCMetrics(funcNode, null); |
There was a problem hiding this comment.
collectResult computes LOC and MI with no language context
computeLOCMetrics(funcNode, null) falls back to C-style comment prefixes (//, /*, *, */) for all languages. For Python, Ruby, and PHP the comment-line count will be wrong, which makes the derived mi value in the returned metrics object inaccurate.
Engine.js correctly recomputes both loc and mi with the proper langId after finish() returns (line 224), so the stored database value is correct. However, any direct consumer of walkWithVisitors with the complexity visitor in function-level mode that uses metrics.loc or metrics.mi from the raw visitor result would receive wrong values for non-C-style languages.
Consider threading langId through the visitor initialization so collectResult can pass it to computeLOCMetrics:
| const loc = computeLOCMetrics(funcNode, null); | |
| const loc = computeLOCMetrics(funcNode, langId); |
There was a problem hiding this comment.
Fixed in 77b4516 — createComplexityVisitor now accepts langId via its options parameter and forwards it to computeLOCMetrics(funcNode, langId). Both call sites (engine.js and complexity.js) now pass langId, so Python (#-style), Ruby (#-style), PHP (// and #), etc. get correct comment-line counts and derived MI values.
…langId to LOC Address two Greptile review comments: 1. computeAllMetrics added functionNodes to nestingNodeTypes, inflating cognitive complexity by +1 for every function body (same bug as the engine.js fix in a47eb47, but in the per-function code path). 2. collectResult in complexity-visitor passed null as langId to computeLOCMetrics, causing C-style comment detection for all languages. Now accepts langId via options and forwards it, so Python/Ruby/PHP get correct comment-line counts and MI values. Impact: 4 functions changed, 5 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
| { | ||
| const { runAnalyses } = await import('./ast-analysis/engine.js'); | ||
| const analysisTiming = await runAnalyses(db, astComplexitySymbols, rootDir, opts, engineOpts); | ||
| _t.astMs = analysisTiming.astMs; | ||
| _t.complexityMs = analysisTiming.complexityMs; | ||
| _t.cfgMs = analysisTiming.cfgMs; | ||
| _t.dataflowMs = analysisTiming.dataflowMs; | ||
| } |
There was a problem hiding this comment.
runAnalyses call is unguarded — a walk-phase throw crashes the entire build
The four previous buildXxx blocks each had an independent try/catch, so a failure in one analysis (e.g. a null-ref from a malformed WASM tree in the visitor loop) would log a debug message and let the build continue with partial data. The new single call has no guard at all.
Inside runAnalyses, the delegate buildXxx calls are all individually wrapped, but the unified pre-walk loop is not:
// engine.js ~line 199
const results = walkWithVisitors(symbols._tree.rootNode, visitors, langId, walkerOpts);If walkWithVisitors throws for any file (e.g. symbols._tree.rootNode is unexpectedly null, or a visitor throws due to an unexpected node shape), the error propagates through runAnalyses and surfaces here as an unhandled rejection, halting the entire build. The previous code would have silently continued.
Wrapping the call (or the walk loop inside runAnalyses) in a try/catch with a debug() log would restore the original resilience:
{
const { runAnalyses } = await import('./ast-analysis/engine.js');
try {
const analysisTiming = await runAnalyses(db, astComplexitySymbols, rootDir, opts, engineOpts);
_t.astMs = analysisTiming.astMs;
_t.complexityMs = analysisTiming.complexityMs;
_t.cfgMs = analysisTiming.cfgMs;
_t.dataflowMs = analysisTiming.dataflowMs;
} catch (err) {
debug(`Unified analysis engine failed: ${err.message}`);
}
}| // Handle nested function definitions (increase nesting) | ||
| // In file-level mode funcDepth handles this; in function-level mode the | ||
| // nestingNodeTypes option should include function nodes |
There was a problem hiding this comment.
Function-level mode underestimates cognitive complexity for nested functions
The comment says nestingNodeTypes should include function nodes for function-level mode — but the caller (computeAllMetrics in complexity.js) explicitly does NOT add them to nestingNodeTypes, and enterFunction is a no-op in this mode, so funcDepth stays 0 throughout. The combined effect: entering a nested function never increments nesting.
Original computeAllMetrics passed nestingLevel + 1 to children of nested functions, so a branch inside a nested callback would produce cognitive += 1 + 1 = 2. The new code produces cognitive += 1 + 0 = 1.
A concrete example:
function outer() {
[1, 2].forEach(function inner() {
if (condition) { /* ... */ } // should be cognitive += 2, gets 1
});
}This path is used as a fallback (when def.complexity is not pre-computed by the engine's file-level walk), so the primary build path via runAnalyses is unaffected. However, the complexity CLI command and any direct API caller of computeAllMetrics will receive incorrect (lower) cognitive scores for functions containing nested function expressions or callbacks.
The comment should be updated to accurately describe the current limitation and tracked as a known divergence from the original algorithm, or funcDepth should be incremented/decremented in enterFunction/exitFunction even in function-level mode to restore parity.
There was a problem hiding this comment.
Fixed in a84b7e4 — enterFunction/exitFunction now increment/decrement funcDepth in function-level mode too. Previously they were no-ops when fileLevelWalk was false, so nested functions (callbacks, closures) contributed zero nesting, producing cognitive += 1+0 instead of 1+1 for branches inside them.
src/ast-analysis/engine.js
Outdated
| const t0 = performance.now(); | ||
| try { | ||
| const { buildAstNodes } = await import('../ast.js'); | ||
| await buildAstNodes(db, fileSymbols, rootDir, _engineOpts); |
There was a problem hiding this comment.
Misleading _engineOpts parameter name
The leading underscore convention in this codebase signals an intentionally unused parameter (as in _rootDir, _engineOpts elsewhere). However, _engineOpts is actively forwarded to all four buildXxx delegate calls (lines 266, 277, 288, 299). Renaming it to engineOpts avoids the false signal and matches its usage:
| await buildAstNodes(db, fileSymbols, rootDir, _engineOpts); | |
| export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) { |
There was a problem hiding this comment.
Fixed in a84b7e4 — renamed _engineOpts to engineOpts throughout engine.js since it is actively forwarded to all four buildXxx delegate calls.
…ineOpts Address three Greptile review comments: 1. Wrap runAnalyses call in builder.js with try/catch + debug() so a walk-phase throw doesn't crash the entire build — matches the resilience of the previous individual buildXxx try/catch blocks. 2. In function-level mode, enterFunction/exitFunction were no-ops, so funcDepth stayed 0 for nested functions (callbacks, closures). Now increments/decrements funcDepth in function-level mode too, restoring parity with the original computeAllMetrics algorithm. 3. Rename _engineOpts to engineOpts in engine.js since it's actively forwarded to all four buildXxx delegate calls — the underscore prefix falsely signaled an unused parameter. Impact: 5 functions changed, 4 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
.claude/hooks/check-dead-exports.sh
Outdated
| // Extract imported names from destructuring: const { X } = await import(...) | ||
| const line = src.substring(Math.max(0, src.lastIndexOf('\n', m.index) + 1), src.indexOf('\n', m.index + m[0].length)); | ||
| const destructure = line.match(/\{\s*([^}]+)\s*\}/); | ||
| if (destructure) { | ||
| for (const part of destructure[1].split(',')) { | ||
| const name = part.trim().split(/\s+as\s+/).pop().trim(); | ||
| if (name && /^\w+$/.test(name)) publicAPI.add(name); | ||
| } | ||
| } |
There was a problem hiding this comment.
Single-line-only destructuring extraction misses multi-line patterns
The line variable captures only the source line that contains import(...). When a dynamic import is consumed with a multi-line destructuring pattern, the opening { and the imported names are on different lines from the import() call:
// This pattern is NOT detected:
const {
runAnalyses,
buildExtensionSet,
} = await import('./ast-analysis/engine.js');In that case the "line" captured is ) = await import('./ast-analysis/engine.js'); — no {…} on that line — so the regex at line 99 produces no match and none of the names are added to publicAPI. As a result the hook may continue to flag runAnalyses and buildExtensionSet as dead exports even after this fix, causing spurious pre-commit failures.
A more robust approach is to search the surrounding window (e.g. a few lines before the import() match) for the opening brace, or to scan the full source once with a multi-line regex that captures const\s*\{([^}]+)\}\s*=\s*(?:await\s+)?import(:
for (const m of src.matchAll(/const\s*\{([^}]+)\}\s*=\s*(?:await\s+)?import\s*\(['"]/gs)) {
for (const part of m[1].split(',')) {
const name = part.trim().split(/\s+as\s+/).pop().trim().split('\n').pop().trim();
if (name && /^\w+$/.test(name)) publicAPI.add(name);
}
}(The s flag makes . match newlines, covering multi-line destructuring blocks.)
There was a problem hiding this comment.
Fixed in 6d0f34e — replaced the single-line regex with a multi-line-safe pattern using the s flag: const\s*\{([^}]+)\}\s*=\s*(?:await\s+)?import\s*\(['"/gs. This correctly handles multi-line destructuring like const {\n runAnalyses,\n} = await import(...). Also added a second pattern for single-binding default imports (const X = await import(...)).
| // Track which nodes we've already processed to avoid double-handling | ||
| // when the unified walker visits all children (including unnamed ones) | ||
| const processed = new Set(); |
There was a problem hiding this comment.
processed Set is redundant — each node is visited exactly once
walkWithVisitors calls enterNode for every node in the tree exactly once via the recursive walk function, which descends via node.child(i) without revisiting nodes. A tree-sitter AST is a proper tree (no shared subtrees, no cycles), so node.id is unique per node and can only be encountered at one point during the walk.
Because of this, processed.has(node.id) is always false at the top of enterNode, the early-return guard never fires, and processed.add(node.id) is a no-op from a correctness standpoint. The Set accrues one entry per matched node throughout the entire file traversal, adding unnecessary memory pressure on large files.
The comment explaining it — "avoid double-handling when the unified walker visits all children (including unnamed ones)" — reflects a concern that is already non-existent: unnamed nodes do get visited, but each one is visited exactly once, not multiple times.
The processed Set and all its .has/.add calls can be removed without changing behavior.
There was a problem hiding this comment.
Fixed in 6d0f34e — removed the processed Set and all its .has/.add calls. walkWithVisitors visits each node exactly once via DFS on a proper tree (no shared subtrees, no cycles), so the deduplication guard never fires.
…in hook
Address two Greptile review comments:
1. Remove the `processed` Set from dataflow-visitor.js — walkWithVisitors
visits each node exactly once via DFS on a proper tree, so the
deduplication guard never fires and just wastes memory.
2. Replace single-line destructuring regex in check-dead-exports.sh with
a multi-line-safe pattern (using the `s` flag) so multi-line
`const { ... } = await import(...)` patterns are correctly detected
and their names added to publicAPI.
Impact: 2 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job PR Review Analysis ✅Completed comprehensive review of PR #388: unified AST analysis framework (Phase 3.1) Review ResultsRoot Cause AnalysisProblem Being Solved: Performance optimization - eliminating redundant AST traversals during graph building. The existing codebase made 4+ separate tree walks per file (complexity, dataflow, AST-store, pre-parse), causing unnecessary computational overhead. Why This Approach: The unified visitor framework consolidates multiple analysis passes into a single DFS traversal per file, reducing tree-sitter overhead while maintaining identical output through a pluggable visitor pattern. This is a legitimate architectural improvement, not a workaround. Risk Assessment: Medium-Low. Well-structured refactor with appropriate fallbacks and testing coverage. Configuration Analysis✅ No concerning configuration changes detected
Test Integrity Assessment✅ Test integrity maintained
Code Quality ReviewArchitectural Strengths:
Specific Review Points:
Potential Concerns:
Critical ConcernsNone identified. This is a legitimate performance optimization with proper testing and fallback mechanisms. Final Recommendation
Approval Conditions:
|
1 participant
Summary
walkWithVisitors) that runs complexity, dataflow, and AST-store visitors in a single DFS walk per filebuilder.jscollapsed from 4 sequentialbuildXxxblocks + WASM pre-parse into a singlerunAnalysescall via the newengine.jsorchestratorreturnkeyword token insidereturnstatement node (both typereturn), causing duplicate return entries — now skips keyword tokens via parent checkcheck-dead-exports.shhook: exclude public API exports (re-exported fromindex.js) and dynamicimport()consumers from dead export detectioncheck-readme.shhook: matchROADMAP.mdat any path (was only matching repo root); allow commit when at least one doc is stagedimport()and re-exports as graph edgesNew files (8)
src/ast-analysis/visitor.jsenterNode/exitNode/enterFunction/exitFunctionhooks, per-visitorskipChildren, nesting/scope trackingsrc/ast-analysis/engine.jssrc/ast-analysis/metrics.jssrc/ast-analysis/visitor-utils.jssrc/ast-analysis/visitors/complexity-visitor.jssrc/ast-analysis/visitors/ast-store-visitor.jssrc/ast-analysis/visitors/dataflow-visitor.jstests/unit/visitor.test.jsNet effect
Test plan
node src/cli.js build . --no-incremental— full rebuild succeeds (3971 nodes, 8575 edges)node src/cli.js complexity -T— correct outputnode src/cli.js cfg buildGraph -T— correct outputnode src/cli.js dataflow buildGraph -T— correct outputnode src/cli.js ast "build*" -T— correct output