fix(subgraph): address promotion review gaps

Amp-Thread-ID: https://ampcode.com/threads/T-019e05c3-bed1-706a-a7a7-27733a6ab1e4
Co-authored-by: Amp <amp@ampcode.com>

src/components/builder/AppBuilder.vue

@@ -29,6 +29,7 @@ import { renameWidget } from '@/utils/widgetUtil'
 import { useAppMode } from '@/composables/useAppMode'
 import { nodeTypeValidForApp, useAppModeStore } from '@/stores/appModeStore'
 import { resolveNodeWidget } from '@/utils/litegraphUtil'
+import { createNodeLocatorId } from '@/types/nodeIdentification'
 import { cn } from '@comfyorg/tailwind-utils'
 
 type BoundStyle = { top: string; left: string; width: string; height: string }
@@ -157,10 +158,12 @@ function handleClick(e: MouseEvent) {
   }
   if (!isSelectInputsMode.value || widget.options.canvasOnly) return
 
-  const storeId = isPromotedWidgetView(widget) ? widget.sourceNodeId : node.id
-  const storeName = isPromotedWidgetView(widget)
-    ? widget.sourceWidgetName
-    : widget.name
+  const isPromoted = isPromotedWidgetView(widget)
+  const storeId =
+    isPromoted && app.rootGraph?.id
+      ? createNodeLocatorId(app.rootGraph.id, node.id)
+      : node.id
+  const storeName = widget.name
   const index = appModeStore.selectedInputs.findIndex(
     ([nodeId, widgetName]) => storeId == nodeId && storeName === widgetName
   )

src/composables/node/usePromotedPreviews.test.ts

@@ -268,4 +268,55 @@ describe(usePromotedPreviews, () => {
     expect(promotedPreviews.value).toHaveLength(1)
     expect(promotedPreviews.value[0].urls).toEqual(mockUrls)
   })
+
+  it('renders leaf media exposed through a nested subgraph host', () => {
+    const innerSetup = createSetup()
+    const leafNode = addInteriorNode(innerSetup, {
+      id: 10,
+      previewMediaType: 'image'
+    })
+
+    const outerSetup = createSetup()
+    const innerHost = createTestSubgraphNode(innerSetup.subgraph, { id: 20 })
+    outerSetup.subgraph.add(innerHost)
+
+    const store = usePreviewExposureStore()
+    store.addExposure(
+      outerSetup.subgraphNode.rootGraph.id,
+      createNodeLocatorId(outerSetup.subgraphNode.rootGraph.id, innerHost.id),
+      {
+        sourceNodeId: String(leafNode.id),
+        sourcePreviewName: '$$canvas-image-preview'
+      }
+    )
+    store.addExposure(
+      outerSetup.subgraphNode.rootGraph.id,
+      createNodeLocatorId(
+        outerSetup.subgraphNode.rootGraph.id,
+        outerSetup.subgraphNode.id
+      ),
+      {
+        sourceNodeId: String(innerHost.id),
+        sourcePreviewName: '$$canvas-image-preview'
+      }
+    )
+
+    const mockUrls = ['/view?filename=leaf.png']
+    seedOutputs(innerSetup.subgraph.id, [leafNode.id])
+    getNodeImageUrls.mockImplementation((node: LGraphNode) =>
+      node === leafNode ? mockUrls : []
+    )
+
+    const { promotedPreviews } = usePromotedPreviews(
+      () => outerSetup.subgraphNode
+    )
+    expect(promotedPreviews.value).toEqual([
+      {
+        sourceNodeId: '10',
+        sourceWidgetName: '$$canvas-image-preview',
+        type: 'image',
+        urls: mockUrls
+      }
+    ])
+  })
 })

src/composables/node/usePromotedPreviews.ts

@@ -6,6 +6,7 @@ import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode'
 import { useNodeOutputStore } from '@/stores/nodeOutputStore'
 import { usePreviewExposureStore } from '@/stores/previewExposureStore'
 import { createNodeLocatorId } from '@/types/nodeIdentification'
+import type { UUID } from '@/lib/litegraph/src/utils/uuid'
 
 interface PromotedPreview {
   /** Source node id resolved on the host's interior subgraph. */
@@ -41,6 +42,7 @@ export function usePromotedPreviews(
     )
 
     const exposurePairs = exposures.map((exposure) => ({
+      exposureName: exposure.name,
       sourceNodeId: exposure.sourceNodeId,
       sourceWidgetName: exposure.sourcePreviewName
     }))
@@ -48,16 +50,49 @@ export function usePromotedPreviews(
     if (!exposurePairs.length) return []
 
     const previews: PromotedPreview[] = []
+    const hostNodesByLocator = new Map<string, SubgraphNode>([
+      [hostLocator, node]
+    ])
+
+    const resolveNestedHost = (
+      rootGraphId: UUID,
+      currentHostLocator: string,
+      sourceNodeId: string
+    ) => {
+      const currentHost = hostNodesByLocator.get(currentHostLocator)
+      const sourceNode = currentHost?.subgraph.getNodeById(sourceNodeId)
+      if (!(sourceNode instanceof SubgraphNode)) return undefined
+
+      const nestedHostLocator = createNodeLocatorId(rootGraphId, sourceNode.id)
+      hostNodesByLocator.set(nestedHostLocator, sourceNode)
+      return { rootGraphId, hostNodeLocator: nestedHostLocator }
+    }
 
     for (const pair of exposurePairs) {
-      const interiorNode = node.subgraph.getNodeById(pair.sourceNodeId)
+      const resolved = previewExposureStore.resolveChain(
+        rootGraphId,
+        hostLocator,
+        pair.exposureName,
+        resolveNestedHost
+      )
+      const leaf = resolved?.leaf ?? {
+        sourceNodeId: pair.sourceNodeId,
+        sourcePreviewName: pair.sourceWidgetName
+      }
+      const leafHostLocator =
+        resolved?.steps.at(-1)?.hostNodeLocator ?? hostLocator
+      const leafHost = hostNodesByLocator.get(leafHostLocator) ?? node
+      const interiorNode = leafHost.subgraph.getNodeById(leaf.sourceNodeId)
       if (!interiorNode) continue
 
       // Read from both reactive refs to establish Vue dependency
       // tracking. getNodeImageUrls reads from non-reactive
       // app.nodeOutputs / app.nodePreviewImages, so without this
       // access the computed would never re-evaluate.
-      const locatorId = createNodeLocatorId(node.subgraph.id, pair.sourceNodeId)
+      const locatorId = createNodeLocatorId(
+        leafHost.subgraph.id,
+        leaf.sourceNodeId
+      )
       const reactiveOutputs = nodeOutputStore.nodeOutputs[locatorId]
       const reactivePreviews = nodeOutputStore.nodePreviewImages[locatorId]
       if (!reactiveOutputs?.images?.length && !reactivePreviews?.length)
@@ -74,8 +109,8 @@ export function usePromotedPreviews(
             : 'image'
 
       previews.push({
-        sourceNodeId: pair.sourceNodeId,
-        sourceWidgetName: pair.sourceWidgetName,
+        sourceNodeId: leaf.sourceNodeId,
+        sourceWidgetName: leaf.sourcePreviewName,
         type,
         urls
       })

src/core/graph/subgraph/migration/classifyProxyEntry.test.ts

@@ -114,6 +114,35 @@ describe(classifyProxyEntry, () => {
         subgraphInputName: 'second_seed'
       })
     })
+
+    it('quarantines ambiguous already-linked inputs without a disambiguator', () => {
+      const host = buildHost()
+      const innerNode = new LGraphNode('Inner')
+      innerNode.addWidget('number', 'seed', 0, () => {})
+      host.subgraph.add(innerNode)
+
+      for (const inputName of ['first_seed', 'second_seed']) {
+        const input = host.addInput(inputName, '*')
+        input._widget = fromPartial<PromotedWidgetView>({
+          node: host,
+          name: 'seed',
+          sourceNodeId: String(innerNode.id),
+          sourceWidgetName: 'seed'
+        })
+      }
+
+      const normalized = makeSource(String(innerNode.id), 'seed')
+      const result = classifyProxyEntry({
+        hostNode: host,
+        normalized,
+        cohort: [normalized]
+      })
+
+      expect(result).toEqual({
+        classification: 'unknown',
+        plan: { kind: 'quarantine', reason: 'ambiguousSubgraphInput' }
+      })
+    })
   })
 
   describe('quarantine branches', () => {

src/core/graph/subgraph/migration/classifyProxyEntry.ts

@@ -26,10 +26,16 @@ interface ClassifyProxyEntryArgs {
 
 const PRIMITIVE_NODE_TYPE = 'PrimitiveNode'
 
-function findLinkedSubgraphInputName(
+type LinkedInputMatch =
+  | { kind: 'none' }
+  | { kind: 'one'; inputName: string }
+  | { kind: 'ambiguous' }
+
+function findLinkedSubgraphInputMatch(
   hostNode: SubgraphNode,
   normalized: PromotedWidgetSource
-): string | undefined {
+): LinkedInputMatch {
+  const matches: string[] = []
   for (const input of hostNode.inputs) {
     const widget = input._widget
     if (!widget || !isPromotedWidgetView(widget)) continue
@@ -40,10 +46,12 @@ function findLinkedSubgraphInputName(
         widget.disambiguatingSourceNodeId ===
           normalized.disambiguatingSourceNodeId)
     ) {
-      return input.name
+      matches.push(input.name)
     }
   }
-  return undefined
+  if (matches.length === 0) return { kind: 'none' }
+  if (matches.length === 1) return { kind: 'one', inputName: matches[0] }
+  return { kind: 'ambiguous' }
 }
 
 function collectPrimitiveTargets(
@@ -84,11 +92,17 @@ export function classifyProxyEntry(
 ): ClassificationResult {
   const { hostNode, normalized, cohort } = args
 
-  const linkedInputName = findLinkedSubgraphInputName(hostNode, normalized)
-  if (linkedInputName !== undefined) {
+  const linkedInput = findLinkedSubgraphInputMatch(hostNode, normalized)
+  if (linkedInput.kind === 'one') {
     return {
       classification: 'value',
-      plan: { kind: 'alreadyLinked', subgraphInputName: linkedInputName }
+      plan: { kind: 'alreadyLinked', subgraphInputName: linkedInput.inputName }
+    }
+  }
+  if (linkedInput.kind === 'ambiguous') {
+    return {
+      classification: 'unknown',
+      plan: { kind: 'quarantine', reason: 'ambiguousSubgraphInput' }
     }
   }
 

src/core/graph/subgraph/migration/repairValueWidget.test.ts

@@ -158,6 +158,47 @@ describe(repairValueWidget, () => {
       expect(secondInput._widget?.value).toBe(99)
     })
 
+    it('does not apply host value when already-linked inputs are ambiguous', () => {
+      const host = buildHost()
+      const innerNode = new LGraphNode('Inner')
+      innerNode.addWidget('number', 'seed', 0, () => {})
+      host.subgraph.add(innerNode)
+
+      const firstInput = host.addInput('first_seed', '*')
+      firstInput._widget = fromPartial<PromotedWidgetView>({
+        node: host,
+        name: 'seed',
+        sourceNodeId: String(innerNode.id),
+        sourceWidgetName: 'seed',
+        value: 1
+      })
+      const secondInput = host.addInput('second_seed', '*')
+      secondInput._widget = fromPartial<PromotedWidgetView>({
+        node: host,
+        name: 'seed',
+        sourceNodeId: String(innerNode.id),
+        sourceWidgetName: 'seed',
+        value: 2
+      })
+
+      const result = repairValueWidget({
+        hostNode: host,
+        entry: buildEntry({
+          sourceNodeId: String(innerNode.id),
+          sourceWidgetName: 'seed',
+          plan: {
+            kind: 'alreadyLinked',
+            subgraphInputName: undefined as never
+          },
+          hostValue: 99
+        })
+      })
+
+      expect(result).toEqual({ ok: false, reason: 'ambiguousSubgraphInput' })
+      expect(firstInput._widget?.value).toBe(1)
+      expect(secondInput._widget?.value).toBe(2)
+    })
+
     it('returns missingSubgraphInput when the linked SubgraphInput is gone', () => {
       const host = buildHost()
       const innerNode = new LGraphNode('Inner')

src/core/graph/subgraph/migration/repairValueWidget.ts

@@ -25,9 +25,17 @@ function findHostInputForLinkedSource(
   hostNode: SubgraphNode,
   sourceNodeId: string,
   sourceWidgetName: string,
+  subgraphInputName: string | undefined,
   disambiguatingSourceNodeId?: string
-) {
-  return hostNode.inputs.find((input) => {
+):
+  | { kind: 'none' }
+  | { kind: 'one'; input: INodeInputSlot }
+  | { kind: 'ambiguous' } {
+  const candidates = subgraphInputName
+    ? hostNode.inputs.filter((input) => input.name === subgraphInputName)
+    : hostNode.inputs
+
+  const matches = candidates.filter((input) => {
     const widget = input._widget
     if (!widget || !isPromotedWidgetView(widget)) return false
     return (
@@ -37,6 +45,9 @@ function findHostInputForLinkedSource(
         widget.disambiguatingSourceNodeId === disambiguatingSourceNodeId)
     )
   })
+  if (matches.length === 0) return { kind: 'none' }
+  if (matches.length === 1) return { kind: 'one', input: matches[0] }
+  return { kind: 'ambiguous' }
 }
 
 function applyHostValue(
@@ -55,14 +66,20 @@ function repairAlreadyLinked(
     hostNode,
     entry.normalized.sourceNodeId,
     entry.normalized.sourceWidgetName,
+    entry.plan.kind === 'alreadyLinked'
+      ? entry.plan.subgraphInputName
+      : undefined,
     entry.normalized.disambiguatingSourceNodeId
   )
-  if (!hostInput?._widget) {
+  if (hostInput.kind === 'ambiguous') {
+    return { ok: false, reason: 'ambiguousSubgraphInput' }
+  }
+  if (hostInput.kind === 'none' || !hostInput.input._widget) {
     return { ok: false, reason: 'missingSubgraphInput' }
   }
 
-  applyHostValue(hostInput._widget, entry.hostValue)
-  return { ok: true, subgraphInputName: hostInput.name }
+  applyHostValue(hostInput.input._widget, entry.hostValue)
+  return { ok: true, subgraphInputName: hostInput.input.name }
 }
 
 function repairCreateSubgraphInput(

src/lib/litegraph/src/subgraph/SubgraphNode.ts

@@ -726,7 +726,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
     let valueIndex = 0
     for (const input of this.inputs) {
       const widget = input._widget
-      if (!widget) continue
+      if (!widget || !isPromotedWidgetView(widget)) continue
       if (valueIndex >= widgetValues.length) return
       widget.value = widgetValues[valueIndex]
       valueIndex += 1
@@ -1232,18 +1232,24 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
 
     const serialized = super.serialize()
     const widgetStore = useWidgetValueStore()
-    const widgetValues = this.inputs.flatMap((input) => {
+    const widgetValues: TWidgetValue[] = []
+    let hasSerializableValue = false
+
+    for (const input of this.inputs) {
       const widget = input._widget
-      if (!widget || !isPromotedWidgetView(widget)) return []
+      if (!widget || !isPromotedWidgetView(widget)) continue
       const state = widgetStore.getWidget(
         rootGraphId,
         this.id,
         getPromotedWidgetHostStateName(widget)
       )
-      return state && isWidgetValue(state.value) ? [state.value] : []
-    })
+      const value =
+        state && isWidgetValue(state.value) ? state.value : undefined
+      widgetValues.push(value)
+      hasSerializableValue ||= value !== undefined
+    }
 
-    if (widgetValues.length > 0) serialized.widgets_values = widgetValues
+    if (hasSerializableValue) serialized.widgets_values = widgetValues
 
     return serialized
   }

src/stores/appModeStore.ts

@@ -20,7 +20,7 @@ import { ChangeTracker } from '@/scripts/changeTracker'
 import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes'
 import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
 import { createNodeLocatorId } from '@/types/nodeIdentification'
-import { resolveNode } from '@/utils/litegraphUtil'
+import { resolveNode, resolveNodeWidget } from '@/utils/litegraphUtil'
 
 export function nodeTypeValidForApp(type: string) {
   return !['Note', 'MarkdownNote'].includes(type)
@@ -61,7 +61,7 @@ export const useAppModeStore = defineStore('appMode', () => {
         ? rawInputs
             .map(migrateLegacyInputTuple)
             .filter((entry): entry is LinearInput => entry !== null)
-            .filter(([nodeId]) => resolveNode(nodeId))
+            .filter(selectedInputExists)
         : rawInputs,
       outputs: app.rootGraph
         ? rawOutputs.filter((nodeId) => resolveNode(nodeId))
@@ -69,6 +69,15 @@ export const useAppModeStore = defineStore('appMode', () => {
     }
   }
 
+  function selectedInputExists([nodeId, widgetName]: LinearInput): boolean {
+    if (typeof nodeId === 'string' && nodeId.includes(':')) {
+      if (typeof app.rootGraph?.getNodeById !== 'function') return true
+      const [, widget] = resolveNodeWidget(nodeId, widgetName)
+      return Boolean(widget)
+    }
+    return Boolean(resolveNode(nodeId))
+  }
+
   /**
    * If a legacy tuple references the interior `(sourceNodeId, widgetName)`
    * of a now-promoted widget, project it through the wrapping host
@@ -102,6 +111,9 @@ export const useAppModeStore = defineStore('appMode', () => {
     const rootGraph = app.rootGraph
     if (!rootGraph) return null
 
+    const matches: Array<{ hostLocator: string; subgraphInputName: string }> =
+      []
+
     for (const node of rootGraph.nodes) {
       if (!(node instanceof SubgraphNode)) continue
 
@@ -112,14 +124,21 @@ export const useAppModeStore = defineStore('appMode', () => {
           widget.sourceNodeId === String(legacySourceNodeId) &&
           widget.sourceWidgetName === legacyWidgetName
         ) {
-          return {
+          matches.push({
             hostLocator: createNodeLocatorId(rootGraph.id, node.id),
             subgraphInputName: inputSlot.name
-          }
+          })
         }
       }
     }
 
+    if (matches.length === 1) return matches[0]
+    if (matches.length > 1) {
+      console.warn(
+        '[appModeStore] dropping ambiguous legacy selectedInput tuple',
+        { storedId: legacySourceNodeId, widgetName: legacyWidgetName }
+      )
+    }
     return null
   }
 

src/utils/litegraphUtil.ts

@@ -26,6 +26,7 @@ import type { InputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
 import { useToastStore } from '@/platform/updates/common/toastStore'
 import { app } from '@/scripts/app'
 import { t } from '@/i18n'
+import { parseNodeLocatorId } from '@/types/nodeIdentification'
 
 type ImageNode = LGraphNode & { imgs: HTMLImageElement[] | undefined }
 type VideoNode = LGraphNode & {
@@ -333,6 +334,17 @@ export function resolveNodeWidget(
   widgetName?: string,
   graph: LGraph = app.rootGraph
 ): [LGraphNode, IBaseWidget] | [LGraphNode] | [] {
+  if (widgetName && typeof nodeId === 'string') {
+    const locator = parseNodeLocatorId(nodeId)
+    if (locator?.subgraphUuid) {
+      const host = graph.getNodeById(locator.localNodeId)
+      if (host?.isSubgraphNode()) {
+        const widget = host.widgets?.find((w) => w.name === widgetName)
+        return widget ? [host, widget] : []
+      }
+    }
+  }
+
   const node = graph.getNodeById(nodeId)
   if (!widgetName) return node ? [node] : []
   if (node) {