feat(derive): compile subcommands from an enum

[jdx]

First of the next stack, and the largest gap in the derive: it could describe one command, so nothing with subcommands could use it — which is every CLI it is meant for.

Shape

#[derive(Cli)]
#[usage(bin = "ex")]
struct Ex {
    #[usage(short = 'v', long, global)]
    verbose: bool,
    #[usage(subcommand)]
    command: Option<Commands>,
}

#[derive(Subcommands)]
enum Commands {
    /// Install a tool
    Install(Install),
    /// Run a task
    #[usage(name = "run")]
    RunTask(Run),
}

/// Install a tool
#[derive(Args)]
struct Install {
    #[usage(short = 'f', long)]
    force: bool,
    tools: Vec<String>,
}

Two problems worth explaining, because they shaped the design

A macro sees one item. The three derives cannot share a counter or read each other's fields, so keys carry a hash of the type they came from in the high half, the index in the low half, and a tag for command/flag/argument. Independently expanded macros therefore cannot hand two fields the same key — which matters because a parse dispatches on it. Two type names would have to collide in 32 bits, and Spec::to_kdl now asserts the tree holds no duplicates, so a collision fails a test instead of binding the wrong field. A test checks that invariant directly.

A static cannot call a method. The parent splices its children's tables into its own static, so the traits expose them as associated consts, not functions. That keeps the tables static all the way down — nothing is assembled at run time, which was the whole point.

Restructuring that fell out

Every derived struct now collects into a generated Partial rather than into locals, since a subcommand's values cannot live in the root's parse function. That replaced the prefixed-locals machinery from #803 entirely, and made apply shared between a root and a subcommand.

A doc comment on a variant overrides the struct's, because that is where a reader of the enum expects to describe the command — ignoring it would lose the description silently. Overriding one field of the struct's metadata is possible in a const, so this costs nothing at run time.

Verified

Eight tests: routing by name and by a renamed variant, per-command fields with defaults, a global flag on either side of the command word, one command not answering to another's flag, an absent subcommand, key uniqueness across separately expanded types, and a snapshot of the emitted spec:

name "ex"
bin "ex"
flag "-v --verbose" help="Say more" global=#true
cmd "install" help="Install a tool" {
    flag "-f --force" help="Overwrite an existing install"
    flag "-j --jobs" help="How many at once" default="4" {
        arg "<jobs>"
    }
    arg "[tools]..." help="What to install"
}
cmd "run" help="Run a task" {
    arg "<task>" help="The task to run"
}

Nested cmd blocks that read like something a person would have written, which is what usage g markdown|manpage needs.

Stated limits

• A subcommand field must be Option<T>: reporting that one was required is a required-ness question, and that layer lands in the next PR of this stack.
• Nesting stops at one level — a subcommand's own subcommands need Args to carry a subcommand field too.

AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.

---

Note

Medium Risk
Large derive/codegen refactor plus a public key-type change (u32 → u64) that affects parse dispatch. Mitigated by conformance tests and debug duplicate-key checks, but wrong keys would bind the wrong fields.

Overview
Enables declaring subcommands with the derive: a #[usage(subcommand)] field holds an enum (Subcommands), and each variant wraps an Args struct for that command's flags and arguments.

Independently expanded macros cannot share a counter, so keys widen from u32 to u64 and carry a type-name hash. New CommandArgs / Subcommands traits expose associated consts so parents splice children's tables into static parse tables with no runtime assembly. Spec::to_kdl debug-asserts the tree has no duplicate keys.

Codegen now accumulates into a generated Partial (shared apply) instead of parse-local variables, which is what lets a root route events into subcommands it cannot see. Variant doc comments override the wrapped struct's help; nesting and required subcommands remain out of scope for now.

^{Reviewed by Cursor Bugbot for commit 5373abb. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• New Features

  • Added support for defining and parsing CLI subcommands, including nested structures, command-specific options, global flags, defaults, and optional subcommands.
  • Added Args and Subcommands derive macros for generating command-line parsers and metadata.
  • Expanded command keys to support larger values and improved uniqueness checks.

• Bug Fixes

  • Added validation and diagnostics for duplicate command keys and subcommand names.
  • Improved error reporting for invalid subcommand definitions.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds derive-generated CLI subcommands with typed routing, partial-state parsing, metadata generation, key collision detection, and conformance tests. Public parser keys now use u64.

Changes

Subcommand parsing

Layer / File(s)	Summary
Public contracts and key validation argv/src/lib.rs, argv/src/spec.rs	Public command, flag, and argument keys now use u64. CommandArgs and Subcommands expose generated parsing APIs. Specification generation checks recursive key uniqueness.
Subcommand model and derive entry points derive/src/model.rs, derive/src/lib.rs	The derive model validates optional subcommand fields and enum variants. Args and Subcommands derive macros generate the related implementations.
Keyed parser generation derive/src/codegen.rs	Generated parsers use type-derived keys, partial accumulators, defaults, qualified type paths, and event application.
Subcommand routing and value construction derive/src/codegen.rs	Generated code routes events to subcommands, aggregates metadata, selects variants, and builds final values.
End-to-end conformance coverage conformance/tests/subcommands.rs	Tests cover routing, defaults, global flags, metadata, key uniqueness, and KDL snapshots.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant GeneratedArgs
  participant GeneratedSubcommands
  participant CommandArgs
  CLI->>GeneratedArgs: Provide parse events
  GeneratedArgs->>CommandArgs: Apply root events
  GeneratedArgs->>GeneratedSubcommands: Route subcommand events
  GeneratedSubcommands->>GeneratedSubcommands: Select variant by key
  GeneratedArgs->>CommandArgs: Build final CLI value

Loading

Possibly related PRs

• jdx/usage#797: Adds related subcommand parsing and conformance tests.
• jdx/usage#798: Adds related argv parser types and event routing.
• jdx/usage#803: Introduces the derive-based parsing infrastructure extended by this PR.

Poem

A rabbit hops through commands bright,
Keys grow wide in moonlit light.
Flags and args join the queue,
Subcommands know just what to do.
KDL records the trail anew.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding derive support for compiling subcommands from an enum.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
markdown	▁▁▁▁▁▁▁▁▁███████	148,166,271 → 148,227,681	+0.04%	14.44 → 13.82ms	-4.32%
startup	▄▄▄▄▄▄▄▄▄█████▁▁	1,199,527 → 1,199,593	+0.01%	0.99 → 0.97ms	-1.97%

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.}

_{5373abbdc400 vs 2cdd69da7ec6 · measured on the runner, not pushed to the history.}

[greptile-apps]

Greptile Summary

The PR adds derive-based subcommand compilation and addresses the previously reported naming, qualified-path, and sibling key-collision failures.

• Generates static command and metadata tables for Args and Subcommands.
• Routes parser events into per-command partial values and constructs the selected enum variant.
• Uses variant-level command names and descriptions in emitted specifications.
• Widens parser keys and validates duplicate keys during debug KDL emission.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
derive/src/codegen.rs	Generates subcommand tables, partial-state dispatch, variant-specific names, scope-aware type paths, and widened keys; the previously reported code-generation defects are addressed.
derive/src/model.rs	Models and validates optional subcommand fields and enum variants, including duplicate names and wrapped-type collisions.
argv/src/spec.rs	Adds the static traits used by generated command trees and a debug duplicate-key assertion for emitted specifications.
argv/src/lib.rs	Widens command, flag, and argument keys to support independently generated command tables.
conformance/tests/subcommands.rs	Exercises renamed variant routing, command-specific values, global flags, optional selection, key uniqueness, and emitted KDL.

_{Reviews (2): Last reviewed commit: "fix(derive): let the variant name its co..." | Re-trigger Greptile}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 4d9599f. Configure here.}

[coderabbitai]

Actionable comments posted: 11

🧹 Nitpick comments (4)

conformance/tests/subcommands.rs (1)

63-65: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test that an explicit value overrides the default.

Lines 63-65 only test the absent-value path. Add a parse case with --jobs 8 and assert install.jobs.as_deref() == Some("8"). This detects implementations that apply defaults after event binding and overwrite user input.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conformance/tests/subcommands.rs` around lines 63 - 65, Extend the relevant
parsing test around the existing install.jobs default assertion to parse an
explicit --jobs 8 value, then assert install.jobs.as_deref() is Some("8"). Keep
the untouched-flag assertion for the default and verify the explicit user value
remains unchanged after parsing.

derive/src/model.rs (1)

883-899: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Point the duplicate-name error at the variant.

dup receives variant.ty.span(), which is the wrapped struct's span. Two variants that wrap the same struct type therefore report both spans at the same place. Use variant.ident.span() so the message points at each variant.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/model.rs` around lines 883 - 899, Update the duplicate-name error
construction in the variants loop to pass variant.ident.span() to dup instead of
variant.ty.span(), so each duplicate diagnostic points to its variant identifier
while preserving the existing first-duplicate span.

argv/src/spec.rs (1)

747-767: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Drop the Default bound from CommandArgs::Partial.

The generated parser uses CommandArgs::start() for command partials. It does not call CommandArgs::Partial::default(). Keep Subcommands::Partial: Default, because the generated root parser calls it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@argv/src/spec.rs` around lines 747 - 767, Remove the Default bound from the
associated type Partial in the CommandArgs trait, since command parsing
initializes partials through CommandArgs::start(). Preserve the Default bound on
Subcommands::Partial because the generated root parser still calls its default
constructor.

derive/src/codegen.rs (1)

163-175: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The two byte-to-string helpers are emitted twice with identical bodies.

__usage_text and __usage_value_text appear in the emit module and again in the emit_args module. The bodies are the same. A change to the lossy-conversion policy, which the comment says arrives with typed fields, must then be made in two places.

Extract the pair into one function that returns the token stream, and call it from both emitters.

♻️ Proposed refactor

+/// The byte-to-string helpers every generated module needs.
+fn text_helpers() -> TokenStream {
+    quote! {
+        pub fn __usage_text(value: &[u8]) -> ::std::string::String {
+            ::std::string::String::from_utf8_lossy(value).into_owned()
+        }
+
+        pub fn __usage_value_text(
+            value: ::std::option::Option<&[u8]>,
+        ) -> ::std::string::String {
+            value.map(__usage_text).unwrap_or_default()
+        }
+    }
+}

Then splice #helpers into both generated modules.

Also applies to: 649-657

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/codegen.rs` around lines 163 - 175, Extract the shared
`__usage_text` and `__usage_value_text` definitions into a single helper
function that returns their token stream. Update both `emit` and `emit_args` to
call this helper and splice the resulting `#helpers` into their generated
modules, removing the duplicated definitions while preserving their current
behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@argv/src/lib.rs`:
- Around line 108-115: Update the documentation for the public key field to
state that key 0 is reserved for “no subcommand selected” and must not be
assigned to subcommands, including commands created with Command::EMPTY.

In `@argv/src/spec.rs`:
- Around line 29-52: Update collect_keys to track visited command addresses and
skip commands already seen before collecting their key, flags, arguments, or
recursing into subcommands. Pass the visited set through recursive calls so
shared command statics are not reported as duplicates and self-referential
command graphs terminate safely; keep duplicate_key’s sorting and duplicate
detection behavior unchanged.

In `@derive/src/codegen.rs`:
- Around line 571-577: The emit_args path must not generate invalid builds for
Kind::Subcommand fields: either add complete subcommand support matching
emit—including filtered field_finals, COMMAND and COMMAND_META subcommands
entries, and Event::Command routing in apply—or reject such fields during model
validation with a clear error. At minimum, update field_finals in emit_args to
exclude subcommand fields, and add the model-level rejection if routing is not
implemented.
- Around line 95-103: Update the generated command-event handling in the
`quote!` block so `__usage_selected` records only the first command key
encountered during descent, preserving the direct root subcommand key when
nested `Event::Command` values arrive. Keep forwarding every event to `<`#ty` as
::usage_argv::spec::Subcommands>::apply`, and ensure direct command selection
continues to match the recorded key.
- Around line 504-507: Update the bool-default handling in the Shape::Bool
branch so unrecognized values cannot silently become false. First check model
validation for boolean defaults; if absent, add validation there to reject
values other than the supported boolean spellings, or explicitly accept the
intended common spellings before generating the assignment. Preserve generation
of the correct enabled or disabled boolean value for valid defaults.
- Around line 386-401: Update key_base and its callers to hash a caller-supplied
type qualifier in addition to the bare type name, ensuring same-named types from
different modules receive distinct keys. Trace all key_base call sites in the
code-generation paths, including emit, emit_args, and emit_subcommands, and pass
the available disambiguating identity through consistently. Revise the key_base
documentation to describe the qualifier and retain the probabilistic collision
guarantee.
- Around line 629-635: Update emit_args and the generated COMMAND initializer to
honor cli.unknown_flags using the same resolution as emit, while preserving the
intended nested-command precedence; alternatively, explicitly reject
unknown_flags on Args during model validation. Do not leave the attribute
silently ignored or rely on Command::EMPTY for its value.

In `@derive/src/lib.rs`:
- Around line 155-162: Add `Cli::check_args` beside `Cli::check` to reject
`Kind::Subcommand` fields with the documented nesting error and reject any
`bin`, `version`, or `unknown_flags` declarations with an item-level diagnostic.
Invoke this validation from `derive_args` after `Cli::from_input` succeeds and
before `codegen::emit_args`, returning its compile error instead of generating
invalid or incomplete code.

In `@derive/src/model.rs`:
- Around line 911-930: Validate the variant name after processing attributes in
the variant-name derivation flow, rejecting it when strip_dashes produces an
empty string. Mirror the existing validation behavior used by Field::from_field,
while preserving the default kebab-case name and valid explicit names.
- Around line 307-323: Update the subcommand handling in from_field and
Self::subcommand so subcommand fields do not silently discard metadata: reject a
field doc comment and sibling usage options such as long or default with clear
diagnostics, while preserving the existing subcommand construction for valid
fields.
- Around line 288-305: Update the subcommand type handling around type_name to
inspect the original syn::Type::Path and extract the inner Option<T> argument
directly, preserving qualified paths such as commands::Commands. Remove the
syn::parse_str round trip and the unused _optional binding while retaining the
existing validation and error for non-Option subcommand fields.

---

Nitpick comments:
In `@argv/src/spec.rs`:
- Around line 747-767: Remove the Default bound from the associated type Partial
in the CommandArgs trait, since command parsing initializes partials through
CommandArgs::start(). Preserve the Default bound on Subcommands::Partial because
the generated root parser still calls its default constructor.

In `@conformance/tests/subcommands.rs`:
- Around line 63-65: Extend the relevant parsing test around the existing
install.jobs default assertion to parse an explicit --jobs 8 value, then assert
install.jobs.as_deref() is Some("8"). Keep the untouched-flag assertion for the
default and verify the explicit user value remains unchanged after parsing.

In `@derive/src/codegen.rs`:
- Around line 163-175: Extract the shared `__usage_text` and
`__usage_value_text` definitions into a single helper function that returns
their token stream. Update both `emit` and `emit_args` to call this helper and
splice the resulting `#helpers` into their generated modules, removing the
duplicated definitions while preserving their current behavior.

In `@derive/src/model.rs`:
- Around line 883-899: Update the duplicate-name error construction in the
variants loop to pass variant.ident.span() to dup instead of variant.ty.span(),
so each duplicate diagnostic points to its variant identifier while preserving
the existing first-duplicate span.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b5ea04d-5b77-4314-8fa6-cacf69196b1e

📥 Commits

Reviewing files that changed from the base of the PR and between 2cdd69d and 4d9599f.

⛔ Files ignored due to path filters (1)

• conformance/tests/snapshots/subcommands__the_emitted_spec_reads_the_way_a_handwritten_one_would.snap is excluded by !**/*.snap

📒 Files selected for processing (6)

• argv/src/lib.rs
• argv/src/spec.rs
• conformance/tests/subcommands.rs
• derive/src/codegen.rs
• derive/src/lib.rs
• derive/src/model.rs

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

collect_keys treats a shared subcommand as a duplicate.

The command tree is a graph of &'static Command references, not necessarily a tree. Subcommands::COMMANDS splices <Ty as CommandArgs>::COMMAND in, so the same static can be reached from two parents. Examples: one Args type listed in two enums, or one enum used by two commands. collect_keys then pushes that command's key, flag keys, and arg keys twice, and duplicate_key reports a collision that does not exist. The debug_assert! in to_kdl turns that into a test failure for legal code.

A self-referential static Command also makes this recursion unbounded, which overflows the stack instead of reporting anything.

Track visited command addresses to fix both.

🐛 Proposed fix: skip commands already visited

 fn duplicate_key(cmd: &Command<'_>) -> Option<u64> {
     let mut keys = std::vec::Vec::new();
-    collect_keys(cmd, &mut keys);
+    let mut seen: std::vec::Vec<*const Command<'_>> = std::vec::Vec::new();
+    collect_keys(cmd, &mut keys, &mut seen);
     keys.sort_unstable();
     keys.windows(2)
         .find(|pair| pair[0] == pair[1])
         .map(|pair| pair[0])
 }
 
-fn collect_keys(cmd: &Command<'_>, keys: &mut std::vec::Vec<u64>) {
+fn collect_keys(
+    cmd: &Command<'_>,
+    keys: &mut std::vec::Vec<u64>,
+    seen: &mut std::vec::Vec<*const Command<'_>>,
+) {
+    // The same command can be reached from two parents, and a static can even
+    // reference itself. Counting it twice would report a collision that is not one.
+    let addr = cmd as *const _;
+    if seen.contains(&addr) {
+        return;
+    }
+    seen.push(addr);
     keys.push(cmd.key);
     keys.extend(cmd.flags.iter().map(|f| f.key));
     keys.extend(cmd.args.iter().map(|a| a.key));
     for sub in cmd.subcommands {
-        collect_keys(sub, keys);
+        collect_keys(sub, keys, seen);
     }
 }

Run the following script to check whether any generated or hand-written spec reaches one command static from two parents:

#!/bin/bash
# Description: Find command statics spliced into more than one parent table.
rg -n -C6 'COMMANDS|subcommands:' derive/src/codegen.rs
rg -n -C4 'subcommands:' --type=rust -g '!derive/**'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@argv/src/spec.rs` around lines 29 - 52, Update collect_keys to track visited
command addresses and skip commands already seen before collecting their key,
flags, arguments, or recursing into subcommands. Pass the visited set through
recursive calls so shared command statics are not reported as duplicates and
self-referential command graphs terminate safely; keep duplicate_key’s sorting
and duplicate detection behavior unchanged.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Any Event::Command overwrites __usage_selected, so a nested command discards the whole subcommand.

sub_route assigns __usage_selected = __usage_cmd.key for every command event that the root does not claim. apply in the generated root returns false for all Event::Command values (Line 564), so every command event in the parse reaches this block, including one for a grandchild command.

After a two-level descent, __usage_selected holds the grandchild key. Subcommands::select compares that key against each direct variant's COMMAND.key, finds no match, and returns None. The parse then produces None for the subcommand field even though the variant's partial was filled. No error is reported.

Record the first command key instead, or match the key against the direct variants before overwriting.

🐛 Proposed fix: keep the first command key

                 quote! {
                     if let ::usage_argv::Event::Command(__usage_cmd) = &__usage_event {
-                        __usage_selected = __usage_cmd.key;
+                        // The first command word selects the variant. A deeper command
+                        // belongs to that variant, and must not replace the selection.
+                        if __usage_selected == 0 {
+                            __usage_selected = __usage_cmd.key;
+                        }
                     }

Run the following script to confirm that the parser emits a command event per descent level:

#!/bin/bash
# Description: Inspect how the parser emits Event::Command while descending.
rg -n -C8 'Event::Command' argv/src/lib.rs

# Check whether any conformance test covers two levels of commands.
rg -n -C10 'Subcommands' conformance/tests/subcommands.rs

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/codegen.rs` around lines 95 - 103, Update the generated
command-event handling in the `quote!` block so `__usage_selected` records only
the first command key encountered during descent, preserving the direct root
subcommand key when nested `Event::Command` values arrive. Keep forwarding every
event to `<`#ty` as ::usage_argv::spec::Subcommands>::apply`, and ensure direct
command selection continues to match the recorded key.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

A bool default that is not the exact text true becomes false without a diagnostic.

default == "true" treats default = "1", default = "yes", and default = "True" as false. The user asked for an enabled default and receives a disabled one. Reject an unrecognized value in the model, or accept the common spellings here.

Run the following script to check whether the model already validates this:

#!/bin/bash
# Description: Look for validation of a bool field's default value.
rg -n -C6 'default' derive/src/model.rs | rg -n -C6 'Bool|bool|true|false'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/codegen.rs` around lines 504 - 507, Update the bool-default
handling in the Shape::Bool branch so unrecognized values cannot silently become
false. First check model validation for boolean defaults; if absent, add
validation there to reject values other than the supported boolean spellings, or
explicitly accept the intended common spellings before generating the
assignment. Preserve generation of the correct enabled or disabled boolean value
for valid defaults.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

emit_args drops cli.unknown_flags, so the setting is silent on a subcommand.

emit resolves cli.unknown_flags into UnknownFlags::Error or UnknownFlags::Value and writes it into ROOT. This COMMAND omits the field and takes whatever Command::EMPTY provides. A user who writes unknown_flags = "error" on an Args struct gets no effect and no warning.

Either apply the same resolution here, or reject the attribute on an Args struct in the model.

♻️ Proposed fix: honour the declared mode

+    let unknown_flags = match cli.unknown_flags.as_deref() {
+        Some("error") => quote!(::usage_argv::UnknownFlags::Error),
+        _ => quote!(::usage_argv::UnknownFlags::Value),
+    };
...
             pub static COMMAND: Command = Command {
+                unknown_flags: `#unknown_flags`,
                 name: `#name`,
                 key: `#command_key`,

Note that emit inherits nothing from a parent, and the parser may resolve inheritance differently for a nested command. Confirm the intended precedence before applying.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/codegen.rs` around lines 629 - 635, Update emit_args and the
generated COMMAND initializer to honor cli.unknown_flags using the same
resolution as emit, while preserving the intended nested-command precedence;
alternatively, explicitly reject unknown_flags on Args during model validation.
Do not leave the attribute silently ignored or rely on Command::EMPTY for its
value.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Args accepts declarations it cannot compile.

derive_args reuses model::Cli::from_input, so an Args struct passes the same validation as a Cli struct. Two cases get through and then behave badly:

1. A #[usage(subcommand)] field. partial_struct skips Kind::Subcommand fields, but the field_finals in emit_args does not filter them, so build names a field that Partial does not have. The user gets an error inside generated code rather than the "nesting deeper than one level is not supported yet" message that lines 120-121 promise.
2. bin, version, and unknown_flags. emit_args reads none of them, and its COMMAND falls back to Command::EMPTY for unknown_flags. The options are accepted and then dropped without a diagnostic.

Validate both in the Args path and report them as errors on the offending item.

🐛 Proposed fix: reject what `Args` cannot express

 #[proc_macro_derive(Args, attributes(usage))]
 pub fn derive_args(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
-    match model::Cli::from_input(&input) {
-        Ok(cli) => codegen::emit_args(&cli).into(),
-        Err(e) => e.to_compile_error().into(),
-    }
+    match model::Cli::from_input(&input).and_then(|cli| {
+        cli.check_args()?;
+        Ok(cli)
+    }) {
+        Ok(cli) => codegen::emit_args(&cli).into(),
+        Err(e) => e.to_compile_error().into(),
+    }
 }

Add check_args next to Cli::check in derive/src/model.rs:

impl Cli {
    /// Reject what an `Args` struct declares but cannot compile into.
    pub fn check_args(&self) -> syn::Result<()> {
        if let Some(field) = self
            .fields
            .iter()
            .find(|f| matches!(f.kind, Kind::Subcommand { .. }))
        {
            return Err(syn::Error::new(
                field.span,
                "a subcommand cannot declare its own subcommands yet: nesting is one \
                 level deep",
            ));
        }
        if self.bin.is_some() || self.version.is_some() || self.unknown_flags.is_some() {
            return Err(syn::Error::new(
                self.ident.span(),
                "`bin`, `version`, and `unknown_flags` describe a program rather than \
                 one of its commands, so usage::Args does not take them",
            ));
        }
        Ok(())
    }
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/lib.rs` around lines 155 - 162, Add `Cli::check_args` beside
`Cli::check` to reject `Kind::Subcommand` fields with the documented nesting
error and reject any `bin`, `version`, or `unknown_flags` declarations with an
item-level diagnostic. Invoke this validation from `derive_args` after
`Cli::from_input` succeeds and before `codegen::emit_args`, returning its
compile error instead of generating invalid or incomplete code.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

A subcommand field silently drops its doc comment and its sibling options.

from_field reads the doc comment before it calls Self::subcommand, and this constructor then sets help: None and long_help: None. The doc comment on the field disappears without a diagnostic. The conformance fixture writes /// What to do on command, so this is reachable.

Self::subcommand also returns as soon as it sees subcommand, so #[usage(subcommand, long, default = "x")] compiles and ignores everything except subcommand.

Choose one behavior for each and make it explicit: carry the values, or reject them with a message.

♻️ Proposed change: reject the options a subcommand field cannot use

         let mut is_subcommand = false;
+        let mut extra: Option<proc_macro2::Span> = None;
         for attr in attrs(&field.attrs) {
             for meta in nested(attr)? {
-                if ident_of(&meta.path().clone()) == "subcommand" {
+                let path = meta.path().clone();
+                if ident_of(&path) == "subcommand" {
                     if !matches!(meta, Meta::Path(_)) {
                         return Err(syn::Error::new_spanned(
                             meta.path(),
                             "`subcommand` takes no value: the enum it holds is the \
                              field's type",
                         ));
                     }
                     is_subcommand = true;
+                } else if extra.is_none() {
+                    extra = Some(path.span());
                 }
             }
         }
         if !is_subcommand {
             return Ok(None);
         }
+        if let Some(span) = extra {
+            return Err(syn::Error::new(
+                span,
+                "a `subcommand` field holds a command rather than a value, so it \
+                 takes no other options",
+            ));
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/model.rs` around lines 307 - 323, Update the subcommand handling
in from_field and Self::subcommand so subcommand fields do not silently discard
metadata: reject a field doc comment and sibling usage options such as long or
default with clear diagnostics, while preserving the existing subcommand
construction for valid fields.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject an empty variant name.

name is written from strip_dashes, so #[usage(name = "-")] leaves an empty string. An empty command name can never be selected on the command line, and it is written into the spec as an empty node name. Field::from_field already rejects this case for a flag or an argument. Apply the same check here.

🛡️ Proposed fix

                 match ident_of(&path).as_str() {
-                    "name" => name = strip_dashes(&string_value(&meta)?),
+                    "name" => {
+                        name = strip_dashes(&string_value(&meta)?);
+                        if name.is_empty() {
+                            return Err(syn::Error::new_spanned(
+                                path,
+                                "`name` is empty once its dashes are removed, and a \
+                                 subcommand needs something to be called",
+                            ));
+                        }
+                    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	let mut name = to_kebab(&variant.ident.to_string());
	for attr in attrs(&variant.attrs) {
	for meta in nested(attr)? {
	let path = meta.path().clone();
	match ident_of(&path).as_str() {
	"name" => name = strip_dashes(&string_value(&meta)?),
	other => {
	return Err(syn::Error::new_spanned(
	path,
	format!(
	"unknown option `{other}` on a variant; a subcommand \
	variant takes `name` here, and its description comes \
	from the doc comment"
	),
	));
	}
	}
	}
	}
	let mut name = to_kebab(&variant.ident.to_string());
	for attr in attrs(&variant.attrs) {
	for meta in nested(attr)? {
	let path = meta.path().clone();
	match ident_of(&path).as_str() {
	"name" => {
	name = strip_dashes(&string_value(&meta)?);
	if name.is_empty() {
	return Err(syn::Error::new_spanned(
	path,
	"`name` is empty once its dashes are removed, and a \
	subcommand needs something to be called",
	));
	}
	}
	other => {
	return Err(syn::Error::new_spanned(
	path,
	format!(
	"unknown option `{other}` on a variant; a subcommand \
	variant takes `name` here, and its description comes \
	from the doc comment"
	),
	));
	}
	}
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@derive/src/model.rs` around lines 911 - 930, Validate the variant name after
processing attributes in the variant-name derivation flow, rejecting it when
strip_dashes produces an empty string. Mirror the existing validation behavior
used by Field::from_field, while preserving the default kebab-case name and
valid explicit names.

[jdx]

Four findings fixed, and two of them my tests had passed by coincidence — worth spelling out, because it is the same trap twice.

The variant's name never reached the tables. Subcommands read it, validated it against its siblings, documented it as the command name — and then spliced the wrapped struct's command unchanged, so the struct's name selected the command. My test wrote RunTask(Run) with name = "run", where the two spellings agree, so it passed while the feature did not work. Each variant now gets its own command and metadata overriding the name, and the new test uses Install(AddArgs) named add, plus an assertion that the struct's name selects nothing.

Two variants wrapping one struct cannot work, and writing the test for it is what showed why: a command's values collect into the struct that declares them, and its fields' keys come from that struct — so two commands sharing one collect into whichever was reached first, and select picks between them by a key they share. It is now a compile error naming both sites:

error: two variants both wrap `S`, and a command collects into the struct that
       declares it — so give each command its own struct, even if the fields
       are identical

in_module prepended super:: to any path, turning crate::cmds::X into nonsense and quietly redirecting self::X and super::X to the wrong module. Each prefix now shifts by the right amount, and an absolute path is left alone.

The duplicate-key check counted one table met twice as a collision. It now pairs each key with the address it came from: two different tables sharing a key is the fault; meeting the same table twice is not.

Plus two smaller ones: "no subcommand given" is an Option rather than a reserved key, and apply arms evaluate to whether they claimed the event instead of returning — a command with no flags of its own had every arm diverge, which clippy caught as an unreachable tail.

10 tests on this file now. The coverage failure earlier was a runner TLS error in actions/checkout, not the code; re-run and green.

AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@conformance/tests/subcommands.rs`:
- Around line 49-51: Update the conformance test cases around the wrapped-struct
command routing to use the actual command names “add-args” and “remove-args”
instead of the nonexistent “shared” command. Revise the nearby comment to remove
the claim that one argument struct is shared, while preserving coverage that
wrapped struct names cannot route.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: ac09f1c0-d51b-46b0-a1a4-951bc0b47096

📥 Commits

Reviewing files that changed from the base of the PR and between 4d9599f and 5373abb.

📒 Files selected for processing (3)

• conformance/tests/subcommands.rs
• derive/src/codegen.rs
• derive/src/model.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• derive/src/model.rs

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test the actual wrapped struct command names.

No Shared struct exists in this test. "shared" only tests an arbitrary unknown command. Test add-args and remove-args so the test verifies that wrapped struct command names cannot route. Update the stale comment that says one struct is shared.

Also applies to: 89-94

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@conformance/tests/subcommands.rs` around lines 49 - 51, Update the
conformance test cases around the wrapped-struct command routing to use the
actual command names “add-args” and “remove-args” instead of the nonexistent
“shared” command. Revise the nearby comment to remove the claim that one
argument struct is shared, while preserving coverage that wrapped struct names
cannot route.