Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions haystack/dataclasses/answer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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})
Original file line number Diff line number Diff line change
@@ -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.
48 changes: 48 additions & 0 deletions test/dataclasses/test_answer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# SPDX-License-Identifier: Apache-2.0

import warnings
from copy import deepcopy

import pytest

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
Loading