codeant-ai-for-open-source[bot] commented on code in PR #41182:
URL: https://github.com/apache/superset/pull/41182#discussion_r3432807136
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/utils.ts:
##########
@@ -37,7 +37,7 @@ export const parseMetricValue = (metricValue: number | string
| null) => {
if (dateObject.isValid()) {
return dateObject.valueOf();
}
- return null;
+ return metricValue;
Review Comment:
**Suggestion:** Returning raw non-date strings here changes the contract of
this shared parser and breaks BigNumber variants that do numeric math (for
example trendline aggregation and compare-lag percent calculations), which will
now receive strings and produce `NaN`/invalid results. Keep this helper
returning numeric-or-null for shared paths, and apply string-preservation only
in the BigNumber Total path (or behind an explicit flag) where direct string
display is intended. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Big Number with Trendline percent change becomes NaN for strings.
- ⚠️ Client-side trendline aggregation receives strings, may compute NaN.
- ⚠️ Dashboards using string KPIs show broken or confusing metrics.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a "Big Number with Trendline" chart (plugin wired in
`superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/index.ts:55-65`)
using a non-null VARCHAR metric such as `MAX(varchar_column)` so that the
metric values
returned from the backend are non-date strings.
2. When the chart runs, `transformProps` at
`superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts:75-127`
is invoked; it imports `parseMetricValue` from `../utils` (line 42) and
builds
`sortedData` by mapping each row to `[d[xAxisLabel],
parseMetricValue(d[metricName])]` at
lines 156-163.
3. For each non-date string metric value, `parseMetricValue` in
`superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/utils.ts:34-42`
calls
`dayjs.utc(metricValue, undefined, true)`; `dateObject.isValid()` is false
for a non-date
string, so execution falls through to `return metricValue;` at line 40,
placing raw
strings as the numeric slot in `sortedData`.
4. With a positive compare lag configured (e.g. `compareLag` > 0 in form
data), the
compare block at `BigNumberWithTrendline/transformProps.ts:202-212` reads
`compareFromValue` and `compareToValue` directly from
`sortedData[compareIndex][1]` and
`sortedData[0][1]`, which are strings, then evaluates `percentChange =
compareFromValue ?
(Number(compareToValue) - compareFromValue) / Math.abs(compareFromValue) :
0;`; because
`compareFromValue` is a string, these arithmetic operations yield `NaN`,
causing the
percent change subheader (and potentially other client-side aggregations
relying on
numeric values) to produce invalid results in the rendered Big Number with
Trendline
chart.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c6fcc77723884087b412c0159bee7a4b&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=c6fcc77723884087b412c0159bee7a4b&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/BigNumber/utils.ts
**Line:** 40:40
**Comment:**
*Type Error: Returning raw non-date strings here changes the contract
of this shared parser and breaks BigNumber variants that do numeric math (for
example trendline aggregation and compare-lag percent calculations), which will
now receive strings and produce `NaN`/invalid results. Keep this helper
returning numeric-or-null for shared paths, and apply string-preservation only
in the BigNumber Total path (or behind an explicit flag) where direct string
display is intended.
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%2F41182&comment_hash=53ac6d3833de8f018507568466091db9c503769fcc73a0d3ded31c6066c4f01d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41182&comment_hash=53ac6d3833de8f018507568466091db9c503769fcc73a0d3ded31c6066c4f01d&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]
