codeant-ai-for-open-source[bot] commented on code in PR #40961:
URL: https://github.com/apache/superset/pull/40961#discussion_r3501406850
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1717,3 +1732,225 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+# Per-dataset caps keep responses small enough for LLM context: wide
+# datasets can have hundreds of columns, which would dwarf the fields an
+# agent actually needs to configure native filters.
+MAX_DASHBOARD_DATASET_COLUMNS: int = 100
+MAX_DASHBOARD_DATASET_METRICS: int = 50
+
+
+class DashboardDatasetColumn(BaseModel):
+ """Lean column representation for dashboard dataset context."""
+
+ column_name: str = Field(..., description="Column name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ type: str | None = Field(None, description="Column data type")
+ is_dttm: bool | None = Field(None, description="Is datetime column")
+
+
+class DashboardDatasetMetric(BaseModel):
+ """Lean metric representation for dashboard dataset context."""
+
+ metric_name: str = Field(..., description="Saved metric name")
+ verbose_name: str | None = Field(None, description="Verbose (display)
name")
+ expression: str | None = Field(None, description="SQL expression")
+
+
+class DashboardDatasetDatabaseInfo(BaseModel):
+ """Database connection summary for a dashboard dataset."""
+
+ id: int | None = Field(None, description="Database ID")
+ name: str | None = Field(None, description="Database name")
+ backend: str | None = Field(None, description="Database backend (engine)")
+
+
+class DashboardDatasetSummary(BaseModel):
+ """A dataset used by a dashboard's charts, with columns and metrics."""
+
+ id: int | None = Field(None, description="Dataset ID")
+ uuid: str | None = Field(None, description="Dataset UUID")
+ table_name: str | None = Field(None, description="Table name")
+ schema_name: str | None = Field(None, description="Schema name")
+ database: DashboardDatasetDatabaseInfo | None = Field(
+ None, description="Database the dataset belongs to"
+ )
+ chart_count: int = Field(
+ 0, description="Number of charts on the dashboard using this dataset"
+ )
+ columns: List[DashboardDatasetColumn] = Field(
+ default_factory=list, description="Dataset columns"
+ )
+ metrics: List[DashboardDatasetMetric] = Field(
+ default_factory=list, description="Dataset metrics"
+ )
+ total_column_count: int = Field(
+ 0, description="Total number of columns on the dataset"
+ )
+ total_metric_count: int = Field(
+ 0, description="Total number of metrics on the dataset"
+ )
+ columns_truncated: bool = Field(
+ False,
+ description=(
+ "True when the columns list was truncated to keep the response
small"
+ ),
+ )
+ metrics_truncated: bool = Field(
+ False,
+ description=(
+ "True when the metrics list was truncated to keep the response
small"
+ ),
+ )
+
+ @model_serializer(mode="wrap")
+ def _rename_schema_field(self, serializer: Any, info: Any) -> Dict[str,
Any]:
+ """Serialize 'schema_name' as 'schema' to match API conventions."""
+ data = serializer(self)
+ if "schema_name" in data:
+ data["schema"] = data.pop("schema_name")
+ return data
+
Review Comment:
**Suggestion:** The response model declares `schema_name` as the field but
then rewrites it to `schema` only during serialization, which creates an API
contract mismatch: generated schema/introspection will advertise `schema_name`
while runtime payloads return `schema`. This can break clients and tool
consumers that rely on the declared schema. Define the field with an
alias/serialization alias for `schema` directly instead of renaming in a custom
serializer so the declared schema and actual payload stay consistent. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MCP tool schema advertises schema_name, payload returns schema.
- ⚠️ Strict JSON-schema clients may reject mismatched responses.
- ⚠️ Typed SDKs for MCP tools get incorrect field mapping.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the MCP service so FastMCP registers the `get_dashboard_datasets`
tool defined in
`superset/mcp_service/dashboard/tool/get_dashboard_datasets.py:50-62`, which
declares
`DashboardDatasets` (containing `DashboardDatasetSummary`) as the
`output_schema` at
`get_dashboard_datasets.py:119-121`.
2. Observe that the tool framework (via `superset_core.mcp.decorators.tool`
and Pydantic
v2) uses `DashboardDatasets.model_json_schema()` to describe the tool
output; in
`superset/mcp_service/dashboard/schemas.py:170-176`
`DashboardDatasetSummary` defines a
field named `schema_name`, so the generated JSON schema will advertise a
property called
`schema_name` for each dataset.
3. Trigger a normal tool invocation by calling `get_dashboard_datasets` with
any valid
dashboard identifier (e.g. ID 123) so `ModelGetInfoCore.run_tool` is
executed with
`serializer=dashboard_datasets_serializer`
(`get_dashboard_datasets.py:117-126`), which in
turn builds `DashboardDatasetSummary` instances and serializes them using
Pydantic’s
`model_dump`.
4. During serialization, the custom `@model_serializer`
`_rename_schema_field` in
`superset/mcp_service/dashboard/schemas.py:208-214` rewrites the
`schema_name` key to
`schema` (`data["schema"] = data.pop("schema_name")`), so the actual runtime
JSON payload
contains a `schema` property while the advertised tool schema still documents
`schema_name`, creating a contract mismatch that will break clients or tool
consumers that
rely on the declared schema (e.g. strict JSON-schema-driven tooling or typed
SDKs).
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c47604dc752a46158d4b7e119482a4ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c47604dc752a46158d4b7e119482a4ee&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/dashboard/schemas.py
**Line:** 1776:1814
**Comment:**
*Api Mismatch: The response model declares `schema_name` as the field
but then rewrites it to `schema` only during serialization, which creates an
API contract mismatch: generated schema/introspection will advertise
`schema_name` while runtime payloads return `schema`. This can break clients
and tool consumers that rely on the declared schema. Define the field with an
alias/serialization alias for `schema` directly instead of renaming in a custom
serializer so the declared schema and actual payload stay consistent.
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%2F40961&comment_hash=8ab7b637361f9fc486be41349063b648925445185a5d17f8a6d884fcd9995a08&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40961&comment_hash=8ab7b637361f9fc486be41349063b648925445185a5d17f8a6d884fcd9995a08&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]
