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 000000000..7446b6d7c --- /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 1fbd0de41..a5c46131a 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 ae7e25d95..2ccf9611e 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -62,6 +62,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' @@ -1026,8 +1027,10 @@ export class ComfyApp { } // Collect models metadata from node - if (n.properties?.models?.length) - embeddedModels.push(...n.properties.models) + const selectedModels = getSelectedModelsMetadata(n) + if (selectedModels?.length) { + 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 000000000..662704159 --- /dev/null +++ b/src/utils/modelMetadataUtil.ts @@ -0,0 +1,51 @@ +import type { ModelFile } from '@/schemas/comfyWorkflowSchema' + +/** + * 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 + */ +export function getSelectedModelsMetadata(node: { + type: string + widgets_values?: unknown[] | Record + properties?: { models?: ModelFile[] } +}): ModelFile[] | undefined { + 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 + + const stringWidgetValues = new Set() + for (const widgetValue of widgetValues) { + if (typeof widgetValue === 'string' && widgetValue.trim()) { + stringWidgetValues.add(widgetValue) + } + } + + // Return the node's models that are present in the widget values + return node.properties.models.filter((model) => + stringWidgetValues.has(model.name) + ) + } catch (error) { + console.error('Error filtering models by current selection:', error) + } +} diff --git a/tests-ui/utils/modelMetadataUtil.test.ts b/tests-ui/utils/modelMetadataUtil.test.ts new file mode 100644 index 000000000..eb72c4d8e --- /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 undefined for nodes without model metadata', () => { + const node = { + type: 'SomeNode', + widgets_values: ['model_a.safetensors'] + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toBeUndefined() + }) + + it('should return undefined 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).toBeUndefined() + }) + + it('should return undefined 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).toBeUndefined() + }) + + it('should return undefined for nodes with empty models array', () => { + const node = { + type: 'SomeNode', + widgets_values: ['model_a.safetensors'], + properties: { + models: [] + } + } + + const result = getSelectedModelsMetadata(node) + + expect(result).toBeUndefined() + }) + + 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') + }) + }) +})