From 27f0f8ea111721d1c8869f97709f127aec8fbfcb Mon Sep 17 00:00:00 2001 From: Brummel Date: Wed, 1 Jul 2026 20:11:11 +0200 Subject: [PATCH] =?UTF-8?q?audit(0099):=20cycle=20close=20=E2=80=94=20CLI?= =?UTF-8?q?=20clap=20adoption;=20ledger=20refreshed=20(C14);=20rm=20epheme?= =?UTF-8?q?ral=20spec/plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Architect drift review (scope 780d823..HEAD, the sole gate — no regression script) returned one medium ledger-gap, now resolved; otherwise drift-clean with a positive What-holds: - invariant 8 / C16 engine-domain separation preserved: clap is confined to crates/aura-cli/Cargo.toml; the whole diff touches only aura-cli/ + Cargo.lock + ephemeral docs; no engine/std/composites/analysis/registry/core/ingest crate mentions clap, so the frozen deploy artifact structurally cannot pick it up (aura-cli is a leaf binary). The 16 new lock entries are exactly clap's tree. - C16 per-case admission honest: clap admitted with a documented rationale at the Cargo.toml site (replaces the hand-rolled parser + ten help surfaces; dev-loop tax, not frozen-artifact tax), the same admission form as sha2/libc. - C14 automation-face served, execution layer intact: the usage=2/runtime=1 partition, scoped --help, --version, and GNU flags match the spec; the domain fns keep their signatures (arg-plumbing via thin *_from adapters); new behaviour is pinned (partition property test, version, scoped-help, dual-grammar stray-positional guard). Ledger refreshed: the C14 realization block now records the clap adoption + the exit-code partition (usage=2/runtime=1) as a durable automation contract, before the ephemeral spec that first stated it is removed (the drift item the architect raised). Deferred/forward: error-message casing normalization (#179, idea); the machine-first help surface stays on the #157/C21 track (Fork B settled this cycle as human/GNU convention compliance). Verified green (orchestrator, this session): cargo build --workspace clean; cargo clippy --workspace --all-targets -- -D warnings clean; cargo test --workspace green. Cycle drift-clean. closes #175 --- docs/design/INDEX.md | 20 + docs/plans/0098-cli-clap-migration-iter1.md | 659 ------------------- docs/plans/0099-cli-exit-code-split-iter2.md | 233 ------- docs/specs/0098-cli-clap-gnu-compliance.md | 373 ----------- 4 files changed, 20 insertions(+), 1265 deletions(-) delete mode 100644 docs/plans/0098-cli-clap-migration-iter1.md delete mode 100644 docs/plans/0099-cli-exit-code-split-iter2.md delete mode 100644 docs/specs/0098-cli-clap-gnu-compliance.md diff --git a/docs/design/INDEX.md b/docs/design/INDEX.md index 46de599..3084d6e 100644 --- a/docs/design/INDEX.md +++ b/docs/design/INDEX.md @@ -1068,6 +1068,26 @@ runnable substrate but is core to aura's identity, not optional. The pivot keeps this contract's core intact — and arguably tightens it: a browser reading a serialized trace file is a stricter "no UI knowledge in the engine" than egui reaching zero-copy into the live SoA columns. +**Realization (cycles 0098/0099, #175 — the CLI meets GNU/clig.dev conventions +via clap).** The programmatic/CLI face's argument surface moved from a hand-rolled +argv parser to a `clap` derive parser (admitted under the C16 per-case dependency +policy — research-side, a dev-loop/compile tax, never a frozen-artifact tax; +invariant 8 untouched, the change is confined to `aura-cli`, a leaf binary the +frozen deploy artifact cannot pick up). One declarative source now yields scoped +`aura --help`, `--version`/`-V`, a per-flag Options section, and GNU +`--flag=value` / `--` / long-option abbreviation. The **exit-code partition** is a +durable part of the automation contract, so a caller can branch on the failure +class without parsing stderr: **exit 0 = success**; **exit 2 = usage error** — a +command-line fault (clap parse errors + aura's post-parse argument-structure +validations, *including* the content of an argv-named blueprint file, which is +itself an argument); **exit 1 = runtime failure** — a well-formed command whose +needed environment / recorded state is missing, or bad piped stdin data. The four +dual-grammar subcommands (run/sweep/walkforward/mc) keep both grammars under one +token via an optional `[blueprint]` positional + a post-parse `is_file()` +dispatch; the execution layer is unchanged (arg-plumbing via thin `*_from` +adapters). Error-message casing normalization was deferred (#179); the +machine-first help surface (JSON/manifest help, stdin op-scripts) stays on the +#157/C21 track, not this cycle (settled as human/GNU convention compliance). ### C15 — Resampling-as-node; sessions/calendars **Guarantee.** A resampler is a node (finer stream → coarser bar stream), diff --git a/docs/plans/0098-cli-clap-migration-iter1.md b/docs/plans/0098-cli-clap-migration-iter1.md deleted file mode 100644 index 03c3ceb..0000000 --- a/docs/plans/0098-cli-clap-migration-iter1.md +++ /dev/null @@ -1,659 +0,0 @@ -# clap migration (iteration 1) — Implementation Plan - -> **Parent spec:** `docs/specs/0098-cli-clap-gnu-compliance.md` -> -> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run -> this plan. Steps use `- [ ]` checkboxes for tracking. - -**Goal:** Replace the hand-rolled argv parser in `aura-cli` with a `clap` -derive parser, delivering scoped `aura --help`, `--version`/`-V`, a -per-flag Options section, and native `--flag=value`/`--`/long-opt abbreviation — -behaviour-preserving on exit codes (the exit-code split is iteration 2). - -**Architecture:** One root `Cli` with a `Command` subcommand enum; one `*Cmd` -struct per subcommand (the `Cmd` suffix avoids colliding with the existing -`RunArgs`/`McArgs` arg-types the untouched execution layer still uses). The four -dual-grammar subcommands (run/sweep/walkforward/mc) carry an optional -`[blueprint]` positional + the union of both branches' flags, dispatched -post-parse on a single-sourced `is_file()` predicate with flag-applicability -validation. Each handler converts its `*Cmd` into the exact argument shapes the -existing `run_*`/`emit_*`/`runs_*`/`reproduce_*` fns already accept — those fns -are untouched. clap parse errors exit 2 (matching today's usage-error=2); domain -refusals stay exit 2; reproduce-diverged stays exit 1. - -**Tech Stack:** Rust 2024, `clap` 4 (derive), `aura-cli` binary. - ---- - -**Files this plan creates or modifies:** - -- Modify: `crates/aura-cli/Cargo.toml:12-42` — add `clap` dependency -- Modify: `crates/aura-cli/src/main.rs` — replace `fn main` dispatch + all - `parse_*_args` with clap derive structs + typed handlers; delete `USAGE`, - `COST_FLAGS_NOTE`, the global `--help` short-circuit, and the ten `usage()` - closures. Keep every `run_*`/`emit_*`/`runs_*`/`reproduce_*` execution fn, the - existing arg-types they consume (`RunArgs`, `McArgs`, `Stage1RGrid`, …), and - every value helper (`parse_csv_list`, `parse_scalar_csv`, `parse_select`, - `parse_param_cells`, `DataSource`, `Strategy`, `Selection`) verbatim. -- Modify: `crates/aura-cli/src/graph_construct.rs:127-237` — `graph build` / - `graph introspect` become clap args under the `Graph` subcommand -- Test: `crates/aura-cli/tests/cli_run.rs` — new tests (`--version`, - scoped-help, `--flag=value`, `--`) + renegotiate the broken help/usage pins -- Unchanged (must stay green): `crates/aura-cli/tests/cli_broken_pipe.rs`, - `crates/aura-cli/tests/graph_construct.rs` - ---- - -## Task 1: clap dependency + new-behaviour RED tests - -**Files:** -- Modify: `crates/aura-cli/Cargo.toml:12-42` -- Test: `crates/aura-cli/tests/cli_run.rs` - -- [ ] **Step 1: Add clap to Cargo.toml** - -In `crates/aura-cli/Cargo.toml`, under `[dependencies]`, add: - -```toml -# clap: the vetted standard argument parser. Adopted (C16 per-case review) to -# replace the hand-rolled argv parser + ten duplicated help surfaces with one -# declarative source, giving scoped --help, --version, --flag=value, and -# long-option abbreviation. Research-side CLI only (invariant 8): a dev-loop -# compile tax, never a frozen-artifact tax. -clap = { version = "4", features = ["derive"] } -``` - -- [ ] **Step 2: Verify clap resolves and the tree still builds** - -Run: `cargo build --workspace` -Expected: PASS (clap and its transitive deps download and compile; no source -change yet, so the existing binary is unaffected). - -- [ ] **Step 3: Write the four new-behaviour tests (RED)** - -Append to `crates/aura-cli/tests/cli_run.rs`: - -```rust -/// `--version` / `-V` surface the workspace version on stdout, exit 0 (GNU MUST; -/// today `aura --version` hits the catch-all → USAGE on stderr, exit 2). -#[test] -fn version_flag_prints_version_to_stdout_and_exits_zero() { - for flag in ["--version", "-V"] { - let out = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .arg(flag) - .output() - .expect("spawn aura"); - assert_eq!(out.status.code(), Some(0), "{flag}: exit {:?}", out.status); - let stdout = String::from_utf8_lossy(&out.stdout); - assert!(stdout.contains("0.1.0"), "{flag} stdout: {stdout:?}"); - } -} - -/// Subcommand help is SCOPED: `aura sweep --help` names sweep's own flags and is -/// NOT byte-identical to `aura run --help` (today both print the global blob, so -/// they are equal — this asserts the #131 uniform-help behaviour is retired). -#[test] -fn subcommand_help_is_scoped_not_uniform() { - let sweep = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["sweep", "--help"]).output().expect("spawn"); - let run = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["run", "--help"]).output().expect("spawn"); - assert_eq!(sweep.status.code(), Some(0), "sweep --help exit"); - assert_eq!(run.status.code(), Some(0), "run --help exit"); - let sweep_out = String::from_utf8_lossy(&sweep.stdout); - let run_out = String::from_utf8_lossy(&run.stdout); - assert!(sweep_out.contains("--strategy"), "sweep help names its flags: {sweep_out:?}"); - assert_ne!(sweep_out, run_out, "per-subcommand help must be scoped, not uniform"); -} - -/// The GNU `--flag=value` equals form is accepted (today only space-separated). -#[test] -fn gnu_equals_form_is_accepted() { - let out = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["run", "--harness=sma"]).output().expect("spawn"); - assert_eq!(out.status.code(), Some(0), - "run --harness=sma should run; stderr: {}", String::from_utf8_lossy(&out.stderr)); -} - -/// The `--` end-of-options terminator is recognized (a bare `--` after the flags -/// is a no-op that still runs the default harness). -#[test] -fn double_dash_terminator_is_recognized() { - let out = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["run", "--harness", "sma", "--"]).output().expect("spawn"); - assert_eq!(out.status.code(), Some(0), - "run ... -- should run; stderr: {}", String::from_utf8_lossy(&out.stderr)); -} -``` - -- [ ] **Step 4: Run the new tests to verify they FAIL (RED)** - -Run each filter separately (one positional filter per invocation): - -Run: `cargo test -p aura-cli --test cli_run version_flag_prints_version_to_stdout_and_exits_zero` -Expected: FAIL — exit is `Some(2)` (catch-all USAGE), not `Some(0)`. - -Run: `cargo test -p aura-cli --test cli_run subcommand_help_is_scoped_not_uniform` -Expected: FAIL — `sweep --help` == `run --help` today (global blob), the -`assert_ne!` fails. - -Run: `cargo test -p aura-cli --test cli_run gnu_equals_form_is_accepted` -Expected: FAIL — `--harness=sma` is not split today, `sma` is not matched, exit 2. - -Run: `cargo test -p aura-cli --test cli_run double_dash_terminator_is_recognized` -Expected: FAIL — the trailing `--` is an unexpected token today, exit 2. - ---- - -## Task 2: clap migration + pin renegotiation - -The atomic migration (build cannot be green until `fn main` and all subcommands -are on clap together). Work Steps 1-7 as one coherent change, then the Step-8 -build gate, then the Step-9/10 pin renegotiation, then the Step-11 suite gate. -Ends with the full suite green (the four Task-1 tests flip to PASS). - -**Files:** -- Modify: `crates/aura-cli/src/main.rs` -- Modify: `crates/aura-cli/src/graph_construct.rs:127-237` -- Test: `crates/aura-cli/tests/cli_run.rs` - -- [ ] **Step 1: Add clap imports and the root parser** - -At the top of `crates/aura-cli/src/main.rs` add `use clap::{Parser, Subcommand, Args};` -and define the root (note: `*Cmd` names, to avoid colliding with the existing -`RunArgs`/`McArgs` arg-types): - -```rust -#[derive(Parser)] -#[command(name = "aura", version, about = "A game engine for traders — research CLI")] -struct Cli { - #[command(subcommand)] - command: Command, -} - -#[derive(Subcommand)] -enum Command { - /// Run a single backtest (built-in harness or a loaded blueprint). - Run(RunCmd), - /// Render a recorded run's trace to an HTML chart. - Chart(ChartCmd), - /// Emit / construct / introspect a graph. - Graph(GraphCmd), - /// Sweep a parameter grid over a strategy or a loaded blueprint. - Sweep(SweepCmd), - /// Walk-forward validation over a strategy or a loaded blueprint. - Walkforward(WalkforwardCmd), - /// Grade one candidate across multiple instruments. - Generalize(GeneralizeCmd), - /// Monte-Carlo over synthetic draws, an R-bootstrap, or a loaded blueprint. - Mc(McCmd), - /// List or inspect recorded run families. - Runs(RunsCmd), - /// Reproduce a recorded family by content id. - Reproduce(ReproduceCmd), -} -``` - -`#[command(version)]` reads `CARGO_PKG_VERSION` (= workspace `0.1.0`) → `aura 0.1.0`. - -- [ ] **Step 2: Define the non-dual-grammar subcommand `*Cmd` structs** - -Add (flag inventory lifted verbatim from the current `parse_*` fns): - -```rust -#[derive(Args)] -struct ChartCmd { - /// The recorded run or family name to chart. - name: String, - /// Chart only the given tap. - #[arg(long)] tap: Option, - /// Render stacked panels instead of an overlay. - #[arg(long)] panels: bool, -} - -#[derive(Args)] -struct GraphCmd { - #[command(subcommand)] - sub: Option, -} - -#[derive(Subcommand)] -enum GraphSub { - /// Construct a graph from a stdin op-list. - Build, - /// Introspect a graph. - Introspect(GraphIntrospectCmd), -} - -#[derive(Args)] -struct GraphIntrospectCmd { - #[arg(long)] vocabulary: bool, - #[arg(long)] node: Option, - #[arg(long)] unwired: bool, - #[arg(long)] content_id: bool, -} - -#[derive(Args)] -struct GeneralizeCmd { - #[arg(long)] strategy: Option, - /// Comma-separated instrument list (≥2 distinct, required). - #[arg(long)] real: Option, - #[arg(long)] fast: Option, - #[arg(long)] slow: Option, - #[arg(long)] stop_length: Option, - #[arg(long)] stop_k: Option, - #[arg(long)] from: Option, - #[arg(long)] to: Option, - #[arg(long)] metric: Option, - #[arg(long)] name: Option, -} - -#[derive(Args)] -struct RunsCmd { - #[command(subcommand)] - sub: RunsSub, -} - -#[derive(Subcommand)] -enum RunsSub { - /// List recorded families. - Families, - /// Inspect one family, optionally ranked by a metric. - Family { - id: String, - /// The literal keyword `rank`, if a ranking metric follows. - rank_kw: Option, - /// The metric to rank by (only valid after `rank`). - metric: Option, - }, -} - -#[derive(Args)] -struct ReproduceCmd { - /// The family content id to reproduce. - id: String, -} -``` - -- [ ] **Step 3: Define the four dual-grammar `*Cmd` structs** - -Each carries an optional `[blueprint]` positional + the union of both branches' -flags. The three cost fields carry the #153 units note as `long_help` (see -Step 9 for the wrap-safe assertion): - -```rust -#[derive(Args)] -struct RunCmd { - /// A serialized signal blueprint (.json). An existing file selects the - /// loaded-blueprint grammar; otherwise the built-in harness grammar. - blueprint: Option, - /// Built-in harness: sma | macd | stage1-r (default sma). - #[arg(long)] harness: Option, - /// Blueprint params (JSON scalar-cell array; .json mode). - #[arg(long)] params: Option, - /// Blueprint seed (.json mode). - #[arg(long)] seed: Option, - #[arg(long)] real: Option, - #[arg(long)] from: Option, - #[arg(long)] to: Option, - #[arg(long)] trace: Option, - #[arg(long, long_help = "per-trade cost in price units. stage1-r only; charged in R as cost/|entry-stop|.")] - cost_per_trade: Option, - #[arg(long, long_help = "slippage multiplier on realized vol. stage1-r only.")] - slip_vol_mult: Option, - #[arg(long, long_help = "carry in price units per ENGINE cycle (not per day, not an overnight swap). stage1-r only.")] - carry_per_cycle: Option, -} - -#[derive(Args)] -struct SweepCmd { - /// A loaded blueprint (.json); omit for the built-in --strategy grammar. - blueprint: Option, - #[arg(long)] strategy: Option, - #[arg(long)] real: Option, - #[arg(long)] from: Option, - #[arg(long)] to: Option, - #[arg(long)] name: Option, - #[arg(long)] trace: Option, - #[arg(long)] fast: Option, - #[arg(long)] slow: Option, - #[arg(long)] stop_length: Option, - #[arg(long)] stop_k: Option, - #[arg(long)] channel: Option, - #[arg(long)] window: Option, - #[arg(long)] band_k: Option, - /// Blueprint sweep axis `=` (repeatable; .json mode). - #[arg(long)] axis: Vec, - /// List a loaded blueprint's sweepable axes and exit (.json mode, stands alone). - #[arg(long)] list_axes: bool, -} - -#[derive(Args)] -struct WalkforwardCmd { - /// A loaded blueprint (.json); omit for the built-in --strategy grammar. - blueprint: Option, - #[arg(long)] strategy: Option, - #[arg(long)] real: Option, - #[arg(long)] from: Option, - #[arg(long)] to: Option, - #[arg(long)] name: Option, - #[arg(long)] trace: Option, - #[arg(long)] fast: Option, - #[arg(long)] slow: Option, - #[arg(long)] stop_length: Option, - #[arg(long)] stop_k: Option, - #[arg(long)] select: Option, - /// Blueprint IS-refit axis `=` (repeatable, ≥1 required; .json mode). - #[arg(long)] axis: Vec, -} - -#[derive(Args)] -struct McCmd { - /// A loaded blueprint (.json); omit for the built-in grammar. - blueprint: Option, - #[arg(long)] strategy: Option, - #[arg(long)] real: Option, - #[arg(long)] from: Option, - #[arg(long)] to: Option, - #[arg(long)] name: Option, - #[arg(long)] trace: Option, - #[arg(long)] fast: Option, - #[arg(long)] slow: Option, - #[arg(long)] stop_length: Option, - #[arg(long)] stop_k: Option, - #[arg(long)] block_len: Option, - #[arg(long)] resamples: Option, - #[arg(long)] seed: Option, - /// Number of synthetic draws (required; .json mode). - #[arg(long)] seeds: Option, -} -``` - -- [ ] **Step 4: Single-source the dual-grammar predicate** - -Add a helper near the `*Cmd` structs: - -```rust -/// The dual-grammar discriminator: a first-positional that names an existing -/// `.json` file selects the loaded-blueprint branch. Single-sourced so the -/// four dual-grammar subcommands stay in lockstep. -fn is_blueprint_file(arg: &Option) -> Option<&str> { - arg.as_deref() - .filter(|a| a.ends_with(".json") && std::path::Path::new(a).is_file()) -} -``` - -- [ ] **Step 5a: Write the four dual-grammar handlers** - -Each branches on `is_blueprint_file`, validates that no flag from the *other* -branch is set (usage error, exit 2, reusing existing message wording), enforces -the same intra-branch constraints the old `parse_*` fns did, and converts its -`*Cmd` fields into the exact argument shapes the existing execution fns accept. -The value-mapping is done inline with the existing helpers (`DataSource::from_choice`, -`Strategy`, `Stage1RGrid`, `Selection`, `parse_scalar_csv`, `parse_csv_list`, -`parse_select`, `parse_param_cells`) — on a bad enum/csv value, emit the existing -message + `exit(2)`. `RunCmd` example (the other three follow the same three-part -shape — branch, validate, convert-and-call — with their own flag sets and exec -fns, given explicitly below): - -```rust -fn dispatch_run(a: RunCmd) { - match is_blueprint_file(&a.blueprint) { - Some(path) => { // .json branch - if a.harness.is_some() { - eprintln!("aura: --harness is not valid with a blueprint file"); - std::process::exit(2); - } - let doc = std::fs::read_to_string(path).unwrap_or_else(|e| { - eprintln!("aura: {path}: {e}"); std::process::exit(2); - }); - let signal = blueprint_from_json(&doc, &|t| std_vocabulary(t)) - .unwrap_or_else(|e| { eprintln!("aura: {path}: {e:?}"); std::process::exit(2); }); - let params = match a.params.as_deref() { - Some(j) => parse_param_cells(j).unwrap_or_else(|m| { eprintln!("aura: {m}"); std::process::exit(2); }), - None => Vec::new(), - }; - let data = run_data_from(a.real.as_deref(), a.from, a.to); // new thin adapter (Step 5c) - let report = run_signal_stage1r(signal, ¶ms, data, a.seed.unwrap_or(0)); - println!("{}", report.to_json()); - } - None => { // built-in branch - if a.params.is_some() || a.seed.is_some() { - eprintln!("aura: --params/--seed require a blueprint file"); - std::process::exit(2); - } - let run_args = run_args_from(&a).unwrap_or_else(|m| { // new thin adapter (Step 5c) - eprintln!("aura: {m}"); std::process::exit(2); - }); - match run_dispatch(run_args) { - Ok(report) => println!("{}", report.to_json()), - Err(msg) => { eprintln!("aura: {msg}"); std::process::exit(2); } - } - } - } -} -``` - -The other three dual-grammar handlers, explicitly: - -- `dispatch_sweep(a: SweepCmd)`: `.json` branch → validate the `.json`-only rule - (`--list-axes` must stand alone: if set and any other flag is present, the - existing stand-alone-refusal message + exit 2), parse each `--axis` - `name=csv` (empty/duplicate name → existing refusal + exit 2), then call - `run_blueprint_sweep` (or the `--list-axes` probe path) exactly as the current - `.json` arm does. Built-in branch → map `--strategy`/`--real`/grid flags via - `Strategy`/`Stage1RGrid`/`DataSource::from_choice` and call `run_sweep`. -- `dispatch_walkforward(a: WalkforwardCmd)`: `.json` branch → `--axis` ≥1 - required (existing refusal + exit 2), `--select` via `parse_select`, call the - blueprint walk-forward path (`DataSource::Synthetic`, as today). Built-in - branch → map flags and call `run_walkforward`. -- `dispatch_mc(a: McCmd)`: `.json` branch → `--seeds` required and `>0` (existing - refusal + exit 2), reject `--real`/`--from`/`--to` (unknown-in-this-branch → - exit 2), call `run_blueprint_mc` (`DataSource::Synthetic`). Built-in branch → - the `r_path` split (any of `--strategy`/grid/`--block-len`/`--resamples`/`--seed` - ⇒ `run_mc_r_bootstrap`; otherwise `--name`/`--trace` ⇒ `run_mc`), reusing the - exact `r_path` precedence and the "name flags rejected when r_path" refusal - (exit 2). - -For every intra-branch constraint (`--from`/`--to` require `--real`; cost flags -require an R-harness; an empty/duplicate axis) reuse the exact message string -the old `parse_*` fn emitted so the message-substring pins survive. - -- [ ] **Step 5b: Write the five straight-through handlers** - -```rust -fn dispatch_chart(a: ChartCmd) { - emit_chart(&a.name, a.tap.as_deref(), if a.panels { ChartMode::Panels } else { ChartMode::Overlay }); -} - -fn dispatch_graph(a: GraphCmd) { - match a.sub { - None => print!("{}", render::render_html(&sample_blueprint())), - Some(GraphSub::Build) => graph_construct::build_cmd(), - Some(GraphSub::Introspect(i)) => graph_construct::introspect_cmd(i), // Step 7 - } -} - -fn dispatch_generalize(a: GeneralizeCmd) { - // Convert to the existing generalize call: strategy must be stage1-r; --real - // is a ≥2-distinct comma list; fast/slow/stop-length/stop-k all required; - // metric default expectancy_r; name default generalize. Reuse the exact - // refusal messages the old parse_generalize_args emitted (+ exit 2) for each - // violated constraint, then call run_generalize(...). - let (name, symbols, grid, metric, from_ms, to_ms) = generalize_args_from(&a) // new thin adapter (Step 5c) - .unwrap_or_else(|m| { eprintln!("aura: {m}"); std::process::exit(2); }); - run_generalize(&name, &symbols, &grid, &metric, from_ms, to_ms); -} - -fn dispatch_runs(a: RunsCmd) { - match a.sub { - RunsSub::Families => runs_families(), - RunsSub::Family { id, rank_kw, metric } => match (rank_kw.as_deref(), metric) { - (None, None) => runs_family(&id, None), - (Some("rank"), Some(m)) => runs_family(&id, Some(&m)), - _ => { eprintln!("aura: usage: aura runs family [rank ]"); std::process::exit(2); } - }, - } -} - -fn dispatch_reproduce(a: ReproduceCmd) { reproduce_family(&a.id); } -``` - -- [ ] **Step 5c: Add the thin `*_from` adapters** - -Add small private fns that convert a `*Cmd` into the argument shapes the existing -exec fns accept, reusing the existing value helpers. These are NEW (they replace -the body of the old `parse_*_args`, minus the argv tokenizing clap now owns): - -- `run_data_from(real: Option<&str>, from: Option, to: Option) -> RunData` - — mirror the old `--real`/`--from`/`--to` resolution (`--from`/`--to` require - `--real`; on violation emit the existing message + exit 2). -- `run_args_from(a: &RunCmd) -> Result` — build the existing - `RunArgs` (the type `run_dispatch` consumes) from the built-in-branch fields, - applying the harness-enum map and the cost-flags-require-R-harness / - non-negative-rate checks with the existing message strings. -- `generalize_args_from(a: &GeneralizeCmd) -> Result<(String, Vec, Stage1RGrid, String, Option, Option), String>` - — the old `parse_generalize_args` body minus tokenizing. - -Keep the `Err(String)` message text identical to the old closures so the pins in -Step 10 survive. - -- [ ] **Step 6: Rewrite `fn main`; delete the old parser surface** - -Replace the body of `fn main` (keep the SIGPIPE reset verbatim) with: - -```rust -fn main() { - #[cfg(unix)] - unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL); } // unchanged - match Cli::parse().command { - Command::Run(a) => dispatch_run(a), - Command::Chart(a) => dispatch_chart(a), - Command::Graph(a) => dispatch_graph(a), - Command::Sweep(a) => dispatch_sweep(a), - Command::Walkforward(a) => dispatch_walkforward(a), - Command::Generalize(a) => dispatch_generalize(a), - Command::Mc(a) => dispatch_mc(a), - Command::Runs(a) => dispatch_runs(a), - Command::Reproduce(a) => dispatch_reproduce(a), - } -} -``` - -Delete: the global `--help`/`-h` short-circuit; the `USAGE` const; the -`COST_FLAGS_NOTE` const; every local `usage()` closure inside the old -`parse_*_args` fns; and the old `parse_*_args` fns themselves (`parse_run_args`, -`parse_blueprint_run_args`, `parse_chart_args`, `parse_sweep_args`, -`parse_sweep_blueprint_args`, `parse_walkforward_args`, -`parse_walkforward_blueprint_args`, `parse_generalize_args`, `parse_mc_args`, -`parse_mc_blueprint_args`, `parse_nonneg_rate`). Keep the *existing arg-types* -they fed (`RunArgs`, `McArgs`, `Stage1RGrid`, `Selection`, `DataSource`, -`Strategy`) and the *value helpers* (`parse_csv_list`, `parse_scalar_csv`, -`parse_select`, `parse_param_cells`) — the adapters in Step 5c reuse them. - -- [ ] **Step 7: Migrate `graph build` / `graph introspect`** - -In `graph_construct.rs`, `build_cmd()` keeps its body verbatim. Change -`introspect_cmd` to take the parsed `GraphIntrospectCmd` instead of `rest: -&[&str]`: replace its hand-parsing of `--vocabulary`/`--node`/`--unwired`/ -`--content-id` with reading the struct fields, and validate that exactly one is -set (reuse the existing usage message + exit 2 if not). The stdin op-list reading -and the execution bodies are unchanged. - -- [ ] **Step 8: Build gate** - -Run: `cargo build --workspace` -Expected: PASS (0 errors). - -Run: `cargo clippy --workspace --all-targets -- -D warnings` -Expected: PASS (0 warnings — no dead code, all structs/handlers wired). - -- [ ] **Step 9: Renegotiate the help pins** - -In `crates/aura-cli/tests/cli_run.rs`: - -Replace `per_subcommand_help_is_uniform_stdout_exit_zero` (#131) — it asserts -help is UNIFORM, which the migration retires — with a scoped-help assertion: - -```rust -#[test] -fn per_subcommand_help_is_scoped_stdout_exit_zero() { - for sub in ["run", "chart", "sweep", "walkforward", "mc", "graph", "runs"] { - for help in ["--help", "-h"] { - let out = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args([sub, help]).output().expect("spawn"); - assert_eq!(out.status.code(), Some(0), "{sub} {help} exit"); - assert!(!out.stdout.is_empty(), "{sub} {help} stdout empty"); - assert!(out.stderr.is_empty(), "{sub} {help} stderr not empty: {:?}", - String::from_utf8_lossy(&out.stderr)); - } - } -} -``` - -`run_help_surfaces_cost_flag_units_note` (#153) — clap wraps `long_help` at the -render width, which can split a multi-word substring across lines. Make the pin -wrap-robust: assert `run --help` stdout contains the single tokens `"ENGINE"`, -`"cycle"`, and `"stage1-r"` (clap does not hyphenate or split a single word), -rather than the multi-word `"per ENGINE cycle"`. Rewrite the two assertions to -those single-word substrings. - -`help_flag_prints_usage_to_stdout_and_exits_zero` — `aura --help` now prints the -top-level Commands list to stdout, exit 0. Keep exit 0; change the assertion to -stdout contains `"run"` (still listed as a command) and change any -`contains("usage")` (lowercase) to `contains("Usage")` (clap capitalizes). Keep -the "unknown subcommand exits 2" sub-assertion. - -- [ ] **Step 10: Renegotiate the generic usage-substring pins** - -clap parse errors go to stderr with a capitalized `"Usage:"` line, not the old -lowercase `"usage:"`. For each test asserting stderr `contains("usage")` on a -PARSE error, change the substring to `"Usage"` (capital); the exit-2 expectation -is PRESERVED. Affected (assert exit 2 + `"usage"`): - -- `run_with_trailing_token_is_strict_and_exits_two` → `"Usage"` -- `no_args_prints_usage_and_exits_two` (bare `aura`: clap errors "subcommand - required" → stderr `"Usage"`, exit 2, stdout empty) → `"Usage"` -- `sweep_with_trailing_token_is_strict_and_exits_two` → `"Usage"` - -For tests asserting an AURA-SPECIFIC message (from a post-parse validation, NOT -`"usage"`) the message is PRESERVED, so NO change is needed — verify each still -passes: -`mc_rejects_real_flag_with_usage_exit_2` (`"--real"`), -`family_window_flag_without_real_refuses_with_usage_exit_2` (cmd + `"--real"`), -`run_cost_flag_on_non_r_harness_is_refused_exit_2` (`"R-evaluator harness"`), -`run_negative_cost_per_trade_refused_non_negative_exit_2` (`"must be non-negative"`), -the `generalize_refuses_*` family, the axis refusals, `aura_sweep_list_axes_rejects_other_flags`, -and the domain-refusal message pins (`"no local data"`, `"no recorded geometry"`, -`"no recorded run or family"`, `"run has no tap named"`, `"UnknownNodeType"`). -For `runs_list_and_rank_are_retired_and_exit_two` / `runs_bare_subcommand_is_strict_and_exits_two`: -`runs list` / `runs rank ` / bare `runs` are now clap "unknown/ missing -subcommand" errors → exit 2 preserved; adjust any `"usage"` substring to -`"Usage"` (or drop the substring assertion, keeping exit 2). - -- [ ] **Step 11: Full-suite gate** - -Run: `cargo test --workspace` -Expected: PASS — every test green, including the four Task-1 tests (now GREEN), -the renegotiated help/usage pins, and all preserved-message + happy-path pins. -`cli_broken_pipe.rs` and `graph_construct.rs` stay green untouched. - -Run: `cargo clippy --workspace --all-targets -- -D warnings` -Expected: PASS. - ---- - -## Notes for the executor - -- **Task 2 is the atomic migration** — the build cannot be green until `fn main` - and all subcommands are on clap together (build-atomicity). Steps 1-7 are one - coherent change; the first gate is the Step-8 build, not per-sub. -- **The execution layer is untouched.** Only arg-plumbing changes: the `*Cmd` - structs (clap) + the handlers + the thin `*_from` adapters. Every - `run_*`/`emit_*`/`runs_*`/`reproduce_*` body and every existing arg-type/value - helper stays verbatim. If a step tempts you to change an exec-fn body, stop. -- **Exit codes are behaviour-preserved.** clap parse errors exit 2; every domain - refusal and post-parse validation stays exit 2; reproduce-diverged stays exit - 1. The exit-code split (2=usage/1=runtime) is iteration 2, out of scope here. -- **Preserve message substrings verbatim** where a test pins them — reuse the - exact strings the old `parse_*`/refusal paths emitted. diff --git a/docs/plans/0099-cli-exit-code-split-iter2.md b/docs/plans/0099-cli-exit-code-split-iter2.md deleted file mode 100644 index e60545f..0000000 --- a/docs/plans/0099-cli-exit-code-split-iter2.md +++ /dev/null @@ -1,233 +0,0 @@ -# Exit-code split (iteration 2) — Implementation Plan - -> **Parent spec:** `docs/specs/0098-cli-clap-gnu-compliance.md` -> -> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run -> this plan. Steps use `- [ ]` checkboxes for tracking. - -**Goal:** Apply the clean exit-code partition (#175 deviation #8): runtime -failures exit 1, usage errors exit 2, with no same-class inconsistency — on top -of the clap migration (iteration 1, behaviour-preserving on exit codes). - -**Architecture:** The partition rule (attribution principle): **exit 1 = -runtime** — the command line was well-formed but the environment / recorded state -it needs is missing, or piped stdin data is bad; **exit 2 = usage** — something -about the command-line arguments (including a named file's content) is wrong or -inapplicable. Only the "missing environment/recorded state" `exit(2)` sites in -the dispatch handlers + execution fns flip to `exit(1)`; every argument-fault -site (clap parse errors, post-parse validations, malformed/open argv blueprint -files, invalid flag values, unsupported combinations, unknown axis/metric names) -stays `exit(2)`. `reproduce`-diverged stays `exit(1)`. No parser structure -changes. - -**Tech Stack:** Rust 2024, `aura-cli` binary — `crates/aura-cli/src/main.rs`, -`crates/aura-cli/src/graph_construct.rs`, `crates/aura-cli/tests/cli_run.rs`. - ---- - -**Files this plan creates or modifies:** - -- Modify: `crates/aura-cli/src/main.rs` — flip the classified RUNTIME - `std::process::exit(2)` sites to `exit(1)`; usage sites unchanged -- Modify: `crates/aura-cli/src/graph_construct.rs:127-240` — stdin op-script - content + stdin-read I/O → `exit(1)`; argv-flag errors stay `exit(2)` -- Test: `crates/aura-cli/tests/cli_run.rs` — flip the runtime domain-refusal - exit pins (`Some(2)`→`Some(1)`), fix the hidden guard/match arms, add the - partition property test -- Unchanged (must stay green): `crates/aura-cli/tests/cli_broken_pipe.rs` - (SIGPIPE), `crates/aura-cli/tests/graph_construct.rs` (no exit-integer pins) - ---- - -## Task 1: Renegotiate the runtime exit pins to 1 (RED) - -Flip the tests first — they go RED because the code still exits 2. Task 2 flips -the code to make them GREEN. The message substrings are UNCHANGED; only the exit -integer flips. - -**Files:** -- Test: `crates/aura-cli/tests/cli_run.rs` - -- [ ] **Step 1: Flip the direct runtime exit-2 pins to exit-1** - -In `crates/aura-cli/tests/cli_run.rs`, change `assert_eq!(out.status.code(), -Some(2), ...)` to `Some(1)` in these tests (locate by name; the message -`.contains(...)` assertions stay verbatim): - -- `run_real_no_geometry_symbol_refuses_with_exit_2` — the `Some(2)` on the "no - recorded geometry for symbol 'NONEXISTENT'" refusal → `Some(1)` -- `run_real_refusal_names_no_authored_floor` — the `Some(2)` → `Some(1)` -- `run_real_no_geometry_symbol_emits_no_stdout_report` — the `Some(2)` → `Some(1)` -- `chart_emits_self_contained_html_for_a_persisted_run` — the missing-run - sub-assertion `Some(2)` → `Some(1)` -- `chart_tap_nonexistent_on_run_refuses_with_exit_2` — `Some(2)` → `Some(1)` - (keeps "run has no tap named 'nope'", "available:") -- `chart_unknown_name_refuses_with_exit_2_and_message` — `Some(2)` → `Some(1)` - (keeps "no recorded run or family 'ghost'") -- `sweep_real_no_geometry_symbol_refuses_with_exit_2` — `Some(2)` → `Some(1)` - (keeps "no recorded geometry") -- `trace_name_collision_across_kinds_is_refused` — BOTH `Some(2)` → `Some(1)` - -(Optional but consistent: rename these tests dropping the `_exit_2` suffix or -leaving it — a rename is not required and risks a filter miss; leave the names, -only the integer changes.) - -- [ ] **Step 2: Fix the hidden guard / match arms (drift-prone)** - -These tests branch on `code()` and would silently misbehave if left at `Some(2)`: - -- `run_real_sidecar_symbol_never_hits_the_pip_refusal` — the - `match out.status.code() { Some(2) => {..."no local data for symbol - 'EURUSD'"...}, other => panic!(...) }`: change the `Some(2)` arm to `Some(1)` - (a no-data host now exits 1; otherwise the arm misses and hits the panic). -- `run_real_sidecar_index_pip_reaches_the_emitted_manifest` — the skip-guard - `if out.status.code() == Some(2)` on "no local data for symbol 'GER40'": - change to `== Some(1)`. -- `run_real_nonvetted_symbol_resolves_pip_from_sidecar` — the skip-guard `if - out.status.code() == Some(2)` on "no recorded geometry" / "no local data": - change to `== Some(1)`. - -- [ ] **Step 3: Add the partition property test** - -Append to `crates/aura-cli/tests/cli_run.rs`: - -```rust -/// The exit-code partition (#175 iteration 2, deviation #8): a USAGE error (a -/// malformed command line) exits 2; a RUNTIME failure (a well-formed command -/// whose needed data/state is missing) exits 1. Pins the partition as a property. -#[test] -fn exit_codes_partition_usage_two_from_runtime_one() { - // usage: an unknown flag is a command-line error → exit 2 - let usage = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["run", "--bogus"]).output().expect("spawn"); - assert_eq!(usage.status.code(), Some(2), - "unknown flag is a usage error → 2; stderr: {}", String::from_utf8_lossy(&usage.stderr)); - // runtime: a well-formed command whose symbol has no recorded data → exit 1 - let runtime = std::process::Command::new(env!("CARGO_BIN_EXE_aura")) - .args(["run", "--real", "NONEXISTENT"]).output().expect("spawn"); - assert_eq!(runtime.status.code(), Some(1), - "no data for a valid command is a runtime failure → 1; stderr: {}", - String::from_utf8_lossy(&runtime.stderr)); -} -``` - -- [ ] **Step 4: Run the flipped tests to verify they FAIL (RED)** - -Run: `cargo test -p aura-cli --test cli_run exit_codes_partition_usage_two_from_runtime_one` -Expected: FAIL — the runtime arm gets `Some(2)` (code still exits 2), not `Some(1)`. - -Run: `cargo test -p aura-cli --test cli_run run_real_no_geometry_symbol_refuses` -Expected: FAIL — `Some(2)` received where `Some(1)` now asserted. - ---- - -## Task 2: Flip the runtime code sites to exit 1 (GREEN) - -Change `std::process::exit(2)` to `std::process::exit(1)` at the classified -RUNTIME sites in `main.rs` and `graph_construct.rs`. Locate each by its enclosing -fn + message string (line numbers drift as edits land). USAGE sites are NOT -touched. After this, the Task-1 tests go green. - -**Files:** -- Modify: `crates/aura-cli/src/main.rs` -- Modify: `crates/aura-cli/src/graph_construct.rs` - -- [ ] **Step 1: Flip the missing-data / missing-recorded-state sites (main.rs)** - -Change `exit(2)` → `exit(1)` at each (by fn + message): -- `no_real_data` — `"no local data for symbol '{symbol}' …"` -- `pip_or_refuse` — `"no recorded geometry for symbol '{symbol}' …"` -- `emit_chart` — the store-read failure, the tap failure ("run has no tap - named"), the read_family failure, the build_comparison failure, and `"no - recorded run or family '{name}'…"` (all five exit sites in `emit_chart`) -- `runs_families` — the `load_family_members` failure -- `runs_family` — the `load_family_members` failure - -- [ ] **Step 2: Flip the name-collision sites (main.rs)** - -Change `exit(2)` → `exit(1)` at every `ensure_name_free` refusal (the `"{e}"` -after `ensure_name_free(...)`), in: `run_sample`, `run_sample_real`, `run_sweep`, -`run_walkforward`, `run_mc`, `run_macd`, `run_stage1_r`. (These are the -trace-name-collision-across-kinds class the spec lists as runtime.) - -- [ ] **Step 3: Flip the reproduce missing/corrupted-stored-state sites (main.rs)** - -Change `exit(2)` → `exit(1)` in `reproduce_family_in` at: `load_family_members` -failure, `"no such family '{id}'"`, `"family member has no topology_hash; not a -generated run"`, the `get_blueprint` failure, `"blueprint {hash} missing from -store"`, `"stored blueprint {hash} does not parse: {e:?}"`; and in -`point_from_params` at `"manifest is missing param {}"`. (These are missing / -corrupt content-addressed store state = runtime.) LEAVE `reproduce_family`'s -diverged-member `exit(1)` unchanged (already 1). - -- [ ] **Step 4: Normalize the persist-write class to 1 (main.rs)** - -Change `exit(2)` → `exit(1)` at every persist WRITE failure so the whole class is -consistent (`run_generalize`'s `"failed to persist family"` is already `exit(1)` -— leave it; the others flip to match): -- `persist_traces` — `"trace persist failed: {e}"` (TraceStore::write) -- `persist_traces_r` — `"trace persist failed: {e}"` (TraceStore::write) -- `run_sweep` — `append_family` failure -- `run_walkforward` — `append_family` failure -- `run_mc` — `append_family` failure -- `run_blueprint_sweep` — `put_blueprint` and `append_family` failures -- `run_blueprint_walkforward` — `put_blueprint` and `append_family` failures -- `run_blueprint_mc` — `put_blueprint` and `append_family` failures - -- [ ] **Step 5: Flip the stdin runtime sites (graph_construct.rs)** - -Change `exit(2)` → `exit(1)` where the fault is stdin op-script CONTENT or a -stdin-read I/O failure; LEAVE argv-flag usage errors at `exit(2)`: -- `build_cmd` — `"reading stdin: {e}"` (stdin-read I/O) → 1 -- `build_cmd` — `build_from_str` `"{msg}"` (op-script content) → 1 -- `introspect_cmd` — the two `"reading stdin: {e}"` (stdin-read I/O) → 1 -- `introspect_cmd` — `introspect_unwired` `"{m}"` (op-script content) → 1 -- `introspect_cmd` — `build_from_str` `"{m}"` for `--content-id` (op-script - content) → 1 -- LEAVE at `exit(2)`: `introspect_cmd` — `"usage: aura graph introspect …"` - (argv-flag error) and `introspect_node` `"{m}"` for `--node ` (an invalid - argv flag value = usage). - -- [ ] **Step 6: Confirm the usage sites are untouched** - -Verify (read, do not edit) that these STAY `exit(2)`: every `dispatch_*` -argument-applicability guard; `run_data_from` (`--from`/`--to` without `--real`); -`run_args_from` (harness / cost-flags-require-R / non-negative / window); -`generalize_args_from` (required knobs / ≥2 instruments / duplicate / strategy); -`dispatch_runs` rank grammar; all the dual-grammar blueprint FILE read/parse -sites in the four `.json` branches (`read_to_string` + `blueprint_from_json` in -`dispatch_run`/`dispatch_sweep`/`dispatch_walkforward`/`dispatch_mc` — a named -argv file's bad content is a usage error, staying 2); the applicability refusals -(walkforward-form-for-strategy, mc-requires-closed-blueprint, unknown `--axis`, -invalid `--metric`/`--select`/`--strategy`). - -- [ ] **Step 7: Build + full-suite gate** - -Run: `cargo build --workspace` -Expected: PASS (0 errors). - -Run: `cargo clippy --workspace --all-targets -- -D warnings` -Expected: PASS. - -Run: `cargo test --workspace` -Expected: PASS — the Task-1 flipped pins + partition property test now GREEN; the -usage-error pins unchanged and green; `cli_broken_pipe.rs` and -`graph_construct.rs` green untouched. - ---- - -## Notes for the executor - -- **Behaviour change, pinned.** This iteration deliberately changes observable - exit codes for runtime failures (2→1). Task 1 flips the pins RED-first; Task 2 - flips the code GREEN. Do not "fix" a red Task-1 pin by reverting it — the red - is the point until Task 2 lands. -- **Attribution rule for any site not listed:** if the user must fix the COMMAND - LINE (a flag value, an unsupported combination, a named file's content) → stays - `exit(2)`; if the command was well-formed but the ENVIRONMENT/recorded state is - missing or the STDIN data is bad → `exit(1)`. -- **The dual-grammar quartet stays in lockstep:** the four `.json`-branch file - read/parse sites are all usage (exit 2) — do not flip any one of them. -- **No message-string edits.** Only the exit integer changes; every `.contains()` - substring pin stays satisfied. The mixed "Usage:"/"usage:" casing is left as-is - this iteration (a separate cosmetic-polish concern). diff --git a/docs/specs/0098-cli-clap-gnu-compliance.md b/docs/specs/0098-cli-clap-gnu-compliance.md deleted file mode 100644 index d9f38d1..0000000 --- a/docs/specs/0098-cli-clap-gnu-compliance.md +++ /dev/null @@ -1,373 +0,0 @@ -# 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.