Comfy-Org / Comfy-Org/ComfyUI_frontend
fix: subgraph-output floating links write origin side instead of target
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
## Summary
`FloatingRenderLink.connectToSubgraphOutput` writes the subgraph-output boundary to the **origin** side of the floating link (`origin_id` / `origin_slot`), but a subgraph output node is a data *sink* and should be written to the **target** side (`target_id` / `target_slot`).
Writing the output node into `origin_id` leaves the link's target unbound and bypasses `LLink.targetIsIoNode` resolution.
## Location
`src/lib/litegraph/src/canvas/FloatingRenderLink.ts` (`connectToSubgraphOutput`)
```ts
connectToSubgraphOutput(
output: SubgraphOutput,
_events?: CustomEventTarget
): void {
const floatingLink = this.link
floatingLink.origin_id = SUBGRAPH_OUTPUT_NODE_ID
floatingLink.origin_slot = output.parent.slots.indexOf(output)
this.fromSlot._floatingLinks?.delete(floatingLink)
output._floatingLinks ??= new Set()
output._floatingLinks.add(floatingLink)
}
```
## Why this looks wrong
The sibling method `connectToSubgraphInput` correctly writes the **origin** side, because a subgraph input node is a data *source*:
```ts
floatingLink.origin_id = SUBGRAPH_INPUT_NODE_ID
floatingLink.origin_slot = input.parent.slots.indexOf(input)
```
By symmetry, the output (sink) side should write `target_*`. This is corroborated by `src/lib/litegraph/src/subgraph/subgraphUtils.ts`, which builds subgraph-output links using the target side:
```ts
linkData.target_id = SUBGRAPH_OUTPUT_NODE_ID
linkData.target_slot = outputs.length
```
So the rest of the system treats subgraph output as the target side; the floating-link path here is inconsistent.
## Proposed fix
```diff
connectToSubgraphOutput(
output: SubgraphOutput,
_events?: CustomEventTarget
): void {
const floatingLink = this.link
- floatingLink.origin_id = SUBGRAPH_OUTPUT_NODE_ID
- floatingLink.origin_slot = output.parent.slots.indexOf(output)
+ floatingLink.target_id = SUBGRAPH_OUTPUT_NODE_ID
+ floatingLink.target_slot = output.parent.slots.indexOf(output)
this.fromSlot._floatingLinks?.delete(floatingLink)
output._floatingLinks ??= new Set()
output._floatingLinks.add(floatingLink)
}
```
## Acceptance criteria
- Dragging a floating link onto a subgraph output produces a link whose **target** endpoint is `SUBGRAPH_OUTPUT_NODE_ID` with the correct slot.
- Add a regression test covering drag-to-subgraph-output that asserts the resulting link endpoints and survives a serialize/deserialize round-trip.
- Verify floating-link rendering for subgraph outputs is unaffected.
## Notes
- Surfaced by CodeRabbit during review of #12958 (NodeId branded-number refactor). It is a **pre-existing** behavior unrelated to that typing change, so it was deliberately split out rather than bundled into the refactor PR.
- Needs a litegraph/subgraph-owner review given the impact on floating-link resolution, rendering, and serialization.
Contributor guide
Research direction
Start in src/lib/litegraph/src/canvas/FloatingRenderLink.ts at connectToSubgraphOutput, then compare connectToSubgraphInput and the target-side construction in src/lib/litegraph/src/subgraph/subgraphUtils.ts. Add a regression test for dragging to a subgraph output, including endpoint assertions and a serialize/deserialize round-trip; verify floating-link rendering remains unaffected.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100