refactor!: consolidate MCP tools and CLI commands#263
Conversation
Reduce MCP tool surface from 32 to 29 by merging overlapping tools: - Rename query_function → query with deps/path modes (absorbs fn_deps + symbol_path) - Add list mode to execution_flow (absorbs list_entry_points) - Remove path mode from dataflow tool (now edges + impact only) - Merge fn and path CLI commands into query (--path flag for path mode) - Remove --path option from dataflow CLI command - Update batch commands: remove fn, add dataflow, query uses fnDepsData - Update MCP_DEFAULTS pagination keys BREAKING CHANGE: MCP tools fn_deps, symbol_path, list_entry_points removed. CLI commands fn and path removed. Use query instead. Impact: 1 functions changed, 1 affected
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisReviewing PR #263: refactor!: consolidate MCP tools and CLI commands Todo List:
Phase 1: Root Cause Analysis ✅Problem Being Solved: The PR consolidates overlapping MCP tools from 32 to 29 by merging redundant functionality:
Why This Approach: This is legitimate refactoring addressing API surface complexity. The consolidation is justified - having separate tools for Risk Assessment: LOW - This is architectural cleanup, not hiding symptoms. Phase 2: Configuration Changes ✅✅ No configuration relaxation detected This is clean refactoring without any red flag configuration changes. Phase 3: Test Integrity ✅✅ Tests properly updated to reflect new API
✅ No assertions weakened or removed The test changes are legitimate updates to match the new consolidated API structure. Phase 4: Code Quality Review ✅API Design - EXCELLENT
Implementation Quality - SOLID
Security & Performance - NO CONCERNS
Test Coverage - COMPREHENSIVE
Root Cause AnalysisProblem Being Solved: Consolidating redundant MCP tools that provide overlapping functionality (query vs fn-deps vs symbol-path). This reduces API surface complexity without losing functionality. Why This Approach: Mode-based consolidation is the right architectural choice. Having 3 separate tools for dependency chain queries created confusion for AI agents and increased maintenance burden. Risk Assessment: LOW RISK - This is legitimate architectural cleanup with proper test coverage and backward compatibility preservation. Critical ConcernsNone identified. This is a well-executed refactoring that:
Final Recommendation
Minor suggestions for consideration (non-blocking):
This is exactly the kind of thoughtful refactoring that improves long-term maintainability. |
Greptile SummaryThis PR successfully consolidates the MCP tool and CLI command surface by merging overlapping functionality into fewer, more versatile tools. The refactor reduces MCP tools from 32 to 29 by introducing mode parameters instead of separate tools. Key changes:
Implementation quality: Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[MCP/CLI Tool Consolidation] --> B[query tool]
A --> C[execution_flow tool]
A --> D[dataflow tool]
B --> B1[query_function REMOVED]
B --> B2[fn_deps REMOVED]
B --> B3[symbol_path REMOVED]
B1 -.merged into.-> B4[query with mode param]
B2 -.merged into.-> B4
B3 -.merged into.-> B4
B4 --> B5{mode?}
B5 -->|deps default| B6[fnDepsData]
B5 -->|path| B7[pathData]
C --> C1[list_entry_points REMOVED]
C1 -.merged into.-> C2[execution_flow with list param]
C2 --> C3{list?}
C3 -->|true| C4[listEntryPointsData]
C3 -->|false| C5[flowData]
D --> D1[path mode REMOVED]
D --> D2[edges mode default]
D --> D3[impact mode]
D2 --> D4[dataflowData]
D3 --> D5[dataflowImpactData]
A --> E[CLI Commands]
E --> E1[fn command REMOVED]
E --> E2[path command REMOVED]
E1 -.merged into.-> E3[query command]
E2 -.merged into.-> E3
E3 --> E4{--path flag?}
E4 -->|yes| E5[symbolPath]
E4 -->|no| E6[fnDeps]
A --> F[Batch Commands]
F --> F1[fn REMOVED]
F --> F2[query updated to fnDepsData]
F --> F3[dataflow ADDED]
style B1 fill:#ffcccc
style B2 fill:#ffcccc
style B3 fill:#ffcccc
style C1 fill:#ffcccc
style D1 fill:#ffcccc
style E1 fill:#ffcccc
style E2 fill:#ffcccc
style F1 fill:#ffcccc
style F3 fill:#ccffcc
style B4 fill:#ccffcc
style C2 fill:#ccffcc
Last reviewed commit: bbd60db |
Summary
query_function→querywithdeps/pathmodes (absorbsfn_deps+symbol_path)execution_flowgainslistmode (absorbslist_entry_points)dataflowsimplified toedges+impactonly (path mode removed)fnandpathmerged intoquery(--pathflag for path mode),dataflow --pathremovedfnremoved,dataflowadded,querynow usesfnDepsDataquery: 10replacesquery_function: 50,fn_deps: 10,list_entry_points: 100*Datafunctions remain exported fromsrc/index.jsContinuation of #171.
Test plan
npm run lint— clean (only pre-existing warning in queries.test.js)tests/unit/mcp.test.js(35 tests)tests/integration/batch.test.js(26 tests),tests/integration/pagination.test.js(55 tests)tests/integration/cli.test.jsbuildToolList(false)→ 28 (single-repo),buildToolList(true)→ 29 (multi-repo, includeslist_repos)