From 4c6e7f106bbfcde18c3aa63150f5841506901f60 Mon Sep 17 00:00:00 2001 From: Jin Yi Date: Sun, 3 Aug 2025 11:45:05 +0900 Subject: [PATCH] [fix] Detect missing nodes in subgraphs (#4639) Co-authored-by: bymyself --- .../assets/missing_nodes_in_subgraph.json | 182 ++++++++++++++++++ browser_tests/tests/dialog.spec.ts | 17 +- src/scripts/app.ts | 59 ++++-- 3 files changed, 242 insertions(+), 16 deletions(-) create mode 100644 browser_tests/assets/missing_nodes_in_subgraph.json diff --git a/browser_tests/assets/missing_nodes_in_subgraph.json b/browser_tests/assets/missing_nodes_in_subgraph.json new file mode 100644 index 000000000..246d52d38 --- /dev/null +++ b/browser_tests/assets/missing_nodes_in_subgraph.json @@ -0,0 +1,182 @@ +{ + "id": "test-missing-nodes-in-subgraph", + "revision": 0, + "last_node_id": 2, + "last_link_id": 0, + "nodes": [ + { + "id": 1, + "type": "KSampler", + "pos": [100, 100], + "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 + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": [] + } + ], + "properties": { + "Node name for S&R": "KSampler" + }, + "widgets_values": [0, "randomize", 20, 8, "euler", "simple", 1] + }, + { + "id": 2, + "type": "subgraph-with-missing-node", + "pos": [400, 100], + "size": [144, 46], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "name": "input1", + "type": "CONDITIONING", + "link": null + } + ], + "outputs": [ + { + "name": "output1", + "type": "LATENT", + "links": null + } + ], + "properties": {}, + "widgets_values": [] + } + ], + "links": [], + "groups": [], + "definitions": { + "subgraphs": [ + { + "id": "subgraph-with-missing-node", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 2, + "lastLinkId": 2, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "Subgraph with Missing Node", + "inputNode": { + "id": -10, + "bounding": [100, 200, 120, 60] + }, + "outputNode": { + "id": -20, + "bounding": [500, 200, 120, 60] + }, + "inputs": [ + { + "id": "input1-id", + "name": "input1", + "type": "CONDITIONING", + "linkIds": [1], + "pos": { + "0": 150, + "1": 220 + } + } + ], + "outputs": [ + { + "id": "output1-id", + "name": "output1", + "type": "LATENT", + "linkIds": [2], + "pos": { + "0": 520, + "1": 220 + } + } + ], + "widgets": [], + "nodes": [ + { + "id": 1, + "type": "MISSING_NODE_TYPE_IN_SUBGRAPH", + "pos": [250, 180], + "size": [200, 100], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "name": "input", + "type": "CONDITIONING", + "link": 1 + } + ], + "outputs": [ + { + "name": "output", + "type": "LATENT", + "links": [2] + } + ], + "properties": { + "Node name for S&R": "MISSING_NODE_TYPE_IN_SUBGRAPH" + }, + "widgets_values": ["some", "widget", "values"] + } + ], + "links": [ + { + "id": 1, + "origin_id": -10, + "origin_slot": 0, + "target_id": 1, + "target_slot": 0, + "type": "CONDITIONING" + }, + { + "id": 2, + "origin_id": 1, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "LATENT" + } + ] + } + ] + }, + "config": {}, + "extra": { + "ds": { + "scale": 1, + "offset": [0, 0] + } + }, + "version": 0.4 +} \ No newline at end of file diff --git a/browser_tests/tests/dialog.spec.ts b/browser_tests/tests/dialog.spec.ts index a5c46131a..1223d6940 100644 --- a/browser_tests/tests/dialog.spec.ts +++ b/browser_tests/tests/dialog.spec.ts @@ -13,6 +13,21 @@ test.describe('Load workflow warning', () => { const missingNodesWarning = comfyPage.page.locator('.comfy-missing-nodes') await expect(missingNodesWarning).toBeVisible() }) + + test('Should display a warning when loading a workflow with missing nodes in subgraphs', async ({ + comfyPage + }) => { + await comfyPage.loadWorkflow('missing_nodes_in_subgraph') + + // Wait for the element with the .comfy-missing-nodes selector to be visible + const missingNodesWarning = comfyPage.page.locator('.comfy-missing-nodes') + await expect(missingNodesWarning).toBeVisible() + + // Verify the missing node text includes subgraph context + const warningText = await missingNodesWarning.textContent() + expect(warningText).toContain('MISSING_NODE_TYPE_IN_SUBGRAPH') + expect(warningText).toContain('in subgraph') + }) }) test('Does not report warning on undo/redo', async ({ comfyPage }) => { @@ -369,7 +384,7 @@ test.describe('Signin dialog', () => { await textBox.press('Control+c') await comfyPage.page.evaluate(() => { - window['app'].extensionManager.dialog.showSignInDialog() + void window['app'].extensionManager.dialog.showSignInDialog() }) const input = comfyPage.page.locator('#comfy-org-sign-in-password') diff --git a/src/scripts/app.ts b/src/scripts/app.ts index fe6023405..67240b3af 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -23,7 +23,8 @@ import { ComfyApiWorkflow, type ComfyWorkflowJSON, type ModelFile, - type NodeId + type NodeId, + isSubgraphDefinition } from '@/schemas/comfyWorkflowSchema' import { type ComfyNodeDef as ComfyNodeDefV1, @@ -1092,23 +1093,51 @@ export class ComfyApp { const embeddedModels: ModelFile[] = [] - for (let n of graphData.nodes) { - // Patch T2IAdapterLoader to ControlNetLoader since they are the same node now - if (n.type == 'T2IAdapterLoader') n.type = 'ControlNetLoader' - if (n.type == 'ConditioningAverage ') n.type = 'ConditioningAverage' //typo fix - if (n.type == 'SDV_img2vid_Conditioning') - n.type = 'SVD_img2vid_Conditioning' //typo fix + const collectMissingNodesAndModels = ( + nodes: ComfyWorkflowJSON['nodes'], + path: string = '' + ) => { + for (let n of nodes) { + // Patch T2IAdapterLoader to ControlNetLoader since they are the same node now + if (n.type == 'T2IAdapterLoader') n.type = 'ControlNetLoader' + if (n.type == 'ConditioningAverage ') n.type = 'ConditioningAverage' //typo fix + if (n.type == 'SDV_img2vid_Conditioning') + n.type = 'SVD_img2vid_Conditioning' //typo fix - // Find missing node types - if (!(n.type in LiteGraph.registered_node_types)) { - missingNodeTypes.push(n.type) - n.type = sanitizeNodeName(n.type) + // Find missing node types + if (!(n.type in LiteGraph.registered_node_types)) { + // Include context about subgraph location if applicable + if (path) { + missingNodeTypes.push({ + type: n.type, + hint: `in subgraph '${path}'` + }) + } else { + missingNodeTypes.push(n.type) + } + n.type = sanitizeNodeName(n.type) + } + + // Collect models metadata from node + const selectedModels = getSelectedModelsMetadata(n) + if (selectedModels?.length) { + embeddedModels.push(...selectedModels) + } } + } - // Collect models metadata from node - const selectedModels = getSelectedModelsMetadata(n) - if (selectedModels?.length) { - embeddedModels.push(...selectedModels) + // Process nodes at the top level + collectMissingNodesAndModels(graphData.nodes) + + // Process nodes in subgraphs + if (graphData.definitions?.subgraphs) { + for (const subgraph of graphData.definitions.subgraphs) { + if (isSubgraphDefinition(subgraph)) { + collectMissingNodesAndModels( + subgraph.nodes, + subgraph.name || subgraph.id + ) + } } }