Skip to content

Collapse top-level CLI help into group rows with registry-backed sub help#284

Merged
platypii merged 2 commits into
masterfrom
cli-help-collapse
Jul 8, 2026
Merged

Collapse top-level CLI help into group rows with registry-backed sub help#284
platypii merged 2 commits into
masterfrom
cli-help-collapse

Conversation

@platypii

@platypii platypii commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

hyp --help was listing all 47 commands flat, and group help like hyp query --help showed bare subcommand names with no descriptions.

  • Top-level help now renders one row per command group (19 rows); subcommand summaries moved into group help.
  • New shared renderer (src/core/cli/group_help.js) builds group help from the command registry, so top-level and group help cannot drift.
  • Dispatch intercepts a leading --help for every command: group table when the command has subcommands, otherwise summary + usage + optional long help (new CommandRegistration.help field, used by ignore, unignore, join, leave, remote login).
  • Bare group commands (query, daemon, sink, remote, config, plugin, agents, skills) come from one factory; plugin groups without a bare command get a synthesized subcommand listing.
  • Design documented in LLP 0009 (Layered help, Central help interception).

Also includes dependency bumps in package.json (@aws-sdk, squirreling).

Tests: 2122 pass, typecheck clean, command_dispatch and core_boot_noop smokes pass.

…help

One row per command group at the top level; 'hyp <group> --help' now
lists each subcommand with its registered summary. Dispatch intercepts
a leading --help for every command (group table or usage + optional
long help), so command bodies stay help-free. Documented in LLP 0009.

Also includes dependency bumps in package.json (@aws-sdk, squirreling).
@philcunliffe philcunliffe added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Jul 8, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

neutral review round 1 - cli-help-collapse

Head reviewed: 238ebc7f0cb40d6cf6ce893c9e77ae4a424f206e
Recommended verdict: approve (one minor process note handed back; no code defects, nothing pushed)

Reviewed as an adopted contributor PR (neutral:adopt). Clean, well-structured change: top-level hyp --help collapses to one row per command group, a new registry-backed renderer (src/core/cli/group_help.js) makes top-level and group help share one source of truth, and dispatch centrally intercepts --help. The design is documented in the same diff via the new LLP 0009 sections, and the @ref annotations resolve.

Verification performed

  • Full suite green: npm test = 2122 pass / 1 pre-existing skip; test/core/command-dispatch.test.js = 24/24.
  • npx tsc --noEmit clean (exit 0). New optional CommandRegistration.help field is backward-compatible.
  • No dangling importers of the five removed help functions (runQuery, runConfig, runDaemonHelp, runSinkHelp, runRemoteHelp); the runQuery grep hits are unrelated file-local test/smoke helpers.
  • group_help.js logic sound: listGroupChildren prefers a registered child summary over a synthesized one, sorts, skips hidden, handles grandchild-only children; padEnd width has a safe 8 fallback on empty input; makeGroupCommand reads the registry at run time so late-registered plugin subcommands appear.
  • Dispatch --help interception correctly distinguishes group (has children) vs leaf commands; matched.rest[0] is the right position; no in-repo command uses -h as a non-help short flag, so nothing legitimate is swallowed.
  • @ref LLP 0009#layered-help and #central-help-interception resolve to the new section headers; pre-existing #core-owns-dispatch still resolves.
  • Style: no semicolons and no U+2014 em dashes in any newly added line (code, .d.ts JSDoc, or the new LLP prose). Pre-existing em dashes in older parts of 0009 were left untouched.

Reviewers

  • Codex: ran; no new issues.
  • Claude (bug scan): no issues found.
  • Claude (guidance + contract): clean on contract/code; raised the process note below.

Findings

F1 (minor, process — handed back to maintainer, NOT a code change request).
llp/0009-cli-registry.spec.md is Status: Active, and this PR adds two substantive new design sections to it ("Layered help", "Central help interception", ~38 lines). The house rule for Accepted/Active LLPs is "a record, not a worksheet - do not edit what it decided... trivial editorial fixes are fine", with non-trivial intent changes prescribed as a new request (spec/rfc) that @refs the old doc. This is debatable rather than clear-cut: the new sections refine the adjacent "Core owns dispatch" / "Top-level help" material of the same spec, so they read as clarifying-in-place rather than overturning a prior decision. Because this is a maintainer-authorization judgment (not something to rewrite on the contributor's behalf), it is surfaced here for the maintainer to accept the in-place edit or ask for a follow-on spec. Not a blocker.

Nothing was pushed to the branch. No actionable code defect at review confidence.

Central `--help` interception (this PR) renders the registration `usage`
for a leaf command instead of running the command body. `runDaemonInstall`
still supports `--bin <path>`, and its now-bypassed self-help documented it,
but the registration `usage` omitted it, so `hyp daemon install --help`
silently dropped the flag. Add `--bin <path>` to the registration usage so
the primary help path matches what the command accepts, plus a regression
test asserting the flag stays documented.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor

neutral round 2 review, PR #284 (adopt), reviewed head 238ebc7

Verdict: findings. One real (minor) code defect stood at the reviewed head; I fixed it as an additive commit and pushed it. F1 (the LLP 0009 in-place edit) is judged an acceptable clarify-in-place refinement, so it is a maintainer FYI, not a requested change. Recommended verdict once the new head is confirmed: approve (held for the maintainer; neutral does not merge adopted PRs).

Both reviewers ran this round: the Claude-side review plus Codex (gpt-5.5). Codex was intermittent because HypAware intercepts /backend-api/codex/responses traffic, but it completed and independently surfaced the finding below.


Code finding, FIXED and pushed

F2 (defect, minor, fixed): hyp daemon install --help silently dropped the --bin <path> flag.
Central --help interception (this PR's design) renders a leaf command's registration usage instead of running the command body. runDaemonInstall still accepts --bin <path>, and its now-bypassed self-help documented it, but the daemon install registration usage omitted it. So the primary help path lost a supported flag that master's hyp daemon install --help used to show.

Fix pushed as an additive commit (4397f1a, non-force): added [--bin <path>] to the daemon install registration usage so central help matches what the command accepts, plus a regression test asserting the flag stays documented. npm test 2122 pass / 0 fail / 1 skip and tsc --noEmit clean at the new head.

I checked every other leaf command that still self-handles --help for the same drift: join was correctly migrated to the new CommandRegistration.help field (content preserved), and plugin doctor / plugin new / daemon uninstall / daemon start self-help already matches their registration usage. daemon install was the only content regression.

FYI (not changed): the migration leaves several now-dead if (parsed.help) / --help branches in command bodies (runDaemonInstall, runJoin, runVerbCommand, daemon uninstall/start, plugin doctor/new). They are unreachable under central interception and harmless; a follow-up could prune them, but that is cleanup, not a defect.

Rest of the change, clean

  • Shared renderer (src/core/cli/group_help.js): top-level help, group help, and central --help all read the live registry, so they cannot drift. Grandchild synthesis handled.
  • Top-level hyp --help: one row per token; a bare command that also has children (query, daemon, backfill) keeps its own summary (the if (!rows.has(head)) guard prevents a synthesized summary clobbering a real one); a group with no bare command (graph) gets Subcommands: a, b, c.
  • Group-vs-leaf split in dispatch.js: isHelpFlag(matched.rest[0]) fires only when the help flag is first after the fully matched command; help still runs inside the command.run span, preserving the analytics behavior the @ref claims.
  • The five removed per-command help functions have no dangling importers anywhere. @ref anchors (#layered-help, #central-help-interception) resolve.

F1: LLP 0009 in-place edit, acceptable clarify-in-place (FYI, not a requested change)

The PR adds two ### subsections ("Layered help", "Central help interception") to llp/0009-cli-registry.spec.md, which is Status: Active. Weighed against the house rule ("Immutable docs; change is a new request... do not edit what it decided or required"):

  • Purely additive. No existing sentence is edited, deleted, or contradicted; both subsections land under the existing ## Core owns dispatch parent, which already governs "dispatch and help assembly".
  • Reinforces, not reverses, the settled invariant. 0009 already states core owns dispatch and renders help by asking the registry. Central --help interception makes core own more of help, which is more faithful to that invariant, not a reversal of it.
  • Single canonical home. This is literally the CLI dispatch-and-help spec; splitting "how help renders" into a separate request LLP would fragment one coherent spec for no corpus benefit, and the repo's own "living docs, land the doc edit in the same commit as the code" convention contemplates exactly this.

On balance this is additive refinement of the same spec rather than rewriting settled intent, so it is not actionable and does not warrant asking the contributor to split it out.

Maintainer FYI (your call, not a blocker): the central-help-interception + collapse-to-group-rows model is a real design choice that supersedes the prior per-command-help pattern. If you want that decision individually citable per the "one small decision per chose-X-over-Y" house preference, you may prefer a short decision LLP that @refs 0009. That is a citation-granularity preference for you as author, not something the contributor should be asked to change.

Minor FYI (non-blocking): the same commit bundles an unrelated dependency bump in package.json (@aws-sdk/client-s3 3.1079.0 to 3.1081.0, @aws-sdk/credential-provider-ini 3.972.60 to 3.972.62, squirreling 0.13.0 to 0.14.0). Not a defect (no committed lockfile, CI green, tests green), just scope worth noting; keep or drop at your discretion.

@philcunliffe

Copy link
Copy Markdown
Contributor

neutral review - round 3 (confirmation)

Verdict: clean (recommend approve). Reviewed head 4397f1a3cfd29d66819904ae0ae1d4de9aae4ab2 in a detached worktree.

Round-2 fix confirmed

  • src/core/cli/core_commands.js:285: the daemon install registration usage now reads hyp daemon install [--config <path>] [--bin <path>] [--dry-run [--json]], matching what runDaemonInstall accepts and the now-bypassed self-help documented. Correct and complete.
  • The regression test test/core/command-dispatch.test.js ("a leaf subcommand --help documents every flag the command accepts") dispatches ['daemon','install','--help'] through the real central --help interception path and asserts /--bin <path>/. It genuinely pins the line. Full command-dispatch.test.js suite: 25/25 pass at this head.

No other leaf lost documented flags (round-2 failure mode re-swept)

I enumerated every registered leaf whose run body could self-handle --help and compared its in-body usage string against its registration usage/help:

  • daemon uninstall/run/start/stop/status/restart, query schema/status/refresh/maintain, config validate, plugin install/info/update/remove/doctor/new, sink force/maintain, mcp, backfill[ plan], remote add/remove, smoke, ignore/unignore, join/leave: each registration usage (plus new help fields where relevant) matches the in-body text. No flag omitted.
  • init is safe: --help is not in INIT_FLAG_NAMES, so bare hyp init --help never printed the full flag list before this PR (it fell through to the walkthrough). The full flag usage only surfaces for hyp init <realflag> --help, where central interception does not fire because rest[0] is not a help flag. No documented content was lost.

One cosmetic nit (not a defect, not requesting changes)

skills install and agents install register usage: hyp skills install [--client <name>] while the bypassed in-body help said [--client <name>|all]. The all value is still accepted and is the default, so no capability is lost - only the value hint is slightly less discoverable. Optional to add |all back for parity.

Settled items

  • F1 (contributor's additive edit to Active llp/0009) remains settled as acceptable clarify-in-place - maintainer FYI, not a requested change.
  • Scope note for the maintainer: this PR also bundles an unrelated dependency bump in package.json (squirreling 0.13->0.14 plus aws-sdk bumps). Awareness only, not a defect.

Codex did not run this round: the full dual-review machinery drives the main checkout and posts its own comment, both of which conflict with the adopt round constraints (worktree-only, exactly one marker-signed record), and Codex has been hanging here. Proceeded on an independent Claude-side diff read per the sanctioned fallback. No code was pushed - nothing actionable remained.

@philcunliffe

Copy link
Copy Markdown
Contributor

🤖 neutral: approved (adopted PR — held for maintainer merge)

Reviewed under neutral:adopt (LLP 0025, full-heal). Verdict at head 4397f1a: approve — mergeable, CI-green, reviewed clean. Held for a maintainer to merge; neutral does not merge or ready a contributor's PR.

Review summary (3 rounds)

  • Code: clean. The registry-backed shared help renderer (group_help.js), the dispatch.js --help interception, the optional CommandRegistration.help field, and the removal of the five redundant per-command help functions are all sound. No dangling importers; tsc --noEmit clean; npm test 2122 pass.
  • One real regression, found and fixed (Codex flagged, independently confirmed): central --help interception renders a leaf's registration usage, and runDaemonInstall's usage had omitted the supported --bin <path> flag, so hyp daemon install --help silently dropped it vs master. Fixed additively in 4397f1a (added [--bin <path>] + a regression test that dispatches daemon install --help and asserts the line). Round 3 swept every other migrated leaf command and confirmed no other documented flag was lost.

For your call at merge (non-blocking)

  • LLP 0009 edit: this PR adds two new ### subsections to the Active llp/0009-cli-registry.spec.md. Judged an acceptable clarify-in-place refinement (purely additive, no settled text edited/reversed, under the existing ## Core owns dispatch parent, single canonical home). If you'd rather it be individually citable, mint a small decision LLP that @refs 0009 — otherwise the in-place addition is fine.
  • Cosmetic nit (not fixed): skills install / agents install registration usage shows [--client <name>] while the old in-body help said [--client <name>|all]; all is still accepted (and default), just less discoverable. Optional to restore.
  • Dependency bumps: the PR bundles @aws-sdk + squirreling 0.13→0.14 bumps (disclosed in the PR description) — flagging for awareness since they're unrelated to the help change.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this adopted PR and approves; held for maintainer merge (LLP 0025) label Jul 8, 2026
@platypii platypii merged commit 74e6bfc into master Jul 8, 2026
4 checks passed
@platypii platypii deleted the cli-help-collapse branch July 8, 2026 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:adopt Foreign PR adopted into neutral's reconcile scope neutral:approved neutral reviewed this adopted PR and approves; held for maintainer merge (LLP 0025)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants