From 88667a65fd878e364182971f812f2b6c89aa1cbf Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 15 Jul 2026 20:08:33 +0200 Subject: [PATCH] audit: cycle-close tidy for #275 (by-name source binding) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #275 Drift review (architect) over ad4249f..HEAD found the cycle drift-clean at the core and confirmed what holds: C1 preserved (`run()`'s k-way merge / event loop has zero diff — `run_bound` only reorders the supply via `bind_sources` then delegates to `run`, byte-identity pinned); the C4 realization and C23 refinement ledger notes are accurate and tightly scoped (`SourceSpec.role` is read in exactly one place, sourced only from the lowering — no leak); and all three binding-carrying production run sites moved to `run_bound` + `key_supply`, every remaining `.run(` being `#[cfg(test)]`. No regression harness exists, so the architect is the sole drift gate. One medium drift item fixed: - FIX: the CLI's `.expect()` on `run_bound` covered Missing/Extra/Unnamed feed but not `DuplicateFeed`. The blueprint path does not enforce root-role-name uniqueness (only the construction op-script path does), so a duplicate-role blueprint would reach `bind_sources`' `DuplicateFeed` and panic behind the `.expect()` instead of refusing. Closed at the binding boundary: `resolve_binding` now refuses a duplicate input-role name — a named `aura:`-register refusal on every CLI run path (single run, sweep, and campaign all resolve the binding upfront), matching its existing price/close-ambiguity refusal. The `.expect()` invariant is now genuinely true on all paths. Pinned by `binding::duplicate_role_name_refuses`. Two low items: - `SourceBindError::Display` had no test (its renderer stays dead until a decoupled-supply caller lands, #124). Added `source_bind_error_display_names_the_role` pinning the message strings; reachability is deferred by design. - The reverted fieldtest `SourceSpec` sweep leaves the corpus one more revival break against the current engine API — carried, as the corpus is pre-existing bit-rot unrelated to this cycle (documented in 9e30805 and on #275). Suite: `cargo test --workspace` green (1321 passed, 0 failed); `cargo clippy --workspace --all-targets -- -D warnings` clean. Spec + plan (git-ignored working files) discarded. --- crates/aura-cli/src/binding.rs | 37 +++++++++++++++++++++++++++++++ crates/aura-engine/src/harness.rs | 14 ++++++++++++ 2 files changed, 51 insertions(+) diff --git a/crates/aura-cli/src/binding.rs b/crates/aura-cli/src/binding.rs index ec24a28..88fb621 100644 --- a/crates/aura-cli/src/binding.rs +++ b/crates/aura-cli/src/binding.rs @@ -166,6 +166,17 @@ pub(crate) fn resolve_binding( \"price\" is the alias of \"close\", so the pair is ambiguous; rename one role" )); } + // Duplicate role names would lower to duplicate keyed feeds, which the by-name + // bind (`run_bound`) rejects as `DuplicateFeed`. The blueprint path does not + // enforce role-name uniqueness (only the construction op-script path does), so + // refuse here — a named refusal on every CLI run path, not a panic behind the + // `.expect()` the single-binding invariant relies on. + if let Some(dup) = first_duplicate_role(roles) { + return Err(format!( + "strategy \"{strategy}\" declares input role \"{dup}\" more than once — \ + input role names must be unique" + )); + } let mut entries: Vec = Vec::with_capacity(roles.len() + 1); for role in roles { let column = match overrides.get(&role.name) { @@ -200,6 +211,18 @@ pub(crate) fn resolve_binding( Ok(finish(entries)) } +/// The first role name that appears more than once in `roles`, in declaration +/// order, or `None` if every name is unique. +fn first_duplicate_role(roles: &[Role]) -> Option<&str> { + let mut seen = std::collections::BTreeSet::new(); + for r in roles { + if !seen.insert(r.name.as_str()) { + return Some(r.name.as_str()); + } + } + None +} + /// The lenient PROBE binding for the throwaway param-space wrap /// (`blueprint_axis_probe`): every unresolvable role falls back to the close /// column. The probe composite is built but never run over data — the @@ -358,6 +381,20 @@ mod tests { ); } + /// Two input roles with the same name would lower to a duplicate keyed feed; + /// `resolve_binding` refuses at the binding boundary (a named refusal, not a + /// `run_bound` `DuplicateFeed` panic behind the CLI's `.expect()`). + #[test] + fn duplicate_role_name_refuses() { + let err = + resolve_binding("x", &[role("high"), role("high")], &BTreeMap::new()).unwrap_err(); + assert_eq!( + err, + "strategy \"x\" declares input role \"high\" more than once — \ + input role names must be unique" + ); + } + /// A role literally named "close" overridden away from the close column /// would otherwise collide with `finish`'s synthesized close entry (two /// entries both named "close") — refused instead of double-naming. diff --git a/crates/aura-engine/src/harness.rs b/crates/aura-engine/src/harness.rs index 85b70d1..a40dbc1 100644 --- a/crates/aura-engine/src/harness.rs +++ b/crates/aura-engine/src/harness.rs @@ -758,6 +758,20 @@ mod tests { ); } + /// The named-error prose a keyed-supply caller renders (the by-name refusal + /// surface for decoupled-supply callers, #124/#275). + #[test] + fn source_bind_error_display_names_the_role() { + assert_eq!( + SourceBindError::MissingFeed { role: "close".into() }.to_string(), + "the graph declares source role \"close\" but the supply provides no such feed" + ); + assert_eq!( + SourceBindError::DuplicateFeed { role: "high".into() }.to_string(), + "the supply provides feed \"high\" more than once" + ); + } + #[test] fn window_of_folds_union_extent_across_sources() { let a: Box = Box::new(VecSource::new(f64_stream(&[(10, 1.0), (40, 2.0)])));