aminghadersohi commented on PR #43571:
URL: https://github.com/apache/superset/pull/43571#issuecomment-5620477172
Re-reviewed **e08d75c5d5aa66331791ee9df294cdcb387be2e5** against 29f350b.
Confirmed the shared-query ordering fix and the radar/spider discovery
documentation. Product-completeness gaps remain:
- **Ordering is only partially fixed.** Public
`generate_chart(save_chart=False, preview_formats=["vega_lite"])` still sends
`orderby=[]` to both compile and preview queries: `compile.py:145` and
`preview_utils.py:108` read `orderby`, not `sort_by_metric`. Public
`get_chart_data` uses the shared builder and orders correctly for MCP-generated
radar configs. Native radar form data without that flag still lacks ordering,
including its `series_limit_metric` override.
- **Deferred round-trip/preservation and preview gaps reproduce.** The
mapper's native output fails generate/update/update-preview request validation.
Public saved `update_chart` and cached `update_chart_preview` drop native
bounds/`column_config`, `shape`, and `series_limit_metric`. Generate's Vega
preview is a first-metric bar; saved and unsaved `get_chart_preview` fall back
to scatter, not radar. Shared origins do not make these paths chart-faithful;
an explicit unsupported-preview response would avoid misrepresentation.
- **Public preview validation still accepts invalid effective
metrics/casing.** `update_chart_preview` reports success for
`metrics=[{"name":"Model"}]` on VARCHAR and emits `SUM(Model)` without a
compile check. It also accepts lowercase `speed`/`model` but retains them
instead of canonical `Speed`/`Model`, despite the plugin's normalization hook
working independently.
- Schema discovery, explicit text-SUM rejection, and dimension-role checks
pass. Radar still has no recommendation candidate. One metric is accepted,
while missing-metric guidance still says “two or more”; align that contract.
**Validation:** 501 existing focused tests passed (including 17 radar
tests); 28 temporary probes confirmed the above, including public tool-function
paths with mocked external boundaries. No live Superset server was available.
CI checks are successful/skipped/neutral, but GitHub reports **CONFLICTING /
DIRTY** against the current base.
--
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]
