Comfy-Org / Comfy-Org/ComfyUI_frontend

Load-time subgraph node id deduplication does not remap previewExposures, so a promoted preview dies on reload

Open
#16,656 0 comments 0 reactions 1 assignee Claimed by @DrJKL View on GitHub
area:previews area:subgraph Potential Bug
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Problem

`LGraph.configure` renumbers interior node ids that collide across subgraph definitions, but never updates the `previewExposures` that point at them. A host's exposure is left referencing an id that no longer exists in its own subgraph, so its promoted preview silently disappears after the workflow is reloaded.

`normalizeSubgraphDefinitions` → `deduplicateSubgraphNodeIds` (`src/lib/litegraph/src/subgraph/subgraphDeduplication.ts:175`) patches links, floating links, promoted widgets (`patchPromotedWidgets`) and legacy `proxyWidgets` (`patchProxyWidgets`). Nothing in that file touches `previewExposures` — the string does not appear in it. A `remapPreviewExposures` helper already exists at `src/lib/litegraph/src/LGraphCanvas.ts:9023`, but it is wired only into the clipboard path (`remapClipboardSubgraphNodeIds`), not into the load path.

`Comfy.Graph.DeduplicateSubgraphNodeIds` defaults to `true`, so this is the default configuration.

## Evidence

Loading a workflow with two subgraph definitions that both use interior id `3`, where the host of the second declares `previewExposures: [{ sourceNodeId: "3", ... }]`, then reading the live graph:

```
Host A previewExposures: [{ sourceNodeId: "3" }] interior: [{ id: "4", title: "A Sampler" }]
Host B (none) interior: [{ id: "3", title: "B Sampler" }]
```

`Host A`'s interior sampler was renumbered to `4`; its exposure still points at `3`. `usePromotedPreviews` resolves the leaf with `leafHost.subgraph.getNodeById(3)`, gets `undefined`, and returns `[]` — the promoted preview is gone, with no error.

Driving a live preview frame at the real path `1:4` on that fixture renders **0** previews on the pre-#14569 code path.

A unit-level repro against `deduplicateSubgraphNodeIds` directly asserts that the host exposure follows the renumbered interior node, and fails with `expected "4", received "3"`.

Both repros are in #16654 (`browser_tests/tests/subgraph/subgraphPreviewIdentity.spec.ts` and `src/lib/litegraph/src/subgraph/previewExposureRemap.repro.test.ts`). That PR is verification-only and not for merging.

## Provenance

* **Found by:** building reproductions while reviewing #14569
* **How:** read the dedup path in full, built a colliding-id fixture, inspected the live graph after load, then re-derived the same result as a unit test against `deduplicateSubgraphNodeIds`
* **Confidence:** verified by execution, at `52f6404d3` (the #14569 head at the time), with a live backend
* **Predates #14569.** Worth noting that #14569's ambient rollup incidentally hides the user-visible symptom for root-level hosts, since it does not consult exposures at all — so this may become invisible rather than fixed once that lands.

## Suggested fix

Patch `previewExposures` inside `deduplicateClonedSubgraphNodeIds` the way `patchProxyWidgets` already handles host-side references: for each host node whose `type` matches a remapped definition id, rewrite its `previewExposures[].sourceNodeId` through that definition's remap map. `remapPreviewExposures` at `LGraphCanvas.ts:9023` already does exactly this rewrite and can be reused.

## Related

* #15715 — exposures are never removed when their host or interior node is removed. Same class of gap (exposure references not maintained against node lifecycle), different trigger.
* #15627 — a separate renumbering path (`configure(keep_old = true)` colliding in `add()`'s duplicate-id guard) with a similar consequence for identity.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.