mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
[bugfix] Filter model metadata by current widget selection (#4021)
Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com>
This commit is contained in:
59
browser_tests/assets/model_metadata_widget_mismatch.json
Normal file
59
browser_tests/assets/model_metadata_widget_mismatch.json
Normal file
@@ -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
|
||||
}
|
||||
@@ -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 ({
|
||||
|
||||
@@ -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
|
||||
|
||||
51
src/utils/modelMetadataUtil.ts
Normal file
51
src/utils/modelMetadataUtil.ts
Normal file
@@ -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<string, unknown>
|
||||
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<string>()
|
||||
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)
|
||||
}
|
||||
}
|
||||
248
tests-ui/utils/modelMetadataUtil.test.ts
Normal file
248
tests-ui/utils/modelMetadataUtil.test.ts
Normal file
@@ -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')
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user