From ca035bfec9fe7d5d88577c341ab6961b42353e9b Mon Sep 17 00:00:00 2001 From: anakin87 Date: Wed, 15 Jul 2026 13:33:54 +0200 Subject: [PATCH 1/2] fix: improve Document compatibility/validation layer --- .../components/rankers/lost_in_the_middle.py | 2 +- haystack/dataclasses/document.py | 54 ++++------ ...-fields-in-post-init-6b2e9f4a1c8d375e.yaml | 11 ++ .../rankers/test_lost_in_the_middle.py | 7 ++ test/dataclasses/test_document.py | 102 ++++++------------ 5 files changed, 76 insertions(+), 100 deletions(-) create mode 100644 releasenotes/notes/document-validate-fields-in-post-init-6b2e9f4a1c8d375e.yaml diff --git a/haystack/components/rankers/lost_in_the_middle.py b/haystack/components/rankers/lost_in_the_middle.py index 930643cc1b6..fe4087da4c9 100644 --- a/haystack/components/rankers/lost_in_the_middle.py +++ b/haystack/components/rankers/lost_in_the_middle.py @@ -100,7 +100,7 @@ def run( return {"documents": documents_to_reorder} # Raise an error if any document is not textual - if any(not doc.content_type == "text" for doc in documents_to_reorder): + if any(doc.content is None for doc in documents_to_reorder): raise ValueError("Some provided documents are not textual; LostInTheMiddleRanker can process only text.") # Initialize word count and indices for the "lost in the middle" order diff --git a/haystack/dataclasses/document.py b/haystack/dataclasses/document.py index 7792eaee610..62fda47a8fa 100644 --- a/haystack/dataclasses/document.py +++ b/haystack/dataclasses/document.py @@ -13,32 +13,15 @@ from haystack.dataclasses.sparse_embedding import SparseEmbedding from haystack.utils.dataclasses import _warn_on_inplace_mutation -LEGACY_FIELDS = ["content_type", "id_hash_keys", "dataframe"] +_LEGACY_FIELDS = ["content_type", "id_hash_keys", "dataframe"] -class _BackwardCompatible(type): - """ - Metaclass that handles Document backward compatibility. - """ - +class _RemoveLegacyFields(type): def __call__(cls, *args: Any, **kwargs: Any) -> Any: """ - Called before Document.__init__, handles legacy fields. - - Embedding was stored as NumPy arrays in 1.x, so we convert it to a list of floats. - Other legacy fields are removed. + Called before Document.__init__, removes the legacy fields. """ - ### Conversion from 1.x Document ### - content = kwargs.get("content") - if content and not isinstance(content, str): - raise ValueError("The `content` field must be a string or None.") - - # Embedding were stored as NumPy arrays in 1.x, so we convert it to the new type - if isinstance(embedding := kwargs.get("embedding"), ndarray): - kwargs["embedding"] = embedding.tolist() - - # Remove legacy fields - for field_name in LEGACY_FIELDS: + for field_name in _LEGACY_FIELDS: kwargs.pop(field_name, None) return super().__call__(*args, **kwargs) @@ -46,7 +29,7 @@ def __call__(cls, *args: Any, **kwargs: Any) -> Any: @_warn_on_inplace_mutation @dataclass -class Document(metaclass=_BackwardCompatible): # noqa: PLW1641 +class Document(metaclass=_RemoveLegacyFields): # noqa: PLW1641 """ Base data class containing some data to be queried. @@ -70,6 +53,20 @@ class Document(metaclass=_BackwardCompatible): # noqa: PLW1641 embedding: list[float] | None = field(default=None) sparse_embedding: SparseEmbedding | None = field(default=None) + def __post_init__(self) -> None: + """ + Checks content type, converts embedding from 1.x type and generates the ID based on the init parameters. + """ + if self.content is not None and not isinstance(self.content, str): + raise ValueError("The `content` field must be a string or None.") + + # Embeddings were stored as NumPy arrays in 1.x, so we convert them to the new type + if isinstance(self.embedding, ndarray): + self.embedding = self.embedding.tolist() + + # Generate an id only if not explicitly set + self.id = self.id or self._create_id() + def __repr__(self) -> str: fields = [] if self.content is not None: @@ -99,13 +96,6 @@ def __eq__(self, other: object) -> bool: return False return self.to_dict() == other.to_dict() - def __post_init__(self) -> None: - """ - Generate the ID based on the init parameters. - """ - # Generate an id only if not explicitly set - self.id = self.id or self._create_id() - def _create_id(self) -> str: """ Creates a hash of the given content that acts as the document's ID. @@ -162,10 +152,10 @@ def from_dict(cls, data: dict[str, Any]) -> "Document": # ValueError later if this is the case. meta = data.pop("meta", {}) # Unflatten metadata if it was flattened. We assume any keyword argument that's not - # a document field is a metadata key. We treat legacy fields as document fields - # for backward compatibility. + # a document field is a metadata key. We treat legacy fields as document fields, so that + # `_RemoveLegacyFields` drops them instead of them ending up in `meta`. flatten_meta = {} - document_fields = LEGACY_FIELDS + [f.name for f in fields(cls)] + document_fields = _LEGACY_FIELDS + [f.name for f in fields(cls)] for key in list(data.keys()): if key not in document_fields: flatten_meta[key] = data.pop(key) diff --git a/releasenotes/notes/document-validate-fields-in-post-init-6b2e9f4a1c8d375e.yaml b/releasenotes/notes/document-validate-fields-in-post-init-6b2e9f4a1c8d375e.yaml new file mode 100644 index 00000000000..eea6d7a7af2 --- /dev/null +++ b/releasenotes/notes/document-validate-fields-in-post-init-6b2e9f4a1c8d375e.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Improved the validation and compatibility layer of ``Document``: + - since Haystack 2.x, ``embedding`` is a list of floats, while it was a NumPy array in 1.x, so a NumPy + ``embedding`` is converted to a list for backward compatibility. The conversion only happened when ``embedding`` + was passed as a keyword argument: passed positionally, the array was silently left as is, potentially causing + downstream issues. + - ``content`` type was validated only when passed as a keyword argument. + - ``content`` type validation was also skipped when the value was falsy, such as ``0`` or ``[]``, and failed later + with an unrelated error. diff --git a/test/components/rankers/test_lost_in_the_middle.py b/test/components/rankers/test_lost_in_the_middle.py index 69618f6bde2..9ba045d42e0 100644 --- a/test/components/rankers/test_lost_in_the_middle.py +++ b/test/components/rankers/test_lost_in_the_middle.py @@ -6,6 +6,7 @@ from haystack import Document from haystack.components.rankers.lost_in_the_middle import LostInTheMiddleRanker +from haystack.dataclasses.byte_stream import ByteStream class TestLostInTheMiddleRanker: @@ -35,6 +36,12 @@ def test_lost_in_the_middle_order_two_docs(self): assert result["documents"][0].content == "1" assert result["documents"][1].content == "2" + def test_lost_in_the_middle_with_non_textual_documents(self): + ranker = LostInTheMiddleRanker() + docs = [Document(content="1"), Document(blob=ByteStream(b"some bytes"))] + with pytest.raises(ValueError, match="Some provided documents are not textual"): + ranker.run(documents=docs) + def test_lost_in_the_middle_init(self): # tests that LostInTheMiddleRanker initializes with default values ranker = LostInTheMiddleRanker() diff --git a/test/dataclasses/test_document.py b/test/dataclasses/test_document.py index 18e6566328c..b8e921b4611 100644 --- a/test/dataclasses/test_document.py +++ b/test/dataclasses/test_document.py @@ -6,6 +6,7 @@ from copy import deepcopy from dataclasses import replace +import numpy import pytest from haystack import Document @@ -63,47 +64,47 @@ def test_init_with_parameters(): assert doc.sparse_embedding == sparse_embedding -def test_init_with_legacy_fields(): - doc = Document( - content="test text", - content_type="text", - id_hash_keys=["content"], - dataframe="placeholder", - score=0.812, - embedding=[0.1, 0.2, 0.3], # type: ignore - ) - assert doc.id == "18fc2c114825872321cf5009827ca162f54d3be50ab9e9ffa027824b6ec223af" - assert doc.content == "test text" - assert doc.blob is None - assert doc.meta == {} - assert doc.score == 0.812 - assert doc.embedding == [0.1, 0.2, 0.3] - assert doc.sparse_embedding is None +@pytest.mark.parametrize( + "legacy_fields,meta,expected_id", + [ + ( + {"content_type": "text", "id_hash_keys": ["content"], "dataframe": "placeholder"}, + {}, + "18fc2c114825872321cf5009827ca162f54d3be50ab9e9ffa027824b6ec223af", + ), + ( + {"content_type": "text", "id_hash_keys": ["content"]}, + {"date": "10-10-2023", "type": "article"}, + "dcd4914f727544e89ce8082f6f2e298d244dd0803a4dc167f19d24e7d43b28ac", + ), + ], +) +def test_init_with_legacy_fields(legacy_fields, meta, expected_id): + doc = Document(content="test text", score=0.812, embedding=[0.1, 0.2, 0.3], meta=meta, **legacy_fields) + # legacy fields are dropped, so the Document is the same as if they were not passed at all + assert doc == Document(content="test text", score=0.812, embedding=[0.1, 0.2, 0.3], meta=meta) + assert doc.id == expected_id assert doc.content_type == "text" # this is a property now - assert not hasattr(doc, "id_hash_keys") assert not hasattr(doc, "dataframe") -def test_init_with_legacy_field(): - doc = Document( - content="test text", - content_type="text", # type: ignore - id_hash_keys=["content"], - score=0.812, - embedding=[0.1, 0.2, 0.3], - meta={"date": "10-10-2023", "type": "article"}, - ) - assert doc.id == "dcd4914f727544e89ce8082f6f2e298d244dd0803a4dc167f19d24e7d43b28ac" - assert doc.content == "test text" - assert doc.meta == {"date": "10-10-2023", "type": "article"} - assert doc.score == 0.812 - assert doc.embedding == [0.1, 0.2, 0.3] - assert doc.sparse_embedding is None +@pytest.mark.parametrize("content", [123, 0, []]) +def test_init_with_non_string_content(content): + with pytest.raises(ValueError, match="must be a string or None"): + Document(content=content) - assert doc.content_type == "text" # this is a property now - assert not hasattr(doc, "id_hash_keys") + with pytest.raises(ValueError, match="must be a string or None"): + Document("", content) + + +def test_init_with_numpy_embedding(): + expected = Document(content="test text", embedding=[0.1, 0.2, 0.3]) + + assert Document(content="test text", embedding=numpy.array([0.1, 0.2, 0.3])) == expected # type: ignore[arg-type] + + assert Document("", "test text", None, {}, None, numpy.array([0.1, 0.2, 0.3])) == expected # type: ignore[arg-type] def test_basic_equality_type_mismatch(): @@ -360,39 +361,6 @@ def test_from_dict_with_flat_and_non_flat_meta(): ) -def test_from_dict_with_dataframe(): - """ - Test for legacy support of Document.from_dict() with dataframe field. - - Test that Document.from_dict() can properly deserialize a Document dictionary obtained with - document.to_dict(flatten=False) in haystack-ai<=2.10.0. - We make sure that Document.from_dict() does not raise an error and that dataframe is skipped (legacy field). - """ - - # Document dictionary obtained with document.to_dict(flatten=False) in haystack-ai<=2.10.0 - doc_dict = { - "id": "my_id", - "content": "my_content", - "dataframe": None, - "blob": None, - "meta": {"key": "value"}, - "score": None, - "embedding": None, - "sparse_embedding": None, - } - - doc = Document.from_dict(doc_dict) - - assert doc.id == "my_id" - assert doc.content == "my_content" - assert doc.meta == {"key": "value"} - assert doc.score is None - assert doc.embedding is None - assert doc.sparse_embedding is None - - assert not hasattr(doc, "dataframe") - - def test_content_type(): assert Document(content="text").content_type == "text" From 84c93cf0b78d97a71590edb39966b04365aed731 Mon Sep 17 00:00:00 2001 From: anakin87 Date: Wed, 15 Jul 2026 13:37:05 +0200 Subject: [PATCH 2/2] re-add test --- test/dataclasses/test_document.py | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/dataclasses/test_document.py b/test/dataclasses/test_document.py index b8e921b4611..85cce38df53 100644 --- a/test/dataclasses/test_document.py +++ b/test/dataclasses/test_document.py @@ -361,6 +361,39 @@ def test_from_dict_with_flat_and_non_flat_meta(): ) +def test_from_dict_with_dataframe(): + """ + Test for legacy support of Document.from_dict() with dataframe field. + + Test that Document.from_dict() can properly deserialize a Document dictionary obtained with + document.to_dict(flatten=False) in haystack-ai<=2.10.0. + We make sure that Document.from_dict() does not raise an error and that dataframe is skipped (legacy field). + """ + + # Document dictionary obtained with document.to_dict(flatten=False) in haystack-ai<=2.10.0 + doc_dict = { + "id": "my_id", + "content": "my_content", + "dataframe": None, + "blob": None, + "meta": {"key": "value"}, + "score": None, + "embedding": None, + "sparse_embedding": None, + } + + doc = Document.from_dict(doc_dict) + + assert doc.id == "my_id" + assert doc.content == "my_content" + assert doc.meta == {"key": "value"} + assert doc.score is None + assert doc.embedding is None + assert doc.sparse_embedding is None + + assert not hasattr(doc, "dataframe") + + def test_content_type(): assert Document(content="text").content_type == "text"