mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-14 09:27:41 +00:00
7131c274f3
## 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)
78 lines
3.0 KiB
TypeScript
78 lines
3.0 KiB
TypeScript
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 })
|
|
})
|
|
}
|
|
)
|