From c7a71e8321fbb6b1294181d28dff87a8164edd64 Mon Sep 17 00:00:00 2001 From: manjunathbhaskar Date: Sun, 2 Aug 2026 10:48:07 +0200 Subject: [PATCH 1/3] fix: DocumentJoiner top_k=0 is treated as unset instead of returning zero documents Both the run() and __init__() top_k parameters were checked with a truthy check (if top_k / elif self.top_k), so top_k=0, a legitimate request to return no documents, was silently treated as unset and fell back to the other value instead. Introduced in #7709 when the run() top_k parameter was added. Adds a regression test covering the run-time top_k=0 case. --- haystack/components/joiners/document_joiner.py | 4 ++-- test/components/joiners/test_document_joiner.py | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/haystack/components/joiners/document_joiner.py b/haystack/components/joiners/document_joiner.py index c63b821c9da..c7915b919e7 100644 --- a/haystack/components/joiners/document_joiner.py +++ b/haystack/components/joiners/document_joiner.py @@ -160,9 +160,9 @@ def run(self, documents: Variadic[list[Document]], top_k: int | None = None) -> "score, so those with score=None were sorted as if they had a score of -infinity." ) - if top_k: + if top_k is not None: output_documents = output_documents[:top_k] - elif self.top_k: + elif self.top_k is not None: output_documents = output_documents[: self.top_k] return {"documents": output_documents} diff --git a/test/components/joiners/test_document_joiner.py b/test/components/joiners/test_document_joiner.py index 4105f4415f5..0ae78637317 100644 --- a/test/components/joiners/test_document_joiner.py +++ b/test/components/joiners/test_document_joiner.py @@ -294,6 +294,15 @@ def test_run_with_top_k_in_run_method(self): output = joiner.run([documents_1, documents_2], top_k=top_k) assert len(output["documents"]) == top_k + def test_run_with_top_k_zero_in_run_method_overrides_init_top_k(self): + # A run-time top_k=0 must be honored (return no documents), not treated as "unset" + # and fall back to the instance's top_k. + joiner = DocumentJoiner(top_k=5) + documents_1 = [Document(content="a"), Document(content="b"), Document(content="c")] + documents_2 = [Document(content="d"), Document(content="e"), Document(content="f")] + output = joiner.run([documents_1, documents_2], top_k=0) + assert len(output["documents"]) == 0 + def test_sort_by_score_without_scores(self, caplog): joiner = DocumentJoiner() with caplog.at_level(logging.INFO): From 21814daa38dfe382bbfcffa3209c0dda5a4d4266 Mon Sep 17 00:00:00 2001 From: "David S. Batista" Date: Wed, 5 Aug 2026 16:27:48 +0200 Subject: [PATCH 2/3] also check for negative at init and apply same beahviour to AnswerJoiner + tests --- haystack/components/joiners/answer_joiner.py | 18 +++++++-- .../components/joiners/document_joiner.py | 13 ++++++- test/components/joiners/test_answer_joiner.py | 38 +++++++++++++++++++ .../joiners/test_document_joiner.py | 21 ++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/haystack/components/joiners/answer_joiner.py b/haystack/components/joiners/answer_joiner.py index 0a8b683833d..95148e1f02b 100644 --- a/haystack/components/joiners/answer_joiner.py +++ b/haystack/components/joiners/answer_joiner.py @@ -94,11 +94,16 @@ def __init__( Specifies the join mode to use. Available modes: - `concatenate`: Concatenates multiple lists of Answers into a single list. :param top_k: - The maximum number of Answers to return. + The maximum number of Answers to return. Must be `None` or greater than 0. :param sort_by_score: If `True`, sorts the documents by score in descending order. If a document has no score, it is handled as if its score is -infinity. + + :raises ValueError: + If `top_k` is not `None` and is less than or equal to 0. """ + if top_k is not None and top_k <= 0: + raise ValueError("top_k must be greater than 0.") if isinstance(join_mode, str): join_mode = JoinMode.from_str(join_mode) join_mode_functions: dict[JoinMode, Callable[[list[list[AnswerType]]], list[AnswerType]]] = { @@ -119,10 +124,14 @@ def run(self, answers: Variadic[list[AnswerType]], top_k: int | None = None) -> :param top_k: The maximum number of Answers to return. Overrides the instance's `top_k` if provided. + A value of 0 returns no answers. Must not be negative. :returns: A dictionary with the following keys: - `answers`: Merged list of Answers + + :raises ValueError: + If `top_k` is negative. """ answers_list = list(answers) join_function = self.join_mode_function @@ -135,9 +144,12 @@ def run(self, answers: Variadic[list[AnswerType]], top_k: int | None = None) -> reverse=True, ) - top_k = top_k or self.top_k - if top_k: + if top_k is not None: + if top_k < 0: + raise ValueError("top_k must not be negative.") output_answers = output_answers[:top_k] + elif self.top_k is not None: + output_answers = output_answers[: self.top_k] return {"answers": output_answers} def _concatenate(self, answer_lists: list[list[AnswerType]]) -> list[AnswerType]: diff --git a/haystack/components/joiners/document_joiner.py b/haystack/components/joiners/document_joiner.py index c7915b919e7..39563f24494 100644 --- a/haystack/components/joiners/document_joiner.py +++ b/haystack/components/joiners/document_joiner.py @@ -108,11 +108,16 @@ def __init__( `concatenate` or `distribution_based_rank_fusion` join modes. Weight for each list of documents must match the number of inputs. :param top_k: - The maximum number of documents to return. + The maximum number of documents to return. Must be `None` or greater than 0. :param sort_by_score: If `True`, sorts the documents by score in descending order. If a document has no score, it is handled as if its score is -infinity. + + :raises ValueError: + If `top_k` is not `None` and is less than or equal to 0. """ + if top_k is not None and top_k <= 0: + raise ValueError("top_k must be greater than 0.") if isinstance(join_mode, str): join_mode = JoinMode.from_str(join_mode) join_mode_functions = { @@ -142,10 +147,14 @@ def run(self, documents: Variadic[list[Document]], top_k: int | None = None) -> List of list of documents to be merged. :param top_k: The maximum number of documents to return. Overrides the instance's `top_k` if provided. + A value of 0 returns no documents. Must not be negative. :returns: A dictionary with the following keys: - `documents`: Merged list of Documents + + :raises ValueError: + If `top_k` is negative. """ documents = list(documents) output_documents = self.join_mode_function(documents) @@ -161,6 +170,8 @@ def run(self, documents: Variadic[list[Document]], top_k: int | None = None) -> ) if top_k is not None: + if top_k < 0: + raise ValueError("top_k must not be negative.") output_documents = output_documents[:top_k] elif self.top_k is not None: output_documents = output_documents[: self.top_k] diff --git a/test/components/joiners/test_answer_joiner.py b/test/components/joiners/test_answer_joiner.py index fdc64d3f465..6cfc6f2885c 100644 --- a/test/components/joiners/test_answer_joiner.py +++ b/test/components/joiners/test_answer_joiner.py @@ -22,6 +22,20 @@ def test_init_with_custom_parameters(self): assert joiner.top_k == 5 assert joiner.sort_by_score is True + def test_init_with_top_k_none_is_valid(self): + joiner = AnswerJoiner(top_k=None) + assert joiner.top_k is None + + @pytest.mark.parametrize("top_k", [1, 5]) + def test_init_with_positive_top_k_is_valid(self, top_k): + joiner = AnswerJoiner(top_k=top_k) + assert joiner.top_k == top_k + + @pytest.mark.parametrize("top_k", [0, -1]) + def test_init_with_non_positive_top_k_raises(self, top_k): + with pytest.raises(ValueError, match="top_k must be greater than 0"): + AnswerJoiner(top_k=top_k) + def test_to_dict(self): joiner = AnswerJoiner() data = joiner.to_dict() @@ -101,6 +115,30 @@ def test_unsupported_join_mode(self): with pytest.raises(ValueError): AnswerJoiner(join_mode=unsupported_mode) + def test_run_with_top_k_in_run_method_overrides_init_top_k(self): + joiner = AnswerJoiner(top_k=5) + answers1 = [GeneratedAnswer(query="a", data="a", meta={}, documents=[Document(content="a")])] + answers2 = [GeneratedAnswer(query="b", data="b", meta={}, documents=[Document(content="b")])] + answers3 = [GeneratedAnswer(query="c", data="c", meta={}, documents=[Document(content="c")])] + result = joiner.run([answers1, answers2, answers3], top_k=2) + assert len(result["answers"]) == 2 + + def test_run_with_top_k_zero_in_run_method_overrides_init_top_k(self): + # A run-time top_k=0 must be honored (return no answers), not treated as "unset" + # and fall back to the instance's top_k. + joiner = AnswerJoiner(top_k=5) + answers1 = [GeneratedAnswer(query="a", data="a", meta={}, documents=[Document(content="a")])] + answers2 = [GeneratedAnswer(query="b", data="b", meta={}, documents=[Document(content="b")])] + result = joiner.run([answers1, answers2], top_k=0) + assert len(result["answers"]) == 0 + + def test_run_with_negative_top_k_in_run_method_raises(self): + joiner = AnswerJoiner(top_k=5) + answers1 = [GeneratedAnswer(query="a", data="a", meta={}, documents=[Document(content="a")])] + answers2 = [GeneratedAnswer(query="b", data="b", meta={}, documents=[Document(content="b")])] + with pytest.raises(ValueError, match="top_k must not be negative"): + joiner.run([answers1, answers2], top_k=-1) + def test_sort_by_score(self): joiner = AnswerJoiner(sort_by_score=True) answers1 = [ExtractedAnswer(query="a", score=0.3, meta={}, document=Document(content="a"))] diff --git a/test/components/joiners/test_document_joiner.py b/test/components/joiners/test_document_joiner.py index 0ae78637317..3bc72331779 100644 --- a/test/components/joiners/test_document_joiner.py +++ b/test/components/joiners/test_document_joiner.py @@ -31,6 +31,20 @@ def test_init_with_zero_sum_weights_raises(self): with pytest.raises(ValueError, match="must not sum to zero"): DocumentJoiner(join_mode="merge", weights=[0.0, 0.0, 0.0]) + def test_init_with_top_k_none_is_valid(self): + joiner = DocumentJoiner(top_k=None) + assert joiner.top_k is None + + @pytest.mark.parametrize("top_k", [1, 5]) + def test_init_with_positive_top_k_is_valid(self, top_k): + joiner = DocumentJoiner(top_k=top_k) + assert joiner.top_k == top_k + + @pytest.mark.parametrize("top_k", [0, -1]) + def test_init_with_non_positive_top_k_raises(self, top_k): + with pytest.raises(ValueError, match="top_k must be greater than 0"): + DocumentJoiner(top_k=top_k) + def test_to_dict(self): joiner = DocumentJoiner() data = joiner.to_dict() @@ -303,6 +317,13 @@ def test_run_with_top_k_zero_in_run_method_overrides_init_top_k(self): output = joiner.run([documents_1, documents_2], top_k=0) assert len(output["documents"]) == 0 + def test_run_with_negative_top_k_in_run_method_raises(self): + joiner = DocumentJoiner(top_k=5) + documents_1 = [Document(content="a"), Document(content="b"), Document(content="c")] + documents_2 = [Document(content="d"), Document(content="e"), Document(content="f")] + with pytest.raises(ValueError, match="top_k must not be negative"): + joiner.run([documents_1, documents_2], top_k=-1) + def test_sort_by_score_without_scores(self, caplog): joiner = DocumentJoiner() with caplog.at_level(logging.INFO): From 474359154f0c32758a357c01fce049a2bb22e83e Mon Sep 17 00:00:00 2001 From: "David S. Batista" Date: Wed, 5 Aug 2026 16:31:09 +0200 Subject: [PATCH 3/3] adding release notes --- .../joiners-top-k-validation-feb0e2c38da8703e.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 releasenotes/notes/joiners-top-k-validation-feb0e2c38da8703e.yaml diff --git a/releasenotes/notes/joiners-top-k-validation-feb0e2c38da8703e.yaml b/releasenotes/notes/joiners-top-k-validation-feb0e2c38da8703e.yaml new file mode 100644 index 00000000000..1ee44b6865b --- /dev/null +++ b/releasenotes/notes/joiners-top-k-validation-feb0e2c38da8703e.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + ``DocumentJoiner`` and ``AnswerJoiner`` now resolve ``top_k`` consistently and validate it. + Previously, a runtime ``top_k=0`` was treated as "unset" and silently fell back to the + instance's ``top_k``, instead of returning an empty list as requested. Both components now: + + - Raise a ``ValueError`` at initialization if ``top_k`` is not ``None`` and is less than or + equal to ``0``. + - Raise a ``ValueError`` at runtime if ``top_k`` passed to ``run()`` is negative. + - Return an empty list when ``run()`` is called with ``top_k=0``, regardless of the + instance's configured ``top_k``.