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

BaseLLM bug #47

Closed
XavierRex opened this issue Mar 19, 2023 · 7 comments
Closed

BaseLLM bug #47

XavierRex opened this issue Mar 19, 2023 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@XavierRex
Copy link

bug

I can't connect the model to any tool or LLMMathChain. OpenAI model input type is str. (0.0.40 version on windows)

@ogabrielluiz
Copy link
Contributor

Hey!
We'll look into that ASAP.

@ibiscp do you know what could be causing this? I tested it here and it happened to me too.

I ran pip install langflow -Uand langflow put the OpenAI node and the PAL-MATH node and I can't connect them to each other.

Maybe the output to OpenAI shouldn't be str.

@ogabrielluiz ogabrielluiz added the bug Something isn't working label Mar 20, 2023
@ibiscp
Copy link
Contributor

ibiscp commented Mar 20, 2023

I'll debug that.

@ScripterSugar
Copy link
Contributor

ScripterSugar commented Mar 20, 2023

In frontend/src/CustomNodes/GenericNode.js L84

id={data.type + "|" + data.id + "|" + data.node.base_classes.map((b) => ("|" + b))}

This line is problematic because it produces id in the form like this OpenAI|dndnode_26|BaseOpenAI,|BaseLLM,|BaseLanguageModel
You can see the trailing commas making isValidConnection to return false-negatives on valid connections.
So changing that line to

id={data.type + "|" + data.id + data.node.base_classes.join('|')}

or more likely to

id={[data.type, data.id, ...data.node.base_classes].join('|')}

will fix the problem in current implementation.

We also need to fix hard-coded "type:str" notation on ParameterComponent. I'd like to also find a way to actually use type parameter rather than just parse handle's id field when we're validating connections between nodes (Because current implementation is eager to be broken in future if we try to introduce way to manage custom components. Using reserved eparators for component ID will break the validation), but I'm not sure that's doable as this is my first time using reactflow library.

May I open PR from my fork to resolve this issue or is someone already working on it?

@ogabrielluiz
Copy link
Contributor

ogabrielluiz commented Mar 20, 2023

Thanks for the help! I'll test it out now.
Let's take the hard-coded "type:str" discussion to another issue or is it related to this?

@ScripterSugar
Copy link
Contributor

ScripterSugar commented Mar 20, 2023

I fixed the tooltip part, but changing type={'str'} part will trigger some side effects within the handling code so I decided not to touch that until I'm certain about its behavior 😄

Will take a further investigation afterwards if needed :)

@ogabrielluiz
Copy link
Contributor

Ok. I'll open a discussion based on your comment so we can maybe think of a better solution.

@ogabrielluiz ogabrielluiz mentioned this issue Mar 20, 2023
@ibiscp
Copy link
Contributor

ibiscp commented Mar 20, 2023

Fixed in version 0.0.44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants