You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/09/23 11:05:32 UTC
[PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
aminghadersohi opened a new pull request, #44575:
URL: https://github.com/apache/superset/pull/44575
### SUMMARY
Make chart-tool argument schemas retrievable without expanding shared models or weakening their contract. Addresses [SC-121707](https://app.shortcut.com/preset/story/121707).
The search serializer recursively inlined every reference and removed `$defs`. This duplicated chart submodels, discarded nullable alternatives and reference siblings, and replaced recursive references with unconstrained objects. Remove that lossy pass: retain the original definitions, references, discriminator mappings, nullable unions, and constraints. Existing title/output-schema stripping and description limits remain unchanged. Authorization, sanitization, and response-size guards are untouched.
The legacy `compact_schemas` setting continues to select the default description limit; documentation explains reference resolution relative to each tool's `inputSchema`. Search ranking and description-truncation behavior are deliberately outside this change (SC-121708 / SC-121711).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI change. Measured the actual registered Apache inventory entries, including tool metadata, using compact JSON (`ensure_ascii=False`, `separators=(',', ':')`), UTF-8 byte counts, and **tiktoken 0.14.0 / cl100k_base**. Tokens are BPE client estimates, not Claude-tokenizer counts; no bytes/token conversion is used.
| Tool | Before bytes | After bytes | Before tokens | After tokens |
|---|---:|---:|---:|---:|
| generate_chart | 97,013 | 49,531 | 23,543 | 11,849 |
| update_chart | 104,163 | 53,395 | 25,227 | 12,759 |
| generate_explore_link | 96,581 | 49,230 | 23,478 | 11,806 |
Reference duplication accounts for approximately half the inventory size. Preserving the complete contract does **not** meet the proposed 50,000-byte target for `update_chart`: its regression budget is explicitly 55,000 bytes, not a raised production cap. The other two budgets are 50,000 bytes; each token budget is independently 20,000 tokens.
A local fixture using the existing gateway renderer, MCP Tool canonicalization and one-definition pagination also validates delivery overhead:
| Tool | Rendered content bytes, before → after | Rendered content tokens, before → after | After gateway guard token estimate |
|---|---:|---:|---:|
| generate_chart | 103,564 → 52,807 | 29,322 → 14,724 | 14,801 |
| update_chart | 111,144 → 56,883 | 31,388 → 15,823 | 15,900 |
| generate_explore_link | 103,093 → 52,466 | 29,219 → 14,641 | 14,718 |
All three fit the existing 100KB/25k-token delivery limits individually, with no required constraint removed.
### TESTING INSTRUCTIONS
Using the development dependencies (including the pinned test-only tokenizer):
```bash
pytest tests/unit_tests/mcp_service/test_tool_search_transform.py tests/unit_tests/mcp_service/test_chart_tool_inventory.py
pytest tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py tests/unit_tests/mcp_service/chart/tool/test_update_chart.py tests/unit_tests/mcp_service/explore/tool/test_generate_explore_link.py
```
- **62 passed** in the search/inventory suite; **178 passed** in the existing tool execution suites.
- **11 new regression cases**: three real per-tool inventory budgets, three full-schema/reference fidelity checks, three schema-first FastMCP invocations with typed side-effect-free fixtures, and two shared-schema fidelity tests.
- Each published schema is checked by a Draft 2020-12 validator, every reference/discriminator target resolves locally, and its content equals the original input schema except the existing title stripping. Valid table arguments pass; invalid column-name constraints remain enforced.
- Restoring only the original `server.py` makes **all three size regression cases fail** (97,013 / 104,163 / 96,581 bytes).
- Read-only prod/staging/SDX inventory rechecks match the previous recorded sizes. SDX authentication initially returned 502 and succeeded on one retry. No production writes or staging-chart changes; deployment acceptance remains necessary after merge/release.
- Ran staged-file pre-commit, including mypy and pylint. All applicable hooks pass except **ruff-format's pre-existing unrelated assertion formatting** in `test_call_tool_proxy_rejects_synthetic_names_with_warning_log_level`. Its auto-format was reverted to honor the shared-file, hunk-local constraint; no unrelated formatting change is included.
### ADDITIONAL INFORMATION
- [x] Has associated issue: [SC-121707](https://app.shortcut.com/preset/story/121707)
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5797437404
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5800376150
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5793703886
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5797453111
Measured all **72 registered tools**, not only chart tools, comparing the pre-change serializer at `f8f293d266` with `90fe26612e`. Same registered tool models/config on both sides; complete serialized entries including metadata; compact JSON (`ensure_ascii=False`, `separators=(',', ':')`), UTF-8 bytes, and **tiktoken 0.14.0 / cl100k_base**. Tokens are BPE estimates, not Claude-tokenizer counts.
**Totals (sum of individual entries): 531,433 → 348,099 bytes (−34.50%); 126,042 → 81,354 tokens (−35.45%).** Four tools shrink, two are unchanged, and **66 grow**. Every increase is identified by its positive deltas below. The additional large win outside the original three is `update_chart_preview`: −49,943 bytes / −12,275 tokens.
The increases total +12,210 bytes / +3,421 tokens across those 66 tools. The largest individual increase is `manage_native_filters`, +706 bytes / +210 tokens, ending at 6,544 bytes / 1,438 tokens. These are small absolute increases from retaining nullable alternatives, `$defs`/`$ref` wrappers, and reference-sibling metadata that the old pass discarded. They are accepted costs of preserving the original published schema, not repeated-model expansion; reinstating the old simplification would lose fidelity. No growing entry exceeds 6,544 bytes or 1,438 tokens, so no material delivery-size regression was found and no additional runtime change was needed.
Added `test_tool_inventory.py`: 72 independent byte/token budget cases, 72 complete-schema/non-mutation checks, and an inventory-completeness check so new tools cannot silently escape coverage. **207 inventory/search tests and 178 execution tests pass**; all applicable branch-file pre-commit hooks pass. Injecting schema growth into `get_database_info` makes its budget case fail as expected.
<details>
<summary>Full inventory: every tool, including all 66 increases</summary>
| Tool | Bytes before → after | Δ bytes | Tokens before → after | Δ tokens |
|---|---:|---:|---:|---:|
| `add_chart_to_existing_dashboard` | 1,259 → 1,372 | +113 | 281 → 312 | +31 |
| `apply_dashboard_filters` | 2,553 → 2,789 | +236 | 560 → 625 | +65 |
| `create_dataset` | 1,491 → 1,648 | +157 | 321 → 365 | +44 |
| `create_theme` | 920 → 989 | +69 | 203 → 221 | +18 |
| `create_virtual_dataset` | 2,887 → 3,536 | +649 | 618 → 792 | +174 |
| `delete_chart` | 877 → 946 | +69 | 192 → 210 | +18 |
| `delete_dashboard` | 878 → 955 | +77 | 188 → 206 | +18 |
| `duplicate_dashboard` | 1,645 → 1,728 | +83 | 340 → 359 | +19 |
| `execute_sql` | 1,791 → 1,970 | +179 | 377 → 429 | +52 |
| `find_users` | 1,319 → 1,384 | +65 | 301 → 320 | +19 |
| `generate_bug_report` | 2,040 → 2,426 | +386 | 449 → 559 | +110 |
| `generate_chart` | 97,013 → 49,531 | -47,482 | 23,543 → 11,849 | -11,694 |
| `generate_dashboard` | 3,047 → 3,296 | +249 | 637 → 709 | +72 |
| `generate_explore_link` | 96,581 → 49,230 | -47,351 | 23,478 → 11,806 | -11,672 |
| `get_annotation_layer_info` | 712 → 803 | +91 | 158 → 181 | +23 |
| `get_chart_data` | 2,526 → 2,709 | +183 | 554 → 608 | +54 |
| `get_chart_info` | 3,279 → 3,462 | +183 | 721 → 777 | +56 |
| `get_chart_preview` | 3,027 → 3,300 | +273 | 661 → 744 | +83 |
| `get_chart_sql` | 1,780 → 1,905 | +125 | 390 → 428 | +38 |
| `get_chart_type_schema` | 701 → 701 | +0 | 149 → 149 | +0 |
| `get_compatible_dimensions` | 1,236 → 1,385 | +149 | 242 → 281 | +39 |
| `get_compatible_metrics` | 1,227 → 1,370 | +143 | 243 → 282 | +39 |
| `get_dashboard_data` | 2,188 → 2,295 | +107 | 484 → 514 | +30 |
| `get_dashboard_datasets` | 911 → 998 | +87 | 195 → 218 | +23 |
| `get_dashboard_info` | 2,818 → 2,953 | +135 | 609 → 648 | +39 |
| `get_dashboard_layout` | 1,356 → 1,467 | +111 | 284 → 314 | +30 |
| `get_database_info` | 1,222 → 1,299 | +77 | 277 → 298 | +21 |
| `get_dataset_info` | 2,126 → 2,201 | +75 | 507 → 528 | +21 |
| `get_instance_info` | 604 → 710 | +106 | 127 → 156 | +29 |
| `get_layer_annotation_info` | 905 → 996 | +91 | 196 → 219 | +23 |
| `get_query_info` | 848 → 919 | +71 | 195 → 216 | +21 |
| `get_report_info` | 1,159 → 1,232 | +73 | 248 → 269 | +21 |
| `get_rls_filter_info` | 761 → 840 | +79 | 176 → 201 | +25 |
| `get_role_info` | 675 → 744 | +69 | 156 → 177 | +21 |
| `get_saved_query_info` | 935 → 1,016 | +81 | 224 → 247 | +23 |
| `get_schema` | 876 → 941 | +65 | 183 → 202 | +19 |
| `get_table` | 3,453 → 3,707 | +254 | 798 → 877 | +79 |
| `get_tag_info` | 825 → 892 | +67 | 184 → 205 | +21 |
| `get_task_info` | 888 → 957 | +69 | 200 → 221 | +21 |
| `get_theme_info` | 809 → 880 | +71 | 173 → 194 | +21 |
| `get_user_info` | 805 → 874 | +69 | 184 → 203 | +19 |
| `health_check` | 590 → 590 | +0 | 127 → 127 | +0 |
| `list_annotation_layers` | 2,245 → 2,540 | +295 | 499 → 574 | +75 |
| `list_charts` | 4,635 → 4,946 | +311 | 1,030 → 1,118 | +88 |
| `list_dashboards` | 4,120 → 4,419 | +299 | 918 → 1,000 | +82 |
| `list_databases` | 3,119 → 3,386 | +267 | 694 → 767 | +73 |
| `list_datasets` | 4,195 → 4,486 | +291 | 919 → 1,000 | +81 |
| `list_layer_annotations` | 2,481 → 2,738 | +257 | 569 → 634 | +65 |
| `list_metrics` | 1,527 → 1,708 | +181 | 315 → 368 | +53 |
| `list_queries` | 2,555 → 2,812 | +257 | 577 → 648 | +71 |
| `list_reports` | 3,458 → 3,717 | +259 | 776 → 847 | +71 |
| `list_rls_filters` | 2,225 → 2,496 | +271 | 509 → 587 | +78 |
| `list_roles` | 2,340 → 2,591 | +251 | 541 → 612 | +71 |
| `list_saved_queries` | 2,524 → 2,801 | +277 | 581 → 656 | +75 |
| `list_tags` | 2,671 → 2,918 | +247 | 634 → 705 | +71 |
| `list_tasks` | 2,245 → 2,508 | +263 | 513 → 586 | +73 |
| `list_themes` | 2,563 → 2,818 | +255 | 600 → 671 | +71 |
| `list_users` | 2,512 → 2,763 | +251 | 580 → 650 | +70 |
| `manage_dashboard_certification` | 1,633 → 1,792 | +159 | 359 → 400 | +41 |
| `manage_dashboard_owners` | 1,943 → 2,032 | +89 | 420 → 441 | +21 |
| `manage_dashboard_roles` | 1,693 → 1,780 | +87 | 360 → 381 | +21 |
| `manage_native_filters` | 5,838 → 6,544 | +706 | 1,228 → 1,438 | +210 |
| `open_sql_lab_with_context` | 1,489 → 1,668 | +179 | 319 → 372 | +53 |
| `query_dataset` | 3,320 → 3,562 | +242 | 770 → 840 | +70 |
| `remove_chart_from_dashboard` | 1,030 → 1,125 | +95 | 216 → 239 | +23 |
| `restore_chart` | 892 → 963 | +71 | 195 → 214 | +19 |
| `restore_dashboard` | 753 → 832 | +79 | 157 → 176 | +19 |
| `save_sql_query` | 1,294 → 1,449 | +155 | 285 → 331 | +46 |
| `update_chart` | 104,163 → 53,395 | -50,768 | 25,227 → 12,759 | -12,468 |
| `update_chart_preview` | 102,352 → 52,409 | -49,943 | 24,838 → 12,563 | -12,275 |
| `update_dashboard` | 3,598 → 3,983 | +385 | 726 → 843 | +117 |
| `update_dataset_metric` | 2,477 → 2,972 | +495 | 554 → 688 | +134 |
</details>
**CI:** Re-read the rollup at `d6e5bafb`: **85 SUCCESS / 10 SKIPPED / 3 NEUTRAL / 0 FAILURE / 0 PENDING**. `test-load-examples`, `testcontainers (starrocks, 15)`, `playwright-tests (chromium)`, `babel-extract`, and every other Playwright job all passed. Between `25e553307f` and that head, only pytest-mark formatting changed; the `tiktoken`/`regex` dependency pins did not change. This does not establish the cause of the earlier failures.
The test-only follow-up head `90fe26612e` has a new CI run: 3 IN_PROGRESS / 1 PENDING / 28 QUEUED / 3 SKIPPED as of 2026-09-23 15:14 UTC. Those new-head checks are not yet complete.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5806628164
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5793725563
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 25e553307f845403f35e79d8dd2d21a926669f22 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ab3b27d667d7b000930a256 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44575--superset-docs-preview.netlify.app](https://deploy-preview-44575--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8y6gVHJoW7QMcYPGGSKtqno7rl7i_dCbSXXC1aACVyT06fh7lFpxcSYfcXuU8yCfX8i6tFCrEe5tzCiFWec5mAtB3ZEtBgSPVv4fg) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44575--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on code in PR #44575:
URL: https://github.com/apache/superset/pull/44575#discussion_r4096733836
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -763,7 +763,7 @@ MCP_TOOL_SEARCH_CONFIG = {
"search_tool_name": "search_tools",
"call_tool_name": "call_tool",
"include_schemas": False, # False=summary mode (name + parameters_hint)
Review Comment:
Just a small NIT that does not hold up the approval, and the Bito run flagged it too: fixing the table row two lines below left this example block saying the opposite. It still has `"include_schemas": False` while the row now documents the default as `True`.
I checked every other key in this block against `MCP_TOOL_SEARCH_CONFIG` in `superset/mcp_service/mcp_config.py`, and `enabled`, `strategy`, `max_results`, `always_visible`, `search_tool_name`, `call_tool_name`, `compact_schemas` and `max_description_length` all match the shipped values. The `always_visible` row also points back here with `See above`, so a reader does take this block as the defaults. `include_schemas` is now the only key in it that disagrees with what ships.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44575:
URL: https://github.com/apache/superset/pull/44575#discussion_r4088422285
##########
tests/unit_tests/mcp_service/test_tool_inventory.py:
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Per-tool size and schema fidelity budgets for the entire registered inventory."""
+
+from copy import deepcopy
+
+import pytest
+import tiktoken
+from jsonschema import Draft202012Validator
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.mcp_config import MCP_TOOL_SEARCH_CONFIG
+from superset.mcp_service.server import _create_search_result_serializer, _strip_titles
+from superset.utils import json
+
+# Compact JSON including tool metadata, measured independently as UTF-8 bytes
+# and tiktoken 0.14.0 / cl100k_base tokens (BPE estimates, not Claude counts).
+# Small tools round up to 100 bytes / 25 tokens; large chart tools retain their
+# explicit delivery budgets. Keep both limits: bytes are not a token estimate.
+TOOL_BUDGETS = {
+ "add_chart_to_existing_dashboard": (1_400, 325),
+ "apply_dashboard_filters": (2_800, 625),
+ "create_dataset": (1_700, 375),
+ "create_theme": (1_000, 225),
+ "create_virtual_dataset": (3_600, 800),
+ "delete_chart": (1_000, 225),
+ "delete_dashboard": (1_000, 225),
+ "duplicate_dashboard": (1_800, 375),
+ "execute_sql": (2_000, 450),
+ "find_users": (1_400, 325),
+ "generate_bug_report": (2_500, 575),
+ "generate_chart": (50_000, 20_000),
+ "generate_dashboard": (3_300, 725),
+ "generate_explore_link": (50_000, 20_000),
+ "get_annotation_layer_info": (900, 200),
+ "get_chart_data": (2_800, 625),
+ "get_chart_info": (3_500, 800),
+ "get_chart_preview": (3_300, 750),
Review Comment:
Fixed in b5a91a3f4ae94a0c3fb994bb2a5e12f2ba30cd15. Regenerated the fixed small-tool byte budgets as ceil(measured_bytes / 100) * 100 + 100, so they have 100–199 bytes of headroom rather than just 1–100. All 72 tools have at least 100 bytes of measured slack; the large chart caps remain 50/55 kB. Your exact wording change measures 3,302 bytes against a 3,400-byte budget and passes. Added a regression test for an incidental description edit. Token budgets are removed as discussed in the dependency thread.
Validation: pytest tests/unit_tests/mcp_service: 5,205 passed, 4 skipped (518.56s). The skips are three Vega/Node prerequisites and one unavailable optional sqlalchemy_bigquery import; the latter accounts for the extra skip versus your baseline. Focused inventory/search tests: 209 passed. Pre-commit and explicit mypy hooks passed.
##########
pyproject.toml:
##########
@@ -327,6 +327,7 @@ development = [
"sqloxide",
"statsd",
"syntaqlite>=0.9.0,<0.10.0",
+ "tiktoken==0.14.0", # reproducible MCP inventory token budgets
Review Comment:
Fixed in b5a91a3f4ae94a0c3fb994bb2a5e12f2ba30cd15. Removed tiktoken imports/assertions from both inventory test files and its development dependency, including the lockfile entry and tokenizer-only regex dependency. I chose removal rather than skip/cache logic because the byte checks independently reject the regression: running the f8f293d2 serializer against the registered tools produced 97,013 / 104,163 / 96,581 bytes for generate_chart / update_chart / generate_explore_link, above the unchanged 50,000 / 55,000 / 50,000 caps. update_chart_preview is also caught (102,352 > 55,000).
Both inventory files passed with socket connections blocked and tiktoken imports deliberately made unavailable: 155 passed. This keeps the checks active offline rather than conditionally skipping them; no vocabulary download or approximate token substitution is needed.
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -775,7 +775,7 @@ MCP_TOOL_SEARCH_CONFIG = {
| `max_results` | `5` | Maximum tools returned per search query |
| `always_visible` | See above | Tools that always appear in `list_tools`, regardless of search |
| `include_schemas` | `False` | When `False` (default, "summary mode"), search results omit `inputSchema` entirely and include a lightweight `parameters_hint` listing top-level parameter names. Set to `True` to include the full `inputSchema` in search results. Full schemas are always used when a tool is actually invoked via `call_tool`. |
-| `compact_schemas` | `True` | Strip `$defs` / `$ref` and replace with `{"type": "object"}` in search results to reduce token cost. Only takes effect when `include_schemas=True` — ignored in summary mode. |
+| `compact_schemas` | `True` | Legacy setting selecting the default description limit (300 when `True`, 0 when `False`) if `max_description_length` is omitted. Input schemas preserve `$defs`, `$ref`, nullable unions, and constraints in either mode; titles are omitted. Clients should resolve references within each tool's `inputSchema`. |
Review Comment:
Fixed in b5a91a3f4ae94a0c3fb994bb2a5e12f2ba30cd15. Updated mcp_config.py's schema serialization block to describe preservation of $defs, $ref, nullable unions and constraints, and local reference resolution. Removed the obsolete compact_schemas=False rollback bullet and replaced the inline “Strip $defs/$ref” comment. The legacy setting is documented as selecting only the default description limit when max_description_length is omitted, consistent with the MDX documentation.
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -775,7 +775,7 @@ MCP_TOOL_SEARCH_CONFIG = {
| `max_results` | `5` | Maximum tools returned per search query |
| `always_visible` | See above | Tools that always appear in `list_tools`, regardless of search |
| `include_schemas` | `False` | When `False` (default, "summary mode"), search results omit `inputSchema` entirely and include a lightweight `parameters_hint` listing top-level parameter names. Set to `True` to include the full `inputSchema` in search results. Full schemas are always used when a tool is actually invoked via `call_tool`. |
Review Comment:
Fixed in b5a91a3f4ae94a0c3fb994bb2a5e12f2ba30cd15. The include_schemas table row now says True and explains that full inputSchema is the shipped default, with False opting into summary mode. Also removed the misleading “False (default)” wording from the serializer docstring. The empty-config fallback remains unchanged.
##########
superset/mcp_service/server.py:
##########
@@ -535,22 +430,20 @@ def _create_search_result_serializer(
~80% vs compact mode while still conveying what parameters a tool
accepts.
- When ``include_schemas`` is True, the full ``compact_schemas``/
- ``max_description_length`` pipeline applies (existing behavior):
-
- * ``$defs`` sections and ``$ref`` pointers are collapsed when
- ``compact_schemas`` is True (see :func:`_compact_schema`).
- * Tool descriptions are truncated to ``max_description_length`` chars.
+ When ``include_schemas`` is True, input schemas retain their definitions,
+ references, and validation constraints. Inlining references duplicates shared
+ chart models and can make a single tool exceed client result limits.
- Full schemas remain available when the tool is invoked via ``call_tool``.
+ Titles and output schemas are stripped by the base serializer. The legacy
+ ``compact_schemas`` setting only selects the default description limit;
+ ``max_description_length`` explicitly controls description truncation.
"""
include_schemas = config.get("include_schemas", False)
if not include_schemas:
max_desc = config.get("max_description_length", 300)
return _build_summary_serializer(max_desc)
- # include_schemas=True: apply full compact_schemas/max_description_length pipeline
compact = config.get("compact_schemas", True)
Review Comment:
Fixed in b5a91a3f4ae94a0c3fb994bb2a5e12f2ba30cd15. Applied the simplified max_desc default and early return for any falsy max_desc; the wrapper only runs when truncation is enabled. This preserves serialized output: I compared the 90fe26612 serializer with this version across 36 configurations (include_schemas omitted/False/True, compact_schemas omitted/False/True, max_description_length omitted/0/50/300) for all 72 registered tools and obtained identical results. Parameterized the explicit-zero test over both compact_schemas values. Focused inventory/search tests: 209 passed; full MCP suite: 5,205 passed, 4 skipped.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44575:
URL: https://github.com/apache/superset/pull/44575
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5793984166
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44575?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 66.09%. Comparing base ([`f8f293d`](https://app.codecov.io/gh/apache/superset/commit/f8f293d2666b1f6b44aaa53478cf357618df6575?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`25e5533`](https://app.codecov.io/gh/apache/superset/commit/25e553307f845403f35e79d8dd2d21a926669f22?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44575 +/- ##
==========================================
- Coverage 66.14% 66.09% -0.05%
==========================================
Files 2950 2950
Lines 177666 177615 -51
Branches 41177 41159 -18
==========================================
- Hits 117509 117400 -109
- Misses 57653 57705 +52
- Partials 2504 2510 +6
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.88% <ø> (+0.02%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.05% <ø> (+0.03%)` | :arrow_up: |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.82% <ø> (+0.02%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <ø> (-0.09%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44575/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.78% <ø> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44575?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44575:
URL: https://github.com/apache/superset/pull/44575#discussion_r4099903846
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -763,7 +763,7 @@ MCP_TOOL_SEARCH_CONFIG = {
"search_tool_name": "search_tools",
"call_tool_name": "call_tool",
"include_schemas": False, # False=summary mode (name + parameters_hint)
Review Comment:
Fixed in commit 24812ac219ea8776b8acc967af6c29ac78b24609: the example sets include_schemas to True, matching the shipped MCP_TOOL_SEARCH_CONFIG 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5826312512
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on code in PR #44575:
URL: https://github.com/apache/superset/pull/44575#discussion_r4087613490
##########
tests/unit_tests/mcp_service/test_tool_inventory.py:
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Per-tool size and schema fidelity budgets for the entire registered inventory."""
+
+from copy import deepcopy
+
+import pytest
+import tiktoken
+from jsonschema import Draft202012Validator
+
+from superset.mcp_service.app import mcp
+from superset.mcp_service.mcp_config import MCP_TOOL_SEARCH_CONFIG
+from superset.mcp_service.server import _create_search_result_serializer, _strip_titles
+from superset.utils import json
+
+# Compact JSON including tool metadata, measured independently as UTF-8 bytes
+# and tiktoken 0.14.0 / cl100k_base tokens (BPE estimates, not Claude counts).
+# Small tools round up to 100 bytes / 25 tokens; large chart tools retain their
+# explicit delivery budgets. Keep both limits: bytes are not a token estimate.
+TOOL_BUDGETS = {
+ "add_chart_to_existing_dashboard": (1_400, 325),
+ "apply_dashboard_filters": (2_800, 625),
+ "create_dataset": (1_700, 375),
+ "create_theme": (1_000, 225),
+ "create_virtual_dataset": (3_600, 800),
+ "delete_chart": (1_000, 225),
+ "delete_dashboard": (1_000, 225),
+ "duplicate_dashboard": (1_800, 375),
+ "execute_sql": (2_000, 450),
+ "find_users": (1_400, 325),
+ "generate_bug_report": (2_500, 575),
+ "generate_chart": (50_000, 20_000),
+ "generate_dashboard": (3_300, 725),
+ "generate_explore_link": (50_000, 20_000),
+ "get_annotation_layer_info": (900, 200),
+ "get_chart_data": (2_800, 625),
+ "get_chart_info": (3_500, 800),
+ "get_chart_preview": (3_300, 750),
Review Comment:
This block worries me a bit. The header says small tools round up to 100 bytes and 25 tokens, but rounding a measurement that already lands on the increment gives the measurement back, so six entries ship with zero headroom.
Measured on this branch with the exact arithmetic `test_tool_inventory_size` uses: `get_chart_preview` is 3300 bytes against a 3300 byte budget, and `manage_dashboard_certification` (400), `apply_dashboard_filters` (625), `list_users` (650), `list_datasets` (1000) and `list_dashboards` (1000) each sit exactly on their token budget. Eleven tools have 10 bytes or less of slack.
I checked what that costs the next contributor. On this branch I changed the `get_chart_preview` docstring from `Get chart preview by ID or UUID.` to `Get a chart preview by ID or UUID.` and got:
```
FAILED tests/unit_tests/mcp_service/test_tool_inventory.py::test_tool_inventory_size[get_chart_preview]
AssertionError: ('get_chart_preview', 3301, 3300)
```
Two added bytes in a docstring, in a file that PR never opened. Green CI here will not warn us about it either.
Could the ceiling always add a full increment before the table is regenerated, something like:
```python
byte_budget = (measured_bytes // 100 + 1) * 100
token_budget = (measured_tokens // 25 + 1) * 25
```
That keeps everything you are guarding (the chart tools stay pinned roughly 45 kB under master) without making every description edit a red build.
##########
pyproject.toml:
##########
@@ -327,6 +327,7 @@ development = [
"sqloxide",
"statsd",
"syntaqlite>=0.9.0,<0.10.0",
+ "tiktoken==0.14.0", # reproducible MCP inventory token budgets
Review Comment:
Not a blocker on its own, but I want to flag what this dependency costs at test time. `tiktoken` does not ship the vocabulary in the wheel, it fetches it on first use.
I pointed `TIKTOKEN_CACHE_DIR` at an empty directory and ran `tiktoken.get_encoding("cl100k_base")` on this branch: it wrote a 1,681,126 byte blob pulled from `https://openaipublic.blob.core.windows.net/encodings/cl100k_base.tiktoken`. With the network unreachable the same call raises, so `test_tool_inventory.py` and `test_chart_tool_inventory.py` error at collection time rather than fail with a useful message. That puts an external CDN in the path of `pytest ./tests/unit_tests` for every contributor and every CI run, and `pytest.importorskip("tiktoken")` would not help since the import is the part that succeeds.
The byte budgets already catch the regression this PR is about. I confirmed `test_chart_tool_inventory_size` fails on `f8f293d2` for all three tools on the byte assertion alone, before the token one is reached. So the simplest option is to drop the token assertions and this dependency. If the token numbers are worth keeping, could we gate them on the encoding already being cached so an offline or network restricted run still passes?
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -775,7 +775,7 @@ MCP_TOOL_SEARCH_CONFIG = {
| `max_results` | `5` | Maximum tools returned per search query |
| `always_visible` | See above | Tools that always appear in `list_tools`, regardless of search |
| `include_schemas` | `False` | When `False` (default, "summary mode"), search results omit `inputSchema` entirely and include a lightweight `parameters_hint` listing top-level parameter names. Set to `True` to include the full `inputSchema` in search results. Full schemas are always used when a tool is actually invoked via `call_tool`. |
-| `compact_schemas` | `True` | Strip `$defs` / `$ref` and replace with `{"type": "object"}` in search results to reduce token cost. Only takes effect when `include_schemas=True` — ignored in summary mode. |
+| `compact_schemas` | `True` | Legacy setting selecting the default description limit (300 when `True`, 0 when `False`) if `max_description_length` is omitted. Input schemas preserve `$defs`, `$ref`, nullable unions, and constraints in either mode; titles are omitted. Clients should resolve references within each tool's `inputSchema`. |
Review Comment:
Good catch updating this row. The same text lives in code and still describes the removed behaviour: `superset/mcp_service/mcp_config.py` has a `Schema Compaction:` block starting at line 466 saying search results `strip $defs sections and replace $ref pointers with {"type": "object"}`, a rollback bullet at line 476 offering `compact_schemas=False` to get `full $defs ... in search results`, and the inline comment on line 505 still reads `# Strip $defs/$ref (requires include_schemas=True)`.
That module is what an operator reads when they copy `MCP_TOOL_SEARCH_CONFIG` into `superset_config.py`, so it is worth the same edit you made here.
##########
superset/mcp_service/server.py:
##########
@@ -535,22 +430,20 @@ def _create_search_result_serializer(
~80% vs compact mode while still conveying what parameters a tool
accepts.
- When ``include_schemas`` is True, the full ``compact_schemas``/
- ``max_description_length`` pipeline applies (existing behavior):
-
- * ``$defs`` sections and ``$ref`` pointers are collapsed when
- ``compact_schemas`` is True (see :func:`_compact_schema`).
- * Tool descriptions are truncated to ``max_description_length`` chars.
+ When ``include_schemas`` is True, input schemas retain their definitions,
+ references, and validation constraints. Inlining references duplicates shared
+ chart models and can make a single tool exceed client result limits.
- Full schemas remain available when the tool is invoked via ``call_tool``.
+ Titles and output schemas are stripped by the base serializer. The legacy
+ ``compact_schemas`` setting only selects the default description limit;
+ ``max_description_length`` explicitly controls description truncation.
"""
include_schemas = config.get("include_schemas", False)
if not include_schemas:
max_desc = config.get("max_description_length", 300)
return _build_summary_serializer(max_desc)
- # include_schemas=True: apply full compact_schemas/max_description_length pipeline
compact = config.get("compact_schemas", True)
Review Comment:
Just a small NIT: with the compaction gone, `compact` only picks the default for `max_desc`, and both return paths are now the same function. `compact_schemas=True` with `max_description_length=0` falls through to `_serializer`, which calls `_serialize_tools_without_output_schema` and then does nothing to the results.
The whole block reads the same as:
```python
compact = config.get("compact_schemas", True)
max_desc = config.get("max_description_length", 300 if compact else 0)
if not max_desc:
return _serialize_tools_without_output_schema
```
Fewer moving parts around a setting we are now calling legacy. Non blocking either way.
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -775,7 +775,7 @@ MCP_TOOL_SEARCH_CONFIG = {
| `max_results` | `5` | Maximum tools returned per search query |
| `always_visible` | See above | Tools that always appear in `list_tools`, regardless of search |
| `include_schemas` | `False` | When `False` (default, "summary mode"), search results omit `inputSchema` entirely and include a lightweight `parameters_hint` listing top-level parameter names. Set to `True` to include the full `inputSchema` in search results. Full schemas are always used when a tool is actually invoked via `call_tool`. |
Review Comment:
Just a small NIT, and it predates this PR, but you are already in this table. This row says `include_schemas` defaults to `False`, while `MCP_TOOL_SEARCH_CONFIG` in `superset/mcp_service/mcp_config.py:507` ships `"include_schemas": True`. Only the `config.get()` fallback inside `_create_search_result_serializer` is `False`, and nothing reaches it with the shipped config.
So an operator reading this row concludes summary mode is the shipped default when full schemas actually are, which is exactly the mode your change affects. Cheap to correct in the same pass.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5793739486
Measured, reproducible, and roughly a 49% cut on all three tools — and the handoff note on line ranges is genuinely useful for the two follow-ups that touch this file next. Two things before this is ready.
**1. Removing `_compact_schema` changes every tool, not just the three in the ticket.** The block at original 302–406 is gone and the serializer no longer compacts, so the schema shipped for all ~63–68 tools in the inventory is now whatever the raw model emits. For the three chart tools that is a large win because they were paying for recursive inlining. For a tool whose schema has few or no repeated sub-models, compaction may have been doing nothing — but for anything in between, this could make it *bigger*, and the new `test_chart_tool_inventory.py` only covers 9 chart cases.
Please measure the full inventory before and after, not just the three. If some non-chart tool regresses, that is worth knowing now rather than from a client. The ticket asked for per-tool inventory budgets; the inventory is more than the chart tools.
**2. Three CI failures need attribution.** `babel-extract` is the fleet-wide one (SC-121633, fixed upstream by #44509) — ignore it. But `test-load-examples`, `testcontainers (starrocks, 15)` and `playwright-tests (chromium)` are failing here and not on the adjacent PRs I checked. They look unrelated to an MCP schema change, and the dev-extra addition of `tiktoken`/`regex` is the one plausible connection worth ruling out. Either attribute them to something upstream with evidence, or fix them.
Nothing else from me. `tiktoken` sitting under `development` rather than as a runtime dependency is the right call, and keeping the 9 regression cases in a new file instead of the shared one was thoughtful.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): preserve deduplicated chart tool input schemas [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44575:
URL: https://github.com/apache/superset/pull/44575#issuecomment-5830033974
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org