aminghadersohi commented on code in PR #43771:
URL: https://github.com/apache/superset/pull/43771#discussion_r4206811004
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -2158,6 +3048,56 @@ def map_mixed_timeseries_config(
_add_adhoc_filters(form_data, config.filters)
+ fields_set = config.model_fields_set
+ if "adhoc_filters_b" in fields_set:
+ form_data["adhoc_filters_b"] = config.adhoc_filters_b
+ elif "filters_secondary" in fields_set:
+ form_data["adhoc_filters_b"] = [
Review Comment:
Fixed in 31bff3652ae76ff7db15a230ad8eeeb9563d2fdd.
Mixed secondary filters now participate in column-reference extraction, the
same physical-column filter validation as primary filters, and canonical-name
normalization before adhoc_filters_b mapping.
test_mixed_secondary_filters_validate_and_normalize rejects regoin and a
saved-metric-only name, normalizes REGION to region, and checks the resulting
Query B filter without leaking it into Query A. All three cases failed before
and pass after.
Validation: MCP/common unit suites: 9,198 passed, 4 skipped; touched
modules: 343 passed plus 351 Sunburst tests passed. Pre-commit passed on all 83
branch-changed files, including mypy.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2627,6 +3810,37 @@ class XYChartConfig(BaseChartConfig):
le=10000,
)
+ series_limit_metric: ColumnRef | None = Field(
+ None,
+ description="Metric used to rank limited series; null clears it",
+ )
+ timeseries_limit_metric: ColumnRef | None = Field(
+ None,
+ description="Native timeseries ranking metric; null clears it",
+ )
+
+ @field_validator("series_limit_metric", "timeseries_limit_metric",
mode="before")
+ @classmethod
+ def coerce_series_ranking_metric(cls, value: Any) -> Any:
+ """Accept the same native metric references as the Y-axis."""
+ if isinstance(value, dict) and "expressionType" in value:
Review Comment:
Fixed in 31bff3652ae76ff7db15a230ad8eeeb9563d2fdd.
Both series_limit_metric and timeseries_limit_metric pass saved-metric
strings through _coerce_native_metric, just like native metrics entries.
test_xy_native_saved_ranking_metric_round_trips covers both spellings through
the native GenerateChartRequest payload, verifies saved_metric semantics, and
maps the ranking metric back to count. Both cases failed before and pass after;
dimension-reference rejection remains covered.
Validation: MCP/common unit suites: 9,198 passed, 4 skipped; touched
modules: 343 passed plus 351 Sunburst tests passed. Pre-commit passed on all 83
branch-changed files, including mypy.
##########
tests/unit_tests/mcp_service/chart/test_preview_utils.py:
##########
@@ -154,11 +172,41 @@ def
test_build_query_columns_empty_columns_key_keeps_groupby():
) == ["country"]
+def test_generate_preview_seeds_form_data_before_query_execution():
+ """Preview execution seeds the form data consumed by virtual-dataset
Jinja."""
+ with (
+ patch(
+ "superset.charts.data.form_data.set_query_context_form_data"
+ ) as mock_set_form_data,
+ patch(
+ "superset.commands.chart.data.get_data_command.ChartDataCommand"
+ ) as mock_cmd_cls,
+ patch(
+ "superset.common.query_context_factory.QueryContextFactory"
+ ) as mock_factory,
+ patch("superset.extensions.db") as mock_db,
+ ):
+ mock_db.session.get.return_value = MagicMock(id=12)
+ query_context = MagicMock()
+ mock_factory.return_value.create.return_value = query_context
+ mock_cmd_cls.return_value.run.return_value = chart_data_command_result(
+ rows=[], columns=["value"]
+ )
+
+ preview_utils.generate_preview_from_form_data(
+ form_data={"metrics": [{"label": "count"}]},
+ dataset_id=12,
+ preview_format="table",
+ )
+
+ mock_set_form_data.assert_called_once_with(query_context, 12, "table")
Review Comment:
Fixed in 31bff3652ae76ff7db15a230ad8eeeb9563d2fdd.
The unsaved-preview regression no longer mocks the seeder. At
ChartDataCommand construction it reads the real
ExtraCache.filter_values("region") and url_param("tenant") macros, checks
EMEA/acme instead of deliberately stale inputs, and then asserts a successful
one-row TablePreview containing count=2. The query-builder/command boundaries
remain isolated. As a mutation check, moving real seeding after command.run()
makes this strengthened test fail; restoring the correct order passes.
Validation: MCP/common unit suites: 9,198 passed, 4 skipped; touched
modules: 343 passed plus 351 Sunburst tests passed. Pre-commit passed on all 83
branch-changed files, including mypy.
##########
docs/docs/using-superset/using-ai-with-superset.mdx:
##########
@@ -461,6 +472,24 @@ also have `data: null`. For each statement, `executed_sql`
is `null` when it equ
- Restart your AI client if you recently changed the configuration
+### Sunburst chart data
+
+MCP chart tools support ordered Sunburst hierarchies with a primary metric and
+an optional secondary metric. When supplied, the secondary-to-primary ratio
+drives a sequential color scale (for example, profit divided by revenue shows
+margin); when omitted, arc colors are categorical. Use `chart_type: "sunburst"`
+in typed requests; the native chart uses `viz_type: "sunburst_v2"`.
+
+SQL NULL aggregate values are rendered as zero, matching the Sunburst frontend,
+including during compile checks, previews, and data exports. Missing result
+columns, numeric strings, booleans, and infinite metric values remain invalid.
Review Comment:
Fixed in 31bff3652ae76ff7db15a230ad8eeeb9563d2fdd.
The Sunburst documentation explicitly describes non-finite floating-point
metrics becoming null and then zero during compile checks, previews, and
exports, and warns that an overflowing aggregate can appear as zero rather than
a validation error. Runtime normalization is unchanged.
test_sunburst_docs_warn_about_nonfinite_metric_normalization fails against the
previous documentation and passes with this wording; the existing compile
boundary tests also pass.
Validation: MCP/common unit suites: 9,198 passed, 4 skipped; touched
modules: 343 passed plus 351 Sunburst tests passed. Pre-commit passed on all 83
branch-changed files, including mypy.
--
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]
