refactor: split presentation formatting and extract CLI/MCP dispatch#493
refactor: split presentation formatting and extract CLI/MCP dispatch#493carlos-alm merged 27 commits intomainfrom
Conversation
- Remove dead `truncate` function from ast-analysis/shared.js (0 consumers) - Remove dead `truncStart` function from presentation/table.js (0 consumers) - Un-export `BATCH_CHUNK` in builder/helpers.js (only used internally) Skipped sync.json targets that were false positives: - BUILTIN_RECEIVERS: used by incremental.js + build-edges.js - TRANSIENT_CODES/RETRY_DELAY_MS: internal to readFileSafe - MAX_COL_WIDTH: internal to printAutoTable - findFunctionNode: re-exported from index.js, used in tests Impact: 1 functions changed, 32 affected
Impact: 29 functions changed, 105 affected
…ures Impact: 5 functions changed, 7 affected
connection.js: add debug() logging to all 8 catch-with-fallback blocks so failures are observable without changing behavior. migrations.js: replace 14 try/catch blocks in initSchema with hasColumn() and hasTable() guards. CREATE INDEX calls use IF NOT EXISTS directly. getBuildMeta uses hasTable() check instead of try/catch. Impact: 10 functions changed, 19 affected
Add debug() logging to 10 empty catch blocks across context.js, symbol-lookup.js, exports.js, impact.js, and module-map.js. All catches retain their fallback behavior but failures are now observable via debug logging. Impact: 6 functions changed, 18 affected
Add debug() logging to 6 empty catch blocks: 3 in disposeParsers() for WASM resource cleanup, 2 in ensureWasmTrees() for file read and parse failures, and 1 in getActiveEngine() for version lookup. Impact: 3 functions changed, 0 affected
Add debug() logging to 9 empty catch blocks across complexity.js (5), cfg.js (2), and dataflow.js (2). All catches for file read and parse failures now log the error message before continuing. Impact: 4 functions changed, 2 affected
Split the monolithic walkJavaScriptNode switch (13 cases, cognitive 228) into 11 focused handler functions. The dispatcher is now a thin switch that delegates to handleFunctionDecl, handleClassDecl, handleMethodDef, handleInterfaceDecl, handleTypeAliasDecl, handleVariableDecl, handleEnumDecl, handleCallExpr, handleImportStmt, handleExportStmt, and handleExpressionStmt. The expression_statement case now reuses the existing handleCommonJSAssignment helper, eliminating ~50 lines of duplication. Worst handler complexity: handleVariableDecl (cognitive 20), down from the original monolithic function (cognitive 279). Impact: 13 functions changed, 3 affected
Split walkPythonNode switch into 7 focused handlers: handlePyFunctionDef, handlePyClassDef, handlePyCall, handlePyImport, handlePyExpressionStmt, handlePyImportFrom, plus the decorated_definition inline dispatch. Moved extractPythonParameters, extractPythonClassProperties, walkInitBody, and findPythonParentClass from closures to module-scope functions. Impact: 12 functions changed, 5 affected
Split walkJavaNode switch into 8 focused handlers plus an extractJavaInterfaces helper. Moved findJavaParentClass to module scope. The class_declaration case (deepest nesting in the file) is now split between handleJavaClassDecl and extractJavaInterfaces. Impact: 12 functions changed, 5 affected
Apply the same per-category handler decomposition to all remaining language extractors: Go (6 handlers), Ruby (8 handlers), PHP (11 handlers), C# (11 handlers), Rust (9 handlers), HCL (4 handlers). Each extractor now follows the template established by the JS extractor: - Thin entry function creates ctx, delegates to walkXNode - walkXNode is a thin dispatcher switch - Each case is a named handler function at module scope - Helper functions (findParentClass, etc.) moved to module scope Impact: 66 functions changed, 23 affected
…pers Move nested handler functions to module level in cfg-visitor.js, dataflow-visitor.js, and complexity-visitor.js — reducing cognitive complexity of each factory function from 100-337 down to thin coordinators. Extract WASM pre-parse, visitor setup, result storage, and build delegation from runAnalyses into focused helper functions. Impact: 66 functions changed, 43 affected
Extract edge-building by type (import, call-native, call-JS, class hierarchy) from buildEdges. Extract per-phase insertion logic from insertNodes. Extract scoped/incremental/full-build paths and reverse-dep cascade from detectChanges. Extract setup, engine init, alias loading from pipeline.js. Extract node/edge-building helpers from incremental.js rebuildFile. Impact: 44 functions changed, 19 affected
Impact: 37 functions changed, 29 affected
Impact: 5 functions changed, 3 affected
…sification Impact: 8 functions changed, 3 affected
Impact: 10 functions changed, 5 affected
Impact: 5 functions changed, 2 affected
Impact: 5 functions changed, 2 affected
Impact: 12 functions changed, 6 affected
…age) Extract per-section validators from validateBoundaryConfig (cog 101→2). Extract buildCommunityObjects and analyzeDrift from communitiesData (cog 32→4). Extract buildTriageItems and computeTriageSummary from triageData (bugs 1.4→0.48). Impact: 13 functions changed, 11 affected
Extract printDiffFunctions/Coupled/Ownership/Boundaries/Summary from diffImpact (cog 28→6, cyc 21→7). Extract printExportHeader/Symbols from fileExports. Extract printNotFound/PathSteps from symbolPath. Impact: 12 functions changed, 7 affected
Extract runManifesto/validateKind from check execute (cyc 14→10). Extract runHotspots/validateFilters/parseWeights from triage execute (cyc 13→4). Extract loadMCPSdk/createLazyLoaders/resolveDbPath/validateMultiRepoAccess from startMCPServer (cog 34→13, cyc 19→7). Impact: 14 functions changed, 4 affected
|
Claude finished @carlos-alm's task —— View job Code Review CompleteConducted a comprehensive review of PR #493 following the 5-phase analysis process. Root Cause AnalysisProblem Being Solved: This PR addresses excessive cyclomatic complexity in presentation layer functions by extracting smaller, focused functions from larger ones. Specific complexity reductions achieved: Why This Approach: Function extraction is the correct approach for reducing complexity while maintaining behavior. The extracted functions have clear, single responsibilities and are logically separated. Risk Assessment: Low risk - purely organizational refactoring with no behavioral changes. Backlog Compliance
Code Quality Assessment✅ Excellent refactoring execution:
Minor observation: While the refactoring is excellent, some extracted functions are quite small (e.g., Final Recommendation
|
Greptile SummaryThis PR is the fifth phase of the Titan quality sweep, decomposing the presentation layer and CLI/MCP entry points into smaller, focused helper functions. The primary goal is reducing cyclomatic and cognitive complexity across Key changes:
Notable concern: In Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[startMCPServer] --> B[loadMCPSdk]
A --> C[createLazyLoaders]
A --> D[server.setRequestHandler - CallTool]
D --> E[validateMultiRepoAccess]
D --> F[resolveDbPath]
F --> G{args.repo?}
G -- yes --> H[resolveRepoDbPath]
G -- no --> I[use customDbPath]
D --> J[TOOL_HANDLERS.get - name]
J --> K[toolEntry.handler]
C --> L[getQueries - lazy]
C --> M[getDatabase - lazy]
subgraph CLI
N[check execute] --> O[runManifesto]
O --> P[validateKind]
O --> Q[manifesto]
R[triage execute] --> S{level?}
S -- file/dir --> T[runHotspots]
S -- function --> U[validateFilters]
U --> V[parseWeights]
V --> W[triage]
end
subgraph Presentation
X[diffImpact] --> X1[printDiffFunctions]
X --> X2[printDiffCoupled]
X --> X3[printDiffOwnership]
X --> X4[printDiffBoundaries]
X --> X5[printDiffSummary]
Y[fileExports] --> Y1[printExportHeader]
Y --> Y2[printExportSymbols]
Z[symbolPath] --> Z1[printNotFound]
Z --> Z2[printPathSteps]
end
|
|
Addressed the misplaced JSDoc issue: fixed in fc721f3 — moved the startMCPServer JSDoc block from above loadMCPSdk() to directly above export async function startMCPServer(), where it belongs. |
Move createRequire assignment after all import declarations to satisfy Biome's import sorting rule. Impact: 3 functions changed, 1 affected
|
Addressed both Greptile observations from the latest review:
|
1 similar comment
Impact: 28 functions changed, 35 affected
Summary
Titan quality sweep — phase 5c + 6.2–6.3: presentation layer and CLI/MCP entry points.
Presentation
explain()→ extract section renderers (header, deps, callers, source)stats()→ extract section printers (graph, quality, community, complexity)diffImpact(cyc 21→7) → extract printDiffFunctions/Coupled/Ownership/Boundaries/SummaryfileExports→ extract header and symbol formattingsymbolPath→ extract not-found and path-step printingCLI commands
execute(check.js) (cyc 14→10) → extractrunManifestoandvalidateKindexecute(triage.js) (cyc 13→4) → extractrunHotspots,validateFilters,parseWeightsMCP server
startMCPServer(cog 34→13, cyc 19→7) → extractloadMCPSdk,createLazyLoaders,resolveDbPath,validateMultiRepoAccessStacked on
Files changed (8)
src/presentation/queries-cli/inspect.js,overview.js,impact.js,exports.js,path.jssrc/cli/commands/check.js,triage.jssrc/mcp/server.jsTest plan