From 7131c274f3f64e797b66482187f4d70fa4076d16 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Fri, 13 Mar 2026 13:04:10 -0700 Subject: [PATCH] fix: clear stale progress bar on SubgraphNode after navigation (#9865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When navigating back from a subgraph to the root graph, the SubgraphNode can retain a stale progress bar. This happens because the progress watcher in `GraphCanvas.vue` watches `[nodeLocationProgressStates, canvasStore.canvas]`, but neither value changes reference during subgraph navigation: - `nodeLocationProgressStates` is already `{}` (execution completed while viewing the subgraph) - `canvasStore.canvas` is a `shallowRef` set once at startup — only `canvas.graph` changes (via `setGraph()`) **Reproduction** (from PR #4382 comment thread by @guill): 1. Create a subgraph with a KSampler 2. Execute the workflow 3. While progress bar is halfway, enter the subgraph 4. Wait for execution to complete 5. Navigate back to root graph 6. Progress bar is stuck at 50% ## Root Cause `canvasStore.canvas` is a `shallowRef` — subgraph navigation mutates `canvas.graph` (a nested property) via `LGraphCanvas.setGraph()`, which doesn't trigger a shallow watch. The watcher never re-fires to clear stale `node.progress` values. ## Fix Add `canvasStore.currentGraph` to the watcher's dependency array. This is already a `shallowRef` in `canvasStore` that's updated on every `litegraph:set-graph` event. Zero overhead, precise targeting. ## Context - Original discussion: https://github.com/Comfy-Org/ComfyUI_frontend/pull/4382/files/BASE..868e047272f6c5d710db7e607b8997d4c243490f#r2202024855 - PR #9248 correctly removed `deep: true` from this watcher but missed the subgraph edge case - `deep: true` was the wrong fix — `canvasStore.currentGraph` is the precise solution ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9865-fix-clear-stale-progress-bar-on-SubgraphNode-after-navigation-3226d73d3650811c8f9de612d81ef98a) by [Unito](https://www.unito.io) --- .../tests/subgraphProgressClear.spec.ts | 77 +++++++++++++++++++ src/components/graph/GraphCanvas.vue | 9 ++- 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 browser_tests/tests/subgraphProgressClear.spec.ts diff --git a/browser_tests/tests/subgraphProgressClear.spec.ts b/browser_tests/tests/subgraphProgressClear.spec.ts new file mode 100644 index 0000000000..8e54a90c22 --- /dev/null +++ b/browser_tests/tests/subgraphProgressClear.spec.ts @@ -0,0 +1,77 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe( + 'Subgraph progress clear on navigation', + { tag: ['@subgraph'] }, + () => { + test('Stale progress is cleared on subgraph node after navigating back', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow('subgraphs/basic-subgraph') + await comfyPage.nextFrame() + + // Find the subgraph node + const subgraphNodeId = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph! + const subgraphNode = graph.nodes.find( + (n) => typeof n.isSubgraphNode === 'function' && n.isSubgraphNode() + ) + return subgraphNode ? String(subgraphNode.id) : null + }) + expect(subgraphNodeId).not.toBeNull() + + // Simulate a stale progress value on the subgraph node. + // This happens when: + // 1. User views root graph during execution + // 2. Progress watcher sets node.progress = 0.5 + // 3. User enters subgraph + // 4. Execution completes (nodeProgressStates becomes {}) + // 5. Watcher fires, clears subgraph-internal nodes, but root-level + // SubgraphNode isn't visible so it keeps stale progress + // 6. User navigates back — watcher should fire and clear it + await comfyPage.page.evaluate((nodeId) => { + const node = window.app!.canvas.graph!.getNodeById(nodeId)! + node.progress = 0.5 + }, subgraphNodeId!) + + // Verify progress is set + const progressBefore = await comfyPage.page.evaluate((nodeId) => { + return window.app!.canvas.graph!.getNodeById(nodeId)!.progress + }, subgraphNodeId!) + expect(progressBefore).toBe(0.5) + + // Navigate into the subgraph + const subgraphNode = await comfyPage.nodeOps.getNodeRefById( + subgraphNodeId! + ) + await subgraphNode.navigateIntoSubgraph() + + // Verify we're inside the subgraph + const inSubgraph = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph + return !!graph && 'inputNode' in graph + }) + expect(inSubgraph).toBe(true) + + // Navigate back to the root graph + await comfyPage.page.keyboard.press('Escape') + await comfyPage.nextFrame() + + // The progress watcher should fire when graph changes (because + // nodeLocationProgressStates is empty {} and the watcher should + // iterate canvas.graph.nodes to clear stale node.progress values). + // + // BUG: Without watching canvasStore.currentGraph, the watcher doesn't + // fire on subgraph->root navigation when progress is already empty, + // leaving stale node.progress = 0.5 on the SubgraphNode. + await expect(async () => { + const progressAfter = await comfyPage.page.evaluate((nodeId) => { + return window.app!.canvas.graph!.getNodeById(nodeId)!.progress + }, subgraphNodeId!) + expect(progressAfter).toBeUndefined() + }).toPass({ timeout: 2_000 }) + }) + } +) diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 48ffc52092..4a7ad30007 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -381,10 +381,17 @@ watch( * No `deep: true` needed — `nodeLocationProgressStates` is a computed that * returns a new `Record` object on every progress event (the underlying * `nodeProgressStates` ref is replaced wholesale by the WebSocket handler). + * + * `currentGraph` triggers this watcher on subgraph navigation so stale + * progress bars are cleared when returning to the root graph. */ watch( () => - [executionStore.nodeLocationProgressStates, canvasStore.canvas] as const, + [ + executionStore.nodeLocationProgressStates, + canvasStore.canvas, + canvasStore.currentGraph + ] as const, ([nodeLocationProgressStates, canvas]) => { if (!canvas?.graph) return for (const node of canvas.graph.nodes) {