Summary
lib/python/base_cli/app.py is 3,179 lines — roughly a third of the entire package's ~9,280 lines — and bundles at least four separable concerns into one file. This was flagged in an external review (Grok) as a maintainability risk, and it's independently corroborated: nearly every bug found in this codebase so far (JSON output capture, display_command/argv handling, exit-code classification, retention-policy selection) has lived in or immediately adjacent to this file.
Details
Reading app.py's top-level structure shows four natural module boundaries already implicit in the code:
- The
App class itself — :457-1282 (~825 lines).
- Native lifecycle-option installation and collision detection (
_install_native_lifecycle_options, _reject_duplicate_lifecycle_declarations, _normalize_lifecycle_values, etc.) — :1511-2034 (~400 lines).
- Click/Typer attachment instrumentation for
attach()/attach_typer-style adoption (_instrument_attached_click_command, _AttachedLifecycleResource, _selected_click_paths, etc.) — :2038-2646 (~600 lines).
run_app() and its helpers — JSON envelope emission, error display, argv resolution (_json_requested, _emit_json_success/_emit_json_error, _show_unexpected_error, _effective_invocation_argv) — :2726-3179 (~450 lines).
Impact
The package is pre-1.0 and its API surface is still evolving; complexity this concentrated makes it harder to review changes safely and correlates with where defects have actually clustered in this codebase historically. It's cheaper to split now than after the public API contract locks at 1.0.
Suggested fix
Split app.py along the four boundaries above into cohesive submodules (e.g. _app_core.py, _lifecycle_install.py, _attach.py, _run.py), re-exporting the current public names from app.py (or __init__.py) so the documented public API and docs/api-stability.md contract are unaffected. No behavior change intended — pure decomposition.
Summary
lib/python/base_cli/app.pyis 3,179 lines — roughly a third of the entire package's ~9,280 lines — and bundles at least four separable concerns into one file. This was flagged in an external review (Grok) as a maintainability risk, and it's independently corroborated: nearly every bug found in this codebase so far (JSON output capture,display_command/argv handling, exit-code classification, retention-policy selection) has lived in or immediately adjacent to this file.Details
Reading
app.py's top-level structure shows four natural module boundaries already implicit in the code:Appclass itself —:457-1282(~825 lines)._install_native_lifecycle_options,_reject_duplicate_lifecycle_declarations,_normalize_lifecycle_values, etc.) —:1511-2034(~400 lines).attach()/attach_typer-style adoption (_instrument_attached_click_command,_AttachedLifecycleResource,_selected_click_paths, etc.) —:2038-2646(~600 lines).run_app()and its helpers — JSON envelope emission, error display, argv resolution (_json_requested,_emit_json_success/_emit_json_error,_show_unexpected_error,_effective_invocation_argv) —:2726-3179(~450 lines).Impact
The package is pre-1.0 and its API surface is still evolving; complexity this concentrated makes it harder to review changes safely and correlates with where defects have actually clustered in this codebase historically. It's cheaper to split now than after the public API contract locks at 1.0.
Suggested fix
Split
app.pyalong the four boundaries above into cohesive submodules (e.g._app_core.py,_lifecycle_install.py,_attach.py,_run.py), re-exporting the current public names fromapp.py(or__init__.py) so the documented public API anddocs/api-stability.mdcontract are unaffected. No behavior change intended — pure decomposition.