codeant-ai-for-open-source[bot] commented on code in PR #38458:
URL: https://github.com/apache/superset/pull/38458#discussion_r2943739458
##########
superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.tsx:
##########
@@ -288,10 +302,15 @@ class ScatterPlotGlowOverlay extends
PureComponent<ScatterPlotGlowOverlayProps>
});
}
} else {
- const defaultRadius = radius / 6;
+ const defaultRadius = radius * MIN_CLUSTER_RADIUS_RATIO;
const rawRadius = location.properties.radius;
+ const numericRadiusProperty =
+ rawRadius != null ? Number(rawRadius) : null;
const radiusProperty =
- typeof rawRadius === 'number' ? rawRadius : null;
+ numericRadiusProperty != null &&
+ Number.isFinite(numericRadiusProperty)
+ ? numericRadiusProperty
+ : null;
Review Comment:
**Suggestion:** The new radius coercion treats blank string values as
numeric zero (`Number('') === 0`), so missing radius values can be
misinterpreted as valid zeros. This will incorrectly shrink those points to the
minimum size and skew radius scaling/labels in `Pixels` mode. Treat
empty-string values as missing instead of numeric. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MapBox point-size scaling skewed by blank-radius rows.
- ⚠️ Missing radius values displayed as zero-value bubbles.
```
</details>
```suggestion
const numericRadiusProperty =
rawRadius != null &&
!(typeof rawRadius === 'string' && rawRadius.trim() === '')
? Number(rawRadius)
: null;
const radiusProperty =
numericRadiusProperty != null &&
Number.isFinite(numericRadiusProperty)
? numericRadiusProperty
: null;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Explore, configure legacy MapBox chart `point_radius` to a column
containing blank
strings; this is selectable because `point_radius` choices are all
datasource columns in
`src/controlPanel.ts:113-118`.
2. Run chart query; backend copies raw dataframe values directly into GeoJSON
`properties.radius` at `superset/viz.py:1467-1495`, so `''` reaches frontend
unchanged.
3. Render chart in Pixels mode; call chain is `MapBox.render()`
(`src/MapBox.tsx:157,169-174`) → `ScatterPlotGlowOverlay.redraw()`
(`src/ScatterPlotGlowOverlay.tsx:162+`).
4. In redraw, blank radius is coerced by `Number(rawRadius)` at
`src/ScatterPlotGlowOverlay.tsx:307-313`, so `''` becomes `0`, treated as
valid, affecting
min/max scan (`:203-214`) and rendering/labeling as numeric instead of
missing.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.tsx
**Line:** 307:313
**Comment:**
*Logic Error: The new radius coercion treats blank string values as
numeric zero (`Number('') === 0`), so missing radius values can be
misinterpreted as valid zeros. This will incorrectly shrink those points to the
minimum size and skew radius scaling/labels in `Pixels` mode. Treat
empty-string values as missing instead of numeric.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38458&comment_hash=2e8a40af6a9f2f6dd1c35cf4405292405c2e8a7756016d7b7e2c6299350b0ed1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38458&comment_hash=2e8a40af6a9f2f6dd1c35cf4405292405c2e8a7756016d7b7e2c6299350b0ed1&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]
