diff --git a/docs/specs/0040-wiring-totality-check.md b/docs/specs/0040-wiring-totality-check.md new file mode 100644 index 0000000..510963d --- /dev/null +++ b/docs/specs/0040-wiring-totality-check.md @@ -0,0 +1,311 @@ +# Wiring-totality check: reject unwired or double-wired input ports — Design Spec + +**Date:** 2026-06-14 +**Status:** Draft — awaiting user spec review +**Authors:** orchestrator + Claude + +> Narrowed scope of issue #65 (see the reconciliation comment on that issue). The +> original "promote names to load-bearing wiring keys / close the exposure-price +> swap structurally" framing is **dropped**: #64 (the typed-handle, named-port +> `GraphBuilder`, commit `a6a314b`) already moved authoring to handles + visible +> port names, so the residual same-kind swap is a valid-but-wrong name choice no +> structural check can catch, and a structural fix (per-port semantic tags) would +> push domain semantics into the deliberately domain-free engine (C10/C7). This +> cycle ships only the **name-free** half that stands on its own. + +## Goal + +Make a graph that leaves an interior input slot **unwired**, or wires one slot +from **more than one** producer, a compile-time error instead of a silently-wrong +run. Today an unwired input slot is bootstrapped with an empty column +(`crates/aura-engine/src/harness.rs:138-148`: every declared slot gets an +`AnyColumn` and a `SlotState { last_ts: i64::MIN }`, fresh only when fed) — so a +forgotten connection runs a deterministic but wrong backtest, and two producers +into one slot is ill-formed (a slot holds exactly one column) yet uncompiled- +against. A single new structural check at the existing compile boundary closes +both, for both authoring surfaces, without touching the compilat (C23). + +Non-goal (explicitly out of scope, dropped this cycle): promoting port/field/node +names to load-bearing wiring keys; node-name or port/field-name uniqueness; +per-port semantic tags; and any claim of closing the SimBroker exposure/price +swap. This spec is purely about **wiring totality** (every required slot wired +exactly once), an index-based property. + +## Architecture + +The check lives in one place: `validate_wiring` +(`crates/aura-engine/src/blueprint.rs:496`), the pre-build structural validation +that `Composite::compile_with_params` already calls once (blueprint.rs:201) and +that already **recurses into nested composites** (blueprint.rs:540-543). A new +`check_ports_connected(nodes, edges, roles)` added to that function therefore runs +at **every nesting level** automatically, and — because the typed `GraphBuilder` +lowers to `Composite::new` and is compiled through the same `compile_with_params` +(no separate compile path) — it is inherited identically by the raw +`Composite::new` path **and** the `GraphBuilder::build()` path with zero +builder-side code. This is the same single-site, both-surfaces shape +`check_param_namespace_injective` (blueprint.rs:409) already uses for the param +address space; this is its wiring-side sibling. + +Coverage is counted **uniformly across the two wiring mechanisms**: an interior +`Edge { to, slot }` and a role `Target { node, slot }` each contribute one unit of +coverage to the pair `(node_index, slot)`. A composite's own input **roles** +(`Role`, blueprint.rs:120) are coverage **providers** — their targets feed +interior slots — never consumers: a composite's own open roles (`source: None`) +are the legitimate "unwired here, wired by the enclosing graph" boundary and are +not themselves subject to the rule; the root case is already guarded by the +existing `UnboundRootRole` check (blueprint.rs:202-205). So the rule is scoped to +**interior nodes' input slots**, and a role's `source` field is irrelevant to +coverage counting (root-bound and nested-open roles count their targets the same). + +The check is index-based and name-free; it reads only `signature().inputs.len()` +and the existing index-wired `Edge`/`Target`, and emits nothing into the +`FlatGraph`. C23 is untouched: it proves the existing raw-index wiring is **total** +(every slot ≥1) and **single-valued** (every slot ≤1) without introducing a name. + +## Concrete code shapes + +### Worked acceptance example (criterion evidence) — a forgotten leg now fails fast + +The empirical evidence the criterion demands: the SimBroker harness an author +writes via the `GraphBuilder`, with the exposure leg forgotten. Today this +compiles and runs a wrong equity curve (broker slot 0 fed an empty column); after +this cycle it is rejected at compile. + +```rust +let mut g = GraphBuilder::new("root"); +let expo = g.add(Exposure::builder()); +let broker = g.add(SimBroker::builder(0.0001)); +let eq = g.add(Recorder::builder(vec![ScalarKind::F64], Firing::Any, tx_eq)); +let price = g.source_role("price", ScalarKind::F64); + +g.feed(price, [expo.input("signal"), broker.input("price")]); +// BUG: the exposure producer is never connected to broker.input("exposure"). +// (g.connect(expo.output("exposure"), broker.input("exposure")) is missing.) +g.connect(broker.output("equity"), eq.input("col[0]")); + +// Every NAME resolves (both ports exist), so the builder succeeds: +let composite = g.build().expect("all port/field names resolve"); +// The wiring-totality check fires at compile: broker's exposure slot is uncovered. +let compiled = composite.compile_with_params(¶ms); +assert!(matches!(compiled, Err(CompileError::UnconnectedPort { .. }))); +``` + +The complementary double-wire, on the **raw** `Composite::new` surface (showing the +same check guards the non-builder path) — one slot fed by an edge **and** a role +target: + +```rust +// broker input slot 1 ("price") is targeted by BOTH a role target and an edge. +let bp = Composite::new( + "root", + vec![/* ... exposure producer @0, broker @1, ... */], + vec![Edge { from: 0, to: 1, slot: 1, from_field: 0 }], // edge -> (broker, slot 1) + vec![Role { name: "price".into(), + targets: vec![Target { node: 1, slot: 1 }], // role ALSO -> (broker, slot 1) + source: Some(ScalarKind::F64) }], + vec![/* output */], +); +assert!(matches!(bp.compile_with_params(¶ms), + Err(CompileError::DoubleWiredPort { node: 1, slot: 1 }))); +``` + +### Before → after: the new check (the load-bearing change) + +New function, mirroring `check_param_namespace_injective`'s single-pass shape: + +```rust +/// Every interior node's every declared input slot must be covered by exactly one +/// wiring act — one interior edge OR one role target, counted uniformly. Zero = +/// a forgotten connection (runs a silent empty column); >1 = an ill-formed slot +/// (one slot holds one column). Index-based, name-free; presupposes in-range +/// edge/role indices (run after the existing index-range checks). +fn check_ports_connected( + nodes: &[BlueprintNode], + edges: &[Edge], + roles: &[Role], +) -> Result<(), CompileError> { + let mut coverage: HashMap<(usize, usize), usize> = HashMap::new(); + for e in edges { + *coverage.entry((e.to, e.slot)).or_insert(0) += 1; + } + for role in roles { + for t in &role.targets { + *coverage.entry((t.node, t.slot)).or_insert(0) += 1; + } + } + for (n, item) in nodes.iter().enumerate() { + for slot in 0..item.signature().inputs.len() { + match coverage.get(&(n, slot)).copied().unwrap_or(0) { + 1 => {} + 0 => return Err(CompileError::UnconnectedPort { node: n, slot }), + _ => return Err(CompileError::DoubleWiredPort { node: n, slot }), + } + } + } + Ok(()) +} +``` + +Call site, inside `validate_wiring` — after the existing edge/role/output +index-range + kind checks (so all indices are known in-range) and before the +nested-composite recursion, so it runs once per level: + +```rust +fn validate_wiring(nodes, edges, roles, output) -> Result<(), CompileError> { + // ... existing edge index+kind checks (blueprint.rs:506-517) ... + // ... existing role target index+kind checks (blueprint.rs:520-531) ... + // ... existing output re-export index checks (blueprint.rs:533-538) ... + + check_ports_connected(nodes, edges, roles)?; // <-- NEW: totality, this level + + for item in nodes { // existing recursion (540-543) + if let BlueprintNode::Composite(c) = item { + validate_wiring(c.nodes(), c.edges(), c.input_roles(), c.output())?; + } + } + Ok(()) +} +``` + +### Before → after: two new `CompileError` variants + +```rust +pub enum CompileError { + // ... existing variants (BadInteriorIndex, RoleKindMismatch, ... + // DuplicateParamPath, UnboundRootRole) unchanged ... + + /// An interior node's input `slot` is covered by no edge and no role target — + /// a required port left unconnected (it would bootstrap a silent empty column). + UnconnectedPort { node: usize, slot: usize }, + /// An interior node's input `slot` is covered by more than one edge/role target + /// combined — a slot holds exactly one column, so >1 producer is ill-formed. + DoubleWiredPort { node: usize, slot: usize }, +} +``` + +## Components + +- **`check_ports_connected`** (new, `crates/aura-engine/src/blueprint.rs`) — the + coverage check above. Pure, pre-build, reads `signature()` only. +- **`validate_wiring`** (modified, blueprint.rs:496) — one new call after the + index-range checks, before the recursion. +- **`CompileError`** (modified, blueprint.rs:465) — two new variants. The enum + derives `Debug, PartialEq, Eq`; the new variants carry only `usize` fields, so + the derives still hold (no manual impl). +- **Design ledger** (`docs/design/INDEX.md`) — a small realization note under C8 + recording the new structural invariant (text below). No C23 amendment: names are + not involved. + +### Ledger amendment (under C8, after the cycle-0027 realization block) + +> **Realization (cycle 0040 — wiring totality: every input slot connected exactly +> once, #65).** The node contract's "the engine provides a window into each input" +> presupposes each input is actually fed; until now an unwired interior input slot +> was accepted and bootstrapped to a silent empty column +> (`harness.rs`), and two producers into one slot — ill-formed, since a slot holds +> one column — was likewise uncompiled-against. Cycle 0040 makes a valid graph +> **total and single-valued** over its interior input slots: `check_ports_connected` +> (in `validate_wiring`, run at every nesting level) requires every interior node's +> every declared input slot to be covered by **exactly one** wiring act — one +> interior `Edge { to, slot }` or one role `Target { node, slot }`, edges and role +> targets counted uniformly. Zero coverage is `CompileError::UnconnectedPort`, more +> than one is `CompileError::DoubleWiredPort`. A composite's **own** input roles +> (`source: None`) are coverage *providers* — the wired-by-enclosing boundary, the +> root case already guarded by `UnboundRootRole` — never consumers, so only +> interior input slots are subject to the rule. There is **no optional-input +> concept**: every declared input port is required (no shipped node runs +> meaningfully without one; an unwarmed mode-A input is *wired-but-not-yet-valued*, +> not unwired). The check is **index-based and name-free** — it touches no name +> machinery and emits nothing into the compilat, so C23 is untouched (it proves the +> existing raw-index wiring is total and single-valued). Inherited identically by +> the raw `Composite::new` path and the `GraphBuilder::build()` path (both compile +> via `compile_with_params`). + +## Data flow + +`Composite::compile_with_params` → `validate_wiring(nodes, edges, roles, output)` +→ [existing index/kind checks] → `check_ports_connected(nodes, edges, roles)` → +[recurse per nested composite] → (on Ok) `UnboundRootRole` check → `lower_items` +→ `FlatGraph`. The new check is a pure read over the same three slices +`validate_wiring` already holds; it adds no data to the compilat and no work to the +run loop. + +## Error handling + +Two new typed `CompileError` variants, each carrying the offending +`(node, slot)` (per-level interior node index — the same addressing convention +`BadInteriorIndex`-adjacent code uses). They surface from `compile_with_params` +exactly like the existing construction-phase faults. No panics, no new error +channel. When multiple slots are faulty, the first encountered (lowest node index, +then lowest slot) is returned — deterministic by iteration order; callers that +need all faults are out of scope (the existing variants are first-fault too). + +## Testing strategy + +1. **Unwired interior slot → `UnconnectedPort`.** Build the forgotten-exposure-leg + harness (the worked example above, builder surface) and a raw `Composite::new` + variant; assert `compile_with_params(¶ms)` is + `Err(CompileError::UnconnectedPort { .. })` with the expected `(node, slot)`. +2. **Double-wired slot → `DoubleWiredPort`.** The raw double-wire above (edge + + role into one slot) and an edge+edge variant; assert + `Err(CompileError::DoubleWiredPort { node, slot })`. +3. **Nested-level enforcement.** A composite whose *interior* leaves a slot unwired, + nested inside a root whose own wiring is complete; assert the nested fault still + surfaces (the recursion carries the check down). +4. **Open-role boundary stays legal.** A composite with an open input role + (`source: None`) whose target feeds an interior slot compiles when used as a + nested node with that role wired by the enclosing graph — the role-as-provider + path is not mis-flagged as an unwired slot. +5. **Regression — valid graphs still compile.** The canonical harness + (`crates/aura-engine/src/test_fixtures.rs`) and the builder harness + (`builder_harness_compiles_identically_to_hand_wired`) still compile green; the + aura-cli sample / sweep / param_space / render tests stay green. +6. **C23 regression — compilat byte-identical.** For any graph that still compiles, + the lowered `FlatGraph` is unchanged (the existing identity test covers this; the + check adds nothing to lowering). + +### Blast radius (named — fixed in this iteration, not deferred) + +Three currently-green negative tests in `crates/aura-engine/src/blueprint.rs` wrap a +single-role composite `c` in a root with **empty** `edges` and **empty** `roles` +(`vec![BlueprintNode::Composite(c)], vec![], vec![], vec![]`). That leaves the root's +one interior node (a composite exposes one derived input port per role, +blueprint.rs:76-99) **uncovered**, so `check_ports_connected` — running at the root +level before the recursion — returns `UnconnectedPort { node: 0, slot: 0 }` and +pre-empts the deeper fault each test targets: + +- `bad_interior_index_rejected` (~1372) — asserts `BadInteriorIndex` (a fault inside `c`). +- `role_kind_mismatch_rejected` (~1440) — asserts `RoleKindMismatch { role: 0 }`. +- `output_port_out_of_range_rejected` (~1463) — asserts `OutputPortOutOfRange`. + +Resolution (in this iteration): make each root wrapper **minimally valid** so the test +still reaches its intended fault — give the root a covering input `Role` whose target is +`c`'s input slot (or, where the fault under test does not require `c` to expose an input +port, drop that port). The asserted deep-fault variant is unchanged; these fixtures +merely relied incidentally on under-wiring being accepted, which this cycle makes +invalid. This is the load-bearing test maintenance a new structural invariant entails — +named here, not discovered at implement time. + +Two adjacent negative tests are **safe by ordering** and need no change: +`compile_rejects_kind_mismatch_without_building` (~2013) — the edge kind-check fires +before `check_ports_connected`; and `unbound_root_role_is_rejected` (~2042) — its root +slot is covered by its role target, and `UnboundRootRole` is checked after +`validate_wiring` returns. The fully-wired Composite-wrap tests (blueprint.rs ~1146, +1247, 1285, 1292, 1337) all carry a covering root role/edge and stay green. + +## Acceptance criteria + +- `check_ports_connected` exists in `validate_wiring`; the two `CompileError` + variants exist; both the raw `Composite::new` and the `GraphBuilder::build()` + surfaces reject an unwired and a double-wired slot (proved by a negative test on + **each** surface for at least `UnconnectedPort`, and at least one + `DoubleWiredPort` test). +- The nested-level test (3) and the open-role-boundary test (4) pass. +- The three named blast-radius tests (`bad_interior_index_rejected`, + `role_kind_mismatch_rejected`, `output_port_out_of_range_rejected`) are re-wired to a + minimally-valid root and still assert their original deep-fault variant (green). +- The full existing suite stays green (regression 5) and the compilat is + byte-identical for graphs that still compile (regression 6). +- The C8 ledger realization note is landed. +- `cargo test --workspace` green; `cargo clippy --workspace --all-targets -D + warnings` clean.