diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 4f67f4882d..8da9a77c44 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -419,17 +419,15 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", page_size = max(1, min(page_size, 100)) c = console or _console all_results, source_counts, timed_out = _fetch_browse_results(c, source) + from tools.skills_hub_github import _provider_filter_of if not all_results: - c.print("[dim]No skills found in the Skills Hub.[/]\n") - return - # Provider filter (nvidia/openai/...) narrows GitHub-tap skills by their per-tap - # ``extra.provider`` label (the runtime index stores them all under source="github"). - from tools.skills_hub_github import _filter_results_by_provider, _provider_filter_of - if _provider_filter_of(source): - all_results = _filter_results_by_provider(all_results, source) - if not all_results: + # Provider narrowing happens inside parallel_search_sources; keep the + # provider-specific empty message. + if _provider_filter_of(source): c.print(f"[dim]No skills found for provider '{source}'.[/]\n") - return + else: + c.print("[dim]No skills found in the Skills Hub.[/]\n") + return deduped, page_items, page, total_pages, start = _rank_and_page(all_results, page, page_size) _render_browse_page(c, deduped, page_items, page, total_pages, start, source, source_counts, timed_out) @@ -437,7 +435,6 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", def browse_skills(page: int = 1, page_size: int = 20, source: str = "all") -> dict: """Paginated hub browse for programmatic callers (e.g. TUI gateway).""" - from tools.skills_hub_github import _filter_results_by_provider, _provider_filter_of from tools.skills_hub_search import parallel_search_sources page_size = max(1, min(page_size, 100)) # The shared parallel walker carries the index-aware source-skip logic — querying @@ -445,10 +442,6 @@ def browse_skills(page: int = 1, page_size: int = 20, source: str = "all") -> di all_results, _counts, _timed_out = parallel_search_sources( _sources(), query="", per_source_limits=_BROWSE_API_LIMITS, source_filter=source, overall_timeout=30) - # Same provider rule as do_browse: sources that cannot filter per-tap (official, - # url, ...) are only cut on the merged set. - if _provider_filter_of(source): - all_results = _filter_results_by_provider(all_results, source) if not all_results: return {"items": [], "page": 1, "total_pages": 1, "total": 0} deduped, page_items, page, total_pages, _start = _rank_and_page(all_results, page, page_size) diff --git a/tests/hermes_cli/test_skills_provider_filter.py b/tests/hermes_cli/test_skills_provider_filter.py index 0dfd2533ba..53e5f48edd 100644 --- a/tests/hermes_cli/test_skills_provider_filter.py +++ b/tests/hermes_cli/test_skills_provider_filter.py @@ -7,8 +7,8 @@ import pytest from hermes_cli import skills_hub as cli_hub from hermes_cli.subcommands.skills import build_skills_parser -from tools.skills_hub_github import GitHubAuth, GitHubSource, _tap_cache_key -from tools.skills_hub_models import SkillMeta, _cache_metas +from tools.skills_hub_github import GitHubAuth, GitHubSource, _tap_cache_key, github_provider_for +from tools.skills_hub_models import SkillMeta, _cache_metas, _skill_meta_to_dict from tools.skills_hub_official import HermesIndexSource from tools.skills_hub_search import _hermes_index_cache_file, parallel_search_sources @@ -30,17 +30,14 @@ def catalog(request, monkeypatch): if request.param == "index": path = _hermes_index_cache_file() path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps({"skills": [vars(m) for m in others + wanted]}), encoding="utf-8") + path.write_text(json.dumps({"skills": [_skill_meta_to_dict(m) for m in others + wanted]}), encoding="utf-8") source = HermesIndexSource(auth) else: source = GitHubSource(auth) for tap in source.taps: key = _tap_cache_key(tap["repo"], tap.get("path", ""), tap.get("bucket")) - entries = [] - if tap["repo"] == "openai/skills": - entries = others - elif tap["repo"] == "NVIDIA/skills": - entries = wanted + label = github_provider_for(tap["repo"]) + entries = others if label == "OpenAI" else wanted if label == "NVIDIA" else [] _cache_metas(key, entries) monkeypatch.setattr(cli_hub, "_sources", lambda: [source]) return source, others, wanted diff --git a/tools/skills_hub_official.py b/tools/skills_hub_official.py index 752e900032..0475d8a5ab 100644 --- a/tools/skills_hub_official.py +++ b/tools/skills_hub_official.py @@ -323,8 +323,8 @@ class HermesIndexSource(SkillSource): index order as tiebreaker — a raw break-at-limit slice buried the most relevant skills. Provider filters narrow the catalog before ranking and limiting.""" skills = self._skills() - if provider_filter: - want = provider_filter.strip().lower() + want = provider_filter.strip().lower() + if want: skills = [s for s in skills if _entry_provider(s) == want] if not skills: return [] diff --git a/tools/skills_hub_search.py b/tools/skills_hub_search.py index 8e879fb274..2ecfca9914 100644 --- a/tools/skills_hub_search.py +++ b/tools/skills_hub_search.py @@ -122,9 +122,9 @@ def _select_active_sources(sources: List[SkillSource], source_filter: str) -> Li A provider filter (nvidia/openai/...) is not a source id — the data lives in the index/github source under ``extra.provider`` — so it selects like - "all". Mixed-provider sources filter before limiting; the merged results - are filtered again. "official" is always queried alongside an explicit - source filter. + "all". Mixed-provider sources narrow before their top-N cut; the walker + cuts every source's results. "official" is always queried alongside an + explicit source filter. """ effective = "all" if _provider_filter_of(source_filter) else source_filter index_available = effective == "all" and any( @@ -148,7 +148,9 @@ def parallel_search_sources( """Search all sources in parallel with an overall timeout. Returns ``(all_results, source_counts, timed_out_ids)``. *on_source_done* - is an optional ``(source_id, count) -> None`` progress callback. + is an optional ``(source_id, count) -> None`` progress callback. Under a + provider filter every source's results are narrowed before they are + counted and merged, so callers need no provider logic of their own. """ from concurrent.futures import as_completed @@ -176,6 +178,11 @@ def parallel_search_sources( for fut in as_completed(futures, timeout=overall_timeout): try: sid, results = fut.result(timeout=0) + if provider_filter: + # One owner for the merged provider cut: sources that cannot + # filter per-tap (official, url, ...) are narrowed here, so + # every caller (CLI, TUI gateway, dashboard) sees one rule. + results = _filter_results_by_provider(results, provider_filter) source_counts[sid] = len(results) all_results.extend(results) if on_source_done: @@ -195,9 +202,6 @@ def unified_search(query: str, sources: List[SkillSource], source_filter: str = "all", limit: int = 10) -> List[SkillMeta]: """Search all sources (in parallel) and merge results.""" all_results, _, _ = parallel_search_sources(sources, query=query, source_filter=source_filter, overall_timeout=30) - # Provider filters target ``extra.provider`` on the merged set, not a source id. - if _provider_filter_of(source_filter): - all_results = _filter_results_by_provider(all_results, source_filter) deduped = _dedupe_by_trust(all_results) # Stable-sort by trust before truncating so the limit cut never drops a # builtin/official entry because a high-volume community source finished