perf(cli): route command aliases through the help fast path - #1641
perf(cli): route command aliases through the help fast path#1641thymikee wants to merge 2 commits into
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Reviewed exact head |
9418f11 to
842469b
Compare
|
bin.ts's `--help` fast path resolved aliases through a hand-written two-entry table that had drifted out of sync with the real CLI_COMMAND_ALIASES registry (five entries). `tap`, `launch`, and `relaunch` missed the table and silently fell through to a full runCli() bootstrap just to print static help text (~150-165ms vs ~45-50ms for aliases already in the table). Delegate to the shared normalizeCliCommandAlias registry instead of the stale local table, so every alias the registry knows about gets the fast path automatically.
The unit test added for the alias fast-path fix (cli-help-alias-fast-path.test.ts) calls normalizeCliCommandAlias directly, so it stays green even if bin.ts itself reverts to a hand-rolled table — it pins the registry composition, not bin.ts's own wiring, and bin.ts cannot be safely unit-imported (it runs unguarded top-level dispatch on import and is deliberately excluded from coverage). Add an AST-based structural guard instead, in the style already established by scripts/layering/session-state.ts, facade-exports.ts, and zero-dep-jobs.ts (oxc-parser's module/program records, not a line scan, so a fixture's string literal can't produce a false hit). R12 asserts two facts about src/bin.ts: it holds a value import of normalizeCliCommandAlias from commands/cli-command-aliases.ts, and it contains none of the registry's own alias tokens as string literals. The token list is read out of the registry's own source (CLI_COMMAND_ALIASES's `alias:` property values), not hard-coded, so a future sixth alias is covered automatically. Both facts were false on the pre-fix bin.ts, verified by reverting locally and capturing the failure before restoring the fix. Wired into the existing check:layering chain (already part of check:tooling), next to R7's session-state ownership rule, which pins the same "delegate to your single owner" shape.
842469b to
7dc6f0f
Compare
|
Re-reviewed exact head
The current PR body is also stale relative to the six-file structural-guard diff. Current head is mergeable and completed checks are green, with required lanes still pending; no |
1 participant
Summary
agent-device <command> --helphas a fast path insrc/bin.tsthat prints static help without booting the full CLI. It resolved aliases through a hand-written two-entry table while the real registry (src/commands/cli-command-aliases.ts) has five, sotap,launch, andrelaunchsilently missed it and fell through to a fullrunCli()bootstrap just to print static text.It now delegates to
normalizeCliCommandAlias, so every alias the registry knows about gets the fast path automatically.Measured on the built CLI, 3 runs each, warmed:
--helppress(control)long-press(control, was in the old table)taplaunchrelaunchlong-pressvstapis the controlled comparison: both are aliases printing identical help, differing only by whether the stale table knew about them. Output is byte-identical to each alias's canonical command (diffclean on all four pairs).rotatedeliberately still misses the fast path, so its rename migration error keeps rendering through the slow path. No carve-out was added tobin.tsfor it — it simply isn't in the alias registry.Validation
pnpm check:affected --rungreen. The size report on the batch PR this was split from showedCLI --version29.4 → 28.4 ms, confirming the one static import added tobin.tscosts nothing measurable — tsdown inlines the alias table intobin.jsrather than emitting a chunk.On the regression test, one thing a reviewer should know.
bin.tsruns unguarded top-level dispatch on import and is excluded from coverage by design, so it cannot be imported in a unit test. The committed tests pin the registry compositionbin.tscalls, which makes them a durable guard against a future sixth alias lacking help text — but they are not a revert-pin on this specific bug, and revertingbin.tsdoes not fail them. The test file header says so explicitly rather than overclaiming.The genuine red/green proof was done out-of-suite via the module graph, since output is identical on both paths and only the loaded modules differ:
This was not added as a committed test: spawning real
nodesubprocesses inunit-corewould violate the suite's "unit tests must not wait real time" budget and has no precedent in the repo.Scope
2 files, +69/−7. Not device-facing, so no simulator/emulator evidence applies.
Split out of #1639 per review. Found by a read-only codebase audit.