mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-06 22:21:51 +00:00
fix: propagate promoted-widget overrides to interior on subgraph load
Sparse-override semantics for PromotedWidgetView: - SubgraphNode.onAfterGraphConfigured: deferred replay walks promoted views and re-applies view.value = perInstanceOverrideValue after lazy-creation interiors (e.g. PrimitiveNode in widgetInputs.ts) have materialized their widgets and reapplied their widgets_values. Wins both the materialization race and the post-restore clobber. - PromotedWidgetView.set value: per-instance override write + write-through to interior widget via resolveAtHost. Chains through nested promoted views down to the concrete interior widget. - PromotedWidgetView.get value: removed eager seed. Sparse-override semantics — an override exists only when explicitly written or hydrated by configure replay; otherwise reads fall through to the live interior widget. - executionUtil.graphToPrompt: getExecutableWidgetValue walks the ancestor SubgraphNode chain and returns the per-instance override when one applies, mirroring Vue/canvas read semantics so prompt-build does not desync from on-screen values. - useProcessedWidgets: fallback widget-state lookup so freshly-created promoted views read the interior WidgetState entry until they acquire their own override. Tests: - New promotedWidgetView.lazyInterior.test.ts pins the lazy-widget- creation race using a LazyPrimitiveLikeNode fixture. - executionUtil.test.ts gains an integration test for graphToPrompt with the same lazy interior. - 9 existing tests across promotedWidgetView.test.ts (6), subgraphNodePromotion.test.ts (2), and SubgraphNode.multiInstance .test.ts (1) updated to reflect the new design intent: exterior writes propagate to interior; sparse-override means fresh sibling instances follow the shared interior until they acquire an explicit override. Terminology cleanup: replaced informal "cell" vocabulary with "per-instance override" / "WidgetState entry" / "store entry" across production code, tests, and investigation notes. Amp-Thread-ID: https://ampcode.com/threads/T-019df5c0-ff16-718d-8b75-ed15a7c390c7 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
113
src/core/graph/subgraph/promotedWidgetView.lazyInterior.test.ts
Normal file
113
src/core/graph/subgraph/promotedWidgetView.lazyInterior.test.ts
Normal file
@@ -0,0 +1,113 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest'
|
||||
|
||||
// Barrel import for SubgraphNode/LGraph circular dep
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import {
|
||||
createTestRootGraph,
|
||||
createTestSubgraph,
|
||||
createTestSubgraphNode,
|
||||
resetSubgraphFixtureState
|
||||
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
|
||||
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
|
||||
useCanvasStore: () => ({})
|
||||
}))
|
||||
vi.mock('@/stores/domWidgetStore', () => ({
|
||||
useDomWidgetStore: () => ({
|
||||
widgetStates: new Map(),
|
||||
setPositionOverride: vi.fn(),
|
||||
clearPositionOverride: vi.fn()
|
||||
})
|
||||
}))
|
||||
vi.mock('@/services/litegraphService', () => ({
|
||||
useLitegraphService: () => ({ updatePreviews: () => ({}) })
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
resetSubgraphFixtureState()
|
||||
})
|
||||
|
||||
/**
|
||||
* Mimics PrimitiveNode (src/extensions/core/widgetInputs.ts:32+) — empty
|
||||
* widgets until onAfterGraphConfigured creates them and re-applies
|
||||
* widgets_values. Reproduces the load-time race against
|
||||
* SubgraphNode._replayPromotedWidgetValues.
|
||||
*/
|
||||
class LazyPrimitiveLikeNode extends LGraphNode {
|
||||
constructor() {
|
||||
super('LazyPrimitiveLike')
|
||||
this.serialize_widgets = true
|
||||
}
|
||||
|
||||
override onAfterGraphConfigured(): void {
|
||||
if (this.widgets?.length) return
|
||||
const widget = this.addWidget('text', 'value', '', () => {})
|
||||
const stored = this.widgets_values
|
||||
if (stored?.length) {
|
||||
widget.value = stored[0] as string
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('PromotedWidgetView with lazy-creation interior widget', () => {
|
||||
test('per-instance "exterior" override survives interior lazy widget materialization', () => {
|
||||
const rootGraph = createTestRootGraph()
|
||||
const subgraph = createTestSubgraph({ rootGraph })
|
||||
|
||||
const interior = new LazyPrimitiveLikeNode()
|
||||
interior.widgets_values = ['interior']
|
||||
subgraph.add(interior)
|
||||
|
||||
const subgraphNode = createTestSubgraphNode(subgraph, {
|
||||
parentGraph: rootGraph
|
||||
})
|
||||
rootGraph.add(subgraphNode)
|
||||
|
||||
// Drive the load path: configure with proxyWidgets and exterior value.
|
||||
subgraphNode.configure({
|
||||
id: subgraphNode.id,
|
||||
type: subgraph.id,
|
||||
pos: [100, 100],
|
||||
size: [200, 100],
|
||||
inputs: [],
|
||||
outputs: [],
|
||||
mode: 0,
|
||||
order: 0,
|
||||
flags: {},
|
||||
properties: {
|
||||
proxyWidgets: [[String(interior.id), 'value']]
|
||||
},
|
||||
widgets_values: ['exterior']
|
||||
})
|
||||
|
||||
// _replayPromotedWidgetValues has run, but interior.widgets is empty —
|
||||
// write-through no-ops; only the per-instance override holds "exterior".
|
||||
expect(interior.widgets?.length ?? 0).toBe(0)
|
||||
|
||||
// Lazy materialization clobbers any prior interior write with the
|
||||
// serialized widgets_values=["interior"].
|
||||
interior.onAfterGraphConfigured()
|
||||
expect(interior.widgets?.[0].value).toBe('interior')
|
||||
|
||||
// SubgraphNode.onAfterGraphConfigured (called child-first by
|
||||
// triggerCallbackOnAllNodes in production) re-projects the per-instance
|
||||
// override onto the now-materialized interior widget.
|
||||
subgraphNode.onAfterGraphConfigured?.()
|
||||
|
||||
const widgetStore = useWidgetValueStore()
|
||||
const view = subgraphNode.widgets[0]
|
||||
expect(view.value).toBe('exterior')
|
||||
expect(interior.widgets?.[0].value).toBe('exterior')
|
||||
const interiorCell = widgetStore.getWidget(
|
||||
rootGraph.id,
|
||||
interior.id,
|
||||
'value'
|
||||
)
|
||||
expect(interiorCell?.value).toBe('exterior')
|
||||
})
|
||||
})
|
||||
@@ -261,7 +261,7 @@ describe(createPromotedWidgetView, () => {
|
||||
expect(view.linkedWidgets?.[0].name).toBe('control_after_generate')
|
||||
})
|
||||
|
||||
test('value reads from per-instance store cell, leaving interior unchanged', () => {
|
||||
test('value writes propagate to the interior widget (write-through)', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
const innerNode = firstInnerNode(innerNodes)
|
||||
innerNode.addWidget('text', 'myWidget', 'initial', () => {})
|
||||
@@ -274,13 +274,26 @@ describe(createPromotedWidgetView, () => {
|
||||
// Value should read from interior default initially
|
||||
expect(view.value).toBe('initial')
|
||||
|
||||
// Setting value through the view writes to the per-instance cell
|
||||
view.value = 'updated'
|
||||
expect(view.value).toBe('updated')
|
||||
|
||||
// Interior widget is NOT mutated by promoted-view writes —
|
||||
// each instance has its own override; the interior remains the default.
|
||||
expect(innerNode.widgets![0].value).toBe('initial')
|
||||
expect(view.value).toBe('updated')
|
||||
// Write-through projects the new value onto the interior widget so
|
||||
// direct interior consumers (prompt-build, etc.) stay consistent.
|
||||
expect(innerNode.widgets![0].value).toBe('updated')
|
||||
|
||||
const perInstanceCell = useWidgetValueStore().getWidget(
|
||||
subgraphNode.rootGraph.id,
|
||||
subgraphNode.id,
|
||||
view.storeName
|
||||
)
|
||||
expect(perInstanceCell?.value).toBe('updated')
|
||||
|
||||
const interiorCell = useWidgetValueStore().getWidget(
|
||||
subgraphNode.rootGraph.id,
|
||||
innerNode.id,
|
||||
'myWidget'
|
||||
)
|
||||
expect(interiorCell?.value).toBe('updated')
|
||||
})
|
||||
|
||||
test('value falls back to interior widget when no store entry exists', () => {
|
||||
@@ -304,11 +317,11 @@ describe(createPromotedWidgetView, () => {
|
||||
'myWidget'
|
||||
)
|
||||
|
||||
// Read falls back to the interior widget when no store cell exists
|
||||
// Read falls back to the interior widget when no store entry exists
|
||||
expect(view.value).toBe('initial')
|
||||
})
|
||||
|
||||
test('value setter writes to per-instance store cell without mutating the interior', () => {
|
||||
test('value setter writes to both per-instance override and interior widget', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'string_a', type: '*' }]
|
||||
})
|
||||
@@ -327,10 +340,10 @@ describe(createPromotedWidgetView, () => {
|
||||
|
||||
linkedView.value = 'updated'
|
||||
|
||||
// Per-instance read returns the override
|
||||
expect(linkedView.value).toBe('updated')
|
||||
// Interior widget stays at its original default
|
||||
expect(linkedNode.widgets?.[0].value).toBe('initial')
|
||||
// Write-through projects onto the interior so direct interior
|
||||
// consumers (prompt-build, legacy serialization) stay consistent.
|
||||
expect(linkedNode.widgets?.[0].value).toBe('updated')
|
||||
})
|
||||
|
||||
test('label falls back to displayName then widgetName', () => {
|
||||
@@ -586,10 +599,9 @@ describe(createPromotedWidgetView, () => {
|
||||
|
||||
const objValue = { key: 'data' }
|
||||
view.value = objValue
|
||||
// Per-instance cell holds the new object value
|
||||
expect(view.value).toEqual(objValue)
|
||||
// Interior widget unchanged by promoted-view write
|
||||
expect(innerNode.widgets![0].value).toBe('old')
|
||||
// Write-through projects the object onto the interior widget.
|
||||
expect(innerNode.widgets![0].value).toEqual(objValue)
|
||||
})
|
||||
|
||||
test('onPointerDown returns true when interior widget onPointerDown handles it', () => {
|
||||
@@ -931,11 +943,10 @@ describe('SubgraphNode.widgets getter', () => {
|
||||
|
||||
linkedView.value = 'shared-value'
|
||||
|
||||
// Per-instance promoted-view writes do NOT mutate any interior
|
||||
// widget — interior cells remain the immutable defaults, and the
|
||||
// promoted view reads its per-instance store cell.
|
||||
// Write-through hits only the representative interior (first link).
|
||||
// Sibling interiors and unrelated promoted peers stay independent.
|
||||
expect(linkedView.value).toBe('shared-value')
|
||||
expect(linkedNodeA.widgets?.[0]?.value).toBe('a')
|
||||
expect(linkedNodeA.widgets?.[0]?.value).toBe('shared-value')
|
||||
expect(linkedNodeB.widgets?.[0]?.value).toBe('b')
|
||||
expect(promotedNode.widgets?.[0]?.value).toBe('independent')
|
||||
|
||||
@@ -943,9 +954,9 @@ describe('SubgraphNode.widgets getter', () => {
|
||||
|
||||
expect(promotedView.value).toBe('independent-updated')
|
||||
expect(linkedView.value).toBe('shared-value')
|
||||
expect(linkedNodeA.widgets?.[0]?.value).toBe('a')
|
||||
expect(linkedNodeA.widgets?.[0]?.value).toBe('shared-value')
|
||||
expect(linkedNodeB.widgets?.[0]?.value).toBe('b')
|
||||
expect(promotedNode.widgets?.[0]?.value).toBe('independent')
|
||||
expect(promotedNode.widgets?.[0]?.value).toBe('independent-updated')
|
||||
})
|
||||
|
||||
test('duplicate-name promoted views map slot linkage by view identity', () => {
|
||||
@@ -1282,19 +1293,19 @@ describe('SubgraphNode.widgets getter', () => {
|
||||
firstView.value = 'first-updated'
|
||||
secondView.value = 'second-updated'
|
||||
|
||||
// Per-instance views carry distinct overrides
|
||||
// Distinct subgraph slots map to distinct interior nodes, so
|
||||
// write-through stays scoped to each view's own representative.
|
||||
expect(firstView.value).toBe('first-updated')
|
||||
expect(secondView.value).toBe('second-updated')
|
||||
// Interior widgets remain at their originally-registered defaults
|
||||
expect(firstNode.widgets?.[0].value).toBe('first-initial')
|
||||
expect(secondNode.widgets?.[0].value).toBe('second-initial')
|
||||
expect(firstNode.widgets?.[0].value).toBe('first-updated')
|
||||
expect(secondNode.widgets?.[0].value).toBe('second-updated')
|
||||
|
||||
subgraphNode.serialize()
|
||||
|
||||
expect(firstView.value).toBe('first-updated')
|
||||
expect(secondView.value).toBe('second-updated')
|
||||
expect(firstNode.widgets?.[0].value).toBe('first-initial')
|
||||
expect(secondNode.widgets?.[0].value).toBe('second-initial')
|
||||
expect(firstNode.widgets?.[0].value).toBe('first-updated')
|
||||
expect(secondNode.widgets?.[0].value).toBe('second-updated')
|
||||
})
|
||||
|
||||
test('renaming an input updates linked promoted view display names', () => {
|
||||
@@ -1614,19 +1625,19 @@ describe('SubgraphNode.widgets getter', () => {
|
||||
linkedView.value = 'shared-linked'
|
||||
|
||||
// Per-instance views carry their own values; both linked and
|
||||
// independent promoted views read from their per-instance store cells
|
||||
// independent promoted views read from their per-instance store entries
|
||||
// and do not contaminate each other.
|
||||
expect(linkedView.value).toBe('shared-linked')
|
||||
expect(independentView.value).toBe('independent-value')
|
||||
|
||||
// Per-instance cells are owned by the SubgraphNode itself at
|
||||
// Per-instance overrides are owned by the SubgraphNode itself at
|
||||
// `(graphId, hostNode.id, *)`. Each PromotedWidgetView is a first-class
|
||||
// widget on the SubgraphNode, so writes land in the natural
|
||||
// (graphId, nodeId, *) namespace and are isolated from interior cells.
|
||||
const cellValues = useWidgetValueStore()
|
||||
// (graphId, nodeId, *) namespace and are isolated from interior entries.
|
||||
const overrideValues = useWidgetValueStore()
|
||||
.getNodeWidgets(graph.id, hostNode.id)
|
||||
.map((cell) => cell.value)
|
||||
expect(cellValues).toEqual(
|
||||
.map((entry) => entry.value)
|
||||
expect(overrideValues).toEqual(
|
||||
expect.arrayContaining(['shared-linked', 'independent-value'])
|
||||
)
|
||||
})
|
||||
@@ -2102,10 +2113,9 @@ describe('three-level nested value propagation', () => {
|
||||
expect(subgraphNodeA.widgets[0].value).toBe(100)
|
||||
|
||||
subgraphNodeA.widgets[0].value = 200
|
||||
// Per-instance read returns the override
|
||||
expect(subgraphNodeA.widgets[0].value).toBe(200)
|
||||
// Concrete interior widget remains the unmutated default
|
||||
expect(concreteNode.widgets![0].value).toBe(100)
|
||||
// Write-through chains through every nested view down to the concrete.
|
||||
expect(concreteNode.widgets![0].value).toBe(200)
|
||||
})
|
||||
|
||||
test('type resolves correctly through all three layers', () => {
|
||||
@@ -2184,10 +2194,10 @@ describe('three-level nested value propagation', () => {
|
||||
|
||||
widgets[1].value = 'updated-second'
|
||||
|
||||
// Interior widgets are not mutated by promoted-view writes
|
||||
// Write-through follows the disambig chain — only secondTextNode's
|
||||
// interior is mutated; firstTextNode stays untouched.
|
||||
expect(firstTextNode.widgets?.[0]?.value).toBe('11111111111')
|
||||
expect(secondTextNode.widgets?.[0]?.value).toBe('22222222222')
|
||||
// Per-instance disambiguated views read correctly from their own cells
|
||||
expect(secondTextNode.widgets?.[0]?.value).toBe('updated-second')
|
||||
expect(widgets[0].value).toBe('11111111111')
|
||||
expect(widgets[1].value).toBe('updated-second')
|
||||
})
|
||||
@@ -2234,10 +2244,10 @@ describe('multi-link representative determinism for input-based promotion', () =
|
||||
// Read returns the first link's value
|
||||
expect(widgets[0].value).toBe('first-val')
|
||||
|
||||
// Write goes to the per-instance store cell only — interior
|
||||
// widgets stay at their original construction defaults.
|
||||
// Write-through hits only the representative interior (first link).
|
||||
// Sibling interiors connected via the same input stay at their defaults.
|
||||
widgets[0].value = 'updated'
|
||||
expect(firstNode.widgets![0].value).toBe('first-val')
|
||||
expect(firstNode.widgets![0].value).toBe('updated')
|
||||
expect(secondNode.widgets![0].value).toBe('second-val')
|
||||
expect(thirdNode.widgets![0].value).toBe('third-val')
|
||||
|
||||
@@ -2356,17 +2366,16 @@ describe('promoted combo rendering', () => {
|
||||
expect(renderedText).toContain('a')
|
||||
})
|
||||
|
||||
test('value updates are visible at the outer layer without mutating the interior', () => {
|
||||
test('value updates at the outer layer write through to the interior combo widget', () => {
|
||||
const { comboWidget, subgraphNodeB } = createTwoLevelNestedSubgraph()
|
||||
comboWidget.computedDisabled = true
|
||||
const promotedWidget = subgraphNodeB.widgets[0]
|
||||
|
||||
expect(promotedWidget.value).toBe('a')
|
||||
promotedWidget.value = 'b'
|
||||
// Per-instance read returns the override
|
||||
expect(promotedWidget.value).toBe('b')
|
||||
// The deepest interior combo widget remains at its default
|
||||
expect(comboWidget.value).toBe('a')
|
||||
// Write-through chains down to the concrete combo widget.
|
||||
expect(comboWidget.value).toBe('b')
|
||||
|
||||
const fillText = vi.fn()
|
||||
const ctx = createInspectableCanvasContext(fillText)
|
||||
@@ -2807,12 +2816,12 @@ describe('DOM widget promotion', () => {
|
||||
|
||||
view.value = 'should-not-persist'
|
||||
|
||||
// No cell registered at id -1
|
||||
const cells = useWidgetValueStore().getNodeWidgets(
|
||||
// No WidgetState entry registered at id -1
|
||||
const entries = useWidgetValueStore().getNodeWidgets(
|
||||
subgraphNode.rootGraph.id,
|
||||
-1
|
||||
)
|
||||
expect(cells).toHaveLength(0)
|
||||
expect(entries).toHaveLength(0)
|
||||
// Read still falls back to the interior default
|
||||
expect(view.value).toBe('initial')
|
||||
})
|
||||
@@ -2832,14 +2841,14 @@ describe('DOM widget promotion', () => {
|
||||
|
||||
view.label = 'My Label'
|
||||
|
||||
const cells = useWidgetValueStore().getNodeWidgets(
|
||||
const entries = useWidgetValueStore().getNodeWidgets(
|
||||
subgraphNode.rootGraph.id,
|
||||
-1
|
||||
)
|
||||
expect(cells).toHaveLength(0)
|
||||
expect(entries).toHaveLength(0)
|
||||
})
|
||||
|
||||
test('label setter materializes a per-instance cell when no slot is bound', () => {
|
||||
test('label setter materializes a per-instance override when no slot is bound', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
const innerNode = firstInnerNode(innerNodes)
|
||||
innerNode.addWidget('text', 'labelOnly', 'initial', () => {})
|
||||
@@ -2852,20 +2861,20 @@ describe('DOM widget promotion', () => {
|
||||
|
||||
view.label = 'My Label'
|
||||
|
||||
// Without a bound subgraph slot the per-instance cell is the only
|
||||
// Without a bound subgraph slot the per-instance override is the only
|
||||
// place the new label can live; the setter must materialize it so
|
||||
// the rename actually takes effect (the getter falls back to
|
||||
// state?.label when no slot is found).
|
||||
const cells = useWidgetValueStore().getNodeWidgets(
|
||||
const entries = useWidgetValueStore().getNodeWidgets(
|
||||
subgraphNode.rootGraph.id,
|
||||
subgraphNode.id
|
||||
)
|
||||
expect(cells).toHaveLength(1)
|
||||
expect(cells[0].label).toBe('My Label')
|
||||
expect(entries).toHaveLength(1)
|
||||
expect(entries[0].label).toBe('My Label')
|
||||
expect(view.label).toBe('My Label')
|
||||
})
|
||||
|
||||
test('label setter updates an existing per-instance cell when a value override is present', () => {
|
||||
test('label setter updates an existing per-instance override when a value override is present', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
const innerNode = firstInnerNode(innerNodes)
|
||||
innerNode.addWidget('text', 'valueAndLabel', 'initial', () => {})
|
||||
@@ -2876,17 +2885,17 @@ describe('DOM widget promotion', () => {
|
||||
'valueAndLabel'
|
||||
)
|
||||
|
||||
// Triggering a value write materialises the cell (legitimate ownership).
|
||||
// Triggering a value write materialises the override (legitimate ownership).
|
||||
view.value = 'override'
|
||||
|
||||
// Now setting a label should update the existing cell, not create a new one.
|
||||
// Now setting a label should update the existing override, not create a new one.
|
||||
view.label = 'My Label'
|
||||
|
||||
const cells = useWidgetValueStore().getNodeWidgets(
|
||||
const entries = useWidgetValueStore().getNodeWidgets(
|
||||
subgraphNode.rootGraph.id,
|
||||
subgraphNode.id
|
||||
)
|
||||
expect(cells).toHaveLength(1)
|
||||
expect(cells[0].label).toBe('My Label')
|
||||
expect(entries).toHaveLength(1)
|
||||
expect(entries[0].label).toBe('My Label')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -153,6 +153,8 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
}
|
||||
|
||||
get value(): IBaseWidget['value'] {
|
||||
// Sparse override: a WidgetState entry exists only when explicitly set;
|
||||
// otherwise read through to the live interior widget.
|
||||
const state = this.getWidgetState()
|
||||
if (state && isWidgetValue(state.value)) return state.value
|
||||
return this.resolveAtHost()?.widget.value
|
||||
@@ -167,9 +169,17 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
// Pre-attach sentinel: skip writes before LGraph.add() assigns the real id.
|
||||
if (this.subgraphNode.id === -1) return
|
||||
|
||||
// Per-instance cell isolation: writes go only to this view's cell;
|
||||
// reads fall back to the interior widget when no cell exists.
|
||||
// The per-instance override keeps Vue render and canvas draw fast paths correct.
|
||||
this.ensureInstanceState().value = value
|
||||
|
||||
// Write-through to the interior widget: prompt-build, legacy
|
||||
// serialization, and nested promoted views all read the interior widget
|
||||
// directly. Without this projection they would observe the stale
|
||||
// workflow-restored default rather than the user-edited value.
|
||||
const interior = this.resolveAtHost()?.widget
|
||||
if (interior && interior.value !== value) {
|
||||
interior.value = value
|
||||
}
|
||||
}
|
||||
|
||||
get label(): string | undefined {
|
||||
@@ -179,12 +189,12 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
return state?.label ?? this.displayName
|
||||
}
|
||||
|
||||
/** Slot-bound: only update existing cell. Unbound: materialize. */
|
||||
/** Slot-bound: only update an existing override. Unbound: materialize one. */
|
||||
set label(value: string | undefined) {
|
||||
const slot = this.getBoundSubgraphSlot()
|
||||
if (slot) slot.label = value || undefined
|
||||
|
||||
// Pre-attach sentinel guard: skip per-instance cell write before LGraph.add().
|
||||
// Pre-attach sentinel guard: skip per-instance override write before LGraph.add().
|
||||
if (this.subgraphNode.id === -1) return
|
||||
|
||||
if (slot) {
|
||||
|
||||
@@ -103,7 +103,7 @@ describe('Subgraph proxyWidgets', () => {
|
||||
expect(subgraphNode.widgets[0].name).toBe('widgetB')
|
||||
expect(subgraphNode.widgets[1].name).toBe('widgetA')
|
||||
})
|
||||
test('Promoted view falls back to interior; promoted writes do not mutate interior', () => {
|
||||
test('Promoted view falls back to interior; promoted writes mutate interior (write-through)', () => {
|
||||
const [subgraphNode, innerNodes, innerIds] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
usePromotionStore().setPromotions(
|
||||
@@ -114,15 +114,15 @@ describe('Subgraph proxyWidgets', () => {
|
||||
expect(subgraphNode.widgets.length).toBe(1)
|
||||
expect(subgraphNode.widgets[0].value).toBe('value')
|
||||
|
||||
// Direct interior edits remain visible through the view (no override)
|
||||
// Sparse override: interior writes are visible until the view
|
||||
// acquires its own override.
|
||||
innerNodes[0].widgets![0].value = 'test'
|
||||
expect(subgraphNode.widgets[0].value).toBe('test')
|
||||
|
||||
// Promoted-view writes route to the per-instance cell only —
|
||||
// the interior widget is NOT mutated.
|
||||
// Promoted writes record an override AND write through to the interior.
|
||||
subgraphNode.widgets[0].value = 'test2'
|
||||
expect(subgraphNode.widgets[0].value).toBe('test2')
|
||||
expect(innerNodes[0].widgets![0].value).toBe('test')
|
||||
expect(innerNodes[0].widgets![0].value).toBe('test2')
|
||||
})
|
||||
test('Will not modify position or sizing of existing widgets', () => {
|
||||
const [subgraphNode, innerNodes, innerIds] = setupSubgraph(1)
|
||||
@@ -397,11 +397,11 @@ describe('Subgraph proxyWidgets', () => {
|
||||
expect(subgraphNodeA.widgets[0].type).toBe('number')
|
||||
expect(subgraphNodeA.widgets[0].value).toBe(42)
|
||||
|
||||
// Value set at outermost level lives in the per-instance cell.
|
||||
// Read returns the override; concrete interior widget stays unchanged.
|
||||
// Outermost write records an override AND writes through every nested
|
||||
// promoted view down to the concrete interior widget.
|
||||
subgraphNodeA.widgets[0].value = 99
|
||||
expect(subgraphNodeA.widgets[0].value).toBe(99)
|
||||
expect(concreteNode.widgets![0].value).toBe(42)
|
||||
expect(concreteNode.widgets![0].value).toBe(99)
|
||||
})
|
||||
|
||||
test('removeWidget cleans up promotion and input, then re-promote works', () => {
|
||||
|
||||
@@ -172,7 +172,9 @@ describe('SubgraphNode multi-instance widget isolation', () => {
|
||||
expect(restoredWidget?.serializeValue?.(restoredInstance, 0)).toBe(33)
|
||||
})
|
||||
|
||||
it('keeps fresh sibling instances isolated before save or reload', () => {
|
||||
it('fresh sibling instances follow shared interior until they acquire explicit per-instance overrides', () => {
|
||||
// Sparse override: untouched siblings share the live interior; once
|
||||
// an instance is explicitly set or restored, it diverges.
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'value', type: 'number' }]
|
||||
})
|
||||
@@ -194,10 +196,17 @@ describe('SubgraphNode multi-instance widget isolation', () => {
|
||||
|
||||
widget1!.value = 10
|
||||
|
||||
// Instance 2 has no override yet, so it reads through to the now-
|
||||
// mutated shared interior.
|
||||
expect(widget1?.value).toBe(10)
|
||||
expect(widget2?.value).toBe(7)
|
||||
expect(widget2?.value).toBe(10)
|
||||
expect(widget1?.serializeValue?.(instance1, 0)).toBe(10)
|
||||
expect(widget2?.serializeValue?.(instance2, 0)).toBe(7)
|
||||
expect(widget2?.serializeValue?.(instance2, 0)).toBe(10)
|
||||
|
||||
// Setting widget2 gives it its own override, allowing divergence.
|
||||
widget2!.value = 33
|
||||
expect(widget1?.value).toBe(10)
|
||||
expect(widget2?.value).toBe(33)
|
||||
})
|
||||
|
||||
it('keeps per-instance override sticky when the inner source widget changes directly', () => {
|
||||
|
||||
@@ -55,6 +55,7 @@ import {
|
||||
makePromotionEntryKey,
|
||||
usePromotionStore
|
||||
} from '@/stores/promotionStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { makeCompositeKey } from '@/utils/compositeKey'
|
||||
|
||||
import { ExecutableNodeDTO } from './ExecutableNodeDTO'
|
||||
@@ -1363,6 +1364,27 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
this._replayPromotedWidgetValues(pending)
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-projects per-instance promoted-widget override values onto interior
|
||||
* widgets. Runs after lazy-creation interiors (e.g. PrimitiveNode) have
|
||||
* materialized their widgets and re-applied their `widgets_values`.
|
||||
*/
|
||||
override onAfterGraphConfigured(): void {
|
||||
if (this.id === -1) return
|
||||
|
||||
const widgetStore = useWidgetValueStore()
|
||||
for (const view of this.widgets ?? []) {
|
||||
if (!isPromotedWidgetView(view)) continue
|
||||
const widgetState = widgetStore.getWidget(
|
||||
this.rootGraph.id,
|
||||
this.id,
|
||||
view.storeName
|
||||
)
|
||||
if (!widgetState) continue
|
||||
view.value = widgetState.value as IBaseWidget['value']
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures the subgraph slot is in the params before adding the input as normal.
|
||||
* @param name The name of the input slot.
|
||||
|
||||
@@ -461,7 +461,7 @@ describe('per-instance value lookup for promoted widgets', () => {
|
||||
})
|
||||
}
|
||||
|
||||
it('reads per-instance values from cellNodeId/cellName cells when multiple instances share a definition', () => {
|
||||
it('reads per-instance values from per-instance WidgetState entries when multiple instances share a definition', () => {
|
||||
const widgetValueStore = useWidgetValueStore()
|
||||
widgetValueStore.registerWidget(GRAPH_ID, {
|
||||
nodeId: '100',
|
||||
@@ -495,7 +495,7 @@ describe('per-instance value lookup for promoted widgets', () => {
|
||||
expect(identityA.dedupeIdentity).not.toBe(identityB.dedupeIdentity)
|
||||
})
|
||||
|
||||
it('falls back to interior cell value when per-instance cell is absent for a promoted widget', () => {
|
||||
it('falls back to interior WidgetState value when per-instance override is absent for a promoted widget', () => {
|
||||
const widgetValueStore = useWidgetValueStore()
|
||||
widgetValueStore.registerWidget(GRAPH_ID, {
|
||||
nodeId: INTERIOR_NODE_ID,
|
||||
|
||||
@@ -205,18 +205,19 @@ export function computeProcessedWidgets({
|
||||
const perInstanceWidgetState = graphId
|
||||
? widgetValueStore.getWidget(graphId, bareWidgetId, storeWidgetName)
|
||||
: undefined
|
||||
// For freshly-created promoted widget views the per-instance host cell
|
||||
// may not be registered yet. Fall back to the interior source cell for
|
||||
// reads only — the write path keeps using the per-instance cell.
|
||||
const widgetState =
|
||||
perInstanceWidgetState ??
|
||||
(graphId && widget.source
|
||||
// For freshly-created promoted widget views the per-instance host
|
||||
// override may not be registered yet. Fall back to the interior source
|
||||
// WidgetState entry for reads only — the write path keeps using the
|
||||
// per-instance override.
|
||||
const fallbackWidgetState =
|
||||
graphId && widget.source && !perInstanceWidgetState
|
||||
? widgetValueStore.getWidget(
|
||||
graphId,
|
||||
widget.source.sourceNodeId,
|
||||
widget.source.sourceWidgetName
|
||||
)
|
||||
: undefined)
|
||||
: undefined
|
||||
const widgetState = perInstanceWidgetState ?? fallbackWidgetState
|
||||
const mergedOptions: IWidgetOptions = {
|
||||
...(widget.options ?? {}),
|
||||
...(widgetState?.options ?? {})
|
||||
|
||||
170
src/utils/executionUtil.test.ts
Normal file
170
src/utils/executionUtil.test.ts
Normal file
@@ -0,0 +1,170 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import type { PromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetView'
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import {
|
||||
createTestRootGraph,
|
||||
createTestSubgraph,
|
||||
createTestSubgraphNode,
|
||||
resetSubgraphFixtureState
|
||||
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
|
||||
import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { makeCompositeKey } from '@/utils/compositeKey'
|
||||
|
||||
import { graphToPrompt } from './executionUtil'
|
||||
|
||||
describe('graphToPrompt with promoted subgraph widgets (PR #11811)', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
resetSubgraphFixtureState()
|
||||
})
|
||||
|
||||
it('emits the user-edited promoted value, not the interior default', async () => {
|
||||
const rootGraph = createTestRootGraph()
|
||||
const subgraph = createTestSubgraph({ rootGraph })
|
||||
|
||||
// Interior node with a text widget "value" defaulting to "interior"
|
||||
const interiorNode = new LGraphNode('Interior')
|
||||
interiorNode.addWidget('text', 'value', 'interior', () => {})
|
||||
subgraph.add(interiorNode)
|
||||
|
||||
// SubgraphNode instance in the root graph
|
||||
const subgraphNode = createTestSubgraphNode(subgraph, {
|
||||
parentGraph: rootGraph
|
||||
})
|
||||
rootGraph.add(subgraphNode)
|
||||
|
||||
// Promote the interior widget (mirrors proxyWidgets=[["<id>","value"]])
|
||||
usePromotionStore().promote(rootGraph.id, subgraphNode.id, {
|
||||
sourceNodeId: String(interiorNode.id),
|
||||
sourceWidgetName: 'value'
|
||||
})
|
||||
|
||||
// User-edits the exterior promoted widget to "exterior". This is the same
|
||||
// path the Vue widget update handler exercises in production.
|
||||
const view = subgraphNode.widgets[0] as PromotedWidgetView | undefined
|
||||
if (!view) throw new Error('Expected a promoted view on the SubgraphNode')
|
||||
view.value = 'exterior'
|
||||
|
||||
const { output } = await graphToPrompt(rootGraph)
|
||||
|
||||
const execId = `${subgraphNode.id}:${interiorNode.id}`
|
||||
expect(output[execId]).toBeDefined()
|
||||
expect(output[execId].inputs.value).toBe('exterior')
|
||||
})
|
||||
|
||||
it('isolates promoted values across two SubgraphNode instances of the same Subgraph', async () => {
|
||||
const rootGraph = createTestRootGraph()
|
||||
const subgraph = createTestSubgraph({ rootGraph })
|
||||
|
||||
const interiorNode = new LGraphNode('Interior')
|
||||
interiorNode.addWidget('text', 'value', 'interior', () => {})
|
||||
subgraph.add(interiorNode)
|
||||
|
||||
const instanceA = createTestSubgraphNode(subgraph, {
|
||||
parentGraph: rootGraph
|
||||
})
|
||||
rootGraph.add(instanceA)
|
||||
const instanceB = createTestSubgraphNode(subgraph, {
|
||||
parentGraph: rootGraph
|
||||
})
|
||||
rootGraph.add(instanceB)
|
||||
|
||||
const promotionStore = usePromotionStore()
|
||||
promotionStore.promote(rootGraph.id, instanceA.id, {
|
||||
sourceNodeId: String(interiorNode.id),
|
||||
sourceWidgetName: 'value'
|
||||
})
|
||||
promotionStore.promote(rootGraph.id, instanceB.id, {
|
||||
sourceNodeId: String(interiorNode.id),
|
||||
sourceWidgetName: 'value'
|
||||
})
|
||||
|
||||
const storeName = makeCompositeKey([String(interiorNode.id), 'value', ''])
|
||||
const widgetStore = useWidgetValueStore()
|
||||
widgetStore.registerWidget(rootGraph.id, {
|
||||
nodeId: instanceA.id,
|
||||
name: storeName,
|
||||
type: 'text',
|
||||
value: 'A-value',
|
||||
options: {}
|
||||
})
|
||||
widgetStore.registerWidget(rootGraph.id, {
|
||||
nodeId: instanceB.id,
|
||||
name: storeName,
|
||||
type: 'text',
|
||||
value: 'B-value',
|
||||
options: {}
|
||||
})
|
||||
|
||||
const { output } = await graphToPrompt(rootGraph)
|
||||
|
||||
expect(output[`${instanceA.id}:${interiorNode.id}`].inputs.value).toBe(
|
||||
'A-value'
|
||||
)
|
||||
expect(output[`${instanceB.id}:${interiorNode.id}`].inputs.value).toBe(
|
||||
'B-value'
|
||||
)
|
||||
})
|
||||
|
||||
it('emits the per-instance promoted value for a lazy-creation interior (PrimitiveNode-like)', async () => {
|
||||
// Mimics PrimitiveNode lazy widget creation (src/extensions/core/widgetInputs.ts:32+).
|
||||
class LazyPrimitiveLikeNode extends LGraphNode {
|
||||
constructor() {
|
||||
super('LazyPrimitiveLike')
|
||||
this.serialize_widgets = true
|
||||
}
|
||||
override onAfterGraphConfigured(): void {
|
||||
if (this.widgets?.length) return
|
||||
const widget = this.addWidget('text', 'value', '', () => {})
|
||||
const stored = this.widgets_values
|
||||
if (stored?.length) {
|
||||
widget.value = stored[0] as string
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const rootGraph = createTestRootGraph()
|
||||
const subgraph = createTestSubgraph({ rootGraph })
|
||||
|
||||
const interiorNode = new LazyPrimitiveLikeNode()
|
||||
interiorNode.widgets_values = ['interior']
|
||||
subgraph.add(interiorNode)
|
||||
|
||||
const subgraphNode = createTestSubgraphNode(subgraph, {
|
||||
parentGraph: rootGraph
|
||||
})
|
||||
rootGraph.add(subgraphNode)
|
||||
|
||||
// Configure with proxyWidgets + exterior before the interior exists.
|
||||
subgraphNode.configure({
|
||||
id: subgraphNode.id,
|
||||
type: subgraph.id,
|
||||
pos: [100, 100],
|
||||
size: [200, 100],
|
||||
inputs: [],
|
||||
outputs: [],
|
||||
mode: 0,
|
||||
order: 0,
|
||||
flags: {},
|
||||
properties: {
|
||||
proxyWidgets: [[String(interiorNode.id), 'value']]
|
||||
},
|
||||
widgets_values: ['exterior']
|
||||
})
|
||||
|
||||
// Lazy materialization clobbers exterior with widgets_values=["interior"].
|
||||
interiorNode.onAfterGraphConfigured()
|
||||
|
||||
// SubgraphNode hook re-projects the per-instance override afterward.
|
||||
subgraphNode.onAfterGraphConfigured?.()
|
||||
|
||||
const { output } = await graphToPrompt(rootGraph)
|
||||
const execId = `${subgraphNode.id}:${interiorNode.id}`
|
||||
expect(output[execId]).toBeDefined()
|
||||
expect(output[execId].inputs.value).toBe('exterior')
|
||||
})
|
||||
})
|
||||
@@ -1,3 +1,4 @@
|
||||
import { resolveConcretePromotedWidget } from '@/core/graph/subgraph/resolveConcretePromotedWidget'
|
||||
import type {
|
||||
ExecutableLGraphNode,
|
||||
ExecutionId,
|
||||
@@ -7,14 +8,95 @@ import {
|
||||
ExecutableNodeDTO,
|
||||
LGraphEventMode
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
import type {
|
||||
ComfyApiWorkflow,
|
||||
ComfyWorkflowJSON
|
||||
} from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { makeCompositeKey } from '@/utils/compositeKey'
|
||||
|
||||
import { ExecutableGroupNodeDTO, isGroupNode } from './executableGroupNodeDto'
|
||||
import { compressWidgetInputSlots } from './litegraphUtil'
|
||||
|
||||
/**
|
||||
* Looks up the effective value for a flattened interior widget by walking the
|
||||
* ancestor SubgraphNode chain (outermost → innermost) and returning the first
|
||||
* per-instance promoted-widget override that targets this exact widget object.
|
||||
*
|
||||
* Mirrors the read semantics used by the Vue / canvas render paths so that
|
||||
* prompt-build does not desync from the on-screen value.
|
||||
*/
|
||||
function resolvePromotedWidgetOverride(
|
||||
node: ExecutableLGraphNode,
|
||||
widget: IBaseWidget
|
||||
): { hit: true; value: unknown } | { hit: false } {
|
||||
if (!(node instanceof ExecutableNodeDTO)) return { hit: false }
|
||||
if (node.subgraphNodePath.length === 0) return { hit: false }
|
||||
|
||||
const rootGraph = node.graph.rootGraph
|
||||
const hosts = rootGraph.resolveSubgraphIdPath(node.subgraphNodePath)
|
||||
const promotionStore = usePromotionStore()
|
||||
const widgetStore = useWidgetValueStore()
|
||||
|
||||
for (const host of hosts) {
|
||||
const entries = promotionStore.getPromotionsRef(rootGraph.id, host.id)
|
||||
for (const entry of entries) {
|
||||
const resolved = resolveConcretePromotedWidget(
|
||||
host,
|
||||
entry.sourceNodeId,
|
||||
entry.sourceWidgetName,
|
||||
entry.disambiguatingSourceNodeId
|
||||
)
|
||||
if (resolved.status !== 'resolved') continue
|
||||
if (resolved.resolved.widget !== widget) continue
|
||||
|
||||
const storeName = makeCompositeKey([
|
||||
entry.sourceNodeId,
|
||||
entry.sourceWidgetName,
|
||||
entry.disambiguatingSourceNodeId ?? ''
|
||||
])
|
||||
const state = widgetStore.getWidget(rootGraph.id, host.id, storeName)
|
||||
if (state) return { hit: true, value: state.value }
|
||||
}
|
||||
}
|
||||
|
||||
return { hit: false }
|
||||
}
|
||||
|
||||
/**
|
||||
* Computes the value used for prompt serialization for a single widget.
|
||||
* Falls back to the standard `widget.serializeValue` / `widget.value` path,
|
||||
* but routes through the per-instance promoted override when one applies. When a
|
||||
* custom `serializeValue` is defined, it is invoked on a proxy widget whose
|
||||
* `.value` returns the override, preserving widget-specific serialization.
|
||||
*/
|
||||
async function getExecutableWidgetValue(
|
||||
node: ExecutableLGraphNode,
|
||||
widget: IBaseWidget,
|
||||
index: number
|
||||
): Promise<unknown> {
|
||||
const override = resolvePromotedWidgetOverride(node, widget)
|
||||
|
||||
if (!override.hit) {
|
||||
return widget.serializeValue
|
||||
? await widget.serializeValue(node, index)
|
||||
: widget.value
|
||||
}
|
||||
|
||||
if (!widget.serializeValue) return override.value
|
||||
|
||||
const widgetProxy = Object.create(widget) as IBaseWidget
|
||||
Object.defineProperty(widgetProxy, 'value', {
|
||||
get: () => override.value,
|
||||
set: () => {},
|
||||
enumerable: true,
|
||||
configurable: true
|
||||
})
|
||||
return await widget.serializeValue.call(widgetProxy, node, index)
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts the current graph workflow for sending to the API.
|
||||
* @note Node widgets are updated before serialization to prepare queueing.
|
||||
@@ -99,9 +181,7 @@ export const graphToPrompt = async (
|
||||
for (const [i, widget] of widgets.entries()) {
|
||||
if (!widget.name || widget.options?.serialize === false) continue
|
||||
|
||||
const widgetValue = widget.serializeValue
|
||||
? await widget.serializeValue(node, i)
|
||||
: widget.value
|
||||
const widgetValue = await getExecutableWidgetValue(node, widget, i)
|
||||
// By default, Array values are reserved to represent node connections.
|
||||
// We need to wrap the array as an object to avoid the misinterpretation
|
||||
// of the array as a node connection.
|
||||
|
||||
Reference in New Issue
Block a user