Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Prevent circular connections between nodes #321

Merged
merged 10 commits into from
Feb 13, 2025
62 changes: 39 additions & 23 deletions packages/components/properties-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
} from "../contexts/graph";
import { usePropertiesPanel } from "../contexts/properties-panel";
import { useToast } from "../contexts/toast";
import { GraphError } from "../lib/errors";
import { textGenerationPrompt } from "../lib/prompts";
import {
createConnectionId,
Expand Down Expand Up @@ -259,6 +260,7 @@ export function PropertiesPanel() {
const selectedNode = useSelectedNode();
const { open, setOpen, tab, setTab } = usePropertiesPanel();
const { executeNode } = useExecution();
const { addToast } = useToast();
return (
<div
className={clsx(
Expand Down Expand Up @@ -419,34 +421,48 @@ export function PropertiesPanel() {
id: createNodeHandleId(),
label: `Source${selectedNode.content.sources.length + 1}`,
};
dispatch([
{
type: "updateNode",
input: {
nodeId: selectedNode.id,
node: {
...selectedNode,
content: {
...selectedNode.content,
sources: [...selectedNode.content.sources, source],

try {
dispatch([
{
type: "updateNode",
input: {
nodeId: selectedNode.id,
node: {
...selectedNode,
content: {
...selectedNode.content,
sources: [
...selectedNode.content.sources,
source,
],
},
},
},
},
},
{
type: "addConnection",
input: {
connection: {
id: createConnectionId(),
sourceNodeId: sourceNode.id,
sourceNodeType: sourceNode.type,
targetNodeId: selectedNode.id,
targetNodeType: selectedNode.type,
targetNodeHandleId: source.id,
{
type: "addConnection",
input: {
connection: {
id: createConnectionId(),
sourceNodeId: sourceNode.id,
sourceNodeType: sourceNode.type,
targetNodeId: selectedNode.id,
targetNodeType: selectedNode.type,
targetNodeHandleId: source.id,
},
},
},
},
]);
]);
} catch (error) {
addToast({
type: "error",
message:
error instanceof GraphError
? toErrorWithMessage(error).message
: "An unexpected error occurred",
});
}
}}
onSourceRemove={(sourceNode) => {
const connection = graph.connections.find(
Expand Down
20 changes: 13 additions & 7 deletions packages/contexts/graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,16 +260,22 @@ export function GraphContextProvider({

const dispatch = useCallback(
(actionOrActions: GraphActionOrActions) => {
graphRef.current = applyActions(graphRef.current, actionOrActions);
setGraph(graphRef.current);
try {
graphRef.current = applyActions(graphRef.current, actionOrActions);
setGraph(graphRef.current);

isPendingPersistRef.current = true;
isPendingPersistRef.current = true;
} catch (error) {
console.error("Failed to dispatch actions:", error);

if (persistTimeoutRef.current) {
clearTimeout(persistTimeoutRef.current);
}
throw error;
} finally {
if (persistTimeoutRef.current) {
clearTimeout(persistTimeoutRef.current);
}

persistTimeoutRef.current = setTimeout(persist, 500);
persistTimeoutRef.current = setTimeout(persist, 500);
}
},
[persist],
);
Expand Down
11 changes: 11 additions & 0 deletions packages/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,14 @@ export class AgentTimeNotAvailableError extends Error {
}
}
}

export class GraphError extends Error {
constructor(
message: string,
public readonly systemMessage: string,
public readonly code: "CIRCULAR_DEPENDENCY" | "SELF_REFERENCE",
) {
super(message);
this.name = "GraphError";
}
}
218 changes: 216 additions & 2 deletions packages/lib/graph.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import { describe, expect, test } from "vitest";
import type { Flow, Graph } from "../types";
import { deriveFlows, isLatestVersion, migrateGraph } from "./graph";
import type {
Connection,
Graph,
Node,
TextGenerateActionContent,
} from "../types";
import { GraphError } from "./errors";
import {
deriveFlows,
isLatestVersion,
migrateGraph,
validateConnection,
} from "./graph";

// graph is the following structure
// ┌────────────────────────┐ ┌───────────────────┐
Expand Down Expand Up @@ -236,3 +247,206 @@ describe("migrateGraph", () => {
expect(after.nodes[0].content.type).toBe("files");
});
});

describe("validateConnection", () => {
const baseNode = {
name: "Test Node",
position: { x: 0, y: 0 },
selected: false,
content: {
type: "textGeneration" as const,
llm: "anthropic:claude-3-5-sonnet-latest",
temperature: 0.7,
topP: 1,
instruction: "test",
sources: [],
} as TextGenerateActionContent,
};

test("prevents self-reference connections", () => {
const nodes: Node[] = [
{
...baseNode,
id: "nd_node1",
type: "action",
},
];

const selfConnection: Connection = {
id: "cnnc_conn1",
sourceNodeId: "nd_node1",
targetNodeId: "nd_node1",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle1",
};

const result = validateConnection(selfConnection, [], nodes);
expect(result.isValid).toBe(false);
expect(result.error).toBeInstanceOf(GraphError);
expect(result.error?.code).toBe("SELF_REFERENCE");
expect(result.error?.message).toBe("Cannot connect a node to itself");
expect(result.error?.systemMessage).toBe(
"Self-reference connections are not allowed",
);
});

test("prevents self-reference in existing connections", () => {
const nodes: Node[] = [
{
...baseNode,
id: "nd_node1",
type: "action",
},
{
...baseNode,
id: "nd_node2",
type: "action",
},
];

const existingConnections: Connection[] = [
{
id: "cnnc_conn1",
sourceNodeId: "nd_node2",
targetNodeId: "nd_node2", // self-reference in existing connection
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle1",
},
];

const newConnection: Connection = {
id: "cnnc_conn2",
sourceNodeId: "nd_node1",
targetNodeId: "nd_node2",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle2",
};

const result = validateConnection(
newConnection,
existingConnections,
nodes,
);
expect(result.isValid).toBe(false);
expect(result.error).toBeInstanceOf(GraphError);
expect(result.error?.code).toBe("SELF_REFERENCE");
expect(result.error?.message).toBe("Cannot connect a node to itself");
expect(result.error?.systemMessage).toBe(
"Self-reference connections are not allowed",
);
});

test("prevents circular dependencies", () => {
const nodes: Node[] = [
{
...baseNode,
id: "nd_node1",
type: "action",
},
{
...baseNode,
id: "nd_node2",
type: "action",
},
{
...baseNode,
id: "nd_node3",
type: "action",
},
];

const existingConnections: Connection[] = [
{
id: "cnnc_conn1",
sourceNodeId: "nd_node1",
targetNodeId: "nd_node2",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle1",
},
{
id: "cnnc_conn2",
sourceNodeId: "nd_node2",
targetNodeId: "nd_node3",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle2",
},
];

const circularConnection: Connection = {
id: "cnnc_conn3",
sourceNodeId: "nd_node3",
targetNodeId: "nd_node1",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle3",
};

const result = validateConnection(
circularConnection,
existingConnections,
nodes,
);
expect(result.isValid).toBe(false);
expect(result.error).toBeInstanceOf(GraphError);
expect(result.error?.code).toBe("CIRCULAR_DEPENDENCY");
expect(result.error?.message).toBe(
"Cannot create a circular connection between nodes. Please review the connections.",
);
expect(result.error?.systemMessage).toBe(
"Adding this connection would create a circular dependency",
);
});

test("allows valid connections", () => {
const nodes: Node[] = [
{
...baseNode,
id: "nd_node1",
type: "action",
},
{
...baseNode,
id: "nd_node2",
type: "action",
},
{
...baseNode,
id: "nd_node3",
type: "action",
},
];

const existingConnections: Connection[] = [
{
id: "cnnc_conn1",
sourceNodeId: "nd_node1",
targetNodeId: "nd_node2",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle1",
},
];

const validConnection: Connection = {
id: "cnnc_conn2",
sourceNodeId: "nd_node2",
targetNodeId: "nd_node3",
sourceNodeType: "action",
targetNodeType: "action",
targetNodeHandleId: "ndh_handle2",
};

const result = validateConnection(
validConnection,
existingConnections,
nodes,
);
expect(result.isValid).toBe(true);
expect(result.error).toBeUndefined();
});
});
Loading