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.

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

New Feature - Output Parsers #1115

Merged
merged 14 commits into from
Nov 3, 2023

Conversation

vinodkiran
Copy link
Contributor

@HenryHengZJ Need your (early) feedback on this new Feature.

Lanchain Output Parsers

Very initial (draft) implementation for handing output parsers in Flowise.

image

image

Do not spend time reviewing the code, it needs a lot of cleanup / refactoring.

@vinodkiran
Copy link
Contributor Author

image

@HenryHengZJ
Copy link
Contributor

awesome! will use this as the base for further improvement.

1.) In this PR we will be returning the reponse as a JSON instead of string - { text: 'some output' }. If we have output parser, would prefer to have smtg like { json: {key: val} }. And from the UI, we can parse it and return as a nice JSON UI;
image

2.) For Zod Schema, would love to have a UI that allow easier schema input. Similar to custom tool:
image

3.) From the PR, we have if else check to cater for PromptTemplate and ChatPromptTemplate, what about other prompt like FewShot, can we use output parser for that, if not, how do we disable connection

Just few thoughts, don't have to implement all of these yet

@vinodkiran
Copy link
Contributor Author

  1. Fewshot and other templates: Yes, I am working on this. was eager to get some early directional feedback, so I created PR. I will push a new commit with changes to handle the other template types.

1 & 2): Could be a new PR, as they will add a lot of value. After the 1.0 release of this component node.

@vinodkiran
Copy link
Contributor Author

image

@HenryHengZJ
Copy link
Contributor

image

looks great! are you going to add more output parser? let me know when its ready to review

@vinodkiran vinodkiran marked this pull request as ready for review October 26, 2023 04:44
@HenryHengZJ
Copy link
Contributor

@vinodkiran would love to have the 1.) implemented for the next PR, so we can properly return the JSON when calling API, and this can lead to us starting to implement extraction/tagging chain - https://js.langchain.com/docs/modules/chains/popular/structured_output. What do you think?

@vinodkiran
Copy link
Contributor Author

@vinodkiran would love to have the 1.) implemented for the next PR, so we can properly return the JSON when calling API, and this can lead to us starting to implement extraction/tagging chain - https://js.langchain.com/docs/modules/chains/popular/structured_output. What do you think?

@HenryHengZJ Agreed. Makes perfect sense. I will update this PR to return json...when will the base PR be merged?

@HenryHengZJ
Copy link
Contributor

will be testing it today

@vinodkiran
Copy link
Contributor Author

will be testing it today

Sporadically, I see double responses to the front end. Also, sometimes the response is shown before the output parser (parse method) is invoked (in a debug mode). Kindly validate.

@HenryHengZJ
Copy link
Contributor

you mean this?
image

seems to happen when running yarn dev, yarn start works fine though

@HenryHengZJ
Copy link
Contributor

@vinodkiran can we use this as base branch for Output Parser, and create branch for further PRs and merge into this. Once everything is in, then we merge to main and release to public.

@HenryHengZJ
Copy link
Contributor

HenryHengZJ commented Oct 29, 2023

@vinodkiran can you allow edits from maintainer on your fork? here's guide

@vinodkiran
Copy link
Contributor Author

@vinodkiran can you allow edits from maintainer on your fork? here's guide

Yes, maintainer edits are enabled for this PR.

try {
parsedStructure = JSON.parse(structure)

// NOTE: When we change Flowise to return a json response, the following has to be changed to: JsonStructuredOutputParser
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@HenryHengZJ we could probably use the JsonStructuredOutputParser and not hardcode ```json in the downstream classes.

@HenryHengZJ
Copy link
Contributor

Update: Changed StructuredOutputParser to use Zod Schema:
image

@HenryHengZJ
Copy link
Contributor

@vinodkiran let me know if you have any question before merging

@vinodkiran
Copy link
Contributor Author

@vinodkiran let me know if you have any question before merging

@HenryHengZJ 1) do we continue to use the word experimental in the category? 2) currently the input parameter is defined for LLMchain only and not for others. Do we target the rest as a separate PR?

@HenryHengZJ
Copy link
Contributor

@vinodkiran let me know if you have any question before merging

@HenryHengZJ 1) do we continue to use the word experimental in the category? 2) currently the input parameter is defined for LLMchain only and not for others. Do we target the rest as a separate PR?

  1. Yeah we can remove the word experimental
  2. I think for starter we can have output parser only for Llmchain. And we can add them to other chains if there is further demand or requests

@HenryHengZJ
Copy link
Contributor

Merged in #1009, resolving the conflict now

# Conflicts:
#	packages/ui/src/views/chatmessage/ChatMessage.js
@HenryHengZJ HenryHengZJ merged commit dede259 into FlowiseAI:main Nov 3, 2023
2 checks passed
hemati pushed a commit to hemati/Flowise that referenced this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants