From 18e8f37fb6eef217515b43725de9c86761acfc8a Mon Sep 17 00:00:00 2001 From: Anne He Date: Tue, 15 Sep 2026 14:51:21 -0700 Subject: [PATCH 1/3] feat(knowledge): flag oss picks a local ComfyUI cannot run with `pick --check-local` (BE-8974) `available_locally` on a knowledge pick never looked at model files, so the best oss pick for a capability could be one whose weights are not on this machine. `comfy templates check` already answers that per template, and every pick names its template. `knowledge pick --check-local` runs that check on each oss pick. A pick with missing files gets `available_locally: false`, an `unavailable_reason` and a `missing_models` count. A template absent from the gallery is flagged too. `local_check` says whether the check ran. When the server is down, the gallery cannot load or a folder listing is over the size cap, it carries that code and nothing is flagged. Off by default because it fetches uncached workflows and calls the local server. missing_models is a count because pick envelopes are kept under the 4096 bytes the cloud agent passes through unchanged. With every oss pick flagged, the largest capability under that cap grows from 3650 to 4095 bytes. image-edit was already over at 4272 before this change. The gallery lookup, workflow fetch and folder matching move out of check_cmd into helpers both commands call. templates check output is unchanged. The pick check loads the gallery without stale-while-revalidate, like show and fetch, so a stale index cannot flag a new template. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 8 ++ comfy_cli/command/knowledge.py | 85 +++++++++++- comfy_cli/command/templates.py | 215 +++++++++++++++++------------- comfy_cli/knowledge.py | 3 + comfy_cli/schemas/knowledge.json | 8 +- comfy_cli/skills/comfy/SKILL.md | 5 + tests/comfy_cli/test_knowledge.py | 133 ++++++++++++++++++ 7 files changed, 364 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f5a3d0f..c7272a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,14 @@ history. release, or one of the build's releases). The builder's message is carried whole up to 8 KiB, so the blocking deployment ids it names are no longer lost to the 1000-byte cap on the raw body. +- `comfy knowledge pick CAPABILITY --check-local` checks each `oss` pick's + template against the local ComfyUI's model folders, the same check + `comfy templates check` runs. A pick whose model files are missing gets + `available_locally: false`, an `unavailable_reason` and a `missing_models` + count, and a template absent from the gallery is flagged too. `local_check` + is `ok` when the check ran; when the server is down or the gallery cannot load + it carries that error code and no pick is flagged. The flag is off by default + because it fetches uncached template workflows and calls the local server. ### Fixed diff --git a/comfy_cli/command/knowledge.py b/comfy_cli/command/knowledge.py index d5449206..32e7f321 100644 --- a/comfy_cli/command/knowledge.py +++ b/comfy_cli/command/knowledge.py @@ -2,7 +2,7 @@ comfy knowledge status [--refresh] comfy knowledge resolve - comfy knowledge pick [capability] + comfy knowledge pick [capability] [--check-local] Backed by :mod:`comfy_cli.knowledge`. JSON mode is the contract; pretty mode is a short courtesy view. @@ -181,6 +181,71 @@ def _emit_capabilities(renderer, bundle: knowledge.Bundle, *, query: str | None renderer.emit(payload, command="knowledge pick") +def _check_picks_locally(picks: list[dict[str, Any]]) -> str: + """Flag each ``oss`` pick whose template the local ComfyUI cannot run. + + Returns ``ok`` when the check ran. A pick whose workflow could not be + fetched or parsed is skipped and stays unflagged. A failure that would + repeat for every pick (server down, no gallery) returns its error code + instead and flags nothing. + """ + from comfy_cli.command import templates as templates_cmd + from comfy_cli.http import ResponseTooLarge + + flags: list[tuple[dict[str, Any], dict[str, Any]]] = [] + listings: dict[str, list[str] | None] = {} + rows: list[dict[str, Any]] | None = None + try: + for pick in picks: + template = pick.get("template") + if pick.get("route") != "oss" or not template: + continue + if rows is None: + # Not stale-while-revalidate: a stale index would flag a template + # added upstream since as missing from the gallery. + try: + rows = templates_cmd._gallery_rows(None, refresh=False, background_ok=False) + except templates_cmd._GALLERY_LOAD_ERRORS: + return "gallery_load_failed" + try: + _row, wf = templates_cmd._template_workflow(template, rows, refresh=False) + except (RuntimeError, ResponseTooLarge): + continue + except templates_cmd.TemplateCheckError as e: + if e.code == "template_not_found": + flags.append( + ( + pick, + { + "available_locally": False, + "unavailable_reason": knowledge.UNAVAILABLE_TEMPLATE_NOT_FOUND, + }, + ) + ) + continue + _present, missing, _warnings = templates_cmd._match_local_models( + templates_cmd._collect_model_requirements(wf), listings + ) + if missing: + flags.append( + ( + pick, + { + "available_locally": False, + "unavailable_reason": knowledge.UNAVAILABLE_MISSING_MODELS, + "missing_models": len(missing), + }, + ) + ) + except templates_cmd.TemplateCheckError as e: + return e.code + except ResponseTooLarge: + return "model_listing_too_large" + for pick, flag in flags: + pick.update(flag) + return "ok" + + @app.command("pick", help="Ranked model picks for a capability; omit it to list every capability.") @tracking.track_command("knowledge") def pick_cmd( @@ -188,6 +253,16 @@ def pick_cmd( str | None, typer.Argument(help="Capability id (e.g. lipsync, text-to-video). Omit to list every capability."), ] = None, + check_local: Annotated[ + bool, + typer.Option( + "--check-local", + help=( + "Check each oss pick's template against the local ComfyUI's model folders and flag " + "the picks it cannot run. Fetches uncached template workflows and calls the local server." + ), + ), + ] = False, ): renderer = get_renderer() bundle = _require_bundle(renderer) @@ -218,15 +293,21 @@ def pick_cmd( "compiled_at": bundle.compiled_at, "stale": bundle.stale, } + if check_local: + payload["local_check"] = _check_picks_locally(picks) if renderer.is_pretty(): from rich.table import Table columns = ("rank", "model", "route", "template", "status", "caveat", "best_for") + if check_local: + columns += ("unavailable_reason",) tbl = Table(show_header=True, header_style="bold") for col in columns: tbl.add_column(col) for p in picks: cells = {**p, "best_for": ", ".join(p.get("best_for") or [])} - tbl.add_row(*(sanitize_markup("" if cells[c] is None else cells[c]) for c in columns)) + tbl.add_row(*(sanitize_markup("" if cells.get(c) is None else cells[c]) for c in columns)) renderer.console().print(tbl) + if check_local and payload["local_check"] != "ok": + rprint(f"[yellow]local check did not run:[/yellow] {payload['local_check']}") renderer.emit(payload, command="knowledge pick") diff --git a/comfy_cli/command/templates.py b/comfy_cli/command/templates.py index 578ff58f..16e315d4 100644 --- a/comfy_cli/command/templates.py +++ b/comfy_cli/command/templates.py @@ -1365,53 +1365,41 @@ def _compute_verdict(*, api_dependent: bool, missing: list, required_count: int, return "unknown" if loaderish else "runnable" -@app.command( - "check", - help=( - "Report whether a gallery template is runnable on THIS install: which of " - "its models are present vs missing locally, whether it needs partner-API " - "access, and any custom nodes it declares. Resolves the name against the " - "gallery, fetches the workflow, and intersects its model refs with the " - "local server's model folders." - ), -) -@tracking.track_command("templates") -def check_cmd( - name: Annotated[str, typer.Argument(help="Template name (matches `comfy templates ls` rows).")], - gallery_path: Annotated[ - str | None, - typer.Option("--gallery", show_default=False, help="Path to a local index.json (skips the cache + fetch)."), - ] = None, - refresh: Annotated[ - bool, - typer.Option("--refresh", help="Re-fetch the gallery index AND the template workflow before checking."), - ] = False, -): - from comfy_cli.target import resolve_target +class TemplateCheckError(Exception): + """A template-check failure, carrying the fields of its error envelope.""" + + def __init__(self, code: str, message: str, *, hint: str | None = None, details: dict[str, Any] | None = None): + super().__init__(message) + self.code = code + self.message = message + self.hint = hint + self.details = details - renderer = get_renderer() - # 1. Resolve the name against the gallery index (same affordance as `fetch`). +def _gallery_rows(gallery_path: str | None, *, refresh: bool, background_ok: bool = True) -> list[dict[str, Any]]: try: - cats = _load_gallery(gallery_path, refresh=refresh) + cats = _load_gallery(gallery_path, refresh=refresh, background_ok=background_ok) except (urllib.error.URLError, OSError, json.JSONDecodeError) as e: - renderer.error(code="gallery_load_failed", message=str(e)) - raise typer.Exit(code=1) from e + raise TemplateCheckError("gallery_load_failed", str(e)) from e + return _flatten_templates(cats) - rows = _flatten_templates(cats) + +def _template_workflow( + name: str, rows: list[dict[str, Any]], *, refresh: bool +) -> tuple[dict[str, Any], dict[str, Any]]: + """Resolve ``name`` against the gallery ``rows`` and return ``(row, workflow)``, + reading the workflow JSON from the per-template cache or fetching it.""" match = next((r for r in rows if r["name"] == name), None) if match is None: lower = name.lower() close = [r["name"] for r in rows if lower in r["name"].lower()][:5] - renderer.error( - code="template_not_found", - message=f"no template named {name!r} in the gallery", + raise TemplateCheckError( + "template_not_found", + f"no template named {name!r} in the gallery", hint="try `comfy templates ls --name ` to search", details={"close_matches": close}, ) - raise typer.Exit(code=1) - # 2. Fetch (or read from cache) the per-template workflow JSON. cache_path = _template_workflow_cache_path(name) body: bytes | None = None if not refresh and cache_path.exists(): @@ -1424,9 +1412,9 @@ def check_cmd( body = _fetch_template_workflow(name) except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e: status = getattr(e, "code", None) - renderer.error( - code="template_fetch_failed", - message=f"failed to fetch workflow for {name!r}: {e}", + raise TemplateCheckError( + "template_fetch_failed", + f"failed to fetch workflow for {name!r}: {e}", hint=( "the gallery index references a template whose workflow JSON " "is missing upstream — report at " @@ -1435,8 +1423,7 @@ def check_cmd( else "check network connectivity" ), details={"status": status} if status else None, - ) - raise typer.Exit(code=1) from e + ) from e # Write atomically: a truncated file (interrupted write / full disk) would # otherwise be trusted by the read path above on the next non-refresh run # and fail `template_workflow_invalid_json` until the user passed --refresh. @@ -1455,19 +1442,105 @@ def check_cmd( try: wf = json.loads(body) except (json.JSONDecodeError, UnicodeDecodeError) as e: - renderer.error( - code="template_workflow_invalid_json", - message=f"template workflow for {name!r} is not valid JSON: {e}", + raise TemplateCheckError( + "template_workflow_invalid_json", + f"template workflow for {name!r} is not valid JSON: {e}", hint="re-run with --refresh to re-fetch, or report upstream", - ) - raise typer.Exit(code=1) from e + ) from e if not isinstance(wf, dict): - renderer.error( - code="template_workflow_invalid_json", - message=f"template workflow for {name!r} is not a JSON object", + raise TemplateCheckError( + "template_workflow_invalid_json", + f"template workflow for {name!r} is not a JSON object", hint="re-run with --refresh to re-fetch, or report upstream", ) - raise typer.Exit(code=1) + return match, wf + + +def _match_local_models( + required: list[dict[str, str]], listings: dict[str, list[str] | None] +) -> tuple[list[str], list[dict[str, str]], list[str]]: + """Split ``required`` into ``(present, missing, warnings)`` against the local + server's model folders, matching by basename. + + ``listings`` caches each folder's listing by directory, so a caller checking + several templates lists a shared folder once. + """ + warnings: list[str] = [] + present: list[str] = [] + missing: list[dict[str, str]] = [] + if not required: + return present, missing, warnings + + from comfy_cli.target import resolve_target + + target = resolve_target(where="local") + try: + for directory in dict.fromkeys(req["directory"] for req in required): + if not directory or ".." in directory or "/" in directory or "\\" in directory: + # Not addressable as a `/models/` segment — treat as absent. + listings[directory] = None + warnings.append( + f"model directory {directory!r} isn't a valid model folder — its files are reported missing" + ) + continue + if directory not in listings: + listings[directory] = _list_local_folder(target, directory) + if listings[directory] is None: + warnings.append( + f"model folder {directory!r} not found on the local server " + f"(custom-node folder?) — its files are reported missing" + ) + except (urllib.error.URLError, OSError, ValueError, json.JSONDecodeError) as e: + raise TemplateCheckError( + "server_not_running", + f"local ComfyUI server is unreachable, cannot check installed models: {e}", + hint="run `comfy launch` to start a local server", + ) from e + + for req in required: + listing = listings.get(req["directory"]) + # Normalize BOTH sides: a model ref may itself carry a subfolder + # (e.g. ``SDXL/model.safetensors``, as ComfyUI loader widgets emit), + # so compare basenames on the required side too. + req_base = _basename(req["name"]) + if listing and any(_basename(entry) == req_base for entry in listing): + present.append(req["name"]) + else: + missing.append(dict(req)) + return present, missing, warnings + + +@app.command( + "check", + help=( + "Report whether a gallery template is runnable on THIS install: which of " + "its models are present vs missing locally, whether it needs partner-API " + "access, and any custom nodes it declares. Resolves the name against the " + "gallery, fetches the workflow, and intersects its model refs with the " + "local server's model folders." + ), +) +@tracking.track_command("templates") +def check_cmd( + name: Annotated[str, typer.Argument(help="Template name (matches `comfy templates ls` rows).")], + gallery_path: Annotated[ + str | None, + typer.Option("--gallery", show_default=False, help="Path to a local index.json (skips the cache + fetch)."), + ] = None, + refresh: Annotated[ + bool, + typer.Option("--refresh", help="Re-fetch the gallery index AND the template workflow before checking."), + ] = False, +): + renderer = get_renderer() + + # 1-2. Resolve the name against the gallery index (same affordance as `fetch`) + # and read the per-template workflow JSON from cache or fetch it. + try: + match, wf = _template_workflow(name, _gallery_rows(gallery_path, refresh=refresh), refresh=refresh) + except TemplateCheckError as e: + renderer.error(code=e.code, message=e.message, hint=e.hint, details=e.details) + raise typer.Exit(code=1) from e # 3. Model requirements (top-level + subgraph walk) and node class types. required = _collect_model_requirements(wf) @@ -1498,47 +1571,11 @@ def check_cmd( # 5. Installed intersection: list each distinct folder once on the local server # and match required files by basename. - warnings: list[str] = [] - present: list[str] = [] - missing: list[dict[str, str]] = [] - distinct_dirs = list(dict.fromkeys(req["directory"] for req in required)) - if required: - target = resolve_target(where="local") - listings: dict[str, list[str] | None] = {} - try: - for directory in distinct_dirs: - if not directory or ".." in directory or "/" in directory or "\\" in directory: - # Not addressable as a `/models/` segment — treat as absent. - listings[directory] = None - warnings.append( - f"model directory {directory!r} isn't a valid model folder — its files are reported missing" - ) - continue - folder_files = _list_local_folder(target, directory) - listings[directory] = folder_files - if folder_files is None: - warnings.append( - f"model folder {directory!r} not found on the local server " - f"(custom-node folder?) — its files are reported missing" - ) - except (urllib.error.URLError, OSError, ValueError, json.JSONDecodeError) as e: - renderer.error( - code="server_not_running", - message=f"local ComfyUI server is unreachable, cannot check installed models: {e}", - hint="run `comfy launch` to start a local server", - ) - raise typer.Exit(code=1) from e - - for req in required: - listing = listings.get(req["directory"]) - # Normalize BOTH sides: a model ref may itself carry a subfolder - # (e.g. ``SDXL/model.safetensors``, as ComfyUI loader widgets emit), - # so compare basenames on the required side too. - req_base = _basename(req["name"]) - if listing and any(_basename(entry) == req_base for entry in listing): - present.append(req["name"]) - else: - missing.append(dict(req)) + try: + present, missing, warnings = _match_local_models(required, {}) + except TemplateCheckError as e: + renderer.error(code=e.code, message=e.message, hint=e.hint, details=e.details) + raise typer.Exit(code=1) from e # 6. Custom nodes are report-only in v1 (surfaced verbatim, not verified). custom_nodes_required = list(match.get("requires_custom_nodes") or []) diff --git a/comfy_cli/knowledge.py b/comfy_cli/knowledge.py index 950c9bdc..caeb6dac 100644 --- a/comfy_cli/knowledge.py +++ b/comfy_cli/knowledge.py @@ -59,6 +59,9 @@ MAX_VERSION_CHARS = 64 UNAVAILABLE_LOCALLY = "the templates or nodes this row resolves to are absent from this install" +# `knowledge pick --check-local` reasons, kept short: each flagged pick repeats one. +UNAVAILABLE_MISSING_MODELS = "model files are missing locally" +UNAVAILABLE_TEMPLATE_NOT_FOUND = "template is not in the gallery" # Grammatical filler dropped before subset matching. Deliberately generic English: # naming a capability, model or gallery tag here would be a second copy of the diff --git a/comfy_cli/schemas/knowledge.json b/comfy_cli/schemas/knowledge.json index ae9875a3..40f47c76 100644 --- a/comfy_cli/schemas/knowledge.json +++ b/comfy_cli/schemas/knowledge.json @@ -56,9 +56,13 @@ "status": { "type": ["string", "null"], "description": "Copied from the pick's model row when that row exists." }, "superseded_by": { "type": ["string", "null"], "description": "From the deprecations list or the pick's model row." }, "best_for": { "type": "array", "items": { "type": "string" }, "maxItems": 1, "description": "Head of the model row's best_for list, present only when the row has one. The caveat is per capability; this is the row's own routing opinion." }, - "fits": { "type": "object", "description": "The model row's fits block (vram_gb per variant, a credit rate, max_refs, source), present only when the row carries one." } + "fits": { "type": "object", "description": "The model row's fits block (vram_gb per variant, a credit rate, max_refs, source), present only when the row carries one." }, + "available_locally": { "type": "boolean", "const": false, "description": "Present only under pick --check-local, on an oss pick the local ComfyUI cannot run." }, + "unavailable_reason": { "type": "string", "description": "Why available_locally is false (pick --check-local)." }, + "missing_models": { "type": "integer", "minimum": 1, "description": "How many of the pick template's model files the local ComfyUI lacks (pick --check-local). `comfy templates check