codeant-ai-for-open-source[bot] commented on code in PR #38035: URL: https://github.com/apache/superset/pull/38035#discussion_r2868053349
########## superset-frontend/plugins/preset-chart-deckgl/src/layers/Heatmap/Heatmap.tsx: ########## @@ -0,0 +1,211 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { HeatmapLayer } from '@deck.gl/aggregation-layers'; +import { Position } from '@deck.gl/core'; +import { t } from '@apache-superset/core'; +import { + getSequentialSchemeRegistry, + JsonObject, + QueryFormData, +} from '@superset-ui/core'; +import { isPointInBonds } from '../../utilities/utils'; +import { commonLayerProps, getColorRange } from '../common'; +import sandboxedEval from '../../utils/sandbox'; +import { GetLayerType, createDeckGLComponent } from '../../factory'; +import TooltipRow from '../../TooltipRow'; +import { createTooltipContent } from '../../utilities/tooltipUtils'; +import { HIGHLIGHT_COLOR_ARRAY } from '../../utils'; + +function setTooltipContent(formData: QueryFormData) { + const defaultTooltipGenerator = (o: JsonObject) => { + const metricLabel = + formData.size?.label || formData.size?.value || 'Weight'; + const lon = o.coordinate?.[0]; + const lat = o.coordinate?.[1]; + + const hasCustomTooltip = + formData.tooltip_template || + (formData.tooltip_contents && formData.tooltip_contents.length > 0); + const hasObjectData = o.object && Object.keys(o.object).length > 0; + + return ( + <div className="deckgl-tooltip"> + <TooltipRow + label={`${t('Longitude and Latitude')}: `} + value={`${lon?.toFixed(6)}, ${lat?.toFixed(6)}`} + /> + <TooltipRow label={`${t('LON')}: `} value={lon?.toFixed(6)} /> + <TooltipRow label={`${t('LAT')}: `} value={lat?.toFixed(6)} /> + <TooltipRow + label={`${metricLabel}: `} + value={`${o.object?.weight ?? o.object?.value ?? 'Aggregated Cell'}`} + /> + {hasCustomTooltip && !hasObjectData && ( + <TooltipRow + label={`${t('Note')}: `} + value={t('Custom fields not available in aggregated heatmap cells')} + /> + )} + </div> + ); + }; + + return (o: JsonObject) => { + // Try to find the closest data point to the hovered coordinate + let closestPoint = null; + if (o.coordinate && o.layer?.props?.data) { + const [hoveredLon, hoveredLat] = o.coordinate; + let minDistance = Infinity; + + for (const point of o.layer.props.data) { + if (point.position) { + const [pointLon, pointLat] = point.position; + const distance = Math.sqrt( + Math.pow(hoveredLon - pointLon, 2) + + Math.pow(hoveredLat - pointLat, 2), + ); + if (distance < minDistance) { + minDistance = distance; + closestPoint = point; + } + } + } + } + const modifiedO = { + ...o, + object: closestPoint || o.object, + }; + + return createTooltipContent(formData, defaultTooltipGenerator)(modifiedO); Review Comment: **Suggestion:** When building the heatmap tooltip, the code replaces the hovered cell's aggregated object with the nearest raw point object, so the metric row shows a single point's weight instead of the aggregated cell weight, which misrepresents the data; instead, the nearest point's fields should be merged into the existing object while preserving the aggregated metrics. [logic error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Heatmap tooltips show single-point weight, not aggregate. - ⚠️ Analysts may misinterpret dense regions' metric values. ``` </details> ```suggestion const modifiedO = { ...o, object: closestPoint ? { ...closestPoint, ...o.object } : o.object, }; return createTooltipContent(formData, defaultTooltipGenerator)(modifiedO); ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Render a DeckGL Heatmap (MapLibre) chart that uses this layer via the default export `createDeckGLComponent(getLayer, getPoints, getHighlightLayer)` in `superset-frontend/plugins/preset-chart-deckgl/src/layers/Heatmap/Heatmap.tsx:211`. 2. Ensure the underlying dataset has multiple rows falling into the same heatmap cell so that the HeatmapLayer aggregation (configured in `getLayer` at lines 99–159) produces an aggregated `weight` for that cell. 3. In the running app (Explore or a dashboard), hover the mouse over such an aggregated heatmap cell; Deck.gl passes a picking info object `o` whose `object` property contains the aggregated cell metrics into the tooltip handler created by `setTooltipContent(fd)` at line 136 and wired through `commonLayerProps` at lines 150–153. 4. Observe that inside the tooltip handler at lines 69–96, `closestPoint` is computed from `o.layer.props.data`, then `modifiedO.object` is set to `closestPoint || o.object` at lines 90–93; the `defaultTooltipGenerator` at lines 36–57 then reads `o.object?.weight` for the metric row, which now reflects the nearest raw point's weight instead of the aggregated cell weight, causing the tooltip metric to misrepresent the actual aggregated value visualized by the heatmap. ``` </details> <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/Heatmap/Heatmap.tsx **Line:** 90:95 **Comment:** *Logic Error: When building the heatmap tooltip, the code replaces the hovered cell's aggregated object with the nearest raw point object, so the metric row shows a single point's weight instead of the aggregated cell weight, which misrepresents the data; instead, the nearest point's fields should be merged into the existing object while preserving the aggregated 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%2F38035&comment_hash=ff2384b16ca491cc063ee829c446d9f4130c1d87964d3d5f13c2954ac45095d0&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38035&comment_hash=ff2384b16ca491cc063ee829c446d9f4130c1d87964d3d5f13c2954ac45095d0&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]
