Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bug-fix] langchain-Fix bug in OutputFixingParser #24220

Closed
wants to merge 7 commits into from
Closed

[bug-fix] langchain-Fix bug in OutputFixingParser #24220

wants to merge 7 commits into from

Conversation

ZhangShenao
Copy link
Contributor

#24219 Fix bug in OutputFixingParser.
The full error stack have been shown in issue #24219
The param input should be completion

@vercel Vercel
Copy link

vercel bot commented Jul 13, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Jul 15, 2024 11:03pm

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. langchain Related to the langchain package Ɑ: parsing Related to output parser module 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Jul 13, 2024
@ZhangShenao
Copy link
Contributor Author

ZhangShenao commented Jul 13, 2024 •
edited
Loading

@baskaryan @efriis Please review

@ZhangShenao
Copy link
Contributor Author

#24219

@ZhangShenao
Copy link
Contributor Author

@baskaryan @efriis Is there any problems, please?

@eyurtsev eyurtsev self-assigned this Jul 15, 2024
@lynscott
Copy link

I am experiencing this bug as well. Is there a temp work around you have found @ZhangShenao ?

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jul 15, 2024
efriis
efriis approved these changes Jul 15, 2024
View reviewed changes
Copy link
Member

@efriis efriis left a comment

Choose a reason for hiding this comment

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

Would love to get @hmasdev or @eyurtsev eyes to make sure this wasn't intentional on #19792

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jul 15, 2024
@ZhangShenao
Copy link
Contributor Author

@lynscott I encountered this problem in my own project

@ZhangShenao
Copy link
Contributor Author

@efriis OK

@hmasdev
Copy link
Contributor

hmasdev commented Jul 16, 2024 •
edited
Loading

Thanks for fix @ZhangShenao

Would love to get @hmasdev or @eyurtsev eyes to make sure this wasn't intentional on #19792

@efriis
this PR looks correct to me.
'input' keyword was not intentional in #19792.
I already created #23967 with tests in order to fix this bug.

@lynscott
Copy link

@ZhangShenao well yes of course, so have I.
I was asking if you've found a temporary solution to the problem until your fix is merged, but I'll patch your fix myself for now. Thanks

@hmasdev hmasdev mentioned this pull request Jul 16, 2024
Merged
@efriis
Copy link
Member

efriis commented Jul 17, 2024

ah great merging in the one with unit tests!

@efriis efriis closed this Jul 17, 2024
@ZhangShenao ZhangShenao deleted the bugfix_fix_OutputFixingParser branch July 18, 2024 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efriis efriis efriis approved these changes

Assignees

@eyurtsev eyurtsev

Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature langchain Related to the langchain package lgtm PR looks good. Use to confirm that a PR is ready for merging. Ɑ: parsing Related to output parser module size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ZhangShenao @lynscott @hmasdev @efriis @eyurtsev