mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-13 09:00:16 +00:00
fix: return undefined for muted node output resolution (#9302)
## Summary Muted (NEVER mode) subgraph nodes throw "No inner node DTO found" during prompt serialization because `resolveOutput()` falls through to subgraph resolution for nodes whose inner DTOs were never registered. ## Changes - **What**: Add early return in `ExecutableNodeDTO.resolveOutput()` for `NEVER` mode nodes, matching the existing `BYPASS` mode guard. Add 5 tests covering muted, bypassed, and normal mode resolution. ## Review Focus The fix is a single-line early return. The key insight is that `graphToPrompt` in `executionUtil.ts` correctly skips `getInnerNodes()` for muted/bypassed nodes, so their inner DTOs are never in the map — but `resolveOutput()` was missing the corresponding guard for `NEVER` mode. Fixes #8986 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9302-fix-return-undefined-for-muted-node-output-resolution-3156d73d3650811e9697c7281f11cf96) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,9 +1,12 @@
|
||||
// TODO: Fix these tests after migration
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
LGraph,
|
||||
LGraphNode,
|
||||
LGraphEventMode,
|
||||
ExecutableNodeDTO
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
@@ -249,6 +252,136 @@ describe.skip('ExecutableNodeDTO Output Resolution', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('Muted node output resolution', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
})
|
||||
|
||||
it('should return undefined for NEVER mode nodes', () => {
|
||||
const graph = new LGraph()
|
||||
const node = new LGraphNode('Muted Node')
|
||||
node.addOutput('out', 'string')
|
||||
node.mode = LGraphEventMode.NEVER
|
||||
graph.add(node)
|
||||
|
||||
const dto = new ExecutableNodeDTO(node, [], new Map(), undefined)
|
||||
const resolved = dto.resolveOutput(0, 'string', new Set())
|
||||
|
||||
expect(resolved).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined for muted subgraph nodes without throwing', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
outputs: [{ name: 'out', type: 'IMAGE' }],
|
||||
nodeCount: 1
|
||||
})
|
||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||
|
||||
subgraphNode.mode = LGraphEventMode.NEVER
|
||||
|
||||
// Empty map simulates executionUtil skipping getInnerNodes() for muted nodes
|
||||
const nodesByExecutionId = new Map()
|
||||
const dto = new ExecutableNodeDTO(
|
||||
subgraphNode,
|
||||
[],
|
||||
nodesByExecutionId,
|
||||
undefined
|
||||
)
|
||||
nodesByExecutionId.set(dto.id, dto)
|
||||
|
||||
const resolved = dto.resolveOutput(0, 'IMAGE', new Set())
|
||||
expect(resolved).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should resolve undefined when input is connected to a muted node', () => {
|
||||
const graph = new LGraph()
|
||||
|
||||
const mutedNode = new LGraphNode('Muted Node')
|
||||
mutedNode.addOutput('result', 'IMAGE')
|
||||
mutedNode.mode = LGraphEventMode.NEVER
|
||||
graph.add(mutedNode)
|
||||
|
||||
const downstreamNode = new LGraphNode('Downstream')
|
||||
downstreamNode.addInput('input', 'IMAGE')
|
||||
graph.add(downstreamNode)
|
||||
|
||||
mutedNode.connect(0, downstreamNode, 0)
|
||||
|
||||
const nodeDtoMap = new Map()
|
||||
const mutedDto = new ExecutableNodeDTO(mutedNode, [], nodeDtoMap, undefined)
|
||||
nodeDtoMap.set(mutedDto.id, mutedDto)
|
||||
|
||||
const downstreamDto = new ExecutableNodeDTO(
|
||||
downstreamNode,
|
||||
[],
|
||||
nodeDtoMap,
|
||||
undefined
|
||||
)
|
||||
nodeDtoMap.set(downstreamDto.id, downstreamDto)
|
||||
|
||||
const resolved = downstreamDto.resolveInput(0)
|
||||
expect(resolved).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Bypass node output resolution', () => {
|
||||
it('should still resolve bypass for BYPASS mode nodes', () => {
|
||||
const graph = new LGraph()
|
||||
|
||||
const upstreamNode = new LGraphNode('Upstream')
|
||||
upstreamNode.addOutput('out', 'IMAGE')
|
||||
graph.add(upstreamNode)
|
||||
|
||||
const bypassedNode = new LGraphNode('Bypassed')
|
||||
bypassedNode.addInput('in', 'IMAGE')
|
||||
bypassedNode.addOutput('out', 'IMAGE')
|
||||
bypassedNode.mode = LGraphEventMode.BYPASS
|
||||
graph.add(bypassedNode)
|
||||
|
||||
upstreamNode.connect(0, bypassedNode, 0)
|
||||
|
||||
const nodeDtoMap = new Map()
|
||||
const upstreamDto = new ExecutableNodeDTO(
|
||||
upstreamNode,
|
||||
[],
|
||||
nodeDtoMap,
|
||||
undefined
|
||||
)
|
||||
nodeDtoMap.set(upstreamDto.id, upstreamDto)
|
||||
|
||||
const bypassedDto = new ExecutableNodeDTO(
|
||||
bypassedNode,
|
||||
[],
|
||||
nodeDtoMap,
|
||||
undefined
|
||||
)
|
||||
nodeDtoMap.set(bypassedDto.id, bypassedDto)
|
||||
|
||||
const resolved = bypassedDto.resolveOutput(0, 'IMAGE', new Set())
|
||||
expect(resolved).toBeDefined()
|
||||
expect(resolved?.node).toBe(upstreamDto)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ALWAYS mode node output resolution', () => {
|
||||
it('should attempt normal resolution for ALWAYS mode nodes', () => {
|
||||
const graph = new LGraph()
|
||||
const node = new LGraphNode('Normal Node')
|
||||
node.addOutput('out', 'IMAGE')
|
||||
node.mode = LGraphEventMode.ALWAYS
|
||||
graph.add(node)
|
||||
|
||||
const nodeDtoMap = new Map()
|
||||
const dto = new ExecutableNodeDTO(node, [], nodeDtoMap, undefined)
|
||||
nodeDtoMap.set(dto.id, dto)
|
||||
|
||||
const resolved = dto.resolveOutput(0, 'IMAGE', new Set())
|
||||
expect(resolved).toBeDefined()
|
||||
expect(resolved?.node).toBe(dto)
|
||||
expect(resolved?.origin_slot).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe.skip('ExecutableNodeDTO Properties', () => {
|
||||
it('should provide access to basic properties', () => {
|
||||
const graph = new LGraph()
|
||||
|
||||
@@ -266,6 +266,9 @@ export class ExecutableNodeDTO implements ExecutableLGraphNode {
|
||||
}
|
||||
visited.add(uniqueId)
|
||||
|
||||
// Muted nodes produce no output
|
||||
if (this.mode === LGraphEventMode.NEVER) return
|
||||
|
||||
// Upstreamed: Bypass nodes are bypassed using the first input with matching type
|
||||
if (this.mode === LGraphEventMode.BYPASS) {
|
||||
// Bypass nodes by finding first input with matching type
|
||||
|
||||
30
src/utils/executableGroupNodeDto.test.ts
Normal file
30
src/utils/executableGroupNodeDto.test.ts
Normal file
@@ -0,0 +1,30 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
LGraph,
|
||||
LGraphNode,
|
||||
LGraphEventMode
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import { ExecutableGroupNodeDTO } from './executableGroupNodeDto'
|
||||
|
||||
describe('Muted group node output resolution', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
})
|
||||
|
||||
it('should return undefined for NEVER mode group nodes', () => {
|
||||
const graph = new LGraph()
|
||||
const node = new LGraphNode('Muted Group')
|
||||
node.addOutput('out', 'IMAGE')
|
||||
node.mode = LGraphEventMode.NEVER
|
||||
graph.add(node)
|
||||
|
||||
const dto = new ExecutableGroupNodeDTO(node, [], new Map(), undefined)
|
||||
const resolved = dto.resolveOutput(0, 'IMAGE', new Set())
|
||||
|
||||
expect(resolved).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -24,6 +24,9 @@ export class ExecutableGroupNodeDTO extends ExecutableNodeDTO {
|
||||
}
|
||||
|
||||
override resolveOutput(slot: number, type: ISlotType, visited: Set<string>) {
|
||||
// Muted nodes produce no output
|
||||
if (this.mode === LGraphEventMode.NEVER) return
|
||||
|
||||
// Temporary duplication: Bypass nodes are bypassed using the first input with matching type
|
||||
if (this.mode === LGraphEventMode.BYPASS) {
|
||||
const { inputs } = this
|
||||
|
||||
Reference in New Issue
Block a user