From 8758945d39bbea44e13aa60587f588f3d5b3a85e Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 29 May 2025 20:21:39 -0700 Subject: [PATCH 1/8] [bugfix] Filter model metadata by current widget selection Only include model metadata for models actually selected in node widgets. This prevents false positives in the missing models dialog when nodes contain outdated metadata from previously selected models. Fixes #3664 --- .../model_metadata_widget_mismatch.json | 59 +++++ browser_tests/tests/dialog.spec.ts | 16 +- src/scripts/app.ts | 9 +- src/utils/modelMetadataUtil.ts | 42 +++ tests-ui/utils/modelMetadataUtil.test.ts | 248 ++++++++++++++++++ 5 files changed, 369 insertions(+), 5 deletions(-) create mode 100644 browser_tests/assets/model_metadata_widget_mismatch.json create mode 100644 src/utils/modelMetadataUtil.ts create mode 100644 tests-ui/utils/modelMetadataUtil.test.ts diff --git a/browser_tests/assets/model_metadata_widget_mismatch.json b/browser_tests/assets/model_metadata_widget_mismatch.json new file mode 100644 index 0000000000..7446b6d7c4 --- /dev/null +++ b/browser_tests/assets/model_metadata_widget_mismatch.json @@ -0,0 +1,59 @@ +{ + "last_node_id": 1, + "last_link_id": 0, + "nodes": [ + { + "id": 1, + "type": "CheckpointLoaderSimple", + "pos": [256, 256], + "size": [315, 98], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "MODEL", + "type": "MODEL", + "links": null + }, + { + "name": "CLIP", + "type": "CLIP", + "links": null + }, + { + "name": "VAE", + "type": "VAE", + "links": null + } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple", + "models": [ + { + "name": "outdated_model.safetensors", + "url": "http://localhost:8188/api/devtools/fake_model.safetensors", + "directory": "text_encoders" + }, + { + "name": "another_outdated_model.safetensors", + "url": "http://localhost:8188/api/devtools/fake_model.safetensors", + "directory": "text_encoders" + } + ] + }, + "widgets_values": ["current_selected_model.safetensors"] + } + ], + "links": [], + "groups": [], + "config": {}, + "extra": { + "ds": { + "offset": [0, 0], + "scale": 1 + } + }, + "version": 0.4 +} diff --git a/browser_tests/tests/dialog.spec.ts b/browser_tests/tests/dialog.spec.ts index 1fbd0de41f..a5c46131ab 100644 --- a/browser_tests/tests/dialog.spec.ts +++ b/browser_tests/tests/dialog.spec.ts @@ -103,7 +103,7 @@ test.describe('Missing models warning', () => { } ]) } - comfyPage.page.route( + await comfyPage.page.route( '**/api/experiment/models', (route) => route.fulfill(modelFoldersRes), { times: 1 } @@ -121,7 +121,7 @@ test.describe('Missing models warning', () => { } ]) } - comfyPage.page.route( + await comfyPage.page.route( '**/api/experiment/models/text_encoders', (route) => route.fulfill(clipModelsRes), { times: 1 } @@ -133,6 +133,18 @@ test.describe('Missing models warning', () => { await expect(missingModelsWarning).not.toBeVisible() }) + test('Should not display warning when model metadata exists but widget values have changed', async ({ + comfyPage + }) => { + // This tests the scenario where outdated model metadata exists in the workflow + // but the actual selected models (widget values) have changed + await comfyPage.loadWorkflow('model_metadata_widget_mismatch') + + // The missing models warning should NOT appear + const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models') + await expect(missingModelsWarning).not.toBeVisible() + }) + // Flaky test after parallelization // https://github.com/Comfy-Org/ComfyUI_frontend/pull/1400 test.skip('Should download missing model when clicking download button', async ({ diff --git a/src/scripts/app.ts b/src/scripts/app.ts index ad4ed91fc4..bb1ab62c5c 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -67,6 +67,7 @@ import { findLegacyRerouteNodes, noNativeReroutes } from '@/utils/migration/migrateReroute' +import { getSelectedModelsMetadata } from '@/utils/modelMetadataUtil' import { deserialiseAndCreate } from '@/utils/vintageClipboard' import { type ComfyApi, PromptExecutionError, api } from './api' @@ -1036,9 +1037,11 @@ export class ComfyApp { n.type = sanitizeNodeName(n.type) } - // Collect models metadata from node - if (n.properties?.models?.length) - embeddedModels.push(...n.properties.models) + // Collect models metadata from node if the model names are present in the widget values + const selectedModels = getSelectedModelsMetadata(n) + if (selectedModels && selectedModels.length > 0) { + embeddedModels.push(...selectedModels) + } } // Merge models from the workflow's root-level 'models' field diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts new file mode 100644 index 0000000000..7d99553387 --- /dev/null +++ b/src/utils/modelMetadataUtil.ts @@ -0,0 +1,42 @@ +import type { ModelFile } from '@/schemas/comfyWorkflowSchema' + +/** + * Filters model metadata to only include models currently selected in the node's widget values. + * This prevents false positives in the missing models dialog when outdated model metadata + * exists but the actual selected models have changed. + * + * @param node - The workflow node to process + * @returns Filtered array containing only models that are currently selected + */ +export function getSelectedModelsMetadata(node: { + type: string + widgets_values?: unknown[] | Record + properties?: { models?: ModelFile[] } +}): ModelFile[] { + try { + if (!node.properties?.models?.length) return [] + if (!node.widgets_values) return [] + + const widgetValues = Array.isArray(node.widgets_values) + ? node.widgets_values + : Object.values(node.widgets_values) + + if (!widgetValues.length) return [] + + // Create set of selected model names from widget values (only process combo inputs) + const selectedModelNames = new Set() + for (const widgetValue of widgetValues) { + if (typeof widgetValue === 'string' && widgetValue.trim()) { + selectedModelNames.add(widgetValue) + } + } + + // Filter models to only include those currently selected + return node.properties.models.filter((model) => + selectedModelNames.has(model.name) + ) + } catch (error) { + console.error('Error filtering models by current selection:', error) + return [] + } +} diff --git a/tests-ui/utils/modelMetadataUtil.test.ts b/tests-ui/utils/modelMetadataUtil.test.ts new file mode 100644 index 0000000000..c40857212f --- /dev/null +++ b/tests-ui/utils/modelMetadataUtil.test.ts @@ -0,0 +1,248 @@ +import { describe, expect, it } from 'vitest' + +import { getSelectedModelsMetadata } from '@/utils/modelMetadataUtil' + +describe('modelMetadataUtil', () => { + describe('filterModelsByCurrentSelection', () => { + it('should filter models to only include those selected in widget values', () => { + const node = { + type: 'CheckpointLoaderSimple', + widgets_values: ['model_a.safetensors'], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + }, + { + name: 'model_b.safetensors', + url: 'https://example.com/model_b.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(1) + expect(result[0].name).toBe('model_a.safetensors') + }) + + it('should return empty array when no models match selection', () => { + const node = { + type: 'SomeNode', + widgets_values: ['unmatched_model.safetensors'], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(0) + }) + + it('should handle multiple widget values', () => { + const node = { + type: 'SomeNode', + widgets_values: ['model_a.safetensors', 42, 'model_c.ckpt'], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + }, + { + name: 'model_b.safetensors', + url: 'https://example.com/model_b.safetensors', + directory: 'checkpoints' + }, + { + name: 'model_c.ckpt', + url: 'https://example.com/model_c.ckpt', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(2) + expect(result.map((m) => m.name)).toEqual([ + 'model_a.safetensors', + 'model_c.ckpt' + ]) + }) + + it('should ignore non-string widget values', () => { + const node = { + type: 'SomeNode', + widgets_values: [42, true, null, undefined, 'model_a.safetensors'], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(1) + expect(result[0].name).toBe('model_a.safetensors') + }) + + it('should ignore empty strings', () => { + const node = { + type: 'SomeNode', + widgets_values: ['', ' ', 'model_a.safetensors'], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(1) + expect(result[0].name).toBe('model_a.safetensors') + }) + + it('should return empty array for nodes without model metadata', () => { + const node = { + type: 'SomeNode', + widgets_values: ['model_a.safetensors'] + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(0) + }) + + it('should return empty array for nodes without widgets_values', () => { + const node = { + type: 'SomeNode', + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(0) + }) + + it('should return empty array for nodes with empty widgets_values', () => { + const node = { + type: 'SomeNode', + widgets_values: [], + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(0) + }) + + it('should return empty array for nodes with empty models array', () => { + const node = { + type: 'SomeNode', + widgets_values: ['model_a.safetensors'], + properties: { + models: [] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(0) + }) + + it('should handle object widget values', () => { + const node = { + type: 'SomeNode', + widgets_values: { + ckpt_name: 'model_a.safetensors', + seed: 42 + }, + properties: { + models: [ + { + name: 'model_a.safetensors', + url: 'https://example.com/model_a.safetensors', + directory: 'checkpoints' + }, + { + name: 'model_b.safetensors', + url: 'https://example.com/model_b.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(1) + expect(result[0].name).toBe('model_a.safetensors') + }) + + it('should work end-to-end to filter outdated metadata', () => { + const node = { + type: 'CheckpointLoaderSimple', + widgets_values: ['current_model.safetensors'], + properties: { + models: [ + { + name: 'current_model.safetensors', + url: 'https://example.com/current_model.safetensors', + directory: 'checkpoints' + }, + { + name: 'old_model.safetensors', + url: 'https://example.com/old_model.safetensors', + directory: 'checkpoints' + } + ] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toHaveLength(1) + expect(result![0].name).toBe('current_model.safetensors') + }) + }) +}) From 928899d0e0f83ff3855c74e9066e251d5616d3a2 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 29 May 2025 20:33:06 -0700 Subject: [PATCH 2/8] improve comments clarity --- src/scripts/app.ts | 2 +- src/utils/modelMetadataUtil.ts | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/scripts/app.ts b/src/scripts/app.ts index bb1ab62c5c..fc63eb41f8 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -1037,7 +1037,7 @@ export class ComfyApp { n.type = sanitizeNodeName(n.type) } - // Collect models metadata from node if the model names are present in the widget values + // Collect models metadata from node const selectedModels = getSelectedModelsMetadata(n) if (selectedModels && selectedModels.length > 0) { embeddedModels.push(...selectedModels) diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts index 7d99553387..810b4ad5ec 100644 --- a/src/utils/modelMetadataUtil.ts +++ b/src/utils/modelMetadataUtil.ts @@ -1,9 +1,20 @@ import type { ModelFile } from '@/schemas/comfyWorkflowSchema' /** - * Filters model metadata to only include models currently selected in the node's widget values. - * This prevents false positives in the missing models dialog when outdated model metadata - * exists but the actual selected models have changed. + * Gets models from the node's `properties.models` field, excluding those + * not currently selected in at least 1 of the node's widget values. + * + * @example + * ```ts + * const node = { + * type: 'CheckpointLoaderSimple', + * widgets_values: ['model1', 'model2'], + * properties: { models: [{ name: 'model1' }, { name: 'model2' }, { name: 'model3' }] } + * ... other properties + * } + * const selectedModels = getSelectedModelsMetadata(node) + * // selectedModels = [{ name: 'model1' }, { name: 'model2' }] + * ``` * * @param node - The workflow node to process * @returns Filtered array containing only models that are currently selected @@ -23,17 +34,16 @@ export function getSelectedModelsMetadata(node: { if (!widgetValues.length) return [] - // Create set of selected model names from widget values (only process combo inputs) - const selectedModelNames = new Set() + const stringWidgetValues = new Set() for (const widgetValue of widgetValues) { if (typeof widgetValue === 'string' && widgetValue.trim()) { - selectedModelNames.add(widgetValue) + stringWidgetValues.add(widgetValue) } } - // Filter models to only include those currently selected + // Return the node's models that are present in the widget values return node.properties.models.filter((model) => - selectedModelNames.has(model.name) + stringWidgetValues.has(model.name) ) } catch (error) { console.error('Error filtering models by current selection:', error) From c7c0adebadb767a906022373b94663bf28a18364 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 31 May 2025 13:33:27 -0700 Subject: [PATCH 3/8] Update src/scripts/app.ts Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- src/scripts/app.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scripts/app.ts b/src/scripts/app.ts index fc63eb41f8..236bd060ed 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -1039,7 +1039,7 @@ export class ComfyApp { // Collect models metadata from node const selectedModels = getSelectedModelsMetadata(n) - if (selectedModels && selectedModels.length > 0) { + if (selectedModels?.length) { embeddedModels.push(...selectedModels) } } From e3a8b48a28aa89b884baefd4d9be4cd5a0be2213 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 31 May 2025 13:33:46 -0700 Subject: [PATCH 4/8] Update src/utils/modelMetadataUtil.ts Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- src/utils/modelMetadataUtil.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts index 810b4ad5ec..032446a441 100644 --- a/src/utils/modelMetadataUtil.ts +++ b/src/utils/modelMetadataUtil.ts @@ -25,8 +25,8 @@ export function getSelectedModelsMetadata(node: { properties?: { models?: ModelFile[] } }): ModelFile[] { try { - if (!node.properties?.models?.length) return [] - if (!node.widgets_values) return [] + if (!node.properties?.models?.length) return + if (!node.widgets_values) return const widgetValues = Array.isArray(node.widgets_values) ? node.widgets_values From 3950c1ef4432b3b9c1d9779073fc58a812ca871a Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 31 May 2025 13:33:51 -0700 Subject: [PATCH 5/8] Update src/utils/modelMetadataUtil.ts Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- src/utils/modelMetadataUtil.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts index 032446a441..6e3992c0f1 100644 --- a/src/utils/modelMetadataUtil.ts +++ b/src/utils/modelMetadataUtil.ts @@ -32,7 +32,7 @@ export function getSelectedModelsMetadata(node: { ? node.widgets_values : Object.values(node.widgets_values) - if (!widgetValues.length) return [] + if (!widgetValues.length) return const stringWidgetValues = new Set() for (const widgetValue of widgetValues) { From be36e589b399346f5606f664bdf9a36e9bffdc25 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 31 May 2025 13:34:03 -0700 Subject: [PATCH 6/8] Update src/utils/modelMetadataUtil.ts Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- src/utils/modelMetadataUtil.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts index 6e3992c0f1..c16a069c2f 100644 --- a/src/utils/modelMetadataUtil.ts +++ b/src/utils/modelMetadataUtil.ts @@ -47,6 +47,5 @@ export function getSelectedModelsMetadata(node: { ) } catch (error) { console.error('Error filtering models by current selection:', error) - return [] } } From 968a8736f2d9e93d1524ace8a39232388d5d2890 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 31 May 2025 13:34:14 -0700 Subject: [PATCH 7/8] Update src/utils/modelMetadataUtil.ts Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com> --- src/utils/modelMetadataUtil.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/modelMetadataUtil.ts b/src/utils/modelMetadataUtil.ts index c16a069c2f..6627041599 100644 --- a/src/utils/modelMetadataUtil.ts +++ b/src/utils/modelMetadataUtil.ts @@ -23,7 +23,7 @@ export function getSelectedModelsMetadata(node: { type: string widgets_values?: unknown[] | Record properties?: { models?: ModelFile[] } -}): ModelFile[] { +}): ModelFile[] | undefined { try { if (!node.properties?.models?.length) return if (!node.widgets_values) return From 1d5bdefd01091d1763ef429919c862eb7015148e Mon Sep 17 00:00:00 2001 From: bymyself Date: Sat, 31 May 2025 16:43:32 -0700 Subject: [PATCH 8/8] [bugfix] Fix TypeScript errors in modelMetadataUtil tests Add non-null assertions for test cases expecting arrays and correct test expectations for cases that should return undefined. --- tests-ui/utils/modelMetadataUtil.test.ts | 26 ++++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests-ui/utils/modelMetadataUtil.test.ts b/tests-ui/utils/modelMetadataUtil.test.ts index c40857212f..eb72c4d8e0 100644 --- a/tests-ui/utils/modelMetadataUtil.test.ts +++ b/tests-ui/utils/modelMetadataUtil.test.ts @@ -27,7 +27,7 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) expect(result).toHaveLength(1) - expect(result[0].name).toBe('model_a.safetensors') + expect(result![0].name).toBe('model_a.safetensors') }) it('should return empty array when no models match selection', () => { @@ -78,7 +78,7 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) expect(result).toHaveLength(2) - expect(result.map((m) => m.name)).toEqual([ + expect(result!.map((m) => m.name)).toEqual([ 'model_a.safetensors', 'model_c.ckpt' ]) @@ -102,7 +102,7 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) expect(result).toHaveLength(1) - expect(result[0].name).toBe('model_a.safetensors') + expect(result![0].name).toBe('model_a.safetensors') }) it('should ignore empty strings', () => { @@ -123,10 +123,10 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) expect(result).toHaveLength(1) - expect(result[0].name).toBe('model_a.safetensors') + expect(result![0].name).toBe('model_a.safetensors') }) - it('should return empty array for nodes without model metadata', () => { + it('should return undefined for nodes without model metadata', () => { const node = { type: 'SomeNode', widgets_values: ['model_a.safetensors'] @@ -134,10 +134,10 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) - expect(result).toHaveLength(0) + expect(result).toBeUndefined() }) - it('should return empty array for nodes without widgets_values', () => { + it('should return undefined for nodes without widgets_values', () => { const node = { type: 'SomeNode', properties: { @@ -153,10 +153,10 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) - expect(result).toHaveLength(0) + expect(result).toBeUndefined() }) - it('should return empty array for nodes with empty widgets_values', () => { + it('should return undefined for nodes with empty widgets_values', () => { const node = { type: 'SomeNode', widgets_values: [], @@ -173,10 +173,10 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) - expect(result).toHaveLength(0) + expect(result).toBeUndefined() }) - it('should return empty array for nodes with empty models array', () => { + it('should return undefined for nodes with empty models array', () => { const node = { type: 'SomeNode', widgets_values: ['model_a.safetensors'], @@ -187,7 +187,7 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) - expect(result).toHaveLength(0) + expect(result).toBeUndefined() }) it('should handle object widget values', () => { @@ -216,7 +216,7 @@ describe('modelMetadataUtil', () => { const result = getSelectedModelsMetadata(node) expect(result).toHaveLength(1) - expect(result[0].name).toBe('model_a.safetensors') + expect(result![0].name).toBe('model_a.safetensors') }) it('should work end-to-end to filter outdated metadata', () => {