Brummel/Aura
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

audit(0099): cycle close — CLI clap adoption; ledger refreshed (C14); rm ephemeral spec/plan

Browse Source 
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
This commit is contained in:
Brummel
2026-07-01 20:11:11 +02:00
parent fa42bf3878
commit 27f0f8ea11
4 changed files with 20 additions and 1265 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/0098-cli-clap-migration-iter1.md
-659
View File
@@ -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 <sub> --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<String>,
/// Render stacked panels instead of an overlay.
#[arg(long)] panels: bool,
}
#[derive(Args)]
struct GraphCmd {
#[command(subcommand)]
sub: Option<GraphSub>,
}
#[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<String>,
#[arg(long)] unwired: bool,
#[arg(long)] content_id: bool,
}
#[derive(Args)]
struct GeneralizeCmd {
#[arg(long)] strategy: Option<String>,
/// Comma-separated instrument list (≥2 distinct, required).
#[arg(long)] real: Option<String>,
#[arg(long)] fast: Option<i64>,
#[arg(long)] slow: Option<i64>,
#[arg(long)] stop_length: Option<i64>,
#[arg(long)] stop_k: Option<f64>,
#[arg(long)] from: Option<i64>,
#[arg(long)] to: Option<i64>,
#[arg(long)] metric: Option<String>,
#[arg(long)] name: Option<String>,
}
#[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<String>,
/// The metric to rank by (only valid after `rank`).
metric: Option<String>,
},
}
#[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<String>,
/// Built-in harness: sma | macd | stage1-r (default sma).
#[arg(long)] harness: Option<String>,
/// Blueprint params (JSON scalar-cell array; .json mode).
#[arg(long)] params: Option<String>,
/// Blueprint seed (.json mode).
#[arg(long)] seed: Option<u64>,
#[arg(long)] real: Option<String>,
#[arg(long)] from: Option<i64>,
#[arg(long)] to: Option<i64>,
#[arg(long)] trace: Option<String>,
#[arg(long, long_help = "per-trade cost in price units. stage1-r only; charged in R as cost/|entry-stop|.")]
cost_per_trade: Option<f64>,
#[arg(long, long_help = "slippage multiplier on realized vol. stage1-r only.")]
slip_vol_mult: Option<f64>,
#[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<f64>,
}
#[derive(Args)]
struct SweepCmd {
/// A loaded blueprint (.json); omit for the built-in --strategy grammar.
blueprint: Option<String>,
#[arg(long)] strategy: Option<String>,
#[arg(long)] real: Option<String>,
#[arg(long)] from: Option<i64>,
#[arg(long)] to: Option<i64>,
#[arg(long)] name: Option<String>,
#[arg(long)] trace: Option<String>,
#[arg(long)] fast: Option<String>,
#[arg(long)] slow: Option<String>,
#[arg(long)] stop_length: Option<String>,
#[arg(long)] stop_k: Option<String>,
#[arg(long)] channel: Option<String>,
#[arg(long)] window: Option<String>,
#[arg(long)] band_k: Option<String>,
/// Blueprint sweep axis `<name>=<csv>` (repeatable; .json mode).
#[arg(long)] axis: Vec<String>,
/// 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<String>,
#[arg(long)] strategy: Option<String>,
#[arg(long)] real: Option<String>,
#[arg(long)] from: Option<i64>,
#[arg(long)] to: Option<i64>,
#[arg(long)] name: Option<String>,
#[arg(long)] trace: Option<String>,
#[arg(long)] fast: Option<String>,
#[arg(long)] slow: Option<String>,
#[arg(long)] stop_length: Option<String>,
#[arg(long)] stop_k: Option<String>,
#[arg(long)] select: Option<String>,
/// Blueprint IS-refit axis `<name>=<csv>` (repeatable, ≥1 required; .json mode).
#[arg(long)] axis: Vec<String>,
}
#[derive(Args)]
struct McCmd {
/// A loaded blueprint (.json); omit for the built-in grammar.
blueprint: Option<String>,
#[arg(long)] strategy: Option<String>,
#[arg(long)] real: Option<String>,
#[arg(long)] from: Option<i64>,
#[arg(long)] to: Option<i64>,
#[arg(long)] name: Option<String>,
#[arg(long)] trace: Option<String>,
#[arg(long)] fast: Option<String>,
#[arg(long)] slow: Option<String>,
#[arg(long)] stop_length: Option<String>,
#[arg(long)] stop_k: Option<String>,
#[arg(long)] block_len: Option<usize>,
#[arg(long)] resamples: Option<usize>,
#[arg(long)] seed: Option<u64>,
/// Number of synthetic draws (required; .json mode).
#[arg(long)] seeds: Option<u64>,
}
```
- [ ] **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<String>) -> 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, &params, 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 <id> [rank <metric>]"); 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<i64>, to: Option<i64>) -> 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<RunArgs, String>` — 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<String>, Stage1RGrid, String, Option<i64>, Option<i64>), 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 <m>` / 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.
docs/plans/0099-cli-exit-code-split-iter2.md
-233
View File
@@ -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 <T>` (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).