Skip to content

fix: guard remove_special_tokens against tokenizers without a BOS token - #7048

Merged
oobabooga merged 2 commits into
unslothai:mainfrom
vineethsaivs:fix/remove-special-tokens-none-bos
Jul 10, 2026
Merged

oobabooga merged 2 commits into
unslothai:mainfrom
vineethsaivs:fix/remove-special-tokens-none-bos

Conversation

@vineethsaivs

Copy link
Copy Markdown
Contributor

Summary

remove_special_tokens strips a leading BOS token but calls prompt.startswith(tokenizer.bos_token) without checking that the tokenizer actually has a BOS token. Many popular tokenizers, including Qwen2 / Qwen2.5, GPT-2, Falcon and GPT-NeoX, define no BOS token, so tokenizer.bos_token is None and the call raises:

TypeError: startswith first arg must be str or a tuple of str, not NoneType

remove_special_tokens is the final step of test_hf_gguf_equivalence, so running the HF vs GGUF equivalence check on any of those models crashes.

Fix

Guard bos_token with getattr(tokenizer, "bos_token", None) before the comparison, mirroring the None checks already used elsewhere in this same file (get_ollama_eos_tokens and _change_system_message). When the tokenizer has no BOS token the prompt is returned unchanged; the existing single-BOS-stripping behaviour is preserved for tokenizers that do have one.

Test

tests/python/test_remove_special_tokens_no_bos.py loads the function via ast (no GPU, no import unsloth) and covers:

  • a no-BOS tokenizer (bos_token=None) returns the prompt unchanged (this raises the TypeError before the fix),
  • a BOS-bearing tokenizer still strips one leading BOS token,
  • a prompt without a leading BOS is left untouched.

remove_special_tokens called prompt.startswith(tokenizer.bos_token) without
checking bos_token first. Tokenizers such as Qwen2/Qwen2.5, GPT-2, Falcon and
GPT-NeoX have no BOS token (tokenizer.bos_token is None), so the call raised
"TypeError: startswith first arg must be str or a tuple of str, not NoneType"
and crashed test_hf_gguf_equivalence for those models.

Guard bos_token with getattr(tokenizer, "bos_token", None), mirroring the
None checks already used elsewhere in this file (get_ollama_eos_tokens and
_change_system_message).
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@oobabooga

Copy link
Copy Markdown
Member

Confirmed this on main: the HF vs GGUF equivalence check crashes outright on any tokenizer without a BOS token (Qwen2, GPT-2, Falcon, GPT-NeoX), so those models can't be verified at all. Your guard fixes it. I ran it across the cases that matter: no-BOS tokenizers now pass through untouched, BOS-bearing ones still get their leading BOS stripped as before, and prompts without a leading BOS are left alone. Nothing regresses. I also checked the other spots that strip a BOS the same way, and they were already handled. Tests pass 3/3.

Thanks @vineethsaivs. Merging.

@oobabooga
oobabooga merged commit 33119c9 into unslothai:main Jul 10, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants