From 347cee4e5289cd8ada558af1f084d0e82103137a Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 17 Jul 2026 17:18:01 -0700 Subject: [PATCH 1/2] fix(workflow_to_api): widget-aware dynamic-combo expansion + name-aligned control-marker filtering (BE-3370) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pristine COMFY_DYNAMICCOMBO_V3 template (e.g. Seedream) converted to invalid API JSON: node 1 produced a phantom "model.images": 0 (stealing the seed value) and "seed": "randomize" (unfiltered control marker), which the server rejected (seed: expected INT, got str). Two walks disagreed. _get_widget_name_order expanded a dynamic combo's options into EVERY sub-input name — including connection-only subs like Seedream's COMFY_AUTOGROW_V3 images, which own no widgets_values slot — so every later widget shifted by one. _filter_control_values walked schema inputs positionally WITHOUT dynamic expansion, so when a dynamic combo preceded a control_after_generate seed it checked the companion at the wrong index and the marker survived. Unify both into a single ordered pass, _schema_widget_pairs: expand dynamic combos in place (applying _is_widget_input to each sub-input so connection subs are skipped; recursing for nested dynamic combos) and drop control markers inline as each widget is consumed, so name order and marker filtering can never disagree. Non-dynamic nodes keep identical behavior; unknown/ schema-less nodes fall back to the positional heuristic as before. Add a faithful /object_info fixture for ByteDanceSeedreamNodeV2 and vendor the pristine pro-t2i template; add regression tests for the pristine template and a batch-capable option case (both fail before the fix). --- comfy_cli/workflow_to_api.py | 126 ++++++---- .../object_info_bytedance_seedream_v2.json | 227 ++++++++++++++++++ .../fixtures/seedream_5_0_pro_t2i_ui.json | 117 +++++++++ tests/comfy_cli/test_workflow_to_api.py | 94 +++++++- 4 files changed, 512 insertions(+), 52 deletions(-) create mode 100644 tests/comfy_cli/fixtures/object_info_bytedance_seedream_v2.json create mode 100644 tests/comfy_cli/fixtures/seedream_5_0_pro_t2i_ui.json diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 5f094cab..5fb6e05c 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -1041,63 +1041,84 @@ def _is_widget_input(input_spec: Any) -> tuple[bool, bool]: return False, False -def _dynamic_combo_sub_inputs( - input_name: str, input_spec: Any, widget_values: list[Any], current_idx: int -) -> list[str]: +def _dynamic_combo_selected_subs(input_name: str, input_spec: Any, selected: Any) -> list[tuple[str, Any]]: + """``(dotted_name, spec)`` pairs for the selected option's sub-inputs. + + The dynamic combo's ``widgets_values`` selector value picks one option; that + option's ``inputs`` mirror an INPUT_TYPES dict. We return every sub-input's + *spec* (dotted with the parent name, e.g. ``model.size_preset``) and leave + the widget-vs-connection decision to the caller's ``_is_widget_input`` — the + same test applied to top-level inputs — so a connection-only sub-input (e.g. + a ``COMFY_AUTOGROW_V3`` image list) consumes no ``widgets_values`` slot. + Returns ``[]`` for an unknown selection or a malformed option block (the + latter would otherwise crash the per-node wrapper and drop the whole node). + """ 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") - # 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] = [] + subs: list[tuple[str, Any]] = [] 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 + for sub_name, sub_spec in section_def.items(): + subs.append((f"{input_name}.{sub_name}", sub_spec)) + return subs 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_input_def(schema) - 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 +def _schema_widget_pairs(schema: Any, widget_values: list[Any]) -> list[tuple[str, Any]]: + """Pair a schema's widget inputs with their ``widgets_values`` slots. + + One ordered walk that unifies name-order and control-marker filtering so the + two can never disagree. For each schema input in order: + + * skip non-widget inputs (connections, ``forceInput`` demotions, wildcards) + — they own no slot; + * consume one slot for a widget input, emitting ``(name, value)``; + * for a V3 dynamic combo (``COMFY_*COMBO*``) the consumed value is the + selector — recurse into the selected option's *widget* sub-inputs, each + consuming a following slot (connection-only subs are skipped, nested + dynamic combos recurse), so ``model.size_preset``/``model.width`` land in + order and ``model.images`` never steals a slot; + * drop a trailing ``control_after_generate`` marker string when the + just-consumed input is control-flagged (explicit flag or an implicit INT + ``seed``/``noise_seed``) — sub-inputs are handled identically via recursion. + + Returns ``[]`` when the schema declares no widget inputs, so the caller can + fall back to node-input inspection exactly as before. + """ + input_def = _schema_input_def(schema) + pairs: list[tuple[str, Any]] = [] + vidx = 0 - # Fallback: inspect the node's own input list. Some nodes mark widget-flagged inputs. - return _fallback_widget_names(node, widget_values) + def consume(name: str, spec: Any) -> None: + nonlocal vidx + is_widget, is_dynamic = _is_widget_input(spec) + if not is_widget or vidx >= len(widget_values): + return + value = widget_values[vidx] + pairs.append((name, value)) + vidx += 1 + if is_dynamic: + for sub_name, sub_spec in _dynamic_combo_selected_subs(name, spec, value): + consume(sub_name, sub_spec) + elif vidx < len(widget_values) and _has_control_after_generate_companion(name, spec, widget_values[vidx]): + vidx += 1 + + 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(): + consume(input_name, input_spec) + return pairs def _fallback_widget_names(node: dict, widget_values: list[Any]) -> list[str | None]: @@ -1252,13 +1273,26 @@ def emit(name: str, value: Any) -> Any: _absorb_dict_widget_values(widget_values, out, link_inputs) return out + # When a schema is available, a single walk pairs names with values while + # expanding V3 dynamic combos and dropping control markers together — the + # name order and the marker filtering can never disagree (which is what let + # a dynamic combo before a seed steal the seed's slot, e.g. Seedream's + # ``model`` before ``seed``). It also skips connection-only sub-inputs + # (e.g. ``COMFY_AUTOGROW_V3`` images) so they never consume a value slot. + schema = _schema_for(node_type, node, object_info) + pairs = _schema_widget_pairs(schema, widget_values) if schema else [] + if pairs: + for name, value in pairs: + if not name or name in link_inputs: + continue + out[name] = emit(name, value) + return out + + # No schema, or the schema declares no widget inputs: fall back to the + # positional control-marker heuristic + node-input name inspection, which + # matches the reference behavior for unknown node types. filtered = _filter_control_values(widget_values, node_type, node, object_info) - # ``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) + names = _fallback_widget_names(node, filtered) if not names: if filtered: logger.warning( diff --git a/tests/comfy_cli/fixtures/object_info_bytedance_seedream_v2.json b/tests/comfy_cli/fixtures/object_info_bytedance_seedream_v2.json new file mode 100644 index 00000000..87279241 --- /dev/null +++ b/tests/comfy_cli/fixtures/object_info_bytedance_seedream_v2.json @@ -0,0 +1,227 @@ +{ + "ByteDanceSeedreamNodeV2": { + "input": { + "required": { + "prompt": [ + "STRING", + { + "multiline": true, + "default": "", + "tooltip": "Text prompt for creating or editing an image." + } + ], + "model": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "seedream 5.0 pro", + "inputs": { + "required": { + "size_preset": [ + "COMBO", + { + "options": [ + "(1K) 1024x1024 (1:1)", + "(1K) 864x1152 (3:4)", + "(1K) 1152x864 (4:3)", + "(2K) 2048x2048 (1:1)", + "Custom" + ], + "tooltip": "Pick a recommended size. Select Custom to use the width and height below." + } + ], + "width": [ + "INT", + {"default": 2048, "min": 1024, "max": 6240, "step": 2} + ], + "height": [ + "INT", + {"default": 2048, "min": 1024, "max": 4992, "step": 2} + ], + "images": [ + "COMFY_AUTOGROW_V3", + { + "template": {"image": ["IMAGE", {}]}, + "names": ["image_1", "image_2", "image_3", "image_4"], + "min": 0, + "tooltip": "Optional reference image(s) for image-to-image or multi-reference generation." + } + ] + } + } + }, + { + "key": "seedream 5.0 lite", + "inputs": { + "required": { + "size_preset": [ + "COMBO", + { + "options": [ + "(1K) 1024x1024 (1:1)", + "(2K) 2048x2048 (1:1)", + "(3K) 3072x3072 (1:1)", + "(4K) 4096x4096 (1:1)", + "Custom" + ], + "tooltip": "Pick a recommended size. Select Custom to use the width and height below." + } + ], + "width": [ + "INT", + {"default": 2048, "min": 1024, "max": 6240, "step": 2} + ], + "height": [ + "INT", + {"default": 2048, "min": 1024, "max": 4992, "step": 2} + ], + "max_images": [ + "INT", + {"default": 1, "min": 1, "max": 14, "step": 1} + ], + "images": [ + "COMFY_AUTOGROW_V3", + { + "template": {"image": ["IMAGE", {}]}, + "names": ["image_1", "image_2", "image_3", "image_4"], + "min": 0, + "tooltip": "Optional reference image(s) for image-to-image or multi-reference generation." + } + ], + "fail_on_partial": [ + "BOOLEAN", + {"default": false, "advanced": true} + ] + } + } + }, + { + "key": "seedream-4-5-251128", + "inputs": { + "required": { + "size_preset": [ + "COMBO", + { + "options": [ + "(2K) 2048x2048 (1:1)", + "(4K) 4096x4096 (1:1)", + "Custom" + ] + } + ], + "width": [ + "INT", + {"default": 2048, "min": 1024, "max": 6240, "step": 2} + ], + "height": [ + "INT", + {"default": 2048, "min": 1024, "max": 4992, "step": 2} + ], + "max_images": [ + "INT", + {"default": 1, "min": 1, "max": 10, "step": 1} + ], + "images": [ + "COMFY_AUTOGROW_V3", + { + "template": {"image": ["IMAGE", {}]}, + "names": ["image_1", "image_2", "image_3", "image_4"], + "min": 0 + } + ], + "fail_on_partial": [ + "BOOLEAN", + {"default": false, "advanced": true} + ] + } + } + }, + { + "key": "seedream-4-0-250828", + "inputs": { + "required": { + "size_preset": [ + "COMBO", + { + "options": [ + "(1K) 1024x1024 (1:1)", + "(2K) 2048x2048 (1:1)", + "(4K) 4096x4096 (1:1)", + "Custom" + ] + } + ], + "width": [ + "INT", + {"default": 2048, "min": 1024, "max": 6240, "step": 2} + ], + "height": [ + "INT", + {"default": 2048, "min": 1024, "max": 4992, "step": 2} + ], + "max_images": [ + "INT", + {"default": 1, "min": 1, "max": 10, "step": 1} + ], + "images": [ + "COMFY_AUTOGROW_V3", + { + "template": {"image": ["IMAGE", {}]}, + "names": ["image_1", "image_2", "image_3", "image_4"], + "min": 0 + } + ], + "fail_on_partial": [ + "BOOLEAN", + {"default": false, "advanced": true} + ] + } + } + } + ], + "tooltip": "Model variant to use for generation." + } + ], + "seed": [ + "INT", + { + "default": 0, + "min": 0, + "max": 2147483647, + "step": 1, + "control_after_generate": true, + "tooltip": "Seed to use for generation." + } + ], + "watermark": [ + "BOOLEAN", + { + "default": false, + "advanced": true, + "tooltip": "Whether to add an \"AI generated\" watermark to the image." + } + ] + }, + "optional": { + "thinking": [ + "BOOLEAN", + {"default": true, "advanced": true} + ] + } + }, + "input_order": { + "required": ["prompt", "model", "seed", "watermark"], + "optional": ["thinking"] + }, + "output": ["IMAGE"], + "output_is_list": [false], + "output_name": ["IMAGE"], + "output_node": false, + "name": "ByteDanceSeedreamNodeV2", + "display_name": "ByteDance Seedream 4.5 & 5.0", + "description": "Unified text-to-image generation and precise single-sentence editing at up to 4K resolution.", + "category": "partner/image/ByteDance", + "api_node": true + } +} diff --git a/tests/comfy_cli/fixtures/seedream_5_0_pro_t2i_ui.json b/tests/comfy_cli/fixtures/seedream_5_0_pro_t2i_ui.json new file mode 100644 index 00000000..6dfe0969 --- /dev/null +++ b/tests/comfy_cli/fixtures/seedream_5_0_pro_t2i_ui.json @@ -0,0 +1,117 @@ +{ + "id": "bcb1c6ea-1b65-481f-99a3-2a17a6ab92dc", + "revision": 0, + "last_node_id": 2, + "last_link_id": 1, + "nodes": [ + { + "id": 1, + "type": "ByteDanceSeedreamNodeV2", + "pos": [ + 760, + 430 + ], + "size": [ + 400, + 328 + ], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "label": "image_1", + "name": "model.images.image_1", + "shape": 7, + "type": "IMAGE", + "link": null + } + ], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "links": [ + 1 + ] + } + ], + "properties": { + "Node name for S&R": "ByteDanceSeedreamNodeV2" + }, + "widgets_values": [ + "A sci-fi cyberpunk graphic design poster. In the center, a striking portrait of a female android with glossy, liquid chrome skin. A vivid swirling streak of neon orange, yellow, and pink liquid paint brush stroke horizontally covers her eyes, soft smudged color overlay with smooth flowing pigment texture, no cracks or broken facial surfaces. The background is a dark, textured charcoal gray. Behind the central figure, large bold white futuristic typography reads \"Seedream 5.0 Pro\" with a subtle digital glitch and halftone texture. The composition is decorated with technical HUD elements, thin white lines, geometric wireframe diagrams, barcodes, and small sci-fi text boxes. Semi-transparent glassy spheres with grid textures overlap the foreground, creating depth. The overall mood is high-tech, dystopian, and avant-garde.", + "seedream 5.0 pro", + "(1K) 1024x1024 (1:1)", + 2048, + 2048, + 0, + "randomize", + false + ], + "color": "#432", + "bgcolor": "#653" + }, + { + "id": 2, + "type": "SaveImageAdvanced", + "pos": [ + 1200, + 430 + ], + "size": [ + 580, + 168 + ], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "name": "images", + "type": "IMAGE", + "link": 1 + } + ], + "outputs": [ + { + "name": "images", + "type": "IMAGE", + "links": null + } + ], + "properties": { + "Node name for S&R": "SaveImageAdvanced" + }, + "widgets_values": [ + "Seedream5.0_Pro_T2I", + "png", + "8-bit", + "sRGB" + ] + } + ], + "links": [ + [ + 1, + 1, + 0, + 2, + 0, + "IMAGE" + ] + ], + "groups": [], + "config": {}, + "extra": { + "ds": { + "scale": 1.2461538461538462, + "offset": [ + -552.2540509259259, + -179.88908179012347 + ] + }, + "frontendVersion": "1.45.20" + }, + "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 index aa786bb1..7ea61615 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -903,7 +903,7 @@ def test_outer_subgraph_node_with_non_dict_inputs_does_not_crash(self, object_in 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 + # used to crash _dynamic_combo_selected_subs; 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 = { @@ -1902,11 +1902,11 @@ def test_bypassed_subgraph_passes_external_input_through(self, object_info): 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. + """Regression: the unified widget walk (_schema_widget_pairs) must consume + the control_after_generate marker inline so a V3 dynamic combo whose schema + sits after a seed still reads its selector from the right slot. Before the + unified pass the selector was read from the wrong slot (the control marker), + the option was never identified, and every sub-input value was dropped. Affects 38 stock API nodes that pair a seed with a dynamic combo: Bria*, ByteDance*, Grok*, Kling*, Meshy*, Recraft*, Reve*, Vidu*, Wan2*, @@ -2199,3 +2199,85 @@ def test_simple_subgraph_expansion(self, object_info): assert "100:50" in result # Link from the external EmptyLatentImage was retargeted at the internal node. assert result["100:50"]["inputs"]["images"] == ["7", 0] + + +class TestSeedreamDynamicCombo: + """Regression: a pristine ``COMFY_DYNAMICCOMBO_V3`` template must convert to + valid API JSON. + + Two defects used to compound on the ByteDance Seedream node (whose ``model`` + dynamic combo precedes a ``control_after_generate`` seed): + + 1. The connection-only ``images`` sub-input (``COMFY_AUTOGROW_V3``) was + treated as a widget, so it consumed a value slot and shifted every later + widget by one — the seed value landed in a phantom ``model.images`` key. + 2. The control-marker filter walked the schema without dynamic expansion, so + the ``"randomize"`` marker after the (mis-aligned) seed survived and the + server rejected ``seed`` as a string. + """ + + @pytest.fixture + def seedream_object_info(self): + return json.loads((FIXTURES / "object_info_bytedance_seedream_v2.json").read_text()) + + def test_pristine_pro_t2i_template_converts(self, seedream_object_info): + ui = json.loads((FIXTURES / "seedream_5_0_pro_t2i_ui.json").read_text()) + result = convert_ui_to_api(ui, seedream_object_info) + inputs = result["1"]["inputs"] + + assert isinstance(inputs["prompt"], str) and inputs["prompt"] + assert inputs["model"] == "seedream 5.0 pro" + assert inputs["model.size_preset"] == "(1K) 1024x1024 (1:1)" + assert inputs["model.width"] == 2048 + assert inputs["model.height"] == 2048 + assert inputs["seed"] == 0 + assert isinstance(inputs["seed"], int) and not isinstance(inputs["seed"], bool) + assert inputs["watermark"] is False + # Optional input not in widgets_values is filled from its schema default. + assert inputs["thinking"] is True + + # The connection-only ``images`` sub-input must never become a widget, + # and the control marker must never leak into the value map. + assert "model.images" not in inputs + assert "randomize" not in inputs.values() + + def test_batch_option_maps_max_images_and_fail_on_partial(self, seedream_object_info): + # The "seedream 5.0 lite" option carries the batch shape: max_images and + # fail_on_partial widgets around the connection-only images sub-input. + workflow = { + "nodes": [ + { + "id": 1, + "type": "ByteDanceSeedreamNodeV2", + "inputs": [], + "outputs": [{"links": None}], + "widgets_values": [ + "a prompt", + "seedream 5.0 lite", + "(1K) 1024x1024 (1:1)", + 2048, + 2048, + 2, + False, + 7, + "fixed", + True, + ], + "mode": 0, + } + ], + "links": [], + } + inputs = convert_ui_to_api(workflow, seedream_object_info)["1"]["inputs"] + + assert inputs["model"] == "seedream 5.0 lite" + assert inputs["model.size_preset"] == "(1K) 1024x1024 (1:1)" + assert inputs["model.width"] == 2048 + assert inputs["model.height"] == 2048 + assert inputs["model.max_images"] == 2 + assert inputs["model.fail_on_partial"] is False + assert inputs["seed"] == 7 + assert inputs["watermark"] is True + + assert "model.images" not in inputs + assert "fixed" not in inputs.values() From 9467b30eb1a8e409eb3af768ec374c0f66f5df66 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 17 Jul 2026 17:45:04 -0700 Subject: [PATCH 2/2] fix(workflow_to_api): harden dynamic-combo expansion per Cursor review (BE-3370) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address three cursor-review findings on the widget-aware dynamic-combo walk: - Implicit control_after_generate companion for a seed/noise_seed *sub-input* now matches on the dotted name's leaf segment (model.seed), so its trailing "randomize"/"fixed" marker is stripped instead of shifting later sub-inputs. - An unresolved dynamic-combo selector (value names no option in the current schema — model renamed/removed server-side or version skew) now logs a warning instead of silently misaligning every following widget. - Bound the dynamic-combo recursion at depth 16 so a pathological nested chain from a third-party /object_info degrades to a warning rather than an uncaught RecursionError that aborts the whole conversion. Adds regression tests for each. Co-Authored-By: Claude Opus 4.8 --- comfy_cli/workflow_to_api.py | 49 ++++++++- tests/comfy_cli/test_workflow_to_api.py | 138 ++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 4 deletions(-) diff --git a/comfy_cli/workflow_to_api.py b/comfy_cli/workflow_to_api.py index 5fb6e05c..91adab2b 100644 --- a/comfy_cli/workflow_to_api.py +++ b/comfy_cli/workflow_to_api.py @@ -55,6 +55,13 @@ # the widget-value list before mapping to input names. _CONTROL_AFTER_GENERATE_VALUES = frozenset({"fixed", "increment", "decrement", "randomize"}) +# A dynamic combo may nest another dynamic combo among its sub-inputs. Real +# schemas nest at most a couple of levels; a deeper chain (e.g. a pathological +# third-party ``/object_info`` entry) would otherwise recurse the widget walk +# without bound. Cap it so a malformed schema degrades to a warning instead of a +# ``RecursionError`` that aborts the whole conversion. +_MAX_DYNAMIC_COMBO_DEPTH = 16 + class WorkflowConversionError(Exception): """Raised when a workflow can't be converted to API format.""" @@ -1073,6 +1080,15 @@ def _dynamic_combo_selected_subs(input_name: str, input_spec: Any, selected: Any return [] +def _dynamic_combo_option_keys(input_spec: Any) -> list[Any]: + """The ``key`` of every option a dynamic combo declares (order preserved).""" + 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 [] + return [option.get("key") for option in options if isinstance(option, dict)] + + def _schema_widget_pairs(schema: Any, widget_values: list[Any]) -> list[tuple[str, Any]]: """Pair a schema's widget inputs with their ``widgets_values`` slots. @@ -1098,7 +1114,7 @@ def _schema_widget_pairs(schema: Any, widget_values: list[Any]) -> list[tuple[st pairs: list[tuple[str, Any]] = [] vidx = 0 - def consume(name: str, spec: Any) -> None: + def consume(name: str, spec: Any, depth: int = 0) -> None: nonlocal vidx is_widget, is_dynamic = _is_widget_input(spec) if not is_widget or vidx >= len(widget_values): @@ -1107,8 +1123,29 @@ def consume(name: str, spec: Any) -> None: pairs.append((name, value)) vidx += 1 if is_dynamic: - for sub_name, sub_spec in _dynamic_combo_selected_subs(name, spec, value): - consume(sub_name, sub_spec) + subs = _dynamic_combo_selected_subs(name, spec, value) + if not subs and value not in _dynamic_combo_option_keys(spec): + # The saved selector no longer names any option in the current + # schema (model renamed/removed server-side, or object_info / + # workflow version skew). Its sub-input value slots go + # unconsumed, so every following widget reads a shifted slot. + # We can't recover the alignment, but warn so the corruption + # isn't silent. + logger.warning( + "Dynamic-combo input %r selector %r matched no option in the current " + "schema; following widget values may be misaligned", + name, + value, + ) + elif depth >= _MAX_DYNAMIC_COMBO_DEPTH: + logger.warning( + "Dynamic-combo nesting for input %r exceeded depth %d; stopping sub-input expansion", + name, + _MAX_DYNAMIC_COMBO_DEPTH, + ) + else: + for sub_name, sub_spec in subs: + consume(sub_name, sub_spec, depth + 1) elif vidx < len(widget_values) and _has_control_after_generate_companion(name, spec, widget_values[vidx]): vidx += 1 @@ -1238,7 +1275,11 @@ def _has_control_after_generate_companion(input_name: str, input_spec: Any, next 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"): + # ``input_name`` may be dotted for a dynamic-combo sub-input (e.g. + # ``model.seed``); the frontend's implicit companion keys off the leaf + # widget name, so match on the final segment. + leaf_name = input_name.rsplit(".", 1)[-1] + if input_type == "INT" and leaf_name in ("seed", "noise_seed"): return isinstance(next_value, str) and next_value in _CONTROL_AFTER_GENERATE_VALUES return False diff --git a/tests/comfy_cli/test_workflow_to_api.py b/tests/comfy_cli/test_workflow_to_api.py index 7ea61615..cfffe891 100644 --- a/tests/comfy_cli/test_workflow_to_api.py +++ b/tests/comfy_cli/test_workflow_to_api.py @@ -1956,6 +1956,144 @@ def test_dynamic_combo_selector_reads_from_filtered_slot(self): # Without the fix the sub-input was silently dropped. assert inputs["shape.side"] == 10.0 + def test_dynamic_combo_sub_seed_strips_implicit_control_marker(self): + # BE-3370 review: an INT ``seed``/``noise_seed`` *sub-input* of a + # dynamic combo relies on the frontend's implicit companion, but its + # dotted name (``model.seed``) never matched the leaf-name check, so the + # trailing control marker was kept as a real value and shifted every + # later sub-input by one slot. + object_info = { + "SeedInCombo": { + "input": { + "required": { + "model": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "fast", + "inputs": { + "required": { + # implicit companion: no control_after_generate flag + "seed": ["INT", {"default": 0}], + "steps": ["INT", {"default": 20}], + } + }, + } + ] + }, + ] + } + }, + "input_order": {"required": ["model"]}, + "output_node": True, + "display_name": "SIC", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "SeedInCombo", + "inputs": [], + "outputs": [], + # selector, seed, control_marker, steps + "widgets_values": ["fast", 7, "randomize", 30], + "mode": 0, + } + ], + "links": [], + } + result = convert_ui_to_api(workflow, object_info) + inputs = result["1"]["inputs"] + assert inputs["model"] == "fast" + assert inputs["model.seed"] == 7 + # Without the fix "randomize" landed here and steps was dropped. + assert inputs["model.steps"] == 30 + + def test_unresolved_selector_warns(self, caplog): + import logging + + # BE-3370 review: a selector value that matches no option key leaves the + # option's sub-input slots unconsumed, silently shifting later widgets. + # We can't recover the alignment, but the mismatch must not be silent. + object_info = { + "StaleCombo": { + "input": { + "required": { + "model": [ + "COMFY_DYNAMICCOMBO_V3", + {"options": [{"key": "known", "inputs": {"required": {"x": ["FLOAT"]}}}]}, + ] + } + }, + "input_order": {"required": ["model"]}, + "output_node": True, + "display_name": "SC", + } + } + workflow = { + "nodes": [ + { + "id": 1, + "type": "StaleCombo", + "inputs": [], + "outputs": [], + "widgets_values": ["renamed_server_side", 1.5], + "mode": 0, + } + ], + "links": [], + } + with caplog.at_level(logging.WARNING): + result = convert_ui_to_api(workflow, object_info) + assert result["1"]["inputs"]["model"] == "renamed_server_side" + assert any("matched no option" in rec.message for rec in caplog.records) + + def test_deeply_nested_dynamic_combos_do_not_recurse_forever(self, caplog): + # BE-3370 review: an unbounded chain of nested COMFY_*COMBO* sub-inputs + # must degrade to a warning, not an uncaught RecursionError that aborts + # the whole conversion. Build a self-referential option chain deeper + # than _MAX_DYNAMIC_COMBO_DEPTH. + import logging + + from comfy_cli.workflow_to_api import _MAX_DYNAMIC_COMBO_DEPTH + + depth = _MAX_DYNAMIC_COMBO_DEPTH + 5 + # Innermost combo has a plain leaf; each outer level nests the next. + spec = ["COMFY_DYNAMICCOMBO_V3", {"options": [{"key": "go", "inputs": {"required": {"leaf": ["INT"]}}}]}] + for _ in range(depth): + spec = [ + "COMFY_DYNAMICCOMBO_V3", + {"options": [{"key": "go", "inputs": {"required": {"next": spec}}}]}, + ] + object_info = { + "DeepCombo": { + "input": {"required": {"root": spec}}, + "input_order": {"required": ["root"]}, + "output_node": True, + "display_name": "DC", + } + } + # Every level selects "go"; only the outermost needs a value to start. + workflow = { + "nodes": [ + { + "id": 1, + "type": "DeepCombo", + "inputs": [], + "outputs": [], + "widgets_values": ["go"] * (depth + 2), + "mode": 0, + } + ], + "links": [], + } + with caplog.at_level(logging.WARNING): + result = convert_ui_to_api(workflow, object_info) # must not raise + assert "1" in result + assert any("exceeded depth" in rec.message for rec in caplog.records) + class TestDynamicPrompts: """Port of frontend's processDynamicPrompt behavior (formatUtil.ts).