fix: clear stale progress bar on SubgraphNode after navigation (#9865)

## 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)
This commit is contained in:
Christian Byrne
2026-03-13 13:04:10 -07:00
committed by GitHub
parent 82556f02a9
commit 7131c274f3
2 changed files with 85 additions and 1 deletions

View File

@@ -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 })
})
}
)

View File

@@ -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) {