Skip to content

bug: fix Agent Final Response Discarding text after newline#6574

Merged
ZanSara merged 5 commits into
deepset-ai:v1.23.xfrom
Tsadoq:agent-final-response-discarding-text-after-newline
Dec 18, 2023
Merged

bug: fix Agent Final Response Discarding text after newline#6574
ZanSara merged 5 commits into
deepset-ai:v1.23.xfrom
Tsadoq:agent-final-response-discarding-text-after-newline

Conversation

@Tsadoq
Copy link
Copy Markdown

@Tsadoq Tsadoq commented Dec 16, 2023

Related Issues

Proposed Changes:

The agent would discard the final answer if this was distributed on multiple lines. I added a modifier in the regex so that the . matches also newline characters.

How did you test it?

One possibility is to use the agent to generate code, in this case we should see the bug and then the fix.

Notes for the reviewer

In the 2.0 I'd change this structure to pass a flag or to give a closing sequence as well.

Checklist

@Tsadoq Tsadoq requested a review from a team as a code owner December 16, 2023 08:55
@Tsadoq Tsadoq requested review from ZanSara and removed request for a team December 16, 2023 08:55
@ZanSara
Copy link
Copy Markdown
Contributor

ZanSara commented Dec 18, 2023

Hello @Tsadoq! Would you mind adding a test to demonstrate that your solution works? There are some indications on how to do so in the CONTRIBUTING file, but I can also help if you need.

@Tsadoq
Copy link
Copy Markdown
Author

Tsadoq commented Dec 18, 2023

Hello @Tsadoq! Would you mind adding a test to demonstrate that your solution works? There are some indications on how to do so in the CONTRIBUTING file, but I can also help if you need.

Done that, sorry but I completely forgot. I hope it is fine. I saw that the regex is hardcoded in the test so I added it there as well.

@ZanSara
Copy link
Copy Markdown
Contributor

ZanSara commented Dec 18, 2023

@Tsadoq looks like you haven't run the pre-commit checks, as Black is failing. Could you do so? There are instructions in CONTRIBUTING about it. Also, we need to add a release note. This is also present in the CONTRIBUTING page, but if you need help let me know 🙂

@Tsadoq Tsadoq requested a review from a team as a code owner December 18, 2023 09:59
@Tsadoq Tsadoq requested review from dfokina and removed request for a team December 18, 2023 09:59
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Dec 18, 2023

Pull Request Test Coverage Report for Build 7246441017

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 45.387%

Totals Coverage Status
Change from base Build 7211483630: 0.0%
Covered Lines: 10542
Relevant Lines: 23227

💛 - Coveralls

@Tsadoq
Copy link
Copy Markdown
Author

Tsadoq commented Dec 18, 2023

@Tsadoq looks like you haven't run the pre-commit checks, as Black is failing. Could you do so? There are instructions in CONTRIBUTING about it. Also, we need to add a release note. This is also present in the CONTRIBUTING page, but if you need help let me know 🙂

For some reason the black commit went after. Added release note :) Hopefully next time I'll be more linear in the PR

@ZanSara
Copy link
Copy Markdown
Contributor

ZanSara commented Dec 18, 2023

Alright, looks good now 🙂 Let's wait for CI to pass and if there are no failures I'll approve and merge.

Copy link
Copy Markdown
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! 😊

@ZanSara ZanSara merged commit 8a9df97 into deepset-ai:v1.23.x Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants