From 337b3b1eb6dfc48c2e5106958ee71ca3aec50044 Mon Sep 17 00:00:00 2001 From: centdix Date: Tue, 7 Apr 2026 15:55:38 +0200 Subject: [PATCH] refactor: extract flow delete helpers Co-Authored-By: Claude Opus 4.5 --- .../components/flows/agentToolUtils.test.ts | 9 +++ .../components/flows/flowDeleteUtils.test.ts | 81 +++++++++++++++++++ .../lib/components/flows/flowDeleteUtils.ts | 51 ++++++++++++ .../flows/map/FlowModuleSchemaMap.svelte | 72 +++-------------- 4 files changed, 152 insertions(+), 61 deletions(-) create mode 100644 frontend/src/lib/components/flows/flowDeleteUtils.test.ts create mode 100644 frontend/src/lib/components/flows/flowDeleteUtils.ts diff --git a/frontend/src/lib/components/flows/agentToolUtils.test.ts b/frontend/src/lib/components/flows/agentToolUtils.test.ts index a88eb2833a..3b31fd3bc8 100644 --- a/frontend/src/lib/components/flows/agentToolUtils.test.ts +++ b/frontend/src/lib/components/flows/agentToolUtils.test.ts @@ -65,4 +65,13 @@ describe('removeAgentToolByIdDeep', () => { expect((((rootAgent.value as any).tools as any[])[0].value as any).tools).toEqual([]) expect(removed).toEqual(['create_ticket']) }) + + it('returns false when the tool does not exist', () => { + const agent = makeAiAgent('agent', [makeFlowModuleTool(makeRawModule('lookup_user'))]) + const removed: string[] = [] + + expect(removeAgentToolByIdDeep([agent], 'missing_tool', (x) => removed.push(x.id))).toBe(false) + expect((agent.value as any).tools).toHaveLength(1) + expect(removed).toEqual([]) + }) }) diff --git a/frontend/src/lib/components/flows/flowDeleteUtils.test.ts b/frontend/src/lib/components/flows/flowDeleteUtils.test.ts new file mode 100644 index 0000000000..9d77446d86 --- /dev/null +++ b/frontend/src/lib/components/flows/flowDeleteUtils.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('../aiProviderStorage', () => ({ + loadStoredConfig: () => undefined +})) + +vi.mock('./flowInfers', () => ({ + AI_AGENT_SCHEMA: { properties: {} } +})) + +import type { FlowModule } from '$lib/gen' +import type { FlowStructureNode } from '$lib/components/graph/flowStructure' +import { partitionDeleteTargets, removeToolIds } from './flowDeleteUtils' + +function makeRawModule(id: string): FlowModule { + return { + id, + summary: id, + value: { type: 'rawscript', content: '', language: 'python3' } as any + } as FlowModule +} + +function makeAiAgent(id: string, tools: any[]): FlowModule { + return { + id, + summary: id, + value: { + type: 'aiagent', + tools, + input_transforms: {} + } as any + } as FlowModule +} + +function makeFlowModuleTool(module: FlowModule) { + return { + id: module.id, + summary: module.summary, + value: { + tool_type: 'flowmodule', + ...module.value + } + } +} + +describe('partitionDeleteTargets', () => { + it('splits structure nodes from AI tool ids in one pass', () => { + const tree: FlowStructureNode[] = [ + { id: 'step_a', kind: 'leaf', branches: [] }, + { + id: 'loop', + kind: 'forloopflow', + branches: [{ children: [{ id: 'nested_step', kind: 'leaf', branches: [] }] }] + } + ] + + expect(partitionDeleteTargets(tree, ['step_a', 'tool_x', 'nested_step'])).toEqual({ + structureIds: ['step_a', 'nested_step'], + toolIds: ['tool_x'] + }) + }) +}) + +describe('removeToolIds', () => { + it('returns only the ids that were actually removed', () => { + const rootAgent = makeAiAgent('root_agent', [ + makeFlowModuleTool(makeRawModule('lookup_user')), + makeFlowModuleTool(makeAiAgent('nested_agent', [makeFlowModuleTool(makeRawModule('create_ticket'))])) + ]) + const removed: string[] = [] + + expect( + removeToolIds([rootAgent], ['lookup_user', 'missing_tool', 'create_ticket'], (tool) => { + removed.push(tool.id) + }) + ).toEqual(['lookup_user', 'create_ticket']) + expect((rootAgent.value as any).tools).toHaveLength(1) + expect((((rootAgent.value as any).tools as any[])[0].value as any).tools).toEqual([]) + expect(removed).toEqual(['lookup_user', 'create_ticket']) + }) +}) diff --git a/frontend/src/lib/components/flows/flowDeleteUtils.ts b/frontend/src/lib/components/flows/flowDeleteUtils.ts new file mode 100644 index 0000000000..c851cce2ff --- /dev/null +++ b/frontend/src/lib/components/flows/flowDeleteUtils.ts @@ -0,0 +1,51 @@ +import type { FlowModule } from '$lib/gen' +import { findInStructure, type FlowStructureNode } from '$lib/components/graph/flowStructure' +import type { AgentTool } from './agentToolUtils' +import { removeAgentToolByIdDeep } from './agentToolUtils' + +export type DeleteTargetPartition = { + structureIds: string[] + toolIds: string[] +} + +/** + * Split delete targets between structure-tree nodes and AI agent tool nodes. + * AI tools are rendered in the graph but are not represented in the grouped structure tree. + */ +export function partitionDeleteTargets( + tree: FlowStructureNode[], + ids: string[] +): DeleteTargetPartition { + const structureIds: string[] = [] + const toolIds: string[] = [] + + for (const id of ids) { + if (findInStructure(tree, id)) { + structureIds.push(id) + } else { + toolIds.push(id) + } + } + + return { structureIds, toolIds } +} + +/** + * Remove AI agent tools by id and return the ids that were actually removed. + */ +export function removeToolIds( + modules: FlowModule[], + ids: string[], + onRemove?: (tool: AgentTool) => void +): string[] { + const removedIds = new Set() + + for (const id of ids) { + removeAgentToolByIdDeep(modules, id, (tool) => { + removedIds.add(tool.id) + onRemove?.(tool) + }) + } + + return [...removedIds] +} diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte index c20c904ca4..82e5803823 100644 --- a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte +++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte @@ -50,10 +50,10 @@ createWebsearchTool, createAiAgentTool, SPECIAL_TOOL_KINDS, - agentToolToFlowModule, - removeAgentToolByIdDeep + agentToolToFlowModule } from '../agentToolUtils' import { loadFlowModuleState } from '../flowStateUtils.svelte' + import { partitionDeleteTargets, removeToolIds } from '../flowDeleteUtils' import { getNoteEditorContext } from '$lib/components/graph/noteEditor.svelte' import { GroupedModulesProxy, @@ -253,48 +253,6 @@ } } - /** - * Helper function to remove an AgentTool by id from the tools array - * Tools are always leaf nodes, so we just need to delete their state directly - */ - function removeAgentToolById(tools: AgentTool[], id: string): AgentTool[] { - const index = tools.findIndex((tool) => tool.id === id) - if (index != -1) { - const [removed] = tools.splice(index, 1) - deleteFlowStateById(removed.id, flowStateStore) - } - return tools - } - - export function removeAtId(modules: FlowModule[], id: string): FlowModule[] { - const index = modules.findIndex((mod) => mod.id == id) - if (index != -1) { - const [removed] = modules.splice(index, 1) - const leaves = dfs([removed], (mod) => mod.id) - leaves.forEach((leafId: string) => deleteFlowStateById(leafId, flowStateStore)) - return modules - } - return modules.map((mod) => { - if (mod.value.type == 'forloopflow' || mod.value.type == 'whileloopflow') { - mod.value.modules = removeAtId(mod.value.modules, id) - } else if (mod.value.type == 'branchall') { - mod.value.branches = mod.value.branches.map((branch) => { - branch.modules = removeAtId(branch.modules, id) - return branch - }) - } else if (mod.value.type == 'branchone') { - mod.value.branches = mod.value.branches.map((branch) => { - branch.modules = removeAtId(branch.modules, id) - return branch - }) - mod.value.default = removeAtId(mod.value.default, id) - } else if (mod.value.type == 'aiagent') { - mod.value.tools = removeAgentToolById(mod.value.tools, id) - } - return mod - }) - } - let sidebarMode: 'list' | 'graph' = 'graph' let minHeight = $state(0) @@ -384,15 +342,7 @@ } export function deleteMultiple(ids: string[]) { - const structureIds: string[] = [] - const toolIds: string[] = [] - for (const id of ids) { - if (findInStructure(proxy.items, id)) { - structureIds.push(id) - } else { - toolIds.push(id) - } - } + const { structureIds, toolIds } = partitionDeleteTargets(proxy.items, ids) const deletingSet = new Set(ids) const allDeps: Record = {} for (const id of ids) { @@ -424,13 +374,13 @@ const cb = () => { push(history, flowStore.val) commit({ removeDuplicates: duplicateGroups.length > 0 }) - for (const id of toolIds) { - removeAgentToolByIdDeep(flowStore.val.value.modules, id, (removed) => { - deleteFlowStateById(removed.id, flowStateStore) - }) - } + const removedToolIds = removeToolIds(flowStore.val.value.modules, toolIds, (tool) => { + deleteFlowStateById(tool.id, flowStateStore) + }) for (const id of ids) { - delete flowStateStore.val[id] + if (structureIds.includes(id) || removedToolIds.includes(id)) { + delete flowStateStore.val[id] + } } selectionManager.clearSelection() refreshStateStore(flowStore) @@ -704,10 +654,10 @@ const cb = () => { push(history, flowStore.val) selectNextId(id) - const removed = removeAgentToolByIdDeep(flowStore.val.value.modules, id, (tool) => { + const removedIds = removeToolIds(flowStore.val.value.modules, [id], (tool) => { deleteFlowStateById(tool.id, flowStateStore) }) - if (!removed) return + if (removedIds.length === 0) return refreshStateStore(flowStore) onDelete?.(id) }