From 17bd286ae9b6cc9c1947d001271176dfe3a912cb Mon Sep 17 00:00:00 2001 From: yousoph Date: Wed, 22 Jul 2026 16:08:57 -0700 Subject: [PATCH] fix(forecast): resolve time grain robustly for Prophet forecasting (#42145) Co-authored-by: Claude Opus 4.8 --- .../src/operators/prophetOperator.ts | 31 +++++- .../test/operators/prophetOperator.test.ts | 105 ++++++++++++++++++ .../utils/pandas_postprocessing/prophet.py | 2 +- .../pandas_postprocessing/test_prophet.py | 34 ++++++ 4 files changed, 169 insertions(+), 3 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts index 269dc1e8016..e2d5ce201c9 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts @@ -16,9 +16,21 @@ * specific language governing permissions and limitationsxw * under the License. */ -import { PostProcessingProphet, getXAxisLabel } from '@superset-ui/core'; +import { + PostProcessingProphet, + TimeGranularity, + getXAxisColumn, + getXAxisLabel, + isAdhocColumn, +} from '@superset-ui/core'; import { PostProcessingFactory } from './types'; +// Fallback grain used only when no time grain can be resolved from the form +// data, query object, or x-axis column. Matches the `time_grain_sqla` control +// default in sharedControls so forecasting stays functional rather than failing +// with an opaque backend error. +const DEFAULT_TIME_GRAIN = TimeGranularity.DAY; + /* eslint-disable @typescript-eslint/no-unused-vars */ export const prophetOperator: PostProcessingFactory = ( formData, @@ -26,10 +38,25 @@ export const prophetOperator: PostProcessingFactory = ( ) => { const xAxisLabel = getXAxisLabel(formData); if (formData.forecastEnabled && xAxisLabel) { + // The effective time grain can live in several places depending on how the + // chart was configured. Prefer, in order: + // 1. the grain popover on an adhoc x-axis column (generic x-axis), + // 2. the grain resolved onto the query object's extras (picks up + // dashboard-applied grains and the panel control), + // 3. the `time_grain_sqla` panel control on the form data directly. + // Fall back to a daily grain so a saved/dashboard chart with the grain + // cleared to "None" still forecasts instead of raising a backend error. + const xAxisColumn = getXAxisColumn(formData); + const timeGrain = + (isAdhocColumn(xAxisColumn) && + (xAxisColumn.timeGrain as TimeGranularity)) || + queryObject.extras?.time_grain_sqla || + formData.time_grain_sqla || + DEFAULT_TIME_GRAIN; return { operation: 'prophet', options: { - time_grain: formData.time_grain_sqla, + time_grain: timeGrain, periods: parseInt(formData.forecastPeriods, 10), confidence_interval: parseFloat(formData.forecastInterval), yearly_seasonality: formData.forecastSeasonalityYearly, diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts index 9bf1f096e9e..aff1901c5d7 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts @@ -43,6 +43,11 @@ const queryObject: QueryObject = { granularity: 'P1Y', }; +// A chart whose Time Grain control was cleared to "None": form_data has no +// `time_grain_sqla` key at all (SC-113749). +const formDataWithoutGrain: SqlaFormData = { ...formData }; +delete formDataWithoutGrain.time_grain_sqla; + test('should skip prophetOperator', () => { expect(prophetOperator(formData, queryObject)).toEqual(undefined); }); @@ -137,3 +142,103 @@ test('should do prophetOperator over adhoc column', () => { }, }); }); + +test('should fall back to a daily grain when no time grain is resolvable', () => { + // Regression for SC-113749: a saved/dashboard chart with the Time Grain + // control cleared to "None" has no `time_grain_sqla` in form_data. Prior to + // the fix this emitted `time_grain: undefined`, which `JSON.stringify` drops, + // causing the backend `prophet()` call to raise a raw `TypeError`. + expect( + prophetOperator( + { + ...formDataWithoutGrain, + granularity_sqla: 'time_column', + forecastEnabled: true, + forecastPeriods: '3', + forecastInterval: '5', + forecastSeasonalityYearly: true, + forecastSeasonalityWeekly: false, + forecastSeasonalityDaily: false, + }, + { ...queryObject, extras: {} }, + ), + ).toEqual({ + operation: 'prophet', + options: { + time_grain: 'P1D', + periods: 3.0, + confidence_interval: 5.0, + yearly_seasonality: true, + weekly_seasonality: false, + daily_seasonality: false, + index: DTTM_ALIAS, + }, + }); +}); + +test('should resolve the time grain from the adhoc x-axis column', () => { + // With the generic x-axis, the grain lives on the column's popover + // (`timeGrain`) rather than the `time_grain_sqla` panel control. + expect( + prophetOperator( + { + ...formDataWithoutGrain, + x_axis: { + label: 'ds', + expressionType: 'SQL', + sqlExpression: 'ds', + timeGrain: 'P1M', + }, + forecastEnabled: true, + forecastPeriods: '3', + forecastInterval: '5', + forecastSeasonalityYearly: true, + forecastSeasonalityWeekly: false, + forecastSeasonalityDaily: false, + }, + { ...queryObject, extras: {} }, + ), + ).toEqual({ + operation: 'prophet', + options: { + time_grain: 'P1M', + periods: 3.0, + confidence_interval: 5.0, + yearly_seasonality: true, + weekly_seasonality: false, + daily_seasonality: false, + index: 'ds', + }, + }); +}); + +test('should resolve the time grain from the query object extras', () => { + // Dashboard-applied grains (e.g. via native filters) land in + // `queryObject.extras.time_grain_sqla` even when form_data has none. + expect( + prophetOperator( + { + ...formDataWithoutGrain, + granularity_sqla: 'time_column', + forecastEnabled: true, + forecastPeriods: '3', + forecastInterval: '5', + forecastSeasonalityYearly: true, + forecastSeasonalityWeekly: false, + forecastSeasonalityDaily: false, + }, + { ...queryObject, extras: { time_grain_sqla: 'P1W' } }, + ), + ).toEqual({ + operation: 'prophet', + options: { + time_grain: 'P1W', + periods: 3.0, + confidence_interval: 5.0, + yearly_seasonality: true, + weekly_seasonality: false, + daily_seasonality: false, + index: DTTM_ALIAS, + }, + }); +}); diff --git a/superset/utils/pandas_postprocessing/prophet.py b/superset/utils/pandas_postprocessing/prophet.py index 0c71807da10..749d40f78c1 100644 --- a/superset/utils/pandas_postprocessing/prophet.py +++ b/superset/utils/pandas_postprocessing/prophet.py @@ -87,9 +87,9 @@ def _prophet_fit_and_predict( # pylint: disable=too-many-arguments def prophet( # pylint: disable=too-many-arguments # noqa: C901 df: DataFrame, - time_grain: str, periods: int, confidence_interval: float, + time_grain: Optional[str] = None, yearly_seasonality: Optional[Union[bool, int]] = None, weekly_seasonality: Optional[Union[bool, int]] = None, daily_seasonality: Optional[Union[bool, int]] = None, diff --git a/tests/unit_tests/pandas_postprocessing/test_prophet.py b/tests/unit_tests/pandas_postprocessing/test_prophet.py index a9c751c15a3..b6f6a97b67e 100644 --- a/tests/unit_tests/pandas_postprocessing/test_prophet.py +++ b/tests/unit_tests/pandas_postprocessing/test_prophet.py @@ -190,6 +190,40 @@ def test_prophet_incorrect_time_grain(): ) +def test_prophet_missing_time_grain_raises_readable_error(): + """ + Regression for SC-113749: when the ``prophet`` post-processing operation is + dispatched with an options dict that lacks ``time_grain`` (e.g. a saved or + dashboard chart whose Time Grain was cleared, so the frontend drops the + ``undefined`` key during ``JSON.stringify``), the call must raise a readable + ``InvalidPostProcessingError`` rather than a raw ``TypeError`` about a + missing positional argument. This mirrors the real dispatch in + ``QueryObject.exec_post_processing`` which invokes the operation with + ``**options`` and no ``time_grain`` key. + """ + options = { + "periods": 3, + "confidence_interval": 0.9, + "index": DTTM_ALIAS, + } + with pytest.raises(InvalidPostProcessingError, match="Time grain missing"): + prophet(prophet_df, **options) + + +def test_prophet_explicit_none_time_grain_raises_readable_error(): + """ + Passing ``time_grain=None`` explicitly (the resolved value when no grain is + determinable) must also surface the graceful "Time grain missing" error. + """ + with pytest.raises(InvalidPostProcessingError, match="Time grain missing"): + prophet( + df=prophet_df, + time_grain=None, + periods=3, + confidence_interval=0.9, + ) + + def test_prophet_insufficient_data(): single_row_df = pd.DataFrame( {