mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(providers): stop a shorter model family shadowing a longer one (#2762)
## Description > **Stacked on #2761** — that PR splits `_lookup_encoding_name` out of `_get_encoding_name_for_model`, which this one builds on. Please merge #2761 first; the diff here will shrink to just this commit afterwards. `_MODEL_ENCODINGS` and `_CONTEXT_LIMITS` are matched by prefix, iterating in **plain dict order** — so the first *inserted* prefix wins rather than the most specific one. `gpt-4.1` matched the `gpt-4` entry: | model | resolved | actual | | |---|---|---|---| | `gpt-4.1` | 8192 | 1,047,576 | **128× under** | | `gpt-4.1-mini` | 8192 | 1,047,576 | **128× under** | | `gpt-4.1-nano` | 8192 | 1,047,576 | **128× under** | | `gpt-4-32k-0613` | 8192 | 32,768 | 4× under | | `gpt-5` / `-mini` / `-nano` | 128,000 | 400,000 | fell to unknown-model default | | `o4-mini` | 128,000 | 200,000 | fell to unknown-model default | A 128× under-estimate matters because the context limit is what tells the proxy how much headroom is left: it treats a 1M-context model as nearly full and compresses accordingly. The same shadowing picked the **wrong encoding** — `gpt-4.1` got `cl100k_base` instead of `o200k_base`. Measured cost of that: ```text cl100k o200k error python code 420 420 +0.0% json blob 555 555 +0.0% logs 580 580 +0.0% english 201 201 +0.0% CJK 600 450 +33.3% ``` So the encoding half is narrow but real — it only bites CJK content, which the repo already treats as a case worth testing (`tests/test_evals_cjk_tokenization.py`). **Scope honestly:** `get_context_limit` consults LiteLLM *before* this table, so the limit half only surfaces where LiteLLM is absent or does not know the model. That is not hypothetical — the `litellm` dependency carries a `python_version < '3.14'` marker (`pyproject.toml:56`), so **any install on Python 3.14+ has no LiteLLM** and this table is load-bearing. The encoding half never had a LiteLLM fallback and was always wrong. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - Both prefix loops now iterate `sorted(..., key=len, reverse=True)` — longest prefix wins. This is the root-cause fix: it also protects the *next* model added to these tables. - Added the missing families: `gpt-4.1` (+`-mini`/`-nano`), `gpt-5` (+`-mini`/`-nano`), `o4-mini` to both tables. - Left `supports_model`'s prefix loop alone — it only returns a bool, so order cannot change its answer. Not touched: `_PRICING`. `gpt-4.1`/`gpt-5` also fall through to the GPT-4o pricing tier, which skews cost reporting, but that is a separate concern with its own verification burden (published rates, staleness window) and does not belong in a tokenizer-correctness fix. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check` + `ruff format`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality ### Test Output 12 of 24 new cases fail without the fix; the 12 that pass are the "must not regress" rows (`gpt-4`, `gpt-4-turbo`, `gpt-4o`, `o3`, `gpt-3.5-turbo`) — included precisely so the longest-prefix change can't quietly move them: ```text $ git stash push headroom/providers/openai.py && pytest tests/test_openai_model_table_resolution.py -q FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-4.1-mini-1047576] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-4.1-nano-1047576] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-4.1-2025-04-14-1047576] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-4-32k-0613-32768] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-5-400000] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[gpt-5-mini-400000] FAILED ...::test_context_limit_prefers_the_most_specific_prefix[o4-mini-200000] FAILED ...::test_encoding_prefers_the_most_specific_prefix[gpt-4.1-o200k_base] FAILED ...::test_encoding_prefers_the_most_specific_prefix[gpt-4.1-mini-o200k_base] FAILED ...::test_encoding_prefers_the_most_specific_prefix[gpt-4.1-2025-04-14-o200k_base] FAILED ...::test_cjk_is_not_over_counted_for_gpt_41 12 failed, 12 passed in 0.70s $ git stash pop && pytest tests/test_openai_model_table_resolution.py -q 24 passed in 0.43s ``` Regression check — 119 suites touching openai / cost / savings / token / compress / outcome / budget, this branch vs clean `main` in the same environment, comparing failure *sets*: ```text branch : 5 failed, 1486 passed, 86 skipped in 111.01s main : 5 failed, 1453 passed, 86 skipped in 132.07s NEW failures introduced: (none) pre-existing on both: test_bundled_tools_savings.py::test_compressed_payload_preserves_answer_anthropic test_compress_route_tokenizer_by_model.py::...[deepseek/deepseek-v4] (needs `transformers`) test_compress_route_tokenizer_by_model.py::...[deepseek/deepseek-v4] (needs `transformers`) test_image_compressor_singleton_reuse.py::test_onnx_router_is_built_once_and_cached test_openai_streaming_backend.py::...test_litellm_vertex_streaming_preserves_max_tokens_and_vendor_fields ``` ```text $ ruff check headroom/providers/openai.py tests/test_openai_model_table_resolution.py All checks passed! $ mypy headroom/providers/openai.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - **Environment:** macOS, Python 3.13.7, isolated worktree at `upstream/main` (`ad56dd38`), no `litellm` installed (matching a Python 3.14+ install, where the dep marker excludes it). - **Exact command / steps:** resolve context limit + encoding for each model against published OpenAI values, before and after. - **Observed result:** ```text before after model limit enc model limit enc gpt-4.1 8192 cl100k gpt-4.1 1047576 o200k_base gpt-4.1-mini 8192 cl100k gpt-4.1-mini 1047576 o200k_base gpt-4.1-nano 8192 cl100k gpt-4.1-nano 1047576 o200k_base gpt-4.1-2025-04-14 8192 cl100k gpt-4.1-2025-04-14 1047576 o200k_base gpt-4-32k-0613 8192 cl100k gpt-4-32k-0613 32768 cl100k_base gpt-5 128000 o200k gpt-5 400000 o200k_base o4-mini 128000 o200k o4-mini 200000 o200k_base unchanged: gpt-4=8192/cl100k, gpt-4-turbo=128000/cl100k, gpt-4o=128000/o200k, o3=200000, gpt-3.5-turbo=16385/cl100k ```
This commit is contained in:
Tejas Chopra
GitHub
parent
cd92ed52ff
commit
0cb72f45b2
3 changed files with 127 additions and 8 deletions
|
|
@ -64,6 +64,10 @@ _MODEL_ENCODINGS: dict[str, str] = {
|
|||
"o1-mini": "o200k_base",
|
||||
"o3": "o200k_base",
|
||||
"o3-mini": "o200k_base",
|
||||
"o4": "o200k_base",
|
||||
"o4-mini": "o200k_base",
|
||||
"gpt-4.1": "o200k_base",
|
||||
"gpt-5": "o200k_base",
|
||||
# GPT-4 and GPT-3.5 use cl100k_base
|
||||
"gpt-4": "cl100k_base",
|
||||
"gpt-4-turbo": "cl100k_base",
|
||||
|
|
@ -78,6 +82,18 @@ _CONTEXT_LIMITS: dict[str, int] = {
|
|||
"gpt-4o-2024-11-20": 128000,
|
||||
"gpt-4o-2024-08-06": 128000,
|
||||
"gpt-4o-2024-05-13": 128000,
|
||||
# GPT-4.1 series (~1M input). LiteLLM is still consulted first in
|
||||
# get_context_limit; these are the manual fallback for installs without it
|
||||
# (the litellm dep is gated python_version < '3.14').
|
||||
"gpt-4.1": 1_047_576,
|
||||
"gpt-4.1-mini": 1_047_576,
|
||||
"gpt-4.1-nano": 1_047_576,
|
||||
# GPT-5 series. This table is an INPUT budget (get_context_limit returns
|
||||
# litellm's max_input_tokens when available), so these are 272K input --
|
||||
# not the 400K total window, which is 272K in + 128K out.
|
||||
"gpt-5": 272000,
|
||||
"gpt-5-mini": 272000,
|
||||
"gpt-5-nano": 272000,
|
||||
# GPT-4 Turbo
|
||||
"gpt-4-turbo": 128000,
|
||||
"gpt-4-turbo-preview": 128000,
|
||||
|
|
@ -94,6 +110,7 @@ _CONTEXT_LIMITS: dict[str, int] = {
|
|||
"o1-mini": 128000,
|
||||
"o3": 200000,
|
||||
"o3-mini": 200000,
|
||||
"o4-mini": 200000,
|
||||
# DeepSeek (often accessed via OpenAI-compatible API). Values verified
|
||||
# against api-docs.deepseek.com (V4) and LiteLLM model_cost (deprecated
|
||||
# aliases). LiteLLM lookup is still attempted first in get_context_limit;
|
||||
|
|
@ -275,10 +292,12 @@ def _lookup_encoding_name(model: str, custom_encodings: dict[str, str] | None =
|
|||
if model in _MODEL_ENCODINGS:
|
||||
return _MODEL_ENCODINGS[model]
|
||||
|
||||
# Prefix match for versioned models
|
||||
for prefix, encoding in _MODEL_ENCODINGS.items():
|
||||
# Prefix match for versioned models, longest prefix first. Plain dict order
|
||||
# let a shorter family shadow a longer one: "gpt-4.1" hit the "gpt-4" entry
|
||||
# and got cl100k_base instead of o200k_base, which over-counts CJK by ~33%.
|
||||
for prefix in sorted(_MODEL_ENCODINGS, key=len, reverse=True):
|
||||
if model.startswith(prefix):
|
||||
return encoding
|
||||
return _MODEL_ENCODINGS[prefix]
|
||||
|
||||
# Pattern-based inference
|
||||
family = _infer_model_family(model)
|
||||
|
|
@ -525,10 +544,12 @@ class OpenAIProvider(Provider):
|
|||
if model in self._context_limits:
|
||||
return self._context_limits[model]
|
||||
|
||||
# Prefix match
|
||||
for prefix, limit in self._context_limits.items():
|
||||
# Prefix match, longest prefix first. Plain dict order let a shorter
|
||||
# family shadow a longer one: "gpt-4.1" hit the "gpt-4" entry and got
|
||||
# 8192 instead of ~1M, and "gpt-4-32k-0613" got 8192 instead of 32768.
|
||||
for prefix in sorted(self._context_limits, key=len, reverse=True):
|
||||
if model.startswith(prefix):
|
||||
return limit
|
||||
return self._context_limits[prefix]
|
||||
|
||||
# Pattern-based inference
|
||||
family = _infer_model_family(model)
|
||||
|
|
|
|||
84
tests/test_openai_model_table_resolution.py
Normal file
84
tests/test_openai_model_table_resolution.py
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
"""A shorter model family must not shadow a longer one.
|
||||
|
||||
``_MODEL_ENCODINGS`` and ``_CONTEXT_LIMITS`` are matched by prefix. Iterating
|
||||
them in plain dict order meant the first *inserted* prefix won, not the most
|
||||
specific one, so ``gpt-4.1`` matched the ``gpt-4`` entry:
|
||||
|
||||
* context limit 8192 instead of ~1M -- a 128x under-estimate, which makes the
|
||||
proxy think a 1M-context model is nearly full and compress accordingly;
|
||||
* encoding ``cl100k_base`` instead of ``o200k_base``, which over-counts CJK
|
||||
text by ~33%.
|
||||
|
||||
``gpt-4-32k-0613`` had the same problem (8192 instead of 32768).
|
||||
|
||||
``get_context_limit`` consults LiteLLM before this table, so the limit half only
|
||||
surfaces where LiteLLM is missing or does not know the model -- notably any
|
||||
install on Python >= 3.14, where the ``litellm`` dependency is excluded by its
|
||||
``python_version < '3.14'`` marker. The encoding half has no such fallback and
|
||||
was always wrong.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from headroom.providers.openai import (
|
||||
OpenAIProvider,
|
||||
_get_encoding_name_for_model,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("model", "expected"),
|
||||
[
|
||||
# The shadowing cases.
|
||||
("gpt-4.1", 1_047_576),
|
||||
("gpt-4.1-mini", 1_047_576),
|
||||
("gpt-4.1-nano", 1_047_576),
|
||||
("gpt-4.1-2025-04-14", 1_047_576),
|
||||
("gpt-4-32k-0613", 32768),
|
||||
# Newer families that fell through to the unknown-model default.
|
||||
("gpt-5", 272_000),
|
||||
("gpt-5-mini", 272_000),
|
||||
("o4-mini", 200_000),
|
||||
# Must not regress.
|
||||
("gpt-4", 8192),
|
||||
("gpt-4-turbo", 128_000),
|
||||
("gpt-4o", 128_000),
|
||||
("o3", 200_000),
|
||||
("gpt-3.5-turbo", 16385),
|
||||
],
|
||||
)
|
||||
def test_context_limit_prefers_the_most_specific_prefix(model: str, expected: int) -> None:
|
||||
assert OpenAIProvider()._get_context_limit_manual(model) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("model", "expected"),
|
||||
[
|
||||
("gpt-4.1", "o200k_base"),
|
||||
("gpt-4.1-mini", "o200k_base"),
|
||||
("gpt-4.1-2025-04-14", "o200k_base"),
|
||||
("gpt-5", "o200k_base"),
|
||||
("gpt-5-mini", "o200k_base"),
|
||||
("o4-mini", "o200k_base"),
|
||||
# Must not regress: these genuinely are cl100k_base.
|
||||
("gpt-4", "cl100k_base"),
|
||||
("gpt-4-turbo", "cl100k_base"),
|
||||
("gpt-3.5-turbo", "cl100k_base"),
|
||||
("gpt-4o", "o200k_base"),
|
||||
],
|
||||
)
|
||||
def test_encoding_prefers_the_most_specific_prefix(model: str, expected: str) -> None:
|
||||
assert _get_encoding_name_for_model(model) == expected
|
||||
|
||||
|
||||
def test_cjk_is_not_over_counted_for_gpt_41() -> None:
|
||||
"""The concrete cost of picking cl100k_base for a gpt-4.1 request."""
|
||||
tiktoken = pytest.importorskip("tiktoken")
|
||||
text = "这是一个测试文档，用于验证分词器的差异。" * 30
|
||||
|
||||
chosen = _get_encoding_name_for_model("gpt-4.1")
|
||||
assert len(tiktoken.get_encoding(chosen).encode(text)) == len(
|
||||
tiktoken.get_encoding("o200k_base").encode(text)
|
||||
)
|
||||
|
|
@ -276,10 +276,24 @@ class TestOpenAIModelFallback:
|
|||
"""Test fallback for unknown models."""
|
||||
provider = OpenAIProvider()
|
||||
|
||||
# Unknown model should get 128K default
|
||||
limit = provider.get_context_limit("gpt-5-future")
|
||||
# Unknown model should get 128K default. Deliberately a name that
|
||||
# matches no known family prefix -- this used to say "gpt-5-future",
|
||||
# which stopped being unknown once gpt-5 was added to _CONTEXT_LIMITS.
|
||||
limit = provider.get_context_limit("gpt-9-imaginary")
|
||||
assert limit == 128000
|
||||
|
||||
def test_unknown_variant_inherits_its_family_limit(self):
|
||||
"""An unrecognized variant of a *known* family takes that family's limit.
|
||||
|
||||
This is the same prefix inheritance that gives "gpt-4o-2024-11-20" the
|
||||
gpt-4o limit, and it is strictly better than dropping such a model to
|
||||
the generic 128K default.
|
||||
"""
|
||||
provider = OpenAIProvider()
|
||||
|
||||
assert provider.get_context_limit("gpt-5-future") == 272000
|
||||
assert provider.get_context_limit("gpt-4.1-preview") == 1_047_576
|
||||
|
||||
def test_no_exception_for_unknown_model(self):
|
||||
"""Test that unknown models don't raise exceptions."""
|
||||
provider = OpenAIProvider()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue