fix(aura-cli): emit valid DOT identifiers for inline-expanded composites

Observed bug in the graph viewer shipped in 66dff88: clicking `[+]` to
inline-expand a composite in `aura graph`'s HTML output renders a broken graph —
the interior nodes' edges collapse onto a single phantom node "0" with uncoloured
arrows, and the real pin docking is lost. The collapsed view and the body-drill
view both render correctly; only inline-expand breaks.

Root cause: in graph-viewer.js, genDot's `build` forms a node id by path-joining
the model keys (`cid = cpath.join("__")`). The aura model keys interior nodes by
numeric index (C23), so an inline-expanded composite yields ids like `0__0`. A
DOT identifier that begins with a digit but is not a pure numeral is invalid —
Graphviz parses `0__0` as the numeral `0` plus a leftover token, collapsing
`0__0`/`0__1`/`0__2` onto one phantom node `0`. The collapsed view survives only
because its ids are single-digit numerals (accidentally valid); the prototype
survived because it used name keys, never digit-prefixed ones. The bug surfaced
only against the real index-keyed model.

Fix: letter-prefix the id (`cid = "n" + cpath.join("__")`) so every id is a valid
identifier by construction. The fix lives in the viewer (DOT-syntax is its
responsibility), not in model_to_json — the model's index keys are correct, and
leaking the Graphviz identifier constraint into the model would be a leaky
abstraction.

RED-first guard (this establishes the project's first JS test infra, since the
viewer now carries real logic — normalizeModel + genDot — that the model golden
does not cover):
- tests/viewer_dot_ids.mjs: a headless node guard that loads the real viewer
  module, discovers the expandable composites from a collapsed pass, inline-
  expands them, and asserts every node id in the produced DOT is a valid Graphviz
  identifier (quoted | pure numeral | bare identifier). Version-independent — it
  checks the identifier grammar, not a render. The expand keys are DISCOVERED via
  genDot's `expandable`, never hard-coded, so the guard cannot go vacuously green
  when the fix reshapes the id (a hard-coded "0" key would stop matching "n0" and
  silently test only the collapsed view — a false-green this guard refuses).
- tests/viewer_dot.rs: a Rust integration test that shells out to `node`,
  wiring the guard into `cargo test --workspace`. `node` is required: if it is
  absent the test fails (a skipped guard is not a guard).
- tests/fixtures/sample-model.json: the triggering fixture (numeric index keys,
  a nested composite).
- graph-viewer.js: made node-loadable (typeof window/document guards + a
  module.exports) so the guard can exercise the pure generator headless. This is
  a test-enabler, behaviour-neutral in the browser — verified by the existing
  render_html / graph_emits self-containment tests staying green and by the
  expand re-rendering correctly through the vendored WASM-Graphviz.

Verified RED against the unfixed viewer (the guard trips on `0__0`/`0__1`/`0__2`
and the Rust test fails), GREEN after the prefix; full `cargo test --workspace`
green; clippy clean. Rendered the expanded state through the vendored
WASM-Graphviz to confirm the interior wires up correctly (price -> both SMA
series, SMA value -> Sub lhs/rhs, Sub -> signal, all f64-blue).
This commit is contained in:
2026-06-10 17:48:49 +02:00
parent 53f3fcf198
commit 776bd5432a
4 changed files with 153 additions and 2 deletions
+21 -2
View File
@@ -52,7 +52,12 @@ function normalizeModel(model) {
composites,
};
}
const MODEL = normalizeModel(window.AURA_MODEL);
// In the browser `window.AURA_MODEL` is injected by render_html; under a node
// test harness the caller sets `global.window = { AURA_MODEL: <model> }` before
// requiring this file. The guard keeps a bare require (no window) from throwing.
const MODEL = (typeof window !== "undefined" && window.AURA_MODEL)
? normalizeModel(window.AURA_MODEL)
: { root: { inputs: [], outputs: [], nodes: {}, edges: [] }, composites: {} };
const COMP = MODEL.composites;
const ROOT = MODEL.root;
@@ -94,7 +99,10 @@ function genDot(def, role, expandedSet, showTypes) {
function build(path, def, brole) {
const childRes = {}; let block = "";
for (const key in def.nodes) {
const inst = def.nodes[key], cpath = [...path, key], cid = cpath.join("__");
// `n`-prefix keeps cid a valid DOT identifier: model node keys are numeric
// indices (C23), and a bare `0__0` (digit-led, non-numeral) is misparsed by
// Graphviz — it collapses inline-expanded composites onto a phantom node.
const inst = def.nodes[key], cpath = [...path, key], cid = "n" + cpath.join("__");
if (inst.prim) {
const s = inst.prim;
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`;
@@ -149,6 +157,10 @@ function genDot(def, role, expandedSet, showTypes) {
}
// === styled tooltip + interaction ============================================
// The DOM bootstrap runs only in the browser. Under a node test harness there is
// no `document`/`Viz`, so the whole interaction block is skipped (the pure parts
// — normalizeModel, genDot — are exported below for headless testing).
if (typeof document !== "undefined") {
const tip = document.getElementById("tip");
const md = (s) => s.replace(/\*\*(.+?)\*\*/g, "<b>$1</b>").replace(/`(.+?)`/g, "<code>$1</code>").replace(/\n/g, "<br>");
function showTip(text, x, y) {
@@ -208,3 +220,10 @@ function show() {
}
Viz.instance().then(v => { viz = v; show(); }).catch(e => { document.getElementById("err").textContent = String(e && e.stack || e); });
}
// Headless export: the node DOT-id guard requires this file (after setting
// global.window) and exercises the pure generator. No effect in the browser.
if (typeof module !== "undefined" && module.exports) {
module.exports = { normalizeModel, genDot };
}
+1
View File
@@ -0,0 +1 @@
{"root":{"nodes":{"0":{"comp":"sma_cross"},"1":{"prim":{"type":"Exposure","role":"node","params":[["scale","f64"]],"ins":[["f64","any","signal"]],"outs":[["exposure","f64"]]}},"2":{"prim":{"type":"SimBroker","role":"node","params":[],"ins":[["f64","any","exposure"],["f64","any","price"]],"outs":[["equity","f64"]]}},"3":{"prim":{"type":"Recorder","role":"sink","params":[],"ins":[["f64","any","col[0]"]],"outs":[]}},"4":{"prim":{"type":"Recorder","role":"sink","params":[],"ins":[["f64","any","col[0]"]],"outs":[]}},"src_price":{"prim":{"type":"price","role":"source","params":[],"ins":[],"outs":[["price","f64"]]}}},"edges":[["0.o0","1.i0"],["1.o0","2.i0"],["2.o0","3.i0"],["1.o0","4.i0"],["src_price.o0","0.i0"],["src_price.o0","2.i1"]]},"composites":{"sma_cross":{"inputs":[["f64","any","price"]],"outputs":[["cross","f64"]],"nodes":{"0":{"prim":{"type":"SMA","role":"node","params":[["length","i64"]],"ins":[["f64","any","series"]],"outs":[["value","f64"]]}},"1":{"prim":{"type":"SMA","role":"node","params":[["length","i64"]],"ins":[["f64","any","series"]],"outs":[["value","f64"]]}},"2":{"prim":{"type":"Sub","role":"node","params":[],"ins":[["f64","any","lhs"],["f64","any","rhs"]],"outs":[["value","f64"]]}}},"edges":[["0.o0","2.i0"],["1.o0","2.i1"],["@price","0.i0"],["@price","1.i0"],["2.o0","#0"]]}}}
+48
View File
@@ -0,0 +1,48 @@
//! Integration guard for the `aura graph` viewer's DOT generation.
//!
//! The viewer (`assets/graph-viewer.js`) builds the Graphviz DOT in JavaScript,
//! so the property "every node id genDot emits is a valid Graphviz identifier"
//! cannot be checked from Rust directly. This test shells out to `node`, running
//! the headless guard `tests/viewer_dot_ids.mjs`, which loads the real viewer
//! module and asserts the DOT-identifier rule over an inline-expanded nested
//! composite with numeric index keys (the shape `aura graph` emits).
//!
//! Why it matters: a path-joined id like `0__0` begins with a digit but is not a
//! pure numeral, so Graphviz misparses it and collapses the inline-expanded
//! interior onto a single phantom node — the inline-expand render bug. The guard
//! is Graphviz-version-independent (it checks the identifier grammar, not a
//! render).
//!
//! `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_emits_valid_dot_node_identifiers() {
let script = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("tests")
.join("viewer_dot_ids.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 DOT-id 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 DOT-id guard failed (exit {:?}).\n--- node stdout ---\n{stdout}\n--- node stderr ---\n{stderr}",
out.status.code()
);
}
+83
View File
@@ -0,0 +1,83 @@
// Headless DOT-identifier guard for the `aura graph` viewer (graph-viewer.js).
//
// Property protected: every node identifier genDot writes into the DOT it hands
// to Graphviz is a *valid Graphviz identifier*. Graphviz parses an unquoted id
// that begins with a digit but is not a pure numeral (e.g. `0__0`) as a numeral
// followed by leftover — silently collapsing distinct ids (`0__0`, `0__1`, ...)
// onto one phantom node `0`. That is the inline-expand bug: a nested composite
// whose node keys are numeric indices yields path-joined ids like `0__0`, which
// Graphviz misparses, collapsing the interior onto a single uncoloured node.
//
// The guard is Graphviz-version-INDEPENDENT: it does not render, it checks the
// DOT-identifier rule directly. An id is valid iff it is a quoted string, OR a
// pure numeral, OR matches the bare-identifier grammar `[A-Za-z_][A-Za-z0-9_]*`.
//
// It loads the *real* viewer module (so a fix or a regression there is observed
// here) and drives the exported pure `genDot` over a model with NUMERIC index
// keys containing a nested composite — exactly the shape `aura graph` emits.
import { createRequire } from "node:module";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { dirname, join } from "node:path";
const require = createRequire(import.meta.url);
const here = dirname(fileURLToPath(import.meta.url));
// The triggering fixture: root node "0" is a composite reference `{comp}`, and
// the composite's own nodes are keyed "0"/"1"/"2" (numeric indices). Inline-
// expanding "0" path-joins those into `0__0`, `0__1`, `0__2`.
const model = JSON.parse(readFileSync(join(here, "fixtures", "sample-model.json"), "utf8"));
// genDot closes over module-scoped ROOT/COMP derived from window.AURA_MODEL at
// load. Stub the global BEFORE requiring the viewer (mirrors the browser, where
// render_html injects window.AURA_MODEL ahead of the script).
global.window = { AURA_MODEL: model };
const { normalizeModel, genDot } = require(join(here, "..", "assets", "graph-viewer.js"));
const ROOT = normalizeModel(model).root;
// Inline-EXPAND every expandable composite. The set of expandable ids is
// DISCOVERED from a collapsed pass (genDot returns `expandable`), never hard-
// coded — the guard must drive the real expand path regardless of how genDot
// forms its ids. (A hard-coded key like "0" would stop matching the moment the
// fix reshapes the id to "n0", silently falling back to the collapsed view and
// passing vacuously — the exact false-green this guard must not have.)
const { expandable } = genDot(ROOT, "root", new Set(), false);
if (!expandable.length) {
console.error(
"guard fixture exposes no expandable composite — the inline-expand path\n" +
"(where the digit-prefixed id bug lives) is never exercised. Fix the fixture."
);
process.exit(1);
}
const { dot } = genDot(ROOT, "root", new Set(expandable), false);
// Collect every node identifier referenced in the DOT.
// - node-def lines: `<id> [label=...]` (id is the leading token)
// - edge endpoints: `<a>:port:s -> <b>:port:n` (a and b, before the colon)
// Both forms can carry the SAME digit-prefixed id, so we gather from both.
const ids = new Set();
for (const m of dot.matchAll(/^\s*([^\s\[]+)\s*\[label=/gm)) ids.add(m[1]);
for (const m of dot.matchAll(/(\S+?):[a-z]\w*:[a-z]\s*->\s*(\S+?):[a-z]\w*:[a-z]/g)) {
ids.add(m[1]);
ids.add(m[2]);
}
// A valid Graphviz id: quoted | pure numeral | bare identifier.
const QUOTED = /^".*"$/s;
const NUMERAL = /^-?(\.[0-9]+|[0-9]+(\.[0-9]*)?)$/;
const BARE = /^[A-Za-z_][A-Za-z0-9_]*$/;
const isValid = (id) => QUOTED.test(id) || NUMERAL.test(id) || BARE.test(id);
const bad = [...ids].filter((id) => !isValid(id));
if (bad.length) {
console.error(
"INVALID DOT node identifier(s) — Graphviz will misparse a digit-prefixed,\n" +
"non-numeral id (e.g. `0__0`) and collapse the inline-expanded composite:\n " +
bad.map((id) => JSON.stringify(id)).join("\n ")
);
process.exit(1);
}
console.log(`OK — all ${ids.size} DOT node identifiers are valid.`);
process.exit(0);