diff --git a/openspace/cloud/search.py b/openspace/cloud/search.py index c40012d..525c26d 100644 --- a/openspace/cloud/search.py +++ b/openspace/cloud/search.py @@ -148,8 +148,13 @@ class SkillSearchEngine: ranked = ranker.bm25_only(query, bm25_candidates, top_k=min(limit * 3, len(candidates))) ranked_ids = {sc.skill_id for sc in ranked} + bm25_scores = {sc.skill_id: sc.bm25_score for sc in ranked} filtered = [c for c in candidates if c.get("skill_id") in ranked_ids] + # Persist BM25 scores so the scoring phase can reuse them + for candidate in filtered: + candidate["_bm25_score"] = bm25_scores.get(candidate.get("skill_id"), 0.0) + # If BM25 found nothing, fall back to all candidates return filtered if filtered else candidates @@ -159,7 +164,7 @@ class SkillSearchEngine: query_tokens: list[str], query_embedding: Optional[List[float]], ) -> List[Dict[str, Any]]: - """Compute hybrid score = vector_score + lexical_boost.""" + """Compute semantic/server ranking with BM25 fallback plus lexical boost.""" from openspace.cloud.embedding import cosine_similarity scored = [] @@ -185,7 +190,18 @@ class SkillSearchEngine: if category_slug: lexical_boost += 0.35 * _lexical_boost(query_tokens, "", category_slug) - final_score = ranking_signal_score + lexical_boost + bm25_score = candidate.get("_bm25_score") or 0.0 + + use_bm25_fallback = ( + not query_embedding + and not isinstance(candidate.get("_search_rank"), (int, float)) + ) + + final_score = ( + ranking_signal_score + + lexical_boost + + (bm25_score if use_bm25_fallback else 0.0) + ) result_entry: Dict[str, Any] = { "skill_id": candidate.get("skill_id", ""), diff --git a/tests/cloud/test_search_ranking.py b/tests/cloud/test_search_ranking.py new file mode 100644 index 0000000..812a060 --- /dev/null +++ b/tests/cloud/test_search_ranking.py @@ -0,0 +1,155 @@ +"""Regression tests for local skill search ranking. + +Issue #115: ``SkillSearchEngine._bm25_phase`` computed BM25 scores and used +them for candidate filtering, but never propagated them to +``SkillSearchEngine._score_phase``. Without an embedding provider the ranking +signal collapsed to zero, so multi-word queries could fall back to registry +order instead of BM25 relevance order. + +The BM25 signal is only a fallback final-score signal: it must not alter +existing embedding-backed or server-ranked search semantics. + +These tests use synthetic candidates only (no bundled skill fixtures) so they +exercise the public ``SkillSearchEngine.search()`` API in isolation. +""" + +from openspace.cloud.search import SkillSearchEngine +from openspace.skill_engine.skill_ranker import SkillRanker + + +def _candidate(skill_id: str, name: str, description: str, **extra) -> dict: + return { + "skill_id": skill_id, + "name": name, + "description": description, + "source": "test", + **extra, + } + + +def test_multiword_no_embedding_ranking_uses_bm25_signal(): + """Multi-word query without embeddings must rank by BM25 relevance. + + Candidates are deliberately ordered with the relevant one LAST so that a + registry-order fallback would rank it at the bottom. The relevant + candidate does not satisfy the current all-query-token lexical-boost + condition, so its ranking advantage must come from BM25. + """ + candidates = [ + _candidate( + "apple-notes", + "Manage Apple Notes and local notes", + "Manage Apple Notes and local notes", + ), + _candidate( + "weather", + "Get weather forecasts and conditions", + "Get weather forecasts and conditions", + ), + _candidate( + "docx", + "Word file editing utility", + "Create and edit Word DOCX documents", + ), + ] + + results = SkillSearchEngine().search( + "docx document", + candidates, + query_embedding=None, + limit=5, + ) + + skill_ids = [r["skill_id"] for r in results] + + # The docx candidate must rank above both unrelated candidates. + assert skill_ids.index("docx") < skill_ids.index("apple-notes") + assert skill_ids.index("docx") < skill_ids.index("weather") + + # The docx candidate must carry a non-zero BM25 ranking signal. + docx_result = next(r for r in results if r["skill_id"] == "docx") + assert docx_result["score"] > 0 + + +def test_embedding_ranking_not_affected_by_bm25_signal(monkeypatch): + """Embedding ranking must not receive the BM25 addition. + + With a query embedding available, final ranking is decided by + vector score + lexical boost; BM25 is only a Phase 1 filtering signal. + """ + def fake_bm25_only(self, query, candidates, top_k): + for c in candidates: + c.bm25_score = 10.0 if c.skill_id == "bm25-favored" else 0.0 + return sorted(candidates, key=lambda c: c.bm25_score, reverse=True)[:top_k] + + monkeypatch.setattr(SkillRanker, "bm25_only", fake_bm25_only) + + candidates = [ + _candidate( + "bm25-favored", + "Office utilities for writing files", + "Office utilities for writing files", + _embedding=[0.8, 0.6], + ), + _candidate( + "vector-favored", + "Quick notes manager", + "Quick notes manager", + _embedding=[0.9, 0.435889894], + ), + ] + + results = SkillSearchEngine().search( + "docx document", + candidates, + query_embedding=[1.0, 0.0], + limit=5, + ) + + skill_ids = [r["skill_id"] for r in results] + + # vector-favored has higher vector similarity (0.9 > 0.8) and must rank + # first despite bm25-favored carrying a large BM25 score. + assert skill_ids.index("vector-favored") < skill_ids.index("bm25-favored") + + +def test_server_search_rank_not_affected_by_bm25_signal(monkeypatch): + """Server search rank must not receive the BM25 addition. + + Without a query embedding but with a numeric server ``_search_rank``, + final ranking is decided by server search rank + lexical boost. + """ + def fake_bm25_only(self, query, candidates, top_k): + for c in candidates: + c.bm25_score = 10.0 if c.skill_id == "bm25-favored" else 0.0 + return sorted(candidates, key=lambda c: c.bm25_score, reverse=True)[:top_k] + + monkeypatch.setattr(SkillRanker, "bm25_only", fake_bm25_only) + + candidates = [ + _candidate( + "bm25-favored", + "Office utilities for writing files", + "Office utilities for writing files", + _search_rank=0.2, + ), + _candidate( + "server-favored", + "Quick notes manager", + "Quick notes manager", + _search_rank=0.9, + ), + ] + + results = SkillSearchEngine().search( + "docx document", + candidates, + query_embedding=None, + limit=5, + ) + + skill_ids = [r["skill_id"] for r in results] + + # server-favored has the higher server search rank and must rank first + # despite bm25-favored carrying a large BM25 score. + assert skill_ids.index("server-favored") < skill_ids.index("bm25-favored")