codeant-ai-for-open-source[bot] commented on code in PR #42126:
URL: https://github.com/apache/superset/pull/42126#discussion_r3607297737
##########
superset-frontend/plugins/preset-chart-deckgl/src/layers/Arc/transformProps.ts:
##########
@@ -17,11 +17,7 @@
* under the License.
*/
import { ChartProps } from '@superset-ui/core';
-import {
- processSpatialData,
- addJsColumnsToExtraProps,
- DataRecord,
-} from '../spatialUtils';
+import { processSpatialData, DataRecord } from '../spatialUtils';
Review Comment:
**Suggestion:** Using the shared `processSpatialData` here can break
source/target alignment because it drops records with unparsable coordinates;
`processArcData` still pairs start/end points by original record index, so a
single invalid row can shift subsequent indices and produce incorrect arc
connections. Keep index-preserving outputs (e.g., placeholders) or compute both
positions from each record in one pass before building arcs. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ deck.gl Arc charts can render mispaired origin-destination arcs.
- ⚠️ Arc highlight layer may highlight incorrect arc segments.
- ⚠️ Geospatial analyses using deck_arc become misleading with dirty data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a deck.gl Arc chart (`viz_type = 'deck_arc'`) using the Arc
plugin at
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Arc/index.ts:25-55`,
which wires
`buildQuery` (`buildQuery.ts:39-81`) and `transformProps`
(`transformProps.ts:84-96`) for
this chart type.
2. In the chart’s form data, set `start_spatial` and `end_spatial` to
different spatial
configs (e.g., `start_spatial` as valid `latlong` columns, `end_spatial` as
`geohash`) per
`DeckArcFormData` in `buildQuery.ts:31-37`, and use a dataset where at least
one row has
valid start coordinates but an invalid or undecodable end coordinate (or
vice versa).
`buildQuery` only enforces non-null via `addSpatialNullFilters` at
`buildQuery.ts:64-68`,
so such rows are returned by the query.
3. When the chart renders, `transformProps` at `transformProps.ts:84-96`
pulls query rows
with `getRecordsFromQuery` (`transformUtils.ts:114-118`) and calls
`processSpatialData(records, start_spatial)` and `processSpatialData(records,
end_spatial)` at `transformProps.ts:47-48`. The shared helper
`processSpatialData`
(implementation at `spatialUtils.ts:14-81`) loops over `records` and, for
invalid
coordinates, executes `if (!position) { continue; }` at
`spatialUtils.ts:44-46`, skipping
those rows and pushing only valid ones into the `features` array
(`features.push(spatialPoint);` at `spatialUtils.ts:78`). The test
`processSpatialData
handles invalid coordinates` at `spatialUtils.test.ts:102-120` confirms this
per-record
dropping behavior.
4. Back in `processArcData` (`transformProps.ts:37-81`), the code iterates
over the
original `records` array (`.map((record, index) => { ... }` at
`transformProps.ts:53-55`)
and pairs `startFeatures[index]` and `endFeatures[index]` as
`sourcePosition`/`targetPosition` (`transformProps.ts:62-65`). Because
`processSpatialData` compacts each feature array independently, the first
row where only
one spatial config is invalid causes `startFeatures` and `endFeatures` to
diverge by one
index: from that point on, `startFeatures[index]` and `endFeatures[index]`
reference
different source/target records. The resulting mispaired arcs are passed
through
`createBaseTransformResult` at `transformProps.ts:89-96` into the deck.gl
`ArcLayer`
(`Arc.tsx:61-113`), so the rendered Arc chart shows connections between
incorrect
origin/destination points, and highlight logic in `Arc.tsx:124-143` operates
on these
wrong coordinates.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=48eaa6cf0b164f2ab7b8b0364172b221&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=48eaa6cf0b164f2ab7b8b0364172b221&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/Arc/transformProps.ts
**Line:** 20:20
**Comment:**
*Api Mismatch: Using the shared `processSpatialData` here can break
source/target alignment because it drops records with unparsable coordinates;
`processArcData` still pairs start/end points by original record index, so a
single invalid row can shift subsequent indices and produce incorrect arc
connections. Keep index-preserving outputs (e.g., placeholders) or compute both
positions from each record in one pass before building arcs.
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%2F42126&comment_hash=90b47bcc8e52d2b1fe225784af1f82d9cf7e7ca96c64f59c1154424e37f4ca9b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42126&comment_hash=90b47bcc8e52d2b1fe225784af1f82d9cf7e7ca96c64f59c1154424e37f4ca9b&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]
