codeant-ai-for-open-source[bot] commented on code in PR #38458:
URL: https://github.com/apache/superset/pull/38458#discussion_r2921637780
##########
superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.tsx:
##########
@@ -239,8 +245,10 @@ class ScatterPlotGlowOverlay extends
PureComponent<ScatterPlotGlowOverlayProps>
const safeNumericLabel = Number.isFinite(numericLabel)
? numericLabel
: 0;
+ const minClusterRadius = radius * MIN_CLUSTER_RADIUS_RATIO;
+ const ratio = Math.abs(safeNumericLabel) / safeMaxAbsLabel;
Review Comment:
**Suggestion:** Negative cluster values are currently converted with
`Math.abs`, which makes large-magnitude negatives render as large circles
instead of the minimum size. This contradicts the intended behavior of clamping
negatives before square-root scaling and will mis-size clusters for negative
metrics. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Cluster bubbles misrepresent negative metric magnitude semantics.
- ⚠️ MapBox cluster comparisons become misleading for signed metrics.
```
</details>
```suggestion
const ratio = Math.max(0, safeNumericLabel) / safeMaxAbsLabel;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a MapBox chart and enable custom metric clustering; the aggregator
control
allows `sum`/`min` in
`superset-frontend/plugins/legacy-plugin-chart-map-box/src/controlPanel.ts:35-38`.
2. Use a metric column containing negative values; `transformProps` forwards
raw metric
values into cluster reducers (`min`, `sum`) without non-negative clamping at
`superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.ts:62-83`.
3. Render the chart; `MapBox` passes `aggregation={hasCustomMetric ?
aggregatorName :
undefined}` into `ScatterPlotGlowOverlay` at
`superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx:169-180`.
4. In `redraw`, a negative cluster label returned by `computeClusterLabel`
(`ScatterPlotGlowOverlay.tsx:86-88`) is converted with `Math.abs` at
`ScatterPlotGlowOverlay.tsx:249`, so large-magnitude negative clusters
render large
instead of minimum-size.
```
</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:** 249:249
**Comment:**
*Logic Error: Negative cluster values are currently converted with
`Math.abs`, which makes large-magnitude negatives render as large circles
instead of the minimum size. This contradicts the intended behavior of clamping
negatives before square-root scaling and will mis-size clusters for negative
metrics.
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=dba57e80aa84c546213fe4481dd688090d3d2e40e7ec3d75477a2e8d526ff059&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38458&comment_hash=dba57e80aa84c546213fe4481dd688090d3d2e40e7ec3d75477a2e8d526ff059&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]
