From fbfd3303b2df3aeaa23f3b5f141b8500aae83147 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 22:18:58 -0600 Subject: [PATCH 1/7] fix: add debug logging to empty catch blocks across infrastructure and domain layers Impact: 8 functions changed, 33 affected --- src/domain/graph/resolve.ts | 21 +++++++++++++++------ src/domain/parser.ts | 15 ++++++++++----- src/infrastructure/config.ts | 18 ++++++++++-------- src/infrastructure/native.ts | 10 ++++++++-- 4 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/domain/graph/resolve.ts b/src/domain/graph/resolve.ts index 06c62141..8588c7c9 100644 --- a/src/domain/graph/resolve.ts +++ b/src/domain/graph/resolve.ts @@ -1,5 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; +import { debug } from '../../infrastructure/logger.js'; import { loadNative } from '../../infrastructure/native.js'; import { normalizePath } from '../../shared/constants.js'; import type { BareSpecifier, BatchResolvedMap, ImportBatchItem, PathAliases } from '../../types.js'; @@ -64,7 +65,10 @@ function getPackageExports(packageDir: string): any { const exports = pkg.exports ?? null; _exportsCache.set(packageDir, exports); return exports; - } catch { + } catch (e) { + debug( + `readPackageExports: failed to read package.json in ${packageDir}: ${(e as Error).message}`, + ); _exportsCache.set(packageDir, null); return null; } @@ -515,8 +519,10 @@ export function resolveImportPath( // unresolved ".." components (PathBuf::components().collect() doesn't // collapse parent refs). Apply the remap on the JS side as a fallback. return remapJsToTs(normalized, rootDir); - } catch { - // fall through to JS + } catch (e) { + debug( + `resolveImportPath: native resolution failed, falling back to JS: ${(e as Error).message}`, + ); } } return resolveImportPathJS(fromFile, importSource, rootDir, aliases); @@ -535,8 +541,10 @@ export function computeConfidence( if (native) { try { return native.computeConfidence(callerFile, targetFile, importedFrom || null); - } catch { - // fall through to JS + } catch (e) { + debug( + `computeConfidence: native computation failed, falling back to JS: ${(e as Error).message}`, + ); } } return computeConfidenceJS(callerFile, targetFile, importedFrom); @@ -575,7 +583,8 @@ export function resolveImportsBatch( map.set(`${r.fromFile}|${r.importSource}`, resolved); } return map; - } catch { + } catch (e) { + debug(`batchResolve: native batch resolution failed: ${(e as Error).message}`); return null; } } diff --git a/src/domain/parser.ts b/src/domain/parser.ts index ecbc3f88..0497d5e9 100644 --- a/src/domain/parser.ts +++ b/src/domain/parser.ts @@ -442,7 +442,8 @@ async function backfillTypeMap( if (!code) { try { code = fs.readFileSync(filePath, 'utf-8'); - } catch { + } catch (e) { + debug(`backfillTypeMap: failed to read ${filePath}: ${(e as Error).message}`); return { typeMap: new Map(), backfilled: false }; } } @@ -458,7 +459,9 @@ async function backfillTypeMap( if (extracted?.tree && typeof extracted.tree.delete === 'function') { try { extracted.tree.delete(); - } catch {} + } catch (e) { + debug(`backfillTypeMap: WASM tree cleanup failed: ${(e as Error).message}`); + } } } } @@ -571,14 +574,16 @@ export async function parseFilesAuto( symbols.typeMap = extracted.symbols.typeMap; symbols._typeMapBackfilled = true; } - } catch { - /* skip — typeMap is a best-effort backfill */ + } catch (e) { + debug(`batchExtract: typeMap backfill failed: ${(e as Error).message}`); } finally { // Free the WASM tree to prevent memory accumulation across repeated builds if (extracted?.tree && typeof extracted.tree.delete === 'function') { try { extracted.tree.delete(); - } catch {} + } catch (e) { + debug(`batchExtract: WASM tree cleanup failed: ${(e as Error).message}`); + } } } } diff --git a/src/infrastructure/config.ts b/src/infrastructure/config.ts index fb162f15..d6a7c609 100644 --- a/src/infrastructure/config.ts +++ b/src/infrastructure/config.ts @@ -310,8 +310,10 @@ function resolveWorkspaceEntry(pkgDir: string): string | null { const candidate = path.resolve(pkgDir, idx); if (fs.existsSync(candidate)) return candidate; } - } catch { - /* ignore */ + } catch (e) { + debug( + `resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${(e as Error).message}`, + ); } return null; } @@ -344,8 +346,8 @@ export function detectWorkspaces(rootDir: string): Map { } } } - } catch { - /* ignore */ + } catch (e) { + debug(`detectWorkspaces: failed to parse pnpm-workspace.yaml: ${(e as Error).message}`); } } @@ -363,8 +365,8 @@ export function detectWorkspaces(rootDir: string): Map { // Yarn classic format: { packages: [...], nohoist: [...] } patterns.push(...ws.packages); } - } catch { - /* ignore */ + } catch (e) { + debug(`detectWorkspaces: failed to parse package.json workspaces: ${(e as Error).message}`); } } } @@ -379,8 +381,8 @@ export function detectWorkspaces(rootDir: string): Map { if (Array.isArray(lerna.packages)) { patterns.push(...lerna.packages); } - } catch { - /* ignore */ + } catch (e) { + debug(`detectWorkspaces: failed to parse lerna.json: ${(e as Error).message}`); } } } diff --git a/src/infrastructure/native.ts b/src/infrastructure/native.ts index ed29ae3d..a397405f 100644 --- a/src/infrastructure/native.ts +++ b/src/infrastructure/native.ts @@ -10,6 +10,7 @@ import { createRequire } from 'node:module'; import os from 'node:os'; import { EngineError } from '../shared/errors.js'; import type { NativeAddon } from '../types.js'; +import { debug } from './logger.js'; let _cached: NativeAddon | null | undefined; // undefined = not yet tried, null = failed, NativeAddon = module let _loadError: Error | null = null; @@ -26,7 +27,9 @@ function detectLibc(): 'gnu' | 'musl' { if (files.some((f: string) => f.startsWith('ld-musl-') && f.endsWith('.so.1'))) { return 'musl'; } - } catch {} + } catch (e) { + debug(`detectLibc: failed to read /lib: ${(e as Error).message}`); + } return 'gnu'; } @@ -92,7 +95,10 @@ export function getNativePackageVersion(): string | null { try { const pkgJson = _require(`${pkg}/package.json`) as { version?: string }; return pkgJson.version || null; - } catch { + } catch (e) { + debug( + `getNativePackageVersion: failed to read package.json for ${pkg}: ${(e as Error).message}`, + ); return null; } } From b101d49766ae36b991c7073092f369e1b9067fe2 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 22:33:07 -0600 Subject: [PATCH 2/7] refactor: split impact.ts into fn-impact and diff-impact modules --- src/domain/analysis/diff-impact.ts | 356 ++++++++++++ src/domain/analysis/fn-impact.ts | 242 ++++++++ src/domain/analysis/impact.ts | 727 +----------------------- src/presentation/diff-impact-mermaid.ts | 129 +++++ 4 files changed, 736 insertions(+), 718 deletions(-) create mode 100644 src/domain/analysis/diff-impact.ts create mode 100644 src/domain/analysis/fn-impact.ts create mode 100644 src/presentation/diff-impact-mermaid.ts diff --git a/src/domain/analysis/diff-impact.ts b/src/domain/analysis/diff-impact.ts new file mode 100644 index 00000000..c2c684c5 --- /dev/null +++ b/src/domain/analysis/diff-impact.ts @@ -0,0 +1,356 @@ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { findDbPath, openReadonlyOrFail } from '../../db/index.js'; +import { cachedStmt } from '../../db/repository/cached-stmt.js'; +import { evaluateBoundaries } from '../../features/boundaries.js'; +import { coChangeForFiles } from '../../features/cochange.js'; +import { ownersForFiles } from '../../features/owners.js'; +import { loadConfig } from '../../infrastructure/config.js'; +import { debug } from '../../infrastructure/logger.js'; +import { isTestFile } from '../../infrastructure/test-filter.js'; +import { paginateResult } from '../../shared/paginate.js'; +import type { BetterSqlite3Database, NodeRow, StmtCache } from '../../types.js'; +import { bfsTransitiveCallers } from './fn-impact.js'; + +const _defsStmtCache: StmtCache = new WeakMap(); + +// --- diffImpactData helpers --- + +/** + * Walk up from repoRoot until a .git directory is found. + * Returns true if a git root exists, false otherwise. + */ +function findGitRoot(repoRoot: string): boolean { + let checkDir = repoRoot; + while (checkDir) { + if (fs.existsSync(path.join(checkDir, '.git'))) { + return true; + } + const parent = path.dirname(checkDir); + if (parent === checkDir) break; + checkDir = parent; + } + return false; +} + +/** + * Execute git diff and return the raw output string. + * Returns `{ output: string }` on success or `{ error: string }` on failure. + */ +function runGitDiff( + repoRoot: string, + opts: { staged?: boolean; ref?: string }, +): { output: string; error?: never } | { error: string; output?: never } { + try { + const args = opts.staged + ? ['diff', '--cached', '--unified=0', '--no-color'] + : ['diff', opts.ref || 'HEAD', '--unified=0', '--no-color']; + const output = execFileSync('git', args, { + cwd: repoRoot, + encoding: 'utf-8', + maxBuffer: 10 * 1024 * 1024, + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { output }; + } catch (e: unknown) { + return { error: `Failed to run git diff: ${(e as Error).message}` }; + } +} + +/** + * Parse raw git diff output into a changedRanges map and newFiles set. + */ +function parseGitDiff(diffOutput: string) { + const changedRanges = new Map>(); + const newFiles = new Set(); + let currentFile: string | null = null; + let prevIsDevNull = false; + + for (const line of diffOutput.split('\n')) { + if (line.startsWith('--- /dev/null')) { + prevIsDevNull = true; + continue; + } + if (line.startsWith('--- ')) { + prevIsDevNull = false; + continue; + } + const fileMatch = line.match(/^\+\+\+ b\/(.+)/); + if (fileMatch) { + currentFile = fileMatch[1]!; + if (!changedRanges.has(currentFile)) changedRanges.set(currentFile, []); + if (prevIsDevNull) newFiles.add(currentFile!); + prevIsDevNull = false; + continue; + } + const hunkMatch = line.match(/^@@ .+ \+(\d+)(?:,(\d+))? @@/); + if (hunkMatch && currentFile) { + const start = parseInt(hunkMatch[1]!, 10); + const count = parseInt(hunkMatch[2] || '1', 10); + changedRanges.get(currentFile)!.push({ start, end: start + count - 1 }); + } + } + + return { changedRanges, newFiles }; +} + +/** + * Find all function/method/class nodes whose line ranges overlap any changed range. + */ +function findAffectedFunctions( + db: BetterSqlite3Database, + changedRanges: Map>, + noTests: boolean, +): NodeRow[] { + const affectedFunctions: NodeRow[] = []; + const defsStmt = cachedStmt( + _defsStmtCache, + db, + `SELECT * FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') ORDER BY line`, + ); + for (const [file, ranges] of changedRanges) { + if (noTests && isTestFile(file)) continue; + const defs = defsStmt.all(file) as NodeRow[]; + for (let i = 0; i < defs.length; i++) { + const def = defs[i]!; + const endLine = def.end_line || (defs[i + 1] ? defs[i + 1]!.line - 1 : 999999); + for (const range of ranges) { + if (range.start <= endLine && range.end >= def.line) { + affectedFunctions.push(def); + break; + } + } + } + } + return affectedFunctions; +} + +/** + * Run BFS per affected function, collecting per-function results and the full affected set. + */ +function buildFunctionImpactResults( + db: BetterSqlite3Database, + affectedFunctions: NodeRow[], + noTests: boolean, + maxDepth: number, + includeImplementors = true, +) { + const allAffected = new Set(); + const functionResults = affectedFunctions.map((fn) => { + const edges: Array<{ from: string; to: string }> = []; + const idToKey = new Map(); + idToKey.set(fn.id, `${fn.file}::${fn.name}:${fn.line}`); + + const { levels, totalDependents } = bfsTransitiveCallers(db, fn.id, { + noTests, + maxDepth, + includeImplementors, + onVisit(c, parentId) { + allAffected.add(`${c.file}:${c.name}`); + const callerKey = `${c.file}::${c.name}:${c.line}`; + idToKey.set(c.id, callerKey); + edges.push({ from: idToKey.get(parentId)!, to: callerKey }); + }, + }); + + return { + name: fn.name, + kind: fn.kind, + file: fn.file, + line: fn.line, + transitiveCallers: totalDependents, + levels, + edges, + }; + }); + + return { functionResults, allAffected }; +} + +/** + * Look up historically co-changed files for the set of changed files. + * Returns an empty array if the co_changes table is unavailable. + */ +function lookupCoChanges( + db: BetterSqlite3Database, + changedRanges: Map, + affectedFiles: Set, + noTests: boolean, +) { + try { + db.prepare('SELECT 1 FROM co_changes LIMIT 1').get(); + const changedFilesList = [...changedRanges.keys()]; + const coResults = coChangeForFiles(changedFilesList, db, { + minJaccard: 0.3, + limit: 20, + noTests, + }); + return coResults.filter((r: { file: string }) => !affectedFiles.has(r.file)); + } catch (e: unknown) { + debug(`co_changes lookup skipped: ${(e as Error).message}`); + return []; + } +} + +/** + * Look up CODEOWNERS for changed and affected files. + * Returns null if no owners are found or lookup fails. + */ +function lookupOwnership( + changedRanges: Map, + affectedFiles: Set, + repoRoot: string, +) { + try { + const allFilePaths = [...new Set([...changedRanges.keys(), ...affectedFiles])]; + const ownerResult = ownersForFiles(allFilePaths, repoRoot); + if (ownerResult.affectedOwners.length > 0) { + return { + owners: Object.fromEntries(ownerResult.owners), + affectedOwners: ownerResult.affectedOwners, + suggestedReviewers: ownerResult.suggestedReviewers, + }; + } + return null; + } catch (e: unknown) { + debug(`CODEOWNERS lookup skipped: ${(e as Error).message}`); + return null; + } +} + +/** + * Check manifesto boundary violations scoped to the changed files. + * Returns `{ boundaryViolations, boundaryViolationCount }`. + */ +function checkBoundaryViolations( + db: BetterSqlite3Database, + changedRanges: Map, + noTests: boolean, + // biome-ignore lint/suspicious/noExplicitAny: opts shape varies by caller + opts: any, + repoRoot: string, +) { + try { + const cfg = opts.config || loadConfig(repoRoot); + const boundaryConfig = cfg.manifesto?.boundaries; + if (boundaryConfig) { + const result = evaluateBoundaries(db, boundaryConfig, { + scopeFiles: [...changedRanges.keys()], + noTests, + }); + return { + boundaryViolations: result.violations, + boundaryViolationCount: result.violationCount, + }; + } + } catch (e: unknown) { + debug(`boundary check skipped: ${(e as Error).message}`); + } + return { boundaryViolations: [], boundaryViolationCount: 0 }; +} + +// --- diffImpactData --- + +/** + * Fix #2: Shell injection vulnerability. + * Uses execFileSync instead of execSync to prevent shell interpretation of user input. + */ +export function diffImpactData( + customDbPath: string, + opts: { + noTests?: boolean; + depth?: number; + staged?: boolean; + ref?: string; + includeImplementors?: boolean; + limit?: number; + offset?: number; + // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic + config?: any; + } = {}, +) { + const db = openReadonlyOrFail(customDbPath); + try { + const noTests = opts.noTests || false; + const config = opts.config || loadConfig(); + const maxDepth = opts.depth || config.analysis?.impactDepth || 3; + + const dbPath = findDbPath(customDbPath); + const repoRoot = path.resolve(path.dirname(dbPath), '..'); + + if (!findGitRoot(repoRoot)) { + return { error: `Not a git repository: ${repoRoot}` }; + } + + const gitResult = runGitDiff(repoRoot, opts); + if ('error' in gitResult) return { error: gitResult.error }; + + if (!gitResult.output.trim()) { + return { + changedFiles: 0, + newFiles: [], + affectedFunctions: [], + affectedFiles: [], + summary: null, + }; + } + + const { changedRanges, newFiles } = parseGitDiff(gitResult.output); + + if (changedRanges.size === 0) { + return { + changedFiles: 0, + newFiles: [], + affectedFunctions: [], + affectedFiles: [], + summary: null, + }; + } + + const affectedFunctions = findAffectedFunctions(db, changedRanges, noTests); + const includeImplementors = opts.includeImplementors !== false; + const { functionResults, allAffected } = buildFunctionImpactResults( + db, + affectedFunctions, + noTests, + maxDepth, + includeImplementors, + ); + + const affectedFiles = new Set(); + for (const key of allAffected) affectedFiles.add(key.split(':')[0]!); + + const historicallyCoupled = lookupCoChanges(db, changedRanges, affectedFiles, noTests); + const ownership = lookupOwnership(changedRanges, affectedFiles, repoRoot); + const { boundaryViolations, boundaryViolationCount } = checkBoundaryViolations( + db, + changedRanges, + noTests, + opts, + repoRoot, + ); + + const base = { + changedFiles: changedRanges.size, + newFiles: [...newFiles], + affectedFunctions: functionResults, + affectedFiles: [...affectedFiles], + historicallyCoupled, + ownership, + boundaryViolations, + boundaryViolationCount, + summary: { + functionsChanged: affectedFunctions.length, + callersAffected: allAffected.size, + filesAffected: affectedFiles.size, + historicallyCoupledCount: historicallyCoupled.length, + ownersAffected: ownership ? ownership.affectedOwners.length : 0, + boundaryViolationCount, + }, + }; + return paginateResult(base, 'affectedFunctions', { limit: opts.limit, offset: opts.offset }); + } finally { + db.close(); + } +} diff --git a/src/domain/analysis/fn-impact.ts b/src/domain/analysis/fn-impact.ts new file mode 100644 index 00000000..114b2db7 --- /dev/null +++ b/src/domain/analysis/fn-impact.ts @@ -0,0 +1,242 @@ +import { + findDistinctCallers, + findFileNodes, + findImplementors, + findImportDependents, + findNodeById, + openReadonlyOrFail, +} from '../../db/index.js'; +import { loadConfig } from '../../infrastructure/config.js'; +import { isTestFile } from '../../infrastructure/test-filter.js'; +import { normalizeSymbol } from '../../shared/normalize.js'; +import { paginateResult } from '../../shared/paginate.js'; +import type { BetterSqlite3Database, NodeRow, RelatedNodeRow } from '../../types.js'; +import { findMatchingNodes } from './symbol-lookup.js'; + +// --- Shared BFS: transitive callers --- + +const INTERFACE_LIKE_KINDS = new Set(['interface', 'trait']); + +/** + * Check whether the graph contains any 'implements' edges. + * Cached per db handle so the query runs at most once per connection. + */ +const _hasImplementsCache: WeakMap = new WeakMap(); +function hasImplementsEdges(db: BetterSqlite3Database): boolean { + if (_hasImplementsCache.has(db)) return _hasImplementsCache.get(db)!; + const row = db.prepare("SELECT 1 FROM edges WHERE kind = 'implements' LIMIT 1").get(); + const result = !!row; + _hasImplementsCache.set(db, result); + return result; +} + +/** + * BFS traversal to find transitive callers of a node. + * When an interface/trait node is encountered (either as the start node or + * during traversal), its concrete implementors are also added to the frontier + * so that changes to an interface signature propagate to all implementors. + */ +export function bfsTransitiveCallers( + db: BetterSqlite3Database, + startId: number, + { + noTests = false, + maxDepth = 3, + includeImplementors = true, + onVisit, + }: { + noTests?: boolean; + maxDepth?: number; + includeImplementors?: boolean; + onVisit?: ( + caller: RelatedNodeRow & { viaImplements?: boolean }, + parentId: number, + depth: number, + ) => void; + } = {}, +) { + // Skip all implementor lookups when the graph has no implements edges + const resolveImplementors = includeImplementors && hasImplementsEdges(db); + + const visited = new Set([startId]); + const levels: Record< + number, + Array<{ name: string; kind: string; file: string; line: number; viaImplements?: boolean }> + > = {}; + let frontier = [startId]; + + // Seed: if start node is an interface/trait, include its implementors at depth 1. + // Implementors go into a separate list so their callers appear at depth 2, not depth 1. + const implNextFrontier: number[] = []; + if (resolveImplementors) { + const startNode = findNodeById(db, startId) as NodeRow | undefined; + if (startNode && INTERFACE_LIKE_KINDS.has(startNode.kind)) { + const impls = findImplementors(db, startId) as RelatedNodeRow[]; + for (const impl of impls) { + if (!visited.has(impl.id) && (!noTests || !isTestFile(impl.file))) { + visited.add(impl.id); + implNextFrontier.push(impl.id); + if (!levels[1]) levels[1] = []; + levels[1].push({ + name: impl.name, + kind: impl.kind, + file: impl.file, + line: impl.line, + viaImplements: true, + }); + if (onVisit) onVisit({ ...impl, viaImplements: true }, startId, 1); + } + } + } + } + + for (let d = 1; d <= maxDepth; d++) { + // On the first wave, merge seeded implementors so their callers appear at d=2 + if (d === 1 && implNextFrontier.length > 0) { + frontier = [...frontier, ...implNextFrontier]; + } + const nextFrontier: number[] = []; + for (const fid of frontier) { + const callers = findDistinctCallers(db, fid) as RelatedNodeRow[]; + for (const c of callers) { + if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) { + visited.add(c.id); + nextFrontier.push(c.id); + if (!levels[d]) levels[d] = []; + levels[d]!.push({ name: c.name, kind: c.kind, file: c.file, line: c.line }); + if (onVisit) onVisit(c, fid, d); + } + + // If a caller is an interface/trait, also pull in its implementors + // Implementors are one extra hop away, so record at d+1 + if (resolveImplementors && INTERFACE_LIKE_KINDS.has(c.kind)) { + const impls = findImplementors(db, c.id) as RelatedNodeRow[]; + for (const impl of impls) { + if (!visited.has(impl.id) && (!noTests || !isTestFile(impl.file))) { + visited.add(impl.id); + nextFrontier.push(impl.id); + const implDepth = d + 1; + if (!levels[implDepth]) levels[implDepth] = []; + levels[implDepth].push({ + name: impl.name, + kind: impl.kind, + file: impl.file, + line: impl.line, + viaImplements: true, + }); + if (onVisit) onVisit({ ...impl, viaImplements: true }, c.id, implDepth); + } + } + } + } + } + frontier = nextFrontier; + if (frontier.length === 0) break; + } + + return { totalDependents: visited.size - 1, levels }; +} + +export function impactAnalysisData( + file: string, + customDbPath: string, + opts: { noTests?: boolean } = {}, +) { + const db = openReadonlyOrFail(customDbPath); + try { + const noTests = opts.noTests || false; + const fileNodes = findFileNodes(db, `%${file}%`) as NodeRow[]; + if (fileNodes.length === 0) { + return { file, sources: [], levels: {}, totalDependents: 0 }; + } + + const visited = new Set(); + const queue: number[] = []; + const levels = new Map(); + + for (const fn of fileNodes) { + visited.add(fn.id); + queue.push(fn.id); + levels.set(fn.id, 0); + } + + while (queue.length > 0) { + const current = queue.shift()!; + const level = levels.get(current)!; + const dependents = findImportDependents(db, current) as RelatedNodeRow[]; + for (const dep of dependents) { + if (!visited.has(dep.id) && (!noTests || !isTestFile(dep.file))) { + visited.add(dep.id); + queue.push(dep.id); + levels.set(dep.id, level + 1); + } + } + } + + const byLevel: Record> = {}; + for (const [id, level] of levels) { + if (level === 0) continue; + if (!byLevel[level]) byLevel[level] = []; + const node = findNodeById(db, id) as NodeRow | undefined; + if (node) byLevel[level].push({ file: node.file }); + } + + return { + file, + sources: fileNodes.map((f) => f.file), + levels: byLevel, + totalDependents: visited.size - fileNodes.length, + }; + } finally { + db.close(); + } +} + +export function fnImpactData( + name: string, + customDbPath: string, + opts: { + depth?: number; + noTests?: boolean; + file?: string; + kind?: string; + includeImplementors?: boolean; + limit?: number; + offset?: number; + // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic + config?: any; + } = {}, +) { + const db = openReadonlyOrFail(customDbPath); + try { + const config = opts.config || loadConfig(); + const maxDepth = opts.depth || config.analysis?.fnImpactDepth || 5; + const noTests = opts.noTests || false; + const hc = new Map(); + + const nodes = findMatchingNodes(db, name, { noTests, file: opts.file, kind: opts.kind }); + if (nodes.length === 0) { + return { name, results: [] }; + } + + const includeImplementors = opts.includeImplementors !== false; + + const results = nodes.map((node) => { + const { levels, totalDependents } = bfsTransitiveCallers(db, node.id, { + noTests, + maxDepth, + includeImplementors, + }); + return { + ...normalizeSymbol(node, db, hc), + levels, + totalDependents, + }; + }); + + const base = { name, results }; + return paginateResult(base, 'results', { limit: opts.limit, offset: opts.offset }); + } finally { + db.close(); + } +} diff --git a/src/domain/analysis/impact.ts b/src/domain/analysis/impact.ts index 58882bc7..5cd25214 100644 --- a/src/domain/analysis/impact.ts +++ b/src/domain/analysis/impact.ts @@ -1,721 +1,12 @@ -import { execFileSync } from 'node:child_process'; -import fs from 'node:fs'; -import path from 'node:path'; -import { - findDbPath, - findDistinctCallers, - findFileNodes, - findImplementors, - findImportDependents, - findNodeById, - openReadonlyOrFail, -} from '../../db/index.js'; -import { cachedStmt } from '../../db/repository/cached-stmt.js'; -import { evaluateBoundaries } from '../../features/boundaries.js'; -import { coChangeForFiles } from '../../features/cochange.js'; -import { ownersForFiles } from '../../features/owners.js'; -import { loadConfig } from '../../infrastructure/config.js'; -import { debug } from '../../infrastructure/logger.js'; -import { isTestFile } from '../../infrastructure/test-filter.js'; -import { normalizeSymbol } from '../../shared/normalize.js'; -import { paginateResult } from '../../shared/paginate.js'; -import type { BetterSqlite3Database, NodeRow, RelatedNodeRow, StmtCache } from '../../types.js'; -import { findMatchingNodes } from './symbol-lookup.js'; - -const _defsStmtCache: StmtCache = new WeakMap(); - -// --- Shared BFS: transitive callers --- - -const INTERFACE_LIKE_KINDS = new Set(['interface', 'trait']); - -/** - * Check whether the graph contains any 'implements' edges. - * Cached per db handle so the query runs at most once per connection. - */ -const _hasImplementsCache: WeakMap = new WeakMap(); -function hasImplementsEdges(db: BetterSqlite3Database): boolean { - if (_hasImplementsCache.has(db)) return _hasImplementsCache.get(db)!; - const row = db.prepare("SELECT 1 FROM edges WHERE kind = 'implements' LIMIT 1").get(); - const result = !!row; - _hasImplementsCache.set(db, result); - return result; -} - -/** - * BFS traversal to find transitive callers of a node. - * When an interface/trait node is encountered (either as the start node or - * during traversal), its concrete implementors are also added to the frontier - * so that changes to an interface signature propagate to all implementors. - */ -export function bfsTransitiveCallers( - db: BetterSqlite3Database, - startId: number, - { - noTests = false, - maxDepth = 3, - includeImplementors = true, - onVisit, - }: { - noTests?: boolean; - maxDepth?: number; - includeImplementors?: boolean; - onVisit?: ( - caller: RelatedNodeRow & { viaImplements?: boolean }, - parentId: number, - depth: number, - ) => void; - } = {}, -) { - // Skip all implementor lookups when the graph has no implements edges - const resolveImplementors = includeImplementors && hasImplementsEdges(db); - - const visited = new Set([startId]); - const levels: Record< - number, - Array<{ name: string; kind: string; file: string; line: number; viaImplements?: boolean }> - > = {}; - let frontier = [startId]; - - // Seed: if start node is an interface/trait, include its implementors at depth 1. - // Implementors go into a separate list so their callers appear at depth 2, not depth 1. - const implNextFrontier: number[] = []; - if (resolveImplementors) { - const startNode = findNodeById(db, startId) as NodeRow | undefined; - if (startNode && INTERFACE_LIKE_KINDS.has(startNode.kind)) { - const impls = findImplementors(db, startId) as RelatedNodeRow[]; - for (const impl of impls) { - if (!visited.has(impl.id) && (!noTests || !isTestFile(impl.file))) { - visited.add(impl.id); - implNextFrontier.push(impl.id); - if (!levels[1]) levels[1] = []; - levels[1].push({ - name: impl.name, - kind: impl.kind, - file: impl.file, - line: impl.line, - viaImplements: true, - }); - if (onVisit) onVisit({ ...impl, viaImplements: true }, startId, 1); - } - } - } - } - - for (let d = 1; d <= maxDepth; d++) { - // On the first wave, merge seeded implementors so their callers appear at d=2 - if (d === 1 && implNextFrontier.length > 0) { - frontier = [...frontier, ...implNextFrontier]; - } - const nextFrontier: number[] = []; - for (const fid of frontier) { - const callers = findDistinctCallers(db, fid) as RelatedNodeRow[]; - for (const c of callers) { - if (!visited.has(c.id) && (!noTests || !isTestFile(c.file))) { - visited.add(c.id); - nextFrontier.push(c.id); - if (!levels[d]) levels[d] = []; - levels[d]!.push({ name: c.name, kind: c.kind, file: c.file, line: c.line }); - if (onVisit) onVisit(c, fid, d); - } - - // If a caller is an interface/trait, also pull in its implementors - // Implementors are one extra hop away, so record at d+1 - if (resolveImplementors && INTERFACE_LIKE_KINDS.has(c.kind)) { - const impls = findImplementors(db, c.id) as RelatedNodeRow[]; - for (const impl of impls) { - if (!visited.has(impl.id) && (!noTests || !isTestFile(impl.file))) { - visited.add(impl.id); - nextFrontier.push(impl.id); - const implDepth = d + 1; - if (!levels[implDepth]) levels[implDepth] = []; - levels[implDepth].push({ - name: impl.name, - kind: impl.kind, - file: impl.file, - line: impl.line, - viaImplements: true, - }); - if (onVisit) onVisit({ ...impl, viaImplements: true }, c.id, implDepth); - } - } - } - } - } - frontier = nextFrontier; - if (frontier.length === 0) break; - } - - return { totalDependents: visited.size - 1, levels }; -} - -export function impactAnalysisData( - file: string, - customDbPath: string, - opts: { noTests?: boolean } = {}, -) { - const db = openReadonlyOrFail(customDbPath); - try { - const noTests = opts.noTests || false; - const fileNodes = findFileNodes(db, `%${file}%`) as NodeRow[]; - if (fileNodes.length === 0) { - return { file, sources: [], levels: {}, totalDependents: 0 }; - } - - const visited = new Set(); - const queue: number[] = []; - const levels = new Map(); - - for (const fn of fileNodes) { - visited.add(fn.id); - queue.push(fn.id); - levels.set(fn.id, 0); - } - - while (queue.length > 0) { - const current = queue.shift()!; - const level = levels.get(current)!; - const dependents = findImportDependents(db, current) as RelatedNodeRow[]; - for (const dep of dependents) { - if (!visited.has(dep.id) && (!noTests || !isTestFile(dep.file))) { - visited.add(dep.id); - queue.push(dep.id); - levels.set(dep.id, level + 1); - } - } - } - - const byLevel: Record> = {}; - for (const [id, level] of levels) { - if (level === 0) continue; - if (!byLevel[level]) byLevel[level] = []; - const node = findNodeById(db, id) as NodeRow | undefined; - if (node) byLevel[level].push({ file: node.file }); - } - - return { - file, - sources: fileNodes.map((f) => f.file), - levels: byLevel, - totalDependents: visited.size - fileNodes.length, - }; - } finally { - db.close(); - } -} - -export function fnImpactData( - name: string, - customDbPath: string, - opts: { - depth?: number; - noTests?: boolean; - file?: string; - kind?: string; - includeImplementors?: boolean; - limit?: number; - offset?: number; - // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic - config?: any; - } = {}, -) { - const db = openReadonlyOrFail(customDbPath); - try { - const config = opts.config || loadConfig(); - const maxDepth = opts.depth || config.analysis?.fnImpactDepth || 5; - const noTests = opts.noTests || false; - const hc = new Map(); - - const nodes = findMatchingNodes(db, name, { noTests, file: opts.file, kind: opts.kind }); - if (nodes.length === 0) { - return { name, results: [] }; - } - - const includeImplementors = opts.includeImplementors !== false; - - const results = nodes.map((node) => { - const { levels, totalDependents } = bfsTransitiveCallers(db, node.id, { - noTests, - maxDepth, - includeImplementors, - }); - return { - ...normalizeSymbol(node, db, hc), - levels, - totalDependents, - }; - }); - - const base = { name, results }; - return paginateResult(base, 'results', { limit: opts.limit, offset: opts.offset }); - } finally { - db.close(); - } -} - -// --- diffImpactData helpers --- - -/** - * Walk up from repoRoot until a .git directory is found. - * Returns true if a git root exists, false otherwise. - */ -function findGitRoot(repoRoot: string): boolean { - let checkDir = repoRoot; - while (checkDir) { - if (fs.existsSync(path.join(checkDir, '.git'))) { - return true; - } - const parent = path.dirname(checkDir); - if (parent === checkDir) break; - checkDir = parent; - } - return false; -} - -/** - * Execute git diff and return the raw output string. - * Returns `{ output: string }` on success or `{ error: string }` on failure. - */ -function runGitDiff( - repoRoot: string, - opts: { staged?: boolean; ref?: string }, -): { output: string; error?: never } | { error: string; output?: never } { - try { - const args = opts.staged - ? ['diff', '--cached', '--unified=0', '--no-color'] - : ['diff', opts.ref || 'HEAD', '--unified=0', '--no-color']; - const output = execFileSync('git', args, { - cwd: repoRoot, - encoding: 'utf-8', - maxBuffer: 10 * 1024 * 1024, - stdio: ['pipe', 'pipe', 'pipe'], - }); - return { output }; - } catch (e: unknown) { - return { error: `Failed to run git diff: ${(e as Error).message}` }; - } -} - -/** - * Parse raw git diff output into a changedRanges map and newFiles set. - */ -function parseGitDiff(diffOutput: string) { - const changedRanges = new Map>(); - const newFiles = new Set(); - let currentFile: string | null = null; - let prevIsDevNull = false; - - for (const line of diffOutput.split('\n')) { - if (line.startsWith('--- /dev/null')) { - prevIsDevNull = true; - continue; - } - if (line.startsWith('--- ')) { - prevIsDevNull = false; - continue; - } - const fileMatch = line.match(/^\+\+\+ b\/(.+)/); - if (fileMatch) { - currentFile = fileMatch[1]!; - if (!changedRanges.has(currentFile)) changedRanges.set(currentFile, []); - if (prevIsDevNull) newFiles.add(currentFile!); - prevIsDevNull = false; - continue; - } - const hunkMatch = line.match(/^@@ .+ \+(\d+)(?:,(\d+))? @@/); - if (hunkMatch && currentFile) { - const start = parseInt(hunkMatch[1]!, 10); - const count = parseInt(hunkMatch[2] || '1', 10); - changedRanges.get(currentFile)!.push({ start, end: start + count - 1 }); - } - } - - return { changedRanges, newFiles }; -} - -/** - * Find all function/method/class nodes whose line ranges overlap any changed range. - */ -function findAffectedFunctions( - db: BetterSqlite3Database, - changedRanges: Map>, - noTests: boolean, -): NodeRow[] { - const affectedFunctions: NodeRow[] = []; - const defsStmt = cachedStmt( - _defsStmtCache, - db, - `SELECT * FROM nodes WHERE file = ? AND kind IN ('function', 'method', 'class') ORDER BY line`, - ); - for (const [file, ranges] of changedRanges) { - if (noTests && isTestFile(file)) continue; - const defs = defsStmt.all(file) as NodeRow[]; - for (let i = 0; i < defs.length; i++) { - const def = defs[i]!; - const endLine = def.end_line || (defs[i + 1] ? defs[i + 1]!.line - 1 : 999999); - for (const range of ranges) { - if (range.start <= endLine && range.end >= def.line) { - affectedFunctions.push(def); - break; - } - } - } - } - return affectedFunctions; -} - -/** - * Run BFS per affected function, collecting per-function results and the full affected set. - */ -function buildFunctionImpactResults( - db: BetterSqlite3Database, - affectedFunctions: NodeRow[], - noTests: boolean, - maxDepth: number, - includeImplementors = true, -) { - const allAffected = new Set(); - const functionResults = affectedFunctions.map((fn) => { - const edges: Array<{ from: string; to: string }> = []; - const idToKey = new Map(); - idToKey.set(fn.id, `${fn.file}::${fn.name}:${fn.line}`); - - const { levels, totalDependents } = bfsTransitiveCallers(db, fn.id, { - noTests, - maxDepth, - includeImplementors, - onVisit(c, parentId) { - allAffected.add(`${c.file}:${c.name}`); - const callerKey = `${c.file}::${c.name}:${c.line}`; - idToKey.set(c.id, callerKey); - edges.push({ from: idToKey.get(parentId)!, to: callerKey }); - }, - }); - - return { - name: fn.name, - kind: fn.kind, - file: fn.file, - line: fn.line, - transitiveCallers: totalDependents, - levels, - edges, - }; - }); - - return { functionResults, allAffected }; -} - -/** - * Look up historically co-changed files for the set of changed files. - * Returns an empty array if the co_changes table is unavailable. - */ -function lookupCoChanges( - db: BetterSqlite3Database, - changedRanges: Map, - affectedFiles: Set, - noTests: boolean, -) { - try { - db.prepare('SELECT 1 FROM co_changes LIMIT 1').get(); - const changedFilesList = [...changedRanges.keys()]; - const coResults = coChangeForFiles(changedFilesList, db, { - minJaccard: 0.3, - limit: 20, - noTests, - }); - return coResults.filter((r: { file: string }) => !affectedFiles.has(r.file)); - } catch (e: unknown) { - debug(`co_changes lookup skipped: ${(e as Error).message}`); - return []; - } -} - /** - * Look up CODEOWNERS for changed and affected files. - * Returns null if no owners are found or lookup fails. + * impact.ts — Re-export barrel for backward compatibility. + * + * The implementation has been split into focused modules: + * - fn-impact.ts: bfsTransitiveCallers, impactAnalysisData, fnImpactData + * - diff-impact.ts: diffImpactData and git diff analysis helpers + * - presentation/diff-impact-mermaid.ts: diffImpactMermaid (Mermaid diagram generation) */ -function lookupOwnership( - changedRanges: Map, - affectedFiles: Set, - repoRoot: string, -) { - try { - const allFilePaths = [...new Set([...changedRanges.keys(), ...affectedFiles])]; - const ownerResult = ownersForFiles(allFilePaths, repoRoot); - if (ownerResult.affectedOwners.length > 0) { - return { - owners: Object.fromEntries(ownerResult.owners), - affectedOwners: ownerResult.affectedOwners, - suggestedReviewers: ownerResult.suggestedReviewers, - }; - } - return null; - } catch (e: unknown) { - debug(`CODEOWNERS lookup skipped: ${(e as Error).message}`); - return null; - } -} - -/** - * Check manifesto boundary violations scoped to the changed files. - * Returns `{ boundaryViolations, boundaryViolationCount }`. - */ -function checkBoundaryViolations( - db: BetterSqlite3Database, - changedRanges: Map, - noTests: boolean, - // biome-ignore lint/suspicious/noExplicitAny: opts shape varies by caller - opts: any, - repoRoot: string, -) { - try { - const cfg = opts.config || loadConfig(repoRoot); - const boundaryConfig = cfg.manifesto?.boundaries; - if (boundaryConfig) { - const result = evaluateBoundaries(db, boundaryConfig, { - scopeFiles: [...changedRanges.keys()], - noTests, - }); - return { - boundaryViolations: result.violations, - boundaryViolationCount: result.violationCount, - }; - } - } catch (e: unknown) { - debug(`boundary check skipped: ${(e as Error).message}`); - } - return { boundaryViolations: [], boundaryViolationCount: 0 }; -} - -// --- diffImpactData --- - -/** - * Fix #2: Shell injection vulnerability. - * Uses execFileSync instead of execSync to prevent shell interpretation of user input. - */ -export function diffImpactData( - customDbPath: string, - opts: { - noTests?: boolean; - depth?: number; - staged?: boolean; - ref?: string; - includeImplementors?: boolean; - limit?: number; - offset?: number; - // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic - config?: any; - } = {}, -) { - const db = openReadonlyOrFail(customDbPath); - try { - const noTests = opts.noTests || false; - const config = opts.config || loadConfig(); - const maxDepth = opts.depth || config.analysis?.impactDepth || 3; - - const dbPath = findDbPath(customDbPath); - const repoRoot = path.resolve(path.dirname(dbPath), '..'); - - if (!findGitRoot(repoRoot)) { - return { error: `Not a git repository: ${repoRoot}` }; - } - - const gitResult = runGitDiff(repoRoot, opts); - if ('error' in gitResult) return { error: gitResult.error }; - - if (!gitResult.output.trim()) { - return { - changedFiles: 0, - newFiles: [], - affectedFunctions: [], - affectedFiles: [], - summary: null, - }; - } - - const { changedRanges, newFiles } = parseGitDiff(gitResult.output); - - if (changedRanges.size === 0) { - return { - changedFiles: 0, - newFiles: [], - affectedFunctions: [], - affectedFiles: [], - summary: null, - }; - } - - const affectedFunctions = findAffectedFunctions(db, changedRanges, noTests); - const includeImplementors = opts.includeImplementors !== false; - const { functionResults, allAffected } = buildFunctionImpactResults( - db, - affectedFunctions, - noTests, - maxDepth, - includeImplementors, - ); - - const affectedFiles = new Set(); - for (const key of allAffected) affectedFiles.add(key.split(':')[0]!); - - const historicallyCoupled = lookupCoChanges(db, changedRanges, affectedFiles, noTests); - const ownership = lookupOwnership(changedRanges, affectedFiles, repoRoot); - const { boundaryViolations, boundaryViolationCount } = checkBoundaryViolations( - db, - changedRanges, - noTests, - opts, - repoRoot, - ); - - const base = { - changedFiles: changedRanges.size, - newFiles: [...newFiles], - affectedFunctions: functionResults, - affectedFiles: [...affectedFiles], - historicallyCoupled, - ownership, - boundaryViolations, - boundaryViolationCount, - summary: { - functionsChanged: affectedFunctions.length, - callersAffected: allAffected.size, - filesAffected: affectedFiles.size, - historicallyCoupledCount: historicallyCoupled.length, - ownersAffected: ownership ? ownership.affectedOwners.length : 0, - boundaryViolationCount, - }, - }; - return paginateResult(base, 'affectedFunctions', { limit: opts.limit, offset: opts.offset }); - } finally { - db.close(); - } -} - -export function diffImpactMermaid( - customDbPath: string, - opts: { - noTests?: boolean; - depth?: number; - staged?: boolean; - ref?: string; - includeImplementors?: boolean; - limit?: number; - offset?: number; - // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic - config?: any; - } = {}, -): string { - // biome-ignore lint/suspicious/noExplicitAny: paginateResult returns dynamic shape - const data: any = diffImpactData(customDbPath, opts); - if ('error' in data) return data.error as string; - if (data.changedFiles === 0 || data.affectedFunctions.length === 0) { - return 'flowchart TB\n none["No impacted functions detected"]'; - } - - const newFileSet = new Set(data.newFiles || []); - const lines = ['flowchart TB']; - - // Assign stable Mermaid node IDs - let nodeCounter = 0; - const nodeIdMap = new Map(); - const nodeLabels = new Map(); - function nodeId(key: string, label?: string): string { - if (!nodeIdMap.has(key)) { - nodeIdMap.set(key, `n${nodeCounter++}`); - if (label) nodeLabels.set(key, label); - } - return nodeIdMap.get(key)!; - } - - // Register all nodes (changed functions + their callers) - for (const fn of data.affectedFunctions) { - nodeId(`${fn.file}::${fn.name}:${fn.line}`, fn.name); - for (const callers of Object.values(fn.levels || {})) { - for (const c of callers as Array<{ name: string; file: string; line: number }>) { - nodeId(`${c.file}::${c.name}:${c.line}`, c.name); - } - } - } - - // Collect all edges and determine blast radius - const allEdges = new Set(); - const edgeFromNodes = new Set(); - const edgeToNodes = new Set(); - const changedKeys = new Set(); - - for (const fn of data.affectedFunctions) { - changedKeys.add(`${fn.file}::${fn.name}:${fn.line}`); - for (const edge of fn.edges || []) { - const edgeKey = `${edge.from}|${edge.to}`; - if (!allEdges.has(edgeKey)) { - allEdges.add(edgeKey); - edgeFromNodes.add(edge.from); - edgeToNodes.add(edge.to); - } - } - } - - // Blast radius: caller nodes that are never a source (leaf nodes of the impact tree) - const blastRadiusKeys = new Set(); - for (const key of edgeToNodes) { - if (!edgeFromNodes.has(key) && !changedKeys.has(key)) { - blastRadiusKeys.add(key); - } - } - - // Intermediate callers: not changed, not blast radius - const intermediateKeys = new Set(); - for (const key of edgeToNodes) { - if (!changedKeys.has(key) && !blastRadiusKeys.has(key)) { - intermediateKeys.add(key); - } - } - - // Group changed functions by file - const fileGroups = new Map(); - for (const fn of data.affectedFunctions) { - if (!fileGroups.has(fn.file)) fileGroups.set(fn.file, []); - fileGroups.get(fn.file)!.push(fn); - } - - // Emit changed-file subgraphs - let sgCounter = 0; - for (const [file, fns] of fileGroups) { - const isNew = newFileSet.has(file); - const tag = isNew ? 'new' : 'modified'; - const sgId = `sg${sgCounter++}`; - lines.push(` subgraph ${sgId}["${file} **(${tag})**"]`); - for (const fn of fns) { - const key = `${fn.file}::${fn.name}:${fn.line}`; - lines.push(` ${nodeIdMap.get(key)}["${fn.name}"]`); - } - lines.push(' end'); - const style = isNew ? 'fill:#e8f5e9,stroke:#4caf50' : 'fill:#fff3e0,stroke:#ff9800'; - lines.push(` style ${sgId} ${style}`); - } - - // Emit intermediate caller nodes (outside subgraphs) - for (const key of intermediateKeys) { - lines.push(` ${nodeIdMap.get(key)}["${nodeLabels.get(key)}"]`); - } - - // Emit blast radius subgraph - if (blastRadiusKeys.size > 0) { - const sgId = `sg${sgCounter++}`; - lines.push(` subgraph ${sgId}["Callers **(blast radius)**"]`); - for (const key of blastRadiusKeys) { - lines.push(` ${nodeIdMap.get(key)}["${nodeLabels.get(key)}"]`); - } - lines.push(' end'); - lines.push(` style ${sgId} fill:#f3e5f5,stroke:#9c27b0`); - } - - // Emit edges (impact flows from changed fn toward callers) - for (const edgeKey of allEdges) { - const [from, to] = edgeKey.split('|') as [string, string]; - lines.push(` ${nodeIdMap.get(from)} --> ${nodeIdMap.get(to)}`); - } - return lines.join('\n'); -} +export { diffImpactMermaid } from '../../presentation/diff-impact-mermaid.js'; +export { diffImpactData } from './diff-impact.js'; +export { bfsTransitiveCallers, fnImpactData, impactAnalysisData } from './fn-impact.js'; diff --git a/src/presentation/diff-impact-mermaid.ts b/src/presentation/diff-impact-mermaid.ts new file mode 100644 index 00000000..68fd9999 --- /dev/null +++ b/src/presentation/diff-impact-mermaid.ts @@ -0,0 +1,129 @@ +import { diffImpactData } from '../domain/analysis/diff-impact.js'; + +export function diffImpactMermaid( + customDbPath: string, + opts: { + noTests?: boolean; + depth?: number; + staged?: boolean; + ref?: string; + includeImplementors?: boolean; + limit?: number; + offset?: number; + // biome-ignore lint/suspicious/noExplicitAny: config shape is dynamic + config?: any; + } = {}, +): string { + // biome-ignore lint/suspicious/noExplicitAny: paginateResult returns dynamic shape + const data: any = diffImpactData(customDbPath, opts); + if ('error' in data) return data.error as string; + if (data.changedFiles === 0 || data.affectedFunctions.length === 0) { + return 'flowchart TB\n none["No impacted functions detected"]'; + } + + const newFileSet = new Set(data.newFiles || []); + const lines = ['flowchart TB']; + + // Assign stable Mermaid node IDs + let nodeCounter = 0; + const nodeIdMap = new Map(); + const nodeLabels = new Map(); + function nodeId(key: string, label?: string): string { + if (!nodeIdMap.has(key)) { + nodeIdMap.set(key, `n${nodeCounter++}`); + if (label) nodeLabels.set(key, label); + } + return nodeIdMap.get(key)!; + } + + // Register all nodes (changed functions + their callers) + for (const fn of data.affectedFunctions) { + nodeId(`${fn.file}::${fn.name}:${fn.line}`, fn.name); + for (const callers of Object.values(fn.levels || {})) { + for (const c of callers as Array<{ name: string; file: string; line: number }>) { + nodeId(`${c.file}::${c.name}:${c.line}`, c.name); + } + } + } + + // Collect all edges and determine blast radius + const allEdges = new Set(); + const edgeFromNodes = new Set(); + const edgeToNodes = new Set(); + const changedKeys = new Set(); + + for (const fn of data.affectedFunctions) { + changedKeys.add(`${fn.file}::${fn.name}:${fn.line}`); + for (const edge of fn.edges || []) { + const edgeKey = `${edge.from}|${edge.to}`; + if (!allEdges.has(edgeKey)) { + allEdges.add(edgeKey); + edgeFromNodes.add(edge.from); + edgeToNodes.add(edge.to); + } + } + } + + // Blast radius: caller nodes that are never a source (leaf nodes of the impact tree) + const blastRadiusKeys = new Set(); + for (const key of edgeToNodes) { + if (!edgeFromNodes.has(key) && !changedKeys.has(key)) { + blastRadiusKeys.add(key); + } + } + + // Intermediate callers: not changed, not blast radius + const intermediateKeys = new Set(); + for (const key of edgeToNodes) { + if (!changedKeys.has(key) && !blastRadiusKeys.has(key)) { + intermediateKeys.add(key); + } + } + + // Group changed functions by file + const fileGroups = new Map(); + for (const fn of data.affectedFunctions) { + if (!fileGroups.has(fn.file)) fileGroups.set(fn.file, []); + fileGroups.get(fn.file)!.push(fn); + } + + // Emit changed-file subgraphs + let sgCounter = 0; + for (const [file, fns] of fileGroups) { + const isNew = newFileSet.has(file); + const tag = isNew ? 'new' : 'modified'; + const sgId = `sg${sgCounter++}`; + lines.push(` subgraph ${sgId}["${file} **(${tag})**"]`); + for (const fn of fns) { + const key = `${fn.file}::${fn.name}:${fn.line}`; + lines.push(` ${nodeIdMap.get(key)}["${fn.name}"]`); + } + lines.push(' end'); + const style = isNew ? 'fill:#e8f5e9,stroke:#4caf50' : 'fill:#fff3e0,stroke:#ff9800'; + lines.push(` style ${sgId} ${style}`); + } + + // Emit intermediate caller nodes (outside subgraphs) + for (const key of intermediateKeys) { + lines.push(` ${nodeIdMap.get(key)}["${nodeLabels.get(key)}"]`); + } + + // Emit blast radius subgraph + if (blastRadiusKeys.size > 0) { + const sgId = `sg${sgCounter++}`; + lines.push(` subgraph ${sgId}["Callers **(blast radius)**"]`); + for (const key of blastRadiusKeys) { + lines.push(` ${nodeIdMap.get(key)}["${nodeLabels.get(key)}"]`); + } + lines.push(' end'); + lines.push(` style ${sgId} fill:#f3e5f5,stroke:#9c27b0`); + } + + // Emit edges (impact flows from changed fn toward callers) + for (const edgeKey of allEdges) { + const [from, to] = edgeKey.split('|') as [string, string]; + lines.push(` ${nodeIdMap.get(from)} --> ${nodeIdMap.get(to)}`); + } + + return lines.join('\n'); +} From c859d9bb7c92ad6fb22722c1ef6cd625345668f8 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 23:19:12 -0600 Subject: [PATCH 3/7] refactor: address SLOC warnings in domain and features layers Impact: 2 functions changed, 5 affected --- src/features/complexity-query.ts | 371 +++++++++++++++++++++++++++++ src/features/complexity.ts | 367 +---------------------------- src/features/structure-query.ts | 385 +++++++++++++++++++++++++++++++ src/features/structure.ts | 380 +----------------------------- 4 files changed, 767 insertions(+), 736 deletions(-) create mode 100644 src/features/complexity-query.ts create mode 100644 src/features/structure-query.ts diff --git a/src/features/complexity-query.ts b/src/features/complexity-query.ts new file mode 100644 index 00000000..58de031d --- /dev/null +++ b/src/features/complexity-query.ts @@ -0,0 +1,371 @@ +/** + * Complexity query functions — read-only DB queries for complexity metrics. + * + * Split from complexity.ts to separate query-time concerns (DB reads, filtering, + * pagination) from compute-time concerns (AST traversal, metric algorithms). + */ + +import { openReadonlyOrFail } from '../db/index.js'; +import { buildFileConditionSQL } from '../db/query-builder.js'; +import { loadConfig } from '../infrastructure/config.js'; +import { debug } from '../infrastructure/logger.js'; +import { isTestFile } from '../infrastructure/test-filter.js'; +import { paginateResult } from '../shared/paginate.js'; +import type { CodegraphConfig } from '../types.js'; + +// ─── Query-Time Functions ───────────────────────────────────────────────── + +interface ComplexityRow { + name: string; + kind: string; + file: string; + line: number; + end_line: number | null; + cognitive: number; + cyclomatic: number; + max_nesting: number; + loc: number; + sloc: number; + maintainability_index: number; + halstead_volume: number; + halstead_difficulty: number; + halstead_effort: number; + halstead_bugs: number; +} + +export function complexityData( + customDbPath?: string, + opts: { + target?: string; + limit?: number; + sort?: string; + aboveThreshold?: boolean; + file?: string; + kind?: string; + noTests?: boolean; + config?: CodegraphConfig; + offset?: number; + } = {}, +): Record { + const db = openReadonlyOrFail(customDbPath); + try { + const sort = opts.sort || 'cognitive'; + const noTests = opts.noTests || false; + const aboveThreshold = opts.aboveThreshold || false; + const target = opts.target || null; + const fileFilter = opts.file || null; + const kindFilter = opts.kind || null; + + // Load thresholds from config + const config = opts.config || loadConfig(process.cwd()); + // biome-ignore lint/suspicious/noExplicitAny: thresholds come from config with dynamic keys + const thresholds: any = config.manifesto?.rules || { + cognitive: { warn: 15, fail: null }, + cyclomatic: { warn: 10, fail: null }, + maxNesting: { warn: 4, fail: null }, + maintainabilityIndex: { warn: 20, fail: null }, + }; + + // Build query + let where = "WHERE n.kind IN ('function','method')"; + const params: unknown[] = []; + + if (noTests) { + where += ` AND n.file NOT LIKE '%.test.%' + AND n.file NOT LIKE '%.spec.%' + AND n.file NOT LIKE '%__test__%' + AND n.file NOT LIKE '%__tests__%' + AND n.file NOT LIKE '%.stories.%'`; + } + if (target) { + where += ' AND n.name LIKE ?'; + params.push(`%${target}%`); + } + { + const fc = buildFileConditionSQL(fileFilter as string, 'n.file'); + where += fc.sql; + params.push(...fc.params); + } + if (kindFilter) { + where += ' AND n.kind = ?'; + params.push(kindFilter); + } + + const isValidThreshold = (v: unknown): v is number => + typeof v === 'number' && Number.isFinite(v); + + let having = ''; + if (aboveThreshold) { + const conditions: string[] = []; + if (isValidThreshold(thresholds.cognitive?.warn)) { + conditions.push(`fc.cognitive >= ${thresholds.cognitive.warn}`); + } + if (isValidThreshold(thresholds.cyclomatic?.warn)) { + conditions.push(`fc.cyclomatic >= ${thresholds.cyclomatic.warn}`); + } + if (isValidThreshold(thresholds.maxNesting?.warn)) { + conditions.push(`fc.max_nesting >= ${thresholds.maxNesting.warn}`); + } + if (isValidThreshold(thresholds.maintainabilityIndex?.warn)) { + conditions.push( + `fc.maintainability_index > 0 AND fc.maintainability_index <= ${thresholds.maintainabilityIndex.warn}`, + ); + } + if (conditions.length > 0) { + having = `AND (${conditions.join(' OR ')})`; + } + } + + const orderMap: Record = { + cognitive: 'fc.cognitive DESC', + cyclomatic: 'fc.cyclomatic DESC', + nesting: 'fc.max_nesting DESC', + mi: 'fc.maintainability_index ASC', + volume: 'fc.halstead_volume DESC', + effort: 'fc.halstead_effort DESC', + bugs: 'fc.halstead_bugs DESC', + loc: 'fc.loc DESC', + }; + const orderBy = orderMap[sort] || 'fc.cognitive DESC'; + + let rows: ComplexityRow[]; + try { + rows = db + .prepare( + `SELECT n.name, n.kind, n.file, n.line, n.end_line, + fc.cognitive, fc.cyclomatic, fc.max_nesting, + fc.loc, fc.sloc, fc.maintainability_index, + fc.halstead_volume, fc.halstead_difficulty, fc.halstead_effort, fc.halstead_bugs + FROM function_complexity fc + JOIN nodes n ON fc.node_id = n.id + ${where} ${having} + ORDER BY ${orderBy}`, + ) + .all(...params); + } catch (e: unknown) { + debug(`complexity query failed (table may not exist): ${(e as Error).message}`); + // Check if graph has nodes even though complexity table is missing/empty + let hasGraph = false; + try { + hasGraph = (db.prepare<{ c: number }>('SELECT COUNT(*) as c FROM nodes').get()?.c ?? 0) > 0; + } catch (e2: unknown) { + debug(`nodes table check failed: ${(e2 as Error).message}`); + } + return { functions: [], summary: null, thresholds, hasGraph }; + } + + // Post-filter test files if needed (belt-and-suspenders for isTestFile) + const filtered = noTests ? rows.filter((r) => !isTestFile(r.file)) : rows; + + const functions = filtered.map((r) => { + const exceeds: string[] = []; + if ( + isValidThreshold(thresholds.cognitive?.warn) && + r.cognitive >= (thresholds.cognitive?.warn ?? 0) + ) + exceeds.push('cognitive'); + if ( + isValidThreshold(thresholds.cyclomatic?.warn) && + r.cyclomatic >= (thresholds.cyclomatic?.warn ?? 0) + ) + exceeds.push('cyclomatic'); + if ( + isValidThreshold(thresholds.maxNesting?.warn) && + r.max_nesting >= (thresholds.maxNesting?.warn ?? 0) + ) + exceeds.push('maxNesting'); + if ( + isValidThreshold(thresholds.maintainabilityIndex?.warn) && + r.maintainability_index > 0 && + r.maintainability_index <= (thresholds.maintainabilityIndex?.warn ?? 0) + ) + exceeds.push('maintainabilityIndex'); + + return { + name: r.name, + kind: r.kind, + file: r.file, + line: r.line, + endLine: r.end_line || null, + cognitive: r.cognitive, + cyclomatic: r.cyclomatic, + maxNesting: r.max_nesting, + loc: r.loc || 0, + sloc: r.sloc || 0, + maintainabilityIndex: r.maintainability_index || 0, + halstead: { + volume: r.halstead_volume || 0, + difficulty: r.halstead_difficulty || 0, + effort: r.halstead_effort || 0, + bugs: r.halstead_bugs || 0, + }, + exceeds: exceeds.length > 0 ? exceeds : undefined, + }; + }); + + // Summary stats + let summary: Record | null = null; + try { + const allRows = db + .prepare<{ + cognitive: number; + cyclomatic: number; + max_nesting: number; + maintainability_index: number; + }>( + `SELECT fc.cognitive, fc.cyclomatic, fc.max_nesting, fc.maintainability_index + FROM function_complexity fc JOIN nodes n ON fc.node_id = n.id + WHERE n.kind IN ('function','method') + ${noTests ? `AND n.file NOT LIKE '%.test.%' AND n.file NOT LIKE '%.spec.%' AND n.file NOT LIKE '%__test__%' AND n.file NOT LIKE '%__tests__%' AND n.file NOT LIKE '%.stories.%'` : ''}`, + ) + .all(); + + if (allRows.length > 0) { + const miValues = allRows.map((r) => r.maintainability_index || 0); + summary = { + analyzed: allRows.length, + avgCognitive: +(allRows.reduce((s, r) => s + r.cognitive, 0) / allRows.length).toFixed(1), + avgCyclomatic: +(allRows.reduce((s, r) => s + r.cyclomatic, 0) / allRows.length).toFixed( + 1, + ), + maxCognitive: Math.max(...allRows.map((r) => r.cognitive)), + maxCyclomatic: Math.max(...allRows.map((r) => r.cyclomatic)), + avgMI: +(miValues.reduce((s, v) => s + v, 0) / miValues.length).toFixed(1), + minMI: +Math.min(...miValues).toFixed(1), + aboveWarn: allRows.filter( + (r) => + (isValidThreshold(thresholds.cognitive?.warn) && + r.cognitive >= (thresholds.cognitive?.warn ?? 0)) || + (isValidThreshold(thresholds.cyclomatic?.warn) && + r.cyclomatic >= (thresholds.cyclomatic?.warn ?? 0)) || + (isValidThreshold(thresholds.maxNesting?.warn) && + r.max_nesting >= (thresholds.maxNesting?.warn ?? 0)) || + (isValidThreshold(thresholds.maintainabilityIndex?.warn) && + r.maintainability_index > 0 && + r.maintainability_index <= (thresholds.maintainabilityIndex?.warn ?? 0)), + ).length, + }; + } + } catch (e: unknown) { + debug(`complexity summary query failed: ${(e as Error).message}`); + } + + // When summary is null (no complexity rows), check if graph has nodes + let hasGraph = false; + if (summary === null) { + try { + hasGraph = (db.prepare<{ c: number }>('SELECT COUNT(*) as c FROM nodes').get()?.c ?? 0) > 0; + } catch (e: unknown) { + debug(`nodes table check failed: ${(e as Error).message}`); + } + } + + const base = { functions, summary, thresholds, hasGraph }; + return paginateResult(base, 'functions', { limit: opts.limit, offset: opts.offset }); + } finally { + db.close(); + } +} + +interface IterComplexityRow { + name: string; + kind: string; + file: string; + line: number; + end_line: number | null; + cognitive: number; + cyclomatic: number; + max_nesting: number; + loc: number; + sloc: number; +} + +export function* iterComplexity( + customDbPath?: string, + opts: { + noTests?: boolean; + file?: string; + target?: string; + kind?: string; + sort?: string; + } = {}, +): Generator<{ + name: string; + kind: string; + file: string; + line: number; + endLine: number | null; + cognitive: number; + cyclomatic: number; + maxNesting: number; + loc: number; + sloc: number; +}> { + const db = openReadonlyOrFail(customDbPath); + try { + const noTests = opts.noTests || false; + const sort = opts.sort || 'cognitive'; + + let where = "WHERE n.kind IN ('function','method')"; + const params: unknown[] = []; + + if (noTests) { + where += ` AND n.file NOT LIKE '%.test.%' + AND n.file NOT LIKE '%.spec.%' + AND n.file NOT LIKE '%__test__%' + AND n.file NOT LIKE '%__tests__%' + AND n.file NOT LIKE '%.stories.%'`; + } + if (opts.target) { + where += ' AND n.name LIKE ?'; + params.push(`%${opts.target}%`); + } + { + const fc = buildFileConditionSQL(opts.file as string, 'n.file'); + where += fc.sql; + params.push(...fc.params); + } + if (opts.kind) { + where += ' AND n.kind = ?'; + params.push(opts.kind); + } + + const orderMap: Record = { + cognitive: 'fc.cognitive DESC', + cyclomatic: 'fc.cyclomatic DESC', + nesting: 'fc.max_nesting DESC', + mi: 'fc.maintainability_index ASC', + volume: 'fc.halstead_volume DESC', + effort: 'fc.halstead_effort DESC', + bugs: 'fc.halstead_bugs DESC', + loc: 'fc.loc DESC', + }; + const orderBy = orderMap[sort] || 'fc.cognitive DESC'; + + const stmt = db.prepare( + `SELECT n.name, n.kind, n.file, n.line, n.end_line, + fc.cognitive, fc.cyclomatic, fc.max_nesting, fc.loc, fc.sloc + FROM function_complexity fc + JOIN nodes n ON fc.node_id = n.id + ${where} + ORDER BY ${orderBy}`, + ); + for (const r of stmt.iterate(...params)) { + if (noTests && isTestFile(r.file)) continue; + yield { + name: r.name, + kind: r.kind, + file: r.file, + line: r.line, + endLine: r.end_line || null, + cognitive: r.cognitive, + cyclomatic: r.cyclomatic, + maxNesting: r.max_nesting, + loc: r.loc || 0, + sloc: r.sloc || 0, + }; + } + } finally { + db.close(); + } +} diff --git a/src/features/complexity.ts b/src/features/complexity.ts index 559238d9..b83acfaf 100644 --- a/src/features/complexity.ts +++ b/src/features/complexity.ts @@ -12,15 +12,10 @@ import { } from '../ast-analysis/shared.js'; import { walkWithVisitors } from '../ast-analysis/visitor.js'; import { createComplexityVisitor } from '../ast-analysis/visitors/complexity-visitor.js'; -import { getFunctionNodeId, openReadonlyOrFail } from '../db/index.js'; -import { buildFileConditionSQL } from '../db/query-builder.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { getFunctionNodeId } from '../db/index.js'; import { debug, info } from '../infrastructure/logger.js'; -import { isTestFile } from '../infrastructure/test-filter.js'; -import { paginateResult } from '../shared/paginate.js'; import type { BetterSqlite3Database, - CodegraphConfig, ComplexityRules, HalsteadDerivedMetrics, HalsteadRules, @@ -556,359 +551,7 @@ export async function buildComplexityMetrics( } } -// ─── Query-Time Functions ───────────────────────────────────────────────── - -interface ComplexityRow { - name: string; - kind: string; - file: string; - line: number; - end_line: number | null; - cognitive: number; - cyclomatic: number; - max_nesting: number; - loc: number; - sloc: number; - maintainability_index: number; - halstead_volume: number; - halstead_difficulty: number; - halstead_effort: number; - halstead_bugs: number; -} - -export function complexityData( - customDbPath?: string, - opts: { - target?: string; - limit?: number; - sort?: string; - aboveThreshold?: boolean; - file?: string; - kind?: string; - noTests?: boolean; - config?: CodegraphConfig; - offset?: number; - } = {}, -): Record { - const db = openReadonlyOrFail(customDbPath); - try { - const sort = opts.sort || 'cognitive'; - const noTests = opts.noTests || false; - const aboveThreshold = opts.aboveThreshold || false; - const target = opts.target || null; - const fileFilter = opts.file || null; - const kindFilter = opts.kind || null; - - // Load thresholds from config - const config = opts.config || loadConfig(process.cwd()); - // biome-ignore lint/suspicious/noExplicitAny: thresholds come from config with dynamic keys - const thresholds: any = config.manifesto?.rules || { - cognitive: { warn: 15, fail: null }, - cyclomatic: { warn: 10, fail: null }, - maxNesting: { warn: 4, fail: null }, - maintainabilityIndex: { warn: 20, fail: null }, - }; - - // Build query - let where = "WHERE n.kind IN ('function','method')"; - const params: unknown[] = []; - - if (noTests) { - where += ` AND n.file NOT LIKE '%.test.%' - AND n.file NOT LIKE '%.spec.%' - AND n.file NOT LIKE '%__test__%' - AND n.file NOT LIKE '%__tests__%' - AND n.file NOT LIKE '%.stories.%'`; - } - if (target) { - where += ' AND n.name LIKE ?'; - params.push(`%${target}%`); - } - { - const fc = buildFileConditionSQL(fileFilter as string, 'n.file'); - where += fc.sql; - params.push(...fc.params); - } - if (kindFilter) { - where += ' AND n.kind = ?'; - params.push(kindFilter); - } - - const isValidThreshold = (v: unknown): v is number => - typeof v === 'number' && Number.isFinite(v); - - let having = ''; - if (aboveThreshold) { - const conditions: string[] = []; - if (isValidThreshold(thresholds.cognitive?.warn)) { - conditions.push(`fc.cognitive >= ${thresholds.cognitive.warn}`); - } - if (isValidThreshold(thresholds.cyclomatic?.warn)) { - conditions.push(`fc.cyclomatic >= ${thresholds.cyclomatic.warn}`); - } - if (isValidThreshold(thresholds.maxNesting?.warn)) { - conditions.push(`fc.max_nesting >= ${thresholds.maxNesting.warn}`); - } - if (isValidThreshold(thresholds.maintainabilityIndex?.warn)) { - conditions.push( - `fc.maintainability_index > 0 AND fc.maintainability_index <= ${thresholds.maintainabilityIndex.warn}`, - ); - } - if (conditions.length > 0) { - having = `AND (${conditions.join(' OR ')})`; - } - } - - const orderMap: Record = { - cognitive: 'fc.cognitive DESC', - cyclomatic: 'fc.cyclomatic DESC', - nesting: 'fc.max_nesting DESC', - mi: 'fc.maintainability_index ASC', - volume: 'fc.halstead_volume DESC', - effort: 'fc.halstead_effort DESC', - bugs: 'fc.halstead_bugs DESC', - loc: 'fc.loc DESC', - }; - const orderBy = orderMap[sort] || 'fc.cognitive DESC'; - - let rows: ComplexityRow[]; - try { - rows = db - .prepare( - `SELECT n.name, n.kind, n.file, n.line, n.end_line, - fc.cognitive, fc.cyclomatic, fc.max_nesting, - fc.loc, fc.sloc, fc.maintainability_index, - fc.halstead_volume, fc.halstead_difficulty, fc.halstead_effort, fc.halstead_bugs - FROM function_complexity fc - JOIN nodes n ON fc.node_id = n.id - ${where} ${having} - ORDER BY ${orderBy}`, - ) - .all(...params); - } catch (e: unknown) { - debug(`complexity query failed (table may not exist): ${(e as Error).message}`); - // Check if graph has nodes even though complexity table is missing/empty - let hasGraph = false; - try { - hasGraph = (db.prepare<{ c: number }>('SELECT COUNT(*) as c FROM nodes').get()?.c ?? 0) > 0; - } catch (e2: unknown) { - debug(`nodes table check failed: ${(e2 as Error).message}`); - } - return { functions: [], summary: null, thresholds, hasGraph }; - } - - // Post-filter test files if needed (belt-and-suspenders for isTestFile) - const filtered = noTests ? rows.filter((r) => !isTestFile(r.file)) : rows; - - const functions = filtered.map((r) => { - const exceeds: string[] = []; - if ( - isValidThreshold(thresholds.cognitive?.warn) && - r.cognitive >= (thresholds.cognitive?.warn ?? 0) - ) - exceeds.push('cognitive'); - if ( - isValidThreshold(thresholds.cyclomatic?.warn) && - r.cyclomatic >= (thresholds.cyclomatic?.warn ?? 0) - ) - exceeds.push('cyclomatic'); - if ( - isValidThreshold(thresholds.maxNesting?.warn) && - r.max_nesting >= (thresholds.maxNesting?.warn ?? 0) - ) - exceeds.push('maxNesting'); - if ( - isValidThreshold(thresholds.maintainabilityIndex?.warn) && - r.maintainability_index > 0 && - r.maintainability_index <= (thresholds.maintainabilityIndex?.warn ?? 0) - ) - exceeds.push('maintainabilityIndex'); - - return { - name: r.name, - kind: r.kind, - file: r.file, - line: r.line, - endLine: r.end_line || null, - cognitive: r.cognitive, - cyclomatic: r.cyclomatic, - maxNesting: r.max_nesting, - loc: r.loc || 0, - sloc: r.sloc || 0, - maintainabilityIndex: r.maintainability_index || 0, - halstead: { - volume: r.halstead_volume || 0, - difficulty: r.halstead_difficulty || 0, - effort: r.halstead_effort || 0, - bugs: r.halstead_bugs || 0, - }, - exceeds: exceeds.length > 0 ? exceeds : undefined, - }; - }); - - // Summary stats - let summary: Record | null = null; - try { - const allRows = db - .prepare<{ - cognitive: number; - cyclomatic: number; - max_nesting: number; - maintainability_index: number; - }>( - `SELECT fc.cognitive, fc.cyclomatic, fc.max_nesting, fc.maintainability_index - FROM function_complexity fc JOIN nodes n ON fc.node_id = n.id - WHERE n.kind IN ('function','method') - ${noTests ? `AND n.file NOT LIKE '%.test.%' AND n.file NOT LIKE '%.spec.%' AND n.file NOT LIKE '%__test__%' AND n.file NOT LIKE '%__tests__%' AND n.file NOT LIKE '%.stories.%'` : ''}`, - ) - .all(); - - if (allRows.length > 0) { - const miValues = allRows.map((r) => r.maintainability_index || 0); - summary = { - analyzed: allRows.length, - avgCognitive: +(allRows.reduce((s, r) => s + r.cognitive, 0) / allRows.length).toFixed(1), - avgCyclomatic: +(allRows.reduce((s, r) => s + r.cyclomatic, 0) / allRows.length).toFixed( - 1, - ), - maxCognitive: Math.max(...allRows.map((r) => r.cognitive)), - maxCyclomatic: Math.max(...allRows.map((r) => r.cyclomatic)), - avgMI: +(miValues.reduce((s, v) => s + v, 0) / miValues.length).toFixed(1), - minMI: +Math.min(...miValues).toFixed(1), - aboveWarn: allRows.filter( - (r) => - (isValidThreshold(thresholds.cognitive?.warn) && - r.cognitive >= (thresholds.cognitive?.warn ?? 0)) || - (isValidThreshold(thresholds.cyclomatic?.warn) && - r.cyclomatic >= (thresholds.cyclomatic?.warn ?? 0)) || - (isValidThreshold(thresholds.maxNesting?.warn) && - r.max_nesting >= (thresholds.maxNesting?.warn ?? 0)) || - (isValidThreshold(thresholds.maintainabilityIndex?.warn) && - r.maintainability_index > 0 && - r.maintainability_index <= (thresholds.maintainabilityIndex?.warn ?? 0)), - ).length, - }; - } - } catch (e: unknown) { - debug(`complexity summary query failed: ${(e as Error).message}`); - } - - // When summary is null (no complexity rows), check if graph has nodes - let hasGraph = false; - if (summary === null) { - try { - hasGraph = (db.prepare<{ c: number }>('SELECT COUNT(*) as c FROM nodes').get()?.c ?? 0) > 0; - } catch (e: unknown) { - debug(`nodes table check failed: ${(e as Error).message}`); - } - } - - const base = { functions, summary, thresholds, hasGraph }; - return paginateResult(base, 'functions', { limit: opts.limit, offset: opts.offset }); - } finally { - db.close(); - } -} - -interface IterComplexityRow { - name: string; - kind: string; - file: string; - line: number; - end_line: number | null; - cognitive: number; - cyclomatic: number; - max_nesting: number; - loc: number; - sloc: number; -} - -export function* iterComplexity( - customDbPath?: string, - opts: { - noTests?: boolean; - file?: string; - target?: string; - kind?: string; - sort?: string; - } = {}, -): Generator<{ - name: string; - kind: string; - file: string; - line: number; - endLine: number | null; - cognitive: number; - cyclomatic: number; - maxNesting: number; - loc: number; - sloc: number; -}> { - const db = openReadonlyOrFail(customDbPath); - try { - const noTests = opts.noTests || false; - const sort = opts.sort || 'cognitive'; - - let where = "WHERE n.kind IN ('function','method')"; - const params: unknown[] = []; - - if (noTests) { - where += ` AND n.file NOT LIKE '%.test.%' - AND n.file NOT LIKE '%.spec.%' - AND n.file NOT LIKE '%__test__%' - AND n.file NOT LIKE '%__tests__%' - AND n.file NOT LIKE '%.stories.%'`; - } - if (opts.target) { - where += ' AND n.name LIKE ?'; - params.push(`%${opts.target}%`); - } - { - const fc = buildFileConditionSQL(opts.file as string, 'n.file'); - where += fc.sql; - params.push(...fc.params); - } - if (opts.kind) { - where += ' AND n.kind = ?'; - params.push(opts.kind); - } - - const orderMap: Record = { - cognitive: 'fc.cognitive DESC', - cyclomatic: 'fc.cyclomatic DESC', - nesting: 'fc.max_nesting DESC', - mi: 'fc.maintainability_index ASC', - volume: 'fc.halstead_volume DESC', - effort: 'fc.halstead_effort DESC', - bugs: 'fc.halstead_bugs DESC', - loc: 'fc.loc DESC', - }; - const orderBy = orderMap[sort] || 'fc.cognitive DESC'; - - const stmt = db.prepare( - `SELECT n.name, n.kind, n.file, n.line, n.end_line, - fc.cognitive, fc.cyclomatic, fc.max_nesting, fc.loc, fc.sloc - FROM function_complexity fc - JOIN nodes n ON fc.node_id = n.id - ${where} - ORDER BY ${orderBy}`, - ); - for (const r of stmt.iterate(...params)) { - if (noTests && isTestFile(r.file)) continue; - yield { - name: r.name, - kind: r.kind, - file: r.file, - line: r.line, - endLine: r.end_line || null, - cognitive: r.cognitive, - cyclomatic: r.cyclomatic, - maxNesting: r.max_nesting, - loc: r.loc || 0, - sloc: r.sloc || 0, - }; - } - } finally { - db.close(); - } -} +// ─── Query-Time Functions (re-exported from complexity-query.ts) ────────── +// Split to separate query-time concerns (DB reads, filtering, pagination) +// from compute-time concerns (AST traversal, metric algorithms). +export { complexityData, iterComplexity } from './complexity-query.js'; diff --git a/src/features/structure-query.ts b/src/features/structure-query.ts new file mode 100644 index 00000000..3e2509f4 --- /dev/null +++ b/src/features/structure-query.ts @@ -0,0 +1,385 @@ +/** + * Structure query functions — read-only DB queries for directory structure, + * hotspots, and module boundaries. + * + * Split from structure.ts to separate query-time concerns (DB reads, sorting, + * pagination) from build-time concerns (directory insertion, metrics computation, + * role classification). + */ + +import { openReadonlyOrFail, testFilterSQL } from '../db/index.js'; +import { loadConfig } from '../infrastructure/config.js'; +import { isTestFile } from '../infrastructure/test-filter.js'; +import { normalizePath } from '../shared/constants.js'; +import { paginateResult } from '../shared/paginate.js'; +import type { CodegraphConfig } from '../types.js'; + +// ─── Query functions (read-only) ────────────────────────────────────── + +interface DirRow { + id: number; + name: string; + file: string; + symbol_count: number | null; + fan_in: number | null; + fan_out: number | null; + cohesion: number | null; + file_count: number | null; +} + +interface FileMetricRow { + name: string; + line_count: number | null; + symbol_count: number | null; + import_count: number | null; + export_count: number | null; + fan_in: number | null; + fan_out: number | null; +} + +interface StructureDataOpts { + directory?: string; + depth?: number; + sort?: string; + noTests?: boolean; + full?: boolean; + fileLimit?: number; + limit?: number; + offset?: number; +} + +interface DirectoryEntry { + directory: string; + fileCount: number; + symbolCount: number; + fanIn: number; + fanOut: number; + cohesion: number | null; + density: number; + files: { + file: string; + lineCount: number; + symbolCount: number; + importCount: number; + exportCount: number; + fanIn: number; + fanOut: number; + }[]; + subdirectories: string[]; +} + +export function structureData( + customDbPath?: string, + opts: StructureDataOpts = {}, +): { + directories: DirectoryEntry[]; + count: number; + suppressed?: number; + warning?: string; +} { + const db = openReadonlyOrFail(customDbPath); + try { + const rawDir = opts.directory || null; + const filterDir = rawDir && normalizePath(rawDir) !== '.' ? rawDir : null; + const maxDepth = opts.depth || null; + const sortBy = opts.sort || 'files'; + const noTests = opts.noTests || false; + const full = opts.full || false; + const fileLimit = opts.fileLimit || 25; + + // Get all directory nodes with their metrics + let dirs = db + .prepare(` + SELECT n.id, n.name, n.file, nm.symbol_count, nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n + LEFT JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = 'directory' + `) + .all() as DirRow[]; + + if (filterDir) { + const norm = normalizePath(filterDir); + dirs = dirs.filter((d) => d.name === norm || d.name.startsWith(`${norm}/`)); + } + + if (maxDepth) { + const baseDepth = filterDir ? normalizePath(filterDir).split('/').length : 0; + dirs = dirs.filter((d) => { + const depth = d.name.split('/').length - baseDepth; + return depth <= maxDepth; + }); + } + + // Sort + const sortFn = getSortFn(sortBy); + dirs.sort(sortFn); + + // Get file metrics for each directory + const result: DirectoryEntry[] = dirs.map((d) => { + let files = db + .prepare(` + SELECT n.name, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, nm.fan_in, nm.fan_out + FROM edges e + JOIN nodes n ON e.target_id = n.id + LEFT JOIN node_metrics nm ON n.id = nm.node_id + WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'file' + `) + .all(d.id) as FileMetricRow[]; + if (noTests) files = files.filter((f) => !isTestFile(f.name)); + + const subdirs = db + .prepare(` + SELECT n.name + FROM edges e + JOIN nodes n ON e.target_id = n.id + WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'directory' + `) + .all(d.id) as { name: string }[]; + + const fileCount = noTests ? files.length : d.file_count || 0; + return { + directory: d.name, + fileCount, + symbolCount: d.symbol_count || 0, + fanIn: d.fan_in || 0, + fanOut: d.fan_out || 0, + cohesion: d.cohesion, + density: fileCount > 0 ? (d.symbol_count || 0) / fileCount : 0, + files: files.map((f) => ({ + file: f.name, + lineCount: f.line_count || 0, + symbolCount: f.symbol_count || 0, + importCount: f.import_count || 0, + exportCount: f.export_count || 0, + fanIn: f.fan_in || 0, + fanOut: f.fan_out || 0, + })), + subdirectories: subdirs.map((s) => s.name), + }; + }); + + // Apply global file limit unless full mode + if (!full) { + const totalFiles = result.reduce((sum, d) => sum + d.files.length, 0); + if (totalFiles > fileLimit) { + let shown = 0; + for (const d of result) { + const remaining = fileLimit - shown; + if (remaining <= 0) { + d.files = []; + } else if (d.files.length > remaining) { + d.files = d.files.slice(0, remaining); + shown = fileLimit; + } else { + shown += d.files.length; + } + } + const suppressed = totalFiles - fileLimit; + return { + directories: result, + count: result.length, + suppressed, + warning: `${suppressed} files omitted (showing ${fileLimit}/${totalFiles}). Use --full to show all files, or narrow with --directory.`, + }; + } + } + + const base = { directories: result, count: result.length }; + return paginateResult(base, 'directories', { limit: opts.limit, offset: opts.offset }); + } finally { + db.close(); + } +} + +interface HotspotRow { + name: string; + kind: string; + line_count: number | null; + symbol_count: number | null; + import_count: number | null; + export_count: number | null; + fan_in: number | null; + fan_out: number | null; + cohesion: number | null; + file_count: number | null; +} + +interface HotspotsDataOpts { + metric?: string; + level?: string; + limit?: number; + offset?: number; + noTests?: boolean; +} + +export function hotspotsData( + customDbPath?: string, + opts: HotspotsDataOpts = {}, +): { + metric: string; + level: string; + limit: number; + hotspots: unknown[]; +} { + const db = openReadonlyOrFail(customDbPath); + try { + const metric = opts.metric || 'fan-in'; + const level = opts.level || 'file'; + const limit = opts.limit || 10; + const noTests = opts.noTests || false; + + const kind = level === 'directory' ? 'directory' : 'file'; + + const testFilter = testFilterSQL('n.name', noTests && kind === 'file'); + + const HOTSPOT_QUERIES: Record = { + 'fan-in': db.prepare(` + SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, + nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = ? ${testFilter} ORDER BY nm.fan_in DESC NULLS LAST LIMIT ?`), + 'fan-out': db.prepare(` + SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, + nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = ? ${testFilter} ORDER BY nm.fan_out DESC NULLS LAST LIMIT ?`), + density: db.prepare(` + SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, + nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = ? ${testFilter} ORDER BY nm.symbol_count DESC NULLS LAST LIMIT ?`), + coupling: db.prepare(` + SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, + nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = ? ${testFilter} ORDER BY (COALESCE(nm.fan_in, 0) + COALESCE(nm.fan_out, 0)) DESC NULLS LAST LIMIT ?`), + }; + + const stmt = HOTSPOT_QUERIES[metric] ?? HOTSPOT_QUERIES['fan-in']; + // stmt is always defined: metric is a valid key or the fallback is a concrete property + const rows = stmt!.all(kind, limit); + + const hotspots = rows.map((r) => ({ + name: r.name, + kind: r.kind, + lineCount: r.line_count, + symbolCount: r.symbol_count, + importCount: r.import_count, + exportCount: r.export_count, + fanIn: r.fan_in, + fanOut: r.fan_out, + cohesion: r.cohesion, + fileCount: r.file_count, + density: + (r.file_count ?? 0) > 0 + ? (r.symbol_count || 0) / (r.file_count ?? 1) + : (r.line_count ?? 0) > 0 + ? (r.symbol_count || 0) / (r.line_count ?? 1) + : 0, + coupling: (r.fan_in || 0) + (r.fan_out || 0), + })); + + const base = { metric, level, limit, hotspots }; + return paginateResult(base, 'hotspots', { limit: opts.limit, offset: opts.offset }); + } finally { + db.close(); + } +} + +interface ModuleBoundariesOpts { + threshold?: number; + config?: CodegraphConfig; +} + +export function moduleBoundariesData( + customDbPath?: string, + opts: ModuleBoundariesOpts = {}, +): { + threshold: number; + modules: { + directory: string; + cohesion: number | null; + fileCount: number; + symbolCount: number; + fanIn: number; + fanOut: number; + files: string[]; + }[]; + count: number; +} { + const db = openReadonlyOrFail(customDbPath); + try { + const config = opts.config || loadConfig(); + const threshold = + opts.threshold ?? + (config as unknown as { structure?: { cohesionThreshold?: number } }).structure + ?.cohesionThreshold ?? + 0.3; + + const dirs = db + .prepare(` + SELECT n.id, n.name, nm.symbol_count, nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count + FROM nodes n + JOIN node_metrics nm ON n.id = nm.node_id + WHERE n.kind = 'directory' AND nm.cohesion IS NOT NULL AND nm.cohesion >= ? + ORDER BY nm.cohesion DESC + `) + .all(threshold) as { + id: number; + name: string; + symbol_count: number | null; + fan_in: number | null; + fan_out: number | null; + cohesion: number | null; + file_count: number | null; + }[]; + + const modules = dirs.map((d) => { + // Get files inside this directory + const files = ( + db + .prepare(` + SELECT n.name FROM edges e + JOIN nodes n ON e.target_id = n.id + WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'file' + `) + .all(d.id) as { name: string }[] + ).map((f) => f.name); + + return { + directory: d.name, + cohesion: d.cohesion, + fileCount: d.file_count || 0, + symbolCount: d.symbol_count || 0, + fanIn: d.fan_in || 0, + fanOut: d.fan_out || 0, + files, + }; + }); + + return { threshold, modules, count: modules.length }; + } finally { + db.close(); + } +} + +// ─── Helpers ────────────────────────────────────────────────────────── + +function getSortFn(sortBy: string): (a: DirRow, b: DirRow) => number { + switch (sortBy) { + case 'cohesion': + return (a, b) => (b.cohesion ?? -1) - (a.cohesion ?? -1); + case 'fan-in': + return (a, b) => (b.fan_in || 0) - (a.fan_in || 0); + case 'fan-out': + return (a, b) => (b.fan_out || 0) - (a.fan_out || 0); + case 'density': + return (a, b) => { + const da = (a.file_count ?? 0) > 0 ? (a.symbol_count || 0) / (a.file_count ?? 1) : 0; + const db_ = (b.file_count ?? 0) > 0 ? (b.symbol_count || 0) / (b.file_count ?? 1) : 0; + return db_ - da; + }; + default: + return (a, b) => a.name.localeCompare(b.name); + } +} diff --git a/src/features/structure.ts b/src/features/structure.ts index 9976907f..ec57dfd5 100644 --- a/src/features/structure.ts +++ b/src/features/structure.ts @@ -1,11 +1,8 @@ import path from 'node:path'; -import { getNodeId, openReadonlyOrFail, testFilterSQL } from '../db/index.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { getNodeId, testFilterSQL } from '../db/index.js'; import { debug } from '../infrastructure/logger.js'; -import { isTestFile } from '../infrastructure/test-filter.js'; import { normalizePath } from '../shared/constants.js'; -import { paginateResult } from '../shared/paginate.js'; -import type { BetterSqlite3Database, CodegraphConfig } from '../types.js'; +import type { BetterSqlite3Database } from '../types.js'; // ─── Build-time helpers ─────────────────────────────────────────────── @@ -508,372 +505,7 @@ export function classifyNodeRoles(db: BetterSqlite3Database): RoleSummary { return summary; } -// ─── Query functions (read-only) ────────────────────────────────────── - -interface DirRow { - id: number; - name: string; - file: string; - symbol_count: number | null; - fan_in: number | null; - fan_out: number | null; - cohesion: number | null; - file_count: number | null; -} - -interface FileMetricRow { - name: string; - line_count: number | null; - symbol_count: number | null; - import_count: number | null; - export_count: number | null; - fan_in: number | null; - fan_out: number | null; -} - -interface StructureDataOpts { - directory?: string; - depth?: number; - sort?: string; - noTests?: boolean; - full?: boolean; - fileLimit?: number; - limit?: number; - offset?: number; -} - -interface DirectoryEntry { - directory: string; - fileCount: number; - symbolCount: number; - fanIn: number; - fanOut: number; - cohesion: number | null; - density: number; - files: { - file: string; - lineCount: number; - symbolCount: number; - importCount: number; - exportCount: number; - fanIn: number; - fanOut: number; - }[]; - subdirectories: string[]; -} - -export function structureData( - customDbPath?: string, - opts: StructureDataOpts = {}, -): { - directories: DirectoryEntry[]; - count: number; - suppressed?: number; - warning?: string; -} { - const db = openReadonlyOrFail(customDbPath); - try { - const rawDir = opts.directory || null; - const filterDir = rawDir && normalizePath(rawDir) !== '.' ? rawDir : null; - const maxDepth = opts.depth || null; - const sortBy = opts.sort || 'files'; - const noTests = opts.noTests || false; - const full = opts.full || false; - const fileLimit = opts.fileLimit || 25; - - // Get all directory nodes with their metrics - let dirs = db - .prepare(` - SELECT n.id, n.name, n.file, nm.symbol_count, nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n - LEFT JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = 'directory' - `) - .all() as DirRow[]; - - if (filterDir) { - const norm = normalizePath(filterDir); - dirs = dirs.filter((d) => d.name === norm || d.name.startsWith(`${norm}/`)); - } - - if (maxDepth) { - const baseDepth = filterDir ? normalizePath(filterDir).split('/').length : 0; - dirs = dirs.filter((d) => { - const depth = d.name.split('/').length - baseDepth; - return depth <= maxDepth; - }); - } - - // Sort - const sortFn = getSortFn(sortBy); - dirs.sort(sortFn); - - // Get file metrics for each directory - const result: DirectoryEntry[] = dirs.map((d) => { - let files = db - .prepare(` - SELECT n.name, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, nm.fan_in, nm.fan_out - FROM edges e - JOIN nodes n ON e.target_id = n.id - LEFT JOIN node_metrics nm ON n.id = nm.node_id - WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'file' - `) - .all(d.id) as FileMetricRow[]; - if (noTests) files = files.filter((f) => !isTestFile(f.name)); - - const subdirs = db - .prepare(` - SELECT n.name - FROM edges e - JOIN nodes n ON e.target_id = n.id - WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'directory' - `) - .all(d.id) as { name: string }[]; - - const fileCount = noTests ? files.length : d.file_count || 0; - return { - directory: d.name, - fileCount, - symbolCount: d.symbol_count || 0, - fanIn: d.fan_in || 0, - fanOut: d.fan_out || 0, - cohesion: d.cohesion, - density: fileCount > 0 ? (d.symbol_count || 0) / fileCount : 0, - files: files.map((f) => ({ - file: f.name, - lineCount: f.line_count || 0, - symbolCount: f.symbol_count || 0, - importCount: f.import_count || 0, - exportCount: f.export_count || 0, - fanIn: f.fan_in || 0, - fanOut: f.fan_out || 0, - })), - subdirectories: subdirs.map((s) => s.name), - }; - }); - - // Apply global file limit unless full mode - if (!full) { - const totalFiles = result.reduce((sum, d) => sum + d.files.length, 0); - if (totalFiles > fileLimit) { - let shown = 0; - for (const d of result) { - const remaining = fileLimit - shown; - if (remaining <= 0) { - d.files = []; - } else if (d.files.length > remaining) { - d.files = d.files.slice(0, remaining); - shown = fileLimit; - } else { - shown += d.files.length; - } - } - const suppressed = totalFiles - fileLimit; - return { - directories: result, - count: result.length, - suppressed, - warning: `${suppressed} files omitted (showing ${fileLimit}/${totalFiles}). Use --full to show all files, or narrow with --directory.`, - }; - } - } - - const base = { directories: result, count: result.length }; - return paginateResult(base, 'directories', { limit: opts.limit, offset: opts.offset }); - } finally { - db.close(); - } -} - -interface HotspotRow { - name: string; - kind: string; - line_count: number | null; - symbol_count: number | null; - import_count: number | null; - export_count: number | null; - fan_in: number | null; - fan_out: number | null; - cohesion: number | null; - file_count: number | null; -} - -interface HotspotsDataOpts { - metric?: string; - level?: string; - limit?: number; - offset?: number; - noTests?: boolean; -} - -export function hotspotsData( - customDbPath?: string, - opts: HotspotsDataOpts = {}, -): { - metric: string; - level: string; - limit: number; - hotspots: unknown[]; -} { - const db = openReadonlyOrFail(customDbPath); - try { - const metric = opts.metric || 'fan-in'; - const level = opts.level || 'file'; - const limit = opts.limit || 10; - const noTests = opts.noTests || false; - - const kind = level === 'directory' ? 'directory' : 'file'; - - const testFilter = testFilterSQL('n.name', noTests && kind === 'file'); - - const HOTSPOT_QUERIES: Record = { - 'fan-in': db.prepare(` - SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, - nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = ? ${testFilter} ORDER BY nm.fan_in DESC NULLS LAST LIMIT ?`), - 'fan-out': db.prepare(` - SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, - nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = ? ${testFilter} ORDER BY nm.fan_out DESC NULLS LAST LIMIT ?`), - density: db.prepare(` - SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, - nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = ? ${testFilter} ORDER BY nm.symbol_count DESC NULLS LAST LIMIT ?`), - coupling: db.prepare(` - SELECT n.name, n.kind, nm.line_count, nm.symbol_count, nm.import_count, nm.export_count, - nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = ? ${testFilter} ORDER BY (COALESCE(nm.fan_in, 0) + COALESCE(nm.fan_out, 0)) DESC NULLS LAST LIMIT ?`), - }; - - const stmt = HOTSPOT_QUERIES[metric] ?? HOTSPOT_QUERIES['fan-in']; - // stmt is always defined: metric is a valid key or the fallback is a concrete property - const rows = stmt!.all(kind, limit); - - const hotspots = rows.map((r) => ({ - name: r.name, - kind: r.kind, - lineCount: r.line_count, - symbolCount: r.symbol_count, - importCount: r.import_count, - exportCount: r.export_count, - fanIn: r.fan_in, - fanOut: r.fan_out, - cohesion: r.cohesion, - fileCount: r.file_count, - density: - (r.file_count ?? 0) > 0 - ? (r.symbol_count || 0) / (r.file_count ?? 1) - : (r.line_count ?? 0) > 0 - ? (r.symbol_count || 0) / (r.line_count ?? 1) - : 0, - coupling: (r.fan_in || 0) + (r.fan_out || 0), - })); - - const base = { metric, level, limit, hotspots }; - return paginateResult(base, 'hotspots', { limit: opts.limit, offset: opts.offset }); - } finally { - db.close(); - } -} - -interface ModuleBoundariesOpts { - threshold?: number; - config?: CodegraphConfig; -} - -export function moduleBoundariesData( - customDbPath?: string, - opts: ModuleBoundariesOpts = {}, -): { - threshold: number; - modules: { - directory: string; - cohesion: number | null; - fileCount: number; - symbolCount: number; - fanIn: number; - fanOut: number; - files: string[]; - }[]; - count: number; -} { - const db = openReadonlyOrFail(customDbPath); - try { - const config = opts.config || loadConfig(); - const threshold = - opts.threshold ?? - (config as unknown as { structure?: { cohesionThreshold?: number } }).structure - ?.cohesionThreshold ?? - 0.3; - - const dirs = db - .prepare(` - SELECT n.id, n.name, nm.symbol_count, nm.fan_in, nm.fan_out, nm.cohesion, nm.file_count - FROM nodes n - JOIN node_metrics nm ON n.id = nm.node_id - WHERE n.kind = 'directory' AND nm.cohesion IS NOT NULL AND nm.cohesion >= ? - ORDER BY nm.cohesion DESC - `) - .all(threshold) as { - id: number; - name: string; - symbol_count: number | null; - fan_in: number | null; - fan_out: number | null; - cohesion: number | null; - file_count: number | null; - }[]; - - const modules = dirs.map((d) => { - // Get files inside this directory - const files = ( - db - .prepare(` - SELECT n.name FROM edges e - JOIN nodes n ON e.target_id = n.id - WHERE e.source_id = ? AND e.kind = 'contains' AND n.kind = 'file' - `) - .all(d.id) as { name: string }[] - ).map((f) => f.name); - - return { - directory: d.name, - cohesion: d.cohesion, - fileCount: d.file_count || 0, - symbolCount: d.symbol_count || 0, - fanIn: d.fan_in || 0, - fanOut: d.fan_out || 0, - files, - }; - }); - - return { threshold, modules, count: modules.length }; - } finally { - db.close(); - } -} - -// ─── Helpers ────────────────────────────────────────────────────────── - -function getSortFn(sortBy: string): (a: DirRow, b: DirRow) => number { - switch (sortBy) { - case 'cohesion': - return (a, b) => (b.cohesion ?? -1) - (a.cohesion ?? -1); - case 'fan-in': - return (a, b) => (b.fan_in || 0) - (a.fan_in || 0); - case 'fan-out': - return (a, b) => (b.fan_out || 0) - (a.fan_out || 0); - case 'density': - return (a, b) => { - const da = (a.file_count ?? 0) > 0 ? (a.symbol_count || 0) / (a.file_count ?? 1) : 0; - const db_ = (b.file_count ?? 0) > 0 ? (b.symbol_count || 0) / (b.file_count ?? 1) : 0; - return db_ - da; - }; - default: - return (a, b) => a.name.localeCompare(b.name); - } -} +// ─── Query functions (re-exported from structure-query.ts) ──────────── +// Split to separate query-time concerns (DB reads, sorting, pagination) +// from build-time concerns (directory insertion, metrics computation, role classification). +export { hotspotsData, moduleBoundariesData, structureData } from './structure-query.js'; From ca30a5b4def2d00991b4af8562d62574f3010d89 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 00:04:14 -0600 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20remove=20domain=E2=86=92presentation?= =?UTF-8?q?=20layer=20inversion=20in=20impact=20barrel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the diffImpactMermaid re-export from domain/analysis/impact.ts (which created a domain→presentation dependency inversion). Instead, re-export it directly from domain/queries.ts → presentation/diff-impact-mermaid.ts. All callers already consume through domain/queries.ts, so this is transparent. --- src/domain/analysis/impact.ts | 1 - src/domain/queries.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/domain/analysis/impact.ts b/src/domain/analysis/impact.ts index 5cd25214..933178fb 100644 --- a/src/domain/analysis/impact.ts +++ b/src/domain/analysis/impact.ts @@ -7,6 +7,5 @@ * - presentation/diff-impact-mermaid.ts: diffImpactMermaid (Mermaid diagram generation) */ -export { diffImpactMermaid } from '../../presentation/diff-impact-mermaid.js'; export { diffImpactData } from './diff-impact.js'; export { bfsTransitiveCallers, fnImpactData, impactAnalysisData } from './fn-impact.js'; diff --git a/src/domain/queries.ts b/src/domain/queries.ts index b35ad981..e7db56fb 100644 --- a/src/domain/queries.ts +++ b/src/domain/queries.ts @@ -28,10 +28,10 @@ export { fileDepsData, fnDepsData, pathData } from './analysis/dependencies.js'; export { exportsData } from './analysis/exports.js'; export { diffImpactData, - diffImpactMermaid, fnImpactData, impactAnalysisData, } from './analysis/impact.js'; +export { diffImpactMermaid } from '../presentation/diff-impact-mermaid.js'; export { implementationsData, interfacesData } from './analysis/implementations.js'; export { FALSE_POSITIVE_CALLER_THRESHOLD, From a66ab961d462b9e9fe615df92c554d5c094fc34e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 00:04:28 -0600 Subject: [PATCH 5/7] fix: add missing 'files' case to getSortFn in structure-query The default sortBy value is 'files' but getSortFn had no matching case, causing it to silently fall through to alphabetical sort. Add explicit 'files' case that sorts by file_count descending. Impact: 1 functions changed, 4 affected --- src/features/structure-query.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/features/structure-query.ts b/src/features/structure-query.ts index 3e2509f4..dfec638c 100644 --- a/src/features/structure-query.ts +++ b/src/features/structure-query.ts @@ -379,6 +379,8 @@ function getSortFn(sortBy: string): (a: DirRow, b: DirRow) => number { const db_ = (b.file_count ?? 0) > 0 ? (b.symbol_count || 0) / (b.file_count ?? 1) : 0; return db_ - da; }; + case 'files': + return (a, b) => (b.file_count || 0) - (a.file_count || 0); default: return (a, b) => a.name.localeCompare(b.name); } From 1ee03d723bdcce753405bfee1e9faa9d5df543ec Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 01:25:34 -0600 Subject: [PATCH 6/7] fix: sort imports in queries.ts after merge (#621) --- src/domain/queries.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/queries.ts b/src/domain/queries.ts index e7db56fb..aae22eaf 100644 --- a/src/domain/queries.ts +++ b/src/domain/queries.ts @@ -8,6 +8,7 @@ // ── Re-export from dedicated module for backward compat ─────────────────── export { isTestFile, TEST_PATTERN } from '../infrastructure/test-filter.js'; +export { diffImpactMermaid } from '../presentation/diff-impact-mermaid.js'; export { iterListFunctions, iterRoles, iterWhere } from '../shared/generators.js'; // ── Kind/edge constants (canonical source: kinds.js) ───────────────────── export { @@ -31,7 +32,6 @@ export { fnImpactData, impactAnalysisData, } from './analysis/impact.js'; -export { diffImpactMermaid } from '../presentation/diff-impact-mermaid.js'; export { implementationsData, interfacesData } from './analysis/implementations.js'; export { FALSE_POSITIVE_CALLER_THRESHOLD, From 03fdaccfca5434a812f46067182015613537f908 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 01:25:47 -0600 Subject: [PATCH 7/7] fix: suppress noExplicitAny lint error in visitor.test.ts (#621) --- tests/unit/visitor.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/visitor.test.ts b/tests/unit/visitor.test.ts index 62dc5b3e..992b4307 100644 --- a/tests/unit/visitor.test.ts +++ b/tests/unit/visitor.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest'; // We need a tree-sitter tree to test. Use the JS parser. +// biome-ignore lint/suspicious/noExplicitAny: tree-sitter parser type is complex and not worth typing for tests let parse: any; async function ensureParser() {