From 27d58c07de905577bf7067de9d39d2ba7f3140fb Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 3 Sep 2026 01:21:27 -0700 Subject: [PATCH 1/4] fix(cql): enforce the server's autogrow minimum slot count in validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No validation path consulted an autogrow group's declared minimum, so two prompts the server rejects validated clean: * a dynamic-combo-nested autogrow sub-input with `min >= 1` and zero wired slots (live shape: `GrokVideoReferenceNode.model.reference_images`); * a top-level group filled below its minimum — the old check only caught zero slots, so one slot against `min: 2` passed (`MeshyMultiImageToModelNode.images`). Both now emit a new `autogrow_below_min` error, mirroring the server's own gate: `Autogrow._expand_schema_for_dynamic` places slot `i` in `required` only `if i < min and template_required`, where `template_required` is whether the template's single inner input sits in the template's own `required` section. So a Seedream-style group declaring `min: 0` inside `required` keeps validating clean with zero slots, and so does a `min: 1` group whose inner input sits in the template's `optional` section. The top-level gate moves from `port.required` to that effective minimum, which also drops the latent false positive on a required-section `min: 0` group. A template we cannot read carries no `template_required` signal at all, so that case keeps the historical `port.required` check rather than silently losing it. --- comfy_cli/cql/engine.py | 158 ++++++++-- tests/comfy_cli/cql/test_engine.py | 217 +++++++++++++ tests/comfy_cli/cql/test_nested_autogrow.py | 74 +++++ .../fixtures/dynamic_combo_object_info.json | 297 +++++++++++++++++- 4 files changed, 709 insertions(+), 37 deletions(-) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index ad29f8585..0aa29d9e6 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -242,6 +242,62 @@ def autogrow_limits(self) -> tuple[int, int | None]: hi = t.get("max") return lo, (int(hi) if isinstance(hi, (int, float)) and not isinstance(hi, bool) else None) + @property + def autogrow_template_required(self) -> bool | None: + """The server's ``template_required`` gate for this autogrow input: + whether the template's single inner input sits in the template's OWN + ``required`` section. + + ``comfy_api/latest/_io.py``, ``Autogrow._expand_schema_for_dynamic`` + walks the template's sections, takes the first non-empty one, and sets + ``template_required = _input_type == "required"`` — *"for now, get just + the first value from dict_input; if not required, min can be ignored"*. + This walks the same sections in the same order. + + ``None`` when there is nothing to read — a non-autogrow port, or a + template carrying no ``input`` block (an older/partial catalog capture + whose ``min``/``names`` sit beside ``template`` rather than inside it). + Callers must distinguish that "cannot judge" from a definite ``False``. + """ + if not self.is_autogrow: + return None + t = self.options.template + if not isinstance(t, dict): + return None + inputs = t.get("input") + if not isinstance(inputs, dict): + return None + for section in ("required", "optional"): + section_def = inputs.get(section) + if not isinstance(section_def, dict) or not section_def: + continue + return section == "required" + return None + + @property + def autogrow_effective_min(self) -> int: + """Slots the server actually places in its ``required`` section — the + count a prompt must wire before the server will accept the node. + + Slot ``i`` lands in ``required`` only ``if i < min and + template_required`` (see :attr:`autogrow_template_required`), so a + Seedream-style group declaring ``min: 0`` inside ``required`` keeps + validating clean with zero slots, and so does a ``min: 1`` group whose + inner input sits in the template's ``optional`` section. + + The section the autogrow input ITSELF sits in is deliberately not + consulted, because the server does not consult it either + (``_expand_schema_for_dynamic`` ignores its ``input_type`` argument): + an ``optional``-section group with ``min: 2`` still owes two slots. + + ``0`` whenever the gate is not positively ``True`` — including the + "cannot judge" case, the same "don't reject what you can't read" + leniency :attr:`autogrow_element_type` applies. A caller that wants to + keep checking an unreadable template must consult + :attr:`autogrow_template_required` for the ``None`` itself. + """ + return self.autogrow_limits[0] if self.autogrow_template_required else 0 + @property def autogrow_template(self) -> dict | None: """The V3 autogrow element-naming template from object_info, if the @@ -1566,11 +1622,10 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: port_by_name = {p.name: p for p in m.inputs} # V3 autogrow inputs are declared once (e.g. `images`) but wired as - # slot keys (`images.image0`, `images.image1`, …). Track which - # autogrow ports actually received a slot so the required-but-empty - # case surfaces here instead of as a cryptic server reject. + # slot keys (`images.image0`, `images.image1`, …), so their + # slot-count check (`_check_autogrow_required`) counts those keys + # rather than looking for the base name. autogrow_ports = {p.name: p for p in m.inputs if p.is_autogrow} - autogrow_seen: set[str] = set() node_inputs = node_data.get("inputs") # A truthy non-dict `inputs` (e.g. a string/list from malformed JSON) # sails through `or {}` and crashes `.items()`; treat it as empty so @@ -1595,10 +1650,6 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: 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: - autogrow_seen.add(base) if input_name in autogrow_ports and isinstance(value, list) and len(value) == 2: port = autogrow_ports[input_name] errors.append( @@ -1738,7 +1789,7 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: # of required-presence-type hard errors (missing/unknown # dynamic-combo selection), so it's gated alongside them. if node_id in reachable: - errors.extend(_check_autogrow_required(node_id, autogrow_ports, autogrow_seen, node_data)) + errors.extend(_check_autogrow_required(node_id, autogrow_ports, node_data)) errors.extend(_check_required_present(node_id, m, node_data)) errors.extend(dyn_errors) @@ -2005,8 +2056,9 @@ def _output_reachable_node_ids(workflow: dict[str, Any], graph: Graph) -> set[st output nodes (``OUTPUT_NODE``) and everything reachable by walking their input links backward — any node not reachable from an output is pruned and never validated. We reproduce that reachable set so the promoted hard checks - (required_input_missing, autogrow_no_slots, below_min/above_max) don't - reject a disconnected node the server would silently drop. + (required_input_missing, autogrow_no_slots, autogrow_below_min, + below_min/above_max) don't reject a disconnected node the server would + silently drop. An input value shaped ``[source_node_id, output_index]`` is a link edge (the same predicate the per-input link walk uses); we follow those edges backward @@ -2506,14 +2558,20 @@ def _check_dynamic_combo_sub( ) if port.is_autogrow: - # An autogrow sub-input wires as `.` keys and routinely - # declares `min: 0` even inside the `required` section (Seedream's - # `model.images`), so absence is NOT a server reject — the converter - # emits no key at all for a zero-slot autogrow. Nothing to presence- or + # An autogrow sub-input wires as `.` keys, so absence of + # the dotted base itself is never a server reject — the converter emits + # no key at all for a zero-slot autogrow. Nothing to presence- or # shape-check here. Any slot keys actually present are accepted - # wholesale (not counted, not edge-checked here) so they don't - # surface as unknown_input noise; the generic driver loop still - # edge-checks whichever slot keys ARE present. + # wholesale (not edge-checked here) so they don't surface as + # unknown_input noise; the generic driver loop still edge-checks + # whichever slot keys ARE present. + # + # The one count that IS enforced is the server's own autogrow minimum + # (`Port.autogrow_effective_min` — the `i < min and template_required` + # gate). A group declaring `min: 0` inside `required` (Seedream's + # `model.images`) reads as an effective min of 0 and stays lenient; + # one declaring `min >= 1` (GrokVideoReferenceNode's + # `model.reference_images`) is a hard reject the server would issue. # # A bare `dotted: [src, idx]` link, though, is the exact same mistake # the top-level autogrow_bare_input check catches — the server expects @@ -2541,7 +2599,16 @@ def _check_dynamic_combo_sub( set(), ) slot_prefix = f"{dotted}." - return [], [], {k for k in present if k.startswith(slot_prefix)}, set() + matched = {k for k in present if k.startswith(slot_prefix)} + lo = port.autogrow_effective_min + errors: list[dict] = [] + if len(matched) < lo: + errors.append(_autogrow_below_min_error(node_id, dotted, port, len(matched), lo)) + # The matched keys stay valid keys even when the count is short, so the + # slots that ARE wired don't regress into `unknown_input` noise on top + # of the count error. These errors flow into `dyn_errors`, which the + # driver loop already gates on output-reachability. + return errors, [], matched, set() if dotted not in present: if not sub_required: @@ -2595,18 +2662,53 @@ def _check_dynamic_combo_sub( return errs, warns, set(), set() -def _check_autogrow_required( - node_id: str, autogrow_ports: dict[str, Port], autogrow_seen: set[str], node_data: dict -) -> list[dict]: - """Required autogrow inputs that received no connected slots. +def _autogrow_below_min_error(node_id: str, field: str, port: Port, n: int, lo: int) -> dict: + """One ``autogrow_below_min`` error — an autogrow group wired with fewer + slots than the server places in its ``required`` section.""" + return { + "node_id": node_id, + "field": field, + "code": "autogrow_below_min", + "message": ( + f"autogrow input {field!r} has {n} connected slot(s) but declares a minimum of {lo} — " + f"the server will reject this node" + ), + "hint": f"wire at least {lo} keys, one per connection: {port.autogrow_slot_example()}", + } + + +def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node_data: dict) -> list[dict]: + """Autogrow inputs wired with fewer slots than the server requires. - The server would reject such a node, so surface it here instead of as a - cryptic downstream reject. + The gate is :attr:`Port.autogrow_effective_min`, not ``port.required``: the + server places autogrow slots ``i < min`` in ``required`` whenever the + template's inner input is itself required, regardless of which section the + autogrow input sits in — and conversely ignores ``min`` entirely when it is + not (``comfy_api/latest/_io.py``, + ``Autogrow._expand_schema_for_dynamic``). Either way the server would reject + the node, so surface it here instead of as a cryptic downstream reject. + + One carve-out keeps this strictly additive: a template we cannot read gives + no ``template_required`` signal at all, so rather than silently dropping the + zero-slot check for older/partial catalogs this falls back to the historical + ``port.required`` gate for exactly that case. """ - inputs = node_data.get("inputs") or {} + inputs = node_data.get("inputs") + if not isinstance(inputs, dict): + inputs = {} errors: list[dict] = [] for base, port in autogrow_ports.items(): - if port.required and base not in autogrow_seen and base not in inputs: + lo = port.autogrow_effective_min + if lo < 1: + if port.autogrow_template_required is not None or not port.required: + continue + lo = 1 + # The base name wired directly is a different mistake, already reported + # as `autogrow_bare_input` by the driver loop — don't double-error it. + if base in inputs: + continue + n = sum(1 for k in inputs if k.startswith(f"{base}.")) + if n == 0: errors.append( { "node_id": node_id, @@ -2618,6 +2720,8 @@ def _check_autogrow_required( "hint": f"wire one key per connection: {port.autogrow_slot_example()}", } ) + elif n < lo: + errors.append(_autogrow_below_min_error(node_id, base, port, n, lo)) return errors diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index ea0781317..0aca8be8a 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -146,6 +146,10 @@ def _object_info() -> dict[str, Any]: # V3 autogrow node mirroring the live cloud BatchImagesNode shape: # one declared input `images` (COMFY_AUTOGROW_V3), but the server # expects autogrown slot keys `images.image0`, `images.image1`, … + # Deliberately template-LESS: this is the catalog shape that carries no + # `template_required` signal, so it pins the historical `port.required` + # fallback in `_check_autogrow_required`. Groups whose template IS + # readable are covered by `TestAutogrowMinSlots`. "BatchImagesNode": { "input": { "required": { @@ -3717,6 +3721,219 @@ def test_bare_wired_sub_input_errors_like_top_level(self): assert "model.images.image0" in err["hint"] +class TestAutogrowMinSlots: + """The server places autogrow slots ``i < min`` in its ``required`` + section whenever the template's own inner input is required + (``Autogrow._expand_schema_for_dynamic``: ``if i < min and + template_required``), and rejects the prompt when they aren't wired. + Both the top-level group and a dynamic-combo-nested one must mirror that. + + The fixture's two groups mirror live nodes on ComfyUI master: + ``AutogrowTopNode.images`` is ``MeshyMultiImageToModelNode.images`` + (``TemplatePrefix(min=2, max=4)``), and ``AutogrowNestedNode.model``'s + ``strict`` option is ``GrokVideoReferenceNode.model`` + (``TemplateNames(reference_1…7, min=1)``). + """ + + def _wf(self, node: dict, *, reachable: bool = True) -> dict: + """`node` plus an IMAGE producer and a Sink output node. `reachable` + controls whether the Sink actually consumes the node — the server + prunes (and never validates) anything an output can't reach.""" + return { + "0": {"class_type": "ImageSrc", "inputs": {}}, + "1": node, + "2": {"class_type": "Sink", "inputs": {"image": ["1", 0]} if reachable else {}}, + } + + # -- nested (dynamic-combo sub-input) -------------------------------- + + def test_nested_min_one_with_zero_slots_errors(self, graph_dynamic: Graph): + wf = self._wf({"class_type": "AutogrowNestedNode", "inputs": {"model": "strict"}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + errs = [e for e in result["errors"] if e["code"] == "autogrow_below_min"] + assert len(errs) == 1 + assert errs[0]["node_id"] == "1" + assert errs[0]["field"] == "model.refs" + assert "minimum of 1" in errs[0]["message"] + assert "model.refs.reference_1" in errs[0]["hint"] + + def test_nested_min_one_with_one_slot_is_clean(self, graph_dynamic: Graph): + wf = self._wf( + { + "class_type": "AutogrowNestedNode", + "inputs": {"model": "strict", "model.refs.reference_1": ["0", 0]}, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + # The wired slot must stay a known key, not unknown_input noise. + assert result["warnings"] == [] + + def test_nested_below_min_still_accepts_the_slots_that_are_wired(self, graph_dynamic: Graph): + """A short count is ONE error about the count — the slots that ARE + present stay valid keys, rather than regressing into `unknown_input` + noise on top of it.""" + wf = self._wf( + { + "class_type": "AutogrowNestedNode", + "inputs": {"model": "strict_pair", "model.refs.reference_1": ["0", 0]}, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert [e["code"] for e in result["errors"]] == ["autogrow_below_min"] + assert "1 connected slot(s)" in result["errors"][0]["message"] + assert result["warnings"] == [] + + def test_nested_seedream_style_min_zero_stays_lenient(self, graph_dynamic: Graph): + """Regression pin: a group declaring `min: 0` INSIDE the option's + `required` section legitimately emits no slot keys at all (Seedream's + `model.images`), and the server accepts it.""" + wf = self._wf({"class_type": "AutogrowNestedNode", "inputs": {"model": "lenient"}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + + def test_optional_section_template_ignores_min(self, graph_dynamic: Graph): + """`min: 1`, but the template's inner input sits in the TEMPLATE's + `optional` section — the server's `template_required` gate is False, + so it never promotes a slot to `required` ("if not required, min can + be ignored").""" + wf = self._wf({"class_type": "AutogrowNestedNode", "inputs": {"model": "optional_template"}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + + def test_nested_below_min_on_unreachable_node_is_not_a_hard_error(self, graph_dynamic: Graph): + """The server prunes a node no output reaches and never validates it, + so the count check must not reject one either.""" + wf = self._wf({"class_type": "AutogrowNestedNode", "inputs": {"model": "strict"}}, reachable=False) + result = graph_dynamic.validate_workflow(wf) + assert result["errors"] == [] + + # -- top level -------------------------------------------------------- + + def test_top_level_partial_fill_below_min_errors(self, graph_dynamic: Graph): + """The false negative this closes: one wired slot against `min: 2` + used to pass, because the old check only looked for zero slots.""" + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {"images.image0": ["0", 0]}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + assert err["node_id"] == "1" + assert err["field"] == "images" + assert "1 connected slot(s)" in err["message"] + assert "minimum of 2" in err["message"] + + def test_top_level_at_min_is_clean(self, graph_dynamic: Graph): + wf = self._wf( + { + "class_type": "AutogrowTopNode", + "inputs": {"images.image0": ["0", 0], "images.image1": ["0", 0]}, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + + def test_top_level_zero_slots_keeps_the_no_slots_code(self, graph_dynamic: Graph): + """Zero slots keeps reporting `autogrow_no_slots` — the existing code + and wording — rather than being reclassified as below-min.""" + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + codes = [e["code"] for e in result["errors"]] + assert "autogrow_no_slots" in codes + assert "autogrow_below_min" not in codes + + def test_top_level_bare_wiring_does_not_also_report_a_count_error(self, graph_dynamic: Graph): + """`images: [src, idx]` is the `autogrow_bare_input` mistake; piling a + count error on top of it would just be noise.""" + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {"images": ["0", 0]}}) + result = graph_dynamic.validate_workflow(wf) + codes = [e["code"] for e in result["errors"]] + assert codes == ["autogrow_bare_input"] + + def test_top_level_below_min_on_unreachable_node_is_not_a_hard_error(self, graph_dynamic: Graph): + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {"images.image0": ["0", 0]}}, reachable=False) + result = graph_dynamic.validate_workflow(wf) + assert result["errors"] == [] + + def test_unreadable_template_keeps_the_historical_required_check(self, graph: Graph): + """A template-less group (`BatchImagesNode.images`) carries no + `template_required` signal, so the zero-slot check falls back to + `port.required` rather than being silently dropped — but there is no + min to enforce beyond that, so one slot is enough.""" + one_slot = { + "10": {"class_type": "VAEDecode", "inputs": {}}, + "20": {"class_type": "BatchImagesNode", "inputs": {"images.image0": ["10", 0]}}, + "30": {"class_type": "SaveImage", "inputs": {"images": ["20", 0], "filename_prefix": "out"}}, + } + codes = [e["code"] for e in graph.validate_workflow(one_slot)["errors"] if e["node_id"] == "20"] + assert codes == [] + + +class TestAutogrowEffectiveMin: + """`Port.autogrow_effective_min` in isolation — the `template_required` + gate it mirrors, and the leniency it falls back to.""" + + def _port(self, spec) -> Port: + from comfy_cli.cql.engine import _port_from_spec + + return _port_from_spec("images", spec, True) + + def test_required_template_section_binds_min(self): + p = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "prefix": "image", "min": 2, "max": 4}}, + ] + ) + assert p.autogrow_effective_min == 2 + assert p.autogrow_limits == (2, 4) + + def test_optional_template_section_ignores_min(self): + p = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"optional": {"image": ["IMAGE", {}]}}, "prefix": "image", "min": 2, "max": 4}}, + ] + ) + # `autogrow_limits` still reports the DECLARED min for display; only + # the effective (server-enforced) min is gated. + assert p.autogrow_limits == (2, 4) + assert p.autogrow_effective_min == 0 + + def test_declared_min_zero_binds_nothing(self): + p = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "names": ["a", "b"], "min": 0}}, + ] + ) + assert p.autogrow_effective_min == 0 + + def test_unreadable_template_reports_the_gate_as_unknown(self): + """An older/partial capture whose `min`/`names` sit BESIDE `template` + rather than inside it (the ByteDance Seedream v2 fixture shape) gives + no `template_required` signal at all. That must read as `None` — NOT + as a definite `False` — so `_check_autogrow_required` can tell "the + server ignores min here" apart from "we cannot tell" and keep its + historical `port.required` check for the latter.""" + p = self._port(["COMFY_AUTOGROW_V3", {"template": {"image": ["IMAGE", {}]}, "names": ["a"], "min": 0}]) + assert p.autogrow_template_required is None + assert p.autogrow_effective_min == 0 + p2 = self._port(["COMFY_AUTOGROW_V3", {}]) + assert p2.autogrow_template_required is None + assert p2.autogrow_effective_min == 0 + + def test_gate_is_a_definite_false_when_the_template_is_readable(self): + p = self._port(["COMFY_AUTOGROW_V3", {"template": {"input": {"optional": {"image": ["IMAGE", {}]}}, "min": 1}}]) + assert p.autogrow_template_required is False + + def test_non_autogrow_port_is_zero(self): + p = self._port(["INT", {"min": 3}]) + assert p.autogrow_template_required is None + assert p.autogrow_effective_min == 0 + + # =========================================================================== # `--input ` is an offline path — annotation lookup must not reach out # =========================================================================== diff --git a/tests/comfy_cli/cql/test_nested_autogrow.py b/tests/comfy_cli/cql/test_nested_autogrow.py index 55c5b2640..11d01dad5 100644 --- a/tests/comfy_cli/cql/test_nested_autogrow.py +++ b/tests/comfy_cli/cql/test_nested_autogrow.py @@ -161,3 +161,77 @@ def test_show_payload_top_level_autogrow_uses_schema_names(graph): assert images["autogrow"] is True assert images["element_type"] == "IMAGE" assert images["wire_as"].startswith("images.image0") + + +# --------------------------------------------------------------------------- # +# (D) the production catalog's own autogrow minimums are enforced +# --------------------------------------------------------------------------- # + + +def _grok_edit_v2_inputs(option_index: int = 0) -> tuple[dict, str]: + """Every widget `GrokImageEditNodeV2` requires for one model option, at its + schema defaults — everything except the `images` autogrow slots.""" + info = json.loads(FIXTURE.read_text()) + node = info["GrokImageEditNodeV2"]["input"]["required"] + option = node["model"][1]["options"][option_index] + + def default_of(spec): + opts = spec[1] if isinstance(spec, list) and len(spec) > 1 and isinstance(spec[1], dict) else {} + if "default" in opts: + return opts["default"] + return (opts.get("options") or [""])[0] + + inputs = {"model": option["key"]} + inputs.update({k: default_of(v) for k, v in node.items() if k != "model"}) + inputs.update( + {f"model.{k}": default_of(v) for k, v in option["inputs"].get("required", {}).items() if k != "images"} + ) + return inputs, option["key"] + + +def _grok_edit_v2_workflow(extra: dict) -> dict: + inputs, _ = _grok_edit_v2_inputs() + return { + "9": {"class_type": "LoadImage", "inputs": {"image": "example.png"}}, + "1": {"class_type": "GrokImageEditNodeV2", "inputs": {**inputs, **extra}}, + "2": {"class_type": "SaveImage", "inputs": {"images": ["1", 0], "filename_prefix": "out"}}, + } + + +def test_production_nested_autogrow_min_is_enforced(graph): + """The false negative, on the real captured catalog rather than a synthetic + one: ``GrokImageEditNodeV2.model.images`` declares ``min: 1`` with its inner + input in the template's ``required`` section, so the server places slot 0 in + ``required`` and rejects a prompt that wires none. This validated clean + before the ``autogrow_below_min`` check existed. + + Same declaration shape as ``GrokVideoReferenceNode.model.reference_images`` + (``TemplateNames(reference_1..7, min=1)``) on ComfyUI master. + """ + images = graph.node("GrokImageEditNodeV2") + assert images is not None + result = graph.validate_workflow(_grok_edit_v2_workflow({})) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + assert err["node_id"] == "1" + assert err["field"] == "model.images" + assert "model.images.image_1" in err["hint"] + + +def test_production_nested_autogrow_at_min_validates_clean(graph): + result = graph.validate_workflow(_grok_edit_v2_workflow({"model.images.image_1": ["9", 0]})) + assert result["valid"] is True, result["errors"] + + +def test_production_min_zero_group_stays_lenient(graph): + """``MinimaxHailuo03ReferenceNode``'s `reference_videos`/`reference_audios` + declare ``min: 0`` inside the option's ``required`` section — the deliberate + leniency that must survive the new minimum check.""" + port = next( + p + for p in graph.autogrow_groups("MinimaxHailuo03ReferenceNode", MINIMAX_UI_WIDGETS) + if p.name == "model.reference_videos" + ) + assert port.autogrow_template_required is True + assert port.autogrow_limits[0] == 0 + assert port.autogrow_effective_min == 0 diff --git a/tests/comfy_cli/fixtures/dynamic_combo_object_info.json b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json index d4c95576d..9c22e896c 100644 --- a/tests/comfy_cli/fixtures/dynamic_combo_object_info.json +++ b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json @@ -2,7 +2,13 @@ "ClaudeNode": { "input": { "required": { - "prompt": ["STRING", {"multiline": true, "default": ""}], + "prompt": [ + "STRING", + { + "multiline": true, + "default": "" + } + ], "model": [ "COMFY_DYNAMICCOMBO_V3", { @@ -11,20 +17,37 @@ "key": "Opus 4.6", "inputs": { "required": { - "max_tokens": ["INT", {"default": 1024, "min": 1, "max": 32000}], + "max_tokens": [ + "INT", + { + "default": 1024, + "min": 1, + "max": 32000 + } + ], "mode": [ "COMFY_DYNAMICCOMBO_V3", { "options": [ { "key": "fast", - "inputs": {"required": {}, "optional": {}} + "inputs": { + "required": {}, + "optional": {} + } }, { "key": "thinking", "inputs": { "required": { - "budget": ["INT", {"default": 1024, "min": 1, "max": 8000}] + "budget": [ + "INT", + { + "default": 1024, + "min": 1, + "max": 8000 + } + ] }, "optional": {} } @@ -34,7 +57,14 @@ ] }, "optional": { - "temperature": ["FLOAT", {"default": 1.0, "min": 0.0, "max": 1.0}] + "temperature": [ + "FLOAT", + { + "default": 1.0, + "min": 0.0, + "max": 1.0 + } + ] } } }, @@ -42,8 +72,20 @@ "key": "Haiku 4.5", "inputs": { "required": { - "max_tokens": ["INT", {"default": 512, "min": 1, "max": 8192}], - "style": [["concise", "detailed"]] + "max_tokens": [ + "INT", + { + "default": 512, + "min": 1, + "max": 8192 + } + ], + "style": [ + [ + "concise", + "detailed" + ] + ] }, "optional": {} } @@ -53,14 +95,249 @@ ] } }, - "input_order": {"required": ["prompt", "model"]}, - "output": ["STRING"], - "output_name": ["text"], + "input_order": { + "required": [ + "prompt", + "model" + ] + }, + "output": [ + "STRING" + ], + "output_name": [ + "text" + ], "category": "api/text", "display_name": "Claude", "description": "Synthetic dynamic-combo node for validator tests.", "output_node": true, "api_node": true, "python_module": "nodes" + }, + "ImageSrc": { + "input": { + "required": {} + }, + "input_order": { + "required": [] + }, + "output": [ + "IMAGE" + ], + "output_name": [ + "IMAGE" + ], + "category": "image", + "display_name": "Image Source", + "description": "Synthetic IMAGE producer for autogrow slot-count tests.", + "output_node": false, + "python_module": "nodes" + }, + "AutogrowTopNode": { + "input": { + "required": { + "images": [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": { + "required": { + "image": [ + "IMAGE", + {} + ] + } + }, + "prefix": "image", + "min": 2, + "max": 4 + } + } + ] + } + }, + "input_order": { + "required": [ + "images" + ] + }, + "output": [ + "IMAGE" + ], + "output_name": [ + "IMAGE" + ], + "category": "image", + "display_name": "Autogrow Top", + "description": "Synthetic top-level autogrow group with a min of 2 slots.", + "output_node": false, + "python_module": "nodes" + }, + "AutogrowNestedNode": { + "input": { + "required": { + "model": [ + "COMFY_DYNAMICCOMBO_V3", + { + "options": [ + { + "key": "strict", + "inputs": { + "required": { + "refs": [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": { + "required": { + "reference_image": [ + "IMAGE", + {} + ] + } + }, + "names": [ + "reference_1", + "reference_2", + "reference_3" + ], + "min": 1 + } + } + ] + }, + "optional": {} + } + }, + { + "key": "strict_pair", + "inputs": { + "required": { + "refs": [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": { + "required": { + "reference_image": [ + "IMAGE", + {} + ] + } + }, + "names": [ + "reference_1", + "reference_2", + "reference_3" + ], + "min": 2 + } + } + ] + }, + "optional": {} + } + }, + { + "key": "lenient", + "inputs": { + "required": { + "refs": [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": { + "required": { + "reference_image": [ + "IMAGE", + {} + ] + } + }, + "names": [ + "reference_1", + "reference_2", + "reference_3" + ], + "min": 0 + } + } + ] + }, + "optional": {} + } + }, + { + "key": "optional_template", + "inputs": { + "required": { + "refs": [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": { + "optional": { + "reference_image": [ + "IMAGE", + {} + ] + } + }, + "names": [ + "reference_1", + "reference_2", + "reference_3" + ], + "min": 1 + } + } + ] + }, + "optional": {} + } + } + ] + } + ] + } + }, + "input_order": { + "required": [ + "model" + ] + }, + "output": [ + "IMAGE" + ], + "output_name": [ + "IMAGE" + ], + "category": "image", + "display_name": "Autogrow Nested", + "description": "Synthetic dynamic-combo node whose options nest autogrow sub-inputs.", + "output_node": false, + "python_module": "nodes" + }, + "Sink": { + "input": { + "optional": { + "image": [ + "IMAGE", + {} + ] + } + }, + "input_order": { + "optional": [ + "image" + ] + }, + "output": [], + "output_name": [], + "category": "image", + "display_name": "Sink", + "description": "Synthetic output node; its only input is optional so it validates alone.", + "output_node": true, + "python_module": "nodes" } } From 916b45e1b3213d60cb2d8af79dbd692621926d7b Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 3 Sep 2026 02:27:41 -0700 Subject: [PATCH 2/4] fix(cql): require the slot NAMES the server marks required, not just a count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Counting keys under the autogrow prefix passes a prompt the server still rejects: `_expand_schema_for_dynamic` expands a fixed name list (`names` verbatim, or `prefix` + `range(max)`) and promotes `names[:min]` to `required`, so `images.image1` + `images.image2` satisfies `min: 2` by count while the required `images.image0` is missing. Slot gaps are not hypothetical here — `workflow_ops._first_free_autogrow_index` exists because legacy workflows carry them. `Port.autogrow_required_slot_names` derives those names the same way the server and this repo's own converter do (0-based, clamped to the declared maximum), and both check paths now report the specific missing keys. Where the catalog declares no naming template there is nothing to be precise about, so the count stays the test — including the unreadable- template carve-out, which keeps its historical "at least one slot" semantics rather than inheriting a synthesized min of 1 as a name. Also restores the dynamic-combo fixture's original compact formatting, so its diff is additive instead of a whole-file reflow. --- comfy_cli/cql/engine.py | 117 +++++++-- tests/comfy_cli/cql/test_engine.py | 115 ++++++++- .../fixtures/dynamic_combo_object_info.json | 240 +++--------------- 3 files changed, 240 insertions(+), 232 deletions(-) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 0aa29d9e6..f5eca0f06 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -298,6 +298,37 @@ def autogrow_effective_min(self) -> int: """ return self.autogrow_limits[0] if self.autogrow_template_required else 0 + def autogrow_required_slot_names(self, count: int) -> list[str] | None: + """The first ``count`` slot names the server places in its ``required`` + section — the *specific* keys a prompt owes, not merely how many. + + ``_expand_schema_for_dynamic`` expands a fixed name list + (``names`` verbatim, or ``[f"{prefix}{i}" for i in range(max)]``) and + promotes ``names[:min]`` to ``required``, so a count alone is not the + server's test: ``images.image1`` + ``images.image2`` satisfies ``min: + 2`` by count while the required ``images.image0`` is still missing, and + the server rejects it. Gaps like that are not hypothetical — see + ``workflow_ops._first_free_autogrow_index`` on legacy workflows. + + ``None`` when the catalog declares no naming template at all + (:attr:`autogrow_template`), so callers fall back to counting rather + than hard-erroring on the pluralization *guess* + :attr:`autogrow_element_template` would supply. Clamped to the + declared maximum, as the server's own ``enumerate(names)`` is. + """ + t = self.autogrow_template + if t is None: + return None + count = max(count, 0) + names = t.get("names") + if names: + return [str(n) for n in names[:count]] + hi = self.autogrow_limits[1] + if hi is not None: + count = min(count, hi) + # `autogrow_template` yields `names` or `prefix` and nothing else. + return [f"{t['prefix']}{i}" for i in range(count)] + @property def autogrow_template(self) -> dict | None: """The V3 autogrow element-naming template from object_info, if the @@ -2566,12 +2597,13 @@ def _check_dynamic_combo_sub( # unknown_input noise; the generic driver loop still edge-checks # whichever slot keys ARE present. # - # The one count that IS enforced is the server's own autogrow minimum - # (`Port.autogrow_effective_min` — the `i < min and template_required` - # gate). A group declaring `min: 0` inside `required` (Seedream's - # `model.images`) reads as an effective min of 0 and stays lenient; - # one declaring `min >= 1` (GrokVideoReferenceNode's - # `model.reference_images`) is a hard reject the server would issue. + # What IS enforced is the server's own autogrow minimum + # (`_autogrow_below_min_error` over `Port.autogrow_effective_min` — the + # `i < min and template_required` gate). A group declaring `min: 0` + # inside `required` (Seedream's `model.images`) reads as an effective + # min of 0 and stays lenient; one declaring `min >= 1` + # (GrokVideoReferenceNode's `model.reference_images`) is a hard reject + # the server would issue. # # A bare `dotted: [src, idx]` link, though, is the exact same mistake # the top-level autogrow_bare_input check catches — the server expects @@ -2600,10 +2632,8 @@ def _check_dynamic_combo_sub( ) slot_prefix = f"{dotted}." matched = {k for k in present if k.startswith(slot_prefix)} - lo = port.autogrow_effective_min - errors: list[dict] = [] - if len(matched) < lo: - errors.append(_autogrow_below_min_error(node_id, dotted, port, len(matched), lo)) + shortfall = _autogrow_below_min_error(node_id, dotted, port, matched, port.autogrow_effective_min) + errors: list[dict] = [shortfall] if shortfall else [] # The matched keys stay valid keys even when the count is short, so the # slots that ARE wired don't regress into `unknown_input` noise on top # of the count error. These errors flow into `dyn_errors`, which the @@ -2662,18 +2692,50 @@ def _check_dynamic_combo_sub( return errs, warns, set(), set() -def _autogrow_below_min_error(node_id: str, field: str, port: Port, n: int, lo: int) -> dict: - """One ``autogrow_below_min`` error — an autogrow group wired with fewer - slots than the server places in its ``required`` section.""" +def _autogrow_below_min_error(node_id: str, field: str, port: Port, slots: set[str], lo: int) -> dict | None: + """One ``autogrow_below_min`` error when ``slots`` — the ``{field}.`` keys + the prompt wires — don't cover the ones the server places in its + ``required`` section, else ``None``. + + Where the catalog names the slots, this checks the NAMES the server + expands (``Port.autogrow_required_slot_names``) rather than the count: + ``images.image1`` + ``images.image2`` is two slots against ``min: 2`` and + the server still rejects it for a missing ``images.image0``. Where it names + none, there is nothing to be precise about and the count is the best test + available. + """ + if lo < 1: + return None + names = port.autogrow_required_slot_names(lo) + if names is None: + n = len(slots) + if n >= lo: + return None + message = ( + f"autogrow input {field!r} has {n} connected slot(s) but declares a minimum of {lo} — " + f"the server will reject this node" + ) + hint = f"wire at least {lo} keys, one per connection: {port.autogrow_slot_example()}" + else: + missing = [key for name in names if (key := f"{field}.{name}") not in slots] + if not missing: + return None + # Same truncation as the sibling hints — a group's slot list is + # schema-driven and can run long, so don't dump it all into one line. + shown = ", ".join(repr(m) for m in missing[:8]) + ( + f" (and {len(missing) - 8} more)" if len(missing) > 8 else "" + ) + message = ( + f"autogrow input {field!r} is missing {len(missing)} of the {len(names)} slot(s) the server " + f"places in `required`: {shown} — the server will reject this node" + ) + hint = f"wire the missing slot key(s), one connection each: {shown}" return { "node_id": node_id, "field": field, "code": "autogrow_below_min", - "message": ( - f"autogrow input {field!r} has {n} connected slot(s) but declares a minimum of {lo} — " - f"the server will reject this node" - ), - "hint": f"wire at least {lo} keys, one per connection: {port.autogrow_slot_example()}", + "message": message, + "hint": hint, } @@ -2691,7 +2753,9 @@ def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node One carve-out keeps this strictly additive: a template we cannot read gives no ``template_required`` signal at all, so rather than silently dropping the zero-slot check for older/partial catalogs this falls back to the historical - ``port.required`` gate for exactly that case. + ``port.required`` gate for exactly that case — and to its historical + semantics too, "at least one slot", since the ``1`` it stands in for is + synthesized here rather than declared by the catalog. """ inputs = node_data.get("inputs") if not isinstance(inputs, dict): @@ -2699,16 +2763,17 @@ def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node errors: list[dict] = [] for base, port in autogrow_ports.items(): lo = port.autogrow_effective_min + cannot_judge = False if lo < 1: if port.autogrow_template_required is not None or not port.required: continue - lo = 1 + cannot_judge = True # The base name wired directly is a different mistake, already reported # as `autogrow_bare_input` by the driver loop — don't double-error it. if base in inputs: continue - n = sum(1 for k in inputs if k.startswith(f"{base}.")) - if n == 0: + slots = {k for k in inputs if k.startswith(f"{base}.")} + if not slots: errors.append( { "node_id": node_id, @@ -2720,8 +2785,12 @@ def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node "hint": f"wire one key per connection: {port.autogrow_slot_example()}", } ) - elif n < lo: - errors.append(_autogrow_below_min_error(node_id, base, port, n, lo)) + continue + if cannot_judge: + continue + shortfall = _autogrow_below_min_error(node_id, base, port, slots, lo) + if shortfall: + errors.append(shortfall) return errors diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index 0aca8be8a..0a61fcd84 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -3755,7 +3755,8 @@ def test_nested_min_one_with_zero_slots_errors(self, graph_dynamic: Graph): assert len(errs) == 1 assert errs[0]["node_id"] == "1" assert errs[0]["field"] == "model.refs" - assert "minimum of 1" in errs[0]["message"] + assert "missing 1 of the 1 slot(s)" in errs[0]["message"] + assert "'model.refs.reference_1'" in errs[0]["message"] assert "model.refs.reference_1" in errs[0]["hint"] def test_nested_min_one_with_one_slot_is_clean(self, graph_dynamic: Graph): @@ -3782,7 +3783,7 @@ def test_nested_below_min_still_accepts_the_slots_that_are_wired(self, graph_dyn ) result = graph_dynamic.validate_workflow(wf) assert [e["code"] for e in result["errors"]] == ["autogrow_below_min"] - assert "1 connected slot(s)" in result["errors"][0]["message"] + assert "'model.refs.reference_2'" in result["errors"][0]["message"] assert result["warnings"] == [] def test_nested_seedream_style_min_zero_stays_lenient(self, graph_dynamic: Graph): @@ -3820,8 +3821,60 @@ def test_top_level_partial_fill_below_min_errors(self, graph_dynamic: Graph): err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") assert err["node_id"] == "1" assert err["field"] == "images" - assert "1 connected slot(s)" in err["message"] - assert "minimum of 2" in err["message"] + assert "missing 1 of the 2 slot(s)" in err["message"] + assert "'images.image1'" in err["message"] + + def test_top_level_sparse_slots_meet_the_count_but_not_the_names(self, graph_dynamic: Graph): + """Two slots against `min: 2` — but the server expands `image0…image3` + and marks `image0`/`image1` required, so a workflow that skipped + `image0` is still a reject. Counting alone would pass this. + + Gaps are not hypothetical: `workflow_ops._first_free_autogrow_index` + exists because legacy workflows carry them.""" + wf = self._wf( + { + "class_type": "AutogrowTopNode", + "inputs": {"images.image1": ["0", 0], "images.image2": ["0", 0]}, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + assert "'images.image0'" in err["message"] + # Only the genuinely missing one is named — `images.image1` is wired. + assert "'images.image1'" not in err["message"] + + def test_top_level_undeclared_slot_names_do_not_count(self, graph_dynamic: Graph): + """Keys under the prefix that the schema never declares (`images.bogus`) + are not slots the server expands, so they cannot satisfy the minimum.""" + wf = self._wf( + { + "class_type": "AutogrowTopNode", + "inputs": {"images.image0": ["0", 0], "images.bogus": ["0", 0]}, + } + ) + result = graph_dynamic.validate_workflow(wf) + err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + assert "'images.image1'" in err["message"] + + def test_nested_sparse_slots_meet_the_count_but_not_the_names(self, graph_dynamic: Graph): + """The nested path owes the same precision: `min: 2` over + `reference_1…3` requires those first two names specifically.""" + wf = self._wf( + { + "class_type": "AutogrowNestedNode", + "inputs": { + "model": "strict_pair", + "model.refs.reference_2": ["0", 0], + "model.refs.reference_3": ["0", 0], + }, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert [e["code"] for e in result["errors"]] == ["autogrow_below_min"] + assert "'model.refs.reference_1'" in result["errors"][0]["message"] + # The wired slots stay known keys even while the count is short. + assert result["warnings"] == [] def test_top_level_at_min_is_clean(self, graph_dynamic: Graph): wf = self._wf( @@ -3933,6 +3986,60 @@ def test_non_autogrow_port_is_zero(self): assert p.autogrow_template_required is None assert p.autogrow_effective_min == 0 + def test_required_slot_names_from_a_prefix_template(self): + """The server expands `[f"{prefix}{i}" for i in range(max)]` and marks + the first `min` required — 0-based, exactly as this repo's own + converter names them (`workflow_ops._autogrow_elem_name`).""" + p = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "prefix": "image", "min": 2, "max": 4}}, + ] + ) + assert p.autogrow_required_slot_names(2) == ["image0", "image1"] + + def test_required_slot_names_from_a_names_template(self): + p = self._port( + [ + "COMFY_AUTOGROW_V3", + { + "template": { + "input": {"required": {"image": ["IMAGE", {}]}}, + "names": ["reference_1", "reference_2", "reference_3"], + "min": 2, + } + }, + ] + ) + assert p.autogrow_required_slot_names(2) == ["reference_1", "reference_2"] + + def test_required_slot_names_clamp_to_the_declared_maximum(self): + """`enumerate(names)` can't run past the name list, so neither can + this — a `min` above `max` owes only the names that exist.""" + p = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "prefix": "image", "min": 9, "max": 2}}, + ] + ) + assert p.autogrow_required_slot_names(9) == ["image0", "image1"] + names = self._port( + [ + "COMFY_AUTOGROW_V3", + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "names": ["a"], "min": 9}}, + ] + ) + assert names.autogrow_required_slot_names(9) == ["a"] + + def test_required_slot_names_are_none_without_a_declared_template(self): + """No declared naming template means the only naming available is the + pluralization GUESS — too weak to hard-error on, so callers fall back + to counting.""" + p = self._port(["COMFY_AUTOGROW_V3", {}]) + assert p.autogrow_required_slot_names(2) is None + # …even though the exporter-facing property still supplies the guess. + assert p.autogrow_element_template == {"prefix": "image"} + # =========================================================================== # `--input ` is an offline path — annotation lookup must not reach out diff --git a/tests/comfy_cli/fixtures/dynamic_combo_object_info.json b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json index 9c22e896c..60e623a23 100644 --- a/tests/comfy_cli/fixtures/dynamic_combo_object_info.json +++ b/tests/comfy_cli/fixtures/dynamic_combo_object_info.json @@ -2,13 +2,7 @@ "ClaudeNode": { "input": { "required": { - "prompt": [ - "STRING", - { - "multiline": true, - "default": "" - } - ], + "prompt": ["STRING", {"multiline": true, "default": ""}], "model": [ "COMFY_DYNAMICCOMBO_V3", { @@ -17,37 +11,20 @@ "key": "Opus 4.6", "inputs": { "required": { - "max_tokens": [ - "INT", - { - "default": 1024, - "min": 1, - "max": 32000 - } - ], + "max_tokens": ["INT", {"default": 1024, "min": 1, "max": 32000}], "mode": [ "COMFY_DYNAMICCOMBO_V3", { "options": [ { "key": "fast", - "inputs": { - "required": {}, - "optional": {} - } + "inputs": {"required": {}, "optional": {}} }, { "key": "thinking", "inputs": { "required": { - "budget": [ - "INT", - { - "default": 1024, - "min": 1, - "max": 8000 - } - ] + "budget": ["INT", {"default": 1024, "min": 1, "max": 8000}] }, "optional": {} } @@ -57,14 +34,7 @@ ] }, "optional": { - "temperature": [ - "FLOAT", - { - "default": 1.0, - "min": 0.0, - "max": 1.0 - } - ] + "temperature": ["FLOAT", {"default": 1.0, "min": 0.0, "max": 1.0}] } } }, @@ -72,20 +42,8 @@ "key": "Haiku 4.5", "inputs": { "required": { - "max_tokens": [ - "INT", - { - "default": 512, - "min": 1, - "max": 8192 - } - ], - "style": [ - [ - "concise", - "detailed" - ] - ] + "max_tokens": ["INT", {"default": 512, "min": 1, "max": 8192}], + "style": [["concise", "detailed"]] }, "optional": {} } @@ -95,18 +53,9 @@ ] } }, - "input_order": { - "required": [ - "prompt", - "model" - ] - }, - "output": [ - "STRING" - ], - "output_name": [ - "text" - ], + "input_order": {"required": ["prompt", "model"]}, + "output": ["STRING"], + "output_name": ["text"], "category": "api/text", "display_name": "Claude", "description": "Synthetic dynamic-combo node for validator tests.", @@ -115,58 +64,39 @@ "python_module": "nodes" }, "ImageSrc": { - "input": { - "required": {} - }, - "input_order": { - "required": [] - }, - "output": [ - "IMAGE" - ], - "output_name": [ - "IMAGE" - ], + "input": {"required": {}}, + "input_order": {"required": []}, + "output": ["IMAGE"], + "output_name": ["IMAGE"], "category": "image", "display_name": "Image Source", "description": "Synthetic IMAGE producer for autogrow slot-count tests.", "output_node": false, "python_module": "nodes" }, + "Sink": { + "input": {"optional": {"image": ["IMAGE", {}]}}, + "input_order": {"optional": ["image"]}, + "output": [], + "output_name": [], + "category": "image", + "display_name": "Sink", + "description": "Synthetic output node; its only input is optional so it validates alone.", + "output_node": true, + "python_module": "nodes" + }, "AutogrowTopNode": { "input": { "required": { "images": [ "COMFY_AUTOGROW_V3", - { - "template": { - "input": { - "required": { - "image": [ - "IMAGE", - {} - ] - } - }, - "prefix": "image", - "min": 2, - "max": 4 - } - } + {"template": {"input": {"required": {"image": ["IMAGE", {}]}}, "prefix": "image", "min": 2, "max": 4}} ] } }, - "input_order": { - "required": [ - "images" - ] - }, - "output": [ - "IMAGE" - ], - "output_name": [ - "IMAGE" - ], + "input_order": {"required": ["images"]}, + "output": ["IMAGE"], + "output_name": ["IMAGE"], "category": "image", "display_name": "Autogrow Top", "description": "Synthetic top-level autogrow group with a min of 2 slots.", @@ -186,24 +116,7 @@ "required": { "refs": [ "COMFY_AUTOGROW_V3", - { - "template": { - "input": { - "required": { - "reference_image": [ - "IMAGE", - {} - ] - } - }, - "names": [ - "reference_1", - "reference_2", - "reference_3" - ], - "min": 1 - } - } + {"template": {"input": {"required": {"reference_image": ["IMAGE", {}]}}, "names": ["reference_1", "reference_2", "reference_3"], "min": 1}} ] }, "optional": {} @@ -215,24 +128,7 @@ "required": { "refs": [ "COMFY_AUTOGROW_V3", - { - "template": { - "input": { - "required": { - "reference_image": [ - "IMAGE", - {} - ] - } - }, - "names": [ - "reference_1", - "reference_2", - "reference_3" - ], - "min": 2 - } - } + {"template": {"input": {"required": {"reference_image": ["IMAGE", {}]}}, "names": ["reference_1", "reference_2", "reference_3"], "min": 2}} ] }, "optional": {} @@ -244,24 +140,7 @@ "required": { "refs": [ "COMFY_AUTOGROW_V3", - { - "template": { - "input": { - "required": { - "reference_image": [ - "IMAGE", - {} - ] - } - }, - "names": [ - "reference_1", - "reference_2", - "reference_3" - ], - "min": 0 - } - } + {"template": {"input": {"required": {"reference_image": ["IMAGE", {}]}}, "names": ["reference_1", "reference_2", "reference_3"], "min": 0}} ] }, "optional": {} @@ -273,24 +152,7 @@ "required": { "refs": [ "COMFY_AUTOGROW_V3", - { - "template": { - "input": { - "optional": { - "reference_image": [ - "IMAGE", - {} - ] - } - }, - "names": [ - "reference_1", - "reference_2", - "reference_3" - ], - "min": 1 - } - } + {"template": {"input": {"optional": {"reference_image": ["IMAGE", {}]}}, "names": ["reference_1", "reference_2", "reference_3"], "min": 1}} ] }, "optional": {} @@ -301,43 +163,13 @@ ] } }, - "input_order": { - "required": [ - "model" - ] - }, - "output": [ - "IMAGE" - ], - "output_name": [ - "IMAGE" - ], + "input_order": {"required": ["model"]}, + "output": ["IMAGE"], + "output_name": ["IMAGE"], "category": "image", "display_name": "Autogrow Nested", "description": "Synthetic dynamic-combo node whose options nest autogrow sub-inputs.", "output_node": false, "python_module": "nodes" - }, - "Sink": { - "input": { - "optional": { - "image": [ - "IMAGE", - {} - ] - } - }, - "input_order": { - "optional": [ - "image" - ] - }, - "output": [], - "output_name": [], - "category": "image", - "display_name": "Sink", - "description": "Synthetic output node; its only input is optional so it validates alone.", - "output_node": true, - "python_module": "nodes" } } From cf6da5744bfe4ffb52b238aff1a6fcdcab9d612e Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 3 Sep 2026 02:28:45 -0700 Subject: [PATCH 3/4] test(cql): pin the sparse-slot case against the real captured catalog GrokImageEditNodeV2.model.images declares min: 1 over names image_1..3, so wiring only image_2 is one slot by count and still a server reject. --- tests/comfy_cli/cql/test_nested_autogrow.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/comfy_cli/cql/test_nested_autogrow.py b/tests/comfy_cli/cql/test_nested_autogrow.py index 11d01dad5..695712d7c 100644 --- a/tests/comfy_cli/cql/test_nested_autogrow.py +++ b/tests/comfy_cli/cql/test_nested_autogrow.py @@ -223,6 +223,19 @@ def test_production_nested_autogrow_at_min_validates_clean(graph): assert result["valid"] is True, result["errors"] +def test_production_nested_autogrow_counts_the_declared_names(graph): + """One wired slot against ``min: 1`` — but the wrong one. The server marks + ``model.images.image_1`` required (``names[:min]``) and rejects a prompt + that only wires ``image_2``, so a bare count of the ``model.images.`` keys + is not the server's test.""" + result = graph.validate_workflow(_grok_edit_v2_workflow({"model.images.image_2": ["9", 0]})) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + assert "'model.images.image_1'" in err["message"] + # The slot that IS wired stays a known key rather than unknown_input noise. + assert result["warnings"] == [] + + def test_production_min_zero_group_stays_lenient(graph): """``MinimaxHailuo03ReferenceNode``'s `reference_videos`/`reference_audios` declare ``min: 0`` inside the option's ``required`` section — the deliberate From 5f90897cc9609299cb8bbf4e306d6dd7b20f1eaf Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 3 Sep 2026 03:19:31 -0700 Subject: [PATCH 4/4] fix(cql): gate the autogrow minimum on what the catalog actually declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the autogrow-minimum check, all measured against the server's own `Autogrow._expand_schema_for_dynamic` rather than against the frontend's `applyAutogrow`. - Non-finite template bounds no longer crash validation. `/object_info` is parsed with plain `json.loads`, which accepts the bare `NaN` / `Infinity` literals a Python-serialized payload emits, and `int()` raises `ValueError` / `OverflowError` on those — inside `validate_workflow` and the `run` preflight, where a diagnostic is owed. New `_finite_int` reads them as undeclared instead. - The minimum binds only on a `min` the catalog DECLARES. `autogrow_limits` substitutes the frontend's default of 1 when the template omits `min`; the server reads `template["min"]` with no default at all. A hard reject naming a specific slot must not rest on that guess, so new `Port.autogrow_declared_min` returns `None` there and `autogrow_effective_min` treats it as "cannot judge" — the same leniency an unreadable `template_required` already got. `autogrow_limits` keeps its frontend-faithful contract for `nodes show`. - `autogrow_effective_min` is clamped to the group's capacity, as the server's own `for i, name in enumerate(names)` loop is: `names: ["a"]` with `min: 3` owed an unsatisfiable three slots. - The template's sections are walked in DECLARATION order. The server iterates `input.items()` and takes the first non-empty section; a hardcoded ("required", "optional") sweep read a template that lists a non-empty `optional` first as `template_required=True` while the server reads `False`, hard-rejecting a node the server accepts. Applies to `autogrow_template_required` and `autogrow_element_type` alike. - Only a bare `[src, idx]` link on the base key skips the slot check. The driver loop reports `autogrow_bare_input` for exactly that shape, so `{"images": null}` / `{"images": ""}` / `{"images": [[..],[..],[..]]}` silenced every count check while nothing else flagged them (`validate_shape` no-ops for COMFY_AUTOGROW_V3 and `_check_required_present` exempts autogrow ports) — and the server still rejected the node for the missing `images.image0`. - Zero slots reports `autogrow_no_slots` at BOTH depths, and the message and hint now name the count the gate applies rather than telling the user to wire one key against a `min` of 2. The nested path previously emitted `autogrow_below_min` for the identical condition, so the same authoring mistake carried two codes depending on nesting. Both paths now share `_autogrow_slot_errors`. - A nested slot key the template never declares (`model.images.bogus`) is no longer returned as a valid key, so it surfaces as `unknown_input` instead of being waved through by a bare prefix match. New `Port.autogrow_declared_slot_keys` does the filtering, matching only the server's own spelling (`f"{prefix}{i}"`, so not `image01`). Also narrowed while unifying the two paths: "cannot judge" now means an unreadable `template_required`, or a readable one that WOULD bind but declares no `min`. A gate that reads a definite `False` is the server telling us it ignores `min`, which is an answer, not a gap, so it no longer falls through to the "at least one slot" fallback. Deliberately NOT changed: the nested path stays lenient when it cannot judge, rather than gaining the top level's historical "at least one slot" fallback. That fallback predates this check at the top level only, and extending it downward rejects real workflows — a dynamic-combo option routinely declares an autogrow group in `required` with an effective `min: 0` (Seedream's `model.images`), which `test_autogrow_sub_with_no_slots_is_lenient` and `test_converted_seedream_workflow_is_valid` both pin. Co-Authored-By: Claude Opus 5 --- comfy_cli/cql/engine.py | 288 +++++++++++++++----- tests/comfy_cli/cql/test_engine.py | 224 ++++++++++++++- tests/comfy_cli/cql/test_nested_autogrow.py | 5 +- 3 files changed, 444 insertions(+), 73 deletions(-) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index f5eca0f06..cdfe359fc 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -14,10 +14,12 @@ import hashlib as _hashlib import json import logging +import math import urllib.error import urllib.parse import urllib.request from collections import defaultdict +from collections.abc import Iterable from dataclasses import dataclass, field from typing import Any @@ -99,6 +101,28 @@ # the whole type lattice. _MAX_PATH_SEARCH_STATES = 20_000 +# Digits an autogrow slot index may carry before it cannot be one the server +# generated. `Autogrow._MaxNames` caps a group at 100 slots; the bound exists so +# a prompt-supplied key like `image<5000 digits>` is rejected by length rather +# than by `int()`, which raises above 4300 digits. +_MAX_AUTOGROW_INDEX_DIGITS = 6 + + +def _finite_int(value: Any) -> int | None: + """``int(value)`` for a real, finite JSON number, else ``None``. + + ``/object_info`` is parsed with plain ``json.loads``, which accepts the bare + ``NaN`` / ``Infinity`` literals a Python-serialized payload emits — and + ``int()`` raises ``ValueError`` / ``OverflowError`` on those, a crash inside + ``validate_workflow`` where the validator owes a diagnostic. ``bool`` is + excluded because ``True`` is an ``int`` but never a numeric bound. + """ + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + if isinstance(value, float) and not math.isfinite(value): + return None + return int(value) + @dataclass class PortOptions: @@ -215,8 +239,11 @@ def autogrow_element_type(self) -> str | None: inputs = t.get("input") if not isinstance(inputs, dict): return None - for section in ("required", "optional"): - section_def = inputs.get(section) + # Declaration order, not a hardcoded ("required", "optional"): the + # server's `_expand_schema_for_dynamic` iterates `input.items()` and + # takes the first non-empty section, so a template that lists + # `optional` first is read from `optional`. + for section_def in inputs.values(): if not isinstance(section_def, dict): continue for spec in section_def.values(): @@ -232,15 +259,20 @@ def autogrow_limits(self) -> tuple[int, int | None]: length of ``names`` when the template enumerates them, else the template's ``max`` (``None`` when the schema leaves it open — the frontend's own default there is 100, but that is a UI choice, not a - server limit, so it is not asserted here).""" + server limit, so it is not asserted here). + + Bounds that are not finite JSON numbers read as undeclared rather than + crashing the conversion (see :func:`_finite_int`); the ``min`` default + of 1 is the frontend's, so callers that need the *server's* gate must + use :attr:`autogrow_declared_min` / :attr:`autogrow_effective_min`, + which never synthesize one.""" t = self.options.template if isinstance(self.options.template, dict) else {} - lo = t.get("min") - lo = int(lo) if isinstance(lo, (int, float)) and not isinstance(lo, bool) else 1 + lo = _finite_int(t.get("min")) + lo = 1 if lo is None else lo names = t.get("names") if isinstance(names, list) and names: return lo, len(names) - hi = t.get("max") - return lo, (int(hi) if isinstance(hi, (int, float)) and not isinstance(hi, bool) else None) + return lo, _finite_int(t.get("max")) @property def autogrow_template_required(self) -> bool | None: @@ -249,10 +281,13 @@ def autogrow_template_required(self) -> bool | None: ``required`` section. ``comfy_api/latest/_io.py``, ``Autogrow._expand_schema_for_dynamic`` - walks the template's sections, takes the first non-empty one, and sets - ``template_required = _input_type == "required"`` — *"for now, get just - the first value from dict_input; if not required, min can be ignored"*. - This walks the same sections in the same order. + walks the template's sections *in declaration order* (``input.items()``, + JSON insertion order — not a fixed required-then-optional sweep), takes + the first non-empty one, and sets ``template_required = _input_type == + "required"`` — *"for now, get just the first value from dict_input; if + not required, min can be ignored"*. This walks it the same way, so a + template that lists a non-empty ``optional`` ahead of ``required`` + reads ``False`` here exactly as it does on the server. ``None`` when there is nothing to read — a non-autogrow port, or a template carrying no ``input`` block (an older/partial catalog capture @@ -267,13 +302,28 @@ def autogrow_template_required(self) -> bool | None: inputs = t.get("input") if not isinstance(inputs, dict): return None - for section in ("required", "optional"): - section_def = inputs.get(section) + for section, section_def in inputs.items(): if not isinstance(section_def, dict) or not section_def: continue return section == "required" return None + @property + def autogrow_declared_min(self) -> int | None: + """The template's OWN ``min``, or ``None`` when the catalog declares + none (or declares one that isn't a finite number). + + Distinct from ``autogrow_limits[0]``, which substitutes the frontend's + default of 1: ``applyAutogrow`` picking 1 is a UI choice, while the + server reads ``value[1]["template"]["min"]`` with no default at all. + A hard reject must not be built on a number nothing declared, so the + validation path gates on this and treats ``None`` as "cannot judge". + """ + t = self.options.template + if not self.is_autogrow or not isinstance(t, dict): + return None + return _finite_int(t.get("min")) + @property def autogrow_effective_min(self) -> int: """Slots the server actually places in its ``required`` section — the @@ -290,13 +340,26 @@ def autogrow_effective_min(self) -> int: (``_expand_schema_for_dynamic`` ignores its ``input_type`` argument): an ``optional``-section group with ``min: 2`` still owes two slots. - ``0`` whenever the gate is not positively ``True`` — including the - "cannot judge" case, the same "don't reject what you can't read" - leniency :attr:`autogrow_element_type` applies. A caller that wants to - keep checking an unreadable template must consult - :attr:`autogrow_template_required` for the ``None`` itself. + ``0`` whenever the gate is not positively ``True`` — including the two + "cannot judge" cases (an unreadable ``template_required``, or a + template declaring no ``min`` of its own), the same "don't reject what + you can't read" leniency :attr:`autogrow_element_type` applies. A + caller that wants to keep checking an unreadable template must consult + :attr:`autogrow_template_required` and :attr:`autogrow_declared_min` + for the ``None`` itself. + + Clamped to the group's declared capacity, as the server's own + ``for i, name in enumerate(names)`` loop is: a template naming one slot + while declaring ``min: 3`` owes one slot, not an unsatisfiable three. """ - return self.autogrow_limits[0] if self.autogrow_template_required else 0 + if not self.autogrow_template_required: + return 0 + lo = self.autogrow_declared_min + if lo is None: + return 0 + lo = max(lo, 0) + hi = self.autogrow_limits[1] + return min(lo, hi) if hi is not None else lo def autogrow_required_slot_names(self, count: int) -> list[str] | None: """The first ``count`` slot names the server places in its ``required`` @@ -329,6 +392,54 @@ def autogrow_required_slot_names(self, count: int) -> list[str] | None: # `autogrow_template` yields `names` or `prefix` and nothing else. return [f"{t['prefix']}{i}" for i in range(count)] + def autogrow_declared_slot_keys(self, keys: Iterable[str]) -> set[str] | None: + """Those of ``keys`` that are slot keys this group actually grows — + ``{self.name}.{name}`` for a name the schema's template declares. + + ``_expand_schema_for_dynamic`` expands a fixed name list and puts only + those ids in the node's schema, so ``images.bogus`` (a typo, or a stale + key from another node) is not an input of the node at all: the server + ignores it. Callers use this to keep such a key out of the "known keys" + set, so it surfaces as ``unknown_input`` rather than being waved + through by a bare ``startswith`` on the prefix. + + ``None`` when the catalog declares no naming template + (:attr:`autogrow_template`), so callers fall back to the historical + prefix match rather than filtering against the pluralization *guess* + :attr:`autogrow_element_template` would supply. + """ + t = self.autogrow_template + if t is None: + return None + prefix = f"{self.name}." + suffixes = {k: k[len(prefix) :] for k in keys if k.startswith(prefix)} + names = t.get("names") + if names: + declared = {str(n) for n in names} + return {k for k, suffix in suffixes.items() if suffix in declared} + stem = t["prefix"] + hi = self.autogrow_limits[1] + out: set[str] = set() + for key, suffix in suffixes.items(): + index = suffix[len(stem) :] + # `isascii()` as well as `isdigit()`: the latter is True for + # superscripts and other numeric characters that `int()` rejects + # outright. The length bound is the other half of that — `int()` + # raises above 4300 digits — and both matter because this runs over + # prompt-supplied key names. `Autogrow._MaxNames` is 100, so no id + # the server generates comes anywhere near the bound. + if not suffix.startswith(stem) or not (index.isascii() and index.isdigit()): + continue + if len(index) > _MAX_AUTOGROW_INDEX_DIGITS: + continue + # Round-trip the index so only the server's own spelling matches: + # `image01` is not an id `f"{prefix}{i}"` ever emits. + if f"{stem}{int(index)}" != suffix: + continue + if hi is None or int(index) < hi: + out.add(key) + return out + @property def autogrow_template(self) -> dict | None: """The V3 autogrow element-naming template from object_info, if the @@ -2597,11 +2708,11 @@ def _check_dynamic_combo_sub( # unknown_input noise; the generic driver loop still edge-checks # whichever slot keys ARE present. # - # What IS enforced is the server's own autogrow minimum - # (`_autogrow_below_min_error` over `Port.autogrow_effective_min` — the - # `i < min and template_required` gate). A group declaring `min: 0` - # inside `required` (Seedream's `model.images`) reads as an effective - # min of 0 and stays lenient; one declaring `min >= 1` + # What IS enforced is the server's own autogrow minimum, through the + # same `_autogrow_slot_errors` the top-level driver uses, so the same + # mistake reports the same code at either depth. A group declaring + # `min: 0` inside `required` (Seedream's `model.images`) reads as an + # effective min of 0 and stays lenient; one declaring `min >= 1` # (GrokVideoReferenceNode's `model.reference_images`) is a hard reject # the server would issue. # @@ -2632,13 +2743,29 @@ def _check_dynamic_combo_sub( ) slot_prefix = f"{dotted}." matched = {k for k in present if k.startswith(slot_prefix)} - shortfall = _autogrow_below_min_error(node_id, dotted, port, matched, port.autogrow_effective_min) - errors: list[dict] = [shortfall] if shortfall else [] - # The matched keys stay valid keys even when the count is short, so the - # slots that ARE wired don't regress into `unknown_input` noise on top - # of the count error. These errors flow into `dyn_errors`, which the - # driver loop already gates on output-reachability. - return errors, [], matched, set() + # `required=False` deliberately, unlike the top-level caller: the + # historical "at least one slot" fallback for an unreadable template is + # a TOP-LEVEL behaviour that predates this check, while a nested group + # has always been lenient there — a dynamic-combo option routinely + # declares an autogrow group in `required` with an effective `min: 0` + # (Seedream's `model.images`), and the converter legitimately emits no + # slot keys for it. Extending the fallback down here to make the two + # depths symmetric would reject those real workflows, so the asymmetry + # stays: nested enforces only a minimum the catalog actually declares. + errors = _autogrow_slot_errors(node_id, dotted, port, matched, required=False) + # Only the keys the group actually grows count as known. A key under + # the prefix that the template never declares (`model.images.bogus`) is + # not an input of the node at all, so waving it through here would + # suppress its `unknown_input` warning as well as its slot check. + # Where the catalog declares no names there is nothing to filter + # against, so the historical prefix match stands. + declared = port.autogrow_declared_slot_keys(matched) + valid = matched if declared is None else declared + # The valid keys stay valid even when the count is short, so the slots + # that ARE wired don't regress into `unknown_input` noise on top of the + # count error. These errors flow into `dyn_errors`, which the driver + # loop already gates on output-reachability. + return errors, [], valid, set() if dotted not in present: if not sub_required: @@ -2739,58 +2866,81 @@ def _autogrow_below_min_error(node_id: str, field: str, port: Port, slots: set[s } -def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node_data: dict) -> list[dict]: - """Autogrow inputs wired with fewer slots than the server requires. +def _autogrow_slot_errors(node_id: str, field: str, port: Port, slots: set[str], required: bool) -> list[dict]: + """Every slot-count error one autogrow group owes — shared by the top-level + driver (:func:`_check_autogrow_required`) and the dynamic-combo-nested path + (:func:`_check_dynamic_combo_sub`) so the same authoring mistake reports the + same code at every nesting depth. - The gate is :attr:`Port.autogrow_effective_min`, not ``port.required``: the + The gate is :attr:`Port.autogrow_effective_min`, not ``required``: the server places autogrow slots ``i < min`` in ``required`` whenever the template's inner input is itself required, regardless of which section the autogrow input sits in — and conversely ignores ``min`` entirely when it is - not (``comfy_api/latest/_io.py``, - ``Autogrow._expand_schema_for_dynamic``). Either way the server would reject - the node, so surface it here instead of as a cryptic downstream reject. + not (``comfy_api/latest/_io.py``, ``Autogrow._expand_schema_for_dynamic``). One carve-out keeps this strictly additive: a template we cannot read gives - no ``template_required`` signal at all, so rather than silently dropping the - zero-slot check for older/partial catalogs this falls back to the historical - ``port.required`` gate for exactly that case — and to its historical - semantics too, "at least one slot", since the ``1`` it stands in for is - synthesized here rather than declared by the catalog. + no ``template_required`` signal, and one whose gate WOULD bind but which + carries no ``min`` of its own declares no minimum for it to bind to + (``autogrow_limits`` substitutes the *frontend's* default of 1 there, which + is no basis for a hard reject). For exactly those two cases this falls back + to the historical ``required`` gate — and to its historical semantics too, + "at least one slot", since the ``1`` it stands in for is synthesized here + rather than declared by the catalog. A gate that reads a definite ``False`` + is not one of them: that is the server telling us it ignores ``min``. + """ + lo = port.autogrow_effective_min + # "Cannot judge" is narrower than "no minimum binds": a template whose gate + # reads a definite `False` tells us the server ignores `min` outright, which + # is an answer, not a gap. Only an unreadable gate — or a readable one that + # WOULD bind but declares no `min` for it to bind to — leaves us guessing. + gate = port.autogrow_template_required + cannot_judge = gate is None or (gate is True and port.autogrow_declared_min is None) + if lo < 1: + if not cannot_judge or not required: + return [] + if not slots: + owed = f" but the server places {lo} of them in `required`" if lo >= 1 else "" + return [ + { + "node_id": node_id, + "field": field, + "code": "autogrow_no_slots", + "message": ( + f"autogrow input {field!r} has no connected slots{owed} — the server will reject this node" + ), + "hint": (f"wire {lo if lo >= 1 else 1} key(s), one per connection: {port.autogrow_slot_example()}"), + } + ] + if cannot_judge: + return [] + shortfall = _autogrow_below_min_error(node_id, field, port, slots, lo) + return [shortfall] if shortfall else [] + + +def _check_autogrow_required(node_id: str, autogrow_ports: dict[str, Port], node_data: dict) -> list[dict]: + """Autogrow inputs wired with fewer slots than the server requires. + + Every gate and carve-out lives in :func:`_autogrow_slot_errors`, which the + nested dynamic-combo path shares so both depths report the same code. """ inputs = node_data.get("inputs") if not isinstance(inputs, dict): inputs = {} errors: list[dict] = [] for base, port in autogrow_ports.items(): - lo = port.autogrow_effective_min - cannot_judge = False - if lo < 1: - if port.autogrow_template_required is not None or not port.required: - continue - cannot_judge = True - # The base name wired directly is a different mistake, already reported - # as `autogrow_bare_input` by the driver loop — don't double-error it. - if base in inputs: + # The base name wired directly as a single connection is a different + # mistake, already reported as `autogrow_bare_input` by the driver loop + # — don't double-error it. Only that exact shape, though: the driver + # loop tests `isinstance(value, list) and len(value) == 2` too, so + # `{base: None}` / `{base: ""}` / `{base: [[..], [..], [..]]}` raise no + # error anywhere (`validate_shape` no-ops for COMFY_AUTOGROW_V3 and + # `_check_required_present` exempts autogrow ports) and skipping on + # mere presence of the key would silence the slot check for free. + value = inputs.get(base) + if isinstance(value, list) and len(value) == 2: continue slots = {k for k in inputs if k.startswith(f"{base}.")} - if not slots: - errors.append( - { - "node_id": node_id, - "field": base, - "code": "autogrow_no_slots", - "message": ( - f"required autogrow input {base!r} has no connected slots — the server will reject this node" - ), - "hint": f"wire one key per connection: {port.autogrow_slot_example()}", - } - ) - continue - if cannot_judge: - continue - shortfall = _autogrow_below_min_error(node_id, base, port, slots, lo) - if shortfall: - errors.append(shortfall) + errors.extend(_autogrow_slot_errors(node_id, base, port, slots, port.required)) return errors diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index 0a61fcd84..cbc76040d 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -3748,16 +3748,19 @@ def _wf(self, node: dict, *, reachable: bool = True) -> dict: # -- nested (dynamic-combo sub-input) -------------------------------- def test_nested_min_one_with_zero_slots_errors(self, graph_dynamic: Graph): + """Zero slots reports `autogrow_no_slots` at BOTH depths — the same + authoring mistake must not carry two different codes depending on + nesting, or a consumer filtering on one silently misses the other.""" wf = self._wf({"class_type": "AutogrowNestedNode", "inputs": {"model": "strict"}}) result = graph_dynamic.validate_workflow(wf) assert result["valid"] is False - errs = [e for e in result["errors"] if e["code"] == "autogrow_below_min"] + errs = [e for e in result["errors"] if e["code"] == "autogrow_no_slots"] assert len(errs) == 1 assert errs[0]["node_id"] == "1" assert errs[0]["field"] == "model.refs" - assert "missing 1 of the 1 slot(s)" in errs[0]["message"] - assert "'model.refs.reference_1'" in errs[0]["message"] + assert "places 1 of them in `required`" in errs[0]["message"] assert "model.refs.reference_1" in errs[0]["hint"] + assert [e["code"] for e in result["errors"] if e["code"] == "autogrow_below_min"] == [] def test_nested_min_one_with_one_slot_is_clean(self, graph_dynamic: Graph): wf = self._wf( @@ -3909,6 +3912,52 @@ def test_top_level_below_min_on_unreachable_node_is_not_a_hard_error(self, graph result = graph_dynamic.validate_workflow(wf) assert result["errors"] == [] + def test_top_level_zero_slots_names_the_minimum_it_owes(self, graph_dynamic: Graph): + """The zero-slot message/hint must state the count the gate actually + applies: with `min: 2`, a user who follows a "wire one key" hint gets + rejected a second time by `autogrow_below_min`.""" + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {}}) + result = graph_dynamic.validate_workflow(wf) + err = next(e for e in result["errors"] if e["code"] == "autogrow_no_slots") + assert "places 2 of them in `required`" in err["message"] + assert "wire 2 key(s)" in err["hint"] + + @pytest.mark.parametrize( + "bad", + [None, "", [], ["0", 0, 1], [["0", 0], ["0", 0], ["0", 0]]], + ids=["null", "empty-string", "empty-list", "three-tuple", "list-of-links"], + ) + def test_base_key_wired_as_a_non_link_is_still_slot_checked(self, graph_dynamic: Graph, bad): + """Only a bare `[src, idx]` link is `autogrow_bare_input`, so only that + exact shape may skip the slot check. Any other value on the base key + raises no error anywhere else (`validate_shape` no-ops for + COMFY_AUTOGROW_V3 and `_check_required_present` exempts autogrow + ports), so skipping on mere presence silenced the check for free while + the server still rejected the node for the missing `images.image0`.""" + wf = self._wf({"class_type": "AutogrowTopNode", "inputs": {"images": bad}}) + result = graph_dynamic.validate_workflow(wf) + assert result["valid"] is False + assert "autogrow_no_slots" in [e["code"] for e in result["errors"]] + + def test_nested_undeclared_slot_key_is_not_waved_through(self, graph_dynamic: Graph): + """A key under the group's prefix that the template never declares is + not an input of the node at all — the server ignores it — so it owes an + `unknown_input` warning rather than being accepted by a bare prefix + match alongside the slots that ARE real.""" + wf = self._wf( + { + "class_type": "AutogrowNestedNode", + "inputs": { + "model": "strict", + "model.refs.reference_1": ["0", 0], + "model.refs.bogus": ["0", 0], + }, + } + ) + result = graph_dynamic.validate_workflow(wf) + assert result["errors"] == [] + assert [(w["code"], w["field"]) for w in result["warnings"]] == [("unknown_input", "model.refs.bogus")] + def test_unreadable_template_keeps_the_historical_required_check(self, graph: Graph): """A template-less group (`BatchImagesNode.images`) carries no `template_required` signal, so the zero-slot check falls back to @@ -3923,6 +3972,175 @@ def test_unreadable_template_keeps_the_historical_required_check(self, graph: Gr assert codes == [] +class TestAutogrowSchemaEdges: + """Reading the autogrow template itself: bounds that aren't numbers, a + template that declares no `min`, sections in an unexpected order, and a + `min` larger than the group's own capacity. Each is a shape the server + either handles differently from the frontend or does not accept at all, so + none of them may become a hard reject built on a number we synthesized.""" + + _INFO = """ + { + "ImageSrc": {"input": {"required": {}}, "input_order": {"required": []}, + "output": ["IMAGE"], "output_name": ["image"], "display_name": "S", + "python_module": "nodes"}, + "Sink": {"input": {"required": {}, "optional": {"image": ["IMAGE", {}]}}, + "input_order": {"required": [], "optional": ["image"]}, + "output": [], "output_name": [], "output_node": true, "display_name": "K", + "python_module": "nodes"}, + "Grow": { + "input": {"__SECTION__": {"images": ["COMFY_AUTOGROW_V3", {"template": __TEMPLATE__}]}}, + "input_order": {"__SECTION__": ["images"]}, + "output": ["IMAGE"], "output_name": ["IMAGE"], "display_name": "G", + "python_module": "nodes"} + } + """ + + def _graph(self, template: str, *, required_section: bool = True) -> Graph: + # Parsed the way the loader parses `/object_info` — plain `json.loads`, + # which accepts the bare `NaN`/`Infinity` literals a Python-serialized + # payload emits. Kept as raw text (not a dict) for exactly that reason. + raw = self._INFO.replace("__SECTION__", "required" if required_section else "optional").replace( + "__TEMPLATE__", template + ) + return Graph.from_object_info(json.loads(raw)) + + def _wf(self, inputs: dict) -> dict: + return { + "0": {"class_type": "ImageSrc", "inputs": {}}, + "1": {"class_type": "Grow", "inputs": inputs}, + "2": {"class_type": "Sink", "inputs": {"image": ["1", 0]}}, + } + + IMAGE_TEMPLATE_INPUT = '{"required": {"image": ["IMAGE", {}]}}' + + def _template(self, body: str, template_input: str | None = None) -> str: + """A `template` block with `input` prefilled — `body` is the rest of it + (`"prefix": ..., "min": ...`) verbatim.""" + return '{"input": ' + (template_input or self.IMAGE_TEMPLATE_INPUT) + ", " + body + "}" + + def test_non_finite_bounds_yield_a_diagnostic_not_a_crash(self): + """`json.loads` accepts `NaN`/`Infinity`, and `int()` raises + `ValueError`/`OverflowError` on them — a crash inside + `validate_workflow` and the `run` preflight where the validator owes a + diagnostic. Non-numeric bounds read as undeclared instead.""" + graph = self._graph(self._template('"prefix": "image", "min": NaN, "max": Infinity')) + port = graph.node("Grow").inputs[0] + assert port.autogrow_declared_min is None + assert port.autogrow_limits == (1, None) # the frontend's default min, no max + assert port.autogrow_effective_min == 0 + # Still validates rather than raising, and still reports the historical + # zero-slot error through the `required` fallback. + result = graph.validate_workflow(self._wf({})) + assert [e["code"] for e in result["errors"]] == ["autogrow_no_slots"] + assert graph.validate_workflow(self._wf({"images.image0": ["0", 0]}))["valid"] is True + + def test_a_template_declaring_no_min_is_not_a_hard_reject_on_a_synthesized_one(self): + """`autogrow_limits` substitutes the FRONTEND's default of 1 when the + template omits `min`; the server reads `template["min"]` with no + default at all. A hard reject naming a specific slot must not be built + on that guess, so an undeclared `min` reads as "cannot judge" and only + the historical zero-slot check applies.""" + graph = self._graph(self._template('"prefix": "image", "max": 4')) + port = graph.node("Grow").inputs[0] + assert port.autogrow_limits[0] == 1 # frontend-faithful, unchanged + assert port.autogrow_declared_min is None + assert port.autogrow_effective_min == 0 + # One slot wired — but not slot 0. A synthesized `min: 1` would demand + # `images.image0` by name; nothing declared that, so it stays clean. + assert graph.validate_workflow(self._wf({"images.image3": ["0", 0]}))["valid"] is True + assert [e["code"] for e in graph.validate_workflow(self._wf({}))["errors"]] == ["autogrow_no_slots"] + + def test_optional_section_declared_first_wins_as_it_does_on_the_server(self): + """`_expand_schema_for_dynamic` iterates `input.items()` and takes the + first NON-EMPTY section, so a template listing `optional` ahead of + `required` reads `template_required = False` and the server ignores + `min` entirely. A hardcoded required-then-optional sweep read the + opposite and hard-rejected a node the server accepts.""" + template_input = '{"optional": {"image": ["IMAGE", {}]}, "required": {"other": ["IMAGE", {}]}}' + graph = self._graph(self._template('"prefix": "image", "min": 2', template_input)) + port = graph.node("Grow").inputs[0] + assert port.autogrow_template_required is False + assert port.autogrow_effective_min == 0 + assert graph.validate_workflow(self._wf({}))["valid"] is True + + def test_an_empty_leading_section_is_skipped_like_the_server_skips_it(self): + """`if len(dict_input) == 0: continue` — an empty `optional` declared + first does not make the group optional.""" + template_input = '{"optional": {}, "required": {"image": ["IMAGE", {}]}}' + graph = self._graph(self._template('"prefix": "image", "min": 2', template_input)) + port = graph.node("Grow").inputs[0] + assert port.autogrow_template_required is True + assert port.autogrow_effective_min == 2 + + def test_min_above_the_groups_capacity_is_clamped(self): + """The server promotes slots through `for i, name in enumerate(names)`, + so a `min` past the end of `names` simply never reaches those indices. + Demanding three slots from a group that grows one would be an error the + user cannot clear.""" + graph = self._graph(self._template('"names": ["a"], "min": 3')) + port = graph.node("Grow").inputs[0] + assert port.autogrow_limits == (3, 1) + assert port.autogrow_effective_min == 1 + assert graph.validate_workflow(self._wf({"images.a": ["0", 0]}))["valid"] is True + + def test_declared_slot_keys_match_only_the_servers_own_spelling(self): + """`f"{prefix}{i}"` never emits `image01` or `image-1`, and `max: 4` + stops the expansion at `image3`.""" + graph = self._graph(self._template('"prefix": "image", "min": 1, "max": 4')) + port = graph.node("Grow").inputs[0] + keys = ["images.image0", "images.image01", "images.image-1", "images.image4", "images.bogus", "other.image0"] + assert port.autogrow_declared_slot_keys(keys) == {"images.image0"} + + def test_declared_slot_keys_survive_hostile_index_text(self): + """Slot names come from the prompt, so the index parse must not be + reachable with anything `int()` rejects: `isdigit()` is True for + superscripts, and `int()` raises outright above 4300 digits.""" + graph = self._graph(self._template('"prefix": "image", "min": 1, "max": 4')) + port = graph.node("Grow").inputs[0] + keys = ["images.image\u00b2", "images.image" + "9" * 5000, "images.image1"] + assert port.autogrow_declared_slot_keys(keys) == {"images.image1"} + + def test_a_definite_false_gate_is_an_answer_not_a_gap(self): + """A template whose inner input sits in `optional` tells us the server + ignores `min` outright — that is an answer, so it must NOT fall through + to the historical "at least one slot" gate the way an unreadable + template does, even when the template also declares no `min`.""" + template_input = '{"optional": {"image": ["IMAGE", {}]}}' + graph = self._graph(self._template('"prefix": "image"', template_input)) + port = graph.node("Grow").inputs[0] + assert port.autogrow_template_required is False + assert port.autogrow_declared_min is None + assert graph.validate_workflow(self._wf({}))["valid"] is True + + def test_unreadable_template_still_errors_on_zero_slots(self): + """The other half of `test_unreadable_template_keeps_the_historical_required_check`: + a template with no `input` block gives no `template_required` signal, + so the zero-slot check falls back to the historical `required` gate — + but the declared `min: 2` never binds, and no hard error may name a + *specific* slot on a signal the catalog never gave.""" + graph = self._graph('{"prefix": "image", "min": 2, "max": 4}') + port = graph.node("Grow").inputs[0] + assert port.autogrow_template_required is None + assert port.autogrow_effective_min == 0 + assert [e["code"] for e in graph.validate_workflow(self._wf({}))["errors"]] == ["autogrow_no_slots"] + # One slot clears it — the declared `min: 2` is NOT enforced, because + # the signal that would make it binding is unreadable. + assert graph.validate_workflow(self._wf({"images.image3": ["0", 0]}))["valid"] is True + + def test_unreadable_template_on_an_optional_input_is_not_checked_at_all(self): + """The historical gate is `required`; an optional autogrow input with + an unreadable template stays unchecked, as it always has.""" + graph = self._graph('{"prefix": "image", "min": 2}', required_section=False) + assert graph.validate_workflow(self._wf({}))["valid"] is True + + def test_declared_slot_keys_are_none_without_a_naming_template(self): + """No template to filter against — callers keep the historical prefix + match rather than filtering on a pluralization guess.""" + graph = self._graph(self._template('"min": 1')) + assert graph.node("Grow").inputs[0].autogrow_declared_slot_keys(["images.image0"]) is None + + class TestAutogrowEffectiveMin: """`Port.autogrow_effective_min` in isolation — the `template_required` gate it mirrors, and the leniency it falls back to.""" diff --git a/tests/comfy_cli/cql/test_nested_autogrow.py b/tests/comfy_cli/cql/test_nested_autogrow.py index 695712d7c..d432c2ab7 100644 --- a/tests/comfy_cli/cql/test_nested_autogrow.py +++ b/tests/comfy_cli/cql/test_nested_autogrow.py @@ -212,9 +212,12 @@ def test_production_nested_autogrow_min_is_enforced(graph): assert images is not None result = graph.validate_workflow(_grok_edit_v2_workflow({})) assert result["valid"] is False - err = next(e for e in result["errors"] if e["code"] == "autogrow_below_min") + # Zero slots is `autogrow_no_slots` at every depth; a partial fill is + # `autogrow_below_min` (see test_production_nested_autogrow_counts_the_declared_names). + err = next(e for e in result["errors"] if e["code"] == "autogrow_no_slots") assert err["node_id"] == "1" assert err["field"] == "model.images" + assert "places 1 of them in `required`" in err["message"] assert "model.images.image_1" in err["hint"]