fix: all-enhancements payload valid alone, Generate waits for pending settings, forget is complete and sticks, camera choices listed, Manager lookup case-proof and declared
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
1984dd6c1e
commit
c0cdf9efd9
@@ -28,6 +28,7 @@ export default {
|
||||
return { inputs: withIt.overrides, notes: withIt.unwired > without.unwired ? ["The identity picture was not used: this pipeline has no free reference input for it."] : [] };
|
||||
};
|
||||
const carryHook = async ({ project, scene, slots }) => {
|
||||
if (app.pipeline.allOff?.()) return {}; // Disable all enhancements means a plain run
|
||||
const mine = (app.pipeline.currentValues() || {}).continuity;
|
||||
if (!mine) return {}; // the module that holds the choices is not here: this does nothing at all
|
||||
const carry = mine.carry !== false, guard = mine.dark_guard !== false;
|
||||
|
||||
@@ -68,6 +68,7 @@ export default {
|
||||
if (!root || group.every((s) => s.excluded) || !isGenerative(root)) continue; // removed, all left out, or not made by the model
|
||||
// The unit is made once at the length of its clips together (each cut adds one shared frame).
|
||||
const frames = group.reduce((t, s) => t + effFrames(s, snap || p.project), 0) - (group.length - 1);
|
||||
await app.pipeline.settled?.(); // an edit still on its way (the all-enhancements switch, say) is part of this run
|
||||
const { inputs, unwired, noPrompt, notes } = await buildInputs({ project: snap || p.project, scene: root, slots: app.pipeline.slots(), expand: same, frames, hooks: app.inputHooks, prefix: (app.promptPrefix || []).flatMap((f) => { try { return f(root); } catch { return []; } }) });
|
||||
app.lastRun.typed = (root.text || "").trim(); // what a Chat comment made now would be about
|
||||
if (stopped) break;
|
||||
|
||||
@@ -14,7 +14,7 @@ export default {
|
||||
plain.hidden = true;
|
||||
host.append(chip, plain);
|
||||
const draw = (slots) => { chip.hidden = !(slots && !slots.some((s) => (s.roles || []).some((r) => r.at === "generation.prompt"))); };
|
||||
const drawPlain = () => { plain.hidden = !app.pipeline.allOff(); };
|
||||
const drawPlain = () => { plain.hidden = !app.pipeline.allOff?.(); };
|
||||
drawPlain();
|
||||
draw(app.pipeline.slots());
|
||||
return app.pipeline.subscribe ? app.pipeline.subscribe(() => { draw(app.pipeline.slots()); drawPlain(); }) : undefined;
|
||||
|
||||
@@ -87,6 +87,7 @@ export const models = (app) => function mount() {
|
||||
|
||||
// Which pack gives a missing node (from ComfyUI-Manager's map, when it is installed), and a button to clone it.
|
||||
const providers = {};
|
||||
let managerHere = true;
|
||||
const installRow = (cls) => {
|
||||
const row = c.region.stack({ gap: "sm" });
|
||||
const show = () => {
|
||||
@@ -95,10 +96,10 @@ export const models = (app) => function mount() {
|
||||
: url ? [c.settingsRow.default({ label: url.replace(/^https?:\/\/(www\.)?github\.com\//, ""), hint: "Provides this node. Installs into custom_nodes; restart ComfyUI afterwards.", control: c.button.sm({ label: "Install", tone: "primary", onClick: async () => {
|
||||
try { await api.pack("install", { url }); c.toast.good({ text: "Installed. Restart ComfyUI for the node to appear." }); } catch (err) { c.toast.warn({ text: err.message }); }
|
||||
} }) })]
|
||||
: [c.hint.default({ text: "No known pack provides it. Add one by its git URL in Settings ▸ Custom Nodes." })]);
|
||||
: [c.hint.default({ text: managerHere ? "No known pack provides it. Add one by its git URL in Settings ▸ Custom Nodes." : "ComfyUI-Manager is not installed here, so there is no list to look it up in. Add the pack by its git URL in Settings ▸ Custom Nodes." })]);
|
||||
};
|
||||
show();
|
||||
if (!(cls in providers)) api.packProviders([cls]).then((r) => { providers[cls] = (r.providers || {})[cls] || null; show(); }).catch(() => { providers[cls] = null; show(); });
|
||||
if (!(cls in providers)) api.packProviders([cls]).then((r) => { managerHere = r.manager !== false; providers[cls] = (r.providers || {})[cls] || null; show(); }).catch(() => { providers[cls] = null; show(); });
|
||||
return row;
|
||||
};
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ export const modules = (app) => function mount() {
|
||||
onChange: (on) => ps.setOff(m.id, !on).then(draw) });
|
||||
};
|
||||
const notes = ps.saveNotes();
|
||||
const master = c.toggle.default({ label: "Disable all enhancements", hint: "Runs the plain pipeline: every module below sits out, and so do the rewrites of shot camera. Your choices are kept — switch it back off to restore them. Handy for an A/B test of whether a feature does anything.",
|
||||
const master = c.toggle.default({ label: "Disable all enhancements", hint: "Every module below sits out, and so do shot camera's rewrites and continuity. Your choices are kept — switch it back off to restore them. Values typed into a node in Models & Pipeline (negative erase, int8, SLA…) are not touched. Handy for an A/B test of whether a feature does anything.",
|
||||
checked: ps.allOff(), onChange: (on) => ps.setAllOff(on).then(draw) });
|
||||
page.set([master, notes.length ? c.banner.warn({ text: notes.join(" ") }) : null, c.hint.default({ text: "A module you turn off here is off for this project only, and disappears from Engine settings. One that failed while generating is off for every project until its code is repaired or you turn it back on." }),
|
||||
...CATEGORIES.flatMap(([key, label]) => { const list = mine.filter((m) => (m.category || "") === key); return list.length ? [c.label.section({ text: label }), ...list.map(row)] : []; })].filter(Boolean));
|
||||
|
||||
@@ -20,11 +20,12 @@ 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) 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: "Turn on camera moves or shot views, 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)),
|
||||
data.more ? c.hint.default({ text: `and ${data.more} more` }) : null] : []),
|
||||
...((data.arms || []).length ? [c.label.section({ text: "Camera choices" }), ...data.arms.map((a) => row(a.arm.replace(":", " · "), `liked ${a.good.toFixed(1)} · disliked ${a.bad.toFixed(1)}`, "arm", a.arm))] : []),
|
||||
c.button.sm({ label: "Forget everything", tone: "danger", onClick: () => forget("all") })].filter(Boolean));
|
||||
}
|
||||
draw(); refresh();
|
||||
|
||||
@@ -279,6 +279,8 @@ export function createPipelineState(API) {
|
||||
// choices are kept underneath, so switching it back restores exactly what was on.
|
||||
const allOff = () => Boolean((currentValues()._off || {}).all);
|
||||
const setAllOff = (on) => setModuleValue("_off", "all", Boolean(on));
|
||||
// Resolves once every edit made so far has been sent: a run started now reads what the screen shows.
|
||||
async function settled() { for (let waited = 0; (saving || pending || pendingBody) && waited < 10000; waited += 20) await new Promise((r) => setTimeout(r, 20)); }
|
||||
async function refreshControl() {
|
||||
try { controlState = (await API.modules()).control || {}; } catch (_) { /* keeps the last answer */ }
|
||||
return controlState;
|
||||
@@ -524,7 +526,7 @@ export function createPipelineState(API) {
|
||||
frozenInputs,
|
||||
ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue,
|
||||
modulesById: () => modulesById,
|
||||
activeModules, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState,
|
||||
activeModules, isOff, setOff, allOff, setAllOff, settled, refreshControl, control: () => controlState,
|
||||
removedIds: () => [...removed],
|
||||
unwiredMap: () => JSON.parse(JSON.stringify(unwired)),
|
||||
slots: () => slots,
|
||||
|
||||
@@ -251,6 +251,33 @@ test("a module switched off is remembered with the settings, listed as off, and
|
||||
assert.deepStrictEqual(posts.at(-1).values._off, { modules: ["alg"] });
|
||||
});
|
||||
|
||||
test("Disable all enhancements is saved on its own, hides every module, and keeps the per-module choices underneath", async () => {
|
||||
const posts = [];
|
||||
const PS = load(posts);
|
||||
await PS.ensureLoaded();
|
||||
lastApi.modules = async () => ({ modules: [{ id: "sharpen", settings: {} }, { id: "alg", settings: {} }], control: {} });
|
||||
await PS.refreshControl();
|
||||
await PS.setOff("alg", true);
|
||||
await PS.setAllOff(true);
|
||||
assert.strictEqual(PS.allOff(), true);
|
||||
assert.deepStrictEqual(posts.at(-1).values._off, { modules: ["alg"], all: true });
|
||||
assert.deepStrictEqual(PS.activeModules(), []);
|
||||
await PS.setAllOff(false);
|
||||
assert.strictEqual(PS.allOff(), false);
|
||||
assert.strictEqual(PS.isOff("alg"), true);
|
||||
});
|
||||
|
||||
test("settled() resolves only after the edits made so far were sent", async () => {
|
||||
const posts = [];
|
||||
const PS = load(posts);
|
||||
await PS.ensureLoaded();
|
||||
const first = PS.setAllOff(true);
|
||||
PS.setAllOff(false);
|
||||
await PS.settled();
|
||||
assert.strictEqual(posts.at(-1).values._off.all, false);
|
||||
await first;
|
||||
});
|
||||
|
||||
test("a project opened after another does not inherit the first one's pending switch", async () => {
|
||||
const PS = load([]);
|
||||
await PS.ensureLoaded();
|
||||
|
||||
+1
-1
@@ -59,7 +59,7 @@ def bad_off(raw) -> str | None:
|
||||
"""Why a payload's `_off` entry is malformed, or None."""
|
||||
if raw is None:
|
||||
return None
|
||||
ids = raw.get("modules") if isinstance(raw, dict) else None
|
||||
ids = raw.get("modules", []) if isinstance(raw, dict) else None # either half may stand alone
|
||||
if not isinstance(ids, list) or not all(isinstance(i, str) for i in ids) or not isinstance(raw.get("all", False), bool):
|
||||
return '"_off" must be {"modules": ["module_id", ...], "all": true|false}'
|
||||
return None
|
||||
|
||||
+22
-8
@@ -290,21 +290,35 @@ def check_updates() -> dict:
|
||||
return {"checked": results}
|
||||
|
||||
|
||||
def _manager_map() -> Path | None:
|
||||
"""Manager's node map, wherever this install keeps it (the folder name's case differs between machines)."""
|
||||
try:
|
||||
root = custom_nodes_root()
|
||||
except CustomNodeError:
|
||||
return None
|
||||
for folder in sorted(root.iterdir()) if root.is_dir() else ():
|
||||
if folder.name.lower() == "comfyui-manager" and (folder / "extension-node-map.json").is_file():
|
||||
return folder / "extension-node-map.json"
|
||||
return None
|
||||
|
||||
|
||||
def providers(classes) -> dict:
|
||||
"""{node class: git URL of a pack that provides it, or None}, from ComfyUI-Manager's own node map when that
|
||||
pack is installed (its extension-node-map.json: {url: [[class, ...], meta]}). Without Manager nothing is
|
||||
known, so every answer is None and the person is sent to Custom Nodes with a URL of their own."""
|
||||
"""{"providers": {node class: git URL of a pack that provides it, or None}, "manager": whether the map exists}.
|
||||
The map is ComfyUI-Manager's extension-node-map.json ({url: [[class, ...], meta]}). Without Manager nothing is
|
||||
known, and the caller says so rather than claiming that nobody provides the node."""
|
||||
wanted = {str(c) for c in classes or ()}
|
||||
found = {c: None for c in wanted}
|
||||
path = _manager_map()
|
||||
if path is None:
|
||||
return {"providers": found, "manager": False}
|
||||
try:
|
||||
path = custom_nodes_root() / "ComfyUI-Manager" / "extension-node-map.json"
|
||||
table = json.loads(path.read_text(encoding="utf-8"))
|
||||
except (CustomNodeError, OSError, ValueError):
|
||||
return found
|
||||
except (OSError, ValueError):
|
||||
return {"providers": found, "manager": False}
|
||||
if not isinstance(table, dict):
|
||||
return found
|
||||
return {"providers": found, "manager": False}
|
||||
for url, entry in table.items():
|
||||
names = entry[0] if isinstance(entry, list) and entry and isinstance(entry[0], list) else []
|
||||
for name in wanted.intersection(names):
|
||||
found[name] = found[name] or str(url)
|
||||
return found
|
||||
return {"providers": found, "manager": True}
|
||||
|
||||
+1
-1
@@ -668,7 +668,7 @@ def register(routes, prefix=None):
|
||||
async def _packs_providers(req):
|
||||
"""Which pack gives each of `?classes=A,B` (a git URL, or null when nobody is known to)."""
|
||||
names = [n for n in (part.strip() for part in (req.query.get("classes") or "").split(",")) if n]
|
||||
return web.json_response({"providers": await asyncio.to_thread(nodes_manager.providers, names)})
|
||||
return web.json_response(await asyncio.to_thread(nodes_manager.providers, names))
|
||||
|
||||
@routes.post(P + "/api/packs/check")
|
||||
async def _packs_check(_req):
|
||||
|
||||
@@ -125,3 +125,8 @@ def test_disable_all_enhancements_sits_out_every_modifier_and_nothing_else():
|
||||
assert [s.id for s in kept] == ["loader"] and "all enhancements" in notes[0]
|
||||
assert control.all_off({"_off": {"all": True}}) and not control.all_off({"_off": {"all": "yes"}}) and not control.all_off({})
|
||||
assert control.bad_off({"modules": [], "all": "yes"}) and control.bad_off({"modules": [], "all": True}) is None
|
||||
|
||||
|
||||
def test_all_alone_is_a_valid_payload():
|
||||
assert control.bad_off({"all": True}) is None and control.bad_off({"all": False}) is None
|
||||
assert control.bad_off({"all": "yes"}) and control.bad_off({"modules": "x"})
|
||||
|
||||
@@ -12,12 +12,20 @@ def test_providers_reads_managers_node_map(tmp_path, monkeypatch):
|
||||
}))
|
||||
monkeypatch.setattr(nodes_manager, "custom_nodes_root", lambda: tmp_path)
|
||||
got = nodes_manager.providers(["NodeOne", "NodeTwo", "Nobody"])
|
||||
assert got == {"NodeOne": "https://github.com/a/pack-a", "NodeTwo": "https://github.com/a/pack-a", "Nobody": None}
|
||||
assert got["manager"] is True
|
||||
assert got["providers"] == {"NodeOne": "https://github.com/a/pack-a", "NodeTwo": "https://github.com/a/pack-a", "Nobody": None}
|
||||
|
||||
|
||||
def test_without_manager_nothing_is_known(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(nodes_manager, "custom_nodes_root", lambda: tmp_path)
|
||||
assert nodes_manager.providers(["X"]) == {"X": None}
|
||||
assert nodes_manager.providers(["X"]) == {"providers": {"X": None}, "manager": False}
|
||||
(tmp_path / "ComfyUI-Manager").mkdir()
|
||||
(tmp_path / "ComfyUI-Manager" / "extension-node-map.json").write_text("not json")
|
||||
assert nodes_manager.providers(["X"]) == {"X": None}
|
||||
assert nodes_manager.providers(["X"]) == {"providers": {"X": None}, "manager": False}
|
||||
|
||||
|
||||
def test_manager_folder_case_does_not_matter(tmp_path, monkeypatch):
|
||||
(tmp_path / "comfyui-manager").mkdir()
|
||||
(tmp_path / "comfyui-manager" / "extension-node-map.json").write_text(json.dumps({"https://github.com/a/p": [["N"], {}]}))
|
||||
monkeypatch.setattr(nodes_manager, "custom_nodes_root", lambda: tmp_path)
|
||||
assert nodes_manager.providers(["N"])["providers"]["N"] == "https://github.com/a/p"
|
||||
|
||||
@@ -210,7 +210,9 @@ def summary(limit=40):
|
||||
words.sort(key=lambda r: -(r["picks"] * 3 + r["rejects"] * 3 + r["seen"]))
|
||||
views = [{"view": k, "good": g, "bad": b} for k, (g, b) in view_stats().items() if "@" not in k]
|
||||
views.sort(key=lambda r: -(r["good"] + r["bad"]))
|
||||
return {"words": words[:limit], "more": max(0, len(words) - limit), "views": views,
|
||||
arms = [{"arm": k, "good": g, "bad": b} for k, (g, b) in arm_stats().items() if not k.startswith("word:")]
|
||||
arms.sort(key=lambda r: -(r["good"] + r["bad"]))
|
||||
return {"words": words[:limit], "more": max(0, len(words) - limit), "views": views, "arms": arms,
|
||||
"prompts": int(data.get("prompts", 0)), "shots": int(data.get("shots", 0)),
|
||||
"kept": int(data.get("kept", 0))}
|
||||
|
||||
@@ -227,15 +229,20 @@ def forget(kind, name=None):
|
||||
existed = any(name in (data.get(k) or {}) for k in ("picks", "rejects", "seen"))
|
||||
for k in ("picks", "rejects", "seen"):
|
||||
(data.get(k) or {}).pop(name, None)
|
||||
existed = existed or (data.get("arms") or {}).pop(f"word:{name}", None) is not None
|
||||
existed = ((data.get("arms") or {}).pop(f"word:{name}", None) is not None) or existed
|
||||
elif kind == "view":
|
||||
views = data.get("views") or {}
|
||||
gone = [k for k in views if k == name or k.startswith(f"{name}@")]
|
||||
existed = bool(gone)
|
||||
for k in gone:
|
||||
views.pop(k)
|
||||
elif kind == "arm":
|
||||
existed = (data.get("arms") or {}).pop(name, None) is not None
|
||||
else:
|
||||
raise ValueError(f"unknown kind {kind!r}")
|
||||
if existed: # a forgotten lesson must not be rebuilt by re-rating the run that taught it
|
||||
data.pop("runs", None)
|
||||
data.pop("rated", None)
|
||||
_save(data)
|
||||
return existed
|
||||
|
||||
|
||||
@@ -100,3 +100,16 @@ def test_memory_routes_read_and_forget(tmp_path, monkeypatch):
|
||||
assert asyncio.run(handlers[("POST", "/b/forget")](Req({"kind": "view", "name": "side view"}))).data == {"forgotten": True}
|
||||
assert asyncio.run(handlers[("GET", "/b/memory")](Req())).data["views"] == []
|
||||
assert asyncio.run(handlers[("POST", "/b/forget")](Req({"kind": "nonsense"}))).status == 400
|
||||
|
||||
|
||||
def test_forget_is_complete_and_stays_forgotten(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("SHOT_CAMERA_MEMORY", str(tmp_path / "m.json"))
|
||||
memory.observe("h1", ["lamp"])
|
||||
memory.record_run("a", {"views": [{"view": "side view", "traits": ["none"]}], "arms": ["word:lamp", "split:yes"]})
|
||||
memory.on_rating("a", "liked")
|
||||
assert memory.forget("word", "lamp") and "word:lamp" not in memory.arm_stats()
|
||||
assert memory.forget("view", "side view")
|
||||
memory.on_rating("a", "disliked") # the run that taught it is gone too
|
||||
assert memory.view_stats() == {}
|
||||
assert [a["arm"] for a in memory.summary()["arms"]] == ["split:yes"]
|
||||
assert memory.forget("arm", "split:yes") and memory.summary()["arms"] == []
|
||||
|
||||
Reference in New Issue
Block a user