diff --git a/docs/plans/0089-construction-diagnostics-polish.md b/docs/plans/0089-construction-diagnostics-polish.md new file mode 100644 index 0000000..8571125 --- /dev/null +++ b/docs/plans/0089-construction-diagnostics-polish.md @@ -0,0 +1,276 @@ +# Construction diagnostics polish (#162) — Implementation Plan + +> **Parent spec:** `docs/specs/0089-construction-diagnostics-polish.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run +> this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Make the construction op-script surface read by-identifier and as prose +at the three places it currently leaks raw machine indices or Debug structs — +presentation only, no fault is added, suppressed, or re-attributed. + +**Architecture:** Item 1 (by-identifier finalize) spans both crates atomically: +adding `OpError` variants forces the exhaustive `format_op_error` match to gain +arms (compile gate), and the `finish()` translation flips four behavioural test +pins across both crates (test-run gate) — so all of item 1 + its five test touches +land in Task 1. Items 2 & 3 are additive CLI-presenter changes with no existing-test +breakage (Task 2). + +**Tech Stack:** `aura-engine` (`construction.rs` — `OpError`, `GraphSession::finish`), +`aura-cli` (`graph_construct.rs` — `format_op_error`, `introspect_node`), with +`CompileError`/`Role` in `aura-engine/src/blueprint.rs` and `BindOpError` re-exported +at `aura-engine/src/lib.rs:90`. + +**Files this plan creates or modifies:** + +- Modify: `crates/aura-engine/src/construction.rs:44-71` — three new by-identifier `OpError` variants +- Modify: `crates/aura-engine/src/construction.rs:306-317` — `finish()` translates the index-carrying `CompileError`s +- Modify: `crates/aura-engine/src/construction.rs:540-551` — update `finish_reports_incomplete_on_unwired_slot` +- Test: `crates/aura-engine/src/construction.rs` — new `finish_reports_unbound_input_root_role_by_identifier` +- Modify: `crates/aura-cli/src/graph_construct.rs:71-92` — three item-1 presenter arms in `format_op_error` +- Modify: `crates/aura-cli/src/graph_construct.rs:259-273` — update `build_from_str_reports_unconnected_slot_at_finalize` +- Modify: `crates/aura-engine/tests/construction_e2e.rs:114-131` — update `replay_attributes_holistic_fault_to_finalize_step` (recon-found twin) +- Modify: `crates/aura-cli/tests/graph_construct.rs:114` — update `graph_build_reports_a_holistic_fault_at_finalize` (recon-found twin) +- Modify: `crates/aura-cli/src/graph_construct.rs:89` — `BadParam` arm as prose (item 2) +- Modify: `crates/aura-cli/src/graph_construct.rs:146` — `introspect_node` param line bind hint (item 3) +- Modify: `crates/aura-cli/src/graph_construct.rs:9` — add `BindOpError` to the import +- Test: `crates/aura-cli/src/graph_construct.rs` — two new tests (item 2, item 3) + +--- + +## Task 1: Item 1 — by-identifier finalize faults (engine variants + finish + CLI arms + 5 test touches) + +**Files:** +- Modify: `crates/aura-engine/src/construction.rs:44-71` (OpError), `:306-317` (finish), `:540-551` (test) +- Modify: `crates/aura-cli/src/graph_construct.rs:71-92` (format_op_error arms) +- Modify: `crates/aura-engine/tests/construction_e2e.rs:114-131` +- Modify: `crates/aura-cli/tests/graph_construct.rs:114` + +- [ ] **Step 1: Add the three by-identifier `OpError` variants** + +In `crates/aura-engine/src/construction.rs`, insert between `BadParam { … }` (`:67`) +and `Incomplete(CompileError)` (`:70`): + +```rust + /// Holistic totality fault, by-identifier: an interior input slot covered by + /// no edge or role target (the finalize 0-cover arm). + UnconnectedPort { node: String, slot: String }, + /// Holistic role-kind fault, by-identifier: a root input role fans into slots + /// of differing scalar kinds. + RoleKindMismatch { role: String }, + /// Holistic root fault, by-identifier: a root input role has no bound source. + UnboundRootRole { role: String }, +``` + +- [ ] **Step 2: Translate the index-carrying CompileErrors in `finish()`** + +In `construction.rs`, replace the three holistic-gate `.map_err` calls in +`finish()` (`:313-315`). The gate *calls* are unchanged (C24: no second validator); +only the `.map_err` closures change. The stale doc comment at `:302-305` ("Any +fault wraps as `OpError::Incomplete`") must be updated to note the by-identifier +translation. Replace the body from `check_param_namespace_injective` through +`Ok(c)`: + +```rust + check_param_namespace_injective(&c.param_space()).map_err(OpError::Incomplete)?; + validate_wiring(c.nodes(), c.edges(), c.input_roles(), c.output()).map_err(|e| match e { + CompileError::UnconnectedPort { node, slot } => OpError::UnconnectedPort { + node: self.ids[node].clone(), + slot: self.schemas[node].inputs[slot].name.clone(), + }, + CompileError::RoleKindMismatch { role } => OpError::RoleKindMismatch { + role: c.input_roles()[role].name.clone(), + }, + other => OpError::Incomplete(other), + })?; + check_root_roles_bound(c.input_roles()).map_err(|e| match e { + CompileError::UnboundRootRole { role } => OpError::UnboundRootRole { + role: c.input_roles()[role].name.clone(), + }, + other => OpError::Incomplete(other), + })?; + Ok(c) +``` + +(`self.ids` / `self.schemas` are not moved by `Composite::new` and stay borrowable; +`c.input_roles()` indexes the role by its finalize index.) + +- [ ] **Step 3: Add the three presenter arms in `format_op_error`** + +In `crates/aura-cli/src/graph_construct.rs`, insert before the `Incomplete(ce)` arm +(`:90`). This is the compile-gate co-land — the new variants make the match +non-exhaustive until these arms exist: + +```rust + OpError::UnconnectedPort { node, slot } => format!("slot {node}.{slot} is unconnected"), + OpError::RoleKindMismatch { role } => format!("input role {role} fans into slots of differing kinds"), + OpError::UnboundRootRole { role } => format!("root input role {role} is unbound"), +``` + +- [ ] **Step 4: Build the workspace (compile gate)** + +Run: `cargo build --workspace 2>&1 | tail -3` +Expected: `Finished` with no `error[E0004]` (the `format_op_error` match is exhaustive again). + +- [ ] **Step 5: Update the in-file engine test `finish_reports_incomplete_on_unwired_slot`** + +In `construction.rs`, the assertion at `:550` currently reads +`assert!(matches!(err, OpError::Incomplete(CompileError::UnconnectedPort { .. })));`. +Replace it (and the test's doc/name stays, but the matched variant is now the +by-identifier one) with: + +```rust + let err = s.finish().err().unwrap(); + assert!(matches!(&err, OpError::UnconnectedPort { node, slot } if node == "sub" && slot == "rhs"), + "finalize names the open slot by-identifier, got {err:?}"); +``` + +- [ ] **Step 6: Add the new unbound-Input-root-role finish test** + +In `construction.rs`'s `#[cfg(test)] mod tests`, add (mirror +`input_role_reserved_and_duplicate_rejected` `:485` for reserving an `Input` role +and `finish_reports_incomplete_on_unwired_slot` `:540` for the finish-error shape; +use the same `session(...)` helper the sibling tests use): + +```rust + /// A reserved `Input` root role that is fed but never source-bound finishes with + /// the by-identifier `UnboundRootRole` (not a raw role index). + #[test] + fn finish_reports_unbound_input_root_role_by_identifier() { + let mut s = session("g"); + s.apply(Op::Input { role: "ext".into() }).unwrap(); + s.apply(Op::Add { type_id: "Sub".into(), as_name: Some("sub".into()), bind: vec![] }).unwrap(); + s.apply(Op::Feed { role: "ext".into(), into: vec!["sub.lhs".into(), "sub.rhs".into()] }).unwrap(); + s.apply(Op::Expose { from: "sub.value".into(), as_name: "out".into() }).unwrap(); + let err = s.finish().err().unwrap(); + assert!(matches!(&err, OpError::UnboundRootRole { role } if role == "ext"), + "an unbound Input root role finishes by-identifier, got {err:?}"); + } +``` + +- [ ] **Step 7: Update the engine E2E twin (recon-found)** + +In `crates/aura-engine/tests/construction_e2e.rs`, the assertion at `:131` in +`replay_attributes_holistic_fault_to_finalize_step` matches +`(i, OpError::Incomplete(CompileError::UnconnectedPort { .. })) if i == finalize_index`. +The finalize attribution (`i == finalize_index`) is unchanged; only the wrapped +variant changes. Replace the matched pattern's variant with the by-identifier one, +keeping the index check: + +```rust + assert!(matches!(&err, (i, OpError::UnconnectedPort { .. }) if *i == finalize_index), + "the holistic fault is still attributed to the finalize step, got {err:?}"); +``` + +(Read the test's surrounding lines `:114-131` first to keep the `finalize_index` +binding and any `CompileError` import that becomes unused — drop a now-unused +`CompileError` import if clippy flags it.) + +- [ ] **Step 8: Update the CLI E2E twin (recon-found)** + +In `crates/aura-cli/tests/graph_construct.rs`, the assertion at `:114` in +`graph_build_reports_a_holistic_fault_at_finalize` checks +`stderr.contains("Unconnected")`. The CLI now renders `finalize: slot sub.rhs is +unconnected` (lowercase, by-identifier). Replace that assertion with: + +```rust + assert!(stderr.contains("finalize: slot") && stderr.contains("is unconnected"), + "the holistic fault renders by-identifier, got: {stderr}"); +``` + +(Read the `UNCONNECTED_DOC` const at `:44` to confirm which slot it leaves open; if +that doc leaves `sub.rhs` open the message will be `slot sub.rhs is unconnected`, but +the assertion above is slot-agnostic so it holds regardless.) + +- [ ] **Step 9: Run the full suite + clippy (task gate)** + +Run: `cargo test --workspace 2>&1 | grep -c 'test result: FAILED'` +Expected: `0` + +Run: `cargo clippy --workspace --all-targets -- -D warnings 2>&1 | grep -E 'error|warning|Finished'` +Expected: `Finished` (no error/warning lines). + +--- + +## Task 2: Items 2 & 3 — bind-mismatch prose + discoverable bind form (CLI, additive) + +**Files:** +- Modify: `crates/aura-cli/src/graph_construct.rs:9` (import), `:89` (BadParam arm), `:146` (introspect param line) +- Test: `crates/aura-cli/src/graph_construct.rs` — two new tests + +- [ ] **Step 1: Add `BindOpError` to the import** + +In `crates/aura-cli/src/graph_construct.rs:9`, extend the import (re-export confirmed +at `aura-engine/src/lib.rs:90`): + +```rust +use aura_engine::{blueprint_to_json, replay, BindOpError, GraphSession, Op, OpError, Scalar, ScalarKind}; +``` + +- [ ] **Step 2: Rewrite the `BadParam` arm as prose (item 2)** + +In `format_op_error`, replace the `BadParam` arm at `:89` +(`OpError::BadParam { node, err } => format!("bad param on {node}: {err:?}"),`) with: + +```rust + OpError::BadParam { node, err } => match err { + BindOpError::UnknownParam(p) => format!("node {node} has no param {p:?}"), + BindOpError::AmbiguousParam(p) => format!("node {node} has ambiguous param {p:?}"), + BindOpError::KindMismatch { param, expected, got } => { + format!("param {node}.{param} expects {expected:?} but got {got:?}") + } + }, +``` + +- [ ] **Step 3: Add the bind-form hint to `introspect_node` (item 3)** + +In `introspect_node`, replace the param line at `:146` +(`out.push_str(&format!(" param {}:{:?}\n", p.name, p.kind));`) with: + +```rust + out.push_str(&format!(" param {}:{:?} (bind {{\"{:?}\": }})\n", p.name, p.kind, p.kind)); +``` + +- [ ] **Step 4: Build (compile check)** + +Run: `cargo build -p aura-cli 2>&1 | tail -2` +Expected: `Finished` (no errors). + +- [ ] **Step 5: Add the two CLI tests** + +In `graph_construct.rs`'s `#[cfg(test)] mod tests`, add: + +```rust + /// A bind whose value-kind mismatches the param reads as prose, like the connect + /// mismatch — not a Debug struct (item 2). + #[test] + fn build_from_str_bad_bind_kind_reads_as_prose() { + let doc = r#"[{"op":"add","type":"SMA","as":"fast","bind":{"length":{"F64":2.0}}}]"#; + let err = super::build_from_str(doc).unwrap_err(); + assert_eq!(err, "op 0 (add): param fast.length expects I64 but got F64"); + } + + /// `introspect --node` shows the typed-Scalar bind-value form so a hand-author + /// does not have to read source to learn the `{"I64": }` wrapping (item 3). + #[test] + fn introspect_node_shows_the_bind_form() { + let out = super::introspect_node("SMA").expect("SMA is in the vocabulary"); + assert!(out.contains("(bind {\"I64\": })"), "shows the bind-value form: {out}"); + } +``` + +- [ ] **Step 6: Run the two new tests** + +Run: `cargo test -p aura-cli build_from_str_bad_bind_kind_reads_as_prose 2>&1 | grep -E 'test result|FAILED'` +Expected: `test result: ok. 1 passed` + +Run: `cargo test -p aura-cli introspect_node_shows_the_bind_form 2>&1 | grep -E 'test result|FAILED'` +Expected: `test result: ok. 1 passed` + +- [ ] **Step 7: Full suite + clippy (task gate)** + +Run: `cargo test --workspace 2>&1 | grep -c 'test result: FAILED'` +Expected: `0` + +Run: `cargo clippy --workspace --all-targets -- -D warnings 2>&1 | grep -E 'error|warning|Finished'` +Expected: `Finished` (no error/warning lines).