fix(help): correct skill count, disable-gui verb, missing help strings, install error tone#514
fix(help): correct skill count, disable-gui verb, missing help strings, install error tone#514mattmillerai wants to merge 1 commit into
Conversation
…s, install error tone - skills: derive bundled-skill count from len(BUNDLED_SKILLS) instead of the stale literal '4' (there are 5); restore 'Aider' in the skills app help that the add_typer registration override had dropped - manager disable-gui: help now describes disabling the GUI, not enabling it - add help strings to 'tracking enable/disable' and explicit help to 'model remove/list' (previously relied on docstrings; audit's scan missed them) - install: replace meme error strings with actionable messages stating the detected condition and the fix; the --m-series-on-non-Mac warning is now a yellow warning that matches its non-fatal behavior
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
✅ No high-signal findings.
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
|
Reviews-resolver pass: no unresolved review threads, and the Cursor panel reported no high-signal findings (CodeRabbit was rate-limited and produced no findings). Ruff check + format are clean and the full pytest matrix (ubuntu/macos/windows + GPU) is green. The one red check — Windows Specific Commands — is a repo-wide runner infra issue, not this diff: it fails identically on every PR opened today (BE-2999, BE-3001, BE-2994, BE-2982, BE-2975) with |
ELI-5
Several
comfy ... --helpblurbs were wrong or missing: the skills installer bragged about "4 bundled skills" when there are actually 5, themanager disable-guicommand's help said it enables the GUI, a couple of commands had no help at all, and the installer greeted a bad--nvidia/--m-seriescombo with memes ("What are you smoking? 🤔") instead of telling you what went wrong and how to fix it. This PR makes the help text tell the truth.What changed
comfy skills install --helpandcomfy skills show --helpsaid "all 4 bundled skills";BUNDLED_SKILLShas 5. Both now interpolatelen(BUNDLED_SKILLS)so the count can never drift again.comfy skillshelp came from theadd_typer(..., help=...)override, which listed only "Claude Code, Cursor, and AGENTS.md" — dropping Aider that the sub-app's own help advertises. The registration now lists the full set ("Claude Code, Cursor, Aider, and any AGENTS.md-aware tool").manager disable-guiverb fixed. Help read "Enable ComfyUI-Manager without GUI" → now "Enable ComfyUI-Manager but disable its GUI" (matches the command's docstring and behavior).tracking enable/tracking disablehad no help or docstring — added.model remove/model listalready rendered help via their docstrings, but the audit's decorator scan flagged them; added explicithelp=(belt-and-suspenders, and consistent withmodel download).installare replaced with actionable text stating the detected condition + the fix:--nvidiaon macOS (fatal, unchanged behavior): now explains macOS has no NVIDIA GPU and to re-run with--m-series/--cpuor omit the flag to pick interactively.--m-serieson a non-Mac platform (non-fatal — see judgment call): now a yellow warning naming the detected platform and the alternative flags.Judgment calls
--m-series-on-non-Mac stays non-fatal. The ticket asked to "confirm intended behavior and make the message match reality." The existing code only warns and then proceeds (treating the machine as Apple silicon); it does not exit. I preserved that behavior (Chesterton's fence) and made the message ayellowwarning that honestly describes the continuation, rather than adding a new exit. If a hard exit is actually wanted, that's a behavior change worth its own discussion.model remove/model listalready had help (via docstrings, verified by renderingcomfy model --help). The audit listed them as "missing help strings entirely" because a decorator-level scan doesn't see docstrings. Added explicithelp=anyway so the surface is uniform and robust to future docstring edits.Testing
ruff format+ruff checkclean on all touched files.--helpfor the skills, tracking, and model apps to confirm the new strings and the derived count ("5").