mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-11 10:30:10 +00:00
## Summary Add `ensureGlobalIdUniqueness` to reassign duplicate node IDs across subgraphs when loading workflows, gated behind an experimental setting. ## Changes - **What**: Shared `LGraphState` between root graph and subgraphs so ID counters are global. Added `ensureGlobalIdUniqueness()` method that detects and remaps colliding node IDs in subgraphs, preserving root graph IDs as canonical and patching link references. Gated behind `Comfy.Graph.DeduplicateSubgraphNodeIds` (experimental, default `false`). - **Dependencies**: None ## Review Focus - Shared state override on `Subgraph` (getter delegates to root, setter is no-op) — verify no existing code sets `subgraph.state` directly. - `Math.max` state merging in `configure()` prevents ID counter regression when loading subgraph definitions. - Feature flag wiring: static property on `LGraph`, synced from settings via `useLitegraphSettings`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8762-feat-deduplicate-subgraph-node-IDs-on-workflow-load-experimental-3036d73d36508184b6cee5876dc4d935) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com>
108 lines
3.2 KiB
TypeScript
108 lines
3.2 KiB
TypeScript
import { expect } from '@playwright/test'
|
|
|
|
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
|
|
|
test.describe('Subgraph duplicate ID remapping', { tag: ['@subgraph'] }, () => {
|
|
const WORKFLOW = 'subgraphs/subgraph-nested-duplicate-ids'
|
|
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting(
|
|
'Comfy.Graph.DeduplicateSubgraphNodeIds',
|
|
true
|
|
)
|
|
})
|
|
|
|
test('All node IDs are globally unique after loading', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow(WORKFLOW)
|
|
|
|
const result = await comfyPage.page.evaluate(() => {
|
|
const graph = window.app!.canvas.graph!
|
|
// TODO: Extract allGraphs accessor (root + subgraphs) into LGraph
|
|
// TODO: Extract allNodeIds accessor into LGraph
|
|
const allGraphs = [graph, ...graph.subgraphs.values()]
|
|
const allIds = allGraphs
|
|
.flatMap((g) => g._nodes)
|
|
.map((n) => n.id)
|
|
.filter((id): id is number => typeof id === 'number')
|
|
|
|
return { allIds, uniqueCount: new Set(allIds).size }
|
|
})
|
|
|
|
expect(result.uniqueCount).toBe(result.allIds.length)
|
|
expect(result.allIds.length).toBeGreaterThanOrEqual(10)
|
|
})
|
|
|
|
test('Root graph node IDs are preserved as canonical', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow(WORKFLOW)
|
|
|
|
const rootIds = await comfyPage.page.evaluate(() => {
|
|
const graph = window.app!.canvas.graph!
|
|
return graph._nodes
|
|
.map((n) => n.id)
|
|
.filter((id): id is number => typeof id === 'number')
|
|
.sort((a, b) => a - b)
|
|
})
|
|
|
|
expect(rootIds).toEqual([1, 2, 5])
|
|
})
|
|
|
|
test('All links reference valid nodes in their graph', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow(WORKFLOW)
|
|
|
|
const invalidLinks = await comfyPage.page.evaluate(() => {
|
|
const graph = window.app!.canvas.graph!
|
|
const labeledGraphs: [string, typeof graph][] = [
|
|
['root', graph],
|
|
...[...graph.subgraphs.entries()].map(
|
|
([id, sg]) => [`subgraph:${id}`, sg] as [string, typeof graph]
|
|
)
|
|
]
|
|
|
|
const isNonNegative = (id: number | string) =>
|
|
typeof id === 'number' && id >= 0
|
|
|
|
return labeledGraphs.flatMap(([label, g]) =>
|
|
[...g._links.values()].flatMap((link) =>
|
|
[
|
|
isNonNegative(link.origin_id) &&
|
|
!g._nodes_by_id[link.origin_id] &&
|
|
`${label}: origin_id ${link.origin_id} not found`,
|
|
isNonNegative(link.target_id) &&
|
|
!g._nodes_by_id[link.target_id] &&
|
|
`${label}: target_id ${link.target_id} not found`
|
|
].filter(Boolean)
|
|
)
|
|
)
|
|
})
|
|
|
|
expect(invalidLinks).toEqual([])
|
|
})
|
|
|
|
test('Subgraph navigation works after ID remapping', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow(WORKFLOW)
|
|
|
|
const subgraphNode = await comfyPage.nodeOps.getNodeRefById('5')
|
|
await subgraphNode.navigateIntoSubgraph()
|
|
|
|
const isInSubgraph = () =>
|
|
comfyPage.page.evaluate(
|
|
() => window.app!.canvas.graph?.isRootGraph === false
|
|
)
|
|
|
|
expect(await isInSubgraph()).toBe(true)
|
|
|
|
await comfyPage.page.keyboard.press('Escape')
|
|
await comfyPage.nextFrame()
|
|
|
|
expect(await isInSubgraph()).toBe(false)
|
|
})
|
|
})
|