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(chat): added streaming #808

Merged
merged 2 commits into from
Jul 31, 2023
Merged

feat(chat): added streaming #808

merged 2 commits into from
Jul 31, 2023

Conversation

StanGirard
Copy link
Collaborator

Implement streaming for QA retrieval

@StanGirard StanGirard temporarily deployed to preview July 31, 2023 16:27 — with GitHub Actions Inactive
@vercel
Copy link

vercel bot commented Jul 31, 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 Jul 31, 2023 7:36pm
quivrapp ❌ Failed (Inspect) Jul 31, 2023 7:36pm

@github-actions
Copy link
Contributor

github-actions bot commented Jul 31, 2023

LOGAF Level 2 - /home/runner/work/quivr/quivr/backend/core/vectorstore/supabase.py

  1. The similarity_search function has a hardcoded table name match_vectors. Consider passing this as a parameter to make the function more flexible.
  2. The similarity_search function does not handle the case where res.data is None. This could lead to a TypeError when trying to iterate over None. Consider adding a check for this.

Example:

if res.data is None:
    return []

LOGAF Level 3 - /home/runner/work/quivr/quivr/frontend/app/chat/[chatId]/hooks/useChat.ts

  1. The addQuestion function has a large amount of responsibility. Consider breaking it down into smaller, more manageable functions.
  2. The addQuestion function uses a console.error statement. It's better to use a logging library or a global error handler.
  3. The addQuestion function has a try/catch/finally block that sets setGeneratingAnswer to false in the finally block. If an error is thrown before setGeneratingAnswer(true), it will still be set to false. Consider moving setGeneratingAnswer(true) outside of the try block.

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

  1. Avoid using print statements for error logging. Use a logging library instead. This will give you more control over the output format, destination, and severity level. For example, replace print(e) with logger.error(e).
  2. The delete_chat_from_db function silently passes exceptions. This could make debugging difficult because it hides errors. Consider re-raising the exception or at least logging it.
  3. The check_user_limit function uses environment variables directly. It's better to load these into configuration variables at the start of your program. This makes it easier to manage and validate your configuration.
  4. The get_chat_details and fetch_user_stats functions have similar code for querying the database. Consider creating a helper function to avoid code duplication.

Example:

def query_db(commons, table, filters):
    return (
        commons["supabase"]
        .from_(table)
        .select("*")
        .filter(filters)
        .execute()
    ).data

Then you can replace the similar code in get_chat_details and fetch_user_stats with a call to query_db.


📚🔧🔍


Powered by Code Review GPT

@StanGirard StanGirard temporarily deployed to preview July 31, 2023 19:31 — with GitHub Actions Inactive
@StanGirard StanGirard changed the title feat(tmp): added streaming feat(chat): added streaming Jul 31, 2023
@StanGirard StanGirard marked this pull request as ready for review July 31, 2023 19:32
@StanGirard StanGirard temporarily deployed to preview July 31, 2023 19:33 — with GitHub Actions Inactive
@StanGirard StanGirard merged commit 3166d08 into main Jul 31, 2023
StanGirard added a commit that referenced this pull request Sep 12, 2023
* feat(tmp): added streaming

* feat(streaming): implemented by changing order
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.

1 participant