|
| 1 | +--- |
| 2 | +phase: implementation |
| 3 | +title: Implementation Guide |
| 4 | +description: Technical implementation notes, patterns, and code guidelines |
| 5 | +feature: cli-startup-performance |
| 6 | +--- |
| 7 | + |
| 8 | +# Implementation Guide |
| 9 | + |
| 10 | +## Development Setup |
| 11 | + |
| 12 | +- Work in branch/worktree `feature-cli-startup-performance`. |
| 13 | +- Run `npm ci` in the feature worktree before phase work. |
| 14 | +- Run `npm run build` before benchmark commands so measurements use `packages/cli/dist/cli.js`. |
| 15 | + |
| 16 | +## Code Structure |
| 17 | + |
| 18 | +Expected touch points: |
| 19 | + |
| 20 | +- `packages/cli/src/cli.ts`: root bootstrap and command registration. |
| 21 | +- `packages/cli/src/commands/*.ts`: command metadata/action split and lazy handler imports. |
| 22 | +- `packages/cli/src/__tests__/commands/*.test.ts`: command behavior regression tests. |
| 23 | +- `e2e/`: built CLI smoke coverage if bootstrap/build changes affect published behavior. |
| 24 | +- CI workflow files if benchmark gate is added there. |
| 25 | + |
| 26 | +Current implementation deltas: |
| 27 | + |
| 28 | +- `packages/cli/src/util/cli-benchmark.ts`: local startup benchmark utility and executable built script entrypoint. |
| 29 | +- `packages/cli/src/__tests__/util/cli-benchmark.test.ts`: TDD coverage for timing stats, failure accounting, required benchmark case list, and built-script root resolution. |
| 30 | +- `packages/cli/src/cli-command-manifest.ts`: shared lightweight top-level command manifest used by static help and lazy dispatch. |
| 31 | +- `packages/cli/src/__tests__/util/cli-command-manifest.test.ts`: coverage proving the manifest drives root help, command help, and dispatch paths. |
| 32 | +- `packages/cli/src/cli-runtime.ts`: lightweight static help rendering and lazy top-level command execution. |
| 33 | +- `packages/cli/src/__tests__/util/cli-runtime.test.ts`: TDD coverage for lightweight help/version, dispatch mapping, and lazy command registration. |
| 34 | +- `packages/cli/src/cli.ts`: thin bootstrap that handles static help/version and delegates real commands to the lazy dispatcher. |
| 35 | +- `packages/cli/package.json`: `benchmark:startup` script running `node dist/util/cli-benchmark.js`. |
| 36 | +- `.github/workflows/ci.yml`: CI benchmark step after build. |
| 37 | + |
| 38 | +## Phase 6 Implementation Check |
| 39 | + |
| 40 | +Alignment with the design: |
| 41 | + |
| 42 | +- The root entrypoint now imports only package metadata plus lightweight bootstrap/dispatch helpers before command selection. |
| 43 | +- Root `--version`, root `--help`, and top-level command `--help` paths are served from static metadata and do not load the previous heavy command graph. |
| 44 | +- Real command execution imports only the selected top-level command module before Commander parsing. |
| 45 | +- Unknown command routing uses a lightweight Commander program populated from the shared manifest, preserving the existing unknown-command error without eager command-module imports. |
| 46 | +- The startup benchmark runs locally and in CI after build, with the `<50 ms` p50 gate enforced for version/help paths. |
| 47 | + |
| 48 | +Deviations and follow-ups: |
| 49 | + |
| 50 | +- Static help metadata duplicates command names/descriptions and selected option metadata. This is the main drift risk versus Commander-generated help and should be reviewed when commands change. |
| 51 | +- Lazy loading is currently at the top-level command group boundary. Heavy subcommand-specific dependencies inside groups such as `agent` and `channel` can be split further later, but this was not required to meet the startup/help target. |
| 52 | +- Representative real commands are smoke-measured in the benchmark table, but CI does not enforce a `10%` real-command regression threshold because there is no stored baseline in this implementation. |
| 53 | + |
| 54 | +## Phase 8 Code Review Notes |
| 55 | + |
| 56 | +- Reviewed the lightweight help metadata against real command registration. Fixed two public help parity gaps found during review: option-bearing command help now includes command-specific flags for `init`, `setup`, `lint`, and `install`; `channel --help` now includes `stop [name]`. |
| 57 | +- Refactored the runtime to expose `registerSelectedCommand` for direct branch coverage, while keeping `runSelectedCommand` as the CLI entrypoint dispatch API. |
| 58 | +- Verified exported helper usage with `rg`: new APIs are referenced only by `cli.ts`, tests, benchmark script entrypoint, and feature docs. |
| 59 | +- No new runtime dependencies, config keys, migrations, or irreversible state changes were introduced. |
| 60 | + |
| 61 | +## Simplification Pass |
| 62 | + |
| 63 | +- Consolidated top-level command metadata into `cli-command-manifest.ts`, so adding or changing a top-level command has one lightweight metadata entry used by both help rendering and dispatch resolution. |
| 64 | +- Removed the source `cli-full.ts` eager fallback. Unknown command handling now builds a lightweight Commander shell from the manifest, avoiding a second full command graph. |
| 65 | +- Consolidated `cli-bootstrap.ts` and `cli-dispatch.ts` into `cli-runtime.ts`, keeping one runtime module for help rendering and lazy command execution. |
| 66 | +- Updated the CLI entrypoint to fast-path `--version` before importing runtime code. |
| 67 | +- Added manifest tests to guard against future drift between root help, command help, and dispatch. |
| 68 | + |
| 69 | +## Implementation Notes |
| 70 | + |
| 71 | +### Core Features |
| 72 | + |
| 73 | +- Keep root bootstrap lightweight. Avoid top-level imports of heavy command modules. |
| 74 | +- Keep help-visible command metadata available without importing handler dependencies. |
| 75 | +- Import command handlers dynamically only when a command action actually runs. |
| 76 | +- Implement static command metadata plus lazy dispatch as the first optimization step. |
| 77 | +- Introduce generated or bundled `dist` output only after benchmarking proves the first step misses the `<50 ms` target. |
| 78 | +- If generated or bundled output is introduced, keep the source architecture explicit and testable. |
| 79 | + |
| 80 | +### Patterns & Best Practices |
| 81 | + |
| 82 | +- Prefer small registration helpers over broad abstractions unless generated metadata becomes necessary. |
| 83 | +- Preserve existing `withErrorHandler` behavior around async command actions. |
| 84 | +- Keep command handler functions exported for direct unit testing. |
| 85 | +- Do not introduce new dependencies. |
| 86 | + |
| 87 | +## Integration Points |
| 88 | + |
| 89 | +- `packages/cli/package.json` `bin.ai-devkit` must remain compatible. |
| 90 | +- `packages/cli` build must continue copying `templates` into `dist/templates`. |
| 91 | +- `channel-daemon` launch logic must still resolve dev and built paths correctly. |
| 92 | +- Existing package imports from `@ai-devkit/agent-manager`, `@ai-devkit/memory`, and `@ai-devkit/channel-connector` should move behind lazy boundaries where possible. |
| 93 | + |
| 94 | +## Error Handling |
| 95 | + |
| 96 | +- Lazy import failures should surface as command failures with the same error handling conventions as existing commands. |
| 97 | +- Benchmark failures should print the failing command, p50, threshold, iterations, and failed process count. |
| 98 | +- Generated build failures should fail `npm run build` clearly. |
| 99 | + |
| 100 | +## Performance Considerations |
| 101 | + |
| 102 | +- Optimize for fresh process startup, not long-lived process warm paths. |
| 103 | +- Avoid unnecessary JSON/config/file reads before command selection. |
| 104 | +- Avoid loading TUI/React/Ink unless `agent console` runs. |
| 105 | +- Avoid loading memory database code unless a memory action runs. |
| 106 | +- Avoid loading Telegram/channel bridge code unless channel actions requiring them run. |
| 107 | + |
| 108 | +Benchmark foundation evidence: |
| 109 | + |
| 110 | +- `npm test -w packages/cli -- src/__tests__/util/cli-benchmark.test.ts` passed with 4 tests. |
| 111 | +- `npm test -w packages/cli -- src/__tests__/util/cli-runtime.test.ts src/__tests__/util/cli-command-manifest.test.ts src/__tests__/util/cli-benchmark.test.ts` passed with 18 tests after the final simplification pass. |
| 112 | +- `npm run build` passed for all 4 projects. |
| 113 | +- `AI_DEVKIT_CLI_BENCHMARK_ITERATIONS=1 npm run benchmark:startup -w packages/cli` executed all 15 configured cases with `0` failures. This smoke run captures current unoptimized startup timings around `325-680 ms`, confirming the benchmark exposes the baseline regression target. |
| 114 | +- After lightweight bootstrap, `npm run benchmark:startup -w packages/cli` with 20 iterations produced `0` failures. Startup/help p50 values were `24.070-25.226 ms`; `--version` p50 was `25.080 ms`. |
| 115 | +- After top-level lazy dispatch and CI gate wiring, `npm run benchmark:startup -w packages/cli` with 20 iterations exited `0`. Startup/help p50 values were `29.391-33.132 ms`; real command smoke p50 values were `75.028 ms` for `lint`, `239.437 ms` for `agent-list-json`, and `153.793 ms` for `memory-search`. |
| 116 | +- After the simplification pass, `npm run benchmark:startup -w packages/cli` with 20 iterations exited `0`. Startup/help p50 values were `24.085-25.149 ms`; real command smoke p50 values were `70.889 ms` for `lint`, `227.256 ms` for `agent-list-json`, and `149.253 ms` for `memory-search`. |
| 117 | +- After moving runtime modules next to `cli.ts`, `npm run benchmark:startup -w packages/cli` with 20 iterations exited `0`. Startup/help p50 values were `24.290-26.318 ms`; real command smoke p50 values were `71.420 ms` for `lint`, `255.848 ms` for `agent-list-json`, and `149.797 ms` for `memory-search`. |
| 118 | + |
| 119 | +## Security Notes |
| 120 | + |
| 121 | +- Benchmark scripts must not read or print secrets. |
| 122 | +- Channel and memory smoke tests should use temporary or project-isolated config paths. |
| 123 | +- No Telegram tokens, tmux sessions, or external network calls should be required in CI. |
0 commit comments