From f423f84a30bfda30b98214d454affc1fa3ee5b2f Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:46:12 +0300 Subject: [PATCH] fix: a pipeline retried after a failed first load no longer deadlocks; Undo keeps a pipeline edit in flight and skips needless reloads; opening a project never rewrites it; Enhance follows the open project; readiness names the GPU ComfyUI uses; Sampler says when it cannot read its options Co-Authored-By: Claude Opus 5.5 --- app/ui/features/composer/enhance.js | 2 ++ app/ui/features/composer/enhance.test.js | 19 ++++++++++++++ app/ui/features/sampler/hub.test.js | 13 ++++++++++ app/ui/features/sampler/index.js | 8 +++--- app/ui/features/settings/shotmemory.js | 2 +- app/ui/shell/pipeline_link.js | 16 +++++++----- app/ui/shell/pipeline_state.js | 7 +++-- app/ui/shell/tests/pipeline_link.test.js | 31 +++++++++++++++++++++++ app/ui/shell/tests/pipeline_state.test.js | 14 ++++++++++ core/readiness.py | 11 +++++++- core/tests/test_readiness.py | 13 ++++++++++ 11 files changed, 123 insertions(+), 13 deletions(-) create mode 100644 app/ui/features/composer/enhance.test.js diff --git a/app/ui/features/composer/enhance.js b/app/ui/features/composer/enhance.js index 34b32c08..6be4f5e7 100644 --- a/app/ui/features/composer/enhance.js +++ b/app/ui/features/composer/enhance.js @@ -47,6 +47,8 @@ export function enhance(app, own) { } draw(); ps.ensureLoaded().then(draw); + // Another project's pipeline, or an edit made elsewhere: redraw, but never under a field being typed in. + own(ps.subscribe(() => { if (!page.node.contains(document.activeElement)) draw(); })); app.api.enhancerDefaults().then((d) => { defaults = d; draw(); }).catch(() => {}); app.api.enhancerRuns().then((r) => { last = (r.runs || []).slice(-1)[0] || null; draw(); }).catch(() => {}); return page; diff --git a/app/ui/features/composer/enhance.test.js b/app/ui/features/composer/enhance.test.js new file mode 100644 index 00000000..808b16aa --- /dev/null +++ b/app/ui/features/composer/enhance.test.js @@ -0,0 +1,19 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { setupDom, teardownDom } from "../../composer/tests/_dom.js"; + +let enhance; +test.before(async () => { setupDom(); ({ enhance } = await import("./enhance.js")); }); +test.after(() => teardownDom()); + +test("the Enhance tab follows the open project's pipeline: no enhancer there, no toggle shown", async () => { + let live = [{ id: "e", node: "FunPackEnhancePrompt", inputs: { enabled: true } }], heard; + const ps = { slots: () => live, ensureLoaded: async () => {}, save: async () => {}, subscribe: (fn) => { heard = fn; return () => {}; } }; + const api = { enhancerDefaults: async () => ({}), enhancerRuns: async () => ({ runs: [] }) }; + const page = enhance({ pipeline: ps, api }, () => {}); + document.body.append(page.node); + assert.match(page.node.textContent, /Enhance prompt/); + live = [{ id: "s", node: "FunPackSampler", inputs: {} }]; // another project opened + heard(live); + assert.match(page.node.textContent, /No prompt enhancer here/); +}); diff --git a/app/ui/features/sampler/hub.test.js b/app/ui/features/sampler/hub.test.js index 966333ad..1fdf65d4 100644 --- a/app/ui/features/sampler/hub.test.js +++ b/app/ui/features/sampler/hub.test.js @@ -30,3 +30,16 @@ test("the Sampler window shows the pipeline's steps and sampler, and a change sa await new Promise((r) => setTimeout(r, 0)); assert.deepStrictEqual(saved, [{ inputs: { sampler: { sampler_name: "dpmpp_2m" } } }]); }); + +test("when the sampler node's options cannot be read, the window says where to change them", async () => { + const slot = { id: "sampler", node: "S", inputs: { steps: 20 }, roles: [{ at: "generation.sampling", input: "steps", label: "Steps" }] }; + const ps = { slots: () => [slot], activeModules: () => [], useful: () => true, currentValues: () => ({}), loading: () => false, loadError: () => null, + subscribe: () => () => {}, ensureLoaded: async () => {}, save: async () => {} }; + const host = document.createElement("div"); + document.body.append(host); + hub.setup({ host, app: { pipeline: ps, api: { describeNodes: async () => { throw new Error("offline"); } }, on: () => () => {}, actions: null } }); + await new Promise((r) => setTimeout(r, 0)); + host.querySelector("button").click(); + await new Promise((r) => setTimeout(r, 0)); + assert.match(document.body.textContent, /could not be read from the sampler node/); +}); diff --git a/app/ui/features/sampler/index.js b/app/ui/features/sampler/index.js index a981922b..dbf2d2a5 100644 --- a/app/ui/features/sampler/index.js +++ b/app/ui/features/sampler/index.js @@ -12,12 +12,12 @@ export default { needs: ["pipeline"], setup({ host, app }) { const ps = app.pipeline; - let pop = null, specs = {}; + let pop = null, specs = {}, unreadable = false; const sampling = () => ps.activeModules().filter((m) => (m.category || "") === "sampling" && Object.keys(m.settings || {}).length && ps.useful(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 () => { const missing = [...new Set(marked().map(({ slot }) => slot.node))].filter((n) => !(n in specs)); - if (missing.length) try { Object.assign(specs, (await app.api.describeNodes(missing)).nodes || {}); } catch { /* the rows just stay out until the next change */ } + if (missing.length) try { Object.assign(specs, (await app.api.describeNodes(missing)).nodes || {}); unreadable = false; } catch { unreadable = true; } // asked again on the next change }; const inputRow = ({ slot, role }) => { const w = ((specs[slot.node] || {}).widgets || []).find((x) => x.name === role.input); @@ -35,7 +35,9 @@ export default { if (ps.loadError()) return body.set([c.banner.warn({ text: `Could not load: ${ps.loadError()}` })]); const mods = sampling(), values = ps.currentValues(); const change = async (id, name, value) => { await ps.setModuleValue(id, name, value); draw(); }; - body.set([...marked().map(inputRow), ...mods.flatMap((m) => [mods.length > 1 ? c.label.section({ text: m.title || m.id }) : null, + const rows = marked().map(inputRow); + const unread = marked().length && !rows.some(Boolean) && (unreadable || marked().every(({ slot }) => slot.node in specs)) ? [c.hint.default({ text: "Steps, sampler and scheduler could not be read from the sampler node: change them in Settings ▸ Models & Pipeline." })] : []; + body.set([...unread, ...rows, ...mods.flatMap((m) => [mods.length > 1 ? c.label.section({ text: m.title || m.id }) : null, ...Object.entries(m.settings).filter(([, spec]) => whenSatisfied(values, m.id, spec.when)).map(([name, spec]) => settingRow(m.id, name, spec, values, change))])].filter(Boolean)); }; const typing = () => body.node.contains(document.activeElement) && /^(input|textarea)$/i.test(document.activeElement.tagName); diff --git a/app/ui/features/settings/shotmemory.js b/app/ui/features/settings/shotmemory.js index 8bc3b982..7cc7aa44 100644 --- a/app/ui/features/settings/shotmemory.js +++ b/app/ui/features/settings/shotmemory.js @@ -20,7 +20,7 @@ export const shotMemory = (app) => function mount() { if (error) return page.set([c.banner.warn({ text: `Could not read the camera's memory: ${error}` })]); if (!data) return page.set([c.hint.default({ text: "Loading…" })]); const words = data.words || [], views = data.views || []; - if (!words.length && !views.length && !(data.arms || []).length) return page.set([c.emptyState.default({ icon: "◎", title: "Nothing learned yet", hint: "Turn on camera moves or shot views, then rate renders: the camera remembers which words and views you liked." })]); + if (!words.length && !views.length && !(data.arms || []).length) return page.set([c.emptyState.default({ icon: "◎", title: "Nothing learned yet", hint: "In a pipeline with the Shot camera node, turn on camera moves or shot views in Engine, then rate renders: the camera remembers which words and views you liked." })]); page.set([c.hint.default({ text: `${data.prompts} prompts seen · ${data.shots} shots reviewed. Forget anything that taught it the wrong thing.` }), ...(views.length ? [c.label.section({ text: "Views" }), ...views.map((v) => row(v.view, `liked ${v.good.toFixed(1)} · disliked ${v.bad.toFixed(1)}`, "view", v.view))] : []), ...(words.length ? [c.label.section({ text: "Words the camera aims at" }), ...words.map((w) => row(w.word, `picked ${w.picks} · replaced ${w.rejects} · seen ${w.seen}`, "word", w.word)), diff --git a/app/ui/shell/pipeline_link.js b/app/ui/shell/pipeline_link.js index 2e753d15..34feb79e 100644 --- a/app/ui/shell/pipeline_link.js +++ b/app/ui/shell/pipeline_link.js @@ -7,18 +7,22 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { let owner = null; 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() }; - if (opening && !("whole" in base)) delete next.whole; // an older file is not rewritten just to record the guess; an edit records it + const live = (base, slots) => ({ ...base, slots: JSON.parse(JSON.stringify(slots || [])), removed: pipeline.removedIds(), unwired: pipeline.unwiredMap(), whole: pipeline.whole() }); + const store = (base, slots) => { + const next = live(base, slots); if (JSON.stringify(next) !== JSON.stringify(project.project.models || {})) project.setField("models", next, { quiet: true }); }; async function adopt() { clearTimeout(retry); // a retry still waiting is for a project that may no longer be open + 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). + if (owner && project.project && owner === project.project.id && here && pipeline.slots() + && JSON.stringify(live(here, pipeline.slots())) === JSON.stringify(here)) return; owner = null; const mine = ++opens, target = project.project && project.project.id; const saved = project.project && project.project.models || {}; - const own = (saved.slots || []).length > 0 || (saved.removed || []).length > 0; let ok = false, timer; try { // Bounded: a hung request must not hold the project behind it. @@ -28,8 +32,8 @@ export function linkPipeline({ project, pipeline, onOpen, say = () => {} }) { if (ok && pipeline.slots()) { owner = target; retries = 0; - // 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 + // Opening writes nothing: what an update's defaults fill in is laid again on every open. A new project keeps what it took. + if (project.fresh) store(saved, pipeline.slots()); } else if (retries++ < 3) { retry = setTimeout(adopt, 5000); } else { diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index e6cda033..5d327f72 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -69,6 +69,7 @@ export function createPipelineState(API) { 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 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 let epoch = 0; // bumped whenever a project's pipeline replaces the live one let saving = false; @@ -132,7 +133,8 @@ export function createPipelineState(API) { } loading = false; loadPromise = null; - if (slots !== null && deferred) { const d = deferred; deferred = null; await adopt(...d); } + // An adopt already under way carries the newest project and lays it itself; waiting on it from here would deadlock. + if (slots !== null && deferred) { const d = deferred; deferred = null; if (!adopting) await adopt(...d); } })(); return loadPromise; } @@ -479,11 +481,12 @@ export function createPipelineState(API) { let open; const gate = new Promise((r) => { open = r; }), before = adoptGate; adoptGate = gate; // before anything awaits: an edit right behind this waits too + adopting += 1; 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(); } + } finally { adopting -= 1; 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). */ diff --git a/app/ui/shell/tests/pipeline_link.test.js b/app/ui/shell/tests/pipeline_link.test.js index 0a41c927..d15f95cd 100644 --- a/app/ui/shell/tests/pipeline_link.test.js +++ b/app/ui/shell/tests/pipeline_link.test.js @@ -108,3 +108,34 @@ test("a retry left over from a project that failed to load never reloads the one await new Promise((r) => setImmediate(r)); assert.equal(calls, before); }); + +test("Undo of something else, with a pipeline edit still landing, keeps the edit and puts nothing back in", async () => { + const writes = []; + let opened, heard, land, adopts = 0; + let live = slots; + const models = { slots, removed: [], unwired: {}, whole: false }; + const project = { project: { id: "a", models }, setField: (k, v) => { writes.push(v.slots[0].inputs.ckpt_name); project.project[k] = v; } }; + const pipeline = { adopt: async () => { adopts += 1; return true; }, slots: () => live, removedIds: () => [], unwiredMap: () => ({}), whole: () => false, + subscribe: (fn) => { heard = fn; }, settled: () => new Promise((r) => { land = r; }) }; + linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; } }); + const first = opened(); land(); await first; + const before = adopts; + project.project = JSON.parse(JSON.stringify(project.project)); // Undo puts a copy of the project in place + const undo = opened(); + live = [{ ...slots[0], inputs: { ckpt_name: "b" } }]; // the edit's answer arrives + heard(live); + land(); + await undo; + assert.deepStrictEqual([writes, adopts - before], [["b"], 0]); +}); + +test("opening an older project whose pipeline an update's defaults fill in does not rewrite it", async () => { + const writes = []; + let opened; + const project = { project: { id: "o", models: { slots, removed: [], unwired: {} } }, setField: (k, v) => { writes.push(v); } }; + const filled = [{ ...slots[0], inputs: { ...slots[0].inputs, denoise: 1 } }]; + const pipeline = { adopt: async () => true, slots: () => filled, removedIds: () => [], unwiredMap: () => ({}), whole: () => false, subscribe: () => {} }; + linkPipeline({ project, pipeline, onOpen: (fn) => { opened = fn; } }); + await opened(); + assert.equal(writes.length, 0); +}); diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index 1fd84b7a..c6e28a06 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -420,3 +420,17 @@ test("a modifier's settings count only when the pipeline has both the settings n 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 }); + +test("a project retried after the first load failed goes in, and nothing waits forever", async () => { + const posts = []; + const PS = load(posts); + const real = lastApi.pipeline; + lastApi.pipeline = async () => { throw new Error("tunnel blip"); }; + const saved = [{ id: "model", node: "Loader", inputs: { file: "mine" } }]; + assert.strictEqual(await PS.adopt(saved), false); + lastApi.pipeline = real; + const done = await Promise.race([PS.adopt(saved), new Promise((r) => setTimeout(() => r("HUNG"), 1000))]); + assert.strictEqual(done, true); + assert.strictEqual(PS.slots().find((s) => s.id === "model").inputs.file, "mine"); + await Promise.race([PS.save({ inputs: { model: { file: "next" } } }), new Promise((_, no) => setTimeout(() => no(new Error("save hung")), 1000))]); +}); diff --git a/core/readiness.py b/core/readiness.py index 05040e67..d176787e 100644 --- a/core/readiness.py +++ b/core/readiness.py @@ -36,6 +36,15 @@ def device() -> str: return "cuda" if torch.cuda.is_available() else "mps" if mps is not None and mps.is_available() else "cpu" +def gpu_index() -> int: + """Which CUDA device ComfyUI uses (a rental may start it with --cuda-device 1).""" + try: + import comfy.model_management as mm + return mm.get_torch_device().index or 0 + except Exception: # noqa: BLE001 -- not inside ComfyUI + return 0 + + def machine(disk_free_gb=None) -> List[dict]: out = [] for tool, why in (("ffmpeg", "the final render and previews"), ("ffprobe", "reading clip lengths")): @@ -51,7 +60,7 @@ def machine(disk_free_gb=None) -> List[dict]: elif kind != "cuda": out.append(row("warn", f"ComfyUI generates on a '{kind}' device, which FunPack is not tested on: expect slow runs, and some features may fail.")) else: - props = torch.cuda.get_device_properties(0) + props = torch.cuda.get_device_properties(gpu_index()) out.append(row("ok", f"{props.name}, {props.total_memory / 1024 ** 3:.0f} GB, sm_{props.major}{props.minor}")) if not torch.cuda.is_bf16_supported(): out.append(row("fail", "This GPU does not support bf16, which the H3 VAEs need.")) diff --git a/core/tests/test_readiness.py b/core/tests/test_readiness.py index 4dfa671e..1ce4072b 100644 --- a/core/tests/test_readiness.py +++ b/core/tests/test_readiness.py @@ -42,3 +42,16 @@ def test_the_row_names_the_device_comfyui_generates_on(monkeypatch): monkeypatch.setattr(r, "device", lambda: "xpu") # an Intel GPU is not the CPU rows = r.machine() assert any("'xpu'" in x["text"] for x in rows) and not any("on the CPU" in x["text"] for x in rows) + + +def test_the_gpu_row_names_the_card_comfyui_uses(monkeypatch): + import types + import torch + asked = [] + monkeypatch.setattr(r, "device", lambda: "cuda") + monkeypatch.setattr(r, "gpu_index", lambda: 1) + monkeypatch.setattr(torch.cuda, "get_device_properties", + lambda i: asked.append(i) or types.SimpleNamespace(name=f"GPU{i}", total_memory=32 * 1024 ** 3, major=12, minor=0)) + monkeypatch.setattr(torch.cuda, "is_bf16_supported", lambda: True) + rows = r.machine() + assert asked == [1] and any("GPU1" in x["text"] for x in rows)