From e9aaffc4616b02fd741926d244a68766fc69dc00 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:04:32 +0300 Subject: [PATCH] fix: Generate never waits on a value save, only on a pipeline loading or a node change; a new Generate spares the run it queued; keyless toast states facts, not a cause Co-Authored-By: Claude Opus 5.5 --- app/ui/features/generate/index.js | 6 ++++-- app/ui/features/rating/index.js | 5 +++-- app/ui/features/rating/rating.test.js | 2 +- app/ui/shell/api.js | 2 +- app/ui/shell/pipeline_state.js | 15 +++++++++++---- app/ui/shell/tests/pipeline_state.test.js | 17 +++++++++++++++++ modules/system/taste/__init__.py | 9 ++++++--- modules/system/taste/store.py | 13 ++++++++----- modules/system/taste/tests/test_taste.py | 9 +++++++++ 9 files changed, 60 insertions(+), 18 deletions(-) diff --git a/app/ui/features/generate/index.js b/app/ui/features/generate/index.js index e69d55ca..052c7f2f 100644 --- a/app/ui/features/generate/index.js +++ b/app/ui/features/generate/index.js @@ -61,7 +61,9 @@ export default { if (typing && typing !== document.body && typeof typing.blur === "function") typing.blur(); await Promise.all((app.beforeRun || []).map((f) => f().catch(() => {}))); // an edit still on its way (the Story box) lands first // A project's pipeline still going in is the one this run uses, not the last one's. - if (app.pipeline.settled && !(await app.pipeline.settled())) { tell("Not started: this project's pipeline is still loading. ComfyUI is slow to answer; try again in a moment."); return; } + // a typed value is already live and is never waited on; only a pipeline still going in, or a node change, is + const ready = app.pipeline.readyToRun || app.pipeline.settled; + if (ready && !(await ready())) { tell("Not started: this project's pipeline, or a node change in it, is still on its way. ComfyUI is slow to answer; try again in a moment."); return; } if (app.pipelineOwned && !app.pipelineOwned()) { tell(`Not started: ${(app.pipelineWhy && app.pipelineWhy()) || "this project's pipeline is not loaded yet."}`); return; } // The pipeline as it is NOW, at the click: every shot of this run uses it, whatever is opened or edited meanwhile. const frozen = structuredClone(app.pipeline.slots() || []); @@ -100,7 +102,7 @@ export default { break; } if (stopped) g.cancel(); // Stop landed while this one was being queued - if (!made) app.api.newTasteGeneration().catch(() => {}); // a run really started: the last run's unrated clips can no longer be paired with what it learned + if (!made) app.api.newTasteGeneration(g.run.state.promptId).catch(() => {}); // a run really started: the last run's unrated clips can no longer be paired with what it learned made += 1; const end = await done; if (end === g.CANCELLED) break; diff --git a/app/ui/features/rating/index.js b/app/ui/features/rating/index.js index 1130f53d..61ad8dbd 100644 --- a/app/ui/features/rating/index.js +++ b/app/ui/features/rating/index.js @@ -5,12 +5,13 @@ import { genUnitId, isGenerative, unitRoot } from "../../shell/scenes.js"; import { CHOICES, FORGET, nameOf, tasteOf } from "./choices.js"; /** What to say when a rating taught nothing, or "" when it did. The run captured nothing, a learner is on and no key is - * set: the missing key is why. Any other reason is the server's own (the key on screen now may not be the run's). */ + * set: both are said. Any other reason is the server's own. */ export function notTaught(r, ps) { if (!(r && r.why)) return ""; const taste = ps.modulesById && ps.modulesById().taste; const keyless = r.reason === "nothing" && taste && ps.useful(taste) && !String((ps.currentValues().taste || {}).key || "").trim(); - return keyless ? "Taste not taught: no Taste key is set, so nothing learns from ratings. Name one in Settings ▸ Engine ▸ System ▸ Taste key." : `Taste not taught: ${r.why}.`; + // two facts, both true, not a claimed cause: the key on screen now may not be the one the run had + return keyless ? "Taste not taught: this run captured nothing, and no Taste key is set. Ratings teach only under a key: name one in Settings ▸ Engine ▸ System ▸ Taste key." : `Taste not taught: ${r.why}.`; } export default { diff --git a/app/ui/features/rating/rating.test.js b/app/ui/features/rating/rating.test.js index d71d6644..786ae282 100644 --- a/app/ui/features/rating/rating.test.js +++ b/app/ui/features/rating/rating.test.js @@ -6,7 +6,7 @@ const ps = (key, learning = true) => { const taste = { id: "taste" }; return { m test("the not-taught reason blames a missing key only when the run captured nothing, a learner is on, and no key is set", () => { const nothing = { why: "this run captured nothing", reason: "nothing" }, forgotten = { why: "a new Generate started", reason: "forgotten" }; - assert.match(notTaught(nothing, ps("")), /no Taste key is set/); + assert.match(notTaught(nothing, ps("")), /captured nothing, and no Taste key is set/); assert.match(notTaught(forgotten, ps("")), /new Generate started/, "the key cleared after the run is not why"); assert.match(notTaught(nothing, ps("", false)), /captured nothing/, "nothing learns anyway: a key would not have helped"); assert.match(notTaught(nothing, ps("portraits")), /captured nothing/); diff --git a/app/ui/shell/api.js b/app/ui/shell/api.js index 67011653..c6d8c671 100644 --- a/app/ui/shell/api.js +++ b/app/ui/shell/api.js @@ -50,7 +50,7 @@ export const api = { enhancerDefaults: () => call("GET", "/api/m/conditioning_prompt_enhancer/defaults"), enhancerRuns: () => call("GET", "/api/m/conditioning_prompt_enhancer/runs"), rateTaste: (promptId, rating, axis) => call("POST", "/api/m/taste/rate", { prompt_id: promptId, rating, axis: axis || null }), - newTasteGeneration: () => call("POST", "/api/m/taste/generation", {}), + newTasteGeneration: (promptId) => call("POST", "/api/m/taste/generation", promptId ? { prompt_id: promptId } : {}), shotMemory: () => call("GET", "/api/m/conditioning_shot_camera/memory"), forgetShotMemory: (kind, name) => call("POST", "/api/m/conditioning_shot_camera/forget", { kind, name }), tasteKeys: () => call("GET", "/api/m/taste/keys"), diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index c5631da7..f5a55abe 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -95,6 +95,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { 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; + let shaping = false; // a structural edit (node swapped, added, removed) waiting for the server let pending = false; let pendingBody = null; let saveNotes = []; @@ -435,7 +436,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { const keep = currentValues(); // a settings node added or swapped in starts empty: it gets these let refill = false; const mine = epoch; - saving = true; + saving = true; shaping = true; let refusedNow = []; try { applyGroups(); @@ -474,7 +475,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { } catch (e) { refusedNow = [e && e.message ? e.message : String(e)]; } finally { - saving = false; + saving = false; shaping = false; } if (refill || pending) await save({}); // the filled settings, or a value edit queued behind this one return { refused: refusedNow }; @@ -497,7 +498,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { const keep = currentValues(); let refill = false; const mine = epoch; - saving = true; + saving = true; shaping = true; let refusedNow = []; try { const res = await API.editPipeline({ slots: JSON.parse(JSON.stringify(snap.slots)) }); @@ -572,6 +573,12 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { /** Resolves true once no edit is on its way and no project's pipeline is going in; false if that took over * three minutes (every request gives up after ANSWER_MS, so only a server failing again and again gets there). */ + // What a run needs finished: a project's pipeline going in, or a node swapped/added/removed (that shape exists only once + // the server answers). A value save is not waited on: the value is already in the slots (placeInputs, mirrorValues). + async function readyToRun() { + for (let waited = 0; (shaping || adoptGate) && waited < 180000; waited += 20) await new Promise((r) => setTimeout(r, 20)); + return !(shaping || adoptGate); + } async function settled() { for (let waited = 0; (saving || adoptGate) && waited < 180000; waited += 20) await new Promise((r) => setTimeout(r, 20)); return !(saving || adoptGate); @@ -659,7 +666,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { } return { - ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, settled, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, + ensureLoaded, save, edit, restore, snapshot, setGroup, adopt, settled, readyToRun, subscribe: (fn) => { listeners.add(fn); return () => listeners.delete(fn); }, valuesAlreadyPlaced, currentValues, setModuleValue, modulesById: () => modulesById, setEditGuard: (fn) => { editGuard = fn; }, adoptedOk: () => adoptedOk, unreachable: () => slots === null || unreachableLast, refusal: () => refusalLast, activeModules, useful, usefulness, isOff, setOff, allOff, setAllOff, refreshControl, control: () => controlState, removedIds: () => [...removed], diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index 6cc75bf8..3f5786fb 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -645,3 +645,20 @@ test("a typed node value is in the live slots at once, not when the server answe await Promise.all([first, second]); assert.strictEqual(steps(), 20); }); + +test("a run does not wait on a value save (the value is already live), but does wait for a node change", async () => { + const posts = []; + const PS = load(posts, { delay: 200 }); + await PS.ensureLoaded(); + const valueSave = PS.save({ inputs: { gen: { steps: 12 } } }); + const t0 = Date.now(); + assert.strictEqual(await PS.readyToRun(), true); + assert.ok(Date.now() - t0 < 100, "ready while the value save is still on its way"); + await valueSave; + const shape = PS.edit({ remove: [] }); + await new Promise((r) => setTimeout(r, 10)); + const t1 = Date.now(); + await PS.readyToRun(); + assert.ok(Date.now() - t1 > 100, "a node change is waited for"); + await shape; +}); diff --git a/modules/system/taste/__init__.py b/modules/system/taste/__init__.py index e7ff868a..dea3d784 100644 --- a/modules/system/taste/__init__.py +++ b/modules/system/taste/__init__.py @@ -177,9 +177,12 @@ def routes(table, base, web): return web.json_response(out) @table.post(base + "/generation") - async def _generation(_req): - """A Generate is starting: captures still waiting for a rating are forgotten.""" - return web.json_response({"dropped": store.new_generation()}) + async def _generation(req): + """A Generate is starting: captures still waiting for a rating are forgotten, except the run it just queued + (`{prompt_id}`): a short run can capture before this request lands.""" + body = await req.json() if req.can_read_body else {} + keep = body.get("prompt_id") if isinstance(body, dict) and isinstance(body.get("prompt_id"), str) else None + return web.json_response({"dropped": store.new_generation(keep=keep)}) @table.delete(base + "/keys/{name}") async def _delete(req): diff --git a/modules/system/taste/store.py b/modules/system/taste/store.py index 537a1103..f688fc90 100644 --- a/modules/system/taste/store.py +++ b/modules/system/taste/store.py @@ -171,20 +171,23 @@ def capture(key, kind, rows, prompt_id=None, keep=MAX_ROWS, only=None, mixed=Fal _latest_path().write_text(json.dumps({"key": key, "prompt_id": prompt_id, "runs": runs, "forgot": state.get("forgot", [])})) -def new_generation(): - """A Generate is starting: every capture still waiting for a rating is forgotten. - -> how many were dropped.""" +def new_generation(keep=None): + """A Generate is starting: every capture still waiting for a rating is forgotten, but `keep`'s (the run this + Generate already queued, which may have captured before this call). -> how many were dropped.""" with _LOCK: dropped = 0 for key in keys(): for path in _dir(key).glob("*.pending.pt"): + if keep is not None and (_read(path, None) or {}).get("prompt_id") == keep: + continue path.unlink(missing_ok=True) dropped += 1 state = _read_latest() if state: + runs = state.get("runs", {}) # remembered so a late rating can say this was why, and only then - forgot = [*state.get("forgot", []), *state.get("runs", {})][-256:] - _latest_path().write_text(json.dumps({"key": state.get("key"), "runs": {}, "forgot": forgot})) + forgot = [*state.get("forgot", []), *(p for p in runs if p != keep)][-256:] + _latest_path().write_text(json.dumps({"key": state.get("key"), "runs": {p: v for p, v in runs.items() if p == keep}, "forgot": forgot})) return dropped diff --git a/modules/system/taste/tests/test_taste.py b/modules/system/taste/tests/test_taste.py index 40a94c98..12322cd7 100644 --- a/modules/system/taste/tests/test_taste.py +++ b/modules/system/taste/tests/test_taste.py @@ -260,3 +260,12 @@ def test_a_kind_of_mixed_sizes_exports_and_imports(tmp_path): store.export_key("bear", tmp_path / "b.zip") with pytest.raises(ValueError): store.import_key("cub", tmp_path / "b.zip") + + +def test_a_new_generate_spares_the_run_it_already_queued(): + """The 'new Generate' call goes out after its first run is queued; a short run may have captured first.""" + store.capture("k", "vel", {"a": torch.zeros(2)}, prompt_id="old") + store.capture("k", "vel", {"a": torch.zeros(2)}, prompt_id="mine") + store.new_generation(keep="mine") + assert store.rate("mine", "liked")["recorded"] == ["vel"] + assert store.rate("old", "liked")["reason"] == "forgotten"