mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(tokenizers): use o200k_base for gpt-4.1/gpt-4.5/o4 families (#2108)
## Description
`get_encoding_for_model` returns the wrong tiktoken encoding for the
current OpenAI flagship families, so their token counts are computed
with the wrong vocabulary.
The prefix table is ordered most-specific-first, but it has no entry for
the `gpt-4.1` / `gpt-4.5` / `o4` families:
```python
for prefix, encoding in (
("gpt-4o", "o200k_base"),
("gpt-4-turbo", "cl100k_base"),
("gpt-4", "cl100k_base"),
("gpt-3.5", "cl100k_base"),
("o1", "o200k_base"),
("o3", "o200k_base"),
):
if model.startswith(prefix):
return encoding
return DEFAULT_ENCODING # cl100k_base
```
- `gpt-4.1`, `gpt-4.1-mini`, `gpt-4.5-*` all start with `gpt-4`, so they
match the `gpt-4` prefix and get `cl100k_base`.
- `o4-mini` matches no prefix and falls through to the `cl100k_base`
default.
All three families use `o200k_base`. Since `count_text`/`count_messages`
tokenize with the resolved encoding, every token count for those models
is computed against the wrong BPE vocabulary, which skews budget gating
and the compress/skip decision for a large slice of current OpenAI
traffic.
## Fix
Add explicit `gpt-4.1` and `gpt-4.5` prefixes (ordered ahead of `gpt-4`,
which they would otherwise match) and an `o4` prefix, all mapping to
`o200k_base`. Plain `gpt-4` and `gpt-3.5` snapshots still resolve to
`cl100k_base`, and `gpt-4o` still wins for the 4o family.
Closes #
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/tokenizers/tiktoken_counter.py`: add `gpt-4.1`/`gpt-4.5`
prefixes ahead of `gpt-4`, and an `o4` prefix, all mapping to
`o200k_base`.
- `tests/test_tokenizers.py`: add
`test_gpt41_and_o4_families_use_o200k`.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
All checks passed!
$ python -m py_compile headroom/tokenizers/tiktoken_counter.py tests/test_tokenizers.py
OK
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the resolution with a
dependency-free script that runs the old and new prefix tables, and left
the full pytest to CI.
- Exact command / steps: resolved `gpt-4.1`, `gpt-4.1-mini`,
`gpt-4.5-preview`, and `o4-mini` under the old table and the new table,
plus `gpt-4o-*`, `gpt-4-2025-*`, `gpt-4-turbo-*`, `gpt-3.5-turbo`, and
`o1-mini` as regression guards.
- Observed result: old table returns `cl100k_base` for all four (wrong);
new table returns `o200k_base`; the guard models are unchanged
(`gpt-4o-*` and `o1-*` stay `o200k_base`,
`gpt-4*`/`gpt-4-turbo*`/`gpt-3.5*` stay `cl100k_base`). The new test
asserts the four families resolve to `o200k_base` and a plain `gpt-4`
snapshot stays `cl100k_base`.
- Not tested: loading the actual tiktoken vocabularies to count tokens
end to end; full local `pytest` deferred to CI (OOM, per above).
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change adds three ordered prefix entries to a pure
function, verified by the standalone proof and the new regression test
for CI. I intentionally left `gpt-5` out since I didn't want to assert
an encoding I couldn't confirm here; happy to add it in a follow-up if
you can confirm the intended mapping.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
parent
eecb81e847
commit
6979b5245e
3 changed files with 24 additions and 0 deletions
|
|
@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
### Bug Fixes
|
||||
|
||||
* **tokenizers:** use `o200k_base` for the gpt-4.1 / gpt-4.5 / o4 families in `get_encoding_for_model`. `gpt-4.1*` and `gpt-4.5*` matched the broad `gpt-4` prefix and were encoded with `cl100k_base`, and `o4*` matched no prefix and fell through to the `cl100k_base` default — all three use `o200k_base`, so their token counts were computed with the wrong vocabulary. Added explicit `gpt-4.1`/`gpt-4.5` prefixes ahead of `gpt-4` and an `o4` prefix; `gpt-4` and `gpt-3.5` snapshots still resolve to `cl100k_base`.
|
||||
* **cache/ccr:** stop counting a successful eviction as a retrieval in the compression feedback learner, which inverted the learning signal. When an entry is evicted without ever being retrieved, `CompressionStore` emits a synthetic `retrieval_type="eviction_success"` event to mark that the compression was sufficient (the LLM never needed the original). `CompressionFeedback.record_retrieval` had no branch for it, so — because the type is not `"full"` — it was counted as a *search retrieval*, inflating the tool's `retrieval_rate`/`search_rate`. `get_compression_hints` reads a high retrieval rate as "compressing too aggressively" and backs off, so a compression that actually worked pushed the learner toward *less* compression (a standalone repro scores one successful eviction as a 100% retrieval rate). The event is now recognized and left out of the retrieval counters; the compression is still counted by `record_compression` at store time, so a never-retrieved entry correctly yields a low retrieval rate. Genuine retrievals are unaffected.
|
||||
* **proxy/anthropic:** don't launder a non-2xx upstream into HTTP 200 when enterprise security scans the response. On the non-streaming `/v1/messages` path the response-scan branch rebuilt the reply as `httpx.Response(status_code=200)` and returned it without checking the upstream status, so a rate-limit (429), overloaded (529), or other 4xx error whose JSON body was scanned reached the client as an HTTP 200 — the client's retry/backoff never fired and an error looked like success. The branch is now gated on a 200 upstream, matching the sibling CCR/cache/buffered-stream blocks in the same handler; non-2xx responses fall through and keep their real status.
|
||||
* **learn:** classify timeout and connection tool failures correctly instead of as generic runtime errors. In `classify_error` the generic `RUNTIME_ERROR` pattern (`Traceback|Exception:|Error:`) was checked before the dedicated `TIMEOUT` and `CONNECTION_ERROR` patterns. Because every Python exception repr is `XxxError: ...`, a `TimeoutError: ...` or `ConnectionError: ...` matched the catch-all first and was miscategorized as `RUNTIME_ERROR`, leaving those two categories unreachable for the common colon-repr form (they only fired for tokenless phrasings like `deadline exceeded`). The `TIMEOUT` and `CONNECTION_ERROR` patterns are now checked before the generic catch-all; tokenless generic errors still classify as `RUNTIME_ERROR`.
|
||||
|
|
|
|||
|
|
@ -176,11 +176,18 @@ def get_encoding_for_model(model: str) -> str:
|
|||
# o200k_base instead of cl100k_base for unknown gpt-4 snapshots.
|
||||
for prefix, encoding in (
|
||||
("gpt-4o", "o200k_base"),
|
||||
# gpt-4.1 / gpt-4.5 use o200k_base and MUST precede the "gpt-4" prefix,
|
||||
# which they would otherwise match and be mis-encoded as cl100k_base.
|
||||
("gpt-4.1", "o200k_base"),
|
||||
("gpt-4.5", "o200k_base"),
|
||||
("gpt-4-turbo", "cl100k_base"),
|
||||
("gpt-4", "cl100k_base"),
|
||||
("gpt-3.5", "cl100k_base"),
|
||||
("o1", "o200k_base"),
|
||||
("o3", "o200k_base"),
|
||||
# o4 reasoning models use o200k_base; without this they fell through to
|
||||
# the cl100k_base default.
|
||||
("o4", "o200k_base"),
|
||||
):
|
||||
if model.startswith(prefix):
|
||||
return encoding
|
||||
|
|
|
|||
|
|
@ -51,6 +51,22 @@ class TestTiktokenCounter:
|
|||
# gpt-4-turbo snapshots use cl100k_base.
|
||||
assert get_encoding_for_model("gpt-4-turbo-2099") == "cl100k_base"
|
||||
|
||||
def test_gpt41_and_o4_families_use_o200k(self):
|
||||
"""gpt-4.1 / gpt-4.5 / o4 use o200k_base, not cl100k_base.
|
||||
|
||||
Regression: gpt-4.1* and gpt-4.5* matched the broad "gpt-4" prefix and
|
||||
resolved to cl100k_base, and o4* matched no prefix and fell to the
|
||||
cl100k_base default — both wrong encodings for those models.
|
||||
"""
|
||||
from headroom.tokenizers.tiktoken_counter import get_encoding_for_model
|
||||
|
||||
assert get_encoding_for_model("gpt-4.1") == "o200k_base"
|
||||
assert get_encoding_for_model("gpt-4.1-mini") == "o200k_base"
|
||||
assert get_encoding_for_model("gpt-4.5-preview") == "o200k_base"
|
||||
assert get_encoding_for_model("o4-mini") == "o200k_base"
|
||||
# A plain gpt-4 snapshot must still use cl100k_base (not shadowed).
|
||||
assert get_encoding_for_model("gpt-4-0613") == "cl100k_base"
|
||||
|
||||
def test_count_text_empty(self):
|
||||
"""Test counting empty text."""
|
||||
counter = TiktokenCounter()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue