fix(gguf): Use EOS token ID from GGUF metadata instead of HF tokenizer

[kitaekatt]

Summary

GGUF files store the correct EOS token ID in tokenizer.ggml.eos_token_id metadata. However, vLLM was using the HuggingFace tokenizer's eos_token_id, which can differ from the GGUF value.

This causes generation to not stop properly for models like Gemma 3, where:

• GGUF metadata specifies EOS token ID 106 (<end_of_turn>)
• HF tokenizer reports EOS token ID 1 (<eos>)

The model generates <end_of_turn> to signal completion, but vLLM waits for token ID 1 which never comes, resulting in repeated EOS tokens until max_tokens is reached.

Example output before fix:

Response: Hello! I'm doing well, thank you!<end_of_turn><end_of_turn><end_of_turn>...(repeats until max_tokens)

Changes

• Add extract_eos_token_id_from_gguf() in gguf_utils.py to read EOS from GGUF metadata
• Patch tokenizer.eos_token_id in hf.py when loading GGUF tokenizers
• Log when EOS token ID is patched for debugging

Testing

• Tested with unsloth/gemma-3-12b-it-GGUF model
• Verified GGUF EOS token ID (106) is extracted correctly
• Generation now stops properly at <end_of_turn> token

Notes

This is a common issue with GGUF models that use different EOS tokens than the base HuggingFace tokenizer. The fix ensures vLLM uses the authoritative EOS token ID from the GGUF file itself.

[gemini-code-assist]

Code Review

This pull request correctly identifies and fixes an issue where the EOS token ID from GGUF metadata was not being used, leading to incorrect generation termination for certain models. The introduction of extract_eos_token_id_from_gguf and the patching logic in HfTokenizer are well-implemented.

However, I've identified a potential bug in the way the GGUF file path is resolved for remote models, which could cause the fix to not work in those scenarios. I've provided a detailed comment and a code suggestion to address this. Once this is fixed, the PR should be in good shape.

[gemini-code-assist]

The current logic for constructing gguf_path is incorrect for remote GGUF models. When path_or_repo_id is a HuggingFace repository ID, Path(path_or_repo_id) creates a local relative path that doesn't exist, causing extract_eos_token_id_from_gguf to fail silently by returning None.

To correctly handle both local paths and remote repository IDs, you should first check if path_or_repo_id is a local directory. If not, assume it's a repository ID and use hf_hub_download to get the local path of the GGUF file. This ensures the EOS token ID can be extracted correctly for remote models.

Note that this might cause the GGUF file to be downloaded twice (once here and once during model loading), which is a performance consideration for a follow-up improvement. For now, this change ensures correctness.

Please add the following imports at the top of the file:

from huggingface_hub import hf_hub_download
from huggingface_hub.utils import HfHubHTTPError

            gguf_path = None
            if Path(path_or_repo_id).is_dir():
                gguf_path = Path(path_or_repo_id) / gguf_file
            else:
                try:
                    gguf_path = hf_hub_download(
                        repo_id=str(path_or_repo_id),
                        filename=gguf_file,
                        revision=revision,
                        cache_dir=download_dir,
                    )
                except (HfHubHTTPError, FileNotFoundError) as e:
                    logger.warning("Failed to download GGUF file %s: %s", gguf_file, e)

            if gguf_path and Path(gguf_path).is_file():
                gguf_eos_id = extract_eos_token_id_from_gguf(str(gguf_path))
                if gguf_eos_id is not None:
                    hf_eos_id = tokenizer.eos_token_id
                    if hf_eos_id != gguf_eos_id:
                        logger.info(
                            "Patching tokenizer eos_token_id from %d to %d "
                            "(using GGUF metadata)",
                            hf_eos_id,
                            gguf_eos_id,
                        )
                        tokenizer.eos_token_id = gguf_eos_id

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @kitaekatt.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[kitaekatt]

Testing performed:

Tested with GGUF models on RTX 5090 (32GB, Blackwell architecture):

• bartowski/NousResearch_Hermes-4-14B-GGUF
• tensorblock/Qwen_Qwen3-30B-A3B-Instruct-2507-GGUF

Ran models through a local benchmark runner for HumanEval and GSM8K. Verified server startup completes reliably without hangs.

Related PRs:

This is part of a series of GGUF pipeline fixes for Blackwell GPU compatibility:

• fix(gguf): Auto-select compatible dtype for GGUF models on Blackwell #30410: Fix GGUF dtype selection for Blackwell compatibility
• fix(gguf): Skip lm_head mapping for models with tied word embeddings #30412: Fix tokenizer semaphore leak
• fix(gguf): Use EOS token ID from GGUF metadata instead of HF tokenizer #30434 (this PR): Fix shared memory barriers

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

Comment @cursor review or bugbot run to trigger another review on this PR

[cursor]

Logger crashes when HF tokenizer has no EOS token

Medium Severity

The logger.info call uses %d format specifier for hf_eos_id, but tokenizer.eos_token_id can be None (it's typed as Optional[int] in transformers). When hf_eos_id is None, the comparison hf_eos_id != gguf_eos_id evaluates to True, and the subsequent logging statement raises a TypeError because %d cannot format None.

 

[kitaekatt]

This is technically valid but pre-existing behavior (the original eager initialization had the same pattern). For this to actually leak, cached_tokenizer_from_config() would need to fail transiently then succeed on retry, which doesn't happen in practice - tokenizer failures are deterministic (missing files, corrupt model, OOM).

I’d prefer to keep this PR focused on the semaphore fix.

[kitaekatt]

Validation Results

vLLM	transformers	Cherry-picked PRs	HumanEval	IFEval
HEAD	5.x	#30410, #30411, #30412, #30413, #30424, #30434, #30699, #30702, #31464, #33846	gem2-2b-gguf (42.1%), gemma3-1b (26.8%)	gem2-2b-gguf (65.6%)
HEAD	4.x	#30410, #30411, #30412, #30413, #30424, #30434, #30699, #30702, #31464, #33846	q3-moe-gguf (83.5%)	q3-moe-gguf (85.4%)

Tested on RTX 5090 (Blackwell, SM 120) with all listed PRs cherry-picked together; models listed under each benchmark passed that benchmark in the given environment, while the same models crash or fail without these PRs applied.

Rebased to current upstream HEAD and re-validated on RTX 5090 (Blackwell, SM 120). Fix confirmed still necessary — 3 GGUF models crash without it.

[Isotr0py]

Hi @kitaekatt, can you consolidate those cherry-picked PRs into one PR? Becaue most of them are minor and gemma2-specific.

[kitaekatt]

Closing in favor of consolidated PR #37220, as requested by @Isotr0py in #30434. All fixes from this PR are included in the consolidated version.