diff --git a/docs/specs/0098-cli-clap-gnu-compliance.md b/docs/specs/0098-cli-clap-gnu-compliance.md new file mode 100644 index 0000000..d9f38d1 --- /dev/null +++ b/docs/specs/0098-cli-clap-gnu-compliance.md @@ -0,0 +1,373 @@ +# CLI GNU/clig.dev compliance via clap adoption — Design Spec + +**Date:** 2026-07-01 +**Status:** Draft — awaiting user spec review +**Authors:** orchestrator + Claude + +## Goal + +Replace the hand-rolled argv parser in `aura-cli` with a `clap` derive-based +parser, closing the GNU / clig.dev convention gaps audited in #175 while +preserving every current behaviour that is not itself a deviation. The cycle +delivers, for the CLI's primary consumer (LLM / automation, C14) and the human +author alike: + +- **Scoped subcommand help** — `aura --help` prints that subcommand's own + usage with a per-flag Options section, on stdout, exit 0 (today every + `--help` prints one global blob). +- **`--version` / `-V`** — surfaces the workspace version `0.1.0`, stdout, + exit 0 (today absent; the version exists but is never printed). +- **A per-flag Options section + top-level Commands list**, generated from one + declarative source instead of a `USAGE` const duplicated across ten + hand-written `usage()` closures. +- **`--flag=value` (GNU equals), the `--` end-of-options terminator, and + long-option abbreviation** — native clap behaviour, absorbed for free. +- **A clean exit-code partition** (#175 deviation #8): exit 2 = usage error, + exit 1 = runtime failure (today exit 2 is overloaded for both, and even + self-inconsistent). + +**Out of scope (deferred, recorded on #175):** the machine-first surface +(`--help --json`, a command manifest, typed errors, `aura run -` stdin +op-scripts) — Fork B settled this cycle as human/GNU convention compliance, not +the machine-first reframe; that line rides the #157/C21 op-script track. `-`-as- +stdin is app logic, not clap-automatic, and stays with that deferred track. + +**Dependency note (C16 per-case review).** `clap` is a new top-level +dependency — absent from the tree today, not even transitive. It is admitted +under the C16 per-case policy (`INDEX.md:1132-1139`, "hand-rolling what a vetted +standard crate already does is the anti-pattern"): `clap` is the vetted standard +argument parser, and it replaces a hand-rolled parser plus ten duplicated help +surfaces with one declarative source. The CLI is research-side, **not** the +frozen deploy artifact (invariant 8), so clap's dependency/build surface is a +dev-loop/compile tax, never a frozen-artifact tax — the scrutiny bar C16 +reserves for the bot does not apply. This rationale is lifted to the ledger at +cycle close. + +## Architecture + +### Two iterations + +The cycle is one clap migration split into two reviewable iterations whose test +surfaces are largely disjoint: + +- **Iteration 1 — clap migration (behaviour-preserving on exit codes).** Every + subcommand becomes a clap derive parser. Scoped `--help`, `--version`/`-V`, + the Options section, `--flag=value`, `--`, and long-option abbreviation all + fall out of clap. Exit codes are **preserved**: clap emits exit 2 for parse + errors (matching today's usage-error = 2); domain refusals stay exit 2; + `reproduce`-diverged stays exit 1. The renegotiated tests are the parser-path + usage-substring pins and the three help pins. +- **Iteration 2 — exit-code split (#175 deviation #8).** The runtime failures + that today exit 2 move to exit 1, and the lone `generalize` inconsistency is + normalised. The renegotiated tests are the domain-refusal exit-2 pins. No + parser structure changes. + +The planner takes iteration 1 first. + +### clap shape + +One root parser, one subcommand enum: + +```rust +#[derive(clap::Parser)] +#[command(name = "aura", version, about, disable_help_subcommand = false)] +struct Cli { + #[command(subcommand)] + command: Command, +} + +#[derive(clap::Subcommand)] +enum Command { + Run(RunArgs), + Chart(ChartArgs), + Graph(GraphArgs), // nested: bare | build | introspect + Sweep(SweepArgs), + Walkforward(WalkforwardArgs), + Generalize(GeneralizeArgs), + Mc(McArgs), + Runs(RunsArgs), // nested: families | family [rank ] + Reproduce(ReproduceArgs), +} +``` + +`#[command(version)]` gives `--version`/`-V` → `aura 0.1.0`, exit 0. +`disable_help_subcommand = false` and clap's built-in `--help`/`-h` per +(sub)command give scoped help, exit 0. Bare `aura` (no subcommand) hits clap's +"subcommand required" error → usage message on stderr, **exit 2** (preserves the +current `no_args_prints_usage_and_exits_two` semantics). + +### Dual-grammar subcommands (the load-bearing mapping) + +`run`, `sweep`, `walkforward`, `mc` each carry two disjoint grammars under one +token, discriminated today by `first.ends_with(".json") && is_file()` (a +lockstep quartet at `main.rs:4249/4297/4334/4376`). clap's native subcommand +model cannot express "same token, two grammars keyed on whether a positional is +an existing `.json` file". The mapping (derived decision, recorded on #175): + +**One clap args struct per such subcommand, carrying an optional `[blueprint]` +positional plus the union of both branches' flags; a post-parse dispatch on the +same `is_file()` predicate selects the branch and validates flag applicability.** + +```rust +#[derive(clap::Args)] +struct RunArgs { + /// A serialized signal blueprint (price→bias). If given as an existing + /// .json file, runs the loaded-blueprint grammar; otherwise the built-in + /// harness grammar. + blueprint: Option, + // built-in branch: + #[arg(long)] harness: Option, // sma|macd|stage1-r + // .json branch: + #[arg(long)] params: Option, + #[arg(long)] seed: Option, + // shared: + #[arg(long)] real: Option, + #[arg(long)] from: Option, + #[arg(long)] to: Option, + #[arg(long)] trace: Option, + #[arg(long)] cost_per_trade: Option, + #[arg(long)] slip_vol_mult: Option, + #[arg(long)] carry_per_cycle: Option, +} +``` + +Post-parse (in the `Run` handler): + +```rust +match args.blueprint.as_deref() + .filter(|a| a.ends_with(".json") && Path::new(a).is_file()) +{ + Some(path) => run_blueprint(path, /* .json-branch flags */), // --params/--seed/--real/--from/--to + None => run_builtin(/* built-in-branch flags */), // --harness/--real/.../--cost-* +} +``` + +Flag applicability that clap cannot key on file-existence (e.g. `--harness` is +meaningless in the `.json` branch, `--params` in the built-in branch) is +validated post-parse and rejected as a **usage error (exit 2)** with a message +naming the offending flag — preserving today's strictness (unexpected tokens are +usage errors, never silently ignored). The `is_file()` predicate is +single-sourced in one helper so the quartet stays in lockstep. + +### Exit-code classification (iteration 2) + +The partition rule (derived from Option A): + +- **exit 2 — usage error:** any failure to parse or structurally validate the + command line. clap parse errors (unknown flag/subcommand, missing required + arg, bad enum value, malformed number) are exit 2 natively. aura's own + *argument-structure* validations that clap cannot express — `--from`/`--to` + without `--real`, cost flags without an R-harness, `generalize` requiring ≥2 + distinct instruments and all four knobs, an empty/duplicate `--axis`, the + dual-grammar flag-applicability checks — are post-parse validations that + **exit 2**. +- **exit 1 — runtime failure:** any failure *after* a valid parse, while the + operation runs — "no local data for symbol", "no recorded geometry", "no + recorded run or family", "run has no tap named", trace-name collision across + kinds, a family/trace persist write failure. These move from today's exit 2 to + **exit 1**. The `generalize` persist failure (today the lone exit 1 of this + class) and the `sweep`/`mc` persist failures (today exit 2) all normalise to + **exit 1** together. +- **Unchanged:** `reproduce`-diverged stays **exit 1** — it is a semantic + "reproduction diverged" verdict (the operation ran; its result is a + reproduction failure), which is the runtime class, and is already exit 1. +- **graph build / introspect:** argv-flag errors → exit 2 (usage); op-script + *content* errors read from stdin (unconnected node, kind mismatch, unknown + type) → exit 1 (runtime input). The `graph_construct.rs` tests pin the + `success` bool and message prose, not the exit integer, so this classification + is compatible without renegotiating them. + +## Concrete code shapes + +### User-facing (the acceptance evidence — what a consumer runs) + +``` +$ aura --version +aura 0.1.0 + +$ aura sweep --help +Run a parameter sweep over a strategy or a loaded blueprint. + +Usage: aura sweep [OPTIONS] [BLUEPRINT] + +Arguments: + [BLUEPRINT] A loaded blueprint (.json); omit for the built-in --strategy grammar + +Options: + --strategy sma|momentum|stage1-r|stage1-breakout|stage1-meanrev + --real stream a real symbol instead of synthetic + --from window start (requires --real) + --to window end (requires --real) + --name family name (no-persist) + --trace family name (persisted) + --fast ... grid axis + --axis blueprint axis (repeatable; .json mode) + --list-axes list a loaded blueprint's sweepable axes and exit + -h, --help Print help + -V, --version Print version + +$ aura run --bogus # unknown flag → usage error +error: unexpected argument '--bogus' found +... +$ echo $? +2 + +$ aura run --real EURUSD # valid call, no data → runtime failure (iter 2) +aura: no local data for symbol 'EURUSD' +$ echo $? +1 + +$ aura run examples/sig.json --seed 5 # dual grammar: .json branch still works +{"manifest":...} + +$ aura run --harness=stage1-r # GNU --flag=value form now accepted +{"manifest":...} +``` + +### Implementation shape (before → after, secondary) + +**Global `--help` short-circuit — removed.** +```rust +// before (main.rs:4236-4239): fires before dispatch, prints the global blob for +// EVERY subcommand, so `aura sweep --help` == `aura --help`. +if args.iter().any(|a| a == "--help" || a == "-h") { + println!("{USAGE}{COST_FLAGS_NOTE}"); + return; +} +// after: deleted. clap owns --help/-h per (sub)command → scoped help, exit 0. +``` + +**Dispatch match → clap parse + typed handlers.** +```rust +// before (main.rs:4240-4411): `match args.iter()...as_slice()` with ~13 string +// arms, each calling a hand-rolled parse_*_args returning Result<_, String>, +// and 67 `std::process::exit(2)` sites. +// after: +fn main() { + restore_sigpipe(); // unchanged (main.rs:4227-4230) + let cli = Cli::parse(); // clap: parse errors → stderr, exit 2 + match cli.command { + Command::Run(a) => run::dispatch(a), + Command::Sweep(a) => sweep::dispatch(a), + // ... one handler per subcommand + } +} +``` + +**`USAGE` const + `COST_FLAGS_NOTE` + the ten `usage()` closures — removed; +the cost-flags note becomes flag `long_help`** so `run --help` still surfaces it. +```rust +// after: on the RunArgs cost fields +/// per-trade cost in price units (stage1-r only; charged in R as cost/|entry−stop|) +#[arg(long)] cost_per_trade: Option, +// ...the "per ENGINE cycle" / "stage1-r only" phrasing moves into long_help, +// keeping the #153 discoverability the old note gave, now scoped to `run --help`. +``` + +**Exit codes (iteration 2).** +```rust +// before: domain refusal, e.g. main.rs (no local data) → eprintln! + exit(2) +// after: a small typed boundary — usage-class errors exit 2, runtime-class exit 1. +// enum ExitClass { Usage = 2, Runtime = 1 } +// domain refusals map to Runtime; argument-structure validations to Usage. +// generalize persist failure: exit(1) unchanged; sweep/mc persist failures 2 → 1. +``` + +## Components + +- **`crates/aura-cli/Cargo.toml`** — add `clap = { version = "4", features = + ["derive"] }`. (Version 0.1.0 is inherited via `version.workspace = true`; + `#[command(version)]` reads `CARGO_PKG_VERSION`.) +- **`crates/aura-cli/src/main.rs`** — the derive structs (`Cli`, `Command`, one + `*Args` per subcommand), `fn main` reduced to `Cli::parse()` + typed dispatch, + the per-subcommand handlers wrapping the existing `run_*`/`emit_*`/`runs_*`/ + `reproduce_*` execution functions (which are unchanged — only their arg-plumbing + changes). The `is_file()` dual-grammar helper, single-sourced. +- **`crates/aura-cli/src/graph_construct.rs`** — `graph build`/`introspect` + become clap args under the `Graph` subcommand; the stdin op-list reading and + the execution bodies are unchanged; exit-class applied in iteration 2. +- **The execution layer is untouched.** `run_dispatch`, `run_sweep`, + `run_walkforward`, `run_generalize`, `run_mc`, `run_blueprint_*`, `emit_chart`, + `runs_*`, `reproduce_*`, the manifest/report/render code — none of it changes; + only how their arguments arrive (clap structs instead of hand-parsed tuples). + +## Data flow + +argv → `Cli::parse()` (clap: usage errors → stderr + exit 2) → `Command` enum → +per-subcommand handler → [dual-grammar: post-parse `is_file()` dispatch + +flag-applicability validation, usage errors → exit 2] → existing execution +function → stdout (JSON report / rendered output) or runtime failure (stderr + +exit 1, iteration 2). Stream discipline is preserved: results to stdout, errors +to stderr, never mixed; the SIGPIPE reset stays. + +## Error handling + +- **Parse / usage errors → exit 2.** clap's own errors, plus the post-parse + argument-structure validations clap cannot express (dual-grammar flag + applicability, cross-flag requirements). Messages go to stderr; stdout stays + empty on the error path. +- **Runtime failures → exit 1** (iteration 2). Domain refusals after a valid + parse. Messages keep the `aura: {msg}` stderr shape and their existing + substrings ("no local data for symbol '…'", etc.) — only the exit integer + changes, so the message-substring pins survive while the exit-code pins are + renegotiated. +- **SIGPIPE unchanged.** The closed-pipe path stays a clean signal-13 exit + (`code() == None`), never a panic (exit 101); `cli_broken_pipe.rs` must stay + green untouched. +- **`INFRA`/panic** — clap `.parse()` never panics on bad input (it exits 2); + no new panic paths introduced. + +## Testing strategy + +- **Iteration 1 renegotiates** the parser-path usage pins and the help pins: + - The three help pins flip/adjust: `per_subcommand_help_is_uniform_stdout_exit_zero` + (#131) inverts — help is now **scoped**, not uniform; it is rewritten to + assert each subcommand's help names *its own* flags on stdout, exit 0, + stderr empty. `run_help_surfaces_cost_flag_units_note` (#153) keeps asserting + `run --help` contains "per ENGINE cycle" / "stage1-r only" (now via clap + `long_help`). `help_flag_prints_usage_to_stdout_and_exits_zero` adjusts to + clap's stdout help text. + - The ~30 usage-substring pins (`"usage"` on stderr, exit 2) are renegotiated + to clap's error format. Exit-2 expectations are **preserved** in iteration 1; + only the asserted message substrings change from `"usage"` to clap's wording + (or a stable substring clap emits, e.g. the subcommand name + `--help` + pointer). Where a test asserts an aura-specific message ("R-evaluator + harness", "must be non-negative", "--real"), that message is preserved by the + post-parse validation, so the pin survives. + - Happy-path exit-0 / `.success()` pins (21 sites) must stay green untouched — + valid invocations keep exiting 0 with the same stdout. +- **Iteration 2 renegotiates** the ~10 domain-refusal pins from exit 2 to exit 1 + (no local data, no geometry, no recorded run/family, tap-not-found, name + collision, sweep persist). `reproduce`-diverged exit-1 pin stays. Message + substrings unchanged. +- **New tests** (iteration 1): `--version`/`-V` → `aura 0.1.0`, stdout, exit 0; + `aura sweep --help` names sweep-only flags and does NOT equal `aura run --help` + (the scoped-help property #131's pin used to forbid); `--flag=value` form + accepted on a representative flag; `--` terminator honoured. +- **New tests** (iteration 2): a representative runtime failure exits 1 while a + representative usage error exits 2 — the partition pinned as an explicit + property. +- The full workspace suite is green before iteration 1 starts (baseline + established at HEAD 780d823) and green after each iteration. + +## Acceptance criteria + +1. **`aura --version` / `-V`** prints `aura 0.1.0` on stdout, exit 0. +2. **`aura --help`** prints that subcommand's own usage with a per-flag + Options section on stdout, exit 0; `aura sweep --help` ≠ `aura run --help`. +3. **`aura --help`** and bare `aura` behave per GNU: `--help` → top-level + Commands list on stdout exit 0; bare `aura` → usage error exit 2. +4. **`--flag=value`, `--`, and long-option abbreviation** are accepted (native + clap). +5. **Every current valid invocation still succeeds** with the same stdout and + exit 0 — the dual-grammar `.json` vs built-in dispatch for run/sweep/ + walkforward/mc is preserved byte-for-byte on the happy path, including + `sweep --list-axes` and `walkforward --axis`. +6. **Exit codes partition cleanly** (iteration 2): usage errors → exit 2, + runtime failures → exit 1, with no same-class inconsistency; `reproduce`- + diverged stays exit 1; SIGPIPE stays a clean signal exit. +7. **No engine/registry/analysis change** — the diff is confined to `aura-cli` + (invariant 8 untouched); `clap` is the only new dependency, admitted under + the C16 per-case review documented above. +8. **`cargo build --workspace`, `cargo clippy --workspace --all-targets -- -D + warnings`, and `cargo test --workspace` are green** after each iteration.