perf(cli): Load subcommands and their heavy dependencies on dispatch

[kgilpin]

Why

Every appmap CLI invocation paid about half a second of startup before doing any work, and for the short commands that is nearly all of their run time. appmap --version took 541 ms on a machine where bare Node starts in 69 ms.

The cost was module loading. cli.ts imported all 27 command modules in order to register them with yargs, so running sanitize loaded the code for every other command too. The Navie and RPC commands alone pull in mermaid, jsdom, langchain and express. On top of that, three widely shared imports loaded far more than their callers needed, and telemetry read a config file from disk on every call.

This matters most to tooling that calls the CLI once per file. The gold-traces engine used by the AppMap skills spawns sanitize and sequence-diagram three times per trace; a dry run over 28 traces spent about a minute in CLI startup.

Results

Median of 7 runs, same Node (v22), same 253 KiB recording, this branch vs main:

Command	main	This branch
appmap --version	541 ms	86 ms
appmap sanitize <file>	579 ms	103 ms
appmap sequence-diagram <file> --format json	646 ms	188 ms
Gold-traces engine dry run (28 traces, 84 CLI calls)	65 s	8 s

--version is now within about 17 ms of the Node startup floor, and only 7 ms of that is module loading. What remains in sequence-diagram is its own work: loading the modules it actually uses and about 35 ms of SQL parsing for the recording's queries.

How it works

Commands load on dispatch. packages/cli/src/cliCommands.ts declares each subcommand by name, description and aliases, with a loader for the module that implements it. packages/cli/src/lib/lazyCommand.ts turns each entry into a yargs command module whose builder and handler load the real module the first time yargs dispatches to it. yargs only needs the name and description to list and match commands, so --help and argument parsing are unchanged, and a command's module is loaded only when that command, or its --help, runs. The depends command, which is defined inline in cli.ts, loads its helpers inside its handler the same way.

Because yargs now sees the inline metadata rather than the module's, a new test (tests/unit/cliCommands.spec.ts) loads every module and checks that its command, describe and aliases match the declaration, so the two cannot drift.

Three shared imports are made on demand.

• lib/handleWorkingDirectory imported the RPC configuration, which imports the LLM configuration, which imports the whole Navie package. Nearly every command calls handleWorkingDirectory, which only needs process.chdir. The RPC configuration is now loaded inside configureRpcDirectories.
• rpc/llmConfiguration imported Navie for the single SELECTED_BACKEND constant. It is now read inside getLLMConfiguration.
• cmds/sequenceDiagram imported the browser serving helper, and with it inquirer, rxjs and lodash, which only the PNG format uses. It is now loaded on that path.

Telemetry initializes on first event. TelemetryClient.configure() created the conf store, the session and the backend on every call, loading conf and read-pkg-up and reading the user's config file from disk. They are now created the first time an event or the session id is needed. With telemetry disabled and debug off, sendEvent returns before building an event at all. flush only flushes a backend that was actually created.

Verification

• sanitize and sequence-diagram output is byte-identical to the released 3.201.3 binary on the same input, and the top-level help text is unchanged.
• Every command's --help runs cleanly, including the ones that depend on reflect-metadata being loaded first.
• CLI unit suite: 975 passed, 2 skipped, 158 suites. RPC integration spec: 7 passed. Telemetry package: 42 passed.
• Rebased onto main after the ESLint 8 upgrade; lint is clean on the changed files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SZureVETeSm9qMExQuCSZv

[Copilot]

🟡 Changes recommended

Telemetry.flush can skip its callback when no backend was initialized, breaking installer exit handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves CLI startup performance by loading commands, heavy dependencies, and telemetry state only when needed.

Changes:

• Adds lazy yargs command registration with metadata validation.
• Defers Navie, RPC, and browser-serving imports.
• Lazily initializes telemetry configuration resources and backends.

File summaries

File	Description
packages/telemetry/src/client.ts	Lazily initializes telemetry resources.
packages/cli/tests/unit/cliCommands.spec.ts	Validates command registry metadata.
packages/cli/src/rpc/llmConfiguration.ts	Defers loading Navie.
packages/cli/src/lib/lazyCommand.ts	Implements lazy command dispatch.
packages/cli/src/lib/handleWorkingDirectory.ts	Defers RPC configuration loading.
packages/cli/src/cmds/sequenceDiagram.ts	Defers browser-serving dependencies.
packages/cli/src/cliCommands.ts	Declares the lazy command registry.
packages/cli/src/cli.ts	Registers lazy commands and defers depends helpers.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dividedmind]

I like it, impressive improvement! But please at least use this opportunity to move the depends command into its own file, dropping back the many requires needed to load this dynamically when its defined inline.

Couple nits/questions:

1. Would it be feasible to use dynamic import() instead? We will want to migrate to ESM at some point in the future and adding dynamic requires might make it more difficult.
2. I don't love the duplication of the command specs. While there is a test that guards against drift, I wonder if it wouldn't be cleaner to drop the specs from command modules entirely (though this might make it easier to forget to update them when needed) or have the command index autogenerated (although that would add a build step).
3. It has always bothered me that we aren't passing the command modules through yargs in type-preserving manner (instead, by using top-statement level parser.command();, we were throwing away the return type). This change doubles-down on this limitation, by hiding the builders behind dynamic loads and erasing the types entirely with AnyCommand.
   This might not matter much in practice except from a type discipline viewpoint (it's not really used downstream except to invoke the parser), but it's my belief that whenever you need to fight the type system like this it points to a possible design flaw (or at least, room for improvement).
   Indeed, in this case this suggests a different, IMO cleaner approach (even if needing slightly more changes): instead of loading the full file lazily, why not load only the handlers (spun off to separate files) on demand, and leave the command files loaded eagerly? This will allow the toplevel yargs to see the command shape entirely, including the builders, allowing type-preserving loading, and making appmap <command> --help fast too (because it won't need to load the handler module). This will also dissolve my point 2 above entirely because there will be no duplication and nothing to check and drift.
   Or, combine it with the first suggestion in 2. above, and have one file that has all the specs – including the builders! – and leave the command files as just the handlers. Note preserving type safety in the handler arguments might be tricky in this case – for type safety, the handler argument type needs to be derived from the builder. This is not as useless as the top-level command parser typing, since it makes the typechecker catch any drift in the argument shapes between the handler and the builder (and by extension the real command line as parsed and documented in --help).

[kgilpin]

Thanks for the review. 8a70825 does what you suggested in point 3: the command files are loaded eagerly, each handler lives in its own module and is loaded with a dynamic import() on dispatch (lib/lazyHandler), depends has its own files, and the registry, the AnyCommand stub and the drift test are gone. That dissolves point 2, and point 1 is covered by using import() throughout.

One thing worth flagging about the type-preserving registration.

yargs's command(module) overload types the module as CommandModule<T, any>, so it never checked a handler against its builder; that is why the old require-based cli.ts never complained. The overload that does preserve the link is command(name, describe, builder, handler), where the handler's argument type is inferred from the builder. So cli.ts now registers each module through a small helper, lib/registerCommand.ts, that calls that overload. With it, a handler whose arguments disagree with the options its builder declares fails to type-check, which is the drift check you described.

That check immediately flagged three commands: search, install and status. Their handlers declare hand-written argument interfaces (for example directory: string required) that their builders, which declare options as statements without chaining, do not provide. Rather than retype those builders and handlers in this PR, each handler now takes what yargs actually provides and casts to its interface, with a comment saying the shape is assumed rather than derived. Deriving those types from the builders, along with the fifteen handlers that still take argv: any, is the follow-up PR.

Two smaller notes from the same change:

• The install, status and compare-report builders need values from modules that are expensive to load (the installer list alone costs about 100 ms), so those builders are async and import them on demand. yargs 17 awaits async builders, and the parse(cmd, {}, callback) form the tests use fires after they resolve.
• A yargs handler returns nothing, so the four tests that read a handler's return value (stats, prune, record, rpc-client) now call the handler module directly.

A new test, tests/unit/cliStartup.spec.ts, runs the real entry file under ts-node and fails if any heavy package is loaded for --version or for a command's --help. Help output is unchanged for every command, and the eager command modules add about 4 ms to startup.

[dividedmind]

Thanks, this is what I had in mind! Re type-checking, you're right, it's been a moment since I last touched it so I didn't remember exactly how it worked. It seems like some commands (such as apply) do derive handler argument shape from builder via a HandlerArguments type template, while some others use type erased generic yargs.ArgumentsCamelCase or indeed any type for the handler argument (and some others yet, as you noted, didn't even try to have the types compatible; btw, perhaps a better solution for these would be to change the builders to thread the options if this would make the correct type fall out); it would be good to harmonize it across the board and maybe extract the common template for handling this, but perhaps this is ok to leave this as a future improvement. At least the registerCommand links the types together now and checks if the connection is valid. There are some more opportunities for refactoring if there is appetite (for example, the three commands you flagged as needing more expensive imports in builders just use them to fetch what are essentially some string lists – these could perhaps be refactored to extract those lists to common lighter files), but I'm wary of the scope creep – let's get this merged.