From e4a60bc333c8f937912be38fb2afa02c6a07c38d Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 17 Jul 2026 14:04:31 -0700 Subject: [PATCH 1/2] fix(validate): presence-check required inputs, no-outputs check, hard-error range violations (BE-3357) --- comfy_cli/cql/engine.py | 87 +++++++- tests/comfy_cli/command/test_run.py | 4 +- tests/comfy_cli/cql/test_engine.py | 301 +++++++++++++++++++++------- 3 files changed, 318 insertions(+), 74 deletions(-) diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index a8068aaa..f7c53d0c 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -699,6 +699,11 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: errors: list[dict] = [] warnings: list[dict] = [] all_names = list(self._nodes.keys()) + # No-outputs check: the server rejects any prompt with zero output nodes + # (execution.py:1155-1162, prompt_no_outputs). Track whether we saw any + # recognized node and whether any was an output node. + any_recognized = False + has_output_node = False for node_id, node_data in workflow.items(): # `_meta` is the compose/run provenance block (schema/blueprint/items), @@ -744,6 +749,10 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: ) continue + any_recognized = True + if m.is_output_node: + has_output_node = True + 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 @@ -867,6 +876,18 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: warnings.extend(cat_warnings) errors.extend(_check_autogrow_required(node_id, autogrow_ports, autogrow_seen, node_data)) + errors.extend(_check_required_present(node_id, m, node_data)) + + # No-outputs check: a prompt with recognized nodes but zero output nodes + # is rejected server-side (execution.py:1155-1162, prompt_no_outputs). + if any_recognized and not has_output_node: + errors.append( + { + "code": "prompt_no_outputs", + "message": "workflow has no output nodes — the server will reject it (prompt_no_outputs)", + "hint": "add an output node such as SaveImage/PreviewImage", + } + ) return { "valid": len(errors) == 0, @@ -1020,8 +1041,9 @@ def _validate_catalog_value( ) -> tuple[list[dict], list[dict]]: """Enum-membership and other catalog checks for one scalar input value. - Returns (errors, warnings): an unknown enum value is a hard error carrying - the valid options; every other catalog finding is a namespaced warning. + Returns (errors, warnings): unknown-enum and out-of-range values are hard + errors (the server rejects them); every other catalog finding is a + namespaced warning. """ errors: list[dict] = [] warnings: list[dict] = [] @@ -1046,12 +1068,73 @@ def _validate_catalog_value( "valid_options": list(port.enum_values), } ) + elif w["code"] in ("below_min", "above_max"): + # The server hard-rejects out-of-range values + # (value_smaller_than_min / value_bigger_than_max, execution.py:1008-1033), + # so promote these to errors — same as unknown_enum_value above. One + # theoretical over-strictness: the server skips its built-in range + # checks for args covered by a node's custom VALIDATE_INPUTS + # (execution.py:1007); we accept that trade-off exactly as the + # unknown_enum_value hard error does. + bound = port.options.min if w["code"] == "below_min" else port.options.max + op = ">=" if w["code"] == "below_min" else "<=" + errors.append( + { + "node_id": node_id, + "field": input_name, + "code": w["code"], + "message": w["message"], + "hint": f"use a value {op} {bound}", + } + ) else: w["field"] = f"{node_id}.{class_type}.{w['field']}" warnings.append(w) return errors, warnings +def _check_required_present(node_id: str, m: Morphism, node_data: dict) -> list[dict]: + """Required inputs that are absent from ``node_data["inputs"]`` entirely. + + Mirrors the server (execution.py:884-900): any required input the frontend + didn't serialize is a hard reject (required_input_missing). The per-input + loop only inspects keys that ARE present, so this catches the absent ones. + Skipped, because each is handled by its own path (and would otherwise + double-error): autogrow ports (``_check_autogrow_required``) and the dynamic + types COMFY_DYNAMICCOMBO / COMFY_DYNAMICSLOT (DynamicSlot is always optional + server-side anyway). The server has NO exemption for required inputs that + carry a default — the frontend always serializes widget values, so absence + is a genuine authoring error; we do not skip ports with defaults. + """ + present = node_data.get("inputs") or {} + errors: list[dict] = [] + for port in m.inputs: + if not port.required or port.is_autogrow: + continue + if port.type.startswith("COMFY_DYNAMICCOMBO") or port.type.startswith("COMFY_DYNAMICSLOT"): + continue + if port.name in present: + continue + errors.append( + { + "node_id": node_id, + "field": port.name, + "code": "required_input_missing", + "message": ( + f"required input {port.name!r} is missing — " + f"the server will reject this node (required_input_missing)" + ), + "hint": f"add {port.name!r} to inputs" + + ( + f" (e.g. a {port.type} value)" + if not port.is_link + else f" (wire a {port.type} link: [, ])" + ), + } + ) + return errors + + def _check_autogrow_required( node_id: str, autogrow_ports: dict[str, Port], autogrow_seen: set[str], node_data: dict ) -> list[dict]: diff --git a/tests/comfy_cli/command/test_run.py b/tests/comfy_cli/command/test_run.py index d1729aa1..69d49894 100644 --- a/tests/comfy_cli/command/test_run.py +++ b/tests/comfy_cli/command/test_run.py @@ -1046,7 +1046,9 @@ def test_ui_workflow_converts_and_submits(self, ui_workflow_file, fake_target): patch("comfy_cli.command.run.convert_ui_to_api", return_value=self.CONVERTED) as mock_convert, patch( "comfy_cli.cql.engine._load_from_target", - return_value={"KSampler": {}}, # any truthy dict suffices for the converter + # An output-node KSampler so preflight's no-outputs check passes + # (the converter is mocked and ignores object_info anyway). + return_value={"KSampler": {"output_node": True}}, ), patch("comfy_cli.comfy_client.Client", return_value=mock_client), patch("comfy_cli.command.run._spawn_watcher"), diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index dc26e677..a98228da 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -185,6 +185,17 @@ def graph() -> Graph: return Graph.from_object_info(_object_info()) +@pytest.fixture +def graph_sd15() -> Graph: + """Graph built from the real captured sd15 object_info fixture — the same + catalog the BE-3349 repro / BE-3357 acceptance criterion runs against.""" + import json + from pathlib import Path + + fixture = Path(__file__).parent.parent / "fixtures" / "sd15_object_info.json" + return Graph.from_object_info(json.loads(fixture.read_text())) + + # --------------------------------------------------------------------------- # Direct-mode workflow fixture # --------------------------------------------------------------------------- @@ -356,7 +367,16 @@ def test_find_paths_unreachable_returns_empty(self, graph: Graph): class TestValidateWorkflow: """Tests graph.validate_workflow(api_workflow).""" + @staticmethod + def _errors_excluding_no_outputs(result: dict) -> list[dict]: + """Errors other than the workflow-level no-outputs check — for + single-node fixtures that (deliberately) carry no output node.""" + return [e for e in result["errors"] if e.get("code") != "prompt_no_outputs"] + def _valid_workflow(self) -> dict: + # A complete, server-valid pipeline: every required input present and a + # SaveImage output node (so it passes the required-presence and + # no-outputs checks, not just the edge/shape checks). return { "1": { "class_type": "CheckpointLoaderSimple", @@ -389,6 +409,14 @@ def _valid_workflow(self) -> dict: "denoise": 1.0, }, }, + "6": { + "class_type": "VAEDecode", + "inputs": {"samples": ["2", 0], "vae": ["1", 2]}, + }, + "7": { + "class_type": "SaveImage", + "inputs": {"images": ["6", 0], "filename_prefix": "out"}, + }, } def test_valid_workflow(self, graph: Graph): @@ -398,15 +426,9 @@ def test_valid_workflow(self, graph: Graph): def test_non_node_key_warns(self, graph: Graph): """An unrecognized non-node key should produce a warning, not an error.""" - wf = { - "notanode": {"title": "My Workflow"}, - "1": { - "class_type": "CheckpointLoaderSimple", - "inputs": {"ckpt_name": "sd_xl_base.safetensors"}, - }, - } + wf = {**self._valid_workflow(), "notanode": {"title": "My Workflow"}} result = graph.validate_workflow(wf) - assert result["valid"] is True + assert result["valid"] is True, result["errors"] non_node = [w for w in result["warnings"] if w["code"] == "non_node_key"] assert len(non_node) == 1 assert non_node[0]["node_id"] == "notanode" @@ -415,28 +437,16 @@ def test_non_node_key_warns(self, graph: Graph): def test_meta_provenance_key_is_not_warned(self, graph: Graph): """`_meta` is the compose/run provenance block (stripped before submit), not a stray key — validating composed output must not nag about it.""" - wf = { - "_meta": {"schema": "compose/1", "blueprint": "blueprints/x.yaml"}, - "1": { - "class_type": "CheckpointLoaderSimple", - "inputs": {"ckpt_name": "sd_xl_base.safetensors"}, - }, - } + wf = {"_meta": {"schema": "compose/1", "blueprint": "blueprints/x.yaml"}, **self._valid_workflow()} result = graph.validate_workflow(wf) - assert result["valid"] is True + assert result["valid"] is True, result["errors"] assert [w for w in result["warnings"] if w["node_id"] == "_meta"] == [] def test_non_dict_node_value_warns(self, graph: Graph): """A string value for a key should warn, not crash.""" - wf = { - "_comment": "this is a comment", - "1": { - "class_type": "CheckpointLoaderSimple", - "inputs": {"ckpt_name": "sd_xl_base.safetensors"}, - }, - } + wf = {**self._valid_workflow(), "_comment": "this is a comment"} result = graph.validate_workflow(wf) - assert result["valid"] is True + assert result["valid"] is True, result["errors"] non_node = [w for w in result["warnings"] if w["code"] == "non_node_key"] assert len(non_node) == 1 assert non_node[0]["node_id"] == "_comment" @@ -491,25 +501,7 @@ def test_unknown_enum_value(self, graph: Graph): def test_valid_edges_pass(self, graph: Graph): """Well-wired edges don't produce errors.""" - wf = { - "1": { - "class_type": "CheckpointLoaderSimple", - "inputs": {"ckpt_name": "sd_xl_base.safetensors"}, - }, - "2": { - "class_type": "KSampler", - "inputs": { - "model": ["1", 0], # MODEL from CheckpointLoaderSimple[0] - "seed": 42, - "steps": 20, - "cfg": 8.0, - "sampler_name": "euler", - "scheduler": "normal", - "denoise": 1.0, - }, - }, - } - result = graph.validate_workflow(wf) + result = graph.validate_workflow(self._valid_workflow()) assert result["valid"] is True assert result["errors"] == [] @@ -552,20 +544,13 @@ def test_edge_type_mismatch(self, graph: Graph): This is advisory (warning, not error) — ComfyUI allows cross-type wiring via reroutes and converters; the server is the authority.""" - wf = { - "1": { - "class_type": "CheckpointLoaderSimple", - "inputs": {"ckpt_name": "sd_xl_base.safetensors"}, - }, - "2": { - "class_type": "KSampler", - # Output index 1 is CLIP, but model input expects MODEL - "inputs": {"model": ["1", 1]}, - }, - } + wf = self._valid_workflow() + # Output index 1 is CLIP, but the model input expects MODEL — still a + # present input, so only an advisory warning (not a hard error). + wf["2"]["inputs"]["model"] = ["1", 1] result = graph.validate_workflow(wf) # edge_type_mismatch is a warning, not a hard error - assert result["valid"] is True + assert result["valid"] is True, result["errors"] warns = [w for w in result["warnings"] if w["code"] == "edge_type_mismatch"] assert len(warns) == 1 assert "CLIP" in warns[0]["message"] @@ -581,8 +566,9 @@ def test_int_valued_combo_accepts_int(self, graph: Graph): }, } result = graph.validate_workflow(wf) - assert result["valid"] is True - assert result["errors"] == [] + # This single-node fixture has no output node, so the only error is the + # workflow-level no-outputs one; the int-valued combo itself is clean. + assert self._errors_excluding_no_outputs(result) == [] def test_int_valued_combo_unknown_option_is_enum_error(self, graph: Graph): """An int outside the combo's options is an unknown_enum_value (same as @@ -597,7 +583,7 @@ def test_int_valued_combo_unknown_option_is_enum_error(self, graph: Graph): result = graph.validate_workflow(wf) assert result["valid"] is False # 7 is not in [6, 8, 10, 12]; it's an enum error, not a shape error. - errs = [e for e in result["errors"] if e["field"] == "duration"] + errs = [e for e in result["errors"] if e.get("field") == "duration"] assert len(errs) == 1 assert errs[0]["code"] == "unknown_enum_value" @@ -664,7 +650,8 @@ def test_int_combo_accepts_string_form_leniently(self, graph: Graph): }, } result = graph.validate_workflow(wf) - assert result["valid"] is True + # Lenient on type (no combo error); the only error is the no-outputs one. + assert self._errors_excluding_no_outputs(result) == [] def test_wildcard_type_compatible(self, graph: Graph): """'*' type on either side should not trigger a mismatch.""" @@ -715,28 +702,37 @@ def test_multiple_edge_errors_reported(self, graph: Graph): dangling = [e for e in result["errors"] if e["code"] == "dangling_edge"] assert len(dangling) == 3 - def test_below_min_warning(self, graph: Graph): + def test_below_min_error(self, graph: Graph): + """A value below the catalog min is a hard error (the server rejects it + with value_smaller_than_min) — was a warning before BE-3357.""" wf = { "1": { - "class_type": "KSampler", - "inputs": {"steps": 0}, + "class_type": "EmptyLatentImage", + "inputs": {"width": 0, "height": 512, "batch_size": 1}, }, } result = graph.validate_workflow(wf) - # below_min is a warning, not an error - codes = [w["code"] for w in result["warnings"]] - assert "below_min" in codes + assert result["valid"] is False + errs = [e for e in result["errors"] if e["code"] == "below_min"] + assert len(errs) == 1 + assert errs[0]["field"] == "width" + # No longer surfaced as a warning. + assert "below_min" not in [w["code"] for w in result["warnings"]] - def test_above_max_warning(self, graph: Graph): + def test_above_max_error(self, graph: Graph): + """A value above the catalog max is a hard error (value_bigger_than_max).""" wf = { "1": { - "class_type": "KSampler", - "inputs": {"steps": 99999}, + "class_type": "EmptyLatentImage", + "inputs": {"width": 999999, "height": 512, "batch_size": 1}, }, } result = graph.validate_workflow(wf) - codes = [w["code"] for w in result["warnings"]] - assert "above_max" in codes + assert result["valid"] is False + errs = [e for e in result["errors"] if e["code"] == "above_max"] + assert len(errs) == 1 + assert errs[0]["field"] == "width" + assert "above_max" not in [w["code"] for w in result["warnings"]] class TestAutogrowInputs: @@ -752,12 +748,19 @@ def _loaders(self) -> dict: } def test_dotted_slots_validate_clean(self, graph: Graph): + # A fully server-valid workflow: two IMAGE producers with all their + # required inputs wired, autogrown into BatchImagesNode, terminating in + # a SaveImage output node. wf = { - **self._loaders(), + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "sd_xl_base.safetensors"}}, + "2": {"class_type": "EmptyLatentImage", "inputs": {"width": 512, "height": 512, "batch_size": 1}}, + "10": {"class_type": "VAEDecode", "inputs": {"samples": ["2", 0], "vae": ["1", 2]}}, + "11": {"class_type": "VAEDecode", "inputs": {"samples": ["2", 0], "vae": ["1", 2]}}, "20": { "class_type": "BatchImagesNode", "inputs": {"images.image0": ["10", 0], "images.image1": ["11", 0]}, }, + "30": {"class_type": "SaveImage", "inputs": {"images": ["20", 0], "filename_prefix": "out"}}, } result = graph.validate_workflow(wf) assert result["valid"] is True, result["errors"] @@ -809,6 +812,162 @@ def test_describe_marks_autogrow(self, graph: Graph): assert all("autogrow" not in i for i in ks["inputs"]) +# =========================================================================== +# TestValidateServerParity — BE-3357: presence, no-outputs, range = errors +# =========================================================================== + + +class TestValidateServerParity: + """Validate mirrors the three server-side rejections that `validate` used to + pass silently (BE-3349 / BE-3357), against the captured sd15 catalog: + required-input presence, the no-outputs check, and range violations.""" + + def _sd15_full(self) -> dict: + """A complete, server-valid sd15 txt2img graph (SaveImage output, every + required input present).""" + return { + "4": { + "class_type": "CheckpointLoaderSimple", + "inputs": {"ckpt_name": "v1-5-pruned-emaonly-fp16.safetensors"}, + }, + "6": {"class_type": "CLIPTextEncode", "inputs": {"clip": ["4", 1], "text": "a cat"}}, + "7": {"class_type": "CLIPTextEncode", "inputs": {"clip": ["4", 1], "text": "blurry"}}, + "5": {"class_type": "EmptyLatentImage", "inputs": {"width": 512, "height": 512, "batch_size": 1}}, + "3": { + "class_type": "KSampler", + "inputs": { + "model": ["4", 0], + "positive": ["6", 0], + "negative": ["7", 0], + "latent_image": ["5", 0], + "seed": 42, + "steps": 20, + "cfg": 8.0, + "sampler_name": "euler", + "scheduler": "simple", + "denoise": 1.0, + }, + }, + "8": {"class_type": "VAEDecode", "inputs": {"samples": ["3", 0], "vae": ["4", 2]}}, + "9": {"class_type": "SaveImage", "inputs": {"images": ["8", 0], "filename_prefix": "ComfyUI"}}, + } + + def test_full_workflow_is_valid(self, graph_sd15: Graph): + """Regression guard: a KSampler with all 10 required inputs present, in a + graph with an output node, validates clean.""" + result = graph_sd15.validate_workflow(self._sd15_full()) + assert result["valid"] is True, result["errors"] + assert result["errors"] == [] + + def test_missing_widget_inputs_each_error(self, graph_sd15: Graph): + """KSampler missing `seed`/`steps` → one required_input_missing per + missing input, `field` set to the input name.""" + wf = self._sd15_full() + del wf["3"]["inputs"]["seed"] + del wf["3"]["inputs"]["steps"] + result = graph_sd15.validate_workflow(wf) + assert result["valid"] is False + missing = [e for e in result["errors"] if e["code"] == "required_input_missing"] + assert {e["field"] for e in missing} == {"seed", "steps"} + assert len(missing) == 2 + + def test_missing_required_link_errors(self, graph_sd15: Graph): + """A missing required *link* input (`model`) is also required_input_missing, + and its hint tells you to wire a link.""" + wf = self._sd15_full() + del wf["3"]["inputs"]["model"] + result = graph_sd15.validate_workflow(wf) + assert result["valid"] is False + err = next(e for e in result["errors"] if e["code"] == "required_input_missing" and e["field"] == "model") + assert "wire" in err["hint"] and "MODEL" in err["hint"] + + def test_be3349_repro_only_links_wired(self, graph_sd15: Graph): + """The BE-3349 acceptance case: a KSampler with only its four link inputs + wired is missing all six widget inputs → six required_input_missing errors.""" + wf = self._sd15_full() + wf["3"]["inputs"] = { + "model": ["4", 0], + "positive": ["6", 0], + "negative": ["7", 0], + "latent_image": ["5", 0], + } + result = graph_sd15.validate_workflow(wf) + assert result["valid"] is False + missing = [e for e in result["errors"] if e["code"] == "required_input_missing" and e["node_id"] == "3"] + assert {e["field"] for e in missing} == {"seed", "steps", "cfg", "sampler_name", "scheduler", "denoise"} + assert len(missing) == 6 + + def test_optional_inputs_absent_no_error(self): + """A required input that is absent errors, but an *optional* input that is + absent does not.""" + object_info = { + "OptNode": { + "input": { + "required": {"needed": ["STRING", {}]}, + "optional": {"maybe": ["STRING", {}]}, + }, + "output": [], + "output_name": [], + "output_node": True, + "python_module": "nodes", + }, + } + g = Graph.from_object_info(object_info) + # Optional absent, required present → clean. + clean = g.validate_workflow({"1": {"class_type": "OptNode", "inputs": {"needed": "x"}}}) + assert clean["valid"] is True, clean["errors"] + # Required absent → error; the optional one is never flagged. + missing = g.validate_workflow({"1": {"class_type": "OptNode", "inputs": {}}}) + codes = {(e["field"], e["code"]) for e in missing["errors"]} + assert ("needed", "required_input_missing") in codes + assert not any(e["field"] == "maybe" for e in missing["errors"]) + + def test_no_output_node_errors(self, graph_sd15: Graph): + """A workflow of recognized nodes with no output node is rejected + (prompt_no_outputs); adding SaveImage clears it.""" + wf = self._sd15_full() + del wf["9"] # remove the only output node (SaveImage) + result = graph_sd15.validate_workflow(wf) + no_out = [e for e in result["errors"] if e["code"] == "prompt_no_outputs"] + assert len(no_out) == 1 + + # With SaveImage present, no such error. + result2 = graph_sd15.validate_workflow(self._sd15_full()) + assert [e for e in result2["errors"] if e["code"] == "prompt_no_outputs"] == [] + + def test_no_output_error_emitted_once(self, graph_sd15: Graph): + """The no-outputs error is appended once, not per node.""" + wf = self._sd15_full() + del wf["9"] + result = graph_sd15.validate_workflow(wf) + assert len([e for e in result["errors"] if e["code"] == "prompt_no_outputs"]) == 1 + + def test_all_unknown_nodes_no_false_no_outputs(self, graph_sd15: Graph): + """With zero *recognized* nodes there's nothing to submit, so we don't + pile a no-outputs error on top of the unknown-class errors.""" + result = graph_sd15.validate_workflow({"1": {"class_type": "TotallyMadeUp", "inputs": {}}}) + assert [e for e in result["errors"] if e["code"] == "prompt_no_outputs"] == [] + + def test_width_below_min_is_error(self, graph_sd15: Graph): + """EmptyLatentImage width below the catalog min is a hard error (was a + warning before BE-3357).""" + wf = self._sd15_full() + wf["5"]["inputs"]["width"] = 1 # sd15 min is 16 + result = graph_sd15.validate_workflow(wf) + assert result["valid"] is False + errs = [e for e in result["errors"] if e["code"] == "below_min" and e["field"] == "width"] + assert len(errs) == 1 + assert "16" in errs[0]["hint"] + + def test_meta_key_still_exempt(self, graph_sd15: Graph): + """`_meta` provenance is still ignored — not counted as a node, never a + required/no-outputs trigger.""" + wf = {"_meta": {"schema": "compose/1"}, **self._sd15_full()} + result = graph_sd15.validate_workflow(wf) + assert result["valid"] is True, result["errors"] + assert [e for e in result["errors"] if e.get("node_id") == "_meta"] == [] + + # =========================================================================== # TestDirectModeSlots # =========================================================================== From 187192fe86a262863237ca5e28934faa9b353337 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 17 Jul 2026 14:44:37 -0700 Subject: [PATCH 2/2] fix(validate): flag empty/output-less prompts, dedupe no-outputs w/ unknown-class (BE-3357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address cursor-review findings on the no-outputs check: - Empty or node-less prompts ({} / _meta-only) now surface prompt_no_outputs instead of returning valid:true — the server rejects any zero-output prompt. Gate on has_output_node, not any_recognized. - Suppress prompt_no_outputs when an unknown_class_type error is present: that node could be the real (custom) output node, so the missing-output message was misleading noise stacked on the unknown-class error. - prompt_no_outputs now carries node_id/field (None) for schema consistency with every other error entry; renderers coerce None node_id to "?". Co-Authored-By: Claude Opus 4.8 --- comfy_cli/cmdline.py | 2 +- comfy_cli/command/run/preflight.py | 2 +- comfy_cli/cql/engine.py | 22 +++++++++++++------- tests/comfy_cli/cql/test_engine.py | 33 ++++++++++++++++++++++++++++-- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 4662b788..971fde64 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -995,7 +995,7 @@ def validate( suggestions = e.get("suggestions", []) if suggestions: msg += f" (did you mean: {', '.join(suggestions[:3])}?)" - rprint(f" [red]•[/red] node {e.get('node_id', '?')}: {msg}") + rprint(f" [red]•[/red] node {e.get('node_id') or '?'}: {msg}") for w in result["warnings"]: rprint(f" [yellow]⚠[/yellow] {w.get('message', '')}") renderer.emit(payload, command="validate", ok=result["valid"]) diff --git a/comfy_cli/command/run/preflight.py b/comfy_cli/command/run/preflight.py index 36fcf737..17923c09 100644 --- a/comfy_cli/command/run/preflight.py +++ b/comfy_cli/command/run/preflight.py @@ -92,7 +92,7 @@ def _preflight_validate(renderer, workflow: dict, object_info: dict, *, target_l errors = validation.get("errors", []) hint_parts = [] for e in errors[:5]: - line = f"node {e.get('node_id', '?')}: {e.get('message', '')}" + line = f"node {e.get('node_id') or '?'}: {e.get('message', '')}" suggestions = e.get("suggestions") or [] if suggestions: line += f" (did you mean: {', '.join(suggestions)}?)" diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index f7c53d0c..0af9338f 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -700,9 +700,8 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: warnings: list[dict] = [] all_names = list(self._nodes.keys()) # No-outputs check: the server rejects any prompt with zero output nodes - # (execution.py:1155-1162, prompt_no_outputs). Track whether we saw any - # recognized node and whether any was an output node. - any_recognized = False + # (execution.py:1155-1162, prompt_no_outputs). Track whether any + # recognized node is an output node. has_output_node = False for node_id, node_data in workflow.items(): @@ -749,7 +748,6 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: ) continue - any_recognized = True if m.is_output_node: has_output_node = True @@ -878,11 +876,21 @@ def validate_workflow(self, workflow: dict[str, Any]) -> dict[str, Any]: errors.extend(_check_autogrow_required(node_id, autogrow_ports, autogrow_seen, node_data)) errors.extend(_check_required_present(node_id, m, node_data)) - # No-outputs check: a prompt with recognized nodes but zero output nodes - # is rejected server-side (execution.py:1155-1162, prompt_no_outputs). - if any_recognized and not has_output_node: + # No-outputs check: the server rejects any prompt with zero output + # nodes (execution.py:1155-1162, prompt_no_outputs) — including an + # empty/node-less prompt. Suppress it only when an unknown_class_type + # error is present: that node could be the real (custom) output node we + # just can't see, so the missing-output message would be misleading + # noise on top of the unknown-class error the user must resolve first. + has_unknown_class = any(e.get("code") == "unknown_class_type" for e in errors) + if not has_output_node and not has_unknown_class: errors.append( { + # workflow-level error: no owning node, hence None (keeps the + # node_id/field keys every other error carries, for schema + # consistency). + "node_id": None, + "field": None, "code": "prompt_no_outputs", "message": "workflow has no output nodes — the server will reject it (prompt_no_outputs)", "hint": "add an output node such as SaveImage/PreviewImage", diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index a98228da..b35adf4d 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -943,11 +943,40 @@ def test_no_output_error_emitted_once(self, graph_sd15: Graph): assert len([e for e in result["errors"] if e["code"] == "prompt_no_outputs"]) == 1 def test_all_unknown_nodes_no_false_no_outputs(self, graph_sd15: Graph): - """With zero *recognized* nodes there's nothing to submit, so we don't - pile a no-outputs error on top of the unknown-class errors.""" + """An unknown node could itself be the (custom) output node — we can't + see it — so we don't pile a no-outputs error on top of the + unknown-class errors the user must resolve first.""" result = graph_sd15.validate_workflow({"1": {"class_type": "TotallyMadeUp", "inputs": {}}}) assert [e for e in result["errors"] if e["code"] == "prompt_no_outputs"] == [] + def test_empty_workflow_is_no_outputs(self, graph_sd15: Graph): + """An empty prompt has zero output nodes, which the server rejects + (prompt_no_outputs); a node-less prompt must not slip through as valid.""" + result = graph_sd15.validate_workflow({}) + no_out = [e for e in result["errors"] if e["code"] == "prompt_no_outputs"] + assert len(no_out) == 1 + assert result["valid"] is False + # workflow-level error still carries the node_id/field schema keys. + assert no_out[0]["node_id"] is None + assert no_out[0]["field"] is None + + def test_meta_only_workflow_is_no_outputs(self, graph_sd15: Graph): + """A prompt that is only a `_meta` block (no nodes) has no outputs.""" + result = graph_sd15.validate_workflow({"_meta": {"schema": "x"}}) + assert len([e for e in result["errors"] if e["code"] == "prompt_no_outputs"]) == 1 + + def test_unknown_output_node_no_double_no_outputs(self, graph_sd15: Graph): + """A recognized non-output node plus an unknown node (which could be the + real output) must not stack prompt_no_outputs on the unknown-class + error — the fix is installing the custom node, not adding an output.""" + wf = { + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "v1-5.safetensors"}}, + "2": {"class_type": "MyCustomSaver", "inputs": {"images": ["1", 0]}}, + } + result = graph_sd15.validate_workflow(wf) + assert any(e["code"] == "unknown_class_type" for e in result["errors"]) + assert [e for e in result["errors"] if e["code"] == "prompt_no_outputs"] == [] + def test_width_below_min_is_error(self, graph_sd15: Graph): """EmptyLatentImage width below the catalog min is a hard error (was a warning before BE-3357)."""