RAG & Text-to-SQL Agent (100% Local) #77
Conversation
WalkthroughThis pull request introduces several new files and functionalities into the project. A comprehensive Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as Streamlit App
participant DB as SQLite DB
participant VL as City Data Loader
participant LLM as LLM (Ollama)
participant WS as Workflow Handler
U->>A: Enter query in chat interface
A->>DB: Initialize in-memory SQL database
A->>VL: Load city data and initialize vector store
A->>LLM: Process query via language model
alt Query Requires SQL
A->>DB: Execute SQL query for stats
else Query Requires Semantic Search
A->>WS: Trigger semantic search workflow
WS->>LLM: Execute asynchronous workflow steps
end
A->>U: Display aggregated response
sequenceDiagram
participant U as User
participant RO as RouterOutputAgentWorkflow
participant LLM as LLM Service
participant T as Tool
U->>RO: Submit query
RO->>RO: Prepare chat (add user message to history)
RO->>LLM: Determine necessary tool calls
alt Tools Required
RO->>T: Dispatch tool calls in parallel
T-->>RO: Return tool outputs
RO->>RO: Gather and aggregate results
else No Tools Needed
RO->>LLM: Generate direct answer from conversation
end
RO-->>U: Return final response
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
nikhil-1e9
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
rag-sql-agent/.gitignore (1)
82-85: Distribution / Packaging Artifacts
The ignore patterns for distribution files (*.tar.gz,*.tgz,*.zip) are well specified. As a minor nitpick, you might also consider ignoring wheel files (e.g.,*.whl) if the project will generate them, to further ensure that only source files are tracked.rag-sql-agent/README.md (5)
22-33: Add a language spec to the directory structure code block.Using a language spec helps in syntax highlighting and improves readability for developers browsing your Markdown.
Apply this diff to specify a language for the fenced code block:
-``` +```bash . ├── assets/ ├── docs/ ...🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
22-22: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
44-45: Streamline wording to avoid repeating “Ollama.”Currently, “### Setup Ollama Ollama is required…” is slightly repetitive. Consider simplifying to avoid duplication.
For example:
-### Setup Ollama -Ollama is required to run the LLM locally. +### Setup Ollama +Ollama is required to run the LLM locally.🧰 Tools
🪛 LanguageTool
[duplication] ~44-~44: Possible typo: you repeated a word.
Context: ...:z \ qdrant/qdrant ``` ### Setup Ollama Ollama is required to run the LLM locally. In...(ENGLISH_WORD_REPEAT_RULE)
94-94: Improve grammatical correctness in the heading.Rephrase “How It Works?” to a more standard form in formal documentation.
-## How It Works? +## How It Works🧰 Tools
🪛 LanguageTool
[grammar] ~94-~94: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...(WRB_NN_VB_ORDER)
102-102: Hyphenate “statistics-based” for clarity.This is a common expression that typically includes a hyphen.
- - Statistics based questions (e.g., population, state data) + - Statistics-based questions (e.g., population, state data)🧰 Tools
🪛 LanguageTool
[uncategorized] ~102-~102: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...(BASED_HYPHEN)
133-133: Add missing article “the.”This sentence flows better with “the” before “following cities.”
-This project currently has support for following cities: +This project currently has support for the following cities:🧰 Tools
🪛 LanguageTool
[uncategorized] ~133-~133: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
rag-sql-agent/agent_workflow.html (2)
1-2: Consider adding a DOCTYPE for HTML5 compatibility.A DOCTYPE declaration at the top (e.g.,
<!DOCTYPE html>) ensures consistent rendering across browsers.
10-12: Avoid using the<center>tag.
<center>is deprecated in HTML5. Use CSS properties (e.g.,text-align: center;) on a container element instead.-<center> -<h1></h1> -</center> +<div style="text-align: center;"> + <h1></h1> +</div>rag-sql-agent/workflow.py (1)
1-1: Remove unused imports to keep the codebase clean.
os,Response, andFunctionToolare imported but never used.-import os from typing import Dict, List, Any, Optional from llama_index.core.base.response.schema import Response from llama_index.core.tools import FunctionToolAlso applies to: 15-16
🧰 Tools
🪛 Ruff (0.8.2)
1-1:
osimported but unusedRemove unused import:
os(F401)
rag-sql-agent/app.py (4)
72-114: Consider adding a primary key to the 'city_stats' table.
Currently, the table lacks a primary key column, which might be beneficial for data integrity and performance in certain queries.
145-192: Remove unusedsql_contextvariable and extraneous f-strings.[Line 149] The variable
sql_contextis assigned but never used.
[Lines 172-173, 179]: The f-string declarations lack placeholders, leading to extraneousf.Apply this diff:
- sql_context = """The table 'city_stats' contains: - - city_name (string) - - population (integer) - - state (string) - Use EXACT values from the city_stats table for numerical queries.""" ... - description=f"""Useful for translating a natural language query into a SQL query over - a table containing: city_stats, containing the population/state of cities""", ... - description=f"Useful for answering semantic questions about certain cities in the USA and India.", + # Removed sql_context assignment + + description="""Useful for translating a natural language query into a SQL query over + a table containing: city_stats, containing the population/state of cities""", ... + description="Useful for answering semantic questions about certain cities in the USA and India.",🧰 Tools
🪛 Ruff (0.8.2)
149-149: Local variable
sql_contextis assigned to but never usedRemove assignment to unused variable
sql_context(F841)
172-173: f-string without any placeholders
Remove extraneous
fprefix(F541)
179-179: f-string without any placeholders
Remove extraneous
fprefix(F541)
269-271: Use a context manager when reading image files for base64 encoding.Apply this diff:
- base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), - base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), - base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() + with open("./assets/llamaindex.jpeg", "rb") as f1: + llama_data = base64.b64encode(f1.read()).decode() + with open("./assets/qdrant.png", "rb") as f2: + qdrant_data = base64.b64encode(f2.read()).decode() + with open("./assets/ollama.png", "rb") as f3: + ollama_data = base64.b64encode(f3.read()).decode() + + ... + .format( + llama_data, + qdrant_data, + ollama_data + )🧰 Tools
🪛 Ruff (0.8.2)
269-269: Use a context manager for opening files
(SIM115)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
1-331: Add automated tests for the RAG & Text-to-SQL agent logic.
Comprehensive tests ensure new features work as intended and provide confidence against regressions.Would you like me to generate a basic test suite or open a new issue to track this requirement?
🧰 Tools
🪛 Ruff (0.8.2)
149-149: Local variable
sql_contextis assigned to but never usedRemove assignment to unused variable
sql_context(F841)
172-173: f-string without any placeholders
Remove extraneous
fprefix(F541)
179-179: f-string without any placeholders
Remove extraneous
fprefix(F541)
269-269: Use a context manager for opening files
(SIM115)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
rag-sql-agent/assets/llamaindex.jpegis excluded by!**/*.jpegrag-sql-agent/assets/ollama.pngis excluded by!**/*.pngrag-sql-agent/assets/qdrant.pngis excluded by!**/*.pngrag-sql-agent/demo.gifis excluded by!**/*.gifrag-sql-agent/docs/ahmedabad.pdfis excluded by!**/*.pdfrag-sql-agent/docs/bengaluru.pdfis excluded by!**/*.pdfrag-sql-agent/docs/chicago.pdfis excluded by!**/*.pdfrag-sql-agent/docs/delhi.pdfis excluded by!**/*.pdfrag-sql-agent/docs/houston.pdfis excluded by!**/*.pdfrag-sql-agent/docs/hyderabad.pdfis excluded by!**/*.pdfrag-sql-agent/docs/los-angeles.pdfis excluded by!**/*.pdfrag-sql-agent/docs/miami.pdfis excluded by!**/*.pdfrag-sql-agent/docs/mumbai.pdfis excluded by!**/*.pdfrag-sql-agent/docs/new-york-city.pdfis excluded by!**/*.pdfrag-sql-agent/docs/seattle.pdfis excluded by!**/*.pdf
📒 Files selected for processing (5)
rag-sql-agent/.gitignore(1 hunks)rag-sql-agent/README.md(1 hunks)rag-sql-agent/agent_workflow.html(1 hunks)rag-sql-agent/app.py(1 hunks)rag-sql-agent/workflow.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
rag-sql-agent/README.md
[duplication] ~44-~44: Possible typo: you repeated a word.
Context: ...:z \ qdrant/qdrant ``` ### Setup Ollama Ollama is required to run the LLM locally. In...
(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~94-~94: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...
(WRB_NN_VB_ORDER)
[uncategorized] ~102-~102: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...
(BASED_HYPHEN)
[uncategorized] ~133-~133: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
rag-sql-agent/README.md
22-22: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
46-46: Bare URL used
null
(MD034, no-bare-urls)
🪛 Ruff (0.8.2)
rag-sql-agent/app.py
149-149: Local variable sql_context is assigned to but never used
Remove assignment to unused variable sql_context
(F841)
172-173: f-string without any placeholders
Remove extraneous f prefix
(F541)
179-179: f-string without any placeholders
Remove extraneous f prefix
(F541)
269-269: Use a context manager for opening files
(SIM115)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
rag-sql-agent/workflow.py
1-1: os imported but unused
Remove unused import: os
(F401)
15-15: llama_index.core.base.response.schema.Response imported but unused
Remove unused import: llama_index.core.base.response.schema.Response
(F401)
16-16: llama_index.core.tools.FunctionTool imported but unused
Remove unused import: llama_index.core.tools.FunctionTool
(F401)
🔇 Additional comments (16)
rag-sql-agent/.gitignore (9)
1-5: Environment Variables Section Intact
The environment variable patterns (.env,.env.*) along with the exception for!.env.exampleare well defined. This setup safeguards sensitive configuration files while still providing a template for others.
6-19: Comprehensive macOS Files Ignores
The entries for macOS system files (e.g.,.DS_Store,.AppleDouble,.LSOverride, etc.) are thorough. This prevents OS-specific clutter from being committed.
20-41: Robust Python Artifacts Exclusion
The Python section covers caches, compiled files (*.py[cod],*$py.class), shared objects (*.so), and build/distribution directories (such asbuild/,dist/,eggs/, etc.). This comprehensive list should reliably keep transient and build artifacts out of version control.
42-50: Virtual Environments Ignoring
All common virtual environment directories (venv/,env/,ENV/,.venv/,.env/,.env3/, and.pyenv/) are appropriately excluded. If conventions evolve regarding naming conflicts between environment variables and virtual environments (e.g., use of.env), be sure to update accordingly.
51-53: Jupyter Notebook Checkpoints Exclusion
The inclusion of.ipynb_checkpointsensures that notebook auto-save files are not tracked, which is ideal for maintaining a clean history.
54-62: IDE-specific Files Exclusion
Entries for IDE-specific directories and temporary files (like.idea/,.vscode/, and swap files such as*.swp,*.swo) are well covered. This prevents local user configurations and temporary artifacts from entering the repository.
63-67: Logs and Database Files
Ignoring log files (*.log) as well as common database files (*.sqlite,*.db) is a good practice to avoid committing volatile data.
68-78: Coverage and Test Artifacts Exclusion
The rules for ignoring coverage reports (e.g.,.coverage,.coverage.*,htmlcov/), test caches (.cache), and testing tool outputs (nosetests.xml,coverage.xml, etc.) are comprehensive. This helps keep testing artifacts out of the source control.
79-81: Local Development Configuration
The.local/directory is correctly ignored, ensuring that local configuration files or developer-specific settings remain untracked.rag-sql-agent/workflow.py (1)
37-158: Implementation looks solid.No logic, performance, or security issues were found. The asynchronous flow, event definitions, and usage of parallel tool calls appear consistent and well-structured.
rag-sql-agent/app.py (6)
1-2: No issues with nest_asyncio import.
32-46: Session state initialization looks coherent.
No issues found.
47-64: The LLM loading logic looks good.
Implementation appears correct, and the exception handling is properly used.
65-70: Chat reset logic is straightforward and effective.
116-144: Document loading and vector store initialization look good.
193-215:❓ Verification inconclusive
Verify parameterized queries for user input to prevent SQL injection.
LLM-generated queries might allow malicious or unintended SQL commands if not properly sanitized.
🏁 Script executed:
#!/bin/bash # Description: Inspect usage of parameterized queries in the generated SQL by `NLSQLTableQueryEngine`. rg -A 10 -B 10 "NLSQLTableQueryEngine"Length of output: 2444
SQL Injection Prevention – Verify Parameterized Queries
After reviewing the initialization of the SQL query engine (viaNLSQLTableQueryEngine) inrag-sql-agent/app.py, there's no direct evidence in our code that raw SQL concatenation is taking place. However, because user queries are passed to an LLM-powered workflow, it's essential to ensure that any SQL generated by the process is executed using parameterized queries. Please confirm that:
- The underlying implementation of
NLSQLTableQueryEngineproperly sanitizes inputs and uses bind parameters rather than concatenating user input into SQL strings.- User-supplied data is either validated or explicitly parameterized before being incorporated into any SQL executed against the database.
Given these considerations, please verify that the external query engine and its interactions in our application meet these security requirements.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
rag-sql-agent/README.md (6)
1-3: Clarify Introduction Sentence
Consider adding a comma after “semantic search” in line 3 to improve readability. For example, change the sentence to:"This project builds an agentic workflow combining SQL querying with semantic search, allowing users to ask questions about cities in natural language."
🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...ow combining SQL querying with semantic search allowing users to ask questions about c...(AI_HYDRA_LEO_MISSING_COMMA)
23-35: Specify Language for Directory Structure Code Block
The fenced code block displaying the directory structure does not specify a language. Although it is a directory listing, adding a language tag (for example, usingtext) can help with proper syntax highlighting and consistency with markdown lint recommendations.-``` +```text🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
24-24: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
47-49: Format URL in Setup Ollama Section
In line 49, the URL is presented as a bare URL. To improve readability and adhere to markdown conventions, consider formatting it as a markdown link, for example:[Ollama](https://ollama.com).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
48-48: Bare URL used
null(MD034, no-bare-urls)
96-100: Refine Section Header "How It Works?"
The header "How It Works?" in line 96 might be interpreted as a question; consider rephrasing it for clarity. For example, if intended as a question, use "How Does It Work?" or if as a title, simply "How It Works" (without the question mark).🧰 Tools
🪛 LanguageTool
[grammar] ~96-~96: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...(WRB_NN_VB_ORDER)
103-105: Hyphenate Compound Adjective
In line 105, “Statistics based questions” should be hyphenated to “Statistics-based questions” for grammatical accuracy.🧰 Tools
🪛 LanguageTool
[uncategorized] ~104-~104: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...(BASED_HYPHEN)
135-137: Enhance Supported Cities Description
In line 136, consider adding the article “the” so that the sentence reads: “This project currently has support for the following cities:” This enhances clarity and grammatical correctness.🧰 Tools
🪛 LanguageTool
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
rag-sql-agent/app.py (6)
1-24: Good job incorporating the required libraries and dependencies.The code has all the necessary imports for the RAG and SQL agent implementation. The use of
nest_asyncio.apply()is appropriate for handling async operations within Streamlit, which runs in a synchronous environment.One suggestion would be to organize imports into logical groups (standard library, third-party, local) following PEP 8 guidelines for better readability.
# Standard library imports import asyncio import base64 import gc import io import time import uuid from contextlib import redirect_stdout from pathlib import Path # Third-party imports import nest_asyncio import qdrant_client import streamlit as st from llama_index.core import Settings, SimpleDirectoryReader, SQLDatabase, StorageContext, VectorStoreIndex from llama_index.core.query_engine import NLSQLTableQueryEngine from llama_index.core.tools import QueryEngineTool from llama_index.embeddings.huggingface import HuggingFaceEmbedding from llama_index.llms.ollama import Ollama from llama_index.vector_stores.qdrant import QdrantVectorStore from sqlalchemy import Column, Integer, MetaData, String, Table, create_engine, insert, inspect from sqlalchemy.pool import StaticPool # Local imports from workflow import RouterOutputAgentWorkflow nest_asyncio.apply()
56-64: Good use of caching for LLM loading.Using
@st.cache_resourcefor LLM initialization is efficient as it prevents reloading the model on each Streamlit rerun. The error handling is also appropriate.Consider adding more specific error handling for common Ollama connection issues.
@st.cache_resource def load_llm(): try: llm = Ollama(model="mistral:7b", system_prompt=system_prompt, request_timeout=120.0) return llm except ConnectionRefusedError: st.error("Connection to Ollama server refused. Please ensure Ollama is running.") st.stop() except TimeoutError: st.error("Connection to Ollama timed out. The model may be taking too long to load.") st.stop() except Exception as e: st.error(f"Ollama Error: {str(e)}") st.stop()
150-160: Unused SQL context and commented out parameter.The
sql_contextvariable is defined but never used. Thecontextparameter in theNLSQLTableQueryEngineis commented out, which might impact the SQL engine's understanding of the schema.- sql_context = """The table 'city_stats' contains: - - city_name (string) - - population (integer) - - state (string) - Use EXACT values from the city_stats table for numerical queries.""" sql_query_engine = NLSQLTableQueryEngine( sql_database=sql_database, tables=["city_stats"], - # context=sql_context, + context="""The table 'city_stats' contains: + - city_name (string) + - population (integer) + - state (string) + Use EXACT values from the city_stats table for numerical queries.""", )🧰 Tools
🪛 Ruff (0.8.2)
150-150: Local variable
sql_contextis assigned to but never usedRemove assignment to unused variable
sql_context(F841)
171-176: Remove unnecessary f-string prefixes.The description strings in both tool definitions have
fprefixes but don't contain any placeholders.sql_tool = QueryEngineTool.from_defaults( name="sql_tool", - description=f"""Useful for translating a natural language query into a SQL query over + description="""Useful for translating a natural language query into a SQL query over a table containing: city_stats, containing the population/state of cities""", query_engine=sql_query_engine, ) semantic_search_tool = QueryEngineTool.from_defaults( name="semantic_search_tool", - description=f"Useful for answering semantic questions about certain cities in the USA and India.", + description="Useful for answering semantic questions about certain cities in the USA and India.", query_engine=search_query_engine, )Also applies to: 178-182
🧰 Tools
🪛 Ruff (0.8.2)
173-174: f-string without any placeholders
Remove extraneous
fprefix(F541)
269-273: Use context managers for file operations.Opening files without context managers can lead to resource leaks. Use
withstatements to ensure files are properly closed.""".format( - base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), - base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), - base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() + base64.b64encode(Path("./assets/llamaindex.jpeg").read_bytes()).decode(), + base64.b64encode(Path("./assets/qdrant.png").read_bytes()).decode(), + base64.b64encode(Path("./assets/ollama.png").read_bytes()).decode() ), unsafe_allow_html=True)🧰 Tools
🪛 Ruff (0.8.2)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
272-272: Use a context manager for opening files
(SIM115)
238-243: Add error handling for missing directory in docs lookup.When trying to list document cities, if the directory doesn't exist, this will throw an error. Consider adding a directory existence check similar to what you did in
initialize_city_data().if city_index: try: + docs_path = Path("./docs") + if not docs_path.exists(): + st.warning("Docs directory not found.") + doc_cities = [] + else: doc_cities = [f.stem.replace('_', ' ').title() for f in Path("./docs").glob("*.pdf")] except Exception as e: st.error(f"Error loading document cities: {str(e)}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rag-sql-agent/README.md(1 hunks)rag-sql-agent/app.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
rag-sql-agent/README.md
[uncategorized] ~3-~3: Possible missing comma found.
Context: ...ow combining SQL querying with semantic search allowing users to ask questions about c...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~46-~46: Possible typo: you repeated a word.
Context: ...:z \ qdrant/qdrant ``` ### Setup Ollama Ollama is required to run the LLM locally. In...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...n macOS brew install ollama After installation pull the Mistral-7B modelbash # pul...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~96-~96: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...
(WRB_NN_VB_ORDER)
[uncategorized] ~104-~104: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...
(BASED_HYPHEN)
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
rag-sql-agent/README.md
24-24: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
48-48: Bare URL used
null
(MD034, no-bare-urls)
🪛 Ruff (0.8.2)
rag-sql-agent/app.py
150-150: Local variable sql_context is assigned to but never used
Remove assignment to unused variable sql_context
(F841)
173-174: f-string without any placeholders
Remove extraneous f prefix
(F541)
180-180: f-string without any placeholders
Remove extraneous f prefix
(F541)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
272-272: Use a context manager for opening files
(SIM115)
🔇 Additional comments (7)
rag-sql-agent/app.py (7)
26-30: LGTM! Good page configuration.The page configuration is appropriately set with a descriptive title and wide layout for better user experience.
32-46: Well-structured session state initialization.The session state variables are properly initialized with clear purpose for each variable:
idfor unique session identificationworkflowfor the agent workflowmessagesfor chat historyworkflow_logsfor logging execution detailsThis pattern ensures state persistence across Streamlit reruns.
47-55: Clear and effective system prompt.The system prompt provides good guardrails for the agent's behavior, ensuring it uses the appropriate tools for different query types and doesn't hallucinate answers.
65-70: Good implementation of chat reset function.The
reset_chat()function properly clears all relevant session state variables and calls garbage collection to free resources. This is important for memory management especially when using large language models.
71-114: Well-structured SQL database initialization with appropriate caching.The SQL database initialization is well-implemented with:
- In-memory SQLite database with proper configuration
- Clear table schema definition
- Sample city data for testing
Using
@st.cache_resourceensures the database is only initialized once per session.
195-216: Well-implemented async workflow execution with proper error handling.The
run_workflowfunction effectively:
- Captures stdout for logging
- Handles exceptions with informative error messages
- Includes database verification in case of errors
- Returns appropriate results
Good use of async/await pattern for non-blocking operations.
131-133:✅ Verification successful
Consider using in-memory Qdrant for consistency with SQL approach.
There's a commented out line for in-memory Qdrant client, but the code currently uses a local Qdrant server. This might cause issues if the server isn't running. For consistency with the in-memory SQL approach and to make the application more self-contained, consider using the in-memory option.
Also, verify that users will have access to the Qdrant server at http://localhost:6333, as this might be a deployment requirement that should be documented.
🏁 Script executed:
#!/bin/bash # Check if Qdrant server is configured to run locally grep -r "qdrant" --include="*.md" . || echo "No documentation found for Qdrant setup"Length of output: 1012
Keep Local Qdrant Setup & Ensure Documentation Clarity
After reviewing the repository’s documentation (e.g., in multiple README files including
rag-sql-agent/README.md), it’s clear that a local Qdrant server (via the Docker imageqdrant/qdrant) is the intended deployment model. While an in-memory client option exists (and remains useful for isolated tests), the project consistently expects users to spin up a local Qdrant server athttp://localhost:6333. Please ensure that the deployment instructions clearly emphasize this requirement. If a self-contained (in-memory) option is ever needed for testing or prototyping, consider exposing it via a configuration flag rather than changing the default behavior.
rag-sql-agent/README.md
Outdated
|
|
||
| - The system retrieves relevant information from city documents stored in the Qdrant vector database suing similarity search. | ||
|
|
||
| - The LLM then synthesizes a response based on the retrieved context. | ||
|
|
There was a problem hiding this comment.
Correct Typo in Semantic Search Section
In line 117, the word “suing” appears instead of “using” in the description of the similarity search. Correcting this typo will clarify the intended meaning.
rag-sql-agent/README.md
Outdated
|
|
||
| - The agent system combines the results and displays the final generated reponse in the chat interface. | ||
|
|
There was a problem hiding this comment.
Fix Typo in Response Generation Description
The phrase “final generated reponse” in line 123 contains a typo. Please change “reponse” to “response” for clarity.
| if st.session_state.workflow: | ||
| with st.spinner("Thinking..."): | ||
| result = asyncio.run(run_workflow(prompt)) | ||
|
|
||
| st.session_state.messages.append({"role": "assistant", "content": result}) | ||
|
|
||
| if log_index < len(st.session_state.workflow_logs): | ||
| with st.expander("View Execution Details", expanded=False): | ||
| st.code(st.session_state.workflow_logs[log_index], language="text") | ||
|
|
||
| # Display assistant response with streaming | ||
| with st.chat_message("assistant"): | ||
| message_placeholder = st.empty() | ||
| full_response = "" | ||
|
|
||
| # Stream response word by word | ||
| for chunk in result.split(): | ||
| full_response += chunk + " " | ||
| time.sleep(0.05) | ||
| message_placeholder.markdown(full_response + "▌") | ||
|
|
||
| # Final display without cursor | ||
| message_placeholder.markdown(full_response) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Response streaming implementation is inefficient.
The current implementation of response streaming:
- First runs the entire workflow to get the complete result
- Then "simulates" streaming by splitting the text and adding a delay
This approach doesn't provide true streaming and unnecessarily delays the response. Consider using a true streaming implementation if the LLM supports it.
if st.session_state.workflow:
with st.spinner("Thinking..."):
result = asyncio.run(run_workflow(prompt))
st.session_state.messages.append({"role": "assistant", "content": result})
if log_index < len(st.session_state.workflow_logs):
with st.expander("View Execution Details", expanded=False):
st.code(st.session_state.workflow_logs[log_index], language="text")
# Display assistant response with streaming
with st.chat_message("assistant"):
message_placeholder = st.empty()
- full_response = ""
-
- # Stream response word by word
- for chunk in result.split():
- full_response += chunk + " "
- time.sleep(0.05)
- message_placeholder.markdown(full_response + "▌")
-
- # Final display without cursor
- message_placeholder.markdown(full_response)
+ # Display the response directly, avoiding artificial delay
+ message_placeholder.markdown(result)If you want to keep the streaming effect for UX reasons, consider making it optional with a config setting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if st.session_state.workflow: | |
| with st.spinner("Thinking..."): | |
| result = asyncio.run(run_workflow(prompt)) | |
| st.session_state.messages.append({"role": "assistant", "content": result}) | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Execution Details", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| # Display assistant response with streaming | |
| with st.chat_message("assistant"): | |
| message_placeholder = st.empty() | |
| full_response = "" | |
| # Stream response word by word | |
| for chunk in result.split(): | |
| full_response += chunk + " " | |
| time.sleep(0.05) | |
| message_placeholder.markdown(full_response + "▌") | |
| # Final display without cursor | |
| message_placeholder.markdown(full_response) | |
| if st.session_state.workflow: | |
| with st.spinner("Thinking..."): | |
| result = asyncio.run(run_workflow(prompt)) | |
| st.session_state.messages.append({"role": "assistant", "content": result}) | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Execution Details", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| # Display assistant response with streaming | |
| with st.chat_message("assistant"): | |
| message_placeholder = st.empty() | |
| # Display the response directly, avoiding artificial delay | |
| message_placeholder.markdown(result) |
rag-sql-agent/app.py
Outdated
| ] | ||
|
|
||
| for city in predefined_cities: | ||
| st.markdown(f"- {city}") | ||
|
|
||
| if city_index: | ||
| try: | ||
| doc_cities = [f.stem.replace('_', ' ').title() for f in Path("./docs").glob("*.pdf")] | ||
| except Exception as e: | ||
| st.error(f"Error loading document cities: {str(e)}") | ||
|
|
||
| st.divider() | ||
| st.subheader("➕ Add More Cities") | ||
| st.markdown(""" | ||
| 1. For numerical data: | ||
| - Contact admin to add to SQL database | ||
| 2. For general info: | ||
| - Add city PDF files to `/docs` (preferably from wikipedia) | ||
| - Format: `City_Name.pdf` | ||
| - Include sections for history, culture, landmarks | ||
| 3. Restart the app | ||
| """) | ||
|
|
||
| # Main chat interface | ||
| col1, col2 = st.columns([6, 1]) | ||
|
|
||
| with col1: | ||
| st.markdown("<h1 style='color: #0066cc;'>🕵️ RAG & Text-to-SQL Agent</h1>", unsafe_allow_html=True) | ||
|
|
||
| st.markdown(""" | ||
| <div style='display: flex; align-items: center; gap: 10px; margin-top: -10px;'> | ||
| <span style='font-size: 28px; color: #666;'>Powered by LlamaIndex | ||
| <img src='data:image/png;base64,{}' width='40'> Qdrant | ||
| <img src='data:image/png;base64,{}' width='40'> Ollama | ||
| <img src='data:image/png;base64,{}' width='40'></span> | ||
| </div> | ||
| """.format( | ||
| base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), | ||
| base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), | ||
| base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() | ||
| ), unsafe_allow_html=True) | ||
|
|
||
| with col2: | ||
| st.button("Clear chat", on_click=reset_chat) | ||
|
|
||
| # Chat History | ||
| if "messages" not in st.session_state: | ||
| st.session_state.messages = [ | ||
| {"role": "assistant", "content": "Welcome! Ask me anything about cities."} | ||
| ] | ||
|
|
||
| # Display chat history | ||
| for i, message in enumerate(st.session_state.messages): | ||
| with st.chat_message(message["role"]): | ||
| st.markdown(message["content"]) | ||
|
|
||
| if message["role"] == "user" and "log_index" in message and i < len(st.session_state.messages) - 1: | ||
| log_index = message["log_index"] | ||
| if log_index < len(st.session_state.workflow_logs): | ||
| with st.expander("View Workflow Execution Logs", expanded=False): | ||
| st.code(st.session_state.workflow_logs[log_index], language="text") | ||
|
|
||
| if prompt := st.chat_input("Ask me something about cities..."): | ||
| if not st.session_state.workflow: | ||
| st.error("Workflow not initialized! Check documents and SQL data.") | ||
| st.stop() | ||
|
|
||
| log_index = len(st.session_state.workflow_logs) | ||
| st.session_state.messages.append({"role": "user", "content": prompt, "log_index": log_index}) | ||
|
|
||
| with st.chat_message("user"): | ||
| st.markdown(prompt) | ||
|
|
||
| if st.session_state.workflow: | ||
| with st.spinner("Thinking..."): | ||
| result = asyncio.run(run_workflow(prompt)) | ||
|
|
||
| st.session_state.messages.append({"role": "assistant", "content": result}) | ||
|
|
||
| if log_index < len(st.session_state.workflow_logs): | ||
| with st.expander("View Execution Details", expanded=False): | ||
| st.code(st.session_state.workflow_logs[log_index], language="text") | ||
|
|
||
| # Display assistant response with streaming | ||
| with st.chat_message("assistant"): | ||
| message_placeholder = st.empty() | ||
| full_response = "" | ||
|
|
||
| # Stream response word by word | ||
| for chunk in result.split(): | ||
| full_response += chunk + " " | ||
| time.sleep(0.05) | ||
| message_placeholder.markdown(full_response + "▌") | ||
|
|
||
| # Final display without cursor | ||
| message_placeholder.markdown(full_response) | ||
|
|
||
| # Performance Notes | ||
| st.sidebar.markdown("---") | ||
| st.sidebar.caption("ℹ️ Initial load may take 1-2 minutes for model initialization") |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that all file paths are correctly handled for different operating systems.
The code uses relative paths like ./docs and ./assets which might not work consistently across different operating systems. Consider using Path objects for all file paths to ensure cross-platform compatibility.
Also, ensure that the required directories exist at startup:
# Add this near the beginning of your application
+ # Ensure required directories exist
+ for directory in ["docs", "assets"]:
+ path = Path(f"./{directory}")
+ if not path.exists():
+ path.mkdir(exist_ok=True)
+ st.warning(f"Created missing directory: {directory}")🏁 Script executed:
#!/bin/bash
# Check all file paths used in the application
grep -r "\./" --include="*.py" . Length of output: 2278
Action Required: Standardize File Path Handling for Cross-Platform Compatibility
In rag-sql-agent/app.py (and across other modules), relative paths are used to reference directories and files (e.g., ./docs, ./assets, and PDF/image files). While the docs directory is handled with a Path object, several asset accesses still rely on raw string literals. This inconsistent use might lead to cross-platform issues, especially on systems where path conventions differ.
Recommendations:
-
Use
Pathobjects everywhere: Refactor all file access (e.g., image files under./assets) to leveragePathfrom thepathlibmodule for improved portability. -
Ensure necessary directories exist at startup: Integrate a check at the beginning of the application to create missing directories (e.g.,
docsandassets) automatically. For instance:# Add this near the beginning of your application + from pathlib import Path + import streamlit as st + + # Ensure required directories exist + for directory in ["docs", "assets"]: + dir_path = Path(directory) + if not dir_path.exists(): + dir_path.mkdir(exist_ok=True) + st.warning(f"Created missing directory: {directory}")
Addressing these points will help ensure that file paths work reliably on Windows, macOS, and Linux environments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import nest_asyncio | |
| nest_asyncio.apply() | |
| import asyncio | |
| import streamlit as st | |
| import qdrant_client | |
| import base64 | |
| import io | |
| import gc | |
| import uuid | |
| import time | |
| from pathlib import Path | |
| from contextlib import redirect_stdout | |
| from llama_index.core import VectorStoreIndex, SimpleDirectoryReader | |
| from llama_index.core import Settings, StorageContext, SQLDatabase | |
| from llama_index.core.tools import QueryEngineTool | |
| from llama_index.core.query_engine import NLSQLTableQueryEngine | |
| from llama_index.llms.ollama import Ollama | |
| from llama_index.vector_stores.qdrant import QdrantVectorStore | |
| from llama_index.embeddings.huggingface import HuggingFaceEmbedding | |
| from sqlalchemy import insert, inspect, create_engine, MetaData, Table, Column, String, Integer | |
| from sqlalchemy.pool import StaticPool | |
| from workflow import RouterOutputAgentWorkflow | |
| # Set up page configuration | |
| st.set_page_config( | |
| page_title="RAG & Text-to-SQL Agent", | |
| layout="wide", | |
| ) | |
| # Initialize session state variables | |
| if "id" not in st.session_state: | |
| st.session_state.id = uuid.uuid4() | |
| if "workflow" not in st.session_state: | |
| st.session_state.workflow = None | |
| if "messages" not in st.session_state: | |
| st.session_state.messages = [] | |
| if "workflow_logs" not in st.session_state: | |
| st.session_state.workflow_logs = [] | |
| session_id = st.session_state.id | |
| system_prompt = """ | |
| You MUST follow these rules: | |
| 1. You must ALWAYS use tools for factual questions | |
| 2. Use sql_tool ONLY for population/state/statistics questions | |
| 3. Use semantic_search_tool for other city info | |
| 4. Never answer any question directly without tool usage | |
| 5. If the question is specifically not related to the context, politely refuse to answer." | |
| """ | |
| @st.cache_resource | |
| def load_llm(): | |
| try: | |
| llm = Ollama(model="mistral:7b", system_prompt=system_prompt, request_timeout=120.0) | |
| return llm | |
| except Exception as e: | |
| st.error(f"Ollama Error: {str(e)}") | |
| st.stop() | |
| def reset_chat(): | |
| st.session_state.messages = [] | |
| st.session_state.workflow_logs = [] | |
| st.session_state.workflow = None | |
| gc.collect() | |
| # SQL Database Initialization | |
| @st.cache_resource | |
| def initialize_sql_database(): | |
| # Use shared in-memory database with connection pooling | |
| engine = create_engine( | |
| "sqlite:///file:memdb1?mode=memory&cache=shared", | |
| poolclass=StaticPool, | |
| connect_args={"check_same_thread": False} | |
| ) | |
| metadata_obj = MetaData() | |
| # Create table | |
| table_name = "city_stats" | |
| city_stats_table = Table( | |
| table_name, | |
| metadata_obj, | |
| Column("city_name", String(16)), | |
| Column("population", Integer), | |
| Column("state", String(16), nullable=False), | |
| ) | |
| with engine.connect() as conn: | |
| metadata_obj.create_all(conn) | |
| # Insert data | |
| rows = [ | |
| {"city_name": "New York City", "population": 8336000, "state": "New York"}, | |
| {"city_name": "Los Angeles", "population": 3822000, "state": "California"}, | |
| {"city_name": "Chicago", "population": 2665000, "state": "Illinois"}, | |
| {"city_name": "Houston", "population": 2303000, "state": "Texas"}, | |
| {"city_name": "Miami", "population": 449514, "state": "Florida"}, | |
| {"city_name": "Seattle", "population": 749256, "state": "Washington"}, | |
| {"city_name": "Mumbai", "population": 12691836, "state": "Maharashtra"}, | |
| {"city_name": "Delhi", "population": 10927986, "state": "Delhi"}, | |
| {"city_name": "Ahmedabad", "population": 3719710, "state": "Gujarat"}, | |
| {"city_name": "Hyderabad", "population": 3597816, "state": "Telangana"}, | |
| {"city_name": "Bengaluru", "population": 5104047, "state": "Karnataka"}, | |
| ] | |
| for row in rows: | |
| conn.execute(insert(city_stats_table).values(**row)) | |
| conn.commit() | |
| return SQLDatabase(engine, include_tables=["city_stats"]) | |
| # Document Loading and Vector Store Initialization | |
| @st.cache_data(show_spinner="Loading city knowledge base...") | |
| def initialize_city_data(): | |
| with st.spinner("📂 Loading city documents..."): | |
| docs_dir = Path("./docs") | |
| if not docs_dir.exists(): | |
| docs_dir.mkdir() | |
| st.error("Docs directory not found! Created empty directory.") | |
| return None | |
| documents = SimpleDirectoryReader("./docs").load_data() | |
| if not documents: | |
| st.error("No documents found in /docs directory!") | |
| return None | |
| with st.spinner("🧠 Initializing vector database..."): | |
| # client = qdrant_client.QdrantClient(":memory:") | |
| client = qdrant_client.QdrantClient(url="http://localhost:6333", prefer_grpc=True) | |
| vector_store = QdrantVectorStore(client=client, collection_name="city_docs") | |
| embed_model = HuggingFaceEmbedding(model_name="BAAI/bge-small-en-v1.5") | |
| Settings.embed_model = embed_model | |
| Settings.llm = load_llm() | |
| storage_context = StorageContext.from_defaults(vector_store=vector_store) | |
| return VectorStoreIndex.from_documents( | |
| documents, | |
| storage_context=storage_context, | |
| show_progress=True | |
| ) | |
| @st.cache_resource | |
| def initialize_workflow(_index, _sql_database): | |
| with st.spinner("🚀 Starting query engines..."): | |
| # SQL Query Engine | |
| sql_context = """The table 'city_stats' contains: | |
| - city_name (string) | |
| - population (integer) | |
| - state (string) | |
| Use EXACT values from the city_stats table for numerical queries.""" | |
| sql_query_engine = NLSQLTableQueryEngine( | |
| sql_database=sql_database, | |
| tables=["city_stats"], | |
| # context=sql_context, | |
| ) | |
| # Store SQL engine for fallback access | |
| st.session_state.sql_query_engine = sql_query_engine | |
| # Semantic Search Engine | |
| search_query_engine = _index.as_query_engine( | |
| llm=load_llm(), | |
| ) | |
| # Define Tools | |
| sql_tool = QueryEngineTool.from_defaults( | |
| name="sql_tool", | |
| description=f"""Useful for translating a natural language query into a SQL query over | |
| a table containing: city_stats, containing the population/state of cities""", | |
| query_engine=sql_query_engine, | |
| ) | |
| semantic_search_tool = QueryEngineTool.from_defaults( | |
| name="semantic_search_tool", | |
| description=f"Useful for answering semantic questions about certain cities in the USA and India.", | |
| query_engine=search_query_engine, | |
| ) | |
| # Create Workflow | |
| workflow = RouterOutputAgentWorkflow( | |
| tools=[sql_tool, semantic_search_tool], | |
| verbose=True, | |
| timeout=120, | |
| llm=load_llm() | |
| ) | |
| return workflow | |
| # Helper Functions | |
| async def run_workflow(query): | |
| f = io.StringIO() | |
| with redirect_stdout(f): | |
| try: | |
| result = await st.session_state.workflow.run(message=query) | |
| except Exception as e: | |
| st.error(f"Query Error: {str(e)}") | |
| result = f"Could not process query: {str(e)}" | |
| # Add SQL schema verification | |
| try: | |
| inspector = inspect(sql_database.engine) | |
| tables = inspector.get_table_names() | |
| st.error(f"Existing tables: {tables}") | |
| except Exception as db_error: | |
| st.error(f"Database connection error: {str(db_error)}") | |
| logs = f.getvalue() | |
| if logs: | |
| st.session_state.workflow_logs.append(logs) | |
| return result | |
| # Initialize Components | |
| sql_database = initialize_sql_database() | |
| city_index = initialize_city_data() | |
| if city_index and not st.session_state.workflow: | |
| st.session_state.workflow = initialize_workflow(city_index, sql_database) | |
| # Sidebar Configuration | |
| with st.sidebar: | |
| st.header("🌐 City Configuration") | |
| # Supported Cities | |
| st.subheader("Currently supported Cities") | |
| predefined_cities = [ | |
| "New York City", "Los Angeles", "Chicago", "Houston", | |
| "Miami", "Seattle", "Mumbai", "Delhi", | |
| "Ahmedabad", "Hyderabad", "Bengaluru" | |
| ] | |
| for city in predefined_cities: | |
| st.markdown(f"- {city}") | |
| if city_index: | |
| try: | |
| doc_cities = [f.stem.replace('_', ' ').title() for f in Path("./docs").glob("*.pdf")] | |
| except Exception as e: | |
| st.error(f"Error loading document cities: {str(e)}") | |
| st.divider() | |
| st.subheader("➕ Add More Cities") | |
| st.markdown(""" | |
| 1. For numerical data: | |
| - Contact admin to add to SQL database | |
| 2. For general info: | |
| - Add city PDF files to `/docs` (preferably from wikipedia) | |
| - Format: `City_Name.pdf` | |
| - Include sections for history, culture, landmarks | |
| 3. Restart the app | |
| """) | |
| # Main chat interface | |
| col1, col2 = st.columns([6, 1]) | |
| with col1: | |
| st.markdown("<h1 style='color: #0066cc;'>🕵️ RAG & Text-to-SQL Agent</h1>", unsafe_allow_html=True) | |
| st.markdown(""" | |
| <div style='display: flex; align-items: center; gap: 10px; margin-top: -10px;'> | |
| <span style='font-size: 28px; color: #666;'>Powered by LlamaIndex | |
| <img src='data:image/png;base64,{}' width='40'> Qdrant | |
| <img src='data:image/png;base64,{}' width='40'> Ollama | |
| <img src='data:image/png;base64,{}' width='40'></span> | |
| </div> | |
| """.format( | |
| base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), | |
| base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), | |
| base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() | |
| ), unsafe_allow_html=True) | |
| with col2: | |
| st.button("Clear chat", on_click=reset_chat) | |
| # Chat History | |
| if "messages" not in st.session_state: | |
| st.session_state.messages = [ | |
| {"role": "assistant", "content": "Welcome! Ask me anything about cities."} | |
| ] | |
| # Display chat history | |
| for i, message in enumerate(st.session_state.messages): | |
| with st.chat_message(message["role"]): | |
| st.markdown(message["content"]) | |
| if message["role"] == "user" and "log_index" in message and i < len(st.session_state.messages) - 1: | |
| log_index = message["log_index"] | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Workflow Execution Logs", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| if prompt := st.chat_input("Ask me something about cities..."): | |
| if not st.session_state.workflow: | |
| st.error("Workflow not initialized! Check documents and SQL data.") | |
| st.stop() | |
| log_index = len(st.session_state.workflow_logs) | |
| st.session_state.messages.append({"role": "user", "content": prompt, "log_index": log_index}) | |
| with st.chat_message("user"): | |
| st.markdown(prompt) | |
| if st.session_state.workflow: | |
| with st.spinner("Thinking..."): | |
| result = asyncio.run(run_workflow(prompt)) | |
| st.session_state.messages.append({"role": "assistant", "content": result}) | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Execution Details", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| # Display assistant response with streaming | |
| with st.chat_message("assistant"): | |
| message_placeholder = st.empty() | |
| full_response = "" | |
| # Stream response word by word | |
| for chunk in result.split(): | |
| full_response += chunk + " " | |
| time.sleep(0.05) | |
| message_placeholder.markdown(full_response + "▌") | |
| # Final display without cursor | |
| message_placeholder.markdown(full_response) | |
| # Performance Notes | |
| st.sidebar.markdown("---") | |
| st.sidebar.caption("ℹ️ Initial load may take 1-2 minutes for model initialization") | |
| import nest_asyncio | |
| nest_asyncio.apply() | |
| import asyncio | |
| import streamlit as st | |
| import qdrant_client | |
| import base64 | |
| import io | |
| import gc | |
| import uuid | |
| import time | |
| from pathlib import Path | |
| from contextlib import redirect_stdout | |
| # Ensure required directories exist | |
| for directory in ["docs", "assets"]: | |
| dir_path = Path(directory) | |
| if not dir_path.exists(): | |
| dir_path.mkdir(exist_ok=True) | |
| st.warning(f"Created missing directory: {directory}") | |
| from llama_index.core import VectorStoreIndex, SimpleDirectoryReader | |
| from llama_index.core import Settings, StorageContext, SQLDatabase | |
| from llama_index.core.tools import QueryEngineTool | |
| from llama_index.core.query_engine import NLSQLTableQueryEngine | |
| from llama_index.llms.ollama import Ollama | |
| from llama_index.vector_stores.qdrant import QdrantVectorStore | |
| from llama_index.embeddings.huggingface import HuggingFaceEmbedding | |
| from sqlalchemy import insert, inspect, create_engine, MetaData, Table, Column, String, Integer | |
| from sqlalchemy.pool import StaticPool | |
| from workflow import RouterOutputAgentWorkflow | |
| # Set up page configuration | |
| st.set_page_config( | |
| page_title="RAG & Text-to-SQL Agent", | |
| layout="wide", | |
| ) | |
| # Initialize session state variables | |
| if "id" not in st.session_state: | |
| st.session_state.id = uuid.uuid4() | |
| if "workflow" not in st.session_state: | |
| st.session_state.workflow = None | |
| if "messages" not in st.session_state: | |
| st.session_state.messages = [] | |
| if "workflow_logs" not in st.session_state: | |
| st.session_state.workflow_logs = [] | |
| session_id = st.session_state.id | |
| system_prompt = """ | |
| You MUST follow these rules: | |
| 1. You must ALWAYS use tools for factual questions | |
| 2. Use sql_tool ONLY for population/state/statistics questions | |
| 3. Use semantic_search_tool for other city info | |
| 4. Never answer any question directly without tool usage | |
| 5. If the question is specifically not related to the context, politely refuse to answer." | |
| """ | |
| @st.cache_resource | |
| def load_llm(): | |
| try: | |
| llm = Ollama(model="mistral:7b", system_prompt=system_prompt, request_timeout=120.0) | |
| return llm | |
| except Exception as e: | |
| st.error(f"Ollama Error: {str(e)}") | |
| st.stop() | |
| def reset_chat(): | |
| st.session_state.messages = [] | |
| st.session_state.workflow_logs = [] | |
| st.session_state.workflow = None | |
| gc.collect() | |
| # SQL Database Initialization | |
| @st.cache_resource | |
| def initialize_sql_database(): | |
| # Use shared in-memory database with connection pooling | |
| engine = create_engine( | |
| "sqlite:///file:memdb1?mode=memory&cache=shared", | |
| poolclass=StaticPool, | |
| connect_args={"check_same_thread": False} | |
| ) | |
| metadata_obj = MetaData() | |
| # Create table | |
| table_name = "city_stats" | |
| city_stats_table = Table( | |
| table_name, | |
| metadata_obj, | |
| Column("city_name", String(16)), | |
| Column("population", Integer), | |
| Column("state", String(16), nullable=False), | |
| ) | |
| with engine.connect() as conn: | |
| metadata_obj.create_all(conn) | |
| # Insert data | |
| rows = [ | |
| {"city_name": "New York City", "population": 8336000, "state": "New York"}, | |
| {"city_name": "Los Angeles", "population": 3822000, "state": "California"}, | |
| {"city_name": "Chicago", "population": 2665000, "state": "Illinois"}, | |
| {"city_name": "Houston", "population": 2303000, "state": "Texas"}, | |
| {"city_name": "Miami", "population": 449514, "state": "Florida"}, | |
| {"city_name": "Seattle", "population": 749256, "state": "Washington"}, | |
| {"city_name": "Mumbai", "population": 12691836, "state": "Maharashtra"}, | |
| {"city_name": "Delhi", "population": 10927986, "state": "Delhi"}, | |
| {"city_name": "Ahmedabad", "population": 3719710, "state": "Gujarat"}, | |
| {"city_name": "Hyderabad", "population": 3597816, "state": "Telangana"}, | |
| {"city_name": "Bengaluru", "population": 5104047, "state": "Karnataka"}, | |
| ] | |
| for row in rows: | |
| conn.execute(insert(city_stats_table).values(**row)) | |
| conn.commit() | |
| return SQLDatabase(engine, include_tables=["city_stats"]) | |
| # Document Loading and Vector Store Initialization | |
| @st.cache_data(show_spinner="Loading city knowledge base...") | |
| def initialize_city_data(): | |
| with st.spinner("📂 Loading city documents..."): | |
| docs_dir = Path("./docs") | |
| if not docs_dir.exists(): | |
| docs_dir.mkdir() | |
| st.error("Docs directory not found! Created empty directory.") | |
| return None | |
| documents = SimpleDirectoryReader("./docs").load_data() | |
| if not documents: | |
| st.error("No documents found in /docs directory!") | |
| return None | |
| with st.spinner("🧠 Initializing vector database..."): | |
| # client = qdrant_client.QdrantClient(":memory:") | |
| client = qdrant_client.QdrantClient(url="http://localhost:6333", prefer_grpc=True) | |
| vector_store = QdrantVectorStore(client=client, collection_name="city_docs") | |
| embed_model = HuggingFaceEmbedding(model_name="BAAI/bge-small-en-v1.5") | |
| Settings.embed_model = embed_model | |
| Settings.llm = load_llm() | |
| storage_context = StorageContext.from_defaults(vector_store=vector_store) | |
| return VectorStoreIndex.from_documents( | |
| documents, | |
| storage_context=storage_context, | |
| show_progress=True | |
| ) | |
| @st.cache_resource | |
| def initialize_workflow(_index, _sql_database): | |
| with st.spinner("🚀 Starting query engines..."): | |
| # SQL Query Engine | |
| sql_context = """The table 'city_stats' contains: | |
| - city_name (string) | |
| - population (integer) | |
| - state (string) | |
| Use EXACT values from the city_stats table for numerical queries.""" | |
| sql_query_engine = NLSQLTableQueryEngine( | |
| sql_database=sql_database, | |
| tables=["city_stats"], | |
| # context=sql_context, | |
| ) | |
| # Store SQL engine for fallback access | |
| st.session_state.sql_query_engine = sql_query_engine | |
| # Semantic Search Engine | |
| search_query_engine = _index.as_query_engine( | |
| llm=load_llm(), | |
| ) | |
| # Define Tools | |
| sql_tool = QueryEngineTool.from_defaults( | |
| name="sql_tool", | |
| description=f"""Useful for translating a natural language query into a SQL query over | |
| a table containing: city_stats, containing the population/state of cities""", | |
| query_engine=sql_query_engine, | |
| ) | |
| semantic_search_tool = QueryEngineTool.from_defaults( | |
| name="semantic_search_tool", | |
| description=f"Useful for answering semantic questions about certain cities in the USA and India.", | |
| query_engine=search_query_engine, | |
| ) | |
| # Create Workflow | |
| workflow = RouterOutputAgentWorkflow( | |
| tools=[sql_tool, semantic_search_tool], | |
| verbose=True, | |
| timeout=120, | |
| llm=load_llm() | |
| ) | |
| return workflow | |
| # Helper Functions | |
| async def run_workflow(query): | |
| f = io.StringIO() | |
| with redirect_stdout(f): | |
| try: | |
| result = await st.session_state.workflow.run(message=query) | |
| except Exception as e: | |
| st.error(f"Query Error: {str(e)}") | |
| result = f"Could not process query: {str(e)}" | |
| # Add SQL schema verification | |
| try: | |
| inspector = inspect(sql_database.engine) | |
| tables = inspector.get_table_names() | |
| st.error(f"Existing tables: {tables}") | |
| except Exception as db_error: | |
| st.error(f"Database connection error: {str(db_error)}") | |
| logs = f.getvalue() | |
| if logs: | |
| st.session_state.workflow_logs.append(logs) | |
| return result | |
| # Initialize Components | |
| sql_database = initialize_sql_database() | |
| city_index = initialize_city_data() | |
| if city_index and not st.session_state.workflow: | |
| st.session_state.workflow = initialize_workflow(city_index, sql_database) | |
| # Sidebar Configuration | |
| with st.sidebar: | |
| st.header("🌐 City Configuration") | |
| # Supported Cities | |
| st.subheader("Currently supported Cities") | |
| predefined_cities = [ | |
| "New York City", "Los Angeles", "Chicago", "Houston", | |
| "Miami", "Seattle", "Mumbai", "Delhi", | |
| "Ahmedabad", "Hyderabad", "Bengaluru" | |
| ] | |
| for city in predefined_cities: | |
| st.markdown(f"- {city}") | |
| if city_index: | |
| try: | |
| doc_cities = [f.stem.replace('_', ' ').title() for f in Path("./docs").glob("*.pdf")] | |
| except Exception as e: | |
| st.error(f"Error loading document cities: {str(e)}") | |
| st.divider() | |
| st.subheader("➕ Add More Cities") | |
| st.markdown(""" | |
| 1. For numerical data: | |
| - Contact admin to add to SQL database | |
| 2. For general info: | |
| - Add city PDF files to `/docs` (preferably from wikipedia) | |
| - Format: `City_Name.pdf` | |
| - Include sections for history, culture, landmarks | |
| 3. Restart the app | |
| """) | |
| # Main chat interface | |
| col1, col2 = st.columns([6, 1]) | |
| with col1: | |
| st.markdown("<h1 style='color: #0066cc;'>🕵️ RAG & Text-to-SQL Agent</h1>", unsafe_allow_html=True) | |
| st.markdown(""" | |
| <div style='display: flex; align-items: center; gap: 10px; margin-top: -10px;'> | |
| <span style='font-size: 28px; color: #666;'>Powered by LlamaIndex | |
| <img src='data:image/png;base64,{}' width='40'> Qdrant | |
| <img src='data:image/png;base64,{}' width='40'> Ollama | |
| <img src='data:image/png;base64,{}' width='40'></span> | |
| </div> | |
| """.format( | |
| base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), | |
| base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), | |
| base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() | |
| ), unsafe_allow_html=True) | |
| with col2: | |
| st.button("Clear chat", on_click=reset_chat) | |
| # Chat History | |
| if "messages" not in st.session_state: | |
| st.session_state.messages = [ | |
| {"role": "assistant", "content": "Welcome! Ask me anything about cities."} | |
| ] | |
| # Display chat history | |
| for i, message in enumerate(st.session_state.messages): | |
| with st.chat_message(message["role"]): | |
| st.markdown(message["content"]) | |
| if message["role"] == "user" and "log_index" in message and i < len(st.session_state.messages) - 1: | |
| log_index = message["log_index"] | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Workflow Execution Logs", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| if prompt := st.chat_input("Ask me something about cities..."): | |
| if not st.session_state.workflow: | |
| st.error("Workflow not initialized! Check documents and SQL data.") | |
| st.stop() | |
| log_index = len(st.session_state.workflow_logs) | |
| st.session_state.messages.append({"role": "user", "content": prompt, "log_index": log_index}) | |
| with st.chat_message("user"): | |
| st.markdown(prompt) | |
| if st.session_state.workflow: | |
| with st.spinner("Thinking..."): | |
| result = asyncio.run(run_workflow(prompt)) | |
| st.session_state.messages.append({"role": "assistant", "content": result}) | |
| if log_index < len(st.session_state.workflow_logs): | |
| with st.expander("View Execution Details", expanded=False): | |
| st.code(st.session_state.workflow_logs[log_index], language="text") | |
| # Display assistant response with streaming | |
| with st.chat_message("assistant"): | |
| message_placeholder = st.empty() | |
| full_response = "" | |
| # Stream response word by word | |
| for chunk in result.split(): | |
| full_response += chunk + " " | |
| time.sleep(0.05) | |
| message_placeholder.markdown(full_response + "▌") | |
| # Final display without cursor | |
| message_placeholder.markdown(full_response) | |
| # Performance Notes | |
| st.sidebar.markdown("---") | |
| st.sidebar.caption("ℹ️ Initial load may take 1-2 minutes for model initialization") |
🧰 Tools
🪛 Ruff (0.8.2)
150-150: Local variable sql_context is assigned to but never used
Remove assignment to unused variable sql_context
(F841)
173-174: f-string without any placeholders
Remove extraneous f prefix
(F541)
180-180: f-string without any placeholders
Remove extraneous f prefix
(F541)
270-270: Use a context manager for opening files
(SIM115)
271-271: Use a context manager for opening files
(SIM115)
272-272: Use a context manager for opening files
(SIM115)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
rag-sql-agent/README.md (5)
23-35: Specify Language for Directory Structure Code Block
The fenced code block displaying the directory structure does not specify a language. Adding a language specifier (e.g., using```textinstead of just ``` ) will improve consistency and ensure proper syntax highlighting.Example diff:
-``` +```text . ├── assets/ # Images/logos for the UI ├── docs/ # City PDF files for semantic search ├── app.py # Streamlit application ├── workflow.py # Main agent logic ├── demo.gif # Demo video for the app ├── requirements.txt # Python dependencies ├── agent_workflow.html # Flowchart of the agentic workflow ├── README.md # Readme file containing guidelines for the project └── .gitignore # Files/folders to exclude from version control -``` +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
24-24: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
46-49: Enhance Ollama Setup Instructions
The Ollama setup instructions repeat the product name and include a bare URL. Consider rephrasing to avoid repetition and replacing the bare URL with a Markdown-formatted link (e.g.,[Ollama](https://ollama.com)) for improved readability.🧰 Tools
🪛 LanguageTool
[duplication] ~46-~46: Possible typo: you repeated a word.
Context: ...:z \ qdrant/qdrant ``` ### Setup Ollama Ollama is required to run the LLM locally. In...(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
48-48: Bare URL used
null(MD034, no-bare-urls)
96-96: Consider Revising the 'How It Works' Heading
The heading "How It Works?" is slightly informal and can be misinterpreted as a question. Consider revising it to "How Does It Work?" for grammatical clarity.🧰 Tools
🪛 LanguageTool
[grammar] ~96-~96: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...(WRB_NN_VB_ORDER)
102-106: Hyphenate Descriptive Phrases for Clarity
In the query routing section, the phrase "Statistics based questions" would be clearer if hyphenated as "Statistics-based questions."🧰 Tools
🪛 LanguageTool
[uncategorized] ~104-~104: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...(BASED_HYPHEN)
134-139: Refine Supported Cities Section
The sentence "This project currently has support for following cities:" is missing the article "the." Consider updating it to "This project currently has support for the following cities:" for grammatical correctness.Example diff:
-This project currently has support for following cities: +This project currently has support for the following cities:🧰 Tools
🪛 LanguageTool
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rag-sql-agent/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
rag-sql-agent/README.md
[duplication] ~46-~46: Possible typo: you repeated a word.
Context: ...:z \ qdrant/qdrant ``` ### Setup Ollama Ollama is required to run the LLM locally. In...
(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~96-~96: This question is not grammatically correct. Insert the correct form of ‘do’ after ‘How’ and make sure ‘Works’ is the correct verb form.
Context: ...bash streamlit run app.py ``` ## How It Works? When a user asks a question: 1. **Qu...
(WRB_NN_VB_ORDER)
[uncategorized] ~104-~104: This expression is usually spelled with a hyphen.
Context: ...wledge (landmarks, history, etc.) - Statistics based questions (e.g., population, state data...
(BASED_HYPHEN)
[uncategorized] ~135-~135: You might be missing the article “the” here.
Context: ... This project currently has support for following cities: - USA: New York City, Los ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
rag-sql-agent/README.md
24-24: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
48-48: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (11)
rag-sql-agent/README.md (11)
1-3: Introduction is Clear and Informative
The title and opening description effectively convey the project's purpose and scope.
5-12: Technology Stack Listing is Well-Presented
The bullet list clearly enumerates the key technologies used by the project, making it easy for readers to understand the components involved.
36-44: Qdrant Setup Instructions are Clear
The Docker command for setting up Qdrant VectorDB is clearly documented and easy to follow.
51-58: Ollama Installation Commands are Clear
The separate code blocks for installing Ollama on Linux and macOS are well-presented with proper language specification and clear instructions.
59-63: Model Pull Instructions are Clear
The instructions for pulling the Mistral-7B model using Ollama are straightforward and easy to follow.
66-79: Dependency Setup Instructions are Comprehensive
The section on installing dependencies—including creating and activating a virtual environment—is comprehensive and provides clear instructions for different operating systems.
81-85: Installation of Python Dependencies is Clear
The command for installing the required packages (pip install -r requirements.txt) is succinct and clear.
87-95: Application Run Instructions are Clear
The guidance on how to launch the application via Streamlit is concise and properly formatted.
124-132: Example Usage Section is Clear
The examples provided effectively illustrate the type of queries users can ask, adding practical context to the documentation.
140-148: Customization Section is Well-Outlined
The customization options are clearly described and offer valuable guidance for extending the application's functionality.
149-153: Contact Information is Clear and Accessible
The contact section provides multiple avenues for feedback and support, making it easy for users to reach out.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
rag-sql-agent/app.py (7)
157-162: Unused variable in SQL context definition.The
sql_contextvariable is defined but never used, as shown by the commented line 166. Either remove the unused variable or uncomment line 166 to use it properly.- sql_context = """The table 'city_stats' contains: - - city_name (string) - - population (integer) - - state (string) - Use EXACT values from the city_stats table for numerical queries.""" sql_query_engine = NLSQLTableQueryEngine( sql_database=sql_database, tables=["city_stats"], - # context=sql_context, + # Either uncomment this line or remove the sql_context variable )🧰 Tools
🪛 Ruff (0.8.2)
157-157: Local variable
sql_contextis assigned to but never usedRemove assignment to unused variable
sql_context(F841)
180-181: Remove unnecessary f-string prefixes.These strings don't contain any placeholders (
{}), so they don't need to be f-strings.- description=f"""Useful for translating a natural language query into a SQL query over + description="""Useful for translating a natural language query into a SQL query over a table containing: city_stats, containing the population/state of cities""", - description=f"Useful for answering semantic questions about certain cities in the USA and India.", + description="Useful for answering semantic questions about certain cities in the USA and India.",Also applies to: 187-187
🧰 Tools
🪛 Ruff (0.8.2)
180-181: f-string without any placeholders
Remove extraneous
fprefix(F541)
277-279: Use context managers for file operations.For proper resource handling, use context managers when opening files. This ensures files are properly closed after reading.
- base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(), - base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(), - base64.b64encode(open("./assets/ollama.png", "rb").read()).decode() + base64.b64encode(Path("./assets/llamaindex.jpeg").read_bytes()).decode(), + base64.b64encode(Path("./assets/qdrant.png").read_bytes()).decode(), + base64.b64encode(Path("./assets/ollama.png").read_bytes()).decode()🧰 Tools
🪛 Ruff (0.8.2)
277-277: Use a context manager for opening files
(SIM115)
278-278: Use a context manager for opening files
(SIM115)
279-279: Use a context manager for opening files
(SIM115)
138-139: Document persistence limitations of in-memory storage.You're using an in-memory Qdrant client which means vector embeddings won't persist between application restarts. This might be intentional for development, but users should be aware of this limitation.
Consider adding a comment explaining this limitation or providing a configuration option to switch between in-memory and persistent storage:
client = qdrant_client.QdrantClient(":memory:") # client = qdrant_client.QdrantClient(url="http://localhost:6333", prefer_grpc=True) + # Note: Using in-memory storage. Vectors will not persist between app restarts. + # Uncomment the line above to use a persistent Qdrant server instead.
65-70: Improve error handling for LLM initialization.Currently, when Ollama fails to initialize, the error message doesn't provide enough guidance to the user about how to resolve the issue.
try: llm = Ollama(model="mistral:7b", system_prompt=system_prompt, request_timeout=120.0) return llm except Exception as e: - st.error(f"Ollama Error: {str(e)}") + st.error(f""" + Ollama Error: {str(e)} + + Please ensure that: + 1. Ollama is installed and running locally + 2. The "mistral:7b" model is downloaded (run 'ollama pull mistral:7b') + 3. Ollama server is accessible at the default URL + """) st.stop()
211-216: SQL database validation only happens after workflow error.The code attempts to validate the SQL database schema only after a workflow execution error occurs. Consider moving this validation to the initialization phase to catch database issues early.
+# Add this function after initialize_sql_database +def validate_sql_database(sql_database): + try: + inspector = inspect(sql_database.engine) + tables = inspector.get_table_names() + if "city_stats" not in tables: + st.warning(f"Database validation: 'city_stats' table not found. Available tables: {tables}") + return False + return True + except Exception as db_error: + st.error(f"Database validation error: {str(db_error)}") + return False # Then in the Component Initialization section (around lines 224-228) sql_database = initialize_sql_database() +# Validate database early +database_valid = validate_sql_database(sql_database) +if not database_valid: + st.error("SQL database validation failed. Some queries may not work correctly.") city_index = initialize_city_data() -if city_index and not st.session_state.workflow: +if city_index and database_valid and not st.session_state.workflow: st.session_state.workflow = initialize_workflow(city_index, sql_database)Then simplify the error handling in run_workflow:
try: result = await st.session_state.workflow.run(message=query) except Exception as e: st.error(f"Query Error: {str(e)}") result = f"Could not process query: {str(e)}" - # Add SQL schema verification - try: - inspector = inspect(sql_database.engine) - tables = inspector.get_table_names() - st.error(f"Existing tables: {tables}") - except Exception as db_error: - st.error(f"Database connection error: {str(db_error)}")
247-249: Improve PDF file validation.The code assumes all files with
try: - doc_cities = [f.stem.replace('_', ' ').title() for f in Path("./docs").glob("*.pdf")] + pdf_files = list(Path("./docs").glob("*.pdf")) + doc_cities = [] + for f in pdf_files: + try: + # Minimal validation that file is readable + with open(f, "rb") as test_file: + # Just read a small chunk to verify file is readable + test_file.read(100) + doc_cities.append(f.stem.replace('_', ' ').title()) + except Exception: + st.warning(f"Skipping potentially corrupt PDF: {f.name}") except Exception as e: st.error(f"Error loading document cities: {str(e)}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rag-sql-agent/app.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
rag-sql-agent/app.py
157-157: Local variable sql_context is assigned to but never used
Remove assignment to unused variable sql_context
(F841)
180-181: f-string without any placeholders
Remove extraneous f prefix
(F541)
187-187: f-string without any placeholders
Remove extraneous f prefix
(F541)
277-277: Use a context manager for opening files
(SIM115)
278-278: Use a context manager for opening files
(SIM115)
279-279: Use a context manager for opening files
(SIM115)
🔇 Additional comments (1)
rag-sql-agent/app.py (1)
313-336: Response streaming implementation is inefficient.The current implementation of response streaming:
- First runs the entire workflow to get the complete result
- Then "simulates" streaming by splitting the text and adding a delay
This approach doesn't provide true streaming and unnecessarily delays the response. Consider using a true streaming implementation if the LLM supports it.
if st.session_state.workflow: with st.spinner("Thinking..."): result = asyncio.run(run_workflow(prompt)) st.session_state.messages.append({"role": "assistant", "content": result}) if log_index < len(st.session_state.workflow_logs): with st.expander("View Execution Details", expanded=False): st.code(st.session_state.workflow_logs[log_index], language="text") # Display assistant response with streaming with st.chat_message("assistant"): message_placeholder = st.empty() - full_response = "" - - # Stream response word by word - for chunk in result.split(): - full_response += chunk + " " - time.sleep(0.05) - message_placeholder.markdown(full_response + "▌") - - # Final display without cursor - message_placeholder.markdown(full_response) + # Display the response directly, avoiding artificial delay + message_placeholder.markdown(result)If you want to keep the streaming effect for UX reasons, consider making it optional with a config setting.
| docs_dir = Path("./docs") | ||
| if not docs_dir.exists(): | ||
| docs_dir.mkdir() | ||
| st.error("Docs directory not found! Created empty directory.") | ||
| return None | ||
|
|
||
| documents = SimpleDirectoryReader("./docs").load_data() | ||
| if not documents: | ||
| st.error("No documents found in /docs directory!") | ||
| return None | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Improve file path handling for cross-platform compatibility.
The code uses relative paths for both the docs and assets directories. While the docs directory has error handling, the assets directory doesn't, which could cause issues on different platforms.
Add a centralized directory management approach to ensure both docs and assets directories exist and to handle paths consistently:
+# Add this near the beginning of your application, after imports
+def ensure_directories_exist():
+ directories = {"docs": Path("./docs"), "assets": Path("./assets")}
+ for name, path in directories.items():
+ if not path.exists():
+ path.mkdir(exist_ok=True)
+ st.warning(f"{name.title()} directory not found! Created empty directory.")
+ return directories
# Then in your initialize_city_data function
def initialize_city_data():
with st.spinner("📂 Loading city documents..."):
- docs_dir = Path("./docs")
- if not docs_dir.exists():
- docs_dir.mkdir()
- st.error("Docs directory not found! Created empty directory.")
- return None
+ directories = ensure_directories_exist()
+ docs_dir = directories["docs"]
+ if not any(docs_dir.glob("*")):
+ st.error("No documents found in docs directory!")
+ return None
# And update image loading with Path objects and error handling
- base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(),
- base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(),
- base64.b64encode(open("./assets/ollama.png", "rb").read()).decode()
+ base64.b64encode((directories["assets"] / "llamaindex.jpeg").read_bytes()).decode() if (directories["assets"] / "llamaindex.jpeg").exists() else "",
+ base64.b64encode((directories["assets"] / "qdrant.png").read_bytes()).decode() if (directories["assets"] / "qdrant.png").exists() else "",
+ base64.b64encode((directories["assets"] / "ollama.png").read_bytes()).decode() if (directories["assets"] / "ollama.png").exists() else ""Also applies to: 269-280
🏁 Script executed:
#!/bin/bash
# Check all file paths used in the application
grep -r "\./" --include="*.py" rag-sql-agent/Length of output: 604
Improve file and asset directory handling for cross-platform compatibility
To ensure consistent path management across your application, please centralize directory handling for both the docs and assets directories. The current code creates and checks the docs directory but uses relative paths for assets without error handling, which may become problematic on different platforms. Consider the following actions:
- Centralize Directory Management: Introduce a helper (e.g.,
ensure_directories_exist()) that creates both thedocsandassetsdirectories if they do not exist. - Unified Error Handling: Use uniform warnings or errors when directories are missing. For example, replace the current error in
initialize_city_datawith a call to this helper and validate that each directory contains the necessary files before proceeding. - Consistent File Access: Replace instances of direct file access using relative paths (like
open("./assets/llamaindex.jpeg", "rb")) with methods that use thePathobject (e.g.,(directories["assets"] / "filename").read_bytes()), and include checks for file existence.
Below is the diff snippet illustrating the proposed changes:
+# Add this near the beginning of your application, after imports
+def ensure_directories_exist():
+ directories = {"docs": Path("./docs"), "assets": Path("./assets")}
+ for name, path in directories.items():
+ if not path.exists():
+ path.mkdir(exist_ok=True)
+ st.warning(f"{name.title()} directory not found! Created empty directory.")
+ return directories
# Then in your initialize_city_data function
def initialize_city_data():
with st.spinner("📂 Loading city documents..."):
- docs_dir = Path("./docs")
- if not docs_dir.exists():
- docs_dir.mkdir()
- st.error("Docs directory not found! Created empty directory.")
- return None
+ directories = ensure_directories_exist()
+ docs_dir = directories["docs"]
+ if not any(docs_dir.glob("*")):
+ st.error("No documents found in docs directory!")
+ return None
# And update image loading with Path objects and error handling
- base64.b64encode(open("./assets/llamaindex.jpeg", "rb").read()).decode(),
- base64.b64encode(open("./assets/qdrant.png", "rb").read()).decode(),
- base64.b64encode(open("./assets/ollama.png", "rb").read()).decode()
+ base64.b64encode((directories["assets"] / "llamaindex.jpeg").read_bytes()).decode() if (directories["assets"] / "llamaindex.jpeg").exists() else "",
+ base64.b64encode((directories["assets"] / "qdrant.png").read_bytes()).decode() if (directories["assets"] / "qdrant.png").exists() else "",
+ base64.b64encode((directories["assets"] / "ollama.png").read_bytes()).decode() if (directories["assets"] / "ollama.png").exists() else ""Additional Note: Similar improvements should be applied to the code section around lines 269-280 where asset files are loaded.
This is my submission for the Daily Dose of Data Science Technical Writer Assignment—Task 1: RAG and Text-to-SQL.
Summary by CodeRabbit
New Features
Documentation
Chores