feat: CFG visitor rewrite (Phase 3.1 completion) (#392)

* feat: cfg visitor rewrite — unified walk, derived cyclomatic, removed standalone impl

Rewrite CFG construction as a visitor (createCfgVisitor) that plugs into the
walkWithVisitors framework, eliminating the last redundant tree traversal
(Mode B) in engine.js. All 4 analyses now run in a single DFS walk.

- Create src/ast-analysis/visitors/cfg-visitor.js (~570 lines) with full
  control-flow support: if patterns A/B/C, for/while/do-while/infinite loops,
  switch/match, try/catch/finally, break/continue with labels, nested functions
- Derive cyclomatic complexity from CFG (E - N + 2) as single source of truth,
  overriding the independently-computed value from the complexity visitor
- Replace 813-line buildFunctionCFG with 15-line thin visitor wrapper
- Simplify buildCFGData WASM fallback: file-level visitor walk instead of
  per-function findFunctionNode calls
- Remove Mode A/B split from engine.js; simplify WASM pre-parse (CFG no
  longer triggers it)
- cfg.js reduced from 1242 to 518 lines (-724 lines)
- Add 31 new tests: 23 parity (visitor vs original) + 8 cyclomatic formula

Impact: 35 functions changed, 61 affected

* fix: allow nested function CFG and recompute MI on cyclomatic override

Remove skipChildren from cfg-visitor's enterNode so the walker recurses
into nested function definitions, enabling the funcStateStack to work as
designed.  Recompute maintainabilityIndex in engine.js whenever the
CFG-derived cyclomatic overrides the complexity visitor's value, keeping
metrics consistent.

Addresses Greptile review feedback on PR #392.

Impact: 3 functions changed, 1 affected

* fix: match CFG results by node identity and disambiguate line collisions

- buildFunctionCFG: use .find() with node identity instead of cfgResults[0]
  so nested functions don't shadow the target function's CFG
- engine.js: replace line-only Map keying with line → results[] grouping,
  disambiguating by function name when multiple functions share a line

Addresses Greptile re-review feedback on PR #392.

Impact: 2 functions changed, 0 affected

* fix: disambiguate line collisions in buildCFGData WASM fallback

Apply the same array-plus-name-disambiguation pattern from engine.js to
the visitorCfgByLine map in buildCFGData, preventing silent overwrites
when multiple functions share a source line.

Addresses Greptile re-review feedback on PR #392.

Impact: 1 functions changed, 0 affected

Author: carlos-alm

docs/roadmap/ROADMAP.md

@@ -591,14 +591,18 @@ src/
 - ✅ `builder.js` → single `runAnalyses` call replaces 4 sequential blocks + WASM pre-parse
 - ✅ Extracted pure computations to `metrics.js` (Halstead derived math, LOC, MI)
 - ✅ Extracted shared helpers to `visitor-utils.js` (from dataflow.js)
-- 🔲 **CFG visitor rewrite** (see below)
+- ✅ **CFG visitor rewrite** — `createCfgVisitor` in `ast-analysis/visitors/cfg-visitor.js`, integrated into engine.js unified walk, Mode A/B split eliminated
 
 **Remaining: CFG visitor rewrite.** `buildFunctionCFG` (813 lines) uses a statement-level traversal (`getStatements` + `processStatement` with `loopStack`, `labelMap`, `blockIndex`) that is fundamentally incompatible with the node-level DFS used by `walkWithVisitors`. This is why the engine runs CFG as a separate Mode B pass — the only analysis that can't participate in the shared single-DFS walk.
 
 Rewrite the CFG algorithm as a node-level visitor that builds basic blocks and edges incrementally via `enterNode`/`exitNode` hooks, tracking block boundaries at branch/loop/return nodes the same way the complexity visitor tracks nesting. This eliminates the last redundant tree traversal during build and lets CFG share the exact same DFS pass as complexity, dataflow, and AST extraction. The statement-level `getStatements` helper and per-language `CFG_RULES.statementTypes` can be replaced by detecting block-terminating node types in `enterNode`. Also simplifies `engine.js` by removing the Mode A/B split and WASM pre-parse special-casing for CFG.
 
 **Remaining: Derive cyclomatic complexity from CFG.** Once CFG participates in the unified walk, cyclomatic complexity can be derived directly from CFG edge/block counts (`edges - nodes + 2`) rather than independently computed by the complexity visitor. This creates a single source of truth for control flow metrics and eliminates redundant computation. Can also be done as a simpler SQL-only approach against stored `cfg_blocks`/`cfg_edges` tables (see backlog ID 45).
 
+**Follow-up tasks (post CFG visitor rewrite):**
+- ✅ **Derive cyclomatic complexity from CFG** — CFG visitor computes `E - N + 2` per function; engine.js overrides complexity visitor's cyclomatic with CFG-derived value (single source of truth)
+- ✅ **Remove `buildFunctionCFG` implementation** — 813-line standalone implementation replaced with thin visitor wrapper (~15 lines); `buildCFGData` WASM fallback uses file-level visitor walk instead of per-function `findFunctionNode` calls
+
 **Affected files:** `src/complexity.js`, `src/cfg.js`, `src/dataflow.js`, `src/ast.js` → split into `src/ast-analysis/`
 
 ### 3.2 -- Command/Query Separation ★ Critical 🔄

src/ast-analysis/engine.js

@@ -7,14 +7,12 @@
  *   - CFG construction (basic blocks + edges)
  *   - Dataflow analysis (define-use chains, arg flows, mutations)
  *
- * Two modes:
- *   Mode A (node-level visitor): AST + complexity + dataflow — single DFS per file
- *   Mode B (statement-level): CFG keeps its own traversal via buildFunctionCFG
+ * All 4 analyses run as visitors in a single DFS walk via walkWithVisitors.
  *
  * Optimization strategy: for files with WASM trees, run all applicable visitors
- * in a single walkWithVisitors call, then store results in the format that the
- * existing buildXxx functions expect as pre-computed data. This eliminates ~3
- * redundant tree traversals per file.
+ * in a single walkWithVisitors call. Store results in the format that buildXxx
+ * functions already expect as pre-computed data (same fields as native engine
+ * output). This eliminates redundant tree traversals per file.
  */
 
 import path from 'node:path';
@@ -32,6 +30,7 @@ import { buildExtensionSet, buildExtToLangMap } from './shared.js';
 import { walkWithVisitors } from './visitor.js';
 import { functionName as getFuncName } from './visitor-utils.js';
 import { createAstStoreVisitor } from './visitors/ast-store-visitor.js';
+import { createCfgVisitor } from './visitors/cfg-visitor.js';
 import { createComplexityVisitor } from './visitors/complexity-visitor.js';
 import { createDataflowVisitor } from './visitors/dataflow-visitor.js';
 
@@ -74,25 +73,15 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
   const extToLang = buildExtToLangMap();
 
   // ── WASM pre-parse for files that need it ───────────────────────────
-  if (doCfg || doDataflow) {
+  // CFG now runs as a visitor in the unified walk, so only dataflow
+  // triggers WASM pre-parse when no tree exists.
+  if (doDataflow) {
     let needsWasmTrees = false;
     for (const [relPath, symbols] of fileSymbols) {
       if (symbols._tree) continue;
       const ext = path.extname(relPath).toLowerCase();
 
-      if (doCfg && CFG_EXTENSIONS.has(ext)) {
-        const fnDefs = (symbols.definitions || []).filter(
-          (d) => (d.kind === 'function' || d.kind === 'method') && d.line,
-        );
-        if (
-          fnDefs.length > 0 &&
-          !fnDefs.every((d) => d.cfg === null || Array.isArray(d.cfg?.blocks))
-        ) {
-          needsWasmTrees = true;
-          break;
-        }
-      }
-      if (doDataflow && !symbols.dataflow && DATAFLOW_EXTENSIONS.has(ext)) {
+      if (!symbols.dataflow && DATAFLOW_EXTENSIONS.has(ext)) {
         needsWasmTrees = true;
         break;
       }
@@ -185,6 +174,24 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
       }
     }
 
+    // ─ CFG visitor ─
+    const cfgRulesForLang = CFG_RULES.get(langId);
+    let cfgVisitor = null;
+    if (doCfg && cfgRulesForLang && CFG_EXTENSIONS.has(ext)) {
+      // Only use visitor if some functions lack pre-computed CFG
+      const needsWasmCfg = defs.some(
+        (d) =>
+          (d.kind === 'function' || d.kind === 'method') &&
+          d.line &&
+          d.cfg !== null &&
+          !Array.isArray(d.cfg?.blocks),
+      );
+      if (needsWasmCfg) {
+        cfgVisitor = createCfgVisitor(cfgRulesForLang);
+        visitors.push(cfgVisitor);
+      }
+    }
+
     // ─ Dataflow visitor ─
     const dfRules = DATAFLOW_RULES.get(langId);
     let dataflowVisitor = null;
@@ -216,12 +223,21 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
       for (const r of complexityResults) {
         if (r.funcNode) {
           const line = r.funcNode.startPosition.row + 1;
-          resultByLine.set(line, r);
+          if (!resultByLine.has(line)) resultByLine.set(line, []);
+          resultByLine.get(line).push(r);
         }
       }
       for (const def of defs) {
         if ((def.kind === 'function' || def.kind === 'method') && def.line && !def.complexity) {
-          const funcResult = resultByLine.get(def.line);
+          const candidates = resultByLine.get(def.line);
+          const funcResult = !candidates
+            ? undefined
+            : candidates.length === 1
+              ? candidates[0]
+              : (candidates.find((r) => {
+                  const n = r.funcNode.childForFieldName('name');
+                  return n && n.text === def.name;
+                }) ?? candidates[0]);
           if (funcResult) {
             const { metrics } = funcResult;
             const loc = computeLOCMetrics(funcResult.funcNode, langId);
@@ -247,6 +263,54 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
       }
     }
 
+    // ─ Store CFG results on definitions (buildCFGData will find def.cfg and skip its walk) ─
+    if (cfgVisitor) {
+      const cfgResults = results.cfg || [];
+      const cfgByLine = new Map();
+      for (const r of cfgResults) {
+        if (r.funcNode) {
+          const line = r.funcNode.startPosition.row + 1;
+          if (!cfgByLine.has(line)) cfgByLine.set(line, []);
+          cfgByLine.get(line).push(r);
+        }
+      }
+      for (const def of defs) {
+        if (
+          (def.kind === 'function' || def.kind === 'method') &&
+          def.line &&
+          !def.cfg?.blocks?.length
+        ) {
+          const candidates = cfgByLine.get(def.line);
+          const cfgResult = !candidates
+            ? undefined
+            : candidates.length === 1
+              ? candidates[0]
+              : (candidates.find((r) => {
+                  const n = r.funcNode.childForFieldName('name');
+                  return n && n.text === def.name;
+                }) ?? candidates[0]);
+          if (cfgResult) {
+            def.cfg = { blocks: cfgResult.blocks, edges: cfgResult.edges };
+
+            // Override complexity's cyclomatic with CFG-derived value (single source of truth)
+            // and recompute maintainability index to stay consistent
+            if (def.complexity && cfgResult.cyclomatic != null) {
+              def.complexity.cyclomatic = cfgResult.cyclomatic;
+              const { loc, halstead } = def.complexity;
+              const volume = halstead ? halstead.volume : 0;
+              const commentRatio = loc?.loc > 0 ? loc.commentLines / loc.loc : 0;
+              def.complexity.maintainabilityIndex = computeMaintainabilityIndex(
+                volume,
+                cfgResult.cyclomatic,
+                loc?.sloc ?? 0,
+                commentRatio,
+              );
+            }
+          }
+        }
+      }
+    }
+
     // ─ Store dataflow results (buildDataflowEdges will find symbols.dataflow and skip its walk) ─
     if (dataflowVisitor) {
       symbols.dataflow = results.dataflow;