diff --git a/app/ui/boot.js b/app/ui/boot.js index dfe1e057..b48b8dc2 100644 --- a/app/ui/boot.js +++ b/app/ui/boot.js @@ -22,7 +22,7 @@ import { composer as c } from "./composer/composer.js"; const heard = new Set(); const say = (what) => heard.forEach((fn) => { try { fn(what); } catch (err) { console.error(err); } }); const on = (fn) => { heard.add(fn); return () => heard.delete(fn); }; -const project = createProject({ keepConsistent: syncSeparated, onChange: () => say("change"), onOpen: () => say("open"), onError: (err) => console.warn(err) }); +const project = createProject({ keepConsistent: syncSeparated, onChange: () => say("change"), onOpen: () => say("open"), onError: (err) => console.warn(err), beforeSwitch: () => pipeline.settled() }); const pipeline = createPipelineState(api); const generate = createGenerate({ pipeline }); linkPipeline({ project, pipeline, onOpen: (fn) => on((what) => { if (what === "open") fn(); }), say: (text) => c.toast.warn({ text }) }); diff --git a/app/ui/shell/pipeline_link.js b/app/ui/shell/pipeline_link.js index 3f8d0974..2e753d15 100644 --- a/app/ui/shell/pipeline_link.js +++ b/app/ui/shell/pipeline_link.js @@ -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; + let retries = 0, opens = 0, retry = null; // Only a real change is written: opening a project must not rewrite its file. const store = (base, slots, opening) => { const next = { ...base, slots: JSON.parse(JSON.stringify(slots || [])), removed: pipeline.removedIds(), unwired: pipeline.unwiredMap(), whole: pipeline.whole() }; @@ -14,6 +14,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 owner = null; const mine = ++opens, target = project.project && project.project.id; const saved = project.project && project.project.models || {}; @@ -30,7 +31,7 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { // An old project with nothing saved runs the default and stays as it is on disk; a new one keeps what it took. if (own || project.fresh) store(saved, pipeline.slots(), !project.fresh); // a new project's whole flag is a fact, not a guess } else if (retries++ < 3) { - setTimeout(adopt, 5000); + retry = setTimeout(adopt, 5000); } else { say("This project's saved pipeline could not be loaded; the default is in use."); } @@ -40,6 +41,6 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { if (!project.project || project.project.id !== owner) return; store(project.project.models || {}, slots); }); - onOpen(adopt); + onOpen(() => { retries = 0; return adopt(); }); // each project opened gets its own retries return { adopt }; } diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index a0663da0..e6cda033 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -301,12 +301,18 @@ export function createPipelineState(API) { } function isQuarantined(id) { return !!(controlState[id] && controlState[id].quarantine); } // Whether anything would act on a module's settings now: its own node is in the pipeline (when it has one), - // and what it serves (the taste store) has a user that is on and not set to a manual value. - function useful(m) { - const values = currentValues(); - const working = (u) => { const v = values[u.id] || {}; return (!("enabled" in (u.settings || {})) || v.enabled) && v.mode !== "manual"; }; - const inPipeline = !(m.nodes || []).length || !slots || m.nodes.some((n) => slots.some((s) => s.node === n)); - const served = !(m.serves || []).length || activeModules().some((u) => working(u) && (u.uses || []).some((cap) => m.serves.includes(cap))); + // and what it serves (the modifier loader, the taste store) has a user that is on, not set to a manual value, + // has every node it needs in the pipeline, and is itself acted on (a taste learner the loader never installs is not). + function useful(m, values = currentValues(), seen = new Set([m.id])) { + const has = (n) => !slots || slots.some((s) => s.node === n); + const working = (u) => { + const v = values[u.id] || {}; + return !seen.has(u.id) && (!("enabled" in (u.settings || {})) || v.enabled) && v.mode !== "manual" + && (u.nodes || []).every(has) && useful(u, values, new Set([...seen, u.id])); + }; + const inPipeline = !(m.nodes || []).length || m.nodes.some(has); + // Every capability it offers must reach a working user: the taste key is installed by the loader AND read by a learner. + const served = (m.serves || []).every((cap) => activeModules().some((u) => (u.uses || []).includes(cap) && working(u))); return inPipeline && served; } @@ -338,7 +344,8 @@ export function createPipelineState(API) { // get that from this promise as written -- it would need save() itself // reworked to resolve each queued caller at ITS OWN write's landing, not // at whichever write happens to be in flight when the queue drains. - function setModuleValue(moduleId, name, value) { + 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 pendingValues[moduleId] = { ...(pendingValues[moduleId] || {}), [name]: value }; const values = currentValues(); mirrorValues(values); @@ -465,14 +472,23 @@ export function createPipelineState(API) { // files, node values, the module-settings blob), group and bypass win on every slot the // default still has; slots they added are kept. The server's own defaults fill in whatever // the saved copy predates, so an update's new settings are not lost to an old project. - // Not announced as a change: it IS the project's own copy. + // Announced once it is in, so views redraw; the project link writes nothing while a project is going in. // True when the project's pipeline is in (or it had none to put in); false when it could not be. // `inherit`: a project with nothing saved keeps the live pipeline (a new one); otherwise it gets the default. async function adopt(saved, removedIds, unwiredMap, wholeSaved, inherit = true) { let open; - const gate = new Promise((r) => { open = r; }); + const gate = new Promise((r) => { open = r; }), before = adoptGate; adoptGate = gate; // before anything awaits: an edit right behind this waits too - try { await ensureLoaded(); return await _adopt(saved, removedIds, unwiredMap, wholeSaved, inherit); } finally { if (adoptGate === gate) adoptGate = null; open(); } + try { + if (before) await before; // one at a time: a project opened behind another is put in after it, never under it + await ensureLoaded(); + return await _adopt(saved, removedIds, unwiredMap, wholeSaved, inherit); + } finally { if (adoptGate === gate) adoptGate = null; open(); _changed(); } + } + + /** Resolves once no edit is on its way and no project's pipeline is going in (at most ~10 s). */ + async function settled() { + for (let waited = 0; (saving || adoptGate) && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20)); } async function _adopt(saved, removedIds, unwiredMap, wholeSaved, inherit) { @@ -561,7 +577,7 @@ export function createPipelineState(API) { return { frozenInputs, - ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, + ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, settled, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, modulesById: () => modulesById, activeModules, useful, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, removedIds: () => [...removed], diff --git a/app/ui/shell/project.js b/app/ui/shell/project.js index 53d20fc3..384cd4b5 100644 --- a/app/ui/shell/project.js +++ b/app/ui/shell/project.js @@ -55,7 +55,7 @@ const put = (project) => json("PUT", `${BASE}/${encodeURIComponent(project.id)}` const TYPING = 1200; // ms after the last keystroke that still counts as typing const KEPT = ["editor_settings", "models"]; -export function createProject({ onChange, onError, onOpen, keepConsistent } = {}) { +export function createProject({ onChange, onError, onOpen, keepConsistent, beforeSwitch = async () => {} } = {}) { let project = null; let selected = null; // Every project this browser knows about, for the File menu. Read once at @@ -216,7 +216,7 @@ export function createProject({ onChange, onError, onOpen, keepConsistent } = {} async open(id) { const mine = ++generation; - await flush(); + await beforeSwitch(); await flush(); // an edit still landing elsewhere (the pipeline) reaches THIS project first const found = await read(id); // A newer switch already won this race (another open/new/import started // after this one and has already landed) -- applying this one now would @@ -232,7 +232,7 @@ export function createProject({ onChange, onError, onOpen, keepConsistent } = {} async newProject(name) { const mine = ++generation; - await flush(); + await beforeSwitch(); await flush(); const made = await create(name); if (mine !== generation) { // Superseded by a newer switch -- this one already created a real @@ -264,7 +264,7 @@ export function createProject({ onChange, onError, onOpen, keepConsistent } = {} */ async importProject(data) { const mine = ++generation; - await flush(); + await beforeSwitch(); await flush(); const imported = await json("POST", `${BASE}/import`, data); if (mine !== generation) { remove(imported.id).catch(() => {}); // superseded -- see newProject() diff --git a/app/ui/shell/tests/pipeline_link.test.js b/app/ui/shell/tests/pipeline_link.test.js index 260dfe53..0a41c927 100644 --- a/app/ui/shell/tests/pipeline_link.test.js +++ b/app/ui/shell/tests/pipeline_link.test.js @@ -92,3 +92,19 @@ test("a new project keeps the preset pipeline it inherits as whole, so it reopen await opened(); assert.equal(writes[0].whole, true); }); + +test("a retry left over from a project that failed to load never reloads the one opened after it", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + let opened, calls = 0, fail = true; + const project = { project: { id: "a", models: { slots, removed: [], unwired: {}, whole: false } }, setField: () => {} }; + const pipeline = { adopt: async () => { calls += 1; return !fail; }, slots: () => slots, removedIds: () => [], unwiredMap: () => ({}), whole: () => false, subscribe: () => {} }; + linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; } }); + await opened(); // A fails: a retry is waiting + fail = false; + project.project = { id: "b", models: { slots, removed: [], unwired: {}, whole: false } }; + await opened(); // B opens and loads + const before = calls; + t.mock.timers.tick(6000); + await new Promise((r) => setImmediate(r)); + assert.equal(calls, before); +}); diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index c78fe065..1fd84b7a 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -38,15 +38,15 @@ test("a project's saved pipeline lays its values over the server's default, keep assert.ok(by.extra); }); -test("adopting is the project's own copy, so it is not announced as an edit; a real edit is", async () => { +test("a project's pipeline going in is announced once it is in, so views redraw; a real edit is announced too", async () => { const posts = []; const PS = load(posts); - let heard = 0; - PS.subscribe(() => { heard++; }); + let heard = []; + PS.subscribe((s) => { heard.push(s.find((x) => x.id === "model").inputs.file); }); await PS.adopt([{ id: "model", node: "Loader", inputs: { file: "a" } }]); - assert.strictEqual(heard, 0); + assert.deepStrictEqual(heard, ["a"]); await PS.save({ inputs: { model: { file: "b" } } }); - assert.strictEqual(heard, 1); + assert.deepStrictEqual(heard, ["a", "b"]); }); test("nothing saved leaves the session's pipeline as it is", async () => { @@ -370,3 +370,53 @@ test("a save reaches listeners before the model probe answers, so a project swit answer({ detected: true, traits: [] }); await saving; }); + +const sink = (blob) => ({ id: "settings", node: "Sink", inputs: { settings: JSON.stringify(blob) } }); + +test("a setting changed while another project's pipeline goes in is built on that project's values", async () => { + const posts = []; + const PS = load(posts, { delay: 30 }); + lastApi.modules = async () => ({ modules: [{ id: "sharpen", settings: { enabled: { type: "bool", default: false }, amount: { type: "float", default: 0.5 } } }] }); + const edit = lastApi.editPipeline; + lastApi.editPipeline = (body) => { // the server writes the whole values blob into the sink, as core's place() does + if (body.values) body.slots.filter((x) => x.node === "Sink").forEach((x) => { x.inputs = { settings: JSON.stringify(body.values) }; }); + return edit(body); + }; + await PS.adopt([sink({ sharpen: { enabled: true, amount: 0.9 } })]); + const opening = PS.adopt([sink({ sharpen: { enabled: false, amount: 0.1 } })]); + await new Promise((r) => setTimeout(r, 5)); + await PS.setModuleValue("sharpen", "amount", 0.5); + await opening; + assert.deepStrictEqual(PS.currentValues().sharpen, { enabled: false, amount: 0.5 }); +}); + +test("a new project made while another project's pipeline goes in inherits it, never a half-replaced one", async () => { + const posts = []; + const PS = load(posts, { delay: 30 }); + await PS.ensureLoaded(); + const opening = PS.adopt([{ id: "model", node: "Loader", inputs: { file: "A" } }]); + const fresh = PS.adopt([], [], {}, undefined, true); + let seenByNew = null; + await fresh.then(() => { seenByNew = PS.slots().find((s) => s.id === "model").inputs.file; }); + await opening; + assert.strictEqual(seenByNew, "A"); +}); + +test("a modifier's settings count only when the pipeline has both the settings node and the loader that installs it", async () => { + const loader = { id: "mods", nodes: ["FunPackModifierSettings", "FunPackLoadModifiers"], uses: ["modifier"] }; + const sharpen = { id: "sharpen", serves: ["modifier"], settings: { amount: { default: 0.5 } } }; + const learner = { id: "q", serves: ["modifier"], uses: ["taste_store"], settings: { enabled: { type: "bool", default: true } } }; + const taste = { id: "taste", serves: ["modifier", "taste_store"], settings: { key: { default: "" } } }; + const check = async (nodes, mods = [loader, sharpen, learner, taste]) => { + const PS = load([]); + lastApi.pipeline = async () => ({ slots: nodes.map((node, i) => ({ id: `s${i}`, node, inputs: {} })), incomplete: [], refused: [], queueable: true }); + lastApi.modules = async () => ({ modules: mods }); + await PS.ensureLoaded(); + const by = PS.modulesById(); + return [PS.useful(by.sharpen), PS.useful(by.taste)]; + }; + assert.deepStrictEqual(await check(["FunPackModifierSettings", "FunPackLoadModifiers"]), [true, true]); + assert.deepStrictEqual(await check(["FunPackModifierSettings"]), [false, false]); // loader removed: nothing installs them, so no learner reads taste + assert.deepStrictEqual(await check(["CheckpointLoaderSimple", "KSampler"]), [false, false]); // a plain imported workflow + assert.deepStrictEqual(await check(["FunPackModifierSettings", "FunPackLoadModifiers"], [loader, sharpen, taste]), [true, false]); // the key installed, but no learner reads it +}); diff --git a/app/ui/shell/tests/project.test.js b/app/ui/shell/tests/project.test.js index c19cdc67..25abccb8 100644 --- a/app/ui/shell/tests/project.test.js +++ b/app/ui/shell/tests/project.test.js @@ -307,7 +307,7 @@ test("an edit made WHILE an import is in flight is still saved, against the old }; const importing = p.importProject({ name: "Imported", scenes: [] }); - await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); await Promise.resolve(); await Promise.resolve(); assert.equal(p.project.id, "abcdef012345", "the import switched before the edit -- this test proves nothing"); p.project.editor_settings = { autosave_sec: 0.3 }; @@ -342,9 +342,9 @@ test("two imports racing land on whichever was clicked last, not whichever answe }; const first = p.importProject({ name: "A", scenes: [] }); // clicked first - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); const second = p.importProject({ name: "B", scenes: [] }); // clicked second, while A is still in flight - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); // A answers LAST, after B -- a real, ordinary network race, not a // contrived one. @@ -377,9 +377,9 @@ test("a superseded import deletes its own orphaned project instead of leaving it const p = createProject({}); await p.start(); const first = p.importProject({ name: "A", scenes: [] }); - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); const second = p.importProject({ name: "B", scenes: [] }); - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); pending.B(); await second; pending.A(); await first; @@ -407,7 +407,7 @@ test("start() losing a race to a switch the user already made does not revert it const p = createProject({}); const starting = p.start(); // in flight -- list() has not answered yet - await Promise.resolve(); + await new Promise((r) => setTimeout(r, 0)); await p.newProject("MyNewThing"); // the user's own action completes first assert.equal(p.project.name, "MyNewThing"); @@ -662,3 +662,18 @@ test("an undo keeps the chosen pipeline too", async () => { p.undo(); assert.deepEqual(p.project.models, { slots: ["B"] }); }); + +test("a switch waits for edits still landing elsewhere before it reads the next project", async () => { + const order = []; + let release; + const p = createProject({ beforeSwitch: () => new Promise((r) => { order.push("wait"); release = () => { order.push("settled"); r(); }; }) }); + await p.start(); + globalThis.fetch = async (path) => { order.push(`fetch ${String(path).split("/").pop()}`); return { ok: true, status: 200, json: async () => ({ id: "b", name: "B", scenes: [], video: {}, updated_at: 1 }) }; }; + const opening = p.open("b"); + await new Promise((r) => setTimeout(r, 0)); + assert.deepStrictEqual(order, ["wait"]); + release(); + await opening; + assert.deepStrictEqual(order.slice(0, 2), ["wait", "settled"]); + assert.equal(p.project.name, "B"); +}); diff --git a/core/graph.py b/core/graph.py index 6db4d063..4f30881c 100644 --- a/core/graph.py +++ b/core/graph.py @@ -293,7 +293,10 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic else: problem = unacceptable(name, value, limits.get(name)) if problem: - problems.append(f"{slot_id}.{problem}") + # An input the app writes under a name the person sees (Length, Width) is refused by that name. + 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}") continue inputs[name] = value for name in schemas.required(class_type): diff --git a/core/tests/test_graph.py b/core/tests/test_graph.py index 8104df61..96af6b84 100644 --- a/core/tests/test_graph.py +++ b/core/tests/test_graph.py @@ -296,6 +296,14 @@ def test_a_number_outside_what_the_node_takes_is_refused(): assert any("above the largest" in p for p in problems), problems +def test_a_value_the_app_writes_is_refused_by_the_name_the_person_sees(): + _prompt, problems = graph.build( + [{"id": "model", "node": "Loader", "roles": [{"at": "project.video", "input": "steps", "label": "Length"}], + "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 + + def test_a_bool_is_not_measured_against_a_number_bound(): """True is 1 in Python, so a naive comparison lets a checkbox be judged against a step count -- and quietly passes it.""" diff --git a/core/tests/test_schema.py b/core/tests/test_schema.py index 0e4df91c..73f9eb5e 100644 --- a/core/tests/test_schema.py +++ b/core/tests/test_schema.py @@ -287,7 +287,7 @@ def test_the_taste_key_serves_the_store_and_every_module_reading_it_says_so(comf from pathlib import Path from core import routes found = {m["id"]: m for m in routes.manifest()["modules"]} - assert found["taste"]["serves"] == ["taste_store"] + assert found["taste"]["serves"] == ["modifier", "taste_store"] root = Path(__file__).resolve().parents[2] / "modules" readers = {p.parent.name for p in root.rglob("*.py") if "/tests/" not in str(p) and re.search(r'ask\("taste_store"', p.read_text())} diff --git a/modules/models/sd15/__init__.py b/modules/models/sd15/__init__.py index 6f18fcb9..09831cdd 100644 --- a/modules/models/sd15/__init__.py +++ b/modules/models/sd15/__init__.py @@ -18,13 +18,15 @@ STAGE = "load" CATEGORY = "system" STATUS = "experimental" -_SIGNATURE_KEYS = ("model.diffusion_model.input_blocks.0.0.weight", - "cond_stage_model.transformer.text_model.embeddings.token_embedding.weight") +_UNET = "model.diffusion_model.input_blocks.0.0.weight" +# Older SD1.x files name the CLIP-L embedding without `text_model.`; ComfyUI renames it on load. +_CLIP_L = ("cond_stage_model.transformer.text_model.embeddings.token_embedding.weight", + "cond_stage_model.transformer.embeddings.token_embedding.weight") def detect(keys) -> bool: keyset = set(keys) - return all(key in keyset for key in _SIGNATURE_KEYS) + return _UNET in keyset and any(key in keyset for key in _CLIP_L) def probe_traits(keys) -> list: diff --git a/modules/models/sd15/tests/test_sd15.py b/modules/models/sd15/tests/test_sd15.py index c8ed98a1..c83ada79 100644 --- a/modules/models/sd15/tests/test_sd15.py +++ b/modules/models/sd15/tests/test_sd15.py @@ -25,6 +25,7 @@ def test_sd1_is_recognised_and_sd2_sdxl_h3_are_not(comfyui): from modules.models import sd15 assert sd15.detect({UNET, CLIP_L}) assert sd15.probe_traits({UNET, CLIP_L}) == ["spatial_latent", "predict_eps"] + assert sd15.detect({UNET, "cond_stage_model.transformer.embeddings.token_embedding.weight"}) # an older SD1.x file's CLIP naming assert not sd15.detect({UNET, "cond_stage_model.model.token_embedding.weight"}) # SD2 (OpenCLIP) assert not sd15.detect({UNET, "conditioner.embedders.0.transformer.text_model.embeddings.token_embedding.weight"}) # SDXL assert not sd15.detect({"video_patch_proj.weight", "audio_patch_proj.weight"}) # H3 diff --git a/modules/sampling/modifiers/__init__.py b/modules/sampling/modifiers/__init__.py index f6a256fb..1da5f8d8 100644 --- a/modules/sampling/modifiers/__init__.py +++ b/modules/sampling/modifiers/__init__.py @@ -9,6 +9,7 @@ CATEGORY = "sampling" STATUS = "proven" NODES = [FunPackModifierSettings, FunPackLoadModifiers] +USES = ["modifier"] # the modifiers it installs act only when both its nodes are in the pipeline def settings_sink():