diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py index 600252f2cfa..ca5e1dfeb47 100644 --- a/superset/mcp_service/chart/chart_utils.py +++ b/superset/mcp_service/chart/chart_utils.py @@ -468,6 +468,17 @@ def add_legend_config(form_data: Dict[str, Any], config: XYChartConfig) -> None: form_data["legend_orientation"] = config.legend.position +def add_orientation_config(form_data: Dict[str, Any], config: XYChartConfig) -> None: + """Add orientation configuration to form_data for bar charts. + + Only applies when kind='bar' and an explicit orientation is set. + When orientation is None (the default), Superset uses its own default + (vertical bars). + """ + if config.kind == "bar" and config.orientation: + form_data["orientation"] = config.orientation + + def configure_temporal_handling( form_data: Dict[str, Any], x_is_temporal: bool, @@ -576,6 +587,7 @@ def map_xy_config( # Add configurations add_axis_config(form_data, config) add_legend_config(form_data, config) + add_orientation_config(form_data, config) return form_data diff --git a/superset/mcp_service/chart/resources/chart_configs.py b/superset/mcp_service/chart/resources/chart_configs.py index ea40a89fdc6..2560a503e62 100644 --- a/superset/mcp_service/chart/resources/chart_configs.py +++ b/superset/mcp_service/chart/resources/chart_configs.py @@ -129,6 +129,29 @@ def get_chart_configs_resource() -> str: }, "use_cases": ["Correlation analysis", "Outlier detection"], }, + "horizontal_bar": { + "description": "Horizontal bar chart for categories with long names", + "config": { + "chart_type": "xy", + "kind": "bar", + "orientation": "horizontal", + "x": {"name": "department", "label": "Department"}, + "y": [ + { + "name": "headcount", + "aggregate": "SUM", + "label": "Headcount", + } + ], + "y_axis": {"title": "Department"}, + "x_axis": {"title": "Number of Employees"}, + }, + "use_cases": [ + "Long category labels", + "Rankings and leaderboards", + "Survey results", + ], + }, "stacked_area": { "description": "Stacked area chart for volume composition over time", "config": { @@ -215,6 +238,7 @@ def get_chart_configs_resource() -> str: "Use group_by to split data into series for comparison", "Use stacked=true for bar/area charts showing composition", "Configure axis format for readability ($,.0f for currency, .2% for pct)", + "Use orientation='horizontal' for bar charts with long category names", ], "table_charts": [ "Include only essential columns to avoid clutter", diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py index 10b694d22c2..28fdf1b0d88 100644 --- a/superset/mcp_service/chart/schemas.py +++ b/superset/mcp_service/chart/schemas.py @@ -749,6 +749,14 @@ class XYChartConfig(BaseModel): "If not specified, Superset will use its default behavior." ), ) + orientation: Literal["vertical", "horizontal"] | None = Field( + None, + description=( + "Bar chart orientation. Only applies when kind='bar'. " + "'vertical' (default): bars extend upward. " + "'horizontal': bars extend rightward, useful for long category names." + ), + ) stacked: bool = Field( False, description="Stack bars/areas on top of each other instead of side-by-side", diff --git a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py index ae13bfc8a75..cea45bf0244 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_schemas.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_schemas.py @@ -243,6 +243,61 @@ class TestXYChartConfig: assert config.group_by is not None assert config.group_by.name == "year" + def test_orientation_horizontal_accepted(self) -> None: + """Test that orientation='horizontal' is accepted for bar charts.""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="department"), + y=[ColumnRef(name="headcount", aggregate="SUM")], + kind="bar", + orientation="horizontal", + ) + assert config.orientation == "horizontal" + + def test_orientation_vertical_accepted(self) -> None: + """Test that orientation='vertical' is accepted for bar charts.""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="category"), + y=[ColumnRef(name="sales", aggregate="SUM")], + kind="bar", + orientation="vertical", + ) + assert config.orientation == "vertical" + + def test_orientation_none_by_default(self) -> None: + """Test that orientation defaults to None when not specified.""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="category"), + y=[ColumnRef(name="sales", aggregate="SUM")], + kind="bar", + ) + assert config.orientation is None + + def test_orientation_invalid_value_rejected(self) -> None: + """Test that invalid orientation values are rejected.""" + with pytest.raises(ValidationError): + XYChartConfig( + chart_type="xy", + x=ColumnRef(name="category"), + y=[ColumnRef(name="sales", aggregate="SUM")], + kind="bar", + orientation="diagonal", + ) + + def test_orientation_with_non_bar_chart(self) -> None: + """Test that orientation field is accepted on non-bar charts at schema level.""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="date"), + y=[ColumnRef(name="revenue", aggregate="SUM")], + kind="line", + orientation="horizontal", + ) + # Schema allows it; the chart_utils layer decides whether to apply it + assert config.orientation == "horizontal" + class TestTableChartConfigExtraFields: """Test TableChartConfig rejects unknown fields.""" diff --git a/tests/unit_tests/mcp_service/chart/test_chart_utils.py b/tests/unit_tests/mcp_service/chart/test_chart_utils.py index c142aabb8ba..9b53024b175 100644 --- a/tests/unit_tests/mcp_service/chart/test_chart_utils.py +++ b/tests/unit_tests/mcp_service/chart/test_chart_utils.py @@ -460,6 +460,84 @@ class TestMapXYConfig: assert result["groupby"] == ["category"] assert result["x_axis"] == "order_date" + def test_map_xy_config_bar_horizontal_orientation(self) -> None: + """Test XY config mapping for horizontal bar chart""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="department"), + y=[ColumnRef(name="headcount", aggregate="SUM")], + kind="bar", + orientation="horizontal", + ) + + result = map_xy_config(config) + + assert result["viz_type"] == "echarts_timeseries_bar" + assert result["orientation"] == "horizontal" + + def test_map_xy_config_bar_vertical_orientation(self) -> None: + """Test XY config mapping for vertical bar chart (explicit)""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="category"), + y=[ColumnRef(name="sales", aggregate="SUM")], + kind="bar", + orientation="vertical", + ) + + result = map_xy_config(config) + + assert result["viz_type"] == "echarts_timeseries_bar" + assert result["orientation"] == "vertical" + + def test_map_xy_config_bar_no_orientation(self) -> None: + """Test XY config mapping for bar chart without orientation.""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="category"), + y=[ColumnRef(name="sales", aggregate="SUM")], + kind="bar", + ) + + result = map_xy_config(config) + + assert result["viz_type"] == "echarts_timeseries_bar" + assert "orientation" not in result + + def test_map_xy_config_line_orientation_ignored(self) -> None: + """Test that orientation is ignored for non-bar chart types""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="date"), + y=[ColumnRef(name="revenue", aggregate="SUM")], + kind="line", + orientation="horizontal", + ) + + result = map_xy_config(config) + + assert result["viz_type"] == "echarts_timeseries_line" + assert "orientation" not in result + + def test_map_xy_config_bar_horizontal_with_stacked(self) -> None: + """Test horizontal bar chart with stacked option""" + config = XYChartConfig( + chart_type="xy", + x=ColumnRef(name="department"), + y=[ColumnRef(name="headcount", aggregate="SUM")], + kind="bar", + orientation="horizontal", + stacked=True, + group_by=ColumnRef(name="level"), + ) + + result = map_xy_config(config) + + assert result["viz_type"] == "echarts_timeseries_bar" + assert result["orientation"] == "horizontal" + assert result["stack"] == "Stack" + assert result["groupby"] == ["level"] + class TestMapConfigToFormData: """Test map_config_to_form_data function"""