From 4c0df82aee32d561d10edc8bf1bd03b14191f8cf Mon Sep 17 00:00:00 2001 From: DrJKL Date: Tue, 12 May 2026 16:49:33 -0700 Subject: [PATCH] fix(subgraph): address review blockers on promotion + clone hygiene - proxyWidgetMigration: when a disambiguator is supplied but doesn't match, only fall back to non-promoted widgets with the same name so we don't silently bind to a sibling PromotedWidgetView. - previewExposureChain: chainFromLastStep was using the already-advanced currentRootGraphId for the leaf, producing an inconsistent (rootGraphId, sourceNodeId) pair on cycle / no-exposure / max-depth exits. Use the last pushed step's rootGraphId instead. - promotedWidgetView.applyValueControlToHost: control-after-generate was reassigning this.value, which cascades into the shared interior widget via the value setter. Switch to hydrateHostValue so the per-host overlay is updated without touching shared state. - promotionUtils: capture promoteValueWidgetViaSubgraphInput results at both call sites and emit a warning Sentry breadcrumb on failure instead of silently dropping {ok:false, reason}. - LGraphNode.clone: in UUID mode, configure() was overwriting the borrowed source id with the freshly generated UUID before subclass hydration could run. Generate the UUID up front and reapply it after configure() so hydration sees the borrowed id and the cloned node still gets a fresh identity. Amp-Thread-ID: https://ampcode.com/threads/T-019e1e84-3270-730a-8c95-49a383ffdf20 Co-authored-by: Amp --- .../migration/proxyWidgetMigration.ts | 7 ++++++ .../subgraph/preview/previewExposureChain.ts | 8 +++---- src/core/graph/subgraph/promotedWidgetView.ts | 2 +- src/core/graph/subgraph/promotionUtils.ts | 22 +++++++++++++++++-- src/lib/litegraph/src/LGraphNode.ts | 5 ++++- 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts index f323a51320..338c43603f 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts @@ -50,6 +50,13 @@ function findSourceWidget( w.sourceWidgetName === sourceWidgetName ) if (byDisambiguator) return byDisambiguator + // Disambiguator was provided but missed: only fall back to non-promoted + // widgets with the same name. Returning a sibling PromotedWidgetView + // bound to a different interior node would silently re-introduce the + // cross-binding bug the disambiguator exists to prevent. + return widgets.find( + (w) => !isPromotedWidgetView(w) && w.name === sourceWidgetName + ) } return widgets.find((w) => w.name === sourceWidgetName) diff --git a/src/core/graph/subgraph/preview/previewExposureChain.ts b/src/core/graph/subgraph/preview/previewExposureChain.ts index 2157d4ceba..1562bc93cf 100644 --- a/src/core/graph/subgraph/preview/previewExposureChain.ts +++ b/src/core/graph/subgraph/preview/previewExposureChain.ts @@ -45,13 +45,13 @@ export function resolvePreviewExposureChain( const chainFromLastStep = (): ResolvedPreviewChain | undefined => { if (steps.length === 0) return undefined - const last = steps[steps.length - 1].exposure + const lastStep = steps[steps.length - 1] return { steps, leaf: { - rootGraphId: currentRootGraphId, - sourceNodeId: last.sourceNodeId, - sourcePreviewName: last.sourcePreviewName + rootGraphId: lastStep.rootGraphId, + sourceNodeId: lastStep.exposure.sourceNodeId, + sourcePreviewName: lastStep.exposure.sourcePreviewName } } } diff --git a/src/core/graph/subgraph/promotedWidgetView.ts b/src/core/graph/subgraph/promotedWidgetView.ts index 8a33bc1efa..579bcf2c88 100644 --- a/src/core/graph/subgraph/promotedWidgetView.ts +++ b/src/core/graph/subgraph/promotedWidgetView.ts @@ -449,7 +449,7 @@ class PromotedWidgetView implements IPromotedWidgetView { next = Math.floor(Math.random() * range) * step2 + safeMin } next = Math.min(Math.max(next, min), max) - this.value = next + this.hydrateHostValue(next) } private resolveAtHost(): diff --git a/src/core/graph/subgraph/promotionUtils.ts b/src/core/graph/subgraph/promotionUtils.ts index eb3ffe2a92..d9120a612f 100644 --- a/src/core/graph/subgraph/promotionUtils.ts +++ b/src/core/graph/subgraph/promotionUtils.ts @@ -356,7 +356,18 @@ export function promoteWidget( continue } if ('getSlotFromWidget' in node) { - promoteValueWidgetViaSubgraphInput(parent, node as LGraphNode, widget) + const result = promoteValueWidgetViaSubgraphInput( + parent, + node as LGraphNode, + widget + ) + if (!result.ok) { + Sentry.addBreadcrumb({ + category: 'subgraph', + level: 'warning', + message: `Failed to promote widget "${source.sourceWidgetName}" on node ${node.id}: ${result.reason}` + }) + } } } refreshPromotedWidgetRendering(parents) @@ -571,7 +582,14 @@ export function promoteRecommendedWidgets(subgraphNode: SubgraphNode) { .filter(isRecommendedWidget) .filter(([, widget]) => !isPreviewPseudoWidget(widget)) for (const [n, w] of filteredWidgets) { - promoteValueWidgetViaSubgraphInput(subgraphNode, n, w) + const result = promoteValueWidgetViaSubgraphInput(subgraphNode, n, w) + if (!result.ok) { + Sentry.addBreadcrumb({ + category: 'subgraph', + level: 'warning', + message: `Failed to promote widget "${getWidgetName(w)}" on node ${n.id}: ${result.reason}` + }) + } } subgraphNode.computeSize(subgraphNode.size) } diff --git a/src/lib/litegraph/src/LGraphNode.ts b/src/lib/litegraph/src/LGraphNode.ts index 989f2b7229..6b82ebaa3e 100644 --- a/src/lib/litegraph/src/LGraphNode.ts +++ b/src/lib/litegraph/src/LGraphNode.ts @@ -1023,13 +1023,16 @@ export class LGraphNode // @ts-expect-error Exceptional case: id is removed so that the graph can assign a new one on add. data.id = undefined - if (LiteGraph.use_uuids) data.id = LiteGraph.uuidv4() + // In UUID mode, configure() would overwrite the borrowed id with data.id; + // generate the fresh UUID up front and reapply it after hydration. + const clonedUuid = LiteGraph.use_uuids ? LiteGraph.uuidv4() : undefined // Subclasses' configure() (e.g. SubgraphNode) keys per-instance state // by id; borrow the source's id so that hydration targets the right slot // before graph.add reassigns. node.id = this.id node.configure(data) + if (clonedUuid !== undefined) node.id = clonedUuid return node }