From fc4680b37af1d522fdbeba8e5d3228769dc49ba4 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Tue, 4 Aug 2026 11:31:16 -0700 Subject: [PATCH] fix(tokenizers): resolve gpt-5 and mixed-case model names to the right encoding (#2776) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 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. --- headroom/tokenizers/tiktoken_counter.py | 11 +++ tests/test_tokenizer_encoding_resolution.py | 84 +++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/test_tokenizer_encoding_resolution.py diff --git a/headroom/tokenizers/tiktoken_counter.py b/headroom/tokenizers/tiktoken_counter.py index 5c05f308d..87a4ef0fa 100644 --- a/headroom/tokenizers/tiktoken_counter.py +++ b/headroom/tokenizers/tiktoken_counter.py @@ -164,6 +164,14 @@ def get_encoding_for_model(model: str) -> str: Returns: Encoding name (e.g., 'o200k_base', 'cl100k_base'). """ + # Case-insensitive: TokenizerRegistry lowercases only its *cache key*, then + # constructs the counter from the caller's original string. An uppercase + # deployment name ("GPT-4o", routine on Azure) therefore arrived here + # verbatim, matched no prefix, and silently took DEFAULT_ENCODING -- so the + # encoding a model got depended on the casing of whichever request warmed + # the cache first, and flipped across restarts. + model = model.lower() + # Direct lookup if model in MODEL_TO_ENCODING: return MODEL_TO_ENCODING[model] @@ -180,6 +188,9 @@ def get_encoding_for_model(model: str) -> str: # which they would otherwise match and be mis-encoded as cl100k_base. ("gpt-4.1", "o200k_base"), ("gpt-4.5", "o200k_base"), + # gpt-5 uses o200k_base. Without this it fell through to the + # cl100k_base default, over-counting CJK by ~33%. + ("gpt-5", "o200k_base"), ("gpt-4-turbo", "cl100k_base"), ("gpt-4", "cl100k_base"), ("gpt-3.5", "cl100k_base"), diff --git a/tests/test_tokenizer_encoding_resolution.py b/tests/test_tokenizer_encoding_resolution.py new file mode 100644 index 000000000..3dd1b6be8 --- /dev/null +++ b/tests/test_tokenizer_encoding_resolution.py @@ -0,0 +1,84 @@ +"""``get_encoding_for_model`` must not depend on casing, and must know gpt-5. + +Two defects, both reachable through the normal ``get_tokenizer()`` path: + +1. **gpt-5 had no prefix entry**, so it fell through to ``DEFAULT_ENCODING`` + (``cl100k_base``) instead of ``o200k_base``. On CJK text cl100k emits ~33% + more tokens than o200k, so every gpt-5 count was inflated. + +2. **Resolution was case-sensitive.** ``TokenizerRegistry.get`` lowercases only + its *cache key*, then builds the counter from the caller's original string + (``_create_tokenizer(model, ...)``). An uppercase deployment name -- routine + on Azure, where the deployment name is user-chosen -- reached the resolver + verbatim, matched nothing, and took the default encoding. + + The cache made (2) genuinely nasty: because the key is lowercased but + construction is not, the encoding a model ends up with depended on the + casing of whichever request warmed the cache first, and could differ across + restarts. The tests below call ``clear_cache()`` so the uppercase spelling is + resolved cold, which is the failing order. +""" + +from __future__ import annotations + +import pytest + +from headroom.tokenizers import get_tokenizer +from headroom.tokenizers.registry import TokenizerRegistry +from headroom.tokenizers.tiktoken_counter import get_encoding_for_model + +CJK = "这是一个测试文档，用于验证分词器的差异。" * 30 + + +@pytest.mark.parametrize( + ("model", "expected"), + [ + # gpt-5 family: the missing entry. + ("gpt-5", "o200k_base"), + ("gpt-5-mini", "o200k_base"), + ("gpt-5-nano", "o200k_base"), + ("gpt-5-2025-08-07", "o200k_base"), + # Casing must not change the answer. + ("GPT-4o", "o200k_base"), + ("GPT-4.1", "o200k_base"), + ("Gpt-4O-Mini", "o200k_base"), + ("GPT-5", "o200k_base"), + ("O4-Mini", "o200k_base"), + ("GPT-4", "cl100k_base"), + ("GPT-4-Turbo", "cl100k_base"), + # Must not regress. + ("gpt-4o", "o200k_base"), + ("gpt-4.1", "o200k_base"), + ("gpt-4", "cl100k_base"), + ("gpt-4-turbo", "cl100k_base"), + ("gpt-3.5-turbo", "cl100k_base"), + ("o4-mini", "o200k_base"), + ], +) +def test_encoding_resolution(model: str, expected: str) -> None: + assert get_encoding_for_model(model) == expected + + +@pytest.mark.parametrize("model", ["gpt-5", "GPT-4o", "GPT-4.1"]) +def test_cold_cache_uppercase_still_gets_the_right_encoding(model: str) -> None: + """End-to-end through the registry, with the uppercase spelling resolved first. + + Without clear_cache() a preceding lowercase lookup would populate the shared + (lowercased) cache key and mask the defect entirely. + """ + tiktoken = pytest.importorskip("tiktoken") + o200k = len(tiktoken.get_encoding("o200k_base").encode(CJK)) + + TokenizerRegistry.clear_cache() + assert get_tokenizer(model).count_text(CJK) == o200k + + +def test_casing_is_not_load_order_dependent() -> None: + """The same model must count identically whichever spelling arrives first.""" + TokenizerRegistry.clear_cache() + upper_first = get_tokenizer("GPT-4o").count_text(CJK) + + TokenizerRegistry.clear_cache() + lower_first = get_tokenizer("gpt-4o").count_text(CJK) + + assert upper_first == lower_first