From 7d587d0c4e55dc5ce27bac5a4ef2973577f4bfa0 Mon Sep 17 00:00:00 2001 From: Brummel Date: Fri, 12 Jun 2026 16:26:28 +0200 Subject: [PATCH] =?UTF-8?q?feat(aura-core):=20PrimitiveBuilder::bind=20?= =?UTF-8?q?=E2=80=94=20fix=20a=20node=20param=20as=20a=20structural=20cons?= =?UTF-8?q?tant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `PrimitiveBuilder::bind(slot, value)`: bind a declared param to a structural constant, removed from param_space entirely rather than pinned in the injected vector. This closes the gap #55 names — param-bearing nodes (Sma/Exposure/LinComb) had no constant path, only the no-sweep-mode SimBroker precedent. The motivating case is "SMA2-entry": the 2 is bound to the two-candle construction, so sweeping it would enumerate deformed strategies as valid family members. Approach (Option B, builder-level — the originally-planned direction): bind shrinks the builder's declared param surface (schema.params) and wraps its build closure to re-splice the captured constant at its original positional slot. The construction layer (collect_params / lower_items / param_space / compile_with_params) is byte-unchanged: both dock sites already key off builder.params(), so the shrink propagates for free. Addressing is by param name (the by-name authoring address space, C23/0032); the compilat stays wired by raw index — names never reach it. bind enforces "exactly one open param matches" itself (collect-all-then-reject, panic on zero / on duplicate), mirroring the resolve binder's posture, because a node's own schema.params is not covered by check_param_namespace_injective. Chained binds reconstruct the correct positional vector because each layer computes its slot index relative to the param list it sees (no global position table). Considered and rejected: a per-node builder_const(N) constructor (explodes combinatorially over which subset is constant, not dynamic); a ParamBinding overlay struct on Composite (would need new bookkeeping the builder-level form avoids). Verified: cargo build/clippy/test --workspace all green; new tests cover slot removal, positional reconstruction (chained + partial, via eval), the three panic paths, and the composite param_space projection; blueprint.rs construction layer confirmed byte-unchanged. Deferred (out of scope, follow-up issue): exporting a named frozen strategy as a blueprint-as-values collection — the issue's third fork, needs its own brainstorm. closes #55 --- crates/aura-core/src/node.rs | 144 ++++++++++++++++++++++++++++ crates/aura-engine/src/blueprint.rs | 24 +++++ crates/aura-std/src/lincomb.rs | 34 +++++++ crates/aura-std/src/sma.rs | 17 ++++ 4 files changed, 219 insertions(+) diff --git a/crates/aura-core/src/node.rs b/crates/aura-core/src/node.rs index b53e814..f9eaded 100644 --- a/crates/aura-core/src/node.rs +++ b/crates/aura-core/src/node.rs @@ -127,6 +127,46 @@ impl PrimitiveBuilder { pub fn label(&self) -> String { self.name.to_string() } + /// Bind a declared param `slot` to a structural constant, removing it from the + /// node's param surface (`params()` / `schema().params` / the aggregated + /// `param_space`). Authoring-time: `slot` names the param (the by-name authoring + /// address space, C23/0032) and must match **exactly one** still-open param — + /// bind panics on zero matches (unknown / already-bound name) and on more than + /// one (an ambiguous duplicate-named slot); `value`'s kind must match the slot's + /// declared `ParamSpec.kind`. Returns `Self` so binds chain (`.bind(..).bind(..)`). + pub fn bind(mut self, slot: &str, value: Scalar) -> Self { + // Enforce the "exactly one" precondition rather than assume it: a node's own + // `schema.params` is NOT covered by `check_param_namespace_injective` (which + // guards only the aggregated path-qualified space, after `.bind()`), so + // per-node name uniqueness is not pinned elsewhere. Mirror the collect-all- + // then-reject posture of the `resolve` binder (blueprint.rs). + let matches: Vec = self + .schema + .params + .iter() + .enumerate() + .filter(|(_, p)| p.name == slot) + .map(|(i, _)| i) + .collect(); + let pos = match matches.as_slice() { + [pos] => *pos, + [] => panic!("bind: no open param named `{slot}`"), + _ => panic!("bind: ambiguous — multiple open params named `{slot}`"), + }; + assert_eq!( + value.kind(), + self.schema.params[pos].kind, + "bind: kind mismatch for param `{slot}`", + ); + self.schema.params.remove(pos); // [param_space side] shrink the declared surface + let inner = self.build; // [value side] wrap the build closure + self.build = Box::new(move |open: &[Scalar]| { + let mut full = open.to_vec(); + full.insert(pos, value); // re-splice at the slot's original position + inner(&full) + }); + self + } } /// A node's declared interface: its inputs (in order) and its output record — an @@ -276,4 +316,108 @@ mod tests { vec![ParamSpec { name: "length".into(), kind: ScalarKind::I64 }] ); } + + /// A node whose label echoes the param vector its constructor received — lets a + /// test read back the positional vector `.build()` reconstructed after binds. + struct Probe(Vec); + impl Node for Probe { + fn lookbacks(&self) -> Vec { + vec![] + } + fn eval(&mut self, _ctx: Ctx<'_>) -> Option<&[Scalar]> { + None + } + fn label(&self) -> String { + format!("{:?}", self.0) + } + } + + fn probe3() -> PrimitiveBuilder { + // params [a: I64, b: I64, c: F64]; build stores the received slice + PrimitiveBuilder::new( + "Probe", + NodeSchema { + inputs: vec![], + output: vec![], + params: vec![ + ParamSpec { name: "a".into(), kind: ScalarKind::I64 }, + ParamSpec { name: "b".into(), kind: ScalarKind::I64 }, + ParamSpec { name: "c".into(), kind: ScalarKind::F64 }, + ], + }, + |p| Box::new(Probe(p.to_vec())), + ) + } + + #[test] + fn bind_removes_slot_and_reconstructs_positionally() { + // chained bind of b then a (reverse slot order); c stays open + let bound = probe3().bind("b", Scalar::I64(8)).bind("a", Scalar::I64(7)); + assert_eq!( + bound.params().iter().map(|p| p.name.as_str()).collect::>(), + ["c"], // only c remains open + ); + let built = bound.build(&[Scalar::F64(9.0)]); // inject c + assert_eq!( + built.label(), + format!("{:?}", vec![Scalar::I64(7), Scalar::I64(8), Scalar::F64(9.0)]), + ); + + // partial: bind only b; a and c stay open and keep their positions + let built2 = probe3() + .bind("b", Scalar::I64(8)) + .build(&[Scalar::I64(7), Scalar::F64(9.0)]); // a, c injected in order + assert_eq!( + built2.label(), + format!("{:?}", vec![Scalar::I64(7), Scalar::I64(8), Scalar::F64(9.0)]), + ); + } + + #[test] + #[should_panic(expected = "no open param named")] + fn bind_unknown_slot_panics() { + let b = PrimitiveBuilder::new( + "Probe", + NodeSchema { + inputs: vec![], + output: vec![], + params: vec![ParamSpec { name: "length".into(), kind: ScalarKind::I64 }], + }, + |_| Box::new(Bare), + ); + let _ = b.bind("width", Scalar::I64(2)); // "width" is not a declared param + } + + #[test] + #[should_panic(expected = "ambiguous")] + fn bind_ambiguous_slot_panics() { + let b = PrimitiveBuilder::new( + "Probe", + NodeSchema { + inputs: vec![], + output: vec![], + params: vec![ + ParamSpec { name: "dup".into(), kind: ScalarKind::I64 }, + ParamSpec { name: "dup".into(), kind: ScalarKind::I64 }, + ], + }, + |_| Box::new(Bare), + ); + let _ = b.bind("dup", Scalar::I64(1)); // two slots named "dup" + } + + #[test] + #[should_panic(expected = "kind mismatch")] + fn bind_kind_mismatch_panics() { + let b = PrimitiveBuilder::new( + "Probe", + NodeSchema { + inputs: vec![], + output: vec![], + params: vec![ParamSpec { name: "length".into(), kind: ScalarKind::I64 }], + }, + |_| Box::new(Bare), + ); + let _ = b.bind("length", Scalar::F64(2.0)); // F64 value for an I64 slot + } } diff --git a/crates/aura-engine/src/blueprint.rs b/crates/aura-engine/src/blueprint.rs index b31baf1..5113815 100644 --- a/crates/aura-engine/src/blueprint.rs +++ b/crates/aura-engine/src/blueprint.rs @@ -1882,6 +1882,30 @@ mod tests { assert_eq!(space[2].kind, ScalarKind::F64); } + #[test] + fn param_space_reflects_only_open_knobs() { + use aura_std::{Exposure, Sma}; + // "sma2_entry": Sma "bias" with length bound to 2 (a structural constant) + // plus Exposure "exp" whose `scale` stays open. The bound knob must be + // absent from param_space; only the open one remains. + let strat = Composite::new( + "sma2_entry", + vec![ + Sma::builder().named("bias").bind("length", Scalar::I64(2)).into(), + Exposure::builder().named("exp").into(), + ], + vec![], // edges — irrelevant to param_space() + vec![], // input_roles + vec![], // output + ); + let space = strat.param_space(); + let names: Vec<&str> = space.iter().map(|p| p.name.as_str()).collect(); + // collect_params runs with an empty prefix, so a top-level leaf is qualified + // by its OWN node segment, not the root composite's name → "exp.scale". + // `bias.length` is GONE (bound), not present-but-fixed. + assert_eq!(names, ["exp.scale"]); + } + /// E2E (issue #34): the C23/#31 mirror invariant *under composite nesting*. /// `param_space()` (via `collect_params`) duplicates `lower_items`' depth- /// first traversal rather than sharing it, so the two orders must stay in diff --git a/crates/aura-std/src/lincomb.rs b/crates/aura-std/src/lincomb.rs index bc5bea9..dd3f1c5 100644 --- a/crates/aura-std/src/lincomb.rs +++ b/crates/aura-std/src/lincomb.rs @@ -148,4 +148,38 @@ mod tests { let names: Vec = lc.schema().inputs.iter().map(|p| p.name.clone()).collect(); assert_eq!(names, ["term[0]", "term[1]", "term[2]"]); } + + #[test] + fn chained_bind_reconstructs_positional_vector() { + // bind BOTH weights, in reverse slot order, to DISTINCT values; build empty. + let builder = LinComb::builder(2) + .bind("weights[1]", Scalar::F64(2.0)) + .bind("weights[0]", Scalar::F64(0.5)); + assert!(builder.params().is_empty()); + let mut lc = builder.build(&[]); + let mut inputs = vec![ + AnyColumn::with_capacity(ScalarKind::F64, 1), + AnyColumn::with_capacity(ScalarKind::F64, 1), + ]; + inputs[0].push(Scalar::F64(10.0)).unwrap(); + inputs[1].push(Scalar::F64(3.0)).unwrap(); + // 0.5*10 + 2.0*3 = 11.0 — holds ONLY if each weight landed in its right slot + // (a swap would give 2.0*10 + 0.5*3 = 21.5) + assert_eq!(lc.eval(Ctx::new(&inputs, Timestamp(0))), Some([Scalar::F64(11.0)].as_slice())); + + // partial: bind weights[0], leave weights[1] open → inject it at build + let partial = LinComb::builder(2).bind("weights[0]", Scalar::F64(0.5)); + assert_eq!( + partial.params().iter().map(|p| p.name.as_str()).collect::>(), + ["weights[1]"], + ); + let mut lc2 = partial.build(&[Scalar::F64(2.0)]); // weights[1] = 2.0 injected + let mut inputs2 = vec![ + AnyColumn::with_capacity(ScalarKind::F64, 1), + AnyColumn::with_capacity(ScalarKind::F64, 1), + ]; + inputs2[0].push(Scalar::F64(10.0)).unwrap(); + inputs2[1].push(Scalar::F64(3.0)).unwrap(); + assert_eq!(lc2.eval(Ctx::new(&inputs2, Timestamp(0))), Some([Scalar::F64(11.0)].as_slice())); + } } diff --git a/crates/aura-std/src/sma.rs b/crates/aura-std/src/sma.rs index e9ea9e1..e69578e 100644 --- a/crates/aura-std/src/sma.rs +++ b/crates/aura-std/src/sma.rs @@ -155,4 +155,21 @@ mod tests { fn input_slot_is_named_series() { assert_eq!(Sma::builder().schema().inputs[0].name, "series"); } + + #[test] + fn bind_removes_slot_from_param_space() { + // a bound param-bearing node reports an empty param surface — parity with the + // SimBroker precedent (nodes_declare_expected_params, this file) + let sma2 = Sma::builder().named("bias").bind("length", Scalar::I64(2)); + assert!(sma2.schema().params.is_empty()); + // contrast: the length-generic SMA keeps `length` open + assert_eq!(Sma::builder().named("bias").params().len(), 1); + } + + #[test] + fn bound_node_builds_with_injected_value() { + // built with an empty open slice, the bound builder yields SMA(2) + let node = Sma::builder().bind("length", Scalar::I64(2)).build(&[]); + assert_eq!(node.label(), "SMA(2)"); + } }