codeant-ai-for-open-source[bot] commented on code in PR #41860:
URL: https://github.com/apache/superset/pull/41860#discussion_r3575644735
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -891,6 +893,64 @@ def map_pie_config(config: PieChartConfig) -> Dict[str,
Any]:
return form_data
+def map_histogram_config(config: "HistogramChartConfig") -> Dict[str, Any]:
+ """Map histogram config to Superset form_data (viz_type histogram_v2).
+
+ Matches the frontend Histogram buildQuery contract: a single ``column``
+ string to bin, ``groupby`` name list for series, plus bins/normalize/
+ cumulative passed straight through to the histogram post-processing
+ operator.
+ """
+ form_data: Dict[str, Any] = {
+ "viz_type": "histogram_v2",
+ "column": config.column.name,
+ "groupby": [g.name for g in (config.groupby or [])],
+ "bins": config.bins,
+ "normalize": config.normalize,
+ "cumulative": config.cumulative,
+ "row_limit": config.row_limit,
+ }
+ _add_adhoc_filters(form_data, config.filters)
+ return form_data
+
+
+# The exact strings the frontend boxplotOperator understands; the percentile
+# variant must match its PERCENTILE_REGEX: "<low>/<high> percentiles".
+_WHISKER_TYPE_TO_OPTION = {
+ "tukey": "Tukey",
+ "min_max": "Min/max (no outliers)",
+}
+
+
+def map_box_plot_config(config: "BoxPlotChartConfig") -> Dict[str, Any]:
+ """Map box plot config to Superset form_data (viz_type box_plot).
+
+ Matches the frontend BoxPlot buildQuery contract: ``columns`` are the
+ distribute-across values (one box per value), ``groupby`` the series
+ dimensions, and ``whiskerOptions`` one of the strings the
+ boxplotOperator post-processor parses.
+ """
+ if config.whisker_type == "percentile":
+ whisker_options = (
+ f"{config.percentile_low}/{config.percentile_high} percentiles"
+ )
+ else:
+ whisker_options = _WHISKER_TYPE_TO_OPTION[config.whisker_type]
+
+ form_data: Dict[str, Any] = {
+ "viz_type": "box_plot",
+ "columns": [c.name for c in config.distribute_across],
+ "groupby": [d.name for d in (config.dimensions or [])],
+ "metrics": [create_metric_object(m) for m in config.metrics],
Review Comment:
**Suggestion:** `map_box_plot_config` maps `distribute_across` to `columns`
and leaves `groupby` driven only by optional `dimensions`, but the box-plot
post-processing logic groups by `groupby` (not `columns`). This means a valid
request with only required fields (`metrics` + `distribute_across`) produces an
empty `groupby` and collapses into a single box, so the required split field
does not actually control box separation. Map the split dimension into
`groupby` (and combine additional dimensions intentionally) so box grouping
matches the schema contract. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Box plot MCP charts collapse into single global box.
- ❌ Distribute_across column never controls box separation.
- ⚠️ Agents misinterpret distributions when following schema examples.
- ⚠️ get_chart_type_schema box_plot examples produce incorrect grouping.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the MCP generate_chart tool defined in
superset/mcp_service/chart/tool/generate_chart.py:19-64 with a box plot
config using only
the required fields described in its docstring at lines 62-64, for example:
- chart_type: "box_plot"
- metrics: [{"name": "fare_amount", "aggregate": "AVG"}]
- distribute_across: [{"name": "day_of_week"}]
This matches the first box_plot example in
superset/mcp_service/chart/tool/get_chart_type_schema.py:6-21 where only
metrics and
distribute_across are provided.
2. The request config is validated against BoxPlotChartConfig in
superset/mcp_service/chart/schemas.py:63-80, which requires metrics and
distribute_across
and makes dimensions optional. BoxPlotChartPlugin.pre_validate in
superset/mcp_service/chart/plugins/box_plot.py:44-76 enforces the presence
of "metrics"
and "distribute_across" only, so the minimal config passes when dimensions
is omitted.
3. BoxPlotChartPlugin.to_form_data at
superset/mcp_service/chart/plugins/box_plot.py:90-93
calls map_box_plot_config(config) in
superset/mcp_service/chart/chart_utils.py:76-102. For
the minimal config, map_box_plot_config builds form_data with:
- "columns": [c.name for c in config.distribute_across] (line 93)
- "groupby": [d.name for d in (config.dimensions or [])] (line 94)
Since config.dimensions is None, groupby becomes an empty list while
columns contains
["day_of_week"].
4. The Superset frontend Box Plot buildQuery in
superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts:13-37
consumes
this form_data. It constructs the query object with:
- columns: distributed columns plus ensureIsArray(formData.groupby)
(lines 13-35)
- series_columns: formData.groupby (line 36)
- post_processing: [boxplotOperator(formData, baseQueryObject)] (line 37)
The boxplotOperator implementation in
superset-frontend/packages/superset-ui-chart-controls/src/operators/boxplotOperator.ts:11-44
reads groupby from formData and passes
ensureIsArray(groupby).map(getColumnLabel) into
the backend boxplot post processor. Because groupby is empty,
options.groupby is [], so
superset/utils/pandas_postprocessing/boxplot.py:29-59 aggregates each
metric over the
entire dataset with groupby=[], producing a single box per metric instead
of separate
boxes per distribute_across value on the x axis. This contradicts the
contract
documented in BoxPlotChartConfig at
superset/mcp_service/chart/schemas.py:75-79 and the
generate_chart docstring at 62-64 that promise one box per
distribute_across value.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=795717e795f24f50ba699fcb9694169f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=795717e795f24f50ba699fcb9694169f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/chart_utils.py
**Line:** 943:944
**Comment:**
*Api Mismatch: `map_box_plot_config` maps `distribute_across` to
`columns` and leaves `groupby` driven only by optional `dimensions`, but the
box-plot post-processing logic groups by `groupby` (not `columns`). This means
a valid request with only required fields (`metrics` + `distribute_across`)
produces an empty `groupby` and collapses into a single box, so the required
split field does not actually control box separation. Map the split dimension
into `groupby` (and combine additional dimensions intentionally) so box
grouping matches the schema contract.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41860&comment_hash=81f874efce139e6b83fd4c1745611e75679d4304b626f5b0e7574c2ccf33d92d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41860&comment_hash=81f874efce139e6b83fd4c1745611e75679d4304b626f5b0e7574c2ccf33d92d&reaction=dislike'>👎</a>
--
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]
