codeant-ai-for-open-source[bot] commented on code in PR #41860:
URL: https://github.com/apache/superset/pull/41860#discussion_r3565569213
##########
superset/mcp_service/chart/tool/get_chart_type_schema.py:
##########
@@ -48,6 +50,8 @@
"mixed_timeseries": TypeAdapter(MixedTimeseriesChartConfig),
"handlebars": TypeAdapter(HandlebarsChartConfig),
"big_number": TypeAdapter(BigNumberChartConfig),
+ "histogram": TypeAdapter(HistogramChartConfig),
+ "box_plot": TypeAdapter(BoxPlotChartConfig),
Review Comment:
**Suggestion:** You added new supported chart types but did not update the
tool’s declared valid-type contract text, so callers relying on the function
documentation will incorrectly treat these chart types as unsupported. Update
the public contract text in the function docstring to include the new chart
types so discovery and invocation guidance stays consistent with runtime
behavior. [docstring mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP get_chart_type_schema tool advertises incomplete chart_type set.
- ⚠️ Agents may avoid valid histogram and box_plot schemas.
- ⚠️ Documentation-driven workflows get inconsistent discovery versus
runtime behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset/core/mcp/core_mcp_injection.py` and observe in
`create_tool_decorator`
that the MCP `@tool` decorator derives the tool description from the
function docstring:
`tool_description = description or func.__doc__ or f"Tool:
{base_tool_name}"` (lines
35–36), so whatever is written in the docstring is surfaced to MCP clients
as the tool
contract text.
2. Open `superset/mcp_service/chart/tool/get_chart_type_schema.py` and
confirm that
`_CHART_TYPE_ADAPTERS` includes `"histogram"` and `"box_plot"` (lines
45–55), and that
`VALID_CHART_TYPES = sorted(_CHART_TYPE_ADAPTERS.keys())` (line 57), meaning
the runtime
logic fully supports these two new chart types and they are returned as
valid in error
messages and responses.
3. In the same file, scroll to `get_chart_type_schema` (lines 15–29) and
inspect its
docstring: the “Valid chart_type values” sentence lists only `xy, table,
pie, pivot_table,
mixed_timeseries, handlebars, big_number` (lines 24–25), omitting the newly
added
`histogram` and `box_plot` types that are present in `_CHART_TYPE_ADAPTERS`
and
`VALID_CHART_TYPES`.
4. Start the MCP service (which imports `get_chart_type_schema` in
`superset/mcp_service/app.py` around lines 4–18) and inspect the tool
manifest or let an
MCP client enumerate tools; the tool description for `get_chart_type_schema`
will present
the stale list from the docstring, so an agent or human relying on that text
will
reasonably conclude that `histogram` and `box_plot` are unsupported even
though calls with
`chart_type="histogram"` or `chart_type="box_plot"` succeed at runtime.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f3f6a62b2444eea8de228c513efd6b8&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=1f3f6a62b2444eea8de228c513efd6b8&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/tool/get_chart_type_schema.py
**Line:** 53:54
**Comment:**
*Docstring Mismatch: You added new supported chart types but did not
update the tool’s declared valid-type contract text, so callers relying on the
function documentation will incorrectly treat these chart types as unsupported.
Update the public contract text in the function docstring to include the new
chart types so discovery and invocation guidance stays consistent with runtime
behavior.
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=8e70efa67dbd2070837cfd8ccd884171b45dbaa1adfb5275208806a96b9342d6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41860&comment_hash=8e70efa67dbd2070837cfd8ccd884171b45dbaa1adfb5275208806a96b9342d6&reaction=dislike'>👎</a>
##########
superset/mcp_service/app.py:
##########
@@ -384,6 +384,11 @@ def get_default_instructions(
Requires handlebars_template with Handlebars HTML template string.
Supports query_mode="aggregate" (with metrics/groupby) or "raw" (with
columns).
Data available as {{{{data}}}} array; helpers: dateFormat, formatNumber,
stringify.
+- chart_type="histogram": Histogram of a numeric column's distribution
+ (column required; optional bins, groupby, normalize, cumulative)
+- chart_type="box_plot": Box plot comparing statistical spread
+ (metrics + distribute_across required; optional dimensions,
+ whisker_type: tukey | min_max | percentile)
Review Comment:
**Suggestion:** You added two new creatable chart types here, but the same
instructions block still states later that only 7 chart types are supported by
`generate_chart`. This creates contradictory runtime guidance for MCP clients
and can cause them to mis-handle valid `histogram`/`box_plot` flows. Update the
downstream count/list text in this same instructions payload to stay consistent
with these additions. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP instructions misstate generate_chart supported chart types.
- ⚠️ Clients may mishandle histogram/box_plot based on guidance.
- ⚠️ Agents receive inconsistent chart_type_display_name documentation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Deploy the MCP service with this PR and call `get_default_instructions()`
from
`superset/mcp_service/app.py` (function containing the instructions block
around lines
374–399 in the diff).
2. Inspect the "Chart Types You Can CREATE with
generate_chart/generate_explore_link"
section, which now lists `chart_type="histogram"` and
`chart_type="box_plot"` at lines
387–391 as creatable chart types.
3. Continue reading the "Chart Types in Existing Charts (viewable via
list_charts/get_chart_info)" section immediately below; at line 399 the text
states: "This
field is populated only for the 7 chart types supported by generate_chart
(xy, pie, table,
pivot_table, big_number, mixed_timeseries, handlebars)."
4. Observe that the instructions payload simultaneously describes
histogram/box_plot as
supported creatable chart types while still claiming that only 7 chart types
are supported
by `generate_chart`, creating contradictory guidance for MCP clients
consuming this
instructions string.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=04abc17a29db430f8b4aa006dbd4ae9a&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=04abc17a29db430f8b4aa006dbd4ae9a&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/app.py
**Line:** 387:391
**Comment:**
*Comment Mismatch: You added two new creatable chart types here, but
the same instructions block still states later that only 7 chart types are
supported by `generate_chart`. This creates contradictory runtime guidance for
MCP clients and can cause them to mis-handle valid `histogram`/`box_plot`
flows. Update the downstream count/list text in this same instructions payload
to stay consistent with these additions.
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=e6b0c8629ba3a561e10541d7eef85b6909ae89702cab60711799b989b519e544&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41860&comment_hash=e6b0c8629ba3a561e10541d7eef85b6909ae89702cab60711799b989b519e544&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]
