From c712a079a9b39fe7b0ea2dc7e8872d42e1a3f509 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 31 Oct 2026 10:34:39 +0800 Subject: [PATCH] fix(widgets): stop load-time widget rewrites from marking workflows dirty TriggerWord Toggle serialized the transient `highlighted` UI state into widgets_values, but highlight is recomputed after load from the current selection/library, so an untouched workflow always compared dirty against the saved JSON and ComfyUI prompted to save on close. Strip `highlighted` from serialized tag values (backend never reads it). Also restore saved widget values by widget name instead of positional index (position drift corrupted unrelated widget slots on load), and preserve `locked` in mergeLoras so load-time merging no longer drops it. Workflows saved with `highlighted` keys will show as modified once after upgrading; saving them once writes the clean format. Fixes #893 --- .../triggerWordToggle.serialization.test.js | 197 ++++++++++++++++++ web/comfyui/trigger_word_toggle.js | 35 ++-- web/comfyui/utils.js | 1 + 3 files changed, 216 insertions(+), 17 deletions(-) create mode 100644 tests/frontend/components/triggerWordToggle.serialization.test.js diff --git a/tests/frontend/components/triggerWordToggle.serialization.test.js b/tests/frontend/components/triggerWordToggle.serialization.test.js new file mode 100644 index 00000000..bd58b70c --- /dev/null +++ b/tests/frontend/components/triggerWordToggle.serialization.test.js @@ -0,0 +1,197 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +const { + APP_MODULE, + API_MODULE, + UTILS_MODULE, + TAGS_WIDGET_MODULE, + SETTINGS_MODULE, + TRIGGER_WORD_TOGGLE_MODULE, +} = vi.hoisted(() => ({ + APP_MODULE: new URL("../../../scripts/app.js", import.meta.url).pathname, + API_MODULE: new URL("../../../scripts/api.js", import.meta.url).pathname, + UTILS_MODULE: new URL("../../../web/comfyui/utils.js", import.meta.url).pathname, + TAGS_WIDGET_MODULE: new URL("../../../web/comfyui/tags_widget.js", import.meta.url).pathname, + SETTINGS_MODULE: new URL("../../../web/comfyui/settings.js", import.meta.url).pathname, + TRIGGER_WORD_TOGGLE_MODULE: new URL("../../../web/comfyui/trigger_word_toggle.js", import.meta.url).pathname, +})); + +const extensionState = { current: null }; +const registerExtensionMock = vi.fn((extension) => { + extensionState.current = extension; +}); + +vi.mock(APP_MODULE, () => ({ + app: { + registerExtension: registerExtensionMock, + graph: {}, + }, +})); + +vi.mock(API_MODULE, () => ({ + api: { + addEventListener: vi.fn(), + }, +})); + +vi.mock(UTILS_MODULE, () => ({ + CONVERTED_TYPE: "converted-widget", + getNodeFromGraph: vi.fn(), + getWidgetSerializedValue: (node, widgetName) => { + if (!node || !Array.isArray(node.widgets) || !Array.isArray(node.widgets_values)) { + return undefined; + } + const widgetIndex = node.widgets.findIndex((widget) => widget?.name === widgetName); + return widgetIndex >= 0 ? node.widgets_values[widgetIndex] : undefined; + }, +})); + +vi.mock(TAGS_WIDGET_MODULE, () => ({ + addTagsWidget: (node, name) => { + const widget = { name, value: [] }; + node.widgets.push(widget); + return { widget }; + }, +})); + +vi.mock(SETTINGS_MODULE, () => ({ + getWheelSensitivity: () => 0.02, +})); + +function createNode({ widgetsValues } = {}) { + const node = { + comfyClass: "TriggerWord Toggle (LoraManager)", + widgets: [ + { name: "group_mode", value: true }, + { name: "default_active", value: true }, + { name: "allow_strength_adjustment", value: false }, + ], + widgets_values: widgetsValues, + addInput: vi.fn(), + addWidget: vi.fn((type, name, value) => { + const widget = { type, name, value }; + node.widgets.push(widget); + return widget; + }), + }; + return node; +} + +describe("TriggerWord Toggle serialization", () => { + const originalRequestAnimationFrame = globalThis.requestAnimationFrame; + + beforeEach(async () => { + vi.resetModules(); + + extensionState.current = null; + registerExtensionMock.mockClear(); + + // Run nodeCreated's deferred setup synchronously. + globalThis.requestAnimationFrame = (callback) => { + callback(); + return 0; + }; + + await import(TRIGGER_WORD_TOGGLE_MODULE); + }); + + afterEach(() => { + globalThis.requestAnimationFrame = originalRequestAnimationFrame; + }); + + it("restores saved widget values by widget name", async () => { + const savedTags = [ + { text: "foo", active: true, strength: null, highlighted: true }, + ]; + const node = createNode({ + widgetsValues: [true, true, false, savedTags, "foo, bar"], + }); + + await extensionState.current.nodeCreated(node); + + expect(node.tagWidget).toBeDefined(); + expect(node.tagWidget.value).toHaveLength(1); + expect(node.tagWidget.value[0]).toMatchObject({ text: "foo", active: true }); + expect(node.originalMessageWidget.value).toBe("foo, bar"); + }); + + it("strips transient highlighted state from serialized tag values", async () => { + const node = createNode({ + widgetsValues: [true, true, false, [], ""], + }); + + await extensionState.current.nodeCreated(node); + + node.tagWidget.value = [ + { text: "plain", active: true, highlighted: false, strength: null }, + { + text: "group a, group b", + active: true, + highlighted: true, + strength: null, + items: [ + { text: "group a", active: true, highlighted: true, strength: null }, + { text: "group b", active: false, highlighted: false, strength: null }, + ], + }, + ]; + + const serialized = node.tagWidget.serializeValue(); + + expect(serialized).toEqual([ + { text: "plain", active: true, strength: null }, + { + text: "group a, group b", + active: true, + strength: null, + items: [ + { text: "group a", active: true, strength: null }, + { text: "group b", active: false, strength: null }, + ], + }, + ]); + }); + + it("keeps strength text formatting while stripping highlighted state", async () => { + const node = createNode({ + widgetsValues: [true, true, false, [], ""], + }); + + await extensionState.current.nodeCreated(node); + + node.tagWidget.value = [ + { text: "styled", active: true, highlighted: true, strength: 0.5 }, + ]; + + const serialized = node.tagWidget.serializeValue(); + + expect(serialized).toEqual([ + { text: "(styled:0.50)", active: true, strength: 0.5 }, + ]); + }); + + it("serializes restored values identically to the loaded save data", async () => { + // Simulates a workflow saved by the current version (no highlighted keys): + // load -> restore -> highlight recompute must not change serialized output. + const savedTags = [ + { + text: "masterpiece, best quality", + active: true, + strength: null, + items: [ + { text: "masterpiece", active: true, strength: null }, + { text: "best quality", active: true, strength: null }, + ], + }, + ]; + const node = createNode({ + widgetsValues: [true, true, false, savedTags, "masterpiece, best quality"], + }); + + await extensionState.current.nodeCreated(node); + + // Highlight recompute ran during nodeCreated (empty highlight set). + const serialized = node.tagWidget.serializeValue(); + expect(serialized).toEqual(savedTags); + }); +}); diff --git a/web/comfyui/trigger_word_toggle.js b/web/comfyui/trigger_word_toggle.js index d2cd54e5..1374af66 100644 --- a/web/comfyui/trigger_word_toggle.js +++ b/web/comfyui/trigger_word_toggle.js @@ -1,6 +1,6 @@ import { app } from "../../scripts/app.js"; import { api } from "../../scripts/api.js"; -import { CONVERTED_TYPE, getNodeFromGraph } from "./utils.js"; +import { CONVERTED_TYPE, getNodeFromGraph, getWidgetSerializedValue } from "./utils.js"; import { addTagsWidget } from "./tags_widget.js"; import { getWheelSensitivity } from "./settings.js"; @@ -267,20 +267,17 @@ app.registerExtension({ hiddenWidget.computeSize = () => [0, -4]; node.originalMessageWidget = hiddenWidget; - const tagWidgetIndex = node.widgets.indexOf(result.widget); - const originalMessageWidgetIndex = node.widgets.indexOf(hiddenWidget); + // Restore saved values by widget name rather than position: positional + // lookups break (and corrupt unrelated widget values) whenever the + // widget layout of a saved workflow drifts from the current one. if (node.widgets_values && node.widgets_values.length > 0) { - if (tagWidgetIndex >= 0) { - const savedValue = node.widgets_values[tagWidgetIndex]; - if (savedValue) { - result.widget.value = Array.isArray(savedValue) ? savedValue : []; - } + const savedValue = getWidgetSerializedValue(node, "toggle_trigger_words"); + if (savedValue) { + result.widget.value = Array.isArray(savedValue) ? savedValue : []; } - if (originalMessageWidgetIndex >= 0) { - const originalMessage = node.widgets_values[originalMessageWidgetIndex]; - if (originalMessage) { - hiddenWidget.value = originalMessage; - } + const originalMessage = getWidgetSerializedValue(node, "orinalMessage"); + if (originalMessage) { + hiddenWidget.value = originalMessage; } } @@ -354,25 +351,29 @@ app.registerExtension({ result.widget.serializeValue = function() { const value = this.value || []; return value.map((tag) => { + // `highlighted` is transient UI state recomputed after load from the + // current selection; persisting it makes an untouched workflow + // compare dirty against the saved JSON. Strip it on serialize. + const { highlighted, ...serializableTag } = tag; if (Array.isArray(tag.items)) { return { - ...tag, + ...serializableTag, text: tag.strength !== undefined && tag.strength !== null ? `(${tag.text}:${tag.strength.toFixed(2)})` : tag.text, - items: tag.items.map((item) => ({ ...item })), + items: tag.items.map(({ highlighted: _itemHighlighted, ...item }) => item), }; } if (tag.strength !== undefined && tag.strength !== null) { return { - ...tag, + ...serializableTag, text: `(${tag.text}:${tag.strength.toFixed(2)})`, }; } - return tag; + return serializableTag; }); }; }); diff --git a/web/comfyui/utils.js b/web/comfyui/utils.js index e84000af..ed643023 100644 --- a/web/comfyui/utils.js +++ b/web/comfyui/utils.js @@ -548,6 +548,7 @@ export function mergeLoras(lorasText, lorasArr) { expanded: lora.expanded !== undefined ? lora.expanded : false, clipStrength: lora.clipStrength !== undefined ? lora.clipStrength : parsedLoras[lora.name].clipStrength, selected: !!lora.selected, + locked: lora.locked !== undefined ? lora.locked : false, }); usedNames.add(lora.name); }