diff --git a/comfy_cli/command/knowledge.py b/comfy_cli/command/knowledge.py index f5504b50c..bd5a5a504 100644 --- a/comfy_cli/command/knowledge.py +++ b/comfy_cli/command/knowledge.py @@ -231,14 +231,23 @@ def resolve_cmd( if model_id is None: knowledge.log_query("knowledge resolve", logged, hit_ids=[], zero_hit=True, bundle=bundle) q = query.strip().lower() + candidates = knowledge.model_candidates(bundle, query) + hint = "run `comfy knowledge status` to confirm a bundle is loaded; try a different alias" + details = { + "query": query, + "close_matches": difflib.get_close_matches(q, list(bundle.aliases), n=5, cutoff=0.6), + } + if candidates: + details["candidate_ids"] = candidates + hint = ( + f"possible model ids (up to 5): {', '.join(candidates)}; " + "ask which model is intended, then retry `comfy knowledge resolve `" + ) renderer.error( code="knowledge_unknown_model", message=f"no knowledge row matches {query!r}", - hint="run `comfy knowledge status` to confirm a bundle is loaded; try a different alias", - details={ - "query": query, - "close_matches": difflib.get_close_matches(q, list(bundle.aliases), n=5, cutoff=0.6), - }, + hint=hint, + details=details, ) raise typer.Exit(code=1) knowledge.log_query("knowledge resolve", logged, hit_ids=[model_id], zero_hit=False, bundle=bundle) diff --git a/comfy_cli/knowledge.py b/comfy_cli/knowledge.py index aa65371a5..ab2864cad 100644 --- a/comfy_cli/knowledge.py +++ b/comfy_cli/knowledge.py @@ -145,6 +145,9 @@ class Bundle: # :func:`_resolve_tokens`. capability_tokens: tuple[tuple[tuple[tuple[str, int], ...], str, str], ...] = () capability_context: dict[str, frozenset[str]] = field(default_factory=dict) + # Lowercase lookup keys whose raw spelling is eligible for suggestions. + # Kept separate because lower() can hide non-ASCII alphanumerics (e.g. K). + candidate_aliases: dict[str, str] = field(default_factory=dict) # One-element tuple so a memoized ``None`` is distinguishable from "not loaded yet". @@ -339,6 +342,40 @@ def resolve(bundle: Bundle, query: str) -> dict | None: return bundle.models[model_id] if model_id is not None else None +def _candidate_tokens(value: str) -> list[str]: + # Suggestions support ASCII names only. Never discard a Unicode qualifier + # or digit, or normalize it into a different family/version spelling. + if any(char.isalnum() and not char.isascii() for char in value): + return [] + return re.findall(r"[a-z]+|[0-9]+", value.lower()) + + +def model_candidates(bundle: Bundle, query: str) -> list[str]: + """At most five canonical ids for an incomplete family/version name. + + Suggestions only: even one candidate requires the caller to confirm the + model. Match the family word exactly, preserve every numeric component, + and allow omitted words, not substituted words or reordered tokens. + """ + if len(query) > MAX_QUERY_CHARS: + return [] + parts = _candidate_tokens(query) + numbers = [part for part in parts if part.isdigit()] + if not parts or not parts[0].isalpha() or not numbers: + return [] + candidates: set[str] = set() + for alias, model_id in bundle.candidate_aliases.items(): + if model_id not in bundle.models: + continue + words = _candidate_tokens(alias) + if not words or words[0] != parts[0] or [word for word in words if word.isdigit()] != numbers: + continue + remaining = iter(words[1:]) + if all(part in remaining for part in parts[1:]): + candidates.add(model_id) + return sorted(candidates)[:5] + + def pick(bundle: Bundle, capability: str) -> dict | None: """Ranked picks for a capability, keyed exactly, then by spelling, then by wording. @@ -650,16 +687,26 @@ def _index(data: dict, manifest: dict | None, *, source: str, stale: bool, path: # Exact ids first so no alias can shadow a model's own id; then the compiled # alias map, which wins over row-level aliases. aliases: dict[str, str] = {} + candidate_aliases: dict[str, str] = {} + + def index_alias(alias: str, mid: str) -> None: + key = alias.lower() + owner = aliases.setdefault(key, mid) + # Check before casing loses information, and retain resolution precedence. + # A valid ASCII spelling may still support the same winning target. + if owner == mid and _candidate_tokens(alias): + candidate_aliases.setdefault(key, mid) + for mid in models: - aliases.setdefault(mid.lower(), mid) + index_alias(mid, mid) compiled = data.get("aliases") if isinstance(compiled, dict): for alias, mid in compiled.items(): if isinstance(alias, str) and isinstance(mid, str): - aliases.setdefault(alias.lower(), mid) + index_alias(alias, mid) for mid, row in models.items(): for alias in _str_list(row.get("aliases")): - aliases.setdefault(alias.lower(), mid) + index_alias(alias, mid) caps_raw = data.get("capabilities") capabilities = ( @@ -739,6 +786,7 @@ def _index(data: dict, manifest: dict | None, *, source: str, stale: bool, path: capabilities=capabilities, deprecations=deprecations, aliases=aliases, + candidate_aliases=candidate_aliases, templates=templates, nodes=nodes, model_capabilities=model_capabilities, diff --git a/docs/json-output.md b/docs/json-output.md index 6e39597f4..9a2c493cd 100644 --- a/docs/json-output.md +++ b/docs/json-output.md @@ -777,6 +777,24 @@ are objects in the normal case; treat a non-object item as an opaque message. `node_id` is always taken from the payload's own map key, so a server-supplied `node_id` field inside a record cannot override it. +### Incomplete model names + +`comfy knowledge resolve` resolves declared model ids and aliases. An incomplete +family/version name such as `Qwen 2.1` still returns `knowledge_unknown_model` +with exit code 1. When catalog ids or aliases contain the same family word and +numeric components, in order, `error.details.candidate_ids` lists up to five +sorted, distinct canonical ids. Omitted words are allowed for suggestions; +substituted family words and different versions are not. +Suggestions conservatively skip queries and catalog aliases containing non-ASCII +letters or numbers rather than discard or normalize those qualifiers. Declared +id and alias resolution still uses its existing rules. + +These are suggestions, even when there is only one. Ask which model is intended, +then retry `comfy knowledge resolve `. They grant no permission to +download or run a model. Successful full-name resolution and unknown-name errors +without candidates retain their existing behavior; `close_matches` remains a +separate spelling hint. + ## Output object Entries of `executed.outputs`: diff --git a/tests/comfy_cli/test_knowledge.py b/tests/comfy_cli/test_knowledge.py index 58f538fe1..3e8214ce6 100644 --- a/tests/comfy_cli/test_knowledge.py +++ b/tests/comfy_cli/test_knowledge.py @@ -1506,3 +1506,205 @@ def test_the_bare_listing_asks_nothing_so_logs_nothing(self, tmp_path, monkeypat _env_bundle(tmp_path, monkeypatch) _run(["pick"], capsys) assert queries == [] + + +class TestNamedModelDisambiguation: + """Missing family words warrant a choice, never an inferred model.""" + + @pytest.fixture + def named_bundle(self, monkeypatch): + # Identity/aliases from comfy-knowledge v0.2.2 (7ee63b21a59b), + # canon/models/image/qwen-image-2-1.yaml. Cloud's compiled bundle is + # sha256 53a70dd644dbca30ad7c20b7f8bd8c00d154609ae1edf22e63aa834954f11d48. + data = { + "models": { + "qwen-image-2-1": { + "id": "qwen-image-2-1", + "aliases": [ + "Qwen Image 2.1", + "Qwen-Image-2.1", + "Qwen Image 2.1 T2I", + "Qwen Image 2.1 Edit", + "Qwen Image 2.1 Remove Background", + "Qwen Image 2.1 Turbo", + "Qwen-Image-2.1-Turbo", + ], + }, + "qwen-image-2512": {"id": "qwen-image-2512"}, + "qwen-image-edit": {"id": "qwen-image-edit"}, + } + } + bundle = knowledge._index(data, None, source="env", stale=False, path="fixture", mtime=0.0) + monkeypatch.setattr(knowledge, "load_bundle", lambda **kw: bundle) + return bundle + + @pytest.mark.parametrize("query", ["Qwen 2.1", "QWEN-2-1", "qwen2.1"]) + def test_short_name_offers_canonical_id_without_resolving(self, named_bundle, query, capsys): + assert knowledge.resolve_id(named_bundle, query) is None + rc, env = _run(["resolve", query], capsys) + assert rc == 1 + assert env["ok"] is False + assert env["data"] is None + assert env["error"]["code"] == "knowledge_unknown_model" + assert env["error"]["details"]["candidate_ids"] == ["qwen-image-2-1"] + assert "ask which model" in env["error"]["hint"] + assert "qwen-image-2-1" in env["error"]["hint"] + + @pytest.mark.parametrize("query", ["Qwen Image 2.1", "qwen-image-2-1", "QWEN IMAGE 2.1 TURBO"]) + def test_full_name_still_resolves(self, named_bundle, query, capsys): + rc, env = _run(["resolve", query], capsys) + assert rc == 0 + assert env["data"]["id"] == "qwen-image-2-1" + _validate(env["data"]) + + @pytest.mark.parametrize( + "query", + [ + "Qwen VL 2.1", + "Qwen text 2.1", + "Qwen 2.10", + "Qwen 21", + "Qwen 1.2", + "Qwen 2", + "Qwen", + "other 2.1", + "unknown-pm2214", + "", + "Qwen" + " " * knowledge.MAX_QUERY_CHARS + "2.1", + "Qwen 文本 2.1", + "Qwen 2.1β", + "Qwen 2.12", + "Qwen 2.1K", + "Qwen 2.1İ", + "Qwеn 2.1", # Cyrillic e + ], + ) + def test_unknown_or_different_family_version_stays_unknown(self, named_bundle, query, capsys): + rc, env = _run(["resolve", query], capsys) + assert rc == 1 + assert env["error"]["code"] == "knowledge_unknown_model" + assert env["error"]["details"].get("candidate_ids", []) == [] + assert "ask which model" not in env["error"]["hint"] + + @pytest.mark.parametrize( + "alias", + [ + "Qwen Image 文本 2.1", + "Qwen Image 2.1β", + "Qwen Image 2.12", + "Qwen Image 2.1²", + "Qwen Image 2.1K", + "Qwen Image 2.1İ", + "Qwеn Image 2.1", + ], + ) + @pytest.mark.parametrize("top_level", [False, True]) + def test_unicode_aliases_cannot_offer_ascii_candidates(self, monkeypatch, capsys, alias, top_level): + data = {"models": {"opaque-model": {"aliases": [] if top_level else [alias]}}} + if top_level: + data["aliases"] = {alias: "opaque-model"} + bundle = knowledge._index(data, None, source="env", stale=False, path="fixture", mtime=0.0) + monkeypatch.setattr(knowledge, "load_bundle", lambda **kw: bundle) + rc, env = _run(["resolve", "Qwen 2.1"], capsys) + assert rc == 1 + assert env["error"]["code"] == "knowledge_unknown_model" + assert env["error"]["details"].get("candidate_ids", []) == [] + assert "ask which model" not in env["error"]["hint"] + # Suggestion eligibility must not change declared alias resolution. + for spelling in (alias, alias.lower(), alias.upper()): + assert knowledge.resolve_id(bundle, spelling) == "opaque-model" + rc, env = _run(["resolve", spelling], capsys) + assert rc == 0 + assert env["data"]["id"] == "opaque-model" + + @pytest.mark.parametrize("top_level", [False, True]) + @pytest.mark.parametrize("reverse", [False, True]) + def test_candidate_alias_collisions_preserve_resolution_precedence(self, top_level, reverse): + pairs = [("Qwen Image 2.1K", "opaque-unicode"), ("Qwen Image 2.1k", "opaque-ascii")] + if reverse: + pairs.reverse() + data = {"models": {mid: {"aliases": [] if top_level else [alias]} for alias, mid in pairs}} + if top_level: + data["aliases"] = dict(pairs) + b = knowledge._index(data, None, source="env", stale=False, path="fixture", mtime=0) + assert knowledge.resolve_id(b, "Qwen Image 2.1k") == pairs[0][1] + assert knowledge.model_candidates(b, "Qwen 2.1") == (["opaque-ascii"] if reverse else []) + + @pytest.mark.parametrize("reverse", [False, True]) + def test_eligible_spelling_for_same_target_still_offers_candidate(self, reverse): + aliases = ["Qwen Image 2.1K", "Qwen Image 2.1k"] + if reverse: + aliases.reverse() + b = knowledge._index( + {"models": {"opaque-model": {"aliases": aliases}}}, + None, + source="env", + stale=False, + path="fixture", + mtime=0, + ) + assert knowledge.model_candidates(b, "Qwen 2.1") == ["opaque-model"] + assert knowledge.resolve_id(b, aliases[0]) == "opaque-model" + + def test_canonical_and_top_level_aliases_keep_precedence(self): + b = knowledge._index( + { + "models": { + "qwen-image-2-1": {}, + "opaque-model": {"aliases": ["qwen-image-2-1", "Qwen Text 2.1"]}, + }, + "aliases": {"qwen-image-2-1": "opaque-model", "Qwen Text 2.1": "qwen-image-2-1"}, + }, + None, + source="env", + stale=False, + path="fixture", + mtime=0, + ) + assert knowledge.resolve_id(b, "qwen-image-2-1") == "qwen-image-2-1" + assert knowledge.resolve_id(b, "Qwen Text 2.1") == "qwen-image-2-1" + assert knowledge.model_candidates(b, "Qwen 2.1") == ["qwen-image-2-1"] + + def test_raw_unicode_model_id_cannot_offer_ascii_candidates(self): + mid = "Qwen Image 2.1K" + b = knowledge._index({"models": {mid: {}}}, None, source="env", stale=False, path="fixture", mtime=0) + assert knowledge.resolve_id(b, mid) == mid + assert knowledge.model_candidates(b, "Qwen 2.1") == [] + + def test_future_image_vl_and_text_models_remain_an_explicit_choice(self, monkeypatch, capsys): + # Synthetic future catalog: no preference for Image, VL, or text. + ids = ["qwen-image-2-1", "qwen-vl-2-1", "qwen-text-2-1"] + b = knowledge._index( + {"models": {mid: {"id": mid} for mid in ids}}, + None, + source="env", + stale=False, + path="fixture", + mtime=0.0, + ) + monkeypatch.setattr(knowledge, "load_bundle", lambda **kw: b) + rc, env = _run(["resolve", "Qwen 2.1"], capsys) + assert rc == 1 + assert env["error"]["details"]["candidate_ids"] == sorted(ids) + assert "ask which model" in env["error"]["hint"] + assert knowledge.resolve_id(b, "Qwen 2.1") is None + assert knowledge.model_candidates(b, "Qwen VL 2.1") == ["qwen-vl-2-1"] + + def test_candidates_are_general_sorted_bounded_and_deduplicated(self): + ids = [f"acme-variant{letter}-2-1" for letter in "hgfedcba"] + b = knowledge._index( + { + "models": {mid: {"id": mid, "aliases": [mid.upper(), mid.replace("-", " ") + " turbo"]} for mid in ids}, + "aliases": {"Acme Missing 2.1": "missing-row"}, + }, + None, + source="env", + stale=False, + path="fixture", + mtime=0.0, + ) + assert knowledge.model_candidates(b, "Acme 2.1") == sorted(ids)[:5] + boundary_query = "Acme 2.1".ljust(knowledge.MAX_QUERY_CHARS) + assert len(boundary_query) == 200 + assert knowledge.model_candidates(b, boundary_query) == sorted(ids)[:5] + assert knowledge.model_candidates(b, boundary_query + " ") == []