-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Allow editing chatbot messages #10203
base: main
Are you sure you want to change the base?
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-pypi-previews.s3.amazonaws.com/13a4adf3d973aa5e00fe31537572cdb4188bf4f1/gradio-5.9.0-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@13a4adf3d973aa5e00fe31537572cdb4188bf4f1#subdirectory=client/python" Install Gradio JS Client from this PR npm install https://gradio-npm-previews.s3.amazonaws.com/13a4adf3d973aa5e00fe31537572cdb4188bf4f1/gradio-client-1.8.0.tgz Use Lite from this PR <script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/13a4adf3d973aa5e00fe31537572cdb4188bf4f1/dist/lite.js""></script> |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice @aliabid94 - works well and the e2e is great. Just left some nits on the editing textbox size
@@ -222,6 +224,7 @@ def __init__( | |||
resizeable: If True, the component will be resizeable by the user. | |||
max_height: The maximum height of the component, specified in pixels if a number is passed, or in CSS units if a string is passed. If messages exceed the height, the component will scroll. If messages are shorter than the height, the component will shrink to fit the content. Will not have any effect if `height` is set and is smaller than `max_height`. | |||
min_height: The minimum height of the component, specified in pixels if a number is passed, or in CSS units if a string is passed. If messages exceed the height, the component will expand to fit the content. Will not have any effect if `height` is set and is larger than `min_height`. | |||
editable: If True, the user can edit the text in the chatbot. Can also be set to "all", which allows the user to edit assistant messages as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since "all" is allowed, how about the type iseditable: Literal["user", "all"] | None = None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating a new editable
attribute, why not use interactive
, which is meant for exactly this use case (i.e. that a user can edit the component's value via the UI)?
If the concern is that we want to restrict the scope of interactivity to either user messages or all messages, then I think its time we allow interactive
to take non-boolean values as well, where it makes sense. For example, we've had requests to restrict the scope of interactivity in the DataFrame component, and this is analogous.
In that case, we could use the interactive
parameter like this:
interactive: bool | Literal["user"] = False
if interactive
is True
, then all messages can be edited. If interactive
is "user", then only user messages can be edited. We may choose to add Literal["assistant"]
later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating a new editable attribute, why not use interactive
I didn't go this route because a lot of gr.Blocks() demos use chatbot as input to store history, and therefore by default, interactive would be true, so these chatbots would become editable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, ok that makes sense, I agree with @freddyaboulton's suggestion above 👍
@@ -170,21 +166,15 @@ | |||
|
|||
$: { | |||
if (!dequal(value, old_value)) { | |||
console.log("CHANGE!", JSON.stringify(value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove
> | ||
{#if in_edit_mode && thought_index === messages.length - 1 && message.type === "text"} | ||
<!-- svelte-ignore a11y-autofocus --> | ||
<textarea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would also help with longer messages that include code blocks
Very nice UI @aliabid94! Left a comment on the parameter above. I'd also suggest we document this in https://www.gradio.app/guides/creating-a-custom-chatbot-with-blocks |
Adds the ability to edit chatbot messages. See below:
In a followup PR, I still need to: