From fd81446ba3ccf9348bcd6892e9da1683ff73da91 Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Sat, 9 Mar 2024 19:59:30 +0530 Subject: [PATCH] Do not create new chat session when an old chat session is deleted - Fix `get_conversation_by_user' shouldn't return new conversation if conversation with requested id not found. It should only return new conversation if no specific conversation is requested and no conversations found for user at all - Repro - Delete a new chat, this calls loadChat via window.onload which calls server /chat/history API endpoint with conversationId set to that of just deleted conversation sporadically The call to GET chat/history API with conversationId set occurs when window.onload triggers before the conversationId is deleted by the delete button after the DELETE /chat/history API call (via race) - In such a scenario, get_conversation_by_user called by chat/history API with conversationId of deleted conversation returns a new conversation - Miscellaneous - Chat history load should be logged as call to that chat_history api, not the "chat" api - Show status updates of clearing conversation history in chat input - Simplify web, desktop client code by removing unnecessary new variables --- src/interface/desktop/chat.html | 16 +++++++--------- src/khoj/database/adapters/__init__.py | 4 ++-- src/khoj/interface/web/chat.html | 17 ++++++++--------- src/khoj/routers/api_chat.py | 9 +++++++-- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/interface/desktop/chat.html b/src/interface/desktop/chat.html index 567b9f8c..3da215fe 100644 --- a/src/interface/desktop/chat.html +++ b/src/interface/desktop/chat.html @@ -626,10 +626,9 @@ let chatBody = document.getElementById("chat-body"); chatBody.innerHTML = ""; - let conversationId = chatBody.dataset.conversationId; let chatHistoryUrl = `/api/chat/history?client=desktop`; - if (conversationId) { - chatHistoryUrl += `&conversation_id=${conversationId}`; + if (chatBody.dataset.conversationId) { + chatHistoryUrl += `&conversation_id=${chatBody.dataset.conversationId}`; } fetch(`${hostURL}${chatHistoryUrl}`, { headers }) @@ -650,6 +649,8 @@ // Disable chat input field and update placeholder text document.getElementById("chat-input").setAttribute("disabled", "disabled"); document.getElementById("chat-input").setAttribute("placeholder", "Configure Khoj to enable chat"); + } else if (data.status != "ok") { + throw new Error(data.message); } else { // Set welcome message on load renderMessage("Hey 👋🏾, what's up?", "khoj"); @@ -657,12 +658,9 @@ return data.response; }) .then(response => { - conversationId = response.conversation_id; - const conversationTitle = response.slug || `New conversation 🌱`; - let chatBody = document.getElementById("chat-body"); - chatBody.dataset.conversationId = conversationId; - chatBody.dataset.conversationTitle = conversationTitle; + chatBody.dataset.conversationId = response.conversation_id; + chatBody.dataset.conversationTitle = response.slug || `New conversation 🌱`; const fullChatLog = response.chat || []; @@ -791,7 +789,7 @@ chatBody.dataset.conversationId = ""; chatBody.dataset.conversationTitle = ""; loadChat(); - flashStatusInChatInput("🗑 Cleared conversation history"); + flashStatusInChatInput("🗑 Cleared previous conversation history"); }) .catch(err => { flashStatusInChatInput("⛔️ Failed to clear conversation history"); diff --git a/src/khoj/database/adapters/__init__.py b/src/khoj/database/adapters/__init__.py index c39dc14b..9d74df05 100644 --- a/src/khoj/database/adapters/__init__.py +++ b/src/khoj/database/adapters/__init__.py @@ -405,9 +405,9 @@ class ConversationAdapters: else: conversation = ( Conversation.objects.filter(user=user, client=client_application).order_by("-updated_at").first() - ) + ) or Conversation.objects.create(user=user, client=client_application) - return conversation or Conversation.objects.create(user=user, client=client_application) + return conversation @staticmethod def get_conversation_sessions(user: KhojUser, client_application: ClientApplication = None): diff --git a/src/khoj/interface/web/chat.html b/src/khoj/interface/web/chat.html index 062798a2..ef45b0db 100644 --- a/src/khoj/interface/web/chat.html +++ b/src/khoj/interface/web/chat.html @@ -786,10 +786,9 @@ To get started, just start typing below. You can also type / to see a list of co let chatBody = document.getElementById("chat-body"); chatBody.innerHTML = ""; chatBody.classList.add("relative-position"); - let conversationId = chatBody.dataset.conversationId; let chatHistoryUrl = `/api/chat/history?client=web`; - if (conversationId) { - chatHistoryUrl += `&conversation_id=${conversationId}`; + if (chatBody.dataset.conversationId) { + chatHistoryUrl += `&conversation_id=${chatBody.dataset.conversationId}`; } if (window.screen.width < 700) { @@ -814,6 +813,8 @@ To get started, just start typing below. You can also type / to see a list of co // Disable chat input field and update placeholder text document.getElementById("chat-input").setAttribute("disabled", "disabled"); document.getElementById("chat-input").setAttribute("placeholder", "Configure Khoj to enable chat"); + } else if (data.status != "ok") { + throw new Error(data.message); } else { // Set welcome message on load renderMessage(welcome_message, "khoj"); @@ -822,12 +823,9 @@ To get started, just start typing below. You can also type / to see a list of co }) .then(response => { // Render conversation history, if any - conversationId = response.conversation_id; - const conversationTitle = response.slug || `New conversation 🌱`; - let chatBody = document.getElementById("chat-body"); - chatBody.dataset.conversationId = conversationId; - chatBody.dataset.conversationTitle = conversationTitle; + chatBody.dataset.conversationId = response.conversation_id; + chatBody.dataset.conversationTitle = response.slug || `New conversation 🌱`; let chatBodyWrapper = document.getElementById("chat-body-wrapper"); const fullChatLog = response.chat || []; @@ -1050,9 +1048,10 @@ To get started, just start typing below. You can also type / to see a list of co chatBody.dataset.conversationId = ""; chatBody.dataset.conversationTitle = ""; loadChat(); + flashStatusInChatInput("🗑 Cleared previous conversation history"); }) .catch(err => { - return; + flashStatusInChatInput("⛔️ Failed to clear conversation history"); }); }); conversationMenu.appendChild(deleteButton); diff --git a/src/khoj/routers/api_chat.py b/src/khoj/routers/api_chat.py index 3c170f6f..0fc31b3b 100644 --- a/src/khoj/routers/api_chat.py +++ b/src/khoj/routers/api_chat.py @@ -75,8 +75,13 @@ def chat_history( user=user, client_application=request.user.client_app, conversation_id=conversation_id ) - meta_log = conversation.conversation_log + if conversation is None: + return Response( + content=json.dumps({"status": "error", "message": f"Conversation: {conversation_id} not found"}), + status_code=404, + ) + meta_log = conversation.conversation_log meta_log.update( {"conversation_id": conversation.id, "slug": conversation.title if conversation.title else conversation.slug} ) @@ -84,7 +89,7 @@ def chat_history( update_telemetry_state( request=request, telemetry_type="api", - api="chat", + api="chat_history", **common.__dict__, )