Skip to content

Commit 20d9bc4

Browse files
fix: apply FilterPolicy.MERGE correctly in in-memory retrievers (#12066)
Co-authored-by: David S. Batista <dsbatista@gmail.com>
1 parent 38144f9 commit 20d9bc4

5 files changed

Lines changed: 114 additions & 18 deletions

File tree

haystack/components/retrievers/in_memory/bm25_retriever.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from haystack import Document, component, default_from_dict, default_to_dict
88
from haystack.document_stores.in_memory import InMemoryDocumentStore
9-
from haystack.document_stores.types import FilterPolicy
9+
from haystack.document_stores.types import FilterPolicy, apply_filter_policy
1010

1111

1212
@component
@@ -143,10 +143,7 @@ def run(
143143
:raises ValueError:
144144
If the specified DocumentStore is not found or is not a InMemoryDocumentStore instance.
145145
"""
146-
if self.filter_policy == FilterPolicy.MERGE and filters:
147-
filters = {**(self.filters or {}), **filters}
148-
else:
149-
filters = filters or self.filters
146+
filters = apply_filter_policy(self.filter_policy, self.filters, filters)
150147
if top_k is None:
151148
top_k = self.top_k
152149
if scale_score is None:
@@ -181,10 +178,7 @@ async def run_async(
181178
:raises ValueError:
182179
If the specified DocumentStore is not found or is not a InMemoryDocumentStore instance.
183180
"""
184-
if self.filter_policy == FilterPolicy.MERGE and filters:
185-
filters = {**(self.filters or {}), **filters}
186-
else:
187-
filters = filters or self.filters
181+
filters = apply_filter_policy(self.filter_policy, self.filters, filters)
188182
if top_k is None:
189183
top_k = self.top_k
190184
if scale_score is None:

haystack/components/retrievers/in_memory/embedding_retriever.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from haystack import Document, component, default_from_dict, default_to_dict
88
from haystack.document_stores.in_memory import InMemoryDocumentStore
9-
from haystack.document_stores.types import FilterPolicy
9+
from haystack.document_stores.types import FilterPolicy, apply_filter_policy
1010

1111

1212
@component
@@ -163,10 +163,7 @@ def run(
163163
:raises ValueError:
164164
If the specified DocumentStore is not found or is not an InMemoryDocumentStore instance.
165165
"""
166-
if self.filter_policy == FilterPolicy.MERGE and filters:
167-
filters = {**(self.filters or {}), **filters}
168-
else:
169-
filters = filters or self.filters
166+
filters = apply_filter_policy(self.filter_policy, self.filters, filters)
170167
if top_k is None:
171168
top_k = self.top_k
172169
if scale_score is None:
@@ -214,10 +211,7 @@ async def run_async(
214211
:raises ValueError:
215212
If the specified DocumentStore is not found or is not an InMemoryDocumentStore instance.
216213
"""
217-
if self.filter_policy == FilterPolicy.MERGE and filters:
218-
filters = {**(self.filters or {}), **filters}
219-
else:
220-
filters = filters or self.filters
214+
filters = apply_filter_policy(self.filter_policy, self.filters, filters)
221215
if top_k is None:
222216
top_k = self.top_k
223217
if scale_score is None:
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Fix ``FilterPolicy.MERGE`` in ``InMemoryBM25Retriever`` and
5+
``InMemoryEmbeddingRetriever`` so initialization filters are combined with
6+
runtime comparison filters instead of being silently overwritten.

test/components/retrievers/test_in_memory_bm25_retriever.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,47 @@ def test_retriever_valid_run(self, in_memory_doc_store, mock_docs):
139139
assert len(result["documents"]) == 5
140140
assert result["documents"][0].content == "PHP is a popular programming language"
141141

142+
def test_run_with_filter_policy_merge_combines_init_and_runtime_filters(self, in_memory_doc_store):
143+
in_memory_doc_store.write_documents(
144+
[
145+
Document(content="python article current", meta={"type": "article", "year": 2020}),
146+
Document(content="python blog current", meta={"type": "blog", "year": 2021}),
147+
Document(content="python article archived", meta={"type": "article", "year": 2019}),
148+
]
149+
)
150+
151+
retriever = InMemoryBM25Retriever(
152+
in_memory_doc_store,
153+
filters={"field": "meta.type", "operator": "==", "value": "article"},
154+
filter_policy=FilterPolicy.MERGE,
155+
)
156+
157+
result = retriever.run(query="python", filters={"field": "meta.year", "operator": ">=", "value": 2020})
158+
159+
assert [doc.content for doc in result["documents"]] == ["python article current"]
160+
161+
@pytest.mark.asyncio
162+
async def test_run_async_with_filter_policy_merge_combines_init_and_runtime_filters(self, in_memory_doc_store):
163+
in_memory_doc_store.write_documents(
164+
[
165+
Document(content="python article current", meta={"type": "article", "year": 2020}),
166+
Document(content="python blog current", meta={"type": "blog", "year": 2021}),
167+
Document(content="python article archived", meta={"type": "article", "year": 2019}),
168+
]
169+
)
170+
171+
retriever = InMemoryBM25Retriever(
172+
in_memory_doc_store,
173+
filters={"field": "meta.type", "operator": "==", "value": "article"},
174+
filter_policy=FilterPolicy.MERGE,
175+
)
176+
177+
result = await retriever.run_async(
178+
query="python", filters={"field": "meta.year", "operator": ">=", "value": 2020}
179+
)
180+
181+
assert [doc.content for doc in result["documents"]] == ["python article current"]
182+
142183
def test_invalid_run_wrong_store_type(self):
143184
SomeOtherDocumentStore = document_store_class("SomeOtherDocumentStore")
144185
with pytest.raises(TypeError, match="document_store must be an instance of InMemoryDocumentStore"):

test/components/retrievers/test_in_memory_embedding_retriever.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,67 @@ def test_valid_run(self):
141141
assert len(result["documents"]) == top_k
142142
assert result["documents"][0].embedding == [1.0, 1.0, 1.0, 1.0]
143143

144+
def test_run_with_filter_policy_merge_combines_init_and_runtime_filters(self):
145+
ds = InMemoryDocumentStore(embedding_similarity_function="cosine")
146+
ds.write_documents(
147+
[
148+
Document(
149+
content="python article current",
150+
embedding=[1.0, 0.0, 0.0, 0.0],
151+
meta={"type": "article", "year": 2020},
152+
),
153+
Document(
154+
content="python blog current", embedding=[1.0, 0.0, 0.0, 0.0], meta={"type": "blog", "year": 2021}
155+
),
156+
Document(
157+
content="python article archived",
158+
embedding=[1.0, 0.0, 0.0, 0.0],
159+
meta={"type": "article", "year": 2019},
160+
),
161+
]
162+
)
163+
164+
retriever = InMemoryEmbeddingRetriever(
165+
ds, filters={"field": "meta.type", "operator": "==", "value": "article"}, filter_policy=FilterPolicy.MERGE
166+
)
167+
168+
result = retriever.run(
169+
query_embedding=[1.0, 0.0, 0.0, 0.0], filters={"field": "meta.year", "operator": ">=", "value": 2020}
170+
)
171+
172+
assert [doc.content for doc in result["documents"]] == ["python article current"]
173+
174+
@pytest.mark.asyncio
175+
async def test_run_async_with_filter_policy_merge_combines_init_and_runtime_filters(self):
176+
ds = InMemoryDocumentStore(embedding_similarity_function="cosine")
177+
ds.write_documents(
178+
[
179+
Document(
180+
content="python article current",
181+
embedding=[1.0, 0.0, 0.0, 0.0],
182+
meta={"type": "article", "year": 2020},
183+
),
184+
Document(
185+
content="python blog current", embedding=[1.0, 0.0, 0.0, 0.0], meta={"type": "blog", "year": 2021}
186+
),
187+
Document(
188+
content="python article archived",
189+
embedding=[1.0, 0.0, 0.0, 0.0],
190+
meta={"type": "article", "year": 2019},
191+
),
192+
]
193+
)
194+
195+
retriever = InMemoryEmbeddingRetriever(
196+
ds, filters={"field": "meta.type", "operator": "==", "value": "article"}, filter_policy=FilterPolicy.MERGE
197+
)
198+
199+
result = await retriever.run_async(
200+
query_embedding=[1.0, 0.0, 0.0, 0.0], filters={"field": "meta.year", "operator": ">=", "value": 2020}
201+
)
202+
203+
assert [doc.content for doc in result["documents"]] == ["python article current"]
204+
144205
def test_invalid_run_wrong_store_type(self):
145206
SomeOtherDocumentStore = document_store_class("SomeOtherDocumentStore")
146207
with pytest.raises(TypeError, match="document_store must be an instance of InMemoryDocumentStore"):

0 commit comments

Comments
 (0)