mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-30 03:01:54 +00:00
Fix partial execution inside subgraphs (#6487)
`getExecutionIdsForSelectedNodes` is only used for partial execution. The prior implementation solved the wrong problem. Given a list of nodes, it would explore into subgraphs and return a list of partial ExecutionIds for all contained nodes. Because this does not resolve the partial execution path to the current subgraph, this is incorrect when the current graph is not the root graph. Woefully, this incorrect functionality is never useful because the recursive exploration only applies to subgraph nodes which never satisfy the outputNode filter applied by the parent function. An extra function is used to correctly append the parent execution path, but the existing, probably never useful code for recursively collecting children is otherwise left in place. Resolves #6480 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6487-Fix-partial-execution-inside-subgraphs-29d6d73d36508197924bfb3a0fb6699e) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -526,6 +526,15 @@ export function useCoreCommands(): ComfyCommand[] {
|
|||||||
// Get execution IDs for all selected output nodes and their descendants
|
// Get execution IDs for all selected output nodes and their descendants
|
||||||
const executionIds =
|
const executionIds =
|
||||||
getExecutionIdsForSelectedNodes(selectedOutputNodes)
|
getExecutionIdsForSelectedNodes(selectedOutputNodes)
|
||||||
|
if (executionIds.length === 0) {
|
||||||
|
toastStore.add({
|
||||||
|
severity: 'error',
|
||||||
|
summary: t('toastMessages.failedToQueue'),
|
||||||
|
detail: t('toastMessages.failedExecutionPathResolution'),
|
||||||
|
life: 3000
|
||||||
|
})
|
||||||
|
return
|
||||||
|
}
|
||||||
await app.queuePrompt(0, batchCount, executionIds)
|
await app.queuePrompt(0, batchCount, executionIds)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1472,6 +1472,8 @@
|
|||||||
"toastMessages": {
|
"toastMessages": {
|
||||||
"nothingToQueue": "Nothing to queue",
|
"nothingToQueue": "Nothing to queue",
|
||||||
"pleaseSelectOutputNodes": "Please select output nodes",
|
"pleaseSelectOutputNodes": "Please select output nodes",
|
||||||
|
"failedToQueue": "Failed to queue",
|
||||||
|
"failedExecutionPathResolution": "Could not resolve path to selected nodes",
|
||||||
"no3dScene": "No 3D scene to apply texture",
|
"no3dScene": "No 3D scene to apply texture",
|
||||||
"failedToApplyTexture": "Failed to apply texture",
|
"failedToApplyTexture": "Failed to apply texture",
|
||||||
"no3dSceneToExport": "No 3D scene to export",
|
"no3dSceneToExport": "No 3D scene to export",
|
||||||
|
|||||||
@@ -541,8 +541,16 @@ export function collectFromNodes<T = LGraphNode, C = void>(
|
|||||||
* @returns Array of execution IDs for selected nodes and all nodes within selected subgraphs
|
* @returns Array of execution IDs for selected nodes and all nodes within selected subgraphs
|
||||||
*/
|
*/
|
||||||
export function getExecutionIdsForSelectedNodes(
|
export function getExecutionIdsForSelectedNodes(
|
||||||
selectedNodes: LGraphNode[]
|
selectedNodes: LGraphNode[],
|
||||||
|
startGraph = selectedNodes[0]?.graph
|
||||||
): NodeExecutionId[] {
|
): NodeExecutionId[] {
|
||||||
|
if (!startGraph) return []
|
||||||
|
const rootGraph = startGraph.rootGraph
|
||||||
|
const parentPath = startGraph.isRootGraph
|
||||||
|
? ''
|
||||||
|
: findPartialExecutionPathToGraph(startGraph, rootGraph)
|
||||||
|
if (parentPath === undefined) return []
|
||||||
|
|
||||||
return collectFromNodes<NodeExecutionId, string>(selectedNodes, {
|
return collectFromNodes<NodeExecutionId, string>(selectedNodes, {
|
||||||
collector: (node, parentExecutionId) => {
|
collector: (node, parentExecutionId) => {
|
||||||
const nodeId = String(node.id)
|
const nodeId = String(node.id)
|
||||||
@@ -552,7 +560,22 @@ export function getExecutionIdsForSelectedNodes(
|
|||||||
const nodeId = String(node.id)
|
const nodeId = String(node.id)
|
||||||
return parentExecutionId ? `${parentExecutionId}:${nodeId}` : nodeId
|
return parentExecutionId ? `${parentExecutionId}:${nodeId}` : nodeId
|
||||||
},
|
},
|
||||||
initialContext: '',
|
initialContext: parentPath,
|
||||||
expandSubgraphs: true
|
expandSubgraphs: true
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function findPartialExecutionPathToGraph(
|
||||||
|
target: LGraph,
|
||||||
|
root: LGraph
|
||||||
|
): string | undefined {
|
||||||
|
for (const node of root.nodes) {
|
||||||
|
if (!node.isSubgraphNode()) continue
|
||||||
|
|
||||||
|
if (node.subgraph === target) return `${node.id}`
|
||||||
|
|
||||||
|
const subpath = findPartialExecutionPathToGraph(target, node.subgraph)
|
||||||
|
if (subpath !== undefined) return node.id + ':' + subpath
|
||||||
|
}
|
||||||
|
return undefined
|
||||||
|
}
|
||||||
|
|||||||
@@ -35,14 +35,18 @@ function createMockNode(
|
|||||||
isSubgraph?: boolean
|
isSubgraph?: boolean
|
||||||
subgraph?: Subgraph
|
subgraph?: Subgraph
|
||||||
callback?: () => void
|
callback?: () => void
|
||||||
|
graph?: LGraph
|
||||||
} = {}
|
} = {}
|
||||||
): LGraphNode {
|
): LGraphNode {
|
||||||
return {
|
const node = {
|
||||||
id,
|
id,
|
||||||
isSubgraphNode: options.isSubgraph ? () => true : undefined,
|
isSubgraphNode: options.isSubgraph ? () => true : undefined,
|
||||||
subgraph: options.subgraph,
|
subgraph: options.subgraph,
|
||||||
onExecutionStart: options.callback
|
onExecutionStart: options.callback,
|
||||||
|
graph: options.graph
|
||||||
} as unknown as LGraphNode
|
} as unknown as LGraphNode
|
||||||
|
options.graph?.nodes?.push(node)
|
||||||
|
return node
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mock graph factory
|
// Mock graph factory
|
||||||
@@ -50,20 +54,28 @@ function createMockGraph(nodes: LGraphNode[]): LGraph {
|
|||||||
return {
|
return {
|
||||||
_nodes: nodes,
|
_nodes: nodes,
|
||||||
nodes: nodes,
|
nodes: nodes,
|
||||||
|
isRootGraph: true,
|
||||||
getNodeById: (id: string | number) =>
|
getNodeById: (id: string | number) =>
|
||||||
nodes.find((n) => String(n.id) === String(id)) || null
|
nodes.find((n) => String(n.id) === String(id)) || null
|
||||||
} as unknown as LGraph
|
} as unknown as LGraph
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mock subgraph factory
|
// Mock subgraph factory
|
||||||
function createMockSubgraph(id: string, nodes: LGraphNode[]): Subgraph {
|
function createMockSubgraph(
|
||||||
return {
|
id: string,
|
||||||
|
nodes: LGraphNode[],
|
||||||
|
rootGraph?: LGraph
|
||||||
|
): Subgraph {
|
||||||
|
const graph = {
|
||||||
id,
|
id,
|
||||||
_nodes: nodes,
|
_nodes: nodes,
|
||||||
nodes: nodes,
|
nodes: nodes,
|
||||||
|
isRootGraph: false,
|
||||||
|
rootGraph,
|
||||||
getNodeById: (nodeId: string | number) =>
|
getNodeById: (nodeId: string | number) =>
|
||||||
nodes.find((n) => String(n.id) === String(nodeId)) || null
|
nodes.find((n) => String(n.id) === String(nodeId)) || null
|
||||||
} as unknown as Subgraph
|
} as unknown as Subgraph
|
||||||
|
return graph
|
||||||
}
|
}
|
||||||
|
|
||||||
describe('graphTraversalUtil', () => {
|
describe('graphTraversalUtil', () => {
|
||||||
@@ -983,31 +995,30 @@ describe('graphTraversalUtil', () => {
|
|||||||
|
|
||||||
describe('getExecutionIdsForSelectedNodes', () => {
|
describe('getExecutionIdsForSelectedNodes', () => {
|
||||||
it('should return simple IDs for top-level nodes', () => {
|
it('should return simple IDs for top-level nodes', () => {
|
||||||
const nodes = [
|
const graph = createMockGraph([])
|
||||||
createMockNode('123'),
|
createMockNode('123', { graph })
|
||||||
createMockNode('456'),
|
createMockNode('456', { graph })
|
||||||
createMockNode('789')
|
createMockNode('789', { graph })
|
||||||
]
|
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes(nodes)
|
const executionIds = getExecutionIdsForSelectedNodes(graph.nodes)
|
||||||
|
|
||||||
expect(executionIds).toEqual(['789', '456', '123']) // DFS processes in LIFO order
|
expect(executionIds).toEqual(['789', '456', '123']) // DFS processes in LIFO order
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should expand subgraph nodes to include all children', () => {
|
it('should expand subgraph nodes to include all children', () => {
|
||||||
|
const graph = createMockGraph([])
|
||||||
const subNodes = [createMockNode('10'), createMockNode('11')]
|
const subNodes = [createMockNode('10'), createMockNode('11')]
|
||||||
const subgraph = createMockSubgraph('sub-uuid', subNodes)
|
const subgraph = createMockSubgraph('sub-uuid', subNodes)
|
||||||
const nodes = [
|
createMockNode('1', { graph })
|
||||||
createMockNode('1'),
|
createMockNode('2', { isSubgraph: true, subgraph, graph })
|
||||||
createMockNode('2', { isSubgraph: true, subgraph })
|
|
||||||
]
|
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes(nodes)
|
const executionIds = getExecutionIdsForSelectedNodes(graph.nodes)
|
||||||
|
|
||||||
expect(executionIds).toEqual(['2', '2:10', '2:11', '1']) // DFS: node 2 first, then its children
|
expect(executionIds).toEqual(['2', '2:10', '2:11', '1']) // DFS: node 2 first, then its children
|
||||||
})
|
})
|
||||||
|
|
||||||
it('should handle deeply nested subgraphs correctly', () => {
|
it('should handle deeply nested subgraphs correctly', () => {
|
||||||
|
const graph = createMockGraph([])
|
||||||
const deepNodes = [createMockNode('30'), createMockNode('31')]
|
const deepNodes = [createMockNode('30'), createMockNode('31')]
|
||||||
const deepSubgraph = createMockSubgraph('deep-uuid', deepNodes)
|
const deepSubgraph = createMockSubgraph('deep-uuid', deepNodes)
|
||||||
|
|
||||||
@@ -1019,7 +1030,8 @@ describe('graphTraversalUtil', () => {
|
|||||||
|
|
||||||
const topNode = createMockNode('10', {
|
const topNode = createMockNode('10', {
|
||||||
isSubgraph: true,
|
isSubgraph: true,
|
||||||
subgraph: midSubgraph
|
subgraph: midSubgraph,
|
||||||
|
graph
|
||||||
})
|
})
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes([topNode])
|
const executionIds = getExecutionIdsForSelectedNodes([topNode])
|
||||||
@@ -1028,16 +1040,15 @@ describe('graphTraversalUtil', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
it('should handle mixed selection of regular and subgraph nodes', () => {
|
it('should handle mixed selection of regular and subgraph nodes', () => {
|
||||||
|
const graph = createMockGraph([])
|
||||||
const subNodes = [createMockNode('100'), createMockNode('101')]
|
const subNodes = [createMockNode('100'), createMockNode('101')]
|
||||||
const subgraph = createMockSubgraph('sub-uuid', subNodes)
|
const subgraph = createMockSubgraph('sub-uuid', subNodes)
|
||||||
|
|
||||||
const nodes = [
|
createMockNode('1', { graph })
|
||||||
createMockNode('1'),
|
createMockNode('2', { isSubgraph: true, subgraph, graph })
|
||||||
createMockNode('2', { isSubgraph: true, subgraph }),
|
createMockNode('3', { graph })
|
||||||
createMockNode('3')
|
|
||||||
]
|
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes(nodes)
|
const executionIds = getExecutionIdsForSelectedNodes(graph.nodes)
|
||||||
|
|
||||||
expect(executionIds).toEqual([
|
expect(executionIds).toEqual([
|
||||||
'3',
|
'3',
|
||||||
@@ -1054,10 +1065,12 @@ describe('graphTraversalUtil', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
it('should handle subgraph with no children', () => {
|
it('should handle subgraph with no children', () => {
|
||||||
|
const graph = createMockGraph([])
|
||||||
const emptySubgraph = createMockSubgraph('empty-uuid', [])
|
const emptySubgraph = createMockSubgraph('empty-uuid', [])
|
||||||
const node = createMockNode('1', {
|
const node = createMockNode('1', {
|
||||||
isSubgraph: true,
|
isSubgraph: true,
|
||||||
subgraph: emptySubgraph
|
subgraph: emptySubgraph,
|
||||||
|
graph
|
||||||
})
|
})
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes([node])
|
const executionIds = getExecutionIdsForSelectedNodes([node])
|
||||||
@@ -1079,9 +1092,11 @@ describe('graphTraversalUtil', () => {
|
|||||||
currentSubgraph = createMockSubgraph(`deep-${i}`, [node])
|
currentSubgraph = createMockSubgraph(`deep-${i}`, [node])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const graph = createMockGraph([])
|
||||||
const topNode = createMockNode('1', {
|
const topNode = createMockNode('1', {
|
||||||
isSubgraph: true,
|
isSubgraph: true,
|
||||||
subgraph: currentSubgraph
|
subgraph: currentSubgraph,
|
||||||
|
graph
|
||||||
})
|
})
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes([topNode])
|
const executionIds = getExecutionIdsForSelectedNodes([topNode])
|
||||||
@@ -1103,12 +1118,11 @@ describe('graphTraversalUtil', () => {
|
|||||||
createMockNode('101') // Same ID as in subgraph1
|
createMockNode('101') // Same ID as in subgraph1
|
||||||
])
|
])
|
||||||
|
|
||||||
const nodes = [
|
const graph = createMockGraph([])
|
||||||
createMockNode('1', { isSubgraph: true, subgraph: subgraph1 }),
|
createMockNode('1', { isSubgraph: true, subgraph: subgraph1, graph })
|
||||||
createMockNode('2', { isSubgraph: true, subgraph: subgraph2 })
|
createMockNode('2', { isSubgraph: true, subgraph: subgraph2, graph })
|
||||||
]
|
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes(nodes)
|
const executionIds = getExecutionIdsForSelectedNodes(graph.nodes)
|
||||||
|
|
||||||
expect(executionIds).toEqual([
|
expect(executionIds).toEqual([
|
||||||
'2',
|
'2',
|
||||||
@@ -1128,9 +1142,11 @@ describe('graphTraversalUtil', () => {
|
|||||||
}
|
}
|
||||||
const bigSubgraph = createMockSubgraph('big-uuid', manyNodes)
|
const bigSubgraph = createMockSubgraph('big-uuid', manyNodes)
|
||||||
|
|
||||||
|
const graph = createMockGraph([])
|
||||||
const node = createMockNode('parent', {
|
const node = createMockNode('parent', {
|
||||||
isSubgraph: true,
|
isSubgraph: true,
|
||||||
subgraph: bigSubgraph
|
subgraph: bigSubgraph,
|
||||||
|
graph
|
||||||
})
|
})
|
||||||
|
|
||||||
const start = performance.now()
|
const start = performance.now()
|
||||||
@@ -1157,19 +1173,17 @@ describe('graphTraversalUtil', () => {
|
|||||||
})
|
})
|
||||||
const midSubgraph = createMockSubgraph('mid-uuid', [midNode1, midNode2])
|
const midSubgraph = createMockSubgraph('mid-uuid', [midNode1, midNode2])
|
||||||
|
|
||||||
const topNode = createMockNode('100', {
|
const graph = createMockGraph([])
|
||||||
isSubgraph: true,
|
|
||||||
subgraph: midSubgraph
|
|
||||||
})
|
|
||||||
|
|
||||||
// Select nodes at different nesting levels
|
// Select nodes at different nesting levels
|
||||||
const selectedNodes = [
|
createMockNode('100', {
|
||||||
createMockNode('1'), // Root level
|
isSubgraph: true,
|
||||||
topNode, // Contains subgraph
|
subgraph: midSubgraph,
|
||||||
createMockNode('2') // Root level
|
graph
|
||||||
]
|
})
|
||||||
|
createMockNode('1', { graph })
|
||||||
|
createMockNode('2', { graph })
|
||||||
|
|
||||||
const executionIds = getExecutionIdsForSelectedNodes(selectedNodes)
|
const executionIds = getExecutionIdsForSelectedNodes(graph.nodes)
|
||||||
|
|
||||||
expect(executionIds).toContain('1')
|
expect(executionIds).toContain('1')
|
||||||
expect(executionIds).toContain('2')
|
expect(executionIds).toContain('2')
|
||||||
@@ -1178,6 +1192,17 @@ describe('graphTraversalUtil', () => {
|
|||||||
expect(executionIds).toContain('100:202')
|
expect(executionIds).toContain('100:202')
|
||||||
expect(executionIds).toContain('100:202:300')
|
expect(executionIds).toContain('100:202:300')
|
||||||
})
|
})
|
||||||
|
it('should resolve full execution path of a node inside a subgraph', () => {
|
||||||
|
const graph = createMockGraph([])
|
||||||
|
const subgraph = createMockSubgraph('sub-uuid', [], graph)
|
||||||
|
createMockNode('11', { graph: subgraph })
|
||||||
|
createMockNode('10', { graph: subgraph })
|
||||||
|
createMockNode('2', { isSubgraph: true, subgraph, graph })
|
||||||
|
|
||||||
|
const executionIds = getExecutionIdsForSelectedNodes(subgraph.nodes)
|
||||||
|
|
||||||
|
expect(executionIds).toEqual(['2:10', '2:11'])
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user