aminghadersohi commented on PR #45084:
URL: https://github.com/apache/superset/pull/45084#issuecomment-6058671553
## Acceptance test at head b2558a6d715a772643dfb8c1a6022e8d4c30f264
Setup: fresh Python 3.11 venv from `requirements/development.txt` + editable
install of this head, a throwaway SQLite metadata DB (`superset db upgrade`,
`fab create-admin`, `superset init`), and real `superset mcp run` servers over
streamable HTTP, queried with a `fastmcp.Client` (`tools/list`, `tools/call`).
"Before" is the same setup at the base commit 8609bf70cd (the
native-named-tools branch). Servers run as the dev admin, which sees 77 tools
in this config. Servers, worktree and DB are torn down.
| Server | Config | Port |
| --- | --- | --- |
| before | tool search off, base commit | 5104 |
| default | tool search off, `MCP_NATIVE_TOOL_LIST_CONFIG` unset | 5101 |
| compact | tool search off, `{"compact": True, "max_description_length":
300}` | 5102 |
| search on | tool search on, compact config also set | 5103 |
| default / compact + structured | as above with
`MCP_STRUCTURED_OUTPUT_ENABLED = True` | 5105 / 5106 |
Unit tests: `test_native_tool_surface.py`, `test_mcp_server.py`,
`test_tool_description_constraints.py`, `test_middleware_logging.py`: **266
passed**.
### Acceptance criteria this PR claims
**1. Each tool is listed under its real name with a bounded, useful
description, and its input schema, output schema and annotations stay
accurate.** PASS
- Live `tools/list`: before, default and compact all list the same 77 names.
The compact server logs `Compact native tool list enabled
(max_description_length=300)`.
- Compact: the longest description is 300 chars (none over). Every compact
description is a prefix of the full one after docstring dedent. Before vs
compact: `generate_chart` 6,688 → 33 chars, `update_chart` 2,652 → 45.
- For every tool, every wire field except `description` is byte-identical
between default and compact: `inputSchema`, `annotations` (77/77 present), and
`outputSchema` (77/77 present in structured mode).
- Note, not a failure: `generate_chart`'s compact description is only
"Preview a chart; optionally save.", because its request-parameter instructions
use most of the budget. The pointer to `get_chart_type_schema` is still listed,
in the `config` field description of the input schema, and
`get_chart_type_schema(chart_type="xy")` returns fields and examples.
**2. One canonical definition and execution path, with both
structured-output settings tested.** PASS
- A single `_bounded_description` helper serves both the tool-search
serializers and the compact transform. The transform only overrides
`list_tools`, so `get_tool`/`tools/call` resolve the registered tool.
- Live `tools/call`, default vs compact, 9 cases: `health_check`,
`get_chart_type_schema`, `list_datasets` (valid and `page_size` over the max),
`list_charts` `page=0`, `list_dashboards` with a bad filter column,
`generate_chart` with no config and with an xy config, `update_chart` with a
non-object config. `isError` and the text are identical in every case, apart
from timestamps, uptime and query duration. Example: `Validation error in
list_datasets: request.page_size: Must be at most the allowed maximum`.
- Structured mode: `structuredContent` is present on both servers and
identical apart from `timestamp`. Validation errors are identical.
**3. Schemas stay valid and keep nullability, server validation is
unaffected, and catalog size is measured.** PASS
- Every compact `inputSchema` passes `Draft202012Validator.check_schema`.
There are no unresolved `$ref`s (refs are inlined, 0 remaining) and 227
nullable `anyOf` branches, identical to default.
- Live listing size (compact JSON, chart_type enum included):
| Listing | text-only | structured |
| --- | --- | --- |
| before | 215,049 B | — |
| default (trims only) | 207,339 B (−3.6%) | 508,542 B |
| compact | 155,719 B (−27.6%) | 456,838 B |
These relative reductions match the PR body's table. `tools/list` returns
one page (`nextCursor=None`) on all servers.
**4. No default changes and the opt-in is gated.** PASS
- With the config unset, the default server lists full descriptions and does
not log the compact transform.
- With tool search on (`search on` server), the compact config is ignored: 4
listed tools (pinned tools plus `search_tools`/`call_tool`) and no compact log
line.
- The factory startup path reads both `MCP_TOOL_SEARCH_CONFIG` and
`MCP_NATIVE_TOOL_LIST_CONFIG` from the Flask config, and `test_mcp_server.py`
covers this.
- `docs/admin_docs/configuration/mcp-server.mdx` documents the option and
its default.
Out of scope for this PR, so not tested: RBAC/permission parity between
native and compatibility paths, mixed-version behaviour, and target-client size
qualification.
**Verdict: PASS.** No fixes needed, nothing pushed.
--
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]
