mirror of
https://github.com/Comfy-Org/ComfyUI.git
synced 2026-09-21 13:38:08 -05:00
Fix DynamicGroup review edge cases
This commit is contained in:
+38
-15
@@ -1316,6 +1316,12 @@ class DynamicSlot(ComfyTypeI):
|
||||
out_dict[input_type][finalized_id] = value
|
||||
out_dict["dynamic_paths"][finalized_id] = finalize_prefix(curr_prefix, curr_prefix[-1])
|
||||
|
||||
class DynamicInputError(ValueError):
|
||||
def __init__(self, input_name: str, message: str):
|
||||
super().__init__(message)
|
||||
self.input_name = input_name
|
||||
|
||||
|
||||
@comfytype(io_type="COMFY_DYNAMICGROUP_V3")
|
||||
class DynamicGroup(ComfyTypeI):
|
||||
"""Repeat a widget template and pass its values to execute as a list of row dicts.
|
||||
@@ -1328,10 +1334,12 @@ class DynamicGroup(ComfyTypeI):
|
||||
Each submitted row follows the template's required/optional field declarations.
|
||||
|
||||
Missing positions are dicts whose fields are None. Missing optional fields are
|
||||
also None; widget defaults are not injected.
|
||||
also None; widget defaults are not injected. With INPUT_IS_LIST, missing fields
|
||||
are [None], matching the lists supplied for submitted fields.
|
||||
|
||||
Empty groups are [] in execute and check_lazy_status. Nonempty lazy groups
|
||||
contain (value, original_key) tuples at each field.
|
||||
PriceBadge widget dependencies use the same indexed field names as prompt keys.
|
||||
"""
|
||||
|
||||
Type = list[dict[str, Any]]
|
||||
@@ -1404,14 +1412,14 @@ class DynamicGroup(ComfyTypeI):
|
||||
index, separator, field_id = key[len(prefix):].partition(".")
|
||||
if (not separator or not index.isascii() or not index.isdecimal()
|
||||
or (len(index) > 1 and index.startswith("0")) or field_id not in field_specs):
|
||||
raise ValueError(f"Invalid DynamicGroup input key '{key}'; expected '{finalized_prefix}.<index>.<template field>'.")
|
||||
raise DynamicInputError(key, f"Invalid DynamicGroup input key '{key}'; expected '{finalized_prefix}.<index>.<template field>'.")
|
||||
row = int(index)
|
||||
if row >= max_rows:
|
||||
raise ValueError(f"DynamicGroup input '{key}' exceeds the index limit of {max_rows - 1} (max={max_rows}).")
|
||||
raise DynamicInputError(key, f"DynamicGroup input '{key}' exceeds the index limit of {max_rows - 1} (max={max_rows}).")
|
||||
present_rows.add(row)
|
||||
|
||||
if not min_rows <= len(present_rows) <= max_rows:
|
||||
raise ValueError(f"DynamicGroup input '{finalized_prefix}' received {len(present_rows)} rows; expected between {min_rows} and {max_rows}.")
|
||||
raise DynamicInputError(finalized_prefix, f"DynamicGroup input '{finalized_prefix}' received {len(present_rows)} rows; expected between {min_rows} and {max_rows}.")
|
||||
|
||||
for row in range(max(present_rows, default=-1) + 1):
|
||||
for field_id, (field_value, category) in field_specs.items():
|
||||
@@ -1766,16 +1774,19 @@ class PriceBadgeDepends:
|
||||
raise ValueError("PriceBadgeDepends.input_groups must be a list[str].")
|
||||
|
||||
def as_dict(self, schema_inputs: list["Input"]) -> dict[str, Any]:
|
||||
# Build lookup: widget_id -> io_type
|
||||
input_types: dict[str, str] = {}
|
||||
for inp in schema_inputs:
|
||||
all_inputs = inp.get_all()
|
||||
input_types[inp.id] = inp.get_io_type() # First input is always the parent itself
|
||||
for nested_inp in all_inputs[1:]:
|
||||
# For DynamicCombo/DynamicSlot, nested inputs are prefixed with parent ID
|
||||
# to match frontend naming convention (e.g., "should_texture.enable_pbr")
|
||||
prefixed_id = f"{inp.id}.{nested_inp.id}"
|
||||
input_types[prefixed_id] = nested_inp.get_io_type()
|
||||
|
||||
def collect_inputs(inputs: list[Input], prefix: str = "") -> None:
|
||||
for inp in inputs:
|
||||
name = prefix + inp.id
|
||||
input_types[name] = inp.get_io_type()
|
||||
if isinstance(inp, DynamicGroup.Input):
|
||||
for row in range(inp.max):
|
||||
collect_inputs(inp.template, f"{name}.{row}.")
|
||||
else:
|
||||
collect_inputs(inp.get_all()[1:], name + ".")
|
||||
|
||||
collect_inputs(schema_inputs)
|
||||
|
||||
# Enrich widgets with type information, raising error for unknown widgets
|
||||
widgets_data: list[dict[str, str]] = []
|
||||
@@ -2039,11 +2050,20 @@ def parse_class_inputs(out_dict: dict[str, Any], live_inputs: dict[str, Any], cu
|
||||
if curr_prefix:
|
||||
out_dict["dynamic_paths"][finalized_id] = finalized_id
|
||||
|
||||
def _dynamic_group_prefixes(inputs: list[Input]) -> Iterable[str]:
|
||||
for inp in inputs:
|
||||
if isinstance(inp, DynamicGroup.Input):
|
||||
yield inp.id + "."
|
||||
elif isinstance(inp, DynamicInput):
|
||||
for prefix in _dynamic_group_prefixes(inp.get_all()[1:]):
|
||||
yield f"{inp.id}.{prefix}"
|
||||
|
||||
|
||||
def create_input_dict_v1(inputs: list[Input]) -> dict:
|
||||
input = {
|
||||
"required": {}
|
||||
}
|
||||
group_prefixes = tuple(f"{i.id}." for i in inputs if isinstance(i, DynamicGroup.Input))
|
||||
group_prefixes = tuple(_dynamic_group_prefixes(inputs))
|
||||
for i in inputs:
|
||||
if group_prefixes and i.id.startswith(group_prefixes):
|
||||
raise ValueError(f"Input '{i.id}' conflicts with a DynamicGroup field prefix.")
|
||||
@@ -2061,7 +2081,7 @@ class DynamicPathsDefaultValue:
|
||||
EMPTY_DICT = "empty_dict"
|
||||
EMPTY_LIST = "empty_list"
|
||||
|
||||
def build_nested_inputs(values: dict[str, Any], v3_data: V3Data):
|
||||
def build_nested_inputs(values: dict[str, Any], v3_data: V3Data, *, input_is_list: bool = False):
|
||||
paths = v3_data.get("dynamic_paths", None)
|
||||
default_value_dict = v3_data.get("dynamic_paths_default_value", {})
|
||||
if paths is None:
|
||||
@@ -2080,6 +2100,7 @@ def build_nested_inputs(values: dict[str, Any], v3_data: V3Data):
|
||||
is_last = (i == len(parts) - 1)
|
||||
|
||||
if is_last:
|
||||
missing = key not in values
|
||||
value = values.pop(key, None)
|
||||
default_option = default_value_dict.get(key, None)
|
||||
if default_option == DynamicPathsDefaultValue.EMPTY_LIST:
|
||||
@@ -2087,6 +2108,8 @@ def build_nested_inputs(values: dict[str, Any], v3_data: V3Data):
|
||||
value = []
|
||||
elif value is None and default_option == DynamicPathsDefaultValue.EMPTY_DICT:
|
||||
value = {}
|
||||
elif missing and input_is_list:
|
||||
value = [None]
|
||||
if create_tuple and default_option != DynamicPathsDefaultValue.EMPTY_LIST:
|
||||
value = (value, key)
|
||||
current[p] = value
|
||||
|
||||
+12
-2
@@ -293,7 +293,7 @@ async def _async_map_node_over_list(prompt_id, unique_id, obj, input_data_all, f
|
||||
f = make_locked_method_func(type_obj, func, class_clone)
|
||||
# in case of dynamic inputs, restructure inputs to expected nested dict
|
||||
if v3_data is not None:
|
||||
inputs = _io.build_nested_inputs(inputs, v3_data)
|
||||
inputs = _io.build_nested_inputs(inputs, v3_data, input_is_list=input_is_list)
|
||||
# V1
|
||||
else:
|
||||
f = getattr(obj, func)
|
||||
@@ -880,7 +880,17 @@ async def validate_inputs(prompt_id, prompt, item, validated, visiting=None):
|
||||
if issubclass(obj_class, _ComfyNodeInternal):
|
||||
obj_class: _io._ComfyNodeBaseInternal
|
||||
class_inputs = obj_class.INPUT_TYPES()
|
||||
class_inputs, _, v3_data = _io.get_finalized_class_inputs(class_inputs, inputs)
|
||||
try:
|
||||
class_inputs, _, v3_data = _io.get_finalized_class_inputs(class_inputs, inputs)
|
||||
except _io.DynamicInputError as ex:
|
||||
errors.append({
|
||||
"type": "invalid_dynamic_input",
|
||||
"message": "Invalid dynamic input",
|
||||
"details": str(ex),
|
||||
"extra_info": {"input_name": ex.input_name},
|
||||
})
|
||||
validated[unique_id] = (False, errors, unique_id)
|
||||
return validated[unique_id]
|
||||
validate_function_name = "validate_inputs"
|
||||
validate_function = first_real_override(obj_class, validate_function_name)
|
||||
else:
|
||||
|
||||
@@ -83,6 +83,51 @@ def test_other_dotted_input_ids_are_unchanged():
|
||||
assert schema["required"]["rows_summary.value"] == ("FLOAT", {})
|
||||
|
||||
|
||||
@pytest.mark.parametrize("sibling_id", ["mode.rows.summary", "mode.rows.0.x"])
|
||||
@pytest.mark.parametrize("sibling_first", [False, True])
|
||||
def test_rejects_outer_input_in_nested_group_namespace(sibling_id, sibling_first):
|
||||
inputs = [
|
||||
io.DynamicCombo.Input("mode", options=[io.DynamicCombo.Option("on", [
|
||||
io.DynamicGroup.Input("rows", template=[io.Float.Input("x")]),
|
||||
])]),
|
||||
io.Float.Input(sibling_id),
|
||||
]
|
||||
if sibling_first:
|
||||
inputs.reverse()
|
||||
with pytest.raises(ValueError, match="conflicts with a DynamicGroup field prefix"):
|
||||
create_input_dict_v1(inputs)
|
||||
|
||||
|
||||
def test_group_namespace_is_scoped_to_its_combo_option():
|
||||
combo = io.DynamicCombo.Input("mode", options=[
|
||||
io.DynamicCombo.Option("on", [io.DynamicGroup.Input("rows", template=[io.Float.Input("x")])]),
|
||||
io.DynamicCombo.Option("off", [io.Float.Input("rows.summary")]),
|
||||
])
|
||||
assert _reconstruct(combo, {"mode": "off", "mode.rows.summary": 0.5}) == {
|
||||
"mode": {"mode": "off", "rows": {"summary": 0.5}},
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("nested", [False, True])
|
||||
def test_price_badge_resolves_indexed_group_fields(nested):
|
||||
group = io.DynamicGroup.Input("rows", template=[io.Float.Input("weight"), io.String.Input("name")], max=2)
|
||||
inputs = [group, io.Float.Input("fixed")]
|
||||
prefix = "rows"
|
||||
if nested:
|
||||
inputs = [io.DynamicCombo.Input("mode", options=[io.DynamicCombo.Option("on", inputs)])]
|
||||
prefix = "mode.rows"
|
||||
outer = "mode." if nested else ""
|
||||
badge = io.PriceBadgeDepends(widgets=[f"{prefix}.0.weight", f"{prefix}.1.name", outer + "fixed"])
|
||||
assert badge.as_dict(inputs)["widgets"] == [
|
||||
{"name": f"{prefix}.0.weight", "type": "FLOAT"},
|
||||
{"name": f"{prefix}.1.name", "type": "STRING"},
|
||||
{"name": outer + "fixed", "type": "FLOAT"},
|
||||
]
|
||||
for invalid in (f"{prefix}.weight", f"{prefix}.2.weight"):
|
||||
with pytest.raises(ValueError, match="unknown widget"):
|
||||
io.PriceBadgeDepends(widgets=[invalid]).as_dict(inputs)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("minimum", [0, 1, 2])
|
||||
@pytest.mark.parametrize("optional_group", [False, True])
|
||||
def test_every_submitted_row_keeps_template_requirements(minimum, optional_group):
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import pytest
|
||||
import torch
|
||||
|
||||
from comfy.cli_args import args
|
||||
|
||||
if not torch.cuda.is_available():
|
||||
args.cpu = True
|
||||
|
||||
import execution
|
||||
import nodes
|
||||
from comfy_api.latest import io
|
||||
|
||||
|
||||
pytestmark = pytest.mark.asyncio
|
||||
|
||||
|
||||
@pytest.mark.parametrize("is_input_list", [False, True])
|
||||
@pytest.mark.parametrize("lazy", [False, True])
|
||||
@pytest.mark.parametrize("empty", [False, True])
|
||||
async def test_group_missing_fields_follow_execution_list_mode(is_input_list, lazy, empty):
|
||||
received = []
|
||||
|
||||
class Group(io.ComfyNode):
|
||||
@classmethod
|
||||
def define_schema(cls):
|
||||
return io.Schema(node_id=cls.__name__, is_input_list=is_input_list, inputs=[
|
||||
io.DynamicGroup.Input("rows", template=[
|
||||
io.Float.Input("x"), io.Float.Input("optional", optional=True, default=1.0),
|
||||
], max=3),
|
||||
], outputs=[])
|
||||
|
||||
@classmethod
|
||||
def execute(cls, rows):
|
||||
received.append(rows)
|
||||
return io.NodeOutput()
|
||||
|
||||
@classmethod
|
||||
def check_lazy_status(cls, rows):
|
||||
received.append(rows)
|
||||
return []
|
||||
|
||||
values = {} if empty else {"rows.0.x": 0.5, "rows.2.x": 0.8}
|
||||
inputs, _, metadata = execution.get_input_data(values, Group, "group")
|
||||
metadata["create_dynamic_tuple"] = lazy
|
||||
await execution._async_map_node_over_list(
|
||||
"test", "group", Group, inputs, "check_lazy_status" if lazy else "execute", v3_data=metadata,
|
||||
)
|
||||
expected = []
|
||||
if not empty:
|
||||
for index, value in enumerate([0.5, None, 0.8]):
|
||||
row = {"x": [value] if is_input_list else value, "optional": [None] if is_input_list else None}
|
||||
if lazy:
|
||||
row = {name: (value, f"rows.{index}.{name}") for name, value in row.items()}
|
||||
expected.append(row)
|
||||
assert received == [expected]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("values,input_name", [
|
||||
({"rows.3.x": 0.5}, "rows.3.x"),
|
||||
({"rows.bad.x": 0.5}, "rows.bad.x"),
|
||||
({}, "rows"),
|
||||
])
|
||||
@pytest.mark.parametrize("downstream", [False, True])
|
||||
async def test_group_validation_errors_identify_original_input(monkeypatch, values, input_name, downstream):
|
||||
class Group(io.ComfyNode):
|
||||
@classmethod
|
||||
def define_schema(cls):
|
||||
return io.Schema(node_id=cls.__name__, inputs=[
|
||||
io.DynamicGroup.Input("rows", template=[io.Float.Input("x")], min=1, max=3),
|
||||
], outputs=[io.Float.Output()], is_output_node=not downstream)
|
||||
|
||||
@classmethod
|
||||
def execute(cls, rows):
|
||||
raise AssertionError("Invalid group must not execute")
|
||||
|
||||
class Sink(io.ComfyNode):
|
||||
@classmethod
|
||||
def define_schema(cls):
|
||||
return io.Schema(node_id=cls.__name__, inputs=[io.Float.Input("source")], outputs=[], is_output_node=True)
|
||||
|
||||
@classmethod
|
||||
def execute(cls, source):
|
||||
return io.NodeOutput()
|
||||
|
||||
monkeypatch.setitem(nodes.NODE_CLASS_MAPPINGS, "Group", Group)
|
||||
monkeypatch.setitem(nodes.NODE_CLASS_MAPPINGS, "Sink", Sink)
|
||||
prompt = {"group": {"class_type": "Group", "inputs": values}}
|
||||
if downstream:
|
||||
prompt["sink"] = {"class_type": "Sink", "inputs": {"source": ["group", 0]}}
|
||||
valid, _, _, errors = await execution.validate_prompt("test", prompt, None)
|
||||
assert not valid
|
||||
error, = errors["group"]["errors"]
|
||||
assert error["type"] == "invalid_dynamic_input"
|
||||
assert error["extra_info"] == {"input_name": input_name}
|
||||
assert input_name in error["details"]
|
||||
Reference in New Issue
Block a user