Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the robustness of SQL query extraction from AI responses in two use cases that generate dashboard panels/widgets via AI. It strengthens the system prompt to more explicitly request raw SQL only, and adds logic to cleanQueryResponse to extract SQL statements when the AI includes surrounding explanatory text.
Changes:
- Updated the query correction prompt in both use cases to use a stronger "CRITICAL" instruction telling the AI to respond with raw SQL only.
- Added SQL extraction logic to
cleanQueryResponsein both files to handle cases where the AI wraps the SQL statement in explanatory text, by finding SQL keywords at the start of a line and stripping surrounding non-SQL content.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
backend/src/entities/visualizations/panel-position/use-cases/generate-panel-position-with-ai.use.case.ts |
Strengthened AI prompt wording and added SQL extraction fallback logic to cleanQueryResponse |
backend/src/entities/visualizations/panel-position/use-cases/generate-table-dashboard-with-ai.use.case.ts |
Identical changes: strengthened AI prompt wording and added the same SQL extraction fallback logic to cleanQueryResponse |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| const match = cleaned.match(sqlPattern); | ||
| if (match) { | ||
| const sqlStart = cleaned.indexOf(match[0]); |
There was a problem hiding this comment.
Bug: cleaned.indexOf(match[0]) may find the wrong occurrence of the SQL keyword. When the regex matches a keyword at the start of a line (due to the m flag), it might not be the first occurrence of that keyword in the text. For example, if the AI response is "The SELECT query has been fixed:\nSELECT * FROM table;", the regex correctly matches SELECT at the start of the second line, but indexOf('SELECT') returns the position of SELECT in the first line's explanatory text.
Use match.index instead of cleaned.indexOf(match[0]) — the match object from a non-global regex already includes the position of the match.
| const sqlStart = cleaned.indexOf(match[0]); | |
| const sqlStart = match.index!; |
| ); | ||
| const match = cleaned.match(sqlPattern); | ||
| if (match) { | ||
| const sqlStart = cleaned.indexOf(match[0]); |
There was a problem hiding this comment.
Bug: Same issue as in generate-panel-position-with-ai.use.case.ts — cleaned.indexOf(match[0]) may find an earlier occurrence of the SQL keyword in explanatory text rather than the one the regex matched at the start of a line. Use match.index instead, which directly gives the position of the regex match.
| const sqlStart = cleaned.indexOf(match[0]); | |
| const sqlStart = match.index ?? cleaned.indexOf(match[0]); |
| // If AI included explanatory text around the SQL, extract the SQL statement. | ||
| const sqlPrefixes = ['SELECT', 'WITH', 'SHOW', 'DESCRIBE', 'DESC', 'EXPLAIN']; | ||
| const upperCleaned = cleaned.toUpperCase(); | ||
| const startsWithSql = sqlPrefixes.some((prefix) => upperCleaned.startsWith(prefix)); | ||
|
|
||
| if (!startsWithSql) { | ||
| const sqlPattern = new RegExp( | ||
| `^(${sqlPrefixes.join('|')})\\b`, | ||
| 'im', | ||
| ); | ||
| const match = cleaned.match(sqlPattern); | ||
| if (match) { | ||
| const sqlStart = cleaned.indexOf(match[0]); | ||
| let sqlBody = cleaned.slice(sqlStart); | ||
|
|
||
| sqlBody = sqlBody.replace(/```\s*$/, '').trim(); | ||
|
|
||
| const lastSemicolon = sqlBody.lastIndexOf(';'); | ||
| if (lastSemicolon !== -1) { | ||
| const afterSemicolon = sqlBody.slice(lastSemicolon + 1).trim(); | ||
| if (afterSemicolon.length > 0 && /[a-zA-Z]{3,}/.test(afterSemicolon)) { | ||
| sqlBody = sqlBody.slice(0, lastSemicolon + 1); | ||
| } | ||
| } | ||
|
|
||
| return sqlBody.trim(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The entire cleanQueryResponse method (and buildQueryCorrectionPrompt, normalizeWhitespace, mapPanelType) are duplicated verbatim across this file and generate-panel-position-with-ai.use.case.ts. This duplication means any bug fix or improvement must be applied in both places, which is error-prone. Consider extracting the shared SQL-cleaning logic into a shared utility function (e.g., in a utils file alongside check-query-is-safe.util.ts) that both use cases can import.
No description provided.