feat: extract CLI wrappers into src/commands/ directory (Phase 3.2)#393
feat: extract CLI wrappers into src/commands/ directory (Phase 3.2)#393carlos-alm merged 2 commits intomainfrom
Conversation
Separate CLI display logic from data functions across 15 analysis modules. Each command now lives in src/commands/<name>.js while *Data() functions remain in their original modules (preserving MCP dynamic imports). - Create 16 command files in src/commands/ (audit, batch, cfg, check, cochange, communities, complexity, dataflow, flow, branch-compare, manifesto, owners, sequence, structure, triage, query barrel) - Add shared CommandRunner lifecycle in src/infrastructure/command-runner.js - Move result-formatter.js and test-filter.js to src/infrastructure/ - Update all imports in cli.js, index.js, queries-cli.js, and 7 other modules - Remove ~1,059 lines of CLI wrapper code from original analysis modules Impact: 33 functions changed, 19 affected
|
Claude finished @carlos-alm's task —— View job Root Cause AnalysisProblem Being Solved: This PR implements ROADMAP section 3.2 "Command/Query Separation" - addressing a systematic architectural debt issue across the codebase. The dual-function anti-pattern was replicated across 19 analysis modules where each module implemented both data extraction ( Why This Approach:
Risk Assessment: Low risk. This is legitimate architectural refactoring addressing a documented design issue, not symptom hiding. Critical Concerns (if any)None detected. This is an exemplary refactoring: ✅ Configuration integrity: No linting rules disabled, no build config changes, no test thresholds lowered Architecture benefits:
Implementation quality:
This represents the gold standard for large-scale refactoring: systematic, test-verified, backward-compatible separation of concerns. Final Recommendation
|
Greptile SummaryThis PR completes ROADMAP section 3.2 by extracting ~1,059 lines of CLI display code from 15 analysis modules into dedicated Key observations:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CLI["src/cli.js\n(dynamic imports)"]
IDX["src/index.js\n(public re-exports)"]
subgraph commands["src/commands/ (new)"]
CA[audit.js]
CB[batch.js]
CBC[branch-compare.js]
CCFG[cfg.js]
CCH[check.js]
CCO[cochange.js\nformatters only]
CCM[communities.js]
CCX[complexity.js]
CDF[dataflow.js]
CFL[flow.js]
CMF[manifesto.js]
COW[owners.js]
CQ[query.js\nbarrel re-export]
CSQ[sequence.js]
CST[structure.js\n+re-exports data fns]
CTR[triage.js]
end
subgraph infra["src/infrastructure/ (new)"]
CR[command-runner.js\nrunCommand — UNUSED]
RF[result-formatter.js\noutputResult]
TF[test-filter.js\nisTestFile]
end
subgraph analysis["src/ analysis modules (unchanged *Data fns)"]
A[audit.js]
B[batch.js]
BC[branch-compare.js]
CFG[cfg.js]
CH[check.js]
CO[cochange.js]
CM[communities.js]
CX[complexity.js]
DF[dataflow.js]
FL[flow.js]
MF[manifesto.js]
OW[owners.js]
SQ[sequence.js]
ST[structure.js]
TR[triage.js]
end
CLI -->|dynamic import| commands
IDX -->|re-exports| commands
commands -->|calls *Data| analysis
commands -->|outputResult| RF
CR -.->|exported but\nnever imported| commands
analysis --> TF
Last reviewed commit: 1ce7511 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/infrastructure/command-runner.js
Outdated
| export function runCommand(dataFn, formatFn, opts = {}) { | ||
| const data = dataFn(); | ||
| if (outputResult(data, opts.ndjsonField ?? null, opts)) { | ||
| if (opts.exitOnFail && data.passed === false) process.exit(1); | ||
| return data; | ||
| } | ||
| const text = formatFn(data); | ||
| if (text) console.log(text); | ||
| if (opts.exitOnFail && data.passed === false) process.exit(1); | ||
| return data; | ||
| } |
There was a problem hiding this comment.
runCommand is exported but never used
The PR description explicitly calls out "Introduce CommandRunner shared lifecycle (src/infrastructure/command-runner.js)" as a completed ✅ objective, but runCommand is not imported or called by any of the 16 new command files (src/commands/audit.js, src/commands/check.js, src/commands/manifesto.js, etc.). Every command file independently calls outputResult and handles its own process.exit(1) logic inline, bypassing the lifecycle helper entirely.
This renders runCommand dead code at the time of merge. Either the command files should be refactored to adopt runCommand, or this file should be noted as scaffolding for a follow-up phase rather than claiming it as done in the ROADMAP.
CommandRunner was created but never adopted by any command file — the 16 commands vary too much (async, multi-mode, process.exit) for a single lifecycle helper today. Remove dead code and mark as future work in the ROADMAP rather than claiming it as done.
|
Claude finished @carlos-alm's task —— View job PR Review CompleteConducted comprehensive 4-phase review of CLI command extraction refactoring:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR addresses architectural debt documented in ROADMAP section 3.2 "Command/Query Separation." Analysis modules suffered from a systematic dual-function anti-pattern where each module implemented both data extraction ( Why This Approach:
Risk Assessment: Low risk - This is legitimate architectural refactoring addressing documented tech debt, not symptom hiding. Phase 2: Configuration Change Detection ✅No high-risk configuration changes detected:
Phase 3: Test Integrity ✅Test integrity maintained:
Phase 4: Code Quality ✅Excellent implementation quality: Architecture Benefits:
Implementation Quality:
Critical Concerns: None detected Minor Notes:
Final Assessment
|
1 participant
Summary
src/commands/files (audit, batch, cfg, check, cochange, communities, complexity, dataflow, flow, branch-compare, manifesto, owners, sequence, structure, triage) plus a query barrel re-exportresult-formatter.jsandtest-filter.jstosrc/infrastructure/cli.js,index.js,queries-cli.js, and 7 other modules*Data()functions remain in original modules — MCP dynamic imports unchangedNet effect: ~1,059 lines of CLI formatting code removed from analysis modules, separated into 16 focused command files. Completes most of ROADMAP section 3.2 "Command/Query Separation" (CommandRunner shared lifecycle deferred — command files vary too much for a single pattern today).
Test plan
npm run lint— clean (0 errors)npm test— 85/85 test files pass, 1604/1604 tests passnode -e "import('./src/index.js')"— resolves OKnode -e "import('./src/cli.js')"— resolves OK