aminghadersohi commented on code in PR #40355:
URL: https://github.com/apache/superset/pull/40355#discussion_r3326680195


##########
superset/mcp_service/app.py:
##########
@@ -670,6 +670,9 @@ def create_mcp_app(
     get_schema,
     health_check,
 )
+from superset.mcp_service.theme.tool import (  # noqa: F401, E402
+    create_theme,
+)

Review Comment:
   Fixed in commit `899b9f70cd` (feat(mcp): address review comments for 
create_theme MCP tool). A `Theme Management` section was added to 
`get_default_instructions()` listing both `create_theme` and `update_theme` 
with write-access notes. Both tool names also appear in the prose 
permission-awareness block, so the existing line-based `disabled_tools` filter 
strips them automatically when either is in `MCP_DISABLED_TOOLS`.



##########
superset/mcp_service/theme/schemas.py:
##########
@@ -0,0 +1,48 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Pydantic schemas for theme-related MCP tool requests and responses.
+"""
+
+from __future__ import annotations
+
+from pydantic import BaseModel, ConfigDict, Field
+
+
+class CreateThemeRequest(BaseModel):
+    model_config = ConfigDict(populate_by_name=True)
+
+    theme_name: str = Field(
+        ...,
+        description="Name of the theme to create.",
+    )
+    json_data: str = Field(
+        ...,
+        description=(
+            "JSON string containing the theme configuration. "
+            "Must be valid Ant Design theme token JSON, e.g. "
+            '\'{"token": {"colorPrimary": "#1677ff"}}\'.'
+        ),
+    )

Review Comment:
   Fixed in `899b9f70cd`. `json_data` is now typed as `str | dict[str, Any]` 
with a `@field_validator(mode="before")` that calls `parse_json_or_passthrough` 
and serializes any incoming dict to a JSON string before passing it to 
`ThemePostSchema`. Same pattern applied to `UpdateThemeRequest.json_data`.



##########
superset/mcp_service/theme/tool/create_theme.py:
##########
@@ -0,0 +1,113 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import logging
+
+from fastmcp import Context
+from marshmallow import ValidationError
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.extensions import event_logger
+from superset.mcp_service.theme.schemas import (
+    CreateThemeRequest,
+    CreateThemeResponse,
+)
+
+logger = logging.getLogger(__name__)
+
+
+@tool(
+    tags=["mutate"],
+    class_permission_name="Theme",
+    method_permission_name="write",
+    annotations=ToolAnnotations(
+        title="Create theme",
+        readOnlyHint=False,
+        destructiveHint=False,
+    ),
+)
+async def create_theme(
+    request: CreateThemeRequest, ctx: Context
+) -> CreateThemeResponse:

Review Comment:
   Added in `899b9f70cd`. 
`tests/unit_tests/mcp_service/theme/tool/test_create_theme.py` covers: (1) 
schema validation — `CreateThemeRequest` accepts JSON string or native dict, 
rejects missing fields; (2) happy-path tool call via `Client`; (3) dict 
`json_data` passthrough from LLM clients; (4) validation errors for blank name 
and malformed JSON; (5) `UpdateThemeRequest` schema variants (dict, None, 
missing id); and (6) `update_theme` tool — success, partial update, not-found, 
and system-theme-protected paths.



-- 
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]

Reply via email to