From eb18d1d708b5158c6e5db45214ef73c5db0534ef Mon Sep 17 00:00:00 2001 From: PriNova Date: Mon, 4 Nov 2024 21:51:09 +0000 Subject: [PATCH] refactor(workflow): migrate to UUID-based node identification * Replace numeric IDs with UUID for workflow nodes * Add comprehensive test coverage for topology sorting * Optimize node output handling and sanitization * Clean up redundant comments and simplify code BREAKING CHANGE: Node ID generation now uses UUID instead of incremental numbers --- .../src/workflow/__tests__/workflow.test.ts | 67 +++++++++++++++++++ vscode/src/workflow/workflow-executor.ts | 23 +++---- vscode/webviews/workflow/components/Flow.tsx | 4 +- .../workflow/components/nodes/Nodes.tsx | 12 ++-- 4 files changed, 85 insertions(+), 21 deletions(-) create mode 100644 vscode/src/workflow/__tests__/workflow.test.ts diff --git a/vscode/src/workflow/__tests__/workflow.test.ts b/vscode/src/workflow/__tests__/workflow.test.ts new file mode 100644 index 00000000000..92f90831122 --- /dev/null +++ b/vscode/src/workflow/__tests__/workflow.test.ts @@ -0,0 +1,67 @@ +import { v4 as uuidv4 } from 'uuid' +import { expect, test } from 'vitest' +import type { NodeType, WorkflowNode } from '../../../webviews/workflow/components/nodes/Nodes' +import { topologicalSort } from '../workflow-executor' + +test('workflow executes correctly with UUID node IDs', () => { + const id1 = uuidv4() + const id2 = uuidv4() + + const nodes: WorkflowNode[] = [ + { + id: id1, + type: 'cli' as NodeType, + data: { label: 'CLI Node', command: 'echo "hello"' }, + position: { x: 0, y: 0 }, + }, + { + id: id2, + type: 'preview' as NodeType, + data: { label: 'Preview Node' }, + position: { x: 0, y: 0 }, + }, + ] + const edges = [{ id: uuidv4(), source: id1, target: id2 }] + + const sortedNodes = topologicalSort(nodes, edges) + expect(sortedNodes[0].id).toBe(id1) + expect(sortedNodes[1].id).toBe(id2) +}) + +test('topology sort maintains order with UUID nodes', () => { + const id1 = uuidv4() + const id2 = uuidv4() + const id3 = uuidv4() + + const nodes: WorkflowNode[] = [ + { + id: id1, + type: 'cli' as NodeType, + data: { label: 'First CLI' }, + position: { x: 0, y: 0 }, + }, + { + id: id2, + type: 'llm' as NodeType, + data: { label: 'LLM Node' }, + position: { x: 0, y: 0 }, + }, + { + id: id3, + type: 'preview' as NodeType, + data: { label: 'Preview' }, + position: { x: 0, y: 0 }, + }, + ] + + const edges = [ + { id: uuidv4(), source: id1, target: id2 }, + { id: uuidv4(), source: id2, target: id3 }, + ] + + const sortedNodes = topologicalSort(nodes, edges) + expect(sortedNodes).toHaveLength(3) + expect(sortedNodes[0].id).toBe(id1) + expect(sortedNodes[1].id).toBe(id2) + expect(sortedNodes[2].id).toBe(id3) +}) diff --git a/vscode/src/workflow/workflow-executor.ts b/vscode/src/workflow/workflow-executor.ts index 789a5d8ba87..cb01ba83d38 100644 --- a/vscode/src/workflow/workflow-executor.ts +++ b/vscode/src/workflow/workflow-executor.ts @@ -15,7 +15,7 @@ interface ExecutionContext { const execAsync = promisify(exec) -function topologicalSort(nodes: WorkflowNode[], edges: Edge[]): WorkflowNode[] { +export function topologicalSort(nodes: WorkflowNode[], edges: Edge[]): WorkflowNode[] { const graph = new Map() const inDegree = new Map() @@ -91,13 +91,14 @@ async function executeCLINode(node: WorkflowNode): Promise { if (stderr) { throw new Error(stderr) } - return stdout + return stdout.replace(/\n$/, '') } catch (error) { throw new Error( `Failed to execute command: ${error instanceof Error ? error.message : String(error)}` ) } } + async function executeLLMNode(node: WorkflowNode, chatClient: ChatClient): Promise { if (!node.data.prompt) { throw new Error(`No prompt specified for LLM node ${node.id} with ${node.data.label}`) @@ -154,11 +155,9 @@ async function executeLLMNode(node: WorkflowNode, chatClient: ChatClient): Promi } async function executePreviewNode(node: WorkflowNode, input: string): Promise { - // Preview nodes just pass through their input return input } -// Add new helper function that maintains edge order as connected function combineParentOutputsByConnectionOrder( nodeId: string, edges: Edge[], @@ -167,9 +166,8 @@ function combineParentOutputsByConnectionOrder( ): string { const parentEdges = edges.filter(edge => edge.target === nodeId) const inputs = parentEdges - .map(edge => context.nodeOutputs.get(edge.source)) - .filter(Boolean) - .map(output => { + .map(edge => { + const output = context.nodeOutputs.get(edge.source) if (output === undefined) { return '' } @@ -182,11 +180,12 @@ function combineParentOutputsByConnectionOrder( } return output }) + .filter(output => output !== undefined) + .join('') - return inputs.join('\n') + return inputs } -// Modify the executeWorkflow function export async function executeWorkflow( nodes: WorkflowNode[], edges: Edge[], @@ -269,13 +268,11 @@ export async function executeWorkflow( } function sanitizeForShell(input: string): string { - // Escape special shell characters - return input.replace(/(["\\'$`])/g, '\\$1').replace(/\n/g, ' ') // Replace newlines with spaces for shell safety + return input.replace(/(["\\'$`])/g, '\\$1').replace(/\n/g, ' ') } function sanitizeForPrompt(input: string): string { - // Basic sanitization for prompt interpolation - return input.replace(/\${/g, '\\${') // Escape template literals + return input.replace(/\${/g, '\\${') } const commandsNotAllowed = [ diff --git a/vscode/webviews/workflow/components/Flow.tsx b/vscode/webviews/workflow/components/Flow.tsx index fd73c9fb626..7fe641fc44a 100644 --- a/vscode/webviews/workflow/components/Flow.tsx +++ b/vscode/webviews/workflow/components/Flow.tsx @@ -159,13 +159,13 @@ export const Flow: React.FC<{ (nodeLabel: string, nodeType: NodeType) => { const { x, y, zoom } = getViewport() const position = { x: -x + 100 * zoom, y: -y + 100 * zoom } - const newNode = createNode(nodeType, nodeLabel, position, nodes.length) + const newNode = createNode(nodeType, nodeLabel, position) if (nodeType === NodeType.PREVIEW) { newNode.data.content = '' } setNodes(nodes => [...nodes, newNode]) }, - [getViewport, nodes] + [getViewport] ) const onExecute = useCallback(() => { // Validate all nodes have required fields diff --git a/vscode/webviews/workflow/components/nodes/Nodes.tsx b/vscode/webviews/workflow/components/nodes/Nodes.tsx index e622ba51ceb..d981d6b5a5e 100644 --- a/vscode/webviews/workflow/components/nodes/Nodes.tsx +++ b/vscode/webviews/workflow/components/nodes/Nodes.tsx @@ -1,5 +1,6 @@ import { Handle, Position } from '@xyflow/react' import type React from 'react' +import { v4 as uuidv4 } from 'uuid' // Core type definitions export enum NodeType { @@ -41,10 +42,9 @@ export interface WorkflowNode { export const createNode = ( type: NodeType, label: string, - position: { x: number; y: number }, - nodeCount: number + position: { x: number; y: number } ): WorkflowNode => ({ - id: String(nodeCount + 1), + id: uuidv4(), type, data: { label, @@ -58,9 +58,9 @@ export const createNode = ( // Default workflow template export const defaultWorkflow = { nodes: [ - createNode(NodeType.CLI, 'Git Diff', { x: 0, y: 0 }, 0), - createNode(NodeType.LLM, 'Cody Generate Commit Message', { x: 0, y: 100 }, 1), - createNode(NodeType.CLI, 'Git Commit', { x: 0, y: 200 }, 2), + createNode(NodeType.CLI, 'Git Diff', { x: 0, y: 0 }), + createNode(NodeType.LLM, 'Cody Generate Commit Message', { x: 0, y: 100 }), + createNode(NodeType.CLI, 'Git Commit', { x: 0, y: 200 }), ], edges: [ { id: 'xy-edge__1-2', source: '1', target: '2' },