Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions haystack/components/retrievers/filter_retriever.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand All @@ -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
Expand Down Expand Up @@ -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]:
Expand All @@ -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:
Expand Down
13 changes: 6 additions & 7 deletions haystack/components/retrievers/sentence_window_retriever.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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 = []
Expand All @@ -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.")

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
SentenceWindowRetriever now validates an explicitly provided runtime ``window_size=0``
instead of silently falling back to the constructor value.
13 changes: 13 additions & 0 deletions test/components/retrievers/test_filter_retriever.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand All @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions test/components/retrievers/test_filter_retriever_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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"])
Expand Down
13 changes: 11 additions & 2 deletions test/components/retrievers/test_sentence_window_retriever.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"}),
Expand Down Expand Up @@ -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)
Expand Down
Loading