plan: 0089 construction diagnostics polish (refs #162)
Two tasks: item 1 (by-identifier finalize) lands atomically across both crates — three OpError variants + finish() translation + the CLI presenter arms + five test touches (two of them E2E twins plan-recon found that the spec's testing strategy under-enumerated: construction_e2e.rs and tests/graph_construct.rs); items 2 & 3 (bind-mismatch prose + discoverable bind form) are additive CLI changes.
This commit is contained in:
@@ -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 {{\"{:?}\": <v>}})\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": <v>}` 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\": <v>})"), "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).
|
||||
Reference in New Issue
Block a user