codeant-ai-for-open-source[bot] commented on code in PR #38562:
URL: https://github.com/apache/superset/pull/38562#discussion_r2925217189
##########
superset/mcp_service/server.py:
##########
@@ -151,6 +155,106 @@ def create_event_store(config: dict[str, Any] | None =
None) -> Any | None:
return None
+def _strip_titles(obj: Any) -> Any:
+ """Recursively strip 'title' keys from JSON Schema objects.
+
+ Pydantic auto-generates 'title' for every field and model (e.g.
+ "title": "Chart Type" for a field named chart_type). These are
+ redundant with property names and inflate schema size by ~12%.
+ """
+ if isinstance(obj, dict):
+ return {k: _strip_titles(v) for k, v in obj.items() if k != "title"}
+ if isinstance(obj, list):
+ return [_strip_titles(item) for item in obj]
Review Comment:
**Suggestion:** `_strip_titles` currently removes every dictionary key named
`title`, including legitimate schema property names under
`inputSchema.properties`. That corrupts tool input schemas for tools that have
a real `title` argument (for example SQL Lab requests), so clients won't see or
send that field correctly. Only strip schema metadata `title`, not property
keys in a `properties` map. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ search_tools omits real `title` input fields.
- ⚠️ SQL Lab tab-title parameter hidden from discovery.
- ⚠️ Chart config title properties disappear from schemas.
```
</details>
```suggestion
def _strip_titles(obj: Any, in_properties_map: bool = False) -> Any:
"""Recursively strip schema metadata ``title`` keys.
Keeps real field names inside ``properties`` (e.g. a property literally
named ``title``), while removing auto-generated schema title metadata.
"""
if isinstance(obj, dict):
result: dict[str, Any] = {}
for key, value in obj.items():
if key == "title" and not in_properties_map:
continue
result[key] = _strip_titles(value, in_properties_map=(key ==
"properties"))
return result
if isinstance(obj, list):
return [_strip_titles(item, in_properties_map=False) for item in obj]
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start MCP HTTP server via CLI `superset mcp run`; command `run()` in
`superset/cli/mcp.py:32-37` calls `run_server()` in
`superset/mcp_service/server.py:292`.
2. In `run_server()`, tool search transform is enabled by default because
`MCP_TOOL_SEARCH_CONFIG["enabled"]` is `True` at
`superset/mcp_service/mcp_config.py:251-253`, and
`_apply_tool_search_transform()` is
invoked at `superset/mcp_service/server.py:327-329` (or `367-368`).
3. `_apply_tool_search_transform()` wires
`search_result_serializer=_serialize_tools_without_output_schema`
(`superset/mcp_service/server.py:236`), and serializer calls
`_strip_titles(input_schema)`
(`superset/mcp_service/server.py:185-186`).
4. `_strip_titles()` removes every dict key named `"title"`
(`superset/mcp_service/server.py:166`), including keys inside JSON Schema
`properties`
maps; this deletes legitimate fields like `OpenSqlLabRequest.title` defined
at
`superset/mcp_service/sql_lab/schemas.py:118-131`.
5. The affected tool is actually registered
(`superset/mcp_service/app.py:415-417`) and
consumes `request.title` in runtime logic
(`superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py:42-44` and
`76-77`), so
search results now publish a corrupted input schema that hides/removes a
real callable
parameter.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/server.py
**Line:** 158:168
**Comment:**
*Logic Error: `_strip_titles` currently removes every dictionary key
named `title`, including legitimate schema property names under
`inputSchema.properties`. That corrupts tool input schemas for tools that have
a real `title` argument (for example SQL Lab requests), so clients won't see or
send that field correctly. Only strip schema metadata `title`, not property
keys in a `properties` map.
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%2F38562&comment_hash=f72159198db90c9f95456e6faf53f6fb0e1c036f380addeed8cc357f98a3b525&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38562&comment_hash=f72159198db90c9f95456e6faf53f6fb0e1c036f380addeed8cc357f98a3b525&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]
