mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description
`get_tokenizer_name` can pick the wrong tokenizer for a versioned model,
which silently produces wrong token counts.
For a model that isn't a literal key in `MODEL_TO_TOKENIZER`, it falls
back to prefix matching:
```python
for key, value in MODEL_TO_TOKENIZER.items():
if model_lower.startswith(key):
return value
```
That returns the first key the model merely *starts with*, in
dict-insertion order. The table lists short family keys before their
more-specific siblings — `"qwen"` (→ `Qwen/Qwen-7B`) appears before
`"qwen2"`/`"qwen2-7b"`/`"qwen2.5"`. So
`get_tokenizer_name("qwen2-7b-instruct")` matches `"qwen"` first and
returns the **Qwen1** tokenizer, not Qwen2. Qwen1 and Qwen2 have
different vocabularies, so every `count_text`/`count_messages` for that
model is off. `qwen2.5-*` and `deepseek-v2.x` are mis-resolved the same
way.
The sibling tiktoken resolver already documents and guards this exact
pitfall — `get_encoding_for_model` uses an explicit most-specific-first
prefix list with a comment that scanning "for the first key that merely
starts with the prefix is order-dependent and wrong." The HuggingFace
resolver is the one that still scans insertion order.
## Fix
Match the **longest** (most-specific) prefix instead of the first in
insertion order:
```python
for key in sorted(MODEL_TO_TOKENIZER, key=len, reverse=True):
if model_lower.startswith(key):
return MODEL_TO_TOKENIZER[key]
```
Direct-key lookups and the shorter-family fallback (e.g. `deepseek-chat`
→ `deepseek-ai/deepseek-llm-7b-base`) are unchanged.
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/huggingface.py`: `get_tokenizer_name` prefix
matching now iterates keys longest-first and returns the most-specific
match.
- `tests/test_huggingface_tokenizer_timeout.py`: add
`test_get_tokenizer_name_prefers_most_specific_prefix`.
- `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/huggingface.py tests/test_huggingface_tokenizer_timeout.py
All checks passed!
$ python -m py_compile headroom/tokenizers/huggingface.py tests/test_huggingface_tokenizer_timeout.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 against the real key table
with a dependency-free script that parses `MODEL_TO_TOKENIZER` out of
the source and runs both the old (insertion-order) and new
(longest-first) scans, then left the full pytest to CI.
- Exact command / steps: resolved `qwen2-7b-instruct`, `qwen2.5-turbo`,
and `deepseek-v2.5` under both strategies, plus `deepseek-chat` as a
regression guard.
- Observed result: old scan returns `Qwen/Qwen-7B` (Qwen1) for both
qwen2 models and `deepseek-ai/deepseek-llm-7b-base` (v1) for
`deepseek-v2.5`; new scan returns `Qwen/Qwen2-7B`, `Qwen/Qwen2.5-7B`,
and `deepseek-ai/DeepSeek-V2` respectively. `deepseek-chat` resolves
identically under both (`deepseek-ai/deepseek-llm-7b-base`), so the
existing timeout test's model is unaffected.
- Not tested: loading the actual HuggingFace tokenizers
(network/`transformers`); 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 is a localized swap of the prefix-scan order in
a pure function, verified by the standalone proof (run against the real
key table) and the new regression test for CI. This mirrors the
same-class fix already present in the sibling tiktoken resolver.
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
135 lines
5.3 KiB
Python
135 lines
5.3 KiB
Python
"""HF tokenizer loading must be bounded (GH #1701): AutoTokenizer.from_pretrained
|
|
performs unbounded network downloads/retries; called lazily from the proxy's request
|
|
path it blocked the event loop for ~10 minutes and zombified the server. The fix
|
|
tries the local HF cache first (local_files_only=True), bounds the network attempt
|
|
with HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS on a daemon thread, and fails open to
|
|
estimation — caching the failure so the hub is probed at most once per process.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import sys
|
|
import time
|
|
import types
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
from headroom.tokenizers import huggingface as hf_mod
|
|
from headroom.tokenizers.huggingface import (
|
|
HuggingFaceTokenizer,
|
|
_load_tokenizer,
|
|
get_tokenizer_name,
|
|
)
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _fresh_cache():
|
|
_load_tokenizer.cache_clear()
|
|
yield
|
|
_load_tokenizer.cache_clear()
|
|
|
|
|
|
def _install_fake_transformers(monkeypatch: pytest.MonkeyPatch, from_pretrained) -> None:
|
|
fake = types.ModuleType("transformers")
|
|
fake.AutoTokenizer = type(
|
|
"AutoTokenizer", (), {"from_pretrained": staticmethod(from_pretrained)}
|
|
)
|
|
monkeypatch.setitem(sys.modules, "transformers", fake)
|
|
|
|
|
|
def test_local_cache_tried_before_network(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
calls: list[dict[str, Any]] = []
|
|
|
|
def fake_from_pretrained(name: str, **kwargs: Any):
|
|
calls.append(kwargs)
|
|
if kwargs.get("local_files_only"):
|
|
raise OSError("not in cache")
|
|
return "network-tokenizer"
|
|
|
|
_install_fake_transformers(monkeypatch, fake_from_pretrained)
|
|
monkeypatch.setenv("HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS", "5")
|
|
|
|
assert _load_tokenizer("some/model") == "network-tokenizer"
|
|
assert calls[0].get("local_files_only") is True, "first attempt must be cache-only"
|
|
assert not calls[1].get("local_files_only")
|
|
|
|
|
|
def test_cache_hit_never_touches_network(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
calls: list[dict[str, Any]] = []
|
|
|
|
def fake_from_pretrained(name: str, **kwargs: Any):
|
|
calls.append(kwargs)
|
|
return "cached-tokenizer"
|
|
|
|
_install_fake_transformers(monkeypatch, fake_from_pretrained)
|
|
|
|
assert _load_tokenizer("some/model") == "cached-tokenizer"
|
|
assert len(calls) == 1
|
|
assert calls[0].get("local_files_only") is True
|
|
|
|
|
|
def test_slow_network_load_times_out_and_fails_open(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
def fake_from_pretrained(name: str, **kwargs: Any):
|
|
if kwargs.get("local_files_only"):
|
|
raise OSError("not in cache")
|
|
time.sleep(60) # simulates hung huggingface_hub download
|
|
return "never"
|
|
|
|
_install_fake_transformers(monkeypatch, fake_from_pretrained)
|
|
monkeypatch.setenv("HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS", "0.2")
|
|
|
|
start = time.monotonic()
|
|
assert _load_tokenizer("slow/model") is None
|
|
assert time.monotonic() - start < 5, "load must unblock at the timeout, not the download"
|
|
|
|
# Failure is cached (lru_cache) — the second call must not re-probe the hub.
|
|
start = time.monotonic()
|
|
assert _load_tokenizer("slow/model") is None
|
|
assert time.monotonic() - start < 0.05
|
|
|
|
|
|
def test_timeout_zero_disables_network_loading(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
def fake_from_pretrained(name: str, **kwargs: Any):
|
|
if kwargs.get("local_files_only"):
|
|
raise OSError("not in cache")
|
|
raise AssertionError("network load attempted despite timeout=0")
|
|
|
|
_install_fake_transformers(monkeypatch, fake_from_pretrained)
|
|
monkeypatch.setenv("HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS", "0")
|
|
|
|
assert _load_tokenizer("offline/model") is None
|
|
|
|
|
|
def test_count_messages_fails_open_to_estimation(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
def fake_from_pretrained(name: str, **kwargs: Any):
|
|
raise OSError("unavailable")
|
|
|
|
_install_fake_transformers(monkeypatch, fake_from_pretrained)
|
|
monkeypatch.setenv("HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS", "0.2")
|
|
|
|
counter = HuggingFaceTokenizer("deepseek-chat")
|
|
tokens = counter.count_messages([{"role": "user", "content": "hello world" * 50}])
|
|
assert tokens > 0 # estimation fallback, no exception, no hang
|
|
|
|
|
|
def test_invalid_timeout_env_falls_back_to_default(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
monkeypatch.setenv("HEADROOM_HF_TOKENIZER_LOAD_TIMEOUT_SECS", "not-a-number")
|
|
assert hf_mod._load_timeout_secs() == hf_mod._LOAD_TIMEOUT_DEFAULT
|
|
|
|
|
|
def test_get_tokenizer_name_prefers_most_specific_prefix() -> None:
|
|
"""A more-specific family key must win over a shorter one.
|
|
|
|
Prefix matching used to scan MODEL_TO_TOKENIZER in dict-insertion order, so
|
|
the short "qwen" key preceded "qwen2"/"qwen2.5" and shadowed them —
|
|
"qwen2-7b-instruct" resolved to the Qwen1 tokenizer (a different vocabulary,
|
|
hence wrong counts). The resolver now picks the longest matching prefix.
|
|
"""
|
|
# Versioned models not present as literal keys must hit the right family.
|
|
assert get_tokenizer_name("qwen2-7b-instruct") == "Qwen/Qwen2-7B"
|
|
assert get_tokenizer_name("qwen2.5-turbo") == "Qwen/Qwen2.5-7B"
|
|
assert get_tokenizer_name("deepseek-v2.5") == "deepseek-ai/DeepSeek-V2"
|
|
# Direct hits and the shorter family fallback still resolve as before.
|
|
assert get_tokenizer_name("qwen-14b") == "Qwen/Qwen-14B"
|
|
assert get_tokenizer_name("deepseek-chat") == "deepseek-ai/deepseek-llm-7b-base"
|