fix(heatmap): correct tooltip axis value lookup and percentage calculations and add tests - #41864
Conversation
Add comprehensive tests to ensure the tooltip formatter correctly displays actual axis values instead of numeric indices. This prevents regression of the bug fixed in the previous commit where tooltips showed "0 (1)" instead of actual labels like "Monday (Morning)". Tests cover: - Tooltip displays actual axis values with alphabetical sorting - Tooltip works correctly with different sort orders (asc/desc) - Tooltip works correctly with value-based sorting - Percentage calculations use actual values when normalizeAcross is enabled - Tooltip handles numeric axes correctly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…st metric and assertions Signed-off-by: yousoph <sophieyou12@gmail.com>
Code Review Agent Run #3b73dbActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| }); | ||
| const result = transformProps(chartProps as HeatmapChartProps); | ||
|
|
||
| const tooltipFormatter = (result.echartOptions.tooltip as any).formatter; |
There was a problem hiding this comment.
Suggestion: Replace this any cast on the tooltip option with a concrete tooltip/options type so formatter is accessed through typed properties. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The file still casts result.echartOptions.tooltip to any before accessing formatter, which directly violates the no-any-types rule for changed TypeScript code.
Rule source 📖
.cursor/rules/dev-standard.mdc (line 16)
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
**Line:** 370:370
**Comment:**
*Custom Rule: Replace this `any` cast on the tooltip option with a concrete tooltip/options type so `formatter` is accessed through typed properties.
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| testData, | ||
| ); | ||
| const resultX = transformProps(chartPropsX as HeatmapChartProps); | ||
| const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter; |
There was a problem hiding this comment.
Suggestion: Remove the any cast here and type the tooltip object explicitly to keep the formatter access type-safe. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The code uses an any cast on the tooltip object in new test logic, so the suggestion correctly identifies a real violation of the no-any-types rule.
Rule source 📖
.cursor/rules/dev-standard.mdc (line 16)
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
**Line:** 454:454
**Comment:**
*Custom Rule: Remove the `any` cast here and type the tooltip object explicitly to keep the formatter access type-safe.
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| testData, | ||
| ); | ||
| const resultY = transformProps(chartPropsY as HeatmapChartProps); | ||
| const tooltipFormatterY = (resultY.echartOptions.tooltip as any).formatter; |
There was a problem hiding this comment.
Suggestion: Use a concrete tooltip/options type instead of casting to any before accessing formatter. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
This line also casts to any in TypeScript test code, which is exactly what the no-any-types rule flags.
Rule source 📖
.cursor/rules/dev-standard.mdc (line 16)
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
**Line:** 477:477
**Comment:**
*Custom Rule: Use a concrete tooltip/options type instead of casting to `any` before accessing `formatter`.
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| numericData, | ||
| ); | ||
|
|
||
| (chartProps as any).queriesData[0].colnames = [ |
There was a problem hiding this comment.
Suggestion: Avoid casting chartProps to any; define the correct chart props/query data type and update colnames through typed fields. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The mutation explicitly casts chartProps to any, so this is a genuine instance of prohibited any usage in changed TypeScript code.
Rule source 📖
.cursor/rules/dev-standard.mdc (line 16)
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
**Line:** 508:508
**Comment:**
*Custom Rule: Avoid casting `chartProps` to `any`; define the correct chart props/query data type and update `colnames` through typed fields.
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41864 +/- ##
==========================================
+ Coverage 64.57% 64.63% +0.06%
==========================================
Files 2713 2712 -1
Lines 151269 151142 -127
Branches 34793 34761 -32
==========================================
+ Hits 97684 97697 +13
+ Misses 51760 51622 -138
+ Partials 1825 1823 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Happy to go through bot stuff if you'd like a hand getting this across the finish line. |
…ations and add tests (apache#41864) Signed-off-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* refactor(mcp): dedupe list-tool schemas and delete dead middleware (apache#41923) * fix(ag-grid-table): respect row limit with server pagination (apache#41346) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(table/pivot-table): correct non-additive totals/subtotals via DB rollup [SIP-216] (apache#41184) Co-authored-by: Superset Dev <dev@superset.apache.org> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> * feat(datasets): add RLS filter indicator badge to dataset list and explore view (apache#38807) Co-authored-by: Evan <evan@preset.io> * fix(chart): updates counties of kenya map (apache#38019) Co-authored-by: Zack Adams <zack@Zacks-Laptop.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Evan <evan@preset.io> Co-authored-by: Evan Rusackas <evan@rusackas.com> * chore(deps): bump actions/setup-go from 6.5.0 to 7.0.0 (apache#42303) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump nanoid from 5.0.9 to 6.0.0 in /superset-frontend (apache#42230) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Evan <evan@preset.io> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * test(dashboard): migrate dashboard load smoke test to Playwright (apache#41432) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(superset_app_root): when used with oauth (apache#38033) Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * refactor: engine enforce SQLAlchemy 2.0 (apache#42277) * fix(ag-grid-table): avoid ambiguous build query import (apache#42313) * fix: Revert "chore(deps): bump echarts from 5.6.0 to 6.1.0 in /superset-frontend" (apache#42314) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * adding circleci config * fix(plugin-chart-echarts): import the -obj locale build so time axes render (apache#42317) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(pandas_postprocessing): avoid FutureWarning for max/min in boxplot MINMAX (apache#42272) * fix(heatmap): correct tooltip axis value lookup and percentage calculations and add tests (apache#41864) Signed-off-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(forecast): resolve time grain robustly for Prophet forecasting (apache#42145) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix: Added PostgreSQL 17.X to the supported database versions table in (apache#42280) * fix(helm): add MCP HTTPRoute configuration (apache#42219) * fix(dashboard): offer Exit edit mode when there is nothing to discard (apache#42208) Co-authored-by: Claude Code <noreply@anthropic.com> * chore(deps): bump actions/labeler from 6.2.0 to 7.0.0 (apache#42332) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github/codeql-action/analyze from 4.37.0 to 4.37.1 (apache#42331) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump github/codeql-action/init from 4.37.0 to 4.37.1 (apache#42334) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump @formatjs/intl-durationformat from 0.10.17 to 0.10.18 in /superset-frontend (apache#42337) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump caniuse-lite from 1.0.30001805 to 1.0.30001806 in /docs (apache#42333) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump echarts to 6.1.0 with locale and containLabel guards (apache#42315) (apache#42321) Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> * docs: add pattern to the list of organisations using superset (apache#42341) * chore(deps): bump ag-grid from 36.0.0 to 36.0.1 in /superset-frontend (apache#42338) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> * fix(mcp): trust dataset is_dttm flag when applying time_grain to VARCHAR temporal columns (apache#42288) * chore(deps-dev): update taos-ws-py requirement from >=0.6.9 to >=0.7.0 (apache#42344) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump the storybook group in /superset-frontend with 5 updates (apache#42355) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(native-filters): keep filter value input caret at inline start (apache#42323) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(mcp): truncate query-tool responses instead of hard-failing (apache#42244) * chore(deps): bump nh3 from 0.3.5 to 0.3.6 (apache#42349) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump pydantic from 2.11.7 to 2.13.4 (apache#42350) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump sqlalchemy-continuum from 1.6.0 to 1.7.0 (apache#42351) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * fix(embedded): stop rejecting guest chart data built from control-specific params keys (apache#42295) Co-authored-by: Claude Code <noreply@anthropic.com> * fix(explore): show the beginning date on time-series x-axis line charts (apache#42046) * fix(api): add example to get_export_ids_schema so Swagger "Try it out" pre-fills a valid array (apache#42265) * chore: SQLAlchemy User cascade backref warnings are irrelevant (apache#42360) * fix(charts): handle async (202) chart-data responses in StatefulChart (apache#42157) Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Evan Rusackas <evan@preset.io> * feat(KustoKQL): Add support for NULL / IS NOT NULL operator (apache#37890) Co-authored-by: ag-ramachandran <ramacg@microsoft.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump pillow from 12.2.0 to 12.3.0 (apache#42348) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps): bump flask-compress from 1.17 to 1.24 (apache#42346) Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> * chore(deps-dev): bump databricks-sql-connector from 4.2.6 to 4.3.0 (apache#42347) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Evan Rusackas <evan@preset.io> * fix(native-filters): use FILTER_STATE_CACHE_CONFIG timeout for dynamic filter option queries (apache#38910) * fix(explore): render Jinja before validating legacy chart filters (apache#41996) * fix(dataset): disable duplicate button when name is empty (apache#42217) Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local> Co-authored-by: Evan Rusackas <evan@rusackas.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(gunicorn): correct dead links in values.yaml (apache#42385) * chore(deps): bump @deck.gl/mapbox from 9.3.6 to 9.3.7 in /superset-frontend in the deckgl group (apache#42377) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump actions/checkout from 7.0.0 to 7.0.1 (apache#42376) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump immer from 11.1.11 to 11.1.15 in /superset-frontend (apache#42378) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump body-parser from 1.20.5 to 1.20.6 in /docs (apache#42370) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps-dev): bump brace-expansion from 1.1.15 to 1.1.16 in /superset-embedded-sdk (apache#42369) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(security): bump pyasn1 from 0.6.3 to 0.6.4 (apache#42363) * fix(security): bump pillow from 12.2.0 to 12.3.0 (apache#42362) * docs(map-tiles): add Yandex Maps Tiles API configuration (apache#42375) * ci: improve conditional checks for lillio tests and build --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: hainenber <dotronghai96@gmail.com> Signed-off-by: Grégoire Bellon-Gervais <gregoire.bellon-gervais@docaposte.fr> Signed-off-by: yousoph <sophieyou12@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Superset Dev <dev@superset.apache.org> Co-authored-by: SkinnyPigeon <e.blackledge@stuart.com> Co-authored-by: Zack <adams.z.d@gmail.com> Co-authored-by: Zack Adams <zack@Zacks-Laptop.local> Co-authored-by: Evan Rusackas <evan@rusackas.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: hainenber <dotronghai96@gmail.com> Co-authored-by: Joe Li <joe@preset.io> Co-authored-by: Grégoire <greggbg@gmail.com> Co-authored-by: Hans Yu <hans.yu@outlook.de> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Amitesh Gupta <143833521+singlaamitesh@users.noreply.github.com> Co-authored-by: David <39565245+dmunozv04@users.noreply.github.com> Co-authored-by: Yash Shrivastava <119301033+alephys26@users.noreply.github.com> Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: jesperct <jmilecelento@gmail.com> Co-authored-by: Abdul Rehman <76230556+Abdulrehman-PIAIC80387@users.noreply.github.com> Co-authored-by: jenwitteng <jenwit.amonpongitsara@agoda.com> Co-authored-by: Ramachandran A G <106139410+ag-ramachandran@users.noreply.github.com> Co-authored-by: ag-ramachandran <ramacg@microsoft.com> Co-authored-by: Ujjwal Jain <jainujjwal1609@gmail.com> Co-authored-by: Jean Massucatto <massucattoj@gmail.com> Co-authored-by: suvankardas216 <rohanrohan510@gmail.com> Co-authored-by: AS-MAC-1123 <as-mac-1123@AS-MAC-1123.local> Co-authored-by: Alejandro Solares <219859296+ASolarers-Rodriguez@users.noreply.github.com> Co-authored-by: ViktorGo86 <114023094+ViktorGo86@users.noreply.github.com>
User description
SUMMARY
Fixes the heatmap tooltip so it displays actual axis values (e.g., "Monday", "Morning") instead of numeric indices (0, 1, 2...), corrects the tooltip percentage calculation, and adds comprehensive regression tests.
Context: After PR #36302 changed the heatmap data structure to use axis indices for proper sorting, the tooltip formatter was not updated to look up actual values from the sorted arrays. This caused tooltips to display raw indices instead of formatted axis labels.
What this PR does:
calculateTotalsoff the querycolnames(the real column names present in the data) instead ofgetColumnLabel(...)— the old keying producedNaNpercentages whenever the derived label diverged from the data columns (notably arraygroupby)normalizeAcrossis enabledWhy these tests matter:
These tests prevent future regressions by verifying the tooltip formatter correctly:
sortedXAxisValuesandsortedYAxisValuesarraystotals.x[xValue]andtotals.y[yValue]BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — the change affects tooltip text/percentage values (indices → axis labels,
NaN%→ correct %), which are covered by the added regression tests.TESTING INSTRUCTIONS
CodeAnt-AI Description
Fix heatmap tooltip to show actual axis labels and correct percentage calculations
What Changed
Impact
✅ Clearer heatmap tooltips✅ Correct heatmap percentage values✅ Fewer tooltip regressions💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.