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

Adding abiliy to add multiple AI bots. #175

Merged
merged 11 commits into from
May 21, 2024
Merged

Adding abiliy to add multiple AI bots. #175

merged 11 commits into from
May 21, 2024

Conversation

crspeller
Copy link
Member

Adds the ability to configure multiple AI bots. Each bot can be configured with a different backed and custom instructions.

@crspeller crspeller marked this pull request as ready for review May 3, 2024 23:09
Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Awesome work on this @crspeller. A few things I noticed in my review on corpus:

  1. The blinking cursor often is showing in the wrong place - in the middle of the streaming post rather than the end. I don't think this is related to this PR necessarily though. Do we have a ticket open for fixing this already?

  2. Can we update the UI for the bot selection on the channel and thread summarization menus? This is my bad for not making it more consistent with the bot selector on the RHS. Rather than showing the chevron icon to the right of the bot label, I'd like to incorporate the chevron in to the bot label
    image

  3. For the default selected bot, in the RHS, it doesn't seem to remember what your last selected bot was. It always defaults to AI Copilot. Also, there seems to be inconsistent behavior between the channel summarization (which remembers the last-selected bot), and the thread summarization (which always defaults to AI Copilot). My instinct is to always remember the last-used bot.

  4. I just noticed in the thread summarization menu, when you open it and then hover off, it closes. This is not consistent with the other menus on posts. It should hide when you click away or make a selection from the menu, not when you hover off. Again, not sure if this is related to this PR.

  5. Opened a new ticket for a new problem I discovered while testing this: When in the global Threads view and then open the AI RHS, if I click th 'Brainstorm ideas' button, it populates the Thread input rather than the AI RHS input.

  6. In the system console, for the expand/collapse header of each bot, can we change the cursor to the pointer cursor when hovering?

  7. In the system console, I created a new bot without the complete information (API key field blank). It let me save it and then the new bot showed up in the UI on the chat facing side. It initially shows 'Missing Information', but all it seemed to require was the Display name, username and avatar. We should have checks in place to not show an incorrectly configured bot.

  8. I created a bot with the same username as another existing bot and didn't get any error. When I switched back to the chat-facing side, in the bot list, it replaced the old bot with the same username with the new one I created and no longer showed the old bot in the list. We will need to check for unique usernames.

@crspeller
Copy link
Member Author

  1. Opened a ticket here: 🐛 bug: Somtimes the blinking cursor on responces appears in the wrong place. #181
  2. Makes sense. Updated.
  3. Updated as we discussed. Will now remember your preferences across sections, but not update RHS immediately.
  4. Not related, bug in the webapp. 🐛 bug: Hover state for AI menu inconsistant with other post menus #183
  5. Strange! Thanks.
  6. Updated
    7+8. Added some validation for these cases. Bots without full configuration will not be created or updated. Duplicates will cause errors until duplication is resolved. (The UX is kinda bad around the duplication case, but it's a little complicated to do correctly because of how the system console works with plugins)

Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Thanks for resolving those items. A few more things I noticed:

  1. The dropdown component seems to cut off the bottom of text. Notice the bottom of the g is getting cutoff in Ask Sage
image
  1. When I click 'add an AI bot', I wonder if we should consider having the section expanded right away. It's adding another click to expand it which seems unncessesary right now.

  2. Also, it feels odd to call out immediately that there is missing information before I've even started entered anything. Could we only do this if I've clicked 'save' with missing information? And then are we able to call out the fields that are missing information at that point?
    image

  3. This is odd, but when I create a new bot and start typing in the characters wiz the bot icon automatically changes on its own somehow.
    https://github.com/mattermost/mattermost-plugin-ai/assets/2040554/8608e446-5238-41da-9dcf-17eb5dfc85c8

  4. I wasn't able to test to see if the empty state for bots was added because I didn't want to blow away the bots from corpus. Could you just share a screenshot of that?

  5. I also wasn't able to see what it looks like if I don't have an enterprise license. Could you share a screenshot for that as well?

  6. Are we going to implement a confirmation when deleting a bot? We had this in the designs, but I don't see it yet here in the latest.
    image

  7. In the chat history view, can adjust x replies so when there is only 1 it says reply. Maybe you already have a ticket for this?
    image

@crspeller
Copy link
Member Author

  1. Only happens on Chrome apparently. Fixed by just making the text field slightly bigger.
    2+3. We can make an improvement ticket for those.
  2. Haha fixed.

image
You can just delete all the bots on corpus, just don't press the save button.
6.
image
7. It feels like it might be a bit redundant given the requirement to press save.
8. Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we update this chip with the following styles?

color: var(--button-bg);
background: rgba(var(--button-bg-rgb), 0.12);

image

Copy link
Member Author

Choose a reason for hiding this comment

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

image
Result.

Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Looks great @crspeller. Just a few minor tweaks noted, but I think we can make any other improvements in follow-ups

@crspeller crspeller requested a review from jupenur May 21, 2024 14:57
@crspeller crspeller merged commit bc89035 into master May 21, 2024
5 checks passed
@crspeller crspeller deleted the multi-bot branch May 21, 2024 18:27
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