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)}