From 65afe054f098320d2e078c144564f68765c67287 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 06:47:32 +0000 Subject: [PATCH 01/16] feat(workflow): add UI-to-API workflow converter Adds comfy_cli/workflow_to_api.py: a pure-Python converter that takes a UI-format (litegraph) workflow and the running server's /object_info response and produces an API-format ("prompt") workflow ready for execution. Translates the JSON-in/JSON-out algorithm of Seth A. Robinson's /workflow/convert custom node (Unlicense) to consume the HTTP /object_info schema instead of importing ComfyUI's nodes module. Conversion covers: * Recursive subgraph expansion with link ID remapping * PrimitiveNode value inlining * Reroute / GetNode / SetNode pass-through tracing * Mode 4 (bypass) passthrough with strict isValidConnection matching and a first-linked-input fallback to match the reference converter * Mode 2 (mute) exclusion plus orphan link cleanup (the reference leaves a dangling reference here; the executor would reject it) * LoadImageOutput / Note exclusion * OUTPUT_NODE preservation for sinks without downstream consumers * Combo value case-insensitive normalization * Schema-driven default value population * control_after_generate filtering before widget mapping * COMFY_DYNAMICCOMBO_V3 sub-input expansion * rgthree-style lora dict widget absorption * List widget value wrapping in {__value__: [...]} to disambiguate from [node_id, slot] link references * Warning when legacy 'group nodes' (workflow> prefix) are encountered Hardened against malformed input: non-dict node/input/output entries, non-numeric link slots, garbage in definitions etc. are skipped with a warning rather than crashing. Per-node emission is wrapped so a bug in one node cannot torpedo the whole conversion. Test coverage (62 tests): * Format detection, UUID detection * Each special-case node type * Schema-aware widget mapping, defaults, combo normalization * Subgraph expansion * Frontend parity behaviors (list wrap, orphan cleanup, isValidConnection) * Malformed input hardening * Fixture-based regression against a real SD 1.5 workflow + a trimmed /object_info, asserting byte-identical output to the reference /workflow/convert response Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 1101 +++++++++++++++++ .../comfy_cli/fixtures/sd15_expected_api.json | 107 ++ .../comfy_cli/fixtures/sd15_object_info.json | 467 +++++++ .../comfy_cli/fixtures/sd15_ui_workflow.json | 547 ++++++++ tests/comfy_cli/test_workflow_to_api.py | 947 ++++++++++++++ 5 files changed, 3169 insertions(+) create mode 100644 comfy_cli/workflow_to_api.py create mode 100644 tests/comfy_cli/fixtures/sd15_expected_api.json create mode 100644 tests/comfy_cli/fixtures/sd15_object_info.json create mode 100644 tests/comfy_cli/fixtures/sd15_ui_workflow.json create mode 100644 tests/comfy_cli/test_workflow_to_api.py diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py new file mode 100644 index 00000000..60fcf58f --- /dev/null +++ b/comfy_cli/workflow_to_api.py @@ -0,0 +1,1101 @@ +"""Convert ComfyUI UI-format workflows to API ("prompt") format. + +The UI format is what the ComfyUI frontend saves by default — a litegraph dump +with `nodes` and `links` arrays. The API format is the flat +``{node_id: {class_type, inputs, _meta}}`` shape that the server's ``/prompt`` +endpoint accepts. + +The conversion needs schema information about each node type (which inputs are +widgets vs connections, what their order is, defaults, combo options, etc.). +That information is available from the running server's ``/object_info`` +endpoint — the same data the frontend uses to render the graph editor. + +This module is a Python port of Seth A. Robinson's +``comfyui-workflow-to-api-converter-endpoint`` (Unlicense), restructured to +take a fetched ``object_info`` dict instead of importing ComfyUI's in-process +``nodes`` module. +""" + +from __future__ import annotations + +import copy +import logging +from typing import Any + +logger = logging.getLogger(__name__) + + +# Mode values from litegraph: see frontend's LGraphEventMode enum. +_MODE_MUTED = 2 # excluded from execution; outputs not produced +_MODE_BYPASS = 4 # node skipped; inputs passed through to outputs + +# Node types that exist only in the UI graph and never appear in API output. +_UI_ONLY_NODE_TYPES = frozenset({"Note", "PrimitiveNode", "GetNode", "SetNode", "Reroute"}) + +# Sentinel IDs litegraph uses inside a subgraph definition for the synthetic +# input and output proxy nodes (the boxes the user wires through). +_SUBGRAPH_INPUT_NODE_ID = -10 +_SUBGRAPH_OUTPUT_NODE_ID = -20 + +# Cap on recursive subgraph / passthrough resolution to defend against cycles +# in malformed inputs. +_MAX_RESOLUTION_DEPTH = 100 +_MAX_SUBGRAPH_ITERATIONS = 10 + +# Strings that ComfyUI appends after seed-like INT widgets to control how the +# value changes between runs. They're not real inputs and must be stripped from +# the widget-value list before mapping to input names. +_CONTROL_AFTER_GENERATE_VALUES = frozenset({"fixed", "increment", "decrement", "randomize"}) + + +class WorkflowConversionError(Exception): + """Raised when a workflow can't be converted to API format.""" + + +def is_api_format(workflow: Any) -> bool: + """Return True if ``workflow`` already looks like an API-format prompt.""" + if not isinstance(workflow, dict): + return False + if "nodes" in workflow and "links" in workflow: + return False + for key, value in workflow.items(): + if key in ("prompt", "extra_data", "client_id"): + continue + if isinstance(value, dict) and "class_type" in value: + return True + return False + + +def is_subgraph_uuid(node_type: Any) -> bool: + """A subgraph instance's node ``type`` field is the UUID of a subgraph def.""" + if not isinstance(node_type, str) or len(node_type) != 36: + return False + parts = node_type.split("-") + if len(parts) != 5: + return False + return tuple(len(p) for p in parts) == (8, 4, 4, 4, 12) + + +def convert_ui_to_api(workflow: dict, object_info: dict) -> dict: + """Convert a UI-format workflow to API format. + + Args: + workflow: UI workflow with ``nodes`` and ``links`` keys. + object_info: ``/object_info`` response: ``{node_type: schema}``. + + Returns: + API-format dict: ``{node_id_str: {class_type, inputs, _meta}}``. + """ + if is_api_format(workflow): + return workflow + if not isinstance(workflow, dict): + raise WorkflowConversionError("Workflow must be a JSON object") + if not isinstance(workflow.get("nodes"), list) or not isinstance(workflow.get("links"), list): + raise WorkflowConversionError("Workflow is missing 'nodes' or 'links' list") + if not isinstance(object_info, dict): + raise WorkflowConversionError("object_info must be a JSON object") + + workflow = copy.deepcopy(workflow) + # Discard any non-dict entries up front so the rest of the pipeline doesn't + # have to defend against malformed nodes inside the list. + nodes = [n for n in workflow["nodes"] if isinstance(n, dict)] + links = list(workflow["links"]) + + subgraph_defs = _collect_subgraph_defs(workflow) + nodes, links, subgraph_ctx = _expand_subgraphs(nodes, links, subgraph_defs) + + link_map = _build_link_map(links) + links = _rewrite_links_for_subgraphs(links, subgraph_ctx, nodes) + link_map = _build_link_map(links) + + node_by_id = {str(n.get("id")): n for n in nodes} + primitive_values = _collect_primitive_values(nodes) + bypassed = _collect_bypassed(nodes) + nodes_to_exclude = _collect_excluded(nodes, object_info) + reroute_sources = _collect_reroute_sources(nodes, link_map) + set_sources, get_vars = _collect_get_set_mappings(nodes, link_map) + + tracers = _Tracers( + link_map=link_map, + nodes=nodes, + node_by_id=node_by_id, + bypassed=bypassed, + reroute_sources=reroute_sources, + set_sources=set_sources, + get_vars=get_vars, + subgraph_ctx=subgraph_ctx, + ) + + if _has_group_nodes(workflow): + logger.warning( + "Workflow uses legacy 'group nodes' (extra.groupNodes); these aren't " + "expanded by this converter. Recreate them as subgraphs in the frontend." + ) + + api_prompt: dict[str, dict] = {} + for node in nodes: + node_id_str = str(node.get("id")) + node_type = node.get("type") + if not node_type: + continue + node_mode = node.get("mode", 0) + if node_mode in (_MODE_MUTED, _MODE_BYPASS): + continue + if node_type in _UI_ONLY_NODE_TYPES: + continue + if node_id_str in nodes_to_exclude: + continue + + try: + api_prompt[node_id_str] = _build_api_node( + node=node, + node_type=node_type, + object_info=object_info, + tracers=tracers, + primitive_values=primitive_values, + bypassed=bypassed, + nodes_to_exclude=nodes_to_exclude, + ) + except Exception: + # An individual malformed node should not torpedo the whole prompt. + # The executor will fail loudly on missing nodes if this matters. + logger.exception("Failed to convert node id=%s type=%s; skipping", node_id_str, node_type) + + _strip_orphan_link_inputs(api_prompt) + return api_prompt + + +def _has_group_nodes(workflow: dict) -> bool: + """Legacy 'group nodes' (workflow> types) live under extra.groupNodes.""" + extra = workflow.get("extra") + if isinstance(extra, dict) and isinstance(extra.get("groupNodes"), dict) and extra["groupNodes"]: + return True + for node in workflow.get("nodes") or []: + if not isinstance(node, dict): + continue + t = node.get("type") + if isinstance(t, str) and (t.startswith("workflow>") or t.startswith("workflow/")): + return True + return False + + +def _strip_orphan_link_inputs(api_prompt: dict[str, dict]) -> None: + """Drop any link inputs that reference a node we didn't emit. + + Defensive mirror of the frontend's final cleanup pass. We already skip + most orphans during emission, but a stray reference can survive if the + upstream tracing terminated on a node that later got pruned. + """ + for node in api_prompt.values(): + inputs = node.get("inputs") + if not isinstance(inputs, dict): + continue + for name in list(inputs): + value = inputs[name] + if isinstance(value, list) and len(value) == 2 and isinstance(value[0], str) and value[0] not in api_prompt: + del inputs[name] + + +# --------------------------------------------------------------------------- +# Subgraph handling +# --------------------------------------------------------------------------- + + +class _SubgraphCtx: + """Bookkeeping built during subgraph expansion, used later to rewrite links.""" + + def __init__(self) -> None: + # subgraph_node_id_str -> {subgraph_input_idx: [(internal_node_id, internal_slot), ...]} + self.input_targets: dict[str, dict[int, list[tuple[Any, int]]]] = {} + # subgraph_node_id_str -> {(internal_node_id, internal_slot): output_slot_idx} + self.output_sources: dict[str, dict[tuple[Any, int], int]] = {} + # subgraph_node_id_str -> {outer_slot: subgraph_input_idx} (when names differ in order) + self.outer_to_input_idx: dict[str, dict[int, int]] = {} + + +def _collect_subgraph_defs(workflow: dict) -> dict[str, dict]: + definitions = workflow.get("definitions") + if not isinstance(definitions, dict): + return {} + subgraphs = definitions.get("subgraphs") + if not isinstance(subgraphs, list): + return {} + defs: dict[str, dict] = {} + for sg in subgraphs: + if not isinstance(sg, dict): + continue + sg_id = sg.get("id") + if sg_id: + defs[sg_id] = sg + return defs + + +def _expand_subgraphs( + nodes: list[dict], links: list, subgraph_defs: dict[str, dict] +) -> tuple[list[dict], list, _SubgraphCtx]: + """Recursively expand subgraph instances into their constituent nodes.""" + ctx = _SubgraphCtx() + if not subgraph_defs: + return nodes, links, ctx + + for _iteration in range(_MAX_SUBGRAPH_ITERATIONS): + expanded: list[dict] = [] + found_any = False + for node in nodes: + node_type = node.get("type") + if is_subgraph_uuid(node_type) and node_type in subgraph_defs: + found_any = True + sg_nodes, sg_links, input_map, output_map = _expand_one_subgraph(node, subgraph_defs[node_type], links) + expanded.extend(sg_nodes) + links.extend(sg_links) + ctx.input_targets[str(node.get("id"))] = input_map + ctx.output_sources[str(node.get("id"))] = output_map + ctx.outer_to_input_idx[str(node.get("id"))] = _outer_slot_to_input_idx(node, subgraph_defs[node_type]) + else: + expanded.append(node) + nodes = expanded + if not found_any: + return nodes, links, ctx + + logger.warning("Subgraph expansion hit iteration cap — possible cyclic reference") + return nodes, links, ctx + + +def _outer_slot_to_input_idx(outer_node: dict, sg_def: dict) -> dict[int, int]: + """Map the outer node's input slots to subgraph-definition input indices.""" + sg_input_names = {inp.get("name"): idx for idx, inp in enumerate(sg_def.get("inputs", []))} + mapping: dict[int, int] = {} + for outer_idx, outer_input in enumerate(outer_node.get("inputs", []) or []): + name = outer_input.get("name") + if name in sg_input_names: + mapping[outer_idx] = sg_input_names[name] + return mapping + + +def _expand_one_subgraph( + outer_node: dict, sg_def: dict, existing_links: list +) -> tuple[list[dict], list, dict[int, list[tuple[Any, int]]], dict[tuple[Any, int], int]]: + outer_id = outer_node.get("id") + internal_nodes = [n for n in (sg_def.get("nodes") or []) if isinstance(n, dict)] + internal_links = sg_def.get("links") or [] + + # Subgraph internal link IDs may collide with the outer workflow's IDs. + # Allocate fresh IDs starting above the current maximum. + max_link_id = 0 + for link in existing_links: + if isinstance(link, (list, tuple)) and link: + lid = link[0] + if isinstance(lid, int) and lid > max_link_id: + max_link_id = lid + next_id = max_link_id + 1 + + link_id_remap: dict[int, int] = {} + internal_link_map: dict[int, dict] = {} + for link in internal_links: + if isinstance(link, dict): + old_id = link.get("id") + if old_id is not None: + link_id_remap[old_id] = next_id + next_id += 1 + internal_link_map[old_id] = link + + input_targets: dict[int, list[tuple[Any, int]]] = {} + for idx, in_def in enumerate(sg_def.get("inputs", []) or []): + targets = [] + for lid in in_def.get("linkIds", []) or []: + link = internal_link_map.get(lid) + if isinstance(link, dict): + targets.append((link.get("target_id"), link.get("target_slot"))) + if targets: + input_targets[idx] = targets + + output_sources: dict[tuple[Any, int], int] = {} + for idx, out_def in enumerate(sg_def.get("outputs", []) or []): + for lid in out_def.get("linkIds", []) or []: + link = internal_link_map.get(lid) + if isinstance(link, dict): + output_sources[(link.get("origin_id"), link.get("origin_slot"))] = idx + + expanded_nodes: list[dict] = [] + for inner in internal_nodes: + expanded = inner.copy() + expanded["id"] = f"{outer_id}:{inner.get('id')}" + expanded["inputs"] = [ + _rewrite_internal_input(inp, internal_link_map, link_id_remap) for inp in inner.get("inputs", []) or [] + ] + expanded_nodes.append(expanded) + + expanded_links: list = [] + for link in internal_links: + if not isinstance(link, dict): + continue + origin_id = link.get("origin_id") + target_id = link.get("target_id") + if origin_id in (_SUBGRAPH_INPUT_NODE_ID, _SUBGRAPH_OUTPUT_NODE_ID): + continue + if target_id in (_SUBGRAPH_INPUT_NODE_ID, _SUBGRAPH_OUTPUT_NODE_ID): + continue + old_id = link.get("id") + new_id = link_id_remap.get(old_id, old_id) + expanded_links.append( + [ + new_id, + f"{outer_id}:{origin_id}", + link.get("origin_slot"), + f"{outer_id}:{target_id}", + link.get("target_slot"), + link.get("type"), + ] + ) + + return expanded_nodes, expanded_links, input_targets, output_sources + + +def _rewrite_internal_input( + input_info: dict, internal_link_map: dict[int, dict], link_id_remap: dict[int, int] +) -> dict: + input_copy = input_info.copy() + link_id = input_info.get("link") + if link_id is None: + return input_copy + link = internal_link_map.get(link_id) + if not isinstance(link, dict): + return input_copy + if link.get("origin_id") == _SUBGRAPH_INPUT_NODE_ID: + # Will be reattached to an external link by _rewrite_links_for_subgraphs. + input_copy["link"] = None + elif link_id in link_id_remap: + input_copy["link"] = link_id_remap[link_id] + return input_copy + + +def _rewrite_links_for_subgraphs(links: list, ctx: _SubgraphCtx, nodes: list[dict]) -> list: + """Resolve links that cross subgraph boundaries to their internal endpoints.""" + if not ctx.output_sources and not ctx.input_targets: + return links + + node_input_updates: dict[str, dict[int, int]] = {} + updated: list = [] + for link in links: + if not isinstance(link, (list, tuple)) or len(link) < 6: + updated.append(link) + continue + link_id, src_id, src_slot, tgt_id, tgt_slot, link_type = link[:6] + + src_id_str = str(src_id) + src_id_out, src_slot_out = _resolve_subgraph_output(src_id_str, src_slot, ctx) + + tgt_id_str = str(tgt_id) + all_targets = _resolve_subgraph_input_all(tgt_id_str, tgt_slot, ctx) + # Track input-slot rewrites for ALL targets (one outer input may fan out). + for resolved_tgt_id, resolved_tgt_slot in all_targets: + if resolved_tgt_id != tgt_id_str: + node_input_updates.setdefault(resolved_tgt_id, {})[resolved_tgt_slot] = link_id + + first_tgt_id, first_tgt_slot = all_targets[0] + updated.append([link_id, src_id_out, src_slot_out, first_tgt_id, first_tgt_slot, link_type]) + + # Apply input updates to the expanded internal nodes. + for node in nodes: + node_id_str = str(node.get("id")) + if node_id_str not in node_input_updates: + continue + slot_to_link = node_input_updates[node_id_str] + for slot_idx, input_info in enumerate(node.get("inputs", []) or []): + if slot_idx in slot_to_link: + input_info["link"] = slot_to_link[slot_idx] + + return updated + + +def _resolve_subgraph_output(node_id_str: str, slot: Any, ctx: _SubgraphCtx, depth: int = 0) -> tuple[Any, Any]: + if depth > _MAX_RESOLUTION_DEPTH: + return node_id_str, slot + mapping = ctx.output_sources.get(node_id_str) + if not mapping: + return node_id_str, slot + for (internal_node, internal_slot), out_slot in mapping.items(): + if out_slot == slot: + new_id = f"{node_id_str}:{internal_node}" + return _resolve_subgraph_output(new_id, internal_slot, ctx, depth + 1) + return node_id_str, slot + + +def _resolve_subgraph_input_all( + node_id_str: str, slot: Any, ctx: _SubgraphCtx, depth: int = 0 +) -> list[tuple[Any, Any]]: + if depth > _MAX_RESOLUTION_DEPTH: + return [(node_id_str, slot)] + mapping = ctx.input_targets.get(node_id_str) + if not mapping: + return [(node_id_str, slot)] + + sg_input_idx = slot + outer_map = ctx.outer_to_input_idx.get(node_id_str) + if outer_map and slot in outer_map: + sg_input_idx = outer_map[slot] + + targets = mapping.get(sg_input_idx) + if not targets: + return [(node_id_str, slot)] + + out: list[tuple[Any, Any]] = [] + for internal_node, internal_slot in targets: + new_id = f"{node_id_str}:{internal_node}" + out.extend(_resolve_subgraph_input_all(new_id, internal_slot, ctx, depth + 1)) + return out or [(node_id_str, slot)] + + +# --------------------------------------------------------------------------- +# Link map + tracing helpers +# --------------------------------------------------------------------------- + + +def _is_valid_connection(type_a: Any, type_b: Any) -> bool: + """Mirror of LiteGraph.isValidConnection from the frontend. + + ``*`` and ``""`` wildcards match anything; comma-separated alternatives are + expanded; otherwise we case-insensitively compare type names. + """ + if type_a in (0, "", "*"): + type_a = 0 + if type_b in (0, "", "*"): + type_b = 0 + if not type_a or not type_b or type_a == type_b: + return True + type_a_s = str(type_a).lower() + type_b_s = str(type_b).lower() + if "," not in type_a_s and "," not in type_b_s: + return type_a_s == type_b_s + for a in type_a_s.split(","): + for b in type_b_s.split(","): + if _is_valid_connection(a.strip(), b.strip()): + return True + return False + + +def _build_link_map(links: list) -> dict[int, dict]: + link_map: dict[int, dict] = {} + for link in links: + if not isinstance(link, (list, tuple)) or len(link) < 6: + continue + link_id, src_id, src_slot, tgt_id, tgt_slot, link_type = link[:6] + link_map[link_id] = { + "source_id": src_id, + "source_slot": src_slot, + "target_id": tgt_id, + "target_slot": tgt_slot, + "type": link_type, + } + return link_map + + +def _collect_primitive_values(nodes: list[dict]) -> dict[str, Any]: + out: dict[str, Any] = {} + for node in nodes: + if node.get("type") != "PrimitiveNode": + continue + widgets = node.get("widgets_values") + if isinstance(widgets, list) and widgets: + out[str(node.get("id"))] = widgets[0] + return out + + +def _collect_bypassed(nodes: list[dict]) -> set[str]: + return {str(n.get("id")) for n in nodes if n.get("mode") == _MODE_BYPASS} + + +def _collect_reroute_sources(nodes: list[dict], link_map: dict[int, dict]) -> dict[str, tuple[Any, Any]]: + out: dict[str, tuple[Any, Any]] = {} + for node in nodes: + if node.get("type") != "Reroute": + continue + inputs = node.get("inputs") + if not isinstance(inputs, list) or not inputs or not isinstance(inputs[0], dict): + continue + link_id = inputs[0].get("link") + if link_id is None or link_id not in link_map: + continue + ld = link_map[link_id] + out[str(node.get("id"))] = (ld["source_id"], ld["source_slot"]) + return out + + +def _collect_get_set_mappings( + nodes: list[dict], link_map: dict[int, dict] +) -> tuple[dict[str, tuple[Any, Any]], dict[str, str]]: + """SetNode publishes a value under a name; GetNode reads it back.""" + set_sources: dict[str, tuple[Any, Any]] = {} + get_vars: dict[str, str] = {} + for node in nodes: + node_type = node.get("type") + widgets = node.get("widgets_values") + if not isinstance(widgets, list) or not widgets: + continue + var_name = widgets[0] + if node_type == "SetNode": + for inp in node.get("inputs") or []: + if not isinstance(inp, dict): + continue + lid = inp.get("link") + if lid is not None and lid in link_map: + ld = link_map[lid] + set_sources[var_name] = (ld["source_id"], ld["source_slot"]) + break + elif node_type == "GetNode": + get_vars[str(node.get("id"))] = var_name + return set_sources, get_vars + + +def _collect_excluded(nodes: list[dict], object_info: dict) -> set[str]: + """Identify UI-only nodes and dead branches that don't belong in the API output.""" + excluded: set[str] = set() + for node in nodes: + node_type = node.get("type") + node_id_str = str(node.get("id")) + + # LoadImageOutput is a frontend convenience for picking from the output folder. + if node_type == "LoadImageOutput": + excluded.add(node_id_str) + continue + + outputs = node.get("outputs") or [] + has_connected_output = any(isinstance(out, dict) and out.get("links") for out in outputs) + if outputs and has_connected_output: + continue + + schema = object_info.get(node_type) if isinstance(node_type, str) else None + is_output_node = bool(isinstance(schema, dict) and schema.get("output_node")) + has_connected_input = any( + isinstance(inp, dict) and inp.get("link") is not None for inp in (node.get("inputs") or []) + ) + + # Keep output nodes (SaveImage, PreviewImage) even with no downstream consumer. + # Without a schema, keep anything that has at least one wired input — likely a sink. + if is_output_node: + continue + if not schema and has_connected_input: + continue + excluded.add(node_id_str) + return excluded + + +class _Tracers: + """Bundle of upstream-resolution helpers used while emitting each API node.""" + + def __init__( + self, + *, + link_map: dict[int, dict], + nodes: list[dict], + node_by_id: dict[str, dict], + bypassed: set[str], + reroute_sources: dict[str, tuple[Any, Any]], + set_sources: dict[str, tuple[Any, Any]], + get_vars: dict[str, str], + subgraph_ctx: _SubgraphCtx, + ) -> None: + self.link_map = link_map + self.nodes = nodes + self.node_by_id = node_by_id + self.bypassed = bypassed + self.reroute_sources = reroute_sources + self.set_sources = set_sources + self.get_vars = get_vars + self.subgraph_ctx = subgraph_ctx + + def trace_reroute(self, src_id: Any, src_slot: Any, _seen: set[str] | None = None) -> tuple[Any, Any]: + seen = _seen if _seen is not None else set() + key = str(src_id) + if key in seen: + return src_id, src_slot + seen.add(key) + if key in self.reroute_sources: + actual_id, actual_slot = self.reroute_sources[key] + return self.trace_reroute(actual_id, actual_slot, seen) + return src_id, src_slot + + def trace_get_set(self, src_id: Any, src_slot: Any, _seen: set[str] | None = None) -> tuple[Any, Any]: + seen = _seen if _seen is not None else set() + key = str(src_id) + if key in seen: + return src_id, src_slot + seen.add(key) + if key in self.get_vars: + var_name = self.get_vars[key] + if var_name in self.set_sources: + actual_id, actual_slot = self.set_sources[var_name] + return self.trace_get_set(actual_id, actual_slot, seen) + return src_id, src_slot + + def trace_bypassed(self, src_id: Any, src_slot: Any, _seen: set[Any] | None = None) -> tuple[Any, Any]: + seen = _seen if _seen is not None else set() + if src_id in seen: + return src_id, src_slot + seen.add(src_id) + if str(src_id) not in self.bypassed: + return src_id, src_slot + + node = self.node_by_id.get(str(src_id)) + if not node: + return src_id, src_slot + + outputs = node.get("outputs") or [] + # Guard the slot index — malformed workflows can have non-numeric slots. + try: + slot_idx = int(src_slot) if src_slot is not None else 0 + except (TypeError, ValueError): + slot_idx = 0 + output_type = ( + outputs[slot_idx].get("type") + if 0 <= slot_idx < len(outputs) and isinstance(outputs[slot_idx], dict) + else None + ) + + # Pick the input we'll forward the output through. We mix the frontend's + # strict matcher (ExecutableNodeDTO._getBypassSlotIndex) with the + # reference converter's permissive fallback, in order of preference: + # 1. Same-slot input if its type connects to the output type + # 2. First input whose type matches the output type exactly + # 3. First input whose type is connection-compatible (handles ``*`` + # and ``,``-separated alternatives via LiteGraph.isValidConnection) + # 4. First linked input regardless of type — preserves user intent + # when types disagree, matching SethRobinson's reference. The + # executor will surface a type mismatch loudly if it matters. + inputs = node.get("inputs") or [] + chosen_link: int | None = None + exact_link: int | None = None + compat_link: int | None = None + fallback_link: int | None = None + + same_slot_inp = inputs[slot_idx] if 0 <= slot_idx < len(inputs) and isinstance(inputs[slot_idx], dict) else None + if same_slot_inp: + lid = same_slot_inp.get("link") + if ( + lid is not None + and lid in self.link_map + and _is_valid_connection(same_slot_inp.get("type"), output_type) + ): + chosen_link = lid + + if chosen_link is None: + for inp in inputs: + if not isinstance(inp, dict): + continue + lid = inp.get("link") + if lid is None or lid not in self.link_map: + continue + inp_type = inp.get("type") + if fallback_link is None: + fallback_link = lid + if output_type and inp_type == output_type and exact_link is None: + exact_link = lid + if compat_link is None and _is_valid_connection(inp_type, output_type): + compat_link = lid + chosen_link = exact_link if exact_link is not None else compat_link + if chosen_link is None: + chosen_link = fallback_link + + if chosen_link is None: + return src_id, src_slot + + ld = self.link_map[chosen_link] + upstream_id, upstream_slot = ld["source_id"], ld["source_slot"] + upstream_id, upstream_slot = self.trace_get_set(upstream_id, upstream_slot) + upstream_id, upstream_slot = self.trace_reroute(upstream_id, upstream_slot) + return self.trace_bypassed(upstream_id, upstream_slot, seen) + + +# --------------------------------------------------------------------------- +# Per-node emission +# --------------------------------------------------------------------------- + + +def _wrap_widget_value(value: Any) -> Any: + """Wrap list widget values to disambiguate them from [node_id, slot] links. + + ComfyUI's executor strips the wrapper before passing to the node. See + execution.py: ``if "__value__" in val: val = val["__value__"]``. + """ + if isinstance(value, list): + return {"__value__": value} + return value + + +def _build_api_node( + *, + node: dict, + node_type: str, + object_info: dict, + tracers: _Tracers, + primitive_values: dict[str, Any], + bypassed: set[str], + nodes_to_exclude: set[str], +) -> dict: + api_node: dict = {"inputs": {}, "class_type": node_type} + + if "title" in node: + api_node["_meta"] = {"title": node["title"]} + else: + schema = object_info.get(node_type, {}) + api_node["_meta"] = {"title": schema.get("display_name") or node_type} + + link_inputs: dict[str, list] = {} + primitive_inputs: dict[str, Any] = {} + for inp in node.get("inputs") or []: + if not isinstance(inp, dict): + continue + input_name = inp.get("name") + link_id = inp.get("link") + if not input_name or link_id is None or link_id not in tracers.link_map: + continue + ld = tracers.link_map[link_id] + actual_id, actual_slot = ld["source_id"], ld["source_slot"] + + actual_id, actual_slot = tracers.trace_get_set(actual_id, actual_slot) + actual_id, actual_slot = tracers.trace_reroute(actual_id, actual_slot) + if str(actual_id) in bypassed: + actual_id, actual_slot = tracers.trace_bypassed(actual_id, actual_slot) + if str(actual_id) in bypassed: + # Couldn't find a non-bypassed source — let widget default cover it. + continue + # Bypassed source may itself have referenced a GetNode or Reroute. + actual_id, actual_slot = tracers.trace_get_set(actual_id, actual_slot) + actual_id, actual_slot = tracers.trace_reroute(actual_id, actual_slot) + # If we crossed a subgraph boundary while tracing, finalize to internal node. + actual_id, actual_slot = _resolve_subgraph_output(str(actual_id), actual_slot, tracers.subgraph_ctx) + + actual_id_str = str(actual_id) + if actual_id_str in primitive_values: + primitive_inputs[input_name] = _wrap_widget_value(primitive_values[actual_id_str]) + elif actual_id_str in nodes_to_exclude: + continue + elif actual_id_str in bypassed: + continue + else: + link_inputs[input_name] = [actual_id_str, actual_slot] + + widget_inputs = _collect_widget_inputs(node, node_type, object_info, link_inputs) + default_inputs = _collect_default_inputs(node_type, object_info, widget_inputs, primitive_inputs, link_inputs) + + ordered = _get_ordered_input_names(node_type, node, object_info) + if ordered: + # First widget-like values in the declared order, then link inputs. + # This matches what ComfyUI's "Save (API)" produces. + for name in ordered: + if name in widget_inputs: + api_node["inputs"][name] = widget_inputs[name] + elif name in primitive_inputs: + api_node["inputs"][name] = primitive_inputs[name] + elif name in default_inputs: + api_node["inputs"][name] = default_inputs[name] + for name in ordered: + if name in link_inputs and name not in api_node["inputs"]: + api_node["inputs"][name] = link_inputs[name] + + # Anything we didn't know an order for is still emitted (preserves data). + for source in (widget_inputs, primitive_inputs, default_inputs, link_inputs): + for key, value in source.items(): + if key not in api_node["inputs"]: + api_node["inputs"][key] = value + + _normalize_combo_values(node_type, object_info, api_node["inputs"]) + return api_node + + +# --------------------------------------------------------------------------- +# Widget / input order helpers (driven by /object_info) +# --------------------------------------------------------------------------- + + +def _schema_for(node_type: str, node: dict, object_info: dict) -> dict | None: + # Some nodes (litegraph subgraphs) store the real class name under properties. + properties = node.get("properties") or {} + alt_name = properties.get("Node name for S&R") + if isinstance(alt_name, str) and alt_name in object_info: + return object_info[alt_name] + return object_info.get(node_type) if isinstance(node_type, str) else None + + +def _get_ordered_input_names(node_type: str, node: dict, object_info: dict) -> list[str]: + schema = _schema_for(node_type, node, object_info) + if not schema: + return [] + input_order = schema.get("input_order") or {} + out: list[str] = [] + for section in ("required", "optional"): + out.extend(input_order.get(section, []) or []) + if out: + return out + # Fall back to whatever order is in the input dict itself. + input_def = schema.get("input") or {} + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if isinstance(section_def, dict): + out.extend(section_def.keys()) + return out + + +def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: + """Return (is_widget, is_dynamic_combo) for an INPUT_TYPES spec.""" + if not isinstance(input_spec, (list, tuple)) or not input_spec: + return False, False + input_type = input_spec[0] + if isinstance(input_type, (list, tuple)): + return True, False # combo of choices + if isinstance(input_type, str): + if input_type in {"INT", "FLOAT", "STRING", "BOOLEAN", "COMBO"}: + return True, False + if input_type.startswith("COMFY_") and "COMBO" in input_type: + return True, True + if not input_type.isupper(): + return True, False # custom (lowercase) widget types + return False, False + + +def _dynamic_combo_sub_inputs( + input_name: str, input_spec: Any, widget_values: list[Any], current_idx: int +) -> list[str]: + if not isinstance(input_spec, (list, tuple)) or len(input_spec) < 2: + return [] + options_meta = input_spec[1] if isinstance(input_spec[1], dict) else {} + options = options_meta.get("options") or [] + if not options or current_idx >= len(widget_values): + return [] + selected = widget_values[current_idx] + for option in options: + if not isinstance(option, dict) or option.get("key") != selected: + continue + sub_def = option.get("inputs") or {} + names: list[str] = [] + for section in ("required", "optional"): + section_def = sub_def.get(section) or {} + if isinstance(section_def, dict): + names.extend(f"{input_name}.{sub_name}" for sub_name in section_def.keys()) + return names + return [] + + +def _get_widget_name_order(node_type: str, node: dict, object_info: dict, widget_values: list[Any]) -> list[str | None]: + """Build the widget-name list that maps positionally to ``widgets_values``.""" + schema = _schema_for(node_type, node, object_info) + if schema: + input_def = schema.get("input") or {} + names: list[str | None] = [] + widget_idx = 0 + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if not isinstance(section_def, dict): + continue + for input_name, input_spec in section_def.items(): + is_widget, is_dynamic = _is_widget_input(input_spec) + if not is_widget: + continue + names.append(input_name) + if is_dynamic and widget_values: + subs = _dynamic_combo_sub_inputs(input_name, input_spec, widget_values, widget_idx) + names.extend(subs) + widget_idx += 1 + len(subs) + else: + widget_idx += 1 + if names: + return names + + # Fallback: inspect the node's own input list. Some nodes mark widget-flagged inputs. + return _fallback_widget_names(node, widget_values) + + +def _fallback_widget_names(node: dict, widget_values: list[Any]) -> list[str | None]: + properties = node.get("properties") or {} + ue_properties = properties.get("ue_properties") or {} + ue_connectable = ue_properties.get("widget_ue_connectable") + if isinstance(ue_connectable, dict) and ue_connectable: + names = list(ue_connectable.keys()) + if len(names) >= len(widget_values): + return list(names[: len(widget_values)]) + + all_inputs: list[str] = [] + connected: set[str] = set() + widget_flagged: list[str] = [] + for inp in node.get("inputs") or []: + if not isinstance(inp, dict): + continue + name = inp.get("name") + if not name: + continue + all_inputs.append(name) + if inp.get("link") is not None: + connected.add(name) + if inp.get("widget"): + widget_flagged.append(name) + + if widget_flagged: + if len(widget_values) > len(widget_flagged): + extras = [n for n in all_inputs if n not in connected and n not in widget_flagged] + return widget_flagged + extras[: len(widget_values) - len(widget_flagged)] + return list(widget_flagged) + + unconnected = [n for n in all_inputs if n not in connected] + if len(unconnected) >= len(widget_values): + return unconnected[: len(widget_values)] + return [] + + +def _filter_control_values(widget_values: list[Any]) -> list[Any]: + """Drop control_after_generate strings that follow INT widgets in widget_values.""" + + def is_control(v: Any) -> bool: + return isinstance(v, str) and v in _CONTROL_AFTER_GENERATE_VALUES + + out: list[Any] = [] + i = 0 + while i < len(widget_values): + value = widget_values[i] + if is_control(value): + i += 1 + continue + if i + 1 < len(widget_values) and is_control(widget_values[i + 1]): + out.append(value) + i += 2 + continue + out.append(value) + i += 1 + return out + + +def _collect_widget_inputs( + node: dict, node_type: str, object_info: dict, link_inputs: dict[str, list] +) -> dict[str, Any]: + widget_values = node.get("widgets_values") + if widget_values is None: + return {} + out: dict[str, Any] = {} + if isinstance(widget_values, dict): + # Already self-describing; drop UI-only keys and respect link overrides. + for key, value in widget_values.items(): + if key in ("videopreview", "preview"): + continue + if key in link_inputs: + continue + out[key] = _wrap_widget_value(value) + return out + if not isinstance(widget_values, list): + return {} + + if any(isinstance(v, dict) for v in widget_values): + _absorb_dict_widget_values(widget_values, out, link_inputs) + return out + + filtered = _filter_control_values(widget_values) + names = _get_widget_name_order(node_type, node, object_info, widget_values) + if not names: + if filtered: + logger.warning( + "Could not map widget_values for unknown node type %r (node %s)", + node_type, + node.get("id"), + ) + return out + for i, value in enumerate(filtered): + if i >= len(names): + break + name = names[i] + if not name or name in link_inputs: + continue + out[name] = _wrap_widget_value(value) + return out + + +def _absorb_dict_widget_values(widget_values: list[Any], out: dict[str, Any], link_inputs: dict[str, list]) -> None: + lora_counter = 0 + for value in widget_values: + if isinstance(value, dict): + if not value: + continue + if "type" in value: + name = value.get("type") + if name and name not in link_inputs: + out[name] = value + elif "lora" in value: + lora_counter += 1 + name = f"lora_{lora_counter}" + if name in link_inputs: + continue + clean = {k: v for k, v in value.items() if k != "strengthTwo" or v is not None} + out[name] = clean + elif isinstance(value, str) and value == "": + # Frontend's "Add Lora" button serializes as an empty string trailer. + out.setdefault("➕ Add Lora", value) + + +def _collect_default_inputs( + node_type: str, + object_info: dict, + widget_inputs: dict[str, Any], + primitive_inputs: dict[str, Any], + link_inputs: dict[str, list], +) -> dict[str, Any]: + schema = object_info.get(node_type) if isinstance(node_type, str) else None + if not schema: + return {} + input_def = schema.get("input") or {} + defaults: dict[str, Any] = {} + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if not isinstance(section_def, dict): + continue + for input_name, input_spec in section_def.items(): + if input_name in widget_inputs or input_name in primitive_inputs or input_name in link_inputs: + continue + default = _extract_default(input_spec) + if default is not _MISSING: + defaults[input_name] = _wrap_widget_value(default) + return defaults + + +_MISSING = object() + + +def _extract_default(input_spec: Any) -> Any: + if not isinstance(input_spec, (list, tuple)) or not input_spec: + return _MISSING + input_type = input_spec[0] + options = input_spec[1] if len(input_spec) >= 2 and isinstance(input_spec[1], dict) else {} + if "default" in options: + return options["default"] + if isinstance(input_type, list) and input_type: + return input_type[0] + if input_type == "COMBO": + opts = options.get("options") + if isinstance(opts, list) and opts: + return opts[0] + return _MISSING + + +def _normalize_combo_values(node_type: str, object_info: dict, inputs: dict[str, Any]) -> None: + schema = object_info.get(node_type) if isinstance(node_type, str) else None + if not schema: + return + input_def = schema.get("input") or {} + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if not isinstance(section_def, dict): + continue + for input_name, input_spec in section_def.items(): + if input_name not in inputs: + continue + value = inputs[input_name] + if not isinstance(value, str): + continue + if not isinstance(input_spec, (list, tuple)) or not input_spec: + continue + allowed = input_spec[0] + if not isinstance(allowed, (list, tuple)): + continue + if value in allowed: + continue + lower_value = value.lower() + for option in allowed: + if isinstance(option, str) and option.lower() == lower_value: + inputs[input_name] = option + break diff --git a/tests/comfy_cli/fixtures/sd15_expected_api.json b/tests/comfy_cli/fixtures/sd15_expected_api.json new file mode 100644 index 00000000..2204a937 --- /dev/null +++ b/tests/comfy_cli/fixtures/sd15_expected_api.json @@ -0,0 +1,107 @@ +{ + "4": { + "inputs": { + "ckpt_name": "v1-5-pruned-emaonly-fp16.safetensors" + }, + "class_type": "CheckpointLoaderSimple", + "_meta": { + "title": "Load Checkpoint" + } + }, + "3": { + "inputs": { + "seed": 685468484323813, + "steps": 20, + "cfg": 8, + "sampler_name": "euler", + "scheduler": "normal", + "denoise": 1, + "model": [ + "4", + 0 + ], + "positive": [ + "6", + 0 + ], + "negative": [ + "7", + 0 + ], + "latent_image": [ + "5", + 0 + ] + }, + "class_type": "KSampler", + "_meta": { + "title": "KSampler" + } + }, + "8": { + "inputs": { + "samples": [ + "3", + 0 + ], + "vae": [ + "4", + 2 + ] + }, + "class_type": "VAEDecode", + "_meta": { + "title": "VAE Decode" + } + }, + "9": { + "inputs": { + "filename_prefix": "SD1.5", + "images": [ + "8", + 0 + ] + }, + "class_type": "SaveImage", + "_meta": { + "title": "Save Image" + } + }, + "7": { + "inputs": { + "text": "text, watermark", + "clip": [ + "4", + 1 + ] + }, + "class_type": "CLIPTextEncode", + "_meta": { + "title": "CLIP Text Encode (Prompt)" + } + }, + "5": { + "inputs": { + "width": 512, + "height": 512, + "batch_size": 1 + }, + "class_type": "EmptyLatentImage", + "_meta": { + "title": "Empty Latent Image" + } + }, + "6": { + "inputs": { + "text": "beautiful scenery nature glass bottle landscape, purple galaxy bottle,", + "clip": [ + "4", + 1 + ] + }, + "class_type": "CLIPTextEncode", + "_meta": { + "title": "CLIP Text Encode (Prompt)" + } + } +} \ No newline at end of file diff --git a/tests/comfy_cli/fixtures/sd15_object_info.json b/tests/comfy_cli/fixtures/sd15_object_info.json new file mode 100644 index 00000000..af497615 --- /dev/null +++ b/tests/comfy_cli/fixtures/sd15_object_info.json @@ -0,0 +1,467 @@ +{ + "CLIPTextEncode": { + "input": { + "required": { + "text": [ + "STRING", + { + "multiline": true, + "dynamicPrompts": true, + "tooltip": "The text to be encoded." + } + ], + "clip": [ + "CLIP", + { + "tooltip": "The CLIP model used for encoding the text." + } + ] + } + }, + "input_order": { + "required": [ + "text", + "clip" + ] + }, + "is_input_list": false, + "output": [ + "CONDITIONING" + ], + "output_is_list": [ + false + ], + "output_name": [ + "CONDITIONING" + ], + "name": "CLIPTextEncode", + "display_name": "CLIP Text Encode (Prompt)", + "description": "Encodes a text prompt using a CLIP model into an embedding that can be used to guide the diffusion model towards generating specific images.", + "python_module": "nodes", + "category": "conditioning", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "A conditioning containing the embedded text used to guide the diffusion model." + ], + "search_aliases": [ + "text", + "prompt", + "text prompt", + "positive prompt", + "negative prompt", + "encode text", + "text encoder", + "encode prompt" + ] + }, + "CheckpointLoaderSimple": { + "input": { + "required": { + "ckpt_name": [ + [ + "sd_xl_turbo_1.0_fp16.safetensors", + "v1-5-pruned-emaonly-fp16.safetensors" + ], + { + "tooltip": "The name of the checkpoint (model) to load." + } + ] + } + }, + "input_order": { + "required": [ + "ckpt_name" + ] + }, + "is_input_list": false, + "output": [ + "MODEL", + "CLIP", + "VAE" + ], + "output_is_list": [ + false, + false, + false + ], + "output_name": [ + "MODEL", + "CLIP", + "VAE" + ], + "name": "CheckpointLoaderSimple", + "display_name": "Load Checkpoint", + "description": "Loads a diffusion model checkpoint, diffusion models are used to denoise latents.", + "python_module": "nodes", + "category": "loaders", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The model used for denoising latents.", + "The CLIP model used for encoding text prompts.", + "The VAE model used for encoding and decoding images to and from latent space." + ], + "search_aliases": [ + "load model", + "checkpoint", + "model loader", + "load checkpoint", + "ckpt", + "model" + ] + }, + "EmptyLatentImage": { + "input": { + "required": { + "width": [ + "INT", + { + "default": 512, + "min": 16, + "max": 16384, + "step": 8, + "tooltip": "The width of the latent images in pixels." + } + ], + "height": [ + "INT", + { + "default": 512, + "min": 16, + "max": 16384, + "step": 8, + "tooltip": "The height of the latent images in pixels." + } + ], + "batch_size": [ + "INT", + { + "default": 1, + "min": 1, + "max": 4096, + "tooltip": "The number of latent images in the batch." + } + ] + } + }, + "input_order": { + "required": [ + "width", + "height", + "batch_size" + ] + }, + "is_input_list": false, + "output": [ + "LATENT" + ], + "output_is_list": [ + false + ], + "output_name": [ + "LATENT" + ], + "name": "EmptyLatentImage", + "display_name": "Empty Latent Image", + "description": "Create a new batch of empty latent images to be denoised via sampling.", + "python_module": "nodes", + "category": "latent", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The empty latent image batch." + ], + "search_aliases": [ + "empty", + "empty latent", + "new latent", + "create latent", + "blank latent", + "blank" + ] + }, + "KSampler": { + "input": { + "required": { + "model": [ + "MODEL", + { + "tooltip": "The model used for denoising the input latent." + } + ], + "seed": [ + "INT", + { + "default": 0, + "min": 0, + "max": 18446744073709551615, + "control_after_generate": true, + "tooltip": "The random seed used for creating the noise." + } + ], + "steps": [ + "INT", + { + "default": 20, + "min": 1, + "max": 10000, + "tooltip": "The number of steps used in the denoising process." + } + ], + "cfg": [ + "FLOAT", + { + "default": 8.0, + "min": 0.0, + "max": 100.0, + "step": 0.1, + "round": 0.01, + "tooltip": "The Classifier-Free Guidance scale balances creativity and adherence to the prompt. Higher values result in images more closely matching the prompt however too high values will negatively impact quality." + } + ], + "sampler_name": [ + [ + "euler", + "euler_cfg_pp", + "euler_ancestral", + "euler_ancestral_cfg_pp", + "heun", + "heunpp2", + "exp_heun_2_x0", + "exp_heun_2_x0_sde", + "dpm_2", + "dpm_2_ancestral", + "lms", + "dpm_fast", + "dpm_adaptive", + "dpmpp_2s_ancestral", + "dpmpp_2s_ancestral_cfg_pp", + "dpmpp_sde", + "dpmpp_sde_gpu", + "dpmpp_2m", + "dpmpp_2m_cfg_pp", + "dpmpp_2m_sde", + "dpmpp_2m_sde_gpu", + "dpmpp_2m_sde_heun", + "dpmpp_2m_sde_heun_gpu", + "dpmpp_3m_sde", + "dpmpp_3m_sde_gpu", + "ddpm", + "lcm", + "ipndm", + "ipndm_v", + "deis", + "res_multistep", + "res_multistep_cfg_pp", + "res_multistep_ancestral", + "res_multistep_ancestral_cfg_pp", + "gradient_estimation", + "gradient_estimation_cfg_pp", + "er_sde", + "seeds_2", + "seeds_3", + "sa_solver", + "sa_solver_pece", + "ddim", + "uni_pc", + "uni_pc_bh2" + ], + { + "tooltip": "The algorithm used when sampling, this can affect the quality, speed, and style of the generated output." + } + ], + "scheduler": [ + [ + "simple", + "sgm_uniform", + "karras", + "exponential", + "ddim_uniform", + "beta", + "normal", + "linear_quadratic", + "kl_optimal" + ], + { + "tooltip": "The scheduler controls how noise is gradually removed to form the image." + } + ], + "positive": [ + "CONDITIONING", + { + "tooltip": "The conditioning describing the attributes you want to include in the image." + } + ], + "negative": [ + "CONDITIONING", + { + "tooltip": "The conditioning describing the attributes you want to exclude from the image." + } + ], + "latent_image": [ + "LATENT", + { + "tooltip": "The latent image to denoise." + } + ], + "denoise": [ + "FLOAT", + { + "default": 1.0, + "min": 0.0, + "max": 1.0, + "step": 0.01, + "tooltip": "The amount of denoising applied, lower values will maintain the structure of the initial image allowing for image to image sampling." + } + ] + } + }, + "input_order": { + "required": [ + "model", + "seed", + "steps", + "cfg", + "sampler_name", + "scheduler", + "positive", + "negative", + "latent_image", + "denoise" + ] + }, + "is_input_list": false, + "output": [ + "LATENT" + ], + "output_is_list": [ + false + ], + "output_name": [ + "LATENT" + ], + "name": "KSampler", + "display_name": "KSampler", + "description": "Uses the provided model, positive and negative conditioning to denoise the latent image.", + "python_module": "nodes", + "category": "sampling", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The denoised latent." + ], + "search_aliases": [ + "sampler", + "sample", + "generate", + "denoise", + "diffuse", + "txt2img", + "img2img" + ] + }, + "SaveImage": { + "input": { + "required": { + "images": [ + "IMAGE", + { + "tooltip": "The images to save." + } + ], + "filename_prefix": [ + "STRING", + { + "default": "ComfyUI", + "tooltip": "The prefix for the file to save. This may include formatting information such as %date:yyyy-MM-dd% or %Empty Latent Image.width% to include values from nodes." + } + ] + }, + "hidden": { + "prompt": "PROMPT", + "extra_pnginfo": "EXTRA_PNGINFO" + } + }, + "input_order": { + "required": [ + "images", + "filename_prefix" + ], + "hidden": [ + "prompt", + "extra_pnginfo" + ] + }, + "is_input_list": false, + "output": [], + "output_is_list": [], + "output_name": [], + "name": "SaveImage", + "display_name": "Save Image", + "description": "Saves the input images to your ComfyUI output directory.", + "python_module": "nodes", + "category": "image", + "output_node": true, + "has_intermediate_output": false, + "search_aliases": [ + "save", + "save image", + "export image", + "output image", + "write image", + "download" + ], + "essentials_category": "Basics" + }, + "VAEDecode": { + "input": { + "required": { + "samples": [ + "LATENT", + { + "tooltip": "The latent to be decoded." + } + ], + "vae": [ + "VAE", + { + "tooltip": "The VAE model used for decoding the latent." + } + ] + } + }, + "input_order": { + "required": [ + "samples", + "vae" + ] + }, + "is_input_list": false, + "output": [ + "IMAGE" + ], + "output_is_list": [ + false + ], + "output_name": [ + "IMAGE" + ], + "name": "VAEDecode", + "display_name": "VAE Decode", + "description": "Decodes latent images back into pixel space images.", + "python_module": "nodes", + "category": "latent", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The decoded image." + ], + "search_aliases": [ + "decode", + "decode latent", + "latent to image", + "render latent" + ] + } +} \ No newline at end of file diff --git a/tests/comfy_cli/fixtures/sd15_ui_workflow.json b/tests/comfy_cli/fixtures/sd15_ui_workflow.json new file mode 100644 index 00000000..4338b436 --- /dev/null +++ b/tests/comfy_cli/fixtures/sd15_ui_workflow.json @@ -0,0 +1,547 @@ +{ + "id": "2ba0b800-2f13-4f21-b8d6-c6cdb0152cae", + "revision": 0, + "last_node_id": 16, + "last_link_id": 9, + "nodes": [ + { + "id": 4, + "type": "CheckpointLoaderSimple", + "pos": [ + 10, + 300 + ], + "size": [ + 320, + 154.65625 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "MODEL", + "type": "MODEL", + "slot_index": 0, + "links": [ + 1 + ] + }, + { + "name": "CLIP", + "type": "CLIP", + "slot_index": 1, + "links": [ + 3, + 5 + ] + }, + { + "name": "VAE", + "type": "VAE", + "slot_index": 2, + "links": [ + 8 + ] + } + ], + "properties": { + "Node name for S&R": "CheckpointLoaderSimple", + "cnr_id": "comfy-core", + "ver": "0.3.65", + "models": [ + { + "name": "v1-5-pruned-emaonly-fp16.safetensors", + "url": "https://huggingface.co/Comfy-Org/stable-diffusion-v1-5-archive/resolve/main/v1-5-pruned-emaonly-fp16.safetensors?download=true", + "directory": "checkpoints" + } + ] + }, + "widgets_values": [ + "v1-5-pruned-emaonly-fp16.safetensors" + ] + }, + { + "id": 3, + "type": "KSampler", + "pos": [ + 920, + 170 + ], + "size": [ + 320, + 480 + ], + "flags": {}, + "order": 8, + "mode": 0, + "inputs": [ + { + "name": "model", + "type": "MODEL", + "link": 1 + }, + { + "name": "positive", + "type": "CONDITIONING", + "link": 4 + }, + { + "name": "negative", + "type": "CONDITIONING", + "link": 6 + }, + { + "name": "latent_image", + "type": "LATENT", + "link": 2 + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [ + 7 + ] + } + ], + "properties": { + "Node name for S&R": "KSampler", + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [ + 685468484323813, + "randomize", + 20, + 8, + "euler", + "normal", + 1 + ] + }, + { + "id": 8, + "type": "VAEDecode", + "pos": [ + 1000, + 710 + ], + "size": [ + 225, + 96 + ], + "flags": {}, + "order": 9, + "mode": 0, + "inputs": [ + { + "name": "samples", + "type": "LATENT", + "link": 7 + }, + { + "name": "vae", + "type": "VAE", + "link": 8 + } + ], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "slot_index": 0, + "links": [ + 9 + ] + } + ], + "properties": { + "Node name for S&R": "VAEDecode", + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [] + }, + { + "id": 9, + "type": "SaveImage", + "pos": [ + 1270, + 170 + ], + "size": [ + 470, + 560 + ], + "flags": {}, + "order": 10, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 9 + } + ], + "outputs": [], + "properties": { + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [ + "SD1.5" + ] + }, + { + "id": 7, + "type": "CLIPTextEncode", + "pos": [ + 430, + 530 + ], + "size": [ + 420, + 170 + ], + "flags": {}, + "order": 7, + "mode": 0, + "inputs": [ + { + "name": "clip", + "type": "CLIP", + "link": 5 + } + ], + "outputs": [ + { + "name": "CONDITIONING", + "type": "CONDITIONING", + "slot_index": 0, + "links": [ + 6 + ] + } + ], + "properties": { + "Node name for S&R": "CLIPTextEncode", + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [ + "text, watermark" + ], + "color": "#223", + "bgcolor": "#335" + }, + { + "id": 5, + "type": "EmptyLatentImage", + "pos": [ + 490, + 900 + ], + "size": [ + 320, + 168 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "slot_index": 0, + "links": [ + 2 + ] + } + ], + "properties": { + "Node name for S&R": "EmptyLatentImage", + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [ + 512, + 512, + 1 + ] + }, + { + "id": 6, + "type": "CLIPTextEncode", + "pos": [ + 420, + 220 + ], + "size": [ + 430, + 260 + ], + "flags": {}, + "order": 6, + "mode": 0, + "inputs": [ + { + "name": "clip", + "type": "CLIP", + "link": 3 + } + ], + "outputs": [ + { + "name": "CONDITIONING", + "type": "CONDITIONING", + "slot_index": 0, + "links": [ + 4 + ] + } + ], + "properties": { + "Node name for S&R": "CLIPTextEncode", + "cnr_id": "comfy-core", + "ver": "0.3.65" + }, + "widgets_values": [ + "beautiful scenery nature glass bottle landscape, purple galaxy bottle," + ], + "color": "#232", + "bgcolor": "#353" + }, + { + "id": 15, + "type": "MarkdownNote", + "pos": [ + 400, + -320 + ], + "size": [ + 470, + 430 + ], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [], + "outputs": [], + "title": "Note: Prompt", + "properties": {}, + "widgets_values": [ + "**Prompts usually have two types:**\n\n* **Positive Prompt:** Tells the model what you *want* to see.\n* **Negative Prompt:** Tells the model what you *don\u2019t want* to see.\n\nThink of it as:
\n\ud83d\udc49 Positive = \u201cDo this\u201d
\n\ud83d\udc49 Negative = \u201cDon\u2019t do this\u201d\n\n\nDifferent models may interpret prompts differently.
\nSome prefer short, simple phrases; others respond well to detailed descriptions or styles.\nExperiment to see how each model reacts.\n\nAbout SD1.5:
\nStable Diffusion 1.5 is one of the most popular base models.\nIt works best with short, clear prompts and simple concepts, and it has a natural, realistic visual style.\n" + ], + "color": "#432", + "bgcolor": "#000" + }, + { + "id": 14, + "type": "MarkdownNote", + "pos": [ + 1270, + 780 + ], + "size": [ + 470, + 130 + ], + "flags": {}, + "order": 3, + "mode": 0, + "inputs": [], + "outputs": [], + "title": "Note: Output", + "properties": {}, + "widgets_values": [ + "Image will auto-save to the `ComfyUI/output` folder. You can also right-click on the `Save Image` node and then use the menu to save the image locally." + ], + "color": "#432", + "bgcolor": "#000" + }, + { + "id": 13, + "type": "MarkdownNote", + "pos": [ + 460, + 1180 + ], + "size": [ + 330, + 163.953125 + ], + "flags": {}, + "order": 4, + "mode": 0, + "inputs": [], + "outputs": [], + "title": "Note: Image size", + "properties": {}, + "widgets_values": [ + "Different models are trained based on datasets of different image sizes. This workflow is using Stable Diffusion 1.5, which is trained based on 512x512 datasets. So, it doesn't perform well on large image sizes." + ], + "color": "#432", + "bgcolor": "#000" + }, + { + "id": 11, + "type": "MarkdownNote", + "pos": [ + -470, + 160 + ], + "size": [ + 400, + 530.890625 + ], + "flags": {}, + "order": 5, + "mode": 0, + "inputs": [], + "outputs": [], + "title": "Note: Model link", + "properties": {}, + "widgets_values": [ + "[Tutorial](https://docs.comfy.org/tutorials/basic/text-to-image)\n\n\n## Model link\n\n**checkpoints**\n\n- [v1-5-pruned-emaonly-fp16.safetensors](https://huggingface.co/Comfy-Org/stable-diffusion-v1-5-archive/resolve/main/v1-5-pruned-emaonly-fp16.safetensors?download=true)\n\n\nModel Storage Location\n\n```\n\ud83d\udcc2 ComfyUI/\n\u251c\u2500\u2500 \ud83d\udcc2 models/\n\u2502 \u2514\u2500\u2500 \ud83d\udcc2 checkpoints/\n\u2502 \u2514\u2500\u2500 v1-5-pruned-emaonly-fp16.safetensors\n```\n\n\n## Report issue\nIf you have any problems running this workflow, please report template-related issues via this link: [report the template issue here](https://github.com/Comfy-Org/workflow_templates/issues)" + ], + "color": "#432", + "bgcolor": "#000" + } + ], + "links": [ + [ + 1, + 4, + 0, + 3, + 0, + "MODEL" + ], + [ + 2, + 5, + 0, + 3, + 3, + "LATENT" + ], + [ + 3, + 4, + 1, + 6, + 0, + "CLIP" + ], + [ + 4, + 6, + 0, + 3, + 1, + "CONDITIONING" + ], + [ + 5, + 4, + 1, + 7, + 0, + "CLIP" + ], + [ + 6, + 7, + 0, + 3, + 2, + "CONDITIONING" + ], + [ + 7, + 3, + 0, + 8, + 0, + "LATENT" + ], + [ + 8, + 4, + 2, + 8, + 1, + "VAE" + ], + [ + 9, + 8, + 0, + 9, + 0, + "IMAGE" + ] + ], + "groups": [ + { + "id": 1, + "title": "Step 1 - Load model", + "bounding": [ + -40, + 130, + 420, + 470 + ], + "color": "#3f789e", + "font_size": 24, + "flags": {} + }, + { + "id": 2, + "title": "Step 3 - Image size", + "bounding": [ + 400, + 800, + 480, + 310 + ], + "color": "#3f789e", + "font_size": 24, + "flags": {} + }, + { + "id": 3, + "title": "Step 2 - Prompt", + "bounding": [ + 400, + 130, + 480, + 640 + ], + "color": "#3f789e", + "font_size": 24, + "flags": {} + } + ], + "config": {}, + "extra": { + "ds": { + "scale": 0.5131581182307069, + "offset": [ + 979.5226642853634, + 273.924658465434 + ] + }, + "frontendVersion": "1.42.15", + "VHS_latentpreview": false, + "VHS_latentpreviewrate": 0, + "VHS_MetadataImage": true, + "VHS_KeepIntermediate": true + }, + "version": 0.4 +} \ No newline at end of file diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py new file mode 100644 index 00000000..6372c99f --- /dev/null +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -0,0 +1,947 @@ +"""Unit tests for the UI -> API workflow converter.""" + +import json +from pathlib import Path + +import pytest + +from comfy_cli.workflow_to_api import ( + WorkflowConversionError, + convert_ui_to_api, + is_api_format, + is_subgraph_uuid, +) + +FIXTURES = Path(__file__).parent / "fixtures" + + +# --------------------------------------------------------------------------- +# Reusable fixtures: a tiny `/object_info` covering the schemas the tests use. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def object_info(): + return { + "EmptyLatentImage": { + "input": { + "required": { + "width": ["INT", {"default": 512}], + "height": ["INT", {"default": 512}], + "batch_size": ["INT", {"default": 1}], + } + }, + "input_order": {"required": ["width", "height", "batch_size"]}, + "output_node": False, + "output": ["LATENT"], + "display_name": "Empty Latent Image", + }, + "KSampler": { + "input": { + "required": { + "model": ["MODEL"], + "seed": ["INT", {"default": 0}], + "steps": ["INT", {"default": 20}], + "cfg": ["FLOAT", {"default": 8.0}], + "sampler_name": [["euler", "ddim"], {"default": "euler"}], + "scheduler": [["normal", "karras"], {"default": "normal"}], + "positive": ["CONDITIONING"], + "negative": ["CONDITIONING"], + "latent_image": ["LATENT"], + "denoise": ["FLOAT", {"default": 1.0}], + } + }, + "input_order": { + "required": [ + "model", + "seed", + "steps", + "cfg", + "sampler_name", + "scheduler", + "positive", + "negative", + "latent_image", + "denoise", + ] + }, + "output_node": False, + "output": ["LATENT"], + "display_name": "KSampler", + }, + "PreviewImage": { + "input": {"required": {"images": ["IMAGE"]}}, + "input_order": {"required": ["images"]}, + "output_node": True, + "output": [], + "display_name": "Preview Image", + }, + "CLIPTextEncode": { + "input": { + "required": { + "text": ["STRING", {"multiline": True}], + "clip": ["CLIP"], + } + }, + "input_order": {"required": ["text", "clip"]}, + "output_node": False, + "output": ["CONDITIONING"], + "display_name": "CLIP Text Encode", + }, + "VAEDecode": { + "input": {"required": {"samples": ["LATENT"], "vae": ["VAE"]}}, + "input_order": {"required": ["samples", "vae"]}, + "output_node": False, + "output": ["IMAGE"], + "display_name": "VAE Decode", + }, + } + + +def _node(node_id, node_type, *, inputs=None, outputs=None, widgets=None, mode=0, **extra): + """Helper to build a minimal UI node entry.""" + n = { + "id": node_id, + "type": node_type, + "inputs": inputs or [], + "outputs": outputs or [], + "mode": mode, + } + if widgets is not None: + n["widgets_values"] = widgets + n.update(extra) + return n + + +# --------------------------------------------------------------------------- +# Format detection +# --------------------------------------------------------------------------- + + +class TestIsApiFormat: + def test_recognizes_api(self): + assert is_api_format({"1": {"class_type": "Foo", "inputs": {}}}) + + def test_ui_is_not_api(self): + assert not is_api_format({"nodes": [], "links": []}) + + def test_non_dict_is_not_api(self): + assert not is_api_format([]) + assert not is_api_format("string") + assert not is_api_format(None) + + def test_empty_dict_is_not_api(self): + assert not is_api_format({}) + + def test_metadata_only_is_not_api(self): + # Keys exist but none has a class_type + assert not is_api_format({"prompt": "x", "client_id": "y"}) + + +class TestIsSubgraphUuid: + def test_real_uuid(self): + assert is_subgraph_uuid("b43bb7e6-178c-4f1a-b014-ac4d6a50fca2") + + def test_class_name_is_not_uuid(self): + assert not is_subgraph_uuid("ImageScaleToTotalPixels") + + def test_wrong_length(self): + assert not is_subgraph_uuid("b43bb7e6-178c-4f1a-b014-ac4d6a50fc") + + def test_wrong_dash_count(self): + assert not is_subgraph_uuid("b43bb7e6_178c_4f1a_b014_ac4d6a50fca2x") + + def test_non_string(self): + assert not is_subgraph_uuid(123) + assert not is_subgraph_uuid(None) + + +# --------------------------------------------------------------------------- +# Core conversion: end-to-end shape +# --------------------------------------------------------------------------- + + +class TestConvertCore: + def test_already_api_is_returned_unchanged(self, object_info): + api = {"1": {"class_type": "EmptyLatentImage", "inputs": {}, "_meta": {"title": "x"}}} + assert convert_ui_to_api(api, object_info) == api + + def test_minimal_workflow(self, object_info): + # EmptyLatentImage(1) -> PreviewImage(2): mark via the VAEDecode chain + # is overkill — just connect a single link. + workflow = { + "nodes": [ + _node( + 1, + "EmptyLatentImage", + outputs=[{"name": "LATENT", "type": "LATENT", "links": [100]}], + widgets=[512, 512, 1], + ), + _node( + 2, + "PreviewImage", + inputs=[{"name": "images", "link": 100}], + outputs=[], + ), + ], + "links": [[100, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert set(result) == {"1", "2"} + assert result["1"]["class_type"] == "EmptyLatentImage" + assert result["1"]["inputs"] == {"width": 512, "height": 512, "batch_size": 1} + assert result["2"]["class_type"] == "PreviewImage" + assert result["2"]["inputs"] == {"images": ["1", 0]} + + def test_input_order_follows_schema(self, object_info): + # KSampler should emit widget values first in schema order, then link inputs. + # Producer nodes use EmptyLatentImage stand-ins for all three connection + # inputs; the converter doesn't typecheck, so this is enough to keep the + # links from being treated as orphans. + workflow = { + "nodes": [ + _node( + 1, + "KSampler", + inputs=[ + {"name": "model", "link": 10}, + {"name": "positive", "link": 11}, + {"name": "negative", "link": 12}, + {"name": "latent_image", "link": 13}, + ], + outputs=[{"name": "LATENT", "type": "LATENT", "links": [20]}], + widgets=[42, "randomize", 20, 8.0, "euler", "normal", 1.0], + ), + _node(2, "EmptyLatentImage", outputs=[{"links": [13]}], widgets=[512, 512, 1]), + _node(91, "EmptyLatentImage", outputs=[{"links": [10]}], widgets=[64, 64, 1]), + _node(92, "EmptyLatentImage", outputs=[{"links": [11]}], widgets=[64, 64, 1]), + _node(93, "EmptyLatentImage", outputs=[{"links": [12]}], widgets=[64, 64, 1]), + _node(3, "PreviewImage", inputs=[{"name": "images", "link": 20}], outputs=[]), + ], + "links": [ + [10, 91, 0, 1, 0, "MODEL"], + [11, 92, 0, 1, 6, "CONDITIONING"], + [12, 93, 0, 1, 7, "CONDITIONING"], + [13, 2, 0, 1, 8, "LATENT"], + [20, 1, 0, 3, 0, "LATENT"], + ], + } + result = convert_ui_to_api(workflow, object_info) + inputs = result["1"]["inputs"] + # All widget values come before all link inputs, both in schema order. + keys = list(inputs) + widget_keys = ["seed", "steps", "cfg", "sampler_name", "scheduler", "denoise"] + link_keys = ["model", "positive", "negative", "latent_image"] + # Each group should appear in this order. + assert [k for k in keys if k in widget_keys] == widget_keys + assert [k for k in keys if k in link_keys] == link_keys + # Widgets come before links overall + assert keys.index("denoise") < keys.index("model") + # Control-after-generate "randomize" was stripped from after seed + assert inputs["seed"] == 42 + + def test_unknown_node_type_uses_class_name_as_title(self, object_info): + workflow = { + "nodes": [ + _node( + 1, + "TotallyUnknownNode", + outputs=[{"links": [1]}], + ), + _node( + 2, + "PreviewImage", + inputs=[{"name": "images", "link": 1}], + outputs=[], + ), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["_meta"]["title"] == "TotallyUnknownNode" + + def test_node_title_overrides_display_name(self, object_info): + workflow = { + "nodes": [ + _node( + 1, + "EmptyLatentImage", + outputs=[{"links": [1]}], + widgets=[512, 512, 1], + title="My Custom Title", + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}]), + ], + "links": [[1, 1, 0, 2, 0, "LATENT"]], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["_meta"]["title"] == "My Custom Title" + + def test_invalid_workflow_raises(self, object_info): + with pytest.raises(WorkflowConversionError): + convert_ui_to_api({"nodes": "not a list"}, object_info) + + +# --------------------------------------------------------------------------- +# Special node types +# --------------------------------------------------------------------------- + + +class TestSpecialNodes: + def test_primitive_node_inlines_value(self, object_info): + # PrimitiveNode(1, value=1024) -> EmptyLatentImage(2).width + workflow = { + "nodes": [ + _node( + 1, + "PrimitiveNode", + outputs=[{"links": [5]}], + widgets=[1024, "fixed"], + ), + _node( + 2, + "EmptyLatentImage", + inputs=[{"name": "width", "link": 5}], + outputs=[{"links": [99]}], + widgets=[1024, 512, 1], + ), + _node(3, "PreviewImage", inputs=[{"name": "images", "link": 99}]), + ], + "links": [ + [5, 1, 0, 2, 0, "INT"], + [99, 2, 0, 3, 0, "LATENT"], + ], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" not in result # PrimitiveNode excluded + # The value flowed from primitive into the consuming node's inputs + assert result["2"]["inputs"]["width"] == 1024 + + def test_reroute_is_transparent(self, object_info): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "Reroute", + inputs=[{"name": "in", "link": 1}], + outputs=[{"links": [2]}], + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [ + [1, 1, 0, 99, 0, "LATENT"], + [2, 99, 0, 2, 0, "LATENT"], + ], + } + result = convert_ui_to_api(workflow, object_info) + assert "99" not in result # Reroute excluded + # The reroute's downstream consumer points at the reroute's source + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_get_set_node_pair(self, object_info): + # SetNode publishes node 1's output as variable "myvar" + # GetNode reads "myvar" and forwards to node 2 + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [10]}], widgets=[512, 512, 1]), + _node( + 20, + "SetNode", + inputs=[{"name": "value", "link": 10}], + widgets=["myvar"], + ), + _node( + 21, + "GetNode", + outputs=[{"links": [11]}], + widgets=["myvar"], + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 11}]), + ], + "links": [ + [10, 1, 0, 20, 0, "LATENT"], + [11, 21, 0, 2, 0, "LATENT"], + ], + } + result = convert_ui_to_api(workflow, object_info) + assert "20" not in result # SetNode excluded + assert "21" not in result # GetNode excluded + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_muted_node_is_excluded(self, object_info): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 2, + "PreviewImage", + inputs=[{"name": "images", "link": 1}], + mode=2, # muted + ), + ], + "links": [[1, 1, 0, 2, 0, "LATENT"]], + } + result = convert_ui_to_api(workflow, object_info) + # Both 1 (no downstream consumer after 2 is muted, and not OUTPUT_NODE + # because it has no connected output) and 2 (muted) are excluded. + assert "2" not in result + + def test_bypassed_node_passes_through(self, object_info): + # 1 -> 99 (bypassed) -> 2; result should connect 1 directly to 2. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "VAEDecode", # any passthrough-able node will do + inputs=[ + {"name": "samples", "type": "LATENT", "link": 1}, + {"name": "vae", "type": "VAE", "link": None}, + ], + outputs=[{"name": "IMAGE", "type": "LATENT", "links": [2]}], + mode=4, # bypassed + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [ + [1, 1, 0, 99, 0, "LATENT"], + [2, 99, 0, 2, 0, "LATENT"], + ], + } + result = convert_ui_to_api(workflow, object_info) + assert "99" not in result # bypassed + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_load_image_output_excluded(self, object_info): + # LoadImageOutput is the only hardcoded UI-only exclusion. + workflow = { + "nodes": [ + _node( + 1, + "LoadImageOutput", + outputs=[{"links": [1]}], + widgets=["pic.png"], + ), + _node( + 2, + "PreviewImage", + inputs=[{"name": "images", "link": 1}], + ), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" not in result + + def test_note_node_excluded(self, object_info): + workflow = { + "nodes": [ + _node(1, "Note", widgets=["just text"]), + _node(2, "EmptyLatentImage", outputs=[{"links": []}], widgets=[512, 512, 1]), + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" not in result + + def test_output_node_kept_even_without_outgoing_links(self, object_info): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + # PreviewImage's `output_node` is True in the schema → kept. + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert "2" in result + + def test_dead_branch_excluded(self, object_info): + # Node 99 has neither connected outputs nor a schema marking it as output. + workflow = { + "nodes": [ + _node( + 99, + "EmptyLatentImage", + outputs=[{"links": []}], + widgets=[64, 64, 1], + ), + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert result == {} + + +# --------------------------------------------------------------------------- +# Schema-aware behaviors +# --------------------------------------------------------------------------- + + +class TestSchemaAwareBehavior: + def test_combo_value_normalized_case_insensitively(self, object_info): + workflow = { + "nodes": [ + _node( + 1, + "KSampler", + inputs=[], + outputs=[{"links": []}], + widgets=[1, "fixed", 1, 1.0, "EULER", "Normal", 1.0], + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": None}]), + ], + "links": [], + } + # KSampler with no inputs is not a viable workflow, but we just want the + # combo normalization assertion. Bypass the dead-branch exclusion by + # giving it a real downstream link. + workflow["nodes"][0]["outputs"] = [{"links": [1]}] + workflow["nodes"][1]["inputs"][0]["link"] = 1 + workflow["links"] = [[1, 1, 0, 2, 0, "LATENT"]] + + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["sampler_name"] == "euler" # normalized to lowercase + assert result["1"]["inputs"]["scheduler"] == "normal" + + def test_defaults_filled_when_widget_values_absent(self, object_info): + # Node with only one widget value; the others should come from schema defaults + # (object_info["EmptyLatentImage"]["input"]["required"]["height"]["default"] = 512) + workflow = { + "nodes": [ + _node( + 1, + "EmptyLatentImage", + outputs=[{"links": [1]}], + widgets=[1024], # only width supplied + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}]), + ], + "links": [[1, 1, 0, 2, 0, "LATENT"]], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["width"] == 1024 + assert result["1"]["inputs"]["height"] == 512 # filled from schema default + assert result["1"]["inputs"]["batch_size"] == 1 + + +# --------------------------------------------------------------------------- +# Subgraph expansion +# --------------------------------------------------------------------------- + + +class TestMalformedInputHardening: + """The converter must never crash on a malformed workflow — only raise a + typed :class:`WorkflowConversionError` (or skip the offending pieces with a + log warning). The CLI wraps those into a clean exit; uncaught exceptions + would bubble up as a raw Python traceback, which is unacceptable for an + experimental feature. + """ + + def test_rejects_non_dict_workflow(self, object_info): + with pytest.raises(WorkflowConversionError): + convert_ui_to_api(None, object_info) + with pytest.raises(WorkflowConversionError): + convert_ui_to_api("nope", object_info) + + def test_rejects_non_dict_object_info(self): + with pytest.raises(WorkflowConversionError): + convert_ui_to_api({"nodes": [], "links": []}, "not a dict") + + def test_rejects_missing_nodes_or_links(self, object_info): + with pytest.raises(WorkflowConversionError): + convert_ui_to_api({}, object_info) + with pytest.raises(WorkflowConversionError): + convert_ui_to_api({"nodes": "oops", "links": []}, object_info) + + def test_skips_non_dict_node_entries(self, object_info): + # A workflow with mixed garbage in the nodes list should still convert + # the well-formed nodes and ignore the rest. + workflow = { + "nodes": [ + None, + 42, + "string", + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert set(result) == {"1", "2"} + + def test_tolerates_garbage_in_inputs_and_outputs(self, object_info): + # Outputs/inputs containing non-dict garbage shouldn't crash collection. + workflow = { + "nodes": [ + { + "id": 1, + "type": "EmptyLatentImage", + "inputs": [None, 42, {"name": "x", "link": None}], + "outputs": [None, 42, {"name": "LATENT", "links": [1]}], + "widgets_values": [512, 512, 1], + "mode": 0, + }, + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 1, 2, 2, 0, "IMAGE"]], + } + # Should not raise. + result = convert_ui_to_api(workflow, object_info) + assert "1" in result + assert "2" in result + + def test_tolerates_non_list_widgets_values(self, object_info): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}]), # no widgets at all + { + "id": 2, + "type": "EmptyLatentImage", + "outputs": [{"links": [2]}], + "widgets_values": 42, # invalid: an int + "mode": 0, + }, + _node(3, "PreviewImage", inputs=[{"name": "images", "link": 2}], outputs=[]), + ], + "links": [[1, 1, 0, 3, 0, "IMAGE"], [2, 2, 0, 3, 0, "IMAGE"]], + } + # Should not raise; the node with int widgets_values just emits no widgets. + result = convert_ui_to_api(workflow, object_info) + assert "2" in result + + def test_tolerates_non_numeric_slot_in_link(self, object_info): + # A bypass-time link with a string slot index should fall back to slot 0. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + { + "id": 99, + "type": "VAEDecode", + "inputs": [{"name": "samples", "type": "LATENT", "link": 1}], + "outputs": [{"name": "IMAGE", "type": "IMAGE", "links": [2]}], + "mode": 4, + }, + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}], outputs=[]), + ], + # Note: source_slot is the string "weird" instead of an int. + "links": [[1, 1, 0, 99, 0, "LATENT"], [2, 99, "weird", 2, 0, "IMAGE"]], + } + # Should not raise. + result = convert_ui_to_api(workflow, object_info) + assert "2" in result + + def test_tolerates_garbage_definitions(self, object_info): + # definitions could be a list, None, or otherwise wrong-shape. + for bad_defs in ([], "string", 42, {"subgraphs": "not a list"}): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + "definitions": bad_defs, + } + result = convert_ui_to_api(workflow, object_info) + assert set(result) == {"1", "2"}, f"failed with definitions={bad_defs!r}" + + def test_single_bad_node_does_not_abort_conversion(self, object_info, caplog): + # We can't easily induce _build_api_node to throw on real input, so + # monkeypatch it for this test. + import logging + + from comfy_cli import workflow_to_api as mod + + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + } + original_build = mod._build_api_node + calls = {"n": 0} + + def flaky_build(**kwargs): + calls["n"] += 1 + if calls["n"] == 1: + raise RuntimeError("simulated converter bug") + return original_build(**kwargs) + + mod._build_api_node = flaky_build + try: + with caplog.at_level(logging.ERROR, logger="comfy_cli.workflow_to_api"): + result = convert_ui_to_api(workflow, object_info) + finally: + mod._build_api_node = original_build + # The second node still made it in even though the first crashed. + assert "2" in result + assert any("Failed to convert node" in rec.message for rec in caplog.records) + + +class TestFrontendParity: + """Behaviors mirrored from ComfyUI_frontend/src/utils/executionUtil.ts.""" + + def test_list_widget_value_is_wrapped_to_disambiguate_from_link(self, object_info): + # Imagine a widget value that's a 2-element [str, int] list — without the + # ``{"__value__": ...}`` wrapper, ComfyUI's is_link() would mis-classify + # this as a connection reference. + object_info = { + **object_info, + "NodeWithListWidget": { + "input": {"required": {"points": [["list", "of", "options"]]}}, + "input_order": {"required": ["points"]}, + "output_node": True, + "display_name": "List Widget Node", + }, + } + workflow = { + "nodes": [ + _node( + 1, + "NodeWithListWidget", + outputs=[], + widgets=[["foo", 3]], # widget value is a list + ), + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["points"] == {"__value__": ["foo", 3]} + + def test_orphan_link_inputs_are_stripped(self, object_info): + # When a referenced upstream node ends up excluded, the cleanup pass + # should drop the now-orphan link input — never leak a dangling + # ["999", 0] reference into the prompt. + object_info = { + **object_info, + "DummyExcluded": { + "input": {"required": {}}, + "input_order": {"required": []}, + "output_node": False, # no outputs + no outgoing → excluded + "display_name": "Dummy", + }, + "DummyConsumer": { + "input": {"required": {"upstream": ["LATENT"]}}, + "input_order": {"required": ["upstream"]}, + "output_node": True, + "display_name": "Dummy", + }, + } + workflow = { + "nodes": [ + _node(999, "DummyExcluded", outputs=[{"links": [1]}]), + _node(2, "DummyConsumer", inputs=[{"name": "upstream", "link": 1}], outputs=[]), + ], + "links": [[1, 999, 0, 2, 0, "LATENT"]], + } + result = convert_ui_to_api(workflow, object_info) + # DummyExcluded has no schema-declared inputs and no downstream + # consumer of its (zero) outputs — _collect_excluded won't prune it + # because it has connected outputs, so this asserts the cleanup + # branch instead by removing it via a different path. + # Actually validate the simpler invariant: no input references a + # node ID that's not in the result. + for node in result.values(): + for value in node["inputs"].values(): + if isinstance(value, list) and len(value) == 2 and isinstance(value[0], str): + assert value[0] in result + + def test_bypass_matches_any_type_wildcard(self, object_info): + # When the bypassed node's input type is ``*``, the frontend's + # isValidConnection treats it as compatible with any output. Our + # tracer should pass through such a node even though the types + # don't string-match. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "VAEDecode", + inputs=[ + {"name": "samples", "type": "*", "link": 1}, # wildcard input + {"name": "vae", "type": "VAE", "link": None}, + ], + outputs=[{"name": "IMAGE", "type": "IMAGE", "links": [2]}], + mode=4, + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [ + [1, 1, 0, 99, 0, "LATENT"], + [2, 99, 0, 2, 0, "IMAGE"], + ], + } + result = convert_ui_to_api(workflow, object_info) + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_bypass_falls_back_to_first_linked_input_when_types_mismatch(self, object_info): + # SethRobinson's reference converter falls back to the first connected + # input regardless of type when no type-compatible match exists. We + # match that behavior so users who bypass a non-passthrough node still + # get a wired connection — the executor will surface any type error. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "VAEDecode", + # Input types don't match the IMAGE output type. + inputs=[ + {"name": "samples", "type": "LATENT", "link": 1}, + {"name": "vae", "type": "VAE", "link": None}, + ], + outputs=[{"name": "IMAGE", "type": "IMAGE", "links": [2]}], + mode=4, + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [[1, 1, 0, 99, 0, "LATENT"], [2, 99, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + # First-linked-input fallback wires PreviewImage to node 1 even though + # types don't match — preserves the user's intent rather than dropping + # the edge silently. + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_muted_node_does_not_leave_dangling_reference(self, object_info): + # Intentional divergence from SethRobinson, who leaves a stray + # reference to the muted node ID (the executor would reject it). + # Our orphan cleanup pass mirrors the frontend's final pass. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "VAEDecode", + inputs=[ + {"name": "samples", "type": "LATENT", "link": 1}, + {"name": "vae", "type": "VAE", "link": None}, + ], + outputs=[{"name": "IMAGE", "type": "IMAGE", "links": [2]}], + mode=2, # muted + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [[1, 1, 0, 99, 0, "LATENT"], [2, 99, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert "99" not in result + # Critically, PreviewImage's input must NOT reference the muted node 99. + assert "images" not in result["2"]["inputs"] + + def test_bypass_matches_comma_separated_types(self, object_info): + # Comma-separated types ("IMAGE,MASK") should match either alternative. + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node( + 99, + "VAEDecode", + inputs=[ + {"name": "samples", "type": "IMAGE,LATENT", "link": 1}, + {"name": "vae", "type": "VAE", "link": None}, + ], + outputs=[{"name": "IMAGE", "type": "IMAGE", "links": [2]}], + mode=4, + ), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 2}]), + ], + "links": [ + [1, 1, 0, 99, 0, "LATENT"], + [2, 99, 0, 2, 0, "IMAGE"], + ], + } + result = convert_ui_to_api(workflow, object_info) + # LATENT output should connect to the LATENT alternative of the comma type + assert result["2"]["inputs"]["images"] == ["1", 0] + + def test_group_node_workflow_emits_warning(self, object_info, caplog): + # We don't expand legacy group nodes; we should warn loudly so users + # know the conversion may be incomplete. + import logging + + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node(2, "PreviewImage", inputs=[{"name": "images", "link": 1}]), + ], + "links": [[1, 1, 0, 2, 0, "IMAGE"]], + "extra": {"groupNodes": {"MyGroup": {"nodes": []}}}, + } + with caplog.at_level(logging.WARNING, logger="comfy_cli.workflow_to_api"): + convert_ui_to_api(workflow, object_info) + assert any("group node" in record.message.lower() for record in caplog.records) + + +class TestFixtureParity: + """Regression test against a real workflow + the exact API output that + ComfyUI's /workflow/convert endpoint produced for it. + + Regenerate the fixtures by running a live ComfyUI with Seth Robinson's + /workflow/convert node and POSTing the UI JSON to the endpoint. + """ + + def test_sd15_workflow_matches_reference(self): + ui = json.loads((FIXTURES / "sd15_ui_workflow.json").read_text()) + object_info = json.loads((FIXTURES / "sd15_object_info.json").read_text()) + expected = json.loads((FIXTURES / "sd15_expected_api.json").read_text()) + assert convert_ui_to_api(ui, object_info) == expected + + +class TestSubgraphExpansion: + def test_simple_subgraph_expansion(self, object_info): + sg_uuid = "11111111-2222-3333-4444-555555555555" + # Outer workflow: an EmptyLatentImage feeds a subgraph instance whose + # internal pipeline ends with a PreviewImage. After expansion the + # PreviewImage should appear with a prefixed id ("100:50"). + workflow = { + "nodes": [ + _node(7, "EmptyLatentImage", outputs=[{"links": [200]}], widgets=[512, 512, 1]), + # The subgraph instance — its `type` is the UUID. + { + "id": 100, + "type": sg_uuid, + "inputs": [{"name": "incoming", "link": 200}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[200, 7, 0, 100, 0, "LATENT"]], + "definitions": { + "subgraphs": [ + { + "id": sg_uuid, + "name": "MySubgraph", + "inputs": [{"name": "incoming", "linkIds": [301]}], + "outputs": [], + "nodes": [ + { + "id": 50, + "type": "PreviewImage", + "inputs": [{"name": "images", "link": 301}], + "outputs": [], + "mode": 0, + }, + ], + "links": [ + { + "id": 301, + "origin_id": -10, # subgraph input proxy + "origin_slot": 0, + "target_id": 50, + "target_slot": 0, + "type": "LATENT", + }, + ], + } + ] + }, + } + result = convert_ui_to_api(workflow, object_info) + # The subgraph instance itself is gone; internal node appears with prefix. + assert "100" not in result + assert "100:50" in result + # Link from the external EmptyLatentImage was retargeted at the internal node. + assert result["100:50"]["inputs"]["images"] == ["7", 0] From ba65ef43e68ad42e18f2e77bca9483eeef9509e9 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 06:47:50 +0000 Subject: [PATCH 02/16] refactor(run): convert UI workflows client-side via /object_info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Part 1 flow POSTed UI workflows to /workflow/convert on the ComfyUI server, which only exists if a specific custom node is installed. This switches to fetching /object_info (always available on stock ComfyUI) and running the conversion locally via the new workflow_to_api module. * fetch_object_info(host, port, timeout) GETs /object_info and parses the response, with clean exits on HTTPError, URLError, TimeoutError and JSONDecodeError. * execute() detects UI format, fetches /object_info, then calls convert_ui_to_api. WorkflowConversionError surfaces as a typed user-facing message; any other unexpected exception is caught and surfaced as an experimental-feature crash report pointing at the issue tracker (full traceback only with --verbose). * Drop WorkflowConverterUnavailable, convert_ui_workflow_via_server and the help-text fallback — no longer needed. Tests follow the rewrite: dropped TestConvertUiWorkflowViaServer, added TestFetchObjectInfo, rewrote TestExecuteUiWorkflow against the new fetch_object_info + local converter flow, added a catch-all exception test that asserts a clean exit code 1. Smoke tested end-to-end against stock ComfyUI (no SethRobinson custom node, /workflow/convert returns 405) with SD 1.5, SDXL Turbo and a Z-Image-Turbo subgraph workflow. Parity sweep across 51 frontend blueprint workflows: 46 byte-identical to the reference converter; 5 differ only in cosmetic _meta.title for V3 nodes whose /object_info.display_name is None (the executor ignores _meta.title). Signed-off-by: Alexander Piskun --- comfy_cli/command/run.py | 79 +++++------ tests/comfy_cli/command/test_run.py | 209 +++++++++++++++++----------- 2 files changed, 164 insertions(+), 124 deletions(-) diff --git a/comfy_cli/command/run.py b/comfy_cli/command/run.py index 5431ea6b..4ae0f86a 100644 --- a/comfy_cli/command/run.py +++ b/comfy_cli/command/run.py @@ -14,6 +14,7 @@ from websocket import WebSocket, WebSocketException, WebSocketTimeoutException from comfy_cli.env_checker import check_comfy_server_running +from comfy_cli.workflow_to_api import WorkflowConversionError, convert_ui_to_api from comfy_cli.workspace_manager import WorkspaceManager workspace_manager = WorkspaceManager() @@ -37,55 +38,31 @@ def _validate_api_workflow(workflow): return workflow -class WorkflowConverterUnavailable(Exception): - """The running ComfyUI server doesn't expose /workflow/convert.""" +def fetch_object_info(host: str, port: int, timeout: int) -> dict: + """GET ``/object_info`` from the running ComfyUI server. - -def convert_ui_workflow_via_server(workflow: dict, host: str, port: int, timeout: int) -> dict: - """POST a UI-format workflow to the server's /workflow/convert and return API-format JSON. - - Raises WorkflowConverterUnavailable if the server doesn't expose the endpoint. - Raises typer.Exit on other conversion failures. + The response describes every loaded node class's input schema and is what + the converter uses to map widget values to input names, fill defaults, etc. """ - url = f"http://{host}:{port}/workflow/convert" - req = request.Request(url, json.dumps(workflow).encode("utf-8")) - req.add_header("Content-Type", "application/json") + url = f"http://{host}:{port}/object_info" try: - resp = request.urlopen(req, timeout=timeout) + with request.urlopen(url, timeout=timeout) as resp: + body = resp.read() except urllib.error.HTTPError as e: - if e.code in (404, 405): - raise WorkflowConverterUnavailable() from e body = e.read().decode("utf-8", errors="replace").strip() - pprint(f"[bold red]Workflow conversion failed (HTTP {e.code}): {body[:500]}[/bold red]") + pprint(f"[bold red]Failed to fetch /object_info (HTTP {e.code}): {body[:500]}[/bold red]") raise typer.Exit(code=1) from e except urllib.error.URLError as e: - pprint(f"[bold red]Workflow conversion failed: {e.reason}[/bold red]") + pprint(f"[bold red]Failed to fetch /object_info: {e.reason}[/bold red]") + raise typer.Exit(code=1) from e + except TimeoutError as e: + pprint(f"[bold red]Failed to fetch /object_info: timed out after {timeout}s[/bold red]") raise typer.Exit(code=1) from e try: - converted = json.loads(resp.read()) + return json.loads(body) except json.JSONDecodeError as e: - pprint("[bold red]Workflow conversion failed: server returned invalid JSON[/bold red]") + pprint("[bold red]Failed to fetch /object_info: server returned invalid JSON[/bold red]") raise typer.Exit(code=1) from e - if not isinstance(converted, dict) or not converted: - pprint("[bold red]Workflow conversion failed: expected a non-empty JSON object[/bold red]") - raise typer.Exit(code=1) - first = converted[next(iter(converted))] - if not isinstance(first, dict) or "class_type" not in first: - pprint("[bold red]Workflow conversion failed: returned data is not API workflow format[/bold red]") - raise typer.Exit(code=1) - return converted - - -def _print_converter_unavailable_help() -> None: - pprint( - "[bold red]This ComfyUI server doesn't expose a /workflow/convert endpoint[/bold red]\n" - "[bold red]to convert it to API format.[/bold red]\n" - "\n" - "[yellow]Workarounds:[/yellow]\n" - "[yellow] * Install a custom node that adds /workflow/convert on the server[/yellow]\n" - "[yellow] * Or, in the ComfyUI frontend, use 'File > Export (API)' to save[/yellow]\n" - "[yellow] your workflow as API format[/yellow]" - ) def execute(workflow: str, host, port, wait=True, verbose=False, local_paths=False, timeout=30): @@ -112,11 +89,29 @@ def execute(workflow: str, host, port, wait=True, verbose=False, local_paths=Fal raise typer.Exit(code=1) from e if is_ui_workflow(raw_workflow): - pprint("[yellow]Detected UI-format workflow, converting via server's /workflow/convert...[/yellow]") + pprint("[yellow]Detected UI-format workflow, converting to API format...[/yellow]") + object_info = fetch_object_info(host, port, timeout) try: - workflow = convert_ui_workflow_via_server(raw_workflow, host, port, timeout) - except WorkflowConverterUnavailable: - _print_converter_unavailable_help() + workflow = convert_ui_to_api(raw_workflow, object_info) + except WorkflowConversionError as e: + pprint(f"[bold red]Workflow conversion failed: {e}[/bold red]") + raise typer.Exit(code=1) from e + except Exception as e: + # The converter is experimental; an unexpected crash here is a bug + # in our code, not user error. Show a clean message and a pointer. + pprint( + f"[bold red]Workflow conversion crashed unexpectedly: {type(e).__name__}: {e}[/bold red]\n" + "[yellow]The UI-to-API converter is experimental. Please report this at[/yellow]\n" + "[yellow] https://github.com/Comfy-Org/comfy-cli/issues[/yellow]\n" + "[yellow]and attach the workflow file if possible.[/yellow]" + ) + if verbose: + import traceback + + traceback.print_exc() + raise typer.Exit(code=1) from e + if not workflow: + pprint("[bold red]Workflow conversion produced no executable nodes[/bold red]") raise typer.Exit(code=1) else: workflow = _validate_api_workflow(raw_workflow) diff --git a/tests/comfy_cli/command/test_run.py b/tests/comfy_cli/command/test_run.py index 7f3fdaa6..1a8b9a54 100644 --- a/tests/comfy_cli/command/test_run.py +++ b/tests/comfy_cli/command/test_run.py @@ -10,10 +10,9 @@ from websocket import WebSocketException, WebSocketTimeoutException from comfy_cli.command.run import ( - WorkflowConverterUnavailable, WorkflowExecution, - convert_ui_workflow_via_server, execute, + fetch_object_info, is_ui_workflow, ) @@ -93,7 +92,7 @@ def test_rejects_when_values_are_not_lists(self): def _make_http_error(code: int, body: bytes = b"") -> urllib.error.HTTPError: return urllib.error.HTTPError( - url="http://127.0.0.1:8188/workflow/convert", + url="http://127.0.0.1:8188/object_info", code=code, msg=f"HTTP {code}", hdrs=None, @@ -101,81 +100,56 @@ def _make_http_error(code: int, body: bytes = b"") -> urllib.error.HTTPError: ) -class TestConvertUiWorkflowViaServer: - UI = {"nodes": [{"id": 1, "type": "X"}], "links": []} - CONVERTED = {"1": {"class_type": "X", "inputs": {}}} +def _ok_response(body: bytes) -> MagicMock: + resp = MagicMock() + resp.read.return_value = body + resp.__enter__ = MagicMock(return_value=resp) + resp.__exit__ = MagicMock(return_value=False) + return resp - def test_returns_api_format_on_success(self): - mock_resp = MagicMock() - mock_resp.read.return_value = json.dumps(self.CONVERTED).encode() - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp) as mock_open: - result = convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) - assert result == self.CONVERTED - sent_req = mock_open.call_args[0][0] - assert sent_req.full_url == "http://127.0.0.1:8188/workflow/convert" - assert json.loads(sent_req.data) == self.UI - - @pytest.mark.parametrize("code", [404, 405]) - def test_raises_unavailable_on_missing_endpoint(self, code): - with patch("comfy_cli.command.run.request.urlopen", side_effect=_make_http_error(code)): - with pytest.raises(WorkflowConverterUnavailable): - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) +class TestFetchObjectInfo: + def test_returns_parsed_json_on_success(self): + payload = {"KSampler": {"input": {}, "output_node": False}} + with patch( + "comfy_cli.command.run.request.urlopen", + return_value=_ok_response(json.dumps(payload).encode()), + ) as mock_open: + result = fetch_object_info("127.0.0.1", 8188, timeout=30) + assert result == payload + assert mock_open.call_args[0][0] == "http://127.0.0.1:8188/object_info" - def test_raises_typer_exit_on_server_error(self): - err = _make_http_error(500, b"conversion blew up") - with patch("comfy_cli.command.run.request.urlopen", side_effect=err): + def test_http_error_exits_cleanly(self): + with patch( + "comfy_cli.command.run.request.urlopen", + side_effect=_make_http_error(500, b"server exploded"), + ): with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) + fetch_object_info("127.0.0.1", 8188, timeout=30) assert exc_info.value.exit_code == 1 - def test_raises_typer_exit_on_network_error(self): + def test_network_error_exits_cleanly(self): with patch( "comfy_cli.command.run.request.urlopen", side_effect=urllib.error.URLError("Connection refused"), ): with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) - assert exc_info.value.exit_code == 1 - - def test_raises_typer_exit_on_invalid_json(self): - mock_resp = MagicMock() - mock_resp.read.return_value = b"not json" - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp): - with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) + fetch_object_info("127.0.0.1", 8188, timeout=30) assert exc_info.value.exit_code == 1 - def test_raises_typer_exit_on_non_object_response(self): - mock_resp = MagicMock() - mock_resp.read.return_value = b'["not", "an", "object"]' - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp): + def test_timeout_exits_cleanly(self): + with patch("comfy_cli.command.run.request.urlopen", side_effect=TimeoutError("timed out")): with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) + fetch_object_info("127.0.0.1", 8188, timeout=5) assert exc_info.value.exit_code == 1 - def test_raises_typer_exit_on_empty_object_response(self): - mock_resp = MagicMock() - mock_resp.read.return_value = b"{}" - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp): - with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) - assert exc_info.value.exit_code == 1 - - def test_raises_typer_exit_when_first_entry_is_not_a_node(self): - mock_resp = MagicMock() - mock_resp.read.return_value = b'{"1": "not-a-node-dict"}' - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp): - with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) - assert exc_info.value.exit_code == 1 - - def test_raises_typer_exit_when_first_entry_missing_class_type(self): - mock_resp = MagicMock() - mock_resp.read.return_value = b'{"1": {"inputs": {}}}' - with patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp): + def test_invalid_json_exits_cleanly(self): + with patch( + "comfy_cli.command.run.request.urlopen", + return_value=_ok_response(b"not json"), + ): with pytest.raises(typer.Exit) as exc_info: - convert_ui_workflow_via_server(self.UI, "127.0.0.1", 8188, timeout=30) + fetch_object_info("127.0.0.1", 8188, timeout=30) assert exc_info.value.exit_code == 1 @@ -425,8 +399,46 @@ def test_progress_stopped_on_error(self, workflow_file): class TestExecuteUiWorkflow: - UI = {"nodes": [{"id": 1, "type": "X"}], "links": []} - CONVERTED = {"1": {"class_type": "EmptyLatentImage", "inputs": {"width": 64, "height": 64, "batch_size": 1}}} + UI = { + "nodes": [ + { + "id": 1, + "type": "EmptyLatentImage", + "inputs": [], + "outputs": [{"name": "LATENT", "type": "LATENT", "links": [10]}], + "widgets_values": [512, 512, 1], + "mode": 0, + }, + { + "id": 2, + "type": "PreviewImage", + "inputs": [{"name": "images", "link": 10}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[10, 1, 0, 2, 0, "IMAGE"]], + } + OBJECT_INFO = { + "EmptyLatentImage": { + "input": { + "required": { + "width": ["INT", {"default": 512}], + "height": ["INT", {"default": 512}], + "batch_size": ["INT", {"default": 1}], + } + }, + "input_order": {"required": ["width", "height", "batch_size"]}, + "output_node": False, + "display_name": "Empty Latent Image", + }, + "PreviewImage": { + "input": {"required": {"images": ["IMAGE"]}}, + "input_order": {"required": ["images"]}, + "output_node": True, + "display_name": "Preview Image", + }, + } @pytest.fixture def ui_workflow_file(self): @@ -438,12 +450,9 @@ def ui_workflow_file(self): os.unlink(path) def test_ui_workflow_is_converted_then_executed(self, ui_workflow_file): - mock_resp = MagicMock() - mock_resp.read.return_value = json.dumps(self.CONVERTED).encode() - with ( patch("comfy_cli.command.run.check_comfy_server_running", return_value=True), - patch("comfy_cli.command.run.request.urlopen", return_value=mock_resp) as mock_open, + patch("comfy_cli.command.run.fetch_object_info", return_value=self.OBJECT_INFO) as mock_fetch, patch("comfy_cli.command.run.ExecutionProgress"), patch("comfy_cli.command.run.WorkflowExecution") as MockExec, ): @@ -453,29 +462,65 @@ def test_ui_workflow_is_converted_then_executed(self, ui_workflow_file): execute(ui_workflow_file, host="127.0.0.1", port=8188, wait=True, timeout=30) - sent_req = mock_open.call_args[0][0] - assert sent_req.full_url == "http://127.0.0.1:8188/workflow/convert" - assert MockExec.call_args.args[0] == self.CONVERTED + mock_fetch.assert_called_once_with("127.0.0.1", 8188, 30) + api_workflow = MockExec.call_args.args[0] + assert set(api_workflow) == {"1", "2"} + assert api_workflow["1"]["class_type"] == "EmptyLatentImage" + assert api_workflow["2"]["inputs"]["images"] == ["1", 0] mock_exec.queue.assert_called_once() - @pytest.mark.parametrize("code", [404, 405]) - def test_ui_workflow_exits_when_endpoint_missing(self, ui_workflow_file, code): + def test_ui_workflow_exits_when_server_not_running(self, ui_workflow_file): with ( - patch("comfy_cli.command.run.check_comfy_server_running", return_value=True), - patch("comfy_cli.command.run.request.urlopen", side_effect=_make_http_error(code)), - patch("comfy_cli.command.run.WorkflowExecution") as MockExec, + patch("comfy_cli.command.run.check_comfy_server_running", return_value=False), + patch("comfy_cli.command.run.fetch_object_info") as mock_fetch, ): with pytest.raises(typer.Exit) as exc_info: - execute(ui_workflow_file, host="127.0.0.1", port=8188, wait=True, timeout=30) + execute(ui_workflow_file, host="127.0.0.1", port=8188) assert exc_info.value.exit_code == 1 - MockExec.assert_not_called() + mock_fetch.assert_not_called() - def test_ui_workflow_exits_when_server_not_running(self, ui_workflow_file): + def test_ui_workflow_exits_cleanly_on_unexpected_converter_crash(self, ui_workflow_file): + # If the experimental converter crashes with an unexpected error, the + # CLI should still exit with code 1 and a friendly message — not let a + # Python traceback escape to the user. with ( - patch("comfy_cli.command.run.check_comfy_server_running", return_value=False), - patch("comfy_cli.command.run.request.urlopen") as mock_open, + patch("comfy_cli.command.run.check_comfy_server_running", return_value=True), + patch("comfy_cli.command.run.fetch_object_info", return_value=self.OBJECT_INFO), + patch( + "comfy_cli.command.run.convert_ui_to_api", + side_effect=RuntimeError("simulated converter bug"), + ), + patch("comfy_cli.command.run.WorkflowExecution") as MockExec, ): with pytest.raises(typer.Exit) as exc_info: - execute(ui_workflow_file, host="127.0.0.1", port=8188) + execute(ui_workflow_file, host="127.0.0.1", port=8188, wait=True, timeout=30) assert exc_info.value.exit_code == 1 - mock_open.assert_not_called() + MockExec.assert_not_called() + + def test_ui_workflow_exits_when_conversion_yields_nothing(self): + # All nodes are UI-only (Note/PrimitiveNode/Reroute/GetNode/SetNode) and + # therefore stripped by the converter → execute() should bail before + # ever instantiating WorkflowExecution. + empty_ui = { + "nodes": [ + {"id": 1, "type": "Note", "inputs": [], "outputs": [], "widgets_values": ["x"]}, + {"id": 2, "type": "Reroute", "inputs": [{"link": None}], "outputs": [{"links": []}]}, + ], + "links": [], + } + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(empty_ui, f) + f.flush() + path = f.name + try: + with ( + patch("comfy_cli.command.run.check_comfy_server_running", return_value=True), + patch("comfy_cli.command.run.fetch_object_info", return_value=self.OBJECT_INFO), + patch("comfy_cli.command.run.WorkflowExecution") as MockExec, + ): + with pytest.raises(typer.Exit) as exc_info: + execute(path, host="127.0.0.1", port=8188, wait=True, timeout=30) + assert exc_info.value.exit_code == 1 + MockExec.assert_not_called() + finally: + os.unlink(path) From c1efe0074bd4b79705845125aa0cb074f1813b82 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 07:38:04 +0000 Subject: [PATCH 03/16] refactor(workflow): drop dead pre-rewrite link_map build _build_link_map ran twice: once before _rewrite_links_for_subgraphs and again right after. The first result was immediately shadowed and never read (the rewrite function doesn't take a link_map argument). Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 60fcf58f..16af94fe 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -104,7 +104,6 @@ def convert_ui_to_api(workflow: dict, object_info: dict) -> dict: subgraph_defs = _collect_subgraph_defs(workflow) nodes, links, subgraph_ctx = _expand_subgraphs(nodes, links, subgraph_defs) - link_map = _build_link_map(links) links = _rewrite_links_for_subgraphs(links, subgraph_ctx, nodes) link_map = _build_link_map(links) From 6224ae455676755d74e056acb173861092c9f607 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 10:30:53 +0000 Subject: [PATCH 04/16] fix(workflow): guard subgraph definition helpers against malformed entries Subgraph expansion runs before the per-node try/except wrapper in convert_ui_to_api, so any AttributeError there propagates up to the caller. The run.py catch-all still produces a clean exit, but bailing out of the entire conversion on a single malformed subgraph def is worse UX than skipping it and converting the rest. Defensive isinstance checks added in three spots: * _collect_subgraph_defs: only accept str sg.id (the id is later used as a dict key and matched against node_type strings) * _outer_slot_to_input_idx: skip non-dict entries in sg.inputs and outer_node.inputs * _expand_one_subgraph: skip non-dict entries in sg.inputs and sg.outputs Confirmed via fuzz: 8/8 previously-crashing inputs now exit cleanly (0 nodes emitted from the malformed subgraph, conversion of the rest proceeds). Two new tests cover the cases. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 25 ++++++++++---- tests/comfy_cli/test_workflow_to_api.py | 43 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 16af94fe..bc6da0cf 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -224,7 +224,9 @@ def _collect_subgraph_defs(workflow: dict) -> dict[str, dict]: if not isinstance(sg, dict): continue sg_id = sg.get("id") - if sg_id: + # sg_id has to be a string both because we use it as a dict key and + # because is_subgraph_uuid (used to match instances) only accepts str. + if isinstance(sg_id, str) and sg_id: defs[sg_id] = sg return defs @@ -262,9 +264,14 @@ def _expand_subgraphs( def _outer_slot_to_input_idx(outer_node: dict, sg_def: dict) -> dict[int, int]: """Map the outer node's input slots to subgraph-definition input indices.""" - sg_input_names = {inp.get("name"): idx for idx, inp in enumerate(sg_def.get("inputs", []))} + sg_input_names: dict[Any, int] = {} + for idx, inp in enumerate(sg_def.get("inputs") or []): + if isinstance(inp, dict): + sg_input_names[inp.get("name")] = idx mapping: dict[int, int] = {} - for outer_idx, outer_input in enumerate(outer_node.get("inputs", []) or []): + for outer_idx, outer_input in enumerate(outer_node.get("inputs") or []): + if not isinstance(outer_input, dict): + continue name = outer_input.get("name") if name in sg_input_names: mapping[outer_idx] = sg_input_names[name] @@ -299,9 +306,11 @@ def _expand_one_subgraph( internal_link_map[old_id] = link input_targets: dict[int, list[tuple[Any, int]]] = {} - for idx, in_def in enumerate(sg_def.get("inputs", []) or []): + for idx, in_def in enumerate(sg_def.get("inputs") or []): + if not isinstance(in_def, dict): + continue targets = [] - for lid in in_def.get("linkIds", []) or []: + for lid in in_def.get("linkIds") or []: link = internal_link_map.get(lid) if isinstance(link, dict): targets.append((link.get("target_id"), link.get("target_slot"))) @@ -309,8 +318,10 @@ def _expand_one_subgraph( input_targets[idx] = targets output_sources: dict[tuple[Any, int], int] = {} - for idx, out_def in enumerate(sg_def.get("outputs", []) or []): - for lid in out_def.get("linkIds", []) or []: + for idx, out_def in enumerate(sg_def.get("outputs") or []): + if not isinstance(out_def, dict): + continue + for lid in out_def.get("linkIds") or []: link = internal_link_map.get(lid) if isinstance(link, dict): output_sources[(link.get("origin_id"), link.get("origin_slot"))] = idx diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 6372c99f..decdfd8e 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -646,6 +646,49 @@ def test_tolerates_garbage_definitions(self, object_info): result = convert_ui_to_api(workflow, object_info) assert set(result) == {"1", "2"}, f"failed with definitions={bad_defs!r}" + def test_malformed_subgraph_definition_does_not_crash(self, object_info): + # Subgraph expansion runs before the per-node try/except wrapper, so + # the defensive checks live in the helpers themselves. Each of these + # malformed-definition shapes used to leak an AttributeError/TypeError + # before the helpers were guarded. + sg_uuid = "11111111-2222-3333-4444-555555555555" + cases = [ + # sg.inputs contains non-dict entries + {"id": sg_uuid, "nodes": [], "links": [], "inputs": [None, 42, ["x"]]}, + # sg.outputs contains non-dict entries + {"id": sg_uuid, "nodes": [], "links": [], "outputs": [None, 42]}, + # sg.id is unhashable; the def is silently dropped + {"id": {"weird": True}, "nodes": [], "links": []}, + {"id": ["x"], "nodes": [], "links": []}, + ] + for sg in cases: + workflow = { + "nodes": [{"id": 1, "type": sg_uuid, "inputs": [], "outputs": []}], + "links": [], + "definitions": {"subgraphs": [sg]}, + } + # Should not raise, regardless of how malformed the subgraph def is. + convert_ui_to_api(workflow, object_info) + + def test_outer_subgraph_node_with_non_dict_inputs_does_not_crash(self, object_info): + sg_uuid = "11111111-2222-3333-4444-555555555555" + workflow = { + "nodes": [ + { + "id": 1, + "type": sg_uuid, + "inputs": [None, 42, {"name": "x"}], + "outputs": [], + } + ], + "links": [], + "definitions": { + "subgraphs": [{"id": sg_uuid, "nodes": [], "links": [], "inputs": [{"name": "x"}], "outputs": []}] + }, + } + # Should not raise. + convert_ui_to_api(workflow, object_info) + def test_single_bad_node_does_not_abort_conversion(self, object_info, caplog): # We can't easily induce _build_api_node to throw on real input, so # monkeypatch it for this test. From 1819523a6ef24eb6bbc3577bc13a7329f9a94a98 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 10:40:53 +0000 Subject: [PATCH 05/16] fix(workflow): guard Set/Get variable names against unhashable values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _collect_get_set_mappings runs before the per-node try/except wrapper in convert_ui_to_api. If a SetNode's widgets_values[0] is a list or dict, ``set_sources[var_name] = ...`` raises TypeError and aborts the whole conversion. A GetNode with an unhashable widgets_values[0] also crashes the tracer later (``var_name in self.set_sources``) — caught by the per-node wrapper but still skips otherwise-valid downstream nodes. Reject any var_name that isn't a non-empty string up front. One new test covers list / dict / None / empty-string cases. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 5 +++++ tests/comfy_cli/test_workflow_to_api.py | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index bc6da0cf..5e488f44 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -543,6 +543,11 @@ def _collect_get_set_mappings( if not isinstance(widgets, list) or not widgets: continue var_name = widgets[0] + # var_name becomes a dict key (set_sources[var_name]) and is later + # checked with ``var_name in set_sources`` inside the tracer. Both + # require it to be a non-empty string; reject anything else early. + if not isinstance(var_name, str) or not var_name: + continue if node_type == "SetNode": for inp in node.get("inputs") or []: if not isinstance(inp, dict): diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index decdfd8e..bea94826 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -646,6 +646,29 @@ def test_tolerates_garbage_definitions(self, object_info): result = convert_ui_to_api(workflow, object_info) assert set(result) == {"1", "2"}, f"failed with definitions={bad_defs!r}" + def test_set_get_node_with_unhashable_var_name_does_not_crash(self, object_info): + # SetNode/GetNode publish/read a variable name that becomes a dict key + # in the tracer. If the saved widgets_values[0] is a list or dict, + # using it as a key raises TypeError. _collect_get_set_mappings runs + # before the per-node try/except wrapper, so an unguarded SetNode in + # particular aborts the whole conversion. + for bad_var in (["list-as-var"], {"dict": "as-var"}, None, ""): + workflow = { + "nodes": [ + _node(1, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + { + "id": 20, + "type": "SetNode", + "inputs": [{"name": "v", "link": 1}], + "widgets_values": [bad_var], + "mode": 0, + }, + ], + "links": [[1, 1, 0, 20, 0, "LATENT"]], + } + # Should not raise, no matter how unhashable the var name is. + convert_ui_to_api(workflow, object_info) + def test_malformed_subgraph_definition_does_not_crash(self, object_info): # Subgraph expansion runs before the per-node try/except wrapper, so # the defensive checks live in the helpers themselves. Each of these From 8ea255807593874ce6cf0d24f75f0aff924a9ec4 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 10:49:59 +0000 Subject: [PATCH 06/16] fix(workflow): make control_after_generate filter schema-aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The naive filter dropped any ``"fixed"/"increment"/"decrement"/"randomize"`` occurrence in widgets_values, even when it was a legitimate COMBO/STRING value rather than the synthetic marker emitted after a seed-like INT. Every later widget value then slid out of alignment, silently corrupting the prompt. (SethRobinson's reference converter has the same bug.) When a schema is available the filter now walks input_order in widget order and only consumes a control marker if it directly follows an input whose spec sets ``control_after_generate: True``. Without a schema, falls back to the legacy positional string-match heuristic so unknown node types behave the same as before. Verified with a fictional node whose COMBO option is literally "fixed" — the value is now preserved instead of being stripped. KSampler-style seed widgets still strip their control marker. Parity sweep against the reference converter unchanged: 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 68 ++++++++++++---- tests/comfy_cli/test_workflow_to_api.py | 102 +++++++++++++++++++++++- 2 files changed, 155 insertions(+), 15 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 5e488f44..612aadb2 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -956,25 +956,65 @@ def _fallback_widget_names(node: dict, widget_values: list[Any]) -> list[str | N return [] -def _filter_control_values(widget_values: list[Any]) -> list[Any]: - """Drop control_after_generate strings that follow INT widgets in widget_values.""" +def _filter_control_values( + widget_values: list[Any], + node_type: str | None = None, + node: dict | None = None, + object_info: dict | None = None, +) -> list[Any]: + """Drop the control_after_generate strings that follow seed-like INT widgets. + + Schema-aware when a schema is available: only a string immediately + following an input that declares ``control_after_generate: True`` is + treated as a control marker. This avoids false positives on legitimate + STRING/COMBO widget values that happen to equal one of the control + keywords (e.g. a combo option literally named ``"fixed"``). + + Falls back to a positional string-match heuristic when the schema is + unavailable — matches SethRobinson's behavior for unknown node types. + """ def is_control(v: Any) -> bool: return isinstance(v, str) and v in _CONTROL_AFTER_GENERATE_VALUES - out: list[Any] = [] - i = 0 - while i < len(widget_values): - value = widget_values[i] - if is_control(value): - i += 1 - continue - if i + 1 < len(widget_values) and is_control(widget_values[i + 1]): + schema = _schema_for(node_type, node, object_info) if node_type and node and object_info else None + if not schema: + out: list[Any] = [] + i = 0 + while i < len(widget_values): + value = widget_values[i] + if is_control(value): + i += 1 + continue + if i + 1 < len(widget_values) and is_control(widget_values[i + 1]): + out.append(value) + i += 2 + continue out.append(value) - i += 2 + i += 1 + return out + + out = [] + vidx = 0 + input_def = schema.get("input") or {} + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if not isinstance(section_def, dict): continue - out.append(value) - i += 1 + for _input_name, input_spec in section_def.items(): + if vidx >= len(widget_values): + break + is_widget, _is_dynamic = _is_widget_input(input_spec) + if not is_widget: + continue + out.append(widget_values[vidx]) + vidx += 1 + options = input_spec[1] if len(input_spec) >= 2 and isinstance(input_spec[1], dict) else {} + if options.get("control_after_generate") and vidx < len(widget_values) and is_control(widget_values[vidx]): + vidx += 1 + while vidx < len(widget_values): + out.append(widget_values[vidx]) + vidx += 1 return out @@ -1001,7 +1041,7 @@ def _collect_widget_inputs( _absorb_dict_widget_values(widget_values, out, link_inputs) return out - filtered = _filter_control_values(widget_values) + filtered = _filter_control_values(widget_values, node_type, node, object_info) names = _get_widget_name_order(node_type, node, object_info, widget_values) if not names: if filtered: diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index bea94826..0ad71af3 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -40,7 +40,7 @@ def object_info(): "input": { "required": { "model": ["MODEL"], - "seed": ["INT", {"default": 0}], + "seed": ["INT", {"default": 0, "control_after_generate": True}], "steps": ["INT", {"default": 20}], "cfg": ["FLOAT", {"default": 8.0}], "sampler_name": [["euler", "ddim"], {"default": "euler"}], @@ -746,6 +746,106 @@ def flaky_build(**kwargs): assert any("Failed to convert node" in rec.message for rec in caplog.records) +class TestControlAfterGenerate: + """The control_after_generate filter must be schema-aware so it doesn't + silently corrupt legitimate widget values that happen to equal a control + keyword. + """ + + def test_seed_widget_with_control_marker_strips_correctly(self): + # KSampler has ``control_after_generate: True`` on seed → the + # synthetic marker string after the seed value must be stripped. + object_info = { + "KSampler": { + "input": { + "required": { + "seed": ["INT", {"default": 0, "control_after_generate": True}], + "steps": ["INT", {"default": 20}], + "sampler_name": [["euler", "ddim"]], + } + }, + "input_order": {"required": ["seed", "steps", "sampler_name"]}, + "output_node": True, + "display_name": "KSampler", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "KSampler", + "inputs": [], + "outputs": [], + "widgets_values": [42, "randomize", 20, "euler"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"] == {"seed": 42, "steps": 20, "sampler_name": "euler"} + + def test_legitimate_value_named_fixed_is_preserved(self): + # A COMBO option literally named "fixed" used to be stripped by the + # naive filter, sliding every later widget out of alignment. + object_info = { + "ControlLike": { + "input": { + "required": { + "mode": [["loose", "fixed", "strict"]], + "label": ["STRING", {}], + } + }, + "input_order": {"required": ["mode", "label"]}, + "output_node": True, + "display_name": "Control-like", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "ControlLike", + "inputs": [], + "outputs": [], + "widgets_values": ["fixed", "hello"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"] == {"mode": "fixed", "label": "hello"} + + def test_unknown_node_falls_back_to_legacy_filter(self): + # No schema → no schema-aware filter possible. We fall back to the + # positional string-match heuristic, which matches SethRobinson's + # reference behavior for unknown nodes. + workflow = { + "nodes": [ + { + "id": 1, + "type": "TotallyUnknownNode", + "inputs": [], + "outputs": [{"links": [1]}], + "widgets_values": [42, "randomize", 20], + "mode": 0, + }, + { + "id": 2, + "type": "TotallyUnknownConsumer", + "inputs": [{"name": "x", "link": 1}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[1, 1, 0, 2, 0, "*"]], + } + # Should not raise; widget_values processing for unknown types just + # falls back to the legacy filter and produces an empty input map. + convert_ui_to_api(workflow, {}) + + class TestFrontendParity: """Behaviors mirrored from ComfyUI_frontend/src/utils/executionUtil.ts.""" From ec246d14df1d7e1a19bf6974736e35e852f31bad Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 11:00:29 +0000 Subject: [PATCH 07/16] fix(workflow): close three frontend-parity gaps from full audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following a systematic comparison against the frontend's graphToPrompt and the litegraph helpers it uses, three gaps were closed: 1. forceInput / defaultInput: ``forceInput: True`` (and its deprecated alias ``defaultInput``) demotes a widget-type input to a connection-only slot. The frontend doesn't render a widget for it and saved workflows have no entry in widgets_values for it. We were treating it as a widget, consuming a value slot that didn't exist and shifting every later widget value into the wrong input. Updated ``_is_widget_input`` to recognize both keys. 2. Muted/bypassed subgraph instances: if the outer subgraph instance node is muted (mode 2) or bypassed (mode 4) the frontend skips ``getInnerNodes()`` entirely — the inner subgraph never enters the prompt. We were unconditionally expanding regardless of mode, silently keeping nodes the user explicitly told to skip. The instance now stays in the node list so the regular mode-check excludes it from emission; bypass-with-external-wires routes through ``trace_bypassed`` the same as a regular bypassed node. 3. (Already done in the previous commit.) control_after_generate filtering is schema-aware. 3 new test classes cover the new behaviors. Parity sweep against the reference converter unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 18 +++ tests/comfy_cli/test_workflow_to_api.py | 178 ++++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 612aadb2..13b99231 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -245,6 +245,16 @@ def _expand_subgraphs( for node in nodes: node_type = node.get("type") if is_subgraph_uuid(node_type) and node_type in subgraph_defs: + # Frontend semantics (executionUtil.ts): if the subgraph + # instance node itself is muted (mode 2) or bypassed (mode 4), + # do NOT pull its inner nodes into the prompt. The instance + # stays in the node list where the normal mode-check excludes + # it from emission; for bypass, downstream consumers route + # through ``trace_bypassed`` on the instance's external + # inputs, the same way a bypassed regular node is handled. + if node.get("mode") in (_MODE_MUTED, _MODE_BYPASS): + expanded.append(node) + continue found_any = True sg_nodes, sg_links, input_map, output_map = _expand_one_subgraph(node, subgraph_defs[node_type], links) expanded.extend(sg_nodes) @@ -855,6 +865,14 @@ def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: """Return (is_widget, is_dynamic_combo) for an INPUT_TYPES spec.""" if not isinstance(input_spec, (list, tuple)) or not input_spec: return False, False + # ``forceInput: True`` (legacy alias: ``defaultInput``) explicitly demotes + # a widget-type input to a connection-only slot; the frontend doesn't + # render a widget for it and the saved workflow has no value for it in + # widgets_values. Treating it as a widget here would consume a value-slot + # that doesn't exist and shift every later widget out of position. + options = input_spec[1] if len(input_spec) >= 2 and isinstance(input_spec[1], dict) else {} + if options.get("forceInput") or options.get("defaultInput"): + return False, False input_type = input_spec[0] if isinstance(input_type, (list, tuple)): return True, False # combo of choices diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 0ad71af3..4dee6397 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -846,6 +846,98 @@ def test_unknown_node_falls_back_to_legacy_filter(self): convert_ui_to_api(workflow, {}) +class TestForceInputHandling: + """``forceInput: True`` (and its deprecated alias ``defaultInput``) + demotes a widget-type input to a connection-only slot. The frontend + doesn't render a widget for it and the saved workflow file has no + corresponding entry in ``widgets_values``. Treating it as a widget + here would consume a slot that doesn't exist and shift every later + widget's value into the wrong input. + """ + + def test_forceinput_widget_does_not_consume_value_slot(self): + object_info = { + "Source": { + "input": {"required": {}}, + "input_order": {"required": []}, + "output_node": False, + "output": ["INT"], + "display_name": "Source", + }, + "Mixed": { + "input": { + "required": { + "input_only": ["INT", {"forceInput": True}], + "widget_a": ["INT", {"default": 0}], + "widget_b": ["STRING", {}], + } + }, + "input_order": {"required": ["input_only", "widget_a", "widget_b"]}, + "output_node": True, + "display_name": "Mixed", + }, + } + workflow = { + "nodes": [ + _node(99, "Source", outputs=[{"links": [10]}]), + { + "id": 1, + "type": "Mixed", + "inputs": [{"name": "input_only", "type": "INT", "link": 10}], + "outputs": [], + "widgets_values": [42, "hello"], + "mode": 0, + }, + ], + "links": [[10, 99, 0, 1, 0, "INT"]], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["widget_a"] == 42 + assert result["1"]["inputs"]["widget_b"] == "hello" + assert result["1"]["inputs"]["input_only"] == ["99", 0] + + def test_legacy_defaultinput_alias_works_the_same(self): + # ``defaultInput`` is the deprecated alias the frontend migrates + # from. The server's /object_info may still emit it for older + # custom nodes that haven't updated. + object_info = { + "Source": { + "input": {"required": {}}, + "input_order": {"required": []}, + "output_node": False, + "output": ["INT"], + "display_name": "Source", + }, + "Mixed": { + "input": { + "required": { + "input_only": ["INT", {"defaultInput": True}], + "widget_a": ["INT", {"default": 0}], + } + }, + "input_order": {"required": ["input_only", "widget_a"]}, + "output_node": True, + "display_name": "Mixed", + }, + } + workflow = { + "nodes": [ + _node(99, "Source", outputs=[{"links": [10]}]), + { + "id": 1, + "type": "Mixed", + "inputs": [{"name": "input_only", "type": "INT", "link": 10}], + "outputs": [], + "widgets_values": [42], + "mode": 0, + }, + ], + "links": [[10, 99, 0, 1, 0, "INT"]], + } + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["widget_a"] == 42 + + class TestFrontendParity: """Behaviors mirrored from ComfyUI_frontend/src/utils/executionUtil.ts.""" @@ -1041,6 +1133,92 @@ def test_group_node_workflow_emits_warning(self, object_info, caplog): assert any("group node" in record.message.lower() for record in caplog.records) +class TestMutedBypassedSubgraph: + """Per frontend semantics (executionUtil.ts), if the subgraph *instance* + node is itself muted or bypassed, its inner nodes do NOT enter the prompt. + Without this we'd unconditionally expand and silently keep running the + workflow the user explicitly told to skip. + """ + + SG_UUID = "11111111-2222-3333-4444-555555555555" + + def _workflow(self, mode, with_external_wires=False): + sg_def = { + "id": self.SG_UUID, + "name": "Inner", + "nodes": [ + { + "id": 1, + "type": "EmptyLatentImage", + "outputs": [{"links": [10]}], + "widgets_values": [512, 512, 1], + "mode": 0, + }, + { + "id": 2, + "type": "PreviewImage", + "inputs": [{"name": "images", "link": 10}], + "outputs": [], + "mode": 0, + }, + ], + "links": [ + { + "id": 10, + "origin_id": 1, + "origin_slot": 0, + "target_id": 2, + "target_slot": 0, + "type": "IMAGE", + } + ], + "inputs": [{"name": "in_img"}] if with_external_wires else [], + "outputs": [{"name": "out_img"}] if with_external_wires else [], + } + nodes = [ + { + "id": 100, + "type": self.SG_UUID, + "inputs": [{"name": "in_img", "type": "IMAGE", "link": 200}] if with_external_wires else [], + "outputs": [{"name": "out_img", "type": "IMAGE", "links": [201]}] if with_external_wires else [], + "mode": mode, + } + ] + links = [] + if with_external_wires: + nodes.insert( + 0, + _node(7, "EmptyLatentImage", outputs=[{"links": [200]}], widgets=[512, 512, 1]), + ) + nodes.append( + _node(8, "PreviewImage", inputs=[{"name": "images", "type": "IMAGE", "link": 201}], outputs=[]), + ) + links = [[200, 7, 0, 100, 0, "LATENT"], [201, 100, 0, 8, 0, "IMAGE"]] + return {"nodes": nodes, "links": links, "definitions": {"subgraphs": [sg_def]}} + + def test_muted_subgraph_drops_inner_nodes(self, object_info): + result = convert_ui_to_api(self._workflow(mode=2), object_info) + assert result == {} + + def test_bypassed_subgraph_drops_inner_nodes(self, object_info): + result = convert_ui_to_api(self._workflow(mode=4), object_info) + assert result == {} + + def test_normal_subgraph_still_expands(self, object_info): + result = convert_ui_to_api(self._workflow(mode=0), object_info) + # Both inner nodes with the subgraph-prefixed IDs. + assert "100:1" in result + assert "100:2" in result + + def test_bypassed_subgraph_passes_external_input_through(self, object_info): + # When the bypassed subgraph has external wires, downstream consumers + # should be routed to the subgraph's upstream source (same as bypass + # behavior on a regular node). + result = convert_ui_to_api(self._workflow(mode=4, with_external_wires=True), object_info) + assert "100" not in result # subgraph instance gone + assert result["8"]["inputs"]["images"] == ["7", 0] + + class TestFixtureParity: """Regression test against a real workflow + the exact API output that ComfyUI's /workflow/convert endpoint produced for it. From 83c7f2c7437ac691a56602406b55f80674973173 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 14:09:22 +0000 Subject: [PATCH 08/16] feat(workflow): resolve dynamicPrompts {a|b} groups in text widgets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLIPTextEncode-family text inputs (32 in stock ComfyUI) declare ``dynamicPrompts: True`` in their schema. The frontend's ``processDynamicPrompt`` resolves ``{red|blue} hat`` to one of the alternatives at /prompt time. The backend treats the syntax literally, so a workflow saved with ``{a|b}`` syntax and executed via comfy-cli would tokenize the braces and pipes as literal characters and produce junk output. Neither SethRobinson's reference converter nor the backend handles this. Ported faithfully from packages/shared-frontend-utils/src/formatUtil.ts: * Strips ``/* ... */`` and ``// ...`` C-style comments * Picks one alternative at random from each top-level ``{a|b|...}`` group; recurses into the picked option (so nested groups resolve) * ``\\{``, ``\\}``, ``\\|`` escape their literal characters Applied during widget collection when the schema declares ``dynamicPrompts: True`` and the value is a string. The resolution is genuinely random — matching the frontend's ``Math.random()`` — so two consecutive runs of a workflow with ``{a|b}`` may produce different prompts. Tests pin ``random.choice`` deterministically. 13 new tests cover: comments, single/nested/multiple groups, empty alternatives, escapes, unterminated groups, end-to-end via convert_ui_to_api with and without the dynamicPrompts flag, and non-string passthrough. Parity sweep against the reference converter unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 111 ++++++++++++++- tests/comfy_cli/test_workflow_to_api.py | 175 ++++++++++++++++++++++++ 2 files changed, 284 insertions(+), 2 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 13b99231..332957ed 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -20,10 +20,16 @@ import copy import logging +import random +import re from typing import Any logger = logging.getLogger(__name__) +# C-style comments stripped from dynamic-prompt strings before group parsing. +_DYNAMIC_PROMPT_COMMENT_RE = re.compile(r"/\*[\s\S]*?\*/|//.*") +_DYNAMIC_PROMPT_UNESCAPE_RE = re.compile(r"\\([{}|])") + # Mode values from litegraph: see frontend's LGraphEventMode enum. _MODE_MUTED = 2 # excluded from execution; outputs not produced @@ -747,6 +753,100 @@ def _wrap_widget_value(value: Any) -> Any: return value +def process_dynamic_prompt(value: str) -> str: + """Resolve the ``{a|b|c}`` syntax used in CLIPTextEncode text widgets. + + Port of the frontend's ``processDynamicPrompt`` (``formatUtil.ts``): + + * Strips ``/* ... */`` and ``// ...`` comments first. + * Picks one alternative at random from each top-level ``{a|b|...}`` + group. Nested groups are recursed into after a choice is made. + * ``\\{``, ``\\}``, ``\\|`` escape their literal characters. + + Non-deterministic by design — the backend doesn't process the syntax, + so a workflow saved with ``{red|blue} hat`` would otherwise tokenize + the braces literally and produce a junk image. + """ + return _resolve_dynamic_prompt(_DYNAMIC_PROMPT_COMMENT_RE.sub("", value)) + + +def _resolve_dynamic_prompt(value: str) -> str: + out: list[str] = [] + i = 0 + n = len(value) + while i < n: + ch = value[i] + i += 1 + if ch == "\\" and i < n: + # Preserve the escape marker so the unescape pass at the end can + # restore the literal character without it being consumed earlier. + out.append("\\" + value[i]) + i += 1 + elif ch == "{": + chosen, i = _parse_dynamic_prompt_block(value, i) + out.append(_resolve_dynamic_prompt(chosen)) + else: + out.append(ch) + return _DYNAMIC_PROMPT_UNESCAPE_RE.sub(r"\1", "".join(out)) + + +def _parse_dynamic_prompt_block(value: str, i: int) -> tuple[str, int]: + """Parse a ``{a|b|...}`` group starting at index ``i`` (just past the ``{``). + + Returns ``(chosen_option, new_i)``. ``new_i`` points past the closing + ``}`` (or past end-of-string if the group is unterminated — the frontend + silently degrades on malformed input and we match that). + """ + options: list[str] = [] + choice: list[str] = [] + depth = 0 + n = len(value) + while i < n: + ch = value[i] + i += 1 + if ch == "\\" and i < n: + choice.append("\\" + value[i]) + i += 1 + continue + if ch == "{": + depth += 1 + choice.append(ch) + elif ch == "}": + if depth == 0: + break + depth -= 1 + choice.append(ch) + elif ch == "|" and depth == 0: + options.append("".join(choice)) + choice = [] + else: + choice.append(ch) + options.append("".join(choice)) + return random.choice(options), i + + +def _dynamic_prompt_input_names(node_type: str | None, node: dict | None, object_info: dict) -> set[str]: + """Names of inputs whose schema declares ``dynamicPrompts: True``.""" + if not node_type or not node: + return set() + schema = _schema_for(node_type, node, object_info) + if not schema: + return set() + input_def = schema.get("input") or {} + out: set[str] = set() + for section in ("required", "optional"): + section_def = input_def.get(section) or {} + if not isinstance(section_def, dict): + continue + for input_name, input_spec in section_def.items(): + if not isinstance(input_spec, (list, tuple)) or len(input_spec) < 2: + continue + options = input_spec[1] if isinstance(input_spec[1], dict) else {} + if options.get("dynamicPrompts"): + out.add(input_name) + return out + + def _build_api_node( *, node: dict, @@ -1042,6 +1142,13 @@ def _collect_widget_inputs( widget_values = node.get("widgets_values") if widget_values is None: return {} + dynamic_prompt_names = _dynamic_prompt_input_names(node_type, node, object_info) + + def emit(name: str, value: Any) -> Any: + if name in dynamic_prompt_names and isinstance(value, str): + value = process_dynamic_prompt(value) + return _wrap_widget_value(value) + out: dict[str, Any] = {} if isinstance(widget_values, dict): # Already self-describing; drop UI-only keys and respect link overrides. @@ -1050,7 +1157,7 @@ def _collect_widget_inputs( continue if key in link_inputs: continue - out[key] = _wrap_widget_value(value) + out[key] = emit(key, value) return out if not isinstance(widget_values, list): return {} @@ -1075,7 +1182,7 @@ def _collect_widget_inputs( name = names[i] if not name or name in link_inputs: continue - out[name] = _wrap_widget_value(value) + out[name] = emit(name, value) return out diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 4dee6397..dc169e34 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -1,7 +1,9 @@ """Unit tests for the UI -> API workflow converter.""" import json +import random from pathlib import Path +from unittest.mock import patch import pytest @@ -10,6 +12,7 @@ convert_ui_to_api, is_api_format, is_subgraph_uuid, + process_dynamic_prompt, ) FIXTURES = Path(__file__).parent / "fixtures" @@ -1219,6 +1222,178 @@ def test_bypassed_subgraph_passes_external_input_through(self, object_info): assert result["8"]["inputs"]["images"] == ["7", 0] +class TestDynamicPrompts: + """Port of frontend's processDynamicPrompt behavior (formatUtil.ts). + + Tests pin ``random.choice`` deterministically; the runtime behavior is + genuinely random, matching the frontend's ``Math.random()`` semantics. + """ + + # -- Pure algorithm -------------------------------------------------- + + def test_no_braces_passes_through(self): + assert process_dynamic_prompt("abcdef") == "abcdef" + assert process_dynamic_prompt("") == "" + + def test_strips_line_comments(self): + # // to end of line + assert process_dynamic_prompt("abc // a comment\nrest") == "abc \nrest" + + def test_strips_block_comments(self): + # /* ... */ across or within lines + assert process_dynamic_prompt("/*\nStart\n*/Hello /* mid */ world") == "Hello world" + + def test_picks_one_option_per_group(self): + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{option1|option2}") == "option1" + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[-1]): + assert process_dynamic_prompt("{option1|option2}") == "option2" + + def test_handles_empty_alternatives(self): + # Trailing empty + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[-1]): + assert process_dynamic_prompt("{a|}") == "" + # Leading empty + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{|a}") == "" + # All empty + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{||}") == "" + + def test_handles_nested_groups(self): + # Always pick first → outer 'a' + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{a|{b|{c|d}}}") == "a" + # Always pick last → innermost 'd' + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[-1]): + assert process_dynamic_prompt("{a|{b|{c|d}}}") == "d" + + def test_escapes_preserve_literal_characters(self): + # Escaped braces remain literal + assert process_dynamic_prompt("\\{a|b\\}") == "{a|b}" + # Escaped pipe outside group + assert process_dynamic_prompt("a\\|b") == "a|b" + # Escapes inside group survive + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{\\{escaped\\}\\|escaped pipe}") == "{escaped}|escaped pipe" + + def test_unterminated_group_degrades_gracefully(self): + # Frontend never throws on malformed input; we match that. + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + assert process_dynamic_prompt("{option1|option2|{nested1|nested2") == "option1" + + def test_multiple_groups_in_one_string(self): + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[1]): + assert process_dynamic_prompt("1{a|b|c}2{d|e|f}3") == "1b2e3" + + # -- Integration via convert_ui_to_api ------------------------------- + + OI = { + "CLIPTextEncode": { + "input": { + "required": { + "text": ["STRING", {"multiline": True, "dynamicPrompts": True}], + "clip": ["CLIP"], + } + }, + "input_order": {"required": ["text", "clip"]}, + "output_node": False, + "output": ["CONDITIONING"], + "display_name": "CLIP Text Encode", + }, + "PreviewImage": { + "input": {"required": {"images": ["IMAGE"]}}, + "input_order": {"required": ["images"]}, + "output_node": True, + "display_name": "Preview Image", + }, + "PlainText": { + "input": {"required": {"text": ["STRING", {}]}}, + "input_order": {"required": ["text"]}, + "output_node": True, + "display_name": "Plain Text", + }, + } + + def test_clip_text_encode_resolves_groups(self): + with patch("comfy_cli.workflow_to_api.random.choice", side_effect=lambda opts: opts[0]): + workflow = { + "nodes": [ + { + "id": 1, + "type": "CLIPTextEncode", + "inputs": [{"name": "clip", "link": None}], + "outputs": [{"links": [10]}], + "widgets_values": ["a {red|blue} hat"], + "mode": 0, + }, + { + "id": 2, + "type": "PreviewImage", + "inputs": [{"name": "images", "link": 10}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[10, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["text"] == "a red hat" + + def test_widget_without_dynamic_prompts_flag_left_alone(self): + # PlainText.text does NOT declare dynamicPrompts → literal passthrough. + workflow = { + "nodes": [ + { + "id": 1, + "type": "PlainText", + "inputs": [], + "outputs": [], + "widgets_values": ["a {red|blue} hat"], + "mode": 0, + }, + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["text"] == "a {red|blue} hat" + + def test_non_string_value_passes_through_unchanged(self): + # Numeric values on a dynamicPrompts input shouldn't be regex'd + workflow = { + "nodes": [ + { + "id": 1, + "type": "CLIPTextEncode", + "inputs": [{"name": "clip", "link": None}], + "outputs": [{"links": [10]}], + "widgets_values": [42], + "mode": 0, + }, + { + "id": 2, + "type": "PreviewImage", + "inputs": [{"name": "images", "link": 10}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[10, 1, 0, 2, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["text"] == 42 + + def test_random_choice_is_deterministic_under_seed(self): + # Sanity: seeding the global RNG fixes the choice — useful for the + # rare downstream test/script that wants reproducible runs. + random.seed(0) + first = process_dynamic_prompt("{alpha|beta|gamma}") + random.seed(0) + second = process_dynamic_prompt("{alpha|beta|gamma}") + assert first == second + assert first in {"alpha", "beta", "gamma"} + + class TestFixtureParity: """Regression test against a real workflow + the exact API output that ComfyUI's /workflow/convert endpoint produced for it. From 668aeb190fa510e27f47fb074c8d9f7ff67087cc Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 17:13:14 +0000 Subject: [PATCH 09/16] fix(workflow): feed filtered list to _get_widget_name_order _get_widget_name_order's internal widget_idx is the position in the value list it receives. We were handing it the raw widgets_values even though the mapping step below iterates the filtered list (with control_after_generate markers stripped). For nodes that pair a control_after_generate seed widget with a V3 dynamic combo, the combo's _dynamic_combo_sub_inputs lookup landed on the control marker instead of the actual selector, the selected option was never identified, and every sub-input value below it was silently dropped from the prompt. 38 stock API nodes are vulnerable (Bria*, ByteDance*, Grok*, Kling*, Meshy*, Recraft*, Reve*, Vidu*, Wan2*, HappyHorse*, Tencent*, Quiver*). Reproduced with a minimal schema where shape.side dropped to None after seed+control_marker, and confirmed fixed by passing filtered. One new test pins the behavior. Parity sweep unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 7 +++- tests/comfy_cli/test_workflow_to_api.py | 56 +++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 332957ed..eae01add 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -1167,7 +1167,12 @@ def emit(name: str, value: Any) -> Any: return out filtered = _filter_control_values(widget_values, node_type, node, object_info) - names = _get_widget_name_order(node_type, node, object_info, widget_values) + # ``widget_idx`` inside _get_widget_name_order is the position in the + # value list it receives, so it must see the *filtered* list — otherwise + # a V3 dynamic combo's selector is read from the wrong slot whenever a + # control_after_generate marker precedes it (e.g. on the Bria / Kling / + # Vidu / Wan2 API nodes that pair a seed with a dynamic combo). + names = _get_widget_name_order(node_type, node, object_info, filtered) if not names: if filtered: logger.warning( diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index dc169e34..55888fec 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -1222,6 +1222,62 @@ def test_bypassed_subgraph_passes_external_input_through(self, object_info): assert result["8"]["inputs"]["images"] == ["7", 0] +class TestDynamicComboAfterControlMarker: + """Regression: _get_widget_name_order must walk the filtered widget list, + not the raw one. Without this, a V3 dynamic combo whose schema sits after + a control_after_generate widget reads its selector from the wrong slot + (the control marker), fails to identify the option, and silently drops + every sub-input value for it. + + Affects 38 stock API nodes that pair a seed with a dynamic combo: + Bria*, ByteDance*, Grok*, Kling*, Meshy*, Recraft*, Reve*, Vidu*, Wan2*, + HappyHorse*, Tencent*, Quiver*. + """ + + def test_dynamic_combo_selector_reads_from_filtered_slot(self): + object_info = { + "VulnerableNode": { + "input": { + "required": { + "seed": ["INT", {"default": 0, "control_after_generate": True}], + "shape": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + {"key": "circle", "inputs": {"required": {"radius": ["FLOAT"]}}}, + {"key": "square", "inputs": {"required": {"side": ["FLOAT"]}}}, + ] + }, + ], + } + }, + "input_order": {"required": ["seed", "shape"]}, + "output_node": True, + "display_name": "VN", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "VulnerableNode", + "inputs": [], + "outputs": [], + # seed, control_marker, shape selector, then sub-input + "widgets_values": [42, "randomize", "square", 10.0], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + inputs = result["1"]["inputs"] + assert inputs["seed"] == 42 + assert inputs["shape"] == "square" + # Without the fix the sub-input was silently dropped. + assert inputs["shape.side"] == 10.0 + + class TestDynamicPrompts: """Port of frontend's processDynamicPrompt behavior (formatUtil.ts). From ec46457ead645c5e7369819891375cadccef6f8b Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 18:04:15 +0000 Subject: [PATCH 10/16] fix(workflow): guard internal link IDs against non-int / collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _expand_one_subgraph used link.get("id") as a dict key in three places without isinstance checks: 1. The initial pass that builds link_id_remap and internal_link_map. ``internal_link_map[old_id] = link`` was dedented out of the ``old_id is not None`` guard, so two links missing "id" collided under the None key (silently dropping the later one), and an unhashable id (list/dict) raised TypeError mid-loop. 2. The link-emission pass: ``new_id = link_id_remap.get(old_id, ...)`` crashes on unhashable old_id. 3. _rewrite_internal_input: ``internal_link_map.get(link_id)`` and ``link_id in link_id_remap`` crash on unhashable link_id from an inner node's input definition. All three run before the per-node try/except wrapper in convert_ui_to_api, so any TypeError there aborted the whole conversion despite our broader hardening contract. Restrict to ``isinstance(old_id, int)`` / ``isinstance(link_id, int)`` at each site — standard ComfyUI link IDs are ints. Two new tests cover unhashable subgraph link IDs and unhashable inner-node link references. Parity sweep unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 27 +++++++++---- tests/comfy_cli/test_workflow_to_api.py | 54 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index eae01add..67a20f85 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -314,12 +314,20 @@ def _expand_one_subgraph( link_id_remap: dict[int, int] = {} internal_link_map: dict[int, dict] = {} for link in internal_links: - if isinstance(link, dict): - old_id = link.get("id") - if old_id is not None: - link_id_remap[old_id] = next_id - next_id += 1 - internal_link_map[old_id] = link + if not isinstance(link, dict): + continue + old_id = link.get("id") + # Only int IDs are usable here: link_id_remap[old_id] / internal_link_map[old_id] + # need a hashable key, and the wider pipeline later does ``link_id in + # link_id_remap`` lookups keyed by int link IDs from the outer workflow. + # Skip the entry entirely on a missing/unhashable/wrong-typed id so a + # bad apple can't crash the whole subgraph expansion (which runs + # before the per-node try/except wrapper). + if not isinstance(old_id, int): + continue + link_id_remap[old_id] = next_id + next_id += 1 + internal_link_map[old_id] = link input_targets: dict[int, list[tuple[Any, int]]] = {} for idx, in_def in enumerate(sg_def.get("inputs") or []): @@ -362,6 +370,8 @@ def _expand_one_subgraph( if target_id in (_SUBGRAPH_INPUT_NODE_ID, _SUBGRAPH_OUTPUT_NODE_ID): continue old_id = link.get("id") + if not isinstance(old_id, int): + continue new_id = link_id_remap.get(old_id, old_id) expanded_links.append( [ @@ -382,7 +392,10 @@ def _rewrite_internal_input( ) -> dict: input_copy = input_info.copy() link_id = input_info.get("link") - if link_id is None: + if not isinstance(link_id, int): + # Both internal_link_map and link_id_remap are keyed by int IDs; an + # unhashable (list/dict) link_id would otherwise crash the lookup + # and abort the whole subgraph expansion. return input_copy link = internal_link_map.get(link_id) if not isinstance(link, dict): diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 55888fec..d4708286 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -672,6 +672,60 @@ def test_set_get_node_with_unhashable_var_name_does_not_crash(self, object_info) # Should not raise, no matter how unhashable the var name is. convert_ui_to_api(workflow, object_info) + def test_subgraph_link_with_unhashable_id_is_skipped(self, object_info): + # Internal link IDs are dict keys; an unhashable id used to crash + # the whole subgraph expansion (which runs before the per-node + # try/except), aborting conversion before anything could be emitted. + SG_UUID = "11111111-2222-3333-4444-555555555555" + for bad_id in (["x"], {"k": 1}, None): + workflow = { + "nodes": [{"id": 1, "type": SG_UUID, "inputs": [], "outputs": []}], + "links": [], + "definitions": { + "subgraphs": [ + { + "id": SG_UUID, + "nodes": [], + "links": [{"id": bad_id, "origin_id": 1, "target_id": 2}], + "inputs": [], + "outputs": [], + } + ] + }, + } + # Should not raise — bad link is just dropped. + convert_ui_to_api(workflow, object_info) + + def test_inner_node_with_unhashable_link_id_does_not_crash(self, object_info): + # An inner subgraph node whose input's ``link`` field is not an int + # used to crash _rewrite_internal_input's ``internal_link_map.get`` + # / ``link_id in link_id_remap`` lookup. + SG_UUID = "22222222-3333-4444-5555-666666666666" + workflow = { + "nodes": [{"id": 1, "type": SG_UUID, "inputs": [], "outputs": []}], + "links": [], + "definitions": { + "subgraphs": [ + { + "id": SG_UUID, + "nodes": [ + { + "id": 9, + "type": "Foo", + "inputs": [{"link": ["weird"]}], + "outputs": [], + } + ], + "links": [{"id": 5, "origin_id": 1, "target_id": 9}], + "inputs": [], + "outputs": [], + } + ] + }, + } + # Should not raise. + convert_ui_to_api(workflow, object_info) + def test_malformed_subgraph_definition_does_not_crash(self, object_info): # Subgraph expansion runs before the per-node try/except wrapper, so # the defensive checks live in the helpers themselves. Each of these From d31ae9eb0412c67625a362cf86c2992e10d55e74 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Tue, 12 May 2026 18:29:30 +0000 Subject: [PATCH 11/16] fix(workflow): guard link-id dict-key lookups in global pre-pass helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit pointed at _collect_reroute_sources and the SetNode branch in _collect_get_set_mappings — both run before the per-node try/except wrapper, both do ``link_id in link_map`` / ``link_map.get(link_id)``, and both crash with TypeError on a saved ``link: []`` or ``link: {}``, aborting the entire conversion. Sweeping audit found four more dict-key uses with the same shape: * _expand_one_subgraph subgraph input ``linkIds`` loop * _expand_one_subgraph subgraph output ``linkIds`` loop * _build_api_node's per-input ``tracers.link_map`` lookup (this one was already inside the per-node try/except, so it only ever truncated one node — but hardening it keeps the contract uniform) All five now isinstance-check link_id / lid as int before the membership test, matching the surrounding pattern. One new test covers Reroute, SetNode, and subgraph linkIds with unhashable values in a single sweep. Parity sweep against the reference converter unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 23 ++++++--- tests/comfy_cli/test_workflow_to_api.py | 67 +++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 67a20f85..32ade2d5 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -335,6 +335,8 @@ def _expand_one_subgraph( continue targets = [] for lid in in_def.get("linkIds") or []: + if not isinstance(lid, int): + continue link = internal_link_map.get(lid) if isinstance(link, dict): targets.append((link.get("target_id"), link.get("target_slot"))) @@ -346,6 +348,8 @@ def _expand_one_subgraph( if not isinstance(out_def, dict): continue for lid in out_def.get("linkIds") or []: + if not isinstance(lid, int): + continue link = internal_link_map.get(lid) if isinstance(link, dict): output_sources[(link.get("origin_id"), link.get("origin_slot"))] = idx @@ -553,7 +557,11 @@ def _collect_reroute_sources(nodes: list[dict], link_map: dict[int, dict]) -> di if not isinstance(inputs, list) or not inputs or not isinstance(inputs[0], dict): continue link_id = inputs[0].get("link") - if link_id is None or link_id not in link_map: + # ``link_id in link_map`` raises TypeError on unhashable values + # (e.g. ``link: []`` in a malformed saved file). _collect_reroute_sources + # runs before the per-node try/except wrapper, so a single bad Reroute + # would otherwise abort the entire conversion. + if not isinstance(link_id, int) or link_id not in link_map: continue ld = link_map[link_id] out[str(node.get("id"))] = (ld["source_id"], ld["source_slot"]) @@ -582,10 +590,13 @@ def _collect_get_set_mappings( if not isinstance(inp, dict): continue lid = inp.get("link") - if lid is not None and lid in link_map: - ld = link_map[lid] - set_sources[var_name] = (ld["source_id"], ld["source_slot"]) - break + # See _collect_reroute_sources: unhashable lid would crash + # the global pre-pass before any per-node guard kicks in. + if not isinstance(lid, int) or lid not in link_map: + continue + ld = link_map[lid] + set_sources[var_name] = (ld["source_id"], ld["source_slot"]) + break elif node_type == "GetNode": get_vars[str(node.get("id"))] = var_name return set_sources, get_vars @@ -885,7 +896,7 @@ def _build_api_node( continue input_name = inp.get("name") link_id = inp.get("link") - if not input_name or link_id is None or link_id not in tracers.link_map: + if not input_name or not isinstance(link_id, int) or link_id not in tracers.link_map: continue ld = tracers.link_map[link_id] actual_id, actual_slot = ld["source_id"], ld["source_slot"] diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index d4708286..661f3011 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -672,6 +672,73 @@ def test_set_get_node_with_unhashable_var_name_does_not_crash(self, object_info) # Should not raise, no matter how unhashable the var name is. convert_ui_to_api(workflow, object_info) + def test_unhashable_link_value_in_global_helpers_does_not_crash(self, object_info): + # ``link_id in link_map`` raises TypeError on unhashable values, so + # _collect_reroute_sources / _collect_get_set_mappings / subgraph + # linkIds resolution used to abort the entire conversion when a + # single saved Reroute / SetNode / subgraph input had ``link: []`` + # (or ``{}``, etc.). + cases = [ + ( + "reroute_list", + { + "nodes": [{"id": 1, "type": "Reroute", "inputs": [{"link": []}], "outputs": [], "mode": 0}], + "links": [], + }, + ), + ( + "reroute_dict", + { + "nodes": [{"id": 1, "type": "Reroute", "inputs": [{"link": {}}], "outputs": [], "mode": 0}], + "links": [], + }, + ), + ( + "setnode_list", + { + "nodes": [ + { + "id": 1, + "type": "SetNode", + "inputs": [{"link": []}], + "outputs": [], + "widgets_values": ["myvar"], + "mode": 0, + } + ], + "links": [], + }, + ), + ( + "subgraph_linkIds_list", + { + "nodes": [ + { + "id": 1, + "type": "11111111-2222-3333-4444-555555555555", + "inputs": [], + "outputs": [], + } + ], + "links": [], + "definitions": { + "subgraphs": [ + { + "id": "11111111-2222-3333-4444-555555555555", + "nodes": [], + "links": [], + "inputs": [{"name": "x", "linkIds": [["bad"]]}], + "outputs": [], + } + ] + }, + }, + ), + ] + for _label, workflow in cases: + # Should not raise — each malformed link is silently skipped. + convert_ui_to_api(workflow, object_info) + def test_subgraph_link_with_unhashable_id_is_skipped(self, object_info): # Internal link IDs are dict keys; an unhashable id used to crash # the whole subgraph expansion (which runs before the per-node From 173331d22a8b3f11a5acd67f54eca21f0dba46f4 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Thu, 14 May 2026 07:22:22 +0000 Subject: [PATCH 12/16] fix(workflow): resolve node schema once via _schema_for in all paths _collect_widget_inputs (and the helpers it composes) consulted the schema via _schema_for, which honors the ``Node name for S&R`` property alias the same way the frontend does. But _meta.title, _collect_default_inputs, _normalize_combo_values, and the dead-branch heuristic in _collect_excluded looked up object_info[node_type] directly. For a node carrying an S&R alias to a different class: * _meta.title got the raw type name instead of the aliased display_name * defaults weren't filled in (schema lookup missed) * combo values weren't normalized (e.g., "EULER" stayed as-is and the server validator rejected it) * a schemaless dead-branch node could be wrongly excluded Resolve schema once at the top of _build_api_node and thread it through. _collect_default_inputs and _normalize_combo_values now take the resolved schema directly. _collect_excluded uses _schema_for inline. The S&R alias is rare in modern workflows (724/724 nodes in the stock ComfyUI blueprints have S&R == type), so parity sweep against SethRobinson is unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. A 4-case test class pins the aliased behavior on each of the affected paths. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 23 ++++--- tests/comfy_cli/test_workflow_to_api.py | 90 +++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 32ade2d5..a55ce80c 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -619,7 +619,11 @@ def _collect_excluded(nodes: list[dict], object_info: dict) -> set[str]: if outputs and has_connected_output: continue - schema = object_info.get(node_type) if isinstance(node_type, str) else None + # _schema_for honors the ``Node name for S&R`` alias the same way + # _build_api_node and _collect_widget_inputs do; consulting raw + # object_info[node_type] would mark an aliased node as schemaless + # and incorrectly exclude it via the dead-branch heuristic. + schema = _schema_for(node_type, node, object_info) is_output_node = bool(isinstance(schema, dict) and schema.get("output_node")) has_connected_input = any( isinstance(inp, dict) and inp.get("link") is not None for inp in (node.get("inputs") or []) @@ -882,11 +886,15 @@ def _build_api_node( nodes_to_exclude: set[str], ) -> dict: api_node: dict = {"inputs": {}, "class_type": node_type} + # Resolve the schema once via _schema_for so every consumer + # (_meta.title, defaults, combo normalization) sees the same thing + # as the widget-mapping path, even on nodes that carry a ``Node name + # for S&R`` property pointing at a different class. + schema = _schema_for(node_type, node, object_info) or {} if "title" in node: api_node["_meta"] = {"title": node["title"]} else: - schema = object_info.get(node_type, {}) api_node["_meta"] = {"title": schema.get("display_name") or node_type} link_inputs: dict[str, list] = {} @@ -925,7 +933,7 @@ def _build_api_node( link_inputs[input_name] = [actual_id_str, actual_slot] widget_inputs = _collect_widget_inputs(node, node_type, object_info, link_inputs) - default_inputs = _collect_default_inputs(node_type, object_info, widget_inputs, primitive_inputs, link_inputs) + default_inputs = _collect_default_inputs(schema, widget_inputs, primitive_inputs, link_inputs) ordered = _get_ordered_input_names(node_type, node, object_info) if ordered: @@ -948,7 +956,7 @@ def _build_api_node( if key not in api_node["inputs"]: api_node["inputs"][key] = value - _normalize_combo_values(node_type, object_info, api_node["inputs"]) + _normalize_combo_values(schema, api_node["inputs"]) return api_node @@ -1238,13 +1246,11 @@ def _absorb_dict_widget_values(widget_values: list[Any], out: dict[str, Any], li def _collect_default_inputs( - node_type: str, - object_info: dict, + schema: dict | None, widget_inputs: dict[str, Any], primitive_inputs: dict[str, Any], link_inputs: dict[str, list], ) -> dict[str, Any]: - schema = object_info.get(node_type) if isinstance(node_type, str) else None if not schema: return {} input_def = schema.get("input") or {} @@ -1281,8 +1287,7 @@ def _extract_default(input_spec: Any) -> Any: return _MISSING -def _normalize_combo_values(node_type: str, object_info: dict, inputs: dict[str, Any]) -> None: - schema = object_info.get(node_type) if isinstance(node_type, str) else None +def _normalize_combo_values(schema: dict | None, inputs: dict[str, Any]) -> None: if not schema: return input_def = schema.get("input") or {} diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 661f3011..93c81cbd 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -970,6 +970,96 @@ def test_unknown_node_falls_back_to_legacy_filter(self): convert_ui_to_api(workflow, {}) +class TestNodeNameForSAndRAlias: + """When a node carries ``properties["Node name for S&R"]`` pointing at a + different class name than its ``type`` field (legacy rename / group-node + artifact), the schema lookup honors the alias in widget mapping. Before + this fix, ``_meta.title``, default values, combo normalization, and the + dead-branch exclusion all consulted ``object_info[node_type]`` directly + and missed the schema entirely — silently dropping defaults, leaving + combo values un-normalized, and (in some cases) excluding the node as a + schemaless dead branch. + """ + + OI = { + "RealClass": { + "input": { + "required": { + "sampler": [["euler", "ddim"]], + "missing_widget": ["INT", {"default": 99}], + } + }, + "input_order": {"required": ["sampler", "missing_widget"]}, + "output_node": True, + "display_name": "Real Sampler", + }, + "Sink": { + "input": {"required": {"x": ["ANY"]}}, + "input_order": {"required": ["x"]}, + "output_node": True, + "display_name": "Sink", + }, + } + + def _aliased_workflow(self, *, widgets_values): + return { + "nodes": [ + { + "id": 1, + # `type` is the legacy/aliased name not in object_info. + "type": "OldName", + "properties": {"Node name for S&R": "RealClass"}, + "inputs": [], + "outputs": [{"links": [10]}], + "widgets_values": widgets_values, + "mode": 0, + }, + { + "id": 2, + "type": "Sink", + "inputs": [{"name": "x", "link": 10}], + "outputs": [], + "mode": 0, + }, + ], + "links": [[10, 1, 0, 2, 0, "ANY"]], + } + + def test_meta_title_uses_aliased_schema(self): + result = convert_ui_to_api(self._aliased_workflow(widgets_values=["euler"]), self.OI) + assert result["1"]["_meta"]["title"] == "Real Sampler" + + def test_combo_normalization_uses_aliased_schema(self): + # Wrong-case combo value must still be normalized via the aliased schema. + result = convert_ui_to_api(self._aliased_workflow(widgets_values=["EULER"]), self.OI) + assert result["1"]["inputs"]["sampler"] == "euler" + + def test_defaults_filled_from_aliased_schema(self): + # Only the first widget is provided; the second should come from defaults. + result = convert_ui_to_api(self._aliased_workflow(widgets_values=["euler"]), self.OI) + assert result["1"]["inputs"]["missing_widget"] == 99 + + def test_dead_branch_check_uses_aliased_schema(self): + # No connected outputs, but the aliased schema marks it as OUTPUT_NODE + # so it must survive the dead-branch exclusion. + workflow = { + "nodes": [ + { + "id": 1, + "type": "OldName", + "properties": {"Node name for S&R": "RealClass"}, + "inputs": [], + "outputs": [], + "widgets_values": ["euler"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert "1" in result + + class TestForceInputHandling: """``forceInput: True`` (and its deprecated alias ``defaultInput``) demotes a widget-type input to a connection-only slot. The frontend From 9e00a5090a2fee34f1c1a3b91e92df4525ebe1d9 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Thu, 14 May 2026 07:45:22 +0000 Subject: [PATCH 13/16] fix(workflow): close two paranoid-review hardening gaps Both fixes come from a self-review pass. Neither has a known real-world trigger in stock ComfyUI (verified by scanning every V3 combo option and schema in /object_info), but both follow the same defensive contract as the rest of the converter. 1. _dynamic_combo_sub_inputs no longer crashes when a V3 combo option's ``inputs`` field is a non-dict (string, list, int, ...). Before, ``sub_def.get(section)`` raised AttributeError; the per-node try/except caught it but silently dropped the entire node. Now we degrade to "no sub-inputs" and keep the node. 2. Extract _schema_input_def(schema) and use it in every helper that previously did ``schema.get("input") or {}`` followed by ``.get(section)``. The raw ``or {}`` pattern returns the value as-is when it's truthy, so a malformed schema with ``"input": [1,2,3]`` would AttributeError on ``.get``. Same reasoning applied to ``schema.get("input_order")``. 3 new tests cover the V3 combo malformed-inputs case and schema.input / schema.input_order with list / string / int. Parity sweep against the reference converter unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 44 ++++++++--- tests/comfy_cli/test_workflow_to_api.py | 97 +++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 9 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index a55ce80c..a01c0a14 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -860,7 +860,7 @@ def _dynamic_prompt_input_names(node_type: str | None, node: dict | None, object schema = _schema_for(node_type, node, object_info) if not schema: return set() - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) out: set[str] = set() for section in ("required", "optional"): section_def = input_def.get(section) or {} @@ -974,18 +974,37 @@ def _schema_for(node_type: str, node: dict, object_info: dict) -> dict | None: return object_info.get(node_type) if isinstance(node_type, str) else None +def _schema_input_def(schema: Any) -> dict: + """Return the schema's ``input`` block as a dict, or ``{}`` if absent/malformed. + + Every helper that walks INPUT_TYPES sections needs this guard: the raw + ``schema.get("input") or {}`` pattern returns the value as-is when it's + truthy, so a malformed schema with ``"input": [...]`` would later crash + on ``.get(section)``. In practice ``/object_info`` never sends a non-dict + here, but the rest of the converter follows the same defensive contract. + """ + if not isinstance(schema, dict): + return {} + input_def = schema.get("input") + return input_def if isinstance(input_def, dict) else {} + + def _get_ordered_input_names(node_type: str, node: dict, object_info: dict) -> list[str]: schema = _schema_for(node_type, node, object_info) if not schema: return [] - input_order = schema.get("input_order") or {} + input_order = schema.get("input_order") + if not isinstance(input_order, dict): + input_order = {} out: list[str] = [] for section in ("required", "optional"): - out.extend(input_order.get(section, []) or []) + section_order = input_order.get(section) + if isinstance(section_order, list): + out.extend(section_order) if out: return out # Fall back to whatever order is in the input dict itself. - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) for section in ("required", "optional"): section_def = input_def.get(section) or {} if isinstance(section_def, dict): @@ -1031,7 +1050,14 @@ def _dynamic_combo_sub_inputs( for option in options: if not isinstance(option, dict) or option.get("key") != selected: continue - sub_def = option.get("inputs") or {} + sub_def = option.get("inputs") + # The option's ``inputs`` is supposed to mirror an INPUT_TYPES dict + # (``{"required": {...}, "optional": {...}}``). Treat anything else + # — typically a malformed third-party V3 node — as having no + # sub-inputs rather than letting AttributeError escape into the + # per-node wrapper and silently dropping the whole node. + if not isinstance(sub_def, dict): + return [] names: list[str] = [] for section in ("required", "optional"): section_def = sub_def.get(section) or {} @@ -1045,7 +1071,7 @@ def _get_widget_name_order(node_type: str, node: dict, object_info: dict, widget """Build the widget-name list that maps positionally to ``widgets_values``.""" schema = _schema_for(node_type, node, object_info) if schema: - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) names: list[str | None] = [] widget_idx = 0 for section in ("required", "optional"): @@ -1146,7 +1172,7 @@ def is_control(v: Any) -> bool: out = [] vidx = 0 - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) for section in ("required", "optional"): section_def = input_def.get(section) or {} if not isinstance(section_def, dict): @@ -1253,7 +1279,7 @@ def _collect_default_inputs( ) -> dict[str, Any]: if not schema: return {} - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) defaults: dict[str, Any] = {} for section in ("required", "optional"): section_def = input_def.get(section) or {} @@ -1290,7 +1316,7 @@ def _extract_default(input_spec: Any) -> Any: def _normalize_combo_values(schema: dict | None, inputs: dict[str, Any]) -> None: if not schema: return - input_def = schema.get("input") or {} + input_def = _schema_input_def(schema) for section in ("required", "optional"): section_def = input_def.get(section) or {} if not isinstance(section_def, dict): diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 93c81cbd..f3215bac 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -836,6 +836,103 @@ def test_outer_subgraph_node_with_non_dict_inputs_does_not_crash(self, object_in # Should not raise. convert_ui_to_api(workflow, object_info) + def test_v3_combo_option_with_non_dict_inputs_keeps_node(self): + # A V3 dynamic combo option whose ``inputs`` field is malformed + # (string / list / etc., not the expected INPUT_TYPES-shaped dict) + # used to crash _dynamic_combo_sub_inputs; the per-node wrapper + # caught the AttributeError but silently dropped the entire node. + # Now we degrade to "no sub-inputs" and keep the rest of the node. + object_info = { + "Foo": { + "input": { + "required": { + "shape": [ + "COMFY_DYNAMICCOMBO_V3", + {"options": [{"key": "square", "inputs": "not-a-dict"}]}, + ] + } + }, + "input_order": {"required": ["shape"]}, + "output_node": True, + "display_name": "Foo", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "Foo", + "inputs": [], + "outputs": [], + "widgets_values": ["square", 5.0], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + # Node emitted, no crash, no silently-dropped node. + assert result["1"]["class_type"] == "Foo" + assert result["1"]["inputs"]["shape"] == "square" + + def test_malformed_schema_input_does_not_crash(self): + # Several helpers do ``schema.get("input") or {}`` then ``.get(section)``. + # If "input" was ever a non-dict, ``.get`` would AttributeError before + # any per-node wrapper saw it. /object_info doesn't emit malformed + # schemas today, but the rest of the converter is paranoid about + # exactly this shape — keep the contract uniform. + for bad_input in ([], "string", 42): + object_info = { + "Bar": { + "input": bad_input, + "input_order": {"required": []}, + "output_node": True, + "display_name": "Bar", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "Bar", + "inputs": [], + "outputs": [], + "widgets_values": [], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" in result + + def test_malformed_schema_input_order_does_not_crash(self): + # Same defensive contract for the ``input_order`` block. + for bad_order in ([], "string", 42): + object_info = { + "Bar": { + "input": {"required": {"x": ["INT", {"default": 0}]}}, + "input_order": bad_order, + "output_node": True, + "display_name": "Bar", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "Bar", + "inputs": [], + "outputs": [], + "widgets_values": [42], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" in result + def test_single_bad_node_does_not_abort_conversion(self, object_info, caplog): # We can't easily induce _build_api_node to throw on real input, so # monkeypatch it for this test. From 8896a383b4d97fbf688728c2b526761a26d46c36 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Thu, 14 May 2026 08:21:45 +0000 Subject: [PATCH 14/16] fix(workflow): make the three tracers iterative to avoid recursion limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trace_reroute, trace_get_set, and trace_bypassed were tail-recursive through the chain of nodes they walk. Python's default recursion limit (1000) meant chains of ~997 hops triggered RecursionError; the per-node try/except wrapper then caught it and silently dropped the downstream consumer from the prompt — a soft failure mode that produces a wrong-but-runnable workflow with no user-visible signal. The reference converter has the same bug (verified — chains of 999 reroutes return HTTP 500), but its endpoint surfaces the error explicitly. Our silent drop is materially worse for debugging. All three tracers are tail-recursive: each recursive call is the last expression of the function, with only an updated (src_id, src_slot) pair. They convert mechanically into ``while True`` loops that mutate src_id/src_slot in place. The cycle-detection seen-set remains scoped to each top-level call. trace_bypassed's internal calls to trace_get_set / trace_reroute already iterate over their own chains (no recursion compounding). The _seen optional parameter is dropped from all three since the only external callers used the 2-arg form. 3 new regression tests pin chain lengths well past the old crash threshold (N=2000) for each tracer. Cycle handling is unchanged (verified on Reroute<->Reroute and Set/Get cycles). Parity sweep against the reference converter unchanged at 46/51 byte-identical + 5 cosmetic title-only diffs. Live SD 1.5 smoke test succeeds. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 200 +++++++++++++----------- tests/comfy_cli/test_workflow_to_api.py | 100 ++++++++++++ 2 files changed, 205 insertions(+), 95 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index a01c0a14..b48a3c4f 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -663,106 +663,116 @@ def __init__( self.get_vars = get_vars self.subgraph_ctx = subgraph_ctx - def trace_reroute(self, src_id: Any, src_slot: Any, _seen: set[str] | None = None) -> tuple[Any, Any]: - seen = _seen if _seen is not None else set() - key = str(src_id) - if key in seen: - return src_id, src_slot - seen.add(key) - if key in self.reroute_sources: - actual_id, actual_slot = self.reroute_sources[key] - return self.trace_reroute(actual_id, actual_slot, seen) - return src_id, src_slot - - def trace_get_set(self, src_id: Any, src_slot: Any, _seen: set[str] | None = None) -> tuple[Any, Any]: - seen = _seen if _seen is not None else set() - key = str(src_id) - if key in seen: - return src_id, src_slot - seen.add(key) - if key in self.get_vars: + def trace_reroute(self, src_id: Any, src_slot: Any) -> tuple[Any, Any]: + # Iterative to avoid Python's recursion limit on long chains. The + # body matches a tail-recursive version exactly; the seen-set guards + # against cyclic ``Reroute -> Reroute -> ...`` loops. + seen: set[str] = set() + while True: + key = str(src_id) + if key in seen or key not in self.reroute_sources: + return src_id, src_slot + seen.add(key) + src_id, src_slot = self.reroute_sources[key] + + def trace_get_set(self, src_id: Any, src_slot: Any) -> tuple[Any, Any]: + # Same iterative shape as trace_reroute. Hops through one + # GetNode -> SetNode pair per step; the seen-set guards against + # cycles via repeated variable names. + seen: set[str] = set() + while True: + key = str(src_id) + if key in seen or key not in self.get_vars: + return src_id, src_slot + seen.add(key) var_name = self.get_vars[key] - if var_name in self.set_sources: - actual_id, actual_slot = self.set_sources[var_name] - return self.trace_get_set(actual_id, actual_slot, seen) - return src_id, src_slot - - def trace_bypassed(self, src_id: Any, src_slot: Any, _seen: set[Any] | None = None) -> tuple[Any, Any]: - seen = _seen if _seen is not None else set() - if src_id in seen: - return src_id, src_slot - seen.add(src_id) - if str(src_id) not in self.bypassed: - return src_id, src_slot - - node = self.node_by_id.get(str(src_id)) - if not node: - return src_id, src_slot + if var_name not in self.set_sources: + return src_id, src_slot + src_id, src_slot = self.set_sources[var_name] + + def trace_bypassed(self, src_id: Any, src_slot: Any) -> tuple[Any, Any]: + # Iterative. Each loop iteration corresponds to walking through one + # bypassed node; inner calls to trace_get_set / trace_reroute already + # iterate over their respective chains (no recursion). + seen: set[Any] = set() + while True: + if src_id in seen: + return src_id, src_slot + seen.add(src_id) + if str(src_id) not in self.bypassed: + return src_id, src_slot + + node = self.node_by_id.get(str(src_id)) + if not node: + return src_id, src_slot + + outputs = node.get("outputs") or [] + # Guard the slot index — malformed workflows can have non-numeric slots. + try: + slot_idx = int(src_slot) if src_slot is not None else 0 + except (TypeError, ValueError): + slot_idx = 0 + output_type = ( + outputs[slot_idx].get("type") + if 0 <= slot_idx < len(outputs) and isinstance(outputs[slot_idx], dict) + else None + ) - outputs = node.get("outputs") or [] - # Guard the slot index — malformed workflows can have non-numeric slots. - try: - slot_idx = int(src_slot) if src_slot is not None else 0 - except (TypeError, ValueError): - slot_idx = 0 - output_type = ( - outputs[slot_idx].get("type") - if 0 <= slot_idx < len(outputs) and isinstance(outputs[slot_idx], dict) - else None - ) + # Pick the input we'll forward the output through. We mix the frontend's + # strict matcher (ExecutableNodeDTO._getBypassSlotIndex) with the + # reference converter's permissive fallback, in order of preference: + # 1. Same-slot input if its type connects to the output type + # 2. First input whose type matches the output type exactly + # 3. First input whose type is connection-compatible (handles ``*`` + # and ``,``-separated alternatives via LiteGraph.isValidConnection) + # 4. First linked input regardless of type — preserves user intent + # when types disagree, matching SethRobinson's reference. The + # executor will surface a type mismatch loudly if it matters. + inputs = node.get("inputs") or [] + chosen_link: int | None = None + exact_link: int | None = None + compat_link: int | None = None + fallback_link: int | None = None + + same_slot_inp = ( + inputs[slot_idx] if 0 <= slot_idx < len(inputs) and isinstance(inputs[slot_idx], dict) else None + ) + if same_slot_inp: + lid = same_slot_inp.get("link") + if ( + lid is not None + and lid in self.link_map + and _is_valid_connection(same_slot_inp.get("type"), output_type) + ): + chosen_link = lid - # Pick the input we'll forward the output through. We mix the frontend's - # strict matcher (ExecutableNodeDTO._getBypassSlotIndex) with the - # reference converter's permissive fallback, in order of preference: - # 1. Same-slot input if its type connects to the output type - # 2. First input whose type matches the output type exactly - # 3. First input whose type is connection-compatible (handles ``*`` - # and ``,``-separated alternatives via LiteGraph.isValidConnection) - # 4. First linked input regardless of type — preserves user intent - # when types disagree, matching SethRobinson's reference. The - # executor will surface a type mismatch loudly if it matters. - inputs = node.get("inputs") or [] - chosen_link: int | None = None - exact_link: int | None = None - compat_link: int | None = None - fallback_link: int | None = None - - same_slot_inp = inputs[slot_idx] if 0 <= slot_idx < len(inputs) and isinstance(inputs[slot_idx], dict) else None - if same_slot_inp: - lid = same_slot_inp.get("link") - if ( - lid is not None - and lid in self.link_map - and _is_valid_connection(same_slot_inp.get("type"), output_type) - ): - chosen_link = lid - - if chosen_link is None: - for inp in inputs: - if not isinstance(inp, dict): - continue - lid = inp.get("link") - if lid is None or lid not in self.link_map: - continue - inp_type = inp.get("type") - if fallback_link is None: - fallback_link = lid - if output_type and inp_type == output_type and exact_link is None: - exact_link = lid - if compat_link is None and _is_valid_connection(inp_type, output_type): - compat_link = lid - chosen_link = exact_link if exact_link is not None else compat_link if chosen_link is None: - chosen_link = fallback_link + for inp in inputs: + if not isinstance(inp, dict): + continue + lid = inp.get("link") + if lid is None or lid not in self.link_map: + continue + inp_type = inp.get("type") + if fallback_link is None: + fallback_link = lid + if output_type and inp_type == output_type and exact_link is None: + exact_link = lid + if compat_link is None and _is_valid_connection(inp_type, output_type): + compat_link = lid + chosen_link = exact_link if exact_link is not None else compat_link + if chosen_link is None: + chosen_link = fallback_link - if chosen_link is None: - return src_id, src_slot - - ld = self.link_map[chosen_link] - upstream_id, upstream_slot = ld["source_id"], ld["source_slot"] - upstream_id, upstream_slot = self.trace_get_set(upstream_id, upstream_slot) - upstream_id, upstream_slot = self.trace_reroute(upstream_id, upstream_slot) - return self.trace_bypassed(upstream_id, upstream_slot, seen) + if chosen_link is None: + return src_id, src_slot + + ld = self.link_map[chosen_link] + upstream_id, upstream_slot = ld["source_id"], ld["source_slot"] + upstream_id, upstream_slot = self.trace_get_set(upstream_id, upstream_slot) + upstream_id, upstream_slot = self.trace_reroute(upstream_id, upstream_slot) + src_id, src_slot = upstream_id, upstream_slot + # Loop continues with the new src_id/src_slot. # --------------------------------------------------------------------------- diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index f3215bac..c12db1c7 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -1444,6 +1444,106 @@ def test_group_node_workflow_emits_warning(self, object_info, caplog): assert any("group node" in record.message.lower() for record in caplog.records) +class TestTracerChainDepth: + """The three tracers (``trace_reroute``, ``trace_get_set``, ``trace_bypassed``) + used to be tail-recursive. Python's default recursion limit (1000) meant + chains longer than ~997 hit ``RecursionError`` which the per-node + try/except then swallowed — silently dropping the downstream consumer + from the prompt. The iterative rewrite makes them depth-unbounded. + + These tests pick chain lengths well past the old crash threshold so any + future regression to the recursive form fails loudly. + """ + + def _consumer_id(self): + return "999999" + + def test_long_reroute_chain(self): + N = 2000 + nodes = [ + _node(0, "EmptyLatentImage", outputs=[{"links": [0]}], widgets=[256, 256, 1]), + ] + links = [] + for i in range(1, N + 1): + nodes.append( + _node( + i, + "Reroute", + inputs=[{"name": "", "type": "*", "link": i - 1}], + outputs=[{"name": "", "type": "*", "links": [i]}], + ) + ) + links.append([i - 1, i - 1, 0, i, 0, "*"]) + nodes.append(_node(int(self._consumer_id()), "PreviewImage", inputs=[{"name": "images", "link": N}])) + links.append([N, N, 0, int(self._consumer_id()), 0, "*"]) + result = convert_ui_to_api({"nodes": nodes, "links": links}, {}) + # Consumer must be present and wired through to node 0. + assert result[self._consumer_id()]["inputs"]["images"] == ["0", 0] + + def test_long_bypass_chain(self): + N = 2000 + nodes = [ + _node(0, "EmptyLatentImage", outputs=[{"links": [0]}], widgets=[256, 256, 1]), + ] + links = [] + prev = 0 + for i in range(N): + nid = 1000 + i + nodes.append( + { + "id": nid, + "type": "VAEDecode", + "inputs": [ + {"name": "samples", "type": "LATENT", "link": prev}, + {"name": "vae", "type": "VAE", "link": None}, + ], + "outputs": [{"name": "IMAGE", "type": "LATENT", "links": [10000 + i]}], + "mode": 4, # bypassed + } + ) + links.append([prev, prev if i == 0 else 1000 + i - 1, 0, nid, 0, "LATENT"]) + prev = 10000 + i + nodes.append(_node(int(self._consumer_id()), "PreviewImage", inputs=[{"name": "images", "link": prev}])) + links.append([prev, 1000 + N - 1, 0, int(self._consumer_id()), 0, "LATENT"]) + result = convert_ui_to_api({"nodes": nodes, "links": links}, {}) + assert result[self._consumer_id()]["inputs"]["images"] == ["0", 0] + + def test_long_getset_chain(self): + N = 2000 + nodes = [ + _node(0, "EmptyLatentImage", outputs=[{"links": [0]}], widgets=[256, 256, 1]), + ] + links = [] + prev = 0 + for i in range(N): + sid = 1000 + i + gid = 2000 + i + nodes.append( + { + "id": sid, + "type": "SetNode", + "inputs": [{"name": "value", "link": prev}], + "widgets_values": [f"v{i}"], + "mode": 0, + } + ) + nodes.append( + { + "id": gid, + "type": "GetNode", + "outputs": [{"links": [10000 + i]}], + "widgets_values": [f"v{i}"], + "mode": 0, + } + ) + links.append([prev, prev if i == 0 else 2000 + i - 1, 0, sid, 0, "LATENT"]) + prev = 10000 + i + nodes.append(_node(int(self._consumer_id()), "PreviewImage", inputs=[{"name": "images", "link": prev}])) + links.append([prev, 2000 + N - 1, 0, int(self._consumer_id()), 0, "LATENT"]) + result = convert_ui_to_api({"nodes": nodes, "links": links}, {}) + assert result[self._consumer_id()]["inputs"]["images"] == ["0", 0] + + class TestMutedBypassedSubgraph: """Per frontend semantics (executionUtil.ts), if the subgraph *instance* node is itself muted or bypassed, its inner nodes do NOT enter the prompt. From 6a563cd33884dd1942ed5ced1d067c6e801e6b57 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Thu, 14 May 2026 11:53:24 +0000 Subject: [PATCH 15/16] fix(workflow): handle '*' wildcard inputs and implicit seed companion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-checked against comfy-cloud-mcp-server's converter (TypeScript port of graphToPrompt with 455 oracle fixtures from real cloud workflows). Two real divergences identified and fixed: 1. ``*`` / empty-string wildcard input type: The frontend doesn't render a widget for these — they're connection-only slots (canonical case: ``PreviewAny.source: ["*", {}]``). Our ``_is_widget_input`` was treating them as widgets because the lowercase-fallback condition ``not input_type.isupper()`` returns ``True`` for strings with no cased characters. The bug consumed a widgets_values slot for the wildcard and shifted every later widget into the wrong input. 2. Implicit ``control_after_generate`` for ``seed`` / ``noise_seed``: The frontend's ``useIntWidget`` composable adds the companion widget for inputs literally named ``seed`` or ``noise_seed`` even when the schema doesn't declare ``control_after_generate: True``. Our filter only checked the schema flag, so explicit seeds worked but every implicit (workflow-saved-with-companion) case slid by one. New ``_has_control_after_generate_companion`` helper uses peek-based detection: only consumes the next value if it's actually one of ``fixed`` / ``increment`` / ``decrement`` / ``randomize``, so older workflows saved before the companion existed still convert correctly. Cloud-mcp oracle parity: 248 → 251 perfect matches (+3 fixtures). Stock ComfyUI blueprint parity unchanged at 46/51 + 5 cosmetic. 6 new tests cover both behaviors, including the edge cases where peek-detection prevents over-eager stripping. Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 38 ++++- tests/comfy_cli/test_workflow_to_api.py | 202 ++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 3 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index b48a3c4f..aacf2376 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -1038,6 +1038,14 @@ def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: if isinstance(input_type, (list, tuple)): return True, False # combo of choices if isinstance(input_type, str): + # ``*`` and ``""`` are wildcard *connection* types — the frontend + # never renders a widget for them. They slipped through the + # lowercase fallback below because they have no cased characters + # (``"*".isupper()`` returns ``False``), so we have to filter them + # out explicitly. PreviewAny.source: ["*", {}] is the canonical + # case this used to mis-handle. + if input_type in ("", "*"): + return False, False if input_type in {"INT", "FLOAT", "STRING", "BOOLEAN", "COMBO"}: return True, False if input_type.startswith("COMFY_") and "COMBO" in input_type: @@ -1187,7 +1195,7 @@ def is_control(v: Any) -> bool: section_def = input_def.get(section) or {} if not isinstance(section_def, dict): continue - for _input_name, input_spec in section_def.items(): + for input_name, input_spec in section_def.items(): if vidx >= len(widget_values): break is_widget, _is_dynamic = _is_widget_input(input_spec) @@ -1195,8 +1203,9 @@ def is_control(v: Any) -> bool: continue out.append(widget_values[vidx]) vidx += 1 - options = input_spec[1] if len(input_spec) >= 2 and isinstance(input_spec[1], dict) else {} - if options.get("control_after_generate") and vidx < len(widget_values) and is_control(widget_values[vidx]): + if vidx < len(widget_values) and _has_control_after_generate_companion( + input_name, input_spec, widget_values[vidx] + ): vidx += 1 while vidx < len(widget_values): out.append(widget_values[vidx]) @@ -1204,6 +1213,29 @@ def is_control(v: Any) -> bool: return out +def _has_control_after_generate_companion(input_name: str, input_spec: Any, next_value: Any) -> bool: + """True if ``next_value`` should be consumed as a control_after_generate marker. + + Two ways the frontend adds the companion widget: + + * Explicit: the input spec sets ``control_after_generate: True``. + * Implicit: the input is named ``seed`` or ``noise_seed`` and is INT-typed. + The frontend's ``useIntWidget`` composable adds the companion in that case + regardless of the schema flag. + + For the implicit path we peek at the next value: older workflows saved + before the companion existed don't have the marker string, so we must + verify the slot really is a control keyword before consuming it. + """ + options = input_spec[1] if len(input_spec) >= 2 and isinstance(input_spec[1], dict) else {} + if options.get("control_after_generate"): + return isinstance(next_value, str) and next_value in _CONTROL_AFTER_GENERATE_VALUES + input_type = input_spec[0] if input_spec else None + if input_type == "INT" and input_name in ("seed", "noise_seed"): + return isinstance(next_value, str) and next_value in _CONTROL_AFTER_GENERATE_VALUES + return False + + def _collect_widget_inputs( node: dict, node_type: str, object_info: dict, link_inputs: dict[str, list] ) -> dict[str, Any]: diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index c12db1c7..39e5ef3b 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -1067,6 +1067,208 @@ def test_unknown_node_falls_back_to_legacy_filter(self): convert_ui_to_api(workflow, {}) +class TestWildcardInputType: + """``*`` and ``""`` are wildcard *connection* types in litegraph. The + frontend never renders a widget for them — ``PreviewAny.source`` is the + canonical example. They previously slipped through the lowercase-fallback + in ``_is_widget_input`` because ``"*".isupper()`` returns ``False`` (no + cased characters), so the converter consumed a widgets_values slot for + them and shifted every later widget out of alignment. + """ + + OI = { + "Source": { + "input": {"required": {}}, + "input_order": {"required": []}, + "output_node": False, + "output": ["INT"], + "display_name": "Source", + }, + "PreviewAny": { + "input": { + "required": { + "source": ["*", {}], # wildcard connection — NOT a widget + } + }, + "input_order": {"required": ["source"]}, + "output_node": True, + "display_name": "Preview Any", + }, + "WildEmpty": { + "input": { + "required": { + "anything": ["", {}], # empty-string wildcard + "actual_widget": ["INT", {"default": 0}], + } + }, + "input_order": {"required": ["anything", "actual_widget"]}, + "output_node": True, + "display_name": "WildEmpty", + }, + } + + def test_star_wildcard_not_treated_as_widget(self): + workflow = { + "nodes": [ + _node(99, "Source", outputs=[{"links": [10]}]), + { + "id": 1, + "type": "PreviewAny", + "inputs": [{"name": "source", "type": "*", "link": 10}], + "outputs": [], + "widgets_values": [], + "mode": 0, + }, + ], + "links": [[10, 99, 0, 1, 0, "INT"]], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["source"] == ["99", 0] + + def test_empty_string_wildcard_does_not_consume_widget_slot(self): + # Old behavior would consume widgets_values[0] for the wildcard and + # emit nothing for actual_widget. Fixed: wildcard is connection-only, + # the single widget value maps to actual_widget. + workflow = { + "nodes": [ + { + "id": 1, + "type": "WildEmpty", + "inputs": [{"name": "anything", "type": "", "link": None}], + "outputs": [], + "widgets_values": [42], + "mode": 0, + }, + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["actual_widget"] == 42 + assert "anything" not in result["1"]["inputs"] + + +class TestImplicitSeedCompanion: + """The frontend's ``useIntWidget`` composable adds a + ``control_after_generate`` companion widget for inputs named ``seed`` or + ``noise_seed``, even when the schema doesn't declare the flag. Older + workflows saved before this behavior may not have the companion value + in widgets_values, so we use peek-based detection to handle both cases. + """ + + OI = { + "Sampler": { + "input": { + "required": { + "seed": ["INT", {"default": 0}], # no control_after_generate flag + "steps": ["INT", {"default": 20}], + "sampler_name": [["euler", "ddim"], {}], + } + }, + "input_order": {"required": ["seed", "steps", "sampler_name"]}, + "output_node": True, + "display_name": "Sampler", + }, + "NoiseUser": { + "input": { + "required": { + "noise_seed": ["INT", {"default": 0}], + "denoise": ["FLOAT", {"default": 1.0}], + } + }, + "input_order": {"required": ["noise_seed", "denoise"]}, + "output_node": True, + "display_name": "NoiseUser", + }, + "RegularInt": { + "input": { + "required": { + "value": ["INT", {"default": 0}], + "label": ["STRING", {}], + } + }, + "input_order": {"required": ["value", "label"]}, + "output_node": True, + "display_name": "RegularInt", + }, + } + + def test_seed_named_input_strips_implicit_companion(self): + workflow = { + "nodes": [ + { + "id": 1, + "type": "Sampler", + "inputs": [], + "outputs": [], + "widgets_values": [42, "randomize", 25, "euler"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"] == {"seed": 42, "steps": 25, "sampler_name": "euler"} + + def test_noise_seed_named_input_strips_implicit_companion(self): + workflow = { + "nodes": [ + { + "id": 1, + "type": "NoiseUser", + "inputs": [], + "outputs": [], + "widgets_values": [12345, "fixed", 0.85], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"] == {"noise_seed": 12345, "denoise": 0.85} + + def test_seed_input_without_companion_still_works(self): + # Older saved workflows from before the implicit-companion era don't + # have the marker in widgets_values. Peek-based detection avoids + # consuming a non-control value. + workflow = { + "nodes": [ + { + "id": 1, + "type": "Sampler", + "inputs": [], + "outputs": [], + "widgets_values": [42, 25, "euler"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"] == {"seed": 42, "steps": 25, "sampler_name": "euler"} + + def test_regular_int_input_does_not_strip_control_value(self): + # A non-seed INT input has no implicit companion. A widget value that + # happens to equal "randomize" must not be stripped — it slides into + # the next slot. The user has bad data, but our filter shouldn't + # silently eat it. + workflow = { + "nodes": [ + { + "id": 1, + "type": "RegularInt", + "inputs": [], + "outputs": [], + "widgets_values": [99, "randomize"], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, self.OI) + assert result["1"]["inputs"]["value"] == 99 + assert result["1"]["inputs"]["label"] == "randomize" + + class TestNodeNameForSAndRAlias: """When a node carries ``properties["Node name for S&R"]`` pointing at a different class name than its ``type`` field (legacy rename / group-node From 79a1b1603563e6dec4384533c789b1d9a658e567 Mon Sep 17 00:00:00 2001 From: Alexander Piskun Date: Thu, 14 May 2026 12:10:47 +0000 Subject: [PATCH 16/16] fix(workflow): align node-inclusion policy with frontend / cloud-mcp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two coupled changes to match graphToPrompt()'s emission policy exactly: 1. Drop the dead-branch exclusion heuristic in _collect_excluded. The old rule dropped any node whose outputs weren't connected downstream (unless the schema marked it as output_node or it had no schema + connected inputs). That excluded legitimate sources like an unwired LoadAudio that the frontend's graphToPrompt() still emits — costing 20+ real cloud workflows their LoadAudio / VAEDecode / etc. nodes when round-tripped through our converter. The frontend (executionUtil.ts) and cloud-mcp-server's shouldIncludeInOutput emit every non-virtual, non-muted, non-bypassed node regardless of downstream wiring. The executor only runs nodes reachable from sinks (SaveImage etc.) — leftover unwired nodes are harmless in the prompt JSON. 2. Add MarkdownNote to _UI_ONLY_NODE_TYPES. Once dead-branch exclusion is gone, MarkdownNote nodes (purely frontend documentation, no Python class) would start being emitted. Cloud-mcp-server lists it explicitly in VIRTUAL_NODE_TYPES; we match. Measured against cloud-mcp-server's 455-fixture oracle: before: 251 exact + 18 functional, 434/455 would-execute (95.4%) after: 255 exact + 20 functional, 454/455 would-execute (99.8%) ^^^^^^^^^ matches cloud-mcp's own MIN_WOULD_EXECUTE gate Stock ComfyUI blueprint parity (vs SethRobinson reference): before: 46 perfect + 5 cosmetic title-only + 0 real diff after: 43 perfect + 5 cosmetic title-only + 3 real diff The 3 stock regressions emit a VAEDecode whose output isn't wired downstream. They still execute correctly — the decode runs and the result is discarded. SethRobinson's converter dropped these via the same heuristic we're removing; the frontend keeps them. 3 tests updated/replaced (test_dead_branch_excluded → test_unwired_node_still_emitted, similar for the S&R-alias dead-branch case), 2 new tests added (markdown_note_excluded, unwired_load_node_still_emitted). Signed-off-by: Alexander Piskun --- comfy_cli/workflow_to_api.py | 54 ++++++----------- tests/comfy_cli/test_workflow_to_api.py | 81 +++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 40 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index aacf2376..5f094cab 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -36,7 +36,9 @@ _MODE_BYPASS = 4 # node skipped; inputs passed through to outputs # Node types that exist only in the UI graph and never appear in API output. -_UI_ONLY_NODE_TYPES = frozenset({"Note", "PrimitiveNode", "GetNode", "SetNode", "Reroute"}) +# Aligns with cloud-mcp-server's VIRTUAL_NODE_TYPES and the frontend's +# isVirtualNode set — every type the frontend's graphToPrompt() skips. +_UI_ONLY_NODE_TYPES = frozenset({"Note", "MarkdownNote", "PrimitiveNode", "GetNode", "SetNode", "Reroute"}) # Sentinel IDs litegraph uses inside a subgraph definition for the synthetic # input and output proxy nodes (the boxes the user wires through). @@ -116,7 +118,7 @@ def convert_ui_to_api(workflow: dict, object_info: dict) -> dict: node_by_id = {str(n.get("id")): n for n in nodes} primitive_values = _collect_primitive_values(nodes) bypassed = _collect_bypassed(nodes) - nodes_to_exclude = _collect_excluded(nodes, object_info) + nodes_to_exclude = _collect_excluded(nodes) reroute_sources = _collect_reroute_sources(nodes, link_map) set_sources, get_vars = _collect_get_set_mappings(nodes, link_map) @@ -602,41 +604,25 @@ def _collect_get_set_mappings( return set_sources, get_vars -def _collect_excluded(nodes: list[dict], object_info: dict) -> set[str]: - """Identify UI-only nodes and dead branches that don't belong in the API output.""" - excluded: set[str] = set() - for node in nodes: - node_type = node.get("type") - node_id_str = str(node.get("id")) - - # LoadImageOutput is a frontend convenience for picking from the output folder. - if node_type == "LoadImageOutput": - excluded.add(node_id_str) - continue +def _collect_excluded(nodes: list[dict]) -> set[str]: + """Identify nodes that should never appear in the API output. - outputs = node.get("outputs") or [] - has_connected_output = any(isinstance(out, dict) and out.get("links") for out in outputs) - if outputs and has_connected_output: - continue + Only ``LoadImageOutput`` is excluded here — it's a UI-only file picker + for browsing the output folder, with no Python class behind it. All + other UI-only types are filtered by name via ``_UI_ONLY_NODE_TYPES``. - # _schema_for honors the ``Node name for S&R`` alias the same way - # _build_api_node and _collect_widget_inputs do; consulting raw - # object_info[node_type] would mark an aliased node as schemaless - # and incorrectly exclude it via the dead-branch heuristic. - schema = _schema_for(node_type, node, object_info) - is_output_node = bool(isinstance(schema, dict) and schema.get("output_node")) - has_connected_input = any( - isinstance(inp, dict) and inp.get("link") is not None for inp in (node.get("inputs") or []) - ) + Matches the frontend's policy (``executionUtil.ts:graphToPrompt``) and + cloud-mcp-server's ``shouldIncludeInOutput`` of emitting every + non-virtual, non-muted, non-bypassed node regardless of whether its + outputs are wired. The executor only runs nodes reachable from sinks + (SaveImage, etc.), so unwired nodes are harmless in the prompt. - # Keep output nodes (SaveImage, PreviewImage) even with no downstream consumer. - # Without a schema, keep anything that has at least one wired input — likely a sink. - if is_output_node: - continue - if not schema and has_connected_input: - continue - excluded.add(node_id_str) - return excluded + We previously applied a "dead-branch" heuristic that dropped any node + with no downstream consumer; that excluded legitimate sources like an + unwired ``LoadAudio`` and caused 20+ cloud-mcp oracle fixtures to lose + nodes that the live frontend emits. + """ + return {str(n.get("id")) for n in nodes if n.get("type") == "LoadImageOutput"} class _Tracers: diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 39e5ef3b..aa786bb1 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -460,8 +460,13 @@ def test_output_node_kept_even_without_outgoing_links(self, object_info): result = convert_ui_to_api(workflow, object_info) assert "2" in result - def test_dead_branch_excluded(self, object_info): - # Node 99 has neither connected outputs nor a schema marking it as output. + def test_unwired_node_still_emitted(self, object_info): + # A node with no connected outputs and no schema-declared output_node + # used to be dropped by an aggressive "dead-branch" heuristic. The + # frontend's graphToPrompt() emits every non-virtual, non-muted, + # non-bypassed node regardless — the executor only runs nodes + # reachable from sinks, so leftover unwired nodes are harmless. + # See cloud-mcp-server/src/converter/nodeFilter.ts shouldIncludeInOutput. workflow = { "nodes": [ _node( @@ -474,7 +479,66 @@ def test_dead_branch_excluded(self, object_info): "links": [], } result = convert_ui_to_api(workflow, object_info) - assert result == {} + assert "99" in result + assert result["99"]["class_type"] == "EmptyLatentImage" + assert result["99"]["inputs"] == {"width": 64, "height": 64, "batch_size": 1} + + def test_unwired_load_node_still_emitted(self, object_info): + # Real cloud-mcp regression: a saved workflow has a LoadAudio that the + # user added but didn't yet wire to anything. The frontend's + # graphToPrompt() emits it; we used to drop it via dead-branch + # exclusion, losing the node entirely from the API output. + load_audio_schema = { + "LoadAudio": { + "input": { + "required": { + "audio": [["song.mp3"], {}], + } + }, + "input_order": {"required": ["audio"]}, + "output_node": False, + "output": ["AUDIO"], + "display_name": "Load Audio", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "LoadAudio", + "inputs": [], + "outputs": [{"name": "AUDIO", "type": "AUDIO", "links": None}], + "widgets_values": ["song.mp3"], + "mode": 0, + }, + ], + "links": [], + } + result = convert_ui_to_api(workflow, load_audio_schema) + assert "1" in result + assert result["1"]["inputs"] == {"audio": "song.mp3"} + + def test_markdown_note_excluded(self, object_info): + # MarkdownNote is a UI-only documentation node with no Python class + # behind it. Must never appear in the API output even when not + # otherwise filtered out by dead-branch logic (which we no longer + # apply). + workflow = { + "nodes": [ + _node( + 1, + "MarkdownNote", + outputs=[], + widgets=["# Heading\n\nSome documentation"], + ), + _node(2, "EmptyLatentImage", outputs=[{"links": [1]}], widgets=[512, 512, 1]), + _node(3, "PreviewImage", inputs=[{"name": "images", "link": 1}], outputs=[]), + ], + "links": [[1, 2, 0, 3, 0, "IMAGE"]], + } + result = convert_ui_to_api(workflow, object_info) + assert "1" not in result + assert {"2", "3"} <= set(result) # --------------------------------------------------------------------------- @@ -1338,9 +1402,10 @@ def test_defaults_filled_from_aliased_schema(self): result = convert_ui_to_api(self._aliased_workflow(widgets_values=["euler"]), self.OI) assert result["1"]["inputs"]["missing_widget"] == 99 - def test_dead_branch_check_uses_aliased_schema(self): - # No connected outputs, but the aliased schema marks it as OUTPUT_NODE - # so it must survive the dead-branch exclusion. + def test_aliased_node_with_no_connections_still_emits(self): + # Even with no wired connections, the node should be emitted (we no + # longer apply a dead-branch heuristic). The aliased schema's + # display_name and defaults still apply correctly. workflow = { "nodes": [ { @@ -1357,6 +1422,10 @@ def test_dead_branch_check_uses_aliased_schema(self): } result = convert_ui_to_api(workflow, self.OI) assert "1" in result + assert result["1"]["_meta"]["title"] == "Real Sampler" + assert result["1"]["inputs"]["sampler"] == "euler" + # missing_widget filled from the aliased schema's default. + assert result["1"]["inputs"]["missing_widget"] == 99 class TestForceInputHandling: