From 6723141d2b440233c8efdc0f9c2cbfccc5157466 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Sun, 9 Aug 2026 00:32:28 -0700 Subject: [PATCH 1/5] fix(nodes): constrain `nodes path` by source type and stop overclaiming exactness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `comfy nodes path ` ignored the source-type constraint entirely and still labelled its answer `"exact": true`. The exact walker seeded an `available` set with `from_type` and then enumerated every node whose *required link inputs* were satisfiable — which is trivially true for any loader or text-to-X API node — so it really enumerated things that produce `TO`, alphabetically. `AUDIO -> IMAGE` returned byte-for-byte the same rows as `MODEL -> IMAGE`, every step carried `"from_type": ""`, and a node with a COMBO widget merely *named* `model` was routed through as if it consumed MODEL. Replace both walkers with one `Graph.search_paths`: - Traversal now walks the `_consumers` index, which is keyed on declared link inputs, so a step is only taken through an input whose *type* matches what the previous step produced. Widget-named lookalikes can never be routed through. - Each step reports the type it actually consumes, so `from_type` is populated. - `max_depth` bounds path length, and a frontier still expanding at the bound is reported as `depth_limited` rather than silently cut. - A node's *other* required inputs are satisfied from a fixpoint closure of types obtainable without wiring anything in, and reported per path under `support` instead of being spliced into `steps` as bogus hops. - The result carries `truncated` / `truncated_by` / `depth_limited` / `collapsed`, and `exact` is now the honest claim that the listing is the complete, type-constrained answer — withheld whenever any bound was hit. The `--exact/--loose` flag is echoed separately as `mode`. `collapsed` covers the subtler version of the same overclaim: the walk explores each intermediate state once, so a second node offering the same hop is not re-expanded and its chains never reach the output. That is a real gap in the listing, so it is reported rather than hidden behind `exact`. The empty `AUDIO -> IMAGE` result is a fact about the catalog, never a hard-coded denial — current ComfyUI ships `VAEEncodeAudio` (AUDIO + VAE -> LATENT), and with that node present the walker returns the real two-hop route through `VAEDecode`. Both directions are pinned by tests driven off a recorded `object_info` fixture. --- comfy_cli/command/nodes.py | 34 +- comfy_cli/cql/engine.py | 239 +++-- .../command/test_nodes_introspect.py | 100 ++ tests/comfy_cli/cql/test_engine.py | 200 ++++ .../fixtures/nodes_path_object_info.json | 888 ++++++++++++++++++ 5 files changed, 1395 insertions(+), 66 deletions(-) create mode 100644 tests/comfy_cli/fixtures/nodes_path_object_info.json diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index cd0de649b..71cfa3fcd 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -734,7 +734,7 @@ def path_cmd( bool, typer.Option( "--exact/--loose", - help="Exact: every step's required link inputs must be satisfiable from the path so far. Loose: any routed sequence.", + help="Exact: every step's other required link inputs must be satisfiable (reported per path as 'support'). Loose: any routed sequence.", ), ] = True, input_path: Annotated[str | None, typer.Option("--input", show_default=False)] = None, @@ -755,13 +755,26 @@ def path_cmd( on_stale=lambda key, err: _stale.update(stale=True, source=key, reason=err), ) - finder = graph.exact_paths if exact else graph.find_paths - paths = finder(from_type, to_type, max_depth=max_depth, max_paths=max_paths) + result = graph.search_paths(from_type, to_type, exact=exact, max_depth=max_depth, max_paths=max_paths) + paths = result["paths"] + truncated = bool(result["truncated"]) + depth_limited = bool(result["depth_limited"]) + collapsed = bool(result["collapsed"]) payload = { "from": from_type, "to": to_type, - "exact": exact, + "mode": "exact" if exact else "loose", + # Not the flag echoed back: the honest claim that these paths are the + # complete, type-constrained answer. Any early stop (max_paths, the + # internal state budget), a frontier still expanding at max_depth, or an + # intermediate state reached by a second route that was not re-explored + # means paths may be missing, so the claim is withheld. + "exact": bool(exact and not truncated and not depth_limited and not collapsed), + "truncated": truncated, + "truncated_by": result["truncated_by"], + "depth_limited": depth_limited, + "collapsed": collapsed, "max_depth": max_depth, "max_paths": max_paths, "count": len(paths), @@ -777,6 +790,7 @@ def path_cmd( } for s in (p.get("steps") or []) ], + "support": list(p.get("support") or []), } for p in paths ], @@ -805,7 +819,19 @@ def path_cmd( f"[cyan]{sanitize_markup(p.get('from'))}[/cyan] {chain} " f"[cyan]{sanitize_markup(p.get('to'))}[/cyan]" ) + needs = ", ".join( + f"{sanitize_markup(s.get('type'))} from {sanitize_markup(s.get('node'))}" + for s in (p.get("support") or []) + ) + if needs: + rprint(f" [dim]also needs: {needs}[/dim]") rprint(f"[dim]{len(paths)} path(s)[/dim]") + if truncated: + rprint(f"[dim]Partial result — stopped at {payload['truncated_by']}; more paths may exist.[/dim]") + elif depth_limited: + rprint(f"[dim]Searched to depth {max_depth}; longer paths were not explored.[/dim]") + elif collapsed: + rprint("[dim]Equivalent alternate routes were collapsed; this is a sample, not every path.[/dim]") renderer.emit(payload, command="nodes path") diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 72072623d..2b3fcd5ba 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -30,6 +30,12 @@ _IMPLICIT_WIDGET_TYPES = frozenset({"STRING", "INT", "FLOAT", "NUMBER", "BOOLEAN", "COMBO"}) +# Work budget for ``Graph.search_paths``: the number of frontier states it will +# expand before giving up and reporting ``truncated``. A full cloud catalog has +# thousands of nodes, so an unreachable target must fail fast rather than walk +# the whole type lattice. +_MAX_PATH_SEARCH_STATES = 20_000 + @dataclass class PortOptions: @@ -485,6 +491,9 @@ def __init__(self) -> None: self._consumers: dict[str, list[Morphism]] = defaultdict(list) self._types: set[str] = set() self._annotated = False + # Lazily-computed closure of types obtainable without wiring anything in + # (see ``free_types``). Invalidated implicitly: graphs are built once. + self._free_types: frozenset[str] | None = None # The raw ``/object_info`` payload this graph was built from. Retained # verbatim so callers that also need to lower a UI-format workflow to # API format (``convert_ui_to_api``) can reuse it without a second fetch. @@ -606,42 +615,150 @@ def packs(self) -> list[str]: """All known pack names, sorted.""" return sorted(set(m.pack for m in self._nodes.values() if m.pack)) - def find_paths( + def free_types(self) -> frozenset[str]: + """Types obtainable without wiring anything in — the fixpoint closure + over nodes whose required link inputs are already satisfied (loaders, + primitives, text-to-X API nodes, and whatever those unlock). + + The exact walker uses this to decide whether a step's *other* required + inputs (a ``VAE`` for ``VAEDecode``, say) could be supplied by a support + node. Support nodes are reported per path rather than routed through, so + they never masquerade as steps on the requested path. + """ + if self._free_types is None: + free: set[str] = set() + changed = True + while changed: + changed = False + for m in self._nodes.values(): + if not m.can_apply(free): + continue + for t in m.output_types(): + if t != "*" and t not in free: + free.add(t) + changed = True + self._free_types = frozenset(free) + return self._free_types + + def search_paths( self, from_type: str, to_type: str, *, - max_depth: int = 4, + exact: bool = True, + max_depth: int = 6, max_paths: int = 10, - ) -> list[dict]: - """BFS multi-hop path finding from one type to another.""" - if from_type == to_type: - return [] - # queue items: (current_type, steps[]) - queue: list[tuple[str, list[dict]]] = [(from_type, [])] - visited: set[str] = {from_type} - paths: list[dict] = [] + max_states: int = _MAX_PATH_SEARCH_STATES, + ) -> dict: + """Routed paths from ``from_type`` to ``to_type``, with honest bounds. + + Every step consumes the type the previous step produced — the first step + consumes ``from_type`` — through a **declared link input of that type**, + so a node that merely owns a widget *named* like the type (the COMBO + ``model`` on the partner-API image nodes) is never routed through. Path + length (the number of steps) is bounded by ``max_depth``. + + In ``exact`` mode a step is only taken when the node's other required + link inputs are satisfiable — from types produced earlier on the path, or + from a support node needing no wiring of its own (``free_types``). Those + support inputs are reported per path under ``support`` instead of being + spliced into ``steps``. Loose mode skips the satisfiability check and + reports no support. + + Returns ``{"paths", "truncated", "truncated_by", "depth_limited", + "collapsed"}``: + + - ``truncated`` — the walk stopped early (``max_paths`` reached, or the + internal state budget exhausted), so paths exist that are not listed. + - ``depth_limited`` — the frontier was still expanding at ``max_depth``, + so longer paths may exist beyond the requested bound. + - ``collapsed`` — the walk reached some intermediate state by more than + one route and explored it only once, so alternate chains through that + state are not listed. Reachability is unaffected (the surviving route + explores exactly the same continuations), which is why an **empty** + result with all three flags false is a proof that no path exists — + but a non-empty one is a sample of the routes, not the full set. + + A caller may only treat the listing as exhaustive when all three are + false. Each errs toward true: hitting ``max_paths`` exactly is reported + as truncated even when nothing further existed, and a revisited state is + reported as collapsed even when its alternate route led nowhere. + """ + result: dict = { + "paths": [], + "truncated": False, + "truncated_by": None, + "depth_limited": False, + "collapsed": False, + } + if from_type == to_type or max_depth < 1 or max_paths < 1: + return result + + free = self.free_types() if exact else frozenset() + paths: list[dict] = result["paths"] + # state: (current_type, types produced by the path so far, steps[]) + queue: list[tuple[str, frozenset[str], list[dict]]] = [(from_type, frozenset(), [])] + visited: set[tuple[str, frozenset[str]]] = {(from_type, frozenset())} + states = 0 while queue and len(paths) < max_paths: - next_queue: list[tuple[str, list[dict]]] = [] - for cur_type, steps in queue: + next_queue: list[tuple[str, frozenset[str], list[dict]]] = [] + for cur_type, produced, steps in queue: + consumers = self._consumers.get(cur_type, []) if len(steps) >= max_depth: + if consumers: + result["depth_limited"] = True continue - for consumer in self._consumers.get(cur_type, []): - for out_t in consumer.output_types(): + available = free | produced | {from_type} + for consumer in consumers: + if exact and not consumer.can_apply(available): + continue + outs = [t for t in consumer.output_types() if t != "*"] + # Loose mode ignores availability, so keeping ``produced`` + # empty there collapses the visited key back to the type + # alone — the pruning loose path-finding has always used. + new_produced = produced | frozenset(outs) if exact else produced + for out_t in outs: if out_t == cur_type: continue step = {"node": consumer.id, "input_type": cur_type, "output_type": out_t} new_steps = steps + [step] if out_t == to_type: - paths.append({"from": from_type, "to": to_type, "steps": new_steps}) + paths.append(self._path_record(from_type, to_type, new_steps, free if exact else None)) if len(paths) >= max_paths: - return paths - elif out_t not in visited and len(new_steps) < max_depth: - visited.add(out_t) - next_queue.append((out_t, new_steps)) + result["truncated"] = True + result["truncated_by"] = "max_paths" + return result + continue + key = (out_t, new_produced) + if key in visited: + # A second route into a state already queued. Its + # continuations are covered by the first one, so + # dropping it costs no reachability — but the chains + # it would have printed are lost, so the listing can + # no longer be called complete. + result["collapsed"] = True + continue + if states >= max_states: + result["truncated"] = True + result["truncated_by"] = "max_states" + return result + states += 1 + visited.add(key) + next_queue.append((out_t, new_produced, new_steps)) queue = next_queue - return paths + return result + + def find_paths( + self, + from_type: str, + to_type: str, + *, + max_depth: int = 4, + max_paths: int = 10, + ) -> list[dict]: + """Loose (routing-only) paths — see ``search_paths``.""" + return self.search_paths(from_type, to_type, exact=False, max_depth=max_depth, max_paths=max_paths)["paths"] def exact_paths( self, @@ -651,49 +768,47 @@ def exact_paths( max_depth: int = 6, max_paths: int = 10, ) -> list[dict]: - """Satisfiability-aware BFS: each step's required link inputs must be - available from types produced by prior steps.""" - if from_type == to_type: - return [] - # state: (available_types_frozenset, steps[]) - initial: frozenset[str] = frozenset({from_type}) - queue: list[tuple[frozenset[str], list[dict]]] = [(initial, [])] - visited: set[frozenset[str]] = {initial} - paths: list[dict] = [] - - while queue and len(paths) < max_paths: - next_queue: list[tuple[frozenset[str], list[dict]]] = [] - for available, steps in queue: - if len(steps) >= max_depth: + """Satisfiability-aware paths — see ``search_paths``.""" + return self.search_paths(from_type, to_type, exact=True, max_depth=max_depth, max_paths=max_paths)["paths"] + + def _path_record(self, from_type: str, to_type: str, steps: list[dict], free: frozenset[str] | None) -> dict: + record = {"from": from_type, "to": to_type, "steps": steps} + if free is not None: + record["support"] = self._support_for(from_type, steps, free) + return record + + def _support_for(self, from_type: str, steps: list[dict], free: frozenset[str]) -> list[dict]: + """Required link inputs a routed path needs *besides* the routed type, + each with a node that can supply it without wiring of its own.""" + available: set[str] = {from_type} + support: list[dict] = [] + seen: set[str] = set() + for step in steps: + m = self._nodes.get(step["node"]) + if m is None: + continue + for t in m.required_link_types(): + if t in available or t in seen: continue - for m in sorted(self._nodes.values(), key=lambda m: m.id): - if not m.can_apply(available): - continue - new_outs = [t for t in m.output_types() if t not in available and t != "*"] - if not new_outs: - continue - # Pick one representative input type this node consumes from available - input_type = "" - for t in m.required_link_types(): - if t in available: - input_type = t - break - for out_t in new_outs: - step = {"node": m.id, "input_type": input_type, "output_type": out_t} - new_steps = steps + [step] - new_avail = available | frozenset(new_outs) - if out_t == to_type: - # ``from_type`` seeds ``available`` and the set only - # grows, so every reachable path originates from it by - # construction — no extra consumption guard needed. - paths.append({"from": from_type, "to": to_type, "steps": new_steps}) - if len(paths) >= max_paths: - return paths - elif new_avail not in visited and len(new_steps) < max_depth: - visited.add(new_avail) - next_queue.append((new_avail, new_steps)) - queue = next_queue - return paths + seen.add(t) + support.append({"type": t, "node": self._free_producer(t, free)}) + available.update(m.output_types()) + return support + + def _free_producer(self, type_id: str, free: frozenset[str]) -> str | None: + """A node producing ``type_id`` that needs no incoming links, preferring + one with no link inputs at all. ``None`` when the type can only be + obtained by wiring something up first.""" + if type_id not in free: + return None + producers = self._producers.get(type_id, []) + for m in producers: + if not m.required_link_types(): + return m.id + for m in producers: + if m.can_apply(free): + return m.id + return None # -- Browse -- diff --git a/tests/comfy_cli/command/test_nodes_introspect.py b/tests/comfy_cli/command/test_nodes_introspect.py index 5bbe156ca..47151d6d5 100644 --- a/tests/comfy_cli/command/test_nodes_introspect.py +++ b/tests/comfy_cli/command/test_nodes_introspect.py @@ -458,6 +458,106 @@ def test_non_string_category_does_not_crash(self, monkeypatch, capsys): assert "KSampler" in [r["name"] for r in env["data"]["rows"]] +class TestPath: + """`comfy nodes path` — the envelope an agent plans a graph off (BE-6857).""" + + @pytest.fixture + def patched_loader(self, monkeypatch: pytest.MonkeyPatch): + import json as _json + from pathlib import Path + + from comfy_cli.cql.engine import Graph + + fixture = Path(__file__).parent.parent / "fixtures" / "nodes_path_object_info.json" + graph = Graph.from_object_info(_json.loads(fixture.read_text())) + monkeypatch.setattr(nodes_cmd, "_get_graph", lambda *a, **kw: graph) + + def test_reachable_route(self, patched_loader, capsys): + env = _run(["path", "MODEL", "IMAGE", "--max-depth", "4"], capsys) + data = env["data"] + assert data["mode"] == "exact" + assert data["count"] >= 1 + chains = [[s["node"] for s in p["steps"]] for p in data["paths"]] + assert ["KSampler", "VAEDecode"] in chains + first = data["paths"][0]["steps"][0] + assert first["from_type"] == "MODEL" + assert first["to_type"] == "LATENT" + + def test_unreachable_source_is_an_honest_empty_set(self, patched_loader, capsys): + env = _run(["path", "AUDIO", "IMAGE", "--max-depth", "4", "--max-paths", "3"], capsys) + data = env["data"] + assert data["count"] == 0 + assert data["paths"] == [] + # Nothing was cut short, so the empty answer is genuinely exhaustive. + assert data["truncated"] is False + assert data["depth_limited"] is False + assert data["exact"] is True + + def test_source_type_changes_the_rows(self, patched_loader, capsys): + model = _run(["path", "MODEL", "IMAGE", "--max-depth", "4", "--max-paths", "3"], capsys)["data"] + audio = _run(["path", "AUDIO", "IMAGE", "--max-depth", "4", "--max-paths", "3"], capsys)["data"] + assert model["paths"] != audio["paths"] + assert model["count"] > 0 and audio["count"] == 0 + + def test_partner_api_node_with_a_model_widget_is_not_routed_through(self, patched_loader, capsys): + env = _run(["path", "MODEL", "IMAGE", "--max-depth", "6"], capsys) + nodes = {s["node"] for p in env["data"]["paths"] for s in p["steps"]} + assert "ByteDanceImageNode" not in nodes + + def test_shallow_depth_is_a_subset_and_says_so(self, patched_loader, capsys): + shallow = _run(["path", "MODEL", "IMAGE", "--max-depth", "1"], capsys)["data"] + deep = _run(["path", "MODEL", "IMAGE", "--max-depth", "4"], capsys)["data"] + assert shallow["count"] < deep["count"] + for p in deep["paths"]: + assert len(p["steps"]) <= 4 + # An empty result from a search that stopped at the depth bound is not + # proof of unreachability, so `exact` is withheld. + assert shallow["depth_limited"] is True + assert shallow["exact"] is False + + def test_max_paths_truncation_withholds_the_exact_claim(self, patched_loader, capsys): + env = _run(["path", "LATENT", "IMAGE", "--max-depth", "4", "--max-paths", "1"], capsys)["data"] + assert env["count"] == 1 + assert env["truncated"] is True + assert env["truncated_by"] == "max_paths" + assert env["exact"] is False + + def test_loose_mode_never_claims_exactness(self, patched_loader, capsys): + env = _run(["path", "MODEL", "IMAGE", "--loose", "--max-depth", "4"], capsys)["data"] + assert env["mode"] == "loose" + assert env["exact"] is False + assert env["count"] >= 1 + + def test_support_inputs_are_reported(self, patched_loader, capsys): + env = _run(["path", "MODEL", "IMAGE", "--max-depth", "4"], capsys)["data"] + path = next(p for p in env["paths"] if [s["node"] for s in p["steps"]] == ["KSampler", "VAEDecode"]) + assert {s["type"] for s in path["support"]} == {"CONDITIONING", "LATENT", "VAE"} + + def test_collapsed_alternate_routes_withhold_the_exact_claim(self, monkeypatch, capsys): + """A second node offering the same hop is not re-expanded, so its routes + never reach the output. The envelope has to say so — a silently partial + list labelled `exact` is the defect this ticket is about.""" + import copy + import json as _json + from pathlib import Path + + from comfy_cli.cql.engine import Graph + + fixture = Path(__file__).parent.parent / "fixtures" / "nodes_path_object_info.json" + info = _json.loads(fixture.read_text()) + info["KSamplerAdvanced"] = copy.deepcopy(info["KSampler"]) + info["KSamplerAdvanced"]["name"] = "KSamplerAdvanced" + graph = Graph.from_object_info(info) + monkeypatch.setattr(nodes_cmd, "_get_graph", lambda *a, **kw: graph) + + data = _run(["path", "MODEL", "IMAGE", "--max-depth", "3"], capsys)["data"] + assert data["count"] > 0 + assert data["collapsed"] is True + assert data["truncated"] is False + assert data["depth_limited"] is False + assert data["exact"] is False + + class TestFlattenCategoryTree: """Pin the shape contract for the wasm CategoryTree, since the flattener has to know the (capital-cased) field names the Go side emits.""" diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index f8703d338..7386f4538 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -198,6 +198,19 @@ def graph_sd15() -> Graph: return Graph.from_object_info(json.loads(fixture.read_text())) +@pytest.fixture +def graph_path() -> Graph: + """Graph built from the captured path-search object_info fixture: the sd15 + core nodes, the audio nodes (AUDIO is consumed but never reaches IMAGE), a + second LATENT->IMAGE decoder, and the partner-API image node whose `model` + widget is a COMBO of API ids rather than a MODEL input (BE-6857).""" + import json + from pathlib import Path + + fixture = Path(__file__).parent.parent / "fixtures" / "nodes_path_object_info.json" + return Graph.from_object_info(json.loads(fixture.read_text())) + + # --------------------------------------------------------------------------- # Direct-mode workflow fixture # --------------------------------------------------------------------------- @@ -361,6 +374,193 @@ def test_find_paths_unreachable_returns_empty(self, graph: Graph): assert graph.find_paths("IMAGE", "MODEL") == [] +# =========================================================================== +# TestPathConstraints — BE-6857 +# =========================================================================== + + +def _node_chain(path: dict) -> tuple[str, ...]: + return tuple(s["node"] for s in path["steps"]) + + +class TestPathConstraints: + """`nodes path` used to enumerate anything that *produced* the target type, + ignoring the source type entirely: `AUDIO -> IMAGE` returned the same rows + as `MODEL -> IMAGE`, every step carried an empty `input_type`, and the + result was still labelled exact. These pin the source constraint, the depth + bound, and the honesty of the exhaustiveness claim. + """ + + def test_unreachable_source_type_returns_no_paths(self, graph_path: Graph): + # In THIS fixture AUDIO is consumed (SaveAudio, PreviewAudio) but never + # routed to IMAGE, so the correct answer is the empty set — not MODEL's + # rows. The emptiness is a fact about the catalog, never a hard-coded + # denial; the next test is the falsifier that pins that distinction. + assert graph_path.exact_paths("AUDIO", "IMAGE", max_depth=6) == [] + assert graph_path.find_paths("AUDIO", "IMAGE", max_depth=6) == [] + + def test_real_audio_to_image_route_is_found_when_the_catalog_has_one(self, graph_path: Graph): + """`AUDIO -> IMAGE` is NOT inherently impossible, and this walker must + never treat it that way. + + Current ComfyUI ships `VAEEncodeAudio` (AUDIO + VAE -> LATENT), which + reaches IMAGE through the ordinary `VAEDecode` hop. Add that real node + to the catalog and the route has to appear — with the VAE it also needs + reported as support rather than silently assumed. + """ + info = copy.deepcopy(graph_path.object_info) + # Faithful to comfy_extras/nodes_audio.py::VAEEncodeAudio. + info["VAEEncodeAudio"] = { + "input": {"required": {"audio": ["AUDIO", {}], "vae": ["VAE", {}]}}, + "input_order": {"required": ["audio", "vae"]}, + "output": ["LATENT"], + "output_is_list": [False], + "output_name": ["LATENT"], + "name": "VAEEncodeAudio", + "display_name": "VAE Encode Audio", + "description": "", + "category": "model/latent", + "python_module": "comfy_extras.nodes_audio", + "output_node": False, + "search_aliases": ["audio to latent"], + } + graph = Graph.from_object_info(info) + + paths = graph.exact_paths("AUDIO", "IMAGE", max_depth=6) + chains = {_node_chain(p) for p in paths} + assert ("VAEEncodeAudio", "VAEDecode") in chains + assert ("VAEEncodeAudio", "VAEDecodeTiled") in chains + # Every hop is a declared link of the type it claims to consume. + for p in paths: + assert p["steps"][0]["input_type"] == "AUDIO" + assert graph.node("VAEEncodeAudio").has_input("AUDIO") + # The VAE that VAEEncodeAudio also needs is surfaced, not assumed away. + route = next(p for p in paths if _node_chain(p) == ("VAEEncodeAudio", "VAEDecode")) + assert "VAE" in {s["type"] for s in route["support"]} + + def test_unknown_source_type_returns_no_paths(self, graph_path: Graph): + assert graph_path.exact_paths("NOT_A_TYPE", "IMAGE", max_depth=6) == [] + + def test_source_type_changes_the_answer(self, graph_path: Graph): + model = graph_path.exact_paths("MODEL", "IMAGE", max_depth=6) + audio = graph_path.exact_paths("AUDIO", "IMAGE", max_depth=6) + assert model, "MODEL -> IMAGE should still route through the sampler" + assert model != audio + + def test_first_step_consumes_the_declared_source_type(self, graph_path: Graph): + for from_type in ("MODEL", "LATENT", "CLIP", "CONDITIONING"): + for p in graph_path.exact_paths(from_type, "IMAGE", max_depth=6): + first = p["steps"][0] + assert first["input_type"] == from_type + assert graph_path.node(first["node"]).has_input(from_type) + + def test_every_step_declares_a_link_input_of_its_from_type(self, graph_path: Graph): + for p in graph_path.exact_paths("CLIP", "IMAGE", max_depth=6): + previous_out = "CLIP" + for step in p["steps"]: + node = graph_path.node(step["node"]) + assert step["input_type"] == previous_out + assert step["input_type"], "every step reports the type it consumes" + assert node.has_input(step["input_type"]) + assert node.has_output(step["output_type"]) + previous_out = step["output_type"] + + def test_widget_named_model_is_not_a_model_input(self, graph_path: Graph): + """ByteDanceImageNode produces IMAGE and has a *widget* named `model` + (a COMBO of API ids) — never a MODEL link input, so it is not a routing + step for MODEL, nor for any other type.""" + bytedance = graph_path.node("ByteDanceImageNode") + assert bytedance.has_output("IMAGE") + assert bytedance.input_link_types() == [] + for from_type in ("MODEL", "AUDIO", "CLIP", "LATENT"): + for p in graph_path.exact_paths(from_type, "IMAGE", max_depth=6): + assert "ByteDanceImageNode" not in _node_chain(p) + + def test_max_depth_bounds_path_length(self, graph_path: Graph): + for depth in range(1, 7): + for p in graph_path.exact_paths("CLIP", "IMAGE", max_depth=depth): + assert len(p["steps"]) <= depth + + def test_shallower_depth_is_a_subset(self, graph_path: Graph): + deep = {_node_chain(p) for p in graph_path.exact_paths("CLIP", "IMAGE", max_depth=6)} + assert deep + for depth in range(1, 6): + shallow = {_node_chain(p) for p in graph_path.exact_paths("CLIP", "IMAGE", max_depth=depth)} + assert shallow <= deep + # The reported case: depth 1 is a *strict* subset of depth 4. + shallow = {_node_chain(p) for p in graph_path.exact_paths("MODEL", "IMAGE", max_depth=1)} + deep = {_node_chain(p) for p in graph_path.exact_paths("MODEL", "IMAGE", max_depth=4)} + assert shallow < deep + + def test_support_nodes_cover_the_other_required_inputs(self, graph_path: Graph): + (path,) = [p for p in graph_path.exact_paths("MODEL", "IMAGE", max_depth=6) if "VAEDecode" in _node_chain(p)] + support = {s["type"]: s["node"] for s in path["support"]} + # KSampler needs conditioning + an initial latent, VAEDecode needs a VAE + assert support["CONDITIONING"] == "CLIPTextEncode" + assert support["LATENT"] == "EmptyLatentImage" + assert support["VAE"] == "CheckpointLoaderSimple" + # …and the routed type itself is never listed as support. + assert "MODEL" not in support + + def test_free_types_excludes_types_nothing_can_produce(self, graph_path: Graph): + free = graph_path.free_types() + assert {"MODEL", "LATENT", "IMAGE", "AUDIO"} <= free + assert "NOT_A_TYPE" not in free + + def test_exhausted_search_reports_no_truncation(self, graph_path: Graph): + result = graph_path.search_paths("AUDIO", "IMAGE", max_depth=6) + assert result["paths"] == [] + assert result["truncated"] is False + assert result["depth_limited"] is False + assert result["collapsed"] is False + + def test_collapsed_alternate_routes_are_reported(self, graph_path: Graph): + """The walk explores each intermediate state once, so a second node + offering the same hop is not re-expanded and the chains through it are + never printed. That is a real gap in the *listing*, so it has to be + reported — silently returning a subset while claiming exactness is the + bug this ticket is about, one level down. + """ + info = copy.deepcopy(graph_path.object_info) + # A second MODEL -> LATENT sampler: a genuine alternate first hop. + info["KSamplerAdvanced"] = copy.deepcopy(info["KSampler"]) + info["KSamplerAdvanced"]["name"] = "KSamplerAdvanced" + graph = Graph.from_object_info(info) + + result = graph.search_paths("MODEL", "IMAGE", max_depth=3) + chains = {_node_chain(p) for p in result["paths"]} + # Both decoders are reported off the surviving sampler... + assert chains == {("KSampler", "VAEDecode"), ("KSampler", "VAEDecodeTiled")} + # ...but KSamplerAdvanced's equally valid routes are not, so the result + # must not be advertised as the complete set. + assert result["collapsed"] is True + assert result["truncated"] is False + assert result["depth_limited"] is False + + def test_max_paths_is_reported_as_truncation(self, graph_path: Graph): + full = graph_path.search_paths("LATENT", "IMAGE", max_depth=6) + assert len(full["paths"]) > 1 and full["truncated"] is False + capped = graph_path.search_paths("LATENT", "IMAGE", max_depth=6, max_paths=1) + assert len(capped["paths"]) == 1 + assert capped["truncated"] is True + assert capped["truncated_by"] == "max_paths" + + def test_depth_cut_is_reported(self, graph_path: Graph): + result = graph_path.search_paths("MODEL", "IMAGE", max_depth=1) + assert result["paths"] == [] + assert result["depth_limited"] is True + + def test_state_budget_is_reported_as_truncation(self, graph_path: Graph): + result = graph_path.search_paths("CLIP", "IMAGE", max_depth=6, max_states=1) + assert result["truncated"] is True + assert result["truncated_by"] == "max_states" + + def test_degenerate_bounds_return_nothing(self, graph_path: Graph): + assert graph_path.search_paths("MODEL", "MODEL")["paths"] == [] + assert graph_path.search_paths("MODEL", "IMAGE", max_depth=0)["paths"] == [] + assert graph_path.search_paths("MODEL", "IMAGE", max_paths=0)["paths"] == [] + + # =========================================================================== # TestValidateWorkflow # =========================================================================== diff --git a/tests/comfy_cli/fixtures/nodes_path_object_info.json b/tests/comfy_cli/fixtures/nodes_path_object_info.json new file mode 100644 index 000000000..d591ab3b8 --- /dev/null +++ b/tests/comfy_cli/fixtures/nodes_path_object_info.json @@ -0,0 +1,888 @@ +{ + "CLIPTextEncode": { + "input": { + "required": { + "text": [ + "STRING", + { + "multiline": true, + "dynamicPrompts": true, + "tooltip": "The text to be encoded." + } + ], + "clip": [ + "CLIP", + { + "tooltip": "The CLIP model used for encoding the text." + } + ] + } + }, + "input_order": { + "required": [ + "text", + "clip" + ] + }, + "is_input_list": false, + "output": [ + "CONDITIONING" + ], + "output_is_list": [ + false + ], + "output_name": [ + "CONDITIONING" + ], + "name": "CLIPTextEncode", + "display_name": "CLIP Text Encode (Prompt)", + "description": "Encodes a text prompt using a CLIP model into an embedding that can be used to guide the diffusion model towards generating specific images.", + "python_module": "nodes", + "category": "conditioning", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "A conditioning containing the embedded text used to guide the diffusion model." + ], + "search_aliases": [ + "text", + "prompt", + "text prompt", + "positive prompt", + "negative prompt", + "encode text", + "text encoder", + "encode prompt" + ] + }, + "CheckpointLoaderSimple": { + "input": { + "required": { + "ckpt_name": [ + [ + "sd_xl_turbo_1.0_fp16.safetensors", + "v1-5-pruned-emaonly-fp16.safetensors" + ], + { + "tooltip": "The name of the checkpoint (model) to load." + } + ] + } + }, + "input_order": { + "required": [ + "ckpt_name" + ] + }, + "is_input_list": false, + "output": [ + "MODEL", + "CLIP", + "VAE" + ], + "output_is_list": [ + false, + false, + false + ], + "output_name": [ + "MODEL", + "CLIP", + "VAE" + ], + "name": "CheckpointLoaderSimple", + "display_name": "Load Checkpoint", + "description": "Loads a diffusion model checkpoint, diffusion models are used to denoise latents.", + "python_module": "nodes", + "category": "loaders", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The model used for denoising latents.", + "The CLIP model used for encoding text prompts.", + "The VAE model used for encoding and decoding images to and from latent space." + ], + "search_aliases": [ + "load model", + "checkpoint", + "model loader", + "load checkpoint", + "ckpt", + "model" + ] + }, + "EmptyLatentImage": { + "input": { + "required": { + "width": [ + "INT", + { + "default": 512, + "min": 16, + "max": 16384, + "step": 8, + "tooltip": "The width of the latent images in pixels." + } + ], + "height": [ + "INT", + { + "default": 512, + "min": 16, + "max": 16384, + "step": 8, + "tooltip": "The height of the latent images in pixels." + } + ], + "batch_size": [ + "INT", + { + "default": 1, + "min": 1, + "max": 4096, + "tooltip": "The number of latent images in the batch." + } + ] + } + }, + "input_order": { + "required": [ + "width", + "height", + "batch_size" + ] + }, + "is_input_list": false, + "output": [ + "LATENT" + ], + "output_is_list": [ + false + ], + "output_name": [ + "LATENT" + ], + "name": "EmptyLatentImage", + "display_name": "Empty Latent Image", + "description": "Create a new batch of empty latent images to be denoised via sampling.", + "python_module": "nodes", + "category": "latent", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The empty latent image batch." + ], + "search_aliases": [ + "empty", + "empty latent", + "new latent", + "create latent", + "blank latent", + "blank" + ] + }, + "KSampler": { + "input": { + "required": { + "model": [ + "MODEL", + { + "tooltip": "The model used for denoising the input latent." + } + ], + "seed": [ + "INT", + { + "default": 0, + "min": 0, + "max": 18446744073709551615, + "control_after_generate": true, + "tooltip": "The random seed used for creating the noise." + } + ], + "steps": [ + "INT", + { + "default": 20, + "min": 1, + "max": 10000, + "tooltip": "The number of steps used in the denoising process." + } + ], + "cfg": [ + "FLOAT", + { + "default": 8.0, + "min": 0.0, + "max": 100.0, + "step": 0.1, + "round": 0.01, + "tooltip": "The Classifier-Free Guidance scale balances creativity and adherence to the prompt. Higher values result in images more closely matching the prompt however too high values will negatively impact quality." + } + ], + "sampler_name": [ + [ + "euler", + "euler_cfg_pp", + "euler_ancestral", + "euler_ancestral_cfg_pp", + "heun", + "heunpp2", + "exp_heun_2_x0", + "exp_heun_2_x0_sde", + "dpm_2", + "dpm_2_ancestral", + "lms", + "dpm_fast", + "dpm_adaptive", + "dpmpp_2s_ancestral", + "dpmpp_2s_ancestral_cfg_pp", + "dpmpp_sde", + "dpmpp_sde_gpu", + "dpmpp_2m", + "dpmpp_2m_cfg_pp", + "dpmpp_2m_sde", + "dpmpp_2m_sde_gpu", + "dpmpp_2m_sde_heun", + "dpmpp_2m_sde_heun_gpu", + "dpmpp_3m_sde", + "dpmpp_3m_sde_gpu", + "ddpm", + "lcm", + "ipndm", + "ipndm_v", + "deis", + "res_multistep", + "res_multistep_cfg_pp", + "res_multistep_ancestral", + "res_multistep_ancestral_cfg_pp", + "gradient_estimation", + "gradient_estimation_cfg_pp", + "er_sde", + "seeds_2", + "seeds_3", + "sa_solver", + "sa_solver_pece", + "ddim", + "uni_pc", + "uni_pc_bh2" + ], + { + "tooltip": "The algorithm used when sampling, this can affect the quality, speed, and style of the generated output." + } + ], + "scheduler": [ + [ + "simple", + "sgm_uniform", + "karras", + "exponential", + "ddim_uniform", + "beta", + "normal", + "linear_quadratic", + "kl_optimal" + ], + { + "tooltip": "The scheduler controls how noise is gradually removed to form the image." + } + ], + "positive": [ + "CONDITIONING", + { + "tooltip": "The conditioning describing the attributes you want to include in the image." + } + ], + "negative": [ + "CONDITIONING", + { + "tooltip": "The conditioning describing the attributes you want to exclude from the image." + } + ], + "latent_image": [ + "LATENT", + { + "tooltip": "The latent image to denoise." + } + ], + "denoise": [ + "FLOAT", + { + "default": 1.0, + "min": 0.0, + "max": 1.0, + "step": 0.01, + "tooltip": "The amount of denoising applied, lower values will maintain the structure of the initial image allowing for image to image sampling." + } + ] + } + }, + "input_order": { + "required": [ + "model", + "seed", + "steps", + "cfg", + "sampler_name", + "scheduler", + "positive", + "negative", + "latent_image", + "denoise" + ] + }, + "is_input_list": false, + "output": [ + "LATENT" + ], + "output_is_list": [ + false + ], + "output_name": [ + "LATENT" + ], + "name": "KSampler", + "display_name": "KSampler", + "description": "Uses the provided model, positive and negative conditioning to denoise the latent image.", + "python_module": "nodes", + "category": "sampling", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The denoised latent." + ], + "search_aliases": [ + "sampler", + "sample", + "generate", + "denoise", + "diffuse", + "txt2img", + "img2img" + ] + }, + "SaveImage": { + "input": { + "required": { + "images": [ + "IMAGE", + { + "tooltip": "The images to save." + } + ], + "filename_prefix": [ + "STRING", + { + "default": "ComfyUI", + "tooltip": "The prefix for the file to save. This may include formatting information such as %date:yyyy-MM-dd% or %Empty Latent Image.width% to include values from nodes." + } + ] + }, + "hidden": { + "prompt": "PROMPT", + "extra_pnginfo": "EXTRA_PNGINFO" + } + }, + "input_order": { + "required": [ + "images", + "filename_prefix" + ], + "hidden": [ + "prompt", + "extra_pnginfo" + ] + }, + "is_input_list": false, + "output": [], + "output_is_list": [], + "output_name": [], + "name": "SaveImage", + "display_name": "Save Image", + "description": "Saves the input images to your ComfyUI output directory.", + "python_module": "nodes", + "category": "image", + "output_node": true, + "has_intermediate_output": false, + "search_aliases": [ + "save", + "save image", + "export image", + "output image", + "write image", + "download" + ], + "essentials_category": "Basics" + }, + "VAEDecode": { + "input": { + "required": { + "samples": [ + "LATENT", + { + "tooltip": "The latent to be decoded." + } + ], + "vae": [ + "VAE", + { + "tooltip": "The VAE model used for decoding the latent." + } + ] + } + }, + "input_order": { + "required": [ + "samples", + "vae" + ] + }, + "is_input_list": false, + "output": [ + "IMAGE" + ], + "output_is_list": [ + false + ], + "output_name": [ + "IMAGE" + ], + "name": "VAEDecode", + "display_name": "VAE Decode", + "description": "Decodes latent images back into pixel space images.", + "python_module": "nodes", + "category": "latent", + "output_node": false, + "has_intermediate_output": false, + "output_tooltips": [ + "The decoded image." + ], + "search_aliases": [ + "decode", + "decode latent", + "latent to image", + "render latent" + ] + }, + "LoadAudio": { + "input": { + "required": { + "audio": [ + "COMBO", + { + "multiselect": false, + "options": [ + "bedroom.mp4" + ], + "audio_upload": true + } + ] + } + }, + "input_order": { + "required": [ + "audio" + ] + }, + "is_input_list": false, + "output": [ + "AUDIO" + ], + "output_is_list": [ + false + ], + "output_name": [ + "AUDIO" + ], + "output_tooltips": [ + null + ], + "output_matchtypes": null, + "name": "LoadAudio", + "display_name": "Load Audio", + "description": "", + "python_module": "comfy_extras.nodes_audio", + "category": "audio", + "output_node": false, + "deprecated": false, + "experimental": false, + "dev_only": false, + "api_node": false, + "price_badge": null, + "search_aliases": [ + "import audio", + "open audio", + "audio file" + ], + "essentials_category": "Audio", + "has_intermediate_output": false + }, + "PreviewAudio": { + "input": { + "required": { + "audio": [ + "AUDIO", + {} + ] + }, + "hidden": { + "prompt": [ + "PROMPT" + ], + "extra_pnginfo": [ + "EXTRA_PNGINFO" + ] + } + }, + "input_order": { + "required": [ + "audio" + ], + "hidden": [ + "prompt", + "extra_pnginfo" + ] + }, + "is_input_list": false, + "output": [ + "AUDIO" + ], + "output_is_list": [ + false + ], + "output_name": [ + "audio" + ], + "output_tooltips": [ + null + ], + "output_matchtypes": null, + "name": "PreviewAudio", + "display_name": "Preview Audio", + "description": "", + "python_module": "comfy_extras.nodes_audio", + "category": "audio", + "output_node": true, + "deprecated": false, + "experimental": false, + "dev_only": false, + "api_node": false, + "price_badge": null, + "search_aliases": [ + "play audio" + ], + "essentials_category": null, + "has_intermediate_output": false + }, + "SaveAudio": { + "input": { + "required": { + "audio": [ + "AUDIO", + {} + ], + "filename_prefix": [ + "STRING", + { + "default": "audio/ComfyUI", + "multiline": false + } + ] + }, + "hidden": { + "prompt": [ + "PROMPT" + ], + "extra_pnginfo": [ + "EXTRA_PNGINFO" + ] + } + }, + "input_order": { + "required": [ + "audio", + "filename_prefix" + ], + "hidden": [ + "prompt", + "extra_pnginfo" + ] + }, + "is_input_list": false, + "output": [ + "AUDIO" + ], + "output_is_list": [ + false + ], + "output_name": [ + "audio" + ], + "output_tooltips": [ + null + ], + "output_matchtypes": null, + "name": "SaveAudio", + "display_name": "Save Audio (FLAC) (DEPRECATED)", + "description": "", + "python_module": "comfy_extras.nodes_audio", + "category": "audio", + "output_node": true, + "deprecated": true, + "experimental": false, + "dev_only": false, + "api_node": false, + "price_badge": null, + "search_aliases": [ + "export flac" + ], + "essentials_category": "Audio", + "has_intermediate_output": false + }, + "VAEDecodeTiled": { + "input": { + "required": { + "samples": [ + "LATENT" + ], + "vae": [ + "VAE" + ], + "tile_size": [ + "INT", + { + "default": 512, + "min": 64, + "max": 4096, + "step": 32, + "advanced": true + } + ], + "overlap": [ + "INT", + { + "default": 64, + "min": 0, + "max": 4096, + "step": 32, + "advanced": true + } + ], + "temporal_size": [ + "INT", + { + "default": 64, + "min": 8, + "max": 4096, + "step": 4, + "tooltip": "Only used for video VAEs: Amount of frames to decode at a time.", + "advanced": true + } + ], + "temporal_overlap": [ + "INT", + { + "default": 8, + "min": 4, + "max": 4096, + "step": 4, + "tooltip": "Only used for video VAEs: Amount of frames to overlap.", + "advanced": true + } + ] + } + }, + "input_order": { + "required": [ + "samples", + "vae", + "tile_size", + "overlap", + "temporal_size", + "temporal_overlap" + ] + }, + "is_input_list": false, + "output": [ + "IMAGE" + ], + "output_is_list": [ + false + ], + "output_name": [ + "IMAGE" + ], + "name": "VAEDecodeTiled", + "display_name": "VAE Decode (Tiled)", + "description": "", + "python_module": "nodes", + "category": "model/latent", + "output_node": false, + "has_intermediate_output": false, + "search_aliases": [] + }, + "ByteDanceImageNode": { + "input": { + "required": { + "model": [ + "COMBO", + { + "multiselect": false, + "options": [ + "seedream-3-0-t2i-250415" + ] + } + ], + "prompt": [ + "STRING", + { + "tooltip": "The text prompt used to generate the image", + "multiline": true + } + ], + "size_preset": [ + "COMBO", + { + "tooltip": "Pick a recommended size. Select Custom to use the width and height below", + "multiselect": false, + "options": [ + "1024x1024 (1:1)", + "864x1152 (3:4)", + "1152x864 (4:3)", + "1280x720 (16:9)", + "720x1280 (9:16)", + "832x1248 (2:3)", + "1248x832 (3:2)", + "1512x648 (21:9)", + "2048x2048 (1:1)", + "Custom" + ] + } + ], + "width": [ + "INT", + { + "tooltip": "Custom width for image. Value is working only if `size_preset` is set to `Custom`", + "default": 1024, + "min": 512, + "max": 2048, + "step": 64 + } + ], + "height": [ + "INT", + { + "tooltip": "Custom height for image. Value is working only if `size_preset` is set to `Custom`", + "default": 1024, + "min": 512, + "max": 2048, + "step": 64 + } + ] + }, + "optional": { + "seed": [ + "INT", + { + "tooltip": "Seed to use for generation", + "default": 0, + "min": 0, + "max": 2147483647, + "step": 1, + "control_after_generate": true, + "display": "number" + } + ], + "guidance_scale": [ + "FLOAT", + { + "tooltip": "Higher value makes the image follow the prompt more closely", + "default": 2.5, + "min": 1.0, + "max": 10.0, + "step": 0.01, + "display": "number" + } + ], + "watermark": [ + "BOOLEAN", + { + "tooltip": "Whether to add an \"AI generated\" watermark to the image", + "advanced": true, + "default": false + } + ] + }, + "hidden": { + "auth_token_comfy_org": [ + "AUTH_TOKEN_COMFY_ORG" + ], + "api_key_comfy_org": [ + "API_KEY_COMFY_ORG" + ], + "unique_id": [ + "UNIQUE_ID" + ], + "comfy_usage_source": [ + "COMFY_USAGE_SOURCE" + ] + } + }, + "input_order": { + "required": [ + "model", + "prompt", + "size_preset", + "width", + "height" + ], + "optional": [ + "seed", + "guidance_scale", + "watermark" + ], + "hidden": [ + "auth_token_comfy_org", + "api_key_comfy_org", + "unique_id", + "comfy_usage_source" + ] + }, + "is_input_list": false, + "output": [ + "IMAGE" + ], + "output_is_list": [ + false + ], + "output_name": [ + "IMAGE" + ], + "output_tooltips": [ + null + ], + "output_matchtypes": null, + "name": "ByteDanceImageNode", + "display_name": "ByteDance Image", + "description": "Generate images using ByteDance models via api based on prompt", + "python_module": "comfy_api_nodes.nodes_bytedance", + "category": "partner/image/ByteDance", + "output_node": false, + "deprecated": true, + "experimental": false, + "dev_only": false, + "api_node": true, + "price_badge": { + "engine": "jsonata", + "depends_on": { + "widgets": [], + "inputs": [], + "input_groups": [] + }, + "expr": "{\"type\":\"usd\",\"usd\":0.03}" + }, + "search_aliases": null, + "essentials_category": null, + "has_intermediate_output": false + } +} From 3275fdad8918d76a9ece69b2abceb6d71eb6a232 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Sun, 9 Aug 2026 23:26:18 -0700 Subject: [PATCH 2/5] fix(nodes): reject non-positive path bounds and document the envelope contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two CodeRabbit findings on PR #695. `--max-depth 0` / `--max-paths 0` were accepted by Typer and swallowed by the engine's degenerate-bounds guard, which returns an empty result with every flag false. The command then published `exact: true, count: 0` — under this PR's own contract, a proof that no route exists. That proof came from a typo, not from a walk, which is precisely the overclaiming this PR set out to remove. Bounds below 1 are now refused at the command boundary with a registered `path_bounds_invalid` error, before any object_info I/O. Also documents the `nodes path` envelope on `path_cmd`: `mode` echoes the requested matching mode and says nothing about completeness, while `exact` is the exhaustiveness claim, withheld whenever the walk was truncated, depth-limited, or collapsed. The docstring records one honest exception — a same-type query (FROM == TO) is answered empty by construction, so its empty result is not a proof of unreachability. That short-circuit predates this PR and is covered by a base-branch test, so it is documented here and tracked separately rather than changed under this PR. The repo keeps no changelog file (releases are cut from PR titles), so the contract change is recorded in the code docs and the error-code registry, which is the surface agents actually read via `comfy discover`. --- comfy_cli/command/nodes.py | 36 +++++++++++++++++++ comfy_cli/error_codes.py | 7 ++++ .../command/test_nodes_introspect.py | 23 ++++++++++++ 3 files changed, 66 insertions(+) diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index 71cfa3fcd..605d753d3 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -745,7 +745,43 @@ def path_cmd( typer.Option("--where", show_default=False, help="'cloud' to query Comfy Cloud's catalog; default is local."), ] = None, ): + """Envelope contract (``data``): + + - ``mode`` — the *requested* matching mode, ``"exact"`` or ``"loose"``. It + echoes ``--exact/--loose`` and says nothing about completeness. + - ``exact`` — the exhaustiveness claim, and deliberately NOT the flag echoed + back: true only when the listed paths are the complete, type-constrained + answer. Exact mode that stopped early (``truncated``), was still expanding + at the bound (``depth_limited``), or dropped an alternate route into an + already-explored state (``collapsed``) withholds the claim, as does loose + mode always. ``exact: true`` with ``count: 0`` is therefore a proof that + no route exists; ``exact: false`` means "these paths, maybe not all". + - ``truncated`` / ``truncated_by`` / ``depth_limited`` / ``collapsed`` — the + individual reasons the claim was withheld, so a caller can widen the right + bound instead of guessing. + + One documented exception to the ``exact: true, count: 0`` proof: a query + whose FROM and TO are the *same* type is answered empty by construction + (``Graph.search_paths`` declines to walk it, long-standing behaviour), so it + reports an exhaustive empty set even where a real self-returning route such + as ``MODEL -> LoraLoader -> MODEL`` exists. Tracked separately; do not read + a same-type empty result as a proof of unreachability. + """ renderer = get_renderer() + + # A bound below 1 admits no path at all, so the search would return an empty + # result with every flag false — i.e. `exact: true, count: 0`, a proof that + # no route exists. That proof would come from the typo, not from a walk, so + # refuse the bound instead of emitting it. + if max_depth < 1 or max_paths < 1: + renderer.error( + code="path_bounds_invalid", + message="--max-depth and --max-paths must be at least 1.", + hint="retry with `--max-depth 6 --max-paths 10`", + details={"max_depth": max_depth, "max_paths": max_paths}, + ) + raise typer.Exit(code=1) + _stale: dict = {} graph = _get_graph( input_path, diff --git a/comfy_cli/error_codes.py b/comfy_cli/error_codes.py index e68da5291..3f25159f8 100644 --- a/comfy_cli/error_codes.py +++ b/comfy_cli/error_codes.py @@ -603,6 +603,13 @@ class ErrorCode: "Requested node class isn't in the loaded environment.", "see `details.close_matches` or run `comfy nodes search`", ), + ErrorCode( + "path_bounds_invalid", + "`comfy nodes path` was given `--max-depth` or `--max-paths` below 1. Such a bound admits no " + "path at all, so the search is refused rather than returning an empty result that would read " + "as a proof that no route exists.", + "use `--max-depth 6 --max-paths 10` (or any bound >= 1)", + ), # --- file transfer (upload / download) ----------------------------------- ErrorCode( "upload_failed", diff --git a/tests/comfy_cli/command/test_nodes_introspect.py b/tests/comfy_cli/command/test_nodes_introspect.py index 47151d6d5..b7ba2e7fd 100644 --- a/tests/comfy_cli/command/test_nodes_introspect.py +++ b/tests/comfy_cli/command/test_nodes_introspect.py @@ -522,6 +522,29 @@ def test_max_paths_truncation_withholds_the_exact_claim(self, patched_loader, ca assert env["truncated_by"] == "max_paths" assert env["exact"] is False + @pytest.mark.parametrize( + ("flag", "value"), + [("--max-depth", "0"), ("--max-depth", "-1"), ("--max-paths", "0"), ("--max-paths", "-3")], + ) + def test_non_positive_bounds_are_refused_not_answered(self, patched_loader, capsys, flag, value): + """A bound below 1 admits no path, so the walker returns an empty result + with every flag false — which the envelope would publish as `exact: true, + count: 0`, a proof that no route exists. That proof would come from the + typo, not from a walk, so the bound is rejected up front instead. + """ + env = _run(["path", "MODEL", "IMAGE", flag, value], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "path_bounds_invalid" + assert env["error"]["details"][flag.removeprefix("--").replace("-", "_")] == int(value) + # Crucially: no envelope claiming an exhaustive empty answer. + assert "data" not in env or not (env.get("data") or {}).get("exact") + + def test_smallest_valid_bounds_still_search(self, patched_loader, capsys): + """The rejection is for bounds below 1 only — 1 stays a real search.""" + env = _run(["path", "MODEL", "IMAGE", "--max-depth", "2", "--max-paths", "1"], capsys) + assert env["ok"] is True + assert env["data"]["count"] == 1 + def test_loose_mode_never_claims_exactness(self, patched_loader, capsys): env = _run(["path", "MODEL", "IMAGE", "--loose", "--max-depth", "4"], capsys)["data"] assert env["mode"] == "loose" From d66bc22052cf1dd0288c02934ba64ca443cb73cf Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Sun, 9 Aug 2026 23:46:31 -0700 Subject: [PATCH 3/5] fix(cql): declare declined path queries instead of forging a proof of unreachability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second round of CodeRabbit findings on PR #695. `search_paths` declines two query shapes without walking: FROM == TO, and a bound below 1. Both returned an empty result with every limit flag false, which is precisely this module's encoding of "proof that no path exists" — so the abstention was indistinguishable from an exhaustive negative answer, and the command published it as `exact: true, count: 0`. The same-type case is the one that bites: self-returning routes are real (`MODEL -> LoraLoader -> MODEL` on any stock catalog), but the walker cannot represent them, because the no-op rule drops any step whose output type equals its input type and for a same-type query that is the terminal step. So the command was reporting a reachable route as provably unreachable. The result now carries `not_searched` / `not_searched_reason` (`"same_type"` / `"degenerate_bounds"`), the envelope surfaces both, and `exact` is withheld whenever the walk was declined. Behaviour is otherwise unchanged: declined queries still return no paths, so `find_paths(T, T) == []` and the base-branch test asserting it hold as before. Actually *answering* same-type queries is a larger design change and is left as follow-up; this commit only stops the false claim. Also tightens the two tests from the previous commit per review: the invalid-bounds test now installs a `_get_graph` tripwire instead of the loader fixture, so it genuinely pins that validation precedes object_info I/O, and the lower-bound test exercises `--max-depth 1 --max-paths 1` rather than 2, matching what it claims to cover. Adds `LoraLoaderModelOnly` to the path fixture so the same-type regression test asserts against a route that really exists. --- comfy_cli/command/nodes.py | 32 +++++----- comfy_cli/cql/engine.py | 31 ++++++++-- .../command/test_nodes_introspect.py | 60 ++++++++++++++++--- tests/comfy_cli/cql/test_engine.py | 47 +++++++++++++++ .../fixtures/nodes_path_object_info.json | 52 ++++++++++++++++ 5 files changed, 195 insertions(+), 27 deletions(-) diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index 605d753d3..a91cd1053 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -756,16 +756,16 @@ def path_cmd( already-explored state (``collapsed``) withholds the claim, as does loose mode always. ``exact: true`` with ``count: 0`` is therefore a proof that no route exists; ``exact: false`` means "these paths, maybe not all". - - ``truncated`` / ``truncated_by`` / ``depth_limited`` / ``collapsed`` — the - individual reasons the claim was withheld, so a caller can widen the right - bound instead of guessing. - - One documented exception to the ``exact: true, count: 0`` proof: a query - whose FROM and TO are the *same* type is answered empty by construction - (``Graph.search_paths`` declines to walk it, long-standing behaviour), so it - reports an exhaustive empty set even where a real self-returning route such - as ``MODEL -> LoraLoader -> MODEL`` exists. Tracked separately; do not read - a same-type empty result as a proof of unreachability. + - ``truncated`` / ``truncated_by`` / ``depth_limited`` / ``collapsed`` / + ``not_searched`` — the individual reasons the claim was withheld, so a + caller can widen the right bound instead of guessing. + - ``not_searched`` / ``not_searched_reason`` — the walk declined the query + and never ran, so the empty result is an abstention, not an answer. Today + the only reason reachable from the CLI is ``"same_type"``: a query whose + FROM and TO are the same type is answered empty by construction, even + though real self-returning routes such as ``MODEL -> LoraLoader -> MODEL`` + exist. Such a result reports ``exact: false`` and must not be read as a + proof of unreachability. """ renderer = get_renderer() @@ -796,6 +796,7 @@ def path_cmd( truncated = bool(result["truncated"]) depth_limited = bool(result["depth_limited"]) collapsed = bool(result["collapsed"]) + not_searched = bool(result["not_searched"]) payload = { "from": from_type, @@ -803,14 +804,17 @@ def path_cmd( "mode": "exact" if exact else "loose", # Not the flag echoed back: the honest claim that these paths are the # complete, type-constrained answer. Any early stop (max_paths, the - # internal state budget), a frontier still expanding at max_depth, or an - # intermediate state reached by a second route that was not re-explored - # means paths may be missing, so the claim is withheld. - "exact": bool(exact and not truncated and not depth_limited and not collapsed), + # internal state budget), a frontier still expanding at max_depth, an + # intermediate state reached by a second route that was not re-explored, + # or a query the walk declined outright means paths may be missing, so + # the claim is withheld. + "exact": bool(exact and not truncated and not depth_limited and not collapsed and not not_searched), "truncated": truncated, "truncated_by": result["truncated_by"], "depth_limited": depth_limited, "collapsed": collapsed, + "not_searched": not_searched, + "not_searched_reason": result["not_searched_reason"], "max_depth": max_depth, "max_paths": max_paths, "count": len(paths), diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 2b3fcd5ba..697b1c7ef 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -666,8 +666,14 @@ def search_paths( reports no support. Returns ``{"paths", "truncated", "truncated_by", "depth_limited", - "collapsed"}``: - + "collapsed", "not_searched", "not_searched_reason"}``: + + - ``not_searched`` — the walk declined the query outright and never ran, + so the empty result is an abstention rather than an answer. + ``not_searched_reason`` says which: ``"same_type"`` (FROM and TO are + the same type — self-returning routes exist but this walker cannot + represent them) or ``"degenerate_bounds"`` (``max_depth`` or + ``max_paths`` below 1, a bound no path can satisfy). - ``truncated`` — the walk stopped early (``max_paths`` reached, or the internal state budget exhausted), so paths exist that are not listed. - ``depth_limited`` — the frontier was still expanding at ``max_depth``, @@ -676,10 +682,10 @@ def search_paths( one route and explored it only once, so alternate chains through that state are not listed. Reachability is unaffected (the surviving route explores exactly the same continuations), which is why an **empty** - result with all three flags false is a proof that no path exists — + result with all four flags false is a proof that no path exists — but a non-empty one is a sample of the routes, not the full set. - A caller may only treat the listing as exhaustive when all three are + A caller may only treat the listing as exhaustive when all four are false. Each errs toward true: hitting ``max_paths`` exactly is reported as truncated even when nothing further existed, and a revisited state is reported as collapsed even when its alternate route led nowhere. @@ -690,8 +696,23 @@ def search_paths( "truncated_by": None, "depth_limited": False, "collapsed": False, + "not_searched": False, + "not_searched_reason": None, } - if from_type == to_type or max_depth < 1 or max_paths < 1: + # Query shapes the walk declines outright. The empty result they yield is + # an abstention, not a proof, so it has to say so — otherwise it reads + # as "no route exists" with every limit flag reassuringly false. + if from_type == to_type: + # Self-returning routes are real (``MODEL -> LoraLoader -> MODEL``), + # but the walk cannot represent them: the no-op rule below drops any + # step whose output type equals its input type, and for a same-type + # query that is the terminal step. Declining is the honest option. + result["not_searched"] = True + result["not_searched_reason"] = "same_type" + return result + if max_depth < 1 or max_paths < 1: + result["not_searched"] = True + result["not_searched_reason"] = "degenerate_bounds" return result free = self.free_types() if exact else frozenset() diff --git a/tests/comfy_cli/command/test_nodes_introspect.py b/tests/comfy_cli/command/test_nodes_introspect.py index b7ba2e7fd..97de4b8b4 100644 --- a/tests/comfy_cli/command/test_nodes_introspect.py +++ b/tests/comfy_cli/command/test_nodes_introspect.py @@ -526,12 +526,22 @@ def test_max_paths_truncation_withholds_the_exact_claim(self, patched_loader, ca ("flag", "value"), [("--max-depth", "0"), ("--max-depth", "-1"), ("--max-paths", "0"), ("--max-paths", "-3")], ) - def test_non_positive_bounds_are_refused_not_answered(self, patched_loader, capsys, flag, value): + def test_non_positive_bounds_are_refused_before_any_graph_load(self, monkeypatch, capsys, flag, value): """A bound below 1 admits no path, so the walker returns an empty result - with every flag false — which the envelope would publish as `exact: true, - count: 0`, a proof that no route exists. That proof would come from the - typo, not from a walk, so the bound is rejected up front instead. + with every limit flag false — which the envelope would publish as + `exact: true, count: 0`, a proof that no route exists. That proof would + come from the typo, not from a walk, so the bound is rejected up front. + + Deliberately *not* using the `patched_loader` fixture: `_get_graph` is + replaced with a tripwire, so the test fails if the command does any + object_info I/O before validating the caller's bounds. """ + + def _tripwire(*a, **kw): + raise AssertionError("_get_graph was called before the bounds were validated") + + monkeypatch.setattr(nodes_cmd, "_get_graph", _tripwire) + env = _run(["path", "MODEL", "IMAGE", flag, value], capsys) assert env["ok"] is False assert env["error"]["code"] == "path_bounds_invalid" @@ -539,11 +549,45 @@ def test_non_positive_bounds_are_refused_not_answered(self, patched_loader, caps # Crucially: no envelope claiming an exhaustive empty answer. assert "data" not in env or not (env.get("data") or {}).get("exact") - def test_smallest_valid_bounds_still_search(self, patched_loader, capsys): - """The rejection is for bounds below 1 only — 1 stays a real search.""" - env = _run(["path", "MODEL", "IMAGE", "--max-depth", "2", "--max-paths", "1"], capsys) + def test_smallest_valid_bounds_are_searched_not_refused(self, patched_loader, capsys): + """The rejection is for bounds below 1 only. `1` is a legitimate bound: + it must run a real (if very shallow) search rather than error out, even + though nothing is reachable from MODEL in a single hop.""" + env = _run(["path", "MODEL", "IMAGE", "--max-depth", "1", "--max-paths", "1"], capsys) assert env["ok"] is True - assert env["data"]["count"] == 1 + data = env["data"] + assert data["count"] == 0 + # The walk genuinely ran and hit the depth bound — it was not declined. + assert data["depth_limited"] is True + assert data["not_searched"] is False + assert data["exact"] is False + + def test_same_type_query_declines_rather_than_claiming_unreachability(self, patched_loader, capsys): + """`MODEL -> MODEL` is a *reachable* query — the fixture carries + `LoraLoaderModelOnly`, a stock-shaped node taking a MODEL link input and + emitting MODEL. The walker cannot represent that route (its no-op rule + drops any step whose output type equals its input type, which for a + same-type query is the terminal step), so it declines the query outright. + + Declining is fine. Declining while reporting `exact: true, count: 0` — + the envelope's proof that no route exists — would not be, so the + abstention is declared and the exactness claim withheld. + """ + lora = _run(["show", "LoraLoaderModelOnly"], capsys)["data"] + assert "MODEL" in {o["type"] for o in lora["outputs"]}, "fixture must offer a real MODEL -> MODEL route" + assert "MODEL" in {i["type"] for i in lora["inputs"]} + + data = _run(["path", "MODEL", "MODEL"], capsys)["data"] + assert data["count"] == 0 + assert data["not_searched"] is True + assert data["not_searched_reason"] == "same_type" + # The point of the whole ticket: an empty answer that is not a proof + # must not be labelled exact. + assert data["exact"] is False + # ...and it is the abstention doing it, not a bound that happened to bite. + assert data["truncated"] is False + assert data["depth_limited"] is False + assert data["collapsed"] is False def test_loose_mode_never_claims_exactness(self, patched_loader, capsys): env = _run(["path", "MODEL", "IMAGE", "--loose", "--max-depth", "4"], capsys)["data"] diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index 7386f4538..ec000e834 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -560,6 +560,53 @@ def test_degenerate_bounds_return_nothing(self, graph_path: Graph): assert graph_path.search_paths("MODEL", "IMAGE", max_depth=0)["paths"] == [] assert graph_path.search_paths("MODEL", "IMAGE", max_paths=0)["paths"] == [] + @pytest.mark.parametrize( + ("kwargs", "reason"), + [ + ({"from_type": "MODEL", "to_type": "MODEL"}, "same_type"), + ({"from_type": "MODEL", "to_type": "IMAGE", "max_depth": 0}, "degenerate_bounds"), + ({"from_type": "MODEL", "to_type": "IMAGE", "max_paths": 0}, "degenerate_bounds"), + ], + ) + def test_declined_queries_declare_the_abstention(self, graph_path: Graph, kwargs, reason): + """The query shapes the walk refuses return an empty result. An empty + result with every limit flag false is this module's proof that no path + exists, so a refusal that stayed silent would forge that proof. Each + one says so instead. + """ + from_type = kwargs.pop("from_type") + to_type = kwargs.pop("to_type") + result = graph_path.search_paths(from_type, to_type, **kwargs) + assert result["paths"] == [] + assert result["not_searched"] is True + assert result["not_searched_reason"] == reason + # No limit flag is set — which is exactly why the abstention needs its + # own signal rather than being inferred from the others. + assert result["truncated"] is False + assert result["depth_limited"] is False + assert result["collapsed"] is False + + def test_same_type_query_is_declined_even_though_a_route_exists(self, graph_path: Graph): + """`LoraLoaderModelOnly` in the fixture takes a MODEL link input and + emits MODEL, so `MODEL -> MODEL` is genuinely routable. The walker still + declines it — the no-op rule (`out_t == cur_type`) drops that step — so + the empty result must be flagged as an abstention, never as proof.""" + lora = graph_path.node("LoraLoaderModelOnly") + assert lora is not None and "MODEL" in lora.output_types() + + result = graph_path.search_paths("MODEL", "MODEL") + assert result["paths"] == [] + assert result["not_searched"] is True + + def test_completed_walks_are_not_marked_as_declined(self, graph_path: Graph): + """The abstention flag must stay off for searches that actually ran, + whether they found routes or genuinely exhausted the space.""" + found = graph_path.search_paths("MODEL", "IMAGE", max_depth=4) + assert found["paths"] and found["not_searched"] is False + empty = graph_path.search_paths("AUDIO", "IMAGE", max_depth=6) + assert empty["paths"] == [] and empty["not_searched"] is False + assert empty["not_searched_reason"] is None + # =========================================================================== # TestValidateWorkflow diff --git a/tests/comfy_cli/fixtures/nodes_path_object_info.json b/tests/comfy_cli/fixtures/nodes_path_object_info.json index d591ab3b8..0d2e2f7ee 100644 --- a/tests/comfy_cli/fixtures/nodes_path_object_info.json +++ b/tests/comfy_cli/fixtures/nodes_path_object_info.json @@ -884,5 +884,57 @@ "search_aliases": null, "essentials_category": null, "has_intermediate_output": false + }, + "LoraLoaderModelOnly": { + "input": { + "required": { + "model": [ + "MODEL", + { + "tooltip": "The diffusion model the LoRA will be applied to." + } + ], + "lora_name": [ + [ + "sd15/detail.safetensors", + "sdxl/style.safetensors" + ], + { + "tooltip": "The name of the LoRA." + } + ], + "strength_model": [ + "FLOAT", + { + "default": 1.0, + "min": -100.0, + "max": 100.0, + "step": 0.01 + } + ] + } + }, + "input_order": { + "required": [ + "model", + "lora_name", + "strength_model" + ] + }, + "output": [ + "MODEL" + ], + "output_is_list": [ + false + ], + "output_name": [ + "MODEL" + ], + "name": "LoraLoaderModelOnly", + "display_name": "LoraLoaderModelOnly", + "description": "LoRAs are used to modify diffusion models.", + "python_module": "nodes", + "category": "loaders", + "output_node": false } } From 4b2e20f961f723ec864645491ada576ca006c35e Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 10 Aug 2026 00:37:33 -0700 Subject: [PATCH 4/5] fix(cql): answer same-type path queries instead of declining them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nodes path MODEL MODEL` returned `count: 0` with `not_searched: "same_type"` — an honest abstention, but a useless one: the route is real. `LoraLoaderModelOnly` takes a MODEL link input and emits MODEL, and the same shape covers `LoraLoader`, `CLIPSetLastLayer`, `ConditioningSetArea` and most patcher nodes. Two blocks stood in the way. The early `from_type == to_type` guard declined before any walk started, and the BFS no-op rule (`out_t == cur_type`) dropped the very hop that answers a same-type query. The guard is gone and the no-op rule now exempts the terminal hop (`out_t == cur_type and out_t != to_type`). The exemption cannot leak into any other query: a step whose output matches `to_type` is recorded as a completed path and never queued, so `cur_type == to_type` holds only for the initial frontier item — i.e. exactly when `from_type == to_type`. Verified differentially over 1568 FROM/TO/mode/bound combinations across both catalog fixtures: every non-same-type result is byte-identical to before. `degenerate_bounds` is now the sole `not_searched_reason`, and it is unreachable from the CLI because `path_cmd` rejects those bounds up front. --- comfy_cli/command/nodes.py | 14 ++-- comfy_cli/cql/engine.py | 35 +++++----- .../command/test_nodes_introspect.py | 42 +++++++----- tests/comfy_cli/cql/test_engine.py | 64 ++++++++++++++++--- 4 files changed, 109 insertions(+), 46 deletions(-) diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index a91cd1053..4bd32be69 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -760,12 +760,14 @@ def path_cmd( ``not_searched`` — the individual reasons the claim was withheld, so a caller can widen the right bound instead of guessing. - ``not_searched`` / ``not_searched_reason`` — the walk declined the query - and never ran, so the empty result is an abstention, not an answer. Today - the only reason reachable from the CLI is ``"same_type"``: a query whose - FROM and TO are the same type is answered empty by construction, even - though real self-returning routes such as ``MODEL -> LoraLoader -> MODEL`` - exist. Such a result reports ``exact: false`` and must not be read as a - proof of unreachability. + and never ran, so the empty result is an abstention, not an answer. No + reason is reachable from this command: the only shape ``search_paths`` + declines is a bound below 1, which is rejected up front with + ``path_bounds_invalid`` before the graph is even loaded. Same-type queries + (``MODEL MODEL``) are searched like any other and return the real + self-returning routes. The fields stay in the envelope so a caller can + never mistake an abstention the engine adds later for a proof of + unreachability — an abstention always reports ``exact: false``. """ renderer = get_renderer() diff --git a/comfy_cli/cql/engine.py b/comfy_cli/cql/engine.py index 697b1c7ef..797f4fee9 100644 --- a/comfy_cli/cql/engine.py +++ b/comfy_cli/cql/engine.py @@ -670,10 +670,12 @@ def search_paths( - ``not_searched`` — the walk declined the query outright and never ran, so the empty result is an abstention rather than an answer. - ``not_searched_reason`` says which: ``"same_type"`` (FROM and TO are - the same type — self-returning routes exist but this walker cannot - represent them) or ``"degenerate_bounds"`` (``max_depth`` or - ``max_paths`` below 1, a bound no path can satisfy). + ``not_searched_reason`` names the only shape that does this: + ``"degenerate_bounds"`` (``max_depth`` or ``max_paths`` below 1, a + bound no path can satisfy). A same-type query is *answered*, not + declined — self-returning routes such as + ``MODEL -> LoraLoaderModelOnly -> MODEL`` are real, and the no-op rule + below exempts the terminal hop so they are found like any other. - ``truncated`` — the walk stopped early (``max_paths`` reached, or the internal state budget exhausted), so paths exist that are not listed. - ``depth_limited`` — the frontier was still expanding at ``max_depth``, @@ -699,17 +701,9 @@ def search_paths( "not_searched": False, "not_searched_reason": None, } - # Query shapes the walk declines outright. The empty result they yield is - # an abstention, not a proof, so it has to say so — otherwise it reads - # as "no route exists" with every limit flag reassuringly false. - if from_type == to_type: - # Self-returning routes are real (``MODEL -> LoraLoader -> MODEL``), - # but the walk cannot represent them: the no-op rule below drops any - # step whose output type equals its input type, and for a same-type - # query that is the terminal step. Declining is the honest option. - result["not_searched"] = True - result["not_searched_reason"] = "same_type" - return result + # The one query shape the walk declines outright. The empty result it + # yields is an abstention, not a proof, so it has to say so — otherwise + # it reads as "no route exists" with every limit flag reassuringly false. if max_depth < 1 or max_paths < 1: result["not_searched"] = True result["not_searched_reason"] = "degenerate_bounds" @@ -740,7 +734,16 @@ def search_paths( # alone — the pruning loose path-finding has always used. new_produced = produced | frozenset(outs) if exact else produced for out_t in outs: - if out_t == cur_type: + # A step that hands back the type it consumed is a no-op + # hop — except when that type is the target, where it is + # the terminal step and the only one that can answer the + # query (``MODEL -> LoraLoaderModelOnly -> MODEL``). The + # exemption is confined to same-type queries: any state + # whose output matched ``to_type`` was recorded as a + # completed path and never queued, so ``cur_type == + # to_type`` can only hold for the initial frontier item, + # i.e. exactly when ``from_type == to_type``. + if out_t == cur_type and out_t != to_type: continue step = {"node": consumer.id, "input_type": cur_type, "output_type": out_t} new_steps = steps + [step] diff --git a/tests/comfy_cli/command/test_nodes_introspect.py b/tests/comfy_cli/command/test_nodes_introspect.py index 97de4b8b4..44a7d4804 100644 --- a/tests/comfy_cli/command/test_nodes_introspect.py +++ b/tests/comfy_cli/command/test_nodes_introspect.py @@ -562,32 +562,44 @@ def test_smallest_valid_bounds_are_searched_not_refused(self, patched_loader, ca assert data["not_searched"] is False assert data["exact"] is False - def test_same_type_query_declines_rather_than_claiming_unreachability(self, patched_loader, capsys): + def test_same_type_query_lists_the_route_it_used_to_decline(self, patched_loader, capsys): """`MODEL -> MODEL` is a *reachable* query — the fixture carries `LoraLoaderModelOnly`, a stock-shaped node taking a MODEL link input and - emitting MODEL. The walker cannot represent that route (its no-op rule - drops any step whose output type equals its input type, which for a - same-type query is the terminal step), so it declines the query outright. + emitting MODEL — and the CLI now answers it. - Declining is fine. Declining while reporting `exact: true, count: 0` — - the envelope's proof that no route exists — would not be, so the - abstention is declared and the exactness claim withheld. + It used to decline: the walker's no-op rule dropped any step whose + output type equalled its input type, which for a same-type query is the + terminal step, so the command returned `count: 0` with the abstention + declared. Declining was honest but useless — the route is real, so it is + listed. """ lora = _run(["show", "LoraLoaderModelOnly"], capsys)["data"] assert "MODEL" in {o["type"] for o in lora["outputs"]}, "fixture must offer a real MODEL -> MODEL route" assert "MODEL" in {i["type"] for i in lora["inputs"]} data = _run(["path", "MODEL", "MODEL"], capsys)["data"] - assert data["count"] == 0 - assert data["not_searched"] is True - assert data["not_searched_reason"] == "same_type" - # The point of the whole ticket: an empty answer that is not a proof - # must not be labelled exact. - assert data["exact"] is False - # ...and it is the abstention doing it, not a bound that happened to bite. + assert data["count"] >= 1 + one_step = [p for p in data["paths"] if [s["node"] for s in p["steps"]] == ["LoraLoaderModelOnly"]] + assert one_step, "the one-step MODEL -> MODEL route must be listed" + assert one_step[0]["steps"][0] == { + "node": "LoraLoaderModelOnly", + "from_type": "MODEL", + "to_type": "MODEL", + } + # The walk ran to completion: no abstention, no bound bit it. + assert data["not_searched"] is False + assert data["not_searched_reason"] is None assert data["truncated"] is False assert data["depth_limited"] is False - assert data["collapsed"] is False + # `exact` is still withheld here, and for the ordinary reason rather + # than a leftover of the old refusal: reaching MODEL ends a path, so the + # walk keeps expanding the branches that do not (KSampler -> LATENT), + # and there both decoders land on the same (IMAGE, {IMAGE, LATENT}) + # state — a collapse. It costs no MODEL -> MODEL route (nothing in this + # catalog routes IMAGE back to MODEL), but the flag errs toward true by + # design, so the claim is withheld rather than forged. + assert data["collapsed"] is True + assert data["exact"] is False def test_loose_mode_never_claims_exactness(self, patched_loader, capsys): env = _run(["path", "MODEL", "IMAGE", "--loose", "--max-depth", "4"], capsys)["data"] diff --git a/tests/comfy_cli/cql/test_engine.py b/tests/comfy_cli/cql/test_engine.py index ec000e834..cfcff9ad7 100644 --- a/tests/comfy_cli/cql/test_engine.py +++ b/tests/comfy_cli/cql/test_engine.py @@ -366,8 +366,19 @@ def test_exact_paths_model_to_image(self, graph: Graph): for step in p["steps"]: assert graph.node(step["node"]) is not None - def test_find_paths_same_type_returns_empty(self, graph: Graph): + def test_find_paths_same_type_is_searched_not_declined(self, graph: Graph): + """Same-type queries used to be refused outright; they are now walked + like any other. This small catalog happens to hold no route back to + MODEL — nothing here consumes MODEL and emits it — so the empty result + is a fact about the catalog rather than an abstention. The catalog that + *does* carry one (`LoraLoaderModelOnly`) is the `graph_path` fixture, + pinned by `test_same_type_query_finds_the_route` below. + """ assert graph.find_paths("MODEL", "MODEL") == [] + result = graph.search_paths("MODEL", "MODEL", exact=False, max_depth=4) + assert result["paths"] == [] + assert result["not_searched"] is False + assert result["not_searched_reason"] is None def test_find_paths_unreachable_returns_empty(self, graph: Graph): # No node consumes IMAGE and produces MODEL in this fixture @@ -556,14 +567,16 @@ def test_state_budget_is_reported_as_truncation(self, graph_path: Graph): assert result["truncated_by"] == "max_states" def test_degenerate_bounds_return_nothing(self, graph_path: Graph): - assert graph_path.search_paths("MODEL", "MODEL")["paths"] == [] + # `MODEL -> MODEL` used to sit here as a third degenerate case. It is no + # longer degenerate — a same-type query is a real question with a real + # answer (see `test_same_type_query_finds_the_route`), so only the + # bounds no path can satisfy remain. assert graph_path.search_paths("MODEL", "IMAGE", max_depth=0)["paths"] == [] assert graph_path.search_paths("MODEL", "IMAGE", max_paths=0)["paths"] == [] @pytest.mark.parametrize( ("kwargs", "reason"), [ - ({"from_type": "MODEL", "to_type": "MODEL"}, "same_type"), ({"from_type": "MODEL", "to_type": "IMAGE", "max_depth": 0}, "degenerate_bounds"), ({"from_type": "MODEL", "to_type": "IMAGE", "max_paths": 0}, "degenerate_bounds"), ], @@ -586,17 +599,50 @@ def test_declined_queries_declare_the_abstention(self, graph_path: Graph, kwargs assert result["depth_limited"] is False assert result["collapsed"] is False - def test_same_type_query_is_declined_even_though_a_route_exists(self, graph_path: Graph): + def test_same_type_query_finds_the_route(self, graph_path: Graph): """`LoraLoaderModelOnly` in the fixture takes a MODEL link input and - emits MODEL, so `MODEL -> MODEL` is genuinely routable. The walker still - declines it — the no-op rule (`out_t == cur_type`) drops that step — so - the empty result must be flagged as an abstention, never as proof.""" + emits MODEL, so `MODEL -> MODEL` is genuinely routable — and is now + answered rather than declined. The walker used to refuse the query + outright and report the empty result as an abstention; the no-op rule + (`out_t == cur_type`) no longer drops the hop that answers it. + """ lora = graph_path.node("LoraLoaderModelOnly") assert lora is not None and "MODEL" in lora.output_types() + assert lora.has_input("MODEL") result = graph_path.search_paths("MODEL", "MODEL") - assert result["paths"] == [] - assert result["not_searched"] is True + assert ("LoraLoaderModelOnly",) in {_node_chain(p) for p in result["paths"]} + # A real walk, not an abstention — and the one-step route is a genuine + # MODEL-in/MODEL-out hop, not a mislabelled edge. + assert result["not_searched"] is False + assert result["not_searched_reason"] is None + one_step = next(p for p in result["paths"] if _node_chain(p) == ("LoraLoaderModelOnly",)) + assert one_step["from"] == "MODEL" and one_step["to"] == "MODEL" + assert one_step["steps"] == [{"node": "LoraLoaderModelOnly", "input_type": "MODEL", "output_type": "MODEL"}] + + def test_no_op_hops_are_still_dropped(self, graph_path: Graph): + """The exemption is scoped to the hop that answers a same-type query, + and to nothing else — a step that hands back the type it consumed is + still a no-op everywhere it is not the terminal step. + + For a FROM != TO query that means *no* step may do it at all: a step + whose output equals the target ends the path, so a no-op-looking step + requires the incoming type to already be the target, which only the + first frontier item can satisfy. + """ + for from_type in ("MODEL", "LATENT", "CLIP", "CONDITIONING"): + for p in graph_path.exact_paths(from_type, "IMAGE", max_depth=6): + assert all(s["input_type"] != s["output_type"] for s in p["steps"]), ( + f"no-op hop in {from_type} -> IMAGE via {_node_chain(p)}" + ) + # And within a same-type query it is the terminal hop only. + same_type = graph_path.exact_paths("MODEL", "MODEL", max_depth=6) + assert same_type, "fixture must offer at least one MODEL -> MODEL route" + for p in same_type: + for i, step in enumerate(p["steps"]): + if step["input_type"] == step["output_type"]: + assert i == len(p["steps"]) - 1, f"no-op mid-path in {_node_chain(p)}" + assert step["output_type"] == "MODEL" def test_completed_walks_are_not_marked_as_declined(self, graph_path: Graph): """The abstention flag must stay off for searches that actually ran, From 3b0bb2d4a4a02c9fb2d783819d25d6bd845a8418 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Tue, 11 Aug 2026 03:28:53 -0700 Subject: [PATCH 5/5] fix(nodes): restore the `report_usage_error` wrapper dropped by the main merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge of `origin/main` into this branch (86ed079) resolved `_get_graph` to the outgoing side, silently reverting #687 (BE-6660): the `report_usage_error(get_renderer())` context manager around `resolve_host_port` was deleted along with its comment and import. Effect: every `comfy nodes` verb went back to exit 2 with zero bytes on stdout for a rejected `--host`/`--port`, instead of the terminating `host_port_invalid` envelope — the "machine consumer just sees the stream stop" failure BE-6660 was filed to fix. Reproduced locally on the merged tree: `test_host_port.py::test_nodes_bad_port_terminates_with_envelope` fails deterministically (1 failed, 79 passed under `-p no:randomly`) and passes once the wrapper is restored. Restored verbatim from main. Nothing else changes: the only remaining `nodes.py` delta versus main is this PR's `path_cmd` docstring. Reported by @bigcat88 on #698. --- comfy_cli/command/nodes.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/comfy_cli/command/nodes.py b/comfy_cli/command/nodes.py index b43f70795..9d9271816 100644 --- a/comfy_cli/command/nodes.py +++ b/comfy_cli/command/nodes.py @@ -89,9 +89,14 @@ def _get_graph( # one `comfy run` submits to whenever ComfyUI was launched in the background # on a non-default port (BE-6299). if input_path is None and mode == "local": - from comfy_cli.host_port import resolve_host_port - - host, port = resolve_host_port(host, port) + from comfy_cli.host_port import report_usage_error, resolve_host_port + + # A rejected `--host`/`--port` raises `typer.BadParameter`, which click + # turns into a stderr usage panel + exit 2 with nothing on stdout. + # Emit the terminating envelope first so JSON/NDJSON consumers get a + # parseable final line; the exception still escapes, so exit stays 2. + with report_usage_error(get_renderer()): + host, port = resolve_host_port(host, port) try: if input_path is not None: # Explicit offline dump — let Graph.load read + annotate it.