Skip to content

Commit

Permalink
refactor(workflow): migrate to UUID-based node identification
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
PriNova committed Nov 4, 2024
1 parent dbd5d1b commit eb18d1d
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 21 deletions.
67 changes: 67 additions & 0 deletions vscode/src/workflow/__tests__/workflow.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
23 changes: 10 additions & 13 deletions vscode/src/workflow/workflow-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string[]>()
const inDegree = new Map<string, number>()

Expand Down Expand Up @@ -91,13 +91,14 @@ async function executeCLINode(node: WorkflowNode): Promise<string> {
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<string> {
if (!node.data.prompt) {
throw new Error(`No prompt specified for LLM node ${node.id} with ${node.data.label}`)
Expand Down Expand Up @@ -154,11 +155,9 @@ async function executeLLMNode(node: WorkflowNode, chatClient: ChatClient): Promi
}

async function executePreviewNode(node: WorkflowNode, input: string): Promise<string> {
// 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[],
Expand All @@ -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 ''
}
Expand All @@ -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[],
Expand Down Expand Up @@ -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 = [
Expand Down
4 changes: 2 additions & 2 deletions vscode/webviews/workflow/components/Flow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions vscode/webviews/workflow/components/nodes/Nodes.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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' },
Expand Down

0 comments on commit eb18d1d

Please sign in to comment.