fix(skills-hub): deduplicate search results by identifier, not name#29490
Merged
Conversation
Browse.sh exposes skills by task name (e.g. "search-listings"), which is shared across hundreds of sites. Deduplicating by name silently dropped every browse-sh skill after the first one with a given task name — e.g. only Airbnb's "search-listings" would survive, collapsing Booking.com, Zillow, and every other site's variant into nothing. Switch unified_search() and do_browse() to use r.identifier as the dedup key. identifier is always globally unique (e.g. "browse-sh/airbnb.com/search-listings-ddgioa"), so same-named skills from different browse-sh hostnames are preserved as distinct results. Update existing TestUnifiedSearchDedup tests to model the real scenario (same identifier appearing from two sources) and add a regression test that asserts browse-sh skills with the same name but different hostnames are never collapsed.
browse_skills() is the TUI gateway's API for the web UI skills browser (tui_gateway/server.py:6574). It had the same dedup-by-name bug as do_browse() and unified_search() fixed in the parent commit: r.name is not unique for browse-sh skills (Airbnb, Booking.com, Zillow all publish "search-listings"), so the dedup loop silently dropped all but the first skill with each task name. Switch to r.identifier, which is always globally unique. Add a regression test asserting that two browse-sh skills with the same name but different hostnames both appear in the browse_skills() result.
…tch path Sibling fix on top of @EloquentBrush0x's PR #29441. - tools/skills_hub.py GitHubSource.search() had the same r.name dedup bug. Two configured GitHub taps publishing same-named skills would collapse to one. - tests/hermes_cli/test_skills_hub.py:test_browse_skills_dedup_uses_identifier_not_name patched hermes_cli.skills_hub.create_source_router, but browse_skills() imports it locally from tools.skills_hub. Fixed patch path.
Contributor
🔎 Lint report:
|
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #29441 by @EloquentBrush0x onto current main, plus one sibling fix.
Summary
Skill-hub search no longer silently drops browse-sh skills that share task names across different sites (e.g.
search-listingsfrom Airbnb, Booking.com, Zillow). Dedup now keys onr.identifier(always globally unique) instead ofr.name.Changes
From @EloquentBrush0x (PR #29441):
tools/skills_hub.py—unified_search()dedup key:r.name→r.identifierhermes_cli/skills_hub.py—do_browse()andbrowse_skills()dedup key: sameOur follow-ups:
tools/skills_hub.py— widened the same fix toGitHubSource.search()(line 386), which had the identical bug for multi-tap GitHub configurationstests/hermes_cli/test_skills_hub.py— fixed patch path on the newtest_browse_skills_dedup_uses_identifier_not_nametest (was patchinghermes_cli.skills_hub.create_source_router, butbrowse_skills()imports it locally fromtools.skills_hub)Validation
tests/tools/test_skills_hub.py+tests/hermes_cli/test_skills_hub.py: 133/133 passunified_search()andGitHubSource.search()both return both results after fix; collapsed to 1 before fixCloses #29441.
Infographic