diff --git a/browser_tests/fixtures/helpers/SubgraphHelper.ts b/browser_tests/fixtures/helpers/SubgraphHelper.ts index 23df6401fc..8fd7d593ee 100644 --- a/browser_tests/fixtures/helpers/SubgraphHelper.ts +++ b/browser_tests/fixtures/helpers/SubgraphHelper.ts @@ -8,17 +8,12 @@ import type { import type { ComfyWorkflow } from '@/platform/workflow/management/stores/comfyWorkflow' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' -import { parsePreviewExposures } from '@/core/schemas/previewExposureSchema' - import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { SubgraphEditor } from '@e2e/fixtures/components/SubgraphEditor' import { TestIds } from '@e2e/fixtures/selectors' import type { NodeReference } from '@e2e/fixtures/utils/litegraphUtils' import { SubgraphSlotReference } from '@e2e/fixtures/utils/litegraphUtils' -import { - isNodeProperty, - isPromotedWidgetSource -} from '@e2e/fixtures/utils/promotedWidgets' +import { getAllHostPromotedWidgets } from '@e2e/fixtures/utils/promotedWidgets' import type { PromotedWidgetEntry } from '@e2e/fixtures/utils/promotedWidgets' export class SubgraphHelper { @@ -420,62 +415,7 @@ export class SubgraphHelper { async getHostPromotedTupleSnapshot(): Promise< { hostNodeId: string; promotedWidgets: PromotedWidgetEntry[] }[] > { - const rawHosts = await this.page.evaluate(() => { - const graph = window.app!.canvas.graph! - const serialized = window.app!.graph!.serialize() - return graph._nodes - .filter( - (node) => - typeof node.isSubgraphNode === 'function' && node.isSubgraphNode() - ) - .map((node) => { - const widgetSources = (node.widgets ?? []).flatMap((widget) => { - if (!('sourceNodeId' in widget) || !('sourceWidgetName' in widget)) - return [] - return [ - { - sourceNodeId: widget.sourceNodeId, - sourceWidgetName: widget.sourceWidgetName - } - ] - }) - const serializedNode = serialized.nodes.find( - (candidate) => String(candidate.id) === String(node.id) - ) - return { - hostNodeId: String(node.id), - widgetSources, - previewExposures: serializedNode?.properties?.previewExposures - } - }) - }) - - return rawHosts - .map(({ hostNodeId, widgetSources, previewExposures }) => { - const exposures = isNodeProperty(previewExposures) - ? parsePreviewExposures(previewExposures) - : [] - return { - hostNodeId, - promotedWidgets: [ - ...widgetSources - .filter(isPromotedWidgetSource) - .map( - (source): PromotedWidgetEntry => [ - source.sourceNodeId, - source.sourceWidgetName - ] - ), - ...exposures.map( - (exposure): PromotedWidgetEntry => [ - exposure.sourceNodeId, - exposure.sourcePreviewName - ] - ) - ] - } - }) - .sort((a, b) => Number(a.hostNodeId) - Number(b.hostNodeId)) + return getAllHostPromotedWidgets(this.comfyPage) } /** Reads from `window.app.canvas.graph` (viewed root or nested subgraph). */ diff --git a/browser_tests/fixtures/utils/promotedWidgets.ts b/browser_tests/fixtures/utils/promotedWidgets.ts index e68f55798b..bd7fee5b84 100644 --- a/browser_tests/fixtures/utils/promotedWidgets.ts +++ b/browser_tests/fixtures/utils/promotedWidgets.ts @@ -117,3 +117,31 @@ export async function getPromotedWidgetCountByName( const promotedWidgets = await getPromotedWidgets(comfyPage, nodeId) return promotedWidgets.filter(([, name]) => name === widgetName).length } + +/** + * Returns promoted widget entries for every subgraph host node in the current + * canvas graph, sorted by numeric host id. Delegates to {@link getPromotedWidgets} + * per host so all merge/validation logic lives in one place. + */ +export async function getAllHostPromotedWidgets( + comfyPage: ComfyPage +): Promise<{ hostNodeId: string; promotedWidgets: PromotedWidgetEntry[] }[]> { + const hostNodeIds = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph! + return graph._nodes + .filter( + (node) => + typeof node.isSubgraphNode === 'function' && node.isSubgraphNode() + ) + .map((node) => String(node.id)) + }) + + const entries = await Promise.all( + hostNodeIds.map(async (hostNodeId) => ({ + hostNodeId, + promotedWidgets: await getPromotedWidgets(comfyPage, hostNodeId) + })) + ) + + return entries.sort((a, b) => Number(a.hostNodeId) - Number(b.hostNodeId)) +} diff --git a/src/components/rightSidePanel/subgraph/SubgraphEditor.vue b/src/components/rightSidePanel/subgraph/SubgraphEditor.vue index ad004032eb..2c492ef7f3 100644 --- a/src/components/rightSidePanel/subgraph/SubgraphEditor.vue +++ b/src/components/rightSidePanel/subgraph/SubgraphEditor.vue @@ -9,7 +9,6 @@ import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' import { demoteWidget, getPromotableWidgets, - getSourceNodeId, getWidgetName, isLinkedPromotion, isRecommendedWidget, @@ -168,7 +167,8 @@ const candidateWidgets = computed(() => { // exposures are stored as the interior `[node, widget]` tuple directly. const promotedSourceKeys = new Set( activeWidgets.value.map( - ([n, w]) => `${getSourceNodeId(w) ?? String(n.id)}:${getWidgetName(w)}` + ([n, w]) => + `${isPromotedWidgetView(w) ? w.sourceNodeId : String(n.id)}:${getWidgetName(w)}` ) ) return interiorWidgets.value.filter( @@ -231,10 +231,10 @@ function isItemLinked([node, widget]: WidgetItem): boolean { } function toKey(item: WidgetItem) { - const sid = getSourceNodeId(item[1]) - return sid - ? `${item[0].id}: ${item[1].name}:${sid}` - : `${item[0].id}: ${item[1].name}` + const widget = item[1] + return isPromotedWidgetView(widget) + ? `${item[0].id}: ${widget.name}:${widget.sourceNodeId}` + : `${item[0].id}: ${widget.name}` } function nodeWidgets(n: LGraphNode): WidgetItem[] { return getPromotableWidgets(n).map((w) => [n, w]) diff --git a/src/core/graph/subgraph/legacyProxyWidgetNormalization.test.ts b/src/core/graph/subgraph/legacyProxyWidgetNormalization.test.ts deleted file mode 100644 index 1e22bb3d73..0000000000 --- a/src/core/graph/subgraph/legacyProxyWidgetNormalization.test.ts +++ /dev/null @@ -1,127 +0,0 @@ -import { createTestingPinia } from '@pinia/testing' -import { setActivePinia } from 'pinia' -import { beforeEach, describe, expect, it } from 'vitest' - -import { normalizeLegacyProxyWidgetEntry } from '@/core/graph/subgraph/legacyProxyWidgetNormalization' -import { LGraphNode } from '@/lib/litegraph/src/litegraph' -import { - createTestRootGraph, - createTestSubgraph, - createTestSubgraphNode, - resetSubgraphFixtureState -} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers' - -function createHostWithInnerWidget(widgetName: string) { - const rootGraph = createTestRootGraph() - const innerSubgraph = createTestSubgraph({ - rootGraph, - inputs: [{ name: 'value', type: 'number' }] - }) - - const innerNode = new LGraphNode('InnerNode') - const input = innerNode.addInput('value', 'number') - innerNode.addWidget('number', widgetName, 0, () => {}) - input.widget = { name: widgetName } - innerSubgraph.add(innerNode) - innerSubgraph.inputNode.slots[0].connect(innerNode.inputs[0], innerNode) - - const hostNode = createTestSubgraphNode(innerSubgraph, { - parentGraph: rootGraph - }) - - return { rootGraph, innerSubgraph, innerNode, hostNode } -} - -describe('normalizeLegacyProxyWidgetEntry', () => { - beforeEach(() => { - setActivePinia(createTestingPinia({ stubActions: false })) - resetSubgraphFixtureState() - }) - - it('returns entry unchanged when it already resolves', () => { - const { hostNode, innerNode } = createHostWithInnerWidget('seed') - - const result = normalizeLegacyProxyWidgetEntry( - hostNode, - String(innerNode.id), - 'seed' - ) - - expect(result).toEqual({ - sourceNodeId: String(innerNode.id), - sourceWidgetName: 'seed' - }) - }) - - it('returns entry unchanged with disambiguator when it already resolves', () => { - const { hostNode, innerNode } = createHostWithInnerWidget('seed') - - const result = normalizeLegacyProxyWidgetEntry( - hostNode, - String(innerNode.id), - 'seed', - String(innerNode.id) - ) - - expect(result).toEqual({ - sourceNodeId: String(innerNode.id), - sourceWidgetName: 'seed', - disambiguatingSourceNodeId: String(innerNode.id) - }) - }) - - it('strips a single legacy prefix from widget name', () => { - const rootGraph = createTestRootGraph() - const innerSubgraph = createTestSubgraph({ - rootGraph, - inputs: [{ name: 'seed', type: 'number' }] - }) - - const samplerNode = new LGraphNode('Sampler') - const samplerInput = samplerNode.addInput('seed', 'number') - samplerNode.addWidget('number', 'noise_seed', 42, () => {}) - samplerInput.widget = { name: 'noise_seed' } - innerSubgraph.add(samplerNode) - innerSubgraph.inputNode.slots[0].connect(samplerNode.inputs[0], samplerNode) - - const outerSubgraph = createTestSubgraph({ rootGraph }) - const nestedNode = createTestSubgraphNode(innerSubgraph, { - parentGraph: outerSubgraph - }) - outerSubgraph.add(nestedNode) - - const hostNode = createTestSubgraphNode(outerSubgraph, { - parentGraph: rootGraph - }) - - const prefixedName = `${nestedNode.id}: ${samplerNode.id}: noise_seed` - const result = normalizeLegacyProxyWidgetEntry( - hostNode, - String(nestedNode.id), - prefixedName - ) - - expect(result.sourceWidgetName).toBe('noise_seed') - expect(result.disambiguatingSourceNodeId).toBe(String(samplerNode.id)) - }) - - it('strips legacy prefix and surfaces it as disambiguator even when the bare name does not resolve', () => { - // ADR 0009: each SubgraphNode is opaque, so legacy nested - // disambiguator-based lookup no longer reaches deep widgets. The - // prefix is preserved as `disambiguatingSourceNodeId` lookup metadata - // for migration tooling. - const { hostNode, innerNode } = createHostWithInnerWidget('seed') - - const result = normalizeLegacyProxyWidgetEntry( - hostNode, - String(innerNode.id), - '999: nonexistent_widget' - ) - - expect(result).toEqual({ - sourceNodeId: String(innerNode.id), - sourceWidgetName: 'nonexistent_widget', - disambiguatingSourceNodeId: '999' - }) - }) -}) diff --git a/src/core/graph/subgraph/legacyProxyWidgetNormalization.ts b/src/core/graph/subgraph/legacyProxyWidgetNormalization.ts deleted file mode 100644 index e9c1f9f1d6..0000000000 --- a/src/core/graph/subgraph/legacyProxyWidgetNormalization.ts +++ /dev/null @@ -1,70 +0,0 @@ -import type { LegacyProxyEntrySource } from '@/core/graph/subgraph/promotedWidgetTypes' -import { resolveConcretePromotedWidget } from '@/core/graph/subgraph/resolveConcretePromotedWidget' -import type { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' - -const LEGACY_PROXY_WIDGET_PREFIX_PATTERN = /^\s*(\d+)\s*:\s*(.+)$/ - -function canResolve( - hostNode: SubgraphNode, - sourceNodeId: string, - widgetName: string -): boolean { - return ( - resolveConcretePromotedWidget(hostNode, sourceNodeId, widgetName).status === - 'resolved' - ) -} - -interface StrippedPrefix { - sourceWidgetName: string - /** Deepest legacy `n: ` prefix removed from the original widget name. */ - deepestPrefixId?: string -} - -function stripLegacyPrefixes(sourceWidgetName: string): StrippedPrefix { - let remaining = sourceWidgetName - let deepestPrefixId: string | undefined - while (true) { - const match = LEGACY_PROXY_WIDGET_PREFIX_PATTERN.exec(remaining) - if (!match) return { sourceWidgetName: remaining, deepestPrefixId } - deepestPrefixId = match[1] - remaining = match[2] - } -} - -/** - * Normalize a legacy `proxyWidgets` entry. - * - * Under ADR 0009 each `SubgraphNode` is opaque, so the canonical state never - * resolves through deep nested identities. This helper still recognizes the - * legacy `": "` prefix encoding and surfaces the deepest prefix as - * `disambiguatingSourceNodeId` so migration tooling can preserve it as - * lookup metadata. The bare entry is returned unchanged when it already - * resolves at the immediate level. - */ -export function normalizeLegacyProxyWidgetEntry( - hostNode: SubgraphNode, - sourceNodeId: string, - sourceWidgetName: string, - disambiguatingSourceNodeId?: string -): LegacyProxyEntrySource { - if (canResolve(hostNode, sourceNodeId, sourceWidgetName)) { - return { - sourceNodeId, - sourceWidgetName, - ...(disambiguatingSourceNodeId && { disambiguatingSourceNodeId }) - } - } - - const stripped = stripLegacyPrefixes(sourceWidgetName) - const patchDisambiguatingSourceNodeId = - stripped.deepestPrefixId ?? disambiguatingSourceNodeId - - return { - sourceNodeId, - sourceWidgetName: stripped.sourceWidgetName, - ...(patchDisambiguatingSourceNodeId && { - disambiguatingSourceNodeId: patchDisambiguatingSourceNodeId - }) - } -} diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts index 26352e8b7f..588845b81d 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts @@ -18,6 +18,7 @@ import { import { flushProxyWidgetMigration, + normalizeLegacyProxyWidgetEntry, readHostQuarantine } from '@/core/graph/subgraph/migration/proxyWidgetMigration' import type { PromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' @@ -830,3 +831,93 @@ describe('flushProxyWidgetMigration', () => { }) }) }) + +describe('normalizeLegacyProxyWidgetEntry', () => { + function createHostWithInnerWidget(widgetName: string) { + const subgraph = createTestSubgraph() + const innerNode = new LGraphNode('InnerNode') + const input = innerNode.addInput('value', 'number') + innerNode.addWidget('number', widgetName, 0, () => {}) + input.widget = { name: widgetName } + subgraph.add(innerNode) + + const hostNode = createTestSubgraphNode(subgraph) + hostNode.graph!.add(hostNode) + + return { innerNode, hostNode } + } + + it('returns entry unchanged when it already resolves', () => { + const { hostNode, innerNode } = createHostWithInnerWidget('seed') + + const result = normalizeLegacyProxyWidgetEntry( + hostNode, + String(innerNode.id), + 'seed' + ) + + expect(result).toEqual({ + sourceNodeId: String(innerNode.id), + sourceWidgetName: 'seed' + }) + }) + + it('returns entry unchanged with disambiguator when it already resolves', () => { + const { hostNode, innerNode } = createHostWithInnerWidget('seed') + + const result = normalizeLegacyProxyWidgetEntry( + hostNode, + String(innerNode.id), + 'seed', + String(innerNode.id) + ) + + expect(result).toEqual({ + sourceNodeId: String(innerNode.id), + sourceWidgetName: 'seed', + disambiguatingSourceNodeId: String(innerNode.id) + }) + }) + + it('strips a single legacy prefix from widget name', () => { + const innerSubgraph = createTestSubgraph() + const samplerNode = new LGraphNode('Sampler') + const samplerInput = samplerNode.addInput('seed', 'number') + samplerNode.addWidget('number', 'noise_seed', 42, () => {}) + samplerInput.widget = { name: 'noise_seed' } + innerSubgraph.add(samplerNode) + + const hostNode = createTestSubgraphNode(innerSubgraph) + hostNode.graph!.add(hostNode) + + const prefixedName = `${samplerNode.id}: noise_seed` + const result = normalizeLegacyProxyWidgetEntry( + hostNode, + String(samplerNode.id), + prefixedName + ) + + expect(result.sourceWidgetName).toBe('noise_seed') + expect(result.disambiguatingSourceNodeId).toBe(String(samplerNode.id)) + }) + + it('strips legacy prefix and surfaces it as disambiguator even when the bare name does not resolve', () => { + // ADR 0009: each SubgraphNode is opaque, so legacy nested + // disambiguator-based lookup no longer reaches deep widgets. The + // prefix is preserved as `disambiguatingSourceNodeId` lookup metadata + // for migration tooling. + const { hostNode, innerNode } = createHostWithInnerWidget('seed') + + const result = normalizeLegacyProxyWidgetEntry( + hostNode, + String(innerNode.id), + '999: nonexistent_widget' + ) + + expect(result).toEqual({ + sourceNodeId: String(innerNode.id), + sourceWidgetName: 'nonexistent_widget', + disambiguatingSourceNodeId: '999' + }) + }) +}) diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts index 954d53cd28..b2afee61d1 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts @@ -1,12 +1,13 @@ import { isEqual } from 'es-toolkit/compat' -import { normalizeLegacyProxyWidgetEntry } from '@/core/graph/subgraph/legacyProxyWidgetNormalization' -import type { LegacyProxyEntrySource } from '@/core/graph/subgraph/promotedWidgetTypes' import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' +import type { PromotedWidgetSource } from '@/core/graph/subgraph/promotedWidgetTypes' import { + findHostInputForPromotion, getPromotableWidgets, isPreviewPseudoWidget } from '@/core/graph/subgraph/promotionUtils' +import { resolveConcretePromotedWidget } from '@/core/graph/subgraph/resolveConcretePromotedWidget' import type { SerializedProxyWidgetTuple } from '@/core/schemas/promotionSchema' import { parseProxyWidgets } from '@/core/schemas/promotionSchema' import type { @@ -28,22 +29,101 @@ import { isWidgetValue } from '@/lib/litegraph/src/types/widgets' import { usePreviewExposureStore } from '@/stores/previewExposureStore' /** - * Find a widget on `sourceNode` that matches the legacy proxy entry's source - * identity. When the entry carries a `disambiguatingSourceNodeId`, prefer the - * `PromotedWidgetView` whose interior identity matches it exactly — this lets - * us pick the correct widget after deduplication renamed it (e.g. `text_1`). - * Falls back to a name match for non-promoted widgets and legacy data without - * a disambiguator. + * Legacy proxyWidget tuple shape carried through migration. The optional + * `disambiguatingSourceNodeId` is read from legacy `properties.proxyWidgets` + * payloads only — canonical runtime state never sets it. See ADR 0009. */ -function findSourceWidget( +interface LegacyProxyEntrySource extends PromotedWidgetSource { + disambiguatingSourceNodeId?: string +} + +const LEGACY_PROXY_WIDGET_PREFIX_PATTERN = /^\s*(\d+)\s*:\s*(.+)$/ + +interface StrippedPrefix { + sourceWidgetName: string + /** Deepest legacy `n: ` prefix removed from the original widget name. */ + deepestPrefixId?: string +} + +function stripLegacyPrefixes(sourceWidgetName: string): StrippedPrefix { + let remaining = sourceWidgetName + let deepestPrefixId: string | undefined + while (true) { + const match = LEGACY_PROXY_WIDGET_PREFIX_PATTERN.exec(remaining) + if (!match) return { sourceWidgetName: remaining, deepestPrefixId } + deepestPrefixId = match[1] + remaining = match[2] + } +} + +function canResolveLegacyProxy( + hostNode: SubgraphNode, + sourceNodeId: string, + widgetName: string +): boolean { + return ( + resolveConcretePromotedWidget(hostNode, sourceNodeId, widgetName).status === + 'resolved' + ) +} + +/** + * Normalize a legacy `proxyWidgets` entry. + * + * Under ADR 0009 each `SubgraphNode` is opaque, so the canonical state never + * resolves through deep nested identities. This helper still recognizes the + * legacy `": "` prefix encoding and surfaces the deepest prefix as + * `disambiguatingSourceNodeId` so migration tooling can preserve it as + * lookup metadata. The bare entry is returned unchanged when it already + * resolves at the immediate level. + */ +export function normalizeLegacyProxyWidgetEntry( + hostNode: SubgraphNode, + sourceNodeId: string, + sourceWidgetName: string, + disambiguatingSourceNodeId?: string +): LegacyProxyEntrySource { + if (canResolveLegacyProxy(hostNode, sourceNodeId, sourceWidgetName)) { + return { + sourceNodeId, + sourceWidgetName, + ...(disambiguatingSourceNodeId && { disambiguatingSourceNodeId }) + } + } + + const stripped = stripLegacyPrefixes(sourceWidgetName) + const patchDisambiguatingSourceNodeId = + stripped.deepestPrefixId ?? disambiguatingSourceNodeId + + return { + sourceNodeId, + sourceWidgetName: stripped.sourceWidgetName, + ...(patchDisambiguatingSourceNodeId && { + disambiguatingSourceNodeId: patchDisambiguatingSourceNodeId + }) + } +} + +/** + * Resolve the source widget for a normalized proxy entry. When the entry + * carries a `disambiguatingSourceNodeId`, prefer the `PromotedWidgetView` + * whose interior identity matches exactly — this lets us pick the correct + * widget after deduplication renamed it (e.g. `text_1`). Otherwise match by + * name, falling back to `getPromotableWidgets` (which surfaces virtual + * preview widgets that aren't on `node.widgets`). + * + * `classify` and `repairCreateSubgraphInput` both call this — they must + * agree on the resolved widget, otherwise a legacy nested entry can be + * classified as repairable but quarantined at repair time, leaving the host + * with fewer rendered widgets than expected. + */ +function resolveSourceWidget( sourceNode: LGraphNode, sourceWidgetName: string, disambiguatingSourceNodeId?: string ): IBaseWidget | undefined { const widgets = sourceNode.widgets - if (!widgets) return undefined - - if (disambiguatingSourceNodeId !== undefined) { + if (widgets && disambiguatingSourceNodeId !== undefined) { const byDisambiguator = widgets.find( (w) => isPromotedWidgetView(w) && @@ -55,33 +135,14 @@ function findSourceWidget( // 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( + const byName = widgets.find( (w) => !isPromotedWidgetView(w) && w.name === sourceWidgetName ) + if (byName) return byName } - return widgets.find((w) => w.name === sourceWidgetName) -} - -/** - * Resolve the source widget for a normalized proxy entry, falling back to a - * promotable-widget name match when the strict `findSourceWidget` lookup - * misses. `classify` and `repairCreateSubgraphInput` must agree on this - * resolution — otherwise a legacy nested entry can be classified as - * repairable but then quarantined at repair time, leaving the host with - * fewer rendered widgets than expected. - */ -function resolveSourceWidget( - sourceNode: LGraphNode, - sourceWidgetName: string, - disambiguatingSourceNodeId?: string -): IBaseWidget | undefined { return ( - findSourceWidget( - sourceNode, - sourceWidgetName, - disambiguatingSourceNodeId - ) ?? + widgets?.find((w) => w.name === sourceWidgetName) ?? getPromotableWidgets(sourceNode).find((w) => w.name === sourceWidgetName) ) } @@ -215,29 +276,6 @@ function pickHostValue( return { value: raw, isHole: false } } -function findHostInputForLinkedSource( - hostNode: SubgraphNode, - sourceNodeId: string, - sourceWidgetName: string, - subgraphInputName?: string -): INodeInputSlot | 'ambiguous' | undefined { - const candidates = subgraphInputName - ? hostNode.inputs.filter((input) => input.name === subgraphInputName) - : hostNode.inputs - const matches = candidates.filter((input) => { - const widget = input._widget - return ( - !!widget && - isPromotedWidgetView(widget) && - widget.sourceNodeId === sourceNodeId && - widget.sourceWidgetName === sourceWidgetName - ) - }) - if (matches.length === 0) return undefined - if (matches.length === 1) return matches[0] - return 'ambiguous' -} - function collectTargetsStrict( hostNode: SubgraphNode, primitiveNode: LGraphNode @@ -285,15 +323,28 @@ function classify( normalized: LegacyProxyEntrySource, cohort: readonly LegacyProxyEntrySource[] ): Plan { - const linkedInput = findHostInputForLinkedSource( + const linkedInput = findHostInputForPromotion( hostNode, normalized.sourceNodeId, normalized.sourceWidgetName ) - if (linkedInput === 'ambiguous') { - return { kind: 'quarantine', reason: 'ambiguousSubgraphInput' } - } if (linkedInput) { + // ADR 0009 expects a single host input per source identity. Detect the + // legacy/corruption case where multiple inputs share the same source so + // we quarantine instead of silently picking one and stomping its value. + const ambiguous = + hostNode.inputs.filter((input) => { + const w = input._widget + return ( + !!w && + isPromotedWidgetView(w) && + w.sourceNodeId === normalized.sourceNodeId && + w.sourceWidgetName === normalized.sourceWidgetName + ) + }).length > 1 + if (ambiguous) { + return { kind: 'quarantine', reason: 'ambiguousSubgraphInput' } + } return { kind: 'alreadyLinked', subgraphInputName: linkedInput.name } } diff --git a/src/core/graph/subgraph/promotedWidgetTypes.ts b/src/core/graph/subgraph/promotedWidgetTypes.ts index 02991bd013..3b082f5af2 100644 --- a/src/core/graph/subgraph/promotedWidgetTypes.ts +++ b/src/core/graph/subgraph/promotedWidgetTypes.ts @@ -13,15 +13,6 @@ export interface PromotedWidgetSource { sourceWidgetName: string } -/** - * Legacy proxyWidget tuple shape carried through migration. The optional - * `disambiguatingSourceNodeId` is read from legacy `properties.proxyWidgets` - * payloads only — canonical runtime state never sets it. See ADR 0009. - */ -export interface LegacyProxyEntrySource extends PromotedWidgetSource { - disambiguatingSourceNodeId?: string -} - export interface PromotedWidgetView extends IBaseWidget { readonly node: SubgraphNode /** diff --git a/src/core/graph/subgraph/promotedWidgetView.ts b/src/core/graph/subgraph/promotedWidgetView.ts index e982ad8522..5835886529 100644 --- a/src/core/graph/subgraph/promotedWidgetView.ts +++ b/src/core/graph/subgraph/promotedWidgetView.ts @@ -466,13 +466,10 @@ class PromotedWidgetView implements IPromotedWidgetView { this.resolveAtHost()?.widget.callback?.(value, canvas, node, pos, e) } - beforeQueued(): void { - // Source widgets linked through subgraph inputs are inert for prompt - // serialization. Control-after-generate is applied to the promoted host - // value in afterQueued so the next prompt uses the updated SubgraphNode - // value, not the linked source value. - } - + // No beforeQueued: source widgets linked through subgraph inputs are inert + // for prompt serialization. Control-after-generate is applied to the + // promoted host value in afterQueued so the next prompt uses the updated + // SubgraphNode value, not the linked source value. afterQueued(): void { this.applyValueControlToHost() } diff --git a/src/core/graph/subgraph/promotionUtils.test.ts b/src/core/graph/subgraph/promotionUtils.test.ts index 6198a33e0a..31150012a6 100644 --- a/src/core/graph/subgraph/promotionUtils.test.ts +++ b/src/core/graph/subgraph/promotionUtils.test.ts @@ -9,8 +9,13 @@ import { createTestSubgraphNode } from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' import { usePreviewExposureStore } from '@/stores/previewExposureStore' +function widgetSourceNodeId(w: IBaseWidget): string | undefined { + return isPromotedWidgetView(w) ? w.sourceNodeId : undefined +} + type TestPromotedWidget = IBaseWidget & { sourceNodeId: string sourceWidgetName: string @@ -25,7 +30,6 @@ import { CANVAS_IMAGE_PREVIEW_WIDGET, demoteWidget, getPromotableWidgets, - getSourceNodeId, hasUnpromotedWidgets, isLinkedPromotion, isPreviewPseudoWidget, @@ -605,7 +609,7 @@ describe('reorderSubgraphInputAtIndex', () => { reorderSubgraphInputAtIndex(host, 0, 1) - expect(host.widgets.map((widget) => getSourceNodeId(widget))).toEqual([ + expect(host.widgets.map((widget) => widgetSourceNodeId(widget))).toEqual([ String(secondNode.id), String(firstNode.id) ]) @@ -670,7 +674,7 @@ describe('reorderSubgraphInputsByWidgetOrder', () => { reorderSubgraphInputsByWidgetOrder(host, [host.widgets[1], host.widgets[0]]) - expect(host.widgets.map((widget) => getSourceNodeId(widget))).toEqual([ + expect(host.widgets.map((widget) => widgetSourceNodeId(widget))).toEqual([ String(secondNode.id), String(firstNode.id) ]) @@ -729,7 +733,7 @@ describe('demoteWidget — axiomatic projection retraction', () => { expect( host.widgets.some( (widget) => - getSourceNodeId(widget) === String(interiorNode.id) && + widgetSourceNodeId(widget) === String(interiorNode.id) && widget.name === interiorWidget.name ) ).toBe(false) diff --git a/src/core/graph/subgraph/promotionUtils.ts b/src/core/graph/subgraph/promotionUtils.ts index 9f78d17ce8..3165436884 100644 --- a/src/core/graph/subgraph/promotionUtils.ts +++ b/src/core/graph/subgraph/promotionUtils.ts @@ -46,7 +46,7 @@ export function isLinkedPromotion( /** Find the host input on `subgraphNode` whose `_widget` is the * `PromotedWidgetView` for `(sourceNodeId, sourceWidgetName)`. */ -function findHostInputForPromotion( +export function findHostInputForPromotion( subgraphNode: SubgraphNode, sourceNodeId: string, sourceWidgetName: string @@ -194,11 +194,6 @@ function isSamePromotedWidget(left: IBaseWidget, right: IBaseWidget): boolean { ) } -export function getSourceNodeId(w: IBaseWidget): string | undefined { - if (!isPromotedWidgetView(w)) return undefined - return w.sourceNodeId -} - function isPreviewExposed( subgraphNode: SubgraphNode, source: PromotedWidgetSource @@ -213,24 +208,21 @@ function isPreviewExposed( ) } -function isPromotedOnParent( - subgraphNode: SubgraphNode, - widget: IBaseWidget, - source: PromotedWidgetSource -): boolean { - if (isPreviewPseudoWidget(widget)) - return isPreviewExposed(subgraphNode, source) - return isLinkedPromotion( - subgraphNode, - source.sourceNodeId, - source.sourceWidgetName - ) -} - +/** + * Returns true if the widget identified by `source` is already exposed on + * `subgraphNode` — either as a linked promotion (subgraph input) or as a + * preview exposure. When `widget` is provided and is a preview pseudo-widget, + * only the preview-exposure path is consulted (callers asking about a preview + * widget should not pick up an unrelated linked promotion with the same + * source identity). + */ export function isWidgetPromotedOnSubgraphNode( subgraphNode: SubgraphNode, - source: PromotedWidgetSource + source: PromotedWidgetSource, + widget?: IBaseWidget ): boolean { + if (widget && isPreviewPseudoWidget(widget)) + return isPreviewExposed(subgraphNode, source) return ( isLinkedPromotion( subgraphNode, @@ -450,7 +442,7 @@ export function addWidgetPromotionOptions( const parents = getParentNodes() const source = toPromotionSource(node, widget) const promotableParents = parents.filter( - (parent) => !isPromotedOnParent(parent, widget, source) + (parent) => !isWidgetPromotedOnSubgraphNode(parent, source, widget) ) if (promotableParents.length > 0) options.unshift({ @@ -485,7 +477,7 @@ export function tryToggleWidgetPromotion() { if (!parents.length || !widget) return const source = toPromotionSource(node, widget) const promotableParents = parents.filter( - (parent) => !isPromotedOnParent(parent, widget, source) + (parent) => !isWidgetPromotedOnSubgraphNode(parent, source, widget) ) if (promotableParents.length > 0) promoteWidget(node, widget, promotableParents) @@ -660,10 +652,14 @@ export function hasUnpromotedWidgets(subgraphNode: SubgraphNode): boolean { getPromotableWidgets(interiorNode).some( (widget) => !widget.computedDisabled && - !isPromotedOnParent(subgraphNode, widget, { - sourceNodeId: String(interiorNode.id), - sourceWidgetName: widget.name - }) + !isWidgetPromotedOnSubgraphNode( + subgraphNode, + { + sourceNodeId: String(interiorNode.id), + sourceWidgetName: widget.name + }, + widget + ) ) ) } diff --git a/src/core/graph/subgraph/resolveConcretePromotedWidget.ts b/src/core/graph/subgraph/resolveConcretePromotedWidget.ts index 7a61e55521..57b68f0cd0 100644 --- a/src/core/graph/subgraph/resolveConcretePromotedWidget.ts +++ b/src/core/graph/subgraph/resolveConcretePromotedWidget.ts @@ -41,9 +41,8 @@ function traversePromotedWidgetChain( return { status: 'failure', failure: 'missing-node' } } - const sourceWidget = findWidgetByIdentity( - sourceNode.widgets, - currentWidgetName + const sourceWidget = sourceNode.widgets?.find( + (entry) => entry.name === currentWidgetName ) if (!sourceWidget) { return { status: 'failure', failure: 'missing-widget' } @@ -68,13 +67,6 @@ function traversePromotedWidgetChain( return { status: 'failure', failure: 'max-depth-exceeded' } } -function findWidgetByIdentity( - widgets: IBaseWidget[] | undefined, - widgetName: string -): IBaseWidget | undefined { - return widgets?.find((entry) => entry.name === widgetName) -} - export function resolvePromotedWidgetAtHost( hostNode: SubgraphNode, nodeId: string, @@ -83,7 +75,7 @@ export function resolvePromotedWidgetAtHost( const node = hostNode.subgraph.getNodeById(nodeId) if (!node) return undefined - const widget = findWidgetByIdentity(node.widgets, widgetName) + const widget = node.widgets?.find((entry) => entry.name === widgetName) if (!widget) return undefined return { node, widget } diff --git a/src/core/schemas/parseNodePropertyArray.ts b/src/core/schemas/parseNodePropertyArray.ts new file mode 100644 index 0000000000..30d49a24c6 --- /dev/null +++ b/src/core/schemas/parseNodePropertyArray.ts @@ -0,0 +1,43 @@ +import type { z } from 'zod' +import { fromZodError } from 'zod-validation-error' + +import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode' + +/** + * Parses a node property that is expected to deserialize into an array `T[]`. + * + * Behavior: + * - `undefined` → returns `[]` (no warning) + * - If `property` is a string, attempts `JSON.parse`; on failure, warns and + * returns `[]`. + * - Validates the result with `schema.safeParse`; on failure, warns with the + * given `contextName` and returns `[]`. + * - On success, returns the parsed array. + * + * @param property - The raw node property value. + * @param schema - A zod schema describing the expected array shape. + * @param contextName - Used as the prefix for `console.warn` messages + * (e.g. `properties.proxyWidgets`). + */ +export function parseNodePropertyArray( + property: NodeProperty | undefined, + schema: z.ZodType, + contextName: string +): T[] { + if (property === undefined) return [] + + let parsed: unknown + try { + parsed = typeof property === 'string' ? JSON.parse(property) : property + } catch (e) { + console.warn(`Failed to parse ${contextName}:`, e) + return [] + } + + const result = schema.safeParse(parsed) + if (result.success) return result.data + + const error = fromZodError(result.error) + console.warn(`Invalid assignment for ${contextName}:\n${error}`) + return [] +} diff --git a/src/core/schemas/previewExposureSchema.ts b/src/core/schemas/previewExposureSchema.ts index c5373a1420..7cde49c5c5 100644 --- a/src/core/schemas/previewExposureSchema.ts +++ b/src/core/schemas/previewExposureSchema.ts @@ -1,8 +1,9 @@ import { z } from 'zod' -import { fromZodError } from 'zod-validation-error' import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode' +import { parseNodePropertyArray } from './parseNodePropertyArray' + export const previewExposureSchema = z.object({ name: z.string(), sourceNodeId: z.string(), @@ -15,20 +16,9 @@ const previewExposuresPropertySchema = z.array(previewExposureSchema) export function parsePreviewExposures( property: NodeProperty | undefined ): PreviewExposure[] { - if (property === undefined) return [] - - try { - const parsed = - typeof property === 'string' ? JSON.parse(property) : property - const result = previewExposuresPropertySchema.safeParse(parsed) - if (result.success) return result.data - - const error = fromZodError(result.error) - console.warn( - `Invalid assignment for properties.previewExposures:\n${error}` - ) - } catch (e) { - console.warn('Failed to parse properties.previewExposures:', e) - } - return [] + return parseNodePropertyArray( + property, + previewExposuresPropertySchema, + 'properties.previewExposures' + ) } diff --git a/src/core/schemas/promotionSchema.ts b/src/core/schemas/promotionSchema.ts index d46703d9fb..7907f36784 100644 --- a/src/core/schemas/promotionSchema.ts +++ b/src/core/schemas/promotionSchema.ts @@ -1,8 +1,9 @@ import { z } from 'zod' -import { fromZodError } from 'zod-validation-error' import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode' +import { parseNodePropertyArray } from './parseNodePropertyArray' + export const serializedProxyWidgetTupleSchema = z.union([ z.tuple([z.string(), z.string(), z.string()]), z.tuple([z.string(), z.string()]) @@ -16,17 +17,9 @@ type ProxyWidgetsProperty = z.infer export function parseProxyWidgets( property: NodeProperty | undefined ): ProxyWidgetsProperty { - try { - if (typeof property === 'string') property = JSON.parse(property) - const result = proxyWidgetsPropertySchema.safeParse( - typeof property === 'string' ? JSON.parse(property) : property - ) - if (result.success) return result.data - - const error = fromZodError(result.error) - console.warn(`Invalid assignment for properties.proxyWidgets:\n${error}`) - } catch (e) { - console.warn('Failed to parse properties.proxyWidgets:', e) - } - return [] + return parseNodePropertyArray( + property, + proxyWidgetsPropertySchema, + 'properties.proxyWidgets' + ) } diff --git a/src/core/schemas/proxyWidgetQuarantineSchema.ts b/src/core/schemas/proxyWidgetQuarantineSchema.ts index a33b46f3c2..320b0ef5a5 100644 --- a/src/core/schemas/proxyWidgetQuarantineSchema.ts +++ b/src/core/schemas/proxyWidgetQuarantineSchema.ts @@ -1,9 +1,9 @@ import { z } from 'zod' -import { fromZodError } from 'zod-validation-error' import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode' import type { TWidgetValue } from '@/lib/litegraph/src/types/widgets' +import { parseNodePropertyArray } from './parseNodePropertyArray' import { serializedProxyWidgetTupleSchema } from './promotionSchema' export const proxyWidgetQuarantineReasonSchema = z.enum([ @@ -37,20 +37,9 @@ export type ProxyWidgetErrorQuarantineEntry = Omit< export function parseProxyWidgetErrorQuarantine( property: NodeProperty | undefined ): ProxyWidgetErrorQuarantineEntry[] { - if (property === undefined) return [] - - try { - const result = proxyWidgetErrorQuarantinePropertySchema.safeParse( - typeof property === 'string' ? JSON.parse(property) : property - ) - if (result.success) return result.data as ProxyWidgetErrorQuarantineEntry[] - - const error = fromZodError(result.error) - console.warn( - `Invalid assignment for properties.proxyWidgetErrorQuarantine:\n${error}` - ) - } catch (e) { - console.warn('Failed to parse properties.proxyWidgetErrorQuarantine:', e) - } - return [] + return parseNodePropertyArray( + property, + proxyWidgetErrorQuarantinePropertySchema, + 'properties.proxyWidgetErrorQuarantine' + ) as ProxyWidgetErrorQuarantineEntry[] } diff --git a/src/services/litegraphService.ts b/src/services/litegraphService.ts index ed0ba9333c..f42b0e76f0 100644 --- a/src/services/litegraphService.ts +++ b/src/services/litegraphService.ts @@ -188,12 +188,10 @@ export const useLitegraphService = () => { function getPseudoWidgetPreviewTargets(node: SubgraphNode): LGraphNode[] { const hostLocator = String(node.id) - const promotions = usePreviewExposureStore() - .getExposures(node.rootGraph.id, hostLocator) - .map((exposure) => ({ - sourceNodeId: exposure.sourceNodeId, - sourceWidgetName: exposure.sourcePreviewName - })) + const promotions = usePreviewExposureStore().getExposuresAsPromotionShape( + node.rootGraph.id, + hostLocator + ) const resolved = resolveSubgraphPseudoWidgetCache({ cache: subgraphPseudoWidgetCache.get(node) ?? null, promotions, diff --git a/src/stores/nodeOutputStore.ts b/src/stores/nodeOutputStore.ts index 0922269d7c..7446a079af 100644 --- a/src/stores/nodeOutputStore.ts +++ b/src/stores/nodeOutputStore.ts @@ -109,11 +109,14 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => { return isImageOutputs(node, outputs) ? app.getPreviewFormatParam() : '' } - function getNodeImageUrls(node: LGraphNode): string[] | undefined { - const previews = getNodePreviews(node) - if (previews?.length) return previews - - const outputs = getNodeOutputs(node) + /** + * Builds `/view`-style image URLs for a node's outputs. Returns undefined + * when there are no images so callers can fall back to preview blobs. + */ + function buildImageUrls( + node: LGraphNode, + outputs: ExecutedWsMessage['output'] | undefined + ): string[] | undefined { if (!outputs?.images?.length) return const rand = app.getRandParam() @@ -127,6 +130,13 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => { }) } + function getNodeImageUrls(node: LGraphNode): string[] | undefined { + const previews = getNodePreviews(node) + if (previews?.length) return previews + + return buildImageUrls(node, getNodeOutputs(node)) + } + function getNodeOutputByExecutionId( executionId: string ): ExecutedWsMessage['output'] | undefined { @@ -150,18 +160,7 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => { const previews = getNodePreviewImagesByExecutionId(executionId) if (previews?.length) return previews - const outputs = getNodeOutputByExecutionId(executionId) - if (!outputs?.images?.length) return - - const rand = app.getRandParam() - const previewParam = getPreviewParam(node, outputs) - - return outputs.images - .filter((image) => image != null) - .map((image) => { - const params = new URLSearchParams(image) - return api.apiURL(`/view?${params}${previewParam}${rand}`) - }) + return buildImageUrls(node, getNodeOutputByExecutionId(executionId)) } /** diff --git a/src/stores/previewExposureStore.test.ts b/src/stores/previewExposureStore.test.ts index d341c17d24..6ecdbfd83a 100644 --- a/src/stores/previewExposureStore.test.ts +++ b/src/stores/previewExposureStore.test.ts @@ -122,6 +122,28 @@ describe(usePreviewExposureStore, () => { }) }) + describe('getExposuresAsPromotionShape', () => { + it('returns an empty array for unknown host', () => { + expect(store.getExposuresAsPromotionShape(rootGraphA, hostA)).toEqual([]) + }) + + it('maps each exposure to {sourceNodeId, sourceWidgetName} preserving order', () => { + store.addExposure(rootGraphA, hostA, { + sourceNodeId: '42', + sourcePreviewName: 'preview' + }) + store.addExposure(rootGraphA, hostA, { + sourceNodeId: '43', + sourcePreviewName: 'preview' + }) + + expect(store.getExposuresAsPromotionShape(rootGraphA, hostA)).toEqual([ + { sourceNodeId: '42', sourceWidgetName: 'preview' }, + { sourceNodeId: '43', sourceWidgetName: 'preview' } + ]) + }) + }) + describe('clearGraph', () => { it('removes all hosts under the rootGraphId without affecting others', () => { store.addExposure(rootGraphA, hostA, { diff --git a/src/stores/previewExposureStore.ts b/src/stores/previewExposureStore.ts index 5138279533..5189487fa5 100644 --- a/src/stores/previewExposureStore.ts +++ b/src/stores/previewExposureStore.ts @@ -6,6 +6,7 @@ import type { ResolvedPreviewChain } from '@/core/graph/subgraph/preview/previewExposureChain' import { resolvePreviewExposureChain } from '@/core/graph/subgraph/preview/previewExposureChain' +import type { PromotedWidgetSource } from '@/core/graph/subgraph/promotedWidgetTypes' import type { PreviewExposure } from '@/core/schemas/previewExposureSchema' import { nextUniqueName } from '@/lib/litegraph/src/strings' import type { UUID } from '@/lib/litegraph/src/utils/uuid' @@ -102,6 +103,21 @@ export const usePreviewExposureStore = defineStore('previewExposure', () => { exposures.value.delete(rootGraphId) } + /** + * Returns the host's exposures translated into the {@link PromotedWidgetSource} + * shape consumed by `resolveSubgraphPseudoWidgetCache`. Centralising this + * mapping keeps the exposure → promotion translation policy next to the store. + */ + function getExposuresAsPromotionShape( + rootGraphId: UUID, + hostNodeLocator: string + ): PromotedWidgetSource[] { + return getExposures(rootGraphId, hostNodeLocator).map((exposure) => ({ + sourceNodeId: exposure.sourceNodeId, + sourceWidgetName: exposure.sourcePreviewName + })) + } + /** * Resolve the chain of exposures from a host down to the originating source * preview, optionally walking through nested subgraph hosts. @@ -125,6 +141,7 @@ export const usePreviewExposureStore = defineStore('previewExposure', () => { return { getExposures, + getExposuresAsPromotionShape, setExposures, addExposure, removeExposure,