-
Notifications
You must be signed in to change notification settings - Fork 210
feat: support additional search_kwargs with OpenSearchEmbeddingRetriever
#2825
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?
Conversation
search_kwargs with OpenSearchEmbeddingRetriever
...opensearch/src/haystack_integrations/components/retrievers/opensearch/embedding_retriever.py
Outdated
Show resolved
Hide resolved
...opensearch/src/haystack_integrations/components/retrievers/opensearch/embedding_retriever.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds support for passing additional OpenSearch kNN request parameters (search_kwargs) through OpenSearchEmbeddingRetriever into the OpenSearch document store’s embedding search request, enabling further tuning of approximate kNN search behavior.
Changes:
- Extend
OpenSearchEmbeddingRetrieverto accept, serialize, and forwardsearch_kwargs(sync + async). - Extend
OpenSearchDocumentStoreembedding search request builder to mergesearch_kwargsinto theknnquery body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py |
Adds search_kwargs to the embedding-search request builder and merges into the knn.embedding clause. |
integrations/opensearch/src/haystack_integrations/components/retrievers/opensearch/embedding_retriever.py |
Adds search_kwargs to retriever init/run APIs, persists it on the component, and forwards it to the document store (sync + async). |
Comments suppressed due to low confidence (1)
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py:1420
search_kwargsis accepted by_prepare_embedding_search_request, but whencustom_queryis provided it is currently ignored entirely. This is surprising for callers (they can passsearch_kwargsand see no effect). Consider either explicitly documenting thatsearch_kwargsonly applies to the default query path, or raising aValueErrorwhen bothcustom_queryandsearch_kwargsare provided to avoid silent no-ops.
search_kwargs: dict[str, Any] | None = None,
) -> dict[str, Any]:
if not query_embedding:
msg = "query_embedding must be a non-empty list of floats"
raise ValueError(msg)
body: dict[str, Any]
if isinstance(custom_query, dict):
body = self._render_custom_query(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Related Issues
Proposed Changes:
OpenSearchEmbeddingRetrieveref_searchorkparameters), but also enables rescoringHow did you test it?
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.