codeant-ai-for-open-source[bot] commented on code in PR #40523:
URL: https://github.com/apache/superset/pull/40523#discussion_r3324305135
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -324,7 +325,8 @@ export default function transformProps(
const [rawSeries, sortedTotalValues, minPositiveValue] = extractSeries(
rebasedData,
{
- fillNeighborValue: stack && !forecastEnabled ? 0 : undefined,
+ fillNeighborValue:
+ stack && !forecastEnabled && !anomalyDetectionEnabled ? 0 : undefined,
Review Comment:
**Suggestion:** This condition disables neighbor-value filling for every
stacked series whenever anomaly detection is enabled, not just for anomaly-only
series. In stacked charts with sparse/missing points, this reintroduces null
gaps and incorrect stack continuity for the main observations. Keep stack
gap-filling for normal series and only preserve nulls for anomaly marker series
to avoid breaking stacked rendering semantics. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ⚠️ Stacked timeseries charts show gaps where stacks previously continuous.
- ⚠️ Stacked area/bar totals visually misrepresented for sparse series.
- ⚠️ Behavior changes only when anomaly detection enabled with stack.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a stacked ECharts Timeseries chart (e.g., Bar or Area) using
the Timeseries
plugin that ultimately calls `transformProps` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:181`.
2. Ensure the form data enables stacking and anomaly detection: `stack`
truthy and
`anomalyDetectionEnabled` true, as destructured in `transformProps` at lines
217–223
(`const { ... forecastEnabled, anomalyDetectionEnabled, ... stack, ... } =
formData;`).
3. Provide timeseries data where, for a given metric series, some interior
timestamps are
`null` while neighboring timestamps have numeric values (sparse data in a
stack). When
`transformProps` calls `extractSeries` at lines 325–331 with
`fillNeighborValue: stack &&
!forecastEnabled && !anomalyDetectionEnabled ? 0 : undefined`, the condition
evaluates to
`undefined` because `anomalyDetectionEnabled` is true.
4. Inside `extractSeries`
(`superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:619–699`),
`fillNeighborValue` being `undefined` prevents neighbor gap-filling
(`isFillNeighborValue`
at lines 75–83), so intermediate nulls remain null instead of being filled
with 0. These
nulls are emitted into the stacked series data (returned at lines 90–99),
causing visible
gaps and broken stack continuity for non-anomaly observation series whenever
anomaly
detection is turned on.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f418618dfe0d43859a5c58b4b7ad6dd3&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=f418618dfe0d43859a5c58b4b7ad6dd3&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/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
**Line:** 328:329
**Comment:**
*Incorrect Condition Logic: This condition disables neighbor-value
filling for every stacked series whenever anomaly detection is enabled, not
just for anomaly-only series. In stacked charts with sparse/missing points,
this reintroduces null gaps and incorrect stack continuity for the main
observations. Keep stack gap-filling for normal series and only preserve nulls
for anomaly marker series to avoid breaking stacked rendering semantics.
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%2F40523&comment_hash=14b20f80909b646e76e5dab0102289785dd68eca0a0eca49e691bf45a2e3cfd3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40523&comment_hash=14b20f80909b646e76e5dab0102289785dd68eca0a0eca49e691bf45a2e3cfd3&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -27,7 +27,7 @@ import {
import { sanitizeHtml } from './series';
const seriesTypeRegex = new RegExp(
-
`(.+)(${ForecastSeriesEnum.ForecastLower}|${ForecastSeriesEnum.ForecastTrend}|${ForecastSeriesEnum.ForecastUpper})$`,
+
`(.+)(${ForecastSeriesEnum.ForecastLower}|${ForecastSeriesEnum.ForecastTrend}|${ForecastSeriesEnum.ForecastUpper}|${ForecastSeriesEnum.Anomaly})$`,
);
Review Comment:
**Suggestion:** The suffix regex now treats `__anomaly` as the final suffix
without normalizing nested forecast suffixes, so a series like
`metric__yhat__anomaly` is parsed as base `metric__yhat` instead of `metric`.
This breaks grouping with the corresponding forecast trend/observation series
(tooltip rows, legend/filter coupling, and focus state diverge). Update the
parsing logic to normalize nested forecast+anomaly names so anomaly points for
forecasted data are grouped under the same base series as forecast/observation.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Forecast anomaly points appear under separate tooltip series keys.
- ⚠️ Legend and focus state desync between metric and its anomalies.
- ⚠️ Mixed/regular timeseries tooltips misgroup forecast anomaly values.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. On the backend, enable both Prophet forecast and anomaly detection for a
metric so that
post-processing produces forecast columns and forecast anomalies;
`tests/unit_tests/pandas_postprocessing/test_anomaly.py:286–23` verifies
this creates
columns like `metric__yhat` and `metric__yhat__anomaly`.
2. The resulting dataframe is sent to the frontend timeseries transform. In
`rebaseForecastDatum`
(`superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:127–155`),
each
column key is parsed by `extractForecastSeriesContext`, which uses
`seriesTypeRegex`
defined at lines 29–31 as ``new
RegExp(`(.+)(${ForecastSeriesEnum.ForecastLower}|${ForecastSeriesEnum.ForecastTrend}|${ForecastSeriesEnum.ForecastUpper}|${ForecastSeriesEnum.Anomaly})$`)``.
3. For a forecast anomaly column `metric__yhat__anomaly`, this regex matches
with `name =
'metric__yhat'` and `type = ForecastSeriesEnum.Anomaly`, because the final
suffix is
`__anomaly`. Thus `extractForecastSeriesContext('metric__yhat__anomaly')`
yields a base
name of `'metric__yhat'` instead of `'metric'`.
4. Downstream, `extractForecastValuesFromTooltipParams` (same file lines
57–85) groups
tooltip values into `values[context.name]`, and `transformSeries` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:74–82,
180–201` assigns `name: forecastSeries.name`. Observation and forecast
series for the
metric use base name `'metric'`, while `metric__yhat__anomaly` uses
`'metric__yhat'`. As a
result, forecast anomaly points are grouped and labeled under a separate
`'metric__yhat'`
series key instead of the parent `'metric'`, so tooltips
(`Timeseries/transformProps.ts:1040–80`) and legend/filter behavior treat
forecast
anomalies as a different series, detaching them from the main metric's
forecast/observation row.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2f59365c9fe648009ddefe5dcef6a244&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=2f59365c9fe648009ddefe5dcef6a244&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/plugins/plugin-chart-echarts/src/utils/forecast.ts
**Line:** 29:31
**Comment:**
*Logic Error: The suffix regex now treats `__anomaly` as the final
suffix without normalizing nested forecast suffixes, so a series like
`metric__yhat__anomaly` is parsed as base `metric__yhat` instead of `metric`.
This breaks grouping with the corresponding forecast trend/observation series
(tooltip rows, legend/filter coupling, and focus state diverge). Update the
parsing logic to normalize nested forecast+anomaly names so anomaly points for
forecasted data are grouped under the same base series as forecast/observation.
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%2F40523&comment_hash=597844d383cfecdf8702a175eec62adc022db40735e9ae1b789a0951c24b0803&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40523&comment_hash=597844d383cfecdf8702a175eec62adc022db40735e9ae1b789a0951c24b0803&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]
