diff --git a/comfy_cli/cmdline.py b/comfy_cli/cmdline.py index 4662b788..d9b0e8ce 100644 --- a/comfy_cli/cmdline.py +++ b/comfy_cli/cmdline.py @@ -47,6 +47,7 @@ from comfy_cli.config_manager import ConfigManager from comfy_cli.constants import GPU_OPTION, CUDAVersion, ROCmVersion from comfy_cli.cuda_detect import DEFAULT_CUDA_TAG, detect_cuda_driver_version, resolve_cuda_wheel +from comfy_cli.deprecation import add_deprecated_alias from comfy_cli.discovery import build_discovery from comfy_cli.env_checker import EnvChecker from comfy_cli.help_json import build_help_json @@ -1612,12 +1613,20 @@ def standalone( generate_command.register_with(app) -app.add_typer(models_command.app, name="model", help="Manage models.") +# The `model` noun owns BOTH the local-filesystem ops (download/remove/list) and +# the backend/cloud discovery leaves (list-folders/list-folder/search/show). The +# discovery leaves are implemented on `models_search_command.app`; surface them +# under `model` by borrowing their command registrations (same CommandInfo +# objects — no logic duplication). +models_command.app.registered_commands.extend(models_search_command.app.registered_commands) app.add_typer( - models_search_command.app, - name="models", - help="Discover models — folders, files, and the cloud asset catalog.", + models_command.app, + name="model", + help="Manage models — local files on disk plus backend/cloud discovery.", ) +# `models` (plural) is now a hidden, deprecated alias for the discovery leaves. +# Every old `comfy models …` invocation still works but prints a warning. +add_deprecated_alias(app, models_search_command.app, old_name="models", new_name="model") app.add_typer(custom_nodes.app, name="node", help="Manage custom nodes.") app.add_typer(nodes_command.app, name="nodes", help="Introspect ComfyUI node classes (inputs, outputs, categories).") app.add_typer(templates_command.app, name="templates", help="Browse the Comfy workflow-template gallery.") diff --git a/comfy_cli/command/models/models.py b/comfy_cli/command/models/models.py index a654a18e..493d9775 100644 --- a/comfy_cli/command/models/models.py +++ b/comfy_cli/command/models/models.py @@ -446,7 +446,11 @@ def list_models(path: pathlib.Path) -> list[pathlib.Path]: return sorted(f for f in path.rglob("*") if f.is_file()) -@app.command("list") +@app.command( + "list", + help="List local models on disk (files already downloaded into the current workspace). " + "For backend/cloud discovery use `comfy model list-folders` / `list-folder`.", +) @tracking.track_command("model") def list_command( ctx: typer.Context, diff --git a/comfy_cli/command/models/search.py b/comfy_cli/command/models/search.py index 3f8565e9..156045f6 100644 --- a/comfy_cli/command/models/search.py +++ b/comfy_cli/command/models/search.py @@ -150,7 +150,9 @@ def _emit_http_error(e: urllib.error.HTTPError, *, renderer, target, message: st @app.command( "list-folders", - help="List model folders available to the resolved backend (cloud: /api/experiment/models, local: /models).", + help="List model folders on the backend/cloud — the resolved server " + "(cloud: /api/experiment/models, local server: /models). For files already " + "on disk in your workspace, use `comfy model list`.", ) @tracking.track_command("models") def list_folders_cmd( @@ -216,7 +218,9 @@ def list_folders_cmd( @app.command( "list-folder", - help="List model files in a specific folder. Returns name + pathIndex per entry — no enrichment.", + help="List model files in a specific folder on the backend/cloud (the resolved " + "server). Returns name + pathIndex per entry — no enrichment. For files already " + "on disk in your workspace, use `comfy model list`.", ) @tracking.track_command("models") def list_folder_cmd( diff --git a/comfy_cli/deprecation.py b/comfy_cli/deprecation.py new file mode 100644 index 00000000..46784ed2 --- /dev/null +++ b/comfy_cli/deprecation.py @@ -0,0 +1,99 @@ +"""Shared helper for wiring hidden, deprecated command aliases. + +A deprecation alias keeps an old command/group spelling working while steering +users to the new one. Three things happen to an alias: + + (a) it is hidden from ``--help`` (so the surface advertises only the new name); + (b) its group help is prefixed with a ``[DEPRECATED — use ]`` banner; and + (c) invoking any leaf under it prints a single ``[yellow]`` warning line to + stderr. + +The warning is never silenced — discoverability of the rename is the whole +point, and stderr keeps it out of the one-envelope-on-stdout contract. + +Introduced for the ``models`` → ``model`` merge (BE-2999); the sibling noun +consolidations reuse :func:`add_deprecated_alias`. +""" + +from __future__ import annotations + +import functools + +import typer + +# Use the output shim so the warning lands on stderr (not stdout) in JSON mode, +# preserving the one-envelope-on-stdout contract. +from comfy_cli.output import rprint + + +def deprecated_help(new_name: str, original_help: str | None = None) -> str: + """Prefix ``original_help`` with the deprecation banner. + + ``new_name`` is the command path *without* the ``comfy`` prog prefix + (e.g. ``"model"``); the banner renders it as ``comfy ``. + """ + banner = f"[DEPRECATED — use `comfy {new_name}`]" + original = (original_help or "").strip() + return f"{banner} {original}".strip() + + +def warn_deprecated(old_name: str, new_name: str) -> None: + """Emit the one-line yellow deprecation warning to stderr.""" + rprint(f"[yellow]'comfy {old_name} …' is deprecated; use 'comfy {new_name} …'[/yellow]") + + +def add_deprecated_alias( + parent: typer.Typer, + source_app: typer.Typer, + *, + old_name: str, + new_name: str, +) -> typer.Typer: + """Mount a hidden, deprecated alias of ``source_app`` on ``parent``. + + The alias reuses ``source_app``'s command implementations verbatim (no logic + duplication) and adds a group callback that prints one yellow warning to + stderr before any leaf runs. ``source_app`` itself is left untouched — the + alias is a fresh Typer that borrows the registered commands — so the canonical + mount of the same commands stays warning-free. + + Both leaf commands *and* nested sub-groups (``add_typer``) are carried over, + and if ``source_app`` declares its own group callback the alias composes it + with the deprecation warning rather than dropping it — so the helper stays + correct as the sibling noun consolidations reuse it against richer command + trees. + + Returns the alias app (useful for tests). + """ + alias = typer.Typer( + no_args_is_help=True, + help=deprecated_help(new_name, source_app.info.help), + ) + # Borrow the exact command + sub-group registrations — same CommandInfo / + # TyperInfo objects, so the alias and the canonical surface stay in lockstep + # with zero duplication. + alias.registered_commands.extend(source_app.registered_commands) + alias.registered_groups.extend(source_app.registered_groups) + + # The alias always needs a group callback to emit the deprecation warning. + # If the source app declared one of its own (with its own options / setup), + # compose the two — warn first, then delegate — preserving the source's + # callback signature so its CLI options still resolve on the alias. + source_cb_info = source_app.registered_callback + source_cb = getattr(source_cb_info, "callback", None) if source_cb_info else None + + if source_cb is None: + + @alias.callback() + def _emit_deprecation_warning() -> None: + warn_deprecated(old_name, new_name) + else: + + @alias.callback() + @functools.wraps(source_cb) + def _emit_deprecation_warning(*args, **kwargs): + warn_deprecated(old_name, new_name) + return source_cb(*args, **kwargs) + + parent.add_typer(alias, name=old_name, hidden=True) + return alias diff --git a/comfy_cli/discovery.py b/comfy_cli/discovery.py index f98e9493..9213b0de 100644 --- a/comfy_cli/discovery.py +++ b/comfy_cli/discovery.py @@ -78,6 +78,14 @@ "comfy skill show": "skill", "comfy skill status": "skill", # model discovery (all asset types: checkpoints, loras, controlnets, vae, ...) + # Canonical spelling under the `model` noun. + "comfy model search": "models", + "comfy model show": "models", + "comfy model list-folders": "models", + "comfy model list-folder": "models", + # `comfy models` is the hidden, deprecated plural alias; the discovery + # envelopes still carry the `models …` form in `command`, so both spellings + # register (mirrors the `skill`/`skills` alias above). "comfy models search": "models", "comfy models show": "models", "comfy models list-folders": "models", diff --git a/comfy_cli/skills/comfy/SKILL.md b/comfy_cli/skills/comfy/SKILL.md index 6f9a3649..7cc2550e 100644 --- a/comfy_cli/skills/comfy/SKILL.md +++ b/comfy_cli/skills/comfy/SKILL.md @@ -347,23 +347,23 @@ If no local server is running and you're not signed into cloud, pass ## Models — find what's installed, with metadata -On **cloud**, `comfy models search` hits the live asset catalog +On **cloud**, `comfy model search` hits the live asset catalog (`/api/assets`) and returns enriched rows: `name`, `type`, `tags`, `base_model`, `source_url`, `preview_url`, `size`. On **local**, the same command falls back to `/models/` listings (filenames only). ```bash -comfy --json models list-folders # every model folder (loras, checkpoints, vae, …) -comfy --json models list-folder loras # files in a folder, with pathIndex -comfy --json models search --text "wan2.2" --type lora --limit 10 -comfy --json models search --text "flux" # text search across the catalog -comfy --json models show # full Asset + projected row (cloud-only) +comfy --json model list-folders # every model folder (loras, checkpoints, vae, …) +comfy --json model list-folder loras # files in a folder, with pathIndex +comfy --json model search --text "wan2.2" --type lora --limit 10 +comfy --json model search --text "flux" # text search across the catalog +comfy --json model show # full Asset + projected row (cloud-only) ``` -`models search --type ` accepts the conventional folder names +`model search --type ` accepts the conventional folder names (`lora`/`loras`, `checkpoint`/`checkpoints`, `vae`, `controlnet`, `upscale`, `clip`, `clip_vision`, `unet`/`diffusion_models`, …). Use -`models list-folders` first if you're unsure what types the backend +`model list-folders` first if you're unsure what types the backend exposes. **Discover → wire loop — every asset type, never hardcoded names:** @@ -376,12 +376,12 @@ pattern is the same regardless of type: ```bash # 1. Discover available assets for any type -comfy --json models search --type lora --where cloud --text "detail" --limit 5 -comfy --json models search --type controlnet --where cloud --limit 5 -comfy --json models search --type checkpoint --where cloud --limit 5 -comfy --json models search --type vae --where cloud --limit 5 -comfy --json models search --type upscale --where cloud --limit 5 -comfy --json models search --type embeddings --where cloud --limit 5 +comfy --json model search --type lora --where cloud --text "detail" --limit 5 +comfy --json model search --type controlnet --where cloud --limit 5 +comfy --json model search --type checkpoint --where cloud --limit 5 +comfy --json model search --type vae --where cloud --limit 5 +comfy --json model search --type upscale --where cloud --limit 5 +comfy --json model search --type embeddings --where cloud --limit 5 # 2. Take rows[0].name verbatim — paste it into your fragment's required param @@ -396,8 +396,8 @@ recommendation.** On this backend, today, the survey returned the rows sketched below; yours will differ — pick from YOUR rows: ```bash -comfy --json models search --type checkpoint --where cloud --limit 5 # → picked from rows -comfy --json models search --type lora --where cloud --limit 5 # → picked from rows +comfy --json model search --type checkpoint --where cloud --limit 5 # → picked from rows +comfy --json model search --type lora --where cloud --limit 5 # → picked from rows # Learn the lora wiring from a real graph, not memory — fetch a matching template: comfy --json templates ls --type image --model ", from its row>" comfy templates fetch --out ref.json # read how it wires @@ -481,11 +481,11 @@ comfy --json nodes ls --produces AUDIO --limit 1 # AUDIO producer count comfy --json nodes ls --api-only --limit 1 # partner API node count comfy --json nodes categories --prefix "partner"# API provider categories comfy --json nodes types # all connection types -comfy --json models list-folders # all model folders +comfy --json model list-folders # all model folders comfy --json templates ls --limit 1 # template count ``` -The `total` field in `nodes ls`, `nodes search`, and `models search` +The `total` field in `nodes ls`, `nodes search`, and `model search` gives the full count even when `--limit` caps the returned rows. ## Workflows — what can I tweak? @@ -685,8 +685,8 @@ comfy --json nodes show # If error.code == "node_not_found", check details.close_matches # Confirm a model filename is actually available on the resolved backend (cloud-only) -# On local: use `comfy models list-folder ` instead -comfy --json models show +# On local: use `comfy model list-folder ` instead +comfy --json model show # If error.code == "model_not_found", check details.close_matches and pick one ``` @@ -849,11 +849,11 @@ Hard-won lessons per domain. Not a tutorial — a reference card. ## Image -- Survey first: `comfy nodes ls --produces IMAGE --api-only` (partner APIs), `comfy templates ls --type image`, `comfy models search --type checkpoint` — then choose +- Survey first: `comfy nodes ls --produces IMAGE --api-only` (partner APIs), `comfy templates ls --type image`, `comfy model search --type checkpoint` — then choose - Batch sweeps: `comfy workflow vary` for multi-prompt/seed generation - Text rendering: use Ideogram (IdeogramV3), NOT Flux — Flux garbles text - Partner API escape hatch (one-shots only, via the proxy — not a workflow Job): `comfy generate flux-ultra --prompt "..."` -- Never hardcode checkpoint/LoRA names — discover via `models search` +- Never hardcode checkpoint/LoRA names — discover via `model search` ## Video diff --git a/tests/comfy_cli/command/models/test_model_alias.py b/tests/comfy_cli/command/models/test_model_alias.py new file mode 100644 index 00000000..fd1adf15 --- /dev/null +++ b/tests/comfy_cli/command/models/test_model_alias.py @@ -0,0 +1,246 @@ +"""`models` (plural) → `model` (singular) merge + deprecation alias (BE-2999). + +Asserts the discovery leaves resolve under the singular `model` noun, that the +plural `models` spelling still works as a HIDDEN alias, and that invoking the +alias prints a single yellow deprecation warning to stderr while the canonical +`model` spelling stays warning-free. +""" + +from __future__ import annotations + +import inspect +import json + +import pytest +import typer +from typer.testing import CliRunner + +from comfy_cli.caller import Caller +from comfy_cli.cmdline import app +from comfy_cli.deprecation import add_deprecated_alias, deprecated_help +from comfy_cli.help_json import build_help_json, iter_command_paths +from comfy_cli.output.renderer import OutputMode, Renderer, reset_renderer_for_testing, set_renderer + +_DISCOVERY_LEAVES = ("search", "show", "list-folders", "list-folder") + + +@pytest.fixture(autouse=True) +def reset_singleton(): + reset_renderer_for_testing() + yield + reset_renderer_for_testing() + + +# --------------------------------------------------------------------------- +# Resolution: the discovery leaves live under `model`, and `models` still works. +# --------------------------------------------------------------------------- + + +class TestResolution: + def test_discovery_leaves_resolve_under_model(self): + paths = set(iter_command_paths(app)) + for leaf in _DISCOVERY_LEAVES: + assert f"comfy model {leaf}" in paths, (leaf, sorted(p for p in paths if "model" in p)) + + def test_local_ops_still_under_model(self): + # The merge must not displace the local-filesystem ops. + paths = set(iter_command_paths(app)) + for leaf in ("download", "remove", "list"): + assert f"comfy model {leaf}" in paths, leaf + + def test_plural_alias_still_resolves(self): + paths = set(iter_command_paths(app)) + for leaf in _DISCOVERY_LEAVES: + assert f"comfy models {leaf}" in paths, leaf + + def test_models_group_is_hidden_model_is_visible(self): + commands = build_help_json(app)["commands"] + assert commands["models"]["hidden"] is True + assert commands["model"]["hidden"] is False + # The deprecation banner rides on the hidden group's help. + assert "DEPRECATED" in (commands["models"]["help"] or "") + + +# --------------------------------------------------------------------------- +# Helper unit tests — isolated from the real command tree. +# --------------------------------------------------------------------------- + + +class TestHelper: + def test_deprecated_help_prefixes_banner(self): + assert deprecated_help("model", "Discover things.") == "[DEPRECATED — use `comfy model`] Discover things." + # Empty original help degrades to the bare banner. + assert deprecated_help("model") == "[DEPRECATED — use `comfy model`]" + + def _build_parent_with_alias(self): + source = typer.Typer(help="Do a thing.") + + @source.command("go") + def go(): + typer.echo("ran") + + parent = typer.Typer() + parent.add_typer(source, name="thing") + add_deprecated_alias(parent, source, old_name="things", new_name="thing") + return parent + + def test_alias_invocation_warns_and_runs(self, capsys): + parent = self._build_parent_with_alias() + result = CliRunner().invoke(parent, ["things", "go"]) + assert result.exit_code == 0, result.output + # The wrapped command still executes (reused implementation). + assert "ran" in result.output + # And the yellow warning fires — rprint in pretty mode goes to stdout. + combined = result.output + capsys.readouterr().err + assert "deprecated" in combined.lower() + assert "'comfy things …'" in combined + + def test_canonical_invocation_does_not_warn(self, capsys): + parent = self._build_parent_with_alias() + result = CliRunner().invoke(parent, ["thing", "go"]) + assert result.exit_code == 0, result.output + assert "ran" in result.output + combined = result.output + capsys.readouterr().err + assert "deprecated" not in combined.lower() + + def test_alias_carries_nested_sub_groups(self): + # A source app with a nested sub-group (add_typer) must have that group + # carried onto the alias too — not silently dropped — and the warning + # still fires for leaves reached through the nested group. + source = typer.Typer(help="Root.") + sub = typer.Typer() + + @sub.command("leaf") + def leaf(): + typer.echo("leaf-ran") + + source.add_typer(sub, name="sub") + + parent = typer.Typer() + parent.add_typer(source, name="thing") + add_deprecated_alias(parent, source, old_name="things", new_name="thing") + + result = CliRunner().invoke(parent, ["things", "sub", "leaf"]) + assert result.exit_code == 0, result.output + assert "leaf-ran" in result.output + assert "deprecated" in result.output.lower() + + def test_alias_composes_source_callback(self, capsys): + # A source app that declares its own group callback (with an option) must + # have both the deprecation warning AND its own callback run, with the + # callback's option preserved on the alias. + seen: list[bool] = [] + source = typer.Typer() + + @source.callback() + def _cb(flag: bool = typer.Option(False, "--flag")): + seen.append(flag) + + @source.command("go") + def go(): + typer.echo("ran") + + parent = typer.Typer() + parent.add_typer(source, name="thing") + add_deprecated_alias(parent, source, old_name="things", new_name="thing") + + result = CliRunner().invoke(parent, ["things", "--flag", "go"]) + assert result.exit_code == 0, result.output + assert "ran" in result.output + combined = result.output + capsys.readouterr().err + assert "deprecated" in combined.lower() + # The source callback ran, and its --flag option resolved on the alias. + assert seen == [True], seen + + +# --------------------------------------------------------------------------- +# End-to-end through the real root app: `comfy models list-folders` warns, +# `comfy model list-folders` does not. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def cloud_target(monkeypatch: pytest.MonkeyPatch): + from comfy_cli.target import Target + + fake = Target( + kind="cloud", + base_url="https://cloud.example.com", + path_prefix="/api", + history_path="history_v2", + jobs_path="jobs", + api_key="test-api-key", + ) + monkeypatch.setattr("comfy_cli.target.resolve_target", lambda **kw: fake) + return fake + + +def _fake_resp(body: bytes): + class _Resp: + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def read(self, n: int | None = None): + return body if n is None else body[:n] + + return _Resp() + + +def _patch_urlopen(monkeypatch: pytest.MonkeyPatch, payload): + def _fake(req, timeout=None): + return _fake_resp(json.dumps(payload).encode()) + + monkeypatch.setattr("urllib.request.urlopen", _fake) + + +def _force_json_renderer(): + r = Renderer.resolve( + is_stdout_tty=False, + env={}, + caller=Caller(kind="user", agentic=False, source_env=None), + json_flag=True, + ) + r.mode = OutputMode.JSON + set_renderer(r) + + +_CLOUD_FOLDERS = [{"name": "checkpoints", "folders": ["checkpoints"]}] + + +def _stderr_split_runner() -> CliRunner: + # Keep the envelope (stdout) and the deprecation warning (stderr) separable so + # the JSON contract can assert stdout stays envelope-only. click <8.2 needs the + # explicit mix_stderr=False; click >=8.2 dropped the kwarg and always splits. + if "mix_stderr" in inspect.signature(CliRunner.__init__).parameters: + return CliRunner(mix_stderr=False) + return CliRunner() + + +class TestEndToEnd: + def _invoke(self, monkeypatch, noun): + _force_json_renderer() + _patch_urlopen(monkeypatch, _CLOUD_FOLDERS) + return _stderr_split_runner().invoke( + app, ["--json", noun, "list-folders", "--where", "cloud"], standalone_mode=False + ) + + def test_plural_invocation_warns_on_stderr(self, cloud_target, monkeypatch): + result = self._invoke(monkeypatch, "models") + # Command still works: an ok envelope lands on stdout. + stdout = (result.stdout or "").strip() + assert stdout, f"no envelope on stdout (rc={result.exit_code}, exc={result.exception})" + env = json.loads(stdout.splitlines()[-1]) + assert env["ok"] is True, env + # Deprecation warning fired to stderr (JSON mode keeps stdout clean). + assert "deprecated" in result.stderr.lower(), result.stderr + + def test_singular_invocation_is_silent(self, cloud_target, monkeypatch): + result = self._invoke(monkeypatch, "model") + stdout = (result.stdout or "").strip() + assert stdout, f"no envelope on stdout (rc={result.exit_code}, exc={result.exception})" + env = json.loads(stdout.splitlines()[-1]) + assert env["ok"] is True, env + assert "deprecated" not in result.stderr.lower(), result.stderr diff --git a/tests/comfy_cli/output/test_discovery.py b/tests/comfy_cli/output/test_discovery.py index b4c699a8..c4eb48e9 100644 --- a/tests/comfy_cli/output/test_discovery.py +++ b/tests/comfy_cli/output/test_discovery.py @@ -128,6 +128,12 @@ def test_models_and_templates_registered(): from comfy_cli.discovery import COMMAND_SCHEMAS for cmd in ( + # Canonical singular `model` noun (BE-2999). + "comfy model search", + "comfy model show", + "comfy model list-folders", + "comfy model list-folder", + # Deprecated plural alias — still registered (envelopes carry `models …`). "comfy models search", "comfy models show", "comfy models list-folders",