codeant-ai-for-open-source[bot] commented on code in PR #42311:
URL: https://github.com/apache/superset/pull/42311#discussion_r3631737993
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:
##########
@@ -75,31 +75,38 @@ export const getBaselineSeriesForStream = (
series: [string | number, number][][],
seriesType: EchartsTimeseriesSeriesType,
) => {
- const seriesLength = series[0].length;
+ // Series data can be empty or ragged (unequal lengths, null rows) when a
+ // metric column is missing or a series is all-null; index defensively so a
+ // stream-stacked chart doesn't crash while computing the baseline.
+ const seriesLength = series[0]?.length ?? 0;
const baselineSeriesDelta: [string | number, number][] = Array.from(
{ length: seriesLength },
() => [0, 0],
);
- const getVal = (value: number | null) => value ?? 0;
+ const getVal = (value: number | null | undefined) => value ?? 0;
for (let i = 0; i < seriesLength; i += 1) {
let seriesSum = 0;
let weightedSeriesSum = 0;
for (let j = 0; j < series.length; j += 1) {
const delta =
i > 0
- ? getVal(series[j][i][1]) - getVal(series[j][i - 1][1])
- : getVal(series[j][i][1]);
+ ? getVal(series[j]?.[i]?.[1]) - getVal(series[j]?.[i - 1]?.[1])
+ : getVal(series[j]?.[i]?.[1]);
let deltaPrev = 0;
for (let k = 1; k < j - 1; k += 1) {
deltaPrev +=
i > 0
- ? getVal(series[k][i][1]) - getVal(series[k][i - 1][1])
- : getVal(series[k][i][1]);
+ ? getVal(series[k]?.[i]?.[1]) - getVal(series[k]?.[i - 1]?.[1])
+ : getVal(series[k]?.[i]?.[1]);
}
- weightedSeriesSum += (0.5 * delta + deltaPrev) * getVal(series[j][i][1]);
- seriesSum += getVal(series[j][i][1]);
+ weightedSeriesSum +=
+ (0.5 * delta + deltaPrev) * getVal(series[j]?.[i]?.[1]);
+ seriesSum += getVal(series[j]?.[i]?.[1]);
}
- baselineSeriesDelta[i] = [series[0][i][0], -weightedSeriesSum / seriesSum];
+ baselineSeriesDelta[i] = [
+ series[0][i][0],
+ seriesSum === 0 ? 0 : -weightedSeriesSum / seriesSum,
+ ];
Review Comment:
**Suggestion:** The baseline point key still dereferences nested indices
without a null check, so ragged/null rows in the first series will throw at
runtime while computing stream baselines. Use the same defensive access pattern
here (or skip invalid rows) before reading the x-value. [null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Stream-stacked timeseries charts crash on ragged first-series data.
- ⚠️ Baseline computation unstable when metric columns are missing.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a stream-stacked timeseries chart that uses baseline
computation via
`getBaselineSeriesForStream()` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:74-77`,
with the first metric column missing or all-null so the generated `series`
has an index
`i` where `series[0][i]` is `undefined` or `null` (scenario described in the
comment at
lines 78-80: "Series data can be empty or ragged (unequal lengths, null
rows) when a
metric column is missing or a series is all-null").
2. When the chart is rendered, the transformation pipeline calls
`getBaselineSeriesForStream(series, seriesType)` (function definition at
lines 74-77).
Inside this function, `seriesLength` is computed from `series[0]?.length ??
0` at line 81
and `baselineSeriesDelta` is initialized with `seriesLength` entries at
lines 82-85.
3. The outer loop over `i` at lines 87-105 runs from `0` to `seriesLength -
1`. For the
problematic index `i` where `series[0][i]` is `undefined`/`null`, all
numeric delta
computations use `getVal(series[j]?.[i]?.[1])` with optional chaining and
null-coalescing
(lines 92-105), so those accesses do not throw even when rows are ragged.
4. Still within the same loop, at lines 106-109 the assignment
`baselineSeriesDelta[i] = [
series[0][i][0], seriesSum === 0 ? 0 : -weightedSeriesSum / seriesSum ]`
executes. Because
`series[0][i]` is `undefined`/`null`, `series[0][i][0]` throws a runtime
`TypeError:
Cannot read properties of undefined (reading '0')`, aborting
`getBaselineSeriesForStream()` and causing the stream-stacked chart render
to fail.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0b7491f4dcec4ae6ad91308a6e914472&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0b7491f4dcec4ae6ad91308a6e914472&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/transformers.ts
**Line:** 106:109
**Comment:**
*Null Pointer: The baseline point key still dereferences nested indices
without a null check, so ragged/null rows in the first series will throw at
runtime while computing stream baselines. Use the same defensive access pattern
here (or skip invalid rows) before reading the x-value.
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%2F42311&comment_hash=91f377b1b629447b6d2c164b7c07927de286aa62717ef368131c184d17e0e7fa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42311&comment_hash=91f377b1b629447b6d2c164b7c07927de286aa62717ef368131c184d17e0e7fa&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]
