Skip to content

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

Merged
anakin87 merged 5 commits into
deepset-ai:mainfrom
vidigoat:fix/answer-from-dict-input-mutation
Jul 9, 2026
Merged

fix: prevent Answer.from_dict methods from mutating their input dict#11908
anakin87 merged 5 commits into
deepset-ai:mainfrom
vidigoat:fix/answer-from-dict-input-mutation

Conversation

@vidigoat

@vidigoat vidigoat commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

from_dict should be side-effect free, but both GeneratedAnswer.from_dict and ExtractedAnswer.from_dict wrote parsed Document / ChatMessage / Span objects back into the caller's dictionary. Deserializing the same dict a second time (common in replay/logging flows that keep the serialized form around) then crashed:

from haystack.dataclasses import GeneratedAnswer, ChatMessage, Document
ga = GeneratedAnswer(
    data="answer", query="q",
    documents=[Document(content="d")],
    meta={"all_messages": [ChatMessage.from_user("hi")]},
)
s = ga.to_dict()
GeneratedAnswer.from_dict(s)   # OK, but mutates s in place
GeneratedAnswer.from_dict(s)   # AttributeError: 'Document' object has no attribute 'copy'

Document.from_dict already guards against this with data = data.copy(); this extends the same idea using deepcopy, since the mutated values are nested inside init_parameters / meta.

Test plan

  • Added test_from_dict_does_not_mutate_input to both TestExtractedAnswer and TestGeneratedAnswer, each asserting the input dict is unchanged after from_dict and that a second deserialization of the same dict still succeeds and is equal.
  • The tests fail on main (input mutated / second call raises) and pass with this change.
  • Added a release note.

AI disclosure

This change was prepared with the help of an AI coding assistant; the bug was reproduced and the behavior verified before submitting.

…from mutating their input dict

Both from_dict methods wrote parsed Document/ChatMessage/Span objects back into
the caller's dictionary, so deserializing the same dict a second time received
already-parsed objects and raised AttributeError. Document.from_dict already
guards against this with data.copy(); this extends the same idea with deepcopy,
since the mutated values are nested. Adds regression tests for both classes.
@vidigoat
vidigoat requested a review from a team as a code owner July 8, 2026 07:59
@vidigoat
vidigoat requested review from anakin87 and removed request for a team July 8, 2026 07:59
@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

@vidigoat is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/dataclasses
  answer.py
Project Total  

This report was generated by python-coverage-comment-action

@anakin87 anakin87 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

I found an optimization opportunity.

Comment thread haystack/dataclasses/answer.py Outdated
Comment thread haystack/dataclasses/answer.py Outdated
@github-actions github-actions Bot added the type:documentation Improvements on the docs label Jul 9, 2026
@vidigoat

vidigoat commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, good call — deepcopy was heavier than needed here. Done in both from_dict methods: shallow-copy init_parameters and only rebuild the nested fields (document/offsets for ExtractedAnswer, documents/meta for GeneratedAnswer).

One thing worth flagging: in GeneratedAnswer the meta sub-dict needed its own shallow copy as well — otherwise converting all_messages in place would still mutate the caller's input, which is the case the original test guards against. So I copied that nested level too. The no-mutation tests stay green.

Comment thread releasenotes/notes/fix-answer-from-dict-input-mutation-ac713885d47e54d6.yaml Outdated

@anakin87 anakin87 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@anakin87
anakin87 enabled auto-merge (squash) July 9, 2026 15:30
@anakin87
anakin87 merged commit 3fc8f08 into deepset-ai:main Jul 9, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants