From feabbaaa4bdfee63587d63bfbff5e8d76194fbc4 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:56:34 +0300 Subject: [PATCH] fix: a node change holds runs back from the moment it is asked; values typed for a swapped node are dropped, not misplaced; problems naming no slot first stay visible in the simple view Co-Authored-By: Claude Opus 5.5 --- app/ui/features/settings/models.js | 14 ++++++--- app/ui/features/settings/models.test.js | 14 +++++++++ app/ui/shell/pipeline_state.js | 38 ++++++++++++++++------- app/ui/shell/tests/pipeline_state.test.js | 25 +++++++++++++++ 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/app/ui/features/settings/models.js b/app/ui/features/settings/models.js index 387bd8c9..52f152b1 100644 --- a/app/ui/features/settings/models.js +++ b/app/ui/features/settings/models.js @@ -48,7 +48,8 @@ export const models = (app) => function mount() { missing.forEach((n) => { if (!(n in specs)) specs[n] = null; }); } const setInput = (slot, name) => async (value) => { - if (JSON.stringify((slot.inputs || {})[name]) === JSON.stringify(value)) return; // a blur that changed nothing saves nothing + const live = slotsNow().find((x) => x.id === slot.id) || slot; // the slot as it is now, not as drawn: an answer may have replaced it + if (JSON.stringify((live.inputs || {})[name]) === JSON.stringify(value)) return; // a blur that changed nothing saves nothing await ps.save({ inputs: { [slot.id]: { [name]: value } } }); say([...(ps.refused() || []), ...ps.saveNotes()].join(" ")); }; @@ -165,11 +166,15 @@ export const models = (app) => function mount() { return { node }; }; + // A problem that names no slot first (a loop: "a feeds b feeds a") has no card of its own: its words are shown as they are, + // and every slot it names counts as broken, so the simple view cannot hide it. + const unclaimed = () => ps.incomplete().filter((t) => !slotsNow().some((x) => mine(t, x.id))); + const namedIn = (id, texts) => texts.some((t) => new RegExp(`(^|[^\\w.])${id.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}(?![\\w])`).test(t)); function status(only) { - const all = ps.incomplete(), refused = ps.refused(), notes = ps.saveNotes(); + const all = ps.incomplete(), refused = ps.refused(), notes = ps.saveNotes(), loose = unclaimed(); const incomplete = only ? all.filter((t) => mine(t, only)) : all; return [refused.length ? c.banner.warn({ text: `Could not save: ${refused.join(" ")}` }) : null, notes.length ? c.banner.info({ text: notes.join(" ") }) : null, - incomplete.length ? c.banner.warn({ text: only ? incomplete.join(" ") : `Not ready to generate yet: ${incomplete.length} node${incomplete.length === 1 ? "" : "s"} still need something (the orange dots). Open one to see what.` }) : ps.queueable() && !only ? c.hint.default({ text: "Every slot is filled — this pipeline is ready to generate." }) : null]; + incomplete.length ? c.banner.warn({ text: only ? incomplete.join(" ") : `Not ready to generate yet: ${incomplete.length} node${incomplete.length === 1 ? "" : "s"} still need something (the orange dots). Open one to see what.${loose.length ? ` ${loose.join(" ")}` : ""}` }) : ps.queueable() && !only ? c.hint.default({ text: "Every slot is filled — this pipeline is ready to generate." }) : null]; } const tools = () => c.toolbar.default({ items: [c.button.sm({ label: "Start from…", tone: "ghost", title: "Replace this pipeline with a model's own starting point", onClick: presets }), @@ -223,7 +228,8 @@ export const models = (app) => function mount() { let everything = false; try { everything = localStorage.getItem(ALL_KEY) === "1"; } catch { /* the simple view */ } const show = (on) => { try { on ? localStorage.setItem(ALL_KEY, "1") : localStorage.removeItem(ALL_KEY); } catch { /* this visit only */ } draw(); }; - const chosen = slots.filter((x) => needsChoice(x, specs[x.node], specs[x.node] === null || ps.incomplete().some((t) => mine(t, x.id)))); + const loose = unclaimed(); + const chosen = slots.filter((x) => needsChoice(x, specs[x.node], specs[x.node] === null || ps.incomplete().some((t) => mine(t, x.id)) || namedIn(x.id, loose))); const grid = c.region.stack({ gap: "none", children: everything ? [...slots.filter((x) => (x.group || "Other") === group).map(nodeCard), group ? addCard() : null].filter(Boolean) : chosen.map(nodeCard) }); diff --git a/app/ui/features/settings/models.test.js b/app/ui/features/settings/models.test.js index fb5ef73c..9b72bc39 100644 --- a/app/ui/features/settings/models.test.js +++ b/app/ui/features/settings/models.test.js @@ -66,3 +66,17 @@ test("the simple view shows only nodes that ask for a choice (files, sampler, br assert.ok(page.node.textContent.includes("Prompt"), "the groups are back"); page.node.remove(); localStorage.clear(); delete globalThis.localStorage; }); + +test("a problem that names no slot first (a loop) still shows its nodes in the simple view, and says its words", async () => { + const slots = [{ id: "a", node: "PlumbA", inputs: {} }, { id: "b", node: "PlumbB", inputs: {} }, { id: "c", node: "PlumbC", inputs: {} }]; + const ps = { slots: () => slots, snapshot: () => ({ slots }), ensureLoaded: async () => {}, subscribe: () => () => {}, restore: async () => ({ refused: [] }), + loading: () => false, loadError: () => null, saveNotes: () => [], incomplete: () => ["a feeds b feeds a: a slot cannot end up feeding itself"], + refused: () => [], queueable: () => false, removedIds: () => [], offered: () => [] }; + const nodes = { PlumbA: { widgets: [] }, PlumbB: { widgets: [] }, PlumbC: { widgets: [] } }; + const page = models({ pipeline: ps, api: { describeNodes: async () => ({ nodes }) } })(); + document.body.append(page.node); + await new Promise((r) => setTimeout(r, 0)); + assert.equal(page.node.querySelectorAll(".fp-node-card").length, 2, "a and b, not c"); + assert.match(page.node.textContent, /cannot end up feeding itself/); + page.node.remove(); +}); diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index 112d6ae7..7d6e4f78 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -95,7 +95,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { let deferred = null; // a project's saved pipeline ([slots, removed, unwired, whole]) waiting for the first successful load let epoch = 0; // bumped whenever a project's pipeline replaces the live one let saving = false; - let shaping = false; // a structural edit (node swapped, added, removed) waiting for the server + let shaping = 0; // node changes (swap, add, remove, preset, import) asked for and not yet answered: counted from the ask let pending = false; let pendingBody = null; let saveNotes = []; @@ -220,8 +220,16 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { } // An answer replaces the slots with the server's copy of what was SENT: whatever was typed while it was on its way // (node values still queued, module values) goes back on, or the screen and the next run lose it. - function keepTyped() { - placeInputs(pendingBody && pendingBody.inputs); + // A value typed for a node that the answer swapped for another belongs to nothing now: it is dropped, not put on the + // new node (which has no such input, and would be refused for it). + function keepTyped(before) { + const was = new Map((before || []).map((x) => [x.id, x.node])); + const typed = pendingBody && pendingBody.inputs; + if (typed && slots) for (const id of Object.keys(typed)) { + const now = slots.find((x) => x.id === id); + if (!now || (was.has(id) && was.get(id) !== now.node)) delete typed[id]; + } + placeInputs(typed); if (Object.keys(pendingValues).length) mirrorValues(currentValues()); } async function save(body) { @@ -242,8 +250,9 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // A different pipeline was put in while this was in flight (another project opened): // this answer is about the old one and must not overwrite the new. if (mine !== epoch) continue; + const before = slots; if (res && res.slots) slots = res.slots; - keepTyped(); // an edit queued behind this one stays in the slots a run reads + keepTyped(before); // an edit queued behind this one stays in the slots a run reads applyGroups(); incomplete = (res && res.incomplete) || []; refused = (res && res.refused) || []; @@ -431,7 +440,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // A change to the pipeline's SHAPE (add / replace / remove / wire / unwire), sent on its own: two // of these folded into one request would lose the first. Waits for any value edit in flight and // for a project's pipeline going in. -> {refused: [...]}; empty when it happened. - async function edit(body) { + async function editNow(body) { await ensureLoaded(); if (slots === null) return { refused: ["The pipeline has not loaded: is ComfyUI reachable?"] }; if (adoptGate) await adoptGate; @@ -441,14 +450,14 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { const keep = currentValues(); // a settings node added or swapped in starts empty: it gets these let refill = false; const mine = epoch; - saving = true; shaping = true; + saving = true; let refusedNow = []; try { applyGroups(); const res = await API.editPipeline({ slots, ...body }); if (mine !== epoch) return { refused: ["The project was changed while this was being sent."] }; refusedNow = (res && res.refused) || []; - if (res && res.slots && !refusedNow.length) { slots = res.slots; keepTyped(); applyGroups(); } + if (res && res.slots && !refusedNow.length) { const before = slots; slots = res.slots; keepTyped(before); applyGroups(); } incomplete = (res && res.incomplete) || []; refused = refusedNow; queueable = !!(res && res.queueable); @@ -480,7 +489,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { } catch (e) { refusedNow = [e && e.message ? e.message : String(e)]; } finally { - saving = false; shaping = false; + saving = false; } if (refill || pending) await save({}); // the filled settings, or a value edit queued behind this one return { refused: refusedNow }; @@ -492,7 +501,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { return { slots: JSON.parse(JSON.stringify(slots)), removed: [...removed], unwired: JSON.parse(JSON.stringify(unwired)), whole }; } // -> {refused: [...]}; empty when the snapshot is back. Announced as a change, so the project follows. - async function restore(snap) { + async function restoreNow(snap) { if (!snap || !Array.isArray(snap.slots)) return { refused: ["Nothing to go back to."] }; // Already what is live: nothing to do. Above all not a change of shape, which would let Revert replace a refused saved pipeline. if (JSON.stringify(snap.slots) === JSON.stringify(slots)) return { refused: [] }; @@ -503,15 +512,16 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { const keep = currentValues(); let refill = false; const mine = epoch; - saving = true; shaping = true; + saving = true; let refusedNow = []; try { const res = await API.editPipeline({ slots: JSON.parse(JSON.stringify(snap.slots)) }); if (mine !== epoch) return { refused: ["The project was changed while this was being sent."] }; refusedNow = (res && res.refused) || []; if (res && res.slots && !refusedNow.length) { + const before = slots; slots = res.slots; - keepTyped(); + keepTyped(before); // A default slot the new pipeline lacks is removed, whatever the caller said: otherwise the next // open lays the default back over it (a preset, an import or the wizard would regain the default's loaders). const kept = new Set(res.slots.map((s) => s.id)); @@ -528,7 +538,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { } catch (e) { refusedNow = [e && e.message ? e.message : String(e)]; } finally { - saving = false; shaping = false; + saving = false; } if (refill) await save({}); return { refused: refusedNow }; @@ -580,6 +590,10 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { /** Resolves true once no edit is on its way and no project's pipeline is going in; false if that took over * three minutes (every request gives up after ANSWER_MS, so only a server failing again and again gets there). */ + // Counted from the moment one is asked for, not from when it gets its turn behind a save: a run started in between + // would freeze the pipeline as it was before the change. + const shape = (fn) => async (...args) => { shaping += 1; try { return await fn(...args); } finally { shaping -= 1; } }; + const edit = shape(editNow), restore = shape(restoreNow); // What a run needs finished: a project's pipeline going in, or a node swapped/added/removed (that shape exists only once // the server answers). A value save is not waited on: the value is already in the slots (placeInputs, mirrorValues). async function readyToRun() { diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index 7489effb..546cf1db 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -13,6 +13,7 @@ function load(posts, opts = {}) { editPipeline: async (body) => { body = JSON.parse(JSON.stringify(body)); // sent over HTTP: the server never shares the page's objects posts.push(body); + if (body.action === "replace") { const sl = body.slots.find((x) => x.id === body.slot); if (sl) { sl.node = body.node; sl.inputs = {}; } } if (opts.delay) await new Promise((r) => setTimeout(r, opts.delay)); const bad = (body.slots || []).findIndex((s) => typeof s.node !== "string"); if (bad >= 0) { const e = new Error(`slot ${bad} has no node`); throw e; } for (const [id, edits] of Object.entries(body.inputs || {})) { const sl = body.slots.find((x) => x.id === id); if (sl) sl.inputs = { ...sl.inputs, ...edits }; } @@ -686,3 +687,27 @@ test("after a preset or import goes in, a run is ready at once (the node-change assert.strictEqual(await PS.readyToRun(), true); assert.ok(Date.now() - t0 < 100); }); + +test("a value typed for a node while it is being swapped is dropped, not put on the node that replaces it", async () => { + const posts = []; + const PS = load(posts, { delay: 40 }); + await PS.ensureLoaded(); + const swap = PS.edit({ action: "replace", slot: "gen", node: "Other" }); + await new Promise((r) => setTimeout(r, 10)); + const typed = PS.save({ inputs: { gen: { steps: 33 } } }); + await swap; await typed; await PS.settled(); + const gen = PS.slots().find((x) => x.id === "gen"); + assert.strictEqual(gen.node, "Other"); + assert.strictEqual(gen.inputs.steps, undefined, "no stray input the new node cannot take"); + assert.ok(posts.every((b) => !(b.inputs && b.inputs.gen && "steps" in b.inputs.gen) || !b.slots.some((x) => x.id === "gen" && x.node === "Other"))); +}); + +test("a node change asked for while a value save is on its way holds a run back until it lands", async () => { + const PS = load([], { delay: 60 }); + await PS.ensureLoaded(); + PS.save({ inputs: { gen: { steps: 12 } } }); + const swap = PS.edit({ action: "replace", slot: "gen", node: "Other" }); // queued behind the save + await PS.readyToRun(); + assert.strictEqual(PS.slots().find((x) => x.id === "gen").node, "Other", "the run freezes the pipeline after the swap"); + await swap; +});