From 78c43acd67a2bd1c3c18db374062acc2e80aef18 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sat, 29 Aug 2026 17:29:38 +0300 Subject: [PATCH] contract: a colour type, and which run this is Colour is a preference, so it is a setting: hex in, hex out, one spelling, refused if it is not one. A module handed "red" in one place and "#f00" in another guesses wrong somewhere. Image and mask are NOT settings and deliberately have no type. They are data -- tensors the app uploads and the graph wires -- and ComfyUI already has socket types for both. Putting a drawn mask in a JSON settings payload would be inventing a second way to move something that already moves. Runs now have an id. Previews, progress, results and log lines all have to be attributable to one generation, and ComfyUI gives a node no way to ask which run it is in. So FunPack marks its own, every log record carries it, and the history can be filtered to a single run. Readable rather than a uuid, because it is going to be read: "run 7" is findable in a wall of output and a hex string is not. The colour element was caught by the registry-driven rule test before it was finished -- an element with no catalogue demo is neither shown nor attacked, so coverage keeps up with the registry instead of lagging behind it. Co-Authored-By: Claude Opus 5 --- app/catalogue/demos.js | 1 + app/composer/elements/choice.css | 30 ++++++++++++++++++ app/composer/elements/choice.js | 46 +++++++++++++++++++++++++++ app/composer/panel.js | 3 ++ core/contract.py | 7 ++++- core/log.py | 13 ++++++-- core/run.py | 48 +++++++++++++++++++++++++++++ core/schema.py | 23 ++++++++++++++ core/tests/test_log.py | 43 +++++++++++++++++++++++--- modules/_core.py | 6 ++-- modules/sampling/modifiers/nodes.py | 8 +++-- 11 files changed, 214 insertions(+), 14 deletions(-) create mode 100644 core/run.py diff --git a/app/catalogue/demos.js b/app/catalogue/demos.js index 95550a62..47029680 100644 --- a/app/catalogue/demos.js +++ b/app/catalogue/demos.js @@ -63,6 +63,7 @@ export const DEMOS = { { value: "detail", label: "Detail tail" }, ], }, + "color.swatch": { label: "Overlay colour", value: "#f3a93c" }, "radioGroup.default": { label: "Alignment", value: "beat", diff --git a/app/composer/elements/choice.css b/app/composer/elements/choice.css index 6e600cac..905605b6 100644 --- a/app/composer/elements/choice.css +++ b/app/composer/elements/choice.css @@ -95,3 +95,33 @@ .cx-toggle-input:checked + .cx-check-face .cx-toggle-track { background: var(--accent); } .cx-toggle-input:checked + .cx-check-face .cx-toggle-track .cx-toggle-knob { transform: translateX(14px); } .cx-toggle-input:focus-visible + .cx-check-face .cx-toggle-track { box-shadow: var(--focus-ring); } + +/* Colour: the swatch sized to the control height so it lines up with every + other field in a settings column, and the hex field kept narrow because six + characters in a full-width input reads as an empty box. */ +.cx-swatch-row { + display: flex; + align-items: center; + gap: var(--sp-4); +} + +.cx-swatch { + inline-size: var(--ctl-h-lg); + block-size: var(--ctl-h-lg); + padding: 0; + border: var(--bw) solid var(--line); + border-radius: var(--ctl-r); + background: var(--elev-1); + cursor: pointer; +} + +.cx-swatch::-webkit-color-swatch-wrapper { padding: 2px; } +.cx-swatch::-webkit-color-swatch { border: 0; border-radius: var(--r-2); } +.cx-swatch::-moz-color-swatch { border: 0; border-radius: var(--r-2); } + +.cx-swatch:disabled { opacity: var(--disabled-opacity); cursor: default; } + +.cx-swatch-hex { + inline-size: 10ch; + font-family: var(--font-mono); +} diff --git a/app/composer/elements/choice.js b/app/composer/elements/choice.js index e0742a76..ffa4eb79 100644 --- a/app/composer/elements/choice.js +++ b/app/composer/elements/choice.js @@ -223,3 +223,49 @@ define("toggle", "default", ({ label, hint, checked, disabled, onChange } = {}) destroy: () => node.remove(), }; }); + +// A colour, picked or typed. +// +// Two controls over one value because neither is enough alone: the swatch is how +// you choose one you have not named, the field is how you paste the one you were +// given. They stay in step, and the field only commits when what it holds is a +// real colour -- a half-typed "#ff8" must not repaint the app mid-keystroke. +define("color", "swatch", ({ label, value = "#000000", onChange, disabled = false } = {}) => { + const full = (v) => (v.length === 4 + ? "#" + v.slice(1).split("").map((c) => c + c).join("") + : v.slice(0, 7)); + + const swatch = el("input", { cls: ["cx-swatch", "cx-focusable"], + attrs: { type: "color", value: full(value), disabled: disabled || null, + "aria-label": label ? `${label} colour` : "colour" } }); + + const field = el("input", { cls: ["cx-input", "cx-input-md", "cx-swatch-hex", "cx-focusable"], + attrs: { type: "text", value, spellcheck: "false", disabled: disabled || null, + "aria-label": label ? `${label} hex value` : "hex value" } }); + + let current = value; + + const valid = (v) => /^#([0-9a-f]{3}|[0-9a-f]{6}|[0-9a-f]{8})$/i.test(v); + + function commit(next, from) { + if (!valid(next) || next === current) return; + current = next; + if (from !== "swatch") swatch.value = full(next); + if (from !== "field") field.value = next; + if (onChange) onChange(next); + } + + swatch.addEventListener("input", () => commit(swatch.value, "swatch")); + field.addEventListener("change", () => { + if (valid(field.value)) commit(field.value, "field"); + else field.value = current; // reject rather than half-apply + }); + + const node = el("div", { cls: "cx-swatch-row", children: [swatch, field] }); + return { + node, + get value() { return current; }, + setValue: (next) => commit(next, null), + destroy: () => node.remove(), + }; +}); diff --git a/app/composer/panel.js b/app/composer/panel.js index 1dc5cc73..90caa792 100644 --- a/app/composer/panel.js +++ b/app/composer/panel.js @@ -21,6 +21,7 @@ export const DEFAULT_UI = { text: "input", multiline: "textarea", path: "filterList", + color: "swatch", }; // Renderers that carry their own visible label, so wrapping them in a settings @@ -46,6 +47,8 @@ const RENDERERS = { select: (s, v, on) => composer.select.md({ options: s.options, value: v, label: s.label, onChange: on }), segmented: (s, v, on) => composer.segmented.md({ options: s.options, value: v, label: s.label, onChange: on }), radioGroup: (s, v, on) => composer.radioGroup.default({ options: s.options, value: v, label: s.label, onChange: on }), + swatch: (s, v, on) => composer.color.swatch({ label: s.label, value: v, onChange: on }), + filterList: (s, v, on) => composer.filterList.md({ items: (s.options || []).map((o) => ({ id: o.value, label: o.label, hint: o.hint })), value: v, placeholder: s.placeholder, onChange: on }), diff --git a/core/contract.py b/core/contract.py index 7a6a10c3..b9269c62 100644 --- a/core/contract.py +++ b/core/contract.py @@ -14,7 +14,11 @@ CONTRACT_VERSION = 1 # Closed set. A type outside it has no renderer and no validation, so accepting # one would mean rendering something approximate. -TYPES = frozenset({"bool", "int", "float", "enum", "text", "multiline", "path"}) +# A setting is a PREFERENCE. Anything that is data -- an image, a drawn mask, +# a video -- travels as a ComfyUI socket instead, because it is a tensor the app +# uploads and the graph wires, not a value that belongs in a JSON payload. That +# is why there is no "image" or "mask" here despite both being planned features. +TYPES = frozenset({"bool", "int", "float", "enum", "text", "multiline", "path", "color"}) NUMERIC = frozenset({"int", "float"}) @@ -29,6 +33,7 @@ UI_HINTS: Dict[str, List[str]] = { "text": ["input", "search"], "multiline": ["textarea", "autoTextarea"], "path": ["filterList"], + "color": ["swatch"], } # Ordering is coarse-grained by stage, then by declared relations within it. diff --git a/core/log.py b/core/log.py index aed5f406..9347d7a2 100644 --- a/core/log.py +++ b/core/log.py @@ -53,7 +53,11 @@ def line(level: str, source: str, message: str) -> str: def record(level: str, source: str, message: str) -> dict: - entry = {"at": time.time(), "level": level, "source": source, "message": message} + # Imported here rather than at module level: `run` starts a run by clearing + # what this module holds, so a top-level import would be circular. + from . import run as run_mod + entry = {"at": time.time(), "level": level, "source": source, + "message": message, "run": run_mod.current()} _records.append(entry) print(line(level, source, message), file=sys.stderr, flush=True) return entry @@ -98,9 +102,12 @@ def new_run() -> None: _said.clear() -def history(level: Optional[str] = None, limit: int = HISTORY) -> list: +def history(level: Optional[str] = None, limit: int = HISTORY, + run: Optional[str] = None) -> list: """Recent records, newest last. For the app, and for the modules page.""" - items = [r for r in _records if level is None or r["level"] == level] + items = [r for r in _records + if (level is None or r["level"] == level) + and (run is None or r.get("run") == run)] return items[-limit:] diff --git a/core/run.py b/core/run.py new file mode 100644 index 00000000..e25b2490 --- /dev/null +++ b/core/run.py @@ -0,0 +1,48 @@ +"""Which run this is. + +Previews, progress, results and log lines all have to be attributable to one +generation, and ComfyUI gives a node no way to ask "which run am I in" -- there +is no hook for "a run started". So FunPack marks its own: the first FunPack node +to execute in a graph starts a run, and everything after it reports under that +id until the next one begins. + +The id is short and readable rather than a uuid, because it is going to appear in +log lines a person is scanning: `run 7` is findable, a hex string is not. + +Deliberately not a wall-clock timestamp. The name has to be stable to compare +two runs in a report, and identical prompts a second apart must not collide. +""" + +import itertools +import threading +from typing import Optional + +_counter = itertools.count(1) +_lock = threading.Lock() +_current: Optional[str] = None + + +def start() -> str: + """Begin a run, and return its id. + + Also clears anything scoped to a run -- what has been said once, and which + modifiers were dropped -- so the two cannot disagree about when a run began. + """ + global _current + with _lock: + _current = f"run {next(_counter)}" + from . import log + log.new_run() + return _current + + +def current() -> Optional[str]: + """The run in progress, or None before anything has started one.""" + return _current + + +def _reset() -> None: + """Tests only.""" + global _counter, _current + _counter = itertools.count(1) + _current = None diff --git a/core/schema.py b/core/schema.py index 8f3f7cb5..58cbf5b6 100644 --- a/core/schema.py +++ b/core/schema.py @@ -60,6 +60,8 @@ def validate_setting(key: str, spec: Any, siblings: Dict[str, dict]) -> dict: elif kind == "bool": if not isinstance(spec["default"], bool): raise SchemaError(f"{where} is a bool but its default is {spec['default']!r}.") + elif kind == "color": + _validate_colour(spec["default"], where) else: if not isinstance(spec["default"], str): raise SchemaError(f"{where} is a {kind} but its default is not a string.") @@ -92,6 +94,20 @@ def _validate_enum(spec: dict, where: str) -> None: raise SchemaError(f"{where}'s default {spec['default']!r} is not one of its options.") +# Accepted as #rgb, #rrggbb or #rrggbbaa. One spelling in, one spelling out -- +# a module that has to guess whether it was handed "red", "#f00" or "255,0,0" +# will guess wrong somewhere. +_HEX = frozenset("0123456789abcdefABCDEF") + + +def _validate_colour(value, where: str) -> None: + if not isinstance(value, str) or not value.startswith("#"): + raise SchemaError(f"{where} is a colour and must be a hex string like '#ff8800'; got {value!r}.") + digits = value[1:] + if len(digits) not in (3, 6, 8) or any(c not in _HEX for c in digits): + raise SchemaError(f"{where} is not a valid hex colour: {value!r}.") + + def _validate_numeric(spec: dict, where: str, kind: str) -> None: default = spec["default"] if isinstance(default, bool) or not isinstance(default, (int, float)): @@ -233,6 +249,13 @@ def _value_problem(declared: dict, key: str, value: Any, module_id: str): return f"{where} must be one of {allowed}, got {value!r}." return None + if kind == "color": + try: + _validate_colour(value, where) + except SchemaError as exc: + return str(exc) + return None + if kind in NUMERIC: if isinstance(value, bool) or not isinstance(value, (int, float)): return f"{where} is a {kind}, got {value!r}." diff --git a/core/tests/test_log.py b/core/tests/test_log.py index 6fe22831..9e46102f 100644 --- a/core/tests/test_log.py +++ b/core/tests/test_log.py @@ -6,14 +6,16 @@ so it is pinned rather than left to drift line by line. import pytest -from core import log +from core import log, run @pytest.fixture(autouse=True) def clean(): log._reset() + run._reset() yield log._reset() + run._reset() def test_the_shape_is_label_severity_source_message(): @@ -31,9 +33,42 @@ def test_a_message_with_no_source_does_not_print_an_empty_one(): ]) def test_each_level_records_itself(fn, level): fn("somewhere", "something") - assert log.history()[-1] == { - "at": log.history()[-1]["at"], "level": level, - "source": "somewhere", "message": "something"} + entry = log.history()[-1] + assert entry["level"] == level + assert entry["source"] == "somewhere" + assert entry["message"] == "something" + + +def test_a_record_carries_the_run_it_happened_in(): + """So a report can say which generation a line belongs to, and two runs in + one session can be told apart.""" + log.info("before", "no run yet") + assert log.history()[-1]["run"] is None + + first = run.start() + log.info("during", "inside a run") + assert log.history()[-1]["run"] == first + + second = run.start() + log.info("during", "inside the next one") + assert log.history()[-1]["run"] == second + assert first != second + + +def test_history_can_be_filtered_to_one_run(): + first = run.start() + log.info("a", "one") + run.start() + log.info("b", "two") + assert [r["source"] for r in log.history(run=first)] == ["a"] + + +def test_starting_a_run_clears_what_was_said_once(): + run.start() + log.once("k", log.ALERT, "src", "inert") + run.start() + log.once("k", log.ALERT, "src", "inert") + assert len(log.history()) == 2 def test_it_prints_as_well_as_records(capsys): diff --git a/modules/_core.py b/modules/_core.py index 30418ffb..debdf00a 100644 --- a/modules/_core.py +++ b/modules/_core.py @@ -11,8 +11,8 @@ leading underscore keeps it out of the module scan. """ try: # root on sys.path: tests, dev server - from core import contract, log, patching, registry, relations, schema, traits + from core import contract, log, patching, registry, relations, run, schema, traits except ImportError: # inside ComfyUI: a subpackage of the pack - from ..core import contract, log, patching, registry, relations, schema, traits + from ..core import contract, log, patching, registry, relations, run, schema, traits -__all__ = ["contract", "log", "patching", "registry", "relations", "schema", "traits"] +__all__ = ["contract", "log", "patching", "registry", "relations", "run", "schema", "traits"] diff --git a/modules/sampling/modifiers/nodes.py b/modules/sampling/modifiers/nodes.py index 9cecfa28..cf427f55 100644 --- a/modules/sampling/modifiers/nodes.py +++ b/modules/sampling/modifiers/nodes.py @@ -34,7 +34,8 @@ import json from comfy_api.latest import io from ..._core import (log, patching, registry as registry_mod, - relations as relations_mod, schema as schema_mod) + relations as relations_mod, run as run_mod, + schema as schema_mod) CAPABILITY = "modifier" @@ -156,7 +157,7 @@ class FunPackLoadModifiers(io.ComfyNode): # The limit, stated: a graph without this node never marks a run, so # once-per-run reverts to once-per-process there. That is the honest # cost of ComfyUI having no notion of "a run started" a node can hook. - log.new_run() + run = run_mod.start() dropped = patching.Dropped() patched.funpack_dropped = dropped @@ -215,5 +216,6 @@ class FunPackLoadModifiers(io.ComfyNode): for spec, why in rejected: notes.append(f"{spec.id}: {why}") - headline = f"{len(applied)} modifier(s) applied" + (f": {', '.join(applied)}" if applied else "") + headline = (f"{run}: {len(applied)} modifier(s) applied" + + (f": {', '.join(applied)}" if applied else "")) return io.NodeOutput(patched, "\n".join([headline, f"model traits: {', '.join(available) or 'none read'}", *notes]))