add sql database tool #55
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds SQL database tool functionality to the template_langgraph project, enabling the agent to interact with SQL databases. The changes introduce a configurable SQL database tool with SQLite as the default backend and integrate it into the existing tools framework.
- Implements a new SQL database tool with configurable connection settings
- Integrates the SQL tool into the default tools list for the chat agent
- Enhances the chat interface to maintain conversation history across interactions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| template_langgraph/tools/sql_database_tool.py | Creates the SQL database tool with settings management and toolkit wrapper |
| template_langgraph/tools/common.py | Adds SQL database tools to the default tools list |
| template_langgraph/services/streamlits/pages/chat_with_tools_agent.py | Implements chat history persistence in the Streamlit interface |
| .env.template | Adds SQL database URI configuration template |
| DEFAULT_TOOLS = [ | ||
| search_cosmosdb, | ||
| run_dify_workflow, | ||
| search_qdrant, | ||
| search_elasticsearch, | ||
| ] | ||
| ] + SqlDatabaseClientWrapper().get_tools( | ||
| llm=AzureOpenAiWrapper().chat_model, | ||
| ) |
There was a problem hiding this comment.
Creating SqlDatabaseClientWrapper and AzureOpenAiWrapper instances at module import time can cause initialization delays and potential connection issues. Consider lazy initialization or moving this to a function that's called when needed.
Copilot uses AI. Check for mistakes.
| class SqlDatabaseClientWrapper: | ||
| def __init__( | ||
| self, | ||
| settings: Settings = None, |
There was a problem hiding this comment.
[nitpick] Using mutable default arguments with None and then reassigning is a common pattern, but using 'settings: Settings | None = None' would be more explicit about the type annotation in modern Python.
| settings: Settings = None, | |
| settings: Settings | None = None, |
Copilot uses AI. Check for mistakes.
| if isinstance(msg, dict): | ||
| st.chat_message(msg["role"]).write(msg["content"]) | ||
| else: | ||
| st.chat_message("assistant").write(msg.content) |
There was a problem hiding this comment.
The mixed message format handling (dict vs object with .content attribute) suggests inconsistent data structures. Consider standardizing the message format or adding type hints to clarify the expected structure.
| if isinstance(msg, dict): | |
| st.chat_message(msg["role"]).write(msg["content"]) | |
| else: | |
| st.chat_message("assistant").write(msg.content) | |
| st.chat_message(msg["role"]).write(msg["content"]) |
Copilot uses AI. Check for mistakes.
What this PR does / why we need it
Which issue(s) this PR fixes:
Fixes #48
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
What to Check
Verify that the following are valid
Additional Information