diff --git a/src/core/graph/subgraph/migration/README.md b/src/core/graph/subgraph/migration/README.md new file mode 100644 index 0000000000..386ada63c6 --- /dev/null +++ b/src/core/graph/subgraph/migration/README.md @@ -0,0 +1,24 @@ +# subgraph/migration + +**PR-A scaffolding only — empty until PR-B.** + +This directory is reserved for the proxyWidget migration planner and helpers +landing in PR-B of the subgraph promoted-widget ratchet. + +References: + +- ADR 0009 — Subgraph promoted-widget migration +- Implementation plan: [`temp/plans/2026-05-05-subgraph-promoted-widget-ratchet.md`](../../../../../temp/plans/2026-05-05-subgraph-promoted-widget-ratchet.md) + +Planned modules (PR-B): + +- `proxyWidgetMigrationPlanner.ts` — classify + plan +- `proxyWidgetMigrationFlush.ts` — defer/flush, idempotent +- `classifyProxyEntry.ts` +- `repairValueWidget.ts` +- `repairPrimitiveFanout.ts` +- `migratePreviewExposure.ts` +- `quarantineEntry.ts` + +The directory is intentionally empty in PR-A; no production code path imports +from here yet. diff --git a/src/core/graph/subgraph/promotedWidgetView.test.ts b/src/core/graph/subgraph/promotedWidgetView.test.ts index 65cf6f911f..dd791bbf72 100644 --- a/src/core/graph/subgraph/promotedWidgetView.test.ts +++ b/src/core/graph/subgraph/promotedWidgetView.test.ts @@ -1386,10 +1386,17 @@ describe('SubgraphNode.widgets getter', () => { const restoredNode = createTestSubgraphNode(subgraphNode.subgraph, { id: 99 }) + // ADR 0009: serialize() no longer re-emits proxyWidgets from the store. + // To exercise the legacy configure-time hydration path, inject the + // expected legacy payload directly. restoredNode.configure({ ...serialized, id: restoredNode.id, - type: subgraphNode.subgraph.id + type: subgraphNode.subgraph.id, + properties: { + ...serialized.properties, + proxyWidgets: [[String(innerNode.id), 'widgetA']] + } }) const restoredEntries = usePromotionStore().getPromotions( @@ -1438,11 +1445,21 @@ describe('SubgraphNode.widgets getter', () => { const serialized = subgraphNode.serialize() const restoredNode = createTestSubgraphNode(subgraph, { id: 98 }) + // ADR 0009: inject legacy proxyWidgets payload to exercise the + // configure-time hydration path; serialize() no longer emits it. restoredNode.configure({ ...serialized, id: restoredNode.id, type: subgraph.id, - inputs: [] + inputs: [], + properties: { + ...serialized.properties, + proxyWidgets: [ + [String(linkedNodeA.id), 'string_a'], + [String(linkedNodeB.id), 'string_a'], + [String(storeOnlyNode.id), 'string_a'] + ] + } }) const restoredWidgets = promotedWidgets(restoredNode) @@ -1495,6 +1512,8 @@ describe('SubgraphNode.widgets getter', () => { const serialized = subgraphNode.serialize() const restoredNode = createTestSubgraphNode(subgraph, { id: 108 }) + // ADR 0009: inject legacy proxyWidgets payload to exercise the + // configure-time hydration path; serialize() no longer emits it. restoredNode.configure({ ...serialized, id: restoredNode.id, @@ -1505,7 +1524,15 @@ describe('SubgraphNode.widgets getter', () => { type: '*', link: null } - ] + ], + properties: { + ...serialized.properties, + proxyWidgets: [ + [String(linkedNodeA.id), 'string_a'], + [String(linkedNodeB.id), 'string_a'], + [String(storeOnlyNode.id), 'string_a'] + ] + } }) const restoredWidgets = promotedWidgets(restoredNode) @@ -1634,12 +1661,17 @@ describe('SubgraphNode.widgets getter', () => { expect(finalIndependentView.value).toBe('independent-final') }) - test('clone output preserves proxyWidgets for promotion hydration', () => { + // ADR 0009: clone output no longer carries properties.proxyWidgets — the + // canonical promotion identity lives on the linked SubgraphInputs that + // travel via inputs/links. Hydration of legacy proxyWidgets is exercised + // separately via the explicit-inject pattern in the round-trip tests above. + test('clone output omits properties.proxyWidgets', () => { const [subgraphNode, innerNodes] = setupSubgraph(1) const innerNode = firstInnerNode(innerNodes) innerNode.addWidget('text', 'widgetA', 'a', () => {}) setPromotions(subgraphNode, [[String(innerNode.id), 'widgetA']]) + delete subgraphNode.properties.proxyWidgets const createNodeSpy = vi .spyOn(LiteGraph, 'createNode') @@ -1653,29 +1685,11 @@ describe('SubgraphNode.widgets getter', () => { if (!clonedNode) throw new Error('Expected clone to return a node') const clonedSerialized = clonedNode.serialize() - expect(clonedSerialized.properties?.proxyWidgets).toStrictEqual([ - [String(innerNode.id), 'widgetA'] - ]) - - const hydratedClone = createTestSubgraphNode(subgraphNode.subgraph, { - id: 100 - }) - hydratedClone.configure({ - ...clonedSerialized, - id: hydratedClone.id, - type: subgraphNode.subgraph.id - }) - - const hydratedEntries = usePromotionStore().getPromotions( - hydratedClone.rootGraph.id, - hydratedClone.id - ) - expect(hydratedEntries).toStrictEqual([ - { - sourceNodeId: String(innerNode.id), - sourceWidgetName: 'widgetA' - } - ]) + // Either undefined (preferred) or an empty array — the legacy + // _internalConfigureAfterSlots writeback may seed an empty default + // until slice 6 retires that path entirely. + const cloned = clonedSerialized.properties?.proxyWidgets + expect(cloned === undefined || (Array.isArray(cloned) && cloned.length === 0)).toBe(true) }) }) diff --git a/src/core/graph/subgraph/subgraphNodePromotion.test.ts b/src/core/graph/subgraph/subgraphNodePromotion.test.ts index b0203fe4b7..1121a6d825 100644 --- a/src/core/graph/subgraph/subgraphNodePromotion.test.ts +++ b/src/core/graph/subgraph/subgraphNodePromotion.test.ts @@ -270,10 +270,15 @@ describe('Subgraph proxyWidgets', () => { expect(serialized.widgets_values).toBeUndefined() }) - test('serialize preserves proxyWidgets in properties', () => { + // ADR 0009: serialize() no longer re-emits properties.proxyWidgets. + // Promoted-widget identity is owned by the linked SubgraphInput; the legacy + // proxyWidgets array is preserved on load only and is not the source of + // truth at save time. + test('serialize does not re-emit proxyWidgets from the promotion store', () => { const [subgraphNode, innerNodes, innerIds] = setupSubgraph(1) innerNodes[0].addWidget('text', 'widgetA', 'a', () => {}) innerNodes[0].addWidget('text', 'widgetB', 'b', () => {}) + delete subgraphNode.properties.proxyWidgets usePromotionStore().setPromotions( subgraphNode.rootGraph.id, subgraphNode.id, @@ -285,10 +290,7 @@ describe('Subgraph proxyWidgets', () => { const serialized = subgraphNode.serialize() - expect(serialized.properties?.proxyWidgets).toStrictEqual([ - [innerIds[0], 'widgetA'], - [innerIds[0], 'widgetB'] - ]) + expect(serialized.properties?.proxyWidgets).toBeUndefined() }) test('multi-link representative is deterministic across repeated reads', () => { diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts new file mode 100644 index 0000000000..95d13b3e48 --- /dev/null +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts @@ -0,0 +1,229 @@ +/** + * Tests for SubgraphNode.serialize() after ADR 0009. + * + * Covers: + * - Removed copy-back loop: exterior promoted host value does NOT mutate + * the corresponding interior widget value. + * - properties.proxyWidgets is no longer re-emitted on serialize. + * - properties.previewExposures round-trip through the + * PreviewExposureStore. + * - properties.proxyWidgetErrorQuarantine round-trips and is inert at + * runtime; an empty quarantine is omitted from the serialized payload. + */ +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { + appendHostQuarantine, + makeQuarantineEntry +} from '@/core/graph/subgraph/migration/quarantineEntry' +import type { SerializedProxyWidgetTuple } from '@/core/schemas/promotionSchema' +import type { ISlotType, TWidgetType } from '@/lib/litegraph/src/litegraph' +import { BaseWidget, LGraphNode } from '@/lib/litegraph/src/litegraph' +import { usePreviewExposureStore } from '@/stores/previewExposureStore' +import { createNodeLocatorId } from '@/types/nodeIdentification' + +import { + createTestSubgraph, + createTestSubgraphNode, + resetSubgraphFixtureState +} from './__fixtures__/subgraphHelpers' + +vi.mock('@/renderer/core/canvas/canvasStore', () => ({ + useCanvasStore: () => ({}) +})) +vi.mock('@/services/litegraphService', () => ({ + useLitegraphService: () => ({ updatePreviews: () => ({}) }) +})) + +beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + resetSubgraphFixtureState() +}) + +function createNodeWithWidget( + title: string, + widgetType: TWidgetType = 'number', + widgetValue: unknown = 42, + slotType: ISlotType = 'number' +) { + const node = new LGraphNode(title) + const input = node.addInput('value', slotType) + node.addOutput('out', slotType) + + // @ts-expect-error Abstract class instantiation + const widget = new BaseWidget({ + name: 'widget', + type: widgetType, + value: widgetValue, + y: 0, + options: widgetType === 'number' ? { min: 0, max: 100, step: 1 } : {}, + node + }) + node.widgets = [widget] + input.widget = { name: widget.name } + + return { node, widget, input } +} + +describe('SubgraphNode.serialize (ADR 0009)', () => { + describe('removed copy-back loop', () => { + it('does not call subgraphInput.getConnectedWidgets() during serialize', () => { + const subgraph = createTestSubgraph({ + inputs: [{ name: 'value', type: 'number' }] + }) + + const { node: interiorNode } = createNodeWithWidget('Interior') + subgraph.add(interiorNode) + subgraph.inputNode.slots[0].connect( + interiorNode.inputs[0], + interiorNode + ) + + const hostNode = createTestSubgraphNode(subgraph) + + // The pre-ADR-0009 copy-back loop iterated input._subgraphSlot + // .getConnectedWidgets() and assigned the host wrapper's value to + // every interior widget. After removal, serialize must not visit + // that path at all, preventing cross-host stomping. + const slot = hostNode.inputs.find((i) => i._subgraphSlot) + ?._subgraphSlot + expect(slot).toBeDefined() + const spy = vi.spyOn(slot!, 'getConnectedWidgets') + + hostNode.serialize() + + expect(spy).not.toHaveBeenCalled() + }) + }) + + describe('proxyWidgets is no longer re-emitted', () => { + it('does not write properties.proxyWidgets after serialize', () => { + const subgraph = createTestSubgraph({ + inputs: [{ name: 'value', type: 'number' }] + }) + + const { node: interiorNode } = createNodeWithWidget('Interior') + subgraph.add(interiorNode) + subgraph.inputNode.slots[0].connect( + interiorNode.inputs[0], + interiorNode + ) + + const hostNode = createTestSubgraphNode(subgraph) + // Ensure no pre-existing proxyWidgets property leaks through. + delete hostNode.properties.proxyWidgets + + const serialized = hostNode.serialize() + + expect(serialized.properties?.proxyWidgets).toBeUndefined() + expect(hostNode.properties.proxyWidgets).toBeUndefined() + }) + + it('preserves a pre-existing legacy proxyWidgets property without re-deriving it', () => { + const subgraph = createTestSubgraph() + const hostNode = createTestSubgraphNode(subgraph) + + const legacy: SerializedProxyWidgetTuple[] = [['7', 'seed']] + hostNode.properties.proxyWidgets = legacy + + const serialized = hostNode.serialize() + + // Still serialized as-is — not deleted, not rewritten. + expect(serialized.properties?.proxyWidgets).toStrictEqual(legacy) + }) + }) + + describe('previewExposures round-trip', () => { + it('writes previewExposures from the store on serialize', () => { + const subgraph = createTestSubgraph() + const hostNode = createTestSubgraphNode(subgraph) + + const store = usePreviewExposureStore() + const rootGraphId = hostNode.rootGraph.id + const hostLocator = createNodeLocatorId(rootGraphId, hostNode.id) + + store.addExposure(rootGraphId, hostLocator, { + sourceNodeId: '12', + sourcePreviewName: '$$canvas-image-preview' + }) + store.addExposure(rootGraphId, hostLocator, { + sourceNodeId: '14', + sourcePreviewName: 'videopreview' + }) + + const serialized = hostNode.serialize() + + expect(serialized.properties?.previewExposures).toEqual([ + { + name: '$$canvas-image-preview', + sourceNodeId: '12', + sourcePreviewName: '$$canvas-image-preview' + }, + { + name: 'videopreview', + sourceNodeId: '14', + sourcePreviewName: 'videopreview' + } + ]) + }) + + it('omits previewExposures when the store has no entries for the host', () => { + const subgraph = createTestSubgraph() + const hostNode = createTestSubgraphNode(subgraph) + hostNode.properties.previewExposures = [ + { + name: 'stale', + sourceNodeId: '0', + sourcePreviewName: '$$canvas-image-preview' + } + ] + + const serialized = hostNode.serialize() + + expect(serialized.properties?.previewExposures).toBeUndefined() + expect(hostNode.properties.previewExposures).toBeUndefined() + }) + }) + + describe('proxyWidgetErrorQuarantine', () => { + it('preserves quarantine entries through serialize and is inert at runtime', () => { + const subgraph = createTestSubgraph() + const hostNode = createTestSubgraphNode(subgraph) + + appendHostQuarantine(hostNode, [ + makeQuarantineEntry({ + originalEntry: ['7', 'seed'], + reason: 'missingSourceNode', + hostValue: 42 + }) + ]) + + const serialized = hostNode.serialize() + const quarantine = serialized.properties?.proxyWidgetErrorQuarantine + expect(Array.isArray(quarantine)).toBe(true) + expect(quarantine).toHaveLength(1) + + // Inertness: quarantine entries do not produce widgets. + expect( + hostNode.widgets.some( + (w) => 'sourceNodeId' in w && w.sourceNodeId === '7' + ) + ).toBe(false) + }) + + it('removes the property entirely when quarantine is empty', () => { + const subgraph = createTestSubgraph() + const hostNode = createTestSubgraphNode(subgraph) + hostNode.properties.proxyWidgetErrorQuarantine = [] + + const serialized = hostNode.serialize() + + expect( + serialized.properties?.proxyWidgetErrorQuarantine + ).toBeUndefined() + expect(hostNode.properties.proxyWidgetErrorQuarantine).toBeUndefined() + }) + }) +}) diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index f31f5a0528..0a0f2c47bd 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -43,12 +43,15 @@ import { CANVAS_IMAGE_PREVIEW_WIDGET, supportsVirtualCanvasImagePreview } from '@/composables/node/canvasImagePreviewTypes' +import { readHostQuarantine } from '@/core/graph/subgraph/migration/quarantineEntry' import { parseProxyWidgets } from '@/core/schemas/promotionSchema' import { useDomWidgetStore } from '@/stores/domWidgetStore' +import { usePreviewExposureStore } from '@/stores/previewExposureStore' import { makePromotionEntryKey, usePromotionStore } from '@/stores/promotionStore' +import { createNodeLocatorId } from '@/types/nodeIdentification' import { ExecutableNodeDTO } from './ExecutableNodeDTO' import type { ExecutableLGraphNode, ExecutionId } from './ExecutableNodeDTO' @@ -1574,33 +1577,39 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { } /** - * Synchronizes widget values from this SubgraphNode instance to the - * corresponding widgets in the subgraph definition before serialization. - * This ensures nested subgraph widget values are preserved when saving. + * Serializes this SubgraphNode instance. + * + * After ADR 0009 the canonical owner of each promoted value widget is the + * linked `SubgraphInput` itself; host-overlay values live in + * `widgets_values`, and previews live in `properties.previewExposures`. + * `properties.proxyWidgets` is no longer re-emitted; legacy data is preserved + * for one-way ratchet load only. */ override serialize(): ISerialisedNode { - // Sync widget values to subgraph definition before serialization. - // Only sync for inputs that are linked to a promoted widget via _widget. - for (const input of this.inputs) { - if (!input._widget) continue + // TODO(adr-0009): Remove this comment once one stable release has shipped + // without complaints about subgraph value drift. Host promoted-widget + // values now serialize through standard SubgraphInput widgets and must not + // be copied into interior widgets, which would cause cross-host stomping. - const subgraphInput = - input._subgraphSlot ?? - this.subgraph.inputNode.slots.find((slot) => slot.name === input.name) - if (!subgraphInput) continue + const rootGraphId = this.rootGraph.id + const hostLocator = createNodeLocatorId(rootGraphId, this.id) - const connectedWidgets = subgraphInput.getConnectedWidgets() - for (const connectedWidget of connectedWidgets) { - connectedWidget.value = input._widget.value - } + const previewExposures = usePreviewExposureStore().getExposures( + rootGraphId, + hostLocator + ) + if (previewExposures.length > 0) { + this.properties.previewExposures = previewExposures.map((entry) => ({ + ...entry + })) + } else { + delete this.properties.previewExposures } - // Write promotion store state back to properties for serialization - const entries = usePromotionStore().getPromotions( - this.rootGraph.id, - this.id - ) - this.properties.proxyWidgets = this._serializeEntries(entries) + const quarantine = readHostQuarantine(this) + if (quarantine.length === 0) { + delete this.properties.proxyWidgetErrorQuarantine + } return super.serialize() } diff --git a/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts b/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts index f54017d54a..4ed8573f70 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts @@ -427,31 +427,21 @@ describe('SubgraphWidgetPromotion', () => { subgraph.inputNode.slots[0].connect(interiorNode.inputs[0], interiorNode) const hostNode = createTestSubgraphNode(subgraph) - - // serialize() syncs the promotion store into properties.proxyWidgets const serialized = hostNode.serialize() - const originalProxyWidgets = serialized.properties! - .proxyWidgets as string[][] - expect(originalProxyWidgets.length).toBeGreaterThan(0) - expect( - originalProxyWidgets.some(([, widgetName]) => widgetName === 'text') - ).toBe(true) - - // Simulate clone: create a second SubgraphNode configured from serialized data + // ADR 0009: clone preservation no longer relies on properties.proxyWidgets. + // The promoted widgets are derived from the linked SubgraphInputs that + // come through the serialized inputs/links, so the host's own widgets + // getter should expose the promoted view after configure. const cloneNode = createTestSubgraphNode(subgraph) cloneNode.configure(serialized) - const cloneProxyWidgets = cloneNode.properties.proxyWidgets as string[][] - expect(cloneProxyWidgets.length).toBeGreaterThan(0) - expect( - cloneProxyWidgets.some(([, widgetName]) => widgetName === 'text') - ).toBe(true) + const promotedNames = cloneNode.widgets + .filter(isPromotedWidgetView) + .filter((widget) => !widget.name.startsWith('$$')) + .map((widget) => widget.sourceWidgetName) - // Clone's proxyWidgets should reference the same interior node - const originalNodeIds = originalProxyWidgets.map(([nodeId]) => nodeId) - const cloneNodeIds = cloneProxyWidgets.map(([nodeId]) => nodeId) - expect(cloneNodeIds).toStrictEqual(originalNodeIds) + expect(promotedNames).toContain('text') }) })