Selaa lähdekoodia

fix: stop a disabled node from reading as completed in a past execution

Loading a pinned/viewed execution mapped any status that wasn't
"skipped" or "failed" to "completed" - a disabled node's recorded
status ("disabled") fell into that default and showed up identically
to a node that actually ran, both on the canvas and in the results
panel. Map it explicitly instead.

Also sort nodeExecutions by startedAt before building node states.
The array order coming back from the API was never guaranteed to be
chronological, which is what let a workflow's results render out of
the order things actually happened in.

Two supporting changes for the grouping work landing next:
- Aggregate a loop body's per-iteration results using the engine's new
  loopNodeId/loopIteration pair when present, alongside the existing
  "<id>_iter_<n>" suffix handling for older executions.
- Carry the sorted raw nodeExecutions through on ExecutionState
  (rawNodeExecutions), for the results panel to group by iteration.
  Only set for a pinned/viewed execution - a live run keeps building
  nodeStates incrementally and never populates it.
fszontagh 1 kuukausi sitten
vanhempi
sitoutus
b44f442aa7
1 muutettua tiedostoa jossa 55 lisäystä ja 14 poistoa
  1. 55 14
      webui/src/pages/WorkflowEditorPage.tsx

+ 55 - 14
webui/src/pages/WorkflowEditorPage.tsx

@@ -41,7 +41,7 @@ import { ExecutionListPanel } from '../components/workflow/ExecutionListPanel'
 import { ExecutionViewerBanner } from '../components/workflow/ExecutionViewerBanner'
 import { VersionHistoryPanel } from '../components/workflow/VersionHistoryPanel'
 import { useExecutionPinStore } from '../stores/executionPinStore'
-import type { ExecutionDetail, WorkflowNode as WorkflowNodeType, Connection as WorkflowConnection } from '../api/workflows'
+import type { ExecutionDetail, WorkflowNode as WorkflowNodeType, Connection as WorkflowConnection, NodeExecution } from '../api/workflows'
 
 // Import extracted components
 import { WorkflowNode, NodeExecutionState, IterationResult, resolveOutputs } from '../components/workflow/WorkflowNode'
@@ -70,6 +70,11 @@ interface ExecutionState {
   nodeStates: Record<string, NodeExecutionState>
   startedAt?: number
   completedAt?: number
+  // Time-sorted node execution records for a pinned/viewed execution, kept
+  // alongside nodeStates so the results panel can group loop bodies by
+  // iteration using loopNodeId/loopIteration. Absent for a live run and for
+  // executions recorded before the engine sent those fields.
+  rawNodeExecutions?: NodeExecution[]
 }
 
 interface StorageCollection {
@@ -427,11 +432,30 @@ function WorkflowEditorInner() {
     recomputeChangeStatus,
   } = useExecutionPinStore()
 
+  // A record's status as the engine wrote it, narrowed to what the panel and
+  // canvas actually distinguish. Unrecognized values fall back to
+  // 'completed' rather than disappearing, since that was the pre-existing
+  // behavior for anything this switch doesn't know about.
+  const mapRecordedStatus = (status: string): 'completed' | 'failed' | 'skipped' | 'disabled' => {
+    if (status === 'failed') return 'failed'
+    if (status === 'skipped') return 'skipped'
+    if (status === 'disabled') return 'disabled'
+    return 'completed'
+  }
+
   // Effective execution state: uses pinned execution when available (viewing or pinned), otherwise live state
   const effectiveExecutionState: ExecutionState = useMemo(() => {
     // Use pinned data when viewing OR when pinned (but live execution not running)
     if (pinnedExecution && (isViewingExecution || executionState.status === 'idle')) {
-      // Convert pinned execution to ExecutionState format
+      // Convert pinned execution to ExecutionState format. The array order
+      // coming back from the API is not guaranteed to be chronological -
+      // sort by startedAt up front so every downstream consumer (this
+      // transform, the results panel) sees the run in the order it actually
+      // happened rather than whatever order the store returned.
+      const sortedExecutions = [...pinnedExecution.nodeExecutions].sort(
+        (a, b) => (a.startedAt ?? 0) - (b.startedAt ?? 0)
+      )
+
       const nodeStates: Record<string, NodeExecutionState> = {}
 
       // First pass: collect all node executions, handling iteration-suffixed IDs
@@ -440,25 +464,31 @@ function WorkflowEditorInner() {
       // Track timestamps for iteration nodes (use earliest start, latest end)
       const iterationTimestamps: Record<string, { startedAt?: number; completedAt?: number }> = {}
 
-      for (const nodeExec of pinnedExecution.nodeExecutions) {
+      for (const nodeExec of sortedExecutions) {
         const nodeId = nodeExec.nodeId
+        const mappedStatus = mapRecordedStatus(nodeExec.status)
 
-        // Check if this is an iteration-suffixed ID (loop body node)
+        // Loop body node, either via the legacy "<id>_iter_<n>" suffix or,
+        // once the engine sends it, the explicit loopNodeId/loopIteration
+        // pair. Either way it aggregates into the base node's iterations
+        // rather than overwriting nodeStates[nodeId] on every iteration.
         const iterMatch = nodeId.match(/^(.+)_iter_(\d+)$/)
-        if (iterMatch) {
-          const baseNodeId = iterMatch[1]
-          const iterIndex = parseInt(iterMatch[2], 10)
+        const hasEngineLoopFields = nodeExec.loopNodeId !== undefined && nodeExec.loopIteration !== undefined
+        const baseNodeId = hasEngineLoopFields ? nodeId : iterMatch?.[1]
+        const iterIndex = hasEngineLoopFields ? nodeExec.loopIteration! : iterMatch ? parseInt(iterMatch[2], 10) : undefined
 
+        if (baseNodeId !== undefined && iterIndex !== undefined) {
           if (!iterationMap[baseNodeId]) {
             iterationMap[baseNodeId] = []
             iterationTimestamps[baseNodeId] = {}
           }
           iterationMap[baseNodeId].push({
             index: iterIndex,
-            status: nodeExec.status === 'completed' ? 'completed' :
-                    nodeExec.status === 'failed' ? 'failed' : 'skipped',
+            status: mappedStatus,
             output: nodeExec.output,
             error: nodeExec.error,
+            startedAt: nodeExec.startedAt,
+            completedAt: nodeExec.finishedAt,
           })
           // Track earliest startedAt and latest completedAt
           const ts = iterationTimestamps[baseNodeId]
@@ -471,9 +501,7 @@ function WorkflowEditorInner() {
         } else {
           // Regular node (not loop iteration)
           nodeStates[nodeId] = {
-            status: nodeExec.status === 'completed' ? 'completed' :
-                    nodeExec.status === 'failed' ? 'failed' :
-                    nodeExec.status === 'skipped' ? 'skipped' : 'completed',
+            status: mappedStatus,
             output: nodeExec.output,
             error: nodeExec.error,
             startedAt: nodeExec.startedAt,
@@ -487,13 +515,16 @@ function WorkflowEditorInner() {
         // Sort by iteration index
         iterations.sort((a, b) => a.index - b.index)
 
-        // Use last iteration for the main output/status
+        // Use last iteration for the main output/status. A disabled or
+        // skipped iteration never outranks an actual failure - a loop with
+        // one failed pass and ten disabled-node passes is still a loop that
+        // failed once.
         const lastIter = iterations[iterations.length - 1]
         const hasFailure = iterations.some(it => it.status === 'failed')
         const ts = iterationTimestamps[baseNodeId] || {}
 
         nodeStates[baseNodeId] = {
-          status: hasFailure ? 'failed' : 'completed',
+          status: hasFailure ? 'failed' : lastIter?.status === 'disabled' ? 'disabled' : 'completed',
           output: lastIter?.output,
           error: lastIter?.error,
           iterations: iterations,
@@ -509,6 +540,11 @@ function WorkflowEditorInner() {
         nodeStates,
         startedAt: pinnedExecution.startedAt,
         completedAt: pinnedExecution.finishedAt,
+        // Raw, time-sorted records for the results panel to group by loop
+        // iteration. Only ever set for a pinned/viewed execution - a live
+        // run keeps building nodeStates incrementally via the websocket
+        // handlers below and never populates this.
+        rawNodeExecutions: sortedExecutions,
       }
     }
     return executionState
@@ -1487,6 +1523,11 @@ function WorkflowEditorInner() {
           dynamicOutputs: nodeDef?.dynamicOutputs,
           inputs: nodeDef?.inputs,
           nodeId: n.id,
+          // Reflects the workflow AS IT WAS at execution time (the snapshot),
+          // not whatever the current graph says - this was missing before,
+          // so a node the author has since re-enabled (or disabled) still
+          // showed no "off" marker at all while viewing an old run.
+          disabled: n.disabled === true,
           onExecute: () => {}, // No-op in view mode
           onExecuteTrigger: () => {}, // No-op in view mode
           executionState: effectiveExecutionState.nodeStates[n.id],