codeant-ai-for-open-source[bot] commented on code in PR #38373:
URL: https://github.com/apache/superset/pull/38373#discussion_r2881218996
##########
superset/mcp_service/dashboard/tool/generate_dashboard.py:
##########
@@ -177,10 +211,15 @@ def generate_dashboard(
with event_logger.log_context(action="mcp.generate_dashboard.layout"):
layout = _create_dashboard_layout(chart_objects)
+ # Resolve dashboard title: use provided title or derive from chart
names
+ dashboard_title = request.dashboard_title or
_generate_title_from_charts(
+ chart_objects
+ )
+
# Prepare dashboard data and create dashboard
with
event_logger.log_context(action="mcp.generate_dashboard.db_write"):
Review Comment:
**Suggestion:** The dashboard title resolution uses a truthiness check, so
if a caller explicitly supplies an empty string as the title it will be treated
as "omitted" and replaced with an auto-generated title, which contradicts the
API description that only omitting the field should trigger auto-generation and
can surprise callers that intentionally send an empty title. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP generate_dashboard tool ignores explicitly blank titles.
- ⚠️ Clients sending "" get unexpected auto-generated titles.
- ⚠️ Harder to intentionally create dashboards without a title.
```
</details>
```suggestion
dashboard_title = (
request.dashboard_title
if request.dashboard_title is not None
else _generate_title_from_charts(chart_objects)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the Superset MCP service so that the `generate_dashboard` tool in
`superset/mcp_service/dashboard/tool/generate_dashboard.py` is loaded and
registered
(function defined starting around line 167 in the diff hunk `@@ -167,25
+201,30 @@ def
generate_dashboard(`).
2. From an MCP client, invoke the `generate_dashboard` tool with a request
payload that
includes valid `chart_ids` and explicitly sets an empty string title, e.g.:
```json
{
"chart_ids": [1],
"dashboard_title": ""
}
```
This request is parsed by the `@parse_request(GenerateDashboardRequest)`
decorator from
`superset/mcp_service/utils/schema_utils.py:404-507`, which builds a
`GenerateDashboardRequest` model with `request.dashboard_title == ""`.
3. The `generate_dashboard` function fetches chart objects and builds the
layout, then
executes the title resolution block at
`superset/mcp_service/dashboard/tool/generate_dashboard.py:218-221`:
```python
dashboard_title = request.dashboard_title or _generate_title_from_charts(
chart_objects
)
```
Because `request.dashboard_title` is the empty string (falsy), the `or`
expression
evaluates the right-hand side and calls
`_generate_title_from_charts(chart_objects)`
(defined at lines 144-172).
4. `_generate_title_from_charts()` constructs a title from the chart slice
names (e.g.
`"Revenue"` for a single chart) and returns it. The created dashboard, via
`CreateDashboardCommand` in `generate_dashboard()` (later in the same
function), ends up
with this derived title instead of the explicitly provided empty string.
This contradicts
the documented behavior in the PR description that only omitting
`dashboard_title` should
trigger auto-generation.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/tool/generate_dashboard.py
**Line:** 219:220
**Comment:**
*Logic Error: The dashboard title resolution uses a truthiness check,
so if a caller explicitly supplies an empty string as the title it will be
treated as "omitted" and replaced with an auto-generated title, which
contradicts the API description that only omitting the field should trigger
auto-generation and can surprise callers that intentionally send an empty title.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38373&comment_hash=7cc6040663fb0a1d52ff0bd5e20472e747382d16909d8786284d428ac7912239&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38373&comment_hash=7cc6040663fb0a1d52ff0bd5e20472e747382d16909d8786284d428ac7912239&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]
