fix: edits to a stand-in pipeline are refused with the reason instead of silently reverted; Sampler judges a save by the newest edit; a stand-in does not show the last project's readiness
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
7c6212c319
commit
2d7de26db1
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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.` });
|
||||
|
||||
@@ -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."
|
||||
|
||||
@@ -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)),
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user