diff --git a/haystack/dataclasses/answer.py b/haystack/dataclasses/answer.py index 56ef08f42e7..02070aba4a4 100644 --- a/haystack/dataclasses/answer.py +++ b/haystack/dataclasses/answer.py @@ -79,16 +79,20 @@ def from_dict(cls, data: dict[str, Any]) -> "ExtractedAnswer": :returns: Deserialized object. """ + # Shallow-copy the init parameters so `from_dict` stays side-effect free: the nested + # replacements below otherwise mutate the caller's dict in place, corrupting it for reuse + # (a second deserialization of the same dict would then receive already-parsed objects). init_params = data.get("init_parameters", {}) + new_params = dict(init_params) if (doc := init_params.get("document")) is not None: - data["init_parameters"]["document"] = Document.from_dict(doc) + new_params["document"] = Document.from_dict(doc) if (offset := init_params.get("document_offset")) is not None: - data["init_parameters"]["document_offset"] = ExtractedAnswer.Span(**offset) + new_params["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) + new_params["context_offset"] = ExtractedAnswer.Span(**offset) + return default_from_dict(cls, {**data, "init_parameters": new_params}) @_warn_on_inplace_mutation @@ -133,14 +137,20 @@ def from_dict(cls, data: dict[str, Any]) -> "GeneratedAnswer": :returns: Deserialized object. """ + # Shallow-copy the init parameters so `from_dict` stays side-effect free: the nested + # replacements below otherwise mutate the caller's dict in place, corrupting it for reuse + # (a second deserialization of the same dict would then receive already-parsed objects). init_params = data.get("init_parameters", {}) + new_params = dict(init_params) if (documents := init_params.get("documents")) is not None: - init_params["documents"] = [Document.from_dict(d) for d in documents] + new_params["documents"] = [Document.from_dict(d) for d in documents] - meta = init_params.get("meta", {}) + # Shallow-copy `meta` before touching `all_messages` so the caller's nested dict is + # left untouched as well. + meta = dict(init_params.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 + new_params["meta"] = meta - return default_from_dict(cls, data) + return default_from_dict(cls, {**data, "init_parameters": new_params}) diff --git a/releasenotes/notes/fix-answer-from-dict-input-mutation-ac713885d47e54d6.yaml b/releasenotes/notes/fix-answer-from-dict-input-mutation-ac713885d47e54d6.yaml new file mode 100644 index 00000000000..901d0f2013b --- /dev/null +++ b/releasenotes/notes/fix-answer-from-dict-input-mutation-ac713885d47e54d6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed ``GeneratedAnswer.from_dict`` and ``ExtractedAnswer.from_dict`` mutating their input + dictionary in place. They replaced nested values (``documents``, ``document``, offsets and + ``meta["all_messages"]``) directly inside the passed dictionary, so deserializing the same + dictionary a second time received already-parsed objects and raised an ``AttributeError``. + Both methods now copy their input before deserializing and are side-effect free. diff --git a/test/dataclasses/test_answer.py b/test/dataclasses/test_answer.py index 68f46486373..90b3fe1d457 100644 --- a/test/dataclasses/test_answer.py +++ b/test/dataclasses/test_answer.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 import warnings +from copy import deepcopy import pytest @@ -101,6 +102,30 @@ def test_from_dict(self): assert answer.context_offset == ExtractedAnswer.Span(14, 16) assert answer.meta == {"meta_key": "meta_value"} + def test_from_dict_does_not_mutate_input(self): + data = { + "type": "haystack.dataclasses.answer.ExtractedAnswer", + "init_parameters": { + "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"}, + }, + } + snapshot = deepcopy(data) + first = ExtractedAnswer.from_dict(data) + # from_dict must not mutate its input dictionary + assert data == snapshot + # deserializing the same dictionary again must still work and be equal + assert ExtractedAnswer.from_dict(data) == first + def test_no_warning_on_init(self): with warnings.catch_warnings(): warnings.simplefilter("error", Warning) @@ -264,6 +289,29 @@ 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_does_not_mutate_input(self): + data = { + "type": "haystack.dataclasses.answer.GeneratedAnswer", + "init_parameters": { + "data": "42", + "query": "What is the answer?", + "documents": [ + {"id": "1", "content": "The answer is 42."}, + {"id": "2", "content": "I believe the answer is 42."}, + ], + "meta": { + "meta_key": "meta_value", + "all_messages": [ChatMessage.from_user("What is the answer?").to_dict()], + }, + }, + } + snapshot = deepcopy(data) + first = GeneratedAnswer.from_dict(data) + # from_dict must not mutate its input dictionary + assert data == snapshot + # deserializing the same dictionary again must still work and be equal + assert GeneratedAnswer.from_dict(data) == first + def test_from_dict_with_empty_all_messages(self): # An empty `all_messages` list must not crash deserialization: `is not None` # let `[]` through and then indexed `all_messages[0]`, raising IndexError.