Skip to content

feat(tool_search): optional embedding reranker for progressive tool disclosure - #44272

Closed
davidgut1982 wants to merge 2 commits into
NousResearch:mainfrom
davidgut1982:split/tool-search-reranker
Closed

feat(tool_search): optional embedding reranker for progressive tool disclosure#44272
davidgut1982 wants to merge 2 commits into
NousResearch:mainfrom
davidgut1982:split/tool-search-reranker

Conversation

@davidgut1982

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds an optional, opt-in embedding reranker to the tool_search BM25 bridge (built on top of #34493) for progressive tool disclosure. It is default OFF — when disabled the BM25 path is byte-for-byte identical to upstream, so there is zero behavior change unless a user explicitly enables it.

When enabled, tool candidates retrieved by BM25 are reranked against any OpenAI-compatible /v1/embeddings endpoint (cloud, local CPU, or GPU). This substantially improves semantic recall for tool selection while preserving exact lexical matches.

The second commit hardens the reranker's caching: it replaces the single-slot module-level singleton with a bounded, per-scope cache so concurrent agents operating on different toolsets don't evict each other's cached embeddings.

Related Issue

Fulfills #13332.

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tools/tool_search.py: optional embedding reranker on the BM25 bridge. urllib-only (no new deps), task-prefixed and md5-cached tool embeddings, full-catalog retrieve, rerank / RRF(k=10) modes, and graceful BM25 fallback on any endpoint failure. Default OFF.
  • tools/tool_search.py: per-scope reranker cache — replaced the single-slot module-level singleton (_reranker / _reranker_catalog_key) with a bounded scope-keyed dict (_reranker_cache, max 8 entries, FIFO eviction), keyed by md5(endpoint + model + tool_names). Each toolset-scope keeps its own EmbeddingReranker and embedding cache. Thread-safety preserved via double-checked locking guarded by the existing _GLOBAL_LOCK.
  • tests/tools/test_tool_search.py: config parsing, rerank/RRF modes, fallback path, and TestEmbedCacheInvalidation (concurrent scopes do not share/evict; FIFO eviction at max size).
  • website/docs/user-guide/features/tool-search.md: documents the opt-in reranker, config keys, and backend requirements.

How to Test

  1. With the reranker disabled (default), confirm tool_search output is unchanged from upstream BM25.
  2. Point a reranker config at any OpenAI-compatible /v1/embeddings endpoint and confirm semantic queries surface the right tools; kill the endpoint and confirm graceful fallback to BM25.
  3. Run the scoped suite: pytest tests/tools/test_tool_search.py -q63 passed.

Live-validated (194 tools / 98 labeled queries, nomic-embed-text-v2-moe): overall Recall@5 0.617 → 0.810, SEMANTIC 0.500 → 0.849, LEXICAL preserved at 1.000; warm per-query ~146ms, dead-endpoint fallback ~8ms.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run the scoped suite pytest tests/tools/test_tool_search.py -q and all tests pass (63 passed)
  • I've added tests for my changes
  • I've tested on my platform: Ubuntu 24.04 (LXC)

Documentation & Housekeeping

  • I've updated relevant documentation — website/docs/user-guide/features/tool-search.md covers the opt-in reranker
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (documented in the tool-search feature doc; defaults keep it disabled)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — urllib-only, no platform-specific calls
  • I've updated tool descriptions/schemas if I changed tool behavior — reranker is opt-in; base tool_search schema unchanged when disabled

Screenshots / Logs

$ pytest tests/tools/test_tool_search.py -q
...............................................................          [100%]
63 passed in 5.75s

davidgut1982 and others added 2 commits June 11, 2026 12:33
…isclosure

Adds an optional, opt-in embedding reranker to the tool_search BM25 bridge
(PR NousResearch#34493). Default OFF — when disabled the BM25 path is byte-for-byte
identical to upstream. urllib-only (no new deps), task-prefixed, md5-cached
tool embeddings, full-catalog retrieve, rerank/RRF(k=10) modes, graceful
BM25 fallback on any endpoint failure. Backend is any OpenAI-compatible
/v1/embeddings endpoint (cloud, local CPU, or GPU).

Live-validated (194 tools / 98 labeled queries, nomic-embed-text-v2-moe):
overall Recall@5 0.617 -> 0.810, SEMANTIC 0.500 -> 0.849, LEXICAL preserved
at 1.000; warm per-query ~146ms, dead-endpoint fallback ~8ms.

Fulfills NousResearch#13332.
…ding churn (NousResearch#13332)

Replace the single-slot module-level reranker singleton (_reranker /
_reranker_catalog_key) with a bounded scope-keyed dict (_reranker_cache,
max 8 entries, FIFO eviction). Each distinct toolset-scope (keyed by
md5(endpoint + model + tool_names)) now retains its own EmbeddingReranker
instance and its own per-tool embedding cache independently of concurrent
agents operating on different toolsets.

Old behaviour: agent A (toolset X) and agent B (toolset Y) racing through
_get_reranker() caused the second call to rebuild the singleton and discard
the first agent's cached embeddings, forcing repeated endpoint calls.

New behaviour: both scopes coexist in the dict; re-requesting scope A after
scope B is created returns the original scope-A instance with its embedding
cache intact. Thread-safety is preserved via double-checked locking on the
dict + order list, guarded by the existing _GLOBAL_LOCK.

New tests (TestEmbedCacheInvalidation):
- test_concurrent_scopes_do_not_share_reranker: proves scope B creation does
  NOT evict scope A's instance or its embedding cache (mocks _embed and
  asserts zero extra calls on scope-A re-access after scope B is created).
- test_reranker_cache_evicts_oldest_scope_when_full: fills cache to
  _RERANKER_CACHE_MAX_SIZE (8), adds an overflow scope, and asserts FIFO
  eviction dropped the oldest key from _reranker_cache.

Existing tests updated to reset _reranker_cache / _reranker_cache_order
instead of the retired _reranker / _reranker_catalog_key globals.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists labels Jun 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #35457 — same author, identical change (optional opt-in embedding reranker on the tool_search BM25 bridge, default OFF, same files: tools/tool_search.py + tests + docs). #35457 is the earlier open PR.

@davidgut1982

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #35457 (feat/tool-search-hybrid-rerank), which is the canonical PR for the tool_search embedding reranker. This PR was opened in error during a PR-split cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants