villebro opened a new pull request, #44508:
URL: https://github.com/apache/superset/pull/44508

   ### SUMMARY
   The MCP service loaded tiktoken's `cl100k_base` vocabulary at module import 
time to estimate LLM token counts for the response-size guard. `tiktoken` 
doesn't ship this vocabulary as package data — `get_encoding()` downloads it 
from `openaipublic.blob.core.windows.net` on a cold cache. This made an 
unannounced third-party network call on every MCP cold start, and on 
egress-restricted hosts (air-gapped, private-network, `--network none`) it 
crashed the service outright, since the surrounding `except` clauses didn't 
catch the network errors `get_encoding()` can raise.
   
   Rather than lazily loading tiktoken behind a flag, this removes token 
estimation entirely. An MCP server has no way to know which client or tokenizer 
(Claude, GPT, Gemini, a local model) is actually consuming a given response, so 
`cl100k_base` (OpenAI's GPT-3.5/4 encoding) was never a reliable proxy for the 
client's real token budget. The response-size guard now measures the exact 
serialized UTF-8 byte length instead, which is deterministic and 
tokenizer-agnostic, and removes the `tiktoken` dependency — and this whole 
class of bug — entirely.
   
   `MCP_RESPONSE_SIZE_CONFIG['token_limit']` is renamed to `['max_bytes']`, 
default changed from 25,000 (estimated tokens) to 100,000 (bytes). This isn't 
an exact behavioral no-op: bytes-per-token ratios vary by content, and the 
module's own prior fallback heuristic (`CHARS_PER_TOKEN = 3.0`, calibrated 
specifically for JSON-heavy payloads) implies roughly 75,000 bytes would track 
the old limit most closely. 100,000 was chosen deliberately as a rounder, 
slightly more permissive default rather than an exact conversion — operators 
who want closer parity with the old effective limit should set `max_bytes` 
accordingly.
   
   ### TESTING INSTRUCTIONS
   - `pytest tests/unit_tests/mcp_service/utils/test_response_size_utils.py 
tests/unit_tests/mcp_service/test_middleware.py 
tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py`
   - `grep -rn "tiktoken" superset/ tests/ pyproject.toml requirements/` 
returns nothing
   - `docker run --rm --network none apache/superset:<local-build> superset mcp 
run` no longer requires network access to start, since `tiktoken` is no longer 
imported anywhere
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [x] Removes existing feature or API (tiktoken-based token estimation; 
`MCP_RESPONSE_SIZE_CONFIG['token_limit']` key, renamed to `max_bytes` — 
documented in UPDATING.md)


-- 
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