From 31a33a0ba2d399aa33aa54cf4b7e4c03284bf68e Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Fri, 13 Mar 2026 23:49:44 +0900 Subject: [PATCH] feat: auto-resolve simple validation errors on widget change and slot connection (#9464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Automatically clears transient validation errors (`value_bigger_than_max`, `value_smaller_than_min`, `value_not_in_list`, `required_input_missing`) when the user modifies a widget value or connects an input slot, so resolved errors don't linger in the error panel. Also clears missing model state when the user changes a combo widget value. ## Changes - **`useNodeErrorAutoResolve` composable**: watches widget changes and slot connections, clears matching errors via `executionErrorStore` - **`executionErrorStore`**: adds `clearSimpleNodeErrors` and `clearSimpleWidgetErrorIfValid` with granular per-slot error removal - **`executionErrorUtil`**: adds `isValueStillOutOfRange` to prevent premature clearing when a new value still violates the constraint - **`graphTraversalUtil`**: adds `getExecutionIdFromNodeData` for subgraph-aware execution ID resolution - **`GraphCanvas.vue`**: fixes subgraph error key lookup by using `getExecutionIdByNode` instead of raw `node.id` - **`NodeWidgets.vue`**: wires up the new composable to the widget layer - **`missingModelStore`**: adds `removeMissingModelByWidget` to clear missing model state on widget value change - **`useGraphNodeManager`**: registers composable per node - **Tests**: 126 new unit tests covering error clearing, range validation, and graph traversal edge cases ## Screenshots https://github.com/user-attachments/assets/515ea811-ff84-482a-a866-a17e5c779c39 https://github.com/user-attachments/assets/a2b30f02-4929-4537-952c-a0febe20f02e ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9464-feat-auto-resolve-simple-validation-errors-on-widget-change-and-slot-connection-31b6d73d3650816b8afdc34f4b40295a) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 --- src/components/graph/GraphCanvas.vue | 53 ++- .../graph/useErrorClearingHooks.test.ts | 352 +++++++++++++++++ .../graph/useErrorClearingHooks.ts | 108 ++++++ .../graph/useGraphNodeManager.test.ts | 280 ++++++++++++++ src/composables/graph/useGraphNodeManager.ts | 25 ++ .../missingModel/missingModelScan.test.ts | 26 ++ src/platform/missingModel/missingModelScan.ts | 3 + .../missingModel/missingModelStore.test.ts | 58 +++ .../missingModel/missingModelStore.ts | 11 + .../vueNodes/components/NodeWidgets.vue | 72 +++- src/stores/executionErrorStore.test.ts | 366 ++++++++++++++++++ src/stores/executionErrorStore.ts | 212 ++++++++-- src/utils/executionErrorUtil.test.ts | 154 +++++++- src/utils/executionErrorUtil.ts | 35 ++ src/utils/graphTraversalUtil.test.ts | 88 ++++- src/utils/graphTraversalUtil.ts | 52 ++- 16 files changed, 1790 insertions(+), 105 deletions(-) create mode 100644 src/composables/graph/useErrorClearingHooks.test.ts create mode 100644 src/composables/graph/useErrorClearingHooks.ts diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 92dc81ba9b..48ffc52092 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -143,6 +143,7 @@ import TopbarBadges from '@/components/topbar/TopbarBadges.vue' import TopbarSubscribeButton from '@/components/topbar/TopbarSubscribeButton.vue' import WorkflowTabs from '@/components/topbar/WorkflowTabs.vue' import { useChainCallback } from '@/composables/functional/useChainCallback' +import { installErrorClearingHooks } from '@/composables/graph/useErrorClearingHooks' import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle' import { useNodeBadge } from '@/composables/node/useNodeBadge' @@ -245,6 +246,16 @@ const { shouldRenderVueNodes } = useVueFeatureFlags() // Vue node system const vueNodeLifecycle = useVueNodeLifecycle() +// Error-clearing hooks run regardless of rendering mode (Vue or legacy canvas). +let cleanupErrorHooks: (() => void) | null = null +watch( + () => canvasStore.currentGraph, + (graph) => { + cleanupErrorHooks?.() + cleanupErrorHooks = graph ? installErrorClearingHooks(graph) : null + } +) + const handleVueNodeLifecycleReset = async () => { if (shouldRenderVueNodes.value) { vueNodeLifecycle.disposeNodeManagerAndSyncs() @@ -391,39 +402,12 @@ watch( } ) -// Update node slot errors for LiteGraph nodes -// (Vue nodes read from store directly) +// Repaint canvas when node errors change. +// Slot error flags are reconciled by reconcileNodeErrorFlags in executionErrorStore. watch( () => executionErrorStore.lastNodeErrors, - (lastNodeErrors) => { - if (!comfyApp.graph) return - - forEachNode(comfyApp.rootGraph, (node) => { - // Clear existing errors - for (const slot of node.inputs) { - delete slot.hasErrors - } - for (const slot of node.outputs) { - delete slot.hasErrors - } - - const nodeErrors = lastNodeErrors?.[node.id] - if (!nodeErrors) return - - const validErrors = nodeErrors.errors.filter( - (error) => error.extra_info?.input_name !== undefined - ) - - validErrors.forEach((error) => { - const inputName = error.extra_info!.input_name! - const inputIndex = node.findInputSlot(inputName) - if (inputIndex !== -1) { - node.inputs[inputIndex].hasErrors = true - } - }) - }) - - comfyApp.canvas.setDirty(true, true) + () => { + comfyApp.canvas?.setDirty(true, true) } ) @@ -526,6 +510,11 @@ onMounted(async () => { comfyAppReady.value = true + // Install error-clearing hooks on the initial graph + if (comfyApp.canvas?.graph) { + cleanupErrorHooks = installErrorClearingHooks(comfyApp.canvas.graph) + } + vueNodeLifecycle.setupEmptyGraphListener() } finally { workspaceStore.spinner = false @@ -569,6 +558,8 @@ onMounted(async () => { }) onUnmounted(() => { + cleanupErrorHooks?.() + cleanupErrorHooks = null vueNodeLifecycle.cleanup() }) function forwardPanEvent(e: PointerEvent) { diff --git a/src/composables/graph/useErrorClearingHooks.test.ts b/src/composables/graph/useErrorClearingHooks.test.ts new file mode 100644 index 0000000000..3fe38977c6 --- /dev/null +++ b/src/composables/graph/useErrorClearingHooks.test.ts @@ -0,0 +1,352 @@ +import { setActivePinia } from 'pinia' +import { createTestingPinia } from '@pinia/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { installErrorClearingHooks } from '@/composables/graph/useErrorClearingHooks' +import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' +import { + createTestSubgraph, + createTestSubgraphNode +} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers' +import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' +import { app } from '@/scripts/app' +import { useExecutionErrorStore } from '@/stores/executionErrorStore' + +function seedSimpleError( + store: ReturnType, + executionId: string, + inputName: string +) { + store.lastNodeErrors = { + [executionId]: { + errors: [ + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: inputName } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } +} + +describe('Connection error clearing via onConnectionsChange', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(false) + }) + + function createGraphWithInput() { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addWidget('string', 'prompt', 'hello', () => undefined, {}) + node.addInput('clip', 'CLIP') + graph.add(node) + return { graph, node } + } + + it('clears simple node error when INPUT is connected', () => { + const { graph, node } = createGraphWithInput() + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + seedSimpleError(store, String(node.id), 'clip') + + node.onConnectionsChange!(NodeSlotType.INPUT, 0, true, null, node.inputs[0]) + + expect(store.lastNodeErrors).toBeNull() + }) + + it('does not clear errors on disconnection', () => { + const { graph, node } = createGraphWithInput() + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + seedSimpleError(store, String(node.id), 'clip') + + node.onConnectionsChange!( + NodeSlotType.INPUT, + 0, + false, + null, + node.inputs[0] + ) + + expect(store.lastNodeErrors).not.toBeNull() + }) + + it('does not clear errors on OUTPUT connection', () => { + const { graph, node } = createGraphWithInput() + node.addOutput('out', 'CLIP') + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + seedSimpleError(store, String(node.id), 'clip') + + node.onConnectionsChange!( + NodeSlotType.OUTPUT, + 0, + true, + null, + node.outputs[0] + ) + + expect(store.lastNodeErrors).not.toBeNull() + }) + + it('clears errors for pure input slots without widget property', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addInput('model', 'MODEL') + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + seedSimpleError(store, String(node.id), 'model') + + node.onConnectionsChange!(NodeSlotType.INPUT, 0, true, null, node.inputs[0]) + + expect(store.lastNodeErrors).toBeNull() + }) +}) + +describe('Widget change error clearing via onWidgetChanged', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(false) + }) + + it('clears simple error when widget value changes to valid range', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addWidget('number', 'steps', 20, () => undefined, { + min: 1, + max: 100 + }) + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + store.lastNodeErrors = { + [String(node.id)]: { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Too big', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + node.onWidgetChanged!.call(node, 'steps', 50, 20, node.widgets![0]) + + expect(store.lastNodeErrors).toBeNull() + }) + + it('retains error when widget value is still out of range', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addWidget('number', 'steps', 20, () => undefined, { + min: 1, + max: 100 + }) + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + store.lastNodeErrors = { + [String(node.id)]: { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Too big', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + node.onWidgetChanged!.call(node, 'steps', 150, 20, node.widgets![0]) + + expect(store.lastNodeErrors).not.toBeNull() + }) + + it('does not clear errors when rootGraph is unavailable', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addWidget('number', 'steps', 20, () => undefined, {}) + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue( + undefined as unknown as LGraph + ) + store.lastNodeErrors = { + [String(node.id)]: { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Too big', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + node.onWidgetChanged!.call(node, 'steps', 50, 20, node.widgets![0]) + + expect(store.lastNodeErrors).not.toBeNull() + }) + + it('uses interior node execution ID for promoted widget error clearing', () => { + const subgraph = createTestSubgraph({ + inputs: [{ name: 'ckpt_input', type: '*' }] + }) + const interiorNode = new LGraphNode('CheckpointLoaderSimple') + const interiorInput = interiorNode.addInput('ckpt_input', '*') + interiorNode.addWidget( + 'combo', + 'ckpt_name', + 'model.safetensors', + () => undefined, + { values: ['model.safetensors'] } + ) + interiorInput.widget = { name: 'ckpt_name' } + subgraph.add(interiorNode) + subgraph.inputNode.slots[0].connect(interiorInput, interiorNode) + + const subgraphNode = createTestSubgraphNode(subgraph, { id: 65 }) + subgraphNode._internalConfigureAfterSlots() + const graph = subgraphNode.graph as LGraph + graph.add(subgraphNode) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + const interiorExecId = `${subgraphNode.id}:${interiorNode.id}` + + const promotedWidget = subgraphNode.widgets?.find( + (w) => 'sourceWidgetName' in w && w.sourceWidgetName === 'ckpt_name' + ) + expect(promotedWidget).toBeDefined() + + // PromotedWidgetView.name returns displayName ("ckpt_input"), which is + // passed as errorInputName to clearSimpleNodeErrors. Seed the error + // with that name so the slot-name filter matches. + seedSimpleError(store, interiorExecId, promotedWidget!.name) + + subgraphNode.onWidgetChanged!.call( + subgraphNode, + 'ckpt_name', + 'other_model.safetensors', + 'model.safetensors', + promotedWidget! + ) + + expect(store.lastNodeErrors).toBeNull() + }) +}) + +describe('installErrorClearingHooks lifecycle', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(false) + }) + + it('propagates hooks to nodes added after installation', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addInput('value', 'INT') + graph.add(node) + installErrorClearingHooks(graph) + + // Add a new node after hooks are installed + const lateNode = new LGraphNode('late') + lateNode.addInput('value', 'INT') + graph.add(lateNode) + + // The late-added node should have error-clearing hooks + expect(lateNode.onConnectionsChange).toBeDefined() + expect(lateNode.onWidgetChanged).toBeDefined() + + // Verify the hooks actually work + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + seedSimpleError(store, String(lateNode.id), 'value') + + lateNode.onConnectionsChange!( + NodeSlotType.INPUT, + 0, + true, + null, + lateNode.inputs[0] + ) + + expect(store.lastNodeErrors).toBeNull() + }) + + it('restores original onNodeAdded when cleanup is called', () => { + const graph = new LGraph() + const originalHook = vi.fn() + graph.onNodeAdded = originalHook + + const cleanup = installErrorClearingHooks(graph) + expect(graph.onNodeAdded).not.toBe(originalHook) + + cleanup() + expect(graph.onNodeAdded).toBe(originalHook) + }) +}) + +describe('clearWidgetRelatedErrors parameter routing', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(false) + }) + + it('passes widgetName (not errorInputName) for model lookup', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + const widget = node.addWidget('number', 'steps', 42, () => undefined, { + min: 0, + max: 100 + }) + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + const clearSpy = vi.spyOn(store, 'clearWidgetRelatedErrors') + + node.onWidgetChanged!.call(node, 'steps', 42, 0, widget) + + expect(clearSpy).toHaveBeenCalledWith( + String(node.id), + 'steps', + 'steps', + 42, + { min: 0, max: 100 } + ) + + clearSpy.mockRestore() + }) +}) diff --git a/src/composables/graph/useErrorClearingHooks.ts b/src/composables/graph/useErrorClearingHooks.ts new file mode 100644 index 0000000000..27fcc0922d --- /dev/null +++ b/src/composables/graph/useErrorClearingHooks.ts @@ -0,0 +1,108 @@ +/** + * Installs per-node error-clearing callbacks (onConnectionsChange, + * onWidgetChanged) on all current and future nodes in a graph. + * + * Decoupled from the Vue rendering lifecycle so that error auto-clearing + * works in legacy canvas mode as well. + */ +import { useChainCallback } from '@/composables/functional/useChainCallback' +import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' +import { resolveConcretePromotedWidget } from '@/core/graph/subgraph/resolveConcretePromotedWidget' +import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' +import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' +import { app } from '@/scripts/app' +import { useExecutionErrorStore } from '@/stores/executionErrorStore' +import { getExecutionIdByNode } from '@/utils/graphTraversalUtil' + +function resolvePromotedExecId( + rootGraph: LGraph, + node: LGraphNode, + widget: IBaseWidget, + hostExecId: string +): string { + if (!isPromotedWidgetView(widget)) return hostExecId + const result = resolveConcretePromotedWidget( + node, + widget.sourceNodeId, + widget.sourceWidgetName + ) + if (result.status === 'resolved' && result.resolved.node) { + return getExecutionIdByNode(rootGraph, result.resolved.node) ?? hostExecId + } + return hostExecId +} + +const hookedNodes = new WeakSet() + +function installNodeHooks(node: LGraphNode): void { + if (hookedNodes.has(node)) return + hookedNodes.add(node) + + node.onConnectionsChange = useChainCallback( + node.onConnectionsChange, + function (type, slotIndex, isConnected) { + if (type !== NodeSlotType.INPUT || !isConnected) return + if (!app.rootGraph) return + const slotName = node.inputs?.[slotIndex]?.name + if (!slotName) return + const execId = getExecutionIdByNode(app.rootGraph, node) + if (!execId) return + useExecutionErrorStore().clearSimpleNodeErrors(execId, slotName) + } + ) + + node.onWidgetChanged = useChainCallback( + node.onWidgetChanged, + // _name is the LiteGraph callback arg; re-derive from the widget + // object to handle promoted widgets where sourceWidgetName differs. + function (_name, newValue, _oldValue, widget) { + if (!app.rootGraph) return + const hostExecId = getExecutionIdByNode(app.rootGraph, node) + if (!hostExecId) return + + const execId = resolvePromotedExecId( + app.rootGraph, + node, + widget, + hostExecId + ) + const widgetName = isPromotedWidgetView(widget) + ? widget.sourceWidgetName + : widget.name + + useExecutionErrorStore().clearWidgetRelatedErrors( + execId, + widget.name, + widgetName, + newValue, + { min: widget.options?.min, max: widget.options?.max } + ) + } + ) +} + +function installNodeHooksRecursive(node: LGraphNode): void { + installNodeHooks(node) + if (node.isSubgraphNode?.()) { + for (const innerNode of node.subgraph._nodes ?? []) { + installNodeHooksRecursive(innerNode) + } + } +} + +export function installErrorClearingHooks(graph: LGraph): () => void { + for (const node of graph._nodes ?? []) { + installNodeHooksRecursive(node) + } + + const originalOnNodeAdded = graph.onNodeAdded + graph.onNodeAdded = function (node: LGraphNode) { + installNodeHooksRecursive(node) + originalOnNodeAdded?.call(this, node) + } + + return () => { + graph.onNodeAdded = originalOnNodeAdded || undefined + } +} diff --git a/src/composables/graph/useGraphNodeManager.test.ts b/src/composables/graph/useGraphNodeManager.test.ts index 39a25b2352..4aadcf57da 100644 --- a/src/composables/graph/useGraphNodeManager.test.ts +++ b/src/composables/graph/useGraphNodeManager.test.ts @@ -11,6 +11,9 @@ import { createTestSubgraphNode } from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers' import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' +import { app } from '@/scripts/app' +import { useExecutionErrorStore } from '@/stores/executionErrorStore' +import { useMissingModelStore } from '@/platform/missingModel/missingModelStore' import { usePromotionStore } from '@/stores/promotionStore' import { useWidgetValueStore } from '@/stores/widgetValueStore' @@ -316,3 +319,280 @@ describe('Nested promoted widget mapping', () => { ) }) }) + +describe('Promoted widget sourceExecutionId', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + }) + + it('sets sourceExecutionId to the interior node execution ID for promoted widgets', () => { + const subgraph = createTestSubgraph({ + inputs: [{ name: 'ckpt_input', type: '*' }] + }) + const interiorNode = new LGraphNode('CheckpointLoaderSimple') + const interiorInput = interiorNode.addInput('ckpt_input', '*') + interiorNode.addWidget( + 'combo', + 'ckpt_name', + 'model.safetensors', + () => undefined, + { + values: ['model.safetensors'] + } + ) + interiorInput.widget = { name: 'ckpt_name' } + subgraph.add(interiorNode) + subgraph.inputNode.slots[0].connect(interiorInput, interiorNode) + + const subgraphNode = createTestSubgraphNode(subgraph, { id: 65 }) + subgraphNode._internalConfigureAfterSlots() + const graph = subgraphNode.graph as LGraph + graph.add(subgraphNode) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + + const { vueNodeData } = useGraphNodeManager(graph) + const nodeData = vueNodeData.get(String(subgraphNode.id)) + const promotedWidget = nodeData?.widgets?.find( + (w) => w.name === 'ckpt_name' + ) + + expect(promotedWidget).toBeDefined() + // The interior node is inside subgraphNode (id=65), + // so its execution ID should be "65:" + expect(promotedWidget?.sourceExecutionId).toBe( + `${subgraphNode.id}:${interiorNode.id}` + ) + }) + + it('does not set sourceExecutionId for non-promoted widgets', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + node.addWidget('number', 'steps', 20, () => undefined, {}) + graph.add(node) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + + const { vueNodeData } = useGraphNodeManager(graph) + const nodeData = vueNodeData.get(String(node.id)) + const widget = nodeData?.widgets?.find((w) => w.name === 'steps') + + expect(widget).toBeDefined() + expect(widget?.sourceExecutionId).toBeUndefined() + }) +}) + +describe('reconcileNodeErrorFlags (via lastNodeErrors watcher)', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + }) + + function setupGraphWithStore() { + const graph = new LGraph() + const nodeA = new LGraphNode('KSampler') + nodeA.addInput('model', 'MODEL') + nodeA.addInput('steps', 'INT') + graph.add(nodeA) + + const nodeB = new LGraphNode('LoadCheckpoint') + nodeB.addInput('ckpt_name', 'STRING') + graph.add(nodeB) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(true) + + // Initialize store (triggers watcher registration) + useGraphNodeManager(graph) + const store = useExecutionErrorStore() + return { graph, nodeA, nodeB, store } + } + + it('sets has_errors on nodes referenced in lastNodeErrors', async () => { + const { nodeA, nodeB, store } = setupGraphWithStore() + + store.lastNodeErrors = { + [String(nodeA.id)]: { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Too big', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'KSampler' + } + } + await nextTick() + + expect(nodeA.has_errors).toBe(true) + expect(nodeB.has_errors).toBeFalsy() + }) + + it('sets slot hasErrors for inputs matching error input_name', async () => { + const { nodeA, store } = setupGraphWithStore() + + store.lastNodeErrors = { + [String(nodeA.id)]: { + errors: [ + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: 'model' } + } + ], + dependent_outputs: [], + class_type: 'KSampler' + } + } + await nextTick() + + expect(nodeA.inputs[0].hasErrors).toBe(true) + expect(nodeA.inputs[1].hasErrors).toBe(false) + }) + + it('clears has_errors and slot hasErrors when errors are removed', async () => { + const { nodeA, store } = setupGraphWithStore() + + store.lastNodeErrors = { + [String(nodeA.id)]: { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Too big', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'KSampler' + } + } + await nextTick() + expect(nodeA.has_errors).toBe(true) + expect(nodeA.inputs[1].hasErrors).toBe(true) + + store.lastNodeErrors = null + await nextTick() + + expect(nodeA.has_errors).toBeFalsy() + expect(nodeA.inputs[1].hasErrors).toBe(false) + }) + + it('propagates has_errors to parent subgraph node', async () => { + const subgraph = createTestSubgraph() + const interiorNode = new LGraphNode('InnerNode') + interiorNode.addInput('value', 'INT') + subgraph.add(interiorNode) + + const subgraphNode = createTestSubgraphNode(subgraph, { id: 50 }) + const graph = subgraphNode.graph as LGraph + graph.add(subgraphNode) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(true) + + useGraphNodeManager(graph) + const store = useExecutionErrorStore() + + // Error on interior node: execution ID = "50:" + const interiorExecId = `${subgraphNode.id}:${interiorNode.id}` + store.lastNodeErrors = { + [interiorExecId]: { + errors: [ + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: 'value' } + } + ], + dependent_outputs: [], + class_type: 'InnerNode' + } + } + await nextTick() + + // Interior node should have the error + expect(interiorNode.has_errors).toBe(true) + expect(interiorNode.inputs[0].hasErrors).toBe(true) + // Parent subgraph node should also be flagged + expect(subgraphNode.has_errors).toBe(true) + }) + + it('sets has_errors on nodes with missing models', async () => { + const { nodeA, nodeB } = setupGraphWithStore() + const missingModelStore = useMissingModelStore() + + missingModelStore.setMissingModels([ + { + nodeId: String(nodeA.id), + nodeType: 'CheckpointLoader', + widgetName: 'ckpt_name', + isAssetSupported: false, + name: 'missing.safetensors', + isMissing: true + } + ]) + await nextTick() + + expect(nodeA.has_errors).toBe(true) + expect(nodeB.has_errors).toBeFalsy() + }) + + it('clears has_errors when missing models are removed', async () => { + const { nodeA } = setupGraphWithStore() + const missingModelStore = useMissingModelStore() + + missingModelStore.setMissingModels([ + { + nodeId: String(nodeA.id), + nodeType: 'CheckpointLoader', + widgetName: 'ckpt_name', + isAssetSupported: false, + name: 'missing.safetensors', + isMissing: true + } + ]) + await nextTick() + expect(nodeA.has_errors).toBe(true) + + missingModelStore.clearMissingModels() + await nextTick() + expect(nodeA.has_errors).toBeFalsy() + }) + + it('flags parent subgraph node when interior node has missing model', async () => { + const subgraph = createTestSubgraph() + const interiorNode = new LGraphNode('CheckpointLoader') + subgraph.add(interiorNode) + + const subgraphNode = createTestSubgraphNode(subgraph, { id: 50 }) + const graph = subgraphNode.graph as LGraph + graph.add(subgraphNode) + + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + vi.spyOn(app, 'isGraphReady', 'get').mockReturnValue(true) + + useGraphNodeManager(graph) + useExecutionErrorStore() + const missingModelStore = useMissingModelStore() + + missingModelStore.setMissingModels([ + { + nodeId: `${subgraphNode.id}:${interiorNode.id}`, + nodeType: 'CheckpointLoader', + widgetName: 'ckpt_name', + isAssetSupported: false, + name: 'missing.safetensors', + isMissing: true + } + ]) + await nextTick() + + expect(interiorNode.has_errors).toBe(true) + expect(subgraphNode.has_errors).toBe(true) + }) +}) diff --git a/src/composables/graph/useGraphNodeManager.ts b/src/composables/graph/useGraphNodeManager.ts index 7faef35f98..a0685e66c9 100644 --- a/src/composables/graph/useGraphNodeManager.ts +++ b/src/composables/graph/useGraphNodeManager.ts @@ -36,6 +36,7 @@ import type { import type { TitleMode } from '@/lib/litegraph/src/types/globalEnums' import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' import { app } from '@/scripts/app' +import { getExecutionIdByNode } from '@/utils/graphTraversalUtil' export interface WidgetSlotMetadata { index: number @@ -80,6 +81,13 @@ export interface SafeWidgetData { * which differs from the subgraph node's input slot widget name. */ slotName?: string + /** + * Execution ID of the interior node that owns the source widget. + * Only set for promoted widgets where the source node differs from the + * host subgraph node. Used for missing-model lookups that key by + * execution ID (e.g. `"65:42"` vs the host node's `"65"`). + */ + sourceExecutionId?: string } export interface VueNodeData { @@ -324,10 +332,21 @@ function safeWidgetMapper( } : (extractWidgetDisplayOptions(effectiveWidget) ?? options), slotMetadata: slotInfo, + // For promoted widgets, name is sourceWidgetName while widget.name + // is the subgraph input slot name — store the slot name for lookups. slotName: name !== widget.name ? widget.name : undefined, + sourceExecutionId: + sourceNode && app.rootGraph + ? (getExecutionIdByNode(app.rootGraph, sourceNode) ?? undefined) + : undefined, tooltip: widget.tooltip } } catch (error) { + console.warn( + '[safeWidgetMapper] Failed to map widget:', + widget.name, + error + ) return { name: widget.name || 'unknown', type: widget.type || 'text' @@ -642,6 +661,12 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { title: String(propertyEvent.newValue) }) break + case 'has_errors': + vueNodeData.set(nodeId, { + ...currentData, + hasErrors: Boolean(propertyEvent.newValue) + }) + break case 'flags.collapsed': vueNodeData.set(nodeId, { ...currentData, diff --git a/src/platform/missingModel/missingModelScan.test.ts b/src/platform/missingModel/missingModelScan.test.ts index e29659e62d..4c098d8619 100644 --- a/src/platform/missingModel/missingModelScan.test.ts +++ b/src/platform/missingModel/missingModelScan.test.ts @@ -388,6 +388,32 @@ describe('scanAllModelCandidates', () => { expect(result[0].isAssetSupported).toBe(true) expect(result[1].widgetName).toBe('vae_name') }) + + it('skips subgraph container nodes whose promoted widgets are already scanned via interior nodes', () => { + const containerNode = { + id: 65, + type: 'abc-def-uuid', + widgets: [makeComboWidget('ckpt_name', 'model.safetensors', [])], + isSubgraphNode: () => true, + _testExecutionId: '65' + } as unknown as LGraphNode + + const interiorNode = makeNode( + 42, + 'CheckpointLoaderSimple', + [ + makeComboWidget('ckpt_name', 'model.safetensors', ['model.safetensors']) + ], + '65:42' + ) + + const graph = makeGraph([containerNode, interiorNode]) + const result = scanAllModelCandidates(graph, noAssetSupport) + + expect(result).toHaveLength(1) + expect(result[0].nodeId).toBe('65:42') + expect(result[0].nodeType).toBe('CheckpointLoaderSimple') + }) }) function makeCandidate( diff --git a/src/platform/missingModel/missingModelScan.ts b/src/platform/missingModel/missingModelScan.ts index 20f215905d..1307b6e5a3 100644 --- a/src/platform/missingModel/missingModelScan.ts +++ b/src/platform/missingModel/missingModelScan.ts @@ -76,6 +76,9 @@ export function scanAllModelCandidates( for (const node of allNodes) { if (!node.widgets?.length) continue + // Skip subgraph container nodes: their promoted widgets are synthetic + // views of interior widgets, which are already scanned via recursion. + if (node.isSubgraphNode?.()) continue const executionId = getExecutionIdByNode(rootGraph, node) if (!executionId) continue diff --git a/src/platform/missingModel/missingModelStore.test.ts b/src/platform/missingModel/missingModelStore.test.ts index 5e53394617..3d5e9aba74 100644 --- a/src/platform/missingModel/missingModelStore.test.ts +++ b/src/platform/missingModel/missingModelStore.test.ts @@ -186,4 +186,62 @@ describe('missingModelStore', () => { expect(store.isWidgetMissingModel('1', 'ckpt_name')).toBe(false) }) }) + + describe('removeMissingModelByWidget', () => { + it('removes the matching model entry by nodeId and widgetName', () => { + const store = useMissingModelStore() + store.setMissingModels([ + makeModelCandidate('model_a.safetensors', { + nodeId: '5', + widgetName: 'ckpt_name' + }), + makeModelCandidate('model_b.safetensors', { + nodeId: '8', + widgetName: 'lora_name' + }) + ]) + + store.removeMissingModelByWidget('5', 'ckpt_name') + + expect(store.missingModelCandidates).toHaveLength(1) + expect(store.missingModelCandidates![0].name).toBe('model_b.safetensors') + }) + + it('sets candidates to null when last entry is removed', () => { + const store = useMissingModelStore() + store.setMissingModels([ + makeModelCandidate('model_a.safetensors', { + nodeId: '5', + widgetName: 'ckpt_name' + }) + ]) + + store.removeMissingModelByWidget('5', 'ckpt_name') + + expect(store.missingModelCandidates).toBeNull() + expect(store.hasMissingModels).toBe(false) + }) + + it('does nothing when no candidates exist', () => { + const store = useMissingModelStore() + store.removeMissingModelByWidget('5', 'ckpt_name') + expect(store.missingModelCandidates).toBeNull() + }) + + it('does nothing when nodeId or widgetName does not match', () => { + const store = useMissingModelStore() + store.setMissingModels([ + makeModelCandidate('model_a.safetensors', { + nodeId: '5', + widgetName: 'ckpt_name' + }) + ]) + + store.removeMissingModelByWidget('5', 'lora_name') + expect(store.missingModelCandidates).toHaveLength(1) + + store.removeMissingModelByWidget('99', 'ckpt_name') + expect(store.missingModelCandidates).toHaveLength(1) + }) + }) }) diff --git a/src/platform/missingModel/missingModelStore.ts b/src/platform/missingModel/missingModelStore.ts index be4fd5c1d1..33b22d707a 100644 --- a/src/platform/missingModel/missingModelStore.ts +++ b/src/platform/missingModel/missingModelStore.ts @@ -116,6 +116,15 @@ export const useMissingModelStore = defineStore('missingModel', () => { missingModelCandidates.value = null } + function removeMissingModelByWidget(nodeId: string, widgetName: string) { + if (!missingModelCandidates.value) return + missingModelCandidates.value = missingModelCandidates.value.filter( + (m) => !(String(m.nodeId) === nodeId && m.widgetName === widgetName) + ) + if (!missingModelCandidates.value.length) + missingModelCandidates.value = null + } + function hasMissingModelOnNode(nodeLocatorId: string): boolean { return missingModelNodeIds.value.has(nodeLocatorId) } @@ -175,9 +184,11 @@ export const useMissingModelStore = defineStore('missingModel', () => { missingModelCount, missingModelNodeIds, activeMissingModelGraphIds, + missingModelAncestorExecutionIds, setMissingModels, removeMissingModelByNameOnNodes, + removeMissingModelByWidget, clearMissingModels, createVerificationAbortController, diff --git a/src/renderer/extensions/vueNodes/components/NodeWidgets.vue b/src/renderer/extensions/vueNodes/components/NodeWidgets.vue index 3883e117f4..c9e73669ff 100644 --- a/src/renderer/extensions/vueNodes/components/NodeWidgets.vue +++ b/src/renderer/extensions/vueNodes/components/NodeWidgets.vue @@ -86,6 +86,7 @@ import { computed, onErrorCaptured, ref, toValue } from 'vue' import type { Component } from 'vue' import type { + SafeWidgetData, VueNodeData, WidgetSlotMetadata } from '@/composables/graph/useGraphNodeManager' @@ -93,6 +94,7 @@ import { useAppMode } from '@/composables/useAppMode' import { showNodeOptions } from '@/composables/graph/useMoreOptionsMenu' import { useErrorHandling } from '@/composables/useErrorHandling' import { st } from '@/i18n' +import type { IWidgetOptions } from '@/lib/litegraph/src/types/widgets' import { LGraphEventMode } from '@/lib/litegraph/src/types/globalEnums' import { useSettingStore } from '@/platform/settings/settingStore' import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions' @@ -110,6 +112,7 @@ import { shouldRenderAsVue } from '@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry' import { nodeTypeValidForApp } from '@/stores/appModeStore' +import type { WidgetState } from '@/stores/widgetValueStore' import { stripGraphPrefix, useWidgetValueStore @@ -119,6 +122,8 @@ import { useMissingModelStore } from '@/platform/missingModel/missingModelStore' import { useExecutionErrorStore } from '@/stores/executionErrorStore' import type { SimplifiedWidget, WidgetValue } from '@/types/simplifiedWidget' import { cn } from '@/utils/tailwindUtil' +import { getExecutionIdFromNodeData } from '@/utils/graphTraversalUtil' +import { app } from '@/scripts/app' import InputSlot from './InputSlot.vue' @@ -181,6 +186,26 @@ const { getWidgetTooltip, createTooltipConfig } = useNodeTooltips( ) const widgetValueStore = useWidgetValueStore() +function createWidgetUpdateHandler( + widgetState: WidgetState | undefined, + widget: SafeWidgetData, + nodeExecId: string, + widgetOptions: IWidgetOptions | Record +): (newValue: WidgetValue) => void { + return (newValue: WidgetValue) => { + if (widgetState) widgetState.value = newValue + widget.callback?.(newValue) + const effectiveExecId = widget.sourceExecutionId ?? nodeExecId + executionErrorStore.clearWidgetRelatedErrors( + effectiveExecId, + widget.slotName ?? widget.name, + widget.name, + newValue, + { min: widgetOptions?.min, max: widgetOptions?.max } + ) + } +} + interface ProcessedWidget { advanced: boolean handleContextMenu: (e: PointerEvent) => void @@ -198,13 +223,37 @@ interface ProcessedWidget { slotMetadata?: WidgetSlotMetadata } +function hasWidgetError( + widget: SafeWidgetData, + nodeExecId: string, + nodeErrors: { errors: { extra_info?: { input_name?: string } }[] } | undefined +): boolean { + const errors = widget.sourceExecutionId + ? executionErrorStore.lastNodeErrors?.[widget.sourceExecutionId]?.errors + : nodeErrors?.errors + const inputName = widget.slotName ?? widget.name + return ( + !!errors?.some((e) => e.extra_info?.input_name === inputName) || + missingModelStore.isWidgetMissingModel( + widget.sourceExecutionId ?? nodeExecId, + widget.name + ) + ) +} + const processedWidgets = computed((): ProcessedWidget[] => { if (!nodeData?.widgets) return [] - const nodeErrors = executionErrorStore.lastNodeErrors?.[nodeData.id ?? ''] + + // nodeData.id is the local node ID; subgraph nodes need the full execution + // path (e.g. "65:63") to match keys in lastNodeErrors. + const nodeExecId = app.rootGraph + ? getExecutionIdFromNodeData(app.rootGraph, nodeData) + : String(nodeData.id ?? '') + + const nodeErrors = executionErrorStore.lastNodeErrors?.[nodeExecId] const graphId = canvasStore.canvas?.graph?.rootGraph.id const nodeId = nodeData.id - const nodeIdStr = String(nodeId) const { widgets } = nodeData const result: ProcessedWidget[] = [] @@ -260,12 +309,12 @@ const processedWidgets = computed((): ProcessedWidget[] => { spec: widget.spec } - function updateHandler(newValue: WidgetValue) { - // Update value in store - if (widgetState) widgetState.value = newValue - // Invoke LiteGraph callback wrapper (handles triggerDraw, etc.) - widget.callback?.(newValue) - } + const updateHandler = createWidgetUpdateHandler( + widgetState, + widget, + nodeExecId, + widgetOptions + ) const tooltipText = getWidgetTooltip(widget) const tooltipConfig = createTooltipConfig(tooltipText) @@ -286,12 +335,7 @@ const processedWidgets = computed((): ProcessedWidget[] => { advanced: widget.options?.advanced ?? false, handleContextMenu, hasLayoutSize: widget.hasLayoutSize ?? false, - hasError: - (nodeErrors?.errors?.some( - (error) => error.extra_info?.input_name === widget.name - ) ?? - false) || - missingModelStore.isWidgetMissingModel(nodeIdStr, widget.name), + hasError: hasWidgetError(widget, nodeExecId, nodeErrors), hidden: widget.options?.hidden ?? false, id: String(bareWidgetId), name: widget.name, diff --git a/src/stores/executionErrorStore.test.ts b/src/stores/executionErrorStore.test.ts index 7a1f729a9c..4c89fa1372 100644 --- a/src/stores/executionErrorStore.test.ts +++ b/src/stores/executionErrorStore.test.ts @@ -164,3 +164,369 @@ describe('executionErrorStore — missing node operations', () => { }) }) }) + +describe('executionErrorStore — node error operations', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + describe('clearSimpleNodeErrors', () => { + it('does nothing if lastNodeErrors is null', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = null + // Should not error + store.clearSimpleNodeErrors('123', 'widgetName') + expect(store.lastNodeErrors).toBeNull() + }) + + it('clears entirely if there are only simple errors for the same slot', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'testSlot' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123', 'testSlot') + + // Should be entirely removed (empty object becomes null) + expect(store.lastNodeErrors).toBeNull() + }) + + it('clears only the specific slot errors, leaving other errors alone', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'testSlot' } + }, + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: 'otherSlot' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123', 'testSlot') + + // otherSlot error should still exist + expect(store.lastNodeErrors).not.toBeNull() + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(1) + expect( + store.lastNodeErrors?.['123'].errors[0].extra_info?.input_name + ).toBe('otherSlot') + }) + + it('does nothing if executionId is not found in lastNodeErrors', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'testSlot' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('999', 'testSlot') + + // Original error should remain untouched + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(1) + }) + + it('preserves complex errors when slot has both simple and complex errors', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'testSlot' } + }, + { + type: 'exception_type', + message: 'Runtime error', + details: '', + extra_info: { input_name: 'testSlot' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123', 'testSlot') + + // Mixed simple+complex: not all are simple, so none are cleared + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(2) + }) + + it('clears one node while preserving another in multi-node errors', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'steps' } + } + ], + dependent_outputs: [], + class_type: 'KSampler' + }, + '456': { + errors: [ + { + type: 'exception_type', + message: 'Runtime failure', + details: '', + extra_info: { input_name: 'model' } + } + ], + dependent_outputs: [], + class_type: 'LoadModel' + } + } + + store.clearSimpleNodeErrors('123', 'steps') + + // Node 123 cleared, node 456 remains + expect(store.lastNodeErrors?.['123']).toBeUndefined() + expect(store.lastNodeErrors?.['456'].errors).toHaveLength(1) + }) + + it('clears entire node when no slotName and all errors are simple', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'steps' } + }, + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: 'model' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123') + + expect(store.lastNodeErrors).toBeNull() + }) + + it('does not clear when no slotName and some errors are not simple', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: 'Max exceeded', + details: '', + extra_info: { input_name: 'steps' } + }, + { + type: 'exception_type', + message: 'Runtime error', + details: '', + extra_info: { input_name: 'model' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123') + + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(2) + }) + + it('does not clear if the error is not simple', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'exception_type', // Complex error + message: 'Failed execution', + details: '', + extra_info: { input_name: 'testSlot' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + store.clearSimpleNodeErrors('123', 'testSlot') + + // Error should remain + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(1) + }) + }) + + describe('clearWidgetRelatedErrors', () => { + it('clears error if value is valid (isValueStillOutOfRange is false)', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: '...', + details: '', + extra_info: { input_name: 'testWidget' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + // Valid value (5 < 10) + store.clearWidgetRelatedErrors('123', 'testWidget', 'testWidget', 5, { + max: 10 + }) + + expect(store.lastNodeErrors).toBeNull() + }) + + it('optimistically clears value_not_in_list error for string combo values', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_not_in_list', + message: 'Value not in list', + details: '', + extra_info: { input_name: 'sampler' } + } + ], + dependent_outputs: [], + class_type: 'KSampler' + } + } + + store.clearWidgetRelatedErrors('123', 'sampler', 'sampler', 'euler_a') + + expect(store.lastNodeErrors).toBeNull() + }) + + it('does not clear error if value is still out of range', () => { + const store = useExecutionErrorStore() + store.lastNodeErrors = { + '123': { + errors: [ + { + type: 'value_bigger_than_max', + message: '...', + details: '', + extra_info: { input_name: 'testWidget' } + } + ], + dependent_outputs: [], + class_type: 'TestNode' + } + } + + // Invalid value (15 > 10) + store.clearWidgetRelatedErrors('123', 'testWidget', 'testWidget', 15, { + max: 10 + }) + + expect(store.lastNodeErrors).not.toBeNull() + expect(store.lastNodeErrors?.['123'].errors).toHaveLength(1) + }) + }) +}) + +describe('clearAllErrors', () => { + let store: ReturnType + + beforeEach(() => { + const pinia = createPinia() + setActivePinia(pinia) + store = useExecutionErrorStore() + }) + + it('resets all error categories and closes error overlay', () => { + store.lastExecutionError = { + prompt_id: 'test', + timestamp: 0, + node_id: '1', + node_type: 'Test', + executed: [], + exception_message: 'fail', + exception_type: 'RuntimeError', + traceback: [] + } + store.lastPromptError = { type: 'execution', message: 'fail', details: '' } + store.lastNodeErrors = { + '1': { + errors: [ + { + type: 'required_input_missing', + message: 'Missing', + details: '', + extra_info: { input_name: 'x' } + } + ], + dependent_outputs: [], + class_type: 'Test' + } + } + store.missingNodesError = { + message: 'Missing nodes', + nodeTypes: [{ type: 'MissingNode', hint: '' }] + } + store.showErrorOverlay() + + store.clearAllErrors() + + expect(store.lastExecutionError).toBeNull() + expect(store.lastPromptError).toBeNull() + expect(store.lastNodeErrors).toBeNull() + expect(store.missingNodesError).toBeNull() + expect(store.isErrorOverlayOpen).toBe(false) + expect(store.hasAnyError).toBe(false) + }) +}) diff --git a/src/stores/executionErrorStore.ts b/src/stores/executionErrorStore.ts index e82ece7526..8dc0f6773d 100644 --- a/src/stores/executionErrorStore.ts +++ b/src/stores/executionErrorStore.ts @@ -29,48 +29,81 @@ import { getExecutionIdByNode, getActiveGraphNodeIds } from '@/utils/graphTraversalUtil' +import { + isValueStillOutOfRange, + SIMPLE_ERROR_TYPES +} from '@/utils/executionErrorUtil' interface MissingNodesError { message: string nodeTypes: MissingNodeType[] } -function clearAllNodeErrorFlags(rootGraph: LGraph): void { - forEachNode(rootGraph, (node) => { - node.has_errors = false - if (node.inputs) { - for (const slot of node.inputs) { - slot.hasErrors = false - } - } +function setNodeHasErrors(node: LGraphNode, hasErrors: boolean): void { + if (node.has_errors === hasErrors) return + const oldValue = node.has_errors + node.has_errors = hasErrors + node.graph?.trigger('node:property:changed', { + type: 'node:property:changed', + nodeId: node.id, + property: 'has_errors', + oldValue, + newValue: hasErrors }) } -function markNodeSlotErrors(node: LGraphNode, nodeError: NodeError): void { - if (!node.inputs) return - for (const error of nodeError.errors) { - const slotName = error.extra_info?.input_name - if (!slotName) continue - const slot = node.inputs.find((s) => s.name === slotName) - if (slot) slot.hasErrors = true - } -} - -function applyNodeError( +/** + * Single-pass reconciliation of node error flags. + * Collects the set of nodes that should have errors, then walks all nodes + * once, setting each flag exactly once. This avoids the redundant + * true→false→true transition (and duplicate events) that a clear-then-apply + * approach would cause. + */ +function reconcileNodeErrorFlags( rootGraph: LGraph, - executionId: NodeExecutionId, - nodeError: NodeError + nodeErrors: Record | null, + missingModelExecIds: Set ): void { - const node = getNodeByExecutionId(rootGraph, executionId) - if (!node) return + // Collect nodes and slot info that should be flagged + // Includes both error-owning nodes and their ancestor containers + const flaggedNodes = new Set() + const errorSlots = new Map>() - node.has_errors = true - markNodeSlotErrors(node, nodeError) + if (nodeErrors) { + for (const [executionId, nodeError] of Object.entries(nodeErrors)) { + const node = getNodeByExecutionId(rootGraph, executionId) + if (!node) continue - for (const parentId of getParentExecutionIds(executionId)) { - const parentNode = getNodeByExecutionId(rootGraph, parentId) - if (parentNode) parentNode.has_errors = true + flaggedNodes.add(node) + const slotNames = new Set() + for (const error of nodeError.errors) { + const name = error.extra_info?.input_name + if (name) slotNames.add(name) + } + if (slotNames.size > 0) errorSlots.set(node, slotNames) + + for (const parentId of getParentExecutionIds(executionId)) { + const parentNode = getNodeByExecutionId(rootGraph, parentId) + if (parentNode) flaggedNodes.add(parentNode) + } + } } + + for (const execId of missingModelExecIds) { + const node = getNodeByExecutionId(rootGraph, execId) + if (node) flaggedNodes.add(node) + } + + forEachNode(rootGraph, (node) => { + setNodeHasErrors(node, flaggedNodes.has(node)) + + if (node.inputs) { + const nodeSlotNames = errorSlots.get(node) + for (const slot of node.inputs) { + slot.hasErrors = !!nodeSlotNames?.has(slot.name) + } + } + }) } /** Execution error state: node errors, runtime errors, prompt errors, and missing assets. */ @@ -112,6 +145,99 @@ export const useExecutionErrorStore = defineStore('executionError', () => { lastPromptError.value = null } + /** + * Removes a node's errors if they consist entirely of simple, auto-resolvable + * types. When `slotName` is provided, only errors for that slot are checked. + */ + function clearSimpleNodeErrors(executionId: string, slotName?: string): void { + if (!lastNodeErrors.value) return + const nodeError = lastNodeErrors.value[executionId] + if (!nodeError) return + + const isSlotScoped = slotName !== undefined + + const relevantErrors = isSlotScoped + ? nodeError.errors.filter((e) => e.extra_info?.input_name === slotName) + : nodeError.errors + + if (relevantErrors.length === 0) return + if (!relevantErrors.every((e) => SIMPLE_ERROR_TYPES.has(e.type))) return + + const updated = { ...lastNodeErrors.value } + + if (isSlotScoped) { + // Remove only the target slot's errors if they were all simple + const remainingErrors = nodeError.errors.filter( + (e) => e.extra_info?.input_name !== slotName + ) + if (remainingErrors.length === 0) { + delete updated[executionId] + } else { + updated[executionId] = { + ...nodeError, + errors: remainingErrors + } + } + } else { + // If no slot specified and all errors were simple, clear the whole node + delete updated[executionId] + } + + lastNodeErrors.value = Object.keys(updated).length > 0 ? updated : null + } + + /** + * Attempts to clear an error for a given widget, but avoids clearing it if + * the error is a range violation and the new value is still out of bounds. + * + * Note: `value_not_in_list` errors are optimistically cleared without + * list-membership validation because combo widgets constrain choices to + * valid values at the UI level, and the valid-values source varies + * (asset system vs objectInfo) making runtime validation non-trivial. + */ + function clearSlotErrorsWithRangeCheck( + executionId: string, + widgetName: string, + newValue: unknown, + options?: { min?: number; max?: number } + ): void { + if (typeof newValue === 'number' && lastNodeErrors.value) { + const nodeErrors = lastNodeErrors.value[executionId] + if (nodeErrors) { + const errs = nodeErrors.errors.filter( + (e) => e.extra_info?.input_name === widgetName + ) + if (isValueStillOutOfRange(newValue, errs, options || {})) return + } + } + clearSimpleNodeErrors(executionId, widgetName) + } + + /** + * Clears both validation errors and missing model state for a widget. + * + * @param errorInputName Name matched against `error.extra_info.input_name`. + * For promoted subgraph widgets this is the subgraph input slot name + * (`widget.slotName`), which differs from the interior widget name. + * @param widgetName The actual widget name, used for missing model lookup. + * At the legacy canvas call site both names are identical (`widget.name`). + */ + function clearWidgetRelatedErrors( + executionId: string, + errorInputName: string, + widgetName: string, + newValue: unknown, + options?: { min?: number; max?: number } + ): void { + clearSlotErrorsWithRangeCheck( + executionId, + errorInputName, + newValue, + options + ) + missingModelStore.removeMissingModelByWidget(executionId, widgetName) + } + /** Set missing node types and open the error overlay if the Errors tab is enabled. */ function surfaceMissingNodes(types: MissingNodeType[]) { setMissingNodeTypes(types) @@ -375,20 +501,18 @@ export const useExecutionErrorStore = defineStore('executionError', () => { return missingAncestorExecutionIds.value.has(execId) } - watch(lastNodeErrors, () => { - if (!app.isGraphReady) return - const rootGraph = app.rootGraph - - clearAllNodeErrorFlags(rootGraph) - - if (!lastNodeErrors.value) return - - for (const [executionId, nodeError] of Object.entries( - lastNodeErrors.value - )) { - applyNodeError(rootGraph, executionId, nodeError) - } - }) + watch( + [lastNodeErrors, () => missingModelStore.missingModelNodeIds], + () => { + if (!app.isGraphReady) return + reconcileNodeErrorFlags( + app.rootGraph, + lastNodeErrors.value, + missingModelStore.missingModelAncestorExecutionIds + ) + }, + { flush: 'post' } + ) return { // Raw state @@ -418,6 +542,10 @@ export const useExecutionErrorStore = defineStore('executionError', () => { activeGraphErrorNodeIds, activeMissingNodeGraphIds, + // Clearing (targeted) + clearSimpleNodeErrors, + clearWidgetRelatedErrors, + // Missing node actions setMissingNodeTypes, surfaceMissingNodes, diff --git a/src/utils/executionErrorUtil.test.ts b/src/utils/executionErrorUtil.test.ts index 68aafcfda1..763b237b5e 100644 --- a/src/utils/executionErrorUtil.test.ts +++ b/src/utils/executionErrorUtil.test.ts @@ -3,7 +3,8 @@ import { describe, expect, it } from 'vitest' import { isCloudValidationError, tryExtractValidationError, - classifyCloudValidationError + classifyCloudValidationError, + isValueStillOutOfRange } from '@/utils/executionErrorUtil' describe('executionErrorUtil', () => { @@ -187,4 +188,155 @@ describe('executionErrorUtil', () => { expect(result?.kind).toBe('promptError') }) }) + + describe('isValueStillOutOfRange', () => { + it('should return false if there are no errors', () => { + expect(isValueStillOutOfRange(5, [], {})).toBe(false) + }) + + it('should return true if value is bigger than max', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(15, errors, { max: 10 })).toBe(true) + }) + + it('should return false if value is equal to max but error was value_bigger_than_max', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(10, errors, { max: 10 })).toBe(false) + }) + + it('should return false if value is less than max', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(5, errors, { max: 10 })).toBe(false) + }) + + it('should return true if value is smaller than min', () => { + const errors = [ + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(1, errors, { min: 5 })).toBe(true) + }) + + it('should return false if value is equal to min but error was value_smaller_than_min', () => { + const errors = [ + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(5, errors, { min: 5 })).toBe(false) + }) + + it('should return false if value is greater than min', () => { + const errors = [ + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(10, errors, { min: 5 })).toBe(false) + }) + + it('should return true if both max and min errors exist and value is still out of range', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + }, + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + // Value above max — still out of range for the max error + expect(isValueStillOutOfRange(15, errors, { min: 1, max: 10 })).toBe(true) + }) + + it('should return false if both max and min errors exist but value is in range', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + }, + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(5, errors, { min: 1, max: 10 })).toBe(false) + }) + + it('should return true if max is undefined but error was value_bigger_than_max (conservative)', () => { + const errors = [ + { + type: 'value_bigger_than_max', + message: 'too big', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(15, errors, {})).toBe(true) + }) + + it('should return true if min is undefined but error was value_smaller_than_min (conservative)', () => { + const errors = [ + { + type: 'value_smaller_than_min', + message: 'too small', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(0, errors, {})).toBe(true) + }) + + it('should return false when errors contain only non-range types', () => { + const errors = [ + { + type: 'value_not_in_list', + message: 'not in list', + details: '', + extra_info: {} + } + ] + expect(isValueStillOutOfRange(5, errors, { min: 1, max: 10 })).toBe(false) + }) + }) }) diff --git a/src/utils/executionErrorUtil.ts b/src/utils/executionErrorUtil.ts index c2e3ae44e9..e1696b2936 100644 --- a/src/utils/executionErrorUtil.ts +++ b/src/utils/executionErrorUtil.ts @@ -90,3 +90,38 @@ export function classifyCloudValidationError( return null } + +/** + * Error types that can be resolved automatically when the user changes a + * widget value or establishes a connection, without requiring a re-run. + * + * When adding new types, review {@link isValueStillOutOfRange} to ensure + * the new type does not require range validation before auto-clearing. + */ +export const SIMPLE_ERROR_TYPES = new Set([ + 'value_bigger_than_max', + 'value_smaller_than_min', + 'value_not_in_list', + 'required_input_missing' +]) + +/** + * Returns true if `value` still violates a recorded range constraint. + * Pass errors already filtered to the target widget (by `input_name`). + * `options` should contain the widget's configured `min` / `max`. + * + * Returns true (keeps the error) when a bound is unknown (`undefined`). + */ +export function isValueStillOutOfRange( + value: number, + errors: NodeError['errors'], + options: { min?: number; max?: number } +): boolean { + const hasMaxError = errors.some((e) => e.type === 'value_bigger_than_max') + const hasMinError = errors.some((e) => e.type === 'value_smaller_than_min') + + return ( + (hasMaxError && (options.max === undefined || value > options.max)) || + (hasMinError && (options.min === undefined || value < options.min)) + ) +} diff --git a/src/utils/graphTraversalUtil.test.ts b/src/utils/graphTraversalUtil.test.ts index 604ba81382..f21f0db327 100644 --- a/src/utils/graphTraversalUtil.test.ts +++ b/src/utils/graphTraversalUtil.test.ts @@ -20,14 +20,17 @@ import { getNodeByLocatorId, getRootGraph, getSubgraphPathFromExecutionId, + getExecutionIdFromNodeData, mapAllNodes, mapSubgraphNodes, parseExecutionId, traverseNodesDepthFirst, traverseSubgraphPath, triggerCallbackOnAllNodes, - visitGraphNodes + visitGraphNodes, + getExecutionIdByNode } from '@/utils/graphTraversalUtil' + import { createMockLGraphNode } from './__tests__/litegraphTestUtils' // Mock node factory @@ -596,6 +599,89 @@ describe('graphTraversalUtil', () => { }) }) + describe('getExecutionIdByNode', () => { + it('should return node id if graph is rootGraph', () => { + const node = createMockNode('123') + const graph = createMockGraph([node]) + node.graph = graph + const execId = getExecutionIdByNode(graph, node) + expect(execId).toBe('123') + }) + + it('should return path using subgraph nodes if deeply nested', () => { + const targetNode = createMockNode('999') + const deepSubgraph = createMockSubgraph('deep-uuid', [targetNode]) + + const midNode = createMockNode('456', { + isSubgraph: true, + subgraph: deepSubgraph + }) + const midSubgraph = createMockSubgraph('mid-uuid', [midNode]) + + const topNode = createMockNode('123', { + isSubgraph: true, + subgraph: midSubgraph + }) + + const rootGraph = createMockGraph([topNode]) + + // set up parent graphs + ;(midSubgraph as Subgraph & { rootGraph: LGraph }).rootGraph = rootGraph + ;( + deepSubgraph as Subgraph & { rootGraph: LGraph | Subgraph } + ).rootGraph = midSubgraph + + // also need a way for nodes to point to their parent graphs + // assuming node.graph === graph + targetNode.graph = deepSubgraph + midNode.graph = midSubgraph + topNode.graph = rootGraph + + const execId = getExecutionIdByNode(rootGraph, targetNode) + expect(execId).toBe('123:456:999') + }) + }) + + describe('getExecutionIdFromNodeData', () => { + it('should return the correct execution ID for a normal node', () => { + const node = createMockNode('123') + const graph = createMockGraph([node]) + node.graph = graph + const nodeData = { id: 123 } + + const execId = getExecutionIdFromNodeData(graph, nodeData) + expect(execId).toBe('123') + }) + + it('should fallback to stringified nodeData id if node cannot be resolved', () => { + const graph = createMockGraph([]) + const nodeData = { id: 777 } + + const execId = getExecutionIdFromNodeData(graph, nodeData) + expect(execId).toBe('777') + }) + + it('should return full execution ID for node inside a subgraph', () => { + const targetNode = createMockNode('999') + const subgraphUuid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890' + const subgraph = createMockSubgraph(subgraphUuid, [targetNode]) + const topNode = createMockNode('123', { + isSubgraph: true, + subgraph + }) + const rootGraph = createMockGraph([topNode]) + + ;(subgraph as Subgraph & { rootGraph: LGraph }).rootGraph = rootGraph + targetNode.graph = subgraph + topNode.graph = rootGraph + + const nodeData = { id: 999, subgraphId: subgraphUuid } + const execId = getExecutionIdFromNodeData(rootGraph, nodeData) + + expect(execId).toBe('123:999') + }) + }) + describe('getNodeByLocatorId', () => { it('should find node in root graph', () => { const nodes = [createMockNode('123'), createMockNode('456')] diff --git a/src/utils/graphTraversalUtil.ts b/src/utils/graphTraversalUtil.ts index f4dec3d7aa..9794259f01 100644 --- a/src/utils/graphTraversalUtil.ts +++ b/src/utils/graphTraversalUtil.ts @@ -11,18 +11,16 @@ import { import { isSubgraphIoNode } from './typeGuardUtil' -interface NodeWithId { - id: string | number - subgraphId?: string | null -} - /** * Constructs a locator ID from node data with optional subgraph context. * * @param nodeData - Node data containing id and optional subgraphId * @returns The locator ID string */ -export function getLocatorIdFromNodeData(nodeData: NodeWithId): string { +export function getLocatorIdFromNodeData(nodeData: { + id: string | number + subgraphId?: string | null +}): string { return nodeData.subgraphId ? `${nodeData.subgraphId}:${String(nodeData.id)}` : String(nodeData.id) @@ -227,13 +225,17 @@ export function findSubgraphByUuid( graph: LGraph | Subgraph, targetUuid: string ): Subgraph | null { - // Check all nodes in the current graph + // Fast O(1) lookup via the root graph's centralized subgraph registry. + if ('subgraphs' in graph && graph.subgraphs instanceof Map) { + return graph.subgraphs.get(targetUuid) ?? null + } + + // Fallback: recursive traversal for non-root graphs without the registry. for (const node of graph.nodes) { if (node.isSubgraphNode?.() && node.subgraph) { if (node.subgraph.id === targetUuid) { return node.subgraph } - // Recursively search in nested subgraphs const found = findSubgraphByUuid(node.subgraph, targetUuid) if (found) return found } @@ -360,6 +362,26 @@ export function getExecutionIdByNode( return `${parentPath}:${node.id}` } +/** + * Returns the execution ID for a node described by plain data (id + subgraphId), + * without requiring a pre-existing {@link LGraphNode} reference. + * Subgraph nodes return the full colon-separated path (e.g. `"65:70:63"`). + * Falls back to `String(nodeData.id)` if the node cannot be resolved. + * + * @param rootGraph - The root graph to resolve from + * @param nodeData - Object with `id` (local node ID) and optional `subgraphId` (UUID) + */ +export function getExecutionIdFromNodeData( + rootGraph: LGraph, + nodeData: { id: string | number; subgraphId?: string | null } +): string { + const locatorId = getLocatorIdFromNodeData(nodeData) + const node = getNodeByLocatorId(rootGraph, locatorId) + return node + ? (getExecutionIdByNode(rootGraph, node) ?? String(nodeData.id)) + : String(nodeData.id) +} + /** * Get a node by its locator ID from anywhere in the graph hierarchy. * Locator IDs use UUID format like "uuid:nodeId" for subgraph nodes. @@ -635,15 +657,13 @@ export function getExecutionIdsForSelectedNodes( : findPartialExecutionPathToGraph(startGraph, rootGraph) if (parentPath === undefined) return [] + const buildExecId = (node: LGraphNode, parentExecutionId: string) => { + const nodeId = String(node.id) + return parentExecutionId ? `${parentExecutionId}:${nodeId}` : nodeId + } return collectFromNodes(selectedNodes, { - collector: (node, parentExecutionId) => { - const nodeId = String(node.id) - return parentExecutionId ? `${parentExecutionId}:${nodeId}` : nodeId - }, - contextBuilder: (node, parentExecutionId) => { - const nodeId = String(node.id) - return parentExecutionId ? `${parentExecutionId}:${nodeId}` : nodeId - }, + collector: buildExecId, + contextBuilder: buildExecId, initialContext: parentPath, expandSubgraphs: true })