codeant-ai-for-open-source[bot] commented on code in PR #41341:
URL: https://github.com/apache/superset/pull/41341#discussion_r3541417516
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -143,7 +147,9 @@ function buildFieldBasedTooltipItems(
if (!label || !fieldName) return;
let { value } = extractValue(o, fieldName);
- if (!value && item.item_type === 'metric') {
+ // we use an empty string as sentinal for "missing value"
Review Comment:
**Suggestion:** There is an obvious misspelling in the inline comment
(`sentinal`), which should be corrected to `sentinel` to avoid confusion and
keep code comments clear. [typo]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Comment typo slightly reduces maintainability and clarity.
- โ ๏ธ Suggestion purely cosmetic; no functional impact.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Inspect the tooltip value handling code in
`superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:149-153`,
where `extractValue` results are adjusted for metric items.
2. Observe the inline comment at line `150` reading `// we use an empty
string as sentinal
for "missing value"`, containing the misspelling `sentinal` instead of
`sentinel`.
3. Note that this typo has no runtime effect but slightly reduces comment
clarity for
future maintainers reviewing tooltip value semantics.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e5539e459e0847cd8773a57bc9a727b4&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=e5539e459e0847cd8773a57bc9a727b4&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/utilities/tooltipUtils.tsx
**Line:** 150:150
**Comment:**
*Typo: There is an obvious misspelling in the inline comment
(`sentinal`), which should be corrected to `sentinel` to avoid confusion and
keep code comments clear.
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%2F41341&comment_hash=af0c5f36a06c17838218acc99c0462ac161cbd4299299ba31d85130767aaeb8a&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41341&comment_hash=af0c5f36a06c17838218acc99c0462ac161cbd4299299ba31d85130767aaeb8a&reaction=dislike'>๐</a>
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -170,6 +176,46 @@ function buildFieldBasedTooltipItems(
}
});
+ // For example, in geojson polygon charts, users can only pick columns as
tooltip_contents.
+ // We still include the configured metric, so hover also shows the
aggregation value.
+ // formData.metric represents one configured metric; but depending on its
type,
+ // it might be represented differently:
+ // 1. String metric (legacy/simple)
+ // sum__value
+ // 2. Select option style object
+ // { label: SUM(value), value: sum__value }
+ // 3. Adhoc metric style object
+ // { label: ..., value: ... (optional), expressionType: ..., ... }
+ const metricAliasCandidates: string[] = [];
+ if (typeof formData.metric === 'string') {
+ metricAliasCandidates.push(formData.metric);
+ } else if (formData.metric) {
+ const metric = formData.metric as { label?: string; value?: string };
+ if (metric.label) metricAliasCandidates.push(metric.label);
+ if (metric.value) metricAliasCandidates.push(metric.value);
+ }
+ const foundMetricAlias = metricAliasCandidates[0] || '';
+
+ const hasSelectedMetricItem = formData.tooltip_contents.some(
+ (item: string | { item_type?: string }) =>
+ (item && typeof item === 'object' && item.item_type === 'metric') ||
+ (typeof item === 'string' && metricAliasCandidates.includes(item)),
+ );
+
+ if (!hasSelectedMetricItem && foundMetricAlias) {
+ const { value } = extractValue(o, foundMetricAlias, false);
+ const metricValue = value === '' ? (o.object?.metric ?? '') : value;
+ if (metricValue !== '') {
+ tooltipItems.push(
+ <TooltipRow
+ key="tooltip-configured-metric"
+ label={`${foundMetricAlias}: `}
+ value={formatValue(metricValue)}
+ />,
+ );
Review Comment:
**Suggestion:** This new metric-appending block runs for every deck.gl chart
that uses shared tooltip rendering, not just polygon charts. That means charts
where users can intentionally omit metrics from `tooltip_contents` will now
still show a metric row, changing existing tooltip behavior and violating
user-selected tooltip content. Restrict this logic to polygon/geojson polygon
viz types (or to the specific charts that cannot select metric items) before
appending. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Non-polygon deck.gl charts show unintended metric rows.
- โ ๏ธ User-selected tooltip_contents ignored when metric omitted.
- โ ๏ธ Tooltip behavior inconsistent across deck.gl visualizations.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open the deck.gl Path chart implementation at
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Path/Path.tsx:2-11`,
where
`setTooltipContent(formData)` returns `createTooltipContent(formData,
defaultTooltipGenerator)`.
2. Note that the Path control panel config at
`superset-frontend/plugins/preset-chart-deckgl/src/layers/Path/controlPanel.ts:20-44`
wires the shared `tooltipContents` control from `Shared_DeckGL.tsx:16-48`,
allowing users
to drop both columns and metrics into `formData.tooltip_contents`.
3. Configure a Path chart (viz_type `deck_path`) so that `formData.metric`
is set (as
exercised in `Path/transformProps.test.ts` where `rawFormData.metric` is
populated) but
`formData.tooltip_contents` only contains a dimension (e.g., `['region']`)
and
intentionally excludes any metric items or string aliases.
4. When hovering a path, the tooltip generator in `tooltipUtils.tsx:402-425`
calls
`buildFieldBasedTooltipItems(o, formData)` at lines `385-386`; after
iterating
user-selected items, the metric appending block at `189-215` runs, finds
`foundMetricAlias` from `formData.metric`, detects `!hasSelectedMetricItem`,
and pushes a
`TooltipRow` for the configured metric, so the tooltip shows an extra metric
row even
though the user omitted metrics from `tooltip_contents`, altering tooltip
behavior for
this non-polygon chart.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=baefde9c89ea417d98c05286261accc9&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=baefde9c89ea417d98c05286261accc9&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/utilities/tooltipUtils.tsx
**Line:** 189:215
**Comment:**
*Logic Error: This new metric-appending block runs for every deck.gl
chart that uses shared tooltip rendering, not just polygon charts. That means
charts where users can intentionally omit metrics from `tooltip_contents` will
now still show a metric row, changing existing tooltip behavior and violating
user-selected tooltip content. Restrict this logic to polygon/geojson polygon
viz types (or to the specific charts that cannot select metric items) before
appending.
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%2F41341&comment_hash=e24fa8f53778a479f73608127399cb617febb20f53492dc97987667384809ee5&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41341&comment_hash=e24fa8f53778a479f73608127399cb617febb20f53492dc97987667384809ee5&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]
