[test-only] upstream mirror of #2268 — validate tokenizer fixes with HF_TOKEN#2269
Closed
bhimrazy wants to merge 8 commits into
Closed
[test-only] upstream mirror of #2268 — validate tokenizer fixes with HF_TOKEN#2269bhimrazy wants to merge 8 commits into
bhimrazy wants to merge 8 commits into
Conversation
AutoTokenizer.from_pretrained raises a bare OSError for gated repos, which _is_gated_hf_error doesn't recognise — causing the test to fail and retry 3×120s per model (6 min each) until the 35-min job times out. Switch to huggingface_hub.snapshot_download with allow_patterns limited to tokenizer/config files (no weights, no safetensors). It raises the typed GatedRepoError that the existing skip check already handles, so fork PRs without HF_TOKEN skip cleanly in seconds. Using the default HF cache (no local_dir) also means retries and CI caching reuse downloads.
pytest 9 removed the `path` argument from the pytest_collect_file hookspec. pytest-check-links 0.9.1 still declares the old signature, causing a PluginValidationError on startup. The uv migration (#2257) un-pinned pytest, which let 9.x get resolved. Pin to <9 until pytest-check-links ships a compatible release.
Fork PRs have no HF_TOKEN, so 6 parallel CI jobs hit HF anonymously and get rate-limited (HTTP 429). The HfHubHTTPError wasn't caught, causing each rate-limited model to fail and retry 3x120s until the job timed out. Extend the skip check to include 429 and rename to _is_hf_skip_error to reflect that it now covers both gated repos and anonymous rate-limits.
bhimrazy
requested review from
andyland,
k223kim,
lianakoleva and
t-vi
as code owners
June 18, 2026 08:33
snapshot_download returns a commit-hash path; litgpt's Tokenizer infers BOS from the directory name (SmolLM2-*-Instruct, Llama-3*, etc.) so symlink the cached files into tmp_path/<model-name> to preserve that. Also temporarily scope pytester to test_tokenizer.py for faster CI iteration while this is being validated.
CI sets a relative HF_HOME (.cache-HF), so snapshot_download returns a relative path; symlinking from the test's tmp_path produced dangling links and every tokenizer test failed. Copy the small tokenizer/config files into the model-named dir instead — robust regardless of HF_HOME.
Drop the check-links and cpu-tests.yml edits (check-links fix lives in a separate PR; restore the full CPU test suite) and trim the tokenizer test comments to the essentials.
bhimrazy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Upstream mirror of #2268 to validate the tokenizer test fixes against CI with
HF_TOKENavailable. Not for merge — see #2268 for the actual change and discussion.