Skip to content

Commit 984fc43

Browse files
vinkiYujulian-rischclaude
authored
fix!: validate runtime SentenceWindowRetriever window size (#12183)
Co-authored-by: Julian Risch <julian.risch@deepset.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8e64b9b commit 984fc43

4 files changed

Lines changed: 91 additions & 17 deletions

File tree

haystack/components/retrievers/sentence_window_retriever.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ def __init__(
104104
metadata fields. If False, it will skip retrieving the context for documents that are missing
105105
the required metadata fields, but will still include the original document in the results.
106106
"""
107-
if window_size < 1:
108-
raise ValueError("The window_size parameter must be greater than 0.")
107+
self._validate_window_size(window_size)
109108

110109
self.window_size = window_size
111110
self.document_store = document_store
@@ -187,7 +186,8 @@ def run(self, retrieved_documents: list[Document], window_size: int | None = Non
187186
188187
:param retrieved_documents: List of retrieved documents from the previous retriever.
189188
:param window_size: The number of documents to retrieve before and after the relevant one. This will overwrite
190-
the `window_size` parameter set in the constructor.
189+
the `window_size` parameter set in the constructor. It must be greater than 0; values of
190+
0 and negative values are rejected.
191191
:returns:
192192
A dictionary with the following keys:
193193
- `context_windows`: A list of strings, where each string represents the concatenated text from the
@@ -197,8 +197,8 @@ def run(self, retrieved_documents: list[Document], window_size: int | None = Non
197197
meta field.
198198
199199
"""
200-
window_size = window_size or self.window_size
201-
SentenceWindowRetriever._raise_if_windows_size_is_negative(window_size)
200+
window_size = self.window_size if window_size is None else window_size
201+
self._validate_window_size(window_size)
202202
self._raise_if_documents_do_not_have_expected_metadata(retrieved_documents)
203203

204204
context_text = []
@@ -220,7 +220,8 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int
220220
221221
:param retrieved_documents: List of retrieved documents from the previous retriever.
222222
:param window_size: The number of documents to retrieve before and after the relevant one. This will overwrite
223-
the `window_size` parameter set in the constructor.
223+
the `window_size` parameter set in the constructor. It must be greater than 0; values of
224+
0 and negative values are rejected.
224225
:returns:
225226
A dictionary with the following keys:
226227
- `context_windows`: A list of strings, where each string represents the concatenated text from the
@@ -230,8 +231,8 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int
230231
meta field.
231232
232233
"""
233-
window_size = window_size or self.window_size
234-
SentenceWindowRetriever._raise_if_windows_size_is_negative(window_size)
234+
window_size = self.window_size if window_size is None else window_size
235+
self._validate_window_size(window_size)
235236
self._raise_if_documents_do_not_have_expected_metadata(retrieved_documents)
236237

237238
context_text = []
@@ -244,7 +245,7 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int
244245
return {"context_windows": context_text, "context_documents": context_documents}
245246

246247
@staticmethod
247-
def _raise_if_windows_size_is_negative(window_size: int) -> None:
248+
def _validate_window_size(window_size: int) -> None:
248249
if window_size < 1:
249250
raise ValueError("The window_size parameter must be greater than 0.")
250251

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
upgrade:
3+
- |
4+
Passing ``window_size=0`` to ``SentenceWindowRetriever.run`` or ``SentenceWindowRetriever.run_async``
5+
now raises a ``ValueError`` instead of silently using the ``window_size`` set in the constructor. You are
6+
affected if you pass ``window_size=0`` at runtime, either directly or from an upstream component in a
7+
pipeline. If you were relying on ``0`` to mean "use the value from the constructor", omit the argument
8+
(or pass ``None``) instead:
9+
10+
.. code:: python
11+
12+
retriever = SentenceWindowRetriever(document_store=document_store, window_size=3)
13+
14+
# Before: silently used window_size=3
15+
retriever.run(retrieved_documents=docs, window_size=0)
16+
17+
# After: omit the argument to use the constructor value
18+
retriever.run(retrieved_documents=docs)
19+
fixes:
20+
- |
21+
``SentenceWindowRetriever.run`` and ``SentenceWindowRetriever.run_async`` now validate an explicitly
22+
provided ``window_size=0`` instead of treating it as unset and falling back to the constructor value.

test/components/retrievers/test_sentence_window_retriever.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,35 @@ def test_document_without_all_source_ids(self, in_memory_doc_store):
174174
)
175175
retriever.run(retrieved_documents=docs)
176176

177-
def test_run_invalid_window_size(self, in_memory_doc_store):
178-
docs = [Document(content="This is a text with some words. There is a ", meta={"id": "doc_0", "split_id": 0})]
177+
def test_init_rejects_zero_window_size(self, in_memory_doc_store):
179178
with pytest.raises(ValueError):
180-
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0)
181-
retriever.run(retrieved_documents=docs)
179+
SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0)
180+
181+
def test_run_rejects_invalid_runtime_window_size(self, in_memory_doc_store):
182+
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=3)
183+
184+
with pytest.raises(ValueError, match="window_size parameter must be greater than 0"):
185+
retriever.run(retrieved_documents=[], window_size=0)
186+
187+
with pytest.raises(ValueError, match="window_size parameter must be greater than 0"):
188+
retriever.run(retrieved_documents=[], window_size=-1)
189+
190+
def test_run_without_runtime_window_size_uses_constructor_value(self, in_memory_doc_store):
191+
docs = [
192+
Document(content=f"Sentence {sent}.", meta={"id": f"doc_{sent}", "source_id": "source1", "split_id": sent})
193+
for sent in range(10)
194+
]
195+
in_memory_doc_store.write_documents(docs)
196+
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=2)
197+
retrieved_documents = [doc for doc in docs if doc.content == "Sentence 4."]
198+
199+
# window_size omitted: the constructor value is used, so 2 documents on each side
200+
result = retriever.run(retrieved_documents=retrieved_documents)
201+
assert len(result["context_documents"]) == 5
202+
203+
# window_size passed explicitly: it overrides the constructor value
204+
result = retriever.run(retrieved_documents=retrieved_documents, window_size=1)
205+
assert len(result["context_documents"]) == 3
182206

183207
def test_constructor_parameter_does_not_change(self, in_memory_doc_store):
184208
retriever = SentenceWindowRetriever(in_memory_doc_store, window_size=5)

test/components/retrievers/test_sentence_window_retriever_async.py

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616

1717
class TestSentenceWindowRetrieverAsync:
18+
@pytest.mark.asyncio
1819
async def test_document_without_split_id(self, in_memory_doc_store):
1920
docs = [
2021
Document(content="This is a text with some words. There is a ", meta={"id": "doc_0"}),
@@ -62,11 +63,37 @@ async def test_document_without_all_source_ids(self, in_memory_doc_store):
6263
await retriever.run_async(retrieved_documents=docs)
6364

6465
@pytest.mark.asyncio
65-
async def test_run_async_invalid_window_size(self, in_memory_doc_store):
66-
docs = [Document(content="This is a text with some words. There is a ", meta={"id": "doc_0", "split_id": 0})]
66+
async def test_init_rejects_zero_window_size(self, in_memory_doc_store):
6767
with pytest.raises(ValueError):
68-
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0)
69-
await retriever.run_async(retrieved_documents=docs)
68+
SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0)
69+
70+
@pytest.mark.asyncio
71+
async def test_run_async_rejects_invalid_runtime_window_size(self, in_memory_doc_store):
72+
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=3)
73+
74+
with pytest.raises(ValueError, match="window_size parameter must be greater than 0"):
75+
await retriever.run_async(retrieved_documents=[], window_size=0)
76+
77+
with pytest.raises(ValueError, match="window_size parameter must be greater than 0"):
78+
await retriever.run_async(retrieved_documents=[], window_size=-1)
79+
80+
@pytest.mark.asyncio
81+
async def test_run_async_without_runtime_window_size_uses_constructor_value(self, in_memory_doc_store):
82+
docs = [
83+
Document(content=f"Sentence {sent}.", meta={"id": f"doc_{sent}", "source_id": "source1", "split_id": sent})
84+
for sent in range(10)
85+
]
86+
in_memory_doc_store.write_documents(docs)
87+
retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=2)
88+
retrieved_documents = [doc for doc in docs if doc.content == "Sentence 4."]
89+
90+
# window_size omitted: the constructor value is used, so 2 documents on each side
91+
result = await retriever.run_async(retrieved_documents=retrieved_documents)
92+
assert len(result["context_documents"]) == 5
93+
94+
# window_size passed explicitly: it overrides the constructor value
95+
result = await retriever.run_async(retrieved_documents=retrieved_documents, window_size=1)
96+
assert len(result["context_documents"]) == 3
7097

7198
@pytest.mark.asyncio
7299
async def test_constructor_parameter_does_not_change(self, in_memory_doc_store):

0 commit comments

Comments
 (0)