feat: merge models discovery into the model noun + deprecation-alias helper (BE-2999)

[mattmillerai]

ELI-5

comfy had two confusingly-split command groups: comfy model (singular — your
local model files on disk: download / remove / list) and comfy models (plural —
asking a backend/cloud server what models exist: list-folders / list-folder /
search / show). Two nouns for "models" is easy to fumble, and model list vs
models list-folder read like typos of each other.

This merges everything under the single model noun. Now comfy model search,
comfy model show, comfy model list-folders, and comfy model list-folder all
work. The old comfy models … spelling still works exactly as before, but it's
hidden from --help and prints a one-line yellow "deprecated; use comfy model …"
warning to stderr. Nothing breaks.

What changed

• New shared helper comfy_cli/deprecation.py — add_deprecated_alias(parent, source_app, old_name, new_name) mounts a hidden alias of a Typer group whose
  group help is prefixed [DEPRECATED — use <new>] and which emits a single
  [yellow] warning to stderr on invocation (never silenced — the warning is
  the point, and stderr keeps the JSON-envelope-on-stdout contract intact). The
  sibling noun consolidations reuse this helper.
• model now owns the discovery leaves — the four leaves are surfaced under
  comfy model by borrowing search.app's command registrations (the same
  CommandInfo objects — the implementations are reused, not duplicated). This
  mirrors the existing skill/skills alias reuse pattern.
• models is a hidden, deprecated alias wired through the new helper; its four
  leaves still work and print the warning.
• Help text disambiguated — comfy model list says "local models on disk";
  list-folders/list-folder say "backend/cloud", so list vs list-folder no
  longer reads as a typo.
• Self-describing surface — registered the canonical comfy model … paths in
  COMMAND_SCHEMAS (keeping the plural too, since the emitted envelope command
  field still carries models …, so both spellings must resolve — same dual-reg
  as skill/skills). Updated the bundled SKILL.md examples to the canonical
  spelling.

Non-breaking

Yes. Every old comfy models … invocation still works via the alias, and the JSON
envelope command field is unchanged (models list-folders, etc.) — no contract
change for JSON consumers.

Tests

• New tests/comfy_cli/command/models/test_model_alias.py: asserts the four
  discovery leaves resolve under comfy model; the local ops (download/remove/list)
  are undisplaced; comfy models … still resolves but is hidden with the deprecation
  banner; unit tests for the helper; and end-to-end that comfy models list-folders
  emits an ok envelope on stdout and the warning on stderr, while
  comfy model list-folders is warning-free.
• Extended tests/comfy_cli/output/test_discovery.py to require the canonical
  comfy model … schema registrations alongside the plural.
• Full suite green: 2480 passed, 13 skipped. ruff format --check + ruff check
  clean on all touched files. (The 14 pre-existing repo-wide UP038 lint findings
  are in untouched files — not introduced here.)

Judgment calls

• Kept the envelope command field as models … (not model …) to stay strictly
  non-breaking for JSON consumers; that's why both spellings are registered in
  COMMAND_SCHEMAS, exactly like the pre-existing skill/skills alias.
• @tracking.track_command("models") on the reused leaves is left as-is (a cosmetic
  analytics group label), out of scope for this rename.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e6275689-e93c-495f-a465-3d2d77559425

📥 Commits

Reviewing files that changed from the base of the PR and between c5db184 and ad92035.

📒 Files selected for processing (5)

• comfy_cli/cmdline.py
• comfy_cli/command/models/models.py
• comfy_cli/command/models/search.py
• comfy_cli/discovery.py
• comfy_cli/skills/comfy/SKILL.md

📝 Walkthrough

Walkthrough

The CLI now uses comfy model as the unified model-management namespace. comfy models remains available as a hidden deprecated alias, with warnings routed separately from JSON output. Help text, discovery schemas, documentation, and tests reflect both spellings.

Changes

Model command migration

Layer / File(s)	Summary
Deprecated alias helper comfy_cli/deprecation.py	Adds hidden Typer aliases with deprecation help, warnings, copied commands, nested groups, and callback preservation.
Canonical command wiring and guidance comfy_cli/cmdline.py, comfy_cli/command/models/..., comfy_cli/discovery.py, comfy_cli/skills/comfy/SKILL.md	Mounts discovery and local operations under model, retains models through the deprecated alias, and updates help, schemas, and examples.
Alias and discovery validation tests/comfy_cli/command/models/test_model_alias.py, tests/comfy_cli/output/test_discovery.py	Tests canonical and deprecated resolution, warnings, callbacks, nested groups, JSON output, help metadata, and schema registration.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant model
  participant modelsAlias
  participant Command
  CLI->>model: invoke comfy model search
  model->>Command: execute discovery command
  Command-->>CLI: return command result
  CLI->>modelsAlias: invoke comfy models search
  modelsAlias->>modelsAlias: emit deprecation warning
  modelsAlias->>Command: execute aliased command
  Command-->>CLI: return command result

Loading

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-2999-merge-models-into-model

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-2999-merge-models-into-model

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

⚠️ Review failed

Judge call failed (status=parse_error): Could not parse JSON findings from output. First 500 chars:
Based on my investigation of the actual code, I can now adjudicate the panel findings.

Key verifications:
- `comfy_cli/command/models/search.py`'s `app` has **no `@​app.callback()` and no sub-groups** (`add_typer`) — only four `@​app.command` leaves. So gemini's findings 3/4/5 (dropped `registered_groups`/`registered_callback`) describe **no current bug**; they're a latent robustness trap for the documented-reusable helper. These three are near-duplicates of one root cause and merge into one.
- `

Re-trigger by removing and re-adding the cursor-review label.

[mattmillerai]

Addressed the one substantive point surfaced by the Cursor panel (its judge crashed with a parse_error and posted no formal thread, but the leaked reasoning flagged a real latent trap): add_deprecated_alias is documented as a reusable helper for the sibling noun consolidations, yet it only copied registered_commands, silently dropping nested sub-groups (add_typer) and the source app's own group callback.

Harmless for the current models→model mount (a flat, callback-less command tree — verified search.py's app has 4 leaf commands, no sub-groups, no callback), but a robustness trap for future reuse. The helper now also carries registered_groups and, when the source declares a callback, composes it with the deprecation warning (functools.wraps preserves its option signature) instead of overriding it. Added regression tests for both cases (test_alias_carries_nested_sub_groups, test_alias_composes_source_callback). The source_cb is None path is byte-identical to before, so zero behavior change for the shipped feature.

The failing Windows test check was an infra flake, not code — uv couldn't remove pydantic_core/_pydantic_core.cp312-win_amd64.pyd during dependency install (Access is denied. (os error 5)), a Windows file-lock race in the install step; re-running.

[mattmillerai]

Re-ran the failed `Windows Specific Commands` job; it failed again in the dependency-install step (not the test run):

```
error: failed to remove file ...\pydantic_core/_pydantic_core.cp312-win_amd64.pyd: Access is denied. (os error 5)
→ CalledProcessError (uv pip install) → ImportError: cannot import name 'version' from 'pydantic_core'
```

Confirmed repo-wide, pre-existing infra flake — the same job is currently red on every open branch (be-3000, be-2994, be-2982, be-2978, be-3001, be-2975, …), so it is unrelated to this diff (model discovery + deprecation alias, no dependency changes). Fixing it belongs in the CI workflow, not this PR.

The model/models code itself is green: the 24 alias + discovery tests pass locally, and `build`, `ruff_check`, CodeQL, and Socket all pass. Nothing actionable remains on this PR.

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 3 finding(s).

Severity	Count
🟡 Medium	2
⚪ Nit	1

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)

[mattmillerai]

Resolver pass — nothing actionable left on this PR; it is ready to merge.

Review threads: all 3 Cursor panel findings (sub-group mirroring on the canonical mount, get_type_hints resolution against the source callback's globals, and the stderr docstring wording) were fixed in c5db184 and their threads are resolved. CodeRabbit's latest run reports no actionable comments. No merge conflict — the branch is MERGEABLE against main.

Verified locally (in an isolated worktree, at c5db184): full suite 2584 passed, 37 skipped; ruff format --check + ruff check clean on all touched files. Also exercised the real CLI rather than trusting tests alone — comfy model --help lists all seven leaves (download/remove/list + list-folders/list-folder/search/show), comfy models is absent from root --help yet still resolves with the [DEPRECATED — use comfy model] banner. Non-breaking as claimed.

The red Windows Specific Commands check is a pre-existing repo-wide CI bug, unrelated to this diff, and I have now root-caused it rather than just re-running it. It fails in Install Dependencies, before any test executes:

error: failed to remove file `...\venv\Lib\site-packages\pydantic_core\_pydantic_core.cp312-win_amd64.pyd`: Access is denied. (os error 5)
  -> CalledProcessError (the uv pip install behind --fast-deps)
  -> ImportError: cannot import name '__version__' from 'pydantic_core' (unknown location)

test-windows.yml:29-35 creates venv, activates it, pip install -e ., then runs comfy … install --fast-deps. That comfy process runs from venv, and --fast-deps has uv reinstall pydantic_core into that same venv — Windows cannot unlink a .pyd a live process has loaded, so the replace aborts and leaves the package gutted. It is intermittent because it only fires when the resolver picks a different pydantic_core build. Consistent with the evidence: the workflow is currently failing on ~every open branch (be-3001, be-2975, be-2982, be-3311, be-3297, be-3300, be-3285, be-3276 …) with one sporadic success, and this PR changes no dependency, packaging metadata, or workflow file — it only trips the paths: comfy_cli/** trigger.

The fix (give the ComfyUI workspace its own env so comfy install never mutates the interpreter's own venv) is a CI-config change that cannot be validated from macOS, so I have deferred it to a follow-up ticket rather than bolt it onto this PR. Left unlabeled for human triage since choosing among the fix options is a design call.

[mattmillerai]

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

• https://linear.app/comfyorg/issue/BE-3323|BE-3323 — Fix repo-wide 'Windows Specific Commands' CI failure: uv cannot replace pydantic_core in the in-use venv

[mattmillerai]

Status check: all three cursor-review threads (canonical model mount sub-group asymmetry, functools.wraps hint resolution against the wrong globals, and the stderr docstring wording) were addressed in c5db184 and are resolved.

Local suite on the PR head is green: 2584 passed, 37 skipped; ruff format --check and ruff check clean on all touched files.

The one red check — Windows Specific Commands / test — is not caused by this PR. It fails in the "Install Dependencies" step, where uv pip install of ComfyUI's cu126 requirements.compiled exits 2 and leaves a broken pydantic_core (ImportError: cannot import name '__version__' from 'pydantic_core' (unknown location)). That workflow is currently failing on essentially every branch in the repo (14 of the last 15 runs across unrelated PRs), so it's a repo-wide infra breakage rather than a regression here.

[bigcat88]

This PR currently conflicts with main — GitHub reports mergeable: CONFLICTING, so it needs a rebase before I can review it and I'm skipping it in the current review sweep.

Please rebase (or merge main in) and I'll pick it up on the next pass. main moved a fair bit in the last day, including #614 (ANSI sanitisation across the pretty-print call sites) and #628 (the duplicate server_died error-code fix that had main red), so a refresh may also clear unrelated CI noise on this branch.