codeant-ai-for-open-source[bot] commented on code in PR #41311:
URL: https://github.com/apache/superset/pull/41311#discussion_r3457154187
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -98,15 +98,21 @@ export const formatForecastTooltipSeries = ({
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
- if (forecastTrend || forecastLower || forecastUpper) {
+ // Use explicit numeric checks rather than truthiness so that legitimate
+ // zero values (e.g. a forecast that crosses zero, or a confidence bound of
+ // exactly 0) are not dropped from the tooltip.
+ const hasTrend = typeof forecastTrend === 'number';
+ const hasLower = typeof forecastLower === 'number';
+ const hasUpper = typeof forecastUpper === 'number';
Review Comment:
**Suggestion:** The new numeric guards treat `NaN` (and `Infinity`) as valid
because `typeof value === 'number'` is true for them, so malformed forecast
points can now render tooltip text like `ŷ = NaN` instead of being skipped. Use
finite-number checks (`Number.isFinite`) for forecast values so zero is still
allowed but invalid numeric payloads are not displayed. [incorrect condition
logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries forecast tooltips may show `ŷ = NaN` text.
- ⚠️ MixedTimeseries forecast tooltips can display NaN confidence bands.
- ⚠️ Users may misinterpret corrupted forecasts as meaningful numbers.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In a unit test (similar to
`superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts`),
import
`formatForecastTooltipSeries` from
`superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts`
(function defined
around line 86).
2. Call `formatForecastTooltipSeries` with a non-finite forecast value, for
example:
```ts
const row = formatForecastTooltipSeries({
seriesName: 'abc',
marker: '<img>',
observation: 10,
forecastTrend: Number.NaN,
formatter: getNumberFormatter(NumberFormats.INTEGER),
});
```
This follows the same usage pattern as the tests at
`forecast.test.ts:73-120`.
3. Inside `formatForecastTooltipSeries`, the guards at `forecast.ts:104-106`
treat
`forecastTrend` as present because `typeof forecastTrend === 'number'` is
true for `NaN`,
so `hasTrend` is `true` and the `if (hasTrend)` block at
`forecast.ts:111-113` executes.
4. The formatter is invoked as `formatter(forecastTrend)` at
`forecast.ts:113`, producing
a string representation of the non-finite value (typically `"NaN"` or
`"∞"`), and the
function returns a tooltip row like `['<img>abc', '10, ŷ = NaN']`, which is
then rendered
by the tooltip builders in `Timeseries/transformProps.ts:1083-1087` and
`MixedTimeseries/transformProps.ts:23-29`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=be589d45777e4856aa65ccb0703a8c11&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=be589d45777e4856aa65ccb0703a8c11&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:** 104:106
**Comment:**
*Incorrect Condition Logic: The new numeric guards treat `NaN` (and
`Infinity`) as valid because `typeof value === 'number'` is true for them, so
malformed forecast points can now render tooltip text like `ŷ = NaN` instead of
being skipped. Use finite-number checks (`Number.isFinite`) for forecast values
so zero is still allowed but invalid numeric payloads are not displayed.
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%2F41311&comment_hash=83784de351317911497bc4ab003c92a1c95cb99dc99db99421645b42a47d0b4b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41311&comment_hash=83784de351317911497bc4ab003c92a1c95cb99dc99db99421645b42a47d0b4b&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]
