Copilot commented on code in PR #41922:
URL: https://github.com/apache/superset/pull/41922#discussion_r3561825595
##########
superset/mcp_service/chart/tool/get_chart_type_schema.py:
##########
@@ -172,6 +172,7 @@ def _get_chart_type_schema_impl(
@tool(
tags=["discovery"],
+ class_permission_name="Chart",
annotations=ToolAnnotations(
Review Comment:
Adding `class_permission_name="Chart"` changes runtime authorization when
`MCP_RBAC_ENABLED` is on: previously this tool would "fail open" (authenticated
but no RBAC check), now it will require Chart read permission. That’s likely
correct, but it contradicts the PR description claim of "No tool behavior
changes" (this can affect who can call/see the tool).
##########
superset/mcp_service/app.py:
##########
@@ -176,15 +176,23 @@ def get_default_instructions(
- create_virtual_dataset: Save a SQL query as a virtual dataset for charting
(requires write access)
- query_dataset: Query a dataset using its semantic layer (saved metrics,
dimensions, filters) without needing a saved chart
+Semantic Layer:
+- list_metrics: Discover metrics across built-in datasets and external
semantic views (1-based pagination)
+- get_table: Query a data source (built-in dataset or external semantic view)
by metric/dimension names
+- get_compatible_dimensions: Get dimensions compatible with a given
metric/dimension selection
+- get_compatible_metrics: Get metrics compatible with a given dimension/metric
selection
+
Chart Management:
- list_charts: List charts with advanced filters (1-based pagination)
- get_chart_info: Get detailed chart information by ID
- get_chart_preview: Get a visual preview of a chart as formatted content or
URL
- get_chart_data: Get underlying chart data in text-friendly format
- get_chart_sql: Get the rendered SQL query for a chart (without executing it)
+- get_chart_type_schema: Get the JSON Schema and examples for a chart_type
before calling generate_chart/update_chart
- generate_chart: Create and save a new chart permanently (requires write
access)
Review Comment:
The DEFAULT_INSTRUCTIONS tool list says `generate_chart` "Create and save a
new chart permanently", but `generate_chart` defaults to `save_chart=False`
(preview-only) and only persists when explicitly requested. This description
can mislead clients/LLMs into claiming a chart was saved when it wasn’t.
--
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]
