From 43a1cc44d3bb6ca176828a4bbf52c209af56b1a2 Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 18 Jul 2026 16:22:18 +0200 Subject: [PATCH] =?UTF-8?q?feat(graph):=20name-addressed=20`tap`=20constru?= =?UTF-8?q?ction=20op=20=E2=80=94=20declared=20taps=20without=20raw=20indi?= =?UTF-8?q?ces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declared taps (C27, #282) were authorable only by hand-patching the raw blueprint envelope's `taps` array, where `node` is a raw interior index — an index miscount was the expected mistake and the root friction behind the two `aura run` panics the measurement-milestone fieldtest found. Add a `tap` op to the construction op-script vocabulary — the exact output-side twin of `expose`. It names its source wire by the dotted `"node.field"` identifier the op surface uses everywhere (`split_port` + `resolve_output_field` + `node_index`), so `{"op":"tap","from":"fast.value","as":"fast_ma"}` replaces patching `{"node":14,"field":0}`. Engine (construction.rs): `Op::Tap`, its `tap()` handler (a verbatim twin of `expose` — same resolvers, same name-dedup, appending a `Tap` to a new `taps`/`tap_names` namespace), and `OpError::DuplicateTap` (the twin of `DuplicateOutput`, stricter than the raw envelope, which enforces no tap-name uniqueness). `finish()` now threads `.with_taps(self.taps)` into the built `Composite` — fixing a latent drop #282 left (it built `.with_gangs` but never `.with_taps`, so op-built composites silently lost their taps). CLI (graph_construct.rs): `OpDoc::Tap` (externally tagged, `"as"`-renamed like `expose`) + the `kind_label` / `From` / `format_op_error` arms (the last compiler-enumerated by the exhaustive match). No change to the `Tap` / `FlatTap` / `bind_tap` / `aura run` machinery — the op-declared tap flows through the existing #282 compile + record path unchanged. Verification: 7 new tests green (4 engine: resolve+append, bad-port / ghost-node refusal, duplicate-name refusal, finish-threads-into-FlatGraph; 1 CLI serde round-trip; 2 CLI E2E over the real `aura graph build` / `aura run` subprocess — one pinning the exact serialized `"taps":[{"name":"fast_ma","from":{"node":0,"field":0}}]`, one proving an op-authored tap round-trips through build -> run -> persisted trace). Full workspace suite green; clippy clean. The RED-before-GREEN step was verified by a temporary stash of the production edits (the `Tap` / `with_taps` machinery pre-existed from #282, so a fresh RED-only test could not be authored) — the new tests fail without the edits, pass with them. Docs (the authoring-guide `tap` row, the ledger op-list `tap` line, and a worked tap-authoring example) are deliberately out of this commit's scope — they land in #285, the docs follow-up in this same cycle. closes #284 --- crates/aura-cli/src/graph_construct.rs | 34 ++++++++ crates/aura-cli/tests/graph_construct.rs | 104 ++++++++++++++++++++++ crates/aura-engine/src/construction.rs | 106 ++++++++++++++++++++++- 3 files changed, 243 insertions(+), 1 deletion(-) diff --git a/crates/aura-cli/src/graph_construct.rs b/crates/aura-cli/src/graph_construct.rs index b41bbaa..c72adbb 100644 --- a/crates/aura-cli/src/graph_construct.rs +++ b/crates/aura-cli/src/graph_construct.rs @@ -44,6 +44,11 @@ enum OpDoc { #[serde(rename = "as")] as_name: String, }, + Tap { + from: String, + #[serde(rename = "as")] + as_name: String, + }, Gang { #[serde(rename = "as")] as_name: String, @@ -61,6 +66,7 @@ impl OpDoc { OpDoc::Feed { .. } => "feed", OpDoc::Connect { .. } => "connect", OpDoc::Expose { .. } => "expose", + OpDoc::Tap { .. } => "tap", OpDoc::Gang { .. } => "gang", } } @@ -77,6 +83,7 @@ impl From for Op { OpDoc::Feed { role, into } => Op::Feed { role, into }, OpDoc::Connect { from, to } => Op::Connect { from, to }, OpDoc::Expose { from, as_name } => Op::Expose { from, as_name }, + OpDoc::Tap { from, as_name } => Op::Tap { from, as_name }, OpDoc::Gang { as_name, into } => Op::Gang { as_name, into }, } } @@ -90,6 +97,7 @@ fn format_op_error(e: &OpError) -> String { OpError::UnknownNodeType(t) => format!("unknown node type {t:?}"), OpError::DuplicateIdentifier(s) => format!("duplicate identifier {s:?}"), OpError::DuplicateOutput(s) => format!("duplicate output name {s:?}"), + OpError::DuplicateTap(s) => format!("duplicate tap name {s:?}"), OpError::DuplicateRole(s) => format!("duplicate role {s:?}"), OpError::UnknownIdentifier(s) => format!("unknown node identifier {s:?}"), OpError::UnknownRole(s) => format!("unknown role {s:?}"), @@ -702,4 +710,30 @@ mod tests { assert!(out.contains("fast.series"), "fast.series is still open: {out}"); assert!(!out.contains("sub.lhs"), "sub.lhs is covered: {out}"); } + + /// The `tap` op-doc parses (externally tagged, `"as"`-renamed like `expose`) + /// and `aura graph build` emits a blueprint whose serialized `taps` names the + /// wire — the name-addressed authoring path the fieldtest wanted, no raw index. + #[test] + fn tap_opdoc_builds_a_blueprint_carrying_the_tap() { + let doc = r#"[ + {"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":"expose","from":"sub.value","as":"bias"} + ]"#; + // the tap-doc kind_label registers as "tap" + let docs: Vec = serde_json::from_str(doc).expect("document parses"); + assert_eq!(docs[7].kind_label(), "tap"); + // and the built blueprint carries the tap (an un-tapped composite emits no + // `taps` key, so its presence is the proof the op threaded through) + let json = super::build_from_str(doc, &crate::project::Env::std()).expect("tap op-doc builds"); + assert!(json.contains("\"taps\""), "blueprint carries the taps section: {json}"); + assert!(json.contains("fast_ma"), "the tap names the wire: {json}"); + } } diff --git a/crates/aura-cli/tests/graph_construct.rs b/crates/aura-cli/tests/graph_construct.rs index 9561a3b..05a7090 100644 --- a/crates/aura-cli/tests/graph_construct.rs +++ b/crates/aura-cli/tests/graph_construct.rs @@ -1294,3 +1294,107 @@ fn graph_build_hints_attach_for_namespaced_ids_in_a_data_only_project() { "the hint points at the attach verb: {stderr}" ); } + +/// Property (#284): the `tap` op-script op, driven through the real `aura +/// graph build` process (not an in-crate call to `build_from_str`), resolves +/// a name-addressed interior wire (`"fast.value"`, session node 0 / output +/// field 0) into the exact same `{node, field}` shape a raw-index-authored +/// blueprint would carry — the emitted blueprint's `taps` array names the +/// wire by resolved index. This is the process-boundary counterpart of +/// `construction.rs`'s `tap_op_resolves_and_appends` (in-crate call): it pins +/// that the CLI's serde front end (`OpDoc::Tap` → `Op::Tap`) actually reaches +/// the engine and back out to stdout across a real subprocess, the surface a +/// data-level author drives. +#[test] +fn graph_build_emits_a_tap_op_declared_wire() { + let ops = r#"[ + {"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":"expose","from":"sub.value","as":"bias"} + ]"#; + let (stdout, stderr, ok) = run(&["graph", "build"], ops); + assert!(ok, "graph build: stderr: {stderr}"); + assert!( + stdout.contains(r#""taps":[{"name":"fast_ma","from":{"node":0,"field":0}}]"#), + "the tap resolves to node 0 (fast), field 0 (value): {stdout}" + ); +} + +/// Property (#284): a blueprint authored through the name-addressed `tap` op +/// — built by the real `aura graph build` process, never hand-edited by raw +/// node index — is not merely well-formed JSON: `aura run` actually binds and +/// persists its declared tap, exactly as a raw-index-authored tap does +/// (`tap_recording.rs`'s coverage of the underlying bind/persist mechanism). +/// This is the whole point of #284 (removing the raw-index miscount class): +/// the op-authored artifact must round-trip through the full build -> run -> +/// persisted-trace pipeline, not just parse. Taps `"slow.value"` (SMA(4), +/// deliberately not the node-0 `"fast"` port the sibling build-only test +/// checks) so this test cannot pass by accident on the wrong node. +#[test] +fn tap_authored_via_op_script_runs_and_persists_the_series() { + const PRICES: [f64; 18] = [ + 1.0000, 1.0008, 1.0021, 1.0039, 1.0062, 1.0090, 1.0083, 1.0061, 1.0034, 1.0012, 0.9998, + 1.0006, 1.0024, 1.0047, 1.0069, 1.0086, 1.0097, 1.0092, + ]; + let ops = r#"[ + {"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":"add","type":"Bias","name":"bias","bind":{"scale":{"F64":0.5}}}, + {"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":"connect","from":"sub.value","to":"bias.signal"}, + {"op":"tap","from":"slow.value","as":"slow_ma"}, + {"op":"expose","from":"bias.bias","as":"bias"} + ]"#; + let (blueprint_json, stderr, ok) = run(&["graph", "build"], ops); + assert!(ok, "graph build: stderr: {stderr}"); + assert!(blueprint_json.contains("\"taps\""), "blueprint carries the tap: {blueprint_json}"); + + let dir = temp_cwd("tap-op-run-roundtrip"); + let bp_path = dir.join("tapped.json"); + std::fs::write(&bp_path, &blueprint_json).expect("write built blueprint"); + + let (_stdout, stderr2, code2) = run_in(&dir, &["run", bp_path.to_str().unwrap()]); + assert_eq!(code2, Some(0), "aura run over the op-authored tap: {stderr2}"); + + // `aura graph build` always names the root composite "graph" (`composite_from_str`), + // so the trace store's run-name directory is fixed. + let trace_path = dir.join("runs/traces/graph/slow_ma.json"); + let trace_text = std::fs::read_to_string(&trace_path) + .unwrap_or_else(|e| panic!("expected a persisted tap trace at {}: {e}", trace_path.display())); + let trace: serde_json::Value = serde_json::from_str(&trace_text).expect("parse tap trace json"); + + assert_eq!(trace["tap"], "slow_ma"); + assert_eq!(trace["kinds"], serde_json::json!(["F64"])); + + // SMA(4), silent until warm (4 samples): first emission at the 4th sample + // (ts=4), a rolling mean of the last four prices per subsequent cycle. + let expected: Vec = (3..PRICES.len()) + .map(|i| PRICES[i - 3..=i].iter().sum::() / 4.0) + .collect(); + let expected_ts: Vec = (4..=PRICES.len() as i64).collect(); + + let got_ts: Vec = + trace["ts"].as_array().expect("ts array").iter().map(|v| v.as_i64().unwrap()).collect(); + assert_eq!(got_ts, expected_ts, "recorded timestamps"); + + let got: Vec = trace["columns"][0] + .as_array() + .expect("columns[0] array") + .iter() + .map(|v| v.as_f64().unwrap()) + .collect(); + assert_eq!(got.len(), expected.len(), "recorded row count"); + for (i, (g, e)) in got.iter().zip(expected.iter()).enumerate() { + assert!((g - e).abs() < 1e-9, "row {i}: got {g}, expected {e}"); + } +} diff --git a/crates/aura-engine/src/construction.rs b/crates/aura-engine/src/construction.rs index 8f258e0..74e842f 100644 --- a/crates/aura-engine/src/construction.rs +++ b/crates/aura-engine/src/construction.rs @@ -14,7 +14,7 @@ use aura_core::{BindOpError, NodeSchema, PrimitiveBuilder, Scalar, ScalarKind}; use crate::blueprint::{ check_param_namespace_injective, check_root_roles_bound, edge_kind_check, validate_wiring, - BlueprintNode, Composite, Gang, GangMember, OutField, Role, + BlueprintNode, Composite, Gang, GangMember, OutField, Role, Tap, TapWire, }; use crate::builder::{resolve_input_slot, resolve_output_field, PortResolveError}; use crate::harness::{Edge, Target}; @@ -37,6 +37,9 @@ pub enum Op { Connect { from: String, to: String }, /// Re-export `"node.field"` at the boundary under `as_name`. Expose { from: String, as_name: String }, + /// Declare a measurement tap on `"node.field"` under `as_name` (#284) — the + /// output-side twin of `Expose` (a `Tap`, not an `OutField`). + Tap { from: String, as_name: String }, /// Fuse two or more sibling params into one public knob (#61). Gang { as_name: String, into: Vec }, } @@ -51,6 +54,11 @@ pub enum OpError { /// distinct fault class from an interior node-id clash, so callers can tell the /// two apart from the error value alone. DuplicateOutput(String), + /// A tap-name collision (the separate `tap_names` namespace) — two taps named + /// alike would collide as recorded series. A distinct fault class from an + /// interior id (`DuplicateIdentifier`) or a boundary-output clash + /// (`DuplicateOutput`). + DuplicateTap(String), DuplicateRole(String), UnknownIdentifier(String), UnknownRole(String), @@ -101,6 +109,8 @@ pub struct GraphSession<'v> { role_names: HashMap, out: Vec, out_names: HashSet, + taps: Vec, + tap_names: HashSet, coverage: HashMap<(usize, usize), usize>, gangs: Vec, } @@ -121,6 +131,8 @@ impl<'v> GraphSession<'v> { role_names: HashMap::new(), out: Vec::new(), out_names: HashSet::new(), + taps: Vec::new(), + tap_names: HashSet::new(), coverage: HashMap::new(), gangs: Vec::new(), } @@ -136,6 +148,7 @@ impl<'v> GraphSession<'v> { Op::Feed { role, into } => self.feed(role, into), Op::Connect { from, to } => self.connect(from, to), Op::Expose { from, as_name } => self.expose(from, as_name), + Op::Tap { from, as_name } => self.tap(from, as_name), Op::Gang { as_name, into } => self.gang(as_name, into), } } @@ -304,6 +317,25 @@ impl<'v> GraphSession<'v> { Ok(()) } + /// Declare a measurement tap on a resolved interior output wire — the + /// output-side twin of `expose`. Node resolution (`node_index` → + /// `UnknownIdentifier`), the output-field resolver, and the name-dedup are the + /// same as `expose`'s; only the appended item differs: a `Tap` on the separate + /// `taps`/`tap_names` namespace (dedup → `DuplicateTap`), not an `OutField`. + fn tap(&mut self, from: String, as_name: String) -> Result<(), OpError> { + let (from_node, from_field_name) = Self::split_port(&from)?; + let fi = self.node_index(&from_node)?; + let field = resolve_output_field(&self.schemas[fi], &from_field_name).map_err(|e| match e { + PortResolveError::Unknown => OpError::UnknownOutPort { node: from_node.clone(), name: from_field_name.clone() }, + PortResolveError::Ambiguous => OpError::AmbiguousOutPort { node: from_node.clone(), name: from_field_name.clone() }, + })?; + if !self.tap_names.insert(as_name.clone()) { + return Err(OpError::DuplicateTap(as_name)); + } + self.taps.push(Tap { name: as_name, from: TapWire { node: fi, field } }); + Ok(()) + } + /// Eager arm of the gang gate: resolve every member against the CURRENT /// (post-bind) open param lists — a member bound at `Add` has left the open /// schema and correctly fails as `UnknownParam`, mirroring `try_bind` — enforce @@ -396,6 +428,7 @@ impl<'v> GraphSession<'v> { // doc-carrying surface yet (#125 scope cut) — an op-built composite // carries no authored rationale until that vocabulary grows a slot. let c = Composite::new(self.name, self.nodes, self.edges, roles, self.out) + .with_taps(self.taps) .with_gangs(self.gangs) .map_err(OpError::Incomplete)?; check_param_namespace_injective(&c.param_space()).map_err(OpError::Incomplete)?; @@ -639,6 +672,77 @@ mod tests { ); } + /// `tap` names an interior output wire by `"node.field"` (the output-side twin + /// of `expose`), appending a resolved `Tap` to the composite — the + /// name-addressed wire resolves to the same interior `{node, field}` a + /// raw-index tap would carry (`fast` is session node 0, its `value` field 0). + #[test] + fn tap_op_resolves_and_appends() { + use crate::blueprint::{Tap, TapWire}; + let mut s = scaffold(); + s.apply(Op::Feed { role: "price".into(), into: vec!["fast.series".into(), "slow.series".into()] }).unwrap(); + s.apply(Op::Connect { from: "fast.value".into(), to: "sub.lhs".into() }).unwrap(); + s.apply(Op::Connect { from: "slow.value".into(), to: "sub.rhs".into() }).unwrap(); + s.apply(Op::Expose { from: "sub.value".into(), as_name: "bias".into() }).unwrap(); + assert_eq!(s.apply(Op::Tap { from: "fast.value".into(), as_name: "fast_ma".into() }), Ok(())); + let c = s.finish().expect("finishes"); + assert_eq!(c.taps(), &[Tap { name: "fast_ma".into(), from: TapWire { node: 0, field: 0 } }]); + } + + /// `tap` reuses `expose`'s resolvers: a bad output field maps to the + /// by-identifier `UnknownOutPort`; an unknown source node to `UnknownIdentifier` + /// (via `node_index`), never the nonexistent `UnknownNode`. + #[test] + fn tap_op_bad_out_port_and_ghost_node_rejected() { + let mut s = scaffold(); + assert_eq!( + s.apply(Op::Tap { from: "fast.nope".into(), as_name: "t".into() }), + Err(OpError::UnknownOutPort { node: "fast".into(), name: "nope".into() }) + ); + assert_eq!( + s.apply(Op::Tap { from: "ghost.value".into(), as_name: "t".into() }), + Err(OpError::UnknownIdentifier("ghost".into())) + ); + } + + /// A second tap under the same boundary name is a `DuplicateTap` — the twin of + /// `expose`'s `DuplicateOutput`, over the separate `tap_names` namespace. + #[test] + fn tap_op_duplicate_name_rejected() { + let mut s = scaffold(); + assert_eq!(s.apply(Op::Tap { from: "fast.value".into(), as_name: "t".into() }), Ok(())); + assert_eq!( + s.apply(Op::Tap { from: "slow.value".into(), as_name: "t".into() }), + Err(OpError::DuplicateTap("t".into())) + ); + } + + /// `finish()` threads op-declared taps into `Composite.taps`, and the #282 + /// compile path hoists them into `FlatGraph.taps` — proving the `.with_taps` + /// thread this op adds reaches the flat graph (guards the latent drop `finish` + /// had: it built `.with_gangs` but never `.with_taps`). `fast` lowers to flat + /// node 0, its `value` field 0. + #[test] + fn tap_op_threads_into_flat_graph() { + use crate::harness::FlatTap; + let ops = vec![ + Op::Source { role: "price".into(), kind: ScalarKind::F64 }, + Op::Add { type_id: "SMA".into(), as_name: Some("fast".into()), bind: vec![] }, + Op::Add { type_id: "SMA".into(), as_name: Some("slow".into()), bind: vec![] }, + Op::Add { type_id: "Sub".into(), as_name: None, bind: vec![] }, + Op::Feed { role: "price".into(), into: vec!["fast.series".into(), "slow.series".into()] }, + Op::Connect { from: "fast.value".into(), to: "sub.lhs".into() }, + Op::Connect { from: "slow.value".into(), to: "sub.rhs".into() }, + Op::Tap { from: "fast.value".into(), as_name: "fast_ma".into() }, + Op::Expose { from: "sub.value".into(), as_name: "bias".into() }, + ]; + let flat = super::replay("g", ops, &aura_std::std_vocabulary) + .expect("replays") + .compile_with_params(&[Scalar::i64(2), Scalar::i64(4)]) + .expect("compiles"); + assert_eq!(flat.taps, vec![FlatTap { name: "fast_ma".into(), node: 0, field: 0 }]); + } + /// The gang op fuses two sibling params into one public knob; the replayed /// composite's `param_space()` carries exactly the gang address. #[test]