codeant-ai-for-open-source[bot] commented on code in PR #40954:
URL: https://github.com/apache/superset/pull/40954#discussion_r3391454078
##########
superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts:
##########
@@ -135,6 +135,23 @@ export type PostProcessingProphet =
| _PostProcessingProphet
| DefaultPostProcessing;
+interface _PostProcessingAnomalyDetection {
+ operation: 'anomaly_detection';
+ options: {
+ method: string;
+ rolling_window?: number;
+ sensitivity?: number;
+ index?: string;
+ confidence_interval?: number;
+ yearly_seasonality?: boolean | number;
+ weekly_seasonality?: boolean | number;
+ daily_seasonality?: boolean | number;
Review Comment:
**Suggestion:** The seasonality option types do not include `null`, but the
UI/operator flow uses `null` as a valid value to represent default Prophet
behavior. This mismatch breaks the frontend type contract and forces unsafe
casts even though `null` is a legitimate payload value for the backend. Include
`null` in these unions to match actual runtime behavior. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Anomaly detection Prophet seasonality defaults misrepresented in
typings.
- ⚠️ Timeseries plugin developers rely on unsafe type casts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The anomaly detection control defaults in
`superset-frontend/packages/superset-ui-chart-controls/src/sections/anomalyDetection.tsx:46-55`
define `ANOMALY_DEFAULT_DATA` with `anomalyDetectionSeasonalityYearly`,
`anomalyDetectionSeasonalityWeekly`, and `anomalyDetectionSeasonalityDaily`
all
initialized to `null` (lines 52-54), and the corresponding `SelectControl`
definitions
(lines 141-201) include `[null, t('default')]` as a valid choice, so user
interaction
produces `formData` where these seasonality fields are `null`.
2. The timeseries form data type `EchartsTimeseriesFormData` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts:54-73`
models
these properties as `anomalyDetectionSeasonalityYearly: null | boolean |
number`,
`anomalyDetectionSeasonalityWeekly: null | boolean | number`, and
`anomalyDetectionSeasonalityDaily: null | boolean | number` (lines 71-73),
confirming that
`null` is part of the intended runtime shape for Prophet-based anomaly
detection.
3. The anomaly detection operator at
`superset-frontend/packages/superset-ui-chart-controls/src/operators/anomalyDetectionOperator.ts:25-55`
builds a post-processing options object and, in the Prophet branch (lines
40-45), assigns
`options.yearly_seasonality = formData.anomalyDetectionSeasonalityYearly`,
`options.weekly_seasonality = formData.anomalyDetectionSeasonalityWeekly`,
and
`options.daily_seasonality = formData.anomalyDetectionSeasonalityDaily`,
meaning the
`options` payload can legitimately contain `null` values for these keys
before being
returned as `PostProcessingAnomalyDetection` (lines 52-55).
4. However, the `PostProcessingAnomalyDetection` interface in
`superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts:138-149`
declares `yearly_seasonality?: boolean | number;`, `weekly_seasonality?:
boolean |
number;`, and `daily_seasonality?: boolean | number;` (lines 146-148),
excluding `null`,
while the backend implementation `anomaly_detection` in
`superset/utils/pandas_postprocessing/anomaly.py:194-271` accepts
`Optional[Union[bool,
int]]` for these parameters (lines 201-203) and converts `None` to `"auto"`
via
`_parse_seasonality` (lines 140-150); this mismatch forces the frontend to
use unsafe type
assertions (`as PostProcessingAnomalyDetection`) even though `null` is a
legitimate
payload value for the backend, and misleads other TypeScript consumers into
thinking
`null` is not allowed.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d6ed46dda25146228a92654931cc86f7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d6ed46dda25146228a92654931cc86f7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts
**Line:** 146:148
**Comment:**
*Api Mismatch: The seasonality option types do not include `null`, but
the UI/operator flow uses `null` as a valid value to represent default Prophet
behavior. This mismatch breaks the frontend type contract and forces unsafe
casts even though `null` is a legitimate payload value for the backend. Include
`null` in these unions to match actual runtime behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40954&comment_hash=a7f76b4882dd8e69716fe1661be0b9f57f223d02b6ef0b7ab935bdfbb4508bec&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40954&comment_hash=a7f76b4882dd8e69716fe1661be0b9f57f223d02b6ef0b7ab935bdfbb4508bec&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts:
##########
@@ -135,6 +135,23 @@ export type PostProcessingProphet =
| _PostProcessingProphet
| DefaultPostProcessing;
+interface _PostProcessingAnomalyDetection {
+ operation: 'anomaly_detection';
+ options: {
+ method: string;
Review Comment:
**Suggestion:** The anomaly detection post-processing contract is too
permissive because `method` is typed as any string, but the backend only
accepts `zscore`, `mad`, or `prophet`. This allows invalid values to pass
TypeScript checks and fail at runtime with `InvalidPostProcessingError`. Narrow
this field to a string-literal union so invalid methods are caught at compile
time. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Timeseries anomaly detection returns error for unsupported methods.
- ⚠️ ECharts Timeseries chart data requests fail unpredictably.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the ECharts timeseries form data type at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts:54-73`,
the field
`anomalyDetectionMethod` is declared as `string`, so any string value (e.g.
`'invalid'`)
is accepted by TypeScript.
2. The timeseries query builder `buildQuery` at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts:44-117`
constructs a `post_processing` array including
`anomalyDetectionOperator(formData,
baseQueryObject)` (lines 102-114), passing through this unchecked
`anomalyDetectionMethod`
value.
3. The anomaly detection post-processing factory at
`superset-frontend/packages/superset-ui-chart-controls/src/operators/anomalyDetectionOperator.ts:25-55`
reads `formData.anomalyDetectionMethod` into `const method: string =
formData.anomalyDetectionMethod || 'zscore';` (line 30), builds `options: {
method, index:
xAxisLabel }`, and returns `{ operation: 'anomaly_detection', options } as
PostProcessingAnomalyDetection` (lines 39-55); the
`PostProcessingAnomalyDetection`
contract in
`superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts:138-149`
types `options.method` as a plain `string` (line 141), so TypeScript does
not restrict
values to the supported methods.
4. On the backend, the pandas post-processing function `anomaly_detection` in
`superset/utils/pandas_postprocessing/anomaly.py:194-271` validates its
`method` parameter
via `_validate_anomaly_inputs` (lines 153-170), which explicitly rejects any
method not in
`("zscore", "mad", "prophet")` and raises `InvalidPostProcessingError`
(lines 162-169);
this behavior is enforced by `test_anomaly_detection_invalid_method` in
`tests/unit_tests/pandas_postprocessing/test_anomaly.py:118-125`, so a
frontend
`PostProcessingRule` with `options.method: 'invalid'`—allowed by the current
`string`
typing—will cause a runtime `InvalidPostProcessingError` instead of being
caught at
compile time.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=804d37b9cced47baa999e4612da208d9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=804d37b9cced47baa999e4612da208d9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts
**Line:** 141:141
**Comment:**
*Api Mismatch: The anomaly detection post-processing contract is too
permissive because `method` is typed as any string, but the backend only
accepts `zscore`, `mad`, or `prophet`. This allows invalid values to pass
TypeScript checks and fail at runtime with `InvalidPostProcessingError`. Narrow
this field to a string-literal union so invalid methods are caught at compile
time.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40954&comment_hash=c1d0239f238e0237acecf64bf4d28da04c4dca7e37682247718a7745f298819a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40954&comment_hash=c1d0239f238e0237acecf64bf4d28da04c4dca7e37682247718a7745f298819a&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
