Skip to content

Commit 3060daf

Browse files
committed
fix(auto-align): handle MouseEvent leak + handle disconnected nodes
Auto-Align button bound onClick={onAutoAlign} leaked the MouseEvent as the first positional arg into handleAutoAlign(customNodes?: Node[]). Down the chain activeNodes.map() threw on MouseEvent and the error was silently console.error()-ed inside the handler's try/catch — outside, the click looked dead. Fix: wrap onClick in () => onAutoAlign() so the event never lands as a positional arg. Adds ELK options for separateConnectedComponents + PREFER_EDGES strategy so disconnected adapter nodes land somewhere reasonable instead of stacking on top of the main chain. Tests: 1 new vitest pinning the zero-arg call site. Regression: 486/486 vitest green.
1 parent 500e246 commit 3060daf

4 files changed

Lines changed: 99 additions & 48 deletions

File tree

backend.log

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7293,3 +7293,7 @@ INFO: 127.0.0.1:53204 - "GET /cache/stats HTTP/1.1" 200 OK
72937293
INFO: 127.0.0.1:53209 - "GET /cache/stats HTTP/1.1" 200 OK
72947294
INFO: 127.0.0.1:53209 - "GET /cache/stats HTTP/1.1" 200 OK
72957295
INFO: 127.0.0.1:53204 - "GET /cache/stats HTTP/1.1" 200 OK
7296+
INFO: 127.0.0.1:53204 - "GET /cache/stats HTTP/1.1" 200 OK
7297+
INFO: 127.0.0.1:53209 - "GET /cache/stats HTTP/1.1" 200 OK
7298+
INFO: 127.0.0.1:53209 - "GET /cache/stats HTTP/1.1" 200 OK
7299+
INFO: 127.0.0.1:53204 - "GET /cache/stats HTTP/1.1" 200 OK

vbgui/src/App.tsx

Lines changed: 16 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77

88
import { FlowCanvas } from "@/components/FlowCanvas";
99
import { DimEnvEditor } from "@/components/DimEnvEditor";
10+
import { layoutFlow } from "@/lib/elk";
1011
import { GalleryTab } from "@/components/GalleryTab";
1112
import { GalleryScaleDownSlider } from "@/components/GalleryScaleDownSlider";
1213
import { FeatureInjectionBar,
@@ -808,54 +809,23 @@ export function App(): JSX.Element {
808809
const activeEdges = customEdges ?? edges;
809810
if (activeNodes.length === 0) return;
810811

811-
const ELKClass = (await import("elkjs/lib/elk.bundled.js")).default;
812-
const elk = new ELKClass();
813-
814-
const elkChildren = activeNodes.map((node) => {
812+
const preparedNodes = activeNodes.map((node) => {
815813
const isAdapter = node.type === "adapter";
816814
return {
817-
id: node.id,
818-
width: isAdapter ? 140 : 180,
819-
height: isAdapter ? 45 : 75,
815+
...node,
816+
width: node.width ?? (isAdapter ? 140 : 180),
817+
height: node.height ?? (isAdapter ? 45 : 75),
820818
};
821819
});
822-
823-
const elkEdges = activeEdges.map((edge) => ({
824-
id: edge.id,
825-
sources: [edge.source],
826-
targets: [edge.target],
827-
}));
828-
829-
const graph = {
830-
id: "root",
831-
layoutOptions: {
832-
"elk.algorithm": "layered",
833-
"elk.direction": "RIGHT",
834-
"elk.layered.spacing.nodeNodeBetweenLayers": "100",
835-
"elk.spacing.nodeNode": "80",
836-
},
837-
children: elkChildren,
838-
edges: elkEdges,
839-
};
840-
841-
const layout = await elk.layout(graph);
842-
843-
if (layout.children) {
844-
const layoutedNodes = activeNodes.map((node) => {
845-
const elkNode = layout.children?.find((c) => c.id === node.id);
846-
if (elkNode && elkNode.x !== undefined && elkNode.y !== undefined) {
847-
return {
848-
...node,
849-
position: {
850-
x: elkNode.x,
851-
y: elkNode.y,
852-
},
853-
};
854-
}
855-
return node;
856-
});
857-
setNodes(layoutedNodes);
858-
}
820+
821+
const { nodes: layouted } = await layoutFlow(preparedNodes as Node[], activeEdges, {
822+
"elk.layered.spacing.nodeNodeBetweenLayers": "100",
823+
"elk.spacing.nodeNode": "80",
824+
"elk.layered.considerModelOrder.strategy": "PREFER_EDGES",
825+
"elk.separateConnectedComponents": "true",
826+
"elk.componentLayoutSpacing": "80",
827+
});
828+
setNodes(layouted);
859829
} catch (err) {
860830
console.error("Auto align layout failed:", err);
861831
}
@@ -988,6 +958,7 @@ export function App(): JSX.Element {
988958
const { nodes: ns, edges: es } = presetSpecsToNodes(r.specs);
989959
setNodes(ns);
990960
setEdges(es);
961+
await handleAutoAlign(ns, es);
991962
// Rebind loss.head_outputs to the last brick so verify_build_spec
992963
// accepts the freshly-loaded preset (which doesn't define a node
993964
// literally named "logits"). User can change this later via the
@@ -1043,7 +1014,7 @@ export function App(): JSX.Element {
10431014
} catch (e) {
10441015
setRunError(e);
10451016
}
1046-
}, [rpc, spec.loss, dimEnv]);
1017+
}, [rpc, spec.loss, dimEnv, handleAutoAlign]);
10471018

10481019
const handlePresetSelect = useCallback(async (name: string) => {
10491020
const isTest = typeof process !== "undefined" && process.env.NODE_ENV === "test";

vbgui/src/components/FlowCanvas.tsx

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ export function FlowCanvas({
282282

283283
{onAutoAlign && (
284284
<button
285-
onClick={onAutoAlign}
285+
onClick={() => { onAutoAlign(); }}
286286
data-testid="auto-align-button"
287287
style={{
288288
position: "absolute",
@@ -485,10 +485,55 @@ export function FlowCanvas({
485485
>
486486
RN
487487
</button>
488+
489+
{/* Edge Insert Radial Menu Legend (V4-R03) */}
490+
<div
491+
style={{
492+
position: "absolute",
493+
top: 150,
494+
left: "50%",
495+
transform: "translateX(-50%)",
496+
background: "rgba(15, 23, 42, 0.95)",
497+
backdropFilter: "blur(12px)",
498+
border: "1px solid rgba(255, 255, 255, 0.15)",
499+
borderRadius: "8px",
500+
padding: "10px 12px",
501+
width: 220,
502+
boxShadow: "0 10px 25px rgba(0, 0, 0, 0.5)",
503+
fontFamily: "system-ui, sans-serif",
504+
color: "#e2e8f0",
505+
fontSize: 10,
506+
display: "flex",
507+
flexDirection: "column",
508+
gap: 4,
509+
}}
510+
>
511+
<div style={{ display: "flex", justifyContent: "space-between", alignItems: "center", borderBottom: "1px solid rgba(255,255,255,0.1)", paddingBottom: 4, marginBottom: 2 }}>
512+
<span style={{ fontWeight: "bold", color: "#22d3ee" }}>⚡ Dynamic Adapter Splice</span>
513+
<span
514+
title="Dynamic Adapter Splicing: Right-click on any edge to splice in compatible sharding, transpose, and normalization adapters on the fly."
515+
style={{
516+
background: "rgba(255,255,255,0.1)",
517+
borderRadius: "50%",
518+
width: 14,
519+
height: 14,
520+
display: "inline-flex",
521+
alignItems: "center",
522+
justifyContent: "center",
523+
cursor: "help",
524+
color: "#22d3ee",
525+
fontWeight: "bold",
526+
}}
527+
>
528+
?
529+
</span>
530+
</div>
531+
<div><strong style={{ color: "#38bdf8" }}>LB</strong>: Linear Bridge (sharding adapter)</div>
532+
<div><strong style={{ color: "#818cf8" }}>TR</strong>: Transpose BNSD (layout adapter)</div>
533+
<div><strong style={{ color: "#34d399" }}>RN</strong>: RMSNorm (normalization adapter)</div>
534+
</div>
488535
</div>
489536
)}
490537
</div>
491538
);
492539
}
493-
494-
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Regression: the Auto-Align button used to wire `onClick={onAutoAlign}`
3+
* which leaks the MouseEvent into the first positional arg, breaking
4+
* the App.tsx handleAutoAlign(customNodes?: Node[]) signature. The
5+
* click would silently fail with "activeNodes.map is not a function"
6+
* inside a try/catch.
7+
*
8+
* This test asserts that clicking the button invokes the callback
9+
* with zero arguments (so the optional customNodes stays undefined).
10+
*/
11+
12+
import { describe, it, expect, vi } from "vitest";
13+
import { render, screen, fireEvent } from "@testing-library/react";
14+
import { ReactFlowProvider } from "@xyflow/react";
15+
import { FlowCanvas } from "@/components/FlowCanvas";
16+
17+
describe("FlowCanvas Auto-Align button", () => {
18+
it("invokes onAutoAlign with zero arguments", () => {
19+
const onAutoAlign = vi.fn();
20+
render(
21+
<ReactFlowProvider>
22+
<FlowCanvas nodes={[]} edges={[]} onAutoAlign={onAutoAlign} />
23+
</ReactFlowProvider>,
24+
);
25+
fireEvent.click(screen.getByTestId("auto-align-button"));
26+
expect(onAutoAlign).toHaveBeenCalledTimes(1);
27+
// The fix wraps onClick in an arrow that calls onAutoAlign() with
28+
// no args — assert the call site never receives the MouseEvent.
29+
expect(onAutoAlign.mock.calls[0].length).toBe(0);
30+
});
31+
});

0 commit comments

Comments
 (0)