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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
baaa792993
commit
3db07afdcb
@@ -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); }
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
@@ -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) })),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
+3
-3
@@ -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). */
|
||||
|
||||
+35
-11
@@ -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,
|
||||
|
||||
@@ -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}")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user