Skip to content
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { BadRequestException, Inject, Injectable, NotFoundException, Scope } from '@nestjs/common';
import { BadRequestException, Inject, Injectable, Logger, NotFoundException, Scope } from '@nestjs/common';
import { getDataAccessObject } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/create-data-access-object.js';
import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js';
import { IDataAccessObject } from '@rocketadmin/shared-code/dist/src/shared/interfaces/data-access-object.interface.js';
import { IDataAccessObjectAgent } from '@rocketadmin/shared-code/dist/src/shared/interfaces/data-access-object-agent.interface.js';
import AbstractUseCase from '../../../../common/abstract-use.case.js';
import { IGlobalDatabaseContext } from '../../../../common/application/global-database-context.interface.js';
import { BaseType } from '../../../../common/data-injection.tokens.js';
Expand Down Expand Up @@ -36,11 +38,29 @@ interface AIGeneratedWidgetResponse {
};
}

const MAX_FEEDBACK_ITERATIONS = 3;

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The constant naming convention appears inconsistent with the rest of the codebase. Based on common TypeScript/NestJS conventions and the SCREAMING_SNAKE_CASE pattern used for EXPLAIN_SUPPORTED_TYPES, the constant should be named MAX_REFINEMENT_ITERATIONS or MAX_EXPLAIN_ITERATIONS instead of MAX_FEEDBACK_ITERATIONS to better describe its purpose in the context of query refinement with EXPLAIN.

Copilot uses AI. Check for mistakes.

const EXPLAIN_SUPPORTED_TYPES: ReadonlySet<ConnectionTypesEnum> = new Set([
ConnectionTypesEnum.postgres,
ConnectionTypesEnum.agent_postgres,
ConnectionTypesEnum.mysql,
ConnectionTypesEnum.agent_mysql,
ConnectionTypesEnum.clickhouse,
ConnectionTypesEnum.agent_clickhouse,
Comment on lines +43 to +49

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

For agent connection types (agent_postgres, agent_mysql, agent_clickhouse), EXPLAIN queries may behave differently through the agent proxy compared to direct connections. The implementation doesn't account for potential differences in how agents handle EXPLAIN statements. Consider whether EXPLAIN queries should be validated or tested specifically for agent connections before including them in EXPLAIN_SUPPORTED_TYPES.

Suggested change
const EXPLAIN_SUPPORTED_TYPES: ReadonlySet<ConnectionTypesEnum> = new Set([
ConnectionTypesEnum.postgres,
ConnectionTypesEnum.agent_postgres,
ConnectionTypesEnum.mysql,
ConnectionTypesEnum.agent_mysql,
ConnectionTypesEnum.clickhouse,
ConnectionTypesEnum.agent_clickhouse,
// Only direct DB connections are treated as safely supporting EXPLAIN.
// Agent connection types (agent_postgres, agent_mysql, agent_clickhouse) are
// intentionally excluded because their EXPLAIN behavior may differ when
// routed through the agent proxy and has not been explicitly validated here.
const EXPLAIN_SUPPORTED_TYPES: ReadonlySet<ConnectionTypesEnum> = new Set([
ConnectionTypesEnum.postgres,
ConnectionTypesEnum.mysql,
ConnectionTypesEnum.clickhouse,

Copilot uses AI. Check for mistakes.
]);

interface TableInfo {
table_name: string;
columns: Array<{ name: string; type: string; nullable: boolean }>;
}

@Injectable({ scope: Scope.REQUEST })
export class GeneratePanelPositionWithAiUseCase
extends AbstractUseCase<GeneratePanelPositionWithAiDs, GeneratedPanelWithPositionDto>
implements IGeneratePanelPositionWithAi
{
private readonly logger = new Logger(GeneratePanelPositionWithAiUseCase.name);

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The logger is instantiated using NestJS's built-in Logger class, which deviates from the codebase convention. Other use cases in the codebase (e.g., find-tables-in-connection.use.case.ts:38) use WinstonLogger injected via constructor dependency injection. This pattern should be followed for consistency and to ensure proper logging configuration across the application.

Copilot uses AI. Check for mistakes.

constructor(
@Inject(BaseType.GLOBAL_DB_CONTEXT)
protected _dbContext: IGlobalDatabaseContext,
Expand Down Expand Up @@ -83,10 +103,7 @@ export class GeneratePanelPositionWithAiUseCase

const dao = getDataAccessObject(foundConnection);

let tableInfo: {
table_name: string;
columns: Array<{ name: string; type: string; nullable: boolean }>;
};
let tableInfo: TableInfo;

try {
const structure = await dao.getTableStructure(table_name, null);
Expand Down Expand Up @@ -116,15 +133,23 @@ export class GeneratePanelPositionWithAiUseCase

validateQuerySafety(generatedWidget.query_text, foundConnection.type as ConnectionTypesEnum);

const refinedWidget = await this.validateAndRefineQueryWithExplain(
dao,
generatedWidget,
tableInfo,
foundConnection.type as ConnectionTypesEnum,
chart_description,
);

return {
name: name || generatedWidget.name,
description: generatedWidget.description || null,
widget_type: this.mapWidgetType(generatedWidget.widget_type),
chart_type: generatedWidget.chart_type || null,
widget_options: generatedWidget.widget_options
? (generatedWidget.widget_options as unknown as Record<string, unknown>)
name: name || refinedWidget.name,
description: refinedWidget.description || null,
widget_type: this.mapWidgetType(refinedWidget.widget_type),
chart_type: refinedWidget.chart_type || null,
widget_options: refinedWidget.widget_options
? (refinedWidget.widget_options as unknown as Record<string, unknown>)
: null,
query_text: generatedWidget.query_text,
query_text: refinedWidget.query_text,
connection_id: connectionId,
panel_position: {
position_x: position_x ?? 0,
Expand Down Expand Up @@ -219,6 +244,118 @@ Respond ONLY with the JSON object, no additional text or explanation.`;
}
}

private async validateAndRefineQueryWithExplain(
dao: IDataAccessObject | IDataAccessObjectAgent,
generatedWidget: AIGeneratedWidgetResponse,
tableInfo: TableInfo,
connectionType: ConnectionTypesEnum,
chartDescription: string,
): Promise<AIGeneratedWidgetResponse> {
if (!EXPLAIN_SUPPORTED_TYPES.has(connectionType)) {
return generatedWidget;
}

let currentQuery = generatedWidget.query_text;

for (let iteration = 0; iteration < MAX_FEEDBACK_ITERATIONS; iteration++) {
const explainResult = await this.runExplainQuery(dao, currentQuery, tableInfo.table_name);

const correctionPrompt = this.buildQueryCorrectionPrompt(
currentQuery,
explainResult.success ? explainResult.result : explainResult.error,
!explainResult.success,
tableInfo,
connectionType,
chartDescription,
);

const aiResponse = await this.aiCoreService.completeWithProvider(AIProviderType.BEDROCK, correctionPrompt, {
temperature: 0.2,
});

const correctedQuery = this.cleanQueryResponse(aiResponse);

validateQuerySafety(correctedQuery, connectionType);

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

When validateQuerySafety fails on line 278, it will throw an exception that propagates up without any specific handling. This means that if the AI-generated correction introduces unsafe SQL (e.g., contains forbidden keywords), the entire operation fails. Consider catching this exception and either breaking the loop to return the previous safe query, or adding retry logic with a different prompt to the AI. The current behavior could be confusing to users since the EXPLAIN-based refinement is meant to improve the query, but could cause the operation to fail when the original query was already safe.

Copilot uses AI. Check for mistakes.

if (this.normalizeWhitespace(correctedQuery) === this.normalizeWhitespace(currentQuery)) {
this.logger.log(`Query accepted by AI without changes after EXPLAIN (iteration ${iteration + 1})`);
break;
}

this.logger.log(`Query corrected by AI after EXPLAIN (iteration ${iteration + 1})`);
currentQuery = correctedQuery;

if (explainResult.success) {
break;
}
Comment on lines +287 to +290

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The logic for breaking out of the refinement loop may be flawed. On line 288-290, the loop breaks if explainResult.success is true, even though the AI just corrected the query. This means if the original query fails EXPLAIN, the AI corrects it, and the corrected query succeeds EXPLAIN, the loop breaks immediately without verifying if the AI made additional changes. The break should occur after checking if the query is unchanged AND the EXPLAIN succeeded, or consider removing the second break condition entirely since the unchanged check on line 280-283 already provides a termination condition.

Suggested change
if (explainResult.success) {
break;
}

Copilot uses AI. Check for mistakes.
}
Comment on lines +260 to +291

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

Running EXPLAIN queries up to MAX_FEEDBACK_ITERATIONS times (3) with AI correction calls in between could significantly increase response time and AI token usage. Each iteration involves: 1) executing an EXPLAIN query, 2) calling the AI service with a potentially large EXPLAIN result, and 3) parsing and validating the AI response. Consider adding timeout handling or making the iteration count configurable based on the complexity of the operation. Additionally, consider logging execution time for each iteration to help monitor performance.

Copilot uses AI. Check for mistakes.

return { ...generatedWidget, query_text: currentQuery };
}
Comment on lines +247 to +294

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The new validateAndRefineQueryWithExplain method adds significant complexity and multiple code paths (success/failure EXPLAIN, iterations, AI corrections) but appears to lack test coverage. Given that other similar use-cases in the codebase (request-info-from-table-with-ai-v5/v6/v7) have comprehensive test coverage, tests should be added to verify: 1) behavior for supported vs unsupported connection types, 2) handling of EXPLAIN failures vs successes, 3) iteration limits, 4) AI correction acceptance/rejection logic, and 5) error handling for unsafe queries generated by AI.

Copilot uses AI. Check for mistakes.

private async runExplainQuery(
dao: IDataAccessObject | IDataAccessObjectAgent,
query: string,
tableName: string,
): Promise<{ success: boolean; result?: string; error?: string }> {
try {
const explainQuery = `EXPLAIN ${query.replace(/;\s*$/, '')}`;

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The EXPLAIN query is constructed by simply prepending "EXPLAIN " to the original query. However, different database types have different EXPLAIN syntax. For example, MySQL uses "EXPLAIN" but ClickHouse might use "EXPLAIN PLAN" or have different options. PostgreSQL supports extended EXPLAIN options like "EXPLAIN ANALYZE". The implementation should account for database-specific EXPLAIN syntax differences based on the connectionType parameter.

Copilot uses AI. Check for mistakes.
const result = await (dao as IDataAccessObject).executeRawQuery(explainQuery, tableName);
return { success: true, result: JSON.stringify(result, null, 2) };
Comment on lines +303 to +304

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The method assumes that calling JSON.stringify on the EXPLAIN result will always produce a useful string for the AI to analyze. However, executeRawQuery can return different data structures depending on the connection type (as seen in execute-panel.use.case.ts lines 67-86 where postgres returns {rows: []}, mysql returns [[]], etc.). The result should be normalized or processed based on connectionType before stringifying to ensure the AI receives consistent, meaningful EXPLAIN output format.

Copilot uses AI. Check for mistakes.
} catch (error) {
return { success: false, error: error.message };
Comment on lines +303 to +306

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The executeRawQuery method requires a userEmail parameter (third parameter) for agent connection types (agent_postgres, agent_mysql, agent_clickhouse). However, this implementation casts dao to IDataAccessObject and only passes two parameters. This will cause the executeRawQuery call to fail for agent connections that are included in EXPLAIN_SUPPORTED_TYPES. The userId should be extracted from inputData in the implementation method, converted to userEmail using isConnectionTypeAgent check and getUserEmailOrReturnNull, and then passed to validateAndRefineQueryWithExplain and ultimately to executeRawQuery.

Suggested change
const result = await (dao as IDataAccessObject).executeRawQuery(explainQuery, tableName);
return { success: true, result: JSON.stringify(result, null, 2) };
} catch (error) {
return { success: false, error: error.message };
const result = await (dao as IDataAccessObject).executeRawQuery(explainQuery, tableName, null);
return { success: true, result: JSON.stringify(result, null, 2) };
} catch (error) {
return { success: false, error: (error as Error).message };

Copilot uses AI. Check for mistakes.

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

The error.message access is unsafe. If the error object doesn't have a message property or is not an Error instance, this will cause a runtime error. Consider using a type guard or optional chaining: error?.message || 'Unknown error'

Suggested change
return { success: false, error: error.message };
const errorMessage =
error instanceof Error
? error.message
: (typeof error === 'string' ? error : undefined) || 'Unknown error';
return { success: false, error: errorMessage };

Copilot uses AI. Check for mistakes.
}
}

private buildQueryCorrectionPrompt(
currentQuery: string,
explainResultOrError: string,
isError: boolean,
tableInfo: TableInfo,
connectionType: ConnectionTypesEnum,
chartDescription: string,
): string {
const schemaDescription = `Table: ${tableInfo.table_name}\n Columns:\n${tableInfo.columns
.map((col) => ` - ${col.name}: ${col.type}${col.nullable ? ' (nullable)' : ''}`)
.join('\n')}`;

const feedbackSection = isError
? `The query FAILED with the following error:\n${explainResultOrError}\n\nPlease fix the query to resolve this error.`
: `The EXPLAIN output for the query is:\n${explainResultOrError}\n\nReview the execution plan. If the query has performance issues (full table scans on large datasets, inefficient joins, etc.), optimize it. If the query is already acceptable, return it unchanged.`;

return `You are a database query optimization assistant. A SQL query was generated and needs validation.

DATABASE TYPE: ${connectionType}

DATABASE SCHEMA:
${schemaDescription}

ORIGINAL USER REQUEST:
"${chartDescription}"

CURRENT QUERY:
${currentQuery}

${feedbackSection}

IMPORTANT:
- Preserve the same column aliases used in the original query.
- Write valid ${connectionType} SQL syntax.
- Return ONLY the SQL query, no explanations, no markdown, no JSON wrapping.`;
}

private cleanQueryResponse(aiResponse: string): string {
return aiResponse
.trim()
.replace(/^```[a-zA-Z]*\n?/, '')
.replace(/```\s*$/, '')
.trim();
}

private normalizeWhitespace(query: string): string {
return query.replace(/\s+/g, ' ').trim();
}

private mapWidgetType(type: string): DashboardWidgetTypeEnum {
switch (type) {
case 'chart':
Expand Down
Loading