776bd5432a
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).
84 lines
4.1 KiB
JavaScript
84 lines
4.1 KiB
JavaScript
// 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);
|