From ef24f06547ee0f07ec010ecc7d01602f400c4066 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 13:37:01 +0200 Subject: [PATCH] =?UTF-8?q?fix(aura-cli,=20ledger):=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20override=20stamps=20the=20base=20hash=20(#343=20rev?= =?UTF-8?q?ised),=20prose=20corrections?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/aura-cli/src/diag.rs | 11 +++-- crates/aura-cli/src/graph_construct.rs | 2 +- crates/aura-cli/src/main.rs | 29 +++++++++--- crates/aura-cli/tests/exec.rs | 41 ++++++++++------ crates/aura-cli/tests/graph_construct.rs | 41 +++++++++++++++- crates/aura-engine/src/blueprint.rs | 2 +- crates/aura-research/src/lib.rs | 9 ++-- crates/aura-runner/src/measure.rs | 10 ++-- docs/authoring-guide.md | 3 ++ docs/design/contracts/c12-atomic-sim-unit.md | 7 ++- .../contracts/c14-headless-two-faces.md | 47 +++++++++++++------ docs/design/contracts/c18-registry.md | 11 ++--- .../contracts/c24-blueprint-data.history.md | 8 ++++ docs/design/contracts/c24-blueprint-data.md | 42 +++++++++-------- 14 files changed, 183 insertions(+), 80 deletions(-) diff --git a/crates/aura-cli/src/diag.rs b/crates/aura-cli/src/diag.rs index e6c63d4..5b1f804 100644 --- a/crates/aura-cli/src/diag.rs +++ b/crates/aura-cli/src/diag.rs @@ -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) { let n_reports = report_trades.len(); if n_reports > 0 && report_trades.all(|n| n == 0) { diff --git a/crates/aura-cli/src/graph_construct.rs b/crates/aura-cli/src/graph_construct.rs index 9df2de5..abe0a54 100644 --- a/crates/aura-cli/src/graph_construct.rs +++ b/crates/aura-cli/src/graph_construct.rs @@ -1070,7 +1070,7 @@ fn taps_lines(target: &str, env: &aura_runner::project::Env) -> Result` resolves +/// through the project store exactly like `--params ` 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() { diff --git a/crates/aura-engine/src/blueprint.rs b/crates/aura-engine/src/blueprint.rs index db39d2d..fc6aa0e 100644 --- a/crates/aura-engine/src/blueprint.rs +++ b/crates/aura-engine/src/blueprint.rs @@ -372,7 +372,7 @@ impl Composite { /// so this view renders exactly the load-bearing name `compile` keeps. /// Bounds-total like `derive_signature`: a structurally-invalid wire (out /// of a producer's output arity) yields no row rather than a panic — the - /// real gate is `compile`'s own `validate_wiring`. + /// real gate is `compile`'s own `resolve_tap_wire`. pub fn declared_taps(&self) -> Vec<(String, String, ScalarKind)> { let mut out = Vec::new(); collect_taps(self, &mut out); diff --git a/crates/aura-research/src/lib.rs b/crates/aura-research/src/lib.rs index 408795b..5309f3a 100644 --- a/crates/aura-research/src/lib.rs +++ b/crates/aura-research/src/lib.rs @@ -1745,7 +1745,8 @@ mod tests { assert!(!faults.contains(&DocFault::BadRegime { index: 0 }), "valid regime not flagged: {faults:?}"); } - /// #338: a non-positive fixed-stop distance must be refused as a graceful + /// #338 (harvest audit item 11 extends the NaN branch): a non-positive OR + /// NaN fixed-stop distance must be refused as a graceful /// `DocFault::BadRegime` at the doc tier — otherwise it reaches /// `FixedStop::new`'s `distance > 0.0` assert and panics instead of a /// validation error (the `validate_campaign_flags_non_positive_period_minutes_regime` @@ -1754,11 +1755,13 @@ mod tests { fn validate_campaign_flags_non_positive_fixed_distance_regime() { let mut doc: CampaignDoc = serde_json::from_str(CAMPAIGN_FIXTURE).unwrap(); doc.risk = vec![ - RiskRegime::Fixed { distance: 10.0 }, // 0: valid - RiskRegime::Fixed { distance: 0.0 }, // 1: distance <= 0 + RiskRegime::Fixed { distance: 10.0 }, // 0: valid + RiskRegime::Fixed { distance: 0.0 }, // 1: distance <= 0 + RiskRegime::Fixed { distance: f64::NAN }, // 2: NaN (unreachable via JSON; defensive) ]; let faults = validate_campaign(&doc); assert!(faults.contains(&DocFault::BadRegime { index: 1 }), "{faults:?}"); + assert!(faults.contains(&DocFault::BadRegime { index: 2 }), "{faults:?}"); assert!(!faults.contains(&DocFault::BadRegime { index: 0 }), "valid regime not flagged: {faults:?}"); } diff --git a/crates/aura-runner/src/measure.rs b/crates/aura-runner/src/measure.rs index db9e00f..f1ce7e8 100644 --- a/crates/aura-runner/src/measure.rs +++ b/crates/aura-runner/src/measure.rs @@ -57,9 +57,13 @@ pub fn measurement_manifest( /// `signal.input_roles()`'s own names, read before `compile_with_params` /// consumes `signal` (mirrors `member::compile_error_prose`'s pre-consumption /// `names` capture for its `ParamKindMismatch` prose). Every other -/// `CompileError` variant keeps the existing Debug fallback — unreachable on -/// this direct-compile path in practice (arity/kind/wiring faults are caught -/// earlier at `finish()`/`introspect`), but kept total rather than partial. +/// `CompileError` variant keeps the existing Debug fallback deliberately — +/// they ARE reachable on this direct-compile path (a hand-authored +/// measurement envelope with an out-of-range declared-tap wire reaches +/// `TapWireOutOfRange` here exactly as `run_signal_r`'s own compile call +/// does, see `run_refuses_unrunnable_blueprint.rs`), so the fallback stays +/// total rather than partial; only `UnboundRootRole` gets dedicated prose +/// above. fn compile_error_prose(e: &CompileError, role_names: &[String]) -> String { let CompileError::UnboundRootRole { role } = e else { return format!("this blueprint does not compile to a runnable harness: {e:?}"); diff --git a/docs/authoring-guide.md b/docs/authoring-guide.md index d4916de..944db19 100644 --- a/docs/authoring-guide.md +++ b/docs/authoring-guide.md @@ -497,6 +497,9 @@ SMA param length:I64 (bind {"I64": }) $ aura graph introspect --unwired < partial.json # open slots of a partial op-script sub.rhs:F64 +$ aura graph introspect --taps tapped.json # one row per declared tap: name, source wire, kind +fast_ma fast.value F64 +spread sub.value F64 $ aura graph introspect --content-id smacross.json # SHA-256 of the canonical form 597d719b7ac607158cda3e68cd497387620397a5e93087e23da512876dafba9a $ aura graph introspect --content-id smacross.json --identity-id # + debug-name-blind identity id, combinable diff --git a/docs/design/contracts/c12-atomic-sim-unit.md b/docs/design/contracts/c12-atomic-sim-unit.md index 76a6eca..14bc030 100644 --- a/docs/design/contracts/c12-atomic-sim-unit.md +++ b/docs/design/contracts/c12-atomic-sim-unit.md @@ -11,10 +11,9 @@ reload, and the axis binds it per cell; `exec`'s blueprint leg still requires every param resolved (a truly open param refuses), and its own `--override NODE.PARAM=VALUE` (#246 residue, #319) reopens exactly the named bound param for that one execution — the single-member analogue of a campaign axis. The -reopened execution's `topology_hash` is therefore the reopened topology's own, -not the pre-override document's — ratified as the deliberate reading, not a -defect (#343; full ratification in [C24](c24-blueprint-data.md)'s -"Reproduction identity"). Identity is untouched: `content_id_of` (`crates/aura-research/src/lib.rs`) is +reopened execution's `topology_hash` stays the base document's own — reference +semantics, not the reopened topology's (#343, revised; full clause in +[C24](c24-blueprint-data.md)'s "Reproduction identity"). Identity is untouched: `content_id_of` (`crates/aura-research/src/lib.rs`) is the one hashing primitive, read directly off the authored document by `aura_runner::member`'s `run_signal_r`/`run_blueprint_member` — where the manifest's `topology_hash` is computed inline (#319 Task 9 retired the diff --git a/docs/design/contracts/c14-headless-two-faces.md b/docs/design/contracts/c14-headless-two-faces.md index 7c2a042..bbfc445 100644 --- a/docs/design/contracts/c14-headless-two-faces.md +++ b/docs/design/contracts/c14-headless-two-faces.md @@ -75,27 +75,44 @@ The machine-first help surface (JSON/manifest help, stdin op-scripts) is deferred to the #157 / [C21](c21-world.md) track, distinct from this human/GNU-convention compliance. -**`exec`'s unresolved-target exit partition is pinned (#342 item 5).** A -target fault splits along the same usage/runtime line the exit-code partition -above already draws, keyed on whether a *real, existing document's content* -is being judged (usage, exit 2 — the content of an argv-named file is itself +**`exec`'s target-classification exit partition is pinned, precisely scoped +(#342 item 5).** The partition below governs exec's TARGET-CLASSIFICATION +refusals at the routing seam — the shape-peek (`classify_exec_document_file`) +that decides which leg, if any, a target reaches, strictly before either +leg's own document-shape validation ever runs. A target fault at this seam +splits along the same usage/runtime line the exit-code partition above +already draws, keyed on whether a *real, existing document's content* is +being judged (usage, exit 2 — the content of an argv-named file is itself part of the argument) or the target simply fails to resolve to any recorded state at all (runtime, exit 1 — the needed environment/recorded state is missing). Missing-state faults (exit 1): a target naming no readable file and no plausible content id (`'{target}' is neither a readable .json file nor a 64-hex content id`), and a well-formed-but-unregistered 64-hex content id (`no campaign {id} in the project store`) — no file or store entry exists to -have content in either case. File-content faults (exit 2): a file that does -not parse as JSON at all, a valid-JSON op-script array (not a document exec -can execute at all — it names the `graph build` escalation instead), and a -kind-bearing document whose `"kind"` is not `"campaign"` (e.g. a process -document handed to `exec`) — all three are a real, readable file whose -content is the fault. The wrong-kind case is the one realignment this pin -made (#342 item 5): it shipped at exit 1 (mirroring the campaign leg's own -missing-state refusals) until this pass, despite being a file-content fault -like its op-script and not-valid-JSON siblings, not a missing-state fault -like the file-vs-id cases — realigned to exit 2 to match its true class, the -one sanctioned behaviour change of this pin. +have content in either case. File-content faults at the routing seam (exit +2): a file that does not parse as JSON at all, a valid-JSON op-script array +(not a document exec can execute at all — it names the `graph build` +escalation instead), and a kind-bearing document whose `"kind"` is not +`"campaign"` (e.g. a process document handed to `exec`) — all three are a +real, readable file whose content the ROUTING seam itself cannot classify +into a leg. The wrong-kind case is the one realignment this pin made (#342 +item 5): it shipped at exit 1 (mirroring the campaign leg's own missing-state +refusals) until this pass, despite being a routing-seam content fault like +its op-script and not-valid-JSON siblings, not a missing-state fault like the +file-vs-id cases — realigned to exit 2 to match its true class, the one +sanctioned behaviour change of this pin. + +Once a target classifies as campaign or blueprint, it falls under that leg's +**own** established validation partition, not this one — and the two legs' +content-validation exits differ today, named honestly rather than harmonized: +a `kind:"campaign"` document whose *content* then fails the campaign +doc-tier gate (e.g. an empty axis) exits 1, runtime-class, per the +established campaign contract (pinned by +`campaign_run_invalid_file_refuses_before_touching_store`), while the +blueprint leg's own content faults (a blueprint envelope that fails to parse +or build) exit 2, usage-class. No behaviour changes here — only this pin's +scope is named precisely: it is the routing seam's partition, not a +uniform content-validation rule across both legs. **Two artifact classes, two redundancy budgets** (#249, ratified 2026-07-13). The data layer the programmatic face emits is a public interface read raw (by diff --git a/docs/design/contracts/c18-registry.md b/docs/design/contracts/c18-registry.md index cf698c9..24c6ba1 100644 --- a/docs/design/contracts/c18-registry.md +++ b/docs/design/contracts/c18-registry.md @@ -148,12 +148,11 @@ the referenced blueprint is the single source of truth for which namespace a nam falls in. Pinned by `referential_tier_accepts_a_kind_correct_axis_over_a_bound_param` (`aura-registry/src/lib.rs`). The single-run analogue — `exec`'s own -`--override NODE.PARAM=VALUE` reopening a bound param for one execution — has -the identical hash consequence, ratified rather than fixed (#343): the -executed topology's `topology_hash` is the reopened topology's own, not the -referenced document's, exactly as a reopened campaign member's has always -been ([C24](c24-blueprint-data.md)'s "Reproduction identity" carries the full -ratification). +`--override NODE.PARAM=VALUE` reopening a bound param for one execution — now +truthfully has the identical hash consequence (#343, revised): both legs +stamp the referenced/loaded document's own id, never the reopened topology's, +exactly as a reopened campaign member's has always been ([C24](c24-blueprint-data.md)'s +"Reproduction identity" carries the full clause). **The campaign executor.** `aura exec ` (#319) executes a campaign (a file is register-then-run sugar; the content id is canonical): a diff --git a/docs/design/contracts/c24-blueprint-data.history.md b/docs/design/contracts/c24-blueprint-data.history.md index 547dd3e..d65992d 100644 --- a/docs/design/contracts/c24-blueprint-data.history.md +++ b/docs/design/contracts/c24-blueprint-data.history.md @@ -246,3 +246,11 @@ registered artifact is never retroactively invalidated." (superseded by the #319 sugar retirement, 2026-07-25 — audit-close correction: no family-minting path draws synthetic walks after the family-builder retirement; only `reproduce` consumes them) + +**"Registered-blueprint splice" clause, build-free-introspection sentence +(amended by the 2026-07-26 harvest, #339 item 4):** "build-free introspection +paths pass `&|_| None`." (The CLI's introspection paths now resolve `use` +refs through the store exactly like `graph build` does, via the shared +`parse_and_resolve_ops` phase — retiring the earlier build-free resolver on +the introspection side; the engine's `subgraph` closure contract itself is +unchanged.) diff --git a/docs/design/contracts/c24-blueprint-data.md b/docs/design/contracts/c24-blueprint-data.md index c0a5c21..63e4510 100644 --- a/docs/design/contracts/c24-blueprint-data.md +++ b/docs/design/contracts/c24-blueprint-data.md @@ -119,24 +119,23 @@ itself blueprint-data. The **identity id** (#171) is an additive sibling: --identity-id`. The byte-exact content id keeps the store/reproduce roles; the identity id is introspection-only until a dedup consumer exists. -**The override-reopened hash is ratified, not a defect (#343).** `exec`'s -`--override NODE.PARAM=VALUE` (above) reopens a bound param before bootstrap, so -the executed signal's canonical bytes — and therefore its `topology_hash` — are -the **reopened topology's own**, which may correspond to no blueprint ever -`put_blueprint`-stored, even for a "no-op" override that re-binds a param to its -existing bound value (the bound value still moves from `defaults` to `params`, -#249, changing the canonical bytes `topology_hash` covers). This is deliberate, -not accidental: an override **is** a topology-parameter variation, exactly as -the campaign leg's #246 axis-over-bound-param path has always minted the -reopened member's hash, never the referenced document's ([C18](c18-registry.md)'s -bound-override coincidence) — stamping the pre-override document's stored hash -on the single-run leg instead would make the two reopen mechanisms disagree -about what identity means for the same mechanism. Reproduction identity survives -regardless: the manifest's `params` map plus the base document deterministically -reconstruct the executed variant, byte-identically. An unresolved -`topology_hash` is the honest name of a real, reconstructible topology, not a -dangling reference — store-resolution (`get_blueprint`) is a lookup -convenience, never an identity guarantee. +**`topology_hash` carries reference semantics on every leg (#343, revised).** +`exec`'s `--override NODE.PARAM=VALUE` (above) reopens a bound param before +bootstrap, but the executed run's `topology_hash` stamps the **loaded (or +stored) base document's own content id** — computed before the reopen — +never a transient reopened variant, exactly as the campaign leg's #246 +axis-over-bound-param path has always stamped the referenced strategy's id +for a reopened member ([C18](c18-registry.md)'s bound-override coincidence). +The manifest's `params` map carries the variation (the bound value still +moves from `defaults` to `params`, #249); `params` plus the base document +deterministically reconstruct the executed run, byte-identically — +reproduction identity, not the hash, is where the variation lives. A no-op +override (re-binding a param to its existing bound value) therefore changes +nothing about identity: the bare run's and the no-op-override run's +`topology_hash` are equal. An unresolved `topology_hash` (a file-target exec +never registered via `graph register`) is still the honest name of a real, +reconstructible topology, not a dangling reference — store-resolution +(`get_blueprint`) is a lookup convenience, never an identity guarantee. ### Axis discovery @@ -199,7 +198,12 @@ it ever reaches the session, so a doc-less fetched entry cannot enter a NEW composition — a **backstop**: the register verb already gates both input forms, so this fires only for store content written before C29 or through the raw in-crate path), and the resolution echo — happens CLI-side, at DTO conversion, before -replay; build-free introspection paths pass `&|_| None`. The echo +replay; the CLI's introspection paths resolve `use` refs through the store +exactly like `graph build` does (`parse_and_resolve_ops`, the shared first +phase both callers run — #339 item 4 harvest retired the earlier build-free +`&|_| None` resolver on the introspection side). The engine's closure +contract above is unchanged: only the CLI-side caller supplied for +introspection now always threads a real store-backed resolver. The echo (`aura: note: use "": -> `) is the existing `aura: note:` benign-diagnostic marker (C14) — a new instance of the existing class, not a new one, so the exit-code/marker taxonomy is unchanged.