From bb136c86e32694f34467c20fd0f480bc779889fc Mon Sep 17 00:00:00 2001 From: carey-bk <180496115+carey-bk@users.noreply.github.com> Date: Sun, 27 Sep 2026 17:39:35 +0800 Subject: [PATCH 1/2] fix: ignore dimension for custom mock embeddings --- .../embedders/mock_document_embedder.py | 6 +- .../embedders/mock_text_embedder.py | 6 +- ...k-embedder-dimension-f58b01fc4f125811.yaml | 6 ++ .../embedders/test_mock_document_embedder.py | 64 +++++++++++++++---- .../embedders/test_mock_text_embedder.py | 51 +++++++++++---- 5 files changed, 105 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/fix-mock-embedder-dimension-f58b01fc4f125811.yaml diff --git a/haystack/components/embedders/mock_document_embedder.py b/haystack/components/embedders/mock_document_embedder.py index 9832267e1e3..de9c07fac4a 100644 --- a/haystack/components/embedders/mock_document_embedder.py +++ b/haystack/components/embedders/mock_document_embedder.py @@ -81,13 +81,13 @@ def __init__( :param meta_fields_to_embed: List of metadata fields to embed along with the document text. :param embedding_separator: Separator used to concatenate the metadata fields to the document text. :param progress_bar: Accepted for interface compatibility with real Document Embedders and ignored. - :raises ValueError: If both `embedding` and `embedding_fn` are provided, if `dimension` is not positive, or - if `embedding` is an empty list. + :raises ValueError: If both `embedding` and `embedding_fn` are provided, if `embedding` is an empty list, + or if neither is provided and `dimension` is not positive. :raises TypeError: If `embedding` is not a sequence of numbers. """ if embedding is not None and embedding_fn is not None: raise ValueError("Pass either 'embedding' or 'embedding_fn', not both.") - if dimension <= 0: + if embedding is None and embedding_fn is None and dimension <= 0: raise ValueError("'dimension' must be a positive integer.") self.embedding = _coerce_embedding(embedding, name="'embedding'") if embedding is not None else None diff --git a/haystack/components/embedders/mock_text_embedder.py b/haystack/components/embedders/mock_text_embedder.py index 59650cdd976..7592cead155 100644 --- a/haystack/components/embedders/mock_text_embedder.py +++ b/haystack/components/embedders/mock_text_embedder.py @@ -70,13 +70,13 @@ def __init__( :param meta: Additional metadata merged into the output `meta`. :param prefix: A string to add at the beginning of the text before embedding. :param suffix: A string to add at the end of the text before embedding. - :raises ValueError: If both `embedding` and `embedding_fn` are provided, if `dimension` is not positive, or - if `embedding` is an empty list. + :raises ValueError: If both `embedding` and `embedding_fn` are provided, if `embedding` is an empty list, + or if neither is provided and `dimension` is not positive. :raises TypeError: If `embedding` is not a sequence of numbers. """ if embedding is not None and embedding_fn is not None: raise ValueError("Pass either 'embedding' or 'embedding_fn', not both.") - if dimension <= 0: + if embedding is None and embedding_fn is None and dimension <= 0: raise ValueError("'dimension' must be a positive integer.") self.embedding = _coerce_embedding(embedding, name="'embedding'") if embedding is not None else None diff --git a/releasenotes/notes/fix-mock-embedder-dimension-f58b01fc4f125811.yaml b/releasenotes/notes/fix-mock-embedder-dimension-f58b01fc4f125811.yaml new file mode 100644 index 00000000000..9342506586d --- /dev/null +++ b/releasenotes/notes/fix-mock-embedder-dimension-f58b01fc4f125811.yaml @@ -0,0 +1,6 @@ +fixes: + - | + ``MockTextEmbedder`` and ``MockDocumentEmbedder`` now accept non-positive + ``dimension`` values when ``embedding`` or ``embedding_fn`` is provided, + matching the documented behavior. The default deterministic embedding mode + still requires a positive ``dimension``. diff --git a/test/components/embedders/test_mock_document_embedder.py b/test/components/embedders/test_mock_document_embedder.py index 26ebc19787a..ab518facd26 100644 --- a/test/components/embedders/test_mock_document_embedder.py +++ b/test/components/embedders/test_mock_document_embedder.py @@ -2,6 +2,8 @@ # # SPDX-License-Identifier: Apache-2.0 +from unittest.mock import Mock, call + import pytest from haystack import Document, Pipeline @@ -14,16 +16,23 @@ def _ones(text: str) -> list[float]: class TestMockDocumentEmbedder: + @pytest.mark.parametrize("dimension", [768, 0, -1]) @pytest.mark.parametrize( - ("args", "kwargs", "match"), + ("args", "kwargs", "exception", "match"), [ - (([0.1],), {"embedding_fn": _ones}, "either 'embedding' or 'embedding_fn'"), - ((), {"dimension": -1}, "must be a positive integer"), + (([0.1],), {"embedding_fn": _ones}, ValueError, "either 'embedding' or 'embedding_fn'"), + (([],), {}, ValueError, "must not be empty"), + ((["not", "numbers"],), {}, TypeError, "must be a sequence of numbers"), ], ) - def test_init_rejects_invalid_config(self, args, kwargs, match): - with pytest.raises(ValueError, match=match): - MockDocumentEmbedder(*args, **kwargs) + def test_init_rejects_invalid_config(self, args, kwargs, exception, match, dimension): + with pytest.raises(exception, match=match): + MockDocumentEmbedder(*args, dimension=dimension, **kwargs) + + @pytest.mark.parametrize("dimension", [0, -1]) + def test_deterministic_embedding_rejects_non_positive_dimension(self, dimension): + with pytest.raises(ValueError, match=r"^'dimension' must be a positive integer\.$"): + MockDocumentEmbedder(dimension=dimension) def test_embeds_documents(self): embedder = MockDocumentEmbedder(dimension=16) @@ -38,13 +47,33 @@ def test_consistent_with_text_embedder(self): doc_embedding = MockDocumentEmbedder(dimension=8).run([Document(content="pizza")])["documents"][0].embedding assert text_embedding == doc_embedding - def test_fixed_embedding(self): - result = MockDocumentEmbedder([0.5, 0.5]).run([Document(content="a"), Document(content="b")]) + @pytest.mark.parametrize("dimension", [768, 0, -1]) + def test_fixed_embedding_ignores_dimension(self, dimension): + result = MockDocumentEmbedder([0.5, 0.5], dimension=dimension).run( + [Document(content="a"), Document(content="b")] + ) assert all(doc.embedding == [0.5, 0.5] for doc in result["documents"]) - def test_embedding_fn(self): - result = MockDocumentEmbedder(embedding_fn=_ones).run([Document(content="a")]) - assert result["documents"][0].embedding == [1.0, 1.0, 1.0] + @pytest.mark.parametrize("dimension", [768, 0, -1]) + def test_embedding_fn_ignores_dimension(self, dimension): + embedding_fn = Mock(wraps=_ones) + result = MockDocumentEmbedder(embedding_fn=embedding_fn, dimension=dimension).run( + [Document(content="a"), Document(content="b")] + ) + assert [doc.embedding for doc in result["documents"]] == [[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]] + assert embedding_fn.call_args_list == [call("a"), call("b")] + + @pytest.mark.parametrize("dimension", [768, 0, -1]) + @pytest.mark.parametrize( + ("embedding", "exception", "match"), + [([], ValueError, "must not be empty"), ("not a vector", TypeError, "must be a sequence of numbers")], + ) + def test_embedding_fn_invalid_return_raises(self, dimension, embedding, exception, match): + embedding_fn = Mock(return_value=embedding) + embedder = MockDocumentEmbedder(embedding_fn=embedding_fn, dimension=dimension) + with pytest.raises(exception, match=match): + embedder.run([Document(content="hello")]) + embedding_fn.assert_called_once_with("hello") def test_meta_fields_to_embed_affect_embedding(self): document = Document(content="hello", meta={"title": "Greetings"}) @@ -111,6 +140,19 @@ def test_serialization_roundtrip(self, embedder): document = Document(content="hello", meta={"title": "t"}) assert restored.run([document])["documents"][0].embedding == embedder.run([document])["documents"][0].embedding + @pytest.mark.parametrize("dimension", [0, -1]) + @pytest.mark.parametrize( + "kwargs", [{"embedding": [0.1, 0.2]}, {"embedding_fn": _ones}], ids=["fixed", "embedding_fn"] + ) + def test_custom_embedding_serialization_roundtrip(self, dimension, kwargs): + embedder = MockDocumentEmbedder(dimension=dimension, **kwargs) + restored = MockDocumentEmbedder.from_dict(embedder.to_dict()) + assert restored.dimension == dimension + assert restored.embedding == embedder.embedding + assert restored.embedding_fn is embedder.embedding_fn + document = Document(content="hello") + assert restored.run([document])["documents"][0].embedding == embedder.run([document])["documents"][0].embedding + def test_in_pipeline(self): pipeline = Pipeline() pipeline.add_component("embedder", MockDocumentEmbedder(dimension=8)) diff --git a/test/components/embedders/test_mock_text_embedder.py b/test/components/embedders/test_mock_text_embedder.py index 2649edffa9d..be3b2e296f9 100644 --- a/test/components/embedders/test_mock_text_embedder.py +++ b/test/components/embedders/test_mock_text_embedder.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 import math +from unittest.mock import Mock import pytest @@ -22,18 +23,23 @@ def test_l2_normalize_handles_zero_vector(): class TestMockTextEmbedder: + @pytest.mark.parametrize("dimension", [768, 0, -1]) @pytest.mark.parametrize( ("args", "kwargs", "exception", "match"), [ (([0.1, 0.2],), {"embedding_fn": _ones}, ValueError, "either 'embedding' or 'embedding_fn'"), - ((), {"dimension": 0}, ValueError, "must be a positive integer"), (([],), {}, ValueError, "must not be empty"), ((["not", "numbers"],), {}, TypeError, "must be a sequence of numbers"), ], ) - def test_init_rejects_invalid_config(self, args, kwargs, exception, match): + def test_init_rejects_invalid_config(self, args, kwargs, exception, match, dimension): with pytest.raises(exception, match=match): - MockTextEmbedder(*args, **kwargs) + MockTextEmbedder(*args, dimension=dimension, **kwargs) + + @pytest.mark.parametrize("dimension", [0, -1]) + def test_deterministic_embedding_rejects_non_positive_dimension(self, dimension): + with pytest.raises(ValueError, match=r"^'dimension' must be a positive integer\.$"): + MockTextEmbedder(dimension=dimension) def test_deterministic_embedding(self): embedding = MockTextEmbedder(dimension=16).run("hello")["embedding"] @@ -51,19 +57,30 @@ def test_deterministic_distinguishes_texts(self): MockTextEmbedder(dimension=8).run("x")["embedding"] == MockTextEmbedder(dimension=8).run("x")["embedding"] ) - def test_fixed_embedding(self): - embedder = MockTextEmbedder([0.1, 0.2, 0.3]) + @pytest.mark.parametrize("dimension", [768, 0, -1]) + def test_fixed_embedding_ignores_dimension(self, dimension): + embedder = MockTextEmbedder([0.1, 0.2, 0.3], dimension=dimension) assert embedder.run("anything")["embedding"] == [0.1, 0.2, 0.3] assert embedder.run("something else")["embedding"] == [0.1, 0.2, 0.3] - def test_embedding_fn(self): - assert MockTextEmbedder(embedding_fn=_ones).run("hello")["embedding"] == [1.0, 1.0, 1.0] + @pytest.mark.parametrize("dimension", [768, 0, -1]) + def test_embedding_fn_ignores_dimension(self, dimension): + embedding_fn = Mock(wraps=_ones) + embedder = MockTextEmbedder(embedding_fn=embedding_fn, dimension=dimension) + assert embedder.run("hello")["embedding"] == [1.0, 1.0, 1.0] + embedding_fn.assert_called_once_with("hello") - def test_embedding_fn_invalid_return_raises(self): - # embedding_fn deliberately returns a non-vector to exercise the runtime type check - embedder = MockTextEmbedder(embedding_fn=lambda text: "not a vector") # type: ignore[arg-type, return-value] - with pytest.raises(TypeError, match="must be a sequence of numbers"): + @pytest.mark.parametrize("dimension", [768, 0, -1]) + @pytest.mark.parametrize( + ("embedding", "exception", "match"), + [([], ValueError, "must not be empty"), ("not a vector", TypeError, "must be a sequence of numbers")], + ) + def test_embedding_fn_invalid_return_raises(self, dimension, embedding, exception, match): + embedding_fn = Mock(return_value=embedding) + embedder = MockTextEmbedder(embedding_fn=embedding_fn, dimension=dimension) + with pytest.raises(exception, match=match): embedder.run("hello") + embedding_fn.assert_called_once_with("hello") def test_prefix_suffix_affect_embedding(self): plain = MockTextEmbedder(dimension=8).run("hello")["embedding"] @@ -112,6 +129,18 @@ def test_serialization_roundtrip(self, embedder): assert isinstance(restored, MockTextEmbedder) assert restored.run("hello")["embedding"] == embedder.run("hello")["embedding"] + @pytest.mark.parametrize("dimension", [0, -1]) + @pytest.mark.parametrize( + "kwargs", [{"embedding": [0.1, 0.2]}, {"embedding_fn": _ones}], ids=["fixed", "embedding_fn"] + ) + def test_custom_embedding_serialization_roundtrip(self, dimension, kwargs): + embedder = MockTextEmbedder(dimension=dimension, **kwargs) + restored = MockTextEmbedder.from_dict(embedder.to_dict()) + assert restored.dimension == dimension + assert restored.embedding == embedder.embedding + assert restored.embedding_fn is embedder.embedding_fn + assert restored.run("hello")["embedding"] == embedder.run("hello")["embedding"] + def test_in_pipeline(self): pipeline = Pipeline() pipeline.add_component("embedder", MockTextEmbedder(dimension=8)) From eb0adee9ddccf438c46aac32486d68fa4a86b916 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Mon, 28 Sep 2026 11:13:58 +0200 Subject: [PATCH 2/2] simplify tests --- .../embedders/test_mock_document_embedder.py | 65 ++++--------------- .../embedders/test_mock_text_embedder.py | 47 ++++---------- 2 files changed, 25 insertions(+), 87 deletions(-) diff --git a/test/components/embedders/test_mock_document_embedder.py b/test/components/embedders/test_mock_document_embedder.py index ab518facd26..21a6f2b6fef 100644 --- a/test/components/embedders/test_mock_document_embedder.py +++ b/test/components/embedders/test_mock_document_embedder.py @@ -2,8 +2,6 @@ # # SPDX-License-Identifier: Apache-2.0 -from unittest.mock import Mock, call - import pytest from haystack import Document, Pipeline @@ -16,23 +14,17 @@ def _ones(text: str) -> list[float]: class TestMockDocumentEmbedder: - @pytest.mark.parametrize("dimension", [768, 0, -1]) @pytest.mark.parametrize( - ("args", "kwargs", "exception", "match"), + ("args", "kwargs", "match"), [ - (([0.1],), {"embedding_fn": _ones}, ValueError, "either 'embedding' or 'embedding_fn'"), - (([],), {}, ValueError, "must not be empty"), - ((["not", "numbers"],), {}, TypeError, "must be a sequence of numbers"), + (([0.1],), {"embedding_fn": _ones}, "either 'embedding' or 'embedding_fn'"), + ((), {"dimension": 0}, "must be a positive integer"), + ((), {"dimension": -1}, "must be a positive integer"), ], ) - def test_init_rejects_invalid_config(self, args, kwargs, exception, match, dimension): - with pytest.raises(exception, match=match): - MockDocumentEmbedder(*args, dimension=dimension, **kwargs) - - @pytest.mark.parametrize("dimension", [0, -1]) - def test_deterministic_embedding_rejects_non_positive_dimension(self, dimension): - with pytest.raises(ValueError, match=r"^'dimension' must be a positive integer\.$"): - MockDocumentEmbedder(dimension=dimension) + def test_init_rejects_invalid_config(self, args, kwargs, match): + with pytest.raises(ValueError, match=match): + MockDocumentEmbedder(*args, **kwargs) def test_embeds_documents(self): embedder = MockDocumentEmbedder(dimension=16) @@ -48,32 +40,16 @@ def test_consistent_with_text_embedder(self): assert text_embedding == doc_embedding @pytest.mark.parametrize("dimension", [768, 0, -1]) - def test_fixed_embedding_ignores_dimension(self, dimension): - result = MockDocumentEmbedder([0.5, 0.5], dimension=dimension).run( - [Document(content="a"), Document(content="b")] - ) + def test_fixed_embedding(self, dimension): + embedder = MockDocumentEmbedder([0.5, 0.5], dimension=dimension) + result = embedder.run([Document(content="a"), Document(content="b")]) assert all(doc.embedding == [0.5, 0.5] for doc in result["documents"]) @pytest.mark.parametrize("dimension", [768, 0, -1]) - def test_embedding_fn_ignores_dimension(self, dimension): - embedding_fn = Mock(wraps=_ones) - result = MockDocumentEmbedder(embedding_fn=embedding_fn, dimension=dimension).run( - [Document(content="a"), Document(content="b")] - ) - assert [doc.embedding for doc in result["documents"]] == [[1.0, 1.0, 1.0], [1.0, 1.0, 1.0]] - assert embedding_fn.call_args_list == [call("a"), call("b")] - - @pytest.mark.parametrize("dimension", [768, 0, -1]) - @pytest.mark.parametrize( - ("embedding", "exception", "match"), - [([], ValueError, "must not be empty"), ("not a vector", TypeError, "must be a sequence of numbers")], - ) - def test_embedding_fn_invalid_return_raises(self, dimension, embedding, exception, match): - embedding_fn = Mock(return_value=embedding) - embedder = MockDocumentEmbedder(embedding_fn=embedding_fn, dimension=dimension) - with pytest.raises(exception, match=match): - embedder.run([Document(content="hello")]) - embedding_fn.assert_called_once_with("hello") + def test_embedding_fn(self, dimension): + embedder = MockDocumentEmbedder(embedding_fn=_ones, dimension=dimension) + result = embedder.run([Document(content="a")]) + assert result["documents"][0].embedding == [1.0, 1.0, 1.0] def test_meta_fields_to_embed_affect_embedding(self): document = Document(content="hello", meta={"title": "Greetings"}) @@ -140,19 +116,6 @@ def test_serialization_roundtrip(self, embedder): document = Document(content="hello", meta={"title": "t"}) assert restored.run([document])["documents"][0].embedding == embedder.run([document])["documents"][0].embedding - @pytest.mark.parametrize("dimension", [0, -1]) - @pytest.mark.parametrize( - "kwargs", [{"embedding": [0.1, 0.2]}, {"embedding_fn": _ones}], ids=["fixed", "embedding_fn"] - ) - def test_custom_embedding_serialization_roundtrip(self, dimension, kwargs): - embedder = MockDocumentEmbedder(dimension=dimension, **kwargs) - restored = MockDocumentEmbedder.from_dict(embedder.to_dict()) - assert restored.dimension == dimension - assert restored.embedding == embedder.embedding - assert restored.embedding_fn is embedder.embedding_fn - document = Document(content="hello") - assert restored.run([document])["documents"][0].embedding == embedder.run([document])["documents"][0].embedding - def test_in_pipeline(self): pipeline = Pipeline() pipeline.add_component("embedder", MockDocumentEmbedder(dimension=8)) diff --git a/test/components/embedders/test_mock_text_embedder.py b/test/components/embedders/test_mock_text_embedder.py index be3b2e296f9..78883ef02e6 100644 --- a/test/components/embedders/test_mock_text_embedder.py +++ b/test/components/embedders/test_mock_text_embedder.py @@ -3,7 +3,6 @@ # SPDX-License-Identifier: Apache-2.0 import math -from unittest.mock import Mock import pytest @@ -23,23 +22,19 @@ def test_l2_normalize_handles_zero_vector(): class TestMockTextEmbedder: - @pytest.mark.parametrize("dimension", [768, 0, -1]) @pytest.mark.parametrize( ("args", "kwargs", "exception", "match"), [ (([0.1, 0.2],), {"embedding_fn": _ones}, ValueError, "either 'embedding' or 'embedding_fn'"), + ((), {"dimension": 0}, ValueError, "must be a positive integer"), + ((), {"dimension": -1}, ValueError, "must be a positive integer"), (([],), {}, ValueError, "must not be empty"), ((["not", "numbers"],), {}, TypeError, "must be a sequence of numbers"), ], ) - def test_init_rejects_invalid_config(self, args, kwargs, exception, match, dimension): + def test_init_rejects_invalid_config(self, args, kwargs, exception, match): with pytest.raises(exception, match=match): - MockTextEmbedder(*args, dimension=dimension, **kwargs) - - @pytest.mark.parametrize("dimension", [0, -1]) - def test_deterministic_embedding_rejects_non_positive_dimension(self, dimension): - with pytest.raises(ValueError, match=r"^'dimension' must be a positive integer\.$"): - MockTextEmbedder(dimension=dimension) + MockTextEmbedder(*args, **kwargs) def test_deterministic_embedding(self): embedding = MockTextEmbedder(dimension=16).run("hello")["embedding"] @@ -58,29 +53,21 @@ def test_deterministic_distinguishes_texts(self): ) @pytest.mark.parametrize("dimension", [768, 0, -1]) - def test_fixed_embedding_ignores_dimension(self, dimension): + def test_fixed_embedding(self, dimension): embedder = MockTextEmbedder([0.1, 0.2, 0.3], dimension=dimension) assert embedder.run("anything")["embedding"] == [0.1, 0.2, 0.3] assert embedder.run("something else")["embedding"] == [0.1, 0.2, 0.3] @pytest.mark.parametrize("dimension", [768, 0, -1]) - def test_embedding_fn_ignores_dimension(self, dimension): - embedding_fn = Mock(wraps=_ones) - embedder = MockTextEmbedder(embedding_fn=embedding_fn, dimension=dimension) + def test_embedding_fn(self, dimension): + embedder = MockTextEmbedder(embedding_fn=_ones, dimension=dimension) assert embedder.run("hello")["embedding"] == [1.0, 1.0, 1.0] - embedding_fn.assert_called_once_with("hello") - @pytest.mark.parametrize("dimension", [768, 0, -1]) - @pytest.mark.parametrize( - ("embedding", "exception", "match"), - [([], ValueError, "must not be empty"), ("not a vector", TypeError, "must be a sequence of numbers")], - ) - def test_embedding_fn_invalid_return_raises(self, dimension, embedding, exception, match): - embedding_fn = Mock(return_value=embedding) - embedder = MockTextEmbedder(embedding_fn=embedding_fn, dimension=dimension) - with pytest.raises(exception, match=match): + def test_embedding_fn_invalid_return_raises(self): + # embedding_fn deliberately returns a non-vector to exercise the runtime type check + embedder = MockTextEmbedder(embedding_fn=lambda text: "not a vector") # type: ignore[arg-type, return-value] + with pytest.raises(TypeError, match="must be a sequence of numbers"): embedder.run("hello") - embedding_fn.assert_called_once_with("hello") def test_prefix_suffix_affect_embedding(self): plain = MockTextEmbedder(dimension=8).run("hello")["embedding"] @@ -129,18 +116,6 @@ def test_serialization_roundtrip(self, embedder): assert isinstance(restored, MockTextEmbedder) assert restored.run("hello")["embedding"] == embedder.run("hello")["embedding"] - @pytest.mark.parametrize("dimension", [0, -1]) - @pytest.mark.parametrize( - "kwargs", [{"embedding": [0.1, 0.2]}, {"embedding_fn": _ones}], ids=["fixed", "embedding_fn"] - ) - def test_custom_embedding_serialization_roundtrip(self, dimension, kwargs): - embedder = MockTextEmbedder(dimension=dimension, **kwargs) - restored = MockTextEmbedder.from_dict(embedder.to_dict()) - assert restored.dimension == dimension - assert restored.embedding == embedder.embedding - assert restored.embedding_fn is embedder.embedding_fn - assert restored.run("hello")["embedding"] == embedder.run("hello")["embedding"] - def test_in_pipeline(self): pipeline = Pipeline() pipeline.add_component("embedder", MockTextEmbedder(dimension=8))