feat(chatMessages): add brain_id and prompt_id columns #912

mamadoudicko · 2023-08-09T15:47:02Z

Description

Update chat_history table
Update some entities and DTOs
Update chat controller

vercel · 2023-08-09T15:47:10Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 10, 2023 7:53am
quivrapp	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 10, 2023 7:53am

github-actions · 2023-08-09T15:50:03Z

Risk Level 2 - /home/runner/work/quivr/quivr/backend/core/models/databases/supabase/chats.py

The code is generally well written, but there are a few areas that could be improved:

The update_chat_history method uses string conversion for UUIDs. This could potentially lead to issues if the UUID is not valid. Consider adding error handling for this.
The update_chat_history method also directly inserts the input dictionary into the database. This could potentially lead to SQL injection attacks if the input is not properly sanitized. Consider using parameterized queries or ensuring that the input is properly sanitized.

Risk Level 2 - /home/runner/work/quivr/quivr/backend/core/repository/chat/update_chat_history.py

The function update_chat_history does not handle the case where supabase_db.update_chat_history(chat_history) fails and does not return a response. This could lead to an AttributeError when trying to access the data attribute of None. Consider adding error handling for this case. For example:

response = supabase_db.update_chat_history(chat_history)
if response is None or len(response.data) == 0:
    raise HTTPException(
        status_code=500, detail=\"An exception occurred while updating chat history.\"
    )

This will ensure that an appropriate error is raised when the update operation fails.

Risk Level 3 - /home/runner/work/quivr/quivr/backend/core/routes/chat_routes.py

The function create_question_handler has a high cyclomatic complexity due to multiple conditional checks and could be simplified for better readability and maintainability. Consider refactoring the code to reduce complexity.
The function delete_chat_from_db suppresses all exceptions and just prints them. This could lead to silent failures that are hard to debug. Consider re-raising the exception or handling specific exceptions that you expect may occur.
The function check_user_limit has a pass statement in the else clause which is unnecessary and can be removed for cleaner code.
The function create_stream_question_handler has duplicate code for retrieving the user's OpenAI API key and chat model. Consider refactoring this into a separate function to adhere to the DRY (Don't Repeat Yourself) principle.

🔒🐛🔄

Powered by Code Review GPT

gozineb · 2023-08-09T15:55:28Z

backend/core/repository/brain/get_brain_prompt_id.py

+
+
+def get_brain_prompt_id(brain_id: UUID) -> UUID | None:
+    if not brain_id:


but brain_id is typed as UUID in the input props

gozineb · 2023-08-09T15:57:30Z

backend/core/repository/chat/get_chat_history.py

+    user_message: str
+    assistant: str
+    message_time: str
+    prompt_title: Optional[str] = ""


not at the chatHistory level, its at the message level no ? a chat history could be generated using multiple brains in the future

gozineb · 2023-08-09T16:00:27Z

backend/core/repository/chat/get_chat_history.py

    supabase_db = get_supabase_db()
-    history: List[ChatHistory] = supabase_db.get_chat_history(chat_id).data
+    history: List[dict] = supabase_db.get_chat_history(chat_id).data


why dict instead of ChatHistory ?

gozineb · 2023-08-09T16:02:54Z

scripts/tables.sql

@@ -23,7 +23,9 @@ CREATE TABLE IF NOT EXISTS chat_history (
    user_message TEXT,
    assistant TEXT,
    message_time TIMESTAMP DEFAULT current_timestamp,
-    PRIMARY KEY (chat_id, message_id)
+    PRIMARY KEY (chat_id, message_id),
+    prompt_id UUID,


prompt_id and brain_id should reference the prompts and brains table respectively, can be null

gozineb

Reference prompts and brains tables in chat_history

gozineb · 2023-08-10T07:37:16Z

scripts/20230809154300_add_prompt_id_brain_id_to_chat_history_table.sql

+BEGIN 
+    IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'chat_history' AND column_name = 'brain_id') THEN
+        -- Add brain_id column
+        ALTER TABLE chat_history ADD COLUMN brain_id UUID;


reference brains table

gozineb · 2023-08-10T07:37:26Z

scripts/20230809154300_add_prompt_id_brain_id_to_chat_history_table.sql

+    IF NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'chat_history' AND column_name = 'prompt_id') THEN
+        -- Add prompt_id column
+        ALTER TABLE chat_history ADD COLUMN prompt_id UUID;
+    END IF;


reference prompts table

gozineb

Missing references to tables in migration script

gozineb

Bien vuuuu

* feat: add prompt_id and brain_id to chat history) * feat: add prompt_id and brain_id to chat routes

mamadoudicko temporarily deployed to preview August 9, 2023 15:47 — with GitHub Actions Inactive

mamadoudicko had a problem deploying to preview August 9, 2023 15:47 — with GitHub Actions Failure

vercel bot deployed to Preview – quivrapp August 9, 2023 15:49 View deployment

gozineb reviewed Aug 9, 2023

View reviewed changes

gozineb suggested changes Aug 9, 2023

View reviewed changes

mamadoudicko force-pushed the ux-chat-2a branch from fb6acc9 to 0eb87b5 Compare August 9, 2023 16:13

mamadoudicko had a problem deploying to preview August 9, 2023 16:13 — with GitHub Actions Failure

mamadoudicko temporarily deployed to preview August 9, 2023 16:13 — with GitHub Actions Inactive

vercel bot deployed to Preview – quivrapp August 9, 2023 16:17 View deployment

vercel bot deployed to Preview – docs August 9, 2023 16:17 View deployment

gozineb reviewed Aug 10, 2023

View reviewed changes

gozineb approved these changes Aug 10, 2023

View reviewed changes

gozineb suggested changes Aug 10, 2023

View reviewed changes

mamadoudicko added 2 commits August 10, 2023 09:50

feat: add prompt_id and brain_id to chat history)

2dcd525

feat: add prompt_id and brain_id to chat routes

01ebbc8

mamadoudicko force-pushed the ux-chat-2a branch from 0eb87b5 to 01ebbc8 Compare August 10, 2023 07:50

mamadoudicko temporarily deployed to preview August 10, 2023 07:50 — with GitHub Actions Inactive

mamadoudicko had a problem deploying to preview August 10, 2023 07:50 — with GitHub Actions Failure

vercel bot deployed to Preview – quivrapp August 10, 2023 07:52 View deployment

vercel bot deployed to Preview – docs August 10, 2023 07:53 View deployment

gozineb approved these changes Aug 10, 2023

View reviewed changes

gozineb merged commit 6e77732 into main Aug 10, 2023

StanGirard mentioned this pull request Aug 10, 2023

chore(main): release 0.0.55 #906

Merged

ZongZiWang mentioned this pull request Aug 14, 2023

[Issue] Chat question endpoint 500 #952

Closed

ZongZiWang mentioned this pull request Aug 14, 2023

fix(chatMessages): Fix error on answering question #953

Merged

5 tasks

StanGirard pushed a commit that referenced this pull request Sep 12, 2023

feat(chatMessages): add brain_id and prompt_id columns (#912)

6694ea1

* feat: add prompt_id and brain_id to chat history) * feat: add prompt_id and brain_id to chat routes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(chatMessages): add brain_id and prompt_id columns #912

feat(chatMessages): add brain_id and prompt_id columns #912

mamadoudicko commented Aug 9, 2023 •

edited

Loading

vercel bot commented Aug 9, 2023 •

edited

Loading

github-actions bot commented Aug 9, 2023 •

edited

Loading

gozineb Aug 9, 2023

gozineb Aug 9, 2023

gozineb Aug 9, 2023

gozineb Aug 9, 2023

gozineb left a comment

gozineb Aug 10, 2023

gozineb Aug 10, 2023

gozineb left a comment

gozineb left a comment



		def get_brain_prompt_id(brain_id: UUID) -> UUID \| None:
		if not brain_id:

feat(chatMessages): add brain_id and prompt_id columns #912

feat(chatMessages): add brain_id and prompt_id columns #912

Conversation

mamadoudicko commented Aug 9, 2023 • edited Loading

Description

vercel bot commented Aug 9, 2023 • edited Loading

github-actions bot commented Aug 9, 2023 • edited Loading

Powered by Code Review GPT

gozineb Aug 9, 2023

Choose a reason for hiding this comment

gozineb Aug 9, 2023

Choose a reason for hiding this comment

gozineb Aug 9, 2023

Choose a reason for hiding this comment

gozineb Aug 9, 2023

Choose a reason for hiding this comment

gozineb left a comment

Choose a reason for hiding this comment

gozineb Aug 10, 2023

Choose a reason for hiding this comment

gozineb Aug 10, 2023

Choose a reason for hiding this comment

gozineb left a comment

Choose a reason for hiding this comment

gozineb left a comment

Choose a reason for hiding this comment

mamadoudicko commented Aug 9, 2023 •

edited

Loading

vercel bot commented Aug 9, 2023 •

edited

Loading

github-actions bot commented Aug 9, 2023 •

edited

Loading