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.

Sign up for GitHub

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

Jump to bottom

[PARKED] Create /chat endpoint #412

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

[PARKED] Create /chat endpoint #412

wants to merge 10 commits into from

Conversation

suzinyou
Copy link
Collaborator

Reviewer:
Estimate:

Ticket

Fixes: -

Description

Creates a /chat endpoint

Goal

Changes

Future Tasks (optional)

How has this been tested?

To-do before merge (optional)

  1. Unit tests
  2. We'll probably want to create different request schema for /search and /chat. For functions relying on QueryBase, and QueryResponse etc. we can create a union type QueryBase = SearchQueryBase | ChatQueryBase or something? Need to think.
  3. Using session_id's null-ness to distinguish /searhc vs. /chat is too messy.

Checklist

Fill with x for completed.

  • My code follows the style guidelines of this project
  • I have reviewed my own code to ensure good quality
  • I have tested the functionality of my code to ensure it works as intended
  • I have resolved merge conflicts

(Delete any items below that are not relevant)

  • I have updated the automated tests
  • I have updated the scripts in scripts/
  • I have updated the requirements
  • I have updated the README file
  • I have updated affected documentation
  • I have added a blogpost in Latest Updates
  • I have updated the CI/CD scripts in .github/workflows/
  • I have updated the Terraform code

suzinyou
suzinyou commented Aug 26, 2024
View reviewed changes
core_backend/app/llm_call/llm_rag.py
@@ -13,23 +13,28 @@


async def get_llm_rag_answer(
question: str,
question: str | list[dict[str, str]],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To undo

suzinyou
suzinyou commented Aug 26, 2024
View reviewed changes
core_backend/app/llm_call/llm_rag.py
Comment on lines +27 to +28
The question to ask the LLM model, or list of chat history messages in the form
of {"content": str, "role": str}.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To undo

suzinyou
suzinyou commented Aug 26, 2024
View reviewed changes
core_backend/app/llm_call/utils.py
@@ -7,8 +7,9 @@


async def _ask_llm_async(
user_message: str,
user_message: str | list[dict[str, str]],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to undo

suzinyou
suzinyou commented Aug 26, 2024
View reviewed changes
core_backend/app/question_answer/models.py
Comment on lines +264 to +266
async def check_user_and_session_id(
user_id: int, session_id: str, asession: AsyncSession
) -> bool:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not used anywhere yet. Should call this and raise appropriate error

@suzinyou suzinyou force-pushed the refactor-search-base branch from 294b796 to 8726dee Compare August 27, 2024 12:09
Base automatically changed from refactor-search-base to main August 27, 2024 12:17
@amiraliemami amiraliemami changed the title Create /chat endpoint [PARKED] Create /chat endpoint Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sidravi1 sidravi1 Awaiting requested review from sidravi1 sidravi1 will be requested when the pull request is marked ready for review sidravi1 is a code owner

@lickem22 lickem22 Awaiting requested review from lickem22 lickem22 will be requested when the pull request is marked ready for review lickem22 is a code owner

@amiraliemami amiraliemami Awaiting requested review from amiraliemami amiraliemami will be requested when the pull request is marked ready for review amiraliemami is a code owner

@markbotterill markbotterill Awaiting requested review from markbotterill markbotterill will be requested when the pull request is marked ready for review markbotterill is a code owner

@tonyzhao6 tonyzhao6 Awaiting requested review from tonyzhao6 tonyzhao6 will be requested when the pull request is marked ready for review tonyzhao6 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@suzinyou