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

Reply via email to