From 00e15b937158e5406ae426ea0bc8c3137cc8c5e5 Mon Sep 17 00:00:00 2001 From: Brummel Date: Fri, 10 Jul 2026 22:16:09 +0200 Subject: [PATCH] feat(cli): sweep/walkforward --trace writes every member's tap series (#168 refusal lifted) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --trace now translates through the verb sugar into a persist_taps campaign-presentation block (was persist_taps: vec![] — the #168 no-op) and persist_campaign_traces writes EVERY member of a selection-free swept cell, one subdirectory per member under the cell dir, keyed by the member's manifest params label in reproduce's exact format (filesystem-sanitized, ordinal fallback); nominee cells keep their flat layout. Members re-run at the cell's own stop regime and resolved pip, streaming one member at a time, with the C1 drift alarm preserved per member. The walkforward sibling lifts identically; run/mc --trace stay refused by design (no campaign context / no family-name concept — their help was already honest). Clap help lines and the chart NotFound hint restored; the old #168 refusal pins re-pinned to the delivered behaviour. Post-review repairs: member_trace_key + cell_member_fanout extracted pure and unit-pinned (6 tests), persisted_cells counts cells again, cell-invariant space/geo hoisted out of the member loop, verb_sugar tail comment corrected. Verified: headline e2e green (GER40 archive), cli_run 124/0, full workspace suite green, clippy -D warnings clean; independent spec review compliant, quality review findings all repaired. closes #224 refs #168 --- crates/aura-cli/src/campaign_run.rs | 363 +++++++++++++++++++++------- crates/aura-cli/src/main.rs | 79 +++--- crates/aura-cli/src/verb_sugar.rs | 82 ++++++- crates/aura-cli/tests/cli_run.rs | 18 +- 4 files changed, 411 insertions(+), 131 deletions(-) diff --git a/crates/aura-cli/src/campaign_run.rs b/crates/aura-cli/src/campaign_run.rs index 1a2eb52..0a72bb9 100644 --- a/crates/aura-cli/src/campaign_run.rs +++ b/crates/aura-cli/src/campaign_run.rs @@ -17,7 +17,7 @@ use std::path::{Path, PathBuf}; use std::sync::{mpsc, Arc}; -use aura_campaign::{CampaignOutcome, CellSpec, ExecFault, MemberFault, MemberRunner}; +use aura_campaign::{CampaignOutcome, CellOutcome, CellSpec, ExecFault, MemberFault, MemberRunner}; use aura_core::{Cell, ParamSpec, Scalar, ScalarKind, Timestamp}; use aura_engine::{ blueprint_from_json, f64_field, summarize, summarize_r, ColumnarTrace, FamilySelection, @@ -635,22 +635,72 @@ fn campaign_cell_key( crate::sanitize_component(&base) } -/// Persist the requested `persist_taps` for every nominee cell under -/// `traces///` (0109, #201 d5): re-run each nominee +/// The per-member subdirectory key under a swept cell's trace dir (#224): +/// the member's own manifest params label — the exact `"name=value, ..."` +/// join `aura reproduce` prints — sanitized for filesystem use (a raw label +/// carries `=`/`, ` which `sanitize_component` maps to `_`). The member's +/// ordinal into the family is the fallback when the label is empty (a closed +/// blueprint / a monte-carlo seed member carries no tuning params). +fn member_trace_key(report: &RunReport, ordinal: usize) -> String { + let label = report + .manifest + .params + .iter() + .map(|(n, v)| format!("{n}={}", crate::render_value(v))) + .collect::>() + .join(", "); + let key = if label.is_empty() { ordinal.to_string() } else { label }; + crate::sanitize_component(&key) +} + +/// The per-cell member fan-out (#224): a nominee cell writes its one trace +/// directly at `/` (`None` subdir key, unchanged since 0109); a +/// no-nominee cell with a completed terminal family (the selection-free sweep +/// shape) writes every one of that family's members under its own +/// `//` subdirectory instead of silently dropping every +/// member but a (non-existent) nominee. An empty return (no nominee AND no +/// non-empty terminal family) is the caller's cue to skip the cell loudly. +/// Pure over the executed outcome's own `CellOutcome`, so it is unit-testable +/// without booting a real archive run. +fn cell_member_fanout(cell_out: &CellOutcome) -> Vec<(Option, &RunReport)> { + match &cell_out.nominee { + Some((_, nominee_report)) => vec![(None, nominee_report)], + None => match cell_out.families.last() { + Some(fam) if !fam.reports.is_empty() => fam + .reports + .iter() + .enumerate() + .map(|(i, r)| (Some(member_trace_key(r, i)), r)) + .collect(), + _ => Vec::new(), + }, + } +} + +/// Persist the requested `persist_taps` for every cell under +/// `traces///` (0109, #201 d5; #224): a cell with a +/// nominee (walkforward/generalize/mc — selection-bearing pipelines) writes +/// its single trace directly at `/`, unchanged since 0109. A cell +/// with NO nominee but a completed terminal family (a selection-free sweep, +/// #224's headline: "each swept member's tap series") writes EVERY member of +/// that family under its own `//` subdirectory — never +/// narrowing to one nominated member, which would silently drop the others +/// the sweep actually produced. Each written member is independently re-run /// once in non-reduce trace mode over its own recorded `manifest.window`, -/// assert the re-run METRICS equal the recorded nominee metrics (the C1 +/// asserting the re-run METRICS equal the recorded member metrics (the C1 /// drift alarm — manifest fields are fresh-context and not compared), and -/// write the requested-AND-producible taps through the sweep verbs' +/// writes the requested-AND-producible taps through the sweep verbs' /// member-dir mechanism: a slash-joined `"{name}/{key}"` handed to /// `TraceStore::write` (the `persist_traces_r(&format!("{name}/{key}"), ..)` /// layout — the fn itself is not reused: it writes a fixed tap set and /// process-exits on error). The written index manifest is the RECORDED -/// nominee's — the trace documents that run's provenance, not a re-derived -/// fresh-context one. Loud stderr per no-nominee cell and ONCE per -/// unproducible requested tap (producibility is run-configuration-level, -/// not per-cell); one summary line at the end. Every `Err` is a refusal -/// `campaign_cmd` renders as `aura: {msg}` + exit 1. -fn persist_campaign_traces( +/// member's — the trace documents that run's provenance, not a re-derived +/// fresh-context one. Loud stderr per no-candidate cell (neither a nominee +/// nor a non-empty terminal family) and ONCE per unproducible requested tap +/// (producibility is run-configuration-level, not per-cell); one summary +/// line at the end. Every `Err` is a refusal `campaign_cmd` renders as +/// `aura: {msg}` + exit 1. +pub(crate) fn persist_campaign_traces( trace_name: &str, taps: &[String], outcome: &CampaignOutcome, @@ -681,13 +731,20 @@ fn persist_campaign_traces( // construction: `execute` pushes both in the same cell-loop iteration. let mut persisted_cells = 0usize; for (cell_rec, cell_out) in outcome.record.cells.iter().zip(&outcome.cells) { - let Some((_, nominee_report)) = &cell_out.nominee else { + // A nominee cell writes its one trace directly at `/` + // (`None` subdir key, unchanged since 0109); a no-nominee cell with a + // completed terminal family (the selection-free sweep shape, #224) + // writes every one of that family's members under its own + // `//` subdirectory instead of silently + // dropping every member but a (non-existent) nominee. + let members = cell_member_fanout(cell_out); + if members.is_empty() { eprintln!( "aura: cell {}/{}/[{}, {}]: no nominee; no traces persisted", cell_rec.strategy, cell_rec.instrument, cell_rec.window_ms.0, cell_rec.window_ms.1 ); continue; - }; + } if routed.is_empty() { continue; } @@ -724,18 +781,12 @@ fn persist_campaign_traces( cell_rec.strategy ) })?; - - // Re-run the nominee, non-reduce: the SAME member the executor ran - // (same wrapped space, same params, same window, seed-free real - // data), mirroring `CliMemberRunner::run_member` with the reduce - // fold off so the per-cycle tap streams exist. The window is the - // nominee report's own `manifest.window` — already epoch-ns - // (`run_blueprint_member` stamped the post-seam bounds), so no - // second ms->ns crossing here. + // Cell-invariant across every member of this cell (the wrapped param + // space depends only on the cell's blueprint; the resolved geometry + // only on the cell's instrument) — computed once here rather than + // redundantly inside the member loop below. let space = crate::blueprint_axis_probe(blueprint_json, env).param_space(); - let point = crate::point_from_params(&space, &nominee_report.manifest.params); - let (from, to) = nominee_report.manifest.window; - // Same resolved pip the nominee ran at (`CliMemberRunner::run_member`'s + // Same resolved pip the member ran at (`CliMemberRunner::run_member`'s // `geo.pip_size`), else the C1 drift-alarm equality below would trip // spuriously on a re-run computed at a different (default) pip. let geo = instrument_geometry(server, &cell_rec.instrument).ok_or_else(|| { @@ -746,76 +797,92 @@ fn persist_campaign_traces( env.data_path() ) })?; - let no_data = || { - format!( - "no data for instrument {} in the nominee window [{}, {}] (epoch-ns)", - cell_rec.instrument, from.0, to.0 + + for (member_subdir, member_report) in members.iter().cloned() { + // Re-run the member, non-reduce: the SAME member the executor ran + // (same wrapped space, same params, same window, seed-free real + // data), mirroring `CliMemberRunner::run_member` with the reduce + // fold off so the per-cycle tap streams exist. The window is the + // member report's own `manifest.window` — already epoch-ns + // (`run_blueprint_member` stamped the post-seam bounds), so no + // second ms->ns crossing here. + let point = crate::point_from_params(&space, &member_report.manifest.params); + let (from, to) = member_report.manifest.window; + let no_data = || { + format!( + "no data for instrument {} in the member window [{}, {}] (epoch-ns)", + cell_rec.instrument, from.0, to.0 + ) + }; + let source = M1FieldSource::open_window( + server, + &cell_rec.instrument, + Some(from), + Some(to), + M1Field::Close, ) - }; - let source = M1FieldSource::open_window( - server, - &cell_rec.instrument, - Some(from), - Some(to), - M1Field::Close, - ) - .ok_or_else(no_data)?; - if aura_engine::Source::peek(&source).is_none() { - return Err(no_data()); - } - let signal = blueprint_from_json(blueprint_json, &|t| env.resolve(t)) - .expect("stored blueprint passed the referential gate; reload is infallible"); - // The cell's OWN regime (#219/#212), not the hardcoded default: the - // recorded nominee ran under `cell_rec.regime`, bound through the same - // `stop_rule_for_regime` helper `CliMemberRunner::run_member` uses, so - // the two sites cannot drift apart again (the #219 divergence class) - // and the re-run binds the same stop the C1 drift alarm below relies on. - let stop = stop_rule_for_regime(cell_rec.regime); - let (tx_eq, rx_eq) = mpsc::channel(); - let (tx_ex, rx_ex) = mpsc::channel(); - let (tx_r, rx_r) = mpsc::channel(); - let (tx_req, rx_req) = mpsc::channel(); - let mut h = crate::wrap_r(signal, tx_eq, tx_ex, tx_r, tx_req, stop, false, geo.pip_size) - .bootstrap_with_cells(&point) - .expect("the nominee's point re-bootstraps (it already ran this realization)"); - let sources: Vec> = vec![Box::new(source)]; - h.run(sources); - // Drain ALL FOUR channels (`run_signal_r` leaves req undrained; the - // trace path must not): eq/ex/req feed the taps, r feeds the metrics. - let eq_rows: Vec<(Timestamp, Vec)> = rx_eq.try_iter().collect(); - let ex_rows: Vec<(Timestamp, Vec)> = rx_ex.try_iter().collect(); - let r_rows: Vec<(Timestamp, Vec)> = rx_r.try_iter().collect(); - let req_rows: Vec<(Timestamp, Vec)> = rx_req.try_iter().collect(); + .ok_or_else(no_data)?; + if aura_engine::Source::peek(&source).is_none() { + return Err(no_data()); + } + let signal = blueprint_from_json(blueprint_json, &|t| env.resolve(t)) + .expect("stored blueprint passed the referential gate; reload is infallible"); + // The cell's OWN regime (#219/#212), not the hardcoded default: the + // recorded member ran under `cell_rec.regime`, bound through the same + // `stop_rule_for_regime` helper `CliMemberRunner::run_member` uses, so + // the two sites cannot drift apart again (the #219 divergence class) + // and the re-run binds the same stop the C1 drift alarm below relies on. + let stop = stop_rule_for_regime(cell_rec.regime); + let (tx_eq, rx_eq) = mpsc::channel(); + let (tx_ex, rx_ex) = mpsc::channel(); + let (tx_r, rx_r) = mpsc::channel(); + let (tx_req, rx_req) = mpsc::channel(); + let mut h = crate::wrap_r(signal, tx_eq, tx_ex, tx_r, tx_req, stop, false, geo.pip_size) + .bootstrap_with_cells(&point) + .expect("the member's point re-bootstraps (it already ran this realization)"); + let sources: Vec> = vec![Box::new(source)]; + h.run(sources); + // Drain ALL FOUR channels (`run_signal_r` leaves req undrained; the + // trace path must not): eq/ex/req feed the taps, r feeds the metrics. + let eq_rows: Vec<(Timestamp, Vec)> = rx_eq.try_iter().collect(); + let ex_rows: Vec<(Timestamp, Vec)> = rx_ex.try_iter().collect(); + let r_rows: Vec<(Timestamp, Vec)> = rx_r.try_iter().collect(); + let req_rows: Vec<(Timestamp, Vec)> = rx_req.try_iter().collect(); - // The C1 drift alarm: metrics equality against the recorded - // nominee. The reduce-mode fold shares its arithmetic with this - // non-reduce reduction (SeriesFold via `summarize`; GatedRecorder - // emits exactly the rows `summarize_r`'s ledger reads), so equality - // is bit-exact. - let mut rerun_metrics = summarize(&f64_field(&eq_rows, 0), &f64_field(&ex_rows, 0)); - rerun_metrics.r = Some(summarize_r(&r_rows, &[])); - if rerun_metrics != nominee_report.metrics { - return Err(format!( - "trace re-run diverged from the recorded nominee (C1 violation): cell \ - {cell_key} of trace {trace_name} does not reproduce its recorded \ - metrics; refusing to persist a silently-wrong trace" - )); - } + // The C1 drift alarm: metrics equality against the recorded + // member. The reduce-mode fold shares its arithmetic with this + // non-reduce reduction (SeriesFold via `summarize`; GatedRecorder + // emits exactly the rows `summarize_r`'s ledger reads), so equality + // is bit-exact. + let mut rerun_metrics = summarize(&f64_field(&eq_rows, 0), &f64_field(&ex_rows, 0)); + rerun_metrics.r = Some(summarize_r(&r_rows, &[])); + if rerun_metrics != member_report.metrics { + return Err(format!( + "trace re-run diverged from the recorded member (C1 violation): cell \ + {cell_key} of trace {trace_name} does not reproduce its recorded \ + metrics; refusing to persist a silently-wrong trace" + )); + } - let traces: Vec = routed - .iter() - .map(|&(tap, ch)| { - let rows: &[(Timestamp, Vec)] = match ch { - TapChannel::Equity => &eq_rows, - TapChannel::Exposure => &ex_rows, - TapChannel::REquity => &req_rows, - }; - ColumnarTrace::from_rows(tap, &[ScalarKind::F64], rows) - }) - .collect(); - store - .write(&format!("{trace_name}/{cell_key}"), &nominee_report.manifest, &traces) - .map_err(|e| e.to_string())?; + let traces: Vec = routed + .iter() + .map(|&(tap, ch)| { + let rows: &[(Timestamp, Vec)] = match ch { + TapChannel::Equity => &eq_rows, + TapChannel::Exposure => &ex_rows, + TapChannel::REquity => &req_rows, + }; + ColumnarTrace::from_rows(tap, &[ScalarKind::F64], rows) + }) + .collect(); + let write_key = match &member_subdir { + Some(sub) => format!("{trace_name}/{cell_key}/{sub}"), + None => format!("{trace_name}/{cell_key}"), + }; + store + .write(&write_key, &member_report.manifest, &traces) + .map_err(|e| e.to_string())?; + } persisted_cells += 1; } @@ -829,12 +896,34 @@ fn persist_campaign_traces( #[cfg(test)] mod tests { use super::*; + use aura_campaign::StageFamily; use aura_core::ScalarKind; + use aura_engine::RunManifest; fn spec(name: &str) -> ParamSpec { ParamSpec { name: name.to_string(), kind: ScalarKind::I64 } } + /// A minimal `RunReport` fixture carrying exactly the manifest params + /// `member_trace_key`/`cell_member_fanout` read; the metrics are an empty + /// `summarize`, irrelevant to either function under test. + fn report_with_params(params: Vec<(String, Scalar)>) -> RunReport { + RunReport { + manifest: RunManifest { + commit: "test".to_string(), + params, + window: (Timestamp(0), Timestamp(0)), + seed: 0, + broker: "test".to_string(), + selection: None, + instrument: None, + topology_hash: None, + project: None, + }, + metrics: summarize(&[], &[]), + } + } + #[test] /// #197 (fieldtest 0107 F9): the executor's preflight refusal for a /// non-rankable selection metric points the author at the verb that @@ -1010,6 +1099,98 @@ mod tests { assert_eq!(campaign_cell_key(&strategy, "GER40", 0, 2), "bb34aa55-GER40-w0-r2"); } + #[test] + /// #224: `member_trace_key` renders the SAME `"name=value, ..."` join + /// `aura reproduce` prints for a member's params (single-sourced label + /// format), then sanitizes it for filesystem use — a raw label's `=` and + /// `, ` separators are hostile to a path component and must map to `_` + /// (the shared `sanitize_component` charset), never survive verbatim. + fn member_trace_key_mirrors_reproduce_label_and_sanitizes_it() { + let report = report_with_params(vec![ + ("fast".to_string(), Scalar::i64(2)), + ("slow".to_string(), Scalar::i64(4)), + ]); + // The raw reproduce-format label is "fast=2, slow=4"; sanitizing maps + // each of `=`, `,`, ` ` individually to `_` (so the ", " separator + // becomes two underscores, not one). + assert_eq!(member_trace_key(&report, 0), "fast_2__slow_4"); + } + + #[test] + /// #224: a member with no tuning params (a closed blueprint, or a + /// monte-carlo seed member) renders an empty reproduce label, so + /// `member_trace_key` falls back to the member's own ordinal into the + /// family — never an empty (invalid) path component. + fn member_trace_key_falls_back_to_the_ordinal_when_params_are_empty() { + let report = report_with_params(Vec::new()); + assert_eq!(member_trace_key(&report, 3), "3"); + } + + #[test] + /// #224: two members with different params must land at distinct keys — + /// the whole point of per-member subdirectories is that the sweep's + /// members never collide into one on-disk slot. + fn member_trace_key_differs_across_members_with_different_params() { + let a = report_with_params(vec![("length".to_string(), Scalar::i64(10))]); + let b = report_with_params(vec![("length".to_string(), Scalar::i64(20))]); + assert_ne!(member_trace_key(&a, 0), member_trace_key(&b, 1)); + } + + /// A no-nominee `CellOutcome` whose terminal family holds two members + /// (a plain selection-free sweep, #224's headline shape). + fn two_member_family_cell() -> CellOutcome { + CellOutcome { + families: vec![StageFamily { + stage: 0, + block: "std::sweep", + family_id: "fam".to_string(), + reports: vec![ + report_with_params(vec![("length".to_string(), Scalar::i64(10))]), + report_with_params(vec![("length".to_string(), Scalar::i64(20))]), + ], + }], + selections: Vec::new(), + nominee: None, + } + } + + #[test] + /// #224: a no-nominee cell with a non-empty terminal family fans out to + /// ONE (subdir, report) pair PER member — never narrowing to a single + /// nominated member, which would silently drop every other member the + /// sweep actually produced. Each pair carries a distinct `Some(key)` + /// subdir (never `None`, which is reserved for the nominee case). + fn cell_member_fanout_yields_one_entry_per_family_member() { + let cell = two_member_family_cell(); + let members = cell_member_fanout(&cell); + assert_eq!(members.len(), 2, "one dir per member, not one nominee"); + let Some(key0) = &members[0].0 else { panic!("member 0 must carry a subdir key") }; + let Some(key1) = &members[1].0 else { panic!("member 1 must carry a subdir key") }; + assert_ne!(key0, key1, "distinct members must land at distinct subdirs"); + } + + #[test] + /// #224: a nominee cell (walkforward/generalize/mc) still writes its one + /// trace directly at `/` — a `None` subdir key, unchanged since + /// 0109 — regardless of how many stage families it also carries. + fn cell_member_fanout_prefers_the_nominee_with_no_subdir() { + let mut cell = two_member_family_cell(); + let nominee_report = report_with_params(vec![("length".to_string(), Scalar::i64(30))]); + cell.nominee = Some((vec![("length".to_string(), Scalar::i64(30))], nominee_report)); + let members = cell_member_fanout(&cell); + assert_eq!(members.len(), 1, "the nominee is the cell's single trace"); + assert!(members[0].0.is_none(), "the nominee writes directly at /, no subdir"); + } + + #[test] + /// #224: a cell with neither a nominee nor a non-empty terminal family + /// (every family is empty, or there are none) fans out to nothing — the + /// caller's cue to skip the cell loudly rather than write an empty dir. + fn cell_member_fanout_is_empty_with_no_nominee_and_no_members() { + let cell = CellOutcome { families: Vec::new(), selections: Vec::new(), nominee: None }; + assert!(cell_member_fanout(&cell).is_empty()); + } + #[test] /// The tap->channel routing mirrors `persist_traces_r` (equity<-eq, /// exposure<-ex, r_equity<-req); `net_r_equity` is unproducible on the diff --git a/crates/aura-cli/src/main.rs b/crates/aura-cli/src/main.rs index 3b64908..0652a8c 100644 --- a/crates/aura-cli/src/main.rs +++ b/crates/aura-cli/src/main.rs @@ -447,7 +447,8 @@ fn emit_chart(name: &str, tap: Option<&str>, mode: ChartMode, env: &project::Env NameKind::NotFound => { eprintln!( "aura: no recorded run or family '{name}' under runs/traces \ - (traces are written by a campaign document's `persist_taps` presentation)" + (run `aura sweep --real --trace {name}` or \ + `aura walkforward --real --trace {name}` first)" ); std::process::exit(1); } @@ -2243,7 +2244,8 @@ struct SweepCmd { /// Family name (records to the registry without persisting per-member traces). #[arg(long)] name: Option, - /// Family name (not yet available on this verb; see #224). + /// Family name that also persists each member's taps (--real mode; mutually + /// exclusive with --name). #[arg(long)] trace: Option, /// Blueprint sweep axis `=` (repeatable; .json mode). @@ -2270,7 +2272,8 @@ struct WalkforwardCmd { /// Family name (records to the registry without persisting per-member traces). #[arg(long)] name: Option, - /// Family name (not yet available on this verb; see #224). + /// Family name that also persists each OOS window's taps (--real mode; + /// mutually exclusive with --name). #[arg(long)] trace: Option, /// Campaign-path stop length (single value; --real mode). @@ -2407,7 +2410,7 @@ fn stop_knob_or( #[allow(clippy::type_complexity)] fn walkforward_args_from( a: &WalkforwardCmd, -) -> Result<(String, String, i64, f64, Option, Option), String> { +) -> Result<(String, bool, String, i64, f64, Option, Option), String> { let symbol = match a.real.as_deref() { None | Some("") => return Err("walkforward dissolves only over --real ".to_string()), Some(s) => s.to_string(), @@ -2415,15 +2418,15 @@ fn walkforward_args_from( let regime = || "walkforward: the stop is a single risk regime; --stop-length and --stop-k take one value each".to_string(); let stop_length = stop_knob_or(a.stop_length.as_deref(), R_SMA_STOP_LENGTH, ®ime)?; let stop_k = stop_knob_or(a.stop_k.as_deref(), R_SMA_STOP_K, ®ime)?; - let name = match (a.name.as_deref(), a.trace.as_deref()) { + let (name, trace) = match (a.name.as_deref(), a.trace.as_deref()) { (Some(_), Some(_)) => { return Err("walkforward: --name and --trace are mutually exclusive".to_string()); } - (Some(n), None) => n.to_string(), - (None, Some(t)) => t.to_string(), - (None, None) => "walkforward".to_string(), + (Some(n), None) => (n.to_string(), false), + (None, Some(t)) => (t.to_string(), true), + (None, None) => ("walkforward".to_string(), false), }; - Ok((name, symbol, stop_length, stop_k, a.from, a.to)) + Ok((name, trace, symbol, stop_length, stop_k, a.from, a.to)) } /// Convert `McCmd` into the resolved argument shape the mc sugar consumes (the @@ -2823,6 +2826,9 @@ fn dispatch_generalize(a: GeneralizeCmd, env: &project::Env) { to_ms, blueprint_canonical: &canonical, stop: Some(verb_sugar::VolStop { length: stop_length, k: stop_k }), + // `generalize` has no `--trace` flag in its own grammar (#224 delivered + // sweep + walkforward only); trace-writing is out of scope here. + trace: false, }; verb_sugar::run_generalize_sugar(&inv, &metric, env).unwrap_or_else(|m| { eprintln!("aura: {m}"); @@ -2878,17 +2884,9 @@ fn dispatch_new(a: NewCmd, _env: &project::Env) { fn dispatch_sweep(a: SweepCmd, env: &project::Env) { // Single-sourced: the blueprint grammar and the no-blueprint usage error must // stay in lockstep, so both arms below read this one closure. - let usage = || "Usage: aura sweep --axis = [--axis …] [--name ] [--real [--from ] [--to ]]".to_string(); + let usage = || "Usage: aura sweep --axis = [--axis …] [--name | --trace ] [--real [--from ] [--to ]]".to_string(); match is_blueprint_file(&a.blueprint) { Some(path) => { - // No blueprint-mode arm persists per-member taps (synthetic drops it via - // `let _ = persist`, `--real` sugar sets `persist_taps: vec![]`) — refuse - // rather than silently accept an advertised-but-unhonoured flag, mirroring - // the `aura run`/`aura mc` --trace refusals. - if a.trace.is_some() { - eprintln!("aura: --trace is not yet available on sweep; see #224"); - std::process::exit(2); - } let doc = std::fs::read_to_string(path).unwrap_or_else(|e| { eprintln!("aura: {path}: {e}"); std::process::exit(2); @@ -2908,6 +2906,7 @@ fn dispatch_sweep(a: SweepCmd, env: &project::Env) { // A query, not a sweep: it must stand alone. if !a.axis.is_empty() || a.name.is_some() + || a.trace.is_some() || a.real.is_some() || a.from.is_some() || a.to.is_some() @@ -2965,6 +2964,10 @@ fn dispatch_sweep(a: SweepCmd, env: &project::Env) { to_ms, blueprint_canonical: &canonical, stop: None, + // #224: the real-data campaign path DELIVERS `--trace` + // (per-member tap-series persistence); `persist` is + // `name_persist`'s bool (true iff `--trace` was given). + trace: persist, }; verb_sugar::run_sweep_sugar(&inv, env).unwrap_or_else(|m| { eprintln!("aura: {m}"); @@ -2972,6 +2975,16 @@ fn dispatch_sweep(a: SweepCmd, env: &project::Env) { }); } DataChoice::Synthetic => { + // The synthetic in-process family path is still reduce-only + // (`blueprint_sweep_family` writes no per-member traces, #224 + // delivered only the real-data campaign path) — refuse rather + // than silently accept an advertised-but-unhonoured flag. + if persist { + eprintln!( + "aura: --trace is not yet available on a synthetic sweep (no --real); see #224" + ); + std::process::exit(2); + } run_blueprint_sweep( &doc, &axes, &name, persist, DataSource::from_choice(data, env), env, @@ -2991,16 +3004,9 @@ fn dispatch_sweep(a: SweepCmd, env: &project::Env) { /// #220: arbitrary user blueprint + axes, formerly the welded r-sma branch). fn dispatch_walkforward(a: WalkforwardCmd, env: &project::Env) { // Single-sourced: both arms below read this one closure (house style, #179). - let usage = || format!("Usage: aura walkforward --axis = [--axis …] [--select ] [--name ] | aura walkforward --real --axis = [--axis …] [--stop-length (default {R_SMA_STOP_LENGTH})] [--stop-k (default {R_SMA_STOP_K:.1})] [--from ] [--to ] [--name ]"); + let usage = || format!("Usage: aura walkforward --axis = [--axis …] [--select ] [--name ] | aura walkforward --real --axis = [--axis …] [--stop-length (default {R_SMA_STOP_LENGTH})] [--stop-k (default {R_SMA_STOP_K:.1})] [--from ] [--to ] [--name | --trace ]"); match is_blueprint_file(&a.blueprint) { Some(path) => { - // No blueprint-mode arm persists per-window taps — refuse rather than - // silently accept an advertised-but-unhonoured flag, mirroring the - // `aura run`/`aura mc` --trace refusals. - if a.trace.is_some() { - eprintln!("aura: --trace is not yet available on walkforward; see #224"); - std::process::exit(2); - } let doc = std::fs::read_to_string(path).unwrap_or_else(|e| { eprintln!("aura: {path}: {e}"); std::process::exit(2); @@ -3031,7 +3037,7 @@ fn dispatch_walkforward(a: WalkforwardCmd, env: &project::Env) { }, None => aura_research::SelectRule::Argmax, }; - let (name, symbol, stop_length, stop_k, from, to) = + let (name, trace, symbol, stop_length, stop_k, from, to) = walkforward_args_from(&a).unwrap_or_else(|m| { eprintln!("aura: {m}"); std::process::exit(2); @@ -3062,6 +3068,10 @@ fn dispatch_walkforward(a: WalkforwardCmd, env: &project::Env) { to_ms, blueprint_canonical: &canonical, stop: Some(verb_sugar::VolStop { length: stop_length, k: stop_k }), + // #224: the walkforward sibling delivers `--trace` identically + // to sweep, riding the SAME per-cell nominee trace mechanism + // (unchanged since 0109 — walkforward always nominates). + trace, }; verb_sugar::run_walkforward_sugar( &inv, @@ -3080,7 +3090,16 @@ fn dispatch_walkforward(a: WalkforwardCmd, env: &project::Env) { }); return; } - // Synthetic in-process family path — unchanged (#220 non-goal). + // Synthetic in-process family path — unchanged (#220 non-goal); it + // persists no per-window taps, so `--trace` stays refused here (#224 + // delivered only the `--real` campaign path above), mirroring + // `dispatch_sweep`'s synthetic-arm refusal with its own named pointer. + if a.trace.is_some() { + eprintln!( + "aura: --trace is not yet available on a synthetic walkforward (no --real); see #224" + ); + std::process::exit(2); + } if a.stop_length.is_some() || a.stop_k.is_some() { eprintln!("aura: {}", usage()); std::process::exit(2); @@ -3164,6 +3183,10 @@ fn dispatch_mc(a: McCmd, env: &project::Env) { to_ms, blueprint_canonical: &canonical, stop: Some(verb_sugar::VolStop { length: stop_length, k: stop_k }), + // mc --real refuses --name/--trace up front (`mc_args_from`) — + // the R-bootstrap records without a family name; trace-writing + // is out of #224's scope here. + trace: false, }; verb_sugar::run_mc_sugar( &inv, diff --git a/crates/aura-cli/src/verb_sugar.rs b/crates/aura-cli/src/verb_sugar.rs index 09e794d..e0b8ae5 100644 --- a/crates/aura-cli/src/verb_sugar.rs +++ b/crates/aura-cli/src/verb_sugar.rs @@ -11,9 +11,9 @@ use std::collections::BTreeMap; use aura_core::{Scalar, ScalarKind}; use aura_research::{ - campaign_to_json, content_id_of, process_to_json, Axis, CampaignDoc, DataSection, DocKind, - DocRef, Presentation, ProcessDoc, ProcessRef, RiskRegime, SelectRule, StageBlock, - StrategyEntry, SweepSelection, WfMode, Window, FORMAT_VERSION, + campaign_to_json, content_id_of, process_to_json, tap_vocabulary, Axis, CampaignDoc, + DataSection, DocKind, DocRef, Presentation, ProcessDoc, ProcessRef, RiskRegime, SelectRule, + StageBlock, StrategyEntry, SweepSelection, WfMode, Window, FORMAT_VERSION, }; /// The two generated documents of one dissolved-verb invocation. @@ -38,6 +38,23 @@ pub(crate) struct SugarInvocation<'a> { pub to_ms: i64, pub blueprint_canonical: &'a str, pub stop: Option, + /// Whether the invocation carried `--trace` (sweep/walkforward, #224): + /// when true, the generated campaign's presentation requests the full + /// tap vocabulary so `persist_campaign_traces` actually writes each + /// member's tap series, instead of the #168 no-op `persist_taps: vec![]`. + pub trace: bool, +} + +/// The generated campaign's `persist_taps` section for one invocation: +/// the full closed tap vocabulary when `--trace` was requested, else empty +/// (the #168 no-op — this is the one seam `translate_sweep`/ +/// `translate_walkforward` share for the decision). +fn persist_taps_from(trace: bool) -> Vec { + if trace { + tap_vocabulary().iter().map(|t| t.to_string()).collect() + } else { + vec![] + } } /// The single protective-stop regime a dissolved verb binds (`RiskRegime::Vol`). @@ -147,7 +164,7 @@ pub(crate) fn translate_sweep(inv: &SugarInvocation) -> Result `trace_name` is `None`), so this is always safe to call. Unlike + // `run_sweep_sugar` (routed through `run_campaign_by_id` -> the shared + // `present_campaign` tail), this walkforward sugar calls + // `run_campaign_returning` directly and prints its own bespoke lines, so + // it must run this tail itself. Its generalize/mc siblings call + // `run_campaign_returning` the same way but do NOT run this tail: neither + // has a `--trace` grammar (generalize never did; `mc --real` refuses + // `--name`/`--trace` up front, #224) and `campaign.presentation.persist_taps` + // is always empty for them, so their `run.outcome.record.trace_name` is + // always `None`. `run`/`mc --trace` stay refused by design (#224), not by + // this tail being skipped. + if let Some(trace_name) = &run.outcome.record.trace_name { + crate::campaign_run::persist_campaign_traces( + trace_name, + &run.campaign.presentation.persist_taps, + &run.outcome, + &run.campaign, + &run.strategies, + &run.server, + env, + )?; + } Ok(()) } @@ -627,6 +669,7 @@ mod tests { to_ms: 200, blueprint_canonical: bp, stop, + trace: false, } } @@ -683,6 +726,22 @@ mod tests { assert!(err.contains("mixed value kinds"), "{err}"); } + /// Property (#224): `SugarInvocation.trace = true` requests the FULL closed + /// tap vocabulary on the generated campaign's `presentation.persist_taps` — + /// the honest inverse of the #168 no-op, where `persist_taps` stayed `vec![]` + /// regardless of `--trace`. `trace = false` (the default, pinned by the + /// sibling `translate_sweep_is_deterministic_and_names_flow`) keeps the + /// vocabulary empty. + #[test] + fn translate_sweep_trace_requests_the_full_tap_vocabulary() { + let ax = axes(); + let mut i = inv(&ax, "probe", &["GER40"], None, "{\"bp\":1}"); + i.trace = true; + let g = translate_sweep(&i).unwrap(); + let want: Vec = aura_research::tap_vocabulary().iter().map(|t| t.to_string()).collect(); + assert_eq!(g.campaign.presentation.persist_taps, want); + } + #[test] fn generated_campaign_validates_intrinsically_and_preflights() { let ax = axes(); @@ -865,6 +924,19 @@ mod tests { assert!(aura_campaign::preflight(&a.process, &a.campaign).is_ok()); } + /// Property (#224): mirrors `translate_sweep_trace_requests_the_full_tap_vocabulary` + /// for walkforward's translator — the walkforward sibling refusal lifts + /// identically, riding the same `persist_taps_from` seam. + #[test] + fn translate_walkforward_trace_requests_the_full_tap_vocabulary() { + let ax = wf_axes(); + let mut i = inv(&ax, "walkforward", &["GER40"], Some(VolStop { length: 14, k: 2.0 }), "{\"bp\":1}"); + i.trace = true; + let g = translate_walkforward(&i, "sqn_normalized", WF_W, SelectRule::Argmax).unwrap(); + let want: Vec = aura_research::tap_vocabulary().iter().map(|t| t.to_string()).collect(); + assert_eq!(g.campaign.presentation.persist_taps, want); + } + #[test] fn translate_walkforward_diverging_regimes_do_not_collide_content_ids() { let ax = wf_axes(); diff --git a/crates/aura-cli/tests/cli_run.rs b/crates/aura-cli/tests/cli_run.rs index 1b36a2e..3105881 100644 --- a/crates/aura-cli/tests/cli_run.rs +++ b/crates/aura-cli/tests/cli_run.rs @@ -1277,6 +1277,7 @@ fn sweep_real_with_trace_writes_tap_series_to_disk() { "--from", GER40_SEPT2024_FROM_MS, "--to", GER40_SEPT2024_TO_MS, "--axis", "sma_signal.fast.length=2,4", + "--axis", "sma_signal.slow.length=8", "--trace", "brp", ]) .current_dir(dir) @@ -2321,10 +2322,11 @@ fn walkforward_dissolved_refuses_an_unknown_select_token() { assert!(stderr.contains("Usage: aura walkforward"), "unknown-select refusal: {stderr}"); } -/// `--trace` is now refused up front on every blueprint-mode arm (#168, before the -/// `--name`/`--trace` mutual-exclusion check even runs) — the dissolved r-sma-real -/// path must surface that #224 forward-pointer refusal (exit 2), not the generic -/// mutual-exclusion message which its early return now pre-empts. +/// `--name`/`--trace` are mutually exclusive on the walkforward real-data path +/// (#224 delivers `--trace` there — the up-front #168 forward-pointer refusal +/// is gone; `walkforward_args_from`'s own mutual-exclusion check is what fires +/// when both are given at once), exit 2, naming the mutual-exclusion contract +/// rather than a now-stale #224 forward pointer. #[test] fn walkforward_dissolved_refuses_name_and_trace_together() { let cwd = temp_cwd("walkforward-name-and-trace"); @@ -2339,10 +2341,12 @@ fn walkforward_dissolved_refuses_name_and_trace_together() { .current_dir(&cwd) .output() .expect("spawn aura"); - assert_eq!(out.status.code(), Some(2), "--trace is refused up front (exit 2)"); + assert_eq!(out.status.code(), Some(2), "--name/--trace together is refused (exit 2)"); let stderr = String::from_utf8_lossy(&out.stderr); - assert!(stderr.contains("--trace"), "refusal names the flag: {stderr}"); - assert!(stderr.contains("#224"), "refusal points forward to #224: {stderr}"); + assert!( + stderr.contains("mutually exclusive"), + "refusal names the mutual-exclusion contract: {stderr}" + ); } /// Property (#210 T3, dispatch split): a successful real-data walkforward