mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-03 18:38:35 +02:00
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>
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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],
|
||||
);
|
||||
|
||||
@@ -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<WorkflowEdge>) => 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({
|
||||
<input
|
||||
checked={edge.isLoop === true}
|
||||
className="size-4 accent-[var(--color-accent)]"
|
||||
disabled={builderMode}
|
||||
onChange={(e) => {
|
||||
if (e.target.checked) {
|
||||
onEdgeChange(edge.id, { isLoop: true, maxIterations: edge.maxIterations ?? 10 });
|
||||
@@ -321,14 +327,17 @@ function EdgeInspector({
|
||||
/>
|
||||
</label>
|
||||
<p className="text-[11px] leading-relaxed text-[var(--color-text-muted)]">
|
||||
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.'}
|
||||
</p>
|
||||
</div>
|
||||
{edge.isLoop && (
|
||||
<label className="block space-y-1.5">
|
||||
<span className="text-[11px] text-[var(--color-text-muted)]">Max Iterations</span>
|
||||
<input
|
||||
className="w-full rounded-lg border border-[var(--color-border)] bg-[var(--color-surface-1)] px-3 py-1.5 text-[12px] text-[var(--color-text-primary)] outline-none transition focus:border-[var(--color-accent)]/50"
|
||||
className="w-full rounded-lg border border-[var(--color-border)] bg-[var(--color-surface-1)] px-3 py-1.5 text-[12px] text-[var(--color-text-primary)] outline-none transition focus:border-[var(--color-accent)]/50 disabled:opacity-60"
|
||||
disabled={builderMode}
|
||||
min={1}
|
||||
onChange={(e) => {
|
||||
const raw = parseInt(e.target.value, 10);
|
||||
@@ -339,6 +348,11 @@ function EdgeInspector({
|
||||
type="number"
|
||||
value={edge.maxIterations ?? 10}
|
||||
/>
|
||||
{builderMode && (
|
||||
<p className="text-[11px] text-[var(--color-text-muted)]">
|
||||
Derived from the {orchestrationMode === 'group-chat' ? 'Max Rounds' : 'Max Iterations'} setting.
|
||||
</p>
|
||||
)}
|
||||
</label>
|
||||
)}
|
||||
</div>
|
||||
@@ -414,6 +428,7 @@ export function WorkflowGraphInspector({
|
||||
<div className="p-4">
|
||||
<EdgeInspector
|
||||
edge={selectedEdge}
|
||||
orchestrationMode={workflow.settings.orchestrationMode}
|
||||
onEdgeChange={onEdgeChange}
|
||||
onEdgeRemove={onEdgeRemove}
|
||||
validationIssues={validationIssues}
|
||||
|
||||
@@ -442,7 +442,7 @@ export function normalizeWorkflowDefinition(workflow: WorkflowDefinition): Workf
|
||||
? Math.round(workflow.settings.maxIterations)
|
||||
: undefined;
|
||||
|
||||
return {
|
||||
const normalized: WorkflowDefinition = {
|
||||
...workflow,
|
||||
name: workflow.name.trim(),
|
||||
description: workflow.description.trim(),
|
||||
@@ -496,6 +496,8 @@ export function normalizeWorkflowDefinition(workflow: WorkflowDefinition): Workf
|
||||
: undefined,
|
||||
},
|
||||
};
|
||||
|
||||
return syncBuilderModeEdgeIterations(normalized);
|
||||
}
|
||||
|
||||
export function resolveWorkflowAgentNodes(workflow: WorkflowDefinition): WorkflowNode[] {
|
||||
@@ -824,11 +826,23 @@ function prepareScaffoldAgentNodes(
|
||||
return agents;
|
||||
}
|
||||
|
||||
export interface ScaffoldGraphOptions {
|
||||
agentNodes?: WorkflowNode[];
|
||||
settings?: WorkflowSettings;
|
||||
}
|
||||
|
||||
const defaultHandoffLoopIterations = 4;
|
||||
const defaultGroupChatLoopIterations = 5;
|
||||
|
||||
export function scaffoldGraphForMode(
|
||||
mode: WorkflowOrchestrationMode,
|
||||
agentNodes?: WorkflowNode[],
|
||||
agentNodesOrOptions?: WorkflowNode[] | ScaffoldGraphOptions,
|
||||
): WorkflowGraph {
|
||||
const preparedAgents = prepareScaffoldAgentNodes(mode, agentNodes);
|
||||
const options: ScaffoldGraphOptions = Array.isArray(agentNodesOrOptions)
|
||||
? { agentNodes: agentNodesOrOptions }
|
||||
: (agentNodesOrOptions ?? {});
|
||||
|
||||
const preparedAgents = prepareScaffoldAgentNodes(mode, options.agentNodes);
|
||||
|
||||
if (mode === 'single') {
|
||||
const agent = cloneAgentNode(preparedAgents[0]!, 0, { x: 220, y: 0 });
|
||||
@@ -874,6 +888,7 @@ export function scaffoldGraphForMode(
|
||||
}
|
||||
|
||||
if (mode === 'handoff') {
|
||||
const loopIterations = options.settings?.maxIterations ?? defaultHandoffLoopIterations;
|
||||
const triage = cloneAgentNode(preparedAgents[0]!, 0, { x: 240, y: 120 });
|
||||
const specialists = preparedAgents.slice(1).map((node, index) =>
|
||||
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,
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user