From 1276673e0e7790dab751dddfcd3823ebee3559dd Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 17 Oct 2026 17:12:41 +0800 Subject: [PATCH] fix(loaders): migrate backslash values in subgraphs and clear the error flag (#1149) The follow-up report found three gaps in the value migration: * applyToAllNodes only walked the root graph, so loader nodes inside subgraphs kept their backslash values. It now recurses through node.subgraph (Subgraph extends LGraph, so .nodes works at every level). Promoted combo widgets share the interior widget's store-backed value, so migrating the interior widget fixes the subgraph boundary too. * Pasted nodes never hit the graph-level configure hook. A per-node onConfigure wrapper now runs the same sanitize + migrate pass after widget values restore. * The corrected value kept its stale "value not in list" flag. The migration now fires the node's onWidgetChanged hook, which the frontend's error-clearing chain uses to drop value_not_in_list errors. --- .../frontend/core/randomLoaderControl.test.js | 54 ++++++++++++++++++- web/comfyui/random_loader_control.js | 40 ++++++++++---- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/tests/frontend/core/randomLoaderControl.test.js b/tests/frontend/core/randomLoaderControl.test.js index 73fad4ce..abcc5ec4 100644 --- a/tests/frontend/core/randomLoaderControl.test.js +++ b/tests/frontend/core/randomLoaderControl.test.js @@ -269,7 +269,7 @@ describe("model widget value migration", () => { for (const node of nodes) { const nodeType = { comfyClass: node.comfyClass, prototype: {} }; extension.beforeRegisterNodeDef(nodeType, {}); - nodeType.prototype.onAdded.call({ graph: state.graph }); + nodeType.prototype.onAdded?.call({ graph: state.graph }); } state.graph.onConfigure({}); @@ -327,4 +327,56 @@ describe("model widget value migration", () => { expect(modelWidget(node, "ckpt_name").value).toBe("dir/model.safetensors"); }); + + it("migrates nodes nested inside subgraphs", async () => { + const inner = makeCheckpointNode(); + modelWidget(inner, "ckpt_name").value = "dir\\model.safetensors"; + const subgraphNode = { + comfyClass: "MySubgraph", + widgets: [], + subgraph: { nodes: [inner] }, + }; + // A root-level loader node installs the graph configure hook. + const root = makeCheckpointNode(); + + await loadGraphWith([root, subgraphNode]); + + expect(modelWidget(inner, "ckpt_name").value).toBe("dir/model.safetensors"); + }); + + it("fires onWidgetChanged so the frontend clears the stale error flag", async () => { + const node = makeCheckpointNode(); + const widget = modelWidget(node, "ckpt_name"); + widget.value = "dir\\model.safetensors"; + node.onWidgetChanged = vi.fn(); + + await loadGraphWith([node]); + + expect(node.onWidgetChanged).toHaveBeenCalledWith( + "ckpt_name", + "dir/model.safetensors", + "dir\\model.safetensors", + widget + ); + }); + + it("migrates values restored through node.onConfigure (paste)", async () => { + const extension = state.registerExtension.mock.calls.map( + (call) => call[0] + )[0]; + await extension.setup(); + + const nodeType = { + comfyClass: "Checkpoint Loader (LoraManager)", + prototype: {}, + }; + extension.beforeRegisterNodeDef(nodeType, {}); + + const node = makeCheckpointNode(); + modelWidget(node, "ckpt_name").value = "dir\\model.safetensors"; + + nodeType.prototype.onConfigure.call(node, {}); + + expect(modelWidget(node, "ckpt_name").value).toBe("dir/model.safetensors"); + }); }); \ No newline at end of file diff --git a/web/comfyui/random_loader_control.js b/web/comfyui/random_loader_control.js index 3d252965..4d1d6f9c 100644 --- a/web/comfyui/random_loader_control.js +++ b/web/comfyui/random_loader_control.js @@ -61,14 +61,16 @@ async function refreshPoolCache() { * to its separator-normalized form when that form is a known model, so old * workflows keep their selection instead of resetting to the first model. */ -function migrateModelWidgetValue(modelWidget, names) { +function migrateModelWidgetValue(node, modelWidget, names) { const value = modelWidget.value; if (typeof value !== "string" || !value.includes("\\")) return; if (names.includes(value)) return; const normalized = value.replace(/\\/g, "/"); - if (names.includes(normalized)) { - modelWidget.value = normalized; - } + if (!names.includes(normalized)) return; + modelWidget.value = normalized; + // Fire the standard widget-changed hook so the frontend's error-clearing + // chain drops the stale "value not in list" flag on the corrected widget. + node?.onWidgetChanged?.(modelWidget.name, normalized, value, modelWidget); } function applyBaseModelFilter(node, config) { @@ -85,6 +87,7 @@ function applyBaseModelFilter(node, config) { // Migrate against the full pool: separator spelling is independent of the // base_model filter, and wired widgets skip the filter entirely below. migrateModelWidgetValue( + node, modelWidget, pool.map((model) => model.name) ); @@ -166,12 +169,19 @@ export function sanitizeControlWidget(node, config) { } function applyToAllNodes() { - app.graph?.nodes?.forEach((node) => { - const config = NODE_CONFIGS[node.comfyClass]; - if (!config) return; - sanitizeControlWidget(node, config); - applyBaseModelFilter(node, config); - }); + // Subgraphs nest loader nodes; Subgraph extends LGraph, so the same .nodes + // list recurses through every level. + const visit = (graph) => { + for (const node of graph?.nodes ?? []) { + const config = NODE_CONFIGS[node.comfyClass]; + if (config) { + sanitizeControlWidget(node, config); + applyBaseModelFilter(node, config); + } + if (node.subgraph) visit(node.subgraph); + } + }; + visit(app.graph); } function ensureGraphConfigureHook(graph) { @@ -228,6 +238,16 @@ app.registerExtension({ ensureGraphConfigureHook(this.graph); return result; }; + + // onConfigure fires after widget values are restored, catching pasted + // nodes that never go through the graph-level configure hook. + const onConfigure = nodeType.prototype.onConfigure; + nodeType.prototype.onConfigure = function () { + const result = onConfigure?.apply(this, arguments); + sanitizeControlWidget(this, config); + applyBaseModelFilter(this, config); + return result; + }; }, async refreshComboInNodes() {