77ddbeb237
Six tasks for the node-name-in-graph-model cycle: (1) aura-core instance_name() accessor + re-grounded named() doc; (2) aura-engine prim_record conditional "name" field + unit test; (3) re-capture both golden twins (inline model_golden + sample-model.json); (4) viewer render guard (RED, headless node guard); (5) viewer adaptNodes/genDot/cellLabel name prefix (GREEN); (6) workspace build/test/clippy gate. refs #58
476 lines
18 KiB
Markdown
476 lines
18 KiB
Markdown
# Node instance name in the graph model — Implementation Plan
|
|
|
|
> **Parent spec:** `docs/specs/0035-node-name-in-graph-model.md`
|
|
>
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run
|
|
> this plan. Steps use `- [ ]` checkboxes for tracking.
|
|
|
|
**Goal:** Surface a node's explicit instance name in the `aura graph` viewer as
|
|
`fast: SMA[length]` — a conditional `"name"` field in the JSON graph model
|
|
(engine half) and a `cellLabel` `name: ` prefix in graph-viewer.js (viewer half),
|
|
for explicitly `.named()` nodes only.
|
|
|
|
**Architecture:** The name crosses two surfaces meeting at the JSON model. Engine:
|
|
a new raw `PrimitiveBuilder::instance_name() -> Option<&str>` (the explicit name,
|
|
default *not* resolved) feeds a conditional leading `"name"` field in
|
|
`prim_record`; both golden twins are re-captured. Viewer: `adaptNodes` carries
|
|
`name` onto the per-node object, the `genDot` leaf-emit forwards it into
|
|
`cellLabel`, and `cellLabel` prepends a `name: ` declaration prefix when present.
|
|
The name is render/model-only (dropped at lowering, C23); the model stays
|
|
deterministic (C14).
|
|
|
|
**Tech Stack:** `crates/aura-core` (PrimitiveBuilder), `crates/aura-engine`
|
|
(graph_model.rs emitter + goldens), `crates/aura-cli` (graph-viewer.js +
|
|
fixture + headless node guard).
|
|
|
|
---
|
|
|
|
**Files this plan creates or modifies:**
|
|
|
|
- Modify: `crates/aura-core/src/node.rs` — add `instance_name()` accessor after `node_name()` (~line 110); re-ground `named()` doc (~line 98); add accessor unit test in `mod tests` (after ~line 290)
|
|
- Modify: `crates/aura-engine/src/graph_model.rs` — `prim_record` conditional leading `"name"` (lines 72-83); new `prim_record` unit test (after line 373); re-capture inline `model_golden` (line 429)
|
|
- Modify: `crates/aura-cli/tests/fixtures/sample-model.json:1` — `+"name"` on the two SMA legs
|
|
- Modify: `crates/aura-cli/assets/graph-viewer.js` — `adaptNodes` (line 32), `genDot` leaf-emit (line 108), `cellLabel` head (line 74)
|
|
- Create: `crates/aura-cli/tests/viewer_name_prefix.mjs` — headless render guard (named leaf → prefix; unnamed leaf → bare)
|
|
- Create: `crates/aura-cli/tests/viewer_name_prefix.rs` — `.rs` wrapper shelling `node` (mirrors `viewer_dot.rs`)
|
|
|
|
---
|
|
|
|
### Task 1: aura-core — `instance_name()` accessor + re-grounded `named()` doc
|
|
|
|
**Files:**
|
|
- Modify: `crates/aura-core/src/node.rs` (`named()` doc ~98; new accessor after `node_name()` ~110; test in `mod tests` after ~290)
|
|
|
|
- [ ] **Step 1: Write the failing accessor test**
|
|
|
|
In `crates/aura-core/src/node.rs`, inside `mod tests`, directly after the
|
|
`node_name_defaults_to_lowercased_type_label` test (which ends at ~line 290),
|
|
add:
|
|
|
|
```rust
|
|
#[test]
|
|
fn instance_name_is_some_only_when_explicitly_named() {
|
|
// unnamed → None (instance_name() does NOT resolve node_name()'s
|
|
// lowercased-type default — that is exactly the value the graph model
|
|
// must not surface)
|
|
let unnamed = PrimitiveBuilder::new(
|
|
"SimBroker",
|
|
NodeSchema::default(),
|
|
|_| panic!("not built in this test"),
|
|
);
|
|
assert_eq!(unnamed.instance_name(), None);
|
|
|
|
// explicitly named → Some(the raw name)
|
|
let named = PrimitiveBuilder::new(
|
|
"SMA",
|
|
NodeSchema::default(),
|
|
|_| panic!("not built in this test"),
|
|
)
|
|
.named("fast");
|
|
assert_eq!(named.instance_name(), Some("fast"));
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Run the test to verify it fails**
|
|
|
|
Run: `cargo test -p aura-core instance_name_is_some_only_when_explicitly_named`
|
|
Expected: FAIL — compile error `no method named `instance_name` found for struct `PrimitiveBuilder``.
|
|
|
|
- [ ] **Step 3: Re-ground the `named()` doc (code unchanged)**
|
|
|
|
In `crates/aura-core/src/node.rs`, replace the single-line doc above `named()`:
|
|
|
|
```rust
|
|
/// Set this node instance's name. Must be non-empty.
|
|
```
|
|
|
|
with:
|
|
|
|
```rust
|
|
/// Set this node instance's explicit name. Must be non-empty: the name forms
|
|
/// a knob-address segment (`<composite>.<name>.<param>`, via `node_name()` in
|
|
/// `collect_params`) and is the `Some`/`None` switch for the graph-model
|
|
/// `"name"` field and the viewer prefix. An empty name would yield a broken
|
|
/// address segment (`sma_cross..length`) and a degenerate `Some("")` —
|
|
/// serialised as `"name":""` yet rendering no prefix, i.e. two encodings for
|
|
/// "no visible name". The `debug_assert` guards that at source.
|
|
```
|
|
|
|
Leave the `named()` body (the `debug_assert!` + the two statements) unchanged.
|
|
|
|
- [ ] **Step 4: Add the `instance_name()` accessor**
|
|
|
|
In `crates/aura-core/src/node.rs`, immediately after `node_name()`'s closing
|
|
brace (the method ending `.unwrap_or_else(|| self.name.to_ascii_lowercase())` +
|
|
`}` at ~line 110), add:
|
|
|
|
```rust
|
|
/// The explicit instance name if one was set via `named()`, else `None`.
|
|
/// Unlike `node_name()`, this does not resolve the default — callers that must
|
|
/// distinguish "explicitly named" from "defaulted" (the graph model) read this.
|
|
pub fn instance_name(&self) -> Option<&str> {
|
|
self.instance_name.as_deref()
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 5: Run the test to verify it passes**
|
|
|
|
Run: `cargo test -p aura-core instance_name_is_some_only_when_explicitly_named`
|
|
Expected: PASS (`test result: ok. 1 passed`).
|
|
|
|
---
|
|
|
|
### Task 2: aura-engine — `prim_record` conditional `"name"` field
|
|
|
|
**Files:**
|
|
- Modify: `crates/aura-engine/src/graph_model.rs` (`prim_record` 72-83; new unit test after line 373)
|
|
|
|
- [ ] **Step 1: Write the failing `prim_record` test**
|
|
|
|
In `crates/aura-engine/src/graph_model.rs`, directly after the
|
|
`primitive_record_carries_type_role_ins_outs` test (which ends at line 373),
|
|
add:
|
|
|
|
```rust
|
|
#[test]
|
|
fn prim_record_emits_name_for_explicit_only() {
|
|
// explicitly named → a leading "name" field, ahead of "type"
|
|
let named = prim_record(&sub_builder().named("fast"));
|
|
assert!(
|
|
named.starts_with(r#"{"prim":{"name":"fast","type":"Sub""#),
|
|
"{named}"
|
|
);
|
|
// unnamed → no "name" field at all
|
|
let unnamed = prim_record(&sub_builder());
|
|
assert!(!unnamed.contains(r#""name""#), "{unnamed}");
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Run the test to verify it fails**
|
|
|
|
Run: `cargo test -p aura-engine prim_record_emits_name_for_explicit_only`
|
|
Expected: FAIL — the `named` record has no `"name"` field, so `starts_with(...)`
|
|
is false (panics with the printed record `{"prim":{"type":"Sub",...}}`).
|
|
|
|
- [ ] **Step 3: Add the conditional `"name"` fragment to `prim_record`**
|
|
|
|
In `crates/aura-engine/src/graph_model.rs`, replace the body of `prim_record`
|
|
(lines 72-83):
|
|
|
|
```rust
|
|
fn prim_record(b: &aura_core::PrimitiveBuilder) -> String {
|
|
let s = b.schema();
|
|
let role = if s.output.is_empty() { "sink" } else { "node" };
|
|
let params = join(s.params.iter().map(|p| named_kind(&p.name, p.kind)));
|
|
let ins = join(s.inputs.iter().map(port_json));
|
|
let outs = join(s.output.iter().map(|f| named_kind(f.name, f.kind)));
|
|
format!(
|
|
r#"{{"prim":{{"type":{},"role":{},"params":[{params}],"ins":[{ins}],"outs":[{outs}]}}}}"#,
|
|
json_str(&b.label()),
|
|
json_str(role),
|
|
)
|
|
}
|
|
```
|
|
|
|
with:
|
|
|
|
```rust
|
|
fn prim_record(b: &aura_core::PrimitiveBuilder) -> String {
|
|
let s = b.schema();
|
|
let role = if s.output.is_empty() { "sink" } else { "node" };
|
|
let params = join(s.params.iter().map(|p| named_kind(&p.name, p.kind)));
|
|
let ins = join(s.inputs.iter().map(port_json));
|
|
let outs = join(s.output.iter().map(|f| named_kind(f.name, f.kind)));
|
|
// explicit instance name only — an unnamed node carries no "name" field
|
|
// (node_name()'s lowercased-type default would be a redundant type-duplicate)
|
|
let name = match b.instance_name() {
|
|
Some(n) => format!(r#""name":{},"#, json_str(n)),
|
|
None => String::new(),
|
|
};
|
|
format!(
|
|
r#"{{"prim":{{{name}"type":{},"role":{},"params":[{params}],"ins":[{ins}],"outs":[{outs}]}}}}"#,
|
|
json_str(&b.label()),
|
|
json_str(role),
|
|
)
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run the unit test to verify it passes**
|
|
|
|
Run: `cargo test -p aura-engine prim_record_emits_name_for_explicit_only`
|
|
Expected: PASS (`test result: ok. 1 passed`).
|
|
|
|
> **Note:** `model_golden` (inline) and the `sample-model.json` byte fixture are
|
|
> now stale — `model_to_json` emits the new `"name"` field, so the content-pin
|
|
> caught the intended change. Task 3 re-captures both. Do NOT run the full
|
|
> `aura-engine` suite at this step (it would show `model_golden` red, which is
|
|
> expected and resolved next).
|
|
|
|
---
|
|
|
|
### Task 3: Re-capture both golden twins
|
|
|
|
**Files:**
|
|
- Modify: `crates/aura-engine/src/graph_model.rs` (inline `model_golden`, line 429)
|
|
- Modify: `crates/aura-cli/tests/fixtures/sample-model.json:1`
|
|
|
|
The two SMA legs inside `sma_cross` are the only `.named()` nodes; each gains a
|
|
leading `"name"`. Both target substrings are unique (verified: 1 occurrence each)
|
|
and contiguous on a single line, so the replacements are exact.
|
|
|
|
- [ ] **Step 1: Re-capture the inline `model_golden`**
|
|
|
|
In `crates/aura-engine/src/graph_model.rs` (the `expected` string at line 429),
|
|
make these two exact replacements:
|
|
|
|
Replace `"0":{"prim":{"type":"SMA"` with `"0":{"prim":{"name":"fast","type":"SMA"`
|
|
|
|
Replace `"1":{"prim":{"type":"SMA"` with `"1":{"prim":{"name":"slow","type":"SMA"`
|
|
|
|
(Every other byte of the golden is unchanged: the root nodes — `comp:sma_cross`,
|
|
`Exposure`, `Recorder`, `src_price` — and the `Sub` leg keyed `"2"` are
|
|
byte-identical.)
|
|
|
|
- [ ] **Step 2: Re-capture the `sample-model.json` fixture**
|
|
|
|
In `crates/aura-cli/tests/fixtures/sample-model.json` (single line), make the
|
|
same two exact replacements:
|
|
|
|
Replace `"0":{"prim":{"type":"SMA"` with `"0":{"prim":{"name":"fast","type":"SMA"`
|
|
|
|
Replace `"1":{"prim":{"type":"SMA"` with `"1":{"prim":{"name":"slow","type":"SMA"`
|
|
|
|
(All other records — `Exposure`, `SimBroker`, the two `Recorder` sinks,
|
|
`src_price`, the `Sub` leg, every edge, the re-export name `"cross"` — are
|
|
byte-identical.)
|
|
|
|
- [ ] **Step 3: Verify the inline golden + determinism are green**
|
|
|
|
Run: `cargo test -p aura-engine model_golden`
|
|
Expected: PASS (`test result: ok. 1 passed`).
|
|
|
|
- [ ] **Step 4: Verify the fixture consumer is still green**
|
|
|
|
Run: `cargo test -p aura-cli --test viewer_dot`
|
|
Expected: PASS — `viewer_emits_valid_dot_node_identifiers` asserts DOT-identifier
|
|
validity, not model bytes, so the added `"name"` field does not perturb it
|
|
(`test result: ok. 1 passed`).
|
|
|
|
---
|
|
|
|
### Task 4: Viewer render guard (RED)
|
|
|
|
**Files:**
|
|
- Create: `crates/aura-cli/tests/viewer_name_prefix.mjs`
|
|
- Create: `crates/aura-cli/tests/viewer_name_prefix.rs`
|
|
|
|
- [ ] **Step 1: Write the headless render guard**
|
|
|
|
Create `crates/aura-cli/tests/viewer_name_prefix.mjs`:
|
|
|
|
```js
|
|
// Headless render guard for the `aura graph` viewer (graph-viewer.js).
|
|
//
|
|
// Property protected: a leaf primitive built with an explicit instance name
|
|
// renders a `name: ` declaration prefix immediately ahead of the bold type head
|
|
// (`fast: SMA…`); an UNNAMED leaf renders the bare type head, no prefix.
|
|
//
|
|
// It loads the *real* viewer module (so a fix or a regression there is observed
|
|
// here) and drives the exported pure `genDot` over a minimal root model with one
|
|
// named leaf (SMA, name "fast") and one unnamed leaf (Sub) — exercising the
|
|
// genDot leaf-emit + cellLabel prefix path directly, no composite expand needed.
|
|
|
|
import { createRequire } from "node:module";
|
|
import { fileURLToPath } from "node:url";
|
|
import { dirname, join } from "node:path";
|
|
|
|
const require = createRequire(import.meta.url);
|
|
const here = dirname(fileURLToPath(import.meta.url));
|
|
|
|
const model = {
|
|
root: {
|
|
nodes: {
|
|
"0": { prim: { name: "fast", type: "SMA", role: "node", params: [["length", "i64"]], ins: [["f64", "any", "series"]], outs: [["value", "f64"]] } },
|
|
"1": { prim: { type: "Sub", role: "node", params: [], ins: [["f64", "any", "lhs"], ["f64", "any", "rhs"]], outs: [["value", "f64"]] } },
|
|
},
|
|
edges: [["0.o0", "1.i0"]],
|
|
},
|
|
composites: {},
|
|
};
|
|
|
|
// genDot closes over module-scoped ROOT/COMP derived from window.AURA_MODEL at
|
|
// load. Stub the global BEFORE requiring the viewer (mirrors the browser).
|
|
global.window = { AURA_MODEL: model };
|
|
const { normalizeModel, genDot } = require(join(here, "..", "assets", "graph-viewer.js"));
|
|
const ROOT = normalizeModel(model).root;
|
|
const { dot } = genDot(ROOT, "root", new Set(), false);
|
|
|
|
// The named leaf renders `fast: ` (colon + one trailing space, no leading space)
|
|
// contiguously ahead of the bold SMA type head.
|
|
const NAMED = '<font color="#cdd6f4">fast</font><font color="#9399b2">: </font><font color="#f5f5f5"><b>SMA</b></font>';
|
|
if (!dot.includes(NAMED)) {
|
|
console.error(
|
|
"named leaf missing the `fast: ` prefix before the SMA head.\n--- dot ---\n" + dot
|
|
);
|
|
process.exit(1);
|
|
}
|
|
|
|
// Exactly ONE name prefix in the whole graph: the named SMA leg. The `: `
|
|
// separator font (#9399b2 colon-space) is unique to the prefix at showTypes=false
|
|
// (sig uses `[`/`]`/`, `, never `: `), so it appears once — the unnamed Sub has
|
|
// no prefix.
|
|
const SEP = '<font color="#9399b2">: </font>';
|
|
const sepCount = dot.split(SEP).length - 1;
|
|
if (sepCount !== 1) {
|
|
console.error(
|
|
`expected exactly 1 name prefix (the named leaf), found ${sepCount} — ` +
|
|
"an unnamed leaf must render no prefix.\n--- dot ---\n" + dot
|
|
);
|
|
process.exit(1);
|
|
}
|
|
|
|
console.log("OK — named leaf renders `fast: ` prefix; unnamed leaf stays bare.");
|
|
process.exit(0);
|
|
```
|
|
|
|
- [ ] **Step 2: Write the `.rs` wrapper**
|
|
|
|
Create `crates/aura-cli/tests/viewer_name_prefix.rs`:
|
|
|
|
```rust
|
|
//! Integration guard: the `aura graph` viewer renders an explicit instance name
|
|
//! as a `name: ` prefix ahead of the type head, and leaves an unnamed leaf bare.
|
|
//!
|
|
//! Like `viewer_dot.rs`, the property lives in JavaScript (`cellLabel` in
|
|
//! assets/graph-viewer.js), so this shells out to `node` running the headless
|
|
//! guard `tests/viewer_name_prefix.mjs`, which loads the real viewer module and
|
|
//! drives the exported `genDot` over a minimal model (a named SMA + an unnamed
|
|
//! Sub).
|
|
//!
|
|
//! `node` is REQUIRED: if it is not on PATH this test FAILS (a skipped guard is
|
|
//! not a guard).
|
|
|
|
use std::path::PathBuf;
|
|
use std::process::Command;
|
|
|
|
#[test]
|
|
fn viewer_renders_name_prefix_for_named_leaf_only() {
|
|
let script = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
|
|
.join("tests")
|
|
.join("viewer_name_prefix.mjs");
|
|
assert!(
|
|
script.exists(),
|
|
"guard script missing at {}",
|
|
script.display()
|
|
);
|
|
|
|
let out = match Command::new("node").arg(&script).output() {
|
|
Ok(out) => out,
|
|
Err(e) => panic!(
|
|
"node is required for the viewer name-prefix guard but could not be run \
|
|
({e}); install Node.js or ensure `node` is on PATH"
|
|
),
|
|
};
|
|
|
|
let stdout = String::from_utf8_lossy(&out.stdout);
|
|
let stderr = String::from_utf8_lossy(&out.stderr);
|
|
assert!(
|
|
out.status.success(),
|
|
"viewer name-prefix guard failed (exit {:?}).\n--- node stdout ---\n{stdout}\n--- node stderr ---\n{stderr}",
|
|
out.status.code()
|
|
);
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 3: Run the guard to verify it fails**
|
|
|
|
Run: `cargo test -p aura-cli --test viewer_name_prefix`
|
|
Expected: FAIL — before the viewer change `adaptNodes` drops `name`, so
|
|
`cellLabel` renders the bare SMA head with no prefix; the guard prints
|
|
"named leaf missing the `fast: ` prefix before the SMA head" and exits 1, and the
|
|
`.rs` test fails with the captured node stderr.
|
|
|
|
---
|
|
|
|
### Task 5: Viewer implementation (GREEN)
|
|
|
|
**Files:**
|
|
- Modify: `crates/aura-cli/assets/graph-viewer.js` (`adaptNodes` line 32, `genDot` leaf-emit line 108, `cellLabel` head line 74)
|
|
|
|
- [ ] **Step 1: `adaptNodes` carries `name` onto the per-node object**
|
|
|
|
In `crates/aura-cli/assets/graph-viewer.js`, replace line 32:
|
|
|
|
```js
|
|
out[key] = { prim: { type: p.type, role: p.role, params: p.params, ins: adaptIns(p.ins), outs: p.outs } };
|
|
```
|
|
|
|
with:
|
|
|
|
```js
|
|
out[key] = { prim: { name: p.name, type: p.type, role: p.role, params: p.params, ins: adaptIns(p.ins), outs: p.outs } };
|
|
```
|
|
|
|
- [ ] **Step 2: `genDot` leaf-emit forwards `name` into `cellLabel`**
|
|
|
|
In `crates/aura-cli/assets/graph-viewer.js`, replace line 108 (the leaf-emit
|
|
`cellLabel` call — NOT the composite-emit call at line 120, which keeps
|
|
`type: cname` and gets no name):
|
|
|
|
```js
|
|
block += `${cid} [label=${cellLabel(cid, { type: s.type, params: s.params, ins: s.ins, outs: s.outs, bodyBg: bodyBgFor(s.role), composite: false }, showTypes)}];\n`;
|
|
```
|
|
|
|
with:
|
|
|
|
```js
|
|
block += `${cid} [label=${cellLabel(cid, { name: s.name, type: s.type, params: s.params, ins: s.ins, outs: s.outs, bodyBg: bodyBgFor(s.role), composite: false }, showTypes)}];\n`;
|
|
```
|
|
|
|
- [ ] **Step 3: `cellLabel` prepends the `name: ` prefix**
|
|
|
|
In `crates/aura-cli/assets/graph-viewer.js`, replace line 74:
|
|
|
|
```js
|
|
const name = `<font color="#f5f5f5"><b>${o.type}</b></font>${sig}`;
|
|
```
|
|
|
|
with:
|
|
|
|
```js
|
|
const prefix = o.name
|
|
? `<font color="#cdd6f4">${o.name}</font><font color="#9399b2">: </font>`
|
|
: "";
|
|
const name = `${prefix}<font color="#f5f5f5"><b>${o.type}</b></font>${sig}`;
|
|
```
|
|
|
|
- [ ] **Step 4: Run the render guard to verify it passes**
|
|
|
|
Run: `cargo test -p aura-cli --test viewer_name_prefix`
|
|
Expected: PASS — the named leaf now renders the `fast: ` prefix ahead of the SMA
|
|
head, the unnamed Sub stays bare (`test result: ok. 1 passed`).
|
|
|
|
---
|
|
|
|
### Task 6: Workspace verification gate
|
|
|
|
**Files:** none (verification only)
|
|
|
|
- [ ] **Step 1: Build the workspace**
|
|
|
|
Run: `cargo build --workspace`
|
|
Expected: `Finished` with 0 errors.
|
|
|
|
- [ ] **Step 2: Run the full test suite**
|
|
|
|
Run: `cargo test --workspace`
|
|
Expected: all test binaries report `test result: ok` (0 failed). In particular
|
|
`instance_name_is_some_only_when_explicitly_named`, `prim_record_emits_name_for_explicit_only`,
|
|
`model_golden`, `viewer_emits_valid_dot_node_identifiers`, and
|
|
`viewer_renders_name_prefix_for_named_leaf_only` all pass.
|
|
|
|
- [ ] **Step 3: Clippy clean**
|
|
|
|
Run: `cargo clippy --workspace --all-targets -- -D warnings`
|
|
Expected: `Finished` with 0 warnings.
|