Working NLQ demo - #24
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a Natural Language Query (NLQ) feature that allows users to convert natural language questions into SQL queries using LangChain and GPT-4o. The implementation adds a new form interface for natural language input and integrates AI-powered SQL generation capabilities.
- Replaces simple pattern-matching NLQ with AI-powered query generation using LangChain and OpenAI
- Adds a new natural language query form to the query interface with help tooltips
- Implements both fresh query generation and query modification functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| app/templates/query.html | Adds natural language query form with help tooltip and adjusts layout styling |
| app/static/query_editor.js | Adds debug console logging and enables line wrapping in the code editor |
| app/nl_query.py | Complete rewrite to use LangChain/OpenAI for AI-powered SQL generation with structured output |
| app/app.py | Updates API endpoint to handle both fresh query generation and query modification scenarios |
| <form class="d-flex flex-column w-100" method="POST" action="{{ url_for('query') }}"> | ||
| <label class="p-2">Custom Query</label> | ||
| <input type="hidden" id="query-input" name="query" value="{{ query }}"> | ||
| <div id="query-editor" class="w-100 border" style="height: 300px; max-width: 300;"></div> |
There was a problem hiding this comment.
The max-width CSS property is missing a unit (px, %, em, etc.). It should be 'max-width: 300px;' or another appropriate unit.
| <div id="query-editor" class="w-100 border" style="height: 300px; max-width: 300;"></div> | |
| <div id="query-editor" class="w-100 border" style="height: 300px; max-width: 300px;"></div> |
| console.log("Keywords:", keywords); | ||
| console.log("Tables:", tables); |
There was a problem hiding this comment.
Debug console.log statements should be removed from production code as they can clutter the browser console and potentially expose sensitive information.
| console.log("Keywords:", keywords); | |
| console.log("Tables:", tables); |
| console.log("Keywords:", keywords); | ||
| console.log("Tables:", tables); |
There was a problem hiding this comment.
Debug console.log statements should be removed from production code as they can clutter the browser console and potentially expose sensitive information.
| console.log("Keywords:", keywords); | |
| console.log("Tables:", tables); |
|
|
||
| def generate_sql_modification(question: str, starting_query: str) -> str: | ||
| graph_builder = StateGraph(State) | ||
| graph_builder.add_node("modify_query", write_query) |
There was a problem hiding this comment.
The modify_query node is incorrectly calling write_query function instead of modify_query function. This should be modify_query to properly handle query modifications.
| graph_builder.add_node("modify_query", write_query) | |
| graph_builder.add_node("modify_query", modify_query) |
| sql = _to_sql(question) | ||
| result = session.execute(text(sql)).fetchall() | ||
| return {"sql": sql, "result": [dict(row._mapping) for row in result]} | ||
| llm = ChatOpenAI( |
There was a problem hiding this comment.
The OpenAI API key should be configured through environment variables or secure configuration rather than relying on default authentication. Consider explicitly setting the api_key parameter or ensuring OPENAI_API_KEY environment variable is properly configured.
| return generate_sql(question), 200 | ||
| except Exception as e: | ||
| print(f"Error creating NL query: {e}") | ||
| return {"error": "Query generation failed"}, 500 | ||
| else: | ||
| try: | ||
| return generate_sql_modification(question, starting_query), 200 |
There was a problem hiding this comment.
The API returns inconsistent response formats. When successful, it returns just the SQL string, but on error it returns a dictionary with an 'error' key. Consider wrapping the successful response in a consistent format like {'query': generate_sql(question)}.
| return generate_sql(question), 200 | ||
| except Exception as e: | ||
| print(f"Error creating NL query: {e}") | ||
| return {"error": "Query generation failed"}, 500 | ||
| else: | ||
| try: | ||
| return generate_sql_modification(question, starting_query), 200 |
There was a problem hiding this comment.
The API returns inconsistent response formats. When successful, it returns just the SQL string, but on error it returns a dictionary with an 'error' key. Consider wrapping the successful response in a consistent format like {'query': generate_sql_modification(question, starting_query)}.
No description provided.