Fix inconsistent subgraphNode usage #1193
Conversation
Prior to this commit, subgraphNode inconsistently refers to either the parent graph, or to indicate the current node is a subgraph. This corrects the usage of subgraphNode to consistently refer to the subgraph instance as defined in the constructor. This requires a corresponding frontend commit to function, and solves a bug where graph serialization fails due to an incorrectly reported infinite loop.
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
This fix correctly addresses a semantic error in ExecutableNodeDTO.getInnerNodes() where subgraphNode was being misused.
The subgraphNode property in ExecutableNodeDTO refers to the parent SubgraphNode instance that contains the current node, not whether the current node itself is a subgraph. The original code was checking 'does this node have a parent subgraph?' when it should have been checking 'is this node itself a subgraph that needs flattening?'
The corrected version using this.node.isSubgraphNode() properly identifies when the wrapped node is a SubgraphNode that contains inner nodes requiring recursive flattening. This aligns with the DTO pattern's purpose of wrapping nodes with execution context while maintaining separation between node properties and execution state.
Without this fix, the incorrect traversal during graph flattening would trigger false positive infinite loop detection during serialization, particularly when dealing with nested subgraph structures. The ExecutableNodeDTO acts as a decorator that adds hierarchical path information (like '1:2:3') for execution tracking, and proper node type checking is essential for maintaining this hierarchy.
This is a clean, minimal fix that resolves the serialization bug without any architectural changes or side effects.
|
Comfy-Org/ComfyUI_frontend#4686 will still be unresolved until Comfy-Org/ComfyUI_frontend#4688 |
Prior to this commit, subgraphNode inconsistently refers to either the parent graph, or to indicate the current node is a subgraph. This corrects the usage of subgraphNode to consistently refer to the subgraph instance as defined in the constructor. This solves a bug where graph serialization fails due to an incorrectly reported infinite loop. Port of Comfy-Org/litegraph.js#1193
2 participants
Prior to this commit, subgraphNode inconsistently refers to either the parent graph, or to indicate the current node is a subgraph.
This corrects the usage of subgraphNode to consistently refer to the subgraph instance as defined in the constructor.
This requires a corresponding frontend commit to function, and solves a bug where graph serialization fails due to an incorrectly reported infinite loop.