aminghadersohi commented on code in PR #44266:
URL: https://github.com/apache/superset/pull/44266#discussion_r4015821303
##########
superset/mcp_service/semantic_layer/schemas.py:
##########
@@ -89,6 +89,11 @@ class MetricInfo(BaseModel):
# ---------------------------------------------------------------------------
+# Measured with 40 dimensions per metric: 20x40 = 42,336 tokens;
+# 10x40 = 21,186; 8x40 = 16,956 (18,284 fallback), against a 25,000 limit.
+EMBEDDED_DIMENSIONS_MAX_PAGE_SIZE = 8
Review Comment:
Calibrated to the default `token_limit`, but the guard reads
`MCP_RESPONSE_SIZE_CONFIG['token_limit']` at runtime (middleware.py:1511).
Raising it cannot relax this cap; lowering it still overflows at 8. No fence:
deriving the cap from config is a design call, not a mechanical edit.
##########
superset/mcp_service/semantic_layer/schemas.py:
##########
@@ -105,17 +110,35 @@ class ListMetricsRequest(BaseModel):
description="Filter to metrics from a specific semantic view.",
)
include_compatible_dimensions: bool = Field(
- default=True,
+ default=False,
description=(
- "When True, each metric includes its list of compatible
dimensions. "
- "Set to False to reduce response size when dimensions aren't
needed."
+ "Embed compatible dimensions only when explicitly requested. "
+ "Use get_compatible_dimensions for the full per-metric list. "
+ "When True, set page_size to at most 8."
),
)
page: int = Field(default=1, ge=1, description="1-based page number.")
page_size: int = Field(
- default=50, ge=1, le=500, description="Number of metrics per page."
+ default=25, ge=1, le=500, description="Number of metrics per page."
)
+ @model_validator(mode="after")
+ def validate_embedded_dimensions_page_size(self) -> "ListMetricsRequest":
+ """Reject embedded pages that risk exceeding the MCP response guard."""
+ if (
+ self.include_compatible_dimensions
+ and self.page_size > EMBEDDED_DIMENSIONS_MAX_PAGE_SIZE
+ ):
+ raise ValueError(
+ "Embedded compatible dimensions require "
+ f"page_size <= {EMBEDDED_DIMENSIONS_MAX_PAGE_SIZE}: each
external "
+ "metric's dimension list can consume roughly 1–2k tokens or
more, "
+ "and the MCP response guard rejects responses above ~25k
tokens. "
Review Comment:
This hardcodes the default of an operator knob the guard honours, so it
misstates the limit on any deployment that retunes
`MCP_RESPONSE_SIZE_CONFIG['token_limit']`. `dashboard/schemas.py:453,473` names
the key instead of a number for the sibling `max_list_items` cap.
```suggestion
"and the MCP response guard rejects responses above "
"MCP_RESPONSE_SIZE_CONFIG['token_limit'] (~25k by default). "
```
--
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]
