fix: incorrect phase reduction order

This commit is contained in:
Gerla, J. (Justin)
2026-01-06 15:12:00 +00:00
committed by Pim Hutting
parent 9c80391fea
commit 9b3414ba98
13 changed files with 520 additions and 79 deletions

View File

@@ -9,8 +9,10 @@ import {
import '@xyflow/react/dist/style.css';
import {useEffect} from "react";
import {useShallow} from 'zustand/react/shallow';
import orderPhaseNodeArray from "../../utils/orderPhaseNodes.ts";
import useProgramStore from "../../utils/programStore.ts";
import {DndToolbar} from './visualProgrammingUI/components/DragDropSidebar.tsx';
import type {PhaseNode} from "./visualProgrammingUI/nodes/PhaseNode.tsx";
import useFlowStore from './visualProgrammingUI/VisProgStores.tsx';
import type {FlowState} from './visualProgrammingUI/VisProgTypes.tsx';
import styles from './VisProg.module.css'
@@ -165,14 +167,15 @@ function runProgram() {
*/
function graphReducer() {
const { nodes } = useFlowStore.getState();
return nodes
.filter((n) => n.type == 'phase')
return orderPhaseNodeArray(nodes.filter((n) => n.type == 'phase') as PhaseNode [])
.map((n) => {
const reducer = NodeReduces['phase'];
return reducer(n, nodes)
});
}
/**
* houses the entire page, so also UI elements
* that are not a part of the Visual Programming UI

View File

@@ -39,10 +39,10 @@ export const UndoRedo = (
* @param {BaseFlowState} state - the current state of the editor
* @returns {FlowSnapshot} - returns a snapshot of the current editor state
*/
const getSnapshot = (state : BaseFlowState) : FlowSnapshot => ({
const getSnapshot = (state : BaseFlowState) : FlowSnapshot => (structuredClone({
nodes: state.nodes,
edges: state.edges
});
}));
const initialState = config(set, get, api);

View File

@@ -28,33 +28,28 @@ import { UndoRedo } from "./EditorUndoRedo.ts";
* @param deletable - Optional flag to indicate if the node can be deleted (can be deleted by default).
* @returns A fully initialized Node object ready to be added to the flow.
*/
function createNode(id: string, type: string, position: XYPosition, data: Record<string, unknown>, deletable?: boolean) {
const defaultData = NodeDefaults[type as keyof typeof NodeDefaults]
return {
id,
type,
position,
deletable,
data: {
...defaultData,
...data,
},
}
function createNode(id: string, type: string, position: XYPosition, data: Record<string, unknown>, deletable? : boolean) {
const defaultData = NodeDefaults[type as keyof typeof NodeDefaults]
return {
id: id,
type: type,
position: position,
data: {...defaultData, ...data},
deletable: deletable
}
}
//* Initial nodes, created by using createNode. */
const initialNodes : Node[] = [
createNode('start', 'start', {x: 100, y: 100}, {label: "Start"}, false),
createNode('end', 'end', {x: 500, y: 100}, {label: "End"}, false),
createNode('phase-1', 'phase', {x:200, y:100}, {label: "Phase 1", children : []}),
createNode('phase-1', 'phase', {x:200, y:100}, {label: "Phase 1", children : [], isFirstPhase: false, nextPhaseId: null}),
createNode('norms-1', 'norm', {x:-200, y:100}, {label: "Initial Norms", normList: ["Be a robot", "get good"], critical:false}),
];
// * Initial edges * /
const initialEdges: Edge[] = [
{ id: 'start-phase-1', source: 'start', target: 'phase-1' },
{ id: 'phase-1-end', source: 'phase-1', target: 'end' },
];
const initialEdges: Edge[] = []; // no initial edges as edge connect events don't fire when using initial edges
/**

View File

@@ -0,0 +1,30 @@
import {
Handle,
useNodeConnections,
type HandleType,
type Position
} from '@xyflow/react';
const LimitedConnectionCountHandle = (props: {
node_id: string,
type: HandleType,
position: Position,
connection_count: number,
id?: string
}) => {
const connections = useNodeConnections({
id: props.node_id,
handleType: props.type,
handleId: props.id,
});
return (
<Handle
{...props}
isConnectable={connections.length < props.connection_count}
/>
);
};
export default LimitedConnectionCountHandle;

View File

@@ -1,9 +1,9 @@
import {
Handle,
type NodeProps,
Position,
type Node,
} from '@xyflow/react';
import LimitedConnectionCountHandle from "../components/CustomNodeHandles.tsx";
import { Toolbar } from '../components/NodeComponents';
import styles from '../../VisProg.module.css';
@@ -32,7 +32,13 @@ export default function EndNode(props: NodeProps<EndNode>) {
<div className={"flex-row gap-sm"}>
End
</div>
<Handle type="target" position={Position.Left} id="target"/>
<LimitedConnectionCountHandle
node_id={props.id}
type="target"
position={Position.Left}
connection_count={1}
id="target"
/>
</div>
</>
);

View File

@@ -8,4 +8,6 @@ export const PhaseNodeDefaults: PhaseNodeData = {
droppable: true,
children: [],
hasReduce: true,
nextPhaseId: null,
isFirstPhase: false,
};

View File

@@ -9,6 +9,7 @@ import styles from '../../VisProg.module.css';
import { NodeReduces, NodesInPhase, NodeTypes} from '../NodeRegistry';
import useFlowStore from '../VisProgStores';
import { TextField } from '../../../../components/TextField';
import LimitedConnectionCountHandle from "../components/CustomNodeHandles.tsx";
/**
* The default data dot a phase node
@@ -16,12 +17,15 @@ import { TextField } from '../../../../components/TextField';
* @param droppable: whether this node is droppable from the drop bar (initialized as true)
* @param children: ID's of children of this node
* @param hasReduce: whether this node has reducing functionality (true by default)
* @param nextPhaseId:
*/
export type PhaseNodeData = {
label: string;
droppable: boolean;
children: string[];
hasReduce: boolean;
nextPhaseId: string | "end" | null;
isFirstPhase: boolean;
};
export type PhaseNode = Node<PhaseNodeData>
@@ -50,9 +54,21 @@ export default function PhaseNode(props: NodeProps<PhaseNode>) {
placeholder={"Phase ..."}
/>
</div>
<Handle type="target" position={Position.Left} id="target"/>
<LimitedConnectionCountHandle
node_id={props.id}
type="target"
position={Position.Left}
connection_count={1}
id="target"
/>
<Handle type="target" position={Position.Bottom} id="norms"/>
<Handle type="source" position={Position.Right} id="source"/>
<LimitedConnectionCountHandle
node_id={props.id}
type="source"
position={Position.Right}
connection_count={1}
id="source"
/>
</div>
</>
);
@@ -65,8 +81,8 @@ export default function PhaseNode(props: NodeProps<PhaseNode>) {
* @returns A collection of all reduced nodes in this phase, starting with this phases' reduced data.
*/
export function PhaseReduce(node: Node, nodes: Node[]) {
const thisnode = node as PhaseNode;
const data = thisnode.data as PhaseNodeData;
const thisNode = node as PhaseNode;
const data = thisNode.data as PhaseNodeData;
// node typings that are not in phase
const nodesNotInPhase: string[] = Object.entries(NodesInPhase)
@@ -85,8 +101,8 @@ export function PhaseReduce(node: Node, nodes: Node[]) {
// Build the result object
const result: Record<string, unknown> = {
id: thisnode.id,
label: data.label,
id: thisNode.id,
label: data.label,
};
nodesInPhase.forEach((type) => {
@@ -109,13 +125,19 @@ export function PhaseReduce(node: Node, nodes: Node[]) {
* @param _sourceNodeId the source of the received connection
*/
export function PhaseConnectionTarget(_thisNode: Node, _sourceNodeId: string) {
const node = _thisNode as PhaseNode
const data = node.data as PhaseNodeData
// we only add none phase nodes to the children
if (!(useFlowStore.getState().nodes.find((node) => node.id === _sourceNodeId && node.type === 'phase'))) {
data.children.push(_sourceNodeId)
}
const data = _thisNode.data as PhaseNodeData
const nodes = useFlowStore.getState().nodes;
const sourceNode = nodes.find((node) => node.id === _sourceNodeId)!
switch (sourceNode.type) {
case "phase": break;
case "start": data.isFirstPhase = true; break;
// we only add none phase or start nodes to the children
// endNodes cannot be the source of an outgoing connection
// so we don't need to cover them with a special case
// before handling the default behavior
default: data.children.push(_sourceNodeId); break;
}
}
/**
@@ -124,7 +146,19 @@ export function PhaseConnectionTarget(_thisNode: Node, _sourceNodeId: string) {
* @param _targetNodeId the target of the created connection
*/
export function PhaseConnectionSource(_thisNode: Node, _targetNodeId: string) {
// no additional connection logic exists yet
const data = _thisNode.data as PhaseNodeData
const nodes = useFlowStore.getState().nodes;
const targetNode = nodes.find((node) => node.id === _targetNodeId)
if (!targetNode) {throw new Error("Source node not found")}
// we set the nextPhaseId to the next target's id if the target is a phaseNode,
// or "end" if the target node is the end node
switch (targetNode.type) {
case 'phase': data.nextPhaseId = _targetNodeId; break;
case 'end': data.nextPhaseId = "end"; break;
default: break;
}
}
/**
@@ -133,9 +167,23 @@ export function PhaseConnectionSource(_thisNode: Node, _targetNodeId: string) {
* @param _sourceNodeId the source of the disconnected connection
*/
export function PhaseDisconnectionTarget(_thisNode: Node, _sourceNodeId: string) {
const node = _thisNode as PhaseNode
const data = node.data as PhaseNodeData
data.children = data.children.filter((child) => { if (child != _sourceNodeId) return child; });
const data = _thisNode.data as PhaseNodeData
const nodes = useFlowStore.getState().nodes;
const sourceNode = nodes.find((node) => node.id === _sourceNodeId)
const sourceType = sourceNode ? sourceNode.type : "deleted";
switch (sourceType) {
case "phase": break;
case "start": data.isFirstPhase = false; break;
// we only add none phase or start nodes to the children
// endNodes cannot be the source of an outgoing connection
// so we don't need to cover them with a special case
// before handling the default behavior
default:
data.children = data.children.filter((child) => { if (child != _sourceNodeId) return child; });
break;
}
}
/**
@@ -144,5 +192,12 @@ export function PhaseDisconnectionTarget(_thisNode: Node, _sourceNodeId: string)
* @param _targetNodeId the target of the diconnected connection
*/
export function PhaseDisconnectionSource(_thisNode: Node, _targetNodeId: string) {
// no additional connection logic exists yet
const data = _thisNode.data as PhaseNodeData
const nodes = useFlowStore.getState().nodes;
// if the target is a phase or end node set the nextPhaseId to null,
// as we are no longer connected to a subsequent phaseNode or to the endNode
if (nodes.some((node) => node.id === _targetNodeId && ['phase', 'end'].includes(node.type!))){
data.nextPhaseId = null;
}
}

View File

@@ -1,9 +1,9 @@
import {
Handle,
type NodeProps,
Position,
type Node,
} from '@xyflow/react';
import LimitedConnectionCountHandle from "../components/CustomNodeHandles.tsx";
import { Toolbar } from '../components/NodeComponents';
import styles from '../../VisProg.module.css';
@@ -31,7 +31,13 @@ export default function StartNode(props: NodeProps<StartNode>) {
<div className={"flex-row gap-sm"}>
Start
</div>
<Handle type="source" position={Position.Right} id="source"/>
<LimitedConnectionCountHandle
node_id={props.id}
type="source"
position={Position.Right}
connection_count={1}
id="source"
/>
</div>
</>
);

View File

@@ -0,0 +1,40 @@
import type {PhaseNode} from "../pages/VisProgPage/visualProgrammingUI/nodes/PhaseNode.tsx";
/**
* takes an array of phaseNodes and orders them according to their nextPhaseId attributes,
* starting with the phase that has isFirstPhase = true
*
* @param {PhaseNode[]} nodes an unordered phaseNode array
* @returns {PhaseNode[]} the ordered phaseNode array
*/
export default function orderPhaseNodeArray(nodes: PhaseNode[]) : PhaseNode[] {
// find the first phaseNode of the sequence
const start = nodes.find(node => node.data.isFirstPhase);
if (!start) {
throw new Error('No phaseNode with isFirstObject = true found');
}
// prepare for ordering of phaseNodes
const orderedPhaseNodes: PhaseNode[] = [];
const IdMap = new Map(nodes.map(node => [node.id, node]));
let currentNode: PhaseNode | undefined = start;
// populate orderedPhaseNodes array with the phaseNodes in the correct order
while (currentNode) {
orderedPhaseNodes.push(currentNode);
if (!currentNode.data.nextPhaseId) {
throw new Error("Incomplete phase sequence, program does not reach the end node");
}
if (currentNode.data.nextPhaseId === "end") break;
currentNode = IdMap.get(currentNode.data.nextPhaseId);
if (!currentNode) {
throw new Error(`Incomplete phase sequence, phaseNode with id "${orderedPhaseNodes.at(-1)?.data.nextPhaseId}" not found`);
}
}
return orderedPhaseNodes;
}

View File

@@ -3,6 +3,9 @@ import useFlowStore from '../../../../src/pages/VisProgPage/visualProgrammingUI/
import { mockReactFlow } from '../../../setupFlowTests.ts';
beforeAll(() => {
mockReactFlow();
});

View File

@@ -1,8 +1,10 @@
import type { Node, Edge, Connection } from '@xyflow/react'
import useFlowStore from '../../../../../src/pages/VisProgPage/visualProgrammingUI/VisProgStores';
import type { PhaseNodeData } from "../../../../../src/pages/VisProgPage/visualProgrammingUI/nodes/PhaseNode";
import { getByTestId, render } from '@testing-library/react';
import type {PhaseNode, PhaseNodeData} from "../../../../../src/pages/VisProgPage/visualProgrammingUI/nodes/PhaseNode";
import {act, getByTestId, render} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import VisProgPage from '../../../../../src/pages/VisProgPage/VisProg';
import {mockReactFlow} from "../../../../setupFlowTests.ts";
class ResizeObserver {
@@ -99,3 +101,194 @@ describe('PhaseNode', () => {
expect(p2_data.children.length == 2);
});
});
// --| Helper functions |--
function createPhaseNode(
id: string,
overrides: Partial<PhaseNodeData> = {},
): Node<PhaseNodeData> {
return {
id: id,
type: 'phase',
position: { x: 0, y: 0 },
data: {
label: 'Phase',
droppable: true,
children: [],
hasReduce: true,
nextPhaseId: null,
isFirstPhase: false,
...overrides,
},
}
}
function createNode(id: string, type: string): Node {
return {
id: id,
type: type,
position: { x: 0, y: 0 },
data: {},
}
}
function connect(source: string, target: string): Connection {
return {
source: source,
target: target,
sourceHandle: null,
targetHandle: null
};
}
function edge(source: string, target: string): Edge {
return {
id: `${source}-${target}`,
source: source,
target: target,
}
}
// --| Connection Tests |--
describe('PhaseNode Connection logic', () => {
beforeAll(() => {
mockReactFlow();
});
describe('PhaseConnections', () => {
test('connecting start => phase sets isFirstPhase to true', () => {
const phase = createPhaseNode('phase-1')
const start = createNode('start', 'start')
useFlowStore.setState({ nodes: [phase, start] })
// verify it starts of false
expect(phase.data.isFirstPhase).toBe(false);
act(() => {
useFlowStore.getState().onConnect(connect('start', 'phase-1'))
})
const updatedPhase = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedPhase.data.isFirstPhase).toBe(true)
})
test('connecting task => phase adds child', () => {
const phase = createPhaseNode('phase-1')
const norm = createNode('norm-1', 'norm')
useFlowStore.setState({ nodes: [phase, norm] })
act(() => {
useFlowStore.getState().onConnect(connect('norm-1', 'phase-1'))
})
const updatedPhase = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedPhase.data.children).toEqual(['norm-1'])
})
test('connecting phase => phase sets nextPhaseId', () => {
const p1 = createPhaseNode('phase-1')
const p2 = createPhaseNode('phase-2')
useFlowStore.setState({ nodes: [p1, p2] })
act(() => {
useFlowStore.getState().onConnect(connect('phase-1', 'phase-2'))
})
const updatedP1 = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedP1.data.nextPhaseId).toBe('phase-2')
})
test('connecting phase to end => phase sets nextPhaseId to "end"', () => {
const phase = createPhaseNode('phase-1')
const end = createNode('end', 'end')
useFlowStore.setState({ nodes: [phase, end] })
act(() => {
useFlowStore.getState().onConnect(connect('phase-1', 'end'))
})
const updatedPhase = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedPhase.data.nextPhaseId).toBe('end')
})
})
describe('PhaseDisconnections', () => {
test('disconnecting task => phase removes child', () => {
const phase = createPhaseNode('phase-1', { children: ['norm-1'] })
const norm = createNode('norm-1', 'norm')
useFlowStore.setState({
nodes: [phase, norm],
edges: [edge('norm-1', 'phase-1')]
})
act(() => {
useFlowStore.getState().onEdgesDelete([edge('norm-1', 'phase-1')])
})
const updatedPhase = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedPhase.data.children).toEqual([])
})
test('disconnecting start => phase sets isFirstPhase to false', () => {
const phase = createPhaseNode('phase-1', { isFirstPhase: true })
const start = createNode('start', 'start')
useFlowStore.setState({
nodes: [phase, start],
edges: [edge('start', 'phase-1')]
})
act(() => {
useFlowStore.getState().onEdgesDelete([edge('start', 'phase-1')])
})
const updatedPhase = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedPhase.data.isFirstPhase).toBe(false)
})
test('disconnecting phase => phase sets nextPhaseId to null', () => {
const p1 = createPhaseNode('phase-1', { nextPhaseId: 'phase-2' })
const p2 = createPhaseNode('phase-2')
useFlowStore.setState({
nodes: [p1, p2],
edges: [edge('phase-1', 'phase-2')]
})
act(() => {
useFlowStore.getState().onEdgesDelete([edge('phase-1', 'phase-2')])
})
const updatedP1 = useFlowStore
.getState()
.nodes.find((n) => n.id === 'phase-1') as PhaseNode
expect(updatedP1.data.nextPhaseId).toBeNull()
})
})
})

View File

@@ -13,17 +13,15 @@ describe('NormNode', () => {
jest.clearAllMocks();
});
function createNode(id: string, type: string, position: XYPosition, data: Record<string, unknown>, deletable?: boolean) {
function createNode(id: string, type: string, position: XYPosition, data: Record<string, unknown>, deletable? : boolean) {
const defaultData = NodeDefaults[type as keyof typeof NodeDefaults]
return {
id,
type,
position,
deletable,
data: {
...defaultData,
...data,
},
id: id,
type: type,
position: position,
data: {...defaultData, ...data},
deletable: deletable
}
}
@@ -47,34 +45,34 @@ describe('NormNode', () => {
describe('Rendering', () => {
test.each(getAllTypes())('it should render %s node with the default data', (nodeType) => {
const lengthBefore = screen.getAllByText(/.*/).length;
const lengthBefore = screen.getAllByText(/.*/).length;
const newNode = createNode(nodeType + "1", nodeType, {x: 200, y:200}, {});
const newNode = createNode(nodeType + "1", nodeType, {x: 200, y:200}, {});
const found = Object.entries(NodeTypes).find(([t]) => t === nodeType);
const uiElement = found ? found[1] : null;
const found = Object.entries(NodeTypes).find(([t]) => t === nodeType);
const uiElement = found ? found[1] : null;
expect(uiElement).not.toBeNull();
const props = {
id: newNode.id,
type: newNode.type as string,
data: newNode.data as any,
selected: false,
isConnectable: true,
zIndex: 0,
dragging: false,
selectable: true,
deletable: true,
draggable: true,
positionAbsoluteX: 0,
positionAbsoluteY: 0,
};
expect(uiElement).not.toBeNull();
const props = {
id: newNode.id,
type: newNode.type as string,
data: newNode.data as any,
selected: false,
isConnectable: true,
zIndex: 0,
dragging: false,
selectable: true,
deletable: true,
draggable: true,
positionAbsoluteX: 0,
positionAbsoluteY: 0,
};
renderWithProviders(createElement(uiElement as React.ComponentType<any>, props));
const lengthAfter = screen.getAllByText(/.*/).length;
renderWithProviders(createElement(uiElement as React.ComponentType<any>, props));
const lengthAfter = screen.getAllByText(/.*/).length;
expect(lengthBefore + 1 === lengthAfter);
});
expect(lengthBefore + 1 === lengthAfter);
});
});

View File

@@ -0,0 +1,110 @@
import type {PhaseNode} from "../../src/pages/VisProgPage/visualProgrammingUI/nodes/PhaseNode.tsx";
import orderPhaseNodeArray from "../../src/utils/orderPhaseNodes.ts";
function createPhaseNode(
id: string,
isFirst: boolean = false,
nextPhaseId: string | null = null
): PhaseNode {
return {
id: id,
type: 'phase',
position: { x: 0, y: 0 },
data: {
label: 'Phase',
droppable: true,
children: [],
hasReduce: true,
nextPhaseId: nextPhaseId,
isFirstPhase: isFirst,
},
}
}
describe("orderPhaseNodes", () => {
test.each([
{
testCase: {
testName: "Throws correct error when there is no first phase (empty input array)",
input: [],
expected: "No phaseNode with isFirstObject = true found"
}
},{
testCase: {
testName: "Throws correct error when there is no first phase",
input: [
createPhaseNode("phase-1", false, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, "end")
],
expected: "No phaseNode with isFirstObject = true found"
}
},{
testCase: {
testName: "Throws correct error when the program doesn't lead to an end node (missing phase-phase connection)",
input: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, null),
createPhaseNode("phase-3", false, "end")
],
expected: "Incomplete phase sequence, program does not reach the end node"
}
},{
testCase: {
testName: "Throws correct error when the program doesn't lead to an end node (missing phase-end connection)",
input: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, null)
],
expected: "Incomplete phase sequence, program does not reach the end node"
}
},{
testCase: {
testName: "Throws correct error when the program leads to a non-existent phase",
input: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, "phase-4")
],
expected: "Incomplete phase sequence, phaseNode with id \"phase-4\" not found"
}
}
])(`Error Handling: $testCase.testName`, ({testCase}) => {
expect(() => { orderPhaseNodeArray(testCase.input) }).toThrow(testCase.expected);
})
test.each([
{
testCase: {
testName: "Already correctly ordered phases stay ordered",
input: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, "end")
],
expected: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, "end")
]
}
},{
testCase: {
testName: "Incorrectly ordered phases get ordered correctly",
input: [
createPhaseNode("phase-3", false, "end"),
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
],
expected: [
createPhaseNode("phase-1", true, "phase-2"),
createPhaseNode("phase-2", false, "phase-3"),
createPhaseNode("phase-3", false, "end")
]
}
}
])(`Functional: $testCase.testName`, ({testCase}) => {
const output = orderPhaseNodeArray(testCase.input);
expect(output).toEqual(testCase.expected);
})
})