refactor(backend): cleaning dead and unused code

[StanGirard]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Checklist before requesting a review

Please delete options that are not relevant.

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented hard-to-understand areas
• I have ideally added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged

Screenshots (if appropriate):

[vercel]

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	Oct 19, 2023 1:00am
quivr-strapi	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 19, 2023 1:00am
quivrapp	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 19, 2023 1:00am

[github-actions]

Risk Level 2 - /home/runner/work/quivr/quivr/backend/routes/brain_routes.py

The code changes are generally good, improving readability by adding docstrings to the functions. However, there are a few potential issues:

1. Error handling: In the retrieve_brain_by_id and update_existing_brain functions, an HTTPException is raised if the brain details are not found. However, it would be better to check if the brain_id is valid before trying to retrieve the brain details. This would prevent unnecessary database queries for invalid IDs.

   if not is_valid_uuid(brain_id):
       raise HTTPException(status_code=400, detail=\"Invalid brain ID\")
   brain_details = get_brain_details(brain_id)

2. Performance: In the create_new_brain function, the get_user_brains function is called to get all the user's brains, but only the length of the result is used. If the function is expensive, it might be better to create a new function that only counts the number of brains.

   num_user_brains = count_user_brains(current_user.id)
   if num_user_brains >= user_settings.get(\"max_brains\", 5):
       # ...

3. Security: The openai_api_key is included in the UserUsage object in the create_new_brain function. If this object is logged or serialized somewhere, it could expose sensitive information. Make sure to handle this data carefully.

---

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

1. The code has a lot of repeated logic, especially in the create_question_handler and create_stream_question_handler functions. This could be refactored into a separate function to improve readability and maintainability. For example, the logic for retrieving the user's OpenAI API key and the chat model could be moved into separate functions.

def get_openai_api_key(current_user, brain_id):
    # Retrieve user's OpenAI API key
    current_user.openai_api_key = request.headers.get(\"Openai-Api-Key\")
    if not current_user.openai_api_key and brain_id:
        brain_details = get_brain_details(brain_id)
        if brain_details:
            current_user.openai_api_key = brain_details.openai_api_key
    if not current_user.openai_api_key:
        user_identity = get_user_identity(current_user.id)
        if user_identity is not None:
            current_user.openai_api_key = user_identity.openai_api_key
    return current_user.openai_api_key

def get_chat_model(chat_question, brain):
    # Retrieve chat model (temperature, max_tokens, model)
    if not chat_question.model or not chat_question.temperature or not chat_question.max_tokens:
        chat_question.model = chat_question.model or brain.model or \"gpt-3.5-turbo\"
        chat_question.temperature = chat_question.temperature or brain.temperature or 0.1
        chat_question.max_tokens = chat_question.max_tokens or brain.max_tokens or 256
    return chat_question

2. The print statements in the code should be replaced with proper logging. This will provide more control over the output and make it easier to manage and filter the logs.

import logging
logger = logging.getLogger(__name__)

# Replace print statements with logger.info or logger.debug
logger.info(\"streaming\")

3. The delete_chat_from_db function catches and ignores all exceptions. This could hide potential issues with the code. It would be better to log the exceptions, even if they are not re-raised.

def delete_chat_from_db(supabase_db: SupabaseDB, chat_id):
    try:
        supabase_db.delete_chat_history(chat_id)
    except Exception as e:
        logger.error(e)
    try:
        supabase_db.delete_chat(chat_id)
    except Exception as e:
        logger.error(e)

---

Risk Level 3 - /home/runner/work/quivr/quivr/backend/llm/qa_base.py

The user_openai_api_key and openai_api_key are being used in the code. If these are API keys, they should not be hardcoded in the code for security reasons. Instead, they should be stored in a secure environment variable or a secure vault. Also, the QABaseBrainPicking class is quite large and does a lot of things. Consider breaking it down into smaller classes or functions to improve readability and maintainability. For example, the _determine_api_key and _determine_streaming methods could be moved to a separate utility class or module.

---

🔒🐌🔍

---

Powered by Code Review GPT