mirror of
https://github.com/Nikhil-Doye/workflow-builder.git
synced 2026-07-22 02:01:56 +02:00
Implement node type validation and position collision detection in WorkflowStore
- Added validation for node types to ensure only predefined types can be added to workflows, enhancing data integrity. - Implemented position collision detection to automatically adjust node positions when overlaps occur, improving user experience. - Introduced logging for invalid node type attempts and position adjustments, providing better feedback during workflow creation. - Enhanced tests to cover new validation and collision detection features, ensuring robust functionality.
This commit is contained in:
Nikhil-Doye
@@ -26,6 +26,153 @@ describe("WorkflowStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Node Type Validation", () => {
|
||||
it("should add node with valid type", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
result.current.addNode("llmTask", { x: 100, y: 100 });
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes).toHaveLength(1);
|
||||
expect(result.current.currentWorkflow?.nodes[0].type).toBe("llmTask");
|
||||
expect(result.current.currentWorkflow?.nodes[0].data.label).toBe(
|
||||
"LLM Task"
|
||||
);
|
||||
});
|
||||
|
||||
it("should reject invalid node type", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
result.current.addNode("invalidType", { x: 100, y: 100 });
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes).toHaveLength(0);
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Invalid node type: "invalidType"'),
|
||||
expect.any(String)
|
||||
);
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should accept all valid node types", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
|
||||
const validTypes = [
|
||||
"webScraping",
|
||||
"structuredOutput",
|
||||
"embeddingGenerator",
|
||||
"similaritySearch",
|
||||
"llmTask",
|
||||
"dataInput",
|
||||
"dataOutput",
|
||||
"database",
|
||||
"slack",
|
||||
"discord",
|
||||
"gmail",
|
||||
];
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
validTypes.forEach((type, index) => {
|
||||
result.current.addNode(type, { x: index * 150, y: 100 });
|
||||
});
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes).toHaveLength(
|
||||
validTypes.length
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Position Collision Detection", () => {
|
||||
it("should place node at requested position if no collision", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
result.current.addNode("llmTask", { x: 100, y: 100 });
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes[0].position).toEqual({
|
||||
x: 100,
|
||||
y: 100,
|
||||
});
|
||||
});
|
||||
|
||||
it("should resolve position collision automatically", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
const consoleLogSpy = jest.spyOn(console, "log").mockImplementation();
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
// Add first node
|
||||
result.current.addNode("llmTask", { x: 100, y: 100 });
|
||||
// Try to add second node at same position
|
||||
result.current.addNode("dataInput", { x: 100, y: 100 });
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes).toHaveLength(2);
|
||||
|
||||
const node1Pos = result.current.currentWorkflow?.nodes[0].position;
|
||||
const node2Pos = result.current.currentWorkflow?.nodes[1].position;
|
||||
|
||||
// Positions should be different
|
||||
expect(node1Pos).not.toEqual(node2Pos);
|
||||
|
||||
// Should have logged collision detection
|
||||
expect(consoleLogSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Position collision detected")
|
||||
);
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should handle multiple collisions", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
// Add multiple nodes at similar positions
|
||||
result.current.addNode("llmTask", { x: 100, y: 100 });
|
||||
result.current.addNode("dataInput", { x: 105, y: 105 });
|
||||
result.current.addNode("dataOutput", { x: 110, y: 110 });
|
||||
});
|
||||
|
||||
expect(result.current.currentWorkflow?.nodes).toHaveLength(3);
|
||||
|
||||
// All nodes should have unique positions
|
||||
const positions = result.current.currentWorkflow?.nodes.map(
|
||||
(n) => `${n.position.x},${n.position.y}`
|
||||
);
|
||||
const uniquePositions = new Set(positions);
|
||||
expect(uniquePositions.size).toBe(3);
|
||||
});
|
||||
|
||||
it("should allow nodes far apart without collision", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
const consoleLogSpy = jest.spyOn(console, "log").mockImplementation();
|
||||
|
||||
act(() => {
|
||||
result.current.createWorkflow("Test Workflow");
|
||||
result.current.addNode("llmTask", { x: 100, y: 100 });
|
||||
result.current.addNode("dataInput", { x: 300, y: 100 }); // 200px apart
|
||||
});
|
||||
|
||||
// No collision should be logged for nodes far apart
|
||||
expect(consoleLogSpy).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining("Position collision detected")
|
||||
);
|
||||
|
||||
consoleLogSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("Workflow Management", () => {
|
||||
it("should create a new workflow", () => {
|
||||
const { result } = renderHook(() => useWorkflowStore());
|
||||
|
||||
+125
-2
@@ -26,6 +26,104 @@ const WORKFLOWS_STORAGE_KEY = "agent-workflow-builder-workflows";
|
||||
const generateNodeId = (): string => uuidv4();
|
||||
const generateEdgeId = (): string => uuidv4();
|
||||
|
||||
// Valid node types registry
|
||||
const VALID_NODE_TYPES = new Set<string>([
|
||||
"webScraping",
|
||||
"structuredOutput",
|
||||
"embeddingGenerator",
|
||||
"similaritySearch",
|
||||
"llmTask",
|
||||
"dataInput",
|
||||
"dataOutput",
|
||||
"database",
|
||||
"slack",
|
||||
"discord",
|
||||
"gmail",
|
||||
]);
|
||||
|
||||
// Validate node type
|
||||
const isValidNodeType = (type: string): boolean => {
|
||||
return VALID_NODE_TYPES.has(type);
|
||||
};
|
||||
|
||||
// Position collision detection
|
||||
const COLLISION_THRESHOLD = 80; // pixels - if nodes are within this distance, consider collision
|
||||
const POSITION_OFFSET = 30; // pixels - offset for collision resolution
|
||||
|
||||
const hasPositionCollision = (
|
||||
position: { x: number; y: number },
|
||||
existingNodes: WorkflowNode[]
|
||||
): boolean => {
|
||||
return existingNodes.some((node) => {
|
||||
const dx = Math.abs(node.position.x - position.x);
|
||||
const dy = Math.abs(node.position.y - position.y);
|
||||
return dx < COLLISION_THRESHOLD && dy < COLLISION_THRESHOLD;
|
||||
});
|
||||
};
|
||||
|
||||
const resolvePositionCollision = (
|
||||
position: { x: number; y: number },
|
||||
existingNodes: WorkflowNode[]
|
||||
): { x: number; y: number } => {
|
||||
let resolvedPosition = { ...position };
|
||||
let attempts = 0;
|
||||
const maxAttempts = 10;
|
||||
|
||||
// Try to find a non-colliding position
|
||||
while (
|
||||
hasPositionCollision(resolvedPosition, existingNodes) &&
|
||||
attempts < maxAttempts
|
||||
) {
|
||||
// Try positions in a spiral pattern: right, down, left, up
|
||||
const offset = POSITION_OFFSET * (attempts + 1);
|
||||
|
||||
switch (attempts % 4) {
|
||||
case 0: // Right
|
||||
resolvedPosition = { x: position.x + offset, y: position.y };
|
||||
break;
|
||||
case 1: // Down
|
||||
resolvedPosition = { x: position.x, y: position.y + offset };
|
||||
break;
|
||||
case 2: // Left
|
||||
resolvedPosition = { x: position.x - offset, y: position.y };
|
||||
break;
|
||||
case 3: // Up
|
||||
resolvedPosition = { x: position.x, y: position.y - offset };
|
||||
break;
|
||||
}
|
||||
|
||||
attempts++;
|
||||
}
|
||||
|
||||
// If still colliding after max attempts, use a random offset
|
||||
if (hasPositionCollision(resolvedPosition, existingNodes)) {
|
||||
resolvedPosition = {
|
||||
x: position.x + Math.random() * 100 - 50,
|
||||
y: position.y + Math.random() * 100 - 50,
|
||||
};
|
||||
}
|
||||
|
||||
return resolvedPosition;
|
||||
};
|
||||
|
||||
// Get default label for node type
|
||||
const getDefaultNodeLabel = (type: string): string => {
|
||||
const labels: Record<string, string> = {
|
||||
webScraping: "Web Scraping",
|
||||
structuredOutput: "Structured Output",
|
||||
embeddingGenerator: "Embedding Generator",
|
||||
similaritySearch: "Similarity Search",
|
||||
llmTask: "LLM Task",
|
||||
dataInput: "Data Input",
|
||||
dataOutput: "Data Output",
|
||||
database: "Database",
|
||||
slack: "Slack",
|
||||
discord: "Discord",
|
||||
gmail: "Gmail",
|
||||
};
|
||||
return labels[type] || `${type} Node`;
|
||||
};
|
||||
|
||||
// Create a mapping from deterministic IDs to UUIDs (used for new generation flows)
|
||||
const createIdMapping = (nodeCount: number): Map<string, string> => {
|
||||
const mapping = new Map<string, string>();
|
||||
@@ -505,15 +603,30 @@ export const useWorkflowStore = create<WorkflowStore>((set, get) => ({
|
||||
const { currentWorkflow } = get();
|
||||
if (!currentWorkflow) return;
|
||||
|
||||
// Validate node type
|
||||
if (!isValidNodeType(type)) {
|
||||
console.error(
|
||||
`[WorkflowStore] Invalid node type: "${type}". Valid types:`,
|
||||
Array.from(VALID_NODE_TYPES).join(", ")
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve position collision
|
||||
const resolvedPosition = resolvePositionCollision(
|
||||
position,
|
||||
currentWorkflow.nodes
|
||||
);
|
||||
|
||||
const nodeId = generateNodeId();
|
||||
const newNode: WorkflowNode = {
|
||||
id: nodeId,
|
||||
type,
|
||||
position,
|
||||
position: resolvedPosition,
|
||||
data: {
|
||||
id: nodeId,
|
||||
type: type as any,
|
||||
label: `${type} Node`,
|
||||
label: getDefaultNodeLabel(type),
|
||||
status: "idle",
|
||||
config: {},
|
||||
inputs: [],
|
||||
@@ -521,6 +634,16 @@ export const useWorkflowStore = create<WorkflowStore>((set, get) => ({
|
||||
},
|
||||
};
|
||||
|
||||
// Log if position was adjusted
|
||||
if (
|
||||
resolvedPosition.x !== position.x ||
|
||||
resolvedPosition.y !== position.y
|
||||
) {
|
||||
console.log(
|
||||
`[WorkflowStore] Position collision detected. Adjusted from (${position.x}, ${position.y}) to (${resolvedPosition.x}, ${resolvedPosition.y})`
|
||||
);
|
||||
}
|
||||
|
||||
set((state) => ({
|
||||
currentWorkflow: state.currentWorkflow
|
||||
? {
|
||||
|
||||
Reference in New Issue
Block a user