mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-13 17:26:22 +00:00
## 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>
339 lines
8.9 KiB
TypeScript
339 lines
8.9 KiB
TypeScript
import { fromAny } from '@total-typescript/shoehorn'
|
|
import { describe, expect, it, vi } from 'vitest'
|
|
|
|
import type { LGraph } from '@/lib/litegraph/src/LGraph'
|
|
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
|
import type { IComboWidget } from '@/lib/litegraph/src/types/widgets'
|
|
import {
|
|
scanAllMediaCandidates,
|
|
scanNodeMediaCandidates,
|
|
verifyCloudMediaCandidates,
|
|
groupCandidatesByName,
|
|
groupCandidatesByMediaType
|
|
} from './missingMediaScan'
|
|
import type { MissingMediaCandidate } from './types'
|
|
|
|
vi.mock('@/utils/graphTraversalUtil', () => ({
|
|
collectAllNodes: (graph: { _testNodes: LGraphNode[] }) => graph._testNodes,
|
|
getExecutionIdByNode: (
|
|
_graph: unknown,
|
|
node: { _testExecutionId?: string; id: number }
|
|
) => node._testExecutionId ?? String(node.id)
|
|
}))
|
|
|
|
function makeCandidate(
|
|
nodeId: string,
|
|
name: string,
|
|
overrides: Partial<MissingMediaCandidate> = {}
|
|
): MissingMediaCandidate {
|
|
return {
|
|
nodeId,
|
|
nodeType: 'LoadImage',
|
|
widgetName: 'image',
|
|
mediaType: 'image',
|
|
name,
|
|
isMissing: true,
|
|
...overrides
|
|
}
|
|
}
|
|
|
|
function makeMediaCombo(
|
|
name: string,
|
|
value: string,
|
|
options: string[] = []
|
|
): IComboWidget {
|
|
return fromAny<IComboWidget, unknown>({
|
|
type: 'combo',
|
|
name,
|
|
value,
|
|
options: { values: options }
|
|
})
|
|
}
|
|
|
|
function makeMediaNode(
|
|
id: number,
|
|
type: string,
|
|
widgets: IComboWidget[],
|
|
mode: number = 0,
|
|
executionId?: string
|
|
): LGraphNode {
|
|
return fromAny<LGraphNode, unknown>({
|
|
id,
|
|
type,
|
|
widgets,
|
|
mode,
|
|
_testExecutionId: executionId ?? String(id)
|
|
})
|
|
}
|
|
|
|
function makeGraph(nodes: LGraphNode[]): LGraph {
|
|
return fromAny<LGraph, unknown>({ _testNodes: nodes })
|
|
}
|
|
|
|
describe('scanNodeMediaCandidates', () => {
|
|
it('returns candidate for a LoadImage node with missing image', () => {
|
|
const graph = makeGraph([])
|
|
const node = makeMediaNode(
|
|
1,
|
|
'LoadImage',
|
|
[makeMediaCombo('image', 'photo.png', ['other.png'])],
|
|
0
|
|
)
|
|
|
|
const result = scanNodeMediaCandidates(graph, node, false)
|
|
|
|
expect(result).toHaveLength(1)
|
|
expect(result[0]).toEqual({
|
|
nodeId: '1',
|
|
nodeType: 'LoadImage',
|
|
widgetName: 'image',
|
|
mediaType: 'image',
|
|
name: 'photo.png',
|
|
isMissing: true
|
|
})
|
|
})
|
|
|
|
it('returns empty for non-media node types', () => {
|
|
const graph = makeGraph([])
|
|
const node = makeMediaNode(
|
|
1,
|
|
'KSampler',
|
|
[makeMediaCombo('sampler', 'euler', ['euler', 'dpm'])],
|
|
0
|
|
)
|
|
|
|
const result = scanNodeMediaCandidates(graph, node, false)
|
|
|
|
expect(result).toEqual([])
|
|
})
|
|
|
|
it('returns empty for node with no widgets', () => {
|
|
const graph = makeGraph([])
|
|
const node = makeMediaNode(1, 'LoadImage', [], 0)
|
|
|
|
const result = scanNodeMediaCandidates(graph, node, false)
|
|
|
|
expect(result).toEqual([])
|
|
})
|
|
})
|
|
|
|
describe('scanAllMediaCandidates', () => {
|
|
it('skips muted nodes (mode === NEVER)', () => {
|
|
const node = makeMediaNode(
|
|
1,
|
|
'LoadImage',
|
|
[makeMediaCombo('image', 'photo.png', ['other.png'])],
|
|
2 // NEVER
|
|
)
|
|
const result = scanAllMediaCandidates(makeGraph([node]), false)
|
|
expect(result).toHaveLength(0)
|
|
})
|
|
|
|
it('skips bypassed nodes (mode === BYPASS)', () => {
|
|
const node = makeMediaNode(
|
|
2,
|
|
'LoadImage',
|
|
[makeMediaCombo('image', 'photo.png', ['other.png'])],
|
|
4 // BYPASS
|
|
)
|
|
const result = scanAllMediaCandidates(makeGraph([node]), false)
|
|
expect(result).toHaveLength(0)
|
|
})
|
|
|
|
it('includes active nodes (mode === ALWAYS)', () => {
|
|
const node = makeMediaNode(
|
|
3,
|
|
'LoadImage',
|
|
[makeMediaCombo('image', 'photo.png', ['other.png'])],
|
|
0 // ALWAYS
|
|
)
|
|
const result = scanAllMediaCandidates(makeGraph([node]), false)
|
|
expect(result).toHaveLength(1)
|
|
expect(result[0].isMissing).toBe(true)
|
|
})
|
|
})
|
|
|
|
describe('groupCandidatesByName', () => {
|
|
it('groups candidates with the same name', () => {
|
|
const candidates = [
|
|
makeCandidate('1', 'photo.png'),
|
|
makeCandidate('2', 'photo.png'),
|
|
makeCandidate('3', 'other.png')
|
|
]
|
|
|
|
const result = groupCandidatesByName(candidates)
|
|
expect(result).toHaveLength(2)
|
|
|
|
const photoGroup = result.find((g) => g.name === 'photo.png')
|
|
expect(photoGroup?.referencingNodes).toHaveLength(2)
|
|
expect(photoGroup?.mediaType).toBe('image')
|
|
|
|
const otherGroup = result.find((g) => g.name === 'other.png')
|
|
expect(otherGroup?.referencingNodes).toHaveLength(1)
|
|
})
|
|
|
|
it('returns empty array for empty input', () => {
|
|
expect(groupCandidatesByName([])).toEqual([])
|
|
})
|
|
})
|
|
|
|
describe('groupCandidatesByMediaType', () => {
|
|
it('groups by media type in order: image, video, audio', () => {
|
|
const candidates = [
|
|
makeCandidate('1', 'sound.mp3', {
|
|
nodeType: 'LoadAudio',
|
|
widgetName: 'audio',
|
|
mediaType: 'audio'
|
|
}),
|
|
makeCandidate('2', 'photo.png'),
|
|
makeCandidate('3', 'clip.mp4', {
|
|
nodeType: 'LoadVideo',
|
|
widgetName: 'file',
|
|
mediaType: 'video'
|
|
})
|
|
]
|
|
|
|
const result = groupCandidatesByMediaType(candidates)
|
|
expect(result).toHaveLength(3)
|
|
expect(result[0].mediaType).toBe('image')
|
|
expect(result[1].mediaType).toBe('video')
|
|
expect(result[2].mediaType).toBe('audio')
|
|
})
|
|
|
|
it('omits media types with no candidates', () => {
|
|
const candidates = [
|
|
makeCandidate('1', 'clip.mp4', {
|
|
nodeType: 'LoadVideo',
|
|
widgetName: 'file',
|
|
mediaType: 'video'
|
|
})
|
|
]
|
|
|
|
const result = groupCandidatesByMediaType(candidates)
|
|
expect(result).toHaveLength(1)
|
|
expect(result[0].mediaType).toBe('video')
|
|
})
|
|
|
|
it('groups multiple names within the same media type', () => {
|
|
const candidates = [
|
|
makeCandidate('1', 'a.png'),
|
|
makeCandidate('2', 'b.png'),
|
|
makeCandidate('3', 'a.png')
|
|
]
|
|
|
|
const result = groupCandidatesByMediaType(candidates)
|
|
expect(result).toHaveLength(1)
|
|
expect(result[0].mediaType).toBe('image')
|
|
expect(result[0].items).toHaveLength(2)
|
|
expect(
|
|
result[0].items.find((i) => i.name === 'a.png')?.referencingNodes
|
|
).toHaveLength(2)
|
|
})
|
|
})
|
|
|
|
describe('verifyCloudMediaCandidates', () => {
|
|
it('marks candidates missing when not in input assets', async () => {
|
|
const candidates = [
|
|
makeCandidate('1', 'abc123.png', { isMissing: undefined }),
|
|
makeCandidate('2', 'def456.png', { isMissing: undefined })
|
|
]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {},
|
|
inputAssets: [{ asset_hash: 'def456.png', name: 'my-photo.png' }]
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, undefined, mockStore)
|
|
|
|
expect(candidates[0].isMissing).toBe(true)
|
|
expect(candidates[1].isMissing).toBe(false)
|
|
})
|
|
|
|
it('calls updateInputs before checking assets', async () => {
|
|
let updateCalled = false
|
|
const candidates = [makeCandidate('1', 'abc.png', { isMissing: undefined })]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {
|
|
updateCalled = true
|
|
},
|
|
inputAssets: []
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, undefined, mockStore)
|
|
|
|
expect(updateCalled).toBe(true)
|
|
})
|
|
|
|
it('respects abort signal before execution', async () => {
|
|
const controller = new AbortController()
|
|
controller.abort()
|
|
|
|
const candidates = [
|
|
makeCandidate('1', 'abc123.png', { isMissing: undefined })
|
|
]
|
|
|
|
await verifyCloudMediaCandidates(candidates, controller.signal)
|
|
|
|
expect(candidates[0].isMissing).toBeUndefined()
|
|
})
|
|
|
|
it('respects abort signal after updateInputs', async () => {
|
|
const controller = new AbortController()
|
|
const candidates = [makeCandidate('1', 'abc.png', { isMissing: undefined })]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {
|
|
controller.abort()
|
|
},
|
|
inputAssets: [{ asset_hash: 'abc.png', name: 'photo.png' }]
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, controller.signal, mockStore)
|
|
|
|
expect(candidates[0].isMissing).toBeUndefined()
|
|
})
|
|
|
|
it('skips candidates already resolved as true', async () => {
|
|
const candidates = [makeCandidate('1', 'abc.png', { isMissing: true })]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {},
|
|
inputAssets: []
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, undefined, mockStore)
|
|
|
|
expect(candidates[0].isMissing).toBe(true)
|
|
})
|
|
|
|
it('skips candidates already resolved as false', async () => {
|
|
const candidates = [makeCandidate('1', 'abc.png', { isMissing: false })]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {},
|
|
inputAssets: []
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, undefined, mockStore)
|
|
|
|
expect(candidates[0].isMissing).toBe(false)
|
|
})
|
|
|
|
it('skips entirely when no pending candidates', async () => {
|
|
let updateCalled = false
|
|
const candidates = [makeCandidate('1', 'abc.png', { isMissing: true })]
|
|
|
|
const mockStore = {
|
|
updateInputs: async () => {
|
|
updateCalled = true
|
|
},
|
|
inputAssets: []
|
|
}
|
|
|
|
await verifyCloudMediaCandidates(candidates, undefined, mockStore)
|
|
|
|
expect(updateCalled).toBe(false)
|
|
})
|
|
})
|