diff --git a/app/ui/features/sampler/hub.test.js b/app/ui/features/sampler/hub.test.js index 317d0b85..c39047ea 100644 --- a/app/ui/features/sampler/hub.test.js +++ b/app/ui/features/sampler/hub.test.js @@ -105,3 +105,24 @@ test("a save queued behind another is not called failed: the window waits for it assert.equal(live[0].inputs.steps, 30); assert.equal(document.querySelectorAll("[class*=toast]").length, toasts); }); + +test("two quick edits of one input: the first is not called failed because the second replaced it", async () => { + let live = [{ id: "sampler", node: "S", inputs: { steps: 7 }, roles: [{ at: "generation.sampling", input: "steps", label: "Steps" }] }]; + let queued = null; + const ps = { slots: () => live, activeModules: () => [], useful: () => true, usefulness: () => () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, + subscribe: () => () => {}, ensureLoaded: async () => {}, saveNotes: () => [], + save: async (b) => { queued = { ...(queued || {}), ...b.inputs.sampler }; }, // queued: lands at settle, the newest winning + settled: async () => { await new Promise((r) => setTimeout(r, 5)); if (queued) { live = [{ ...live[0], inputs: { ...live[0].inputs, ...queued } }]; queued = null; } } }; + const host = document.createElement("div"); + document.body.append(host); + const toasts = document.querySelectorAll("[class*=toast]").length; + hub.setup({ host, app: { pipeline: ps, api: { describeNodes: async () => ({ nodes: { S: { widgets: [{ name: "steps", type: "INT", min: 1, max: 100 }] } } }) }, on: () => () => {}, actions: null } }); + await new Promise((r) => setTimeout(r, 0)); + host.querySelector("button").click(); + await new Promise((r) => setTimeout(r, 0)); + const input = [...document.querySelectorAll("input[type=number]")].pop(); + for (const v of ["20", "25"]) { input.value = v; input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true })); } + await new Promise((r) => setTimeout(r, 30)); + assert.equal(live[0].inputs.steps, 25); + assert.equal(document.querySelectorAll("[class*=toast]").length, toasts); +}); diff --git a/app/ui/features/sampler/index.js b/app/ui/features/sampler/index.js index 95cdd777..fc1c7f9d 100644 --- a/app/ui/features/sampler/index.js +++ b/app/ui/features/sampler/index.js @@ -13,6 +13,7 @@ export default { setup({ host, app }) { const ps = app.pipeline; let pop = null, specs = {}, unreadable = false; + const latest = {}; // the last value the person set per input: an older edit's save is judged by the newest const sampling = () => { const ok = ps.usefulness(); return ps.activeModules().filter((m) => (m.category || "") === "sampling" && Object.keys(m.settings || {}).length && ok(m)); }; const marked = () => (ps.slots() || []).flatMap((slot) => (slot.roles || []).filter((r) => r.at === "generation.sampling" && r.input && !Array.isArray((slot.inputs || {})[r.input])).map((role) => ({ slot, role }))); const describe = async () => { @@ -25,8 +26,11 @@ export default { const set = async (v) => { // compared with the slot as it is NOW: the one drawn may be several saves old const now = (ps.slots() || []).find((s) => s.id === slot.id) || slot; if (JSON.stringify((now.inputs || {})[role.input]) === JSON.stringify(v)) return; + const key = `${slot.id}.${role.input}`; + latest[key] = v; await ps.save({ inputs: { [slot.id]: { [role.input]: v } } }); if (ps.settled) await ps.settled(); // a save queued behind another returns before it lands + if (latest[key] !== v) return; // a newer edit of this input is the one that counts const after = (ps.slots() || []).find((s) => s.id === slot.id) || {}; if (JSON.stringify((after.inputs || {})[role.input]) !== JSON.stringify(v)) { // not saved: show what will really run, and why c.toast.warn({ text: [...(ps.saveNotes ? ps.saveNotes() : [])].find((n) => /could not save/i.test(n)) || `${role.label || role.input} was not saved.` }); diff --git a/app/ui/shell/pipeline_link.js b/app/ui/shell/pipeline_link.js index f02ed178..6b47a7af 100644 --- a/app/ui/shell/pipeline_link.js +++ b/app/ui/shell/pipeline_link.js @@ -60,6 +60,8 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { const reason = pipeline.refusal && pipeline.refusal(); return `ComfyUI refused this project's saved pipeline${reason ? ` (${reason})` : ""}, so the default is in use. Your next pipeline edit replaces the saved one; or open the project again to retry.`; }; + // An edit to the stand-in while this project's pipeline could not go in would be thrown away by the next retry: refused, said. + if (pipeline.setEditGuard) pipeline.setEditGuard(() => (owner === null && project.project && (waiting || retries > 0) && !replaceOnEdit ? `Could not save: ${why()}` : null)); const owns = () => !!project.project && owner === project.project.id; /** Why the open project's pipeline is not live, in words for the person; null when it is. */ const why = () => owns() ? null : waiting ? "ComfyUI is not answering, so this project's pipeline is not loaded yet. It goes in as soon as ComfyUI answers." diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index 7995b81f..3413c79a 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -204,8 +204,13 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // predates the other's. See engine_settings.js's original fix for why the // retry-after-in-flight shape exists (a same-panel double-edit dropped // one edit before this). + // Set by whoever ties the pipeline to a project: a reason when an edit now would land on a stand-in it could not keep. + let editGuard = () => null; + const blocked = () => { const why = editGuard(); if (why) { saveNotes = [why]; _changed("load"); } return why; }; // said, and views redraw to what is true + async function save(body) { if (adoptGate) await adoptGate; // an edit made while a project's pipeline goes in lands on THAT pipeline + if (blocked()) return; pendingBody = mergeBodies(pendingBody, body); if (saving) { pending = true; return; } saving = true; @@ -391,6 +396,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // at whichever write happens to be in flight when the queue drains. async function setModuleValue(moduleId, name, value) { if (adoptGate) await adoptGate; // the values tree must be the pipeline this lands on, not the one going out + if (blocked()) return; pendingValues[moduleId] = { ...(pendingValues[moduleId] || {}), [name]: value }; const values = currentValues(); mirrorValues(values); @@ -404,6 +410,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { await ensureLoaded(); if (slots === null) return { refused: ["The pipeline has not loaded: is ComfyUI reachable?"] }; if (adoptGate) await adoptGate; + { const why = blocked(); if (why) return { refused: [why] }; } for (let waited = 0; saving && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20)); if (saving) return { refused: ["A save is still in progress: try again in a moment."] }; const mine = epoch; @@ -460,6 +467,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { async function restore(snap) { if (!snap || !Array.isArray(snap.slots)) return { refused: ["Nothing to go back to."] }; if (adoptGate) await adoptGate; + { const why = blocked(); if (why) return { refused: [why] }; } for (let waited = 0; saving && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20)); if (saving) return { refused: ["A save is still in progress: try again in a moment."] }; const mine = epoch; @@ -617,6 +625,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // runs, said, and the caller must NOT save it over the project's own copy. slots = JSON.parse(JSON.stringify(offered)); removed = new Set(); unwired = {}; whole = false; // the stand-in is the plain default: no flags of the project it could not load + incomplete = []; refused = []; queueable = false; // nor the last project's readiness saveNotes = ["This project's saved pipeline could not be loaded, so the default is in use and edits are not being saved."]; return false; } @@ -638,7 +647,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { frozenInputs, ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, settled, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, modulesById: () => modulesById, - unreachable: () => slots === null || unreachableLast, refusal: () => refusalLast, activeModules, useful, usefulness, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, + setEditGuard: (fn) => { editGuard = fn; }, unreachable: () => slots === null || unreachableLast, refusal: () => refusalLast, activeModules, useful, usefulness, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, removedIds: () => [...removed], whole: () => whole, unwiredMap: () => JSON.parse(JSON.stringify(unwired)), diff --git a/app/ui/shell/tests/pipeline_link.test.js b/app/ui/shell/tests/pipeline_link.test.js index ae4ed917..e6fcc995 100644 --- a/app/ui/shell/tests/pipeline_link.test.js +++ b/app/ui/shell/tests/pipeline_link.test.js @@ -207,3 +207,22 @@ test("a pipeline ComfyUI refuses says the server's reason, and the person's next heard([{ ...slots[0], inputs: { ckpt_name: "fixed" } }], "edit"); assert.deepStrictEqual(writes, ["fixed"]); }); + +test("while a project's pipeline could not go in, edits to the stand-in are refused with the reason; once it is in, they are not", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + let opened, guard, up = false; + const project = { project: { id: "a", models: { slots, removed: [], unwired: {}, whole: false } }, setField: () => {} }; + const pipeline = { adopt: async () => up, slots: () => slots, unreachable: () => true, setEditGuard: (fn) => { guard = fn; }, + removedIds: () => [], unwiredMap: () => ({}), whole: () => false, subscribe: () => {} }; + linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; } }); + const flush = async () => { for (let i = 0; i < 5; i++) await new Promise((r) => setImmediate(r)); }; + const first = opened(); + assert.strictEqual(guard(), null); // a first load under way: an edit waits for it, it is not refused + await first; + assert.match(guard(), /^Could not save: .*still loading/); + for (let i = 0; i < 4; i++) { t.mock.timers.tick(6000); await flush(); } + assert.match(guard(), /ComfyUI is not answering/); + up = true; + t.mock.timers.tick(16000); await flush(); + assert.strictEqual(guard(), null); +}); diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index 6ea2456c..9c05dfb3 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -558,3 +558,21 @@ test("a second setting changed while the first is saving survives the first land assert.strictEqual(PS.currentValues().sharpen.amount, 0.9); assert.strictEqual(JSON.parse(PS.slots().find((s) => s.id === "settings").inputs.settings).sharpen.amount, 0.9); // what a run reads }); + +test("a guarded edit is refused, said, and leaves nothing on screen that will not run", async () => { + const PS = withSink("{}", [sharpen]); + await PS.ensureLoaded(); + PS.setEditGuard(() => "Could not save: not loaded"); + await PS.setModuleValue("sharpen", "enabled", true); + assert.deepStrictEqual([PS.currentValues().sharpen.enabled, PS.saveNotes()], [false, ["Could not save: not loaded"]]); + assert.deepStrictEqual((await PS.edit({ action: "remove", slot: "other" })).refused, ["Could not save: not loaded"]); +}); + +test("the stand-in for a pipeline that could not go in does not inherit the last project's readiness", async () => { + const PS = load([]); + await PS.ensureLoaded(); + assert.strictEqual(PS.queueable(), true); + lastApi.editPipeline = async () => { throw Object.assign(new Error("refused"), { status: 400 }); }; + await PS.adopt([{ id: "model", node: "Loader", inputs: {} }]); + assert.strictEqual(PS.queueable(), false); +});