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(qa): improve code #886

Merged
merged 2 commits into from
Aug 7, 2023
Merged

feat(qa): improve code #886

merged 2 commits into from
Aug 7, 2023

Conversation

StanGirard
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Aug 7, 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 7, 2023 5:54pm
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2023 5:54pm

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

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

The generate_stream function seems to be doing a lot of things. Consider breaking it down into smaller functions to improve readability and maintainability. For example, the code for creating the answering_llm and doc_chain could be moved to a separate function. Also, the error handling in the wrap_done function could be improved. Instead of logging the error and continuing, it might be better to re-raise the exception after logging it, so that the error can be handled by the caller of generate_stream.

Example:

try:
    await fn
except Exception as e:
    logger.error(f\"Caught exception: {e}\")
    raise
finally:
    event.set()

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

The openai_api_key and user_openai_api_key are being stored as plain text which is a security risk. Consider using a secure method to store and retrieve sensitive data like API keys. For example, you could use environment variables or a secure vault service.


📚🔐🔧


Powered by Code Review GPT

@StanGirard StanGirard temporarily deployed to preview August 7, 2023 17:52 — with GitHub Actions Inactive
@StanGirard StanGirard merged commit 7028505 into main Aug 7, 2023
3 of 6 checks passed
StanGirard added a commit that referenced this pull request Sep 12, 2023
* feat(qa): improve code

* feat: 🎸 customprompt

now in system
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.

None yet

1 participant