Browse Source

fix: keep pinned-execution diff live and stop duplicating execution status

Several inconsistencies around viewing/pinning a past execution:

- nodeChangeStatus (and the pinnedNodeData derived from it) was only
  computed once, at pin time. Editing after pinning left it stale -
  a node the user just changed kept reading as "unchanged, pinned
  data available" until unpin/re-pin. Recompute it whenever the live
  graph changes while something is pinned, skipping the store update
  when nothing actually changed so this can't turn into a render loop.

- ExecutionViewerBanner and ExecutionResultsPanel both showed status
  and duration for the same execution at the same time while viewing.
  The banner now owns that summary; the panel skips its own copy of
  it while viewing (it still shows the execution ID, which the banner
  does not).

- Closing the results panel while viewing used to close the panel,
  exit viewing mode, AND unpin - while the banner's "Return to Editor"
  only exited viewing mode. Two close affordances did different things
  to the same state. Closing the panel now matches "Return to Editor":
  it stops viewing but leaves the pin alone. Clearing the pin is a
  separate, deliberate action reachable from the header in every mode
  (see the toolbar commit).

- "Pin This Data" in the banner is relabeled "Keep Pinned" - the data
  was already pinned the moment "View" was clicked, so the button was
  really "stop viewing, keep what's already pinned."
fszontagh 1 month ago
parent
commit
a244017463

+ 22 - 13
webui/src/components/workflow/ExecutionResultsPanel.tsx

@@ -18,6 +18,10 @@ interface ExecutionResultsPanelProps {
   edges: Edge[]
   expandedNodes: Set<string>
   selectedIterations: Record<string, number> // Now keyed by loop node ID
+  // When true, ExecutionViewerBanner is on screen and already owns the status
+  // word and timing for this execution - this panel skips its own copy of
+  // that summary rather than showing it twice.
+  isViewingExecution?: boolean
   onClose: () => void
   onToggleNodeExpansion: (nodeId: string) => void
   onSelectIteration: (loopNodeId: string, index: number) => void // Changed: takes loop node ID
@@ -29,6 +33,7 @@ export function ExecutionResultsPanel({
   edges,
   expandedNodes,
   selectedIterations,
+  isViewingExecution = false,
   onClose,
   onToggleNodeExpansion,
   onSelectIteration,
@@ -200,25 +205,29 @@ export function ExecutionResultsPanel({
         </button>
       </div>
 
-      {/* Execution summary */}
+      {/* Execution summary. While viewing a past execution, ExecutionViewerBanner
+          already shows status and timing at the top of the page - this panel
+          only adds the execution ID, so the two don't repeat each other. */}
       <div className="flex-shrink-0 p-4 border-b border-gray-200 dark:border-slate-700">
-        <div className="flex items-center gap-2 mb-2">
-          <span className="text-sm text-gray-500 dark:text-gray-400">Status:</span>
-          <span className={`text-sm font-medium ${
-            executionState.status === 'completed' ? 'text-green-600 dark:text-green-400' :
-            executionState.status === 'failed' ? 'text-red-600 dark:text-red-400' :
-            executionState.status === 'running' ? 'text-blue-600 dark:text-blue-400' :
-            'text-gray-600 dark:text-gray-400'
-          }`}>
-            {executionState.status.charAt(0).toUpperCase() + executionState.status.slice(1)}
-          </span>
-        </div>
+        {!isViewingExecution && (
+          <div className="flex items-center gap-2 mb-2">
+            <span className="text-sm text-gray-500 dark:text-gray-400">Status:</span>
+            <span className={`text-sm font-medium ${
+              executionState.status === 'completed' ? 'text-green-600 dark:text-green-400' :
+              executionState.status === 'failed' ? 'text-red-600 dark:text-red-400' :
+              executionState.status === 'running' ? 'text-blue-600 dark:text-blue-400' :
+              'text-gray-600 dark:text-gray-400'
+            }`}>
+              {executionState.status.charAt(0).toUpperCase() + executionState.status.slice(1)}
+            </span>
+          </div>
+        )}
         {executionState.executionId && (
           <div className="text-xs text-gray-400 dark:text-gray-500 font-mono truncate">
             ID: {executionState.executionId}
           </div>
         )}
-        {executionState.startedAt && executionState.completedAt && (
+        {!isViewingExecution && executionState.startedAt && executionState.completedAt && (
           <div className="text-xs text-gray-500 dark:text-gray-400 mt-1">
             Duration: {((executionState.completedAt - executionState.startedAt) / 1000).toFixed(2)}s
           </div>

+ 2 - 1
webui/src/components/workflow/ExecutionViewerBanner.tsx

@@ -50,10 +50,11 @@ export function ExecutionViewerBanner({
           {isCompleted && (
             <button
               onClick={onPinData}
+              title="This execution's data is already pinned - return to the live editor and keep it pinned"
               className="flex items-center gap-1.5 px-3 py-1.5 bg-white/20 hover:bg-white/30 rounded-lg text-sm font-medium transition-colors"
             >
               <Pin className="w-4 h-4" />
-              Pin This Data
+              Keep Pinned
             </button>
           )}
           <button

+ 19 - 3
webui/src/pages/WorkflowEditorPage.tsx

@@ -421,6 +421,7 @@ function WorkflowEditorInner() {
     pinExecution,
     unpinExecution,
     setViewingExecution,
+    recomputeChangeStatus,
   } = useExecutionPinStore()
 
   // Effective execution state: uses pinned execution when available (viewing or pinned), otherwise live state
@@ -571,6 +572,18 @@ function WorkflowEditorInner() {
     setViewingExecution(false)
   }, [setViewingExecution])
 
+  // The pinned/unchanged-vs-changed diff is computed once at pin time. If the
+  // user keeps editing after pinning (a node is live-editable in Pinned-not-
+  // viewing mode), that diff goes stale - a node the user just changed would
+  // keep reading as "unchanged, pinned data available" until unpin/re-pin.
+  // Recompute it whenever the live graph changes while something is pinned.
+  useEffect(() => {
+    if (!pinnedExecution || isViewingExecution) return
+    const { currentNodes, currentConnections } = getCurrentWorkflowData()
+    recomputeChangeStatus(currentNodes, currentConnections)
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [nodes, edges, pinnedExecution, isViewingExecution])
+
   const { data: workflow, isLoading: workflowLoading } = useQuery({
     queryKey: ['workflow', id],
     queryFn: () => workflowsApi.get(id!),
@@ -3022,14 +3035,17 @@ function WorkflowEditorInner() {
             edges={isViewingExecution && viewedEdges ? viewedEdges : edges}
             expandedNodes={expandedNodes}
             selectedIterations={selectedIterations}
+            isViewingExecution={isViewingExecution}
             onClose={() => {
+              // Closing here means the same thing as the banner's "Return to
+              // Editor": stop looking at the read-only snapshot, but leave the
+              // pin alone. Clearing pinned data is a separate, deliberate
+              // action (the "Clear" control), reachable in every mode where
+              // something is pinned - not a side effect of closing this panel.
               setShowExecutionPanel(false)
               if (isViewingExecution) {
                 setViewingExecution(false)
               }
-              if (pinnedExecution) {
-                unpinExecution()
-              }
             }}
             onToggleNodeExpansion={toggleNodeExpansion}
             onSelectIteration={(nodeId, index) => {

+ 38 - 0
webui/src/stores/executionPinStore.ts

@@ -18,6 +18,17 @@ interface ExecutionPinState {
   setViewingExecution: (viewing: boolean) => void
   getPinnedOutput: (nodeId: string) => any | null
   getNodeExecution: (nodeId: string) => NodeExecution | null
+  /**
+   * Re-diffs the pinned execution against the graph as it stands right now.
+   * `pinExecution` only diffs once, at pin time - if the user keeps editing
+   * after pinning, the badges would otherwise keep showing the diff from the
+   * moment of pinning instead of the truth. Call this whenever the current
+   * nodes/connections change while something is pinned.
+   */
+  recomputeChangeStatus: (
+    currentNodes: WorkflowNode[],
+    currentConnections: Connection[]
+  ) => void
 }
 
 /**
@@ -167,4 +178,31 @@ export const useExecutionPinStore = create<ExecutionPinState>((set, get) => ({
       null
     )
   },
+
+  recomputeChangeStatus: (currentNodes, currentConnections) => {
+    const state = get()
+    const { pinnedExecution } = state
+    if (!pinnedExecution) return
+
+    const nodeChangeStatus = analyzeChanges(
+      pinnedExecution,
+      currentNodes,
+      currentConnections
+    )
+    const pinnedNodeData = extractPinnedNodeData(pinnedExecution, nodeChangeStatus)
+
+    // Skip the update when nothing actually changed. The caller re-runs this
+    // on every render where the graph reference changes, which can happen for
+    // reasons unrelated to an actual edit (e.g. presence/lock bookkeeping
+    // touching the nodes array). Writing new state unconditionally here would
+    // turn that into a render loop: this store update triggers a re-render,
+    // which can hand back a "changed" nodes reference, which triggers this
+    // again. Comparing content first breaks that cycle.
+    const unchanged =
+      JSON.stringify(nodeChangeStatus) === JSON.stringify(state.nodeChangeStatus) &&
+      JSON.stringify(pinnedNodeData) === JSON.stringify(state.pinnedNodeData)
+    if (unchanged) return
+
+    set({ nodeChangeStatus, pinnedNodeData })
+  },
 }))