diff --git a/haystack/dataclasses/answer.py b/haystack/dataclasses/answer.py index 56ef08f42e7..37da0d2c402 100644 --- a/haystack/dataclasses/answer.py +++ b/haystack/dataclasses/answer.py @@ -5,7 +5,6 @@ from dataclasses import asdict, dataclass, field from typing import Any, Optional, Protocol, runtime_checkable -from haystack.core.serialization import default_from_dict, default_to_dict from haystack.dataclasses import ChatMessage, Document from haystack.utils.dataclasses import _warn_on_inplace_mutation @@ -54,20 +53,16 @@ def to_dict(self) -> dict[str, Any]: :returns: Serialized dictionary representation of the object. """ - document = self.document.to_dict(flatten=False) if self.document is not None else None - document_offset = asdict(self.document_offset) if self.document_offset is not None else None - context_offset = asdict(self.context_offset) if self.context_offset is not None else None - return default_to_dict( - self, - data=self.data, - query=self.query, - document=document, - context=self.context, - score=self.score, - document_offset=document_offset, - context_offset=context_offset, - meta=self.meta, - ) + return { + "data": self.data, + "query": self.query, + "document": self.document.to_dict(flatten=False) if self.document is not None else None, + "context": self.context, + "score": self.score, + "document_offset": asdict(self.document_offset) if self.document_offset is not None else None, + "context_offset": asdict(self.context_offset) if self.context_offset is not None else None, + "meta": self.meta, + } @classmethod def from_dict(cls, data: dict[str, Any]) -> "ExtractedAnswer": @@ -79,16 +74,32 @@ def from_dict(cls, data: dict[str, Any]) -> "ExtractedAnswer": :returns: Deserialized object. """ - init_params = data.get("init_parameters", {}) - if (doc := init_params.get("document")) is not None: - data["init_parameters"]["document"] = Document.from_dict(doc) - - if (offset := init_params.get("document_offset")) is not None: - data["init_parameters"]["document_offset"] = ExtractedAnswer.Span(**offset) - - if (offset := init_params.get("context_offset")) is not None: - data["init_parameters"]["context_offset"] = ExtractedAnswer.Span(**offset) - return default_from_dict(cls, data) + # backward compat: old format wrapped fields in init_parameters + if "init_parameters" in data: + data = data["init_parameters"] + + document = data.get("document") + if document is not None: + document = Document.from_dict(document) + + document_offset = data.get("document_offset") + if document_offset is not None: + document_offset = ExtractedAnswer.Span(**document_offset) + + context_offset = data.get("context_offset") + if context_offset is not None: + context_offset = ExtractedAnswer.Span(**context_offset) + + return cls( + data=data.get("data"), + query=data["query"], + score=data["score"], + document=document, + context=data.get("context"), + document_offset=document_offset, + context_offset=context_offset, + meta=data.get("meta", {}), + ) @_warn_on_inplace_mutation @@ -110,17 +121,18 @@ def to_dict(self) -> dict[str, Any]: :returns: Serialized dictionary representation of the object. """ - documents = [doc.to_dict(flatten=False) for doc in self.documents] - - # Serialize ChatMessage objects to dicts + # all_messages is either a list of ChatMessage objects or a list of strings meta = self.meta all_messages = meta.get("all_messages") - - # all_messages is either a list of ChatMessage objects or a list of strings if all_messages and isinstance(all_messages[0], ChatMessage): meta = {**meta, "all_messages": [msg.to_dict() for msg in all_messages]} - return default_to_dict(self, data=self.data, query=self.query, documents=documents, meta=meta) + return { + "data": self.data, + "query": self.query, + "documents": [doc.to_dict(flatten=False) for doc in self.documents], + "meta": meta, + } @classmethod def from_dict(cls, data: dict[str, Any]) -> "GeneratedAnswer": @@ -133,14 +145,15 @@ def from_dict(cls, data: dict[str, Any]) -> "GeneratedAnswer": :returns: Deserialized object. """ - init_params = data.get("init_parameters", {}) + # backward compatibility: old format wrapped fields in init_parameters + if "init_parameters" in data: + data = data["init_parameters"] - if (documents := init_params.get("documents")) is not None: - init_params["documents"] = [Document.from_dict(d) for d in documents] + documents = [Document.from_dict(d) for d in data.get("documents", [])] - meta = init_params.get("meta", {}) + # copy to avoid mutating the caller's input dict when converting all_messages + meta = dict(data.get("meta", {})) if (all_messages := meta.get("all_messages")) and isinstance(all_messages[0], dict): meta["all_messages"] = [ChatMessage.from_dict(m) for m in all_messages] - init_params["meta"] = meta - return default_from_dict(cls, data) + return cls(data=data["data"], query=data["query"], documents=documents, meta=meta) diff --git a/releasenotes/notes/fix-dataclasses-deseria-0bb839b5edd3fca6.yaml b/releasenotes/notes/fix-dataclasses-deseria-0bb839b5edd3fca6.yaml new file mode 100644 index 00000000000..c13d586fa5b --- /dev/null +++ b/releasenotes/notes/fix-dataclasses-deseria-0bb839b5edd3fca6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + ``GeneratedAnswer`` and ``ExtractedAnswer`` now serialize to the same flat format as + all other Haystack dataclasses (``Document``, ``ChatMessage``, etc.), removing an + inconsistency that caused redundant type metadata to be embedded in pipeline + snapshots and ``State`` objects. Deserialization is backward compatible: ``from_dict()`` + still accepts the old ``{"type": "...", "init_parameters": {...}}`` format. diff --git a/test/dataclasses/test_answer.py b/test/dataclasses/test_answer.py index 68f46486373..2d4fde3f766 100644 --- a/test/dataclasses/test_answer.py +++ b/test/dataclasses/test_answer.py @@ -57,20 +57,17 @@ def test_to_dict(self): meta={"meta_key": "meta_value"}, ) assert answer.to_dict() == { - "type": "haystack.dataclasses.answer.ExtractedAnswer", - "init_parameters": { - "data": "42", - "query": "What is the answer?", - "document": document.to_dict(flatten=False), - "context": "The answer is 42.", - "score": 1.0, - "document_offset": {"start": 42, "end": 44}, - "context_offset": {"start": 14, "end": 16}, - "meta": {"meta_key": "meta_value"}, - }, + "data": "42", + "query": "What is the answer?", + "document": document.to_dict(flatten=False), + "context": "The answer is 42.", + "score": 1.0, + "document_offset": {"start": 42, "end": 44}, + "context_offset": {"start": 14, "end": 16}, + "meta": {"meta_key": "meta_value"}, } - def test_from_dict(self): + def test_from_dict_legacy(self): answer = ExtractedAnswer.from_dict( { "type": "haystack.dataclasses.answer.ExtractedAnswer", @@ -101,6 +98,34 @@ def test_from_dict(self): assert answer.context_offset == ExtractedAnswer.Span(14, 16) assert answer.meta == {"meta_key": "meta_value"} + def test_from_dict(self): + answer = ExtractedAnswer.from_dict( + { + "data": "42", + "query": "What is the answer?", + "document": { + "id": "8f800a524b139484fc719ecc35f971a080de87618319bc4836b784d69baca57f", + "content": "I thought a lot about this. The answer is 42.", + }, + "context": "The answer is 42.", + "score": 1.0, + "document_offset": {"start": 42, "end": 44}, + "context_offset": {"start": 14, "end": 16}, + "meta": {"meta_key": "meta_value"}, + } + ) + assert answer.data == "42" + assert answer.query == "What is the answer?" + assert answer.document == Document( + id="8f800a524b139484fc719ecc35f971a080de87618319bc4836b784d69baca57f", + content="I thought a lot about this. The answer is 42.", + ) + assert answer.context == "The answer is 42." + assert answer.score == 1.0 + assert answer.document_offset == ExtractedAnswer.Span(42, 44) + assert answer.context_offset == ExtractedAnswer.Span(14, 16) + assert answer.meta == {"meta_key": "meta_value"} + def test_no_warning_on_init(self): with warnings.catch_warnings(): warnings.simplefilter("error", Warning) @@ -158,10 +183,7 @@ def test_protocol(self): def test_to_dict(self): answer = GeneratedAnswer(data="42", query="What is the answer?", documents=[]) - assert answer.to_dict() == { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": {"data": "42", "query": "What is the answer?", "documents": [], "meta": {}}, - } + assert answer.to_dict() == {"data": "42", "query": "What is the answer?", "documents": [], "meta": {}} def test_to_dict_with_meta(self): answer = GeneratedAnswer( @@ -171,13 +193,10 @@ def test_to_dict_with_meta(self): meta={"meta_key": "meta_value", "all_messages": ["What is the answer?"]}, ) assert answer.to_dict() == { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": { - "data": "42", - "query": "What is the answer?", - "documents": [], - "meta": {"meta_key": "meta_value", "all_messages": ["What is the answer?"]}, - }, + "data": "42", + "query": "What is the answer?", + "documents": [], + "meta": {"meta_key": "meta_value", "all_messages": ["What is the answer?"]}, } def test_to_dict_with_chat_message_in_meta(self): @@ -193,19 +212,16 @@ def test_to_dict_with_chat_message_in_meta(self): meta={"meta_key": "meta_value", "all_messages": [ChatMessage.from_user("What is the answer?")]}, ) assert answer.to_dict() == { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": { - "data": "42", - "query": "What is the answer?", - "documents": [d.to_dict(flatten=False) for d in documents], - "meta": { - "meta_key": "meta_value", - "all_messages": [ChatMessage.from_user("What is the answer?").to_dict()], - }, + "data": "42", + "query": "What is the answer?", + "documents": [d.to_dict(flatten=False) for d in documents], + "meta": { + "meta_key": "meta_value", + "all_messages": [ChatMessage.from_user("What is the answer?").to_dict()], }, } - def test_from_dict(self): + def test_from_dict_legacy(self): answer = GeneratedAnswer.from_dict( { "type": "haystack.dataclasses.answer.GeneratedAnswer", @@ -217,7 +233,7 @@ def test_from_dict(self): assert answer.documents == [] assert answer.meta == {} - def test_from_dict_with_meta(self): + def test_from_dict_with_meta_legacy(self): answer = GeneratedAnswer.from_dict( { "type": "haystack.dataclasses.answer.GeneratedAnswer", @@ -235,7 +251,7 @@ def test_from_dict_with_meta(self): assert answer.meta["meta_key"] == "meta_value" assert answer.meta["all_messages"] == ["What is the answer?"] - def test_from_dict_with_chat_message_in_meta(self): + def test_from_dict_with_chat_message_in_meta_legacy(self): answer = GeneratedAnswer.from_dict( { "type": "haystack.dataclasses.answer.GeneratedAnswer", @@ -264,7 +280,7 @@ def test_from_dict_with_chat_message_in_meta(self): assert answer.meta["meta_key"] == "meta_value" assert answer.meta["all_messages"] == [ChatMessage.from_user("What is the answer?")] - def test_from_dict_with_empty_all_messages(self): + def test_from_dict_with_empty_all_messages_legacy(self): # An empty `all_messages` list must not crash deserialization: `is not None` # let `[]` through and then indexed `all_messages[0]`, raising IndexError. answer = GeneratedAnswer.from_dict( @@ -280,6 +296,73 @@ def test_from_dict_with_empty_all_messages(self): ) assert answer.meta["all_messages"] == [] + def test_from_dict(self): + answer = GeneratedAnswer.from_dict({"data": "42", "query": "What is the answer?", "documents": [], "meta": {}}) + assert answer.data == "42" + assert answer.query == "What is the answer?" + assert answer.documents == [] + assert answer.meta == {} + + def test_from_dict_with_meta(self): + answer = GeneratedAnswer.from_dict( + { + "data": "42", + "query": "What is the answer?", + "documents": [], + "meta": {"meta_key": "meta_value", "all_messages": ["What is the answer?"]}, + } + ) + assert answer.data == "42" + assert answer.query == "What is the answer?" + assert answer.documents == [] + assert answer.meta["meta_key"] == "meta_value" + assert answer.meta["all_messages"] == ["What is the answer?"] + + def test_from_dict_with_chat_message_in_meta(self): + answer = GeneratedAnswer.from_dict( + { + "data": "42", + "query": "What is the answer?", + "documents": [ + {"id": "1", "content": "The answer is 42."}, + {"id": "2", "content": "I believe the answer is 42."}, + {"id": "3", "content": "42 is definitely the answer."}, + ], + "meta": { + "meta_key": "meta_value", + "all_messages": [ChatMessage.from_user("What is the answer?").to_dict()], + }, + } + ) + assert answer.data == "42" + assert answer.query == "What is the answer?" + assert answer.documents == [ + Document(id="1", content="The answer is 42."), + Document(id="2", content="I believe the answer is 42."), + Document(id="3", content="42 is definitely the answer."), + ] + assert answer.meta["meta_key"] == "meta_value" + assert answer.meta["all_messages"] == [ChatMessage.from_user("What is the answer?")] + + def test_from_dict_with_empty_all_messages(self): + answer = GeneratedAnswer.from_dict( + {"data": "42", "query": "What is the answer?", "documents": [], "meta": {"all_messages": []}} + ) + assert answer.meta["all_messages"] == [] + + def test_from_dict_does_not_mutate_input(self): + # from_dict must not mutate the caller's input dict while converting + # `all_messages` dicts into ChatMessage objects (regression test). + meta = {"all_messages": [ChatMessage.from_user("What is the answer?").to_dict()]} + serialized = {"data": "42", "query": "What is the answer?", "documents": [], "meta": meta} + answer = GeneratedAnswer.from_dict(serialized) + + # the deserialized answer still holds ChatMessage objects + assert answer.meta["all_messages"] == [ChatMessage.from_user("What is the answer?")] + # but the caller's meta dict is left untouched: it still holds plain dicts + assert isinstance(meta["all_messages"][0], dict) + assert meta["all_messages"] == [ChatMessage.from_user("What is the answer?").to_dict()] + def test_to_dict_from_dict_round_trip_with_empty_all_messages(self): answer = GeneratedAnswer(data="42", query="What is the answer?", documents=[], meta={"all_messages": []}) assert GeneratedAnswer.from_dict(answer.to_dict()) == answer diff --git a/test/utils/test_base_serialization.py b/test/utils/test_base_serialization.py index 8f83c8f3845..e6cbbb99fc8 100644 --- a/test/utils/test_base_serialization.py +++ b/test/utils/test_base_serialization.py @@ -233,44 +233,38 @@ def test_serializing_and_deserializing_empty_structures(value, result): "serialized_data": [ [ { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": { - "data": "Paris", - "query": "What is the capital of France?", - "documents": [ - { - "id": "1", - "content": "Paris is the capital of France", - "blob": None, - "meta": {}, - "score": None, - "embedding": None, - "sparse_embedding": None, - } - ], - "meta": {"page": 1}, - }, + "data": "Paris", + "query": "What is the capital of France?", + "documents": [ + { + "id": "1", + "content": "Paris is the capital of France", + "blob": None, + "meta": {}, + "score": None, + "embedding": None, + "sparse_embedding": None, + } + ], + "meta": {"page": 1}, } ], [ { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": { - "data": "Berlin", - "query": "What is the capital of Germany?", - "documents": [ - { - "id": "2", - "content": "Berlin is the capital of Germany", - "blob": None, - "meta": {}, - "score": None, - "embedding": None, - "sparse_embedding": None, - } - ], - "meta": {"page": 1}, - }, + "data": "Berlin", + "query": "What is the capital of Germany?", + "documents": [ + { + "id": "2", + "content": "Berlin is the capital of Germany", + "blob": None, + "meta": {}, + "score": None, + "embedding": None, + "sparse_embedding": None, + } + ], + "meta": {"page": 1}, } ], ], @@ -364,23 +358,20 @@ def test_serialize_and_deserialize_value_with_schema_with_various_types(): "list_of_dicts": [{"numbers": [1, 2, 3]}], "answers": [ { - "type": "haystack.dataclasses.answer.GeneratedAnswer", - "init_parameters": { - "data": "Paris", - "query": "What is the capital of France?", - "documents": [ - { - "id": "2", - "content": "Paris is the capital of France", - "blob": None, - "meta": {}, - "score": None, - "embedding": None, - "sparse_embedding": None, - } - ], - "meta": {"page": 1}, - }, + "data": "Paris", + "query": "What is the capital of France?", + "documents": [ + { + "id": "2", + "content": "Paris is the capital of France", + "blob": None, + "meta": {}, + "score": None, + "embedding": None, + "sparse_embedding": None, + } + ], + "meta": {"page": 1}, } ], },