fix(aura-cli, ledger): review fixes — override stamps the base hash (#343 revised), prose corrections
Harvest sweep, whole-diff review fix block. The review refuted the #343 ratification's factual premise: the campaign leg does NOT mint a reopened member's hash — runner.rs passes the stored strategy_id as topo even for reopened members (reference semantics). Decision revised on the issue (fix, not ratify): exec's override run now stamps the LOADED base document's content id (computed before reopen_all; RED-first — the inverted pin failed on the old reopened-hash behaviour), keeping topology_hash store-resolvable and identical in meaning across both legs; the variation lives in manifest.params. C24/C12/C18 rewritten to the corrected reading. Also per review: C24's retired build-free-introspection sentence updated (old wording archived in the history sidecar, #339 item 4); C14's partition key precisely scoped to the routing seam, naming the legs' differing content-validation exits honestly; measure.rs's false unreachability comment corrected; --taps added to the guide's introspect cheat-sheet and pinned on the content-id path; the zero-trade-cell note routed through the diag macro; doc-comment, issue-ref, and gate-name corrections; the Fixed-regime NaN branch pinned. refs #339 refs #342 refs #343
This commit is contained in:
@@ -48,9 +48,8 @@ pub(crate) fn note_zero_trade_windows(mut window_trades: impl ExactSizeIterator<
|
||||
/// — a campaign cell whose families are ALL non-walk-forward (grid sweep, MC
|
||||
/// bootstrap, …) has no per-window vocabulary to note through, yet a
|
||||
/// consumer still cannot tell "0.0 = break-even" from "0.0 = never traded"
|
||||
/// without this. Takes the per-report trade counts across every family the
|
||||
/// cell ran; a no-op unless there is at least one report and every one of
|
||||
/// them traded zero times.
|
||||
/// without this. Pure over the count so the wording is unit-pinned (mirrors
|
||||
/// `zero_trade_note_text`'s singular/plural split, #313).
|
||||
fn zero_trade_cell_note_text(n_reports: usize) -> String {
|
||||
if n_reports == 1 {
|
||||
"the cell's one report recorded zero trades; its metrics are vacuous, not a break-even \
|
||||
@@ -64,8 +63,10 @@ fn zero_trade_cell_note_text(n_reports: usize) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// See `zero_trade_cell_note_text`. Called from the campaign cell loop's
|
||||
/// non-walk-forward branch (`campaign_run.rs`), independent of `emit`.
|
||||
/// See `zero_trade_cell_note_text`. Takes the per-report trade counts across
|
||||
/// every family the cell ran; a no-op unless there is at least one report
|
||||
/// and every one of them traded zero times. Called from the campaign cell
|
||||
/// loop's non-walk-forward branch (`campaign_run.rs`), independent of `emit`.
|
||||
pub(crate) fn note_zero_trade_cell(mut report_trades: impl ExactSizeIterator<Item = u64>) {
|
||||
let n_reports = report_trades.len();
|
||||
if n_reports > 0 && report_trades.all(|n| n == 0) {
|
||||
|
||||
@@ -1070,7 +1070,7 @@ fn taps_lines(target: &str, env: &aura_runner::project::Env) -> Result<String, S
|
||||
let composite = if is_file { composite_from_authored_text(&text, env)? } else { composite_from_any(&text, env)? };
|
||||
let taps = composite.declared_taps();
|
||||
if taps.is_empty() {
|
||||
eprintln!("aura: note: {target} declares no taps");
|
||||
crate::diag::note!("{target} declares no taps");
|
||||
return Ok(String::new());
|
||||
}
|
||||
let mut out = String::new();
|
||||
|
||||
@@ -19,16 +19,17 @@ use aura_core::Cell;
|
||||
#[cfg(test)]
|
||||
use aura_composites::StopRule;
|
||||
use aura_engine::{
|
||||
blueprint_from_json, f64_field, join_on_ts,
|
||||
blueprint_from_json, blueprint_to_json, f64_field, join_on_ts,
|
||||
JoinedRow,
|
||||
SelectionMode,
|
||||
};
|
||||
// `blueprint_to_json`/`Composite` lost their last production caller with
|
||||
// `topology_hash` (#319 Task 9 — the CLI-side wrapper retired, its survival
|
||||
// rationale disproven); reached only from the test module now, mirroring
|
||||
// `Bias` below.
|
||||
// `Composite` lost its last production caller with `topology_hash` (#319
|
||||
// Task 9 — the CLI-side wrapper retired, its survival rationale disproven);
|
||||
// reached only from the test module now, mirroring `Bias` below.
|
||||
// (`blueprint_to_json` regained a production caller with the #343 revised
|
||||
// override-hash fix in `exec_blueprint_leg` above.)
|
||||
#[cfg(test)]
|
||||
use aura_engine::{blueprint_to_json, Composite};
|
||||
use aura_engine::Composite;
|
||||
#[cfg(test)]
|
||||
use aura_engine::{window_of, ColumnarTrace, Harness, VecSource};
|
||||
use aura_registry::{group_families, rank_by, FamilyMember, NameKind, RunTraces};
|
||||
@@ -1291,6 +1292,15 @@ fn exec_blueprint_leg(path: &str, overrides: &[String], tap: &[String], env: &au
|
||||
*ov_by_path.get(raw).expect("override set derived from these exact overrides")
|
||||
})
|
||||
.collect();
|
||||
// #343 (revised decision): `topology_hash` carries REFERENCE
|
||||
// semantics on every leg — it names the loaded base document, never
|
||||
// a transient reopened variant, mirroring the campaign leg's own
|
||||
// precedent (`runner.rs::MemberRunner::run_member` passes
|
||||
// `&cell.strategy_id`, the stored base document's id, as `topo` even
|
||||
// for a reopened member). Computed here, over the still-unreopened
|
||||
// `signal`, before `reopen_all` consumes it.
|
||||
let base_topo =
|
||||
content_id(&blueprint_to_json(&signal).expect("a buildable signal serializes"));
|
||||
let signal = reopen_all(signal, &paths);
|
||||
// An out-of-domain override value fails a node's own domain check at
|
||||
// bootstrap (e.g. `Sma::new`'s `assert!`, or a negative length
|
||||
@@ -1302,13 +1312,18 @@ fn exec_blueprint_leg(path: &str, overrides: &[String], tap: &[String], env: &au
|
||||
// `SilencedPanic` + `catch_unwind`) and render it as a runtime-class
|
||||
// refusal (exit 1, C14 partition) instead of an uncaught exit 101 —
|
||||
// the input was well-formed argv; the value is what the node refuses.
|
||||
let report = aura_campaign::catch_member_panic(|| {
|
||||
let mut report = aura_campaign::catch_member_panic(|| {
|
||||
run_signal_r(signal, ¶ms, RunData::Synthetic, 0, env, tap_plan)
|
||||
})
|
||||
.unwrap_or_else(|msg| {
|
||||
eprintln!("aura: {}", panic_refusal_prose(&msg));
|
||||
std::process::exit(1);
|
||||
});
|
||||
// Overwrite `run_signal_r`'s own (reopened-topology) hash with the
|
||||
// base document's id: reference semantics (#343). The manifest's
|
||||
// `params` already carries the override as the variation; the base
|
||||
// document + params deterministically reconstruct this run.
|
||||
report.manifest.topology_hash = Some(base_topo);
|
||||
// #324 (comment 4501): this leg always runs `RunData::Synthetic` (no
|
||||
// direct-blueprint exec ever binds real data), so a validating
|
||||
// caller reading zero trades here as "broken strategy" needs telling
|
||||
|
||||
@@ -553,19 +553,24 @@ fn exec_blueprint_without_override_runs_bound_values_untouched() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Ratification pin (#343): an override IS a topology-parameter variation,
|
||||
/// even a "no-op" one that re-binds a param to its EXISTING bound value —
|
||||
/// `examples/r_sma.json` already binds `fast.length` to 2, so
|
||||
/// `--override fast.length=2` changes nothing observable about the run's
|
||||
/// *behaviour*, yet the reopened param moves from `manifest.defaults` to
|
||||
/// `manifest.params` (the #246 reopen mechanism, unconditionally applied),
|
||||
/// which changes the canonical bytes `topology_hash` covers: the two runs'
|
||||
/// `topology_hash`es must therefore DIFFER, and the overridden run's own
|
||||
/// hash must be stable across repeated runs (determinism, C1) — the
|
||||
/// reopened topology is its own honest, reproducible identity, not a
|
||||
/// dangling reference (see the ratification prose in C12/C24).
|
||||
/// Ratification pin (#343, revised): `topology_hash` carries REFERENCE
|
||||
/// semantics on every leg — it names the (stored/loaded) base document,
|
||||
/// never a transient reopened variant, mirroring the campaign leg's own
|
||||
/// precedent (`runner.rs` passes `&cell.strategy_id`, the stored base
|
||||
/// document's id, as `topo` even for reopened members). `examples/r_sma.json`
|
||||
/// already binds `fast.length` to 2, so `--override fast.length=2` is a
|
||||
/// no-op override value-wise; it still moves the param from
|
||||
/// `manifest.defaults` to `manifest.params` (the #246 reopen mechanism,
|
||||
/// unconditionally applied) — the manifest's `params` carry the variation,
|
||||
/// while `topology_hash` stamps the base document's own content id (computed
|
||||
/// before the reopen) on both legs, so the bare run's and the
|
||||
/// no-op-override run's `topology_hash` are EQUAL, and the overridden run's
|
||||
/// own hash stays stable across repeated runs (determinism, C1). The
|
||||
/// manifest's `params` plus the base document deterministically reconstruct
|
||||
/// the executed run (reproduction identity) — see the corrected ratification
|
||||
/// prose in C12/C24.
|
||||
#[test]
|
||||
fn exec_blueprint_noop_override_mints_a_distinct_but_stable_topology_hash() {
|
||||
fn exec_blueprint_override_keeps_the_base_documents_topology_hash() {
|
||||
let (base_out, base_code) = run_code(&["exec", "examples/r_sma.json"]);
|
||||
assert_eq!(base_code, Some(0), "stdout/stderr: {base_out}");
|
||||
let base_line = base_out.lines().find(|l| l.starts_with('{')).expect("record line");
|
||||
@@ -581,10 +586,16 @@ fn exec_blueprint_noop_override_mints_a_distinct_but_stable_topology_hash() {
|
||||
let ov_hash =
|
||||
ov_v["manifest"]["topology_hash"].as_str().expect("overridden topology_hash").to_string();
|
||||
|
||||
assert_ne!(
|
||||
assert_eq!(
|
||||
base_hash, ov_hash,
|
||||
"a no-op override still reopens the bound param, minting the reopened topology's own \
|
||||
hash: base {base_out} / overridden {ov_out}"
|
||||
"an override stamps the base document's own content id — reference semantics (#343): \
|
||||
base {base_out} / overridden {ov_out}"
|
||||
);
|
||||
|
||||
let ov_params = ov_v["manifest"]["params"].as_array().expect("params array");
|
||||
assert!(
|
||||
ov_params.iter().any(|e| e[0] == "fast.length"),
|
||||
"the variation lives in the manifest's params, not in the hash: {ov_out}"
|
||||
);
|
||||
|
||||
let (ov_out_2, ov_code_2) =
|
||||
|
||||
@@ -876,6 +876,45 @@ fn graph_introspect_taps_lists_declared_taps_wire_and_kind() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Property (#337 follow-up, harvest audit item 8): `--taps <ID>` resolves
|
||||
/// through the project store exactly like `--params <ID>` already does
|
||||
/// (`graph_params_by_content_id_matches_params_by_file`'s sibling) — register
|
||||
/// a tap-bearing blueprint, then introspect its declared taps by content id;
|
||||
/// the store path carries no `FILE`'s root-name gate (C29).
|
||||
#[test]
|
||||
fn graph_introspect_taps_by_content_id_matches_by_file() {
|
||||
let dir = temp_cwd("taps-by-id");
|
||||
let ops = r#"[
|
||||
{"op":"doc","text":"fast/slow spread, tapped, exposed as bias"},
|
||||
{"op":"source","role":"price","kind":"F64"},
|
||||
{"op":"add","type":"SMA","name":"fast","bind":{"length":{"I64":2}}},
|
||||
{"op":"add","type":"SMA","name":"slow","bind":{"length":{"I64":4}}},
|
||||
{"op":"add","type":"Sub"},
|
||||
{"op":"feed","role":"price","into":["fast.series","slow.series"]},
|
||||
{"op":"connect","from":"fast.value","to":"sub.lhs"},
|
||||
{"op":"connect","from":"slow.value","to":"sub.rhs"},
|
||||
{"op":"tap","from":"fast.value","as":"fast_ma"},
|
||||
{"op":"tap","from":"sub.value","as":"spread"},
|
||||
{"op":"expose","from":"sub.value","as":"bias"}
|
||||
]"#;
|
||||
let (build_out, build_err, build_code) = run_in_stdin(&dir, &["graph", "build"], ops);
|
||||
assert_eq!(build_code, Some(0), "graph build: {build_out} {build_err}");
|
||||
let bp = dir.join("tapped.json");
|
||||
std::fs::write(&bp, &build_out).expect("write built blueprint");
|
||||
|
||||
let (reg_out, reg_err, reg_code) = run_in(&dir, &["graph", "register", bp.to_str().unwrap()]);
|
||||
assert_eq!(reg_code, Some(0), "register: {reg_out} {reg_err}");
|
||||
let id = registered_id(®_out);
|
||||
|
||||
let (by_id_out, by_id_err, by_id_code) =
|
||||
run_in(&dir, &["graph", "introspect", "--taps", &id]);
|
||||
assert_eq!(by_id_code, Some(0), "stdout: {by_id_out} stderr: {by_id_err}");
|
||||
assert_eq!(
|
||||
by_id_out, "fast_ma fast.value F64\nspread sub.value F64\n",
|
||||
"same declared-tap rows resolved through the store by content id"
|
||||
);
|
||||
}
|
||||
|
||||
/// Property (#337): a blueprint with no declared taps prints nothing to
|
||||
/// stdout (nothing was declared to list) plus a one-line stderr note — a
|
||||
/// listing, not a fault, exit 0.
|
||||
@@ -2397,7 +2436,7 @@ fn graph_build_accepts_an_open_input_pattern() {
|
||||
/// this cycle), so a bare-tap (no-`bias`) pattern is used here: its
|
||||
/// `run_measurement` path compiles the signal directly, hitting
|
||||
/// `CompileError::UnboundRootRole` at bootstrap — rendered by role NAME
|
||||
/// (#342 item 3), not the raw flat index the earlier form left as a
|
||||
/// (#339 item 3), not the raw flat index the earlier form left as a
|
||||
/// follow-up.
|
||||
#[test]
|
||||
fn running_an_open_blueprint_refuses_at_bootstrap() {
|
||||
|
||||
Reference in New Issue
Block a user