refactor: decompose CLI entry point into focused modules#268
Conversation
Reduces src/index.ts from 1,154 lines to 31 lines by extracting CLI
commands and utilities into a dedicated src/cli/ directory structure.
## Changes
- Extract each CLI command into separate files in src/cli/commands/
- Move output formatters to src/cli/formatters.ts
- Move state helpers to src/cli/helpers.ts
- Move option parsing to src/cli/options.ts
- Move Commander styling to src/cli/styles.ts
- Simplify src/index.ts to public API exports + CLI entry
## File Structure
```
src/cli/
commands/
run.ts - Full pipeline command
analyze.ts - Stage 1 only
generate.ts - Stages 1-2
execute.ts - Stages 1-3
resume.ts - Resume with handlers
report.ts - Report generation
list.ts - List runs
index.ts - Command registration
formatters.ts - Output formatters (JUnit, TAP, CLI)
helpers.ts - State lookup utilities
options.ts - CLI option parsing
styles.ts - Commander help styling
index.ts - Program setup
```
Part of #245 - Phase 1 of file decomposition effort
Fixes #245
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @sjnims's task in 2m 58s —— View job PR Review: CLI Entry Point DecompositionI've completed a comprehensive review of this refactoring. Here are my findings: ✅ Verdict: Looks GoodThis is a well-executed refactoring that successfully decomposes a 1,154-line file into focused, maintainable modules without changing any behavior. Quality Checks Results
What's GoodArchitecture & Design
Code Quality
Testing & Validation
Documentation
SuggestionsMinor Code Duplication (Non-blocking) Several commands share similar config loading patterns. Example from const cliOptions: Partial<CLIOptions> = {};
if (typeof options["plugin"] === "string") {
cliOptions.plugin = options["plugin"];
}
const configPath = typeof options["config"] === "string" ? options["config"] : undefined;
const config = loadConfigWithOverrides(configPath, cliOptions);This is acceptable for now - the duplication is minimal and each command may evolve independently. If this pattern spreads to more commands, consider extracting a shared helper. Test Coverage for CLI (Acknowledged) The PR notes that new CLI files have 0% test coverage, matching the previous lack of CLI testing. While this is being tracked in the "known limitations," consider adding CLI integration tests in a future PR to ensure commands work end-to-end. CLAUDE.md Update (Optional) The Architecture section's "Key Entry Points" table could mention the new Architecture Review4-Stage Pipeline: ✅ Not affected - stage logic remains unchanged Type Safety: ✅ Excellent - uses Zod schemas, proper type guards, no Error Handling: ✅ Proper - all commands have try/catch with Configuration: ✅ Correct - uses State Management: ✅ Proper - resume handlers correctly chain state updates Specific Technical HighlightsGood Pattern: Resume Handler Map (src/cli/commands/resume.ts:240-247) const resumeHandlers: Record<PipelineStage, ResumeHandler> = {
pending: resumeFromAnalysis,
analysis: resumeFromAnalysis,
generation: resumeFromGeneration,
execution: resumeFromExecution,
evaluation: resumeFromEvaluation,
complete: resumeFromEvaluation,
};This replaces complex conditional logic with a clean lookup table. Good Pattern: Command Registration (src/cli/commands/index.ts) Good Pattern: Option Extraction (src/cli/options.ts) SummaryThis refactoring achieves its goal of making the codebase more navigable and maintainable. The 1,154-line Recommendation: Approve and merge. The known limitations (CLI test coverage, minor duplication) are acceptable and don't block this improvement. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Update directory structure and file references across documentation to reflect the CLI decomposition from PR #268. ## Changes - CLAUDE.md: Update Key Entry Points table, Directory Structure, Internal vs Public section, and Resume Handlers reference - README.md: Update Project Structure section to include src/cli/ - CONTRIBUTING.md: Update Project Structure section to include src/cli/ - PR template: Split CLI from Entry Point in Core Infrastructure section Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @sjnims's task in 3m 50s —— View job PR Review: CLI Entry Point DecompositionI've completed a comprehensive review of this refactoring PR. Here are my findings: ✅ Verdict: Looks GoodThis is a well-executed refactoring that successfully decomposes a 1,154-line file into focused, maintainable modules without changing any behavior. Quality Checks Results
What's GoodArchitecture & Design
Code Quality
Testing & Validation
Documentation
SuggestionsMinor Code Duplication (Non-blocking) Several commands share similar config loading patterns. Example from const cliOptions: Partial<CLIOptions> = {};
if (typeof options["plugin"] === "string") {
cliOptions.plugin = options["plugin"];
}
const configPath = typeof options["config"] === "string" ? options["config"] : undefined;
const config = loadConfigWithOverrides(configPath, cliOptions);This is acceptable for now - the duplication is minimal (3-5 lines) and each command may evolve independently. If this pattern spreads to more commands, consider extracting a shared helper like Test Coverage for CLI (Acknowledged) The PR notes that new CLI files have 0% test coverage, matching the previous lack of CLI testing. While this is being tracked in the "known limitations" section of the PR, consider adding CLI integration tests in a future PR to ensure:
CLAUDE.md Enhancement (Optional) The "Code Navigation" section could mention the new Architecture Review4-Stage Pipeline: ✅ Not affected - stage logic remains unchanged, only CLI orchestration was refactored Type Safety: ✅ Excellent - uses Zod schemas, proper type guards, no Error Handling: ✅ Proper - all commands have try/catch with Configuration: ✅ Correct - uses State Management: ✅ Proper - resume handlers correctly chain state updates (resume.ts:69-74) Public API: ✅ Preserved - all public exports remain unchanged in Specific Technical HighlightsGood Pattern: Resume Handler Map (src/cli/commands/resume.ts:240-247) const resumeHandlers: Record<PipelineStage, ResumeHandler> = {
pending: resumeFromAnalysis,
analysis: resumeFromAnalysis,
generation: resumeFromGeneration,
execution: resumeFromExecution,
evaluation: resumeFromEvaluation,
complete: resumeFromEvaluation,
};This replaces complex conditional logic with a clean lookup table, making the code more maintainable. Good Pattern: Command Registration (src/cli/commands/index.ts:18-34) Good Pattern: Option Extraction (src/cli/options.ts:35-66) Good Pattern: Runtime Validation (src/cli/formatters.ts:66-77) const scenarioId = typeof result["scenario_id"] === "string"
? result["scenario_id"]
: "unknown";Good Pattern: Commander v13+ Features (src/cli/commands/run.ts:48-77) Edge Cases Verified✅ Option parsing: Handles undefined, null, and wrong-type options gracefully No Security Issues Found
SummaryThis refactoring achieves its goal of making the codebase more navigable and maintainable. The 1,154-line
Recommendation: Approve and merge. The known limitations (CLI test coverage, minor duplication) are acceptable and don't block this improvement. This is a significant quality-of-life improvement for future development. |
Split src/state/state-manager.ts (691 lines) into 5 focused modules following the pattern established in PR #268: - types.ts: Shared type definitions (PipelineState, PipelineStage, etc.) - core.ts: CRUD operations (create, save, load, paths, migration) - updates.ts: Pure state update functions (updateStateAfter*, etc.) - queries.ts: Query helpers (canResumeFrom, get*Scenarios, etc.) - display.ts: Formatting and listing (formatState, listRuns) - index.ts: Re-exports for backwards compatibility Benefits: - Clear separation of concerns (CRUD vs queries vs updates vs display) - Each file has a single responsibility - Easier to test and maintain - Zero breaking changes via re-exports Also added .gitignore exception for src/state/core.ts to avoid conflict with the core dump pattern (core.*). Fixes #269 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
## Description Split `src/state/state-manager.ts` (691 lines) into 5 focused modules following the pattern established in PR #268: - **types.ts** (87 lines): Shared type definitions (`PipelineState`, `PipelineStage`, etc.) - **core.ts** (207 lines): CRUD operations (create, save, load, paths, migration) - **updates.ts** (177 lines): Pure state update functions (`updateStateAfter*`, etc.) - **queries.ts** (136 lines): Query helpers (`canResumeFrom`, `get*Scenarios`, etc.) - **display.ts** (132 lines): Formatting and listing (`formatState`, `listRuns`) - **index.ts** (67 lines): Re-exports for backwards compatibility Also added `.gitignore` exception for `src/state/core.ts` to avoid conflict with the core dump pattern (`core.*`). ## Type of Change - [x] Refactoring (code change that neither fixes a bug nor adds a feature) ## Component(s) Affected ### Core Infrastructure - [x] State Management (`src/state/`) ### Other - [x] Tests (`tests/`) - [x] Configuration files (`config.yaml`, `eslint.config.js`, `tsconfig.json`, etc.) ## Motivation and Context Following the successful CLI decomposition in PR #268, `src/state/state-manager.ts` remained at 691 lines, making it one of the largest files in the codebase. The file mixed multiple concerns: - CRUD operations (create, save, load) - State update functions - Query helpers - Display/formatting logic This refactoring provides: - Clear separation of concerns (CRUD vs queries vs updates vs display) - Single responsibility per file - Easier testing and maintenance - Zero breaking changes via re-exports Fixes #269 ## How Has This Been Tested? **Test Configuration**: - Node.js version: 25.3.0 - OS: macOS **Test Steps**: 1. `npm run build` - TypeScript compilation passes 2. `npm run lint` - ESLint passes 3. `npm run typecheck` - Type checking passes 4. `npm test -- tests/unit/state/` - All state management tests pass with 100% coverage ## Checklist ### General - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings or errors ### TypeScript / Code Quality - [x] All functions have explicit return types - [x] Strict TypeScript checks pass (`npm run typecheck`) - [x] ESM import/export patterns used correctly - [x] Unused parameters prefixed with `_` - [x] No `any` types without justification ### Documentation - [ ] I have updated CLAUDE.md if behavior or commands changed (N/A - no behavior change) - [x] I have updated inline JSDoc comments where applicable - [x] I have verified all links work correctly ### Linting - [x] I have run `npm run lint` and fixed all issues - [x] I have run `npm run format:check` - [ ] I have run `markdownlint "*.md"` on Markdown files (N/A - no markdown changes) - [ ] I have run `uvx yamllint -c .yamllint.yml` on YAML files (N/A - no YAML changes) - [ ] I have run `actionlint` on workflow files (N/A - no workflow changes) ### Testing - [x] I have run `npm test` and all tests pass - [x] I have added tests for new functionality (existing tests cover all functionality) - [x] Test coverage meets thresholds (100% on state module) - [ ] I have tested with a sample plugin (N/A - no runtime behavior change) ## New File Structure ``` src/state/ ├── index.ts # Re-exports (maintains backwards compatibility) ├── types.ts # Shared type definitions ├── core.ts # CRUD operations (create, save, load, paths, migration) ├── updates.ts # State update functions ├── queries.ts # Query helpers (canResumeFrom, get*Scenarios) └── display.ts # Formatting and listing ``` ## Reviewer Notes **Areas that need special attention**: - Verify all exports are correctly re-exported from `index.ts` - Confirm `.gitignore` exception is the right approach for `core.ts` **Known limitations or trade-offs**: - Slight increase in total line count (785 vs 691) due to module documentation and import/export overhead - The `.gitignore` exception is needed because `core.*` pattern exists for core dumps --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…tory Split the 679-line programmatic-detector.ts into focused modules: - detection/types.ts: Type guards and interfaces (~55 lines) - detection/capture-detection.ts: Real-time capture detection (~170 lines) - detection/commands.ts: Direct command invocation detection (~75 lines) - detection/correlation.ts: Transcript correlation (~60 lines) - detection/hooks.ts: Hook response detection (~105 lines) - detection/agents.ts: Subagent lifecycle detection (~50 lines) - detection/helpers.ts: Utility functions (~75 lines) - detection/core.ts: Main detection orchestration (~170 lines) - detection/index.ts: Re-exports for backwards compatibility Benefits: - Clear separation by detection type (hooks, agents, commands, MCP) - Easier to navigate to specific detection logic - More focused files (each 50-170 lines vs original 679 lines) - Easier to extend with new component types - Follows PR #268 pattern for subdirectory decomposition All existing imports continue to work via re-exports. Fixes #271 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tory (#274) ## Description Split the 679-line `programmatic-detector.ts` into a focused `detection/` subdirectory with specialized modules, following the pattern established in PR #268 (CLI decomposition). **New structure:** ``` src/stages/4-evaluation/detection/ ├── index.ts # Re-exports for backwards compatibility ├── types.ts # Type guards and interfaces (~55 lines) ├── capture-detection.ts # Real-time capture detection (~170 lines) ├── commands.ts # Direct command invocation detection (~75 lines) ├── correlation.ts # Transcript correlation (~60 lines) ├── hooks.ts # Hook response detection (~105 lines) ├── agents.ts # Subagent lifecycle detection (~50 lines) ├── helpers.ts # Utility functions (~75 lines) └── core.ts # Main detection orchestration (~170 lines) ``` ## Type of Change - [x] Refactoring (code change that neither fixes a bug nor adds a feature) ## Component(s) Affected ### Pipeline Stages - [x] Stage 4: Evaluation (`src/stages/4-evaluation/`) ### Other - [x] Tests (`tests/`) - [x] Configuration files (`config.yaml`, `eslint.config.js`, `tsconfig.json`, etc.) ## Motivation and Context This is Phase 2 Part 2 of the decomposition effort (Issue #245). The original `programmatic-detector.ts` contained all detection logic for skills, agents, commands, hooks, and MCP tools in a single 679-line file. **Benefits of this refactoring:** - Clear separation by detection type (hooks, agents, commands, MCP) - Easier to navigate to specific detection logic - More focused files (each 50-170 lines vs original 679 lines) - Easier to extend with new component types - Consistent architecture with PR #268 CLI decomposition pattern All existing imports continue to work via re-exports from `detection/index.ts`. Fixes #271 ## How Has This Been Tested? **Test Configuration**: - Node.js version: v25.3.0 - OS: macOS **Test Steps**: 1. Run `npm run build` - TypeScript compilation passes 2. Run `npm run lint` - All linting checks pass 3. Run `npm run format` - Code is properly formatted 4. Run `npm test` - All 1519 tests pass ## Checklist ### General - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] My changes generate no new warnings or errors ### TypeScript / Code Quality - [x] All functions have explicit return types - [x] Strict TypeScript checks pass (`npm run typecheck`) - [x] ESM import/export patterns used correctly - [x] Unused parameters prefixed with `_` - [x] No `any` types without justification ### Documentation - [ ] I have updated CLAUDE.md if behavior or commands changed - [x] I have updated inline JSDoc comments where applicable - [x] I have verified all links work correctly ### Linting - [x] I have run `npm run lint` and fixed all issues - [x] I have run `npm run format:check` - [x] I have run `markdownlint "*.md"` on Markdown files - [ ] I have run `uvx yamllint -c .yamllint.yml` on YAML files (if modified) - N/A - [ ] I have run `actionlint` on workflow files (if modified) - N/A ### Testing - [x] I have run `npm test` and all tests pass - [ ] I have added tests for new functionality - N/A (no new functionality, just reorganization) - [x] Test coverage meets thresholds (78% lines, 75% functions, 65% branches) - [ ] I have tested with a sample plugin (if applicable) - N/A ## Stage-Specific Checks <details> <summary><strong>Stage 4: Evaluation</strong> (click to expand)</summary> - [x] Programmatic detection correctly parses tool captures: - Skill/Task/SlashCommand via PreToolUse/PostToolUse hooks - Agents via SubagentStart/SubagentStop hooks - Hooks via SDKHookResponseMessage events - MCP tools via `mcp__<server>__<tool>` pattern - [x] LLM judge fallback works for edge cases - [x] Conflict detection identifies overlapping triggers - [x] Result aggregation methods work correctly - [x] Citations link to correct message IDs </details> ## Additional Notes This refactoring maintains complete backwards compatibility. All existing imports from `./programmatic-detector.js` continue to work through re-exports in `detection/index.ts`. ## Reviewer Notes **Areas that need special attention**: - Verify that all detection functions are correctly re-exported - Check that import paths in consuming files are updated correctly **Known limitations or trade-offs**: - Added a new exception to `.gitignore` for `src/stages/4-evaluation/detection/core.ts` (matches the existing pattern for `src/state/core.ts`) --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Description
Reduces
src/index.tsfrom 1,154 lines to 37 lines by extracting CLI commands and utilities into a dedicatedsrc/cli/directory structure. Each CLI command is now in its own file, making the codebase easier to navigate, test, and maintain.Type of Change
Component(s) Affected
Core Infrastructure
src/index.ts)Other
src/cli/directory with command modulesMotivation and Context
Issue #245 identified
src/index.ts(1,154 lines) as the largest file in the codebase, making it hard to navigate and maintain. This refactoring:Fixes #245
How Has This Been Tested?
Test Configuration:
Test Steps:
npm run typecheck- TypeScript passesnpm run lint- ESLint passesnpm run format- Prettier passesnpm test- All 1,499 tests passnpm run build- Build succeedsnode dist/index.js --help- CLI help displays correctlynode dist/index.js run --help- Command help displays correctlyChecklist
General
TypeScript / Code Quality
npm run typecheck)_anytypes without justificationDocumentation
Linting
npm run lintand fixed all issuesnpm run format:checkmarkdownlint "*.md"on Markdown files (no MD changes)uvx yamllint -c .yamllint.ymlon YAML files (if modified) - N/Aactionlinton workflow files (if modified) - N/ATesting
npm testand all tests passNew File Structure
Key metric:
src/index.tsreduced from 1,154 → 37 lines (97% reduction)Example Output
Additional Notes
This is Phase 1 of the file decomposition effort described in #245. Future phases may include:
src/state/state-manager.ts(722 lines)index.tsfiles as they're modified for other workReviewer Notes
Areas that need special attention:
src/index.tsare preserved correctlyKnown limitations or trade-offs:
🤖 Generated with Claude Code