From 3db07afdcb6379409383897b0aa2530d7aeff21f Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 6 Oct 2026 06:39:44 +0300 Subject: [PATCH] fix: log reads ComfyUI's own output; Taste key field visible, chip beside Generate, honest not-taught reason; splitters stop above timeline Co-Authored-By: Claude Opus 5.5 --- app/ui/composer/elements/polish.css | 2 ++ app/ui/features/generate/warn.js | 13 +++++-- app/ui/features/generate/warn.test.js | 23 ++++++++++++ app/ui/features/inspector/scene.js | 2 +- app/ui/features/rating/index.js | 9 +++-- app/ui/frame.css | 6 ++-- core/backend_log.py | 46 ++++++++++++++++++------ core/tests/test_routes_update.py | 17 +++++++++ modules/system/taste/store.py | 16 +++++---- modules/system/taste/tests/test_taste.py | 9 +++++ 10 files changed, 117 insertions(+), 26 deletions(-) create mode 100644 app/ui/features/generate/warn.test.js diff --git a/app/ui/composer/elements/polish.css b/app/ui/composer/elements/polish.css index 68aab6bd..7019b508 100644 --- a/app/ui/composer/elements/polish.css +++ b/app/ui/composer/elements/polish.css @@ -39,6 +39,8 @@ body { -webkit-font-smoothing: antialiased; letter-spacing: -.003em; } /* fields: filled, borderless until focused */ .cx-input, .cx-textarea, .cx-select { border-color: transparent; background: var(--elev-2); } .cx-input:hover:not(:disabled), .cx-textarea:hover:not(:disabled), .cx-select:hover:not(:disabled) { border-color: var(--line-2); } +/* a settings row is itself --elev-2: a field on it would vanish, so it sits sunken with an edge */ +.cx-settings-row :is(.cx-input, .cx-textarea, .cx-select) { background: var(--elev-1); border-color: var(--line); } .cx-step-btn { border-color: transparent; } .cx-filter-list { border-color: var(--line); border-radius: var(--r-4); } diff --git a/app/ui/features/generate/warn.js b/app/ui/features/generate/warn.js index d5508260..26bc05c8 100644 --- a/app/ui/features/generate/warn.js +++ b/app/ui/features/generate/warn.js @@ -1,4 +1,4 @@ -// The chip beside Generate when the pipeline has nowhere to put the scene text. +// The chips beside Generate: the pipeline has nowhere to put the scene text; a learning feature is on with no Taste key. import { composer as c } from "../../composer/composer.js"; export default { @@ -12,9 +12,16 @@ export default { const plain = c.button.sm({ label: "Enhancements off", tone: "ghost", title: "Disable all enhancements is on: runs use the plain pipeline. Switch it off in Settings ▸ Modules.", onClick: () => app.openSettings?.("modules") }).node; plain.hidden = true; - host.append(chip, plain); + const keyless = c.button.sm({ label: "⚠ No Taste key", tone: "ghost", title: "A feature that learns from ratings is on, but no Taste key is set: ratings teach nothing. Name one in Settings ▸ Engine ▸ System.", + onClick: () => app.openSettings?.("engine") }).node; + keyless.hidden = true; + host.append(chip, plain, keyless); 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 = () => { + const ps = app.pipeline, taste = ps.modulesById?.().taste; + plain.hidden = !ps.allOff?.(); + keyless.hidden = !(taste && ps.activeModules().includes(taste) && ps.useful(taste) && !String((ps.currentValues().taste || {}).key || "").trim()); + }; drawPlain(); draw(app.pipeline.slots()); return app.pipeline.subscribe ? app.pipeline.subscribe(() => { draw(app.pipeline.slots()); drawPlain(); }) : undefined; diff --git a/app/ui/features/generate/warn.test.js b/app/ui/features/generate/warn.test.js new file mode 100644 index 00000000..7c949251 --- /dev/null +++ b/app/ui/features/generate/warn.test.js @@ -0,0 +1,23 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { setupDom, teardownDom } from "../../composer/tests/_dom.js"; + +let warn; +test.before(async () => { setupDom(); warn = (await import("./warn.js")).default; }); +test.after(() => teardownDom()); + +const chip = (host) => [...host.querySelectorAll("button")].find((b) => /No Taste key/.test(b.textContent)); + +test("the No Taste key chip shows only while a learning feature is on and the key is empty", () => { + const taste = { id: "taste" }; + let values = { taste: { key: "" } }, learning = true, listener; + const pipeline = { slots: () => [], allOff: () => false, modulesById: () => ({ taste }), activeModules: () => [taste], + useful: (m) => m === taste && learning, currentValues: () => values, subscribe: (fn) => { listener = fn; return () => {}; } }; + const host = document.createElement("div"); + warn.setup({ host, app: { pipeline } }); + assert.equal(chip(host).hidden, false, "learning on, no key: said"); + values = { taste: { key: " portraits " } }; listener(); + assert.equal(chip(host).hidden, true, "a key is set"); + values = { taste: { key: "" } }; learning = false; listener(); + assert.equal(chip(host).hidden, true, "nothing learns, so a missing key changes nothing"); +}); diff --git a/app/ui/features/inspector/scene.js b/app/ui/features/inspector/scene.js index b94a287e..690dc75e 100644 --- a/app/ui/features/inspector/scene.js +++ b/app/ui/features/inspector/scene.js @@ -69,6 +69,6 @@ export function sceneRows(p, app) { drift(sc, open), c.button.md({ label: "Generate this scene", tone: "primary", onClick: () => app.say("generate.scene") })), ...trimRows(p, sc, open), - card(c.checkbox.default({ label: "Exclude from full generation", checked: Boolean(sc.excluded), onChange: (v) => p.setScene(sc.id, "excluded", v) })), + card(c.checkboxRow.default({ label: "Exclude from full generation", hint: "Generate leaves this scene out.", checked: Boolean(sc.excluded), onChange: (v) => p.setScene(sc.id, "excluded", v) })), ]; } diff --git a/app/ui/features/rating/index.js b/app/ui/features/rating/index.js index 69ea4b70..a9a79a23 100644 --- a/app/ui/features/rating/index.js +++ b/app/ui/features/rating/index.js @@ -7,7 +7,7 @@ import { CHOICES, FORGET, nameOf, tasteOf } from "./choices.js"; export default { id: "rating", mount: "timeline.actions", - needs: ["project", "api", "selection"], + needs: ["project", "api", "selection", "pipeline"], setup({ host, app }) { const p = app.project, sel = app.selection; const put = inPlace(host); @@ -25,7 +25,12 @@ export default { const taste = tasteOf(value); if (!t.render.promptId || taste === null) return; // no run to pair it with, or a word that teaches nothing: it stays a label const body = taste === "clear" ? { rating: null, axis: null } : taste; - try { const r = await app.api.rateTaste(t.render.promptId, body.rating, body.axis); if (r && r.why) c.toast.warn({ text: `Taste not taught: ${r.why}.` }); } catch { /* no taste module here: the rating stays a label */ } + try { + const r = await app.api.rateTaste(t.render.promptId, body.rating, body.axis); + // with no key nothing is ever captured: that, not the server's list of other causes, is the reason + const keyless = !String((app.pipeline.currentValues().taste || {}).key || "").trim(); + if (r && r.why) c.toast.warn({ text: 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}.` }); + } catch { /* no taste module here: the rating stays a label */ } }; const draw = () => { const t = target(); diff --git a/app/ui/frame.css b/app/ui/frame.css index 50313dc6..fa6bbdc6 100644 --- a/app/ui/frame.css +++ b/app/ui/frame.css @@ -19,12 +19,12 @@ .cx-zone .cx-stack-fill > .cx-empty { flex: 1 1 auto; justify-content: center; } /* Splitters: thin hit areas straddling the zone borders. */ -.fp-main { position: relative; } +.fp-main, .fp-row { position: relative; } /* side bars live in the row, so they stop above the timeline */ .fp-split { position: absolute; z-index: 20; background: transparent; touch-action: none; } .fp-split:hover { background: var(--accent, var(--line)); opacity: .5; } .fp-split-left, .fp-split-right { top: 0; bottom: 0; inline-size: 7px; cursor: col-resize; } -.fp-split-left { left: calc(var(--pad) + var(--media-w) + var(--pgap) / 2 - 3.5px); } -.fp-split-right { right: calc(var(--pad) + var(--properties-w) + var(--pgap) / 2 - 3.5px); } +.fp-split-left { left: calc(var(--media-w) + var(--pgap) / 2 - 3.5px); } +.fp-split-right { right: calc(var(--properties-w) + var(--pgap) / 2 - 3.5px); } .fp-split-bottom { left: 0; right: 0; block-size: 7px; bottom: calc(var(--pad) + var(--timeline-h) + var(--pgap) / 2 - 3.5px); cursor: row-resize; } /* Dock: a hidden zone leaves the row (and the splitter beside it). */ diff --git a/core/backend_log.py b/core/backend_log.py index ba39021c..b20c16fe 100644 --- a/core/backend_log.py +++ b/core/backend_log.py @@ -3,12 +3,10 @@ Users do not open devtools, and "it did not work" with nothing to read is where a bug report dies. This is the log they can actually reach. -Read from ComfyUI's own file rather than from a buffer in this process. v4 kept -both and its own docstring says why the file is the half that matters: a -CRASH AND RESTART empties an in-memory buffer, and the lines before the crash -are exactly the ones worth reading. A file survives it. So this is the smaller -half and the more useful one, and the buffer can be added if a case turns up -that the file misses. +Read from ComfyUI's own in-memory copy of its output (`app.logger.logs`, what +its terminal printed). Current ComfyUI writes no log file unless ComfyUI-Manager +adds one, so a file-only reader found nothing on a plain install. The copy is +lengthened here from 300 writes, and a file is read only when there is no copy. Never raises. A log panel that fails is worse than one that says it found no file, because the reason someone opened it is that something else already broke. @@ -16,6 +14,8 @@ file, because the reason someone opened it is that something else already broke. from __future__ import annotations +import re +from collections import deque from pathlib import Path #: Read from the end. A ComfyUI log runs to megabytes over a long session and @@ -24,6 +24,22 @@ MAX_BYTES = 512 * 1024 MAX_LINES = 2000 +def _memory(): + """ComfyUI's deque of `{t, m}` writes, lengthened once; None outside ComfyUI's main.""" + try: + import app.logger as comfy_log + except Exception: # noqa: BLE001 + return None + logs = comfy_log.logs + if logs is not None and (logs.maxlen or 0) < 8000: + # its interceptor looks the global up on every write, so the longer deque takes over + comfy_log.logs = logs = deque(logs, maxlen=8000) + return logs + + +_memory() + + def log_file() -> Path | None: """ComfyUI's log, or None when this install does not write one.""" try: @@ -31,11 +47,12 @@ def log_file() -> Path | None: base = Path(folder_paths.base_path) except Exception: # noqa: BLE001 return None - for name in ("comfyui.log", "comfyui.prev.log"): - candidate = base / "user" / name - if candidate.is_file(): - return candidate - return None + # ComfyUI-Manager names it per port (comfyui_8188.log); the newest is this run's + try: + found = [(p.stat().st_mtime, p) for p in (base / "user").glob("comfyui*.log") if ".prev" not in p.name] + except OSError: + return None + return max(found, default=(0, None))[1] def recent(limit: int = 600) -> dict: @@ -46,6 +63,13 @@ def recent(limit: int = 600) -> dict: that looks like a quiet log. """ limit = max(1, min(int(limit or 600), MAX_LINES)) + logs = _memory() + if logs: + text = re.sub(r"\x1b\[[0-9;]*m", "", "".join(str(e.get("m", "")) for e in list(logs))) # terminal colours + lines = [ln.rsplit("\r", 1)[-1] for ln in text.split("\n")] # a progress bar keeps its last state + if lines and not lines[-1]: + lines.pop() + return {"lines": lines[-limit:], "path": None, "detail": ""} path = log_file() if path is None: return {"lines": [], "path": None, diff --git a/core/tests/test_routes_update.py b/core/tests/test_routes_update.py index 78ec84c2..a114085b 100644 --- a/core/tests/test_routes_update.py +++ b/core/tests/test_routes_update.py @@ -529,6 +529,7 @@ def test_no_log_file_is_an_answer_with_a_reason(server, monkeypatch): """An empty list looks exactly like a quiet log. The difference matters when the reason somebody opened the panel is that something already broke.""" from core import backend_log + monkeypatch.setattr(backend_log, "_memory", lambda: None) monkeypatch.setattr(backend_log, "log_file", lambda: None) status, body = _request(server["port"], "GET", "/funpack/api/log") @@ -541,6 +542,7 @@ def test_a_log_that_cannot_be_read_says_so_rather_than_raising(server, monkeypat from core import backend_log missing = tmp_path / "gone.log" missing.write_text("x\n") + monkeypatch.setattr(backend_log, "_memory", lambda: None) monkeypatch.setattr(backend_log, "log_file", lambda: missing) missing.unlink() @@ -550,6 +552,21 @@ def test_a_log_that_cannot_be_read_says_so_rather_than_raising(server, monkeypat assert "gone.log" in body["detail"] +def test_comfyuis_own_output_is_read_when_it_writes_no_file(server, monkeypatch): + """Plain ComfyUI keeps its output in memory and writes no file; that copy is the log. + A progress bar's carriage returns keep only its last state.""" + from collections import deque + from core import backend_log + writes = deque([{"m": "boot\n"}, {"m": " 5%|#\r 100%|##########"}, {"m": "\n"}, {"m": "\x1b[33mPrompt executed\x1b[0m\n"}]) + monkeypatch.setattr(backend_log, "_memory", lambda: writes) + monkeypatch.setattr(backend_log, "log_file", lambda: None) + + status, body = _request(server["port"], "GET", "/funpack/api/log") + assert status == 200, body + assert body["lines"] == ["boot", " 100%|##########", "Prompt executed"] + assert body["detail"] == "" + + def test_a_silly_limit_does_not_read_a_whole_session_into_memory(server): for limit in ("999999", "-4", "0", "not-a-number"): status, body = _request(server["port"], "GET", f"/funpack/api/log?limit={limit}") diff --git a/modules/system/taste/store.py b/modules/system/taste/store.py index 54104f83..683dca76 100644 --- a/modules/system/taste/store.py +++ b/modules/system/taste/store.py @@ -168,7 +168,7 @@ def capture(key, kind, rows, prompt_id=None, keep=MAX_ROWS, only=None, mixed=Fal if kind not in kinds: kinds.append(kind) ROOT.mkdir(parents=True, exist_ok=True) - _latest_path().write_text(json.dumps({"key": key, "prompt_id": prompt_id, "runs": runs})) + _latest_path().write_text(json.dumps({"key": key, "prompt_id": prompt_id, "runs": runs, "forgot": state.get("forgot", [])})) def new_generation(): @@ -182,7 +182,9 @@ def new_generation(): dropped += 1 state = _read_latest() if state: - _latest_path().write_text(json.dumps({"key": state.get("key"), "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})) return dropped @@ -231,7 +233,8 @@ def rate(prompt_id, rating, axis=None): raise ValueError("only a dislike can name what went wrong") out = {"recorded": [], "updated": [], "why": None} with _LOCK: - entry = _read_latest().get("runs", {}).get(prompt_id) or {} + state = _read_latest() + entry = state.get("runs", {}).get(prompt_id) or {} # A clip already recorded: change or remove its row, in every key/kind. for key in keys(): for path in _dir(key).glob("*.pt"): @@ -259,9 +262,10 @@ def rate(prompt_id, rating, axis=None): return out if not entry: - out["why"] = ("no capture is waiting for this clip: a new Generate started before it was rated, " - "or this run captured nothing (no learning feature was on, or ComfyUI reused an " - "earlier result instead of sampling again)") + out["why"] = ("a new Generate started before this clip was rated, and unrated clips are forgotten then" + if prompt_id in state.get("forgot", []) else + "this run captured nothing: no learning feature acted on it, or ComfyUI reused an " + "earlier result instead of sampling again") return out if rating is None: return out diff --git a/modules/system/taste/tests/test_taste.py b/modules/system/taste/tests/test_taste.py index b84c633a..76a73fd7 100644 --- a/modules/system/taste/tests/test_taste.py +++ b/modules/system/taste/tests/test_taste.py @@ -193,6 +193,15 @@ def test_a_cleared_rating_beside_a_newer_run_goes_aside_not_over_it(): def test_a_rating_says_why_when_nothing_was_kept_for_the_clip(): out = store.rate("never-captured", "liked") assert out["recorded"] == [] and "captured nothing" in out["why"] + assert "new Generate" not in out["why"] # nothing started since: blaming one would be false + + +def test_a_clip_that_captured_nothing_is_not_blamed_on_a_later_generate(): + store.capture("k", "vel", {"a": torch.zeros(2)}, prompt_id="p1") + store.new_generation() + store.capture("k", "vel", {"a": torch.zeros(2)}, prompt_id="p2") # p2 ran and kept p1's forgetting on record + assert "new Generate" in store.rate("p1", "liked")["why"] + assert "captured nothing" in store.rate("p3", "liked")["why"] def test_deleting_a_key_leaves_other_keys_runs_ratable_and_one_run_can_wait_under_two_keys():