From 0753fd962a77fabfd59ec2adb07b37cd597f263d Mon Sep 17 00:00:00 2001 From: streamer45 Date: Mon, 29 Dec 2025 19:12:06 +0100 Subject: [PATCH] fix(ui/design): load dynamic samples, preserve edges, order YAML by canvas --- ui/src/hooks/usePipeline.ts | 329 ++++++++++++++------------- ui/src/panes/SamplePipelinesPane.tsx | 8 +- ui/src/services/fragments.ts | 34 ++- ui/src/services/samples.ts | 17 ++ ui/src/views/DesignView.tsx | 157 +++++++++---- 5 files changed, 324 insertions(+), 221 deletions(-) diff --git a/ui/src/hooks/usePipeline.ts b/ui/src/hooks/usePipeline.ts index 6272adee..d2085538 100644 --- a/ui/src/hooks/usePipeline.ts +++ b/ui/src/hooks/usePipeline.ts @@ -9,7 +9,6 @@ import { useState, useEffect, useRef, useCallback } from 'react'; import { useToast } from '@/context/ToastContext'; import { useNodeParamsStore } from '@/stores/nodeParamsStore'; import { useSchemaStore } from '@/stores/schemaStore'; -import { topoOrderFromEdges } from '@/utils/dag'; import { hooksLogger } from '@/utils/logger'; import { parseYamlToPipeline, type EngineMode } from '@/utils/yamlPipeline'; @@ -34,6 +33,113 @@ type EditorNodeData = { type ConnectionMode = 'reliable' | 'best_effort'; type NeedsDependency = string | { node: string; mode?: ConnectionMode }; +function orderNodeIdsTopDown( + nodes: Array>, + edges: Array +): Array { + const nodeIds = nodes.map((n) => n.id); + const posById = new Map(nodeIds.map((id) => [id, { x: 0, y: 0 }])); + nodes.forEach((n) => posById.set(n.id, { x: n.position.x, y: n.position.y })); + + const inDegree: Record = {}; + const outgoing: Record = {}; + nodeIds.forEach((nodeId) => { + inDegree[nodeId] = 0; + outgoing[nodeId] = []; + }); + + edges.forEach((e) => { + if (!(e.source in outgoing) || !(e.target in inDegree)) return; + outgoing[e.source].push(e.target); + inDegree[e.target] += 1; + }); + + const compare = (a: string, b: string) => { + const pa = posById.get(a) ?? { x: 0, y: 0 }; + const pb = posById.get(b) ?? { x: 0, y: 0 }; + if (pa.y !== pb.y) return pa.y - pb.y; + if (pa.x !== pb.x) return pa.x - pb.x; + return a.localeCompare(b); + }; + + const queue = nodeIds.filter((nodeId) => inDegree[nodeId] === 0).sort(compare); + const ordered: string[] = []; + const seen = new Set(); + + while (queue.length > 0) { + const u = queue.shift() as string; + if (seen.has(u)) continue; + seen.add(u); + ordered.push(u); + for (const v of outgoing[u]) { + inDegree[v] -= 1; + if (inDegree[v] === 0) { + queue.push(v); + } + } + queue.sort(compare); + } + + const remaining = nodeIds.filter((nodeId) => !seen.has(nodeId)).sort(compare); + return [...ordered, ...remaining]; +} + +function buildPipelineForYaml( + nodes: Array>, + edges: Array, + mode: EngineMode, + opts?: { includeUiPositions?: boolean } +): { mode: EngineMode; nodes: Record } { + const includeUiPositions = opts?.includeUiPositions ?? false; + const idToLabelMap = new Map(nodes.map((n) => [n.id, n.data.label])); + const idToNode = new Map(nodes.map((n) => [n.id, n])); + const pipeline: { mode: EngineMode; nodes: Record } = { mode, nodes: {} }; + + const orderedIds = orderNodeIdsTopDown(nodes, edges); + + orderedIds.forEach((nodeId) => { + const node = idToNode.get(nodeId); + if (!node) return; + + const needs = edges + .filter((e) => e.target === node.id) + .map((e): NeedsDependency | null => { + const label = idToLabelMap.get(e.source); + if (!label) return null; + const mode = (e.data as { mode?: ConnectionMode } | undefined)?.mode; + return mode === 'best_effort' ? { node: label, mode } : label; + }) + .filter((v): v is NeedsDependency => v !== null); + + const nodeConfig: Record = { kind: node.data.kind }; + + if (includeUiPositions) { + nodeConfig['ui'] = { + position: { + x: Math.round(node.position.x), + y: Math.round(node.position.y), + }, + }; + } + + const overrides = useNodeParamsStore.getState().paramsById[node.id]; + const mergedParams = { ...(node.data.params || {}), ...(overrides || {}) }; + if (Object.keys(mergedParams).length > 0) { + nodeConfig['params'] = mergedParams; + } + + if (needs.length === 1) { + nodeConfig['needs'] = needs[0]; + } else if (needs.length > 1) { + nodeConfig['needs'] = needs; + } + + pipeline.nodes[node.data.label] = nodeConfig; + }); + + return pipeline; +} + export const usePipeline = () => { const [nodes, setNodes, onNodesChange] = useNodesState>([]); const [edges, setEdges, onEdgesChange] = useEdgesState([]); @@ -44,23 +150,21 @@ export const usePipeline = () => { const [isLoading, setIsLoading] = useState(true); const [mode, setMode] = useState('dynamic'); const [yamlError, setYamlError] = useState(''); - const [pipelineName, setPipelineName] = useState(''); - const [pipelineDescription, setPipelineDescription] = useState(''); + const [pipelineName, setPipelineName] = useState(''), + [pipelineDescription, setPipelineDescription] = useState(''); const labelCountersRef = useRef>({}); const toast = useToast(); - // Track update source to prevent circular updates const updateSourceRef = useRef<'canvas' | 'yaml' | null>(null); - // Track previous nodes for structural change detection - const prevNodesRef = useRef[]>([]); + const prevNodesRef = useRef[]>([]), + prevEdgesRef = useRef([]), + prevModeRef = useRef(mode); const yamlDebounceTimerRef = useRef(null); const labelValidationTimerRef = useRef(null); - // Track the validation token to invalidate stale validations const labelValidationTokenRef = useRef(0); const handleLabelChange = useCallback( (nodeId: string, newLabel: string) => { - // Update the label immediately for responsive editing setNodes((nds) => { return nds.map((node) => { if (node.id === nodeId) { @@ -70,41 +174,31 @@ export const usePipeline = () => { }); }); - // Clear any existing validation timer if (labelValidationTimerRef.current) { clearTimeout(labelValidationTimerRef.current); } - // Increment token to invalidate any pending validation callbacks labelValidationTokenRef.current += 1; const currentToken = labelValidationTokenRef.current; - // Debounce the duplicate validation to avoid interrupting typing labelValidationTimerRef.current = setTimeout(() => { - // Check if this validation is still valid (not superseded by newer input) if (currentToken !== labelValidationTokenRef.current) { return; } - // Use a separate function call to access nodes state and validate setNodes((nds) => { - // Find the current label for this node const currentNode = nds.find((n) => n.id === nodeId); if (!currentNode) return nds; - // Double-check token hasn't changed while we were waiting if (currentToken !== labelValidationTokenRef.current) { return nds; } - // Only validate if the current label still matches what we scheduled if (currentNode.data.label !== newLabel) { return nds; } const isDuplicate = nds.some((n) => n.id !== nodeId && n.data.label === newLabel); - - // Schedule toast for next tick to avoid calling setState during render if (isDuplicate) { setTimeout(() => { toast.error( @@ -115,7 +209,7 @@ export const usePipeline = () => { return nds; }); - }, 500); // 500ms debounce + }, 500); }, [setNodes, toast] ); @@ -125,70 +219,42 @@ export const usePipeline = () => { const handleParamChange = useCallback( (nodeId: string, paramName: string, value: unknown) => { - // Write live param changes to a lightweight store instead of mutating the nodes array. - // This keeps ReactFlow props stable and avoids re-rendering the canvas on every slider tick. setParam(nodeId, paramName, value); }, [setParam] ); - const handleExportYaml = () => { - if (nodes.length === 0) return; - - const idToLabelMap = new Map(nodes.map((n) => [n.id, n.data.label])); - const idToNode = new Map(nodes.map((n) => [n.id, n])); - const pipeline: { mode: EngineMode; nodes: Record } = { mode, nodes: {} }; - - // Compute topological order of nodes based on edges (sources first) - const nodeIds = nodes.map((n) => n.id); - const orderedIds = topoOrderFromEdges( - nodeIds, - edges.map((e) => ({ source: e.source, target: e.target })) - ); - - orderedIds.forEach((nodeId) => { - const node = idToNode.get(nodeId); - if (!node) return; - - const needs = edges - .filter((e) => e.target === node.id) - .map((e): NeedsDependency | null => { - const label = idToLabelMap.get(e.source); - if (!label) return null; - const mode = (e.data as { mode?: ConnectionMode } | undefined)?.mode; - return mode === 'best_effort' ? { node: label, mode } : label; - }) - .filter((v): v is NeedsDependency => v !== null); - - const nodeConfig: Record = { - kind: node.data.kind, - ui: { - position: { - x: Math.round(node.position.x), - y: Math.round(node.position.y), - }, - }, - }; - - // Merge any live overrides from the params store so exports reflect current UI values - const overrides = ( - useNodeParamsStore.getState().paramsById as Record> - )[node.id as string]; - const mergedParams = { ...(node.data.params || {}), ...(overrides || {}) }; - if (Object.keys(mergedParams).length > 0) { - nodeConfig['params'] = mergedParams; + const regenerateYamlFromCanvas = useCallback( + (snapshot?: { + nodes?: Array>; + edges?: Array; + mode?: EngineMode; + }) => { + const nodesForYaml = snapshot?.nodes ?? nodes; + const edgesForYaml = snapshot?.edges ?? edges; + const modeForYaml = snapshot?.mode ?? mode; + + if (nodesForYaml.length === 0) { + setYamlString('# Add nodes to the canvas to see YAML output'); + setYamlError(''); + return; } - if (needs.length === 1) { - (nodeConfig as Record)['needs'] = needs[0]; - } else if (needs.length > 1) { - (nodeConfig as Record)['needs'] = needs; - } + setYamlString( + dump(buildPipelineForYaml(nodesForYaml, edgesForYaml, modeForYaml), { skipInvalid: true }) + ); + setYamlError(''); + }, + [nodes, edges, mode] + ); - pipeline.nodes[node.data.label] = nodeConfig; - }); + const handleExportYaml = () => { + if (nodes.length === 0) return; - const yamlToExport = dump(pipeline, { skipInvalid: true }); + const yamlToExport = dump( + buildPipelineForYaml(nodes, edges, mode, { includeUiPositions: true }), + { skipInvalid: true } + ); const blob = new Blob([yamlToExport], { type: 'application/x-yaml' }); const url = URL.createObjectURL(blob); const a = document.createElement('a'); @@ -221,7 +287,6 @@ export const usePipeline = () => { updateSourceRef.current = 'yaml'; - // Clear stale parameter overrides for all nodes so sliders can read fresh values from YAML result.nodes.forEach((node) => { resetNode(node.id); }); @@ -232,28 +297,22 @@ export const usePipeline = () => { setYamlError(''); setPipelineName(name); setPipelineDescription(description); - // Update YAML string immediately so it shows in the editor setYamlString(yamlContent); toast.success('Pipeline imported successfully!'); - // Reset update source after React finishes processing and the YAML regeneration effect has run setTimeout(() => { updateSourceRef.current = null; }, 100); }; - // Handle YAML changes from the editor (debounced) const handleYamlChange = useCallback( (newYaml: string) => { - // Update the YAML string immediately for responsive editing setYamlString(newYaml); - // Clear any existing timer if (yamlDebounceTimerRef.current) { clearTimeout(yamlDebounceTimerRef.current); } - // Debounce the parsing to avoid rapid updates while typing yamlDebounceTimerRef.current = setTimeout(() => { const result = parseYamlToPipeline( newYaml, @@ -273,10 +332,8 @@ export const usePipeline = () => { return; } - // Mark that this update came from YAML to prevent circular updates updateSourceRef.current = 'yaml'; - // Clear stale parameter overrides for all nodes so sliders can read fresh values from YAML result.nodes.forEach((node) => { resetNode(node.id); }); @@ -286,17 +343,14 @@ export const usePipeline = () => { setMode(result.mode); setYamlError(''); - // Reset update source after React finishes processing and the YAML regeneration effect has run - // This needs to be longer than setTimeout(..., 0) to ensure the effect runs with the flag set setTimeout(() => { updateSourceRef.current = null; }, 100); - }, 500); // 500ms debounce + }, 500); }, [nodeDefinitions, handleParamChange, handleLabelChange, setNodes, setEdges, setMode, resetNode] ); - // Load from localStorage on initial mount useEffect(() => { try { const item = window.localStorage.getItem(LOCAL_STORAGE_KEY); @@ -323,10 +377,9 @@ export const usePipeline = () => { }; if (Array.isArray(savedNodes) && Array.isArray(savedEdges)) { - // Re-initialize the global ID counter to avoid collisions let maxId = 0; savedNodes.forEach((node) => { - const match = node.id.match(/^skitnode_(\d+)$/); + const match = node.id.match(/^skitnode_(\\d+)$/); if (match) { const num = parseInt(match[1], 10); if (num > maxId) { @@ -336,10 +389,9 @@ export const usePipeline = () => { }); id = maxId + 1; - // Re-initialize label counters const newCounters: Record = {}; savedNodes.forEach((node) => { - const match = node.data.label.match(/^(.*)_(\d+)$/); + const match = node.data.label.match(/^(.*)_(\\d+)$/); if (match) { const [, kind, numStr] = match; const num = parseInt(numStr, 10); @@ -350,21 +402,18 @@ export const usePipeline = () => { }); labelCountersRef.current = newCounters; - // Re-hydrate nodes with callback functions const hydratedNodes = savedNodes.map((node) => ({ ...node, data: { ...(node.data as Record), onParamChange: handleParamChange, onLabelChange: handleLabelChange, - // No sessionId - prevents LIVE badge in design view }, })) as unknown as Node[]; setNodes(hydratedNodes); setEdges(savedEdges); - // Restore name and description if saved if (savedName) { setPipelineName(savedName); } @@ -372,11 +421,9 @@ export const usePipeline = () => { setPipelineDescription(savedDescription); } - // Auto-detect mode from loaded nodes if not explicitly saved if (savedMode) { setMode(savedMode); } else { - // Auto-detect from node types const hasOneshotNodes = hydratedNodes.some((node) => { const nodeDef = nodeDefinitions.find((def) => def.kind === node.data.kind); return nodeDef?.categories.includes('oneshot'); @@ -390,20 +437,17 @@ export const usePipeline = () => { } finally { setIsLoading(false); } - }, [handleParamChange, handleLabelChange, setNodes, setEdges, setMode, nodeDefinitions]); // Dependencies are stable; runs effectively once + }, [handleParamChange, handleLabelChange, setNodes, setEdges, setMode, nodeDefinitions]); - // Save to localStorage when nodes, edges, or mode change useEffect(() => { if (isLoading) { - return; // Don't save on initial render before hydration is complete + return; } const serializableNodes = nodes.map((node) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { onParamChange, onLabelChange, ...restData } = node.data as EditorNodeData; - // Merge live parameter overrides from nodeParamsStore into node.data.params - // so they persist across page reloads const liveOverrides = useNodeParamsStore.getState().paramsById[node.id]; const mergedParams = { ...(restData.params || {}), ...(liveOverrides || {}) }; @@ -430,7 +474,6 @@ export const usePipeline = () => { } }, [nodes, edges, mode, pipelineName, pipelineDescription, isLoading]); - // Track per-kind counters to generate default human-readable labels const nextLabelForKind = useCallback((kind: string) => { const current = labelCountersRef.current[kind] ?? 0; const next = current + 1; @@ -439,16 +482,16 @@ export const usePipeline = () => { }, []); useEffect(() => { - // Don't regenerate YAML if the update came from YAML editor if (updateSourceRef.current === 'yaml') { return; } - // Check if only non-structural changes occurred (e.g., position changes) - // This prevents YAML regeneration when dragging nodes around the canvas const prevNodes = prevNodesRef.current; + const prevEdges = prevEdgesRef.current; + const prevMode = prevModeRef.current; + if (prevNodes.length === nodes.length && nodes.length > 0) { - const structurallyEqual = prevNodes.every((prev, i) => { + const nodesStructurallyEqual = prevNodes.every((prev, i) => { const curr = nodes[i]; return ( curr && @@ -458,69 +501,42 @@ export const usePipeline = () => { ); }); - if (structurallyEqual) { - // Only positions changed, skip YAML regeneration + const edgesStructurallyEqual = + prevEdges.length === edges.length && + prevEdges.every((prev, i) => { + const curr = edges[i]; + const prevMode = (prev.data as { mode?: ConnectionMode } | undefined)?.mode; + const currMode = (curr.data as { mode?: ConnectionMode } | undefined)?.mode; + return ( + curr && + prev.id === curr.id && + prev.source === curr.source && + prev.target === curr.target && + prev.sourceHandle === curr.sourceHandle && + prev.targetHandle === curr.targetHandle && + prevMode === currMode + ); + }); + + if (nodesStructurallyEqual && edgesStructurallyEqual && prevMode === mode) { prevNodesRef.current = nodes; + prevEdgesRef.current = edges; + prevModeRef.current = mode; return; } } - // Update ref with current nodes for next comparison prevNodesRef.current = nodes; + prevEdgesRef.current = edges; + prevModeRef.current = mode; - // Generate YAML from nodes and edges if (nodes.length === 0) { setYamlString('# Add nodes to the canvas to see YAML output'); setYamlError(''); return; } - const idToLabelMap = new Map(nodes.map((n) => [n.id, n.data.label])); - const idToNode = new Map(nodes.map((n) => [n.id, n])); - const pipeline: { mode: EngineMode; nodes: Record } = { mode, nodes: {} }; - - // Compute topological order of nodes based on edges (sources first) - const nodeIds = nodes.map((n) => n.id); - const orderedIds = topoOrderFromEdges( - nodeIds, - edges.map((e) => ({ source: e.source, target: e.target })) - ); - - orderedIds.forEach((nodeId) => { - const node = idToNode.get(nodeId); - if (!node) return; - - const needs = edges - .filter((e) => e.target === node.id) - .map((e): NeedsDependency | null => { - const label = idToLabelMap.get(e.source); - if (!label) return null; - const mode = (e.data as { mode?: ConnectionMode } | undefined)?.mode; - return mode === 'best_effort' ? { node: label, mode } : label; - }) - .filter((v): v is NeedsDependency => v !== null); - - const nodeConfig: Record = { - kind: node.data.kind, - }; - - // Merge live overrides from the params store so the YAML reflects real-time UI values - const overrides = useNodeParamsStore.getState().paramsById[node.id]; - const mergedParams = { ...(node.data.params || {}), ...(overrides || {}) }; - if (Object.keys(mergedParams).length > 0) { - (nodeConfig as Record)['params'] = mergedParams; - } - - if (needs.length === 1) { - (nodeConfig as Record)['needs'] = needs[0]; - } else if (needs.length > 1) { - (nodeConfig as Record)['needs'] = needs; - } - - pipeline.nodes[node.data.label] = nodeConfig; - }); - - setYamlString(dump(pipeline, { skipInvalid: true })); + setYamlString(dump(buildPipelineForYaml(nodes, edges, mode), { skipInvalid: true })); setYamlError(''); }, [nodes, edges, mode]); @@ -547,6 +563,7 @@ export const usePipeline = () => { nextLabelForKind, handleParamChange, handleLabelChange, + regenerateYamlFromCanvas, getId, }; }; diff --git a/ui/src/panes/SamplePipelinesPane.tsx b/ui/src/panes/SamplePipelinesPane.tsx index 06697f58..8fabdbf8 100644 --- a/ui/src/panes/SamplePipelinesPane.tsx +++ b/ui/src/panes/SamplePipelinesPane.tsx @@ -10,11 +10,11 @@ import ConfirmModal from '@/components/ConfirmModal'; import { SKTooltip } from '@/components/Tooltip'; import { useToast } from '@/context/ToastContext'; import { - listFragments, deleteFragment as deleteFragmentService, + samplesToFragments, yamlToFragment, } from '@/services/fragments'; -import { listSamples, deleteSample } from '@/services/samples'; +import { listAllSamples, deleteSample } from '@/services/samples'; import { createSession } from '@/services/sessions'; import type { SamplePipeline } from '@/types/generated/api-types'; import { getLogger } from '@/utils/logger'; @@ -281,9 +281,9 @@ const SamplePipelinesPane = forwardRef { + return samples + .filter((sample) => sample.is_fragment) + .map((sample) => { + const { tags, description } = decodeFragmentMetadata(sample.description); + return { + ...sample, + tags, + description, + }; + }); +} + /** * Convert fragment nodes (with needs dependencies) to YAML string * Uses proper pipeline format: nodes with needs field @@ -112,16 +131,7 @@ export async function deleteFragment(id: string): Promise { * List all fragments (filters samples to only return fragments) */ export async function listFragments(): Promise> { - const samples = await listSamples(); + const samples = await listAllSamples(); - return samples - .filter((sample) => sample.is_fragment) - .map((sample) => { - const { tags, description } = decodeDescription(sample.description); - return { - ...sample, - tags, - description, - }; - }); + return samplesToFragments(samples); } diff --git a/ui/src/services/samples.ts b/ui/src/services/samples.ts index 083be28e..51980da3 100644 --- a/ui/src/services/samples.ts +++ b/ui/src/services/samples.ts @@ -46,6 +46,23 @@ export async function listSamples(): Promise { return samples; } +/** + * Lists all available sample pipelines (oneshot + dynamic). + * @returns A promise that resolves to an array of sample pipelines + */ +export async function listAllSamples(): Promise { + const [oneshot, dynamic] = await Promise.all([listSamples(), listDynamicSamples()]); + const merged = [...oneshot, ...dynamic]; + + // De-dupe by ID (defensive; endpoints should not overlap) + const seen = new Set(); + return merged.filter((s) => { + if (seen.has(s.id)) return false; + seen.add(s.id); + return true; + }); +} + /** * Lists all available dynamic sample pipelines * @returns A promise that resolves to an array of dynamic sample pipelines diff --git a/ui/src/views/DesignView.tsx b/ui/src/views/DesignView.tsx index afde06ec..c94e631a 100644 --- a/ui/src/views/DesignView.tsx +++ b/ui/src/views/DesignView.tsx @@ -119,7 +119,6 @@ const TopRightControls = React.memo( onSaveTemplate, onCreateSession, nodesLength, - canSwitchToDynamic, }: { mode: 'oneshot' | 'dynamic'; onModeChange: () => void; @@ -129,13 +128,18 @@ const TopRightControls = React.memo( onSaveTemplate: () => void; onCreateSession: () => void; nodesLength: number; - canSwitchToDynamic: boolean; }) => { const { can } = usePermissions(); return ( - + @@ -242,6 +245,13 @@ type EditorNodeData = { onLabelChange?: (nodeId: string, newLabel: string) => void; }; +type PipelineCanvasCache = { + nodes: RFNode[]; + edges: Edge[]; + name: string; + description: string; +}; + /** * Helper function to handle fragment drops */ @@ -470,16 +480,31 @@ const DesignViewContent: React.FC = () => { mode, setMode, pipelineName, + setPipelineName, pipelineDescription, + setPipelineDescription, handleExportYaml, handleImportYaml, handleYamlChange, nextLabelForKind, handleParamChange, handleLabelChange, + regenerateYamlFromCanvas, getId, } = usePipeline(); + const cachesRef = React.useRef>>({}); + + // Keep per-mode caches updated with the latest canvas state. + React.useEffect(() => { + cachesRef.current[mode] = { + nodes, + edges, + name: pipelineName, + description: pipelineDescription, + }; + }, [mode, nodes, edges, pipelineName, pipelineDescription]); + // Create stable callback refs for node data to prevent unnecessary re-renders const handleParamChangeRef = React.useRef(handleParamChange); const handleLabelChangeRef = React.useRef(handleLabelChange); @@ -500,6 +525,20 @@ const DesignViewContent: React.FC = () => { handleLabelChangeRef.current(nodeId, newLabel); }, []); + const rehydrateNodesForCanvas = React.useCallback( + (cachedNodes: RFNode[]) => { + return cachedNodes.map((n) => ({ + ...n, + data: { + ...n.data, + onParamChange: stableHandleParamChange, + onLabelChange: stableHandleLabelChange, + }, + })); + }, + [stableHandleParamChange, stableHandleLabelChange] + ); + const { isValidConnection: isValidConnectionBase, createOnConnect, @@ -514,6 +553,13 @@ const DesignViewContent: React.FC = () => { edgesRef.current = edges; }, [nodes, edges]); + const handleNodeDragStop = React.useCallback(() => { + // Keep YAML ordering in sync with the canvas (top-down) without regenerating on every drag tick. + // Note: onNodeDragStop's third param only contains the dragged nodes, not all nodes. + // Use nodesRef.current which has all nodes with updated positions from onNodesChange. + regenerateYamlFromCanvas({ nodes: nodesRef.current, edges: edgesRef.current }); + }, [regenerateYamlFromCanvas]); + // Wrap isValidConnection to pass current nodes and edges via refs const isValidConnectionWrapper = React.useCallback( (connection: Connection | Edge) => { @@ -679,12 +725,14 @@ const DesignViewContent: React.FC = () => { edges: edges.map((e) => ({ source: e.source, target: e.target })), }); - setNodes((prev) => - prev.map((n) => ({ - ...n, - position: positions[n.id] ?? n.position, - })) - ); + const nextNodes = nodes.map((n) => ({ + ...n, + position: positions[n.id] ?? n.position, + })); + + setNodes(nextNodes); + // Pass edges explicitly to avoid stale closure issues. + regenerateYamlFromCanvas({ nodes: nextNodes, edges: edgesRef.current }); setTimeout(() => { rf.current?.fitView({ padding: 0.2, duration: 300 }); @@ -1126,19 +1174,23 @@ const DesignViewContent: React.FC = () => { handleAutoLayoutRef.current = handleAutoLayout; }); - // Automatically fit the view whenever the node count changes (e.g., after import or add/remove). - // If we just imported, also run auto layout to arrange nodes nicely. + // Run auto-layout after file imports, or fitView on initial page load. + // We no longer fitView on every node add/remove as it's disruptive to manual placement. useEffect(() => { if (nodes.length > 0) { - const timeoutId = window.setTimeout(() => { - if (pendingAutoLayoutRef.current) { + if (pendingAutoLayoutRef.current) { + const timeoutId = window.setTimeout(() => { pendingAutoLayoutRef.current = false; handleAutoLayoutRef.current(); - } else { + }, 50); + return () => window.clearTimeout(timeoutId); + } else if (pendingInitialFitViewRef.current) { + const timeoutId = window.setTimeout(() => { + pendingInitialFitViewRef.current = false; rf.current?.fitView({ padding: 0.2, duration: 300 }); - } - }, 50); - return () => window.clearTimeout(timeoutId); + }, 50); + return () => window.clearTimeout(timeoutId); + } } }, [nodes.length]); @@ -1157,25 +1209,39 @@ const DesignViewContent: React.FC = () => { const handleModeToggle = React.useCallback(() => { const nextMode = mode === 'oneshot' ? 'dynamic' : 'oneshot'; - // Validate switching to dynamic mode - if (nextMode === 'dynamic') { - const oneshotNodes = nodesRef.current.filter((node) => { - const nodeDef = nodeDefinitions.find((def) => def.kind === node.data.kind); - return nodeDef?.categories.includes('oneshot'); - }); + // Ensure the current canvas is cached even if the user switches immediately after load. + cachesRef.current[mode] = { + nodes: nodesRef.current, + edges: edgesRef.current, + name: pipelineName, + description: pipelineDescription, + }; - if (oneshotNodes.length > 0) { - const nodeList = oneshotNodes.map((n) => `"${n.data.label}" (${n.data.kind})`).join(', '); - toast.error( - `Cannot switch to dynamic mode: Pipeline contains oneshot-only nodes: ${nodeList}. ` + - `Remove these nodes first or stay in oneshot mode.` - ); - return; // Don't switch mode - } - } + // Swap canvases instead of forcing users to clear incompatible nodes. + // Keep a separate "last state" cache per mode. + const nextCache = cachesRef.current[nextMode]; + const nextNodes = nextCache ? rehydrateNodesForCanvas(nextCache.nodes) : []; + const nextEdges = nextCache?.edges ?? []; setMode(nextMode); - }, [mode, setMode, nodeDefinitions, toast]); + setNodes(nextNodes); + setEdges(nextEdges); + setPipelineName(nextCache?.name ?? ''); + setPipelineDescription(nextCache?.description ?? ''); + setSelectedNodes([]); + regenerateYamlFromCanvas({ nodes: nextNodes, edges: nextEdges, mode: nextMode }); + }, [ + mode, + setMode, + setNodes, + setEdges, + setPipelineName, + setPipelineDescription, + pipelineName, + pipelineDescription, + regenerateYamlFromCanvas, + rehydrateNodesForCanvas, + ]); const handleSelectionModeToggle = React.useCallback(() => { setSelectionMode(!selectionMode); @@ -1189,6 +1255,8 @@ const DesignViewContent: React.FC = () => { // Track when we need to auto-layout after import const pendingAutoLayoutRef = React.useRef(false); + // Track initial page load to run fitView once when nodes are restored from localStorage + const pendingInitialFitViewRef = React.useRef(true); // File import handlers - kept at DesignView level so file input survives menu close const handleImportFileChange = React.useCallback((event: React.ChangeEvent) => { @@ -1226,7 +1294,8 @@ const DesignViewContent: React.FC = () => { setPendingSample({ yaml: yamlString, name, description }); handleOpenLoadSampleModal(); } else { - // Canvas is empty, load directly + // Canvas is empty, load directly with auto-layout + pendingAutoLayoutRef.current = true; handleImportYamlRef.current(yamlString, description, name); toastRef.current.success(`Loaded sample: ${name}`); } @@ -1236,6 +1305,7 @@ const DesignViewContent: React.FC = () => { const confirmLoadSample = () => { if (pendingSample) { + pendingAutoLayoutRef.current = true; handleImportYaml(pendingSample.yaml, pendingSample.description, pendingSample.name); toast.success(`Loaded sample: ${pendingSample.name}`); handleCloseLoadSampleModal(); @@ -1249,17 +1319,6 @@ const DesignViewContent: React.FC = () => { toast.success('Canvas cleared'); }, [setNodes, setEdges, toast, handleCloseClearModal]); - // Check if we can switch to dynamic mode (no oneshot-only nodes) - // Only recompute when node count or definitions change, not on position updates - const canSwitchToDynamic = React.useMemo(() => { - return !nodesRef.current.some((node) => { - const nodeDef = nodeDefinitions.find((def) => def.kind === node.data.kind); - return nodeDef?.categories.includes('oneshot'); - }); - // nodes.length is intentional - we need to recompute when nodes are added/removed - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [nodes.length, nodeDefinitions]); - // Memoize left panel to prevent re-renders during drag const leftPanel = React.useMemo( () => ( @@ -1304,7 +1363,6 @@ const DesignViewContent: React.FC = () => { onSaveTemplate={handleOpenSaveModal} onCreateSession={handleOpenCreateModal} nodesLength={nodes.length} - canSwitchToDynamic={canSwitchToDynamic} /> { onPaneClick={onPaneClick} onPaneContextMenu={onPaneContextMenu} onNodeContextMenu={onNodeContextMenu} + onNodeDragStop={handleNodeDragStop} onDrop={onDrop} onDragOver={onDragOver} reactFlowWrapper={reactFlowWrapper}