fix(help): a command's page should say what that command does - #911
Merged
Conversation
The first thing dogfooding turned up. communique keeps `src/command_effects.rs` — two hundred lines of a table keyed by command path, plus its own tests to keep the table in step with the CLI — for one reason: clap cannot say `effect`, so the classification is applied to the generated spec afterwards. mise has the same file for the same reason. Declared beside the command now. `#[usage(effect = "read")]` on an `Args`, and on a flag where the flag is what changes the answer: `communique generate` only prints, and `communique generate --github-release` writes. A table keyed by command path cannot say that at all, which is why the flag half of communique's file exists as a second table keyed by (command, flag). Unsaid stays distinguishable from `read`. A consumer treats the absence as "ask", so it has to survive as `None` rather than collapsing into the safe answer. Refused on the root, beside `mount` and `restart_token`: bare `communique` does nothing to the world, one of its commands does, and the spec writer already asserts it — so accepting it here would trade a message for a `debug_assert!` inside the writer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three from review, one shape: declared, stored, and silently dropped. `effect` on a positional. `arg_meta` has no field for one, so it compiled and vanished. A positional is the thing being acted on rather than a choice to act, which is why the metadata has nowhere to put it — refused with that as the reason. `effect` on a `#[usage(subcommand)]` field. That branch looked for `subcommand` and *ignored* everything else written beside it, so the declaration was never even parsed. Refused as a class rather than one option at a time: the field holds a set of commands, and everything the attribute can otherwise say describes a value or a flag. `long`, `default` and `global` on a subcommand field were equally quiet before this. And the docs table said `effect` describes a flag, when it also goes on an `Args` — where it says what *running* the command does, which is the half communique needed most. Found by Cursor Bugbot, greptile and CodeRabbit, which all three flagged the positional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… can read the spec The other two gaps from the communique dogfood. **A value is named after its flag, shouted.** clap prints `--max-tokens <MAX_TOKENS>` and `<TAG> [PREV_TAG]`; the derive printed `<max-tokens>` and `<tag>`. A visible change in `--help` for a CLI that changed nothing, which is the one thing this crate is trying not to do. From the *form* rather than the field, which the existing rule already had right and clap agrees with: `#[usage(long = "type")] type_` renders `--type <TYPE>`, not `<TYPE_>`. Underscores restored on the way, since the flag is kebab to type and snake to read — all three shapes measured from clap 4 rather than remembered. Set in the derive rather than in the renderers, so the metadata says what help prints. Two fallbacks would be two answers, and the spec is what docs and completions read. **`min_usage_version` is declarable and emitted first**, before anything an old `usage` would choke on. Declared rather than worked out: computing it means a table from every property to the version that introduced it, kept in step by hand, and such a table rots into a spec that claims to be readable by a `usage` that chokes on it. communique writes the line by hand today for exactly this reason. Not done, because it is not a gap: the spec-level `usage "Usage: …"` line that `clap_usage` emits. usage-lib parses it, carries it into the docs model, and renders it nowhere — commands compute their own. Emitting it would be replicating a field no reader has. The gate's help parity over mise's 211 commands still holds, which is what says the placeholder change did not cost fidelity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…m per spec Two from review. A short-only flag names its value before the shouting default runs, so `-j <jobs>` sat beside `--jobs <JOBS>` — one CLI printing a placeholder two ways, and neither of them what clap prints. Measured: clap gives `-j <JOBS>`. And `min_usage_version` on an `Args` was parsed, stored, and dropped, because only the root emits a spec. Refused where it is written, beside the other root-only options. Found by Cursor Bugbot and greptile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…m clap Found by porting communique. Declaring a version put it in the spec and nowhere a user could reach it: `communique --version` printed `error: unexpected argument '--version'` where clap printed `communique 1.3.1`. A straight regression rather than a trade, so it is the first of the dogfood findings to fix. Supplied by the parser and *not* listed in help, exactly as `--help` is: a spec declares neither, and a page listing a flag its spec does not declare disagrees with the spec it was rendered from. That is this crate's existing answer for `--help`, and there is no reason for the two to differ. Three rules, measured from clap 4 rather than remembered: both spellings answer, the root only, and a CLI that declares no version gets no flag — `--version` answering with nothing is worse than not having one. The root only is a *field* on the table rather than a rule about depth, so a CLI wanting clap's `propagate_version` has somewhere to say so later. Where a CLI declares one of the spellings itself, the declaration wins and the other still answers: `-V` for `--verbose` leaves `--version` working, and a `--version <V>` that takes a value leaves `-V` working. clap refuses that collision by panicking at startup and telling you to disable its flag; nothing has to break here, and the rule is the one `--help` and `-h` already follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The second thing porting communique turned up. `parse()` rendered a failure with `{:?}` —
`UnknownFlag { token: [45, 45, 110, 111, 112, 101] }` — while the clap-shaped rendering
sat in this crate unused. So communique's `main.rs` hand-rolled thirty-four lines to
reach it, which is not a thing an adopter should have to work out.
`parse()` is the entry point that *is* the process: it already printed a help page and
exited. It now prints the message to stderr and exits 2, which is clap's status, so a
script that checks for it keeps working. `parse_from` still hands the error back, for a
library embedding a CLI that wants to decide.
The renderer is reached through `render_failure` in usage-argv rather than by generating
a `#[cfg]`. That is not a stylistic choice: whether the good rendering exists is a
feature of *usage-argv in the adopter's graph*, and a `cfg` written into generated code
is evaluated in the adopter's crate, where the feature is not theirs to see — which is
exactly how a metadata field once got silently dropped. Without `diagnostics` the same
function gives the Debug form, which is what a parser-only build asked for.
With this and the commit below it, communique's port matches clap on `--version`, `-V`,
every error message tested, and every exit code — and `main.rs` is thirty-four lines
shorter than the port needed yesterday.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change separates root help metadata from subcommand descriptions. It also populates complete nested command paths in generated specifications and adds conformance coverage for help output and argument parsing. ChangesCLI help rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR makes subcommand help describe the command being invoked while preserving the root program description and adds coverage for the behavior; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThe PR changes root and subcommand help presentation, adds ancestry initialization for clap-derived specs, and expands compiled parser metadata and conformance coverage.
Confidence Score: 4/5The PR does not yet appear safe to merge because programmatically built nested commands can still be rendered as root pages. The clap conversion now initializes command ancestry, but Files Needing Attention: lib/src/spec/mod.rs, lib/src/spec/builder.rs, lib/src/docs/cli/mod.rs Important Files Changed
Reviews (3): Last reviewed commit: "fix(spec): a clap-derived subcommand sho..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes. Shadow comparisonParsing
|
`render_failure` styles for the terminal it finds itself in, so under `CLICOLOR_FORCE=1`
— or any TTY — the assertion read
\x1b[1m\x1b[31merror:\x1b[0m unexpected argument '\x1b[33m--nope\x1b[0m' found
and failed on a message that was perfectly correct. A test whose result depends on the
ambient terminal is a flake waiting for the machine that has one.
Stripped before reading. What this test is about is the wording; the colouring has tests
of its own. Checked green under a plain run, `CLICOLOR_FORCE=1` and `NO_COLOR=1`.
Found by Cursor Bugbot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`communique generate --help` printed
communique 1.3.1
Editorialized release notes powered by AI
Usage: communique generate [FLAGS] <TAG> [PREV_TAG]
and never once said what `generate` does — which is the question that was asked. Every
page carried the *root's* banner and description, so the one thing a subcommand's help
is for was the one thing missing from it. clap prints the command's own description.
Changed in usage-lib and usage-argv together, so the gate still holds them byte-identical
over mise's 211 commands — the point of that gate is that two implementations agree, not
that either is frozen. Every usage-based CLI gets this, not only the ones using the
derive.
The root keeps its banner and the program's description: it has no command of its own to
describe, and a program's page is where a program introduces itself.
Found by dogfooding communique, and it is the first of several help differences worth
weighing — the rest are in the pull request rather than assumed here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`set_subcommand_ancestors` ran only on the KDL path, so every subcommand of a spec built from clap had `full_cmd` empty. Two things read it, and both were wrong: `SpecCommand::usage()` joins `full_cmd`, so a clap-derived subcommand's usage line came out with no command in it — `go`'s usage was the empty string. And the root check this branch adds asks whether `full_cmd` is empty, which classified every such subcommand as the root and gave it the program's banner — the exact thing the commit below is fixing. Reported by greptile against that check; the check is right and the data feeding it was not. One pass, run where the other one is, and it walks all the way down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jdx
force-pushed
the
agent/help-describes-the-command
branch
from
August 16, 2026 21:48
0b891c7 to
66dd3bc
Compare
Owner
Author
|
Confirmed, and the cause is one level below the check — fixed in 66dd3bc. I probed it rather than reasoning about it. For a spec built from clap:
One pass, run where the other one is, walking all the way down: let go = spec.cmd.subcommands.get("go").unwrap();
assert_eq!(go.full_cmd, ["go"]);
assert_eq!(go.usage, "go <SUBCOMMAND>");
let fast = go.subcommands.get("fast").unwrap();
assert_eq!(fast.full_cmd, ["go", "fast"]);
assert_eq!(fast.usage, "go fast");That fixes the misclassification you flagged, for The gate stays green — AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
jdx
force-pushed
the
agent/help-describes-the-command
branch
2 times, most recently
from
August 16, 2026 22:10
b0f7233 to
66dd3bc
Compare
jdx
added a commit
that referenced
this pull request
Aug 17, 2026
PR 2 of the help plan, and the content loss that started it.
`communique generate` accepts `--config`, `--verbose` and `--quiet` from
its root, and its page mentioned **none of them**. A flag a user can
type and cannot discover is the worst way for help to be wrong, and it
was true of every subcommand page in every usage CLI.
```
Flags:
--base-url <BASE_URL> Base URL for the LLM API
-o, --output <OUTPUT> Write output to a file instead of stdout
Global flags:
-v, --verbose Enable verbose logging output
-q, --quiet Suppress progress output
-c, --config <CONFIG> Path to config file (default: communique.toml in repo root)
```
**Under a heading of their own** — this is where the design differs from
clap on purpose, per your call. clap mixes them into the one list;
`--config` belongs to the program rather than to `generate`, and a
reader should be able to see which is which. One column across both
sections, so the page reads as one table with a rule through it rather
than two that happen to be adjacent.
## Shadowing, which I did not plan for and which the first version got
wrong
`in_scope` chains a command's own flags before its ancestors' and takes
the first match, so `mise use --raw` is *use's* `--raw` and never the
root's. My first version listed both:
```
--raw Connect backend install command stdin/stdout/stderr directly to the terminal
...
Global flags:
--raw Read/write directly to stdin/stdout/stderr instead of by line
```
Two descriptions for one spelling, one of which can never apply. Decided
nearest-ancestor-first and emitted root-first, which is the order a
reader meets them walking down. `mise use` now lists `--raw` once, with
the description that will actually be used.
**The root grows no such section**: its flags are its own, `global` or
not, and there is nothing above it to inherit from.
## What made it possible
`find` walked the tree and **threw the chain away**, returning only the
command — so neither renderer had ancestors to ask. It returns the chain
now. That also fixes the help half of the pointer-identity bug I
reported on the diagnostics PR: a `Subcommands` type mounted under two
parents is one `Command` at one address, and a page rendered for the
second mount showed the first one's ancestry.
In usage-lib the chain comes from `full_cmd`, which is the typed path —
exact, with none of the ambiguity a search would have. (That field only
became reliable for clap-derived specs in #911.)
## Verification
| mutation | result |
|---|---|
| globals not listed at all | FAILED |
| shadowing dropped | FAILED |
| non-`global` flags inherited too | FAILED |
| the root grows a global section | FAILED |
Gate green over mise's 211 commands, so both renderers moved together.
Workspace suite green, clippy clean.
*AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5;
version: unavailable.*
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Help output changes for every subcommand page and shadowing must stay
in sync between usage-argv and usage-lib; behavior is heavily tested but
user-visible help text will differ widely.
>
> **Overview**
> Subcommand **`-h` / `--help`** now lists flags inherited from
ancestors (only those marked **`global`**) under a **Global flags**
heading, separate from the command’s own **Flags** section, with one
shared column width across both.
>
> **`help::find`** returns the full **`CommandMeta` ancestry chain**,
not just the leaf command. **`short_help`**, **`long_help`**,
**`render`**, and completion use that chain so inherited globals and
correct ancestry (e.g. shared subcommand mounts) are available
everywhere.
>
> Inherited entries follow **parser shadowing**: nearer commands win on
duplicate spellings; hidden flags still reserve spellings; partial
multi-long/short survival; negations vs longs resolved like
**`long_flag`** (any long in scope before negations). **`usage-lib`**
mirrors the same logic in **`inherited_flags`** and Tera templates so
gate tests stay aligned with usage-argv.
>
> Call sites and tests pass explicit chains (e.g. **`&[spec.root,
config, set]`**); new **`conformance/tests/global_flags.rs`** covers
listing, deduplication, and edge cases.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
5aab55a. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Subcommand help now displays inherited global flags in a dedicated
“Global flags” section.
- Global flag details—including aliases, descriptions, choices, and
environment variables—appear consistently in short and long help.
- Help output respects hidden flags, aliases, negations, and subcommand
overrides.
- **Bug Fixes**
- Improved metadata resolution for nested commands.
- Help layouts now maintain consistent columns and wrapping across local
and inherited flags.
- **Tests**
- Added comprehensive coverage for inherited flags, visibility,
precedence, and nested command help.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of the help findings from the communique dogfood, and the clearest.
communique generate --helpprinted this:and never once said what
generatedoes. Every page carried the root's banner and description, so the one thing a subcommand's help is for was the one thing missing from it. clap prints the command's own description there.After:
The root keeps its banner and the program's description — it has no command of its own to describe, and a program's page is where a program introduces itself.
Changed in usage-lib and usage-argv together, so the gate still holds them byte-identical over mise's 211 commands. The point of that gate is that two implementations agree, not that either is frozen; and doing it this way means every usage-based CLI gets the fix, not only the ones using the derive.
Verification
every_short_help_matches_the_referenceandevery_long_help_matches_the_referencestill green over mise's whole spec. Workspace suite green, clippy clean.The rest of the help differences, for your call
Measured on the same page, clap vs usage. I have not acted on these — they are judgement calls about what is best rather than bugs, and you said you are not tied to either.
1. Global flags are missing from subcommand pages.
communique generateaccepts--config,--verboseand--quiet; its help lists none of them. clap lists all three. This is the other content loss and I think it should be fixed — a flag a user can type and cannot discover is the worst case.2. Long forms do not align. clap indents so every long form starts at the same column:
we print both at column 2:
clap's is easier to scan. This is the one I would change next.
3. No comma between short and long.
-n --dry-runvs clap's-n, --dry-run. The comma is near-universal.4.
-h, --helpis not listed (nor--version, per #909). The existing reasoning is that a spec declares neither, so listing one would make the page disagree with the spec — sound for the spec, but help is for people, and clap lists both. Worth revisiting.5.
[FLAGS]vs clap's[OPTIONS]. Cosmetic;OPTIONSis the commoner convention and what clap 4 settled on.Where we are already better: our wrapping. clap runs to 100+ columns and lets the terminal break lines mid-word; we wrap with a hanging indent, which is much better on a narrow terminal. I would keep that.
Say which of 1–5 you want and I will do them in order; 1 and 2 are the two I would pick.
AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.
Note
Low Risk
User-visible help text only; behavior aligns with clap and keeps usage-argv and usage-lib in sync. Clap conversion fix is correctness for derived specs, not security-sensitive.
Overview
Subcommand
--help/-hno longer repeats the root name, version, and programabout; it leads with that command’s own short or long description (clap-style). The root page still shows the banner and program intro.The same rules are applied in usage-argv (
short_help/long_help) and usage-lib (Tera templates get arootflag from emptyfull_cmd).Clap →
Specconversion now runsset_subcommand_ancestorslike KDL parsing, so nested commands get correctfull_cmdand non-empty usage strings (e.g.go fastinstead of a blank command path).Conformance tests cover subcommand vs root help; a unit test covers clap-derived subcommand paths.
Reviewed by Cursor Bugbot for commit b0f7233. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
--env.