codeant-ai-for-open-source[bot] commented on code in PR #40690: URL: https://github.com/apache/superset/pull/40690#discussion_r3350086401
########## superset/mcp_service/app.py: ########## @@ -198,11 +186,7 @@ def get_default_instructions( - get_query_info: Get SQL query history details by ID Schema Discovery: -- get_schema: Get schema metadata for chart/dataset/dashboard/database/css_template/theme (columns, filters) - -Action Logs (requires SUPERSET_LOG_VIEW and FAB_ADD_SECURITY_VIEWS): -- list_action_logs: List user action logs with filtering and pagination (defaults to last 7 days) -- get_action_log_info: Get a single action log entry by integer ID +- get_schema: Get schema metadata for chart/dataset/dashboard/database (columns, filters) Review Comment: **Suggestion:** The tool instructions now advertise `get_schema` as covering only chart/dataset/dashboard/database, but `get_schema` in this PR supports `report` too. This contract mismatch will cause MCP clients/agents to skip schema discovery for reports and generate poorer or invalid report-list filters/sorts. Update the instruction text to include `report` so runtime behavior and advertised capabilities stay aligned. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Alerts & Reports tool lacks advertised report schema discovery. - ⚠️ LLM agents may skip get_schema before list_reports. - ⚠️ Report filters/sorts risk being guessed or invalid. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Start the MCP service so `init_fastmcp_server()` in `superset/mcp_service/app.py:162-213` runs; with `instructions` unset, it calls `get_default_instructions(branding, all_disabled)` and assigns the returned string to `mcp._mcp_server.instructions`, which is what MCP clients/agents read as the system prompt. 2. Inspect the default instructions text built in `get_default_instructions` around `superset/mcp_service/app.py:69-71` and `app.py:19-22` (shown in the PR hunk as lines 188-189 and 389-402): the "Schema Discovery" section and "General usage tips" both state `get_schema: Get schema metadata for chart/dataset/dashboard/database (columns, filters)` and "Use get_schema (chart/dataset/dashboard/database) …", omitting `report` from the advertised supported model types. 3. Compare this with the actual `get_schema` implementation in `superset/mcp_service/system/tool/get_schema.py:190-38` and its request/response schemas in `superset/mcp_service/common/schema_discovery.py:87-98`: `_SCHEMA_CORE_FACTORIES` includes `"report": "ReportSchedule"` (lines 190-5), and the `get_schema` docstring explicitly documents `model_type: One of "chart", "dataset", "dashboard", "database", or "report"`, confirming that `report` is a supported model type at runtime. 4. Observe how report listing is designed to rely on `get_schema` for discovery: `ReportFilter.col` and `ReportFilter.opr` in `superset/mcp_service/report/schemas.py:54-79` tell clients "Use get_schema(model_type='report') for available filter columns/operators", and `list_reports` in `superset/mcp_service/report/tool/list_reports.py:63-74, 114-125` exposes `filters`, `select_columns`, and `order_column` with configured `REPORT_DEFAULT_COLUMNS`, `REPORT_SEARCH_COLUMNS`, and `REPORT_SORTABLE_COLUMNS`. An MCP client/agent that trusts the top-level instructions in `app.py` (which only mention chart/dataset/dashboard/database) will not realize `get_schema` also covers `report`, is less likely to call `get_schema(model_type="report")` before constructing `ReportFilter`/`ListReportsRequest`, and will therefore guess columns/sorts instead of using the actual report schema, degrading filter/sort quality or causing validation errors. ``` </details> [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f064ce2c1fe248ba9efe014b37d1dd00&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f064ce2c1fe248ba9efe014b37d1dd00&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:** 189:189 **Comment:** *Api Mismatch: The tool instructions now advertise `get_schema` as covering only chart/dataset/dashboard/database, but `get_schema` in this PR supports `report` too. This contract mismatch will cause MCP clients/agents to skip schema discovery for reports and generate poorer or invalid report-list filters/sorts. Update the instruction text to include `report` so runtime behavior and advertised capabilities stay aligned. 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%2F40690&comment_hash=3e19f7a1c560178340667259d9d9b3b5f7cb7377c66a29a067f11c7a94a31e34&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40690&comment_hash=3e19f7a1c560178340667259d9d9b3b5f7cb7377c66a29a067f11c7a94a31e34&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]
