From 581849766fe65088a919ef4d217f0e345e757ba9 Mon Sep 17 00:00:00 2001 From: Brummel Date: Thu, 11 Jun 2026 11:20:22 +0200 Subject: [PATCH] spec: 0031 node-instance naming (boss-signed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design spec for #56 — give every blueprint node a name so param-space path-qualification and fan-in distinguishability both flow from one author-controlled identity, and retire the index-addressed ParamAlias overlay. A primitive builder gains an optional name (default = the node's type-label string, ASCII-lowercased verbatim: "SMA"->"sma", "SimBroker"->"simbroker"; explicit names must be non-empty). param_space() becomes uniformly . at every level including the root. signature_of and the C9 fan-in rule re-key onto the node name: two same-type siblings both defaulting to "sma" collide and IndistinguishableFanIn fires correctly, so one act — naming the legs "fast"/"slow" — fixes both the naming collision and the fan-in check. ParamAlias / Composite.params / the alias index-check are removed (Role.name and OutField.name untouched). Ledger C9 and C23 amended. One cohesive iteration: the engine mechanism + in-workspace migration (CLI sample, engine tests, sweep goldens, graph_model.rs, sweep.rs, lib.rs re-export) land together since removing the struct is compile-coupled; fieldtests/ are frozen non-workspace records, out of scope. Boss-signed under spec_auto_sign: all objective gates green (precondition, self-review, grounding-check PASS) and a unanimous five-lens spec-skeptic panel (criterion / grounding / scope-fork / ambiguity / plan-readiness) returned SOUND after one fix round. The panel's first round caught real defects — a worked example written against a non-existent fluent API, a false [param:] render-marker claim (the marker was removed in cycle 0020), and an under-specified compound default name — all corrected and re-grounded before the unanimous pass. refs #56 --- docs/specs/0031-node-instance-naming.md | 336 ++++++++++++++++++++++++ 1 file changed, 336 insertions(+) create mode 100644 docs/specs/0031-node-instance-naming.md diff --git a/docs/specs/0031-node-instance-naming.md b/docs/specs/0031-node-instance-naming.md new file mode 100644 index 0000000..5f68bcb --- /dev/null +++ b/docs/specs/0031-node-instance-naming.md @@ -0,0 +1,336 @@ +# Node-instance naming — Design Spec + +**Date:** 2026-06-11 +**Status:** Draft — awaiting user spec review +**Authors:** orchestrator + Claude + +## Goal + +Give every blueprint node a **name** so that param-space path-qualification and +fan-in distinguishability both flow from one author-controlled identity, and +**retire** the index-addressed `ParamAlias` overlay (#56). + +Today a freshly-authored two-leg SMA cross is unusable in its minimal form: its +`param_space()` emits `sma_cross.length` twice (a name collision — `UnknownKnob` +/ `AmbiguousKnob` on any by-name bind), and it does not even bootstrap +(`IndistinguishableFanIn`). The only fix is two hand-written +`ParamAlias { name, node, slot }` overlays — bookkeeping the author must *count* +against the compiled graph by node index and slot index. The shipped CLI sample +carries exactly these two overlays (`crates/aura-cli/src/main.rs:150-151`), so +the project's own running example cannot be expressed without the index dance. + +This cycle moves identity from the **param slot** (where `ParamAlias` put it) to +the **node** (a builder name). One author act — naming the two SMAs `"fast"` and +`"slow"` — fixes both the naming collision and the fan-in check, because the node +name feeds both path-qualification and the distinguishability signature. + +## Architecture + +A primitive builder gains an optional **name**. When omitted, the name defaults +to the node's existing **type-label string** (the `NodeSchema` type label each +primitive already declares — e.g. `Sma`'s is `"SMA"`, `Exposure`'s is +`"Exposure"`, `SimBroker`'s is `"SimBroker"`), **ASCII-lowercased verbatim** with +no snake_case insertion: `"SMA"` → `"sma"`, `"SimBroker"` → `"simbroker"`, +`"LinComb"` → `"lincomb"`. When given, the name must be non-empty. There is +therefore *always* a meaningful node name — never an index, never blank. Composites already carry an author-given name (the first argument to +`Composite::new("sma_cross", …)`); this cycle extends the same property to +primitives, so **every** node has a name. + +`param_space()` path-qualification becomes **uniform**: a knob's name is +`..`, with the node-name segment present at every +level including the root. A leaf SMA named `fast` under composite `sma_cross` +yields `sma_cross.fast.length`; the root-level `Exposure` node yields +`exposure.scale`. The previous rule — composite interior path-qualified, +root-level leaves bare — is replaced by the single uniform rule. + +The fan-in distinguishability refinement (C9) is **re-keyed** onto the node name. +Two same-type siblings that both took the default name (`"sma"`) genuinely +collide; the collision is an `IndistinguishableFanIn` compile error, exactly as +intended — it forces the author to give distinct names. Giving the names +(`"fast"`/`"slow"`) is the *same* act that disambiguates the knob paths, so one +fix clears both walls. Genuinely-interchangeable **paramless** same-type nodes +(no knob, nothing to bind) remain legal, as today — the rule keys on a +param-bearing collision. + +`ParamAlias` (the param-projection overlay) is **retired**. The sibling +identity it used to supply now comes from the node name; the cosmetic +slot-rename it also allowed is dropped (factory param names stand; the author's +lever is the node name). The two *other* boundary projections — `Role.name` +(input roles) and `OutField.name` (output fields) — are untouched; only the +param overlay goes. + +This is a contract change, not a refactor: ledger entries **C9** (fan-in +distinguishability) and **C23** (named boundary projections) are amended with the +new rationale. **C11** (value-empty blueprints) is preserved and in fact +explains why this is correct: a blueprint holds no values, so the thing that +would otherwise distinguish two SMAs — their length — is not present at +construction; identity *must* come from a name, not a deferred value. **C12/C19** +are preserved: `param_space()` stays the flat, path-qualified ground truth that +both single-run binding and sweep enumeration address; only the path *shape* +changes, not the aggregation or slot order. + +## Concrete code shapes + +### The worked author example (the acceptance evidence) + +The authoring surface is the real positional `Composite::new(...)` constructor +(this cycle does **not** introduce a fluent builder — that is a separate +ergonomics concern, out of scope). The only new surface is `.named()` on a +primitive builder, and the *removal* of the `params: Vec` argument. + +**Today**, the author disambiguates the two legs by counting node and slot +indices against the compiled graph, in the 5th (`params`) argument: + +```rust +// today — the index-addressed overlay this cycle removes: +Composite::new( + "sma_cross", + vec![Sma::builder().into(), Sma::builder().into(), Sub::builder().into()], + vec![ + Edge { from: 0, to: 2, slot: 0, from_field: 0 }, + Edge { from: 1, to: 2, slot: 1, from_field: 0 }, + ], + vec![Role { name: "price".into(), targets: vec![ /* … */ ], source: None }], + vec![ + ParamAlias { name: "fast".into(), node: 0, slot: 0 }, // index bookkeeping + ParamAlias { name: "slow".into(), node: 1, slot: 0 }, + ], + vec![OutField { node: 2, field: 0, name: "cross".into() }], +); +// param_space() → ["sma_cross.fast", "sma_cross.slow"] +``` + +**After** this cycle, identity is carried inline by `.named()` on the two SMA +builders; the `params` argument is *gone* from `Composite::new`: + +```rust +// after — node identity inline; no ParamAlias, no index counting: +Composite::new( + "sma_cross", + vec![ + Sma::builder().named("fast").into(), + Sma::builder().named("slow").into(), + Sub::builder().into(), + ], + vec![ + Edge { from: 0, to: 2, slot: 0, from_field: 0 }, + Edge { from: 1, to: 2, slot: 1, from_field: 0 }, + ], + vec![Role { name: "price".into(), targets: vec![ /* … */ ], source: None }], + vec![OutField { node: 2, field: 0, name: "cross".into() }], +); +// param_space() → ["sma_cross.fast.length", "sma_cross.slow.length"] + +// bind by name — the names are EARNED by the .named() calls: +sma_cross + .with("sma_cross.fast.length", 2) + .with("sma_cross.slow.length", 4) + .bootstrap()?; +``` + +The same topology written **without** the names is *correctly rejected* — the +deliberate forcing function, not a regression: both SMAs default to `"sma"`, so +they collide and the fan-in is indistinguishable: + +```rust +// the two SMAs are NOT named → both default to "sma" → they collide: +vec![Sma::builder().into(), Sma::builder().into(), Sub::builder().into()] +// Composite::new(…).bootstrap() → Err(CompileError::IndistinguishableFanIn { node: 2 }) +// → the author is told to name the colliding legs. +``` + +### Before → after implementation shapes (secondary) + +**Primitive builder gains a name.** The builder carries an `Option` +name; the lowering/collection layers read it, falling back to the type tag. + +```rust +// after — the name slot and its default: +impl Sma { + pub fn builder() -> PrimitiveBuilder { /* name: None → defaults to "sma" */ } +} +impl PrimitiveBuilder { + // override the default; empty is rejected at construction + pub fn named(self, name: &str) -> PrimitiveBuilder { /* debug_assert non-empty */ } + // the resolved name: explicit if set, else the type-label string + // ASCII-lowercased verbatim (e.g. label "SimBroker" -> "simbroker") + fn node_name(&self) -> Cow { /* self.name, OR self.type_label().to_ascii_lowercase() */ } +} +``` + +**`collect_params` inserts the node segment, uniformly.** The alias lookup is +gone; the leaf contributes `..` (and at the root, where +`prefix` is empty, `.`). + +```rust +// after — no alias param; node-name is always a path segment: +fn collect_params(items: &[BlueprintNode], prefix: &str, out: &mut Vec) { + for item in items { + match item { + BlueprintNode::Primitive(b) => { + let node = join_nonempty(prefix, b.node_name()); // "." + for p in b.params() { + out.push(ParamSpec { name: format!("{node}.{}", p.name), kind: p.kind }); + } + } + BlueprintNode::Composite(c) => { + let child = join_nonempty(prefix, c.name()); + collect_params(c.nodes(), &child, out); // no aliases threaded + } + } + } +} +``` + +**`signature_of` keys on the node name.** The `aliases` parameter is gone; the +per-node initial is the node name (default or explicit) instead of the +type-initial-plus-alias-initials. + +```rust +// after — distinguishability comes from the node name: +fn signature_of(nodes: &[BlueprintNode], edges: &[Edge], roles: &[Role], node: usize) -> NodeSig { + NodeSig { + name: nodes[node].node_name().to_string(), // was: type initial + alias initials + inputs: /* recurse over edges, as today */, + } +} +``` + +**`IndistinguishableFanIn` keys on a param-bearing same-name collision.** The +`leaf_has_unaliased_param` predicate becomes `leaf_has_param` (any node with a +param whose name collides with a sibling is the unnamed-axis the rule catches); +paramless interchangeable nodes stay legal. + +```rust +// after — fires on colliding param-bearing legs; paramless stays legal: +CompileError::IndistinguishableFanIn { node } // when colliding sources share a + // node name AND ≥1 carries a param +``` + +**`ParamAlias` and the `Composite.params` field are removed.** `Composite::new` +loses its `params: Vec` argument. (There is no render-marker work: +the `[param:]` marker was already removed from the render path in cycle +0020 — the current renderer carries no `param:` marker, so nothing migrates +there.) + +## Components + +- **`aura-std` primitives (`Sma`, `Exposure`, `Sub`, `Add`, `LinComb`, …).** Each + builder gains the optional name + the type-tag default. A small shared + mechanism on `PrimitiveBuilder` (the `name: Option` field, `.named()`, + `node_name()`) so each primitive does not re-implement it. +- **`aura-engine` `blueprint.rs`.** `collect_params` (node segment, no alias), + `signature_of` (node-name key, no alias param), the fan-in predicate + (`leaf_has_param`), `param_space()` (unchanged call site, new path shape), + `Composite` (drop the `params` field + the `ParamAlias` struct + + `aliases_on` / `check_alias_indices`). The renderer is unaffected (it carries + no `param:` marker since cycle 0020). +- **`aura-core` `PrimitiveBuilder` (`node.rs`).** The shared `name: Option` + field, `.named()`, and `node_name()` (the type-label default) live here so each + primitive in `aura-std` does not re-implement them; the `NodeSchema` type label + is the default source. +- **`aura-engine` other `ParamAlias` consumers (compile-coupled to the removal).** + `graph_model.rs` (the `model_to_json` test fixtures), `sweep.rs` (sweep test + fixtures), and the `ParamAlias` re-export in `lib.rs` all reference the struct; + removing it breaks their compilation, so they migrate in this same iteration. +- **`aura-cli` `main.rs` (the sample, migrated in this cycle).** The two + `ParamAlias` lines become `.named("fast")`/`.named("slow")`; the sweep goldens + and the single-run `.with(...)` names migrate to the `*.length` / + `exposure.scale` forms. +- **Design ledger.** C9 and C23 amended. + +**Out of scope — frozen fieldtest fixtures.** The crates under `fieldtests/` +(including `fieldtests/milestone-the-world-param-sweep/` whose `mw_*` fixtures use +`ParamAlias` + `Composite::new`) are **frozen historical records** of what each +fieldtest found at its time; they are separate crates *not* members of the +workspace, so `cargo build/test --workspace` neither builds nor is gated by them. +They are deliberately **not** migrated (migrating them would rewrite the recorded +evidence) and will simply bit-rot against engine evolution, as the prior +`milestone-construction-layer` fixtures already do. The migration surface in this +cycle is exactly the workspace members named above. + +## Data flow + +`param_space()` → `collect_params(nodes, "", out)` walks the blueprint tree in +`lower_items` order (declared order, composites depth-first). Each primitive +contributes one `ParamSpec` per param slot, named +`..`; each composite extends the +prefix by its own name and recurses. Slot order and arity are **unchanged** from +today (the mirror test against the compiled flat-node order stays green); only +the per-knob *string* changes (it gains the node segment and, where a +`ParamAlias` used to relabel, reverts to `.`). The compilat +remains name-free and wired by raw index (C23): node names are construction-time +identity and debug symbols, dropped at lowering, exactly as alias names were. + +Bootstrap/bind (`.with()`, `.axis()`) is unchanged in mechanism: it matches the +exact `param_space()` string. The only observable difference is which strings +`param_space()` emits. + +## Error handling + +- **`IndistinguishableFanIn { node }`** — unchanged variant, re-keyed trigger: + fires when a fan-in node's colliding sources share a node name *and* at least + one carries a param slot. The author resolves it by giving the colliding legs + distinct `.named(...)`. Paramless interchangeable same-name legs do not fire. +- **Empty explicit name** — `.named("")` is an author error; rejected at + construction (`debug_assert!`, consistent with the builder's existing + contract-checks). The default path can never produce an empty name (a type tag + is never empty). +- **`UnknownKnob` / `AmbiguousKnob`** (the by-name binder, spec 0030) are + unchanged; they now operate over the node-qualified names. After this cycle the + canonical SMA-cross no longer *produces* an ambiguous knob in its named form, + which is the point. +- **Removed:** `BadInteriorIndex` for a dangling `ParamAlias` (the `check_alias_indices` + path) goes away with the overlay it guarded. Role/output alias index checks + remain. + +## Testing strategy + +RED-first, all in `crates/aura-engine/src/blueprint.rs` tests plus the CLI +goldens: + +- **Path shape.** A two-leg SMA cross under a composite emits + `sma_cross.fast.length` / `sma_cross.slow.length` (named) — replaces the + current alias-based assertion. A root-level param emits `.` + (`exposure.scale`) — **replaces** `top_level_leaf_params_are_unqualified` + (the bare-root pin is intentionally inverted by this cycle). +- **Default name.** An unnamed single primitive emits `.` + (`sma.length`). +- **Forcing function (RED → GREEN).** Two unnamed same-type param-bearing + siblings under a fan-in → `IndistinguishableFanIn`; the same two with distinct + `.named()` → bootstraps and binds. This is the headline executable spec. +- **Paramless interchangeable stays legal.** Two unnamed paramless same-type + siblings under a fan-in → compiles (no error), as today. +- **Slot-order mirror preserved.** `param_space_mirrors_compiled_flat_node_param_order` + and its `_under_nesting` sibling stay green after the names change (arity + + order invariant; only the strings move). +- **Determinism.** `param_space()` stays a pure structural function (C1). +- **CLI goldens.** The sweep JSON goldens (`main.rs`) assert the migrated names + (`sma_cross.fast.length`, `sma_cross.slow.length`, `exposure.scale`); the + single-run sample binds the migrated names. + +## Acceptance criteria + +1. A downstream author writes `Sma::builder().named("fast")` and binds + `sma_cross.fast.length` — no `ParamAlias`, no index counting (the worked + example above compiles and runs). +2. The canonical two-leg SMA cross bootstraps and binds by name in its minimal + authored form, with names but **without** any overlay. +3. An unnamed same-type param-bearing fan-in is rejected with + `IndistinguishableFanIn`, and the rejection names the node; naming the legs + resolves it. +4. `param_space()` is uniformly `.` at every level, collision-free + for distinct-named siblings; slot order and arity are unchanged (the mirror + tests stay green). +5. `ParamAlias`, `Composite.params`, and their index-check path are gone from the + engine; `Role.name` and `OutField.name` are untouched. +6. The CLI sample, the sweep goldens, and the spec-0030 worked example are + migrated; `cargo build/test --workspace` is green and + `cargo clippy --workspace --all-targets -- -D warnings` is clean. +7. Ledger C9 and C23 are amended with the new rationale. + +**Iteration scope:** one cohesive iteration. Removing `ParamAlias` is a +compile-breaking change to `Composite::new`'s signature and to the sample's call +sites, so the engine mechanism (builder name, `collect_params`, `signature_of`, +the fan-in re-key, the overlay removal) and the in-repo migration (CLI sample, +engine tests, goldens) must land together.