From ca4e259169f0c97c4f7e59dd28a37928da868c8f Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Sun, 25 Oct 2026 17:22:47 -0700 Subject: [PATCH] fix(nodes): pin the Classic wildcard behaviour and match queued ids by type (#343) - Classic PromptManager/PromptManagerText expose _pmTextSerializerReady, settled once their own text serializer is installed; the real-host specs wait on it and assert the Classic text is queued as written, as on main. - The Primitive fill only touches an output entry whose class_type matches the graph node at that id. - TESTING.md notes the host's harmless "Invalid widget" warning per Run. --- TESTING.md | 11 ++- tests/comfyui/execution-text-link.spec.mjs | 14 ++-- tests/comfyui/text-link.spec.mjs | 8 +- web/comfy/prompt_manager.js | 91 ++++++++++++---------- 4 files changed, 70 insertions(+), 54 deletions(-) diff --git a/TESTING.md b/TESTING.md index 8c5509a5..249ace84 100644 --- a/TESTING.md +++ b/TESTING.md @@ -3983,8 +3983,11 @@ own serializer (dynamic prompts: `{red|blue}` becomes one choice). Each queued o filled once (the queue hook only fills an output that did not come through the wrapper), so a wildcard is drawn once per run. Inputs still bound to their widget (the Classic nodes) are left as the host resolved them; as on main, the Classic text widget's own -serializer queues its text as written, wildcards included. A Primitive drives the text as -before, also in workflows saved before #343. A linked +serializer queues its text as written, wildcards included (specs wait on the node's +`_pmTextSerializerReady` before queuing). A Primitive drives the text as before, also in +workflows saved before #343. On every Run the host logs "Invalid widget or widget name" +for a Primitive wired to a detached editor input: its own widget write finds no widget +binding there. That is expected and harmless. A linked Classic text box gives its canvas row back while linked (the core keeps an empty gap) and gets its size back on disconnect. The metadata parsers map a known node's positional widget values through its widget table when its widget-bound inputs do not line up with @@ -4020,8 +4023,8 @@ which used to save `null` under a link. real frontend through the sockets, with text from `PrimitiveString`, the legacy Primitive or through `PreviewAny`, for PromptManager, PromptManagerText, Focused, Split and the Pair (positive, negative, both, neither linked, and a Primitive negative). A - wildcard Primitive on PromptManager and Focused saves and counts the one choice that was - queued. The linked text is saved, counted once, recorded as + wildcard Primitive on PromptManager (queued as written) and Focused (one choice) saves + and counts what was queued. The linked text is saved, counted once, recorded as the run's raw positive/negative, and after a scan the run's image links to it. Text only known at run time (`PreviewAny`) cannot be read from the image; the case only requires that it is never linked to another prompt (its attribution from the recorded diff --git a/tests/comfyui/execution-text-link.spec.mjs b/tests/comfyui/execution-text-link.spec.mjs index 26dea2f6..8eefb6da 100644 --- a/tests/comfyui/execution-text-link.spec.mjs +++ b/tests/comfyui/execution-text-link.spec.mjs @@ -68,6 +68,8 @@ function build(page, { kind, positive, negative, prefix }) { const save = add('SaveImage', 2100, 0); save.widgets.find(widget => widget.name === 'filename_prefix').value = prefix; connect(images, 0, save, 'images'); + // The Classic nodes install their own text serializer shortly after creation; queue after it, as a user does. + await prompt._pmTextSerializerReady; const { output, workflow } = await app.graphToPrompt(); return { output, workflow, promptNode: String(prompt.id) }; }, { kind, positive, negative, prefix }); @@ -150,17 +152,17 @@ for (const kind of ['PromptManager', 'PromptManagerText', 'PromptManagerFocused' } } -// A legacy Primitive's wildcard is drawn once when the workflow is queued: the run saves and counts that one choice. +// A legacy Primitive's wildcard is drawn once when the workflow is queued: the run saves and counts what was queued. for (const kind of ['PromptManager', 'PromptManagerFocused']) { - test(`${kind} saves and counts the one choice a wildcard legacy Primitive queued (#343 re-review R1)`, async ({ execution, page }) => { + test(`${kind} saves and counts the text a wildcard legacy Primitive queued (#343 re-review R1)`, async ({ execution, page }) => { test.setTimeout(90000); const stem = unique(`Wildcard ${kind}`); const evidence = await run(execution, page, { kind, positive: { typed: 'Typed under the link', text: `${stem} {alpha|beta}`, via: 'legacy' } }); const queuedText = evidence.output[evidence.promptNode].inputs.text; - // The Classic node keeps its widget binding: the host applies the Primitive, and its text widget queues the text - // as written once its own serializer is installed (50 ms after creation), as on main. - const choices = [`${stem} alpha`, `${stem} beta`, ...(kind === 'PromptManager' ? [`${stem} {alpha|beta}`] : [])]; - expect(choices).toContain(queuedText); + // The Classic node keeps its widget binding: the host applies the Primitive, and its own text serializer + // queues the text as written, as on main. The Focused editor queues one choice. + if (kind === 'PromptManager') expect(queuedText).toBe(`${stem} {alpha|beta}`); + else expect([`${stem} alpha`, `${stem} beta`]).toContain(queuedText); expect(evidence.context.positive.raw).toBe(queuedText); expect(evidence.usage).toEqual([{ role: 'positive', run_count: 1 }]); const saved = await (await execution.request.get(`/prompt_manager/library/${evidence.context.positive_prompt_id}`)).json(); diff --git a/tests/comfyui/text-link.spec.mjs b/tests/comfyui/text-link.spec.mjs index 9b40b38b..6bfaf01a 100644 --- a/tests/comfyui/text-link.spec.mjs +++ b/tests/comfyui/text-link.spec.mjs @@ -223,6 +223,8 @@ test('a wildcard legacy Primitive queues one resolved choice, drawn once per run primitive.properties['Run widget replace on values'] = replace; ids[type] = [String(node.id), name]; }); + // The Classic nodes install their own text serializer shortly after creation; run after it, as a user does. + await Promise.all(app.graph.nodes.map(node => node._pmTextSerializerReady)); // What another extension's graphToPrompt wrapper sees must be what the server gets. const inner = app.graphToPrompt; const seen = window.__pmSeen = []; @@ -240,12 +242,12 @@ test('a wildcard legacy Primitive queues one resolved choice, drawn once per run const year = String(new Date().getFullYear()); const stems = { PromptManagerFocused: 'Focused', PromptManagerSplit: `Split ${year}`, PromptManagerPair: 'Pair', PromptManager: 'Classic', PromptManagerText: 'Text' }; // The Classic nodes keep their widget binding, so the host applies the Primitive and #343 leaves the value alone. - // As on main, their text widget's own serializer (installed 50 ms after creation) queues the text as written. + // As on main, their text widget's own serializer queues the text as written, wildcard included. const classic = new Set(['PromptManager', 'PromptManagerText']); for (const [type, [id, name]] of Object.entries(ids)) { const value = prompt[id].inputs[name]; - const expected = classic.has(type) ? `(?:c\\d|${choices.replace(/[{}|]/g, '\\$&')})` : 'c\\d'; - expect(value, type).toMatch(new RegExp(`^${stems[type]} ${expected}$`)); + if (classic.has(type)) expect(value, type).toBe(`${stems[type]} ${choices}`); + else expect(value, type).toMatch(new RegExp(`^${stems[type]} c\\d$`)); expect(value, `${type}: one draw per run`).toBe(seen[0][id].inputs[name]); } }); diff --git a/web/comfy/prompt_manager.js b/web/comfy/prompt_manager.js index 0c855fc9..51c1bdca 100644 --- a/web/comfy/prompt_manager.js +++ b/web/comfy/prompt_manager.js @@ -255,7 +255,9 @@ async function applyPrimitiveText(output) { primitiveTextApplied.add(output); const replaceText = window.comfyAPI?.utils?.applyTextReplacements; for (const [id, node] of queuedNodes(app.graph)) { - const inputs = promptManagerTypes.has(node.type) ? output[id]?.inputs : null; + const queued = promptManagerTypes.has(node.type) ? output[id] : null; + // Only an output of this graph: the same id must hold the same node type. + const inputs = queued?.class_type === node.type ? queued.inputs : null; if (!inputs) continue; for (const name of ['text', 'negative_text']) { if (Array.isArray(inputs[name])) continue; @@ -340,53 +342,60 @@ app.registerExtension({ // CRITICAL FIX: Ensure text widget always returns current value during serialization // This fixes the issue where graphToPrompt reads stale cached values const self = this; + // Settles once the serializer below is in place (or skipped), for callers that must queue after it. + let textSerializerSettled; + this._pmTextSerializerReady = new Promise((resolve) => { textSerializerSettled = resolve; }); this._textSyncTimer = setTimeout(() => { - if (self._classicScope?.signal.aborted) return; - const textWidget = self.widgets?.find((w) => w.name === "text"); - if (textWidget) { - // Store reference to the widget for the closure - const widgetRef = textWidget; - const originalSerializeValue = textWidget.serializeValue; + try { + if (self._classicScope?.signal.aborted) return; + const textWidget = self.widgets?.find((w) => w.name === "text"); + if (textWidget) { + // Store reference to the widget for the closure + const widgetRef = textWidget; + const originalSerializeValue = textWidget.serializeValue; - // Override serializeValue - use arrow function to capture widgetRef - textWidget.serializeValue = (nodeId, widgetIndex) => { - // Always return the current DOM value using our stored reference - if (widgetRef.inputEl) { - const currentValue = widgetRef.inputEl.value; - // Sync widget.value as well - widgetRef.value = currentValue; - return currentValue; - } - return widgetRef.value; - }; - - // Add aggressive sync listeners for all value change events - if (textWidget.inputEl) { - const syncValue = function() { - const currentVal = textWidget.inputEl.value; - textWidget.value = currentVal; - // Also update widgets_values array - const widgetIndex = self.widgets?.indexOf(textWidget); - if (self.widgets_values && widgetIndex >= 0) { - self.widgets_values[widgetIndex] = currentVal; + // Override serializeValue - use arrow function to capture widgetRef + textWidget.serializeValue = (nodeId, widgetIndex) => { + // Always return the current DOM value using our stored reference + if (widgetRef.inputEl) { + const currentValue = widgetRef.inputEl.value; + // Sync widget.value as well + widgetRef.value = currentValue; + return currentValue; } + return widgetRef.value; }; - // Sync on every keystroke - const listen = self._classicScope - ? (event) => self._classicScope.listen(textWidget.inputEl, event, syncValue) - : (event) => textWidget.inputEl.addEventListener(event, syncValue); - listen('input'); - // Sync on change (for paste, autocomplete, etc.) - listen('change'); - // Sync when focus leaves the field - listen('blur'); - // Sync on keyup as backup - listen('keyup'); + // Add aggressive sync listeners for all value change events + if (textWidget.inputEl) { + const syncValue = function() { + const currentVal = textWidget.inputEl.value; + textWidget.value = currentVal; + // Also update widgets_values array + const widgetIndex = self.widgets?.indexOf(textWidget); + if (self.widgets_values && widgetIndex >= 0) { + self.widgets_values[widgetIndex] = currentVal; + } + }; - // Initial sync - syncValue(); + // Sync on every keystroke + const listen = self._classicScope + ? (event) => self._classicScope.listen(textWidget.inputEl, event, syncValue) + : (event) => textWidget.inputEl.addEventListener(event, syncValue); + listen('input'); + // Sync on change (for paste, autocomplete, etc.) + listen('change'); + // Sync when focus leaves the field + listen('blur'); + // Sync on keyup as backup + listen('keyup'); + + // Initial sync + syncValue(); + } } + } finally { + textSerializerSettled(); } }, 50); // Shorter delay to beat other extensions