-
Notifications
You must be signed in to change notification settings - Fork 35
Open
Labels
maintenanceneeds-triageNew issue, not yet reviewed by maintainersNew issue, not yet reviewed by maintainers
Milestone
Description
Is your feature request related to a problem? Please describe.
After the cli.py and apm_package.py refactoring in PR #224, several newly extracted modules exceed code quality thresholds (400-line file limit, 30-statement function limit). This makes the codebase harder to navigate, test, and maintain. Key pain points:
- 5 god files exceed the 400-line limit:
install.py(1449),dependency.py(1024),deps.py(879),compile.py(739),uninstall.py(559) - 9 god functions exceed 30 statements, with
_install_apm_dependencies()(~250 stmts),uninstall()(~540 stmts), andDependencyReference.parse()(~380 stmts) being the worst - ~85-line integration loop duplicated in full for cached vs. fresh packages in
install.py - 30+ overly broad exception handlers (
except Exception:) across all modules, some swallowing errors silently - String-typed dispatch (
--only "apm"|"mcp") and boolean flag pairs where enums would be safer
Describe the solution you'd like
A phased refactoring (each phase is independently shippable) to bring all modules under quality thresholds:
| Phase | Scope | Risk |
|---|---|---|
| 1. Constants + exception handlers | Extract shared constants to constants.py. Narrow all except Exception: to specific types. Remove dead code. |
Low |
| 2. Decompose god functions | Break _install_apm_dependencies(), uninstall(), and DependencyReference.parse() into ≤30-statement helpers. Extract the duplicated integration loop into _integrate_package_artifacts(). |
Medium |
| 3. Enums for dispatch | Add InstallMode, IntegrationTarget, VirtualPackageType, HostType enums. Replace string/boolean dispatch. |
Medium |
| 4. Split god files | install.py → CLI entry + install engine + integrator orchestrator. deps.py → list/tree/info submodules. compile.py → CLI entry + compiler + watcher. |
Medium-High |
| 5. Flatten nesting + typed results | Guard clauses to reduce nesting ≤3 levels. Dataclasses for InstallResult, PrimitiveCounts, etc. Strategy pattern for Rich/text rendering. |
Low |
Describe alternatives you've considered
- Do nothing: Acceptable short-term since PR Refactor cli.py and apm_package.py into focused modules (#172) #224 already reduced
cli.pyfrom 4500→80 lines, but the extracted modules inherited the same structural issues. - Full rewrite: Over-engineered for current needs. Phased approach lets us ship incrementally and validate each change.
Additional context
- Full analysis was performed against the
feat/172-refactor-cli-and-apm-package-modulesbranch (PR Refactor cli.py and apm_package.py into focused modules (#172) #224). - Files analyzed (with line counts):
install.py(1449),dependency.py(1024),deps.py(879),compile.py(739),uninstall.py(559),_helpers.py(405),validation.py(388),mcp.py(373),run.py(218),init.py(192),runtime.py(188),config.py(169),prune.py(143),update.py(136),list_cmd.py(102). - Refactoring triggers checked: duplicated logic, god functions, primitive obsession, deep nesting, magic numbers/strings, dead code, broad exception handlers, god files, long parameter lists, boolean flag forking, string-typed dispatch, mixed abstraction levels.
- All phases must maintain 100% backward compatibility and pass
uv run pytest tests/ -q --tb=short -x.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
maintenanceneeds-triageNew issue, not yet reviewed by maintainersNew issue, not yet reviewed by maintainers