Skip to content

Integrate plugins into chat#276

Closed
diegoasua wants to merge 30 commits intoBasedHardware:mainfrom
diegoasua:diego/plugin-chat-integration
Closed

Integrate plugins into chat#276
diegoasua wants to merge 30 commits intoBasedHardware:mainfrom
diegoasua:diego/plugin-chat-integration

Conversation

@diegoasua
Copy link
Copy Markdown

First take at integrating plugins into chat.

String qaStreamedBody(String context, List<Message> chatHistory) {
String qaStreamedBody(String personality, String context, List<Message> chatHistory) {
var prompt = '''
This is your personality, you should try to answer in this style: $personality
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we do this part of the prompt optional?

Comment thread apps/AppWithWearable/lib/backend/api_requests/stream_api_response.dart Outdated
);
}

// FIXME: only supports 1 active plugin. When multiple are active uses the first one
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice, is there a way we could have a selector on the chat page so people can select which plugin to chat, maybe like a floating dropdown top right or smth

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't get why more than one plugin can be active at a time. It makes more sense to only allow 1 plugin active. Also for the memories. Is there a reason several plugins can be active simultaneously?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ideally plugins are used only whenever they are useful, so if you are having a medical conversation, it uses x plugin, entrepreneurship, but that requires better plugin prompt, maybe you could take a look so we filter out useless plugins per memory. It's complex to expect the user will remind before every conversation what plugin they might need, so if you activate many you are set and forget

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

btw, how can I find you on discord? would love to chat

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

will dm

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

for the multiple plugin case. We can pass all personalities to the system prompt and ask the LLM to choose which one is most relevant for the current conversation. This will fulfill the promise to the user they don't have to remember turning plugins on and off. They choose what they want, and chat layer is flexible enough to understand and adapt. Reduces strain on what is asked by us from the user improving overall UX.

I still think we should limit how many plugins are active at the same time. The more active plugins the more chances of this failing/confusing the LLM. As a measure for now we can limit active plugins to 5.

@josancamon19
Copy link
Copy Markdown
Contributor

Looking great so far!

@diegoasua
Copy link
Copy Markdown
Author

I am removing BLE scan by name even though it's out out of scope of this PR as we identified here this change was undone #225 (comment)

@diegoasua
Copy link
Copy Markdown
Author

diegoasua commented Jun 17, 2024

Alright this is ready to review @josancamon19 I tested with a single and multiple plugins. I tested the Paul Graham plugin together with other two and asked questions from my memories about neural networks and seems that it selected the right plugin in chat, and the right memory.

Now that being said chat will only be as good with plugins if the plugin prompts are well written. The example here is not great for the purpose of steering behavior in a conversation.

The prompt is good for generating memories but not so much for driving the personality of the conversation. I see two options.

  1. We ask that new plugins have a new field personality that drives the conversation. An example of a good personality plugin would be:
    You are Shakespeare. You will talk in Shakesperian ways, poetic and using old English words here and there. You will not deviate from this character
    We would have to add this field to the existing ones, but there are only like 10 at the moment.
  2. We dynamically generate the personality field by asking an LLM to do it when the plugin is submitted to the database. We pass the plugin and ask it to add the new field with a few shot examples. For me this would be the preferred method.

The prompt itself gives instructions on how to process memories not in how to behave the conversation. LLMs can infer what you mean, but it will drive their behavior stronger if they are actually given a personality based on those instructions.

@josancamon19
Copy link
Copy Markdown
Contributor

Will review today @diegoasua

Future<List<BTDeviceStruct?>> scanDevices() async {
List<BTDeviceStruct> foundDevices = await bleFindDevices();
var filteredDevices = foundDevices.where((device) => device.name == "Friend").toList();
// var filteredDevices = foundDevices.where((device) => device.name == "Friend").toList();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

see here for more discussion #225 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would show all ble available devices right? as far as I understand, if both options work, why to do it like this is what I don't understand

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No it will not show all BLE devices. If the device is named different from "Friend" it will not show up. When that line is active it is filtering BLE devices by name. It will go through the onboarding screen with a real device as long as its name is "Friend" but not with a simulator. Recently we stopped filtering devices by name and use UUID instead. So this line is not only unnecessary but also prevents us from pairing a simulator.

See #225 (comment)

await streamApiResponse(ragContext, _callbackFunctionChatStreaming(memoryIds), widget.messages, () {
widget.messages.last.memoryIds = memoryIds;
prefs.chatMessages = widget.messages;
// await streamApiResponse(ragContext, _callbackFunctionChatStreaming(memoryIds), widget.messages, () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's remove this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

To be clear, you mean remove the commented out part?

}

String qaStreamedBody(String context, List<Message> chatHistory) {
String qaStreamedBody(dynamic personality, String context, List<Message> chatHistory) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For now let's use the description as their personality and reformat the plugins so the description has this personality (ish).

@josancamon19
Copy link
Copy Markdown
Contributor

Also, let's add on chat a plugin selector.

Basically, there'd be one of the plugins to select, in a dropdown in the chat in some way, but not automatically, initially let's do it like this, and see how people interact with it

@diegoasua
Copy link
Copy Markdown
Author

Alright now there is a dropdown in chat that lets you choose which personality to chat with from the list of downloaded plugins. you can only choose one. Give it a spin and let me know. I have only tested it in iOS so it would be good if someone can give it a go in Android too.

@josancamon19 ready for review

Future<List<BTDeviceStruct?>> scanDevices() async {
List<BTDeviceStruct> foundDevices = await bleFindDevices();
var filteredDevices = foundDevices.where((device) => device.name == "Friend").toList();
// var filteredDevices = foundDevices.where((device) => device.name == "Friend").toList();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would show all ble available devices right? as far as I understand, if both options work, why to do it like this is what I don't understand

}

String qaStreamedBody(String context, List<Message> chatHistory) {
String qaStreamedBody(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The only linting rule should be 120 characters line and default android studio rules

CleanShot 2024-06-21 at 16 25 22@2x

Please relint

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This would show all ble available devices right? as far as I understand, if both options work, why to do it like this is what I don't understand

I answered this above but I want to make sure this is clear, so copy pasting the response here again:

No it will not show all BLE devices. If the device is named different from "Friend" it will not show up. When that line is active it is filtering BLE devices by name. It will go through the onboarding screen with a real device as long as its name is "Friend" but not with a simulator. Recently we stopped filtering devices by name and use UUID instead. So this line is not only unnecessary but also prevents us from pairing a simulator.
See #225 (comment)

Copy link
Copy Markdown
Author

@diegoasua diegoasua Jun 22, 2024

Choose a reason for hiding this comment

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

re: linting

ack that will apply that rule

Copy link
Copy Markdown
Author

@diegoasua diegoasua Jun 22, 2024

Choose a reason for hiding this comment

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

I wanted to add, Dart docs suggests we should not use more than 80 characters per line as it affects readability.

@@ -14,6 +14,8 @@ import 'package:friend_private/backend/preferences.dart';
import 'package:friend_private/pages/chat/widgets/ai_message.dart';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be a dropdown that goes on the Appbar only when chat page is selected.

No bottom sheet, a dropdown.

It should contain the plugin name + plugin picture.

It should look good, and match the current design.

When there are no plugins enabled it should have that default dropdown item, and a 2nd one with enable plugins click, that opens the plugins page.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sounds good. I appreciate these more clear deliverables. They guide development better

@diegoasua
Copy link
Copy Markdown
Author

ready for another round of reviews @josancamon19 . What's new:

  • Moved selection to dropdown in AppBar
  • Added two default fields for when plugins are deactivated: default and "enable plugins". The later takes you to the plugin page
  • Followed app design system
  • Show default and enable plugins when no plugins downloaded
  • clicking on enable plugins takes you to the plugin page
  • plugins show image in selection

),
),
// Text(['Memories', 'Device', 'Chat'][_selectedIndex]),
if (_controller!.index == 2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMG_3714

This looks bad, I don't see how this fits with current designs besides de border gradient.

And let's please remove the padding GPT comments.

Put this in a separate widget.

Finally, please put this component in a separate function, could be inside same file.

Remove the "Default" and put only "View Plugins"

Also, messages answered with plugin personality should have the plugin id as optional field in db, and instead of the message sender icon we have on the left of ai message it should use the plugin image as the sender.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Please explain how you want the widget to look like. If you don't set specific deliverables I don't know what you want. You say it looks ugly and doesn't fit in current app designs. Is it the font? Spacing? Size? Palette? I can iterate on it but I need to know what you want. Otherwise this is not going to work. Please advise

@josancamon19
Copy link
Copy Markdown
Contributor

Closing PR, as I don't see significant progress, and reassigning ticket.

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