aminghadersohi commented on PR #43573: URL: https://github.com/apache/superset/pull/43573#issuecomment-5496889029
@gkneighb Thanks for fixing the original Sankey grouping and shared-builder ordering issues. A fresh Red Hat product-completeness pass at head `6a86a0a3` found the typed foundation is still incomplete in the end-to-end MCP paths. Could you please address or explicitly coordinate these before we count Sankey support as complete? 1. **Unsaved query + previews:** `generate_chart(save_chart=False)` still bypasses the Sankey-aware shared query builder, so limited previews can use unordered edges. Saved previews fall through to scatter and unsaved previews to an incomplete bar; add a shared Sankey representation using source, target, and the resolved saved/SIMPLE/SQL metric label, with all-row validation—or return a clear unsupported-format error rather than a false chart. 2. **Native round-trip and updates:** adapt returned native string dimensions, derived `groupby`, and saved/SIMPLE/SQL metric objects back into `SankeyChartConfig`. Preserve omitted `sort_by_metric`, `row_limit`, color, filters, and UI state across immediate, preview-first, and cached updates; explicit values/empty filters should still override. 3. **Sankey semantics:** require finite numeric metric output (including saved/SQL metrics), reject canonically identical or case-ambiguous source/target roles, and normalize casing before every tool path maps form data. 4. **Discovery:** add a real Sankey recommendation candidate for a two-categorical-plus-numeric result, not only the current-viz exclusion mapping. 5. **Product-path tests:** cover generate/update/update-preview/get-data plus saved/form-data-key and unsaved previews, including error/empty/malformed envelopes and row-limit ordering. I ran the 21 Sankey tests and the full MCP chart suite (1,388 passed) plus adversarial reproductions; these gaps remain despite green CI. Shared builder, normalization, and query-error work in #43737 should be reused/rebased rather than duplicated, while retaining this PR’s Sankey ordering behavior. I did not modify or push 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]
