From 490cfd7f3c5dbc66ec1a9c7209b3345731078fe9 Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Fri, 19 Jun 2026 13:19:39 +0900 Subject: [PATCH] fix: limit workflow models to metadata enrichment --- .../fixtures/helpers/WorkflowHelper.ts | 2 +- .../errorsTabMissingModels.spec.ts | 14 +- .../activeSubgraphUnmatchedModel.json | 83 --- .../bypassedSubgraphUnmatchedModel.json | 83 --- .../missingModel/missingModelPipeline.test.ts | 43 +- .../missingModel/missingModelPipeline.ts | 16 +- .../missingModel/missingModelScan.test.ts | 472 ++++++------------ src/platform/missingModel/missingModelScan.ts | 219 ++------ src/platform/missingModel/types.ts | 12 +- 9 files changed, 210 insertions(+), 734 deletions(-) delete mode 100644 src/platform/missingModel/__fixtures__/activeSubgraphUnmatchedModel.json delete mode 100644 src/platform/missingModel/__fixtures__/bypassedSubgraphUnmatchedModel.json diff --git a/browser_tests/fixtures/helpers/WorkflowHelper.ts b/browser_tests/fixtures/helpers/WorkflowHelper.ts index cdb40dd306d..9954295fcc8 100644 --- a/browser_tests/fixtures/helpers/WorkflowHelper.ts +++ b/browser_tests/fixtures/helpers/WorkflowHelper.ts @@ -2,11 +2,11 @@ import { readFileSync } from 'fs' import { test } from '@playwright/test' -import type { AppMode } from '@/composables/useAppMode' import type { ComfyApiWorkflow, ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' +import type { AppMode } from '@/utils/appMode' import type { WorkspaceStore } from '@e2e/types/globals' import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { assetPath } from '@e2e/fixtures/utils/paths' diff --git a/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts b/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts index c071201417c..c41fd6145ec 100644 --- a/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts +++ b/browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts @@ -143,7 +143,7 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { const objectInfo = await response.json() const ckptName = objectInfo.CheckpointLoaderSimple.input.required.ckpt_name - ckptName[0] = [...ckptName[0], 'fake_model.safetensors'] + ckptName[0] = [...ckptName[0], FAKE_MODEL_NAME] await route.fulfill({ response, json: objectInfo }) }) @@ -151,21 +151,11 @@ test.describe('Errors tab - Missing models', { tag: '@ui' }, () => { const url = new URL(response.url()) return url.pathname.endsWith('/object_info') && response.ok() }) - const modelFoldersResponse = comfyPage.page.waitForResponse( - (response) => { - const url = new URL(response.url()) - return url.pathname.endsWith('/experiment/models') && response.ok() - } - ) const refreshButton = comfyPage.page.getByTestId( TestIds.dialogs.missingModelRefresh ) - await Promise.all([ - objectInfoResponse, - modelFoldersResponse, - refreshButton.click() - ]) + await Promise.all([objectInfoResponse, refreshButton.click()]) await expect( comfyPage.page.getByTestId(TestIds.dialogs.missingModelsGroup) ).toBeHidden() diff --git a/src/platform/missingModel/__fixtures__/activeSubgraphUnmatchedModel.json b/src/platform/missingModel/__fixtures__/activeSubgraphUnmatchedModel.json deleted file mode 100644 index 6be16a28d6f..00000000000 --- a/src/platform/missingModel/__fixtures__/activeSubgraphUnmatchedModel.json +++ /dev/null @@ -1,83 +0,0 @@ -{ - "last_node_id": 2, - "last_link_id": 0, - "nodes": [ - { - "id": 1, - "type": "KSampler", - "pos": [0, 0], - "size": [100, 100], - "flags": {}, - "order": 1, - "mode": 0, - "properties": {}, - "widgets_values": [0, "randomize", 20] - }, - { - "id": 2, - "type": "subgraph-x", - "pos": [300, 0], - "size": [100, 100], - "flags": {}, - "order": 0, - "mode": 0, - "properties": {}, - "widgets_values": [] - } - ], - "links": [], - "groups": [], - "config": {}, - "extra": {}, - "version": 0.4, - "definitions": { - "subgraphs": [ - { - "id": "subgraph-x", - "version": 1, - "state": { - "lastGroupId": 0, - "lastNodeId": 1, - "lastLinkId": 0, - "lastRerouteId": 0 - }, - "revision": 0, - "config": {}, - "name": "x", - "nodes": [ - { - "id": 1, - "type": "CheckpointLoaderSimple", - "pos": [0, 0], - "size": [100, 100], - "flags": {}, - "order": 0, - "mode": 0, - "properties": { - "models": [ - { - "name": "rare_model.safetensors", - "directory": "checkpoints" - } - ] - }, - "widgets_values": ["some_other_model.safetensors"] - } - ], - "links": [], - "inputNode": { "id": -10, "bounding": [0, 0, 0, 0] }, - "outputNode": { "id": -20, "bounding": [0, 0, 0, 0] }, - "inputs": [], - "outputs": [], - "widgets": [] - } - ] - }, - "models": [ - { - "name": "rare_model.safetensors", - "url": "https://example.com/rare", - "directory": "checkpoints" - } - ] -} diff --git a/src/platform/missingModel/__fixtures__/bypassedSubgraphUnmatchedModel.json b/src/platform/missingModel/__fixtures__/bypassedSubgraphUnmatchedModel.json deleted file mode 100644 index 959b1034eee..00000000000 --- a/src/platform/missingModel/__fixtures__/bypassedSubgraphUnmatchedModel.json +++ /dev/null @@ -1,83 +0,0 @@ -{ - "last_node_id": 2, - "last_link_id": 0, - "nodes": [ - { - "id": 1, - "type": "KSampler", - "pos": [0, 0], - "size": [100, 100], - "flags": {}, - "order": 1, - "mode": 0, - "properties": {}, - "widgets_values": [0, "randomize", 20] - }, - { - "id": 2, - "type": "subgraph-x", - "pos": [300, 0], - "size": [100, 100], - "flags": {}, - "order": 0, - "mode": 4, - "properties": {}, - "widgets_values": [] - } - ], - "links": [], - "groups": [], - "config": {}, - "extra": {}, - "version": 0.4, - "definitions": { - "subgraphs": [ - { - "id": "subgraph-x", - "version": 1, - "state": { - "lastGroupId": 0, - "lastNodeId": 1, - "lastLinkId": 0, - "lastRerouteId": 0 - }, - "revision": 0, - "config": {}, - "name": "x", - "nodes": [ - { - "id": 1, - "type": "CheckpointLoaderSimple", - "pos": [0, 0], - "size": [100, 100], - "flags": {}, - "order": 0, - "mode": 0, - "properties": { - "models": [ - { - "name": "rare_model.safetensors", - "directory": "checkpoints" - } - ] - }, - "widgets_values": ["some_other_model.safetensors"] - } - ], - "links": [], - "inputNode": { "id": -10, "bounding": [0, 0, 0, 0] }, - "outputNode": { "id": -20, "bounding": [0, 0, 0, 0] }, - "inputs": [], - "outputs": [], - "widgets": [] - } - ] - }, - "models": [ - { - "name": "rare_model.safetensors", - "url": "https://example.com/rare", - "directory": "checkpoints" - } - ] -} diff --git a/src/platform/missingModel/missingModelPipeline.test.ts b/src/platform/missingModel/missingModelPipeline.test.ts index b6c1b1cda3d..4b180417274 100644 --- a/src/platform/missingModel/missingModelPipeline.test.ts +++ b/src/platform/missingModel/missingModelPipeline.test.ts @@ -34,10 +34,6 @@ const { mockHandles } = vi.hoisted(() => { executionErrorStore: { surfaceMissingModels: vi.fn() }, - modelStore: { - loadModelFolders: vi.fn(), - getLoadedModelFolder: vi.fn() - }, modelToNodeStore: { getCategoryForNodeType: vi.fn() }, @@ -49,14 +45,9 @@ const { mockHandles } = vi.hoisted(() => { ): MissingModelCandidate[] => [] ), enrichWithEmbeddedMetadata: vi.fn( - async ( + ( _candidates: readonly MissingModelCandidate[], - _graphData: ComfyWorkflowJSON, - _checkModelInstalled: ( - name: string, - directory: string - ) => Promise, - _isAssetSupported?: (nodeType: string, widgetName: string) => boolean + _graphData: ComfyWorkflowJSON ) => state.enrichedCandidates ), verifyAssetSupportedCandidates: vi.fn( @@ -104,10 +95,6 @@ vi.mock('@/stores/executionErrorStore', () => ({ useExecutionErrorStore: () => mockHandles.executionErrorStore })) -vi.mock('@/stores/modelStore', () => ({ - useModelStore: () => mockHandles.modelStore -})) - vi.mock('@/stores/modelToNodeStore', () => ({ useModelToNodeStore: () => mockHandles.modelToNodeStore })) @@ -121,16 +108,8 @@ vi.mock('@/platform/missingModel/missingModelScan', () => ({ mockHandles.scanAllModelCandidates(graph, isAssetSupported, getDirectory), enrichWithEmbeddedMetadata: ( candidates: readonly MissingModelCandidate[], - graphData: ComfyWorkflowJSON, - checkModelInstalled: (name: string, directory: string) => Promise, - isAssetSupported?: (nodeType: string, widgetName: string) => boolean - ) => - mockHandles.enrichWithEmbeddedMetadata( - candidates, - graphData, - checkModelInstalled, - isAssetSupported - ), + graphData: ComfyWorkflowJSON + ) => mockHandles.enrichWithEmbeddedMetadata(candidates, graphData), verifyAssetSupportedCandidates: ( candidates: readonly MissingModelCandidate[], signal: AbortSignal @@ -186,8 +165,6 @@ describe('missingModelPipeline', () => { mockHandles.missingModelStore.createVerificationAbortController.mockImplementation( () => new AbortController() ) - mockHandles.modelStore.loadModelFolders.mockResolvedValue(undefined) - mockHandles.modelStore.getLoadedModelFolder.mockResolvedValue(undefined) mockHandles.modelToNodeStore.getCategoryForNodeType.mockReturnValue( undefined ) @@ -253,9 +230,7 @@ describe('missingModelPipeline', () => { expect(mockHandles.enrichWithEmbeddedMetadata).toHaveBeenCalledWith( expect.any(Array), - expect.objectContaining({ models: activeModels }), - expect.any(Function), - undefined + expect.objectContaining({ models: activeModels }) ) expect( mockHandles.executionErrorStore.surfaceMissingModels @@ -305,9 +280,7 @@ describe('missingModelPipeline', () => { hash_type: 'sha256' } ] - }), - expect.any(Function), - undefined + }) ) expect( mockHandles.executionErrorStore.surfaceMissingModels @@ -325,9 +298,7 @@ describe('missingModelPipeline', () => { expect(mockHandles.enrichWithEmbeddedMetadata).toHaveBeenCalledWith( expect.any(Array), - graphData, - expect.any(Function), - undefined + graphData ) }) diff --git a/src/platform/missingModel/missingModelPipeline.ts b/src/platform/missingModel/missingModelPipeline.ts index 8ccb71018ed..864a198b5ae 100644 --- a/src/platform/missingModel/missingModelPipeline.ts +++ b/src/platform/missingModel/missingModelPipeline.ts @@ -15,7 +15,6 @@ import type { ComfyWorkflow } from '@/platform/workflow/management/stores/comfyW import type { ModelFile } from '@/platform/workflow/validation/schemas/workflowSchema' import { api } from '@/scripts/api' import { useExecutionErrorStore } from '@/stores/executionErrorStore' -import { useModelStore } from '@/stores/modelStore' import { useModelToNodeStore } from '@/stores/modelToNodeStore' import { useWorkspaceStore } from '@/stores/workspaceStore' import type { MissingNodeType } from '@/types/comfy' @@ -121,20 +120,7 @@ export async function runMissingModelPipeline({ getDirectory ) - const modelStore = useModelStore() - await modelStore.loadModelFolders() - const enrichedAll = await enrichWithEmbeddedMetadata( - candidates, - graphData, - async (name, directory) => { - const folder = await modelStore.getLoadedModelFolder(directory) - const models = folder?.models - return !!( - models && Object.values(models).some((m) => m.file_name === name) - ) - }, - isCloud ? isAssetBrowserWidget : undefined - ) + const enrichedAll = enrichWithEmbeddedMetadata(candidates, graphData) // Drop candidates whose enclosing subgraph is muted/bypassed. Per-node // scans only checked each node's own mode; the cascade from an diff --git a/src/platform/missingModel/missingModelScan.test.ts b/src/platform/missingModel/missingModelScan.test.ts index 879ba8de896..bfe2b1ceb15 100644 --- a/src/platform/missingModel/missingModelScan.test.ts +++ b/src/platform/missingModel/missingModelScan.test.ts @@ -15,8 +15,6 @@ import { verifyAssetSupportedCandidates, MODEL_FILE_EXTENSIONS } from '@/platform/missingModel/missingModelScan' -import activeSubgraphUnmatchedModel from '@/platform/missingModel/__fixtures__/activeSubgraphUnmatchedModel.json' with { type: 'json' } -import bypassedSubgraphUnmatchedModel from '@/platform/missingModel/__fixtures__/bypassedSubgraphUnmatchedModel.json' with { type: 'json' } import type { MissingModelCandidate } from '@/platform/missingModel/types' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' @@ -671,11 +669,8 @@ function makeCandidate( } } -const alwaysMissing = async () => false -const alwaysInstalled = async () => true - describe('enrichWithEmbeddedMetadata', () => { - it('enriches existing candidate with url and directory from embedded metadata', async () => { + it('enriches existing candidate with url and directory from embedded metadata', () => { const candidates = [makeCandidate('model_a.safetensors')] const graphData = fromPartial({ last_node_id: 1, @@ -709,18 +704,14 @@ describe('enrichWithEmbeddedMetadata', () => { ] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) expect(result[0].url).toBe('https://example.com/model_a') expect(result[0].directory).toBe('checkpoints') expect(result[0].hash).toBe('abc123') }) - it('does not overwrite existing fields on candidate', async () => { + it('does not overwrite existing fields on candidate', () => { const candidates = [ makeCandidate('model_a.safetensors', { directory: 'existing_dir', @@ -757,18 +748,13 @@ describe('enrichWithEmbeddedMetadata', () => { ] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - // ??= should not overwrite existing values expect(result[0].url).toBe('https://existing.com') expect(result[0].directory).toBe('existing_dir') }) - it('does not mutate the original candidates array', async () => { + it('does not mutate the original candidates array', () => { const candidates = [makeCandidate('model_a.safetensors')] const graphData = fromPartial({ last_node_id: 1, @@ -801,12 +787,12 @@ describe('enrichWithEmbeddedMetadata', () => { }) const originalUrl = candidates[0].url - await enrichWithEmbeddedMetadata(candidates, graphData, alwaysMissing) + enrichWithEmbeddedMetadata(candidates, graphData) expect(candidates[0].url).toBe(originalUrl) }) - it('adds new candidate for embedded model not found by COMBO scan', async () => { + it('does not add a candidate for embedded metadata without a live candidate', () => { const candidates: MissingModelCandidate[] = [] const graphData = fromPartial({ last_node_id: 1, @@ -838,18 +824,12 @@ describe('enrichWithEmbeddedMetadata', () => { ] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(1) - expect(result[0].name).toBe('model_a.safetensors') - expect(result[0].isMissing).toBe(true) + expect(result).toHaveLength(0) }) - it('does not add candidate when model is already installed', async () => { + it('does not add a candidate from root metadata without live references', () => { const candidates: MissingModelCandidate[] = [] const graphData = fromPartial({ last_node_id: 0, @@ -869,17 +849,15 @@ describe('enrichWithEmbeddedMetadata', () => { ] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysInstalled - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) expect(result).toHaveLength(0) }) - it('skips embedded models from muted nodes', async () => { - const candidates: MissingModelCandidate[] = [] + it('enriches existing candidates from node-sourced metadata', () => { + const candidates = [ + makeCandidate('node_model.safetensors', { nodeId: '1' }) + ] const graphData = fromPartial({ last_node_id: 1, last_link_id: 0, @@ -891,9 +869,17 @@ describe('enrichWithEmbeddedMetadata', () => { size: [100, 100], flags: {}, order: 0, - mode: 2, // NEVER (muted) - properties: {}, - widgets_values: { ckpt_name: 'model.safetensors' } + mode: 0, + properties: { + models: [ + { + name: 'node_model.safetensors', + url: 'https://example.com/node_model', + directory: 'checkpoints' + } + ] + }, + widgets_values: { ckpt_name: 'node_model.safetensors' } } ], links: [], @@ -901,33 +887,19 @@ describe('enrichWithEmbeddedMetadata', () => { config: {}, extra: {}, version: 0.4, - models: [ - { - name: 'model.safetensors', - url: 'https://example.com/model', - directory: 'checkpoints' - } - ] + models: [] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(0) + expect(result).toHaveLength(1) + expect(result[0].url).toBe('https://example.com/node_model') }) - it('drops workflow-level model entries when only referencing nodes are bypassed (other active nodes present)', async () => { - // Regression: a previous `hasActiveNodes` check kept workflow-level - // models in a mixed graph if ANY active node existed, even when every - // node that actually referenced the model was bypassed. The correct - // check drops unmatched workflow-level entries since candidates are - // derived from active-node widgets. - const candidates: MissingModelCandidate[] = [] + it('does not enrich from muted node metadata', () => { + const candidates = [makeCandidate('model.safetensors')] const graphData = fromPartial({ - last_node_id: 2, + last_node_id: 1, last_link_id: 0, nodes: [ { @@ -937,20 +909,17 @@ describe('enrichWithEmbeddedMetadata', () => { size: [100, 100], flags: {}, order: 0, - mode: 4, // BYPASS — only node referencing the model - properties: {}, + mode: 2, + properties: { + models: [ + { + name: 'model.safetensors', + url: 'https://example.com/model', + directory: 'checkpoints' + } + ] + }, widgets_values: { ckpt_name: 'model.safetensors' } - }, - { - id: 2, - type: 'KSampler', - pos: [200, 0], - size: [100, 100], - flags: {}, - order: 1, - mode: 0, // ALWAYS — unrelated active node - properties: {}, - widgets_values: {} } ], links: [], @@ -958,31 +927,16 @@ describe('enrichWithEmbeddedMetadata', () => { config: {}, extra: {}, version: 0.4, - models: [ - { - name: 'model.safetensors', - url: 'https://example.com/model', - directory: 'checkpoints' - } - ] + models: [] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(0) + expect(result[0].url).toBeUndefined() }) - it('keeps unmatched node-sourced entries in a mixed graph', async () => { - // A node-sourced unmatched entry (sourceNodeType !== '') must survive - // the workflow-level filter. This ensures the simplification does not - // over-filter legitimate per-node missing models. - const candidates = [ - makeCandidate('node_model.safetensors', { nodeId: '1' }) - ] + it('does not enrich from bypassed node metadata', () => { + const candidates = [makeCandidate('model.safetensors')] const graphData = fromPartial({ last_node_id: 1, last_link_id: 0, @@ -994,17 +948,17 @@ describe('enrichWithEmbeddedMetadata', () => { size: [100, 100], flags: {}, order: 0, - mode: 0, + mode: 4, properties: { models: [ { - name: 'node_model.safetensors', - url: 'https://example.com/node_model', + name: 'model.safetensors', + url: 'https://example.com/model', directory: 'checkpoints' } ] }, - widgets_values: { ckpt_name: 'node_model.safetensors' } + widgets_values: { ckpt_name: 'model.safetensors' } } ], links: [], @@ -1015,91 +969,110 @@ describe('enrichWithEmbeddedMetadata', () => { models: [] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(1) - expect(result[0].name).toBe('node_model.safetensors') + expect(result[0].url).toBeUndefined() }) - it('skips embedded models from bypassed nodes', async () => { - const candidates: MissingModelCandidate[] = [] - const graphData = fromPartial({ - last_node_id: 1, - last_link_id: 0, - nodes: [ - { - id: 1, - type: 'CheckpointLoaderSimple', - pos: [0, 0], - size: [100, 100], - flags: {}, - order: 0, - mode: 4, // BYPASS - properties: {}, - widgets_values: { ckpt_name: 'model.safetensors' } - } - ], - links: [], - groups: [], - config: {}, - extra: {}, - version: 0.4, - models: [ - { - name: 'model.safetensors', - url: 'https://example.com/model', + it.for([ + { state: 'muted', ancestorMode: 2 }, + { state: 'bypassed', ancestorMode: 4 } + ])( + 'does not enrich from metadata inside a $state ancestor subgraph', + ({ ancestorMode }) => { + const candidates = [ + makeCandidate('shared_model.safetensors', { directory: 'checkpoints' - } + }) ] - }) - - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) - - expect(result).toHaveLength(0) - }) - - it('drops workflow-level entries when only reference is in a bypassed subgraph interior', async () => { - // Interior properties.models references the workflow-level model - // but its widget value does not — forcing the workflow-level entry - // down the unmatched path where isModelReferencedByActiveNode - // decides. Previously the helper ignored the bypassed container. - const result = await enrichWithEmbeddedMetadata( - [], - fromAny(bypassedSubgraphUnmatchedModel), - alwaysMissing - ) - - expect(result).toHaveLength(0) - }) + const graphData = fromPartial({ + last_node_id: 2, + last_link_id: 0, + nodes: [ + { + id: 1, + type: 'CheckpointLoaderSimple', + pos: [0, 0], + size: [100, 100], + flags: {}, + order: 0, + mode: 0, + properties: {}, + widgets_values: { ckpt_name: 'shared_model.safetensors' } + }, + { + id: 2, + type: 'InactiveSubgraph', + pos: [200, 0], + size: [100, 100], + flags: {}, + order: 1, + mode: ancestorMode, + properties: {}, + widgets_values: {} + } + ], + links: [], + groups: [], + config: {}, + extra: {}, + version: 0.4, + models: [], + definitions: { + subgraphs: [ + { + id: 'InactiveSubgraph', + name: 'InactiveSubgraph', + nodes: [ + { + id: 10, + type: 'CheckpointLoaderSimple', + pos: [0, 0], + size: [100, 100], + flags: {}, + order: 0, + mode: 0, + properties: { + models: [ + { + name: 'shared_model.safetensors', + url: 'https://example.com/inactive-branch', + directory: 'checkpoints', + hash: 'inactive-hash', + hash_type: 'sha256' + } + ] + }, + widgets_values: { + ckpt_name: 'shared_model.safetensors' + } + } + ], + links: [], + groups: [], + config: {}, + extra: {}, + inputNode: {}, + outputNode: {} + } + ] + } + }) - it('keeps workflow-level entries when reference is in an active subgraph interior', async () => { - // Positive control for the bypassed case above: identical fixture - // with container mode=0 must still surface the unmatched workflow- - // level model. Guards against a regression where the ancestor gate - // drops every workflow-level entry regardless of context. - const result = await enrichWithEmbeddedMetadata( - [], - fromAny(activeSubgraphUnmatchedModel), - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(1) - expect(result[0].name).toBe('rare_model.safetensors') - }) + expect(result[0].url).toBeUndefined() + expect(result[0].hash).toBeUndefined() + expect(result[0].hashType).toBeUndefined() + } + ) - it('drops workflow-level entries when interior reference is under a different directory', async () => { - // Same name, different directory: the interior's properties.models - // entry is not the same asset as the workflow-level entry, so the - // fallback helper must not treat it as a reference that keeps the - // workflow-level model alive. + it('does not enrich candidates from different-directory metadata', () => { + const candidates = [ + makeCandidate('collide_model.safetensors', { + directory: 'checkpoints' + }) + ] const graphData = fromPartial({ last_node_id: 1, last_link_id: 0, @@ -1132,43 +1105,19 @@ describe('enrichWithEmbeddedMetadata', () => { { name: 'collide_model.safetensors', url: 'https://example.com/collide', - directory: 'checkpoints' + directory: 'loras' } ] }) - const result = await enrichWithEmbeddedMetadata( - [], - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(0) + expect(result[0].url).toBeUndefined() }) }) describe('OSS missing model detection (non-Cloud path)', () => { - it('scanAllModelCandidates returns empty array when not called (simulating isCloud === false guard)', () => { - // In the app, when isCloud is false, scanAllModelCandidates is not called - // and an empty array is used instead. This test verifies the OSS path - // starts with an empty candidates list. - const isCloud = false - const graph = makeGraph([ - makeNode(1, 'CheckpointLoaderSimple', [ - makeComboWidget('ckpt_name', 'missing_model.safetensors', []) - ]) - ]) - - const modelCandidates = isCloud - ? scanAllModelCandidates(graph, noAssetSupport) - : [] - - expect(modelCandidates).toEqual([]) - }) - - it('enrichWithEmbeddedMetadata detects missing embedded models without prior COMBO scan (OSS dialog path)', async () => { - // OSS path: candidates start empty, enrichWithEmbeddedMetadata adds - // missing embedded models so the dialog can show them. + it('does not detect embedded models without prior candidate scan', () => { const candidates: MissingModelCandidate[] = [] const graphData = fromPartial({ last_node_id: 2, @@ -1216,67 +1165,15 @@ describe('OSS missing model detection (non-Cloud path)', () => { ] }) - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) - expect(result).toHaveLength(2) - expect(result.every((c) => c.isMissing === true)).toBe(true) - expect(result.map((c) => c.name)).toEqual([ - 'sd_xl_base_1.0.safetensors', - 'detail_enhancer.safetensors' - ]) - }) - - it('enrichWithEmbeddedMetadata sets isMissing=true when isAssetSupported is not provided (OSS)', async () => { - // When isAssetSupported is omitted (OSS), unmatched embedded models - // should have isMissing=true (not undefined), enabling the dialog. - const candidates: MissingModelCandidate[] = [] - const graphData = fromPartial({ - last_node_id: 1, - last_link_id: 0, - nodes: [ - { - id: 1, - type: 'CheckpointLoaderSimple', - pos: [0, 0], - size: [100, 100], - flags: {}, - order: 0, - mode: 0, - properties: {}, - widgets_values: { ckpt_name: 'missing_model.safetensors' } - } - ], - links: [], - groups: [], - config: {}, - extra: {}, - version: 0.4, - models: [ - { - name: 'missing_model.safetensors', - url: 'https://example.com/model', - directory: 'checkpoints' - } - ] - }) - - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing - ) - - expect(result).toHaveLength(1) - expect(result[0].isMissing).toBe(true) - expect(result[0].isAssetSupported).toBe(false) + expect(result).toHaveLength(0) }) - it('enrichWithEmbeddedMetadata correctly filters for dialog: only isMissing=true with url', async () => { - const candidates: MissingModelCandidate[] = [] + it('enriches live OSS candidates for dialog filtering', () => { + const candidates: MissingModelCandidate[] = [ + makeCandidate('missing_model.safetensors') + ] const graphData = fromPartial({ last_node_id: 1, last_link_id: 0, @@ -1312,64 +1209,13 @@ describe('OSS missing model detection (non-Cloud path)', () => { ] }) - const selectiveInstallCheck = async (name: string) => - name === 'installed_model.safetensors' - - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - selectiveInstallCheck - ) + const result = enrichWithEmbeddedMetadata(candidates, graphData) const dialogModels = result.filter((c) => c.isMissing === true && c.url) expect(dialogModels).toHaveLength(1) expect(dialogModels[0].name).toBe('missing_model.safetensors') expect(dialogModels[0].url).toBe('https://example.com/model') }) - - it('enrichWithEmbeddedMetadata with isAssetSupported leaves isMissing undefined for asset-supported models (Cloud path)', async () => { - const candidates: MissingModelCandidate[] = [] - const graphData = fromPartial({ - last_node_id: 1, - last_link_id: 0, - nodes: [ - { - id: 1, - type: 'CheckpointLoaderSimple', - pos: [0, 0], - size: [100, 100], - flags: {}, - order: 0, - mode: 0, - properties: {}, - widgets_values: { ckpt_name: 'model.safetensors' } - } - ], - links: [], - groups: [], - config: {}, - extra: {}, - version: 0.4, - models: [ - { - name: 'model.safetensors', - url: 'https://example.com/model', - directory: 'checkpoints' - } - ] - }) - - const result = await enrichWithEmbeddedMetadata( - candidates, - graphData, - alwaysMissing, - () => true - ) - - expect(result).toHaveLength(1) - expect(result[0].isMissing).toBeUndefined() - expect(result[0].isAssetSupported).toBe(true) - }) }) const { mockUpdateModelsForNodeType, mockGetAssets } = vi.hoisted(() => ({ diff --git a/src/platform/missingModel/missingModelScan.ts b/src/platform/missingModel/missingModelScan.ts index 34708b4a631..2731ed3cb7e 100644 --- a/src/platform/missingModel/missingModelScan.ts +++ b/src/platform/missingModel/missingModelScan.ts @@ -1,11 +1,7 @@ import type { ModelFile } from '@/platform/workflow/validation/schemas/workflowSchema' import type { FlattenableWorkflowGraph } from '@/platform/workflow/core/utils/workflowFlattening' import { flattenWorkflowNodes } from '@/platform/workflow/core/utils/workflowFlattening' -import type { - MissingModelCandidate, - MissingModelViewModel, - EmbeddedModelWithSource -} from './types' +import type { MissingModelCandidate, MissingModelViewModel } from './types' import { getAssetFilename } from '@/platform/assets/utils/assetMetadataUtils' import type { AssetItem } from '@/platform/assets/schemas/assetSchema' // eslint-disable-next-line import-x/no-restricted-paths @@ -17,13 +13,13 @@ import type { IBaseWidget, IComboWidget } from '@/lib/litegraph/src/types/widgets' -import { getParentExecutionIds } from '@/types/nodeIdentification' import { collectAllNodes, getExecutionIdByNode } from '@/utils/graphTraversalUtil' import { LGraphEventMode } from '@/lib/litegraph/src/types/globalEnums' import { resolveComboValues } from '@/utils/litegraphUtil' +import { getParentExecutionIds } from '@/types/nodeIdentification' export type MissingModelWorkflowData = FlattenableWorkflowGraph & { models?: ModelFile[] @@ -70,6 +66,10 @@ function isAssetWidget(widget: IBaseWidget): widget is IAssetWidget { return widget.type === 'asset' } +function isInactiveMode(mode: number | undefined): boolean { + return mode === LGraphEventMode.NEVER || mode === LGraphEventMode.BYPASS +} + // Full set of model file extensions used for scanning candidate widgets. // Intentionally broader than ALLOWED_SUFFIXES in missingModelDownload.ts, // which restricts which files are eligible for download. @@ -111,11 +111,7 @@ export function scanAllModelCandidates( // Skip subgraph container nodes: their promoted widgets are synthetic // views of interior widgets, which are already scanned via recursion. if (node.isSubgraphNode?.()) continue - if ( - node.mode === LGraphEventMode.NEVER || - node.mode === LGraphEventMode.BYPASS - ) - continue + if (isInactiveMode(node.mode)) continue candidates.push( ...scanNodeModelCandidates( @@ -217,14 +213,12 @@ function scanComboWidget( } } -export async function enrichWithEmbeddedMetadata( +export function enrichWithEmbeddedMetadata( candidates: readonly MissingModelCandidate[], - graphData: MissingModelWorkflowData, - checkModelInstalled: (name: string, directory: string) => Promise, - isAssetSupported?: (nodeType: string, widgetName: string) => boolean -): Promise { + graphData: MissingModelWorkflowData +): MissingModelCandidate[] { const allNodes = flattenWorkflowNodes(graphData) - const embeddedModels = collectEmbeddedModelsWithSource(allNodes, graphData) + const embeddedModels = collectEmbeddedModels(allNodes, graphData) const enriched = candidates.map((c) => ({ ...c })) const candidatesByKey = new Map() @@ -240,7 +234,7 @@ export async function enrichWithEmbeddedMetadata( else candidatesByKey.set(nameKey, [c]) } - const deduped: EmbeddedModelWithSource[] = [] + const deduped: ModelFile[] = [] const enrichedKeys = new Set() for (const model of embeddedModels) { const dedupeKey = `${model.name}::${model.directory}` @@ -249,195 +243,60 @@ export async function enrichWithEmbeddedMetadata( deduped.push(model) } - const unmatched: EmbeddedModelWithSource[] = [] for (const model of deduped) { const dirKey = `${model.name}::${model.directory}` const exact = candidatesByKey.get(dirKey) const fallback = candidatesByKey.get(model.name) const existing = exact?.length ? exact : fallback - if (existing) { - for (const c of existing) { - if (c.directory && c.directory !== model.directory) continue - c.directory ??= model.directory - c.url ??= model.url - c.hash ??= model.hash - c.hashType ??= model.hash_type - } - } else { - unmatched.push(model) + if (!existing) continue + for (const c of existing) { + if (c.directory && c.directory !== model.directory) continue + c.directory ??= model.directory + c.url ??= model.url + c.hash ??= model.hash + c.hashType ??= model.hash_type } } - // Workflow-level entries (sourceNodeType === '') survive only when - // some active (non-muted, non-bypassed) node actually references the - // model — not merely because any unrelated active node exists. A - // reference is any widget value (or node.properties.models entry) - // that matches the model name on an active node. - // Hoist the id→node map once; isModelReferencedByActiveNode would - // otherwise rebuild it on every unmatched entry. - const flattenedNodeById = new Map(allNodes.map((n) => [String(n.id), n])) - const activeUnmatched = unmatched.filter( - (m) => - m.sourceNodeType !== '' || - isModelReferencedByActiveNode( - m.name, - m.directory, - allNodes, - flattenedNodeById - ) - ) - - const settled = await Promise.allSettled( - activeUnmatched.map(async (model) => { - const installed = await checkModelInstalled(model.name, model.directory) - if (installed) return null - - const nodeIsAssetSupported = isAssetSupported - ? isAssetSupported(model.sourceNodeType, model.sourceWidgetName) - : false - - return { - nodeId: model.sourceNodeId, - nodeType: model.sourceNodeType, - widgetName: model.sourceWidgetName, - isAssetSupported: nodeIsAssetSupported, - name: model.name, - directory: model.directory, - url: model.url, - hash: model.hash, - hashType: model.hash_type, - isMissing: nodeIsAssetSupported ? undefined : true - } satisfies MissingModelCandidate - }) - ) - - for (const r of settled) { - if (r.status === 'rejected') { - console.warn( - '[Missing Model Pipeline] checkModelInstalled failed:', - r.reason - ) - continue - } - if (r.value) enriched.push(r.value) - } - return enriched } -function isModelReferencedByActiveNode( - modelName: string, - modelDirectory: string | undefined, - allNodes: ReturnType, - nodeById: Map[number]> -): boolean { - for (const node of allNodes) { - if ( - node.mode === LGraphEventMode.NEVER || - node.mode === LGraphEventMode.BYPASS - ) - continue - if (!isAncestorPathActiveInFlattened(String(node.id), nodeById)) continue - - // Require directory agreement when both sides specify one, so a - // same-name entry under a different folder does not keep an - // unrelated workflow-level model alive as missing. - const embeddedModels = ( - node.properties as - | { models?: Array<{ name: string; directory?: string }> } - | undefined - )?.models - if ( - embeddedModels?.some( - (m) => - m.name === modelName && - (modelDirectory === undefined || - m.directory === undefined || - m.directory === modelDirectory) - ) - ) { - return true - } - - // widgets_values carries only the name, so directory cannot be - // checked here — fall back to filename matching. - const values = node.widgets_values - if (!values) continue - const valueArray = Array.isArray(values) ? values : Object.values(values) - for (const v of valueArray) { - if (typeof v === 'string' && v === modelName) return true - } - } - return false -} - -function isAncestorPathActiveInFlattened( - executionId: string, - nodeById: Map[number]> -): boolean { - for (const ancestorId of getParentExecutionIds(executionId)) { - const ancestor = nodeById.get(ancestorId) - if (!ancestor) continue - if ( - ancestor.mode === LGraphEventMode.NEVER || - ancestor.mode === LGraphEventMode.BYPASS - ) - return false - } - return true -} - -function collectEmbeddedModelsWithSource( +function collectEmbeddedModels( allNodes: ReturnType, graphData: MissingModelWorkflowData -): EmbeddedModelWithSource[] { - const result: EmbeddedModelWithSource[] = [] +): ModelFile[] { + const result: ModelFile[] = [] + const nodesById = new Map(allNodes.map((node) => [String(node.id), node])) for (const node of allNodes) { - if ( - node.mode === LGraphEventMode.NEVER || - node.mode === LGraphEventMode.BYPASS - ) - continue + if (!isNodeAndAncestorsActive(node, nodesById)) continue const selected = getSelectedModelsMetadata(node) if (!selected?.length) continue - for (const model of selected) { - result.push({ - ...model, - sourceNodeId: node.id, - sourceNodeType: node.type, - sourceWidgetName: findWidgetNameForModel(node, model.name) - }) - } + result.push(...selected) } - // Workflow-level model entries have no originating node; sourceNodeId - // remains undefined and empty-string node type/widget are handled by - // groupCandidatesByName (no nodeId → no referencing node entry). - if (graphData.models?.length) { - for (const model of graphData.models) { - result.push({ - ...model, - sourceNodeType: '', - sourceWidgetName: '' - }) - } - } + if (graphData.models?.length) result.push(...graphData.models) return result } -function findWidgetNameForModel( +function isNodeAndAncestorsActive( node: ReturnType[number], - modelName: string -): string { - if (Array.isArray(node.widgets_values) || !node.widgets_values) return '' - for (const [key, val] of Object.entries(node.widgets_values)) { - if (val === modelName) return key + nodesById: ReadonlyMap< + string, + ReturnType[number] + > +): boolean { + if (isInactiveMode(node.mode)) return false + + for (const ancestorId of getParentExecutionIds(String(node.id))) { + const ancestor = nodesById.get(ancestorId) + if (isInactiveMode(ancestor?.mode)) return false } - return '' + + return true } interface AssetVerifier { diff --git a/src/platform/missingModel/types.ts b/src/platform/missingModel/types.ts index dd0b8631276..96478d69b0a 100644 --- a/src/platform/missingModel/types.ts +++ b/src/platform/missingModel/types.ts @@ -1,7 +1,4 @@ -import type { - ModelFile, - NodeId -} from '@/platform/workflow/validation/schemas/workflowSchema' +import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema' /** * A single (node, widget, model) binding detected by the missing model pipeline. @@ -28,13 +25,6 @@ export interface MissingModelCandidate { isMissing: boolean | undefined } -export interface EmbeddedModelWithSource extends ModelFile { - /** Undefined for workflow-level models not tied to a specific node. */ - sourceNodeId?: NodeId - sourceNodeType: string - sourceWidgetName: string -} - /** View model grouping multiple candidate references under a single model name. */ export interface MissingModelViewModel { name: string