mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
1 commit
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
fc4680b37a
|
fix(tokenizers): resolve gpt-5 and mixed-case model names to the right encoding (#2776)
## Description Two defects in `get_encoding_for_model`, both reachable through the normal `get_tokenizer()` path. **1. `gpt-5` had no prefix entry.** It fell through to `DEFAULT_ENCODING` (`cl100k_base`) instead of `o200k_base`. Same class as the `o4` gap already patched in that tuple. cl100k emits ~33% more tokens than o200k on CJK, so every gpt-5 count was inflated there: ```text CJK sample (30x repeated sentence) o200k_base (correct) 450 tokens cl100k_base (actual) 600 tokens +33.3% ``` Note #2758 taught the *registry* that `gpt-5` → the tiktoken backend; this is the next hop, where that backend picks its *encoding*. So gpt-5 got the right tokenizer family and the wrong encoding inside it. **2. Resolution was case-sensitive.** `TokenizerRegistry.get` lowercases only its **cache key**, then constructs the counter from the caller's original string (`_create_tokenizer(model, backend)`). An uppercase deployment name — routine on Azure, where the deployment name is user-chosen — arrived verbatim, matched no prefix, and took the default encoding. The cache makes this one genuinely unpleasant: key lowercased, construction not, so **the encoding a model receives depends on the casing of whichever request warmed the cache first**, and can differ across restarts. ```text cold cache, uppercase resolved first: GPT-4o -> cl100k_base CJK=600 WRONG GPT-4.1 -> cl100k_base CJK=600 WRONG Gpt-4O-Mini -> cl100k_base CJK=600 WRONG gpt-4o -> o200k_base CJK=450 ok ``` I nearly filed this as "not reachable" — my first check ran the lowercase spelling first, which populated the shared lowercased cache key and masked it completely. The tests call `clear_cache()` so the uppercase spelling resolves cold, which is the failing order. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Added `("gpt-5", "o200k_base")` to the ordered prefix tuple. - `get_encoding_for_model` now lowercases its input. The lowercasing is deliberately scoped to this function rather than the registry: every `MODEL_TO_ENCODING` key is already lowercase (asserted), so it is safe here — whereas lowercasing in `TokenizerRegistry` would break HuggingFace repo ids, which *are* case-sensitive (`Qwen/Qwen3-Coder`). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check` + `ruff format`) - [x] Type checking passes (`mypy`) - [x] New tests added for new functionality ### Test Output ```text $ pytest tests/test_tokenizer_encoding_resolution.py -q 21 passed $ git stash push headroom/ && pytest tests/test_tokenizer_encoding_resolution.py -q # gpt-5 family, every uppercase case, and both cache-order tests fail ``` Targeted run across the tokenizer/pricing/provider suites, including `test_evals_cjk_tokenization.py` since CJK is the affected content type: ```text $ pytest tests/test_utils.py tests/test_reporting.py tests/test_cost_pricing_warning_dedup.py \ tests/test_pricing.py tests/test_pricing_litellm.py tests/test_provider_model_fallback.py \ tests/test_models.py tests/test_savings_ledger.py tests/test_tokenizers.py \ tests/test_tokenizer.py tests/test_tokenizer_selection_coverage.py \ tests/test_provider_tokenizer_one_ruler.py tests/test_openai_model_table_resolution.py \ tests/test_evals_cjk_tokenization.py -q 233 passed, 16 skipped ``` ```text $ ruff check <changed files> All checks passed! $ mypy headroom/tokenizers/tiktoken_counter.py # only pre-existing release_version.py tomllib redef, present on main ``` Deferring the full suite to CI — this environment has no maturin/Rust core, so the native-dependent shards can't run locally. ## Real Behavior Proof - **Environment:** macOS, Python 3.13.7, isolated worktree at `upstream/main` (`0cb72f45`). - **Exact command / steps:** `TokenizerRegistry.clear_cache()`, then resolve each spelling cold and count a CJK sample. - **Observed result:** ```text before after gpt-5 cl100k_base CJK=600 o200k_base CJK=450 gpt-5-mini cl100k_base CJK=600 o200k_base CJK=450 GPT-4o cl100k_base CJK=600 o200k_base CJK=450 GPT-4.1 cl100k_base CJK=600 o200k_base CJK=450 Gpt-4O-Mini cl100k_base CJK=600 o200k_base CJK=450 GPT-4 cl100k_base CJK=600 cl100k_base CJK=600 (unchanged, correct) gpt-4o o200k_base CJK=450 o200k_base CJK=450 (unchanged) gpt-3.5-turbo cl100k_base CJK=600 cl100k_base CJK=600 (unchanged) ``` `GPT-4` was previously "correct" only by accident — it missed every prefix and landed on `DEFAULT_ENCODING`, which happens to be `cl100k_base`. It is now correct by resolution. |