villebro commented on PR #44508: URL: https://github.com/apache/superset/pull/44508#issuecomment-5778973401
@aminghadersohi — the 50,000-byte calibration is right, and confirmed it holds up measuring directly against this repo's actual response shapes, not just the fixture numbers in your review. Your commit (d784db26e7) applying it is exactly what was needed. Two things it didn't quite finish, both pushed in b950a5fbe5: - A couple of stale "~100k by default" mentions the search-and-replace missed: the actual `ValueError` raised by `validate_embedded_dimensions_page_size`, and `string_clip_chars`'s docstring. - `EMBEDDED_DIMENSIONS_MAX_PAGE_SIZE` was left at 8, which was only ever safe under the *old* 100k default. Measured against this repo's real `MetricInfo`/`DimensionInfo` schemas: page_size=8 serializes to 54,558–77,014 bytes depending on description length, both already over the new 50,000 default. Since `list_metrics` isn't in the response-size guard's truncation-eligible tool categories, that's a guaranteed `ToolError` rather than a graceful degrade. Lowered it to 4, the largest value with real margin in both cases. On the "retaining estimation as an image-build follow-up" note — worth flagging for context: even with `TIKTOKEN_CACHE_DIR` fully pre-populated, `cl100k_base` only approximates OpenAI's own tokenizer. Anthropic doesn't publish a downloadable vocabulary tiktoken (or anything else) can load — Claude's actual token count is only available via their network `count_tokens` API, which is too slow to call per-response here. So for Claude specifically, which is the primary client this tool surface is built for, tiktoken was always approximating a different vendor's tokenizer regardless of caching. Appreciate you flagging it for completeness — just don't think it changes the calculus on removing it. -- 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]
