feat: lazy command loading for faster CLI startup#261
Conversation
16dd979 to
9228db8
Compare
Replace eager auto-discovery with lazy command loading using a static command-to-module map. Command modules are only imported when the specific command is invoked, not at CLI import time. Key changes: - _GlobalOptionsGroup replaced with _LazyCommandGroup that combines lazy loading with global option hoisting - Static _COMMAND_MODULE_MAP provides O(1) command name to module resolution without importing any command modules - list_commands() returns names from the static map (no imports) - get_command() imports only the requested module on first access - --ai-help injection deferred to per-command first access - Import __version__ from _version directly instead of client.py to avoid pulling in ssl, yaml, urllib at import time Performance (end-to-end subprocess, cold start): - limacharlie --version: ~55ms (was ~280ms) - limacharlie sensor list --help: ~67ms (was ~265ms) - limacharlie completion bash: ~58ms (was ~280ms) - limacharlie --help (all commands): ~134ms (loads all modules) Tests: - 816 regression tests covering full CLI surface: command registration, subcommand structure, module mapping, global options, --help for every command, --ai-help injection and output, explain registry, discovery profiles, completion, and context propagation - End-to-end subprocess benchmarks and regression tests - In-process microbenchmarks for import, help, completion, resolution Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9228db8 to
ae1147f
Compare
maximelb
left a comment
There was a problem hiding this comment.
@tomaz-lc Thorough review below. One confirmed bug that needs fixing before merge.
Bug: limacharlie --ai-help shows zero command groups in fresh process
Severity: High — user-visible breakage of an existing feature.
_top_level_help() in ai_help.py:125 iterates cli.commands (the eagerly-populated dict) directly:
for name in sorted(cli.commands):
sub = cli.commands[name]With lazy loading, cli.commands is empty until get_command() is called. Confirmed locally:
$ python -c "
from limacharlie.cli import cli
from click.testing import CliRunner
r = CliRunner().invoke(cli, ['--ai-help'])
section = r.output.split('## All Command Groups')
if len(section) > 1:
groups_section = section[1].split('## ')[0]
group_lines = [l for l in groups_section.strip().split('\n') if l.startswith('- **')]
print(f'Number of command groups listed: {len(group_lines)}')
"
# Output: Number of command groups listed: 0
Meanwhile, --help works fine because Click's format_help() goes through list_commands() + get_command().
Why tests don't catch it: test_cli_lazy_loading_regression.py:36-38 eagerly loads all commands at module import time before any tests run. The e2e test (test_e2e_ai_help_works) only asserts "LimaCharlie CLI" in result.stdout — that string is in the header, not the command list.
Fix: _top_level_help() should use cli.list_commands(ctx) + cli.get_command(ctx, name) instead of accessing cli.commands directly. Or the show_ai_help callback could trigger a full load before rendering.
Minor Issues
1. Unused import pkgutil — cli.py:17
Was used by the removed _auto_discover_commands(). Dead import now.
2. Unused field import — cli.py:19
from dataclasses import dataclass, fieldfield is never used.
3. Weak test assertion — test_cli_lazy_loading_regression.py:370
assert "--oid" in result.output or result.exit_code == 0The or makes this always true since exit_code == 0 was already asserted on line 369.
4. Silent failure on broken command modules
_import_command() catches all exceptions and returns None unless LC_DEBUG is set. A module with a syntax error silently vanishes — user sees "No such command 'foo'" with no hint the module exists but failed to load. Consider at minimum a one-line stderr warning even without LC_DEBUG.
5. Regression tests don't test lazy loading in realistic conditions
The eager loading at test_cli_lazy_loading_regression.py:36-38:
_ctx = click.Context(cli)
for _name in cli.list_commands(_ctx):
cli.get_command(_ctx, _name)...defeats lazy loading for all tests below it. This is why the --ai-help bug slipped through. The TestAiHelpOutput tests run against a fully-loaded CLI. Consider having the --ai-help e2e test actually verify command groups appear in the output from a fresh subprocess.
6. Duplicated CI bash blocks in cloudbuild_pr.yaml
The 7 dist steps are nearly identical ~30-line bash blocks. A shared shell script taking the Python version as argument would cut ~200 lines of duplication and make maintenance easier. Not a blocker.
What's Good
_COMMAND_MODULE_MAPapproach is clean; the lint tests intest_cli_command_map_lint.pygive excellent DX (tells developer exactly what line to add).__version__fix (importing from_versioninstead ofclient.py) is a real improvement for both startup perf and correctness.- Option hoisting + shadowed-opts detection logic is carefully preserved.
- CI version consistency checks and PyPI stable sanity check are smart permanent additions.
Bottom line: fix the --ai-help bug and add an e2e test that verifies command groups appear in the output from a fresh process, then this is good to go.
_top_level_help() iterated cli.commands directly which is empty with lazy loading. Use list_commands() + get_command() instead, matching how Click itself resolves commands for --help. Also: remove dead pkgutil/field imports from cli.py, always emit stderr warning for broken command modules (not just with LC_DEBUG), fix weak test assertion, and add regression tests + microbenchmarks for --ai-help. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
limacharlie.output pulls in jmespath, tabulate, yaml, and csv which adds significant import overhead. Deferring the import from module level into the cli() callback avoids this cost on fast paths like --help, --version, and --ai-help that never render command output. Benchmark results (e2e subprocess, cold process): - cli import: 2.2ms -> 1.2ms (47% faster) - --version: 114ms -> 55ms (52% faster) - --help: 537ms -> 176ms (67% faster) - --ai-help: 481ms -> 205ms (57% faster) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…265) - Add config_cmd to _COMMAND_MODULE_MAP in cli.py (missing after #257 and #261 were merged independently) - Add config to EXPECTED_TOP_LEVEL_COMMANDS, EXPECTED_MODULE_MAP, and EXPECTED_SUBCOMMANDS in regression tests - Fix test_cli_import_does_not_load_output to handle third-party deps already loaded by other tests in the same pytest process - Add ci.yml GHA workflow that runs unit tests and dist checks on every push and PR Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Details
Every CLI invocation currently pays the cost of eagerly importing all 49 command modules at startup, even when only one command is being run. This pulls in the full SDK,
requests,yaml,ssl, and other heavy dependencies before any command logic executes.This PR replaces eager auto-discovery with lazy command loading. A static
_COMMAND_MODULE_MAPmaps Click command names to their module paths, allowinglist_commands()to return all names without importing anything, andget_command()to import only the specific module needed. Additionally,limacharlie.output(which pulls in jmespath, tabulate, yaml, csv) is deferred from module-level to thecli()callback, avoiding ~14ms of import overhead on fast paths. This benefits every CLI operation:sensor list,auth whoami) - only import the one module needed--help,--ai-help) - subcommand help only imports the target moduleeval "$(limacharlie completion bash)") - faster shell startup when using eval--version- no command modules imported at allAdditionally,
__version__is now imported directly from_version(generated by setuptools-scm) instead of fromclient.py, which avoids pulling inssl,yaml,urllib, andconfigjust to read a version string. This also fixes a bug in the published 5.1.0 wherelimacharlie.__version__returned0.0.0.dev0instead of the real version.How it works
The
_GlobalOptionsGroupclass is replaced with_LazyCommandGroupwhich combines:Lazy loading via static map: A hardcoded dict maps each Click command name (e.g.
"external-adapter") to its module name and attribute (e.g.("adapter", "group")). This is necessary because the mapping is not derivable from filenames alone.Global option hoisting: Same behavior as before -
--oid,--output, etc. can appear anywhere on the command line.Deferred
--ai-helpinjection: Instead of recursively walking all commands at import time,--ai-helpis injected per-command on first access viaget_command().Deferred
limacharlie.outputimport: The output module (jmespath, tabulate, yaml, csv) is imported inside thecli()callback instead of at module level, avoiding the cost on fast paths like--help,--version, and--ai-help.When a new command module is added,
_COMMAND_MODULE_MAPmust be updated. Lint tests intest_cli_command_map_lint.pycatch this at CI time with a developer-friendly error that includes the exact line to add.CI improvements
importlib.metadata.version()matcheslimacharlie.__version__and that__version__is not the fallback0.0.0.dev0.--help, SDK core imports, 8 key command groups, and completion generation. Catches packaging regressions in published releases.Performance
End-to-end subprocess measurements (cold start, pytest-benchmark, min 10 rounds):
import limacharlie.cli(in-process)limacharlie --version(e2e)limacharlie sensor list --help(e2e)limacharlie --help(e2e)limacharlie --ai-help(e2e)All operations are faster, including top-level
--helpand--ai-helpwhich load all modules. The deferredlimacharlie.outputimport accounts for a significant portion of the improvement.Bug fixes included
--ai-helpshowed zero command groups:_top_level_help()iteratedcli.commandsdirectly (empty with lazy loading) instead of usinglist_commands()+get_command(). Fixed._import_command()now always emits a one-line stderr warning when a module fails to load, not just withLC_DEBUG. Full traceback still requiresLC_DEBUG.pkgutilandfield(from dataclasses) in cli.py were leftovers from the removed_auto_discover_commands().assert "--oid" in result.output or result.exit_code == 0was always true due to theor.Trade-offs / potential drawbacks
_COMMAND_MODULE_MAPmust be manually updated when adding/removing/renaming command modules. If a developer forgets, the new command silently won't appear at runtime. Mitigated: lint tests intest_cli_command_map_lint.pyscan the filesystem and fail CI with a developer-friendly message that includes the exact map entry to add (copy-paste ready).Broken command modules surface at invocation time, not startup. With eager loading a syntax error in any module fails immediately. With lazy loading it only fails when that command is run. Mitigated:
test_static_map_all_importableimports every module in CI, the existing unit test suite exercises all commands, and a stderr warning is now always emitted for broken modules.cli.commandsdict is no longer populated at import time. Code that directly accessescli.commands["sensor"]instead of using Click'sget_command()API will see an empty dict until commands are accessed. The proper Click API works fine.Import order side effects with
register_explain(). Command modules callregister_explain()at import time to populate the explain registry. With lazy loading, the registry is only populated for commands that have been imported. In practice this only matters for--ai-helpwhich always goes throughget_command()first, so it works correctly.Blast radius / isolation
limacharlie/cli.py-_GlobalOptionsGroupreplaced with_LazyCommandGroup,_auto_discover_commands()removed,inject_ai_help(cli)replaced with per-command injection,limacharlie.outputimport deferred to callbacklimacharlie/ai_help.py-_top_level_help()useslist_commands()+get_command()instead ofcli.commandsdictlimacharlie/__init__.py-__version__imported from_versioninstead ofclient.py(fixes0.0.0.dev0bug)cloudbuild_pr.yaml- version consistency checks added to all dist steps, new PyPI sanity check stepNotable contracts / APIs
_COMMAND_MODULE_MAPis a new static dict that must be kept in sync when adding/removing command modules. Lint tests enforce this._GlobalOptionsGroupis renamed to_LazyCommandGroup. Any code referencing the old name (unlikely outside cli.py) would need updating.Test plan
_COMMAND_MODULE_MAPcompleteness, correctness, and sync with filesystemRelated PRs
🤖 Generated with Claude Code