Skip to content

Commit 7e0d7a8

Browse files
committed
fix: address ECS expected test failures
Amp-Thread-ID: https://ampcode.com/threads/T-01a02889-c911-708b-bf1a-11dfb0904fff
1 parent d874c0b commit 7e0d7a8

8 files changed

Lines changed: 113 additions & 36 deletions

src/lib/litegraph/src/LGraph.inputSlotRealign.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ describe('normalizeConfiguredTopology', () => {
324324
expect(normalized.links?.map((link) => link.id)).toEqual([2, 3])
325325
expect(normalized.nodes?.[1].inputs?.[0].link).toBe(2)
326326
expect(console.warn).toHaveBeenCalledWith(
327-
'Dropping competing link to an occupied input',
327+
'Dropping competing link to occupied input 2:0',
328328
expect.objectContaining({ droppedLinkId: 2, survivorLinkId: 1 })
329329
)
330330
})
@@ -541,7 +541,7 @@ describe('LGraph.configure realignment with an unmatched input name (#15581)', (
541541
)
542542
})
543543

544-
it.fails('realigns siblings when configure drops an input', () => {
544+
it('realigns siblings when configure drops an input', () => {
545545
const graph = new LGraph()
546546
graph.configure(unmatchedInputNameWorkflow('test/DroppedInputTarget'))
547547

@@ -554,7 +554,7 @@ describe('LGraph.configure realignment with an unmatched input name (#15581)', (
554554
})
555555
})
556556

557-
it.fails('realigns siblings when configure renames an input', () => {
557+
it('realigns siblings when configure renames an input', () => {
558558
const graph = new LGraph()
559559
graph.configure(unmatchedInputNameWorkflow('test/RenamedInputTarget'))
560560

@@ -567,7 +567,7 @@ describe('LGraph.configure realignment with an unmatched input name (#15581)', (
567567
})
568568
})
569569

570-
it.fails('reports no error while realigning around an unmatched name', () => {
570+
it('reports no error while realigning around an unmatched name', () => {
571571
const error = vi.spyOn(console, 'error').mockImplementation(() => {})
572572

573573
const graph = new LGraph()
@@ -582,7 +582,7 @@ describe('realignInputLinkSlots with a rejected batch (#15581)', () => {
582582
setActivePinia(createTestingPinia({ stubActions: false }))
583583
})
584584

585-
it.fails('lands the non-conflicting moves when one move is blocked', () => {
585+
it('lands the non-conflicting moves when one move is blocked', () => {
586586
const graph = new LGraph()
587587
const source = new LGraphNode('Source')
588588
source.addOutput('out', 'number')

src/lib/litegraph/src/LGraph.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2938,6 +2938,7 @@ export class LGraph
29382938

29392939
let error = false
29402940
const nodeDataMap = new Map<SerializedNodeId, ISerialisedNode>()
2941+
const realignmentDataMap = new Map<SerializedNodeId, ISerialisedNode>()
29412942

29422943
// create nodes
29432944
this._nodes = []
@@ -2962,6 +2963,10 @@ export class LGraph
29622963
// add before configure, otherwise configure cannot create links
29632964
this.add(node, true)
29642965
nodeDataMap.set(node.id, n_info)
2966+
realignmentDataMap.set(node.id, {
2967+
...n_info,
2968+
inputs: n_info.inputs?.map((input) => ({ ...input }))
2969+
})
29652970
}
29662971

29672972
// configure nodes afterwards so they can reach each other
@@ -3001,11 +3006,7 @@ export class LGraph
30013006
}
30023007
}
30033008

3004-
// Node configure() overrides may have reordered serialized inputs in
3005-
// place to match current node definitions; re-key links to the slots
3006-
// that reference them. Uses nodeDataMap: the effective normalized data
3007-
// nodes were actually configured from.
3008-
realignInputLinkSlots(this, nodeDataMap.values())
3009+
realignInputLinkSlots(this, realignmentDataMap.values())
30093010

30103011
// groups
30113012
this._groups.length = 0

src/lib/litegraph/src/LLink.spreadCopy.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('plain-object copies of LLink (uncovered)', () => {
5050
setActivePinia(createTestingPinia({ stubActions: false }))
5151
})
5252

53-
it.fails('carries topology onto a spread copy of a link', () => {
53+
it('carries topology onto a spread copy of a link', () => {
5454
const { graph, link } = connectedPair(toRerouteId(7))
5555
const copy: Partial<LLink> = { ...graph.links[link.id] }
5656

@@ -63,7 +63,7 @@ describe('plain-object copies of LLink (uncovered)', () => {
6363
expect(copy.parentId).toBe(link.parentId)
6464
})
6565

66-
it.fails('rewires Custom-Scripts consumers from copied links (#15594)', () => {
66+
it('rewires Custom-Scripts consumers from copied links (#15594)', () => {
6767
const { graph, source, consumers, inserted } = insertionScenario(2)
6868
const saved: Partial<LLink>[] = source.outputs[1].links!.map((id) => ({
6969
...graph.links[id]

src/lib/litegraph/src/LLink.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ export function resolveLinkTopology(topology: LinkTopology): LLink | undefined {
5252
return linkByTopology.get(toRaw(topology))
5353
}
5454

55+
function defineEnumerableTopologyFacade(link: LLink): void {
56+
const descriptors = Object.getOwnPropertyDescriptors(LLink.prototype)
57+
Object.defineProperties(link, {
58+
id: { ...descriptors.id, enumerable: true },
59+
type: { ...descriptors.type, enumerable: true },
60+
origin_id: { ...descriptors.origin_id, enumerable: true },
61+
origin_slot: { ...descriptors.origin_slot, enumerable: true },
62+
target_id: { ...descriptors.target_id, enumerable: true },
63+
target_slot: { ...descriptors.target_slot, enumerable: true },
64+
parentId: { ...descriptors.parentId, enumerable: true }
65+
})
66+
}
67+
5568
// Resolved connection union; eliminates subgraph in/out as a possibility
5669
export type ResolvedConnection = BaseResolvedConnection &
5770
(
@@ -318,6 +331,7 @@ export class LLink implements LinkSegment, Serialisable<SerialisableLLink> {
318331
targetSlot: target_slot,
319332
parentId
320333
}
334+
defineEnumerableTopologyFacade(this)
321335
}
322336

323337
/** @deprecated Use {@link LLink.create} */

src/lib/litegraph/src/linkDeduplication.conflictingOrigins.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,15 @@ describe('normalizeConfiguredTopology with conflicting origins (#15577)', () =>
7171
LiteGraph.registerNodeType('test/DupTestNode', DupTestNode)
7272
})
7373

74-
it.fails('keeps the link that input.link references', () => {
74+
it('keeps the link that input.link references', () => {
7575
const graph = configureConflictingOrigins()
7676

7777
expect(graph.getNodeById(toNodeId(3))?.getInputLink(0)?.origin_id).toBe(
7878
toNodeId(2)
7979
)
8080
})
8181

82-
it.fails('warns when a link is dropped in favour of a different origin', () => {
82+
it('warns when a link is dropped in favour of a different origin', () => {
8383
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
8484

8585
configureConflictingOrigins()
@@ -97,7 +97,7 @@ describe('normalizeConfiguredTopology with conflicting origins (#15577)', () =>
9797
expect(graph.getNodeById(toNodeId(3))?.getInputLink(0)).toBeDefined()
9898
})
9999

100-
it.fails('re-saves the workflow without changing the upstream node', () => {
100+
it('re-saves the workflow without changing the upstream node', () => {
101101
const graph = configureConflictingOrigins()
102102

103103
const [survivor] = linksIntoTargetSlot(

src/lib/litegraph/src/linkDeduplication.ts

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,15 @@ export function normalizeConfiguredTopology<T extends ConfiguredGraph>(
8383
toNodeId(survivor.origin_id) === toNodeId(fields.origin_id) &&
8484
survivor.origin_slot === fields.origin_slot
8585
if (!isExactDuplicate) {
86-
console.warn('Dropping competing link to an occupied input', {
87-
droppedLinkId: fields.id,
88-
survivorLinkId: survivor.id,
89-
targetNodeId: fields.target_id,
90-
targetSlot: fields.target_slot
91-
})
86+
console.warn(
87+
`Dropping competing link to occupied input ${fields.target_id}:${fields.target_slot}`,
88+
{
89+
droppedLinkId: fields.id,
90+
survivorLinkId: survivor.id,
91+
targetNodeId: fields.target_id,
92+
targetSlot: fields.target_slot
93+
}
94+
)
9295
}
9396

9497
if (
@@ -159,7 +162,12 @@ export function realignInputLinkSlots(
159162
referencedNames.set(link, names)
160163
}
161164

162-
for (let pass = 0; pass < referencedNames.size; pass++) {
165+
const removals = [...referencedNames].flatMap(([link, names]) =>
166+
node.inputs.some((input) => names.includes(input.name)) ? [] : [link]
167+
)
168+
for (const link of removals) referencedNames.delete(link)
169+
170+
for (let pass = 0; pass < Math.max(1, referencedNames.size); pass++) {
163171
const moved: { link: LLink; slot: number }[] = []
164172
for (const [link, names] of referencedNames) {
165173
const slots = node.inputs.flatMap((input, slot) =>
@@ -171,17 +179,25 @@ export function realignInputLinkSlots(
171179
: slots[0]
172180
if (link.target_slot !== slot) moved.push({ link, slot })
173181
}
174-
if (!moved.length) break
182+
if (!moved.length && !removals.length) break
175183

176184
const updates: EndpointUpdate[] = moved.map(({ link, slot }) => ({
177185
topology: link._state,
178186
patch: { targetSlot: slot }
179187
}))
180188
const result = useLinkStore().updateEndpoints(
181189
graphScopeOf(graph),
182-
updates
190+
updates,
191+
removals.map((link) => link._state)
183192
)
184193
if (!result.ok) {
194+
if (removals.length) {
195+
useLinkStore().updateEndpoints(
196+
graphScopeOf(graph),
197+
[],
198+
removals.map((link) => link._state)
199+
)
200+
}
185201
for (const { link, slot } of moved) {
186202
const fallback = useLinkStore().updateEndpoint(
187203
graphScopeOf(graph),
@@ -202,6 +218,7 @@ export function realignInputLinkSlots(
202218
}
203219
break
204220
}
221+
removals.length = 0
205222
for (const { link, slot } of moved) {
206223
node.onConnectionsChange?.(
207224
NodeSlotType.INPUT,

src/lib/litegraph/src/subgraph/SubgraphDuplicateDeleteOrder.test.ts

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { beforeEach, describe, expect, it, vi } from 'vitest'
1+
import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'
22

33
import type {
44
ExportedSubgraphInstance,
@@ -12,7 +12,10 @@ import {
1212
LGraphNode as LGraphNodeClass,
1313
LiteGraph
1414
} from '@/lib/litegraph/src/litegraph'
15+
import { createTestNode } from '@/lib/litegraph/src/__fixtures__/nodeHelpers'
1516
import { useWidgetValueStore } from '@/stores/widgetValueStore'
17+
import { UNASSIGNED_NODE_ID } from '@/types/nodeId'
18+
import { widgetId } from '@/types/widgetId'
1619

1720
import {
1821
createTestRootGraph,
@@ -30,6 +33,7 @@ vi.mock('@/services/litegraphService', () => ({
3033

3134
const PROMOTED_INPUT = 'value'
3235
const EXTERNAL_INPUT = 'signal'
36+
const CONVERTIBLE_NODE_TYPE = 'test/convertible-promoted-widget'
3337

3438
function addInteriorNodes(definition: Subgraph) {
3539
const withWidget = new LGraphNodeClass('Interior')
@@ -91,14 +95,30 @@ function promotedId(node: SubgraphNode) {
9195
}
9296

9397
function convertPromotedWidgetNode(rootGraph: LGraph): SubgraphNode {
94-
const producer = new LGraphNodeClass('Producer')
95-
producer.addOutput('out', 'number')
96-
rootGraph.add(producer)
98+
const producer = createTestNode(rootGraph, [], ['number'])
99+
100+
if (!LiteGraph.registered_node_types[CONVERTIBLE_NODE_TYPE]) {
101+
class ConvertibleNode extends LGraphNodeClass {
102+
constructor() {
103+
super('Convertible')
104+
const input = this.addInput(PROMOTED_INPUT, 'number')
105+
input.widget = { name: PROMOTED_INPUT }
106+
this.addWidget('number', PROMOTED_INPUT, 0, () => {})
107+
}
108+
}
109+
LiteGraph.registered_node_types[CONVERTIBLE_NODE_TYPE] = ConvertibleNode
110+
onTestFinished(() => {
111+
if (
112+
LiteGraph.registered_node_types[CONVERTIBLE_NODE_TYPE] ===
113+
ConvertibleNode
114+
) {
115+
delete LiteGraph.registered_node_types[CONVERTIBLE_NODE_TYPE]
116+
}
117+
})
118+
}
97119

98-
const node = new LGraphNodeClass('Convertible')
99-
const input = node.addInput(PROMOTED_INPUT, 'number')
100-
input.widget = { name: PROMOTED_INPUT }
101-
node.addWidget('number', PROMOTED_INPUT, 0, () => {})
120+
const node = LiteGraph.createNode(CONVERTIBLE_NODE_TYPE)
121+
if (!node) throw new Error('expected a convertible node')
102122
rootGraph.add(node)
103123

104124
if (!producer.connect(0, node, 0)) throw new Error('expected an input link')
@@ -199,7 +219,7 @@ describe('duplicated subgraph deleted in both orders (I4)', () => {
199219
expectSurvivorUndamaged(buildScenario(), 1)
200220
})
201221

202-
it.fails('releases promoted widget state when an instance is removed', () => {
222+
it('releases promoted widget state when an instance is removed', () => {
203223
const scenario = buildScenario()
204224
const removed = scenario.instances[0]
205225
const removedWidgetId = promotedId(removed)
@@ -211,14 +231,19 @@ describe('duplicated subgraph deleted in both orders (I4)', () => {
211231
).toBeUndefined()
212232
})
213233

214-
it.fails('gives converted subgraphs independent promoted widgets (#15565)', () => {
234+
it('gives converted subgraphs independent promoted widgets (#15565)', () => {
215235
const rootGraph = createTestRootGraph()
216236
registerTestSubgraphNodeTypes(rootGraph)
217237
const first = convertPromotedWidgetNode(rootGraph)
218238
const second = convertPromotedWidgetNode(rootGraph)
219239

220240
expect(first.id).not.toBe(second.id)
221241
expect(promotedId(first)).not.toBe(promotedId(second))
242+
expect(
243+
useWidgetValueStore().getWidget(
244+
widgetId(rootGraph.id, UNASSIGNED_NODE_ID, PROMOTED_INPUT)
245+
)
246+
).toBeUndefined()
222247

223248
const id = promotedId(first)
224249
if (!id) throw new Error('expected a promoted widget id')
@@ -228,7 +253,7 @@ describe('duplicated subgraph deleted in both orders (I4)', () => {
228253
expect(promotedValueOf(second)).toBe(before)
229254
})
230255

231-
it.fails('keeps the shared definition only while a nested instance references it', () => {
256+
it('keeps the shared definition only while a nested instance references it', () => {
232257
const { rootGraph, definition, instances } = buildScenario()
233258
const outer = rootGraph.createSubgraph(
234259
createTestSubgraphData({ name: 'Outer' })

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,17 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
587587
rebuildInputWidgetBindings(): void {
588588
this.invalidatePromotedViews()
589589

590+
const store = useWidgetValueStore()
591+
const previousBindings = new Map(
592+
this.inputs.flatMap((input) => {
593+
if (!input.widgetId) return []
594+
const state = store.getWidget(input.widgetId)
595+
return state
596+
? [[input, { id: input.widgetId, value: state.value }]]
597+
: []
598+
})
599+
)
600+
590601
for (const input of this.inputs) {
591602
delete input.widget
592603
delete input.pos
@@ -595,6 +606,15 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
595606
const subgraphInput = input._subgraphSlot
596607
if (!subgraphInput) continue
597608
this._resolveInputWidget(subgraphInput, input)
609+
const previous = previousBindings.get(input)
610+
if (previous && input.widgetId) {
611+
store.setValue(input.widgetId, previous.value)
612+
}
613+
}
614+
615+
const activeIds = new Set(this.inputs.map((input) => input.widgetId))
616+
for (const { id } of previousBindings.values()) {
617+
if (!activeIds.has(id)) store.deleteWidget(id)
598618
}
599619

600620
this.invalidatePromotedViews()
@@ -722,7 +742,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
722742
}
723743

724744
override onAdded(_graph: LGraph): void {
725-
this.invalidatePromotedViews()
745+
this.rebuildInputWidgetBindings()
726746
}
727747

728748
/**

0 commit comments

Comments
 (0)