From 88e9c9a539f4685426b69c4f7edf28323bc49e20 Mon Sep 17 00:00:00 2001 From: Brummel Date: Wed, 1 Jul 2026 18:41:07 +0200 Subject: [PATCH] fix(cli): symmetric sweep/mc family-verb polish (closed error, mc seed label, mandatory-knob docs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three polish items surfaced by the World/C21 milestone fieldtest: - A sweep of a fully-bound (closed) blueprint is refused up front with a clear "this blueprint is fully bound; nothing to sweep" message — the symmetric inverse of blueprint_mc_family's closed-blueprint guard — instead of leaking a misleading UnknownKnob for a knob that is not unknown but bound out. blueprint_sweep_family's error contract widens to String; the sweep terminal's BindError (a genuine unknown/kind-mismatch axis) still surfaces verbatim. - mc reproduce lines now carry each member's seed= label. Monte-Carlo members hold no tuning params, so the member label was blank; the seed is each draw's realization identity. Sweep / walk-forward labels (which echo params) unchanged. - docs (design ledger + glossary): state that every open knob --list-axes prints is mandatory on sweep / walkforward — a subset is refused with the missing knob named (BindError::MissingKnob), there is no default; pin a knob you do not want to vary with a single-value axis. RED-first for the two behaviour items. One existing unknown-axis E2E is re-pointed from the closed fixture (which now hits the new "nothing to sweep" pre-check) to the open fixture; it still exercises UnknownKnob("nope") specifically — resolve raises UnknownKnob in phase 1 before MissingKnob in phase 2 — so the property is preserved, not weakened. closes #178 --- crates/aura-cli/src/main.rs | 132 +++++++++++++++++++++++++++---- crates/aura-cli/tests/cli_run.rs | 7 +- docs/design/INDEX.md | 5 +- docs/glossary.md | 4 +- 4 files changed, 125 insertions(+), 23 deletions(-) diff --git a/crates/aura-cli/src/main.rs b/crates/aura-cli/src/main.rs index 2d5e5de..4d87fbf 100644 --- a/crates/aura-cli/src/main.rs +++ b/crates/aura-cli/src/main.rs @@ -2247,13 +2247,20 @@ fn reproduce_family_in(reg: &Registry, id: &str, data: &DataSource) -> Reproduce let space = wrap_stage1r(reload(), tx_eq, tx_ex, tx_r, tx_req, false, true, None).param_space(); let point = point_from_params(&space, &stored.manifest.params); - let label = stored - .manifest - .params - .iter() - .map(|(n, v)| format!("{n}={}", render_value(v))) - .collect::>() - .join(", "); + // A MonteCarlo member carries no tuning params (the params-join is empty), so its + // reproduce line would print a BLANK member label; the seed IS its realization + // identity, so surface `seed=` instead. Sweep / walk-forward members echo their + // tuning params (the params-join), unchanged. + let label = match family.kind { + FamilyKind::MonteCarlo => format!("seed={}", stored.manifest.seed), + _ => stored + .manifest + .params + .iter() + .map(|(n, v)| format!("{n}={}", render_value(v))) + .collect::>() + .join(", "), + }; // Realization-aware: a MonteCarlo member ran over a seed-driven synthetic walk, // not the showcase — reconstruct it from manifest.seed so the re-run matches (C1). // The seed is manifest-carried and identical across realizations; only the sources @@ -2896,14 +2903,19 @@ fn list_blueprint_axes(doc: &str) { /// `!Clone`, so the throwaway param-space probe and each grid point each reload). (3) /// The axes are taken verbatim BY NAME (not the four suffix-resolved stage1-r knobs): /// each `(name, vals)` is fed straight to the `SweepBinder`, so an unknown name or a -/// kind mismatch surfaces as a [`BindError`] from the sweep terminal — a named error, -/// never a panic. Every member manifest carries the shared `topology_hash` of the -/// loaded signal; reduce-mode fold, identical to the mirror's default (no-trace) arm. +/// kind mismatch surfaces as the sweep terminal's [`BindError`], rendered to a message +/// string — a named error, never a panic. A FULLY BOUND (closed) blueprint has an empty +/// `param_space` — nothing to sweep — and is refused up front with a clear message +/// (the symmetric inverse of [`blueprint_mc_family`]'s closed-blueprint requirement), +/// pre-empting the misleading `UnknownKnob()` the per-axis resolve would emit for a +/// knob that is not unknown but bound out. Every member manifest carries the shared +/// `topology_hash` of the loaded signal; reduce-mode fold, identical to the mirror's +/// default (no-trace) arm. fn blueprint_sweep_family( doc: &str, axes: &[(String, Vec)], data: &DataSource, -) -> Result { +) -> Result { // The doc is parse-validated at the dispatch boundary (with file-path context), // so every reload here is infallible: the builder has a single error contract — // the `BindError` returned by the sweep terminal — and no hidden process exit. @@ -2919,6 +2931,18 @@ fn blueprint_sweep_family( // `blueprint_axis_probe` single-sources (identical `false, true, None` wrap). let probe = blueprint_axis_probe(doc); let space = probe.param_space(); + // A fully bound blueprint has no open knob — there is nothing to sweep. Refuse it up + // front (mirroring blueprint_mc_family's closed-blueprint guard, inverted) BEFORE the + // per-axis resolve, which would otherwise return `UnknownKnob()` — misleading, as + // the named knob is not unknown but bound out. Exit-free: the message is returned for + // the IO wrapper to render + exit 2, so the precondition is unit-testable. + if space.is_empty() { + return Err( + "this blueprint is fully bound; nothing to sweep (use `aura sweep --list-axes` \ + to confirm there are no open knobs)" + .to_string(), + ); + } // seed the named axes verbatim: the first via Composite::axis (consumes the probe), // the rest via SweepBinder::axis. resolve_axes name- and kind-checks them at the // sweep terminal, so an UnknownKnob / KindMismatch is returned, not panicked. @@ -2928,11 +2952,15 @@ fn blueprint_sweep_family( for (n, vals) in iter { binder = binder.axis(n, vals.clone()); } - binder.sweep(|point| { - // fresh per-member graph (Composite is !Clone, reload per member) run through - // the shared reduce-mode member path — the same fn reproduction re-runs. - run_blueprint_member(reload(doc), point, &space, data.run_sources(), window, 0, pip, &topo) - }) + binder + .sweep(|point| { + // fresh per-member graph (Composite is !Clone, reload per member) run through + // the shared reduce-mode member path — the same fn reproduction re-runs. + run_blueprint_member(reload(doc), point, &space, data.run_sources(), window, 0, pip, &topo) + }) + // render the sweep terminal's BindError to a message (the fn's String error contract), + // so `UnknownKnob("nope")` still surfaces verbatim at the IO wrapper. + .map_err(|e| format!("{e:?}")) } /// Sweep the LOADED blueprint over the user `--axis` grid on an in-sample window @@ -3125,7 +3153,7 @@ fn blueprint_mc_family(doc: &str, n_seeds: u64, data: &DataSource) -> Result)], name: &str, persist: bool, data: DataSource) { let _ = persist; // reserved for the deferred per-member trace path; the family record below is unconditional let family = blueprint_sweep_family(doc, axes, &data).unwrap_or_else(|e| { - eprintln!("aura: {e:?}"); + eprintln!("aura: {e}"); std::process::exit(2); }); let reg = default_registry(); @@ -5613,6 +5641,76 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Property: sweeping a FULLY BOUND (closed) blueprint is refused up front with a + /// clear "fully bound / nothing to sweep" message — NOT the terse `UnknownKnob()` + /// the per-axis resolve would otherwise emit (misleading: the named knob is not unknown, + /// it is bound out, so there is simply nothing to sweep). Symmetric inverse of + /// `blueprint_mc_family_rejects_an_open_blueprint` (mc requires a closed blueprint; a + /// sweep requires >= 1 open knob). Type-agnostic on the error (renders it via Debug), so + /// the RED phase compiles whether the builder returns a `BindError` or a `String`. + #[test] + fn blueprint_sweep_family_rejects_a_fully_bound_blueprint() { + let closed = stage1_signal(Some(2), Some(4)); // both SMA knobs bound -> empty param_space + let doc = blueprint_to_json(&closed).expect("serializes"); + // an axis naming a bound-out knob: the pre-fix path returns UnknownKnob for it. + let axes = vec![( + "stage1_signal.slow.length".to_string(), + vec![Scalar::i64(4), Scalar::i64(6)], + )]; + let err = match blueprint_sweep_family(&doc, &axes, &DataSource::Synthetic) { + Ok(_) => panic!("a fully-bound blueprint has nothing to sweep"), + Err(e) => format!("{e:?}"), + }; + assert!( + err.contains("fully bound") || err.contains("nothing to sweep"), + "names the fully-bound / nothing-to-sweep condition: {err}" + ); + assert!( + !err.contains("UnknownKnob"), + "must not leak the misleading UnknownKnob: {err}" + ); + } + + /// Property: an MC family's `aura reproduce` lines carry the member's own `seed=` + /// label. MC members hold no tuning params (the params-join is empty), so the line would + /// otherwise print a BLANK member label; the seed is each draw's realization identity and + /// must show. Sweep / walk-forward labels still echo their params (covered elsewhere). + #[test] + fn reproduce_mc_member_labels_carry_the_seed() { + let dir = std::env::temp_dir().join(format!("aura-repro-mc-seed-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("temp dir"); + let reg = Registry::open(dir.join("runs.jsonl")); + + let closed = stage1_signal(Some(2), Some(4)); // MC binds no axis -> closed blueprint + let doc = blueprint_to_json(&closed).expect("serializes"); + let data = DataSource::Synthetic; + let family = blueprint_mc_family(&doc, 3, &data).expect("closed blueprint"); + + let topo = family.draws[0].report.manifest.topology_hash.clone().expect("topo"); + let canonical = + blueprint_to_json(&blueprint_from_json(&doc, &|t| std_vocabulary(t)).unwrap()).unwrap(); + reg.put_blueprint(&topo, &canonical).expect("store blueprint"); + let id = reg + .append_family("mcseed", FamilyKind::MonteCarlo, &mc_member_reports(&family)) + .expect("append"); + + let rep = reproduce_family_in(®, &id, &data); + assert_eq!(rep.outcomes.len(), 3, "three MC members"); + for (label, _) in &rep.outcomes { + assert!( + label.starts_with("seed="), + "an MC reproduce label carries the seed, not a blank params-join: {label:?}" + ); + } + let seen: HashSet<&str> = rep.outcomes.iter().map(|(l, _)| l.as_str()).collect(); + assert!( + seen.contains("seed=1") && seen.contains("seed=2") && seen.contains("seed=3"), + "each MC draw's own seed appears: {seen:?}" + ); + let _ = std::fs::remove_dir_all(&dir); + } + /// Property: an f64 blueprint param survives the content-addressed store's /// serialize -> parse -> re-serialize round-trip **bit-identically**, so `aura /// reproduce` re-derives an f64-bearing member without DIVERGED. This is exactly diff --git a/crates/aura-cli/tests/cli_run.rs b/crates/aura-cli/tests/cli_run.rs index 20c255e..d019d6f 100644 --- a/crates/aura-cli/tests/cli_run.rs +++ b/crates/aura-cli/tests/cli_run.rs @@ -3378,11 +3378,12 @@ fn aura_sweep_persists_the_canonical_blueprint_keyed_by_topology_hash() { } /// An `--axis` name that does not resolve against the loaded blueprint's param_space -/// fails clean (a named BindError, non-zero exit), not a panic. The fully-bound -/// fixture has an empty param_space, so any axis name is unknown. +/// fails clean (a named BindError, non-zero exit), not a panic. The OPEN fixture has +/// sweepable knobs, so a nonsense axis name (`nope`) is genuinely unknown — distinct +/// from a fully-bound blueprint, which is refused earlier as "nothing to sweep". #[test] fn aura_sweep_rejects_an_unknown_axis() { - let fixture = format!("{}/tests/fixtures/stage1_signal.json", env!("CARGO_MANIFEST_DIR")); + let fixture = format!("{}/tests/fixtures/stage1_signal_open.json", env!("CARGO_MANIFEST_DIR")); let out = Command::new(BIN) .args(["sweep", &fixture, "--axis", "nope=1,2", "--name", "f"]) .output() diff --git a/docs/design/INDEX.md b/docs/design/INDEX.md index 3084d6e..de6be30 100644 --- a/docs/design/INDEX.md +++ b/docs/design/INDEX.md @@ -1938,7 +1938,10 @@ is the machinery, not trader-grade MC statistics; a real-data block-bootstrap warms poorly → silent-vacuous draws today) — ride #172. **Axis-name discovery shipped (cycle 0096, #169):** `aura sweep --list-axes` lists a loaded blueprint's open sweepable knobs (one `:` per line, `param_space()` order), then exits — the names it -prints are exactly what `--axis` binds. A single `blueprint_axis_probe` helper now single-sources +prints are exactly what `--axis` binds. Every listed name is **mandatory** on a `sweep` / +`walkforward`: the blueprint must be fully bound before it runs, so a subset grid is refused with +the missing knob named (`BindError::MissingKnob`) and there is no default — pin a knob you do not +want to vary with a single-value axis (`--axis =`). A single `blueprint_axis_probe` helper now single-sources the wrapped probe (`wrap_stage1r(loaded_signal).param_space()`) for the sweep terminal, the MC closed-check, AND the listing (three former inline copies → one), so **listed == swept by construction** (and stays so across #159's harness retirement — the listing tracks whatever the diff --git a/docs/glossary.md b/docs/glossary.md index 4c991f3..744482e 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -261,7 +261,7 @@ The harness's structural parameterization — which strategy, instrument(s), bro ### sweep **Avoid:** param-sweep, parameter sweep -An orchestration axis varying tuning params (grid or random) within a fixed structure. The inner, param-tuning loop, distinct from the structural experiment matrix. +An orchestration axis varying tuning params (grid or random) within a fixed structure. The inner, param-tuning loop, distinct from the structural experiment matrix. On a loaded blueprint every open knob (`--list-axes`) is **required** — a subset is refused with the missing knob named; pin an unwanted knob with a single-value axis (`--axis name=`), there is no default. ### tap **Avoid:** — @@ -273,7 +273,7 @@ The **optional** documented pre-trade-gate seam in the execution chain (`stop-ru ### walk-forward **Avoid:** — -An orchestration axis: rolling in-sample optimize + out-of-sample test across moving windows, stitched into one out-of-sample verdict plus parameter stability. +An orchestration axis: rolling in-sample optimize + out-of-sample test across moving windows, stitched into one out-of-sample verdict plus parameter stability. As with `sweep`, every open knob named by `--list-axes` is **required** on a loaded blueprint — a subset is refused with the missing knob named; pin one with a single-value axis, there is no default. ### World **Avoid:** —