rebenitez1802 commented on code in PR #45547:
URL: https://github.com/apache/superset/pull/45547#discussion_r4341490650
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -190,7 +190,12 @@ selection the control cannot render.
Select values are checked against the target column's distinct values under the
caller's dataset permissions and row-level security. Exact values are
preserved;
a unique match ignoring case and whitespace uses the stored column value.
-Missing values fail without creating a permalink, with bounded suggestions.
+Missing values fail without creating a permalink. Unless an active dashboard
+scope suppresses them, the error lists bounded candidate values (closest
+matches, or the column's first values), and the tool description directs
Review Comment:
Applied your suggestion in 4a514f4ce4. Thanks for catching the unordered
`LIMIT` page.
##########
superset/mcp_service/dashboard/tool/apply_dashboard_filters.py:
##########
@@ -799,10 +801,11 @@ async def apply_dashboard_filters(
) -> ApplyDashboardFiltersResponse:
"""
Apply values to a dashboard's existing native filters for this user.
+ If a value is rejected, propose the listed value the user likely meant
+ and ask before applying it.
Use this to answer "show me the dashboard filtered to X". It returns a
- shareable ``/dashboard/p/<key>/`` permalink that opens the dashboard
- with the requested values already applied. The dashboard's saved
+ shareable ``/dashboard/p/<key>/`` permalink. The dashboard's saved
Review Comment:
Agreed that losing it was a side effect, not intentional. In 4a514f4ce4 I
restored it as the sentence right after the permalink one ("The link opens the
dashboard with the requested values already applied."). It turned out the
budget doesn't need a bump. `test_tool_inventory_size` measures the compact
`search_tools` entry, and this clause falls after the 300-char cut. So the
compact text is unchanged (284 chars) and the entry is still 3,672 B. Only the
full description grows, from 2,706 to 2,782 chars.
##########
tests/unit_tests/mcp_service/native_tool_inventory.json:
##########
@@ -1267,12 +1267,12 @@
},
"totals": {
"catalog_bytes": {
- "structured": 524459,
- "text_only": 214456
+ "structured": 524939,
+ "text_only": 214936
Review Comment:
Done: I merged master and regenerated the snapshot with
`SUPERSET_MCP_UPDATE_TOOL_INVENTORY=1` instead of picking a side by hand. The
regen also refreshed `get_chart_data`, `get_tag_info` and `list_tags`. Those
entries were stale on master after #45250 and #43471, but within the
`size_budget` tolerance, so CI never flagged them.
--
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]
