Commit graph

1 commit

Author SHA1 Message Date
Tejas Chopra
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.
2026-08-04 11:31:16 -07:00