aminghadersohi commented on PR #43569: URL: https://github.com/apache/superset/pull/43569#issuecomment-5489130161
@gkneighb Thanks again for the solid typed Treemap foundation. I need to elevate my earlier non-blocking note after tracing the full customer-facing MCP paths: for Red Hat’s Treemap requirement, registration + mapping alone still leaves materially incorrect behavior, so could you please extend this PR before we count Treemap as complete? 1. **Server-side ordering:** mirror the frontend `buildQuery` contract—when a row limit applies, order by the singular metric descending when `sort_by_metric=true`, then by the ordered hierarchy columns. Today MCP emits no `orderby`, so a limit can return arbitrary leaves. 2. **Treemap previews:** add a real shared `treemap_v2` spec for both saved/form-data-key and unsaved previews, using every ordered hierarchy level plus the resolved singular metric label. Current saved preview falls through to scatter; unsaved falls through to an incomplete bar. 3. **Native round-trip + updates:** adapt native `groupby` strings and saved/SIMPLE/SQL singular metric shapes back into `TreemapChartConfig`; expose/map the frontend Treemap presentation controls and preserve omitted saved values (including color scheme) in both update tools. 4. **Product-path validation:** normalize update-preview column casing and add tests through generate/update/update-preview/get-chart-data/preview—not only direct model/mapper/registry tests. At current head `86648530`, I ran the 16 Treemap tests, 435 focused shared tests, and the full MCP chart suite (1,383 passed); these gaps reproduce despite green CI. Shared query-error-envelope handling can coordinate with #43737 rather than be duplicated here, but the Treemap-specific behavior above needs coverage in this PR. I have not modified or pushed to your branch. -- 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]
