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: [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]
