fix: prevent Answer.from_dict methods from mutating their input dict (#11908)

Co-authored-by: Stefano Fiorucci <stefanofiorucci@gmail.com>

Author: vidigoat

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})

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.

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.