From 1900bf9b601709382329aad1aaa7dcdabe9cbd69 Mon Sep 17 00:00:00 2001 From: Vukasin Date: Thu, 31 Jul 2025 15:37:24 +0200 Subject: [PATCH] fix: workflow saving --- backend/triggers/api.py | 9 ++ backend/triggers/utils.py | 128 ++++++++++++++---- .../[agentId]/workflow/[workflowId]/page.tsx | 35 +++-- .../conditional-workflow-builder.tsx | 33 +++-- .../workflows/steps/conditional-group.tsx | 99 +++++++++++--- 5 files changed, 243 insertions(+), 61 deletions(-) diff --git a/backend/triggers/api.py b/backend/triggers/api.py index 6b8e7f82..9389b328 100644 --- a/backend/triggers/api.py +++ b/backend/triggers/api.py @@ -90,12 +90,14 @@ class UpcomingRunsResponse(BaseModel): # Workflow models class WorkflowStepRequest(BaseModel): + id: Optional[str] = None # CRITICAL: Accept ID from frontend name: str description: Optional[str] = None type: Optional[str] = "instruction" config: Dict[str, Any] = {} conditions: Optional[Dict[str, Any]] = None order: int + parentConditionalId: Optional[str] = None # CRITICAL: Accept parentConditionalId children: Optional[List['WorkflowStepRequest']] = None @@ -563,6 +565,13 @@ def convert_steps_to_json(steps: List[WorkflowStepRequest]) -> List[Dict[str, An 'conditions': step.conditions, 'order': step.order } + + # CRITICAL: Preserve ID and parentConditionalId if they exist + if hasattr(step, 'id') and step.id: + step_dict['id'] = step.id + if hasattr(step, 'parentConditionalId') and step.parentConditionalId: + step_dict['parentConditionalId'] = step.parentConditionalId + if step.children: step_dict['children'] = convert_steps_to_json(step.children) result.append(step_dict) diff --git a/backend/triggers/utils.py b/backend/triggers/utils.py index 5e7a9c48..44a08fbd 100644 --- a/backend/triggers/utils.py +++ b/backend/triggers/utils.py @@ -20,11 +20,9 @@ class ProviderError(TriggerError): class WorkflowParser: def __init__(self): self.step_counter = 0 - self.parsed_steps = [] def parse_workflow_steps(self, steps: List[Dict[str, Any]]) -> List[Dict[str, Any]]: self.step_counter = 0 - self.parsed_steps = [] start_node = next( (step for step in steps if step.get('name') == 'Start' and step.get('description') == 'Click to add steps or use the Add Node button'), @@ -41,23 +39,98 @@ class WorkflowParser: def _parse_steps_recursive(self, steps: List[Dict[str, Any]]) -> List[Dict[str, Any]]: result = [] + processed_ids = set() for step in steps: - parsed_step = self._parse_single_step(step) - if parsed_step: - result.append(parsed_step) + step_id = step.get('id') + + # Skip if already processed as part of a conditional group + if step_id in processed_ids: + continue + + step_type = step.get('type') + parent_conditional_id = step.get('parentConditionalId') + + if step_type == 'condition' and not parent_conditional_id: + # This is a root conditional step - find all its siblings + conditional_group = [step] + + # Find all siblings (steps with parentConditionalId pointing to this step) + for other_step in steps: + if other_step.get('parentConditionalId') == step_id: + conditional_group.append(other_step) + + # Sort by condition type (if, elseif, else) + conditional_group.sort(key=lambda s: self._get_condition_order(s)) + + # Parse the conditional group as an instruction step with conditions + parsed_group = self._parse_conditional_group(conditional_group) + if parsed_group: + result.append(parsed_group) + + # Mark all steps in group as processed + for group_step in conditional_group: + processed_ids.add(group_step.get('id')) + + elif step_type == 'condition' and parent_conditional_id: + # This step belongs to a parent conditional - skip it + # It will be processed when we encounter the parent + continue + + else: + # Regular instruction step + parsed_step = self._parse_single_step(step) + if parsed_step: + result.append(parsed_step) + processed_ids.add(step_id) return result + def _get_condition_order(self, step: Dict[str, Any]) -> int: + """Sort order for conditions: if=0, elseif=1, else=2""" + condition_type = step.get('conditions', {}).get('type', 'if') + return {'if': 0, 'elseif': 1, 'else': 2}.get(condition_type, 0) + + def _parse_conditional_group(self, conditional_group: List[Dict[str, Any]]) -> Dict[str, Any]: + """Parse a group of related conditional steps (if/elseif/else) as one instruction step""" + if not conditional_group: + return None + + self.step_counter += 1 + + # Use first step's name or generate one + first_step = conditional_group[0] + step_name = first_step.get('name', f'Conditional Step {self.step_counter}') + + parsed_step = { + "step": step_name, + "step_number": self.step_counter + } + + # Add description if meaningful + description = first_step.get('description', '').strip() + if description and description not in ['Add conditional logic', 'If/Then']: + parsed_step["description"] = description + + # Parse all conditions in the group + conditions = [] + for condition_step in conditional_group: + parsed_condition = self._parse_condition_step(condition_step) + if parsed_condition: + conditions.append(parsed_condition) + + if conditions: + parsed_step["conditions"] = conditions + + return parsed_step + def _parse_single_step(self, step: Dict[str, Any]) -> Optional[Dict[str, Any]]: step_type = step.get('type') - step_name = step.get('name', '') if step_type == 'instruction': return self._parse_instruction_step(step) - elif step_type == 'condition': - return self._parse_condition_step(step) else: + # For non-instruction steps, treat as instruction by default return self._parse_instruction_step(step) def _parse_instruction_step(self, step: Dict[str, Any]) -> Dict[str, Any]: @@ -69,7 +142,7 @@ class WorkflowParser: } description = step.get('description', '').strip() - if description: + if description and description not in ['Click to add steps or use the Add Node button', 'Add conditional logic', 'Add a custom instruction step']: parsed_step["description"] = description tool_name = step.get('config', {}).get('tool_name') @@ -82,14 +155,23 @@ class WorkflowParser: children = step.get('children', []) if children: - condition_children = [child for child in children if child.get('type') == 'condition'] - instruction_children = [child for child in children if child.get('type') == 'instruction'] + parsed_children = self._parse_steps_recursive(children) + + # Group children by type - conditions vs regular steps + condition_children = [] + instruction_children = [] + + for child in parsed_children: + if child.get('condition'): # This is a condition + condition_children.append(child) + else: # This is a regular step + instruction_children.append(child) if condition_children: - parsed_step["conditions"] = self._parse_condition_branches(condition_children) + parsed_step["conditions"] = condition_children if instruction_children: - parsed_step["then"] = self._parse_steps_recursive(instruction_children) + parsed_step["then"] = instruction_children return parsed_step @@ -113,16 +195,6 @@ class WorkflowParser: return parsed_condition - def _parse_condition_branches(self, condition_steps: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - branches = [] - - for condition_step in condition_steps: - branch = self._parse_condition_step(condition_step) - if branch: - branches.append(branch) - - return branches - def get_workflow_summary(self, steps: List[Dict[str, Any]]) -> Dict[str, Any]: def count_steps_recursive(steps_list): count = 0 @@ -152,6 +224,9 @@ class WorkflowParser: total_steps, total_conditions, max_nesting_depth = count_steps_recursive(steps) + # Parse the workflow to see the actual output + parsed_steps = self.parse_workflow_steps(steps) + # Large debug print to show inputs and parsed output print("=" * 80) print("WORKFLOW PARSER DEBUG - get_workflow_summary") @@ -160,8 +235,11 @@ class WorkflowParser: print(f"Type: {type(steps)}") print(f"Length: {len(steps) if isinstance(steps, list) else 'N/A'}") print(f"Raw steps: {steps}") - print("-" * 80) - print(f"PARSED OUTPUT:") + print("-" * 40) + print(f"PARSED STEPS:") + print(f"Parsed: {parsed_steps}") + print("-" * 40) + print(f"SUMMARY:") print(f"Total steps: {total_steps}") print(f"Total conditions: {total_conditions}") print(f"Max nesting depth: {max_nesting_depth}") diff --git a/frontend/src/app/(dashboard)/agents/config/[agentId]/workflow/[workflowId]/page.tsx b/frontend/src/app/(dashboard)/agents/config/[agentId]/workflow/[workflowId]/page.tsx index 12b8ada0..3d5db299 100644 --- a/frontend/src/app/(dashboard)/agents/config/[agentId]/workflow/[workflowId]/page.tsx +++ b/frontend/src/app/(dashboard)/agents/config/[agentId]/workflow/[workflowId]/page.tsx @@ -15,21 +15,32 @@ import { ConditionalStep } from '@/components/agents/workflows/conditional-workf import { WorkflowBuilder } from '@/components/workflows/workflow-builder'; const convertToNestedJSON = (steps: ConditionalStep[]): any[] => { - // Since we're using the old structure directly, just pass it through + // Clean, simple conversion - preserve the exact structure with order field for validation let globalOrder = 1; + const convertStepsWithNesting = (stepList: ConditionalStep[]): any[] => { return stepList.map((step) => { + // Build clean step object with required fields for backend validation const jsonStep: any = { - id: step.id, + id: step.id, // CRITICAL: Always include ID name: step.name, description: step.description, type: step.type, - config: step.config, - order: globalOrder++ + config: step.config || {}, + order: globalOrder++ // Required by backend validation }; + + // Add conditional metadata if present if (step.type === 'condition' && step.conditions) { jsonStep.conditions = step.conditions; } + + // Add parent relationship if present + if (step.parentConditionalId) { + jsonStep.parentConditionalId = step.parentConditionalId; + } + + // Add children if present if (step.children && step.children.length > 0) { jsonStep.children = convertStepsWithNesting(step.children); } @@ -37,6 +48,7 @@ const convertToNestedJSON = (steps: ConditionalStep[]): any[] => { return jsonStep; }); }; + return convertStepsWithNesting(steps); }; @@ -57,12 +69,12 @@ const reconstructFromNestedJSON = (nestedSteps: any[]): ConditionalStep[] => { const convertStepsFromNested = (stepList: any[]): ConditionalStep[] => { return stepList.map((step) => { const conditionalStep: ConditionalStep = { - id: step.id || Math.random().toString(36).substr(2, 9), + id: step.id, // Always use the existing ID - never regenerate name: step.name, description: step.description || '', type: step.type || 'instruction', config: step.config || {}, - order: step.order || step.step_order || 0, + order: step.order || 0, // Preserve order from backend enabled: step.enabled !== false, hasIssues: step.hasIssues || false, children: [] // Initialize children array @@ -73,6 +85,11 @@ const reconstructFromNestedJSON = (nestedSteps: any[]): ConditionalStep[] => { conditionalStep.conditions = step.conditions; } + // Handle parentConditionalId for conditional grouping + if (step.parentConditionalId) { + conditionalStep.parentConditionalId = step.parentConditionalId; + } + // Handle children - this is crucial for nested conditions if (step.children && Array.isArray(step.children) && step.children.length > 0) { conditionalStep.children = convertStepsFromNested(step.children); @@ -110,7 +127,7 @@ const reconstructFromFlatJSON = (flatSteps: any[]): ConditionalStep[] => { for (const flatStep of flatSteps) { if (flatStep.type === 'condition') { const conditionStep: ConditionalStep = { - id: flatStep.id || Math.random().toString(36).substr(2, 9), + id: flatStep.id, // Always preserve existing ID name: flatStep.name, description: flatStep.description || '', type: 'condition', @@ -127,7 +144,7 @@ const reconstructFromFlatJSON = (flatSteps: any[]): ConditionalStep[] => { for (const [conditionId, conditionStep] of conditionSteps) { if (JSON.stringify(conditionStep.conditions) === JSON.stringify(flatStep.conditions)) { const childStep: ConditionalStep = { - id: flatStep.id || Math.random().toString(36).substr(2, 9), + id: flatStep.id, // Always preserve existing ID name: flatStep.name, description: flatStep.description || '', type: flatStep.type || 'instruction', @@ -165,7 +182,7 @@ const reconstructFromFlatJSON = (flatSteps: any[]): ConditionalStep[] => { result.push(...conditionGroup); } else if (!flatStep.conditions) { const step: ConditionalStep = { - id: flatStep.id || Math.random().toString(36).substr(2, 9), + id: flatStep.id, // Always preserve existing ID name: flatStep.name, description: flatStep.description || '', type: flatStep.type || 'instruction', diff --git a/frontend/src/components/agents/workflows/conditional-workflow-builder.tsx b/frontend/src/components/agents/workflows/conditional-workflow-builder.tsx index 827f319f..df7c4292 100644 --- a/frontend/src/components/agents/workflows/conditional-workflow-builder.tsx +++ b/frontend/src/components/agents/workflows/conditional-workflow-builder.tsx @@ -97,33 +97,46 @@ export function ConditionalWorkflowBuilder({ order: 0, enabled: true, }; - const updateSteps = (items: ConditionalStep[]): ConditionalStep[] => { + + const updateSteps = (items: ConditionalStep[]): { updatedItems: ConditionalStep[], found: boolean } => { if (!parentId) { if (afterStepId) { const index = items.findIndex(s => s.id === afterStepId); - return [...items.slice(0, index + 1), newStep, ...items.slice(index + 1)]; + return { + updatedItems: [...items.slice(0, index + 1), newStep, ...items.slice(index + 1)], + found: true + }; } - return [...items, newStep]; + return { updatedItems: [...items, newStep], found: true }; } - return items.map(step => { + let found = false; + const updatedItems = items.map(step => { if (step.id === parentId) { + found = true; return { ...step, children: [...(step.children || []), newStep] }; } - if (step.children) { - return { - ...step, - children: updateSteps(step.children) - }; + if (step.children && !found) { + const result = updateSteps(step.children); + if (result.found) { + found = true; + return { + ...step, + children: result.updatedItems + }; + } } return step; }); + + return { updatedItems, found }; }; - onStepsChange(updateSteps(steps)); + const result = updateSteps(steps); + onStepsChange(result.updatedItems); }, [steps, onStepsChange]); const addCondition = useCallback((afterStepId: string) => { diff --git a/frontend/src/components/workflows/steps/conditional-group.tsx b/frontend/src/components/workflows/steps/conditional-group.tsx index 438ba56c..b23e165c 100644 --- a/frontend/src/components/workflows/steps/conditional-group.tsx +++ b/frontend/src/components/workflows/steps/conditional-group.tsx @@ -1,10 +1,18 @@ 'use client'; import React, { useState } from 'react'; -import { Plus, AlertTriangle, GripVertical, Settings } from 'lucide-react'; +import { Plus, AlertTriangle, GripVertical, Settings, X } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { Label } from '@/components/ui/label'; import { Input } from '@/components/ui/input'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; import { ConditionalStep } from '@/components/agents/workflows/conditional-workflow-builder'; import { StepCard } from './step-card'; import { cn } from '@/lib/utils'; @@ -63,6 +71,7 @@ export function ConditionalGroup({ const [activeConditionTab, setActiveConditionTab] = useState(conditionSteps[0]?.id || ''); const [activeDragId, setActiveDragId] = useState(null); const [pendingElseStepAdd, setPendingElseStepAdd] = useState(false); + const [deleteConfirmStep, setDeleteConfirmStep] = useState(null); // Set up sensors for drag and drop within this conditional group const sensors = useSensors( @@ -197,6 +206,25 @@ export function ConditionalGroup({ } }, [activeStep, onAddElse, conditionSteps, onAddStep]); + // Handle delete confirmation + const handleDeleteConfirm = React.useCallback(() => { + if (deleteConfirmStep) { + // Switch to another tab before deleting if we're on the tab being deleted + if (activeConditionTab === deleteConfirmStep.id) { + const remainingSteps = conditionSteps.filter(s => s.id !== deleteConfirmStep.id); + if (remainingSteps.length > 0) { + setActiveConditionTab(remainingSteps[0].id); + } + } + onRemove(deleteConfirmStep.id); + setDeleteConfirmStep(null); + } + }, [deleteConfirmStep, activeConditionTab, conditionSteps, onRemove]); + + const handleDeleteCancel = React.useCallback(() => { + setDeleteConfirmStep(null); + }, []); + return (
@@ -232,24 +260,39 @@ export function ConditionalGroup({ step.conditions?.type === 'else' ? 'Else' : 'If'; return ( - - {letter} - • - {conditionType} - {step.hasIssues && ( - + {/* Delete button - only for else-if tabs */} + {step.conditions?.type === 'elseif' && ( + )} - +
); })} @@ -380,6 +423,8 @@ export function ConditionalGroup({ }; const handleNestedAddStep = (index: number, parentStepId?: string) => { + // Call the parent's onAddStep which will open the side panel + // Pass the parentStepId correctly for nested context onAddStep(index, parentStepId); }; @@ -505,6 +550,26 @@ export function ConditionalGroup({
)} + + {/* Delete Confirmation Dialog */} + !open && handleDeleteCancel()}> + + + Delete Condition + + Are you sure you want to delete this "Else If" condition? This action cannot be undone and will remove all steps within this condition. + + + + + + + + ); } \ No newline at end of file