From eca4083837547f4316f4fdcf69747b89dc2418e9 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:35:52 +0300 Subject: [PATCH] core: a dropdown read as a wire, and a legal wire refused ComfyUI spells a combo three ways -- a list where the type goes, the string "COMBO" with the choices under options, and COMFY_DYNAMICCOMBO_V3. Only the first was read, so every V3 node came back with no choices at all and the window said "no files of this kind were found" over dropdowns holding seven entries. A stock install here has 140 of the spelling that was missed. A union type ("IMAGE,MASK") was compared as a whole string, so a MASK feeding it was refused -- a legal wire refused, which is as bad as an illegal one accepted. A MultiType wrapped around a number ("FLOAT,INT" with widgetType) was read as a socket demanding a source for a field you type into. graph.py and widgets.py had their own idea of what a type string means; both now read comfy_types. Co-Authored-By: Claude Opus 5 --- app/shell/tests/widgets.test.js | 87 +++++++++++++++++++ app/shell/widgets.js | 123 ++++++++++++++++++++++++++ core/comfy_types.py | 133 +++++++++++++++++++++++++++++ core/graph.py | 34 ++++++-- core/tests/test_comfy_types.py | 102 ++++++++++++++++++++++ core/tests/test_routes_pipeline.py | 80 +++++++++++++++++ core/tests/test_widgets.py | 81 ++++++++++++++++++ core/widgets.py | 92 ++++++++++++++------ 8 files changed, 701 insertions(+), 31 deletions(-) create mode 100644 app/shell/tests/widgets.test.js create mode 100644 app/shell/widgets.js create mode 100644 core/comfy_types.py create mode 100644 core/tests/test_comfy_types.py diff --git a/app/shell/tests/widgets.test.js b/app/shell/tests/widgets.test.js new file mode 100644 index 00000000..91a11ec6 --- /dev/null +++ b/app/shell/tests/widgets.test.js @@ -0,0 +1,87 @@ +// A node input, turned into something a person can edit. + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { settingFor, label, whyNotEditable } from "../widgets.js"; + +test("an identifier becomes a label a person would read", () => { + assert.equal(label("sampler_name"), "Sampler name"); + assert.equal(label("filename_prefix"), "Filename prefix"); + // Words nobody spells out. "Cfg" and "Vae" read as typos, not as settings. + assert.equal(label("cfg"), "CFG"); + assert.equal(label("vae_name"), "VAE name"); + assert.equal(label("clip_name1"), "CLIP name1"); +}); + +test("each ComfyUI primitive maps to a control that can hold it", () => { + assert.equal(settingFor({ name: "steps", type: "INT", default: 20 }).type, "int"); + assert.equal(settingFor({ name: "cfg", type: "FLOAT", default: 7 }).type, "float"); + assert.equal(settingFor({ name: "on", type: "BOOLEAN", default: true }).type, "bool"); + assert.equal(settingFor({ name: "t", type: "STRING", default: "" }).type, "text"); + assert.equal(settingFor({ name: "t", type: "STRING", multiline: true }).type, "multiline"); +}); + +test("a combo's default is one of its choices", () => { + // ComfyUI leaves the default off a file picker: the first entry is what its + // own frontend shows, and a select rendered with `undefined` reports + // undefined as the value the moment anything reads it. + const setting = settingFor({ name: "ckpt_name", type: "COMBO", choices: ["a.safetensors", "b.safetensors"] }); + assert.equal(setting.default, "a.safetensors"); + assert.deepEqual(setting.options.map((o) => o.value), ["a.safetensors", "b.safetensors"]); +}); + +test("a declared default that is not among the choices does not become the value", () => { + const setting = settingFor({ name: "x", type: "COMBO", choices: ["a", "b"], default: "gone" }); + assert.equal(setting.default, "a"); +}); + +test("an empty combo has no control, and says why rather than showing an empty box", () => { + const widget = { name: "ckpt_name", type: "COMBO", choices: [] }; + assert.equal(settingFor(widget), null); + assert.match(whyNotEditable(widget), /no files of this kind/); +}); + +test("a default of the wrong type does not become NaN in a number box", () => { + // Nothing checks a node's declared default, and a number control handed a + // string reports NaN the moment it is read. + assert.equal(settingFor({ name: "steps", type: "INT", default: "twenty" }).default, 0); + assert.equal(settingFor({ name: "steps", type: "INT", default: 20 }).default, 20); +}); + +test("a type this app has no control for gets no control", () => { + // Not a text box holding "LATENT". An input the app cannot edit has to look + // different from one it can, or the value behind it is a mystery. + assert.equal(settingFor({ name: "samples", type: "LATENT" }), null); + assert.match(whyNotEditable({ name: "samples", type: "LATENT" }), /filled by a wire/); +}); + +test("bounds and tooltips survive the translation", () => { + const setting = settingFor({ + name: "cfg", type: "FLOAT", default: 8, min: 0, max: 100, step: 0.5, + tooltip: "How hard the prompt is enforced.", + }); + assert.equal(setting.min, 0); + assert.equal(setting.max, 100); + assert.equal(setting.step, 0.5); + assert.equal(setting.hint, "How hard the prompt is enforced."); +}); + +test("a combo that takes several choices at once gets no single-select", () => { + // A single-select would save a string where the node wants a list. + const widget = { name: "tags", type: "COMBO", choices: ["a", "b"], multiselect: true }; + assert.equal(settingFor(widget), null); + assert.match(whyNotEditable(widget), /several choices at once/); +}); + +test("a choice that brings settings this window cannot draw says so", () => { + // A dynamic combo's options carry their own inputs. Silently dropping them + // offers an incomplete node as a complete one, and the run fails when it is + // queued over a field nobody was shown. + const setting = settingFor({ + name: "resize_type", type: "COMBO", choices: ["scale dimensions", "longest side"], + reveals_more: true, tooltip: "How the image is resized.", + }); + assert.match(setting.hint, /How the image is resized\./); + assert.match(setting.hint, /not shown here yet/); +}); diff --git a/app/shell/widgets.js b/app/shell/widgets.js new file mode 100644 index 00000000..ea81811b --- /dev/null +++ b/app/shell/widgets.js @@ -0,0 +1,123 @@ +// A node's input, as something a person can edit. +// +// The server describes an input the way ComfyUI declares it -- a type name, a +// list of choices, bounds, a default. The panel renders a SETTING, which is the +// shape a module declares. They are the same idea said twice, so this is the one +// place that translates between them, and every control in the app comes from +// the same renderer table either way. A second table for node inputs would be a +// second answer to "how is a number edited", and the two would drift. +// +// Nothing here names a node. + +const HUMAN = { + cfg: "CFG", + fp16: "FP16", + fp32: "FP32", + bf16: "BF16", + fps: "FPS", + vae: "VAE", + clip: "CLIP", + lora: "LoRA", + ckpt: "Checkpoint", + id: "ID", +}; + +/** + * `sampler_name` -> "Sampler name". An identifier is what a node author writes + * for code; a label is what someone reads while choosing a value, and printing + * the identifier is how a form ends up looking like a debug dump. + */ +export function label(name) { + const words = String(name).split(/[_\s]+/).filter(Boolean); + if (!words.length) return String(name); + return words + .map((word, i) => { + const known = HUMAN[word.toLowerCase()]; + if (known) return known; + if (i > 0) return word.toLowerCase(); + return word.charAt(0).toUpperCase() + word.slice(1); + }) + .join(" "); +} + +/** + * settingFor(widget) -> a declaration `panel.js` can render, or null. + * + * Null when there is nothing sane to draw -- a combo whose list is empty, a + * type this app has no control for. Not a disabled box and not a guess: an + * input the app cannot edit has to look different from one it can, or the value + * behind it is a mystery nobody notices. + */ +export function settingFor(widget) { + if (!widget || typeof widget !== "object") return null; + const common = { + label: label(widget.name), + hint: widget.tooltip || undefined, + }; + + switch (widget.type) { + case "COMBO": { + // Several choices at once. There is no control here that holds a list, and + // a single-select saves a string where the node wants an array. + if (widget.multiselect) return null; + const choices = Array.isArray(widget.choices) ? widget.choices : []; + // An empty list is a real state -- a file picker on a machine with no + // models -- and a select holding nothing offers no way to tell that from + // a control that failed to load. + if (!choices.length) return null; + const value = choices.includes(widget.default) ? widget.default : choices[0]; + return { + ...common, + type: "enum", + default: value, + options: choices.map((c) => ({ value: c, label: String(c) })), + // Each choice on a dynamic combo can bring inputs of its own, and this + // window does not draw those yet. Said rather than dropped: offering an + // incomplete node as a complete one is how a run fails at the point it + // is queued, over a field nobody was shown. + hint: widget.reveals_more + ? [common.hint, "This choice brings further settings that are not shown here yet."] + .filter(Boolean).join(" ") + : common.hint, + }; + } + case "BOOLEAN": + return { ...common, type: "bool", default: Boolean(widget.default) }; + case "INT": + return { + ...common, type: "int", default: numberOr(widget.default, 0), + min: widget.min, max: widget.max, step: widget.step ?? 1, + }; + case "FLOAT": + return { + ...common, type: "float", default: numberOr(widget.default, 0), + min: widget.min, max: widget.max, step: widget.step ?? 0.01, + precision: widget.precision, + }; + case "STRING": + return { + ...common, + type: widget.multiline ? "multiline" : "text", + default: widget.default == null ? "" : String(widget.default), + placeholder: widget.placeholder, + }; + default: + return null; + } +} + +// A node may declare a default of the wrong type -- ComfyUI does not check -- +// and a number control handed a string reports NaN the moment it is read. +function numberOr(value, fallback) { + const n = Number(value); + return Number.isFinite(n) ? n : fallback; +} + +/** Why an input has no control, in words that fit under the input's name. */ +export function whyNotEditable(widget) { + if (widget.type === "COMBO") { + if (widget.multiselect) return "several choices at once, which has no control here yet"; + return "nothing to choose from — no files of this kind were found"; + } + return `${widget.type} is filled by a wire, not typed`; +} diff --git a/core/comfy_types.py b/core/comfy_types.py new file mode 100644 index 00000000..1b3c3d03 --- /dev/null +++ b/core/comfy_types.py @@ -0,0 +1,133 @@ +"""How ComfyUI spells a type, in one place. + +Two files here ask questions about the same declaration -- `graph.py` asks +whether a wire is legal, `widgets.py` asks whether a person can type into it -- +and both had their own idea of what a type string means. That is the drift this +codebase keeps warning about, and it had already happened: a combo was +"a list where the type goes" in one and "the string COMBO" in the other, and +between them they missed the three shapes below. + +The shapes, all current, all present in a stock install: + +* **A combo** is a dropdown. Spelled as a list in the old schema + (`(["a", "b"], {...})`), as the string `"COMBO"` with the choices under + `options` in V3, and as `"COMFY_DYNAMICCOMBO_V3"` -- 140 inputs in a stock + install -- where each option may itself be a dict carrying further inputs. + Anything with COMBO in its type name is a widget, never a wire. +* **A union** is a comma-joined type string: `"IMAGE,MASK"` takes either. An + exact string comparison refuses a MASK feeding it, which is a legal wire + refused -- as bad a failure as an illegal one accepted. +* **A MultiType wrapped around a widget** carries `widgetType` in its options + and lists the widget's own type first: `("FLOAT,INT", {"widgetType": "FLOAT"})` + is a number you type, not a socket. Read as a socket it demands a source for a + field nobody needs to wire. + +Nothing here names a node. +""" + +from typing import Any, Dict, List, Optional, Tuple + +COMBO = "COMBO" + +# What a person types INTO. Everything else arrives on a wire. +PRIMITIVE = frozenset({"STRING", "INT", "FLOAT", "BOOLEAN", COMBO}) + + +def declared(spec_entry: Any) -> Tuple[Any, Dict[str, Any]]: + """(type, options) out of whichever shape a declaration arrived in.""" + if isinstance(spec_entry, (list, tuple)) and spec_entry: + kind = spec_entry[0] + options = spec_entry[1] if len(spec_entry) > 1 and isinstance(spec_entry[1], dict) else {} + return kind, options + return spec_entry, {} + + +def is_combo(kind: Any) -> bool: + """Any dropdown: a list of choices, "COMBO", or a V3 dynamic combo.""" + if isinstance(kind, (list, tuple)): + return True + return isinstance(kind, str) and COMBO in kind.upper() + + +def members(kind: Any) -> List[str]: + """The member types of a type string. A union names several.""" + if not isinstance(kind, str): + return [] + return [part.strip() for part in kind.split(",") if part.strip()] + + +def widget_type(kind: Any, options: Optional[dict] = None) -> Optional[str]: + """The type a person edits this as, or None if it is a socket.""" + options = options or {} + if is_combo(kind): + return COMBO + # The node author saying so outright. A MultiType wrapped around a widget + # input is a widget in ComfyUI's own frontend, and this is how it says so. + declared_widget = options.get("widgetType") + if isinstance(declared_widget, str): + if is_combo(declared_widget): + return COMBO + if declared_widget in PRIMITIVE: + return declared_widget + parts = members(kind) + if parts and all(part in PRIMITIVE for part in parts): + return parts[0] + return None + + +def is_widget(kind: Any, options: Optional[dict] = None) -> bool: + return widget_type(kind, options) is not None + + +def choices(options: Optional[dict]) -> List[Any]: + """A combo's choices, whichever way they were written. + + A dynamic combo's option is a dict whose `key` may be an Enum member, and + `str()` on one of those is "ResizeType.SCALE_DIMENSIONS" -- a value ComfyUI + would then refuse, because what it wants is the enum's value. + """ + options = options or {} + raw = options.get("options") + if raw is None: + raw = options.get("choices") + found = [] + for option in raw or []: + if isinstance(option, dict): + for key in ("key", "value", "content", "name", "label"): + if key in option: + found.append(_plain(option[key])) + break + else: + found.append(_plain(option)) + return found + + +def reveals(options: Optional[dict]) -> bool: + """Whether picking a choice brings inputs with it. + + A dynamic combo's options can each carry their own `inputs`, so the form + changes with the choice. Nothing here renders those yet, and a window that + dropped them silently would be offering an incomplete node as a complete + one. + """ + options = options or {} + raw = options.get("options") or [] + return any(isinstance(option, dict) and option.get("inputs") for option in raw) + + +def accepts(wanted: Any, given: Any) -> bool: + """Whether an output of type `given` may feed an input of type `wanted`.""" + if wanted == "*" or given == "*": + return True + left, right = set(members(wanted)), set(members(given)) + if not left or not right: + return wanted == given + return bool(left & right) + + +def _plain(value: Any) -> Any: + """An Enum member as the value ComfyUI wants, anything else untouched.""" + inner = getattr(value, "value", None) + if inner is not None and not isinstance(value, (str, int, float, bool)): + return inner + return value diff --git a/core/graph.py b/core/graph.py index 7f13c2ec..593717c8 100644 --- a/core/graph.py +++ b/core/graph.py @@ -25,6 +25,8 @@ the user's word for it was "terrifying". from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple +from . import comfy_types + Link = list # [slot_id, output_index] @@ -73,10 +75,12 @@ def from_comfyui() -> Schemas: inputs, required = {}, [] for section in ("required", "optional"): for name, declared in (spec.get(section) or {}).items(): - kind = declared[0] if isinstance(declared, (list, tuple)) else declared - # A list of choices IS the type for a combo; what matters here is - # only that it is not a socket, so it cannot be wired. - inputs[name] = kind if isinstance(kind, str) else "COMBO" + kind, options = comfy_types.declared(declared) + # The type as ComfyUI would WIRE it. A list of choices is a + # combo; so is the string "COMBO" and every V3 dynamic combo, + # and none of them can be fed by a wire. + edited = comfy_types.widget_type(kind, options) + inputs[name] = edited if edited else kind if section == "required": # Kept, because flattening the two sections loses the only # thing that says a slot is incomplete -- and a slot missing @@ -116,6 +120,17 @@ def shape_problems(slots: Any) -> List[str]: inputs = slot.get("inputs") if inputs is not None and not isinstance(inputs, dict): problems.append(f"{where}'s inputs must be an object, not {type(inputs).__name__}") + # Core does not know what a group MEANS -- it is the pipeline's own way + # of arranging itself, and any name is a valid one. It does know a name + # is text: a number here becomes a card titled `5`, and two slots + # grouped under `5` and `"5"` land in two cards reading the same. + group = slot.get("group") + if group is not None: + if not isinstance(group, str): + problems.append(f"{where}'s group must be a name, not {type(group).__name__}") + elif not group.strip(): + problems.append(f"{where}'s group is blank; leave it out to leave the " + f"slot ungrouped") return problems @@ -175,7 +190,11 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic problems.append(f"{slot_id}.{name} reads output {index} of {source!r}, " f"which has {len(produced)}") continue - if produced[index] != declared[name] and declared[name] != "*": + # A union on either side, not an exact string. `"IMAGE,MASK"` + # takes either, and comparing the whole string refused a MASK + # feeding it -- a legal wire refused, which is as bad a failure + # as an illegal one accepted, and harder to argue with. + if not comfy_types.accepts(declared[name], produced[index]): problems.append(f"{slot_id}.{name} wants {declared[name]} but {source!r} " f"gives {produced[index]}") continue @@ -283,7 +302,7 @@ def replace(slots: Sequence[dict], slot_id: str, class_type: str, if index >= len(now): problems.append(f"{consumer}.{name} reads output {index}, which {class_type} " f"does not have") - elif wanted is not None and now[index] != wanted: + elif wanted is not None and not comfy_types.accepts(wanted, now[index]): problems.append(f"{consumer}.{name} needs {wanted} but {class_type} gives " f"{now[index]} there") if problems: @@ -326,7 +345,8 @@ def remove(slots: Sequence[dict], slot_id: str, continue kind = produced[index] matching = [name for name, wants in declared.items() - if wants == kind and name in wired and is_link(wired[name])] + if comfy_types.accepts(wants, kind) + and name in wired and is_link(wired[name])] if len(matching) == 1: replacement[index] = wired[matching[0]] elif not matching: diff --git a/core/tests/test_comfy_types.py b/core/tests/test_comfy_types.py new file mode 100644 index 00000000..eaaf03e0 --- /dev/null +++ b/core/tests/test_comfy_types.py @@ -0,0 +1,102 @@ +"""How ComfyUI spells a type. + +Every declaration below was copied from a real registry, not written from +memory. They came out of a survey run after an old bug report said a node +"showed only its LoRA selection": on this machine a stock install has 140 +dynamic combos and 23 union-typed inputs, and neither shape was being read. + +The registry-wide sweep lives in test_routes_pipeline.py, which is the one place +that loads ComfyUI's extras -- these are the shapes themselves, tested where they +cost nothing to run. +""" + +import enum + +from core import comfy_types as t + + +class ResizeType(enum.Enum): + SCALE_DIMENSIONS = "scale dimensions" + LONGEST_SIDE = "longest side" + + +# Verbatim shapes, as INPUT_TYPES() hands them over. +OLD_COMBO = (["euler", "dpmpp_2m"], {"default": "euler"}) +V3_COMBO = ("COMBO", {"default": "default", "multiselect": False, + "options": ["default", "fp16", "bf16"]}) +DYNAMIC_COMBO = ("COMFY_DYNAMICCOMBO_V3", {"options": [ + {"key": ResizeType.SCALE_DIMENSIONS, + "inputs": {"required": {"width": ("INT", {"default": 512})}}}, + {"key": ResizeType.LONGEST_SIDE}, +]}) +MULTITYPE_WIDGET = ("FLOAT,INT", {"widgetType": "FLOAT", "default": 25}) +MULTITYPE_SOCKET = ("SAM3_TRACK_DATA,MASK", {}) +PLAIN_SOCKET = ("MODEL", {}) + + +def widget_of(entry): + kind, options = t.declared(entry) + return t.widget_type(kind, options) + + +# --- what is a widget ------------------------------------------------------ + +def test_all_three_spellings_of_a_combo_are_a_combo(): + assert widget_of(OLD_COMBO) == t.COMBO + assert widget_of(V3_COMBO) == t.COMBO + # The one that was missed. Matching the literal "COMBO" left 140 inputs + # classified as wires, so their parameters pane offered nothing to edit and + # claimed they were fed by something. + assert widget_of(DYNAMIC_COMBO) == t.COMBO + + +def test_a_multitype_wrapped_around_a_number_is_a_number(): + """Read as a socket it demands a source for a field nobody wires.""" + assert widget_of(MULTITYPE_WIDGET) == "FLOAT" + + +def test_a_union_of_socket_types_stays_a_socket(): + assert widget_of(MULTITYPE_SOCKET) is None + assert widget_of(PLAIN_SOCKET) is None + + +# --- what a combo offers --------------------------------------------------- + +def test_choices_are_read_from_wherever_they_were_written(): + assert t.choices(t.declared(V3_COMBO)[1]) == ["default", "fp16", "bf16"] + assert t.choices({"choices": ["a", "b"]}) == ["a", "b"] + assert t.choices({}) == [] + + +def test_an_enum_choice_becomes_the_value_comfyui_wants(): + """str() on an Enum member is "ResizeType.SCALE_DIMENSIONS", which ComfyUI + refuses -- what it wants is the value behind it.""" + assert t.choices(t.declared(DYNAMIC_COMBO)[1]) == ["scale dimensions", "longest side"] + + +def test_a_choice_that_brings_more_inputs_is_flagged(): + """Nothing renders those inputs yet, and dropping them silently offers an + incomplete node as a complete one.""" + assert t.reveals(t.declared(DYNAMIC_COMBO)[1]) is True + assert t.reveals(t.declared(V3_COMBO)[1]) is False + + +# --- what may feed what ---------------------------------------------------- + +def test_a_union_accepts_any_of_its_members(): + """An exact string comparison refused a MASK feeding "IMAGE,MASK" -- a legal + wire refused, which is as bad a failure as an illegal one accepted.""" + assert t.accepts("IMAGE,MASK", "MASK") + assert t.accepts("IMAGE,MASK", "IMAGE") + assert t.accepts("MASK", "IMAGE,MASK") + assert t.accepts("IMAGE,MASK", "SAM3_TRACK_DATA,MASK") + + +def test_a_union_still_refuses_a_type_it_does_not_name(): + assert not t.accepts("IMAGE,MASK", "MODEL") + assert not t.accepts("MODEL", "CLIP") + + +def test_the_wildcard_takes_anything_either_way(): + assert t.accepts("*", "MODEL") + assert t.accepts("MODEL", "*") diff --git a/core/tests/test_routes_pipeline.py b/core/tests/test_routes_pipeline.py index 5e808f52..535faea8 100644 --- a/core/tests/test_routes_pipeline.py +++ b/core/tests/test_routes_pipeline.py @@ -211,6 +211,11 @@ def test_removing_slots_one_by_one_never_resurrects_the_default(server): ({"action": "replace", "slot": ["a"], "node": "X", "slots": []}, "named by a string"), ({"action": "remove", "slot": {"x": 1}, "slots": []}, "named by a string"), ({"action": "replace", "slot": "a", "node": ["X"], "slots": []}, "named by a string"), + # A group is the pipeline's own arrangement and core knows nothing about + # what the names mean -- only that they are names. A number here becomes a + # card titled `5`, and `5` and `"5"` become two cards reading the same. + ({"action": "check", "slots": [{"id": "a", "node": "X", "group": 5}]}, "must be a name"), + ({"action": "check", "slots": [{"id": "a", "node": "X", "group": " "}]}, "is blank"), ]) def test_a_malformed_pipeline_comes_back_as_a_reason_not_a_500(server, payload, expected): """A payload arrives from HTTP, so "it is a list" is all that has been @@ -222,3 +227,78 @@ def test_a_malformed_pipeline_comes_back_as_a_reason_not_a_500(server, payload, reasons = body.get("refused", []) + body.get("problems", []) assert any(expected in r for r in reasons), reasons assert body.get("queueable") is False + + +def test_the_default_pipeline_arrives_already_arranged(server): + """The window draws a card per group, so a pipeline with no groups is one + card holding everything -- which is the arrangement it exists to avoid.""" + _status, body = _request(server, "GET", "/funpack/api/pipeline") + groups = [slot.get("group") for slot in body["slots"]] + assert all(groups), f"slots with no group: {[s['id'] for s in body['slots'] if not s.get('group')]}" + assert len(set(groups)) > 1, "everything landed in one group" + + +def test_a_group_survives_an_edit(server): + """Replacing a node keeps its arrangement. `replace` deliberately drops the + slot's inputs -- a different node has different inputs -- and dropping the + group with them would scatter the pipeline every time a node was swapped.""" + _status, before = _request(server, "GET", "/funpack/api/pipeline") + was = {slot["id"]: slot.get("group") for slot in before["slots"]} + + _status, after = _request( + server, "POST", "/funpack/api/pipeline", + {"action": "replace", "slot": "save", "node": "PreviewImage", + "slots": before["slots"]}) + assert not after["refused"], after["refused"] + now = {slot["id"]: slot.get("group") for slot in after["slots"]} + assert now == was + + +# --- the whole registry, which is the only place the odd shapes live -------- +# +# This is the one test file that loads ComfyUI's extras, so it is the only place +# a sweep like this means anything. These were written after a v4 bug report +# about a node "showing only its LoRA selection"; the shapes themselves are +# covered in test_comfy_types.py. + +def test_no_installed_node_is_described_with_a_type_nothing_can_edit(registered): + """A widget's type is what decides which control is drawn. A union like + "FLOAT,INT" reaching the form asks for a control for a type that does not + exist, and the whole node then renders as nothing.""" + from core import widgets + + for class_type in registered.NODE_CLASS_MAPPINGS: + described = widgets.describe(class_type) + if not described: + continue + for widget in described["widgets"]: + assert "," not in widget["type"], f"{class_type}.{widget['name']} is {widget['type']}" + assert widget["type"] in widgets.PRIMITIVE, ( + f"{class_type}.{widget['name']} is {widget['type']}, which has no control") + + +def test_every_dropdown_in_the_registry_is_offered_as_one(registered): + """A dropdown classified as a socket is an input the user cannot set and the + window claims is wired. + + The check is spelled out here rather than asked of `comfy_types.is_combo`. + It was written that way first, and reintroducing the bug did not fail this + test: the sweep was asking the broken function whether the function was + broken, and got the answer the bug implies. A test that consults the code + under test for its own verdict cannot fail. + """ + from core import comfy_types, widgets + + dropdowns = 0 + for class_type in registered.NODE_CLASS_MAPPINGS: + described = widgets.describe(class_type) + if not described: + continue + for socket in described["sockets"]: + assert "COMBO" not in str(socket["type"]).upper(), ( + f"{class_type}.{socket['name']} is a dropdown ({socket['type']}) " + f"reported as a wire") + dropdowns += sum(1 for w in described["widgets"] + if w["type"] == comfy_types.COMBO) + + assert dropdowns > 100, f"only {dropdowns} dropdowns found; the sweep is not seeing them" diff --git a/core/tests/test_widgets.py b/core/tests/test_widgets.py index e3373d15..01deb42d 100644 --- a/core/tests/test_widgets.py +++ b/core/tests/test_widgets.py @@ -122,3 +122,84 @@ def test_a_node_whose_declaration_raises_is_absent_not_fatal(monkeypatch): monkeypatch.setitem(comfy_nodes.NODE_CLASS_MAPPINGS, "Explodes", Explodes) assert widgets.describe("Explodes") is None + + +# --- searching for a node to put in a slot --------------------------------- + +def test_searching_finds_a_node_by_its_class_name(): + found = widgets.search("KSampler") + names = [entry["node"] for entry in found["nodes"]] + assert "KSampler" in names + # Exact first. Typing the whole name and being handed KSamplerAdvanced is + # the picker being unhelpful about the one thing it was told. + assert names[0] == "KSampler" + + +def test_searching_finds_a_node_by_its_category(): + found = widgets.search("latent") + assert found["nodes"], "nothing matched a category every install has" + assert all("latent" in f"{e['node']}{e['title']}{e['category']}".lower() + for e in found["nodes"]) + + +def test_a_search_says_how_many_it_did_not_show(): + """A picker that shows forty of four hundred and says nothing implies forty + is all there is.""" + everything = widgets.search("", limit=10) + assert len(everything["nodes"]) == 10 + assert everything["total"] > 10 + + +def test_nothing_matching_is_an_empty_answer_rather_than_everything(): + found = widgets.search("zzz-no-node-is-called-this") + assert found == {"nodes": [], "total": 0} + + +def test_a_found_node_carries_what_a_picker_shows(): + entry = widgets.search("KSampler")["nodes"][0] + assert set(entry) == {"node", "title", "category", "outputs"} + assert entry["outputs"] == ["LATENT"] + + +def test_funpacks_own_nodes_are_findable_too(): + """The registry is one registry: a picker that could not offer FunPack's own + nodes would make the built-in pipeline unrepairable from the window.""" + names = [e["node"] for e in widgets.search("FunPack", limit=200)["nodes"]] + assert "FunPackSampler" in names + + +# --- the two spellings of a combo ------------------------------------------ + +def test_a_v3_combo_carries_its_choices(): + """ComfyUI spells a combo two ways and both are current. + + The old one puts the choices where the type goes -- (["a", "b"], {...}) -- + and the V3 schema puts the string "COMBO" there with the choices under + `options`. Reading only the first found no choices on ANY V3 node, and the + window said "no files of this kind were found" over dropdowns that had + seven entries. It was believable, which is what made it bad. + """ + described = widgets.describe("FunPackDiffusionModelLoader") + weights = next(w for w in described["widgets"] if w["name"] == "weight_dtype") + assert "fp16" in weights["choices"] + assert weights["default"] == "default" + + +def test_an_old_style_combo_still_carries_its_choices(): + described = widgets.describe("KSampler") + scheduler = next(w for w in described["widgets"] if w["name"] == "scheduler") + assert "normal" in scheduler["choices"] + + +def test_every_combo_in_the_default_pipeline_offers_something_or_is_genuinely_empty(): + """A combo with no choices is a real state -- a file picker on a machine + with no files -- so this asserts the ones that cannot be empty are not.""" + from modules.system.pipeline import DEFAULT + for slot in DEFAULT: + for widget in widgets.describe(slot["node"])["widgets"]: + if widget["type"] != widgets.COMBO: + continue + assert "choices" in widget, f"{slot['id']}.{widget['name']} has no choices key" + if "name" in widget["name"] or widget["name"].endswith("_name"): + continue # a file picker; may be empty + assert widget["choices"], f"{slot['id']}.{widget['name']} offers nothing" diff --git a/core/widgets.py b/core/widgets.py index ae2cfa2e..ab697cae 100644 --- a/core/widgets.py +++ b/core/widgets.py @@ -4,7 +4,8 @@ TYPE and nothing else. Editing a value needs the rest -- the choices a combo offers, the bounds on a number, the default, whether the box is multiline -- and that is a different question about the same declaration, so it is asked here -rather than widening the one graph.py asks. +rather than widening the one graph.py asks. What both of them need to know about +the SPELLING of a type is in `comfy_types`, so they cannot disagree about it. Read from ComfyUI's own INPUT_TYPES, the same source its own frontend uses, so a node written for ComfyUI is editable here without knowing anything about @@ -18,17 +19,10 @@ sockets, rather than made editable. from typing import Any, Dict, List, Optional -# Declared by a list of choices rather than a type name: the list IS the type. -COMBO = "COMBO" +from . import comfy_types -# The types a person types INTO. Everything else is filled by a wire. -# -# A short list core holds, deliberately: these five are ComfyUI's primitives and -# the same set its own frontend edits, so a node written for ComfyUI is editable -# here. Deciding by case instead ("socket types are upper case") looked -# type-agnostic and was simply wrong -- STRING is upper case, and every prompt -# box in the pipeline came back as a socket nobody could type in. -PRIMITIVE = frozenset({"STRING", "INT", "FLOAT", "BOOLEAN", COMBO}) +COMBO = comfy_types.COMBO +PRIMITIVE = comfy_types.PRIMITIVE # What a widget's options dict may say that is worth carrying to a form. Named # rather than passed through wholesale: an options dict can hold anything a node @@ -36,24 +30,30 @@ PRIMITIVE = frozenset({"STRING", "INT", "FLOAT", "BOOLEAN", COMBO}) # business rather than a contract. KEPT = ("default", "min", "max", "step", "round", "multiline", "dynamicPrompts", "tooltip", "placeholder", "control_after_generate", "image_upload", - "label_on", "label_off", "precision", "forceInput") + "label_on", "label_off", "precision", "forceInput", "multiselect") def _one(name: str, declared: Any, required: bool) -> Dict[str, Any]: - kind = declared[0] if isinstance(declared, (list, tuple)) and declared else declared - options = {} - if isinstance(declared, (list, tuple)) and len(declared) > 1 and isinstance(declared[1], dict): - options = declared[1] + kind, options = comfy_types.declared(declared) widget: Dict[str, Any] = {"name": name, "required": required} - if isinstance(kind, str): - widget["type"] = kind - else: - # A list of choices. It may be empty -- a file picker on a machine with - # no files -- and that is worth saying rather than rendering a select - # with nothing in it and no explanation. - widget["type"] = COMBO - widget["choices"] = list(kind) if isinstance(kind, (list, tuple)) else [] + editable = comfy_types.widget_type(kind, options) + # The type as it is EDITED, not as it is declared. A MultiType wrapped + # around a number arrives as "FLOAT,INT" and is a box you type into; read + # literally it becomes a socket demanding a source for a field nobody wires. + widget["type"] = editable if editable else (kind if isinstance(kind, str) else COMBO) + + if widget["type"] == COMBO: + # A combo may be empty -- a file picker on a machine with no files -- + # and that is worth saying rather than rendering a select with nothing + # in it and no explanation. + widget["choices"] = (list(kind) if isinstance(kind, (list, tuple)) + else comfy_types.choices(options)) + if comfy_types.reveals(options): + # Each choice carries its own further inputs. Nothing renders those + # yet, and a form that dropped them silently would offer an + # incomplete node as a complete one. + widget["reveals_more"] = True for key in KEPT: if key in options: @@ -128,3 +128,47 @@ def describe_all(class_types) -> Dict[str, Optional[dict]]: other as a bug in the request. """ return {name: describe(name) for name in dict.fromkeys(class_types)} + + +def search(query: str, limit: int = 40) -> Dict[str, Any]: + """Installed nodes whose name, title or category matches, for a picker. + + Names only -- not their inputs. Describing every installed node is what + ComfyUI's own /object_info does, and on a machine with a few packs that is + megabytes to answer "what could go in this slot"; the one the user picks is + then described on its own. + + `total` is the count BEFORE the cut, so a picker can say the list is not all + of it rather than implying forty is everything there is. + + Ranked so an exact name beats a name that merely contains the query: typing + "KSampler" and getting KSamplerAdvanced first is the picker being unhelpful + about the thing it was told. + """ + try: + import nodes as comfy_nodes + except Exception: # noqa: BLE001 -- not inside ComfyUI + return {"nodes": [], "total": 0} + + needle = (query or "").strip().lower() + found = [] + for class_type, node in comfy_nodes.NODE_CLASS_MAPPINGS.items(): + title = _display_name(class_type, node) + category = getattr(node, "CATEGORY", "") or "" + if needle and needle not in f"{class_type}\n{title}\n{category}".lower(): + continue + lowered = class_type.lower() + rank = (0 if lowered == needle + else 1 if lowered.startswith(needle) + else 2 if needle in lowered + else 3) + found.append((rank, lowered, { + "node": class_type, + "title": title, + "category": category, + "outputs": list(getattr(node, "RETURN_TYPES", ()) or ()), + })) + + found.sort(key=lambda entry: entry[:2]) + return {"nodes": [entry[2] for entry in found[:max(0, limit)]], + "total": len(found)}