Skip to content

improved query extraction #1652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Artuomka merged 1 commit into from
Mar 6, 2026
Merged

improved query extraction #1652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions ...ities/visualizations/panel-position/use-cases/generate-panel-position-with-ai.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ ${currentQuery}

${feedbackSection}

Respond with SQL only. Preserve column aliases. Valid ${connectionType} syntax.`;
CRITICAL: Respond with the SQL query ONLY — no explanations, no comments, no markdown, no text before or after the query. Just the raw SQL statement. Preserve column aliases. Valid ${connectionType} syntax.`;
}

private cleanQueryResponse(aiResponse: string): string {
Expand All @@ -437,8 +437,34 @@ Respond with SQL only. Preserve column aliases. Valid ${connectionType} syntax.`
if (parsed.query_text) {
return parsed.query_text;
}
} catch {
// Not valid JSON, return as-is
} catch {}
}

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]);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
const sqlStart = cleaned.indexOf(match[0]);
const sqlStart = match.index!;

Copilot uses AI. Check for mistakes.
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();
}
}

Expand Down
33 changes: 30 additions & 3 deletions ...ties/visualizations/panel-position/use-cases/generate-table-dashboard-with-ai.use.case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ ${currentQuery}

${feedbackSection}

Respond with SQL only. Preserve column aliases. Valid ${connectionType} syntax.`;
CRITICAL: Respond with the SQL query ONLY — no explanations, no comments, no markdown, no text before or after the query. Just the raw SQL statement. Preserve column aliases. Valid ${connectionType} syntax.`;
}

private cleanQueryResponse(aiResponse: string): string {
Expand All @@ -500,8 +500,35 @@ Respond with SQL only. Preserve column aliases. Valid ${connectionType} syntax.`
if (parsed.query_text) {
return parsed.query_text;
}
} catch {
// Not valid JSON, return as-is
} catch {}
}

// 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]);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
const sqlStart = cleaned.indexOf(match[0]);
const sqlStart = match.index ?? cleaned.indexOf(match[0]);

Copilot uses AI. Check for mistakes.
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();
}
}
Comment on lines +506 to 533
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

Expand Down
Loading