diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index d2c941e6..07410dff 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -18,7 +18,7 @@ import urllib.request import uuid as _uuid from collections import defaultdict -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from typing import Any from comfy_cli.cql._net import is_loopback_host @@ -50,6 +50,20 @@ class Port: is_link: bool = False enum_values: list[Any] = field(default_factory=list) # preserves the option's real type (int combos stay int) options: PortOptions = field(default_factory=PortOptions) + # V3 dynamic combos (COMFY_DYNAMICCOMBO_*): the flat value selects one + # option by key, and that option brings its own sub-inputs, wired as + # dotted keys (`model.max_tokens`). Kept separate from enum_values so + # `_is_scalar_choice` semantics (dict options are NOT membership choices) + # stay untouched. + selection_keys: list[str] = field(default_factory=list) + dynamic_options: dict[str, list[Port]] | None = None # selection key → parsed sub-ports + + @property + def is_dynamic_combo(self) -> bool: + """V3 dynamic-combo input (e.g. ClaudeNode.model): the flat value is a + selection key; the selected option's sub-inputs arrive as dotted keys + (``model.max_tokens``, …).""" + return self.type.startswith("COMFY_DYNAMICCOMBO") @property def is_autogrow(self) -> bool: @@ -297,11 +311,59 @@ def _ordered_names(raw: dict, order: list[str] | None) -> list[str]: return out -def _parse_inputs(raw: dict, order: list[str] | None, required: bool) -> list[Port]: +def _parse_dynamic_options(spec: Any, depth: int = 0) -> tuple[list[str], dict[str, list[Port]]]: + """Parse a COMFY_DYNAMICCOMBO_* spec's option sub-schemas. + + The server serializes each option as ``{"key": , "inputs": + {"required": {...}, "optional": {...}}}`` (ComfyUI ``_io.py`` + ``DynamicCombo.Option.as_dict``), with the inputs block in standard v1 + form. Returns ``(selection_keys, key → parsed sub-ports)``. Sub-inputs go + back through ``_parse_inputs``, so a nested dynamic combo recurses + naturally and its sub-ports carry their own ``dynamic_options``. + Malformed options (non-dict, missing/non-string key, non-dict inputs) + are skipped; duplicate keys keep the first occurrence. + + ``depth`` bounds the mutual recursion with ``_parse_inputs``: a hostile or + buggy ``object_info`` payload with pathologically nested dynamic combos + must degrade to the lenient no-options behavior, not crash the CLI with a + ``RecursionError``. Same bound as subgraph expansion + (``_MAX_SUBGRAPH_DEPTH``). + """ + if depth >= _MAX_SUBGRAPH_DEPTH: + return [], {} + opts_raw = spec[1] if isinstance(spec, list) and len(spec) > 1 and isinstance(spec[1], dict) else {} + options = opts_raw.get("options") + selection_keys: list[str] = [] + dynamic_options: dict[str, list[Port]] = {} + if not isinstance(options, list): + return selection_keys, dynamic_options + for opt in options: + if not isinstance(opt, dict): + continue + key = opt.get("key") + inputs_block = opt.get("inputs") + if not isinstance(key, str) or not isinstance(inputs_block, dict) or key in dynamic_options: + continue + req = inputs_block.get("required") + opt_block = inputs_block.get("optional") + sub_ports = _parse_inputs(req if isinstance(req, dict) else {}, None, required=True, depth=depth + 1) + sub_ports += _parse_inputs( + opt_block if isinstance(opt_block, dict) else {}, None, required=False, depth=depth + 1 + ) + selection_keys.append(key) + dynamic_options[key] = sub_ports + return selection_keys, dynamic_options + + +def _parse_inputs(raw: dict, order: list[str] | None, required: bool, depth: int = 0) -> list[Port]: ports: list[Port] = [] for name in _ordered_names(raw, order): spec = raw[name] type_id, is_enum, enum_values, opts = _parse_input_spec(spec) + selection_keys: list[str] = [] + dynamic_options: dict[str, list[Port]] | None = None + if type_id.startswith("COMFY_DYNAMICCOMBO"): + selection_keys, dynamic_options = _parse_dynamic_options(spec, depth) ports.append( Port( name=name, @@ -310,6 +372,8 @@ def _parse_inputs(raw: dict, order: list[str] | None, required: bool) -> list[Po is_link=_is_link(type_id, is_enum, opts.force_input), enum_values=enum_values, options=opts, + selection_keys=selection_keys, + dynamic_options=dynamic_options, ) ) return ports @@ -774,7 +838,24 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: # case surfaces here instead of as a cryptic server reject. autogrow_ports = {p.name: p for p in m.inputs if p.is_autogrow} autogrow_seen: set[str] = set() + # Dynamic combos are checked up front so the generic loop below can + # exempt STALE dynamic sub-keys (left over from a previous + # selection): the server ignores unknown sub-keys, so a hard edge + # check on one would false-error; the unknown_input warning from + # _check_dynamic_combo already covers it. Sub-keys under an + # unresolved selection keep the old generic checks. + dyn_port_names = {p.name for p in m.inputs if p.is_dynamic_combo} + dyn_errors, dyn_warnings, dyn_valid_keys, dyn_unresolved = _check_dynamic_combo( + node_id, class_type, m, node_data + ) for input_name, value in (node_data.get("inputs") or {}).items(): + if ( + "." in input_name + and input_name.split(".", 1)[0] in dyn_port_names + and input_name not in dyn_valid_keys + and not any(input_name.startswith(prefix) for prefix in dyn_unresolved) + ): + continue if autogrow_ports and "." in input_name: base = input_name.split(".", 1)[0] if base in autogrow_ports: @@ -891,6 +972,8 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: errors.extend(_check_autogrow_required(node_id, autogrow_ports, autogrow_seen, node_data)) errors.extend(_check_required_present(node_id, m, node_data)) + errors.extend(dyn_errors) + warnings.extend(dyn_warnings) # No-outputs check: the server rejects any prompt with zero output # nodes (execution.py:1155-1162, prompt_no_outputs) — including an @@ -1044,6 +1127,10 @@ def morphism_to_dict(self, m: Morphism) -> dict[str, Any]: # Autogrow inputs wire as one slot key per connection; # surface that here so `nodes show` is self-documenting. **({"autogrow": True, "wire_as": p.autogrow_slot_example()} if p.is_autogrow else {}), + # Dynamic combos: expose the valid selection keys so agents + # can discover them. `choices` stays [] — selection keys are + # not flat-value enum choices (see _is_scalar_choice). + **({"selection_keys": p.selection_keys} if p.is_dynamic_combo else {}), } for p in m.inputs ], @@ -1125,8 +1212,8 @@ def _check_required_present(node_id: str, m: Morphism, node_data: dict) -> list[ loop only inspects keys that ARE present, so this catches the absent ones. Skipped, because each is handled by its own path (and would otherwise double-error): autogrow ports (``_check_autogrow_required``) and the dynamic - types COMFY_DYNAMICCOMBO / COMFY_DYNAMICSLOT (DynamicSlot is always optional - server-side anyway). The server has NO exemption for required inputs that + types COMFY_DYNAMICCOMBO (``_check_dynamic_combo``) / COMFY_DYNAMICSLOT + (DynamicSlot is always optional server-side anyway). The server has NO exemption for required inputs that carry a default — the frontend always serializes widget values, so absence is a genuine authoring error; we do not skip ports with defaults. """ @@ -1185,6 +1272,242 @@ def _check_autogrow_required( return errors +def _check_dynamic_combo( + node_id: str, class_type: str, m: Morphism, node_data: dict +) -> tuple[list[dict], list[dict], set[str], set[str]]: + """COMFY_DYNAMICCOMBO_* inputs: selection-key membership + per-selection + required sub-inputs. + + Mirrors the server (``_io.py`` ``_expand_schema_for_dynamic`` → + ``parse_class_inputs``): the selected option's inputs expand with dotted + prefixes (``model.max_tokens``), and the expanded required keys hit the + same ``required_input_missing`` presence check as top-level inputs + (execution.py:884-900). Also flags present dotted keys that match no + sub-port of the selected option — a warning, not an error, because the + server ignores extra keys. Dotted keys whose base is an autogrow port + keep the existing autogrow path; bases here are disjoint by type. + + Returns ``(errors, warnings, valid_keys, unresolved)`` — the caller uses + ``valid_keys``/``unresolved`` to exempt stale (server-ignored) dynamic + sub-keys from the generic edge checks, which would otherwise hard-error + on e.g. a dangling link left over from a previous selection. + """ + dynamic_ports = {p.name: p for p in m.inputs if p.is_dynamic_combo} + if not dynamic_ports: + return [], [], set(), set() + inputs = node_data.get("inputs") or {} + errors: list[dict] = [] + warnings: list[dict] = [] + valid_keys: set[str] = set() + unresolved: set[str] = set() + resolved: dict[str, Any] = {} + for name, port in dynamic_ports.items(): + v, u = _expand_dynamic_port(node_id, class_type, port, name, inputs, errors, warnings, resolved) + valid_keys |= v + unresolved |= u + + # Unknown dotted keys under a RESOLVED selection → warning. Under an + # unresolved prefix (selection absent/invalid/link-valued) the sub-keys + # can't be judged — the primary error already covers it, so don't pile on. + for key in inputs: + if "." not in key: + continue + base = key.split(".", 1)[0] + if base not in dynamic_ports: + continue + if key in valid_keys or any(key.startswith(prefix) for prefix in unresolved): + continue + # Attribute the stray key to the DEEPEST resolved combo prefix, so a + # stray `model.mode.bogus` under a resolved `model.mode` names + # `model.mode`'s selection (and lists ITS sub-keys), not `model`'s. + anchor = max((n for n in resolved if key.startswith(f"{n}.")), key=len, default=base) + selection = resolved.get(anchor, inputs.get(anchor)) + known = sorted(k for k in valid_keys if k.startswith(f"{anchor}.")) + warnings.append( + { + "node_id": node_id, + "field": key, + "code": "unknown_input", + "message": ( + f"input {key!r} matches no sub-input of {anchor}={selection!r} — the server will ignore it" + ), + "hint": f"valid sub-keys for this selection: {', '.join(known)}" + if known + else f"selection {selection!r} takes no sub-inputs", + } + ) + return errors, warnings, valid_keys, unresolved + + +def _expand_dynamic_port( + node_id: str, + class_type: str, + port: Port, + full_name: str, + inputs: dict, + errors: list[dict], + warnings: list[dict], + resolved: dict[str, Any], +) -> tuple[set[str], set[str]]: + """Validate ONE dynamic-combo port and return ``(valid_keys, unresolved)``. + + ``valid_keys`` — every dotted input key the selected option (and any + resolved nested selections) accepts; the caller warns on present dotted + keys outside this set. ``unresolved`` — ``"."`` prefixes whose + sub-keys can't be judged, so the caller skips unknown-key warnings there. + ``resolved`` (mutated) — dotted port name → its valid selection, so the + caller can attribute stray keys to the deepest resolved level. + """ + unresolved = {f"{full_name}."} + if full_name not in inputs: + if port.required: + # Truncate like the unknown_enum_value branch: a combo with + # hundreds of options (e.g. model checkpoints) must not produce + # an unreadable hint. + top = port.selection_keys[:8] + extra = len(port.selection_keys) - len(top) + errors.append( + { + "node_id": node_id, + "field": full_name, + "code": "required_input_missing", + "message": ( + f"required input {full_name!r} is missing — " + f"the server will reject this node (required_input_missing)" + ), + "hint": f"add {full_name!r} to inputs" + + (f" (one of: {', '.join(top)}" + (f", and {extra} more …" if extra else "") + ")" if top else ""), + } + ) + return set(), unresolved + value = inputs[full_name] + if isinstance(value, list) and len(value) == 2: + # Link-valued selection: the selection is a widget value, but treat a + # link as a no-op like the pre-BE-3358 behavior (the generic loop + # already edge-checks it) — never crash on it. + return set(), unresolved + if not port.selection_keys: + # No parseable options (absent or all malformed) — degrade to the old + # lenient behavior rather than false-error on a schema we can't read. + return set(), unresolved + if not isinstance(value, str) or value not in port.selection_keys: + top = port.selection_keys[:8] + errors.append( + { + "node_id": node_id, + "field": full_name, + "code": "unknown_enum_value", + "message": f"{value!r} not in {len(port.selection_keys)} known options for {full_name}", + "hint": f"valid options include: {', '.join(str(v) for v in top)}" + + ( + f" (and {len(port.selection_keys) - 8} more — see valid_options)" + if len(port.selection_keys) > 8 + else "" + ), + "suggestions": port.selection_keys[:20], + "valid_options": list(port.selection_keys), + } + ) + return set(), unresolved + + valid: set[str] = set() + nested_unresolved: set[str] = set() + resolved[full_name] = value + for sub in (port.dynamic_options or {}).get(value, []): + sub_name = f"{full_name}.{sub.name}" + valid.add(sub_name) + if sub.is_dynamic_combo: + # Nested dynamic combo: deeper dotted paths (model.mode.detail). + nested_valid, nested_unres = _expand_dynamic_port( + node_id, class_type, sub, sub_name, inputs, errors, warnings, resolved + ) + valid |= nested_valid + nested_unresolved |= nested_unres + continue + if sub.is_autogrow: + # Autogrow sub-input (an option carrying e.g. `images`): wired as + # one slot key per connection (`model.images.image0`, …) — + # mirror the top-level autogrow path: slot keys are valid, a bare + # single connection is an error, and a required subtree with zero + # wired slots is an error. + slot_prefix = f"{sub_name}." + slots = {k for k in inputs if k.startswith(slot_prefix)} + valid |= slots + stem = sub.name[:-1] if sub.name.endswith("s") else sub.name + example = f"{slot_prefix}{stem}0, {slot_prefix}{stem}1, …" + bare = inputs.get(sub_name) + if isinstance(bare, list) and len(bare) == 2: + errors.append( + { + "node_id": node_id, + "field": sub_name, + "code": "autogrow_bare_input", + "message": ( + f"input {sub_name!r} is an autogrow input ({sub.type}) and cannot be " + f"wired as a single connection — the server expects one slot key per " + f"connection" + ), + "hint": f"wire one key per connection: {example}", + } + ) + elif sub.required and not slots and sub_name not in inputs: + errors.append( + { + "node_id": node_id, + "field": sub_name, + "code": "autogrow_no_slots", + "message": ( + f"required autogrow input {sub_name!r} has no connected slots — " + f"the server will reject this node" + ), + "hint": f"wire one key per connection: {example}", + } + ) + continue + if sub_name not in inputs: + # DynamicSlot is always optional server-side; autogrow presence is + # handled above — mirror _check_required_present's exclusions. + if sub.required and not sub.type.startswith("COMFY_DYNAMICSLOT"): + errors.append( + { + "node_id": node_id, + "field": sub_name, + "code": "required_input_missing", + "message": ( + f"required input {sub_name!r} is missing — " + f"the server will reject this node (required_input_missing)" + ), + "hint": f"add {sub_name!r} to inputs" + + ( + f" (e.g. a {sub.type} value)" + if not sub.is_link + else f" (wire a {sub.type} link: [, ])" + ), + } + ) + continue + sub_value = inputs[sub_name] + if isinstance(sub_value, list) and len(sub_value) == 2: + continue # link-valued sub-input: the generic loop edge-checks it + named = replace(sub, name=sub_name) # dotted name so messages/fields read model.max_tokens + shape_err = named.validate_shape(sub_value) + if shape_err: + errors.append( + { + "node_id": node_id, + "field": sub_name, + "code": "shape_mismatch", + "message": shape_err, + "hint": f"expected {sub.type}; check the value type", + } + ) + continue + cat_errors, cat_warnings = _validate_catalog_value(node_id, class_type, sub_name, named, sub_value) + errors.extend(cat_errors) + warnings.extend(cat_warnings) + return valid, nested_unresolved + + # --------------------------------------------------------------------------- # Source loaders # --------------------------------------------------------------------------- diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index b35adf4d..764c1b79 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -1422,3 +1422,432 @@ def test_load_from_target_refuses_non_loopback_local_host(): with pytest.raises(LoadError, match="non-loopback"): _load_from_target(mode="local", host="example.com", port=8188) + + +# =========================================================================== +# TestDynamicComboInputs — BE-3358: selection-key enum + dotted sub-inputs +# =========================================================================== + + +@pytest.fixture +def graph_dynamic() -> Graph: + """Graph built from the synthetic BE-3349-shaped dynamic-combo fixture: + a COMFY_DYNAMICCOMBO_V3 `model` input with two options carrying different + required sub-inputs (INT with min/max, an enum), one of which nests a + second dynamic combo (`model.mode` → `model.mode.budget`).""" + import json + from pathlib import Path + + fixture = Path(__file__).parent.parent / "fixtures" / "dynamic_combo_object_info.json" + return Graph.from_object_info(json.loads(fixture.read_text())) + + +class TestDynamicComboInputs: + """COMFY_DYNAMICCOMBO_V3 inputs (ClaudeNode.model, …): the flat value must + be a known selection key, and the selected option's required sub-inputs + must be present as dotted keys — mirroring the server's + _expand_schema_for_dynamic + required_input_missing checks.""" + + def _node(self, inputs: dict) -> dict: + return {"1": {"class_type": "ClaudeNode", "inputs": inputs}} + + def test_valid_selection_with_all_sub_keys(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "fast", + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + assert result["warnings"] == [] + + def test_missing_required_sub_key_errors(self, graph_dynamic: Graph): + """BE-3349 repro 1: {"model": "Opus 4.6"} with no sub-keys.""" + wf = self._node({"prompt": "hi", "model": "Opus 4.6"}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + missing = [e for e in result["errors"] if e["code"] == "required_input_missing"] + assert {e["field"] for e in missing} == {"model.max_tokens", "model.mode"} + + def test_invalid_selection_key_errors(self, graph_dynamic: Graph): + """BE-3349 repro 2: unknown selection is a hard unknown_enum_value + carrying the full valid_options list.""" + wf = self._node({"prompt": "hi", "model": "NotARealModel", "model.bogus_key": 5}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "unknown_enum_value") + assert err["field"] == "model" + assert err["valid_options"] == ["Opus 4.6", "Haiku 4.5"] + # Sub-keys of an unknown selection can't be judged — no pile-on warning. + assert "unknown_input" not in [w["code"] for w in result["warnings"]] + + def test_garbage_dotted_key_warns(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "fast", + "model.bogus_key": 5, + } + ) + result = graph_dynamic.validate_workflow(wf) + # Extra keys are ignored by the server → warning, not error. + assert result["valid"] is True, result["errors"] + warn = next(w for w in result["warnings"] if w["code"] == "unknown_input") + assert warn["field"] == "model.bogus_key" + assert "model.max_tokens" in warn["hint"] + + def test_out_of_range_sub_value_errors(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 999999, + "model.mode": "fast", + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "above_max") + assert err["field"] == "model.max_tokens" + + def test_sub_value_shape_mismatch_errors(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": "lots", + "model.mode": "fast", + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "shape_mismatch") + assert err["field"] == "model.max_tokens" + assert "model.max_tokens" in err["message"] + + def test_enum_sub_input_membership_checked(self, graph_dynamic: Graph): + """A COMBO sub-input of the selected option gets the same hard enum + check as a top-level combo.""" + wf = self._node( + { + "prompt": "hi", + "model": "Haiku 4.5", + "model.max_tokens": 100, + "model.style": "florid", + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "unknown_enum_value") + assert err["field"] == "model.style" + assert err["valid_options"] == ["concise", "detailed"] + + def test_required_dynamic_port_absent_errors(self, graph_dynamic: Graph): + wf = self._node({"prompt": "hi"}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "required_input_missing") + assert err["field"] == "model" + assert "Opus 4.6" in err["hint"] + + def test_nested_selection_missing_required_sub_key(self, graph_dynamic: Graph): + """Nested dynamic combo: mode=thinking requires model.mode.budget.""" + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "thinking", + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + missing = [e for e in result["errors"] if e["code"] == "required_input_missing"] + assert {e["field"] for e in missing} == {"model.mode.budget"} + + def test_nested_selection_valid_with_budget(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "thinking", + "model.mode.budget": 2048, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + assert result["warnings"] == [] + + def test_nested_invalid_selection_errors_without_pile_on(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "warp", + "model.mode.budget": 2048, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "unknown_enum_value") + assert err["field"] == "model.mode" + assert err["valid_options"] == ["fast", "thinking"] + # model.mode.budget sits under the unresolved selection — no warning. + assert result["warnings"] == [] + + def test_optional_sub_input_absent_is_fine_but_range_checked_when_present(self, graph_dynamic: Graph): + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "fast", + "model.temperature": 3.5, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "above_max") + assert err["field"] == "model.temperature" + + def test_link_valued_selection_is_skipped(self, graph_dynamic: Graph): + """A 2-list selection value is a link — treated as a no-op (no crash, + no selection error), matching the pre-BE-3358 behavior.""" + wf = { + "0": { + "class_type": "ClaudeNode", + "inputs": {"prompt": "src", "model": "Haiku 4.5", "model.max_tokens": 1, "model.style": "concise"}, + }, + "1": {"class_type": "ClaudeNode", "inputs": {"prompt": "hi", "model": ["0", 0]}}, + } + result = graph_dynamic.validate_workflow(wf) + codes = [e["code"] for e in result["errors"]] + assert "unknown_enum_value" not in codes + assert "required_input_missing" not in codes + + def test_malformed_options_are_skipped(self): + """Non-dict options and options missing key/inputs parse to nothing — + validation degrades to the old lenient behavior instead of crashing.""" + info = { + "Foo": { + "input": { + "required": { + "shape": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + "not-a-dict", + {"key": 42, "inputs": {"required": {}}}, + {"key": "no-inputs"}, + {"key": "square", "inputs": "not-a-dict"}, + ] + }, + ] + } + }, + "input_order": {"required": ["shape"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Foo", + "python_module": "nodes", + } + } + g = Graph.from_object_info(info) + port = g.node("Foo").inputs[0] + assert port.selection_keys == [] + result = g.validate_workflow({"1": {"class_type": "Foo", "inputs": {"shape": "anything"}}}) + assert result["valid"] is True, result["errors"] + + def test_describe_exposes_selection_keys(self, graph_dynamic: Graph): + desc = graph_dynamic.morphism_to_dict(graph_dynamic.node("ClaudeNode")) + model = next(i for i in desc["inputs"] if i["name"] == "model") + assert model["selection_keys"] == ["Opus 4.6", "Haiku 4.5"] + # enum_values contract untouched: selection keys are not flat choices. + assert model["choices"] == [] + prompt = next(i for i in desc["inputs"] if i["name"] == "prompt") + assert "selection_keys" not in prompt + + # -- Cursor-review hardening (PR #573 panel findings) ------------------- + + def test_stale_link_valued_sub_key_no_hard_edge_error(self, graph_dynamic: Graph): + """A stale dynamic sub-key left over from a previous selection is + IGNORED by the server even when link-valued — the generic edge checks + must not hard-error (dangling_edge) on it; it only warns.""" + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "fast", + # stale key from a previous selection, pointing at a node that + # no longer exists + "model.old_image": ["99", 0], + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + codes = [e["code"] for e in result["errors"]] + assert "dangling_edge" not in codes + warn = next(w for w in result["warnings"] if w["code"] == "unknown_input") + assert warn["field"] == "model.old_image" + + def test_deep_stray_key_attributed_to_nested_level(self, graph_dynamic: Graph): + """A stray key under a RESOLVED nested combo is attributed to that + level (model.mode='fast'), not the top-level model selection.""" + wf = self._node( + { + "prompt": "hi", + "model": "Opus 4.6", + "model.max_tokens": 800, + "model.mode": "fast", + "model.mode.bogus": 1, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + warn = next(w for w in result["warnings"] if w["code"] == "unknown_input") + assert warn["field"] == "model.mode.bogus" + assert "model.mode='fast'" in warn["message"] + assert "'fast' takes no sub-inputs" in warn["hint"] + + def test_required_missing_hint_truncates_many_selection_keys(self): + """A dynamic combo with hundreds of options must not dump them all + into the required_input_missing hint — first 8, then a count.""" + options = [{"key": f"ckpt-{i:03d}", "inputs": {"required": {}, "optional": {}}} for i in range(30)] + info = { + "Loader": { + "input": {"required": {"model": ["COMFY_DYNAMICCOMBO_V3", {"options": options}]}}, + "input_order": {"required": ["model"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Loader", + "python_module": "nodes", + } + } + g = Graph.from_object_info(info) + result = g.validate_workflow({"1": {"class_type": "Loader", "inputs": {}}}) + err = next(e for e in result["errors"] if e["code"] == "required_input_missing") + hint = err["hint"] + assert "ckpt-007" in hint + assert "ckpt-008" not in hint + assert "and 22 more" in hint + + def test_deeply_nested_dynamic_options_degrade_without_recursion_error(self): + """A hostile object_info with pathologically nested dynamic combos + (deeper than _MAX_SUBGRAPH_DEPTH) parses leniently instead of + crashing from_object_info with a RecursionError.""" + spec = ["COMFY_DYNAMICCOMBO_V3", {"options": [{"key": "leaf", "inputs": {"required": {}}}]}] + for _ in range(200): + spec = [ + "COMFY_DYNAMICCOMBO_V3", + {"options": [{"key": "deeper", "inputs": {"required": {"next": spec}}}]}, + ] + info = { + "Nest": { + "input": {"required": {"root": spec}}, + "input_order": {"required": ["root"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Nest", + "python_module": "nodes", + } + } + g = Graph.from_object_info(info) # must not raise + port = g.node("Nest").inputs[0] + assert port.selection_keys == ["deeper"] + result = g.validate_workflow({"1": {"class_type": "Nest", "inputs": {"root": "deeper"}}}) + assert isinstance(result["valid"], bool) + + +class TestDynamicComboAutogrowSub: + """Autogrow sub-inputs carried by a dynamic-combo option (e.g. an option + whose schema declares `images` as COMFY_AUTOGROW_V3): slot keys wire as + `model.images.image0`, … — mirroring the top-level autogrow path.""" + + INFO = { + "Src": { + "input": {"required": {}}, + "input_order": {"required": []}, + "output": ["IMAGE"], + "output_name": ["image"], + "display_name": "Src", + "python_module": "nodes", + }, + "Batch": { + "input": { + "required": { + "model": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "multi", + "inputs": {"required": {"images": ["COMFY_AUTOGROW_V3", {}]}, "optional": {}}, + }, + {"key": "none", "inputs": {"required": {}, "optional": {}}}, + ] + }, + ] + } + }, + "input_order": {"required": ["model"]}, + "output": [], + "output_name": [], + "output_node": True, + "display_name": "Batch", + "python_module": "nodes", + }, + } + + def _graph(self) -> Graph: + return Graph.from_object_info(self.INFO) + + def test_wired_slot_keys_are_valid(self): + wf = { + "0": {"class_type": "Src", "inputs": {}}, + "1": { + "class_type": "Batch", + "inputs": {"model": "multi", "model.images.image0": ["0", 0], "model.images.image1": ["0", 0]}, + }, + } + result = self._graph().validate_workflow(wf) + assert result["valid"] is True, result["errors"] + # Slot keys must NOT surface as unknown_input noise. + assert [w for w in result["warnings"] if w["code"] == "unknown_input"] == [] + + def test_required_autogrow_sub_with_no_slots_errors(self): + wf = {"1": {"class_type": "Batch", "inputs": {"model": "multi"}}} + result = self._graph().validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_no_slots") + assert err["field"] == "model.images" + assert "model.images.image0" in err["hint"] + + def test_bare_wired_autogrow_sub_errors(self): + wf = { + "0": {"class_type": "Src", "inputs": {}}, + "1": {"class_type": "Batch", "inputs": {"model": "multi", "model.images": ["0", 0]}}, + } + result = self._graph().validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_bare_input") + assert err["field"] == "model.images" + + def test_slot_edges_still_checked(self): + """Valid slot keys keep the generic edge checks — a slot pointing at a + missing node is still a dangling_edge error.""" + wf = {"1": {"class_type": "Batch", "inputs": {"model": "multi", "model.images.image0": ["99", 0]}}} + result = self._graph().validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "dangling_edge") + assert err["field"] == "model.images.image0" diff --git a/tests/comfy_cli/fixtures/dynamic_combo_object_info.json b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json new file mode 100644 index 00000000..92dcf1f2 --- /dev/null +++ b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json @@ -0,0 +1,66 @@ +{ + "ClaudeNode": { + "input": { + "required": { + "prompt": ["STRING", {"multiline": true, "default": ""}], + "model": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "Opus 4.6", + "inputs": { + "required": { + "max_tokens": ["INT", {"default": 1024, "min": 1, "max": 32000}], + "mode": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "fast", + "inputs": {"required": {}, "optional": {}} + }, + { + "key": "thinking", + "inputs": { + "required": { + "budget": ["INT", {"default": 1024, "min": 1, "max": 8000}] + }, + "optional": {} + } + } + ] + } + ] + }, + "optional": { + "temperature": ["FLOAT", {"default": 1.0, "min": 0.0, "max": 1.0}] + } + } + }, + { + "key": "Haiku 4.5", + "inputs": { + "required": { + "max_tokens": ["INT", {"default": 512, "min": 1, "max": 8192}], + "style": [["concise", "detailed"]] + }, + "optional": {} + } + } + ] + } + ] + } + }, + "input_order": {"required": ["prompt", "model"]}, + "output": ["STRING"], + "output_name": ["text"], + "category": "api/text", + "display_name": "Claude", + "description": "Synthetic BE-3349-shaped dynamic-combo node for validator tests.", + "output_node": true, + "api_node": true, + "python_module": "nodes" + } +}