aminghadersohi commented on code in PR #43770: URL: https://github.com/apache/superset/pull/43770#discussion_r4149960129
########## tests/unit_tests/mcp_service/chart/test_bullet_chart.py: ########## @@ -0,0 +1,3786 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Product-path coverage for typed ECharts Bullet MCP support.""" + +import math +from datetime import date, datetime, time, timedelta, timezone, tzinfo +from decimal import Decimal +from enum import Enum, IntEnum, StrEnum +from types import SimpleNamespace +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch +from uuid import UUID +from zoneinfo import ZoneInfo + +import numpy as np +import pandas as pd +import pytest +import pytz +from dateutil import tz as dateutil_tz +from dateutil.zoneinfo import get_zonefile_instance +from pydantic import TypeAdapter, ValidationError + +from superset.mcp_service.chart.chart_helpers import ( + build_query_dicts_from_form_data, +) +from superset.mcp_service.chart.chart_utils import ( + analyze_chart_capabilities, + map_bullet_config, + map_config_to_form_data, + MCP_DASHBOARD_TIME_FILTER_SUBJECT, + merge_update_form_data, + validate_merged_bullet_form_data, +) +from superset.mcp_service.chart.compile import _compile_chart +from superset.mcp_service.chart.plugins.bullet import BulletChartPlugin +from superset.mcp_service.chart.preview_utils import ( + _generate_ascii_preview_from_data, + _generate_vega_lite_preview_from_data, + _javascript_number_string, + BulletOutputError, + generate_preview_from_form_data, + resolve_bullet_render_model, +) +from superset.mcp_service.chart.query_result import ( + _chart_data_duration_text, + _chart_data_temporal_number, +) +from superset.mcp_service.chart.schemas import ( + ASCIIPreview, + BulletChartConfig, + ChartConfig, + ChartError, + ChartInfo, + DataColumn, + GenerateChartRequest, + GetChartPreviewRequest, + UpdateChartPreviewRequest, + UpdateChartRequest, + VegaLitePreview, + XYChartConfig, +) +from superset.mcp_service.chart.tool.generate_chart import generate_chart +from superset.mcp_service.chart.tool.get_chart_data import ( + _candidates_single_numeric, + _VIZ_CATEGORY, +) +from superset.mcp_service.chart.tool.get_chart_preview import ( + ASCIIPreviewStrategy, + TablePreviewStrategy, + VegaLitePreviewStrategy, +) +from superset.mcp_service.chart.tool.get_chart_type_schema import ( + _get_chart_type_schema_impl, + VALID_CHART_TYPES, +) +from superset.mcp_service.chart.tool.update_chart import ( + _build_preview_form_data, + _build_update_payload, + update_chart, +) +from superset.mcp_service.chart.tool.update_chart_preview import update_chart_preview +from superset.mcp_service.chart.validation.dataset_validator import ( + AmbiguousDatasetReferenceError, + DatasetValidator, +) +from superset.mcp_service.chart.validation.pipeline import ValidationPipeline +from superset.mcp_service.common.error_schemas import DatasetContext +from superset.utils.json import json_int_dttm_ser + + +def _reject_scalar_conversion(*_args: object, **_kwargs: object) -> Any: + raise AssertionError("hostile query scalar method must not run") + + +class _PathHostileStr(str): + __getitem__ = _reject_scalar_conversion + __str__ = _reject_scalar_conversion + + +class _PathHostileEnum(str, Enum): + FAILED = "warehouse unavailable" + + @property + def value(self) -> str: + """Reject the public descriptor while preserving Enum's stored value.""" + return _reject_scalar_conversion() + + __getitem__ = _reject_scalar_conversion + __str__ = _reject_scalar_conversion + + +class _OutputHostileInt(int): + __float__ = _reject_scalar_conversion + __str__ = _reject_scalar_conversion + + +class _OutputHostileFloat(float): + __float__ = _reject_scalar_conversion + __str__ = _reject_scalar_conversion + + +class _OutputHostileStr(str): + __str__ = _reject_scalar_conversion + strip = _reject_scalar_conversion + + +class _OutputHostileDecimal(Decimal): + __float__ = _reject_scalar_conversion + __str__ = _reject_scalar_conversion + + +class _OutputSafeIntEnum(IntEnum): + VALUE = 12 + + +class _OutputSafeStrEnum(StrEnum): + VALUE = "12.5" + + +_OutputSafeIntEnum.__float__ = _reject_scalar_conversion # type: ignore[method-assign] +_OutputSafeIntEnum.__str__ = _reject_scalar_conversion # type: ignore[method-assign] +_OutputSafeStrEnum.__float__ = _reject_scalar_conversion # type: ignore[attr-defined] +_OutputSafeStrEnum.__str__ = _reject_scalar_conversion # type: ignore[method-assign] + + +def _simple_metric(name: str = "revenue") -> dict[str, str]: + return {"name": name, "aggregate": "SUM"} + + +def _tool_user() -> SimpleNamespace: + return SimpleNamespace(id=1, username="admin", roles=[], groups=[]) + + +def _orm_dataset() -> SimpleNamespace: + def column( + name: str, type_: str, *, temporal: bool = False, numeric: bool = False + ) -> SimpleNamespace: + return SimpleNamespace( + column_name=name, + type=type_, + is_temporal=temporal, + is_numeric=numeric, + is_dttm=temporal, + python_date_format=None, + ) + + return SimpleNamespace( + id=7, + table_name="sales", + schema=None, + main_dttm_col="OrderDate", + database=SimpleNamespace(database_name="main", db_engine_spec=None), + columns=[ + column("Revenue", "NUMERIC", numeric=True), + column("Region", "VARCHAR"), + column("Team", "VARCHAR"), + column("Status", "VARCHAR"), + column("OrderDate", "TIMESTAMP", temporal=True), + column("EventDate", "TIMESTAMP", temporal=True), + ], + metrics=[ + SimpleNamespace( + metric_name="SavedRevenue", + expression="SUM(Revenue)", + description=None, + ) + ], + ) + + +def test_bullet_discriminated_union_uses_exact_tag() -> None: + config = TypeAdapter(ChartConfig).validate_python( + {"chart_type": "bullet", "metric": _simple_metric()} + ) + assert isinstance(config, BulletChartConfig) + with pytest.raises(ValidationError): + TypeAdapter(ChartConfig).validate_python( + {"chart_type": "bullet_chart", "metric": _simple_metric()} + ) + + +def test_bullet_equal_dimension_aliases_are_order_independent_and_round_trip() -> None: + for payload in ( + { + "dimensions": [{"name": "Region", "label": "Market"}, "Team"], + "groupby": ["Region", {"column_name": "Team"}], + }, + { + "groupby": ["Region", {"column": "Team"}], + "dimensions": [{"name": "Region", "label": "Market"}, "Team"], + }, + ): + config = BulletChartConfig.model_validate( + {"metric": _simple_metric(), **payload} + ) + assert [dimension.name for dimension in config.dimensions or []] == [ + "Region", + "Team", + ] + mapped = map_bullet_config(config) + assert mapped["groupby"] == ["Region", "Team"] + round_trip = BulletChartConfig.model_validate(mapped) + assert [dimension.name for dimension in round_trip.dimensions or []] == [ + "Region", + "Team", + ] + + +def test_bullet_dimension_aliases_require_exact_physical_identity() -> None: + """Schema-only alias resolution must not merge quoted case variants.""" + with pytest.raises(ValidationError, match="Conflicting Bullet dimension aliases"): + BulletChartConfig.model_validate( + { + "metric": _simple_metric(), + "dimensions": ["Region"], + "groupby": ["region"], + } + ) + + [email protected]( + "request_payload", + [ + {"dataset_id": 7}, + {"identifier": 9}, + {"dataset_id": 7, "form_data_key": "preview"}, + ], +) [email protected]("reverse", [False, True]) +def test_bullet_request_models_reject_conflicting_dimension_aliases( + request_payload: dict[str, object], reverse: bool +) -> None: + aliases = [ + ("dimensions", [{"name": "Region"}, {"name": "Team"}]), + ("groupby", ["Team", "Region"]), + ] + if reverse: + aliases.reverse() + config = {"chart_type": "bullet", "metric": _simple_metric(), **dict(aliases)} + payload = {**request_payload, "config": config} + request_type = ( + UpdateChartRequest + if "identifier" in request_payload + else ( + UpdateChartPreviewRequest + if "form_data_key" in request_payload + else GenerateChartRequest + ) + ) + with pytest.raises(ValidationError, match="Conflicting Bullet dimension aliases"): + request_type.model_validate(payload) + + [email protected]( + "metric", + [ + {"name": "revenue", "aggregate": "SUM", "label": "Revenue"}, + {"name": "saved_revenue", "saved_metric": True}, + {"sql_expression": "SUM(revenue) / COUNT(*)", "label": "Average"}, + ], +) +def test_bullet_accepts_simple_saved_and_sql_metrics(metric: dict[str, object]) -> None: + config = BulletChartConfig(metric=metric) + form_data = map_bullet_config(config) + assert form_data["viz_type"] == "bullet" + assert form_data["metric"] + + +def test_bullet_native_form_data_round_trip_is_semantically_stable() -> None: + native = { + "viz_type": "bullet", + "datasource": "7__table", + "metric": { + "aggregate": "SUM", + "column": {"column_name": "Revenue"}, + "expressionType": "SIMPLE", + "label": "Total Revenue", + }, + "groupby": ["Region", "Team"], + "ranges": "100,250,500", + "range_labels": "Minimum,Target,Stretch", + "markers": "300", + "marker_labels": "Plan", + "marker_lines": "400", + "marker_line_labels": "Forecast", + "y_axis_format": "$,.0f", + "show_labels": False, + "show_legend": True, + "row_limit": 250, + "orderby": [["Region", True], ["Total Revenue", False]], + "adhoc_filters": [ + { + "clause": "WHERE", + "expressionType": "SIMPLE", + "subject": "Status", + "operator": "==", + "comparator": "Active", + } + ], + } + config = BulletChartConfig.model_validate(native) + mapped = map_bullet_config(config) + + assert mapped["metric"]["label"] == "Total Revenue" + assert mapped["groupby"] == ["Region", "Team"] + assert mapped["ranges"] == "100,250,500" + assert mapped["range_labels"] == "Minimum,Target,Stretch" + assert mapped["markers"] == "300" + assert mapped["marker_lines"] == "400" + assert mapped["show_labels"] is False + assert mapped["show_legend"] is True + assert mapped["orderby"][0] == ["Region", True] + assert mapped["orderby"][1][0]["label"] == "Total Revenue" + assert mapped["adhoc_filters"][0]["subject"] == "Status" + + +def test_bullet_presentation_numbers_use_shortest_round_trip_safe_tokens() -> None: + ranges = [1.2345678901234567, 1.7976931348623157e308] + markers = [5e-324, -0.0] + marker_lines = [9.876543210987654e-200] + config = BulletChartConfig( + metric=_simple_metric(), + ranges=ranges, + markers=markers, + marker_lines=marker_lines, + show_legend=True, + ) + mapped = map_bullet_config(config) + + for key, expected in ( + ("ranges", ranges), + ("markers", markers), + ("marker_lines", marker_lines), + ): + tokens = mapped[key].split(",") + assert [float(token) for token in tokens] == expected + assert all( + float(token).hex() == value.hex() + for token, value in zip(tokens, expected, strict=True) + ) + + round_trip = BulletChartConfig.model_validate(mapped) + assert round_trip.ranges == ranges + assert round_trip.markers == markers + assert round_trip.marker_lines == marker_lines + + model = resolve_bullet_render_model( + [{"SUM(revenue)": 1.0}], + mapped, + ) + assert model.ranges == ranges + assert model.markers == markers + assert model.marker_lines == marker_lines + assert ( + "1.7976931348623157e+308" + in _generate_ascii_preview_from_data( + [{"SUM(revenue)": 1.0}], mapped + ).ascii_content + ) + vega = _generate_vega_lite_preview_from_data([{"SUM(revenue)": 1.0}], mapped) + assert vega.specification["layer"] + + +def test_bullet_native_saved_metric_and_legacy_metric_aliases() -> None: + saved = BulletChartConfig.model_validate( + {"viz_type": "bullet", "metric": "saved_revenue"} + ) + legacy = BulletChartConfig.model_validate( + {"viz_type": "bullet", "metric": "sum__revenue"} + ) + assert saved.metric.saved_metric is True + assert saved.metric.name == "saved_revenue" + assert legacy.metric.name == "sum__revenue" + assert legacy.metric.saved_metric is True + + [email protected]("metric_name", ["sum__num", "sum__SP_POP_TOTL"]) [email protected]( + ("request_type", "request_fields"), + [ + (GenerateChartRequest, {"dataset_id": 7}), + (UpdateChartRequest, {"identifier": 9}), + (UpdateChartPreviewRequest, {"dataset_id": 7}), + ], +) +def test_bullet_repository_metric_names_round_trip_as_saved_metrics_on_all_requests( + metric_name: str, + request_type: type[ + GenerateChartRequest | UpdateChartRequest | UpdateChartPreviewRequest + ], + request_fields: dict[str, object], +) -> None: + request = request_type.model_validate( + {**request_fields, "config": {"chart_type": "bullet", "metric": metric_name}} + ) + config = request.config + assert isinstance(config, BulletChartConfig) + assert config.metric.saved_metric is True + assert map_bullet_config(config)["metric"] == metric_name + + [email protected]( + ("native_operator", "typed_operator", "round_trip_operator"), + [ + ("EQUALS", "=", "=="), + ("NOT_EQUALS", "!=", "!="), + ("LESS_THAN", "<", "<"), + ("LESS_THAN_OR_EQUAL", "<=", "<="), + ("GREATER_THAN", ">", ">"), + ("GREATER_THAN_OR_EQUAL", ">=", ">="), + ("IN", "IN", "IN"), + ("NOT_IN", "NOT IN", "NOT IN"), + ("LIKE", "LIKE", "LIKE"), + ("ILIKE", "ILIKE", "ILIKE"), + ("IS_NULL", "IS NULL", "IS NULL"), + ("IS_NOT_NULL", "IS NOT NULL", "IS NOT NULL"), + ], +) +def test_bullet_native_filter_operator_names_round_trip( + native_operator: str, typed_operator: str, round_trip_operator: str +) -> None: + comparator: object + if native_operator.startswith("IS_"): + comparator = None + elif native_operator in {"IN", "NOT_IN"}: + comparator = ["North"] + else: + comparator = "North" + config = BulletChartConfig.model_validate( + { + "viz_type": "bullet", + "metric": "SavedRevenue", + "adhoc_filters": [ + { + "clause": "WHERE", + "expressionType": "SIMPLE", + "subject": "Region", + "operator": native_operator, + "comparator": comparator, + } + ], + } + ) + + assert config.filters is not None + assert config.filters[0].op == typed_operator + mapped = map_bullet_config(config) + assert mapped["adhoc_filters"][0]["operator"] == round_trip_operator + + +def test_bullet_legacy_label_only_saved_metric_adapter_is_strict_and_bounded() -> None: + config = BulletChartConfig.model_validate( + {"viz_type": "bullet", "metric": {"label": "sum__num"}} + ) + assert config.metric.saved_metric is True + assert map_bullet_config(config)["metric"] == "sum__num" + + with pytest.raises(ValidationError): + BulletChartConfig.model_validate( + { + "viz_type": "bullet", + "metric": {"label": "sum__num", "aggregate": "SUM"}, + } + ) + with pytest.raises(ValidationError, match="at most 255"): + BulletChartConfig.model_validate( + {"viz_type": "bullet", "metric": {"label": "m" * 256}} + ) + + [email protected]( + "metric", + [ + "SavedRevenue", + { + "aggregate": "SUM", + "column": {"column_name": "Revenue"}, + "expressionType": "SIMPLE", + "label": "Simple Revenue", + }, + { + "aggregate": None, + "column": None, + "expressionType": "SQL", + "sqlExpression": "SUM(Revenue)", + "label": "SQL Revenue", + }, + ], +) +def test_bullet_all_metric_shapes_round_trip_full_native_presentation( + metric: object, +) -> None: + native = { + "viz_type": "bullet", + "metric": metric, + "groupby": ["Region"], + "ranges": "50,100", + "range_labels": "Low,High", + "markers": "75", + "marker_labels": "Plan", + "marker_lines": "90", + "marker_line_labels": "Forecast", + "y_axis_format": "$,.0f", + "show_labels": True, + "show_legend": True, + } + mapped = map_bullet_config(BulletChartConfig.model_validate(native)) + assert validate_merged_bullet_form_data(mapped) is not None + assert mapped["groupby"] == ["Region"] + assert mapped["ranges"] == "50,100" + assert mapped["marker_line_labels"] == "Forecast" + + +def test_bullet_rejects_invalid_roles_and_output_collisions() -> None: + with pytest.raises(ValidationError, match="physical dimension"): + BulletChartConfig( + metric=_simple_metric(), + dimensions=[{"name": "region", "aggregate": "COUNT"}], + ) + with pytest.raises(ValidationError, match="Duplicate Bullet dimension"): + BulletChartConfig( + metric=_simple_metric(), + dimensions=[{"name": "Region"}, {"name": "Region"}], + ) + with pytest.raises(ValidationError, match="conflicts with a dimension"): + BulletChartConfig( + metric={"name": "revenue", "aggregate": "SUM", "label": "Region"}, + dimensions=[{"name": "Region", "label": "Friendly"}], + ) + + +def test_bullet_accepts_short_labels_and_rejects_bad_order_target() -> None: + config = BulletChartConfig( + metric=_simple_metric(), ranges=[1, 2], range_labels=["Only one"] + ) + assert config.range_labels == ["Only one"] + with pytest.raises(ValidationError, match="unknown: not_a_role"): + BulletChartConfig( + metric=_simple_metric(), dimensions=[], order_by=[{"column": "not_a_role"}] + ) + + +def test_bullet_dimension_labels_are_input_aliases_not_result_aliases() -> None: + config = BulletChartConfig( + metric={"name": "Revenue", "aggregate": "SUM", "label": "Total"}, + dimensions=[ + {"name": "Team", "label": "Region"}, + {"name": "Region", "label": "Market"}, + ], + # The exact physical Region must win over Team's display label. + order_by=[ + {"column": "Region", "ascending": True}, + {"column": "Revenue", "ascending": False}, + ], + ) + form_data = map_bullet_config(config) + assert form_data["groupby"] == ["Team", "Region"] + assert form_data["orderby"] == [ + ["Region", True], + [form_data["metric"], False], + ] + + label_order = map_bullet_config( + BulletChartConfig( + metric=config.metric, + dimensions=config.dimensions, + order_by=[{"column": "Market"}], + ) + ) + assert label_order["orderby"] == [["Region", False]] + + model = resolve_bullet_render_model( + [{"Team": "Blue", "Region": "North", "Total": 10}], form_data + ) + assert model.dimensions == ["Team", "Region"] + assert [model.rows[0][name] for name in model.dimensions] == ["Blue", "North"] + + +def test_bullet_rejects_ambiguous_display_alias_for_ordering() -> None: + with pytest.raises(ValidationError, match="ambiguous display alias"): + BulletChartConfig( + metric=_simple_metric(), + dimensions=[ + {"name": "Region", "label": "Area"}, + {"name": "Team", "label": "area"}, + ], + order_by=[{"column": "AREA"}], + ) + + [email protected]( + ("metric", "order_target", "output"), + [ + ( + {"name": "SavedRevenue", "saved_metric": True, "label": "Friendly"}, + "Friendly", + "SavedRevenue", + ), + ( + {"name": "Revenue", "aggregate": "SUM", "label": "Simple Total"}, + "Revenue", + "Simple Total", + ), + ( + {"sql_expression": "SUM(Revenue)", "label": "SQL Total"}, + "SQL Total", + "SQL Total", + ), + ], +) +def test_bullet_metric_shapes_share_physical_dimension_output_contract( + metric: dict[str, object], order_target: str, output: str +) -> None: + config = BulletChartConfig( + metric=metric, + dimensions=[{"name": "Region", "label": "Market"}], + order_by=[{"column": order_target}], + ) + form_data = map_bullet_config(config) + assert form_data["groupby"] == ["Region"] + assert form_data["orderby"] == [[form_data["metric"], False]] + model = resolve_bullet_render_model([{"Region": "North", output: 12}], form_data) + assert model.metric_field == output + assert model.dimensions == ["Region"] + + +def test_bullet_mapper_preserves_omission_and_honors_explicit_values() -> None: + omitted = map_bullet_config(BulletChartConfig(metric=_simple_metric())) + explicit = map_bullet_config( + BulletChartConfig( + metric=_simple_metric(), + dimensions=[], + filters=[], + ranges=[], + show_labels=False, + show_legend=False, + row_limit=42, + time_range=None, + ) + ) + for key in ( + "groupby", + "adhoc_filters", + "ranges", + "show_labels", + "show_legend", + "row_limit", + "time_range", + ): + assert key not in omitted + assert explicit["groupby"] == [] + assert explicit["adhoc_filters"] == [] + assert explicit["ranges"] == "" + assert explicit["show_labels"] is False + assert explicit["show_legend"] is False + assert explicit["row_limit"] == 42 + assert explicit["time_range"] is None + + +def test_bullet_registry_schema_and_recommendation_metadata() -> None: + from superset.mcp_service.app import get_default_instructions + from superset.mcp_service.chart.registry import display_name_for_viz_type, get + + plugin = get("bullet") + assert plugin is not None + assert plugin.resolve_viz_type(None) == "bullet" + assert display_name_for_viz_type("bullet") == "Bullet Chart" + assert "bullet" in VALID_CHART_TYPES + discovered = _get_chart_type_schema_impl("bullet") + assert discovered["chart_type"] == "bullet" + assert discovered["examples"][0]["ranges"] == [100000, 250000, 500000] + assert _VIZ_CATEGORY["bullet"] == "bullet" + candidates = _candidates_single_numeric( + DataColumn( + name="Revenue", + display_name="Revenue", + data_type="numeric", + sample_values=[1], + null_count=0, + unique_count=1, + ), + row_count=1, + ) + assert "bullet chart" in candidates + guidance = get_default_instructions() + assert 'chart_type="bullet": Bullet Chart' in guidance + assert "waterfall, gantt, bullet, bubble_v2, and interactive_pivot" in guidance + + +def test_bullet_dataset_normalization_canonicalizes_every_reference() -> None: + from superset.mcp_service.chart.registry import get + + context = DatasetContext( + id=7, + table_name="sales", + schema=None, + database_name="main", + available_columns=[ + {"name": "Revenue", "type": "NUMERIC", "is_numeric": True}, + {"name": "Region", "type": "VARCHAR"}, + {"name": "OrderDate", "type": "TIMESTAMP", "is_temporal": True}, + {"name": "Status", "type": "VARCHAR"}, + ], + available_metrics=[], + ) + config = BulletChartConfig( + metric={"name": "revenue", "aggregate": "SUM"}, + dimensions=[{"name": "region"}], + temporal_column="orderdate", + filters=[{"column": "status", "op": "=", "value": "active"}], + order_by=[{"column": "region", "ascending": True}], + ) + plugin = get("bullet") + assert plugin is not None + normalized = plugin.normalize_column_refs(config, context) + assert normalized.metric.name == "Revenue" + assert normalized.dimensions[0].name == "Region" + assert normalized.temporal_column == "OrderDate" + assert normalized.filters[0].column == "Status" + assert normalized.order_by[0].column == "Region" + assert normalized.model_fields_set == config.model_fields_set + + +def test_bullet_dataset_normalization_rejects_ambiguous_casefold_candidates() -> None: + from superset.mcp_service.chart.registry import get + + context = DatasetContext( + id=7, + table_name="sales", + schema=None, + database_name="main", + available_columns=[ + {"name": "Revenue", "type": "NUMERIC", "is_numeric": True}, + {"name": "revenue", "type": "NUMERIC", "is_numeric": True}, + ], + available_metrics=[], + ) + plugin = get("bullet") + assert plugin is not None + config = BulletChartConfig(metric={"name": "REVENUE", "aggregate": "SUM"}) + with pytest.raises(ValueError, match="Revenue, revenue"): + plugin.normalize_column_refs(config, context) + + +def test_bullet_numeric_output_constraint_rejects_text_min() -> None: + from superset.mcp_service.chart.registry import get + + context = DatasetContext( + id=7, + table_name="sales", + schema=None, + database_name="main", + available_columns=[{"name": "status", "type": "VARCHAR"}], + available_metrics=[], + ) + plugin = get("bullet") + assert plugin is not None + config = BulletChartConfig(metric={"name": "status", "aggregate": "MIN"}) + with patch.object(DatasetValidator, "_get_dataset_context", return_value=context): + error = plugin.post_map_validate(config, {}, dataset_id=7) + assert error is not None + assert error.error_type == "non_numeric_bullet_metric" + + [email protected]("reverse_metadata", [False, True]) +def test_bullet_exact_case_type_and_role_resolution_is_order_independent( + reverse_metadata: bool, +) -> None: + from superset.mcp_service.chart.registry import get + + columns = [ + {"name": "Revenue", "type": "NUMERIC", "is_numeric": True}, + {"name": "revenue", "type": "VARCHAR", "is_numeric": False}, + {"name": "Region", "type": "VARCHAR"}, + ] + if reverse_metadata: + columns.reverse() + context = DatasetContext( + id=7, + table_name="sales", + schema=None, + database_name="main", + available_columns=columns, + available_metrics=[], + ) + plugin = get("bullet") + assert plugin is not None + + numeric = BulletChartConfig(metric={"name": "Revenue", "aggregate": "SUM"}) + text = BulletChartConfig(metric={"name": "revenue", "aggregate": "MIN"}) + with patch.object(DatasetValidator, "_get_dataset_context", return_value=context): + assert plugin.post_map_validate(numeric, {}, dataset_id=7) is None + error = plugin.post_map_validate(text, {}, dataset_id=7) + assert error is not None + assert error.error_type == "non_numeric_bullet_metric" + + roles = BulletChartConfig( + metric={"name": "Revenue", "aggregate": "SUM"}, + dimensions=[{"name": "revenue"}, {"name": "Region"}], + filters=[{"column": "revenue", "op": "=", "value": "retail"}], + order_by=[{"column": "revenue", "ascending": True}], + ) + normalized = plugin.normalize_column_refs(roles, context) + assert normalized.metric.name == "Revenue" + assert [dimension.name for dimension in normalized.dimensions or []] == [ + "revenue", + "Region", + ] + assert normalized.filters + assert normalized.filters[0].column == "revenue" + assert normalized.order_by[0].column == "revenue" + + ambiguous = BulletChartConfig(metric={"name": "REVENUE", "aggregate": "SUM"}) + with pytest.raises( + AmbiguousDatasetReferenceError, match="Bullet metric column reference" + ): + plugin.normalize_column_refs(ambiguous, context) + + [email protected]("reverse_metadata", [False, True]) +def test_generic_aggregation_validation_uses_exact_case_before_type( + reverse_metadata: bool, +) -> None: + from superset.mcp_service.chart.schemas import PieChartConfig + + columns = [ + {"name": "Amount", "type": "BIGINT", "is_numeric": True}, + {"name": "amount", "type": "VARCHAR", "is_numeric": False}, + ] + if reverse_metadata: + columns.reverse() + context = DatasetContext( + id=7, + table_name="sales", + schema=None, + database_name="main", + available_columns=columns, + available_metrics=[], + ) + + assert ( + DatasetValidator._validate_aggregations( + [BulletChartConfig(metric={"name": "Amount", "aggregate": "SUM"}).metric], + context, + ) + == [] + ) + errors = DatasetValidator._validate_aggregations( + [BulletChartConfig(metric={"name": "amount", "aggregate": "SUM"}).metric], + context, + ) + assert errors + assert errors[0].error_type == "invalid_aggregation" + + ambiguous = DatasetValidator._validate_aggregations( + [BulletChartConfig(metric={"name": "AMOUNT", "aggregate": "SUM"}).metric], + context, + ) + assert ambiguous + assert ambiguous[0].error_type == "ambiguous_column_reference" + + valid, error = DatasetValidator.validate_against_dataset( + PieChartConfig( + dimension={"name": "amount"}, + metric={"name": "Amount", "aggregate": "SUM"}, + ), + 7, + dataset_context=context, + ) + assert valid is True + assert error is None + + [email protected]( + ("metric", "field"), + [ + ({"name": "Revenue", "aggregate": "SUM", "label": "Simple"}, "Simple"), + ({"name": "SavedRevenue", "saved_metric": True}, "SavedRevenue"), + ({"sql_expression": "SUM(Revenue)", "label": "SQL Total"}, "SQL Total"), + ], +) +def test_bullet_result_roles_are_exact_for_every_metric_shape( + metric: dict[str, object], field: str +) -> None: + form_data = map_bullet_config(BulletChartConfig(metric=metric)) + model = resolve_bullet_render_model([{field.swapcase(): "12.5"}], form_data) + assert model.metric_field == field.swapcase() + assert model.measures == [12.5] + + [email protected]( + "rows, message", + [ + ([{"other": 123}], "missing"), + ([{"Revenue": "not a number"}], "non-numeric text"), + ([{"Revenue": math.nan}], "NaN or infinite"), + ([{"Revenue": math.inf}], "NaN or infinite"), + ([{"Revenue": 1}, {}], "row 1.*missing"), + ([{"REVENUE": 1, "revenue": 2}], "ambiguous"), + ], +) +def test_bullet_result_validation_rejects_malformed_rows( + rows: list[dict[str, object]], message: str +) -> None: + form_data = map_bullet_config( + BulletChartConfig( + metric={"name": "amount", "aggregate": "SUM", "label": "Revenue"} + ) + ) + with pytest.raises(BulletOutputError, match=message): + resolve_bullet_render_model(rows, form_data) + + +def test_bullet_result_validation_accepts_null_and_numeric_strings() -> None: + form_data = map_bullet_config( + BulletChartConfig( + metric={"name": "amount", "aggregate": "SUM", "label": "Revenue"}, + dimensions=[{"name": "Region"}], + ) + ) + model = resolve_bullet_render_model( + [ + {"Region": "North", "Revenue": None}, + {"Region": "South", "Revenue": " 4.25 "}, + ], + form_data, + ) + assert model.measures == [0.0, 4.25] + + [email protected]( + ("presentation", "message"), + [ + ({"ranges": "10,nope"}, r"ranges\[1\].*not numeric"), + ({"markers": "NaN"}, r"markers\[0\].*NaN or infinite"), + ], +) +def test_bullet_result_validation_rejects_malformed_presentation( + presentation: dict[str, object], message: str +) -> None: + form_data = { + **map_bullet_config( + BulletChartConfig(metric={"name": "amount", "aggregate": "SUM"}) + ), + **presentation, + } + with pytest.raises(BulletOutputError, match=message): + resolve_bullet_render_model([{"SUM(amount)": 1}], form_data) + + +def test_bullet_compile_accepts_empty_ungrouped_result() -> None: + form_data = map_bullet_config( + BulletChartConfig( + metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"} + ) + ) + factory = MagicMock() + factory.create.return_value = MagicMock() + command = MagicMock() + command.run.return_value = {"queries": [{"data": []}]} + with ( + patch( + "superset.common.query_context_factory.QueryContextFactory", + return_value=factory, + ), + patch( + "superset.commands.chart.data.get_data_command.ChartDataCommand", + return_value=command, + ), + ): + result = _compile_chart(form_data, 7) + assert result.success is True + assert result.row_count == 0 + + +def test_bullet_compile_inspects_top_level_and_query_error_envelopes() -> None: + form_data = map_bullet_config( + BulletChartConfig(metric={"name": "Revenue", "aggregate": "SUM"}) + ) + factory = MagicMock() + factory.create.return_value = MagicMock() + command = MagicMock() + command.run.return_value = { + "status": "success", + "queries": [{"status": "failed", "message": "warehouse timeout"}], + } + with ( + patch( + "superset.common.query_context_factory.QueryContextFactory", + return_value=factory, + ), + patch( + "superset.commands.chart.data.get_data_command.ChartDataCommand", + return_value=command, + ), + ): + result = _compile_chart(form_data, 7) + assert result.success is False + assert "warehouse timeout" in (result.error or "") + + +_MALFORMED_QUERY_ENVELOPES: list[object] = [ + None, + [], + {}, + {"queries": None}, + {"queries": []}, + {"queries": [None]}, + {"queries": [{}]}, + {"queries": [{"data": None}]}, + {"queries": [{"data": []}, {"data": "not-an-array"}]}, + { + "queries": [ + { + "data": [{"Revenue": 12}], + "colnames": ["Revenue"], + "coltypes": [], + } + ] + }, +] + + +def _compile_bullet_with_result(result: object) -> Any: + form_data = map_bullet_config( + BulletChartConfig( + metric={"name": "Revenue", "aggregate": "SUM", "label": "Revenue"} + ) + ) + factory = MagicMock() + factory.create.return_value = MagicMock() + command = MagicMock() + command.run.return_value = result + with ( + patch( + "superset.common.query_context_factory.QueryContextFactory", + return_value=factory, + ), + patch( + "superset.commands.chart.data.get_data_command.ChartDataCommand", + return_value=command, + ), + ): + return _compile_chart(form_data, 7) + + [email protected]("envelope", _MALFORMED_QUERY_ENVELOPES) +def test_bullet_compile_returns_stable_error_for_malformed_envelopes( + envelope: object, +) -> None: + result = _compile_bullet_with_result(envelope) + assert result.success is False + assert result.error_code == "CHART_COMPILE_FAILED" + assert result.error_obj is not None + assert result.error_obj.error_type == "compile_error" + + [email protected]( + ("data", "expected_code", "expected_type"), + [ + ([1], "CHART_COMPILE_FAILED", "compile_error"), + ( + [{"Revenue": 10**10000}], + "CHART_COMPILE_FAILED", + "compile_error", + ), + ], +) +def test_bullet_compile_returns_malformed_output_for_bad_rows( + data: list[object], + expected_code: str, + expected_type: str, +) -> None: + result = _compile_bullet_with_result({"queries": [{"data": data}]}) + assert result.success is False + assert result.error_code == expected_code + assert result.error_obj is not None + assert result.error_obj.error_type == expected_type + + +def test_bullet_shared_query_builder_matches_frontend_build_query() -> None: Review Comment: Fixed in fa297506b5c3dbbbc383135c5c388ac7c018af2c: renamed the Python test to test_bullet_query_builder_forwards_configured_roles_sort_and_limit, which accurately describes its backend-only scope. Added a separate frontend buildQuery test asserting groupby, metric, explicit ordering, and row limit, so dropping those controls on the frontend fails a frontend test rather than relying on a claimed cross-language parity check. Both Bullet frontend suites passed (19 tests). -- 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]
