diff --git a/docs/plans/0020-composite-signature-render.md b/docs/plans/0020-composite-signature-render.md new file mode 100644 index 0000000..16fc2e9 --- /dev/null +++ b/docs/plans/0020-composite-signature-render.md @@ -0,0 +1,380 @@ +# Composite Signature Render — Implementation Plan + +> **Parent spec:** `docs/specs/0020-composite-signature-render.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run +> this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Render a composite's blueprint definition as a typed signature title line +(`macd(fast:i64, slow:i64, signal:i64) -> (macd, signal, histogram)`) with param +names folded into leaf labels (`[EMA(fast)]`), ordered input slots as stubs +(`[Sub(#A,#B)]`), de-prefixed outputs (`[macd]`), and the `[param:*]` marker nodes +removed. + +**Architecture:** Render-only. Two new helpers (`signature`, `leaf_label`) in +`graph.rs` + edits to `render_definition`; a `ScalarKind`→lowercase match (none +exists today); test/golden updates in `main.rs`; two stale doc-comment fixes. No +engine / `ParamAlias` / `param_space()` change. Task 1 changes only render code and +gates on `cargo build -p aura-cli` (the goldens go red and are re-captured in Task +2 — `cargo test` is NOT a Task-1 gate). Task 2 fixes the tests and runs the full +workspace triple. + +**Tech Stack:** `crates/aura-cli/src/graph.rs`, `crates/aura-cli/src/main.rs`, +`crates/aura-core/src/node.rs` (doc comment). + +--- + +**Files this plan creates or modifies:** + +- Modify: `crates/aura-cli/src/graph.rs:16-18` (imports), `:101-137` (`render_definition` + doc), plus two new helpers +- Modify: `crates/aura-core/src/node.rs:93-101` (doc comment only) +- Modify: `crates/aura-cli/src/main.rs:393,394,428,429,452,581` (pins/needles), `:580-587` (MACD render test), `:483-521` (`blueprint_view_golden` re-capture) + +--- + +## Task 1: graph.rs render change + doc fixes + +**Files:** +- Modify: `crates/aura-cli/src/graph.rs:16-18,101-137` +- Modify: `crates/aura-core/src/node.rs:93-101` + +- [ ] **Step 1: Add `ScalarKind` and `LeafFactory` to graph.rs imports** + +`graph.rs:17`. Replace: + +```rust +use aura_core::Node; +``` + +with: + +```rust +use aura_core::{LeafFactory, Node, ScalarKind}; +``` + +- [ ] **Step 2: Add the `signature` + `kind_str` helpers** + +Insert immediately before `render_definition` (currently `graph.rs:106`, after the +doc comment is rewritten in Step 5 — insert these fns just above that doc comment at +`:101`): + +```rust +/// `ScalarKind` as a lowercase type string for a signature (`i64`/`f64`/`bool`/ +/// `timestamp`). The derived `Debug` gives PascalCase (`I64`), so this is explicit. +fn kind_str(kind: ScalarKind) -> &'static str { + match kind { + ScalarKind::I64 => "i64", + ScalarKind::F64 => "f64", + ScalarKind::Bool => "bool", + ScalarKind::Timestamp => "timestamp", + } +} + +/// The composite's typed signature for the definition title: +/// `name(p1:kind, …) -> (o1, …)`. Param kinds come from the aliased interior leaf's +/// declared params; output **names only** (kinds need a pre-build factory interface, +/// #43). An empty alias list renders `name()`. Total: a malformed alias falls back +/// to `?` rather than panicking (compile is the validator, #41). +fn signature(c: &Composite) -> String { + let params: Vec = c + .params() + .iter() + .map(|a| { + let kind = c + .nodes() + .get(a.node) + .and_then(|n| match n { + BlueprintNode::Leaf(f) => f.params().get(a.slot).map(|p| kind_str(p.kind)), + BlueprintNode::Composite(_) => None, + }) + .unwrap_or("?"); + format!("{}:{}", a.name, kind) + }) + .collect(); + let outs: Vec = c.output().iter().map(|of| of.name.clone()).collect(); + format!("{}({}) -> ({})", c.name(), params.join(", "), outs.join(", ")) +} +``` + +- [ ] **Step 3: Add the `leaf_label` helper** + +Insert directly after `signature` (above the `render_definition` doc comment): + +```rust +/// A leaf item's render label: `factory.label()` plus an optional `(...)` listing +/// its aliased param names, then its input-slot stubs (`#A`, `#B`, … one per wired +/// input slot, slot index → letter) when the leaf has more than one wired input +/// slot. Params and stubs are `; `-separated when both present; either alone has no +/// separator; neither yields the bare label. A node's wired slots are the distinct +/// `.slot` values targeting it across interior edges (`Edge.to == index`) and input +/// roles (`Role.targets` with `node == index`). +fn leaf_label(c: &Composite, index: usize, factory: &LeafFactory) -> String { + let params: Vec<&str> = c + .params() + .iter() + .filter(|a| a.node == index) + .map(|a| a.name.as_str()) + .collect(); + + let mut slots: Vec = Vec::new(); + for e in c.edges() { + if e.to == index && !slots.contains(&e.slot) { + slots.push(e.slot); + } + } + for role in c.input_roles() { + for t in &role.targets { + if t.node == index && !slots.contains(&t.slot) { + slots.push(t.slot); + } + } + } + slots.sort_unstable(); + let stubs: Vec = if slots.len() > 1 { + slots.iter().map(|s| format!("#{}", (b'A' + *s as u8) as char)).collect() + } else { + Vec::new() + }; + + let parts: Vec = match (params.is_empty(), stubs.is_empty()) { + (true, true) => return factory.label(), + (false, true) => vec![params.join(", ")], + (true, false) => vec![stubs.join(", ")], + (false, false) => vec![params.join(", "), stubs.join(", ")], + }; + format!("{}({})", factory.label(), parts.join("; ")) +} +``` + +- [ ] **Step 4: Edit `render_definition` — title, leaf labels, drop param markers, de-prefix outputs** + +`graph.rs:107-136`. Replace the leaf-label loop, the `[param:*]` loop, the output +loop, and the title. + +Replace `:108-113` (the leaf loop): + +```rust + for inner in c.nodes() { + labels.push(match inner { + BlueprintNode::Leaf(factory) => factory.label(), + BlueprintNode::Composite(inner_c) => inner_c.name().to_string(), + }); + } +``` + +with (enumerate, fold via `leaf_label`): + +```rust + for (i, inner) in c.nodes().iter().enumerate() { + labels.push(match inner { + BlueprintNode::Leaf(factory) => leaf_label(c, i, factory), + BlueprintNode::Composite(inner_c) => inner_c.name().to_string(), + }); + } +``` + +Delete the `[param:*]` marker loop `:125-129` entirely: + +```rust + for a in c.params() { + let p_id = labels.len(); + labels.push(format!("param:{}", a.name)); + edges.push((p_id, a.node)); + } +``` + +Replace the output marker label `:132`: + +```rust + labels.push(format!("out:{}", of.name)); +``` + +with (drop the `out:` prefix): + +```rust + labels.push(of.name.clone()); +``` + +Replace the title `:136`: + +```rust + format!("{}:\n{}", c.name(), render_flat(&labels, &edges)) +``` + +with: + +```rust + format!("{}:\n{}", signature(c), render_flat(&labels, &edges)) +``` + +- [ ] **Step 5: Fix the stale `render_definition` doc comment** + +`graph.rs:101-105`. Replace: + +```rust +/// Render one composite's interior as a flat graph: interior leaves as `[type]`, +/// nested composites as opaque `[name]`, plus an `[in:]` entry marker per +/// input role (wired to its interior targets), a `[param:]` marker per param +/// alias (wired to the leaf it relabels), and an `[out:]` marker per +/// re-exported output field (wired from its producer). Prefixed `":\n"`. +``` + +with: + +```rust +/// Render one composite's interior as a flat graph: interior leaves as +/// `[type(param…; #slot…)]` (aliased param names + ordered input-slot stubs folded +/// in via `leaf_label`), nested composites as opaque `[name]`, an `[in:]` +/// entry marker per input role (wired to its interior targets), and an `[]` +/// node per re-exported output field (wired from its producer). The title line is +/// the composite's typed `signature` (`name(p:kind, …) -> (out, …)`); params live +/// in the signature, not as marker nodes. +``` + +- [ ] **Step 6: Fix the stale `LeafFactory::label` doc comment** + +`crates/aura-core/src/node.rs:93-101`. Replace: + +```rust + /// The param-generic render label for the blueprint view (C22 "structure + /// before"): just the node type, e.g. `SMA`. A value-empty recipe has no + /// values to show; the tunable knobs are surfaced by `Blueprint::param_space`, + /// not in the graph. The label stays the bare type because the `ascii-dag` + /// renderer writes a label verbatim on one line (no wrapping) and overlaps two + /// wide sibling boxes inside a cluster subgraph — appending the knob names + /// (`SMA(length)`) would garble the blueprint view, and domain labels grow + /// unboundedly wide. The compiled view labels the built node valued (`SMA(2)`) + /// via `Node::label`, unaffected. +``` + +with: + +```rust + /// The param-generic render label for the blueprint view (C22 "structure + /// before"): just the node type, e.g. `SMA`. A value-empty recipe has no values + /// to show; the tunable knobs are surfaced by `Blueprint::param_space`. The + /// factory label stays the bare type because alias / handle names are a + /// *composite-level* concept — the renderer folds them into the leaf label at + /// the composite boundary (`render_definition`'s `leaf_label`), where the + /// `(node, slot)` → name mapping lives, not on the standalone factory. (Wide + /// labels are safe: both `aura graph` views are flat since cycle 0017 — no + /// cluster subgraph, so no sibling-overlap garble.) The compiled view labels the + /// built node valued (`SMA(2)`) via `Node::label`, unaffected. +``` + +- [ ] **Step 7: Build the CLI crate (compile gate; tests deferred to Task 2)** + +Run: `cargo build -p aura-cli` +Expected: PASS — the render code compiles with the two new helpers, the imports, and +the edited `render_definition`. (`cargo test` is NOT run here: the goldens/needle +assertions are now stale and go red until Task 2 re-captures them. `cargo build` +does not compile the `#[cfg(test)]` module, so this gate is a clean compile check of +the render path.) + +--- + +## Task 2: main.rs tests + goldens + workspace gate + +**Files:** +- Modify: `crates/aura-cli/src/main.rs:393,394,428,429,452` (pins/needles), `:580-587` (MACD render test), `:483-521` (`blueprint_view_golden`) + +- [ ] **Step 1: Update the four non-MACD title pins (`name:` → `name(`)** + +The title format changed from `name:` to `name(…) -> (…):`, so every +`.matches(":")` count assert must repin to `"("` (the new title is the +only place `name(` occurs; opaque main nodes are `[name]`, no paren). + +`main.rs:393` — replace: + +```rust + assert_eq!(out.matches("sma_cross:").count(), 1, "definition not rendered once:\n{out}"); +``` + +with: + +```rust + assert_eq!(out.matches("sma_cross(").count(), 1, "definition not rendered once:\n{out}"); +``` + +`main.rs:428` — replace `out.matches("outer:")` with `out.matches("outer(")` (keep +the rest of the line identical). `main.rs:429` — replace `out.matches("inner:")` +with `out.matches("inner(")`. `main.rs:452` — replace `out.matches("dup:")` with +`out.matches("dup(")`. + +- [ ] **Step 2: Update the definition needle array** + +`main.rs:394`. Replace: + +```rust + for needle in ["[SMA]", "[Sub]", "[in:price]", "[out:cross]"] { +``` + +with (the Sub now shows its two ordered inputs; the output marker is de-prefixed): + +```rust + for needle in ["[SMA]", "[Sub(#A,#B)]", "[in:price]", "[cross]"] { +``` + +- [ ] **Step 3: Rewrite the MACD definition render assertions** + +`main.rs:580-587`. Replace: + +```rust + assert!(out.contains("[macd]"), "missing opaque macd node:\n{out}"); + assert_eq!(out.matches("macd:").count(), 1, "macd defined once:\n{out}"); + assert!(out.contains("[EMA]"), "macd interior must show EMA leaves:\n{out}"); + assert!(out.contains("[in:price]"), "named MACD input role: {out}"); + assert!(out.contains("[param:fast]"), "aliased fast length: {out}"); + assert!(out.contains("[param:slow]"), "aliased slow length: {out}"); + assert!(out.contains("[param:signal]"), "aliased signal length: {out}"); +``` + +with: + +```rust + assert!(out.contains("[macd]"), "missing opaque macd node:\n{out}"); + assert_eq!(out.matches("macd(").count(), 1, "macd defined once:\n{out}"); + assert!( + out.contains("macd(fast:i64, slow:i64, signal:i64) -> (macd, signal, histogram)"), + "typed signature line: {out}" + ); + assert!(out.contains("[EMA(fast)]"), "fast EMA folds its param: {out}"); + assert!(out.contains("[EMA(slow)]"), "slow EMA folds its param: {out}"); + assert!(out.contains("[EMA(signal)]"), "signal EMA folds its param: {out}"); + assert!(out.contains("[Sub(#A,#B)]"), "Sub shows its two ordered inputs: {out}"); + assert!(out.contains("[in:price]"), "named MACD input role: {out}"); + assert!(!out.contains("[param:"), "param marker nodes removed: {out}"); + assert!(!out.contains("[out:"), "output prefix dropped: {out}"); +``` + +- [ ] **Step 4: Re-capture `blueprint_view_golden` wholesale** + +Run: `cargo test -p aura-cli blueprint_view_golden` +Expected: FAIL — the `where:` definition section drifted (`sma_cross:` → +`sma_cross() -> (cross):`, `[Sub]` → `[Sub(#A,#B)]`, `[out:cross]` → `[cross]`). +Read the assertion's "left" (actual) rendered string and replace the entire +`expected` raw-string literal (`main.rs:488-519`) with the actual bytes verbatim — +do NOT hand-edit field-by-field. Re-run: PASS. + +- [ ] **Step 5: Confirm `compiled_view_golden` byte-identical (C23 guard)** + +Run: `cargo test -p aura-cli compiled_view_golden` +Expected: PASS with NO edit to the golden (`main.rs:529-547`). The compiled view does +not render a composite definition, so it must be untouched. If it drifts, STOP — that +is a bug, not a re-capture. + +- [ ] **Step 6: Full workspace triple (-D warnings)** + +Run: `cargo build --workspace && cargo test --workspace && cargo clippy --workspace --all-targets -- -D warnings` +Expected: all PASS — every test green (the five repins, the needle update, the MACD +assertions, both goldens), no clippy warnings. Spot-check the acceptance evidence: + +Run: `cargo run -q -p aura-cli -- graph --macd` +Expected: the definition title reads +`macd(fast:i64, slow:i64, signal:i64) -> (macd, signal, histogram):`, the EMAs read +`[EMA(fast)]`/`[EMA(slow)]`/`[EMA(signal)]`, the Subs read `[Sub(#A,#B)]`, outputs +read `[macd]`/`[signal]`/`[histogram]`, and no `[param:*]` marker appears. + +Run: `cargo run -q -p aura-cli -- run --macd` +Expected: deterministic JSON, `total_pips` and `exposure_sign_flips` unchanged from +before (render-only change does not touch the run).