codeant-ai-for-open-source[bot] commented on code in PR #33247:
URL: https://github.com/apache/superset/pull/33247#discussion_r3415401244
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/Geojson.tsx:
##########
@@ -328,7 +328,11 @@ export const getLayer: GetLayerType<GeoJsonLayer> =
function ({
getFillColor(feature, filterState?.value),
getLineColor,
getLineWidth: fd.line_width || 1,
- pointRadiusScale: fd.point_radius_scale,
+ // Use deck.gl defaults as fallbacks for backward compatibility with
existing charts.
+ // New charts will get control panel defaults (point_radius=10,
units='pixels', scale=1).
+ getPointRadius: fd.point_radius ?? 1,
+ pointRadiusUnits: fd.point_radius_units ?? 'meters',
+ pointRadiusScale: fd.point_radius_scale ?? 1,
Review Comment:
**Suggestion:** `point_radius` and `point_radius_scale` come from free-form
`SelectControl`s, which can emit string values for user-typed input; passing
them directly to deck.gl without numeric coercion can send string props to
numeric layer fields, causing incorrect radius rendering or unstable behavior.
Convert both values to numbers (with NaN-safe fallback) before assigning them
to `getPointRadius` and `pointRadiusScale`. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ GeoJSON point sizes may render incorrectly for typed values.
- ⚠️ Fractional radius scales from UI may apply inconsistently.
- ⚠️ deck.gl Geojson charts get string instead of numeric radius props.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Explore, create or open a `deck.gl Geojson` chart, whose plugin is
registered in
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/index.ts`
and uses the
GeoJSON control panel defined in
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Geojson/controlPanel.ts`
(lines
8–22 and 25–37) where `point_radius` and `point_radius_scale` are configured
as `type:
'SelectControl'` with `freeForm: true`.
2. In the chart's control panel, type a custom numeric value not in the
dropdown (for
example `3` for "Point Radius" and `0.25` for "Point Radius Scale"); because
`freeForm:
true` and the `SelectControl` implementation in
`superset-frontend/src/explore/components/controls/SelectControl.tsx` (lines
195–218)
passes the option's `value` through unchanged to `onChange`, these free-form
entries are
stored in form data as *strings* (`"3"`, `"0.25"`), and validation via
`validateInteger` /
`validateNumber`
(`packages/superset-ui-core/src/validator/validateInteger.ts` and
`validateNumber.ts`) only checks but does not coerce types.
3. When the chart runs, `rawFormData` (including `point_radius` and
`point_radius_scale`
as strings) flows unchanged into the chart props via the deck.gl preset
transform in
`superset-frontend/plugins/preset-chart-deckgl/src/transformProps.ts` (lines
24–46), which
returns `formData: rawFormData`; the `DeckGLGeoJson` React component in
`Geojson.tsx`
(lines 19–29 and 400–30) receives this `formData` and calls `getLayer({
formData, ... })`
(lines 257–265 and 420–429).
4. Inside `getLayer` in `Geojson.tsx` (lines 257–267 and 83–99), the
function uses `const
fd = formData;` and directly assigns `getPointRadius: fd.point_radius ?? 1`
and
`pointRadiusScale: fd.point_radius_scale ?? 1` (lines 95–97) when
constructing `new
GeoJsonLayer({...})`, so for free-form inputs deck.gl receives
`getPointRadius` and
`pointRadiusScale` as strings instead of numbers on numeric props, which can
lead to
incorrect or unstable point radius rendering at runtime once users start
entering custom
values in these controls.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=07dd01a19022434dbf9fb7e73d8f7263&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=07dd01a19022434dbf9fb7e73d8f7263&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/preset-chart-deckgl/src/layers/Geojson/Geojson.tsx
**Line:** 333:335
**Comment:**
*Type Error: `point_radius` and `point_radius_scale` come from
free-form `SelectControl`s, which can emit string values for user-typed input;
passing them directly to deck.gl without numeric coercion can send string props
to numeric layer fields, causing incorrect radius rendering or unstable
behavior. Convert both values to numbers (with NaN-safe fallback) before
assigning them to `getPointRadius` and `pointRadiusScale`.
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%2F33247&comment_hash=8964d921ec64d18647ade9ec97540f4a3bc02564a0be6ef504ee302d44253391&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F33247&comment_hash=8964d921ec64d18647ade9ec97540f4a3bc02564a0be6ef504ee302d44253391&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]
