From 092f2654ddf097688990b96ba4feca197f80ee05 Mon Sep 17 00:00:00 2001 From: vinkiYu Date: Wed, 29 Jul 2026 10:41:00 +0800 Subject: [PATCH 1/2] fix: validate runtime sentence window size --- .../retrievers/sentence_window_retriever.py | 19 ++++++++++--------- ...tence-window-runtime-window-size-zero.yaml | 5 +++++ .../test_sentence_window_retriever.py | 17 ++++++++++++----- .../test_sentence_window_retriever_async.py | 17 +++++++++++++---- 4 files changed, 40 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml diff --git a/haystack/components/retrievers/sentence_window_retriever.py b/haystack/components/retrievers/sentence_window_retriever.py index e24b2666d6..0eecfb1297 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 @@ -187,7 +186,8 @@ def run(self, retrieved_documents: list[Document], window_size: int | None = Non :param retrieved_documents: List of retrieved documents from the previous retriever. :param window_size: The number of documents to retrieve before and after the relevant one. This will overwrite - the `window_size` parameter set in the constructor. + the `window_size` parameter set in the constructor. It must be greater than 0; values of + 0 and negative values are rejected. :returns: A dictionary with the following keys: - `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 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 = [] @@ -220,7 +220,8 @@ async def run_async(self, retrieved_documents: list[Document], window_size: int :param retrieved_documents: List of retrieved documents from the previous retriever. :param window_size: The number of documents to retrieve before and after the relevant one. This will overwrite - the `window_size` parameter set in the constructor. + the `window_size` parameter set in the constructor. It must be greater than 0; values of + 0 and negative values are rejected. :returns: A dictionary with the following keys: - `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 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 +245,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-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_sentence_window_retriever.py b/test/components/retrievers/test_sentence_window_retriever.py index d7cb222729..9698c6c5da 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,11 +174,18 @@ 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): - docs = [Document(content="This is a text with some words. There is a ", meta={"id": "doc_0", "split_id": 0})] + def test_init_rejects_zero_window_size(self, in_memory_doc_store): with pytest.raises(ValueError): - retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) - retriever.run(retrieved_documents=docs) + SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) + + def test_run_rejects_invalid_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) diff --git a/test/components/retrievers/test_sentence_window_retriever_async.py b/test/components/retrievers/test_sentence_window_retriever_async.py index 3c776f5b9f..f5a4c0b447 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"}), @@ -62,11 +63,19 @@ async def test_document_without_all_source_ids(self, in_memory_doc_store): await retriever.run_async(retrieved_documents=docs) @pytest.mark.asyncio - async def test_run_async_invalid_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})] + async def test_init_rejects_zero_window_size(self, in_memory_doc_store): with pytest.raises(ValueError): - retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) - await retriever.run_async(retrieved_documents=docs) + SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=0) + + @pytest.mark.asyncio + async def test_run_async_rejects_invalid_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): From 405577ca3b9a6fb4d0818b304226073ce4ead922 Mon Sep 17 00:00:00 2001 From: Julian Risch Date: Wed, 29 Jul 2026 12:19:10 +0200 Subject: [PATCH 2/2] test: cover constructor window_size fallback and expand release note Address review feedback on the runtime window_size validation: - Reword the release note from the user's perspective and add an `upgrade` section, since passing `window_size=0` at runtime now raises instead of silently using the constructor value. - Regenerate the release note filename with `hatch run release-note` so it carries reno's hash suffix like every other note. - Revert the unrelated rename of `test_init_with_invalid_window_size_parameter`. - Add sync and async tests asserting that omitting `window_size` uses the constructor value, covering the other half of the changed line. Co-Authored-By: Claude Opus 5 (1M context) --- ...ime-window-size-zero-774d768cefcde325.yaml | 22 +++++++++++++++++++ ...tence-window-runtime-window-size-zero.yaml | 5 ----- .../test_sentence_window_retriever.py | 19 +++++++++++++++- .../test_sentence_window_retriever_async.py | 18 +++++++++++++++ 4 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/fix-sentence-window-runtime-window-size-zero-774d768cefcde325.yaml delete mode 100644 releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml diff --git a/releasenotes/notes/fix-sentence-window-runtime-window-size-zero-774d768cefcde325.yaml b/releasenotes/notes/fix-sentence-window-runtime-window-size-zero-774d768cefcde325.yaml new file mode 100644 index 0000000000..efd6eb8a27 --- /dev/null +++ b/releasenotes/notes/fix-sentence-window-runtime-window-size-zero-774d768cefcde325.yaml @@ -0,0 +1,22 @@ +--- +upgrade: + - | + Passing ``window_size=0`` to ``SentenceWindowRetriever.run`` or ``SentenceWindowRetriever.run_async`` + now raises a ``ValueError`` instead of silently using the ``window_size`` set in the constructor. You are + affected if you pass ``window_size=0`` at runtime, either directly or from an upstream component in a + pipeline. If you were relying on ``0`` to mean "use the value from the constructor", omit the argument + (or pass ``None``) instead: + + .. code:: python + + retriever = SentenceWindowRetriever(document_store=document_store, window_size=3) + + # Before: silently used window_size=3 + retriever.run(retrieved_documents=docs, window_size=0) + + # After: omit the argument to use the constructor value + retriever.run(retrieved_documents=docs) +fixes: + - | + ``SentenceWindowRetriever.run`` and ``SentenceWindowRetriever.run_async`` now validate an explicitly + provided ``window_size=0`` instead of treating it as unset and falling back to the constructor value. diff --git a/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml b/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml deleted file mode 100644 index be0dede453..0000000000 --- a/releasenotes/notes/fix-sentence-window-runtime-window-size-zero.yaml +++ /dev/null @@ -1,5 +0,0 @@ ---- -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_sentence_window_retriever.py b/test/components/retrievers/test_sentence_window_retriever.py index 9698c6c5da..7d8f45ef90 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_invalid_window_size(self, in_memory_doc_store): + def test_init_with_invalid_window_size_parameter(self, in_memory_doc_store): with pytest.raises(ValueError): SentenceWindowRetriever(in_memory_doc_store, window_size=-2) @@ -187,6 +187,23 @@ def test_run_rejects_invalid_runtime_window_size(self, in_memory_doc_store): with pytest.raises(ValueError, match="window_size parameter must be greater than 0"): retriever.run(retrieved_documents=[], window_size=-1) + def test_run_without_runtime_window_size_uses_constructor_value(self, in_memory_doc_store): + docs = [ + Document(content=f"Sentence {sent}.", meta={"id": f"doc_{sent}", "source_id": "source1", "split_id": sent}) + for sent in range(10) + ] + in_memory_doc_store.write_documents(docs) + retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=2) + retrieved_documents = [doc for doc in docs if doc.content == "Sentence 4."] + + # window_size omitted: the constructor value is used, so 2 documents on each side + result = retriever.run(retrieved_documents=retrieved_documents) + assert len(result["context_documents"]) == 5 + + # window_size passed explicitly: it overrides the constructor value + result = retriever.run(retrieved_documents=retrieved_documents, window_size=1) + assert len(result["context_documents"]) == 3 + 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 f5a4c0b447..de85b08e60 100644 --- a/test/components/retrievers/test_sentence_window_retriever_async.py +++ b/test/components/retrievers/test_sentence_window_retriever_async.py @@ -77,6 +77,24 @@ async def test_run_async_rejects_invalid_runtime_window_size(self, in_memory_doc 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_run_async_without_runtime_window_size_uses_constructor_value(self, in_memory_doc_store): + docs = [ + Document(content=f"Sentence {sent}.", meta={"id": f"doc_{sent}", "source_id": "source1", "split_id": sent}) + for sent in range(10) + ] + in_memory_doc_store.write_documents(docs) + retriever = SentenceWindowRetriever(document_store=in_memory_doc_store, window_size=2) + retrieved_documents = [doc for doc in docs if doc.content == "Sentence 4."] + + # window_size omitted: the constructor value is used, so 2 documents on each side + result = await retriever.run_async(retrieved_documents=retrieved_documents) + assert len(result["context_documents"]) == 5 + + # window_size passed explicitly: it overrides the constructor value + result = await retriever.run_async(retrieved_documents=retrieved_documents, window_size=1) + assert len(result["context_documents"]) == 3 + @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)