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.

Sign up for GitHub

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

Jump to bottom

Improve chat initial experience #885

Merged
merged 13 commits into from
Sep 22, 2024
Merged

Improve chat initial experience #885

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a8e4ed8
show initial messages
mdmohsin7 Sep 18, 2024
22fbf89
clear chat func in backend
mdmohsin7 Sep 19, 2024
a280ead
clear chat func on frontend
mdmohsin7 Sep 19, 2024
eb1b0aa
delete messages using firebase batch on backend
mdmohsin7 Sep 20, 2024
9cbb312
improve experience
mdmohsin7 Sep 20, 2024
83c747d
fetch messages early
mdmohsin7 Sep 20, 2024
2c52e5b
improve ux by showing cached messages while fetching messages in bg
mdmohsin7 Sep 20, 2024
4c4fdae
animated mini banner widget
mdmohsin7 Sep 20, 2024
f38bd58
improve memory in chat loading speed by only fetching it if it does n…
mdmohsin7 Sep 20, 2024
5250c23
Merge branch 'main' into chat-improvements
mdmohsin7 Sep 20, 2024
f945d41
minor ui improvements
mdmohsin7 Sep 20, 2024
8be59e4
show delete and refresh on scroll
mdmohsin7 Sep 21, 2024
47e8f15
soft delete messages
mdmohsin7 Sep 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/lib/backend/http/api/messages.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ Future<List<ServerMessage>> getMessagesServer() async {
return [];
}

Future<List<ServerMessage>> clearChatServer() async {
var response = await makeApiCall(url: '${Env.apiBaseUrl}v1/clear-chat', headers: {}, method: 'DELETE', body: '');
if (response == null) throw Exception('Failed to delete chat');
if (response.statusCode == 200) {
return [ServerMessage.fromJson(jsonDecode(response.body))];
} else {
throw Exception('Failed to delete chat');
}
}
Comment on lines +26 to +34
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The clearChatServer function is making an API call to clear the chat. However, it's not passing any user identification or authentication headers in the request. This could lead to security issues as anyone can clear the chat without proper authorization. Please ensure that you are passing the necessary headers for user identification and authorization.

- var response = await makeApiCall(url: '${Env.apiBaseUrl}v1/clear-chat', headers: {}, method: 'DELETE', body: '');
+ var response = await makeApiCall(url: '${Env.apiBaseUrl}v1/clear-chat', headers: {'Authorization': 'Bearer $userToken'}, method: 'DELETE', body: '');

Also, consider adding error handling for different types of HTTP status codes. Currently, if the status code is not 200, it throws a generic exception. It would be more informative to throw specific exceptions based on the status code.

- if (response.statusCode == 200) {
-   return [ServerMessage.fromJson(jsonDecode(response.body))];
- } else {
-   throw Exception('Failed to delete chat');
- }
+ switch (response.statusCode) {
+   case 200:
+     return [ServerMessage.fromJson(jsonDecode(response.body))];
+   case 401:
+     throw Exception('Unauthorized request');
+   case 404:
+     throw Exception('Chat not found');
+   default:
+     throw Exception('Failed to delete chat');
+ }


Future<ServerMessage> sendMessageServer(String text, {String? pluginId}) {
return makeApiCall(
url: '${Env.apiBaseUrl}v1/messages?plugin_id=$pluginId',
Expand Down
12 changes: 6 additions & 6 deletions app/lib/pages/chat/widgets/ai_message.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ class _AIMessageState extends State<AIMessage> {
style: TextStyle(fontSize: 15.0, fontWeight: FontWeight.w500, color: Colors.grey.shade300),
)),
if (widget.message.id != 1) _getCopyButton(context), // RESTORE ME
// if (message.id == 1 && displayOptions) const SizedBox(height: 8),
// if (message.id == 1 && displayOptions) ..._getInitialOptions(context),
if (widget.displayOptions) const SizedBox(height: 8),
if (widget.displayOptions) ..._getInitialOptions(context),
Comment on lines +121 to +122
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The changes made here are good for improving user engagement. The initial options are now displayed based on the displayOptions flag, which is more flexible than the previous hard-coded condition.

if (messageMemories.isNotEmpty) ...[
const SizedBox(height: 16),
for (var data in messageMemories.indexed) ...[
Expand Down Expand Up @@ -256,7 +256,7 @@ class _AIMessageState extends State<AIMessage> {
_getInitialOption(BuildContext context, String optionText) {
return GestureDetector(
child: Container(
padding: const EdgeInsets.symmetric(horizontal: 12.0, vertical: 8),
padding: const EdgeInsets.symmetric(horizontal: 12.0, vertical: 10),
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The padding for the container has been increased from 8 to 10. This is a minor UI change and doesn't seem to introduce any issues.

-        padding: const EdgeInsets.symmetric(horizontal: 12.0, vertical: 8),
+        padding: const EdgeInsets.symmetric(horizontal: 12.0, vertical: 10),

width: double.maxFinite,
decoration: BoxDecoration(
color: Colors.grey.shade900,
Expand All @@ -273,11 +273,11 @@ class _AIMessageState extends State<AIMessage> {
_getInitialOptions(BuildContext context) {
return [
const SizedBox(height: 8),
_getInitialOption(context, 'What tasks do I have from yesterday?'),
_getInitialOption(context, 'What\'s been on my mind a lot?'),
const SizedBox(height: 8),
_getInitialOption(context, 'What conversations did I have with John?'),
_getInitialOption(context, 'Did I forget to follow up on something?'),
const SizedBox(height: 8),
_getInitialOption(context, 'What advise have I received about entrepreneurship?'),
_getInitialOption(context, 'What\'s the funniest thing I\'ve said lately?'),
Comment on lines +306 to +310
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The text content of the initial options has been updated to be more engaging and interesting for the user. This is a good change for improving user experience.

];
}
}
6 changes: 6 additions & 0 deletions app/lib/providers/message_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ class MessageProvider extends ChangeNotifier {
return messages;
}

Future clearChat() async {
var mes = await clearChatServer();
messages = mes;
notifyListeners();
}
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The clearChat method is not handling potential errors that might occur during the API call to clearChatServer(). It's important to add error handling here to prevent the application from crashing or entering an inconsistent state if the server request fails. You can use a try-catch block to handle any exceptions that might be thrown.

  Future clearChat() async {
    try {
      var mes = await clearChatServer();
      messages = mes;
      notifyListeners();
    } catch (e) {
      // Handle or log the error as appropriate for your application.
      print('Error clearing chat: $e');
    }
  }


void addMessage(ServerMessage message) {
messages.insert(0, message);
notifyListeners();
Expand Down
17 changes: 17 additions & 0 deletions backend/database/chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,20 @@ def get_messages(uid: str, limit: int = 20, offset: int = 0, include_memories: b
]

return messages


def clear_chat(uid,batch_size):
user_ref = db.collection('users').document(uid)
messages_ref = user_ref.collection('messages')
if batch_size == 0:
return
docs = messages_ref.list_documents(page_size=batch_size)
deleted = 0

for doc in docs:
print(f"Deleting doc {doc.id} => {doc.get().to_dict()}")
doc.delete()
deleted = deleted + 1

if deleted >= batch_size:
return clear_chat(uid,batch_size)
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The clear_chat function recursively deletes messages from the database. However, it's not a good practice to use recursion for this kind of operation because it can lead to a stack overflow if the number of recursive calls is too large. Instead, consider using a loop to delete the messages.

Also, the print statement on line 96 could potentially expose sensitive user data in logs. It's best to avoid logging sensitive information.

Here's a suggested change:

- def clear_chat(uid,batch_size):
-     user_ref = db.collection('users').document(uid)
-     messages_ref = user_ref.collection('messages')
-     if batch_size == 0:
-         return
-     docs = messages_ref.list_documents(page_size=batch_size)
-     deleted = 0
- 
-     for doc in docs:
-         print(f"Deleting doc {doc.id} => {doc.get().to_dict()}")
-         doc.delete()
-         deleted = deleted + 1
- 
-     if deleted >= batch_size:
-         return clear_chat(uid,batch_size)

+ def clear_chat(uid, batch_size):
+     user_ref = db.collection('users').document(uid)
+     messages_ref = user_ref.collection('messages')
+     while True:
+         docs = messages_ref.list_documents(page_size=batch_size)
+         deleted = 0
+         for doc in docs:
+             doc.delete()
+             deleted += 1
+         if deleted < batch_size:
+             break

7 changes: 7 additions & 0 deletions backend/routers/chat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ def send_message(
ai_message.memories = memories if len(memories) < 5 else memories[:5]
return ai_message

@router.delete('/v1/clear-chat', tags=['chat'], response_model=Message)
def clear_chat(uid: str = Depends(auth.get_current_user_uid)):

chat_db.clear_chat(uid, 400)
return initial_message_util(uid)



def initial_message_util(uid: str, plugin_id: Optional[str] = None):
plugin = get_plugin_by_id(plugin_id)
Expand Down
Loading