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.

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

feat(Unplug): chatting without brain streaming #970

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

StepanLebedevTheodo
Copy link
Contributor

Description

Adds the possibility to chat with Quivr without selecting some brain

@vercel
Copy link

vercel bot commented Aug 17, 2023

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 18, 2023 7:53am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 18, 2023 7:53am

@StepanLebedevTheodo StepanLebedevTheodo force-pushed the feat/chatting-without-brain-streaming branch from eb7b984 to cc8b086 Compare August 17, 2023 15:37
@StepanLebedevTheodo StepanLebedevTheodo temporarily deployed to preview August 17, 2023 15:37 — with GitHub Actions Inactive
@StepanLebedevTheodo StepanLebedevTheodo changed the title feat: chatting without brain streaming feat(Unplug): chatting without brain streaming Aug 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 17, 2023

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

  1. The delete_chat_from_db function is catching all exceptions and simply printing them. This is not a good practice as it can make debugging difficult. Instead, consider logging the exceptions with more context or re-throwing them after logging.

  2. The check_user_limit function has an else: pass statement which is unnecessary and can be removed for cleaner code.

  3. The create_question_handler function has a large amount of code within a try block. Consider reducing the amount of code within try blocks to only include code that might throw an exception.

  4. The create_question_handler function is using the print function. Consider using a logger for any debugging or informational messages, which can be configured to write to different outputs.

Example changes:

import logging
logger = logging.getLogger(__name__)

# In delete_chat_from_db
except Exception as e:
    logger.error(f'Error deleting chat history: {e}')
    raise

# In check_user_limit
if int(user.requests_count) >= int(max_requests_number):
    raise HTTPException(
        status_code=429,
        detail=\"You have reached the maximum number of requests for today.\"
    )

# In create_question_handler
try:
    check_user_limit(current_user)
    LLMSettings()
except HTTPException as e:
    raise e

# Replace print with logger
logger.info('streaming')

Risk Level 5 - /home/runner/work/quivr/quivr/backend/core/llm/qa_headless.py

The openai_api_key and user_openai_api_key are being stored as plain text which is a high security risk. It's recommended to use environment variables or a secure vault service to store sensitive information. For example:

import os
self.openai_api_key = os.getenv('OPENAI_API_KEY')

Also, the generate_answer and generate_stream methods are quite long and complex. Consider breaking them down into smaller, more manageable functions.


🔒🐛🧹


Powered by Code Review GPT

Copy link
Contributor

@gozineb gozineb left a comment

Choose a reason for hiding this comment

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

Remove the comments

@StepanLebedevTheodo
Copy link
Contributor Author

Remove the comments
Done

@gozineb gozineb merged commit 600ff1e into main Aug 18, 2023
from typing import AsyncIterable, Awaitable, List

logger = get_logger(__name__)
SYSTEM_MESSAGE = "Your name is Quivr. You're a helpful assistant. If you don't know the answer, just say that you don't know, don't try to make up an answer."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace with only -> "Your name is Quivr. You're a helpful assistant."

StanGirard pushed a commit that referenced this pull request Sep 12, 2023
* feat(Unplug): Adds new basic headless llm

* feat(Unplug): adds chatting without brain option when no streaming

* feat(Unplug): adds chatting without brain option when streaming
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.

3 participants