fix: settings are read and written only where the server says they live, so a fresh pipeline mirrors at once and a hidden module's settings survive; a refused pipeline names the server's reason and the next edit replaces it; a refused value is not also called missing
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bd00692f85
commit
62e3c7fa07
@@ -153,7 +153,9 @@ export const models = (app) => function mount() {
|
||||
|
||||
function status(only) {
|
||||
const all = ps.incomplete(), refused = ps.refused(), notes = ps.saveNotes();
|
||||
const incomplete = only ? all.filter((t) => t.startsWith(`${only}:`)) : all;
|
||||
const mine = (t, id) => t.startsWith(`${id}:`) || t.startsWith(`${id}.`)
|
||||
|| ((slotsNow().find((s) => s.id === id) || {}).roles || []).some((r) => r.label && t.startsWith(`${r.label} is `)); // refused by the name the person sees
|
||||
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];
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) {
|
||||
// it is written. A project opened while an open or a save is still in flight must never receive the other's pipeline.
|
||||
// An id, not the object: Undo puts a copy of the same project in place.
|
||||
let owner = null;
|
||||
let retries = 0, opens = 0, retry = null, waiting = false; // waiting: gave up because ComfyUI did not answer
|
||||
let retries = 0, opens = 0, retry = null, waiting = false, replaceOnEdit = false; // waiting: gave up because ComfyUI did not answer
|
||||
// Only a real change is written: opening a project must not rewrite its file.
|
||||
const live = (base, slots) => ({ ...base, slots: JSON.parse(JSON.stringify(slots || [])), removed: pipeline.removedIds(), unwired: pipeline.unwiredMap(), whole: pipeline.whole() });
|
||||
const store = (base, slots) => {
|
||||
@@ -15,7 +15,7 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) {
|
||||
|
||||
async function adopt() {
|
||||
clearTimeout(retry); // a retry still waiting is for a project that may no longer be open
|
||||
waiting = false;
|
||||
waiting = false; replaceOnEdit = false;
|
||||
if (pipeline.settled) await pipeline.settled(); // an edit on its way lands in the project it was made in first
|
||||
const here = project.project && project.project.models;
|
||||
// Undo / Redo of something else: the same project, its pipeline already live, nothing to put in (or to lose).
|
||||
@@ -42,21 +42,28 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) {
|
||||
retry = setTimeout(adopt, 15000);
|
||||
if (retries === 4) say("ComfyUI is not answering, so this project's pipeline is not loaded yet: it goes in as soon as ComfyUI answers.");
|
||||
} else {
|
||||
say("This project's saved pipeline could not be loaded; the default is in use.");
|
||||
replaceOnEdit = true;
|
||||
say(refused());
|
||||
}
|
||||
}
|
||||
|
||||
pipeline.subscribe((slots) => {
|
||||
if (waiting === "load" && owner === null) { retries = 0; adopt(); return; }
|
||||
// Refused for good: the person's next edit of the stand-in pipeline becomes this project's pipeline.
|
||||
if (replaceOnEdit && owner === null && project.project) { replaceOnEdit = false; owner = project.project.id; }
|
||||
if (!project.project || project.project.id !== owner) return;
|
||||
store(project.project.models || {}, slots);
|
||||
});
|
||||
onOpen(() => { retries = 0; return adopt(); }); // each project opened gets its own retries
|
||||
// Whether the live pipeline is the open project's (false while it is going in, or ComfyUI has not answered).
|
||||
const refused = () => {
|
||||
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.`;
|
||||
};
|
||||
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."
|
||||
: retries > 3 ? "This project's saved pipeline could not be loaded. Open the project again to retry, or rebuild it in Settings ▸ Models & Pipeline."
|
||||
: retries > 3 ? refused()
|
||||
: "This project's pipeline is still loading. Try again in a moment.";
|
||||
return { adopt, owns, why };
|
||||
}
|
||||
|
||||
@@ -74,6 +74,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
modulesById = {};
|
||||
(manifest.modules || []).forEach((m) => { modulesById[m.id] = m; });
|
||||
controlState = manifest.control || controlState;
|
||||
if (Array.isArray(manifest.sinks)) sinks = manifest.sinks;
|
||||
if (traits !== undefined) lastProbedFile = file; // only once it worked: a failed probe is tried again
|
||||
loadError = null; // the pipeline is in and its modules are known: nothing is failing now
|
||||
return true;
|
||||
@@ -87,6 +88,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
let removed = new Set(); // default slots the person took out of this project's pipeline
|
||||
let adoptGate = null; // a promise while a project's pipeline is being put in
|
||||
let whole = false; // the pipeline replaced the default (a preset, an import, the wizard) rather than editing it
|
||||
let refusalLast = null; // the server's own reason for refusing the last project's pipeline
|
||||
let unreachableLast = false; // the last project's pipeline failed because a request did not get through, not because it was refused
|
||||
let adopting = 0; // adopts under way (waiting for the load, or putting a pipeline in)
|
||||
let deferred = null; // a project's saved pipeline ([slots, removed, unwired, whole]) waiting for the first successful load
|
||||
@@ -104,17 +106,21 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
// plain object -- and only accepts a decoded key that names a module this
|
||||
// session already knows is installed, so an unrelated node's incidentally
|
||||
// JSON-shaped string can't inject a bogus entry that round-trips forever.
|
||||
// Once the server has named its sinks, only those are read, and EVERY module's entry in them is kept: a module the
|
||||
// current model hides still has its values there, and writing the tree back must not drop them.
|
||||
let sinks = null; // [{node, input}] from the manifest; null until known (then the scan below is the fallback)
|
||||
const isSink = (slot, key) => sinks.some((k) => k.node === slot.node && k.input === key);
|
||||
function valuesAlreadyPlaced(from = slots) {
|
||||
const known = new Set([...Object.keys(modulesById), "_off"]); // _off: the modules this project turned off
|
||||
const merged = {};
|
||||
(from || []).forEach((slot) => {
|
||||
Object.values(slot.inputs || {}).forEach((v) => {
|
||||
if (typeof v !== "string") return;
|
||||
Object.entries(slot.inputs || {}).forEach(([key, v]) => {
|
||||
if (typeof v !== "string" || (sinks && !isSink(slot, key))) return;
|
||||
let parsed;
|
||||
try { parsed = JSON.parse(v); } catch (_) { return; }
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return;
|
||||
Object.entries(parsed).forEach(([moduleId, own]) => {
|
||||
if (!known.has(moduleId)) return;
|
||||
if (!sinks && !known.has(moduleId)) return;
|
||||
if (own && typeof own === "object") merged[moduleId] = { ...(merged[moduleId] || {}), ...own };
|
||||
});
|
||||
});
|
||||
@@ -313,6 +319,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
const known = new Set([...Object.keys(modulesById), "_off"]);
|
||||
(slots || []).forEach((slot) => {
|
||||
Object.entries(slot.inputs || {}).forEach(([key, v]) => {
|
||||
if (sinks) { if (isSink(slot, key) && !Array.isArray(v)) slot.inputs[key] = JSON.stringify(values); return; } // a fresh "{}" too
|
||||
if (typeof v !== "string") return;
|
||||
let parsed;
|
||||
try { parsed = JSON.parse(v); } catch (_) { return; }
|
||||
@@ -578,7 +585,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
groupEdits = {};
|
||||
pendingBody = null; pending = false;
|
||||
for (let waited = 0; saving && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20));
|
||||
let unreachable = false;
|
||||
let unreachable = false, reasons = [];
|
||||
for (const extras of [true, false]) {
|
||||
try {
|
||||
const res = await API.editPipeline({ slots: lay(extras) });
|
||||
@@ -592,12 +599,15 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) {
|
||||
try { await refreshManifest(); } catch (_) { /* the next save retries it */ }
|
||||
return true;
|
||||
}
|
||||
reasons = (res && res.refused) || reasons;
|
||||
} catch (e) {
|
||||
// No answer, or a gateway's (a tunnel, a restarting ComfyUI): try again later. An answer refusing it is a refusal.
|
||||
if (!e || !e.status || e.status >= 502) unreachable = true;
|
||||
else reasons = [e.message];
|
||||
}
|
||||
}
|
||||
unreachableLast = unreachable;
|
||||
refusalLast = reasons.length ? String(reasons[0]) : null;
|
||||
// Nothing saved was accepted (a failed request, or a file the server refuses): the default
|
||||
// runs, said, and the caller must NOT save it over the project's own copy.
|
||||
slots = JSON.parse(JSON.stringify(offered));
|
||||
@@ -622,7 +632,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, activeModules, useful, usefulness, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState,
|
||||
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)),
|
||||
|
||||
@@ -188,3 +188,20 @@ test("a project whose pipeline could not get through mid-session keeps being ret
|
||||
heard([{ ...slots[0], inputs: { ckpt_name: "edited" } }]);
|
||||
assert.deepStrictEqual(writes, ["edited"]);
|
||||
});
|
||||
|
||||
test("a pipeline ComfyUI refuses says the server's reason, and the person's next edit becomes the project's pipeline", async (t) => {
|
||||
t.mock.timers.enable({ apis: ["setTimeout"] });
|
||||
const writes = [], said = [];
|
||||
let opened, heard;
|
||||
const project = { project: { id: "a", models: { slots, removed: [], unwired: {}, whole: false } }, setField: (k, v) => { writes.push(v.slots[0].inputs.ckpt_name); project.project[k] = v; } };
|
||||
const pipeline = { adopt: async () => false, slots: () => slots, unreachable: () => false, refusal: () => "slot 0 role 0 has no input",
|
||||
removedIds: () => [], unwiredMap: () => ({}), whole: () => false, subscribe: (fn) => { heard = fn; } };
|
||||
const link = linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; }, say: (x) => said.push(x) });
|
||||
const flush = async () => { for (let i = 0; i < 5; i++) await new Promise((r) => setImmediate(r)); };
|
||||
await opened();
|
||||
for (let i = 0; i < 4; i++) { t.mock.timers.tick(6000); await flush(); }
|
||||
assert.match(said.join(" "), /refused .*slot 0 role 0 has no input/);
|
||||
assert.match(link.why(), /refused/);
|
||||
heard([{ ...slots[0], inputs: { ckpt_name: "fixed" } }]);
|
||||
assert.deepStrictEqual(writes, ["fixed"]);
|
||||
});
|
||||
|
||||
@@ -507,3 +507,28 @@ test("settings can be read from a frozen pipeline, apart from the live one and e
|
||||
assert.deepStrictEqual([PS.currentValues(frozen).continuity.carry, PS.allOff(frozen)], [true, false]);
|
||||
assert.deepStrictEqual([PS.currentValues().continuity.carry, PS.allOff()], [false, true]);
|
||||
});
|
||||
|
||||
const withSink = (blob, mods) => {
|
||||
const PS = load([]);
|
||||
lastApi.pipeline = async () => ({ slots: [{ id: "settings", node: "Sink", inputs: { settings: blob } }, { id: "other", node: "X", inputs: { note: '{"sharpen": {"amount": 9}}' } }], incomplete: [], refused: [], queueable: true });
|
||||
lastApi.modules = async () => ({ modules: mods, sinks: [{ node: "Sink", input: "settings" }], control: {} });
|
||||
return PS;
|
||||
};
|
||||
const sharpen = { id: "sharpen", settings: { enabled: { type: "bool", default: false }, amount: { type: "float", default: 0.5 } } };
|
||||
|
||||
test("a first setting on a fresh pipeline is in its settings slot at once, so a failed save cannot leave the run on old values", async () => {
|
||||
const PS = withSink("{}", [sharpen]);
|
||||
await PS.ensureLoaded();
|
||||
lastApi.editPipeline = async () => { throw new Error("boom"); };
|
||||
await PS.setModuleValue("sharpen", "enabled", true);
|
||||
assert.strictEqual(JSON.parse(PS.slots().find((s) => s.id === "settings").inputs.settings).sharpen.enabled, true);
|
||||
assert.strictEqual(PS.slots().find((s) => s.id === "other").inputs.note, '{"sharpen": {"amount": 9}}'); // not a sink: never read or written
|
||||
});
|
||||
|
||||
test("settings of a module the current model hides are kept when another setting is changed", async () => {
|
||||
const PS = withSink(JSON.stringify({ reins: { enabled: true } }), [sharpen]);
|
||||
await PS.ensureLoaded();
|
||||
await PS.setModuleValue("sharpen", "enabled", true);
|
||||
const blob = JSON.parse(PS.slots().find((s) => s.id === "settings").inputs.settings);
|
||||
assert.deepStrictEqual([blob.reins, blob.sharpen.enabled, PS.currentValues().sharpen.amount], [{ enabled: true }, true, 0.5]);
|
||||
});
|
||||
|
||||
+3
-2
@@ -266,7 +266,7 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic
|
||||
|
||||
declared = schemas.inputs(class_type)
|
||||
limits = schemas.limits(class_type)
|
||||
inputs = {}
|
||||
inputs, refused_here = {}, set()
|
||||
for name, value in (slot.get("inputs") or {}).items():
|
||||
if name not in declared:
|
||||
problems.append(f"{slot_id}: {class_type} has no input {name!r}")
|
||||
@@ -297,10 +297,11 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic
|
||||
label = next((r.get("label") for r in slot.get("roles") or []
|
||||
if isinstance(r, dict) and r.get("input") == name and r.get("label")), None)
|
||||
problems.append(unacceptable(label, value, limits.get(name)) if label else f"{slot_id}.{problem}")
|
||||
refused_here.add(name)
|
||||
continue
|
||||
inputs[name] = value
|
||||
for name in schemas.required(class_type):
|
||||
if name not in inputs:
|
||||
if name not in inputs and name not in refused_here: # a refused value was just said; "nothing fills it" would be false
|
||||
problems.append(f"{slot_id}: {class_type} needs {name!r} and nothing "
|
||||
f"fills it")
|
||||
|
||||
|
||||
+17
-12
@@ -115,6 +115,20 @@ def modules(rescan=False):
|
||||
return registry_mod.current(rescan)
|
||||
|
||||
|
||||
def settings_sinks():
|
||||
"""Every place a module says the app's settings can be put: [{node, input}]."""
|
||||
found = []
|
||||
for _spec, make in modules().providers("settings_sink"):
|
||||
try:
|
||||
sink = make()
|
||||
except Exception as exc: # noqa: BLE001
|
||||
log.failed("settings_sink", exc)
|
||||
continue
|
||||
if isinstance(sink, dict) and sink.get("node") and sink.get("input"):
|
||||
found.append({"node": sink["node"], "input": sink["input"]})
|
||||
return found
|
||||
|
||||
|
||||
def manifest(traits=None):
|
||||
"""What the browser is told: only modules that loaded and validated.
|
||||
|
||||
@@ -146,6 +160,8 @@ def manifest(traits=None):
|
||||
],
|
||||
# Which modules the person can switch, and which are quarantined after a fault.
|
||||
"control": control_mod.state(ordered),
|
||||
# Where the settings live in a pipeline: the app reads and writes them there, and nowhere else.
|
||||
"sinks": settings_sinks(),
|
||||
}
|
||||
|
||||
|
||||
@@ -200,18 +216,7 @@ def register(routes, prefix=None):
|
||||
return make()
|
||||
return []
|
||||
|
||||
def _sinks():
|
||||
"""Every place a module says the app's settings can be put."""
|
||||
found = []
|
||||
for _spec, make in modules().providers("settings_sink"):
|
||||
try:
|
||||
sink = make()
|
||||
except Exception as exc: # noqa: BLE001
|
||||
log.failed("settings_sink", exc)
|
||||
continue
|
||||
if isinstance(sink, dict):
|
||||
found.append(sink)
|
||||
return found
|
||||
_sinks = settings_sinks
|
||||
|
||||
# A module's own routes live under its id, so two modules can never collide
|
||||
# and turning one off takes its routes with it on the next start.
|
||||
|
||||
@@ -272,6 +272,7 @@ def test_a_combo_value_that_is_no_longer_on_disk_is_refused_before_queueing():
|
||||
_schemas_with_limits())
|
||||
assert any("gone.safetensors" in p and "not one of" in p for p in problems), problems
|
||||
assert any("model.ckpt_name" in p for p in problems), problems
|
||||
assert not any("nothing fills it" in p for p in problems), problems # refused, not missing: said once, truly
|
||||
|
||||
|
||||
def test_a_combo_value_that_is_on_disk_is_accepted():
|
||||
@@ -302,6 +303,7 @@ def test_a_value_the_app_writes_is_refused_by_the_name_the_person_sees():
|
||||
"inputs": {"ckpt_name": "a.safetensors", "steps": 1000}}],
|
||||
_schemas_with_limits())
|
||||
assert any(p.startswith("Length is 1000, above the largest") for p in problems), problems
|
||||
assert not any("nothing fills it" in p for p in problems), problems # the refused value was said; it is not "missing"
|
||||
|
||||
|
||||
def test_a_bool_is_not_measured_against_a_number_bound():
|
||||
|
||||
@@ -312,3 +312,8 @@ def test_uses_when_must_name_a_declared_setting():
|
||||
with pytest.raises(SchemaError, match="uses_when"):
|
||||
validate(announce(uses=["taste_store"], uses_when={"nope": "x"}))
|
||||
assert validate(announce(uses=["taste_store"], uses_when={"enabled": True})).to_manifest()["uses_when"] == {"enabled": True}
|
||||
|
||||
|
||||
def test_the_manifest_names_where_settings_live(comfyui):
|
||||
from core import routes
|
||||
assert {"node": "FunPackModifierSettings", "input": "settings"} in routes.manifest()["sinks"]
|
||||
|
||||
Reference in New Issue
Block a user