mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor: extract helpers from _removeDuplicateLinks and add integration tests (#10332)
This commit is contained in:
169
browser_tests/assets/links/duplicate_links_slot_drift.json
Normal file
169
browser_tests/assets/links/duplicate_links_slot_drift.json
Normal file
@@ -0,0 +1,169 @@
|
||||
{
|
||||
"id": "f1a2b3c4-d5e6-7890-abcd-ef1234567890",
|
||||
"revision": 0,
|
||||
"last_node_id": 2,
|
||||
"last_link_id": 0,
|
||||
"nodes": [
|
||||
{
|
||||
"id": 2,
|
||||
"type": "b2c3d4e5-f6a7-8901-bcde-f12345678901",
|
||||
"pos": [400, 300],
|
||||
"size": [400, 200],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [],
|
||||
"outputs": [],
|
||||
"properties": {},
|
||||
"widgets_values": []
|
||||
}
|
||||
],
|
||||
"links": [],
|
||||
"groups": [],
|
||||
"definitions": {
|
||||
"subgraphs": [
|
||||
{
|
||||
"id": "b2c3d4e5-f6a7-8901-bcde-f12345678901",
|
||||
"version": 1,
|
||||
"state": {
|
||||
"lastGroupId": 0,
|
||||
"lastNodeId": 120,
|
||||
"lastLinkId": 276,
|
||||
"lastRerouteId": 0
|
||||
},
|
||||
"revision": 0,
|
||||
"config": {},
|
||||
"name": "Slot Drift Duplicate Links",
|
||||
"inputNode": {
|
||||
"id": -10,
|
||||
"bounding": [0, 300, 120, 60]
|
||||
},
|
||||
"outputNode": {
|
||||
"id": -20,
|
||||
"bounding": [900, 300, 120, 60]
|
||||
},
|
||||
"inputs": [],
|
||||
"outputs": [],
|
||||
"widgets": [],
|
||||
"nodes": [
|
||||
{
|
||||
"id": 120,
|
||||
"type": "ComfySwitchNode",
|
||||
"title": "Switch (CFG)",
|
||||
"pos": [100, 100],
|
||||
"size": [200, 80],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [{ "name": "value", "type": "FLOAT", "link": null }],
|
||||
"outputs": [
|
||||
{
|
||||
"name": "FLOAT",
|
||||
"type": "FLOAT",
|
||||
"links": [257, 271, 276]
|
||||
}
|
||||
],
|
||||
"properties": { "Node name for S&R": "ComfySwitchNode" },
|
||||
"widgets_values": []
|
||||
},
|
||||
{
|
||||
"id": 85,
|
||||
"type": "KSamplerAdvanced",
|
||||
"pos": [400, 50],
|
||||
"size": [270, 262],
|
||||
"flags": {},
|
||||
"order": 1,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{ "name": "model", "type": "MODEL", "link": null },
|
||||
{ "name": "positive", "type": "CONDITIONING", "link": null },
|
||||
{ "name": "negative", "type": "CONDITIONING", "link": null },
|
||||
{ "name": "latent_image", "type": "LATENT", "link": null },
|
||||
{ "name": "steps", "type": "INT", "link": null },
|
||||
{ "name": "cfg", "type": "FLOAT", "link": 276 }
|
||||
],
|
||||
"outputs": [{ "name": "LATENT", "type": "LATENT", "links": [] }],
|
||||
"properties": { "Node name for S&R": "KSamplerAdvanced" },
|
||||
"widgets_values": [
|
||||
false,
|
||||
0,
|
||||
"randomize",
|
||||
20,
|
||||
8,
|
||||
"euler",
|
||||
"normal",
|
||||
0,
|
||||
10000,
|
||||
false
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": 86,
|
||||
"type": "KSamplerAdvanced",
|
||||
"pos": [400, 350],
|
||||
"size": [270, 262],
|
||||
"flags": {},
|
||||
"order": 2,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{ "name": "model", "type": "MODEL", "link": null },
|
||||
{ "name": "positive", "type": "CONDITIONING", "link": null },
|
||||
{ "name": "negative", "type": "CONDITIONING", "link": null },
|
||||
{ "name": "latent_image", "type": "LATENT", "link": null },
|
||||
{ "name": "steps", "type": "INT", "link": null },
|
||||
{ "name": "cfg", "type": "FLOAT", "link": 271 }
|
||||
],
|
||||
"outputs": [{ "name": "LATENT", "type": "LATENT", "links": [] }],
|
||||
"properties": { "Node name for S&R": "KSamplerAdvanced" },
|
||||
"widgets_values": [
|
||||
false,
|
||||
0,
|
||||
"randomize",
|
||||
20,
|
||||
8,
|
||||
"euler",
|
||||
"normal",
|
||||
0,
|
||||
10000,
|
||||
false
|
||||
]
|
||||
}
|
||||
],
|
||||
"groups": [],
|
||||
"links": [
|
||||
{
|
||||
"id": 257,
|
||||
"origin_id": 120,
|
||||
"origin_slot": 0,
|
||||
"target_id": 85,
|
||||
"target_slot": 5,
|
||||
"type": "FLOAT"
|
||||
},
|
||||
{
|
||||
"id": 271,
|
||||
"origin_id": 120,
|
||||
"origin_slot": 0,
|
||||
"target_id": 86,
|
||||
"target_slot": 5,
|
||||
"type": "FLOAT"
|
||||
},
|
||||
{
|
||||
"id": 276,
|
||||
"origin_id": 120,
|
||||
"origin_slot": 0,
|
||||
"target_id": 85,
|
||||
"target_slot": 5,
|
||||
"type": "FLOAT"
|
||||
}
|
||||
],
|
||||
"extra": {}
|
||||
}
|
||||
]
|
||||
},
|
||||
"config": {},
|
||||
"extra": {
|
||||
"ds": { "scale": 1, "offset": [0, 0] },
|
||||
"frontendVersion": "1.43.2"
|
||||
},
|
||||
"version": 0.4
|
||||
}
|
||||
@@ -23,4 +23,85 @@ test.describe('Graph', { tag: ['@smoke', '@canvas'] }, () => {
|
||||
await comfyPage.workflow.loadWorkflow('links/bad_link')
|
||||
await expect.poll(() => comfyPage.toast.getVisibleToastCount()).toBe(2)
|
||||
})
|
||||
|
||||
// Regression: duplicate links with shifted target_slot (widget-to-input
|
||||
// conversion) caused the wrong link to survive during deduplication.
|
||||
// Switch(CFG) node 120 connects to both KSamplerAdvanced 85 and 86 (2 links).
|
||||
// Links 257 and 276 shared the same tuple (origin=120 → target=85 slot=5).
|
||||
// Node 85's input.link was 276 (valid), but the bug kept 257 (stale) and
|
||||
// removed 276, breaking the cfg connection on KSamplerAdvanced 85.
|
||||
// Ref: https://github.com/Comfy-Org/ComfyUI_frontend/issues/10291
|
||||
test('Deduplicates links without breaking connections on slot-drift workflow', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('links/duplicate_links_slot_drift')
|
||||
|
||||
const result = await comfyPage.page.evaluate(() => {
|
||||
const graph = window.app!.graph!
|
||||
|
||||
const subgraph = graph.subgraphs.values().next().value
|
||||
if (!subgraph) return { error: 'No subgraph found' }
|
||||
|
||||
// Node 120 = Switch (CFG), connects to both KSamplerAdvanced 85 and 86
|
||||
const switchCfg = subgraph.getNodeById(120)
|
||||
const ksampler85 = subgraph.getNodeById(85)
|
||||
const ksampler86 = subgraph.getNodeById(86)
|
||||
if (!switchCfg || !ksampler85 || !ksampler86)
|
||||
return { error: 'Required nodes not found' }
|
||||
|
||||
// Find cfg inputs by name (slot indices shift due to widget-to-input)
|
||||
const cfgInput85 = ksampler85.inputs.find(
|
||||
(i: { name: string }) => i.name === 'cfg'
|
||||
)
|
||||
const cfgInput86 = ksampler86.inputs.find(
|
||||
(i: { name: string }) => i.name === 'cfg'
|
||||
)
|
||||
const cfg85Linked = cfgInput85?.link != null
|
||||
const cfg86Linked = cfgInput86?.link != null
|
||||
|
||||
// Verify the surviving links exist in the subgraph link map
|
||||
const cfg85LinkValid =
|
||||
cfg85Linked && subgraph.links.has(cfgInput85!.link!)
|
||||
const cfg86LinkValid =
|
||||
cfg86Linked && subgraph.links.has(cfgInput86!.link!)
|
||||
|
||||
// Switch(CFG) output should have exactly 2 links (one to each KSampler)
|
||||
const switchOutputLinkCount = switchCfg.outputs[0]?.links?.length ?? 0
|
||||
|
||||
// Count links from Switch(CFG) to node 85 cfg (should be 1, not 2)
|
||||
let cfgLinkToNode85Count = 0
|
||||
for (const link of subgraph.links.values()) {
|
||||
if (link.origin_id === 120 && link.target_id === 85)
|
||||
cfgLinkToNode85Count++
|
||||
}
|
||||
|
||||
return {
|
||||
cfg85Linked,
|
||||
cfg86Linked,
|
||||
cfg85LinkValid,
|
||||
cfg86LinkValid,
|
||||
cfg85LinkId: cfgInput85?.link ?? null,
|
||||
cfg86LinkId: cfgInput86?.link ?? null,
|
||||
switchOutputLinkIds: [...(switchCfg.outputs[0]?.links ?? [])],
|
||||
switchOutputLinkCount,
|
||||
cfgLinkToNode85Count
|
||||
}
|
||||
})
|
||||
|
||||
expect(result).not.toHaveProperty('error')
|
||||
// Both KSamplerAdvanced nodes must have their cfg input connected
|
||||
expect(result.cfg85Linked).toBe(true)
|
||||
expect(result.cfg86Linked).toBe(true)
|
||||
// Links must exist in the subgraph link map
|
||||
expect(result.cfg85LinkValid).toBe(true)
|
||||
expect(result.cfg86LinkValid).toBe(true)
|
||||
// Switch(CFG) output has exactly 2 links (one per KSamplerAdvanced)
|
||||
expect(result.switchOutputLinkCount).toBe(2)
|
||||
// Only 1 link from Switch(CFG) to node 85 (duplicate removed)
|
||||
expect(result.cfgLinkToNode85Count).toBe(1)
|
||||
// Output link IDs must match the input link IDs (source/target integrity)
|
||||
expect(result.switchOutputLinkIds).toEqual(
|
||||
expect.arrayContaining([result.cfg85LinkId, result.cfg86LinkId])
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -18,6 +18,11 @@ import {
|
||||
createTestSubgraphNode
|
||||
} from './subgraph/__fixtures__/subgraphHelpers'
|
||||
|
||||
import {
|
||||
duplicateLinksRoot,
|
||||
duplicateLinksSlotShift,
|
||||
duplicateLinksSubgraph
|
||||
} from './__fixtures__/duplicateLinks'
|
||||
import { duplicateSubgraphNodeIds } from './__fixtures__/duplicateSubgraphNodeIds'
|
||||
import { nestedSubgraphProxyWidgets } from './__fixtures__/nestedSubgraphProxyWidgets'
|
||||
import { nodeIdSpaceExhausted } from './__fixtures__/nodeIdSpaceExhausted'
|
||||
@@ -560,31 +565,39 @@ describe('_removeDuplicateLinks', () => {
|
||||
LiteGraph.registerNodeType('test/DupTestNode', TestNode)
|
||||
}
|
||||
|
||||
it('removes orphaned duplicate links from _links and output.links', () => {
|
||||
function createConnectedGraph() {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
expect(graph._links.size).toBe(1)
|
||||
return { graph, source, target }
|
||||
}
|
||||
|
||||
const existingLink = graph._links.values().next().value!
|
||||
for (let i = 0; i < 3; i++) {
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
existingLink.type,
|
||||
existingLink.origin_id,
|
||||
existingLink.origin_slot,
|
||||
existingLink.target_id,
|
||||
existingLink.target_slot
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
}
|
||||
function injectDuplicateLink(
|
||||
graph: LGraph,
|
||||
source: LGraphNode,
|
||||
target: LGraphNode
|
||||
) {
|
||||
const dup = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dup.id, dup)
|
||||
source.outputs[0].links!.push(dup.id)
|
||||
return dup
|
||||
}
|
||||
|
||||
it('removes orphaned duplicate links from _links and output.links', () => {
|
||||
const { graph, source, target } = createConnectedGraph()
|
||||
|
||||
for (let i = 0; i < 3; i++) injectDuplicateLink(graph, source, target)
|
||||
|
||||
expect(graph._links.size).toBe(4)
|
||||
expect(source.outputs[0].links).toHaveLength(4)
|
||||
@@ -597,27 +610,10 @@ describe('_removeDuplicateLinks', () => {
|
||||
})
|
||||
|
||||
it('keeps the link referenced by input.link', () => {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
const { graph, source, target } = createConnectedGraph()
|
||||
const keptLinkId = target.inputs[0].link!
|
||||
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
const dupLink = injectDuplicateLink(graph, source, target)
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
|
||||
@@ -628,18 +624,8 @@ describe('_removeDuplicateLinks', () => {
|
||||
})
|
||||
|
||||
it('keeps the valid link when input.link is at a shifted slot index', () => {
|
||||
LiteGraph.registerNodeType('test/DupTestNode', TestNode)
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
// Connect source:0 -> target:0, establishing input.link on target
|
||||
source.connect(0, target, 0)
|
||||
const { graph, source, target } = createConnectedGraph()
|
||||
const validLinkId = target.inputs[0].link!
|
||||
expect(graph._links.has(validLinkId)).toBe(true)
|
||||
|
||||
// Simulate widget-to-input conversion shifting the slot: insert a new
|
||||
// input BEFORE the connected one, moving it from index 0 to index 1.
|
||||
@@ -647,26 +633,13 @@ describe('_removeDuplicateLinks', () => {
|
||||
const connectedInput = target.inputs[0]
|
||||
target.inputs[0] = target.inputs[1]
|
||||
target.inputs[1] = connectedInput
|
||||
// Now target.inputs[1].link === validLinkId, but target.inputs[0].link is null
|
||||
|
||||
// Add a duplicate link with the same connection tuple (target_slot=0
|
||||
// in the LLink, matching the original slot before the shift).
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
const dupLink = injectDuplicateLink(graph, source, target)
|
||||
|
||||
expect(graph._links.size).toBe(2)
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
|
||||
// The valid link (referenced by an actual input) must survive
|
||||
expect(graph._links.size).toBe(1)
|
||||
expect(graph._links.has(validLinkId)).toBe(true)
|
||||
expect(graph._links.has(dupLink.id)).toBe(false)
|
||||
@@ -674,50 +647,22 @@ describe('_removeDuplicateLinks', () => {
|
||||
})
|
||||
|
||||
it('repairs input.link when it points to a removed duplicate', () => {
|
||||
LiteGraph.registerNodeType('test/DupTestNode', TestNode)
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
|
||||
// Create a duplicate link
|
||||
const dupLink = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
'number',
|
||||
source.id,
|
||||
0,
|
||||
target.id,
|
||||
0
|
||||
)
|
||||
graph._links.set(dupLink.id, dupLink)
|
||||
source.outputs[0].links!.push(dupLink.id)
|
||||
const { graph, source, target } = createConnectedGraph()
|
||||
|
||||
const dupLink = injectDuplicateLink(graph, source, target)
|
||||
// Point input.link to the duplicate (simulating corrupted state)
|
||||
target.inputs[0].link = dupLink.id
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
|
||||
expect(graph._links.size).toBe(1)
|
||||
// input.link must point to whichever link survived
|
||||
const survivingId = graph._links.keys().next().value!
|
||||
expect(target.inputs[0].link).toBe(survivingId)
|
||||
expect(graph._links.has(target.inputs[0].link!)).toBe(true)
|
||||
})
|
||||
|
||||
it('is a no-op when no duplicates exist', () => {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
|
||||
const source = LiteGraph.createNode('test/DupTestNode', 'Source')!
|
||||
const target = LiteGraph.createNode('test/DupTestNode', 'Target')!
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
const { graph } = createConnectedGraph()
|
||||
const linksBefore = graph._links.size
|
||||
|
||||
graph._removeDuplicateLinks()
|
||||
@@ -738,29 +683,56 @@ describe('_removeDuplicateLinks', () => {
|
||||
subgraph.add(target)
|
||||
|
||||
source.connect(0, target, 0)
|
||||
expect(subgraph._links.size).toBe(1)
|
||||
|
||||
const existingLink = subgraph._links.values().next().value!
|
||||
for (let i = 0; i < 3; i++) {
|
||||
const dup = new LLink(
|
||||
++subgraph.state.lastLinkId,
|
||||
existingLink.type,
|
||||
existingLink.origin_id,
|
||||
existingLink.origin_slot,
|
||||
existingLink.target_id,
|
||||
existingLink.target_slot
|
||||
)
|
||||
subgraph._links.set(dup.id, dup)
|
||||
source.outputs[0].links!.push(dup.id)
|
||||
}
|
||||
for (let i = 0; i < 3; i++) injectDuplicateLink(subgraph, source, target)
|
||||
expect(subgraph._links.size).toBe(4)
|
||||
|
||||
// Serialize and reconfigure - should clean up during configure
|
||||
const serialized = subgraph.asSerialisable()
|
||||
subgraph.configure(serialized as never)
|
||||
|
||||
expect(subgraph._links.size).toBe(1)
|
||||
})
|
||||
|
||||
it('removes duplicate links via root graph configure()', () => {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
graph.configure(duplicateLinksRoot)
|
||||
|
||||
expect(graph._links.size).toBe(1)
|
||||
const survivingLink = graph._links.values().next().value!
|
||||
const targetNode = graph.getNodeById(survivingLink.target_id)!
|
||||
expect(targetNode.inputs[0].link).toBe(survivingLink.id)
|
||||
const sourceNode = graph.getNodeById(survivingLink.origin_id)!
|
||||
expect(sourceNode.outputs[0].links).toEqual([survivingLink.id])
|
||||
})
|
||||
|
||||
it('preserves link integrity after configure() with slot-shifted duplicates', () => {
|
||||
registerTestNodes()
|
||||
const graph = new LGraph()
|
||||
graph.configure(duplicateLinksSlotShift)
|
||||
|
||||
expect(graph._links.size).toBe(1)
|
||||
|
||||
const link = graph._links.values().next().value!
|
||||
const target = graph.getNodeById(link.target_id)!
|
||||
const linkedInput = target.inputs.find((inp) => inp.link === link.id)
|
||||
expect(linkedInput).toBeDefined()
|
||||
|
||||
const source = graph.getNodeById(link.origin_id)!
|
||||
expect(source.outputs[link.origin_slot].links).toContain(link.id)
|
||||
})
|
||||
|
||||
it('deduplicates links inside subgraph definitions during root configure()', () => {
|
||||
const graph = new LGraph()
|
||||
graph.configure(duplicateLinksSubgraph)
|
||||
|
||||
const subgraph = graph.subgraphs.values().next().value!
|
||||
expect(subgraph._links.size).toBe(1)
|
||||
|
||||
const link = subgraph._links.values().next().value!
|
||||
const target = subgraph.getNodeById(link.target_id)!
|
||||
expect(target.inputs[0].link).toBe(link.id)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Subgraph Unpacking', () => {
|
||||
@@ -790,6 +762,21 @@ describe('Subgraph Unpacking', () => {
|
||||
return rootGraph.createSubgraph(createTestSubgraphData())
|
||||
}
|
||||
|
||||
function duplicateExistingLink(graph: LGraph, source: LGraphNode) {
|
||||
const existingLink = graph._links.values().next().value!
|
||||
const dup = new LLink(
|
||||
++graph.state.lastLinkId,
|
||||
existingLink.type,
|
||||
existingLink.origin_id,
|
||||
existingLink.origin_slot,
|
||||
existingLink.target_id,
|
||||
existingLink.target_slot
|
||||
)
|
||||
graph._links.set(dup.id, dup)
|
||||
source.outputs[0].links!.push(dup.id)
|
||||
return dup
|
||||
}
|
||||
|
||||
it('deduplicates links when unpacking subgraph with duplicate links', () => {
|
||||
registerTestNodes()
|
||||
const rootGraph = new LGraph()
|
||||
@@ -800,24 +787,9 @@ describe('Subgraph Unpacking', () => {
|
||||
subgraph.add(sourceNode)
|
||||
subgraph.add(targetNode)
|
||||
|
||||
// Create a legitimate link
|
||||
sourceNode.connect(0, targetNode, 0)
|
||||
expect(subgraph._links.size).toBe(1)
|
||||
|
||||
// Manually add duplicate links (simulating the bug)
|
||||
const existingLink = subgraph._links.values().next().value!
|
||||
for (let i = 0; i < 3; i++) {
|
||||
const dupLink = new LLink(
|
||||
++subgraph.state.lastLinkId,
|
||||
existingLink.type,
|
||||
existingLink.origin_id,
|
||||
existingLink.origin_slot,
|
||||
existingLink.target_id,
|
||||
existingLink.target_slot
|
||||
)
|
||||
subgraph._links.set(dupLink.id, dupLink)
|
||||
sourceNode.outputs[0].links!.push(dupLink.id)
|
||||
}
|
||||
for (let i = 0; i < 3; i++) duplicateExistingLink(subgraph, sourceNode)
|
||||
expect(subgraph._links.size).toBe(4)
|
||||
|
||||
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
|
||||
@@ -839,21 +811,8 @@ describe('Subgraph Unpacking', () => {
|
||||
subgraph.add(sourceNode)
|
||||
subgraph.add(targetNode)
|
||||
|
||||
// Connect source output 0 → target input 0
|
||||
sourceNode.connect(0, targetNode, 0)
|
||||
|
||||
// Add duplicate links to the same connection
|
||||
const existingLink = subgraph._links.values().next().value!
|
||||
const dupLink = new LLink(
|
||||
++subgraph.state.lastLinkId,
|
||||
existingLink.type,
|
||||
existingLink.origin_id,
|
||||
existingLink.origin_slot,
|
||||
existingLink.target_id,
|
||||
existingLink.target_slot
|
||||
)
|
||||
subgraph._links.set(dupLink.id, dupLink)
|
||||
sourceNode.outputs[0].links!.push(dupLink.id)
|
||||
duplicateExistingLink(subgraph, sourceNode)
|
||||
|
||||
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
|
||||
rootGraph.add(subgraphNode)
|
||||
|
||||
@@ -13,6 +13,13 @@ import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { forEachNode } from '@/utils/graphTraversalUtil'
|
||||
|
||||
import {
|
||||
groupLinksByTuple,
|
||||
purgeOrphanedLinks,
|
||||
repairInputLinks,
|
||||
selectSurvivorLink
|
||||
} from './linkDeduplication'
|
||||
|
||||
import type { DragAndScaleState } from './DragAndScale'
|
||||
import { LGraphCanvas } from './LGraphCanvas'
|
||||
import { LGraphGroup } from './LGraphGroup'
|
||||
@@ -168,11 +175,6 @@ export class LGraph
|
||||
static STATUS_STOPPED = 1
|
||||
static STATUS_RUNNING = 2
|
||||
|
||||
/** Generates a unique string key for a link's connection tuple. */
|
||||
static _linkTupleKey(link: LLink): string {
|
||||
return `${link.origin_id}\0${link.origin_slot}\0${link.target_id}\0${link.target_slot}`
|
||||
}
|
||||
|
||||
/** List of LGraph properties that are manually handled by {@link LGraph.configure}. */
|
||||
static readonly ConfigureProperties = new Set([
|
||||
'nodes',
|
||||
@@ -1626,68 +1628,21 @@ export class LGraph
|
||||
* (origin_id, origin_slot, target_id, target_slot). Keeps the link
|
||||
* referenced by input.link and removes orphaned duplicates from
|
||||
* output.links and the graph's _links map.
|
||||
*
|
||||
* Three phases: group links by tuple, select the survivor, purge duplicates.
|
||||
*/
|
||||
_removeDuplicateLinks(): void {
|
||||
// Group all link IDs by their connection tuple.
|
||||
const groups = new Map<string, LinkId[]>()
|
||||
for (const [id, link] of this._links) {
|
||||
const key = LGraph._linkTupleKey(link)
|
||||
let group = groups.get(key)
|
||||
if (!group) {
|
||||
group = []
|
||||
groups.set(key, group)
|
||||
}
|
||||
group.push(id)
|
||||
}
|
||||
const groups = groupLinksByTuple(this._links)
|
||||
|
||||
for (const [, ids] of groups) {
|
||||
for (const ids of groups.values()) {
|
||||
if (ids.length <= 1) continue
|
||||
|
||||
const sampleLink = this._links.get(ids[0])!
|
||||
const node = this.getNodeById(sampleLink.target_id)
|
||||
const keepId = selectSurvivorLink(ids, node)
|
||||
|
||||
// Find which link ID is actually referenced by any input on the target
|
||||
// node. Cannot rely on target_slot index because widget-to-input
|
||||
// conversions during configure() can shift slot indices.
|
||||
let keepId: LinkId | undefined
|
||||
if (node) {
|
||||
for (const input of node.inputs ?? []) {
|
||||
const match = ids.find((id) => input.link === id)
|
||||
if (match != null) {
|
||||
keepId = match
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
keepId ??= ids[0]
|
||||
|
||||
for (const id of ids) {
|
||||
if (id === keepId) continue
|
||||
|
||||
const link = this._links.get(id)
|
||||
if (!link) continue
|
||||
|
||||
// Remove from origin node's output.links array
|
||||
const originNode = this.getNodeById(link.origin_id)
|
||||
if (originNode) {
|
||||
const output = originNode.outputs?.[link.origin_slot]
|
||||
if (output?.links) {
|
||||
const idx = output.links.indexOf(id)
|
||||
if (idx !== -1) output.links.splice(idx, 1)
|
||||
}
|
||||
}
|
||||
|
||||
this._links.delete(id)
|
||||
}
|
||||
|
||||
// Ensure input.link points to the surviving link
|
||||
if (node) {
|
||||
for (const input of node.inputs ?? []) {
|
||||
if (ids.includes(input.link as LinkId) && input.link !== keepId) {
|
||||
input.link = keepId
|
||||
}
|
||||
}
|
||||
}
|
||||
purgeOrphanedLinks(ids, keepId, this._links, (id) => this.getNodeById(id))
|
||||
repairInputLinks(ids, keepId, node)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
240
src/lib/litegraph/src/__fixtures__/duplicateLinks.ts
Normal file
240
src/lib/litegraph/src/__fixtures__/duplicateLinks.ts
Normal file
@@ -0,0 +1,240 @@
|
||||
import type { SerialisableGraph } from '@/lib/litegraph/src/types/serialisation'
|
||||
|
||||
/**
|
||||
* Root graph with two nodes (Source, Target) connected by one valid link
|
||||
* plus two duplicate links sharing the same connection tuple.
|
||||
* Tests that configure() deduplicates to a single link.
|
||||
*/
|
||||
export const duplicateLinksRoot: SerialisableGraph = {
|
||||
id: 'dd000000-0000-4000-8000-000000000001',
|
||||
version: 1,
|
||||
revision: 0,
|
||||
state: {
|
||||
lastNodeId: 2,
|
||||
lastLinkId: 3,
|
||||
lastGroupId: 0,
|
||||
lastRerouteId: 0
|
||||
},
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'test/DupTestNode',
|
||||
pos: [0, 0],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 0,
|
||||
inputs: [{ name: 'input_0', type: 'number', link: null }],
|
||||
outputs: [{ name: 'output_0', type: 'number', links: [1, 2, 3] }],
|
||||
properties: {}
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
type: 'test/DupTestNode',
|
||||
pos: [300, 0],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 1,
|
||||
mode: 0,
|
||||
inputs: [{ name: 'input_0', type: 'number', link: 1 }],
|
||||
outputs: [{ name: 'output_0', type: 'number', links: [] }],
|
||||
properties: {}
|
||||
}
|
||||
],
|
||||
links: [
|
||||
{
|
||||
id: 1,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
},
|
||||
{
|
||||
id: 3,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
* Root graph with slot-shifted duplicates. Target node has an extra input
|
||||
* (simulating widget-to-input conversion) that shifts the connected input
|
||||
* from slot 0 to slot 1. Link 1 is valid (referenced by input.link),
|
||||
* link 2 is a duplicate with the original (pre-shift) target_slot.
|
||||
*/
|
||||
export const duplicateLinksSlotShift: SerialisableGraph = {
|
||||
id: 'dd000000-0000-4000-8000-000000000002',
|
||||
version: 1,
|
||||
revision: 0,
|
||||
state: {
|
||||
lastNodeId: 2,
|
||||
lastLinkId: 2,
|
||||
lastGroupId: 0,
|
||||
lastRerouteId: 0
|
||||
},
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'test/DupTestNode',
|
||||
pos: [0, 0],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 0,
|
||||
inputs: [{ name: 'input_0', type: 'number', link: null }],
|
||||
outputs: [{ name: 'output_0', type: 'number', links: [1, 2] }],
|
||||
properties: {}
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
type: 'test/DupTestNode',
|
||||
pos: [300, 0],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 1,
|
||||
mode: 0,
|
||||
inputs: [
|
||||
{ name: 'extra_widget', type: 'number', link: null },
|
||||
{ name: 'input_0', type: 'number', link: 1 }
|
||||
],
|
||||
outputs: [{ name: 'output_0', type: 'number', links: [] }],
|
||||
properties: {}
|
||||
}
|
||||
],
|
||||
links: [
|
||||
{
|
||||
id: 1,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
* Root graph containing a SubgraphNode whose subgraph definition has
|
||||
* duplicate links. Tests that configure() deduplicates links inside
|
||||
* subgraph definitions during root-level configure.
|
||||
*/
|
||||
export const duplicateLinksSubgraph: SerialisableGraph = {
|
||||
id: 'dd000000-0000-4000-8000-000000000003',
|
||||
version: 1,
|
||||
revision: 0,
|
||||
state: {
|
||||
lastNodeId: 1,
|
||||
lastLinkId: 0,
|
||||
lastGroupId: 0,
|
||||
lastRerouteId: 0
|
||||
},
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'dd111111-1111-4111-8111-111111111111',
|
||||
pos: [0, 0],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 0,
|
||||
properties: {}
|
||||
}
|
||||
],
|
||||
definitions: {
|
||||
subgraphs: [
|
||||
{
|
||||
id: 'dd111111-1111-4111-8111-111111111111',
|
||||
version: 1,
|
||||
revision: 0,
|
||||
state: {
|
||||
lastNodeId: 2,
|
||||
lastLinkId: 3,
|
||||
lastGroupId: 0,
|
||||
lastRerouteId: 0
|
||||
},
|
||||
name: 'Subgraph With Duplicates',
|
||||
config: {},
|
||||
inputNode: { id: -10, bounding: [0, 100, 120, 60] },
|
||||
outputNode: { id: -20, bounding: [500, 100, 120, 60] },
|
||||
inputs: [],
|
||||
outputs: [],
|
||||
widgets: [],
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'test/Source',
|
||||
pos: [100, 100],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 0,
|
||||
inputs: [],
|
||||
outputs: [{ name: 'out', type: 'number', links: [1, 2, 3] }],
|
||||
properties: {}
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
type: 'test/Target',
|
||||
pos: [400, 100],
|
||||
size: [200, 100],
|
||||
flags: {},
|
||||
order: 1,
|
||||
mode: 0,
|
||||
inputs: [{ name: 'in', type: 'number', link: 1 }],
|
||||
outputs: [],
|
||||
properties: {}
|
||||
}
|
||||
],
|
||||
groups: [],
|
||||
links: [
|
||||
{
|
||||
id: 1,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
},
|
||||
{
|
||||
id: 3,
|
||||
origin_id: 1,
|
||||
origin_slot: 0,
|
||||
target_id: 2,
|
||||
target_slot: 0,
|
||||
type: 'number'
|
||||
}
|
||||
],
|
||||
extra: {}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
82
src/lib/litegraph/src/linkDeduplication.ts
Normal file
82
src/lib/litegraph/src/linkDeduplication.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
import type { LGraphNode, NodeId } from './LGraphNode'
|
||||
import type { LLink, LinkId } from './LLink'
|
||||
|
||||
/** Generates a unique string key for a link's connection tuple. */
|
||||
function linkTupleKey(link: LLink): string {
|
||||
return `${link.origin_id}\0${link.origin_slot}\0${link.target_id}\0${link.target_slot}`
|
||||
}
|
||||
|
||||
/** Groups all link IDs by their connection tuple key. */
|
||||
export function groupLinksByTuple(
|
||||
links: Map<LinkId, LLink>
|
||||
): Map<string, LinkId[]> {
|
||||
const groups = new Map<string, LinkId[]>()
|
||||
for (const [id, link] of links) {
|
||||
const key = linkTupleKey(link)
|
||||
if (!groups.has(key)) groups.set(key, [])
|
||||
groups.get(key)!.push(id)
|
||||
}
|
||||
return groups
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds the link ID actually referenced by an input on the target node.
|
||||
* Cannot rely on target_slot index because widget-to-input conversions
|
||||
* during configure() can shift slot indices.
|
||||
*/
|
||||
export function selectSurvivorLink(
|
||||
ids: LinkId[],
|
||||
node: LGraphNode | null
|
||||
): LinkId {
|
||||
if (!node) return ids[0]
|
||||
|
||||
for (const input of node.inputs ?? []) {
|
||||
if (!input) continue
|
||||
const match = ids.find((id) => input.link === id)
|
||||
if (match != null) return match
|
||||
}
|
||||
return ids[0]
|
||||
}
|
||||
|
||||
/** Removes duplicate links from origin outputs and the graph's link map. */
|
||||
export function purgeOrphanedLinks(
|
||||
ids: LinkId[],
|
||||
keepId: LinkId,
|
||||
links: Map<LinkId, LLink>,
|
||||
getNodeById: (id: NodeId) => LGraphNode | null
|
||||
): void {
|
||||
for (const id of ids) {
|
||||
if (id === keepId) continue
|
||||
|
||||
const link = links.get(id)
|
||||
if (!link) continue
|
||||
|
||||
const originNode = getNodeById(link.origin_id)
|
||||
const output = originNode?.outputs?.[link.origin_slot]
|
||||
if (output?.links) {
|
||||
for (let i = output.links.length - 1; i >= 0; i--) {
|
||||
if (output.links[i] === id) output.links.splice(i, 1)
|
||||
}
|
||||
}
|
||||
|
||||
links.delete(id)
|
||||
}
|
||||
}
|
||||
|
||||
/** Ensures input.link on the target node points to the surviving link. */
|
||||
export function repairInputLinks(
|
||||
ids: LinkId[],
|
||||
keepId: LinkId,
|
||||
node: LGraphNode | null
|
||||
): void {
|
||||
if (!node) return
|
||||
|
||||
const duplicateIds = new Set(ids)
|
||||
|
||||
for (const input of node.inputs ?? []) {
|
||||
if (input?.link == null || input.link === keepId) continue
|
||||
if (duplicateIds.has(input.link)) {
|
||||
input.link = keepId
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user