Merge pull request #337 from pathsim/feature/bus-connection-rules

Enforce bus wiring rules and highlight connectable ports

Author: milanofthe

docs/pvm-spec.md

@@ -320,6 +320,8 @@ In PathSim Python code, subsystems map to `Subsystem(blocks=[...], connections=[
 - A subsystem port carrying a bus becomes one port index per leaf signal, in bus order, on the Subsystem and on its Interface. Port indices after it shift accordingly.
 - Wiring that cannot be resolved is left out: a bus into a block that is not a bus block or subsystem, a picked signal missing from the bus, or a wire loop through bus blocks.
 
+Editors should not create such wiring: a bus may only enter a Bus Creator, a Bus Selector or a subsystem port, and a Bus Selector only takes a bus. PathView rejects these wires while connecting and draws existing ones as errors.
+
 The reference implementations are `src/lib/bus/expand.ts` and `pathview/buses.py`; `tests/fixtures/bus_expansion.json` lists the expected wiring for each case.
 
 ---

src/lib/bus/expand.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import type { Connection, NodeInstance } from '$lib/types/nodes';
 import fixtures from '../../../tests/fixtures/bus_expansion.json';
-import { analyzeBuses, expandBuses, isBusBlock, signalLeaves } from './expand';
+import { analyzeBuses, busWiringProblem, expandBuses, isBusBlock, signalLeaves } from './expand';
 
 type Scenario = (typeof fixtures.scenarios)[number];
 
@@ -49,6 +49,26 @@ describe('bus expansion', () => {
 		expect(expanded.connections).toBe(connections);
 	});
 
+	it('keeps buses out of plain blocks and plain signals out of selectors', () => {
+		const { nodes, connections } = load(fixtures.scenarios.find((s) => s.name === 'flat creator and selector')!);
+		const analysis = analyzeBuses(nodes, connections);
+		const root = analysis.root;
+		expect(busWiringProblem(analysis, root, 'C', 0, 'Scope')).toBe('bus-into-block');
+		expect(busWiringProblem(analysis, root, 'A', 0, 'S')).toBe('signal-into-selector');
+		expect(busWiringProblem(analysis, root, 'C', 0, 'S')).toBeNull();
+		expect(busWiringProblem(analysis, root, 'A', 0, 'C')).toBeNull();
+		expect(busWiringProblem(analysis, root, 'S', 0, 'Scope')).toBeNull();
+	});
+
+	it('applies the bus rules across subsystem ports', () => {
+		const { nodes, connections } = load(fixtures.scenarios.find((s) => s.name === 'bus into a subsystem')!);
+		const analysis = analyzeBuses(nodes, connections);
+		const inner = analysis.levelAt(['Sub'])!;
+		expect(busWiringProblem(analysis, analysis.root, 'C', 0, 'Sub')).toBeNull();
+		expect(busWiringProblem(analysis, inner, 'I', 0, 'G')).toBe('bus-into-block');
+		expect(busWiringProblem(analysis, inner, 'Sel', 0, 'G')).toBeNull();
+	});
+
 	it('follows a bus structure into a subsystem', () => {
 		const { nodes, connections } = load(fixtures.scenarios.find((s) => s.name === 'bus into a subsystem')!);
 		const analysis = analyzeBuses(nodes, connections);

src/lib/bus/expand.ts

@@ -209,6 +209,38 @@ export function analyzeBuses(nodes: NodeInstance[], connections: Connection[]) {
 	return { root, levelAt, structureIn, structureOut, elementNames };
 }
 
+export type BusAnalysis = ReturnType<typeof analyzeBuses>;
+
+/** Why a wire breaks the bus rules */
+export type BusWiringProblem = 'bus-into-block' | 'signal-into-selector';
+
+/**
+ * A bus may only enter a Bus Creator, a Bus Selector or a subsystem port, and a
+ * Bus Selector only takes a bus. Returns the rule a wire from the source port to
+ * the target node breaks, or null if it keeps them.
+ */
+export function busWiringProblem(
+	analysis: BusAnalysis,
+	level: BusLevel,
+	sourceNodeId: string,
+	sourcePort: number,
+	targetNodeId: string
+): BusWiringProblem | null {
+	const target = level.nodes.get(targetNodeId);
+	if (!target) return null;
+	const carriesBus = analysis.structureOut(level, sourceNodeId, sourcePort) !== null;
+	switch (target.type) {
+		case NODE_TYPES.BUS_CREATOR:
+		case NODE_TYPES.SUBSYSTEM:
+		case NODE_TYPES.INTERFACE:
+			return null;
+		case NODE_TYPES.BUS_SELECTOR:
+			return carriesBus ? null : 'signal-into-selector';
+		default:
+			return carriesBus ? 'bus-into-block' : null;
+	}
+}
+
 /**
  * The model without bus blocks, for code generation. Models without bus blocks
  * are returned unchanged. Connections that carry several signals are split,

src/lib/components/FlowCanvas.svelte

@@ -39,7 +39,9 @@
 	import { GRID_SIZE, SNAP_GRID, BACKGROUND_GAP } from '$lib/constants/grid';
 	import { createRoutingSync } from './canvas/routingSync';
 	import { isBusBlock } from '$lib/bus/expand';
-	import { updateBusView } from '$lib/stores/busView.svelte';
+	import { busWireAllowed, updateBusView } from '$lib/stores/busView.svelte';
+	import { endConnectionDrag, startConnectionDrag } from '$lib/stores/connectionDrag.svelte';
+	import { HANDLE_ID } from '$lib/constants/handles';
 	import BusBlockNode from './nodes/BusBlockNode.svelte';
 	import { createEdgeHighlighter } from '$lib/stores/edgeHighlight';
 	import { CANVAS_MIN_ZOOM } from '$lib/constants/layout';
@@ -778,6 +780,25 @@
 		isSyncing = false;
 	}
 
+	// Highlight the ports that can take a wire while it is dragged from a port
+	function handleConnectStart(
+		_event: MouseEvent | TouchEvent,
+		params: { nodeId: string | null; handleId: string | null; handleType: 'source' | 'target' | null }
+	) {
+		if (!params.nodeId || !params.handleId || !params.handleType) return;
+		const isOutput = params.handleType === 'source';
+		const port = HANDLE_ID.parseIndex(params.handleId, isOutput ? 'output' : 'input');
+		if (port === null) return;
+		const occupied = new Set(get(graphStore.connections).map((c) => `${c.targetNodeId}:${c.targetPortIndex}`));
+		startConnectionDrag({ nodeId: params.nodeId, port, isOutput }, occupied);
+	}
+
+	// A bus may only enter bus blocks and subsystem ports, and a Bus Selector only takes a bus
+	function isValidConnection(connection: FlowConnection | Edge): boolean {
+		const sourcePort = HANDLE_ID.parseIndex(connection.sourceHandle ?? '', 'output');
+		return sourcePort === null || busWireAllowed(connection.source, sourcePort, connection.target);
+	}
+
 	// Handle new connections
 	function handleConnect(connection: FlowConnection) {
 		if (!connection.source || !connection.target) return;
@@ -973,6 +994,11 @@
 		{nodeTypes}
 		{edgeTypes}
 		onconnect={readonly ? undefined : handleConnect}
+		{isValidConnection}
+		onconnectstart={readonly ? undefined : handleConnectStart}
+		onconnectend={readonly ? undefined : endConnectionDrag}
+		onclickconnectstart={readonly ? undefined : handleConnectStart}
+		onclickconnectend={readonly ? undefined : endConnectionDrag}
 		onnodedragstart={readonly ? undefined : handleNodeDragStart}
 		onnodedrag={readonly ? undefined : handleNodeDrag}
 		onnodedragstop={readonly ? undefined : handleNodeDragStop}

src/lib/components/edges/OrthogonalEdge.svelte

@@ -36,7 +36,7 @@
 	import { GRID_SIZE, EDGE_SOURCE_OFFSET, EDGE_TARGET_OFFSET, EDGE_CORNER_RADIUS } from '$lib/routing/constants';
 	import InlineInput from '$lib/components/InlineInput.svelte';
 	import { BUS } from '$lib/constants/dimensions';
-	import { busWires, busCreatorWires } from '$lib/stores/busView.svelte';
+	import { busWires, busCreatorWires, invalidBusWires } from '$lib/stores/busView.svelte';
 	import type { Direction, RouteResult } from '$lib/routing';
 	import type { Waypoint } from '$lib/types/nodes';
 
@@ -201,6 +201,9 @@
 	// Wires carrying a bus are drawn thicker
 	const carriesBus = $derived(busWires.has(id));
 
+	// Wires breaking the bus rules are drawn as errors
+	const breaksBusRules = $derived(invalidBusWires.has(id));
+
 	// A bus wire starts inside the solid bus port, so the thick line joins it without a gap
 	const adjustedSource = $derived(
 		alongFacing(sourceX, sourceY, sourcePosition, -(carriesBus ? BUS.sourceInset : EDGE_SOURCE_OFFSET))
@@ -403,6 +406,7 @@
 <g
 	class:highlighted={highlightColor !== undefined}
 	class:bus-wire={carriesBus}
+	class:invalid-bus={breaksBusRules}
 	style="{carriesBus ? `--wire-scale: ${BUS.wireScale};` : ''}{highlightColor !== undefined ? ` --highlight-color: ${highlightColor};` : ''}"
 	ondblclick={handleEdgeDoubleClick}
 >
@@ -489,6 +493,16 @@
 		fill: var(--accent);
 	}
 
+	/* A wire breaking the bus rules: dashed in the error color, in every state */
+	.invalid-bus :global(.svelte-flow__edge-path) {
+		stroke: var(--error) !important;
+		stroke-dasharray: 4 3;
+	}
+
+	.invalid-bus .edge-arrow {
+		fill: var(--error) !important;
+	}
+
 	/* Highlight the edge path when handle is hovered */
 	.highlighted :global(.svelte-flow__edge-path) {
 		stroke: var(--highlight-color, var(--accent)) !important;

src/lib/components/nodes/NodePorts.svelte

@@ -5,6 +5,7 @@
 	import { graphStore } from '$lib/stores/graph';
 	import { historyStore } from '$lib/stores/history';
 	import { hoveredHandle } from '$lib/stores/hoveredHandle';
+	import { canTakeDraggedWire } from '$lib/stores/connectionDrag.svelte';
 	import { showTooltip, hideTooltip } from '$lib/components/Tooltip.svelte';
 	import { getPortPositionCalc } from '$lib/constants/dimensions';
 	import { truncatePortLabel } from '$lib/utils/portLabels';
@@ -80,7 +81,8 @@
 
 	const handleClass = (direction: 'input' | 'output', index: number) => {
 		const bus = (direction === 'input' ? busInputs : busOutputs)?.includes(index);
-		return `handle handle-${direction}${bus ? ' handle-bus' : ''}`;
+		const connectable = canTakeDraggedWire(id, index, direction === 'output');
+		return `handle handle-${direction}${bus ? ' handle-bus' : ''}${connectable ? ' handle-connectable' : ''}`;
 	};
 
 	// Calculate actual port positions based on rotation
@@ -397,6 +399,11 @@
 		cursor: not-allowed;
 	}
 
+	/* While a wire is dragged, ports that can take it show their outline in the block color, still hollow */
+	:global(.node .svelte-flow__handle.handle-connectable::before) {
+		background: var(--node-color, var(--accent));
+	}
+
 	/* Ports carrying a bus: the same arrow as other ports with a heavier outline,
 	 * matching the thicker bus wire. Like other ports it fills on hover and selection. */
 	:global(.node .svelte-flow__handle.handle-bus::after) {

src/lib/stores/busView.svelte.ts

@@ -9,11 +9,29 @@
 import { SvelteMap, SvelteSet } from 'svelte/reactivity';
 import type { Connection, NodeInstance } from '$lib/nodes/types';
 import { NODE_TYPES } from '$lib/constants/nodeTypes';
-import { analyzeBuses, containsBusBlocks, signalPaths } from '$lib/bus/expand';
+import {
+	analyzeBuses,
+	busWiringProblem,
+	containsBusBlocks,
+	signalPaths,
+	type BusAnalysis,
+	type BusLevel
+} from '$lib/bus/expand';
 
 /** Wires carrying a bus */
 export const busWires = new SvelteSet<string>();
 
+/** Wires breaking the bus rules: a bus into a plain block, or a plain signal into a Bus Selector */
+export const invalidBusWires = new SvelteSet<string>();
+
+/** Analysis of the model at the last update, reused to judge wires while connecting */
+let current: { analysis: BusAnalysis; level: BusLevel } | null = null;
+
+/** Whether a new wire from the source port to the target node keeps the bus rules */
+export function busWireAllowed(sourceNodeId: string, sourcePort: number, targetNodeId: string): boolean {
+	return !current || busWiringProblem(current.analysis, current.level, sourceNodeId, sourcePort, targetNodeId) === null;
+}
+
 /** Bus Creator ID to the signal name of each of its inputs */
 export const busCreatorSignals = new SvelteMap<string, string[]>();
 
@@ -60,12 +78,18 @@ export function updateBusView(
 	const ports = new Map<string, { inputs: number[]; outputs: number[] }>();
 	const selectorOptions = new Map<string, string[]>();
 	const creatorWires = new Set<string>();
+	const invalidWires = new Set<string>();
+	current = null;
 
 	if (containsBusBlocks(model.nodes)) {
 		const analysis = analyzeBuses(model.nodes, model.connections);
 		const level = analysis.levelAt(path);
 		if (level) {
+			current = { analysis, level };
 			for (const connection of connections) {
+				if (busWiringProblem(analysis, level, connection.sourceNodeId, connection.sourcePortIndex, connection.targetNodeId)) {
+					invalidWires.add(connection.id);
+				}
 				if (analysis.structureOut(level, connection.sourceNodeId, connection.sourcePortIndex)) wires.add(connection.id);
 				if (level.nodes.get(connection.targetNodeId)?.type === NODE_TYPES.BUS_CREATOR) creatorWires.add(connection.id);
 			}
@@ -82,6 +106,7 @@ export function updateBusView(
 	}
 
 	syncSet(busWires, wires);
+	syncSet(invalidBusWires, invalidWires);
 	sync(busCreatorSignals, creators, sameNames);
 	sync(busPorts, ports, (a, b) => sameIndices(a.inputs, b.inputs) && sameIndices(a.outputs, b.outputs));
 	sync(busSelectorOptions, selectorOptions, sameNames);

src/lib/stores/connectionDrag.svelte.ts

@@ -0,0 +1,41 @@
+/**
+ * Connection drag - the port a wire is being dragged from, so ports that can
+ * take the wire are highlighted while connecting
+ */
+
+import { busWireAllowed } from './busView.svelte';
+
+interface DragSource {
+	nodeId: string;
+	port: number;
+	isOutput: boolean;
+}
+
+export const connectionDrag = $state<{ from: DragSource | null; occupiedInputs: Set<string> }>({
+	from: null,
+	occupiedInputs: new Set()
+});
+
+/**
+ * Start highlighting for a wire dragged from a port
+ * @param occupiedInputs - Inputs that already receive a wire, as "nodeId:port"
+ */
+export function startConnectionDrag(from: DragSource, occupiedInputs: Set<string>): void {
+	connectionDrag.occupiedInputs = occupiedInputs;
+	connectionDrag.from = from;
+}
+
+export function endConnectionDrag(): void {
+	connectionDrag.from = null;
+}
+
+/**
+ * Whether a port can take the wire being dragged: a free input for a wire from
+ * an output, any output for a wire from an input, both within the bus rules
+ */
+export function canTakeDraggedWire(nodeId: string, port: number, isOutput: boolean): boolean {
+	const from = connectionDrag.from;
+	if (!from || from.isOutput === isOutput) return false;
+	if (isOutput) return busWireAllowed(nodeId, port, from.nodeId);
+	return !connectionDrag.occupiedInputs.has(`${nodeId}:${port}`) && busWireAllowed(from.nodeId, from.port, nodeId);
+}