From 778c3b4a5bea48e36e0c35ae683810b6dc9f1d5c Mon Sep 17 00:00:00 2001 From: David Kaya Date: Tue, 7 Apr 2026 11:04:38 +0200 Subject: [PATCH] fix: synchronize workflow iteration settings across edges, mode settings, and scaffold For builder-based orchestration modes (group-chat, handoff), the loop edge maxIterations, workflow settings.maxIterations, and mode-specific settings (e.g. groupChat.maxRounds) were independently editable but only the mode settings controlled runtime behavior. This caused confusion when changing one did not update the others. Changes: - Add syncBuilderModeEdgeIterations() to keep loop edge maxIterations in sync with the authoritative mode settings for builder-based modes - Wire sync into normalizeWorkflowDefinition so edges are consistent on load/normalization - Update scaffoldGraphForMode to accept optional settings instead of hardcoded iteration values (4 for handoff, 5 for group-chat) - Sync settings.maxIterations <-> groupChat.maxRounds bidirectionally when either changes in the UI - Make loop edge controls read-only in the graph inspector for builder-based modes with explanatory text - Add 8 consistency tests validating built-in workflows, scaffold behavior, and normalization sync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/renderer/components/WorkflowEditor.tsx | 25 +++- .../workflow/OrchestrationModePanel.tsx | 36 +++--- .../workflow/WorkflowGraphInspector.tsx | 19 +++- src/shared/domain/workflow.ts | 65 +++++++++-- tests/shared/workflow.test.ts | 107 ++++++++++++++++++ 5 files changed, 226 insertions(+), 26 deletions(-) diff --git a/src/renderer/components/WorkflowEditor.tsx b/src/renderer/components/WorkflowEditor.tsx index 778873d..577b1a2 100644 --- a/src/renderer/components/WorkflowEditor.tsx +++ b/src/renderer/components/WorkflowEditor.tsx @@ -13,7 +13,7 @@ import type { SubWorkflowConfig, WorkflowStateScope, } from '@shared/domain/workflow'; -import { validateWorkflowDefinition } from '@shared/domain/workflow'; +import { validateWorkflowDefinition, isBuilderBasedMode, syncBuilderModeEdgeIterations } from '@shared/domain/workflow'; import { createId } from '@shared/utils/ids'; import { ToggleSwitch } from '@renderer/components/ui'; @@ -703,13 +703,30 @@ function WorkflowSettingsPanel({ min={1} onChange={(e) => { const raw = parseInt(e.target.value, 10); - onChange({ + const maxIterations = Number.isNaN(raw) ? undefined : raw; + const updated: WorkflowDefinition = { ...workflow, settings: { ...workflow.settings, - maxIterations: Number.isNaN(raw) ? undefined : raw, + maxIterations, + ...(workflow.settings.orchestrationMode === 'group-chat' && maxIterations !== undefined + ? { + modeSettings: { + ...workflow.settings.modeSettings, + groupChat: { + ...(workflow.settings.modeSettings?.groupChat ?? { selectionStrategy: 'round-robin' as const }), + maxRounds: maxIterations, + }, + }, + } + : {}), }, - }); + }; + onChange( + isBuilderBasedMode(workflow.settings.orchestrationMode) + ? syncBuilderModeEdgeIterations(updated) + : updated, + ); }} placeholder="e.g. 5" type="number" diff --git a/src/renderer/components/workflow/OrchestrationModePanel.tsx b/src/renderer/components/workflow/OrchestrationModePanel.tsx index 24ffc24..47228fc 100644 --- a/src/renderer/components/workflow/OrchestrationModePanel.tsx +++ b/src/renderer/components/workflow/OrchestrationModePanel.tsx @@ -23,6 +23,7 @@ import { isBuilderBasedMode, isGraphBasedMode, scaffoldGraphForMode, + syncBuilderModeEdgeIterations, } from '@shared/domain/workflow'; import { FormField, InfoCallout, SelectInput, ToggleSwitch } from '@renderer/components/ui'; @@ -332,23 +333,30 @@ export function OrchestrationModePanel({ const applyModeChange = useCallback( (newMode: WorkflowOrchestrationMode, restructure: boolean) => { const modeSettings = createDefaultModeSettings(newMode); - const base: WorkflowDefinition = { - ...workflow, - settings: { - ...workflow.settings, - orchestrationMode: newMode, - modeSettings: modeSettings - ? { ...workflow.settings.modeSettings, ...modeSettings } - : workflow.settings.modeSettings, - }, + const mergedSettings = { + ...workflow.settings, + orchestrationMode: newMode, + modeSettings: modeSettings + ? { ...workflow.settings.modeSettings, ...modeSettings } + : workflow.settings.modeSettings, }; + // Sync maxIterations from mode-specific settings for builder-based modes + if (newMode === 'group-chat' && modeSettings?.groupChat) { + mergedSettings.maxIterations = modeSettings.groupChat.maxRounds; + } + + const base: WorkflowDefinition = { ...workflow, settings: mergedSettings }; + if (restructure) { const existingAgents = workflow.graph.nodes.filter((n) => n.kind === 'agent'); - const newGraph = scaffoldGraphForMode(newMode, existingAgents.length > 0 ? existingAgents : undefined); + const newGraph = scaffoldGraphForMode(newMode, { + agentNodes: existingAgents.length > 0 ? existingAgents : undefined, + settings: mergedSettings, + }); onChange({ ...base, graph: newGraph }); } else { - onChange(base); + onChange(syncBuilderModeEdgeIterations(base)); } }, [workflow, onChange], @@ -387,13 +395,15 @@ export function OrchestrationModePanel({ const handleGroupChatChange = useCallback( (groupChat: GroupChatModeSettings) => { - onChange({ + const updated: WorkflowDefinition = { ...workflow, settings: { ...workflow.settings, modeSettings: { ...workflow.settings.modeSettings, groupChat }, + maxIterations: groupChat.maxRounds, }, - }); + }; + onChange(syncBuilderModeEdgeIterations(updated)); }, [workflow, onChange], ); diff --git a/src/renderer/components/workflow/WorkflowGraphInspector.tsx b/src/renderer/components/workflow/WorkflowGraphInspector.tsx index 38b427e..c98af08 100644 --- a/src/renderer/components/workflow/WorkflowGraphInspector.tsx +++ b/src/renderer/components/workflow/WorkflowGraphInspector.tsx @@ -13,9 +13,11 @@ import type { WorkflowEdge, WorkflowNode, WorkflowNodeConfig, + WorkflowOrchestrationMode, WorkflowValidationIssue, AgentNodeConfig, } from '@shared/domain/workflow'; +import { isBuilderBasedMode } from '@shared/domain/workflow'; import { ModelSelect, ReasoningEffortSelect } from '@renderer/components/AgentConfigFields'; import { InvokeFunctionInspector } from '@renderer/components/workflow/InvokeFunctionInspector'; import { ConditionEditor } from '@renderer/components/workflow/ConditionEditor'; @@ -251,16 +253,19 @@ function PlaceholderNodeInspector({ function EdgeInspector({ edge, + orchestrationMode, validationIssues, onEdgeChange, onEdgeRemove, }: { edge: WorkflowEdge; + orchestrationMode?: WorkflowOrchestrationMode; validationIssues?: WorkflowValidationIssue[]; onEdgeChange: (edgeId: string, patch: Partial) => void; onEdgeRemove: (edgeId: string) => void; }) { const isFanIn = edge.kind === 'fan-in'; + const builderMode = isBuilderBasedMode(orchestrationMode); const edgeIssues = validationIssues?.filter((i) => i.edgeId === edge.id) ?? []; const handleConditionChange = useCallback( @@ -310,6 +315,7 @@ function EdgeInspector({ { if (e.target.checked) { onEdgeChange(edge.id, { isLoop: true, maxIterations: edge.maxIterations ?? 10 }); @@ -321,14 +327,17 @@ function EdgeInspector({ />

- Creates an iterative cycle, re-executing the path between these nodes up to a set limit. + {builderMode + ? 'Loop edges are managed by the orchestration mode settings.' + : 'Creates an iterative cycle, re-executing the path between these nodes up to a set limit.'}

{edge.isLoop && ( )} @@ -414,6 +428,7 @@ export function WorkflowGraphInspector({
cloneAgentNode(node, index + 1, { x: 520, y: index * 240 })); @@ -885,12 +900,12 @@ export function scaffoldGraphForMode( createWorkflowEdge(`edge-${triage.id}-to-end`, triage.id, 'end'), ...specialists.map((specialist) => createWorkflowEdge(`edge-${triage.id}-to-${specialist.id}`, triage.id, specialist.id, 'direct', { isLoop: true, - maxIterations: 4, + maxIterations: loopIterations, condition: { type: 'always' }, })), ...specialists.map((specialist) => createWorkflowEdge(`edge-${specialist.id}-to-${triage.id}`, specialist.id, triage.id, 'direct', { isLoop: true, - maxIterations: 4, + maxIterations: loopIterations, condition: { type: 'always' }, })), ...specialists.map((specialist) => createWorkflowEdge(`edge-${specialist.id}-to-end`, specialist.id, 'end')), @@ -898,6 +913,9 @@ export function scaffoldGraphForMode( }; } + const loopIterations = options.settings?.modeSettings?.groupChat?.maxRounds + ?? options.settings?.maxIterations + ?? defaultGroupChatLoopIterations; const agents = preparedAgents.map((node, index) => cloneAgentNode(node, index, { x: 240 + (index * 240), y: 0 })); return { nodes: [createStartNode(0, 0), ...agents, createEndNode(240 + (agents.length * 240), 0)], @@ -910,7 +928,7 @@ export function scaffoldGraphForMode( 'direct', { isLoop: true, - maxIterations: 5, + maxIterations: loopIterations, condition: { type: 'always' }, }, )), @@ -921,7 +939,7 @@ export function scaffoldGraphForMode( 'direct', { isLoop: true, - maxIterations: 5, + maxIterations: loopIterations, condition: { type: 'always' }, label: 'Loop', }, @@ -931,6 +949,39 @@ export function scaffoldGraphForMode( }; } +/** + * For builder-based modes (handoff, group-chat), synchronize loop edge maxIterations + * with the authoritative mode settings. The backend ignores edge maxIterations for these + * modes, so the edges are visual only and should reflect the actual runtime limit. + */ +export function syncBuilderModeEdgeIterations(workflow: WorkflowDefinition): WorkflowDefinition { + const mode = workflow.settings.orchestrationMode; + if (!isBuilderBasedMode(mode)) { + return workflow; + } + + const loopIterations = mode === 'group-chat' + ? (workflow.settings.modeSettings?.groupChat?.maxRounds ?? workflow.settings.maxIterations ?? defaultGroupChatLoopIterations) + : (workflow.settings.maxIterations ?? defaultHandoffLoopIterations); + + const hasStaleEdge = workflow.graph.edges.some( + (edge) => edge.isLoop && edge.maxIterations !== loopIterations, + ); + if (!hasStaleEdge) { + return workflow; + } + + return { + ...workflow, + graph: { + ...workflow.graph, + edges: workflow.graph.edges.map((edge) => + edge.isLoop ? { ...edge, maxIterations: loopIterations } : edge, + ), + }, + }; +} + function createWorkflowEdge( id: string, source: string, diff --git a/tests/shared/workflow.test.ts b/tests/shared/workflow.test.ts index 18093e1..ac82b5b 100644 --- a/tests/shared/workflow.test.ts +++ b/tests/shared/workflow.test.ts @@ -640,3 +640,110 @@ describe('workflow validation', () => { expect(groupChatWorkflow?.settings.modeSettings).toEqual(createDefaultModeSettings('group-chat')); }); }); + +describe('built-in workflow consistency', () => { + const builtinWorkflows = createBuiltinWorkflows(TIMESTAMP); + + test('all built-in workflows pass validation without errors', () => { + for (const workflow of builtinWorkflows) { + const issues = validateWorkflowDefinition(workflow); + const errors = issues.filter((i) => i.level === 'error'); + expect(errors).toEqual([]); + } + }); + + test('all built-in workflows are stable under normalization', () => { + for (const workflow of builtinWorkflows) { + const normalized = normalizeWorkflowDefinition(workflow); + expect(normalized.graph.edges).toEqual(workflow.graph.edges); + expect(normalized.settings).toEqual(workflow.settings); + } + }); + + test('group-chat workflow has consistent maxIterations across settings and edges', () => { + const workflow = builtinWorkflows.find((w) => w.settings.orchestrationMode === 'group-chat')!; + const maxRounds = workflow.settings.modeSettings!.groupChat!.maxRounds; + const maxIterations = workflow.settings.maxIterations; + + expect(maxIterations).toBeDefined(); + expect(maxRounds).toBe(maxIterations!); + + const loopEdges = workflow.graph.edges.filter((e) => e.isLoop); + expect(loopEdges.length).toBeGreaterThan(0); + for (const edge of loopEdges) { + expect(edge.maxIterations).toBe(maxRounds); + } + }); + + test('handoff workflow has consistent maxIterations across settings and edges', () => { + const workflow = builtinWorkflows.find((w) => w.settings.orchestrationMode === 'handoff')!; + const maxIterations = workflow.settings.maxIterations; + + const loopEdges = workflow.graph.edges.filter((e) => e.isLoop); + expect(loopEdges.length).toBeGreaterThan(0); + for (const edge of loopEdges) { + expect(edge.maxIterations).toBe(maxIterations); + } + }); + + test('scaffoldGraphForMode respects provided settings for group-chat', () => { + const graph = scaffoldGraphForMode('group-chat', { + settings: { + checkpointing: { enabled: false }, + executionMode: 'off-thread', + orchestrationMode: 'group-chat', + modeSettings: { groupChat: { selectionStrategy: 'round-robin', maxRounds: 12 } }, + maxIterations: 12, + }, + }); + + const loopEdges = graph.edges.filter((e) => e.isLoop); + expect(loopEdges.length).toBeGreaterThan(0); + for (const edge of loopEdges) { + expect(edge.maxIterations).toBe(12); + } + }); + + test('scaffoldGraphForMode respects provided settings for handoff', () => { + const graph = scaffoldGraphForMode('handoff', { + settings: { + checkpointing: { enabled: false }, + executionMode: 'off-thread', + orchestrationMode: 'handoff', + maxIterations: 8, + }, + }); + + const loopEdges = graph.edges.filter((e) => e.isLoop); + expect(loopEdges.length).toBeGreaterThan(0); + for (const edge of loopEdges) { + expect(edge.maxIterations).toBe(8); + } + }); + + test('normalizeWorkflowDefinition syncs loop edge maxIterations for group-chat', () => { + const workflow = createBuiltinWorkflows(TIMESTAMP).find((w) => w.settings.orchestrationMode === 'group-chat')!; + const modified: WorkflowDefinition = { + ...workflow, + settings: { + ...workflow.settings, + maxIterations: 10, + modeSettings: { + groupChat: { selectionStrategy: 'round-robin', maxRounds: 10 }, + }, + }, + }; + + const normalized = normalizeWorkflowDefinition(modified); + const loopEdges = normalized.graph.edges.filter((e) => e.isLoop); + for (const edge of loopEdges) { + expect(edge.maxIterations).toBe(10); + } + }); + + test('normalizeWorkflowDefinition does not modify loop edges for graph-based modes', () => { + const workflow = createBuiltinWorkflows(TIMESTAMP).find((w) => w.settings.orchestrationMode === 'sequential')!; + const normalized = normalizeWorkflowDefinition(workflow); + expect(normalized.graph.edges).toEqual(workflow.graph.edges); + }); +});