fix: ChatPromptBuilder Fails to JSON Serialize #7849

CarlosFerLo · 2024-06-12T21:27:39Z

Related Issues

fixes ChatPromptBuilder Fails to JSON Serialize due to ChatMessage class #7844

Proposed Changes:

The bug was caused by the 'ChatMessage' class not being serialized correctly, which led to a failure in the serialization of 'ChatPromptTemplate'. To resolve this issue, 'to_dict' and 'from_dict' methods were added to both classes, ensuring proper serialization.

How did you test it?

Added testing for all the new methods and reproduced the scenario presented on #7844 , it works fine now

Notes for the reviewer

Checklist

I have read the contributors guidelines and the code of conduct ✅
I have updated the related issue with new insights and changes ✅
I added unit tests and updated the docstrings ✅
I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test:. ✅
I documented my code ✅
I ran pre-commit hooks and fixed any issue ✅

coveralls · 2024-06-12T21:33:52Z

Pull Request Test Coverage Report for Build 9489988243

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.03%) to 89.808%

Totals
Change from base Build 9480552838:	0.03%
Covered Lines:	6926
Relevant Lines:	7712

💛 - Coveralls

coveralls · 2024-06-13T12:01:43Z

Pull Request Test Coverage Report for Build 9499141713

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
11 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.1%) to 89.678%

Files with Coverage Reduction	New Missed Lines	%
components/converters/docx.py	11	58.33%

Totals
Change from base Build 9480552838:	-0.1%
Covered Lines:	6924
Relevant Lines:	7721

💛 - Coveralls

CarlosFerLo · 2024-06-13T20:48:15Z

I do not know if this is the place for this, but I've found a doc string on 'ChatPromptTemplate' that is misleading:

:param required_variables: An optional list of input variables that must be provided at all times.
            If not provided, an exception will be raised.

I believe that, when reading it, it means that if 'required_variables' argument is not provided the function fails, but instead it means that the run method raises an exception if some of those variables is not provided.

Should I fix this one here or open another PR for it.

coveralls · 2024-06-13T20:48:56Z

Pull Request Test Coverage Report for Build 9506510225

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.04%) to 89.678%

Totals
Change from base Build 9497203825:	0.04%
Covered Lines:	6924
Relevant Lines:	7721

💛 - Coveralls

coveralls · 2024-06-13T20:51:25Z

Pull Request Test Coverage Report for Build 9506509369

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.04%) to 89.678%

Totals
Change from base Build 9497203825:	0.04%
Covered Lines:	6924
Relevant Lines:	7721

💛 - Coveralls

coveralls · 2024-06-13T20:54:24Z

Pull Request Test Coverage Report for Build 9506565575

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.04%) to 89.678%

Totals
Change from base Build 9497203825:	0.04%
Covered Lines:	6924
Relevant Lines:	7721

💛 - Coveralls

coveralls · 2024-06-13T21:08:36Z

Pull Request Test Coverage Report for Build 9506715329

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.04%) to 89.678%

Totals
Change from base Build 9497203825:	0.04%
Covered Lines:	6924
Relevant Lines:	7721

💛 - Coveralls

haystack/dataclasses/chat_message.py

coveralls · 2024-06-15T09:56:33Z

Pull Request Test Coverage Report for Build 9527200328

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
40 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.1%) to 89.77%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%
core/pipeline/base.py	7	92.5%
core/pipeline/pipeline.py	32	68.31%

Totals
Change from base Build 9497203825:	0.1%
Covered Lines:	6950
Relevant Lines:	7742

💛 - Coveralls

coveralls · 2024-06-15T13:51:38Z

Pull Request Test Coverage Report for Build 9528525456

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.03%) to 89.77%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%

Totals
Change from base Build 9518891629:	0.03%
Covered Lines:	6950
Relevant Lines:	7742

💛 - Coveralls

coveralls · 2024-06-18T13:50:47Z

Pull Request Test Coverage Report for Build 9566152365

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.03%) to 89.788%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%

Totals
Change from base Build 9565773860:	0.03%
Covered Lines:	6955
Relevant Lines:	7746

💛 - Coveralls

coveralls · 2024-06-18T13:55:13Z

Pull Request Test Coverage Report for Build 9566276770

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.02%) to 89.786%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%

Totals
Change from base Build 9565773860:	0.02%
Covered Lines:	6953
Relevant Lines:	7744

💛 - Coveralls

haystack/dataclasses/chat_message.py

CarlosFerLo · 2024-06-19T12:58:39Z

Do not know why tests fail 🫠, will work on it later.

davidsbatista · 2024-06-19T13:03:33Z

seems to be just a linting issue

CarlosFerLo · 2024-06-19T13:06:53Z

@davidsbatista how can I solve that? I used the pre-commit hooks and they all passed.

davidsbatista · 2024-06-19T13:16:50Z

I suspect updating the branch might fixe it

davidsbatista · 2024-06-19T15:52:36Z

seems like this file test/dataclasses/test_chat_message.py is not being properly formatted.

CarlosFerLo · 2024-06-19T16:34:58Z

seems like this file test/dataclasses/test_chat_message.py is not being properly formatted.
@davidsbatista I see that, but how do we solve that jajaja

lbux · 2024-06-19T20:11:08Z

seems like this file test/dataclasses/test_chat_message.py is not being properly formatted.
@davidsbatista I see that, but how do we solve that jajaja

You need to make sure you have the same Hatch version as the CI. Ensure it is 1.9.3, Newer versions will produce inconsistent results.

coveralls · 2024-06-19T20:46:24Z

Pull Request Test Coverage Report for Build 9587909214

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.01%) to 90.165%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%

Totals
Change from base Build 9583192592:	0.01%
Covered Lines:	7013
Relevant Lines:	7778

💛 - Coveralls

coveralls · 2024-06-19T20:58:06Z

Pull Request Test Coverage Report for Build 9587996856

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.01%) to 90.165%

Files with Coverage Reduction	New Missed Lines	%
components/builders/chat_prompt_builder.py	1	98.41%

Totals
Change from base Build 9583192592:	0.01%
Covered Lines:	7013
Relevant Lines:	7778

💛 - Coveralls

sherrywang31 · 2024-07-02T14:27:43Z

It seems that this change is not in the v2.2.3? I downloaded the source code and noticed that the ChatMessage as well as ChatPromptBuilder class doesn't have 'to_dict' and 'from_dict' functions. Is it expected...?

sherrywang31 · 2024-07-02T15:20:22Z

Also, I noticed that there's no default value for optional input "name". When using the "from_dict" function in ChatMessage, it would complain "name" is missing in initialization.

implement serialization for chat messages and add tests

124137b

github-actions bot added topic:tests type:documentation Improvements on the docs labels Jun 12, 2024

FrancescoSaverioZuppichini mentioned this pull request Jun 13, 2024

fix: ChatPromptBuilder Fails to JSON Serialize due to ChatMessage class #7851

Closed

6 tasks

CarlosFerLo changed the title ~~ChatPromptBuilder Fails to JSON Serialize due to ChatMessage class~~ fix: ChatPromptBuilder Fails to JSON Serialize Jun 13, 2024

implement serialization for ChatPromptBuilder and test it

025547d

CarlosFerLo and others added 2 commits June 13, 2024 22:42

add reno

08aa8c2

Merge branch 'main' into issue-7844

8738fdf

CarlosFerLo marked this pull request as ready for review June 13, 2024 20:48

CarlosFerLo requested review from a team as code owners June 13, 2024 20:48

CarlosFerLo requested review from dfokina and davidsbatista and removed request for a team June 13, 2024 20:48

solve mypy type error

0814500

shadeMe requested changes Jun 14, 2024

View reviewed changes

haystack/dataclasses/chat_message.py Outdated Show resolved Hide resolved

silvanocerza mentioned this pull request Jun 14, 2024

Verify correct execution order of Components with Variadic input type and greediness #7873

Open

CarlosFerLo requested a review from shadeMe June 15, 2024 09:42

solve mypy type error

3872213

Merge branch 'main' into issue-7844

a8239dc

CarlosFerLo mentioned this pull request Jun 16, 2024

ChatMessage content being str-only doesn't allow user to pass image #7848

Open

CarlosFerLo and others added 2 commits June 18, 2024 15:39

Merge branch 'main' into issue-7844

3298ac1

remove flattening parameter in to_dict

5b021a1

shadeMe requested changes Jun 18, 2024

View reviewed changes

haystack/dataclasses/chat_message.py Outdated Show resolved Hide resolved

haystack/dataclasses/chat_message.py Outdated Show resolved Hide resolved

CarlosFerLo requested a review from shadeMe June 18, 2024 18:13

simplify to jus non-flat metadata

aa29874

shadeMe approved these changes Jun 19, 2024

View reviewed changes

try to fix linting issue

9b102ae

davidsbatista and others added 2 commits June 19, 2024 15:16

Merge branch 'main' into issue-7844

f40a38e

Merge branch 'main' into issue-7844

feecf3a

CarlosFerLo added 2 commits June 19, 2024 22:29

solve format issues

ceec7b5

update test for ChatPromptBuilder

0cb153c

remove unused import

af5304e

CarlosFerLo requested a review from shadeMe June 20, 2024 10:54

shadeMe approved these changes Jun 20, 2024

View reviewed changes

shadeMe merged commit 57c1d47 into deepset-ai:main Jun 20, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ChatPromptBuilder Fails to JSON Serialize #7849

fix: ChatPromptBuilder Fails to JSON Serialize #7849

CarlosFerLo commented Jun 12, 2024 •

edited

Loading

coveralls commented Jun 12, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

CarlosFerLo commented Jun 13, 2024

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 15, 2024 •

edited

Loading

coveralls commented Jun 15, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

CarlosFerLo commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

CarlosFerLo commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

CarlosFerLo commented Jun 19, 2024

lbux commented Jun 19, 2024 •

edited

Loading

coveralls commented Jun 19, 2024 •

edited

Loading

coveralls commented Jun 19, 2024 •

edited

Loading

sherrywang31 commented Jul 2, 2024 •

edited

Loading

sherrywang31 commented Jul 2, 2024

fix: ChatPromptBuilder Fails to JSON Serialize #7849

fix: ChatPromptBuilder Fails to JSON Serialize #7849

Conversation

CarlosFerLo commented Jun 12, 2024 • edited Loading

Related Issues

Proposed Changes:

How did you test it?

Notes for the reviewer

Checklist

coveralls commented Jun 12, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9489988243

Details

💛 - Coveralls

coveralls commented Jun 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9499141713

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

CarlosFerLo commented Jun 13, 2024

coveralls commented Jun 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9506510225

Details

💛 - Coveralls

coveralls commented Jun 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9506509369

Details

💛 - Coveralls

coveralls commented Jun 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9506565575

Details

💛 - Coveralls

coveralls commented Jun 13, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9506715329

Details

💛 - Coveralls

coveralls commented Jun 15, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9527200328

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

coveralls commented Jun 15, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9528525456

Details

💛 - Coveralls

coveralls commented Jun 18, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9566152365

Details

💛 - Coveralls

coveralls commented Jun 18, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9566276770

Details

💛 - Coveralls

CarlosFerLo commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

CarlosFerLo commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

davidsbatista commented Jun 19, 2024

CarlosFerLo commented Jun 19, 2024

lbux commented Jun 19, 2024 • edited Loading

coveralls commented Jun 19, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9587909214

Details

💛 - Coveralls

coveralls commented Jun 19, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9587996856

Details

💛 - Coveralls

sherrywang31 commented Jul 2, 2024 • edited Loading

sherrywang31 commented Jul 2, 2024

CarlosFerLo commented Jun 12, 2024 •

edited

Loading

coveralls commented Jun 12, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 13, 2024 •

edited

Loading

coveralls commented Jun 15, 2024 •

edited

Loading

coveralls commented Jun 15, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

lbux commented Jun 19, 2024 •

edited

Loading

coveralls commented Jun 19, 2024 •

edited

Loading

coveralls commented Jun 19, 2024 •

edited

Loading

sherrywang31 commented Jul 2, 2024 •

edited

Loading