diff --git a/haystack/components/retrievers/filter_retriever.py b/haystack/components/retrievers/filter_retriever.py index 29c68f11ec..c09d48c238 100644 --- a/haystack/components/retrievers/filter_retriever.py +++ b/haystack/components/retrievers/filter_retriever.py @@ -29,7 +29,8 @@ class FilterRetriever: doc_store.write_documents(docs) retriever = FilterRetriever(doc_store, filters={"field": "lang", "operator": "==", "value": "en"}) - # if passed in the run method, filters override those provided at initialization + # If passed in the run method, filters override those provided at initialization. + # Passing an empty dictionary explicitly clears the initialization filters. result = retriever.run(filters={"field": "lang", "operator": "==", "value": "de"}) print(result["documents"]) @@ -44,6 +45,7 @@ def __init__(self, document_store: DocumentStore, filters: dict[str, Any] | None An instance of a Document Store to use with the Retriever. :param filters: A dictionary with filters to narrow down the search space. + Passing an empty dictionary explicitly clears filters provided at initialization. """ self.document_store = document_store self.filters = filters @@ -82,11 +84,12 @@ def run(self, filters: dict[str, Any] | None = None) -> dict[str, Any]: :param filters: A dictionary with filters to narrow down the search space. + Passing an empty dictionary explicitly clears filters provided at initialization. If not specified, the FilterRetriever uses the values provided at initialization. :returns: A list of retrieved documents. """ - return {"documents": self.document_store.filter_documents(filters=filters or self.filters)} + return {"documents": self.document_store.filter_documents(filters=self.filters if filters is None else filters)} @component.output_types(documents=list[Document]) async def run_async(self, filters: dict[str, Any] | None = None) -> dict[str, Any]: @@ -100,7 +103,9 @@ async def run_async(self, filters: dict[str, Any] | None = None) -> dict[str, An A list of retrieved documents. """ # 'ignore' since filter_documents_async is not defined in the Protocol but exists in the implementations - out_documents = await self.document_store.filter_documents_async(filters=filters or self.filters) # type: ignore[attr-defined] + out_documents = await self.document_store.filter_documents_async( # type: ignore[attr-defined] + filters=self.filters if filters is None else filters + ) return {"documents": out_documents} def close(self) -> None: diff --git a/haystack/components/retrievers/sentence_window_retriever.py b/haystack/components/retrievers/sentence_window_retriever.py index e24b2666d6..337f1a5644 100644 --- a/haystack/components/retrievers/sentence_window_retriever.py +++ b/haystack/components/retrievers/sentence_window_retriever.py @@ -104,8 +104,7 @@ def __init__( metadata fields. If False, it will skip retrieving the context for documents that are missing the required metadata fields, but will still include the original document in the results. """ - if window_size < 1: - raise ValueError("The window_size parameter must be greater than 0.") + self._validate_window_size(window_size) self.window_size = window_size self.document_store = document_store @@ -197,8 +196,8 @@ def run(self, retrieved_documents: list[Document], window_size: int | None = Non meta field. """ - window_size = window_size or self.window_size - SentenceWindowRetriever._raise_if_windows_size_is_negative(window_size) + window_size = self.window_size if window_size is None else window_size + self._validate_window_size(window_size) self._raise_if_documents_do_not_have_expected_metadata(retrieved_documents) context_text = [] @@ -230,8 +229,8 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int meta field. """ - window_size = window_size or self.window_size - SentenceWindowRetriever._raise_if_windows_size_is_negative(window_size) + window_size = self.window_size if window_size is None else window_size + self._validate_window_size(window_size) self._raise_if_documents_do_not_have_expected_metadata(retrieved_documents) context_text = [] @@ -244,7 +243,7 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int return {"context_windows": context_text, "context_documents": context_documents} @staticmethod - def _raise_if_windows_size_is_negative(window_size: int) -> None: + def _validate_window_size(window_size: int) -> None: if window_size < 1: raise ValueError("The window_size parameter must be greater than 0.") diff --git a/releasenotes/notes/fix-runtime-empty-retriever-overrides.yaml b/releasenotes/notes/fix-runtime-empty-retriever-overrides.yaml new file mode 100644 index 0000000000..7b90843bf2 --- /dev/null +++ b/releasenotes/notes/fix-runtime-empty-retriever-overrides.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + FilterRetriever now treats an explicitly provided empty runtime filter as an override + of initialization filters in both synchronous and asynchronous execution. diff --git a/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml b/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml new file mode 100644 index 0000000000..be0dede453 --- /dev/null +++ b/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + SentenceWindowRetriever now validates an explicitly provided runtime ``window_size=0`` + instead of silently falling back to the constructor value. diff --git a/test/components/retrievers/test_filter_retriever.py b/test/components/retrievers/test_filter_retriever.py index 6974b4ed04..0b473cbf0e 100644 --- a/test/components/retrievers/test_filter_retriever.py +++ b/test/components/retrievers/test_filter_retriever.py @@ -122,6 +122,13 @@ def test_retriever_init_filter_run_filter_override(self, sample_document_store, assert len(result["documents"]) == 2 assert TestFilterRetriever._documents_equal(result["documents"], sample_docs["de_docs"]) + def test_retriever_runtime_empty_filter_clears_init_filter(self, sample_document_store, sample_docs): + retriever = FilterRetriever(sample_document_store, filters={"field": "lang", "operator": "==", "value": "en"}) + result = retriever.run(filters={}) + + assert len(result["documents"]) == len(sample_docs["all_docs"]) + assert TestFilterRetriever._documents_equal(result["documents"], sample_docs["all_docs"]) + @pytest.mark.integration def test_run_with_pipeline(self, sample_document_store, sample_docs): retriever = FilterRetriever(sample_document_store, filters={"field": "lang", "operator": "==", "value": "de"}) @@ -146,6 +153,12 @@ def test_run_with_pipeline(self, sample_document_store, sample_docs): assert results_docs assert TestFilterRetriever._documents_equal(results_docs, sample_docs["en_docs"]) + result: dict[str, Any] = pipeline.run(data={"retriever": {"filters": {}}}) + + results_docs = result["retriever"]["documents"] + assert len(results_docs) == len(sample_docs["all_docs"]) + assert TestFilterRetriever._documents_equal(results_docs, sample_docs["all_docs"]) + def test_close(self): closable_document_store = Mock(spec=["close"]) retriever = FilterRetriever(document_store=closable_document_store) diff --git a/test/components/retrievers/test_filter_retriever_async.py b/test/components/retrievers/test_filter_retriever_async.py index 1786cbbcf4..f024ca85f4 100644 --- a/test/components/retrievers/test_filter_retriever_async.py +++ b/test/components/retrievers/test_filter_retriever_async.py @@ -71,6 +71,14 @@ async def test_retriever_init_filter_run_filter_override(self, sample_document_s assert len(result["documents"]) == 2 assert TestFilterRetrieverAsync._documents_equal(result["documents"], sample_docs["de_docs"]) + @pytest.mark.asyncio + async def test_retriever_runtime_empty_filter_clears_init_filter(self, sample_document_store, sample_docs): + retriever = FilterRetriever(sample_document_store, filters={"field": "lang", "operator": "==", "value": "en"}) + result = await retriever.run_async(filters={}) + + assert len(result["documents"]) == len(sample_docs["all_docs"]) + assert TestFilterRetrieverAsync._documents_equal(result["documents"], sample_docs["all_docs"]) + @pytest.mark.asyncio @pytest.mark.integration async def test_run_with_pipeline(self, sample_document_store, sample_docs): @@ -96,6 +104,12 @@ async def test_run_with_pipeline(self, sample_document_store, sample_docs): assert results_docs assert TestFilterRetrieverAsync._documents_equal(results_docs, sample_docs["en_docs"]) + result: dict[str, Any] = await pipeline.run_async(data={"retriever": {"filters": {}}}) + + results_docs = result["retriever"]["documents"] + assert len(results_docs) == len(sample_docs["all_docs"]) + assert TestFilterRetrieverAsync._documents_equal(results_docs, sample_docs["all_docs"]) + @pytest.mark.asyncio async def test_close_async(self): closable_document_store = Mock(spec=["close_async"]) diff --git a/test/components/retrievers/test_sentence_window_retriever.py b/test/components/retrievers/test_sentence_window_retriever.py index d7cb222729..183c3c5d90 100644 --- a/test/components/retrievers/test_sentence_window_retriever.py +++ b/test/components/retrievers/test_sentence_window_retriever.py @@ -24,7 +24,7 @@ def test_init_with_parameters(self, in_memory_doc_store): retriever = SentenceWindowRetriever(in_memory_doc_store, window_size=5) assert retriever.window_size == 5 - def test_init_with_invalid_window_size_parameter(self, in_memory_doc_store): + def test_init_invalid_window_size(self, in_memory_doc_store): with pytest.raises(ValueError): SentenceWindowRetriever(in_memory_doc_store, window_size=-2) @@ -174,12 +174,21 @@ def test_document_without_all_source_ids(self, in_memory_doc_store): ) retriever.run(retrieved_documents=docs) - def test_run_invalid_window_size(self, in_memory_doc_store): + def test_init_rejects_zero_window_size(self, in_memory_doc_store): docs = [Document(content="This is a text with some words. There is a ", meta={"id": "doc_0", "split_id": 0})] with pytest.raises(ValueError): retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) retriever.run(retrieved_documents=docs) + def test_run_rejects_zero_runtime_window_size(self, in_memory_doc_store): + retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=3) + + with pytest.raises(ValueError, match="window_size parameter must be greater than 0"): + retriever.run(retrieved_documents=[], window_size=0) + + with pytest.raises(ValueError, match="window_size parameter must be greater than 0"): + retriever.run(retrieved_documents=[], window_size=-1) + def test_constructor_parameter_does_not_change(self, in_memory_doc_store): retriever = SentenceWindowRetriever(in_memory_doc_store, window_size=5) assert retriever.window_size == 5 diff --git a/test/components/retrievers/test_sentence_window_retriever_async.py b/test/components/retrievers/test_sentence_window_retriever_async.py index 3c776f5b9f..3b5cbd5f82 100644 --- a/test/components/retrievers/test_sentence_window_retriever_async.py +++ b/test/components/retrievers/test_sentence_window_retriever_async.py @@ -15,6 +15,7 @@ class TestSentenceWindowRetrieverAsync: + @pytest.mark.asyncio async def test_document_without_split_id(self, in_memory_doc_store): docs = [ Document(content="This is a text with some words. There is a ", meta={"id": "doc_0"}), @@ -68,6 +69,16 @@ async def test_run_async_invalid_window_size(self, in_memory_doc_store): retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) await retriever.run_async(retrieved_documents=docs) + @pytest.mark.asyncio + async def test_run_async_rejects_zero_runtime_window_size(self, in_memory_doc_store): + retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=3) + + with pytest.raises(ValueError, match="window_size parameter must be greater than 0"): + await retriever.run_async(retrieved_documents=[], window_size=0) + + with pytest.raises(ValueError, match="window_size parameter must be greater than 0"): + await retriever.run_async(retrieved_documents=[], window_size=-1) + @pytest.mark.asyncio async def test_constructor_parameter_does_not_change(self, in_memory_doc_store): retriever = SentenceWindowRetriever(in_memory_doc_store, window_size=5)