diff --git a/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.test.ts b/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.test.ts new file mode 100644 index 000000000..cf2ff9836 --- /dev/null +++ b/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.test.ts @@ -0,0 +1,291 @@ +import { describe, expect, it } from 'vitest'; +import { validateAndAdjustBarLineAxes } from './bar-line-axis-validator'; +import type { MetricYml } from './version-history-types'; + +describe('validateAndAdjustBarLineAxes', () => { + it('should return valid for non-bar/line charts', () => { + const metricYml: MetricYml = { + name: 'Test Metric', + description: 'Test description', + timeFrame: '2024', + sql: 'SELECT * FROM test', + chartConfig: { + selectedChartType: 'pie', + columnLabelFormats: {}, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(false); + }); + + it('should return valid when Y axis has numeric columns', () => { + const metricYml: MetricYml = { + name: 'Sales by Month', + description: 'Monthly sales data', + timeFrame: '2024', + sql: 'SELECT month, sales FROM data', + chartConfig: { + selectedChartType: 'line', + columnLabelFormats: { + month: { + columnType: 'date', + style: 'date', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + sales: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + }, + barAndLineAxis: { + x: ['month'], + y: ['sales'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(false); + }); + + it('should swap axes when Y has non-numeric and X has numeric columns', () => { + const metricYml: MetricYml = { + name: 'Sales Rep by Revenue', + description: 'Revenue by sales rep', + timeFrame: '2024', + sql: 'SELECT revenue, sales_rep FROM data', + chartConfig: { + selectedChartType: 'bar', + columnLabelFormats: { + revenue: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + sales_rep: { + columnType: 'string', + style: 'string', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + }, + barAndLineAxis: { + x: ['revenue'], + y: ['sales_rep'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(true); + expect(result.adjustedYml).toBeDefined(); + expect(result.adjustedYml?.chartConfig?.barAndLineAxis?.x).toEqual(['sales_rep']); + expect(result.adjustedYml?.chartConfig?.barAndLineAxis?.y).toEqual(['revenue']); + }); + + it('should return error when Y has non-numeric and X also has non-numeric columns', () => { + const metricYml: MetricYml = { + name: 'Categories by Region', + description: 'Product categories by region', + timeFrame: '2024', + sql: 'SELECT region, category FROM data', + chartConfig: { + selectedChartType: 'line', + columnLabelFormats: { + region: { + columnType: 'string', + style: 'string', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + category: { + columnType: 'string', + style: 'string', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + }, + barAndLineAxis: { + x: ['region'], + y: ['category'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(false); + expect(result.shouldSwapAxes).toBe(false); + expect(result.error).toBeDefined(); + expect(result.error).toContain('Bar and line charts require numeric values on the Y axis'); + expect(result.error).toContain('category (string)'); + }); + + it('should handle multiple columns on axes', () => { + const metricYml: MetricYml = { + name: 'Multi Column Chart', + description: 'Chart with multiple columns', + timeFrame: '2024', + sql: 'SELECT date, region, sales, profit FROM data', + chartConfig: { + selectedChartType: 'line', + columnLabelFormats: { + date: { + columnType: 'date', + style: 'date', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + region: { + columnType: 'string', + style: 'string', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + sales: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + profit: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + }, + barAndLineAxis: { + x: ['date'], + y: ['sales', 'profit'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(false); + }); + + it('should return error when Y has mixed numeric and non-numeric columns', () => { + const metricYml: MetricYml = { + name: 'Mixed Y Axis', + description: 'Chart with mixed column types on Y', + timeFrame: '2024', + sql: 'SELECT month, sales, category FROM data', + chartConfig: { + selectedChartType: 'bar', + columnLabelFormats: { + month: { + columnType: 'date', + style: 'date', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + sales: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + category: { + columnType: 'string', + style: 'string', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + }, + barAndLineAxis: { + x: ['month'], + y: ['sales', 'category'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(false); + expect(result.shouldSwapAxes).toBe(false); + expect(result.error).toContain('category (string)'); + }); + + it('should handle date columns as non-numeric', () => { + const metricYml: MetricYml = { + name: 'Date on Y Axis', + description: 'Chart with date on Y axis', + timeFrame: '2024', + sql: 'SELECT sales, order_date FROM data', + chartConfig: { + selectedChartType: 'line', + columnLabelFormats: { + sales: { + columnType: 'number', + style: 'currency', + replaceMissingDataWith: 0, + numberSeparatorStyle: ',', + }, + order_date: { + columnType: 'date', + style: 'date', + replaceMissingDataWith: null, + numberSeparatorStyle: null, + }, + }, + barAndLineAxis: { + x: ['sales'], + y: ['order_date'], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(true); + expect(result.adjustedYml?.chartConfig?.barAndLineAxis?.x).toEqual(['order_date']); + expect(result.adjustedYml?.chartConfig?.barAndLineAxis?.y).toEqual(['sales']); + }); + + it('should handle missing barAndLineAxis gracefully', () => { + const metricYml: MetricYml = { + name: 'Missing Axis Config', + description: 'Chart without axis config', + timeFrame: '2024', + sql: 'SELECT * FROM data', + chartConfig: { + selectedChartType: 'bar', + columnLabelFormats: {}, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(true); + expect(result.shouldSwapAxes).toBe(false); + }); + + it('should handle empty axis arrays', () => { + const metricYml: MetricYml = { + name: 'Empty Axes', + description: 'Chart with empty axes', + timeFrame: '2024', + sql: 'SELECT * FROM data', + chartConfig: { + selectedChartType: 'line', + columnLabelFormats: {}, + barAndLineAxis: { + x: [], + y: [], + }, + }, + }; + + const result = validateAndAdjustBarLineAxes(metricYml); + expect(result.isValid).toBe(false); + expect(result.shouldSwapAxes).toBe(false); + }); +}); diff --git a/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.ts b/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.ts new file mode 100644 index 000000000..8bf318e1e --- /dev/null +++ b/packages/ai/src/tools/visualization-tools/bar-line-axis-validator.ts @@ -0,0 +1,99 @@ +import type { MetricYml } from './version-history-types'; + +export interface AxisValidationResult { + isValid: boolean; + shouldSwapAxes: boolean; + error?: string; + adjustedYml?: MetricYml; +} + +/** + * Validates and potentially adjusts bar/line chart axes to ensure numeric fields are on Y axis + * @param metricYml The parsed metric YAML configuration + * @returns Validation result indicating if axes should be swapped or if there's an error + */ +export function validateAndAdjustBarLineAxes(metricYml: MetricYml): AxisValidationResult { + // Only validate bar and line charts + const chartType = metricYml.chartConfig?.selectedChartType; + if (chartType !== 'bar' && chartType !== 'line') { + return { isValid: true, shouldSwapAxes: false }; + } + + const barAndLineAxis = metricYml.chartConfig?.barAndLineAxis; + const columnLabelFormats = metricYml.chartConfig?.columnLabelFormats; + + if (!barAndLineAxis || !columnLabelFormats) { + return { isValid: true, shouldSwapAxes: false }; + } + + if (!barAndLineAxis.x?.length || !barAndLineAxis.y?.length) { + return { + isValid: false, + shouldSwapAxes: false, + error: 'Bar and line charts require at least one column for each axis. Please specify both X and Y axis columns.', + }; + } + const xColumns = barAndLineAxis.x; + const yColumns = barAndLineAxis.y; + + // Check if all Y-axis columns are numeric + const yAxisNumericStatus = yColumns.map((col: string) => { + const format = columnLabelFormats[col]; + return format?.columnType === 'number'; + }); + + const allYColumnsNumeric = yAxisNumericStatus.every((isNumeric: boolean) => isNumeric); + + // If all Y columns are numeric, no adjustment needed + if (allYColumnsNumeric) { + return { isValid: true, shouldSwapAxes: false }; + } + + // At least one Y column is non-numeric, check if we can swap with X + const xAxisNumericStatus = xColumns.map((col: string) => { + const format = columnLabelFormats[col]; + return format?.columnType === 'number'; + }); + + const allXColumnsNumeric = xAxisNumericStatus.every((isNumeric: boolean) => isNumeric); + + // If X columns are numeric and Y columns are not, swap them + if (allXColumnsNumeric && !allYColumnsNumeric) { + // Create adjusted YAML with swapped axes + const adjustedYml: MetricYml = { + ...metricYml, + chartConfig: { + ...metricYml.chartConfig, + barAndLineAxis: { + ...barAndLineAxis, + x: yColumns, // Swap: Y becomes X + y: xColumns, // Swap: X becomes Y + }, + }, + }; + + return { + isValid: true, + shouldSwapAxes: true, + adjustedYml, + }; + } + + // Y has non-numeric columns and X is also non-numeric (or empty) + // This is an error condition + const nonNumericYColumns = yColumns.filter( + (_col: string, index: number) => !yAxisNumericStatus[index] + ); + const columnTypes = nonNumericYColumns + .map((col: string) => { + const format = columnLabelFormats[col]; + return `${col} (${format?.columnType || 'unknown'})`; + }) + .join(', '); + + return { + isValid: false, + shouldSwapAxes: false, + error: `Bar and line charts require numeric values on the Y axis. The following columns are non-numeric: ${columnTypes}. Please adjust your SQL query to ensure numeric columns are used for the Y axis, or use a different chart type.`, + }; +} diff --git a/packages/ai/src/tools/visualization-tools/create-metrics-file-tool.ts b/packages/ai/src/tools/visualization-tools/create-metrics-file-tool.ts index d9d04b67e..7f39a3625 100644 --- a/packages/ai/src/tools/visualization-tools/create-metrics-file-tool.ts +++ b/packages/ai/src/tools/visualization-tools/create-metrics-file-tool.ts @@ -9,6 +9,7 @@ import { z } from 'zod'; import { getWorkflowDataSourceManager } from '../../utils/data-source-manager'; import { createPermissionErrorMessage, validateSqlPermissions } from '../../utils/sql-permissions'; import type { AnalystRuntimeContext } from '../../workflows/analyst-workflow'; +import { validateAndAdjustBarLineAxes } from './bar-line-axis-validator'; import { trackFileAssociations } from './file-tracking-helper'; import { createInitialMetricVersionHistory, validateMetricYml } from './version-history-helpers'; import type { MetricYml } from './version-history-types'; @@ -1183,12 +1184,27 @@ async function processMetricFile( const parsedYml = yaml.parse(fixedYmlContent); const metricYml = validateMetricYml(parsedYml); + // Validate and adjust bar/line chart axes + const axisValidation = validateAndAdjustBarLineAxes(metricYml); + if (!axisValidation.isValid) { + return { + success: false, + error: axisValidation.error || 'Invalid bar/line chart axis configuration', + }; + } + + // Use adjusted YAML if axes were swapped + const finalMetricYml = + axisValidation.shouldSwapAxes && axisValidation.adjustedYml + ? axisValidation.adjustedYml + : metricYml; + // Generate deterministic UUID (simplified version) const metricId = randomUUID(); // Validate SQL by running it const sqlValidationResult = await validateSql( - metricYml.sql, + finalMetricYml.sql, dataSourceId, workflowId, userId, @@ -1206,7 +1222,7 @@ async function processMetricFile( const now = new Date().toISOString(); const metricFile: FileWithId = { id: metricId, - name: metricYml.name, + name: finalMetricYml.name, file_type: 'metric', result_message: sqlValidationResult.message || '', results: sqlValidationResult.results || [], @@ -1218,7 +1234,7 @@ async function processMetricFile( return { success: true, metricFile, - metricYml, + metricYml: finalMetricYml, message: sqlValidationResult.message || '', results: sqlValidationResult.results || [], }; diff --git a/packages/ai/src/tools/visualization-tools/modify-metrics-file-tool.ts b/packages/ai/src/tools/visualization-tools/modify-metrics-file-tool.ts index c327ada7e..999f655a0 100644 --- a/packages/ai/src/tools/visualization-tools/modify-metrics-file-tool.ts +++ b/packages/ai/src/tools/visualization-tools/modify-metrics-file-tool.ts @@ -10,6 +10,7 @@ import { z } from 'zod'; import { getWorkflowDataSourceManager } from '../../utils/data-source-manager'; import { createPermissionErrorMessage, validateSqlPermissions } from '../../utils/sql-permissions'; import type { AnalystRuntimeContext } from '../../workflows/analyst-workflow'; +import { validateAndAdjustBarLineAxes } from './bar-line-axis-validator'; import { trackFileAssociations } from './file-tracking-helper'; import { addMetricVersionToHistory, @@ -455,7 +456,34 @@ async function processMetricFileUpdate( const parsedYml = yaml.parse(fixedYmlContent); const metricYml = validateMetricYml(parsedYml); - const newMetricYml = metricYml; + // Validate and adjust bar/line chart axes + const axisValidation = validateAndAdjustBarLineAxes(metricYml); + if (!axisValidation.isValid) { + const error = axisValidation.error || 'Invalid bar/line chart axis configuration'; + modificationResults.push({ + file_id: existingFile.id, + file_name: existingFile.name, + success: false, + error, + modification_type: 'axis_validation', + timestamp, + duration, + }); + return { + success: false, + modificationResults, + validationMessage: '', + validationResults: [], + validatedDatasetIds: [], + error, + }; + } + + // Use adjusted YAML if axes were swapped + const newMetricYml = + axisValidation.shouldSwapAxes && axisValidation.adjustedYml + ? axisValidation.adjustedYml + : metricYml; // Check if SQL has changed to avoid unnecessary validation const existingContent = existingFile.content as MetricYml | null;