fix: address quality issues in ast-analysis, presentation, extractors, and cli

scripts/token-benchmark.ts

@@ -265,6 +265,20 @@ function round1(n) {
 	return Math.round(n * 10) / 10;
 }
 
+/**
+ * Run `fn` `runs` times (default `PERF_RUNS`), recording the elapsed
+ * milliseconds per run, and return the median duration.
+ */
+async function timeMedian(fn, runs = PERF_RUNS) {
+	const timings = [];
+	for (let i = 0; i < runs; i++) {
+		const start = performance.now();
+		await fn();
+		timings.push(performance.now() - start);
+	}
+	return median(timings);
+}
+
 /**
  * Run build/query/stats benchmarks against the Next.js graph.
  * Reuses the same codegraph APIs as the existing benchmark scripts.
@@ -306,24 +320,18 @@ async function runPerfBenchmarks(nextjsDir) {
 	const buildResults = {};
 	for (const engine of engines) {
 		console.error(`  Full build (${engine})...`);
-		const timings = [];
-		for (let i = 0; i < PERF_RUNS; i++) {
-			if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);
-			const start = performance.now();
-			await buildGraph(nextjsDir, { engine, incremental: false });
-			timings.push(performance.now() - start);
-		}
-		const fullBuildMs = Math.round(median(timings));
+		const fullBuildMs = Math.round(
+			await timeMedian(async () => {
+				if (fs.existsSync(dbPath)) fs.unlinkSync(dbPath);
+				await buildGraph(nextjsDir, { engine, incremental: false });
+			}),
+		);
 
 		// No-op rebuild
 		console.error(`  No-op rebuild (${engine})...`);
-		const noopTimings = [];
-		for (let i = 0; i < PERF_RUNS; i++) {
-			const start = performance.now();
-			await buildGraph(nextjsDir, { engine, incremental: true });
-			noopTimings.push(performance.now() - start);
-		}
-		const noopRebuildMs = Math.round(median(noopTimings));
+		const noopRebuildMs = Math.round(
+			await timeMedian(() => buildGraph(nextjsDir, { engine, incremental: true })),
+		);
 
 		buildResults[engine] = { fullBuildMs, noopRebuildMs };
 		console.error(`    full=${fullBuildMs}ms noop=${noopRebuildMs}ms`);
@@ -370,23 +378,14 @@ async function runPerfBenchmarks(nextjsDir) {
 
 		for (const depth of [1, 3, 5]) {
 			// fnDeps
-			const depsTimings = [];
-			for (let i = 0; i < PERF_RUNS; i++) {
-				const start = performance.now();
-				fnDepsData(hubName, dbPath, { depth, noTests: true });
-				depsTimings.push(performance.now() - start);
-			}
+			queryResults[`fnDeps_depth${depth}Ms`] = round1(
+				await timeMedian(() => fnDepsData(hubName, dbPath, { depth, noTests: true })),
+			);
 
 			// fnImpact
-			const impactTimings = [];
-			for (let i = 0; i < PERF_RUNS; i++) {
-				const start = performance.now();
-				fnImpactData(hubName, dbPath, { depth, noTests: true });
-				impactTimings.push(performance.now() - start);
-			}
-
-			queryResults[`fnDeps_depth${depth}Ms`] = round1(median(depsTimings));
-			queryResults[`fnImpact_depth${depth}Ms`] = round1(median(impactTimings));
+			queryResults[`fnImpact_depth${depth}Ms`] = round1(
+				await timeMedian(() => fnImpactData(hubName, dbPath, { depth, noTests: true })),
+			);
 		}
 
 		console.error(

src/ast-analysis/visitor-utils.ts

@@ -93,6 +93,81 @@ export function extractParams(
   return result;
 }
 
+/**
+ * Resolution result for a single node in the parameter-name worklist: either
+ * a base case with names to record, or intermediate `next` nodes that still
+ * need to be resolved.
+ */
+type ParamNodeResolution = { names?: string[]; next?: TreeSitterNode[] };
+
+/** One entry in the node-type -> handler dispatch table used by `resolveParamNode`. */
+interface ParamNodeHandler {
+  matches(nodeType: string, rules: LanguageRules): boolean;
+  resolve(node: TreeSitterNode, rules: LanguageRules): ParamNodeResolution | null;
+}
+
+function resolveWrapperParam(node: TreeSitterNode): ParamNodeResolution | null {
+  const pattern = node.childForFieldName('pattern') || node.childForFieldName('name');
+  return pattern ? { next: [pattern] } : null;
+}
+
+function resolveDefaultParam(node: TreeSitterNode): ParamNodeResolution | null {
+  const left = node.childForFieldName('left') || node.childForFieldName('name');
+  return left ? { next: [left] } : null;
+}
+
+function resolveRestParam(node: TreeSitterNode, rules: LanguageRules): ParamNodeResolution | null {
+  const nameNode = node.childForFieldName('name');
+  if (nameNode) return { names: [nameNode.text] };
+  for (const child of node.namedChildren) {
+    if (child.type === rules.paramIdentifier) return { names: [child.text] };
+  }
+  return null;
+}
+
+function resolveObjectDestructParam(
+  node: TreeSitterNode,
+  rules: LanguageRules,
+): ParamNodeResolution {
+  return { next: collectObjectDestructChildren(node, rules) };
+}
+
+function resolveArrayDestructParam(node: TreeSitterNode): ParamNodeResolution {
+  return { next: [...node.namedChildren] };
+}
+
+/**
+ * Ordered node-type -> handler dispatch table for `resolveParamNode`. Order
+ * matters: earlier entries take precedence, matching the original
+ * if/else-if cascade exactly.
+ */
+const PARAM_NODE_HANDLERS: ParamNodeHandler[] = [
+  {
+    matches: (t, rules) => t === rules.paramIdentifier,
+    resolve: (node) => ({ names: [node.text] }),
+  },
+  {
+    matches: (t, rules) => rules.paramWrapperTypes.has(t),
+    resolve: resolveWrapperParam,
+  },
+  {
+    matches: (t, rules) => !!rules.defaultParamType && t === rules.defaultParamType,
+    resolve: resolveDefaultParam,
+  },
+  {
+    matches: (t, rules) => !!rules.restParamType && t === rules.restParamType,
+    resolve: resolveRestParam,
+  },
+  {
+    matches: (t, rules) => !!rules.objectDestructType && t === rules.objectDestructType,
+    resolve: resolveObjectDestructParam,
+  },
+  {
+    matches: (t, rules) => !!rules.arrayDestructType && t === rules.arrayDestructType,
+    resolve: resolveArrayDestructParam,
+  },
+];
+
 /**
  * Resolve a single parameter node to either a direct list of names (base case)
  * or a list of child nodes that still need processing. Returns `null` if the
@@ -102,46 +177,16 @@ export function extractParams(
  * `extractParamNames`, breaking the 3-node mutual recursion cycle between
  * `extractParamNames`, `extractObjectDestructNames`, and `extractArrayDestructNames`.
  */
-function resolveParamNode(
-  node: TreeSitterNode,
-  rules: LanguageRules,
-): { names?: string[]; next?: TreeSitterNode[] } | null {
-  const t = node.type;
-
+function resolveParamNode(node: TreeSitterNode, rules: LanguageRules): ParamNodeResolution | null {
   if (rules.extractParamName) {
     const result = rules.extractParamName(node);
     if (result) return { names: result };
   }
 
-  if (t === rules.paramIdentifier) return { names: [node.text] };
-
-  if (rules.paramWrapperTypes.has(t)) {
-    const pattern = node.childForFieldName('pattern') || node.childForFieldName('name');
-    return pattern ? { next: [pattern] } : null;
-  }
-
-  if (rules.defaultParamType && t === rules.defaultParamType) {
-    const left = node.childForFieldName('left') || node.childForFieldName('name');
-    return left ? { next: [left] } : null;
-  }
-
-  if (rules.restParamType && t === rules.restParamType) {
-    const nameNode = node.childForFieldName('name');
-    if (nameNode) return { names: [nameNode.text] };
-    for (const child of node.namedChildren) {
-      if (child.type === rules.paramIdentifier) return { names: [child.text] };
-    }
-    return null;
-  }
-
-  if (rules.objectDestructType && t === rules.objectDestructType) {
-    return { next: collectObjectDestructChildren(node, rules) };
-  }
-
-  if (rules.arrayDestructType && t === rules.arrayDestructType) {
-    return { next: [...node.namedChildren] };
+  const t = node.type;
+  for (const handler of PARAM_NODE_HANDLERS) {
+    if (handler.matches(t, rules)) return handler.resolve(node, rules);
   }
-
   return null;
 }
 
@@ -170,6 +215,41 @@ function collectObjectDestructChildren(
   return next;
 }
 
+/** Is this node a shorthand identifier inside an object destructuring pattern? */
+function isShorthandPropPattern(node: TreeSitterNode, rules: LanguageRules): boolean {
+  return !!rules.shorthandPropPattern && node.type === rules.shorthandPropPattern;
+}
+
+/**
+ * Push nodes onto the worklist stack in reverse order so that popping them
+ * (LIFO) visits them in the same left-to-right order as the original
+ * recursive traversal.
+ */
+function pushParamWorklist(stack: TreeSitterNode[], nodes: TreeSitterNode[]): void {
+  for (let i = nodes.length - 1; i >= 0; i--) {
+    const child = nodes[i];
+    if (child) stack.push(child);
+  }
+}
+
+/** Resolve one worklist entry: record any names, queue any further nodes to visit. */
+function visitParamWorklistNode(
+  current: TreeSitterNode,
+  rules: LanguageRules,
+  names: string[],
+  stack: TreeSitterNode[],
+): void {
+  if (isShorthandPropPattern(current, rules)) {
+    names.push(current.text);
+    return;
+  }
+
+  const resolved = resolveParamNode(current, rules);
+  if (!resolved) return;
+  if (resolved.names) names.push(...resolved.names);
+  if (resolved.next) pushParamWorklist(stack, resolved.next);
+}
+
 /**
  * Extract parameter names from a single parameter node.
  *
@@ -184,24 +264,7 @@ export function extractParamNames(node: TreeSitterNode | null, rules: LanguageRu
 
   while (stack.length > 0) {
     const current = stack.pop();
-    if (!current) continue;
-
-    // Shorthand identifier inside an object destructuring is just the node's text.
-    if (rules.shorthandPropPattern && current.type === rules.shorthandPropPattern) {
-      names.push(current.text);
-      continue;
-    }
-
-    const resolved = resolveParamNode(current, rules);
-    if (!resolved) continue;
-    if (resolved.names) names.push(...resolved.names);
-    if (resolved.next) {
-      // Push in reverse so traversal order matches the previous recursive order.
-      for (let i = resolved.next.length - 1; i >= 0; i--) {
-        const child = resolved.next[i];
-        if (child) stack.push(child);
-      }
-    }
+    if (current) visitParamWorklistNode(current, rules, names, stack);
   }
 
   return names;