-
Notifications
You must be signed in to change notification settings - Fork 148
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
Adding chat history to RAG app and refactor to better utilize LangChain #648
base: main
Are you sure you want to change the base?
Conversation
…eep track of and retrieve chat history from Cloud SQL. main.py - removed old langchain and logic to retrieve context. replaced with new chain from rag_chain.py. Introduced browser session with 30 minute ttl. Storing session ID in the session cookie. Session ID is then used to retrieve chat history. Chat history is cleared when timeout is reached. cloud_sql.py - now includes a method to create a PostgresEngine for storing and retrieving history, plus a CustomVectorStore to perform the query embedding and vector search. Old code paths no longer needed were removed. rag_chain.py - contains helper method create_chain to create, update and delete the end-to-end RAG chain with history. various tf files: increased max input and total tokens on HF TGI for mistral. threadded through some parameters needed to instantiate the PostgresEngine. requirements.txt - added some dependencies needed for langchain
/gcbrun |
Reverted breaking change to env var
/gcbrun |
1 similar comment
/gcbrun |
* Working on improvements for rag application: - Working on missing TODO - Fixing issue with credentials - Refactoring vector_storages so you can add different vector storages TODO: Vector Storage factory - Unit test will be added on future PR * Updating changes with db * refactoring app so can be executed using gunicorn * refactory of the code as flask application package * Fixing Bugs - Reviewing issue with IPtypes, currently the fix is to validate if there's an development environment so a public cloud_sql instance can be use. - Fixing issue with Flask App Factory
/gcbrun |
* Working on improvements for rag application: - Working on missing TODO - Fixing issue with credentials - Refactoring vector_storages so you can add different vector storages TODO: Vector Storage factory - Unit test will be added on future PR * Updating changes with db * refactoring app so can be executed using gunicorn * refactory of the code as flask application package * Fixing Bugs - Reviewing issue with IPtypes, currently the fix is to validate if there's an development environment so a public cloud_sql instance can be use. - Fixing issue with Flask App Factory * Working on Custom HuggingFace interface - Adding a custom chat model to send request to HuggingFace TGI API - Applying formatting to code.
* Working on improvements for rag application: - Working on missing TODO - Fixing issue with credentials - Refactoring vector_storages so you can add different vector storages TODO: Vector Storage factory - Unit test will be added on future PR * Updating changes with db * refactoring app so can be executed using gunicorn * refactory of the code as flask application package * Fixing Bugs - Reviewing issue with IPtypes, currently the fix is to validate if there's an development environment so a public cloud_sql instance can be use. - Fixing issue with Flask App Factory * Working on Custom HuggingFace interface - Adding a custom chat model to send request to HuggingFace TGI API - Applying formatting to code. * Improving the CloudSQL vector vector_storage
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
applications/rag/frontend/container/application/models/vector_embeddings.py
Outdated
Show resolved
Hide resolved
level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s" | ||
) | ||
|
||
ENVIRONMENT = os.environ.get("ENVIRONMENT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an environment that flask set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gongmax it's just for local development purposes, the variable was added because an issue on line 66.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean where is the ENVIRONMENT
env get set?
applications/rag/frontend/container/application/rag_langchain/rag_chain.py
Outdated
Show resolved
Hide resolved
applications/rag/frontend/container/application/rag_langchain/rag_chain.py
Outdated
Show resolved
Hide resolved
applications/rag/frontend/container/application/rag_langchain/rag_chain.py
Outdated
Show resolved
Hide resolved
applications/rag/frontend/container/application/vector_storages/cloud_sql.py
Show resolved
Hide resolved
) | ||
|
||
chain = setup_and_retrieval | prompt | model | ||
chain_with_history = RunnableWithMessageHistory( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some comments around how the chain.invoke
works with this chain_with_history
and user input? Especially around how setup_and_retrieval
component works.
applications/rag/frontend/container/application/models/vector_embeddings.py
Outdated
Show resolved
Hide resolved
Please also resolve the conflicts |
first commit
…tor (#769) * Refactor: create module for workload identity service account Change-Id: I29e985e77a1ff2d5f4a8d9493c1e65907c89c100 * fix: add todo Change-Id: I3357e8f9dd16c7958dff0f0cf0f990fce980f474 --------- Co-authored-by: Gen Lu <genlu@google.com>
/gcbrun |
level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s" | ||
) | ||
|
||
ENVIRONMENT = os.environ.get("ENVIRONMENT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean where is the ENVIRONMENT
env get set?
@@ -269,7 +269,7 @@ steps: | |||
kubectl exec -it -n rag-$SHORT_SHA-$_BUILD_ID jupyter-admin -c notebook -- jupyter nbconvert --to script /data/rag-kaggle-ray-sql-interactive.ipynb | |||
kubectl exec -it -n rag-$SHORT_SHA-$_BUILD_ID jupyter-admin -c notebook -- ipython /data/rag-kaggle-ray-sql-interactive.py | |||
|
|||
python3 ./applications/rag/tests/test_rag.py "http://127.0.0.1:8081/prompt" | |||
# python3 ./applications/rag/tests/test_rag.py "http://127.0.0.1:8081/prompt" Ignoring while the test approach is reviewed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add this back to ensure the e2e test pass?
@@ -23,6 +23,14 @@ locals { | |||
}) | |||
} | |||
|
|||
resource "random_string" "application_secret_key" { | |||
length = var.project_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
length should be a number, and CI complains on this line:
Error: Incorrect attribute value type
on frontend/main.tf line 27, in resource "random_string" "application_secret_key":
27: length = var.project_id
├────────────────
│ var.project_id is "gke-ai-eco-dev"
Inappropriate value for attribute "length": a number is required.
See commit log for full description. tl;dr: added chat history to rag-frontend app.