-
Notifications
You must be signed in to change notification settings - Fork 86
LCORE-1853: Add relevance cutoff score to inline BYOK RAG #1720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,24 @@ | |
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| def _relevance_cutoff_for_vector_store(vector_store_id: str) -> float: | ||
| """Return configured relevance cutoff for a Llama Stack vector store ID. | ||
|
|
||
| Args: | ||
| vector_store_id: Llama Stack vector store identifier (``vector_db_id``) | ||
| used to find a matching BYOK RAG entry in configuration. | ||
|
|
||
| Returns: | ||
| ``brag.relevance_cutoff_score`` from the ``ByokRag`` whose ``vector_db_id`` | ||
| matches ``vector_store_id``, or ``constants.DEFAULT_BYOK_RAG_RELEVANCE_CUTOFF_SCORE`` | ||
| when no BYOK entry matches. | ||
| """ | ||
| for brag in configuration.configuration.byok_rag: | ||
| if brag.vector_db_id == vector_store_id: | ||
| return brag.relevance_cutoff_score | ||
| return constants.DEFAULT_BYOK_RAG_RELEVANCE_CUTOFF_SCORE | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def _get_okp_base_url() -> AnyUrl: | ||
| """Return OKP document base URL from configuration (rhokp_url), or default if unset. | ||
|
|
||
|
|
@@ -180,6 +198,7 @@ async def _query_store_for_byok_rag( | |
| vector_store_id: str, | ||
| query: str, | ||
| weight: float, | ||
| score_threshold: float, | ||
| ) -> list[dict[str, Any]]: | ||
| """Query a single vector store for BYOK RAG. | ||
|
|
||
|
|
@@ -188,6 +207,7 @@ async def _query_store_for_byok_rag( | |
| vector_store_id: ID of the vector store to query | ||
| query: Search query string | ||
| weight: Score multiplier to apply | ||
| score_threshold: Minimum raw similarity score (``relevance_cutoff_score``) | ||
|
|
||
| Returns: | ||
| List of weighted result dictionaries, or empty list on error | ||
|
|
@@ -199,6 +219,7 @@ async def _query_store_for_byok_rag( | |
| params={ | ||
| "max_chunks": constants.BYOK_RAG_MAX_CHUNKS, | ||
| "mode": "vector", | ||
| "score_threshold": score_threshold, | ||
| }, | ||
| ) | ||
| return _extract_byok_rag_chunks(search_response, vector_store_id, weight) | ||
|
|
@@ -410,6 +431,7 @@ async def _fetch_byok_rag( | |
| vector_store_id, | ||
| query, | ||
| score_multiplier_mapping.get(vector_store_id, 1.0), | ||
| _relevance_cutoff_for_vector_store(vector_store_id), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tiny nit from SWE perspective. Just wondering: since score multiplier and relevance score have the same data structure, would it make sense to retrieve cutoff score the same way? |
||
| ) | ||
| for vector_store_id in vector_store_ids_to_query | ||
| ] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| from pydantic import ValidationError | ||
|
|
||
| from constants import ( | ||
| DEFAULT_BYOK_RAG_RELEVANCE_CUTOFF_SCORE, | ||
| DEFAULT_EMBEDDING_DIMENSION, | ||
| DEFAULT_EMBEDDING_MODEL, | ||
| DEFAULT_RAG_TYPE, | ||
|
|
@@ -35,6 +36,7 @@ def test_byok_rag_configuration_default_values() -> None: | |
| assert byok_rag.vector_db_id == "vector_db_id" | ||
| assert byok_rag.db_path == "tests/configuration/rag.txt" | ||
| assert byok_rag.score_multiplier == DEFAULT_SCORE_MULTIPLIER | ||
| assert byok_rag.relevance_cutoff_score == DEFAULT_BYOK_RAG_RELEVANCE_CUTOFF_SCORE | ||
|
|
||
|
|
||
| def test_byok_rag_configuration_nondefault_values() -> None: | ||
|
|
@@ -54,6 +56,7 @@ def test_byok_rag_configuration_nondefault_values() -> None: | |
| vector_db_id="vector_db_id", | ||
| db_path="tests/configuration/rag.txt", | ||
| score_multiplier=1.0, | ||
| relevance_cutoff_score=0.72, | ||
| ) | ||
| assert byok_rag is not None | ||
| assert byok_rag.rag_id == "rag_id" | ||
|
|
@@ -62,6 +65,7 @@ def test_byok_rag_configuration_nondefault_values() -> None: | |
| assert byok_rag.embedding_dimension == 1024 | ||
| assert byok_rag.vector_db_id == "vector_db_id" | ||
| assert byok_rag.db_path == "tests/configuration/rag.txt" | ||
| assert byok_rag.relevance_cutoff_score == 0.72 | ||
|
|
||
|
|
||
| def test_byok_rag_configuration_wrong_dimension() -> None: | ||
|
|
@@ -199,3 +203,18 @@ def test_byok_rag_configuration_score_multiplier_must_be_positive() -> None: | |
| db_path="tests/configuration/rag.txt", | ||
| score_multiplier=0.0, | ||
| ) | ||
|
|
||
|
|
||
| def test_byok_rag_configuration_relevance_cutoff_must_be_positive() -> None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, do we also want to test behavior if it is negative? |
||
| """Test that relevance_cutoff_score must be greater than 0.""" | ||
| with pytest.raises(ValidationError, match="greater than 0"): | ||
| _ = ByokRag( | ||
| rag_id="rag_id", | ||
| rag_type="rag_type", | ||
| vector_db_id="vector_db_id", | ||
| embedding_model="embedding_model", | ||
| embedding_dimension=1024, | ||
| db_path="tests/configuration/rag.txt", | ||
| score_multiplier=1.0, | ||
| relevance_cutoff_score=0.0, | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.