gabotorresruiz commented on code in PR #44951:
URL: https://github.com/apache/superset/pull/44951#discussion_r4190435627
##########
superset/mcp_service/utils/response_size_utils.py:
##########
@@ -450,6 +460,27 @@ def _dashboard_layout_suggestions(
)
+def _dashboard_datasets_suggestions(query_params: Dict[str, Any]) -> List[str]:
+ """Suggest the per-dataset caps that remain, or a fallback when none do."""
+ remaining: List[str] = []
+ if query_params.get("max_columns") != 0:
Review Comment:
Just a small NIT, not a blocker: these compare the raw tool arguments
against the int `0`, and Pydantic accepts `"0"` as a string for these fields,
so a client that sends `{"max_columns": "0", "max_metrics": "0"}` on a
dashboard that is still over budget is told to reduce both caps although both
are already 0 (I verified that with 300 one-column datasets through the guard;
the int form correctly lands on the "already omitted" branch). This was already
the shape for `max_columns` on the previous revision, so not a regression from
this commit. `_parse_page_size` is already in this file and handles exactly
that:
```python
if _parse_page_size(query_params.get("max_columns")) != 0:
```
and the same for `max_metrics`. A
`test_dashboard_datasets_zero_caps_as_strings_have_honest_size_advice` next to
the existing zero-caps case would pin it.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -2775,7 +2788,7 @@ def _serialize_dashboard_dataset(
type=getattr(column, "type", None),
is_dttm=getattr(column, "is_dttm", None),
)
- for column in all_columns[:MAX_DASHBOARD_DATASET_COLUMNS]
+ for column in all_columns[:max_columns]
Review Comment:
Confirmed on `384970c`: two calls with different shuffled relationship
orders now return the same sorted prefix for both the table and the semantic
view entry. Thanks.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -417,16 +417,34 @@ def _require_identifier_or_permalink(self) ->
"GetDashboardLayoutRequest":
return self
+# 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 GetDashboardDatasetsRequest(BaseModel):
- """Request schema for get_dashboard_datasets."""
+ """Dataset detail caps."""
Review Comment:
Just a small NIT: the list_tools middleware copies this docstring onto the
served `properties.request.description`, so the whole request wrapper is now
advertised as `Dataset detail caps.` while its only required field is
`identifier`. Something like `Dashboard lookup plus per-dataset detail caps.`
keeps the compaction and still describes the object. The range and default
survive as `minimum`, `maximum` and `default` on both fields, so nothing else
was lost here.
##########
superset/mcp_service/utils/response_size_utils.py:
##########
@@ -450,6 +459,22 @@ def _dashboard_layout_suggestions(
)
+def _dashboard_datasets_suggestions(query_params: Dict[str, Any]) -> List[str]:
+ """Suggest the column cap, or a fallback when columns are already
omitted."""
+ if query_params.get("max_columns") == 0:
+ return [
+ "Column details are already omitted. This tool cannot reduce "
Review Comment:
Verified on `384970c`: the 10 x 120 x 50 shape goes from 69,415 bytes with
only `max_columns=0` to 4,225 bytes with both caps at 0, and the advice names
whichever cap is still above 0. Thanks for folding it in.
--
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]
