mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
feat: normalize subgraph slot identity by canonical name
Amp-Thread-ID: https://ampcode.com/threads/T-019c98b1-8cb2-704f-b7ac-7accee5228d0 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -2879,6 +2879,7 @@ export class Subgraph
|
||||
implements BaseLGraph, Serialisable<ExportedSubgraph>
|
||||
{
|
||||
override readonly events = new CustomEventTarget<SubgraphEventMap>()
|
||||
private static duplicateIdentitySeparator = '__'
|
||||
|
||||
/** Limits the number of levels / depth that subgraphs may be nested. Prevents uncontrolled programmatic nesting. */
|
||||
static MAX_NESTED_SUBGRAPHS = 1000
|
||||
@@ -2946,8 +2947,11 @@ export class Subgraph
|
||||
for (const input of inputs) {
|
||||
const subgraphInput = new SubgraphInput(input, this.inputNode)
|
||||
this.inputs.push(subgraphInput)
|
||||
this.events.dispatch('input-added', { input: subgraphInput })
|
||||
}
|
||||
|
||||
this._normalizeLegacySlotIdentity(this.inputs)
|
||||
for (const subgraphInput of this.inputs)
|
||||
this.events.dispatch('input-added', { input: subgraphInput })
|
||||
}
|
||||
|
||||
if (outputs) {
|
||||
@@ -2955,6 +2959,8 @@ export class Subgraph
|
||||
for (const output of outputs) {
|
||||
this.outputs.push(new SubgraphOutput(output, this.outputNode))
|
||||
}
|
||||
|
||||
this._normalizeLegacySlotIdentity(this.outputs)
|
||||
}
|
||||
|
||||
if (widgets) {
|
||||
@@ -2993,10 +2999,14 @@ export class Subgraph
|
||||
|
||||
this.events.dispatch('adding-input', { name, type })
|
||||
|
||||
const id = createUuidv4()
|
||||
const canonicalName = this._resolveCanonicalSlotName(this.inputs, name, id)
|
||||
|
||||
const input = new SubgraphInput(
|
||||
{
|
||||
id: createUuidv4(),
|
||||
name,
|
||||
id,
|
||||
name: canonicalName,
|
||||
label: canonicalName === name ? undefined : name,
|
||||
type
|
||||
},
|
||||
this.inputNode
|
||||
@@ -3015,10 +3025,14 @@ export class Subgraph
|
||||
|
||||
this.events.dispatch('adding-output', { name, type })
|
||||
|
||||
const id = createUuidv4()
|
||||
const canonicalName = this._resolveCanonicalSlotName(this.outputs, name, id)
|
||||
|
||||
const output = new SubgraphOutput(
|
||||
{
|
||||
id: createUuidv4(),
|
||||
name,
|
||||
id,
|
||||
name: canonicalName,
|
||||
label: canonicalName === name ? undefined : name,
|
||||
type
|
||||
},
|
||||
this.outputNode
|
||||
@@ -3040,13 +3054,20 @@ export class Subgraph
|
||||
if (index === -1) throw new Error('Input not found')
|
||||
|
||||
const oldName = input.displayName
|
||||
const canonicalName = this._resolveCanonicalSlotName(
|
||||
this.inputs,
|
||||
name,
|
||||
input.id
|
||||
)
|
||||
this.events.dispatch('renaming-input', {
|
||||
input,
|
||||
index,
|
||||
oldName,
|
||||
newName: name
|
||||
newName: name,
|
||||
canonicalName
|
||||
})
|
||||
|
||||
input.name = canonicalName
|
||||
input.label = name
|
||||
}
|
||||
|
||||
@@ -3060,16 +3081,51 @@ export class Subgraph
|
||||
if (index === -1) throw new Error('Output not found')
|
||||
|
||||
const oldName = output.displayName
|
||||
const canonicalName = this._resolveCanonicalSlotName(
|
||||
this.outputs,
|
||||
name,
|
||||
output.id
|
||||
)
|
||||
this.events.dispatch('renaming-output', {
|
||||
output,
|
||||
index,
|
||||
oldName,
|
||||
newName: name
|
||||
newName: name,
|
||||
canonicalName
|
||||
})
|
||||
|
||||
output.name = canonicalName
|
||||
output.label = name
|
||||
}
|
||||
|
||||
private _resolveCanonicalSlotName<TSlot extends { id: UUID; name: string }>(
|
||||
slots: readonly TSlot[],
|
||||
requestedName: string,
|
||||
slotId: UUID
|
||||
): string {
|
||||
if (
|
||||
!slots.some((slot) => slot.id !== slotId && slot.name === requestedName)
|
||||
)
|
||||
return requestedName
|
||||
|
||||
return `${requestedName}${Subgraph.duplicateIdentitySeparator}${slotId}`
|
||||
}
|
||||
|
||||
private _normalizeLegacySlotIdentity<
|
||||
TSlot extends { id: UUID; name: string; label?: string }
|
||||
>(slots: TSlot[]): void {
|
||||
const seenCounts = new Map<string, number>()
|
||||
|
||||
for (const slot of slots) {
|
||||
const count = seenCounts.get(slot.name) ?? 0
|
||||
seenCounts.set(slot.name, count + 1)
|
||||
if (count === 0) continue
|
||||
|
||||
slot.label ??= slot.name
|
||||
slot.name = `${slot.name}${Subgraph.duplicateIdentitySeparator}${slot.id}`
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes an input slot from the subgraph.
|
||||
* @param input The input slot to remove.
|
||||
|
||||
@@ -36,12 +36,14 @@ export interface SubgraphEventMap extends LGraphEventMap {
|
||||
index: number
|
||||
oldName: string
|
||||
newName: string
|
||||
canonicalName?: string
|
||||
}
|
||||
'renaming-output': {
|
||||
output: SubgraphOutput
|
||||
index: number
|
||||
oldName: string
|
||||
newName: string
|
||||
canonicalName?: string
|
||||
}
|
||||
|
||||
'widget-promoted': {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// TODO: Fix these tests after migration
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { ToInputFromIoNodeLink } from '@/lib/litegraph/src/canvas/ToInputFromIoNodeLink'
|
||||
import {
|
||||
SUBGRAPH_INPUT_ID,
|
||||
@@ -9,13 +9,98 @@ import {
|
||||
} from '@/lib/litegraph/src/constants'
|
||||
import { LinkDirection } from '@/lib/litegraph/src//types/globalEnums'
|
||||
import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'
|
||||
import { createUuidv4 } from '@/lib/litegraph/src/utils/uuid'
|
||||
|
||||
import { subgraphTest } from './__fixtures__/subgraphFixtures'
|
||||
import {
|
||||
createTestSubgraphData,
|
||||
createTestSubgraph,
|
||||
createTestSubgraphNode
|
||||
} from './__fixtures__/subgraphHelpers'
|
||||
|
||||
describe('SubgraphIO - Slot Identity Normalization', () => {
|
||||
subgraphTest(
|
||||
'adds duplicate input/output names with stable canonical names while preserving display labels',
|
||||
({ simpleSubgraph }) => {
|
||||
const inputA = simpleSubgraph.addInput('duplicate', 'number')
|
||||
const inputB = simpleSubgraph.addInput('duplicate', 'string')
|
||||
const outputA = simpleSubgraph.addOutput('duplicate', 'number')
|
||||
const outputB = simpleSubgraph.addOutput('duplicate', 'string')
|
||||
|
||||
expect(inputA.name).toBe('duplicate')
|
||||
expect(inputA.displayName).toBe('duplicate')
|
||||
expect(inputB.name).toBe(`duplicate__${inputB.id}`)
|
||||
expect(inputB.label).toBe('duplicate')
|
||||
expect(inputB.displayName).toBe('duplicate')
|
||||
|
||||
expect(outputA.name).toBe('duplicate')
|
||||
expect(outputA.displayName).toBe('duplicate')
|
||||
expect(outputB.name).toBe(`duplicate__${outputB.id}`)
|
||||
expect(outputB.label).toBe('duplicate')
|
||||
expect(outputB.displayName).toBe('duplicate')
|
||||
}
|
||||
)
|
||||
|
||||
subgraphTest(
|
||||
'renaming to an existing slot name preserves display labels while assigning stable canonical identities',
|
||||
({ simpleSubgraph }) => {
|
||||
const inputA = simpleSubgraph.addInput('source', 'number')
|
||||
const inputB = simpleSubgraph.addInput('target', 'number')
|
||||
const outputA = simpleSubgraph.addOutput('source', 'number')
|
||||
const outputB = simpleSubgraph.addOutput('target', 'number')
|
||||
|
||||
simpleSubgraph.renameInput(inputB, 'source')
|
||||
simpleSubgraph.renameOutput(outputB, 'source')
|
||||
|
||||
expect(inputA.name).toBe('source')
|
||||
expect(inputB.name).toBe(`source__${inputB.id}`)
|
||||
expect(inputB.displayName).toBe('source')
|
||||
|
||||
expect(outputA.name).toBe('source')
|
||||
expect(outputB.name).toBe(`source__${outputB.id}`)
|
||||
expect(outputB.displayName).toBe('source')
|
||||
}
|
||||
)
|
||||
|
||||
it('normalizes legacy duplicate slot names into stable canonical identities on configure', () => {
|
||||
const rootGraph = new LGraph()
|
||||
const inputIdA = createUuidv4()
|
||||
const inputIdB = createUuidv4()
|
||||
const outputIdA = createUuidv4()
|
||||
const outputIdB = createUuidv4()
|
||||
|
||||
const subgraph = rootGraph.createSubgraph(
|
||||
createTestSubgraphData({
|
||||
inputs: [
|
||||
{ id: inputIdA, name: 'legacy', type: 'number' },
|
||||
{ id: inputIdB, name: 'legacy', type: 'number' }
|
||||
],
|
||||
outputs: [
|
||||
{ id: outputIdA, name: 'legacy', type: 'number' },
|
||||
{ id: outputIdB, name: 'legacy', type: 'number' }
|
||||
]
|
||||
})
|
||||
)
|
||||
|
||||
expect(subgraph.inputs.map((slot) => slot.name)).toEqual([
|
||||
'legacy',
|
||||
`legacy__${inputIdB}`
|
||||
])
|
||||
expect(subgraph.outputs.map((slot) => slot.name)).toEqual([
|
||||
'legacy',
|
||||
`legacy__${outputIdB}`
|
||||
])
|
||||
expect(subgraph.inputs.map((slot) => slot.displayName)).toEqual([
|
||||
'legacy',
|
||||
'legacy'
|
||||
])
|
||||
expect(subgraph.outputs.map((slot) => slot.displayName)).toEqual([
|
||||
'legacy',
|
||||
'legacy'
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
|
||||
subgraphTest(
|
||||
'connect callback payload keeps current subgraph-input asymmetry',
|
||||
|
||||
@@ -211,11 +211,13 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
subgraphEvents.addEventListener(
|
||||
'renaming-input',
|
||||
(e) => {
|
||||
const { index, newName } = e.detail
|
||||
const { index, newName, canonicalName } = e.detail
|
||||
const input = this.inputs.at(index)
|
||||
if (!input) throw new Error('Subgraph input not found')
|
||||
|
||||
input.name = canonicalName ?? newName
|
||||
input.label = newName
|
||||
if (input.widget) input.widget.name = input.name
|
||||
if (input._widget) {
|
||||
input._widget.label = newName
|
||||
}
|
||||
@@ -226,10 +228,11 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
subgraphEvents.addEventListener(
|
||||
'renaming-output',
|
||||
(e) => {
|
||||
const { index, newName } = e.detail
|
||||
const { index, newName, canonicalName } = e.detail
|
||||
const output = this.outputs.at(index)
|
||||
if (!output) throw new Error('Subgraph output not found')
|
||||
|
||||
output.name = canonicalName ?? newName
|
||||
output.label = newName
|
||||
},
|
||||
{ signal }
|
||||
|
||||
Reference in New Issue
Block a user