PERF: Lazy-load CLI subcommands to fix 70s+ startup on Windows#1241
PERF: Lazy-load CLI subcommands to fix 70s+ startup on Windows#1241Luay-Sol wants to merge 4 commits intoSolaceLabs:mainfrom
Conversation
efunneko
left a comment
There was a problem hiding this comment.
PR Review: PERF: Lazy-load CLI subcommands to fix 70s+ startup on Windows
Summary
Introduces a LazyGroup click.Group subclass that defers subcommand imports until the specific command is invoked. This eliminates the ~76s startup penalty on air-gapped Windows environments caused by transitive imports of heavyweight packages (google.adk, litellm, markitdown/onnxruntime). Applied to the main CLI group, add subgroup, and tools subgroup. The init_cmd module also uses __getattr__ for lazy module-level attribute loading.
Architecture
Well-designed. The LazyGroup class is a clean, reusable abstraction that slots naturally into Click's extension points. Key positives:
- Uses Click's own
cls=parameter for groups — zero monkey-patching - Properly delegates to
super()for non-lazy commands, making it a true superset ofclick.Group - The
__getattr__approach incli/commands/init_cmd/__init__.pycorrectly handles the constraint that tests mock attributes on this module (e.g.,mocker.patch("cli.commands.init_cmd.broker_setup_step")) — these trigger__getattr__→_load_all()and work transparently
Minor gap: plugin_cmd and task_cmd still use eager imports. These are lighter-weight than init_cmd but could be candidates for a follow-up if needed.
Code Quality
Generally clean. A few observations:
-
init_cmd/__init__.py:136-155—run_init_flowmanually rebinds every function frommod.__dict__into local variables. This is verbose and fragile — if a new step is added, two places need updating (the_load_all()bindings AND the local rebindings inrun_init_flow). A simpler approach would be to just call_load_all()and use module-level names directly since__getattr__handles first-access. -
lazy_group.py:99— Theformat_commandstruncation logic manually reimplements Click's help text truncation. This works but could diverge from Click's internal formatting if Click changes. Low risk but worth noting. -
lazy_group.py:121— The broadexcept Exceptionis acceptable since all import paths are hardcoded, but thereturn Nonemeans a broken module path silently degrades to "no such command" for the user while only logging to debug. Consider whether a user-visible error message would be more helpful (e.g., "Command 'init' failed to load — see logs"). -
init_cmd/__init__.py:66-133—_build_defaults()re-imports the same modules that_load_all()already imported. This is harmless (Python caches modules) but adds unnecessary duplication. Since_build_defaultsis only called from within_load_all, it could just use the already-imported names.
Security
No security concerns. All import paths are hardcoded string literals in source code. There is no mechanism for user input to influence module loading paths. The importlib.import_module() usage is safe given the hardcoded inputs.
Test Coverage
Adequate but incomplete:
test_main.pywas updated to uselist_commands()via Context instead of directly accessingcli.commandsdict — this correctly adapts toLazyGroup- Existing
tests/unit/cli/commands/init_cmd/test___init__.pytests usemocker.patch("cli.commands.init_cmd.broker_setup_step")which works with__getattr__ - Missing: No unit tests for
LazyGroupitself. Key behaviors that should be tested:list_commands()returns lazy + eager commands sortedget_command()defers import until calledget_command()caches resolved commandsformat_commands()uses help text from dict without importing_resolve_import()handles invalid paths gracefully_resolve_import()handles missing attributes gracefully
- Missing: No test verifying that
sam --helpdoes NOT trigger subcommand imports (the core value proposition)
Recommendations
-
[Blocking] Add unit tests for
LazyGroup— This is the core new abstraction and should have its own test suite covering the behaviors listed above. -
Simplify
run_init_flowlocal rebindings — The manualmod.xxx→ local variable pattern at lines 142-155 is fragile. Just use module-level names after_load_all(). -
Deduplicate imports in
_build_defaults— Since it's called from_load_all(), pass the already-imported names or use module globals. -
Consider user-facing error on import failure — A command silently disappearing is confusing. At minimum,
get_commandcouldclick.echoa warning.
Verdict
Request Changes — The implementation is architecturally sound and the performance improvement is significant. The main gap is the lack of unit tests for LazyGroup itself, which is the key new abstraction. The code quality items are suggestions, not blockers. Adding LazyGroup tests would make this ready to approve.
🤖 Generated with Claude Code
Problem
On air-gapped Windows Server 2022 environments, every SAM CLI command takes 75–80 seconds to return — including
sam --versionandsam --help.Before (Windows Server 2022, air-gapped, Python 3.13):
Root cause
cli/main.pyeagerly imports all 8 subcommand modules at startup. This triggers a transitive import chain that loads heavyweight ML/cloud packages regardless of which command the user actually runs:google.adk(→google.cloud.aiplatform)litellmmarkitdown(→magika→onnxruntime)Even
sam --versionpays this ~76s cost because all imports run at module load time, before Click parses the--versionflag.Solution
Introduce
LazyGroup, a drop-inclick.Groupsubclass that defers subcommand imports until the specific command is actually invoked:sam --version/sam --help→ zero subcommand importssam run --help→ only importsrun_cmdsam init --help→ shows cached help text, no heavy importsam init(actual invocation) → importsinit_cmdon demandThe same pattern is applied to nested groups (
sam add,sam tools) so their--helpalso avoids heavyweight imports.After (same Windows Server 2022 environment):
Files changed
cli/lazy_group.pyLazyGroupclass withformat_commandsoverridecli/main.pyfrom ... import+add_command()withLazyGroupcli/commands/add_cmd/__init__.pyLazyGroupforagent/gateway/proxycli/commands/tools_cmd.py@tools.command("list")registration withLazyGroupforlistHow
LazyGroupworkslist_commands()returns command names from thelazy_commandsdict — no importsformat_commands()renders help text from the dict — no importsget_command()callsimportlib.import_module()only when the command is invoked, then caches itBackward compatibility
sam tools list,sam init --gui,sam add agent --guiall function as beforeLazyGroupis a strict superset ofclick.Group— passes through tosuper()for any non-lazy commandsTesting
sam tools listcorrectly loads the tool registry and displays all toolssam init --guicorrectly launches the web portalsam add agent --helpshows all optionssam runstarts the application normally