This is an automated email from the ASF dual-hosted git repository. kgabryje pushed a commit to branch what-if in repository https://gitbox.apache.org/repos/asf/superset.git
commit a5ef75cc06064b4bba7638eac453833cc080c33d Author: Kamil Gabryjelski <[email protected]> AuthorDate: Thu Dec 18 13:10:39 2025 +0100 Fix ai insights not refreshing --- .../components/WhatIfDrawer/WhatIfAIInsights.tsx | 50 ++++++++++------------ .../dashboard/components/WhatIfDrawer/index.tsx | 33 +++++++++++--- .../components/WhatIfDrawer/useChartComparison.ts | 22 ++++++++-- 3 files changed, 67 insertions(+), 38 deletions(-) diff --git a/superset-frontend/src/dashboard/components/WhatIfDrawer/WhatIfAIInsights.tsx b/superset-frontend/src/dashboard/components/WhatIfDrawer/WhatIfAIInsights.tsx index 4cf94865e5..a845d771f3 100644 --- a/superset-frontend/src/dashboard/components/WhatIfDrawer/WhatIfAIInsights.tsx +++ b/superset-frontend/src/dashboard/components/WhatIfDrawer/WhatIfAIInsights.tsx @@ -114,19 +114,19 @@ const Summary = styled.div` interface WhatIfAIInsightsProps { affectedChartIds: number[]; + modifications: WhatIfModification[]; } -const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { +const WhatIfAIInsights = ({ + affectedChartIds, + modifications, +}: WhatIfAIInsightsProps) => { const [status, setStatus] = useState<WhatIfAIStatus>('idle'); const [response, setResponse] = useState<WhatIfInterpretResponse | null>( null, ); const [error, setError] = useState<string | null>(null); - const whatIfModifications = useSelector<RootState, WhatIfModification[]>( - state => state.dashboardState.whatIfModifications ?? [], - ); - const dashboardTitle = useSelector<RootState, string>( // @ts-ignore state => state.dashboardInfo?.dashboard_title || 'Dashboard', @@ -136,12 +136,12 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { const allChartsLoaded = useAllChartsLoaded(affectedChartIds); // Track modification changes to reset status when user adjusts the slider - const modificationsKey = getModificationsKey(whatIfModifications); + const modificationsKey = getModificationsKey(modifications); const prevModificationsKeyRef = useRef<string>(modificationsKey); // Debug logging for race condition diagnosis const willTriggerFetch = - whatIfModifications.length > 0 && + modifications.length > 0 && chartComparisons.length > 0 && allChartsLoaded && status === 'idle'; @@ -150,7 +150,7 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { affectedChartIds, allChartsLoaded, chartComparisonsLength: chartComparisons.length, - whatIfModificationsLength: whatIfModifications.length, + modificationsLength: modifications.length, status, modificationsKey, willTriggerFetch, @@ -177,7 +177,7 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { useEffect(() => { if ( modificationsKey !== prevModificationsKeyRef.current && - whatIfModifications.length > 0 + modifications.length > 0 ) { console.log( '[WhatIfAIInsights] Modifications changed, resetting status to idle', @@ -187,10 +187,10 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { setResponse(null); prevModificationsKeyRef.current = modificationsKey; } - }, [modificationsKey, whatIfModifications.length]); + }, [modificationsKey, modifications.length]); const fetchInsights = useCallback(async () => { - if (whatIfModifications.length === 0 || chartComparisons.length === 0) { + if (modifications.length === 0 || chartComparisons.length === 0) { return; } @@ -199,7 +199,7 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { try { const result = await fetchWhatIfInterpretation({ - modifications: whatIfModifications, + modifications, charts: chartComparisons, dashboardName: dashboardTitle, }); @@ -213,42 +213,36 @@ const WhatIfAIInsights = ({ affectedChartIds }: WhatIfAIInsightsProps) => { ); setStatus('error'); } - }, [whatIfModifications, chartComparisons, dashboardTitle]); + }, [modifications, chartComparisons, dashboardTitle]); // Automatically fetch insights when all affected charts have finished loading. // We wait for allChartsLoaded to prevent race conditions where we'd send // stale data before charts have re-queried with the what-if modifications. - // The setState call here is intentional - we're synchronizing with Redux state changes. + // The setState call here is intentional - we're synchronizing with prop changes. useEffect(() => { if ( - whatIfModifications.length > 0 && + modifications.length > 0 && chartComparisons.length > 0 && allChartsLoaded && status === 'idle' ) { - // eslint-disable-next-line react-hooks/set-state-in-effect -- Intentional: triggering async fetch based on Redux state + // eslint-disable-next-line react-hooks/set-state-in-effect -- Intentional: triggering async fetch based on prop changes fetchInsights(); } - }, [ - whatIfModifications, - chartComparisons, - allChartsLoaded, - status, - fetchInsights, - ]); + }, [modifications, chartComparisons, allChartsLoaded, status, fetchInsights]); // Reset state when modifications are cleared. - // The setState calls here are intentional - we're resetting local state when Redux state changes. + // The setState calls here are intentional - we're resetting local state when props change. useEffect(() => { - if (whatIfModifications.length === 0) { - // eslint-disable-next-line react-hooks/set-state-in-effect -- Intentional: resetting state when Redux modifications cleared + if (modifications.length === 0) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Intentional: resetting state when modifications cleared setStatus('idle'); setResponse(null); setError(null); } - }, [whatIfModifications]); + }, [modifications]); - if (whatIfModifications.length === 0) { + if (modifications.length === 0) { return null; } diff --git a/superset-frontend/src/dashboard/components/WhatIfDrawer/index.tsx b/superset-frontend/src/dashboard/components/WhatIfDrawer/index.tsx index 34d88300eb..3666af1b47 100644 --- a/superset-frontend/src/dashboard/components/WhatIfDrawer/index.tsx +++ b/superset-frontend/src/dashboard/components/WhatIfDrawer/index.tsx @@ -112,12 +112,14 @@ const FormSection = styled.div` `; const Label = styled.label` - font-weight: ${({ theme }) => theme.fontWeightStrong}; color: ${({ theme }) => theme.colorText}; `; const SliderContainer = styled.div` padding: 0 ${({ theme }) => theme.sizeUnit}px; + & .ant-slider-mark { + font-size: ${({ theme }) => theme.fontSizeSM}px; + } `; const ApplyButton = styled(Button)` @@ -138,7 +140,6 @@ const ModificationsSection = styled.div` `; const ModificationsSectionTitle = styled.div` - font-weight: ${({ theme }) => theme.fontWeightStrong}; color: ${({ theme }) => theme.colorText}; font-size: ${({ theme }) => theme.fontSizeSM}px; `; @@ -204,6 +205,8 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { const [appliedModifications, setAppliedModifications] = useState< ExtendedWhatIfModification[] >([]); + // Counter that increments each time Apply is clicked, used as key to reset AI insights + const [applyCounter, setApplyCounter] = useState(0); const slices = useSelector( (state: RootState) => state.sliceEntities.slices as { [id: number]: Slice }, @@ -246,6 +249,11 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { const handleApply = useCallback(async () => { if (!selectedColumn) return; + // Immediately clear previous results and increment counter to reset AI insights component + setAppliedModifications([]); + setAffectedChartIds([]); + setApplyCounter(c => c + 1); + const multiplier = 1 + sliderValue / 100; // Base user modification @@ -303,9 +311,6 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { }); const chartIdsArray = Array.from(allAffectedChartIds); - // Save affected chart IDs for AI insights - setAffectedChartIds(chartIdsArray); - // Save original chart data before applying what-if modifications chartIdsArray.forEach(chartId => { dispatch(saveOriginalChartData(chartId)); @@ -323,9 +328,15 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { ); // Trigger queries for all affected charts + // This sets chart status to 'loading', which is important for AI insights timing chartIdsArray.forEach(chartId => { dispatch(triggerQuery(true, chartId)); }); + + // Set affected chart IDs AFTER Redux updates and query triggers + // This ensures WhatIfAIInsights mounts when charts are already loading, + // preventing it from immediately fetching with stale data + setAffectedChartIds(chartIdsArray); }, [ dispatch, selectedColumn, @@ -448,7 +459,7 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { {appliedModifications.length > 0 && ( <ModificationsSection> <ModificationsSectionTitle> - {t('Applied Modifications')} + {t('Applied modifications')} </ModificationsSectionTitle> {appliedModifications.map((mod, idx) => ( <ModificationCard key={idx} isAISuggested={mod.isAISuggested}> @@ -476,7 +487,15 @@ const WhatIfPanel = ({ onClose, topOffset }: WhatIfPanelProps) => { )} {affectedChartIds.length > 0 && ( - <WhatIfAIInsights affectedChartIds={affectedChartIds} /> + <WhatIfAIInsights + key={applyCounter} + affectedChartIds={affectedChartIds} + modifications={appliedModifications.map(mod => ({ + column: mod.column, + multiplier: mod.multiplier, + filters: mod.filters, + }))} + /> )} </PanelContent> </PanelContainer> diff --git a/superset-frontend/src/dashboard/components/WhatIfDrawer/useChartComparison.ts b/superset-frontend/src/dashboard/components/WhatIfDrawer/useChartComparison.ts index a5257b2c76..b5d75dd949 100644 --- a/superset-frontend/src/dashboard/components/WhatIfDrawer/useChartComparison.ts +++ b/superset-frontend/src/dashboard/components/WhatIfDrawer/useChartComparison.ts @@ -20,6 +20,7 @@ import { useCallback, useMemo } from 'react'; import { shallowEqual, useSelector } from 'react-redux'; import { QueryData } from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/api/core'; import { ActiveTabs, DashboardLayout, @@ -38,6 +39,7 @@ interface ChartStateWithOriginal { interface QueryResponse { data?: Array<Record<string, unknown>>; colnames?: string[]; + coltypes?: GenericDataType[]; } function extractMetricValue( @@ -138,6 +140,7 @@ interface ChartComparisonData { originalData?: Array<Record<string, unknown>>; modifiedData?: Array<Record<string, unknown>>; colnames?: string[]; + coltypes?: GenericDataType[]; } /** @@ -165,6 +168,7 @@ function useChartComparisonData( originalData: originalResponse?.data, modifiedData: modifiedResponse?.data, colnames: modifiedResponse?.colnames, + coltypes: modifiedResponse?.coltypes, }; } } @@ -242,12 +246,24 @@ export function useChartComparison( continue; } - // Get column names from the response + // Get column names and types from the response const colnames = chartState.colnames || []; + const coltypes = chartState.coltypes || []; const metrics: ChartMetricComparison[] = []; - for (const metricName of colnames) { - // Only include numeric columns + for (let i = 0; i < colnames.length; i++) { + const metricName = colnames[i]; + const coltype = coltypes[i]; + + // Only include numeric columns (not temporal/date, string, or boolean) + // This filters out x-axis date columns and dimension columns + // If coltypes is available, use it; otherwise fall back to runtime check + if (coltype !== undefined && coltype !== GenericDataType.Numeric) { + continue; + } + + // Runtime check: verify the column actually contains numeric values + // This also catches cases where coltypes is not available if (!isNumericColumn(modifiedData, metricName)) continue; const originalValue = extractMetricValue(originalData, metricName);
