mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-26 16:05:11 +00:00
fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary Muted and bypassed nodes are excluded from execution but were still triggering missing model/media/node warnings. This PR makes the error system mode-aware: muted/bypassed nodes no longer produce missing asset errors, and all error lifecycle events (mode toggle, deletion, paste, undo, tab switch) are handled consistently. - Fixes Comfy-Org/ComfyUI#13256 ## Behavioral notes - **Tab switch overlay suppression (intentional)**: Switching back to a workflow with missing assets no longer re-shows the error overlay. This reverses the behavior introduced in #10190. The error state is still restored silently in the errors tab — users can access it via the properties panel without being interrupted by the overlay on every tab switch. ## Changes ### 1. Scan filtering - `scanAllModelCandidates`, `scanAllMediaCandidates`, `scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS` - `collectMissingNodes` (serialized data): skip error reporting for muted/bypassed nodes while still calling `sanitizeNodeName` for safe `configure()` - `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes; workflow-level `graphData.models` only create candidates when active nodes exist - `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models when all referencing nodes are inactive ### 2. Realtime mode change handling - `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect `node:property:changed` (mode) - Deactivation (active → muted/bypassed): remove missing model/media/node errors for the node - Activation (muted/bypassed → active): scan the node and add confirmed errors, show overlay - Subgraph container deactivation: remove all interior node errors (execution ID prefix match) - Subgraph container activation: scan all active interior nodes recursively - Subgraph interior mode change: resolve node via `localGraph.getNodeById()` then compute execution ID from root graph ### 3. Node deletion - `graph.onNodeRemoved`: remove missing model/media/node errors for the deleted node - Handle `node.graph === null` at callback time by using `String(node.id)` for root-level nodes ### 4. Node paste/duplicate - `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after `node.configure()` restores widget values) - Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab switch handled by pipeline) - Guard: skip muted/bypassed nodes ### 5. Workflow tab switch optimization - `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab switch - Cache missing model/media/node state per workflow via `PendingWarnings` - `beforeLoadNewGraph`: save current store state to outgoing workflow's `pendingWarnings` - `showPendingWarnings`: restore cached errors silently (no overlay), always sync missing nodes store (even when null) - Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using `setMissingModels([])` instead of `clearMissingModels()` - `MissingModelRow.vue`: fetch file size on mount via `fetchModelMetadata` memory cache ### 6. Undo/redo overlay suppression - `silentAssetErrors` option propagated through pipeline → `surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option - `showPendingWarnings` `{ silent }` option for missing nodes overlay - `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo ### 7. Error tab node filtering - Selected node filters missing model/media card contents (not just group visibility) - `isAssetErrorInSelection`: resolve execution ID → graph node for selection matching - Missing nodes intentionally unfiltered (pack-level scope) - `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab visibility - Download All button: show only when 2+ downloadable models exist ### 8. New store functions - `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId` - `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId` - `missingNodesErrorStore`: `removeMissingNodesByNodeId` - `missingModelScan`: `scanNodeModelCandidates` (extracted single-node scan) - `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node scan) ### 9. Test infrastructure improvements - `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`) - Error-related TestIds moved from `dialogs` to `errorsTab` namespace in `selectors.ts` - Removed unused `TestIdValue` type - Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts` - Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab` - Added `aria-label` to pencil edit button and subgraph toggle button ## Test plan ### Unit tests (41 new) - Store functions: `addMissing*`, `removeMissing*ByNodeId` - `executionErrorStore`: `surfaceMissing*` silent option - Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`, `scanNodeMediaCandidates` - `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph` caching ### E2E tests (17 new in `errorsTabModeAware.spec.ts`) **Missing nodes** - [x] Deleting a missing node removes its error from the errors tab - [x] Undo after bypass restores error without showing overlay **Missing models** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Deleting a node with missing model removes its error - [x] Undo after bypass restores error without showing overlay - [x] Pasting a node with missing model increases referencing node count - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Missing media** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Subgraph** - [x] Bypassing a subgraph hides interior errors, un-bypassing restores them - [x] Bypassing a node inside a subgraph hides its error, un-bypassing restores it **Workflow switching** - [x] Does not resurface error overlay when switching back to workflow with missing nodes - [x] Restores missing nodes in errors tab when switching back to workflow # Screenshots https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5 ## Follow-up work - Extract error-detection computed properties from `RightSidePanel.vue` into a composable (e.g. `useErrorsTabVisibility`) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
@@ -9,6 +9,7 @@ import type {
|
||||
} from '@/lib/litegraph/src/types/widgets'
|
||||
import {
|
||||
scanAllModelCandidates,
|
||||
scanNodeModelCandidates,
|
||||
isModelFileName,
|
||||
enrichWithEmbeddedMetadata,
|
||||
verifyAssetSupportedCandidates,
|
||||
@@ -111,6 +112,52 @@ describe('MODEL_FILE_EXTENSIONS', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('scanNodeModelCandidates', () => {
|
||||
it('returns candidates for a node with a missing model combo widget', () => {
|
||||
const graph = makeGraph([])
|
||||
const node = makeNode(1, 'CheckpointLoaderSimple', [
|
||||
makeComboWidget('ckpt_name', 'missing_model.safetensors', [
|
||||
'existing_model.safetensors'
|
||||
])
|
||||
])
|
||||
|
||||
const result = scanNodeModelCandidates(graph, node, noAssetSupport)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0]).toEqual({
|
||||
nodeId: '1',
|
||||
nodeType: 'CheckpointLoaderSimple',
|
||||
widgetName: 'ckpt_name',
|
||||
isAssetSupported: false,
|
||||
name: 'missing_model.safetensors',
|
||||
isMissing: true
|
||||
})
|
||||
})
|
||||
|
||||
it('returns empty array for node with no widgets', () => {
|
||||
const graph = makeGraph([])
|
||||
const node = makeNode(1, 'EmptyNode', [])
|
||||
|
||||
const result = scanNodeModelCandidates(graph, node, noAssetSupport)
|
||||
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty array when executionId is null', () => {
|
||||
const graph = makeGraph([])
|
||||
const node = makeNode(
|
||||
1,
|
||||
'CheckpointLoaderSimple',
|
||||
[makeComboWidget('ckpt_name', 'model.safetensors', [])],
|
||||
''
|
||||
)
|
||||
|
||||
const result = scanNodeModelCandidates(graph, node, noAssetSupport)
|
||||
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('scanAllModelCandidates', () => {
|
||||
it('should detect a missing model from a combo widget', () => {
|
||||
const graph = makeGraph([
|
||||
@@ -390,6 +437,58 @@ describe('scanAllModelCandidates', () => {
|
||||
expect(result[1].widgetName).toBe('vae_name')
|
||||
})
|
||||
|
||||
it('skips muted nodes (mode === NEVER)', () => {
|
||||
const mutedNode = fromAny<LGraphNode, unknown>({
|
||||
id: 10,
|
||||
type: 'CheckpointLoaderSimple',
|
||||
widgets: [
|
||||
makeComboWidget('ckpt_name', 'model.safetensors', ['other.safetensors'])
|
||||
],
|
||||
mode: 2, // LGraphEventMode.NEVER
|
||||
_testExecutionId: '10'
|
||||
})
|
||||
|
||||
const graph = makeGraph([mutedNode])
|
||||
const result = scanAllModelCandidates(graph, noAssetSupport)
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('skips bypassed nodes (mode === BYPASS)', () => {
|
||||
const bypassedNode = fromAny<LGraphNode, unknown>({
|
||||
id: 11,
|
||||
type: 'CheckpointLoaderSimple',
|
||||
widgets: [
|
||||
makeComboWidget('ckpt_name', 'model.safetensors', ['other.safetensors'])
|
||||
],
|
||||
mode: 4, // LGraphEventMode.BYPASS
|
||||
_testExecutionId: '11'
|
||||
})
|
||||
|
||||
const graph = makeGraph([bypassedNode])
|
||||
const result = scanAllModelCandidates(graph, noAssetSupport)
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('includes active nodes (mode === ALWAYS)', () => {
|
||||
const activeNode = fromAny<LGraphNode, unknown>({
|
||||
id: 12,
|
||||
type: 'CheckpointLoaderSimple',
|
||||
widgets: [
|
||||
makeComboWidget('ckpt_name', 'model.safetensors', ['other.safetensors'])
|
||||
],
|
||||
mode: 0, // LGraphEventMode.ALWAYS
|
||||
_testExecutionId: '12'
|
||||
})
|
||||
|
||||
const graph = makeGraph([activeNode])
|
||||
const result = scanAllModelCandidates(graph, noAssetSupport)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].isMissing).toBe(true)
|
||||
})
|
||||
|
||||
it('skips subgraph container nodes whose promoted widgets are already scanned via interior nodes', () => {
|
||||
const containerNode = fromAny<LGraphNode, unknown>({
|
||||
id: 65,
|
||||
@@ -638,6 +737,194 @@ describe('enrichWithEmbeddedMetadata', () => {
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('skips embedded models from muted nodes', async () => {
|
||||
const candidates: MissingModelCandidate[] = []
|
||||
const graphData = fromPartial<ComfyWorkflowJSON>({
|
||||
last_node_id: 1,
|
||||
last_link_id: 0,
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'CheckpointLoaderSimple',
|
||||
pos: [0, 0],
|
||||
size: [100, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 2, // NEVER (muted)
|
||||
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
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
|
||||
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[] = []
|
||||
const graphData = fromPartial<ComfyWorkflowJSON>({
|
||||
last_node_id: 2,
|
||||
last_link_id: 0,
|
||||
nodes: [
|
||||
{
|
||||
id: 1,
|
||||
type: 'CheckpointLoaderSimple',
|
||||
pos: [0, 0],
|
||||
size: [100, 100],
|
||||
flags: {},
|
||||
order: 0,
|
||||
mode: 4, // BYPASS — only node referencing the model
|
||||
properties: {},
|
||||
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: [],
|
||||
groups: [],
|
||||
config: {},
|
||||
extra: {},
|
||||
version: 0.4,
|
||||
models: [
|
||||
{
|
||||
name: 'model.safetensors',
|
||||
url: 'https://example.com/model',
|
||||
directory: 'checkpoints'
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
const result = await enrichWithEmbeddedMetadata(
|
||||
candidates,
|
||||
graphData,
|
||||
alwaysMissing
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
|
||||
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' })
|
||||
]
|
||||
const graphData = fromPartial<ComfyWorkflowJSON>({
|
||||
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: {
|
||||
models: [
|
||||
{
|
||||
name: 'node_model.safetensors',
|
||||
url: 'https://example.com/node_model',
|
||||
directory: 'checkpoints'
|
||||
}
|
||||
]
|
||||
},
|
||||
widgets_values: { ckpt_name: 'node_model.safetensors' }
|
||||
}
|
||||
],
|
||||
links: [],
|
||||
groups: [],
|
||||
config: {},
|
||||
extra: {},
|
||||
version: 0.4,
|
||||
models: []
|
||||
})
|
||||
|
||||
const result = await enrichWithEmbeddedMetadata(
|
||||
candidates,
|
||||
graphData,
|
||||
alwaysMissing
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].name).toBe('node_model.safetensors')
|
||||
})
|
||||
|
||||
it('skips embedded models from bypassed nodes', async () => {
|
||||
const candidates: MissingModelCandidate[] = []
|
||||
const graphData = fromPartial<ComfyWorkflowJSON>({
|
||||
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',
|
||||
directory: 'checkpoints'
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
const result = await enrichWithEmbeddedMetadata(
|
||||
candidates,
|
||||
graphData,
|
||||
alwaysMissing
|
||||
)
|
||||
|
||||
expect(result).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('OSS missing model detection (non-Cloud path)', () => {
|
||||
|
||||
Reference in New Issue
Block a user