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

Expand gr.Chatbot() functionality + Refactor Tracking Issue #4800

Closed
8 of 12 tasks
dawoodkhan82 opened this issue Jul 5, 2023 · 8 comments
Closed
8 of 12 tasks

Expand gr.Chatbot() functionality + Refactor Tracking Issue #4800

dawoodkhan82 opened this issue Jul 5, 2023 · 8 comments
Assignees
Labels
💬 Chatbot Related to the Chatbot component tracking Issues that are created for tracking

Comments

@dawoodkhan82
Copy link
Collaborator

dawoodkhan82 commented Jul 5, 2023

TODOs

Additional Chatbot Features

@dawoodkhan82 dawoodkhan82 added the tracking Issues that are created for tracking label Jul 5, 2023
@dawoodkhan82 dawoodkhan82 added this to the 4.0 milestone Jul 5, 2023
@dawoodkhan82 dawoodkhan82 self-assigned this Jul 5, 2023
@abidlabs abidlabs modified the milestones: 4.0, Component Cleanup Jul 9, 2023
@dawoodkhan82
Copy link
Collaborator Author

To simplify the gr.Chatbot() api, we can provide a ChatMessageHistory class (similar to LangChain). The class manages the chatbot history and includes methods such as add_user_message, add_bot_message, get_last_message, etc. Right now, the data structure passed into the chatbot is tuples inside a list of lists, which can get very confusing. ChatMessageHistory is how LangChain manages chat history (https://docs.langchain.com/docs/components/memory/chat_message_history), so it will be familiar with devs using LangChain. Thoughts @abidlabs?

@abidlabs
Copy link
Member

I do see value in ChatMessageHistory, but I worry that it comes with a not-so-insignificant cost, which is creating a layer of abstraction over a relatively simple data structure (nested lists), which the vast majority of Python developers know and understand. This causes confusion later on when people want to do basic things that our class does not support -- this is a problem people run into frequently with langchain and I don't think we should follow that path

@dawoodkhan82
Copy link
Collaborator Author

I see what you mean, but the data structure can get messy fairly quickly. Especially when its tuples nested within lists of lists, and when files get involved. Also we have heard feedback, that the chatbot API is confusing, so I think something needs to be done to simplify it. Of course having the ChatInterface abstraction will help too. Having a ChatMessageHistory class can also be helpful when we want to extend the functionality of the chatbot, without having to worry about breaking changes of the underlying data structure. The criticisms that I see of LangChain are because of the fact that added too. many abstraction layers and pointless helper functions. As long as we make sure we don't over engineer the ChatMessageHistory, that should be fine, no?

@abidlabs
Copy link
Member

Of course having the ChatInterface abstraction will help too. Having a ChatMessageHistory class can also be helpful when we want to extend the functionality of the chatbot, without having to worry about breaking changes of the underlying data structure

We'll need to be backwards compatible anyways since many users will continue to pass lists of data so I don't think this will get us out of it. Plus if someone is accessing the Chatbot via the Clients, they'll still get and pass data as lists of data, so again we'll need to fully support lists of data anyways.

Also we have heard feedback, that the chatbot API is confusing, so I think something needs to be done to simplify it.

The feedback I've seen (e.g. #3510) is that we need a simpler way to create a fully functioning Chatbot demo, not that handling the messages is complicated. I agree that nested lists can be slightly confusing, but this class makes things more verbose in my opinion. Here are some examples:

Function Current with the abstraction
Initializing messages = [] messages = ChatMessageHistory()
Add user and bot message messages.append([user_message, bot_message]) messages.add_user_message(user_message) messages.add_bot_message(bot_message)
Get last message messages.pop()[-1] messages.get_last_message()

Imo, the value of the abstraction is just not there, and now a person has to look up the relevant functions in the documentation every time they need to do something new (e.g. how do I clear the message history) instead of building on their existing knowledge of python

@dawoodkhan82
Copy link
Collaborator Author

I see, yeah didn't consider the gradio client, plus the fact that we would still want the chatbot to be backwards compatible.

Function Current with the abstraction
Initializing messages = [] messages = ChatMessageHistory()
Add user and bot message messages.append([user_message, bot_message]) messages.add_user_message(user_message) messages.add_bot_message(bot_message)
Get last message messages.pop()[-1] messages.get_last_message()

Also, makes sense that the abstraction isn't very useful for the simple functions listed above.

Thanks for talking this thru!

@cpwan
Copy link

cpwan commented Dec 28, 2023

How is the progress on "Allow a developer to add various Gradio components in user/bot responses"?

I have been trying to hack the chatInterface, chatbot to include additional output components, e.g. a Textbox for suggested questions, which could be outside of the chatbot block in the UI. The closest I can get is to write a class for nested component so that I can do sth like ChatInterface(fn=new_fn, chatbot=NestedComponents(components=[gr.Chatbot(), gr.Textbox()])) and the new_fn takes the same arguments message, history and return the modified history, but the history is now a tuple of values for multiple output components.

But error occurs when ChatInterface initializes event listeners for the cancel buttons. Any suggestions?

@edwinjosechittilappilly

I would also appreciate if I could add various gradio component as output.
Example I am making a RAG Chat bot and these components below he bot response could be used to show the sources or just the source title. Just like what we see in bing chat as sources.

@abidlabs abidlabs added the 💬 Chatbot Related to the Chatbot component label Mar 15, 2024
@abidlabs
Copy link
Member

I've added a Chatbot label instead (more appropriate as new issues are being created), I think we can close this tracking issue. Feel free to reopen if you feel differently @dawoodkhan82

@abidlabs abidlabs closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💬 Chatbot Related to the Chatbot component tracking Issues that are created for tracking
Projects
None yet
Development

No branches or pull requests

4 participants