From 0f136691ca25881e3d12ac2cfe3b44216d8c5d94 Mon Sep 17 00:00:00 2001 From: ghazni Date: Wed, 30 Sep 2026 16:42:04 +0400 Subject: [PATCH 1/3] fix(tokenizer): resolve eos/bos ids from tokenizer_config.json tokenizer.json's ByteLevel post_processor carries no TemplateProcessing template, so ExtractBosEos left eos_id_/-bos_id_ unset on every Qwen-family checkpoint. When the model's config.json and generation_config.json also decline to name an eos_token_id, the request goes out with no eos at all and the model's <|im_end|> is generated then ignored: a Qwen3.5-9B-EXL3 chat reply emitted id 248046 at token 10 and ran to finish_reason=length, hallucinating further turns (measured live, ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77). HF resolves the same id from tokenizer_config.json's eos_token name (string or AddedToken object; AutoTokenizer on the shipped config answers 248046, transformers 5.17.0). FromHfJson now reads the sibling tokenizer_config.json and special_tokens_map.json and fills bos_id_/eos_id_ from the declared names -- only where the post_processor left them unset, so a TemplateProcessing id keeps precedence (the pinned OPT case). The add_bos_token/add_eos_token keys stay unhonoured: HF's loaded class ignores them (measured) and EncodeWithSpecialTokens keeps reading only template_bos_/template_eos_. The DeepSeek-V2 pin's BosId()==-1 assertion was updated: HF resolves bos_token_id from the config name even when add_bos_token resolves False (measured, same transformers), so the pinned behaviour is the encode, not the getter. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: OMP:devin/swe-2 [omp] --- .../ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md | 19 +++ src/vllm/tokenizer/tokenizer.cpp | 120 +++++++++++++++++- tests/vllm/test_bpe.cpp | 91 ++++++++++++- 3 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 .agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md diff --git a/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md b/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md new file mode 100644 index 0000000000..58a068a85f --- /dev/null +++ b/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md @@ -0,0 +1,19 @@ +ID: ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77 +Title: No eos_token_id resolution from tokenizer_config.json: requests carry no eos and generation never stops +Row: MODEL-QWEN35-EXL3 +State: OPEN +Kind: bug +GitHub: - +Mirror: PENDING +Availability: FULL +Created: 2026-09-30 +Updated: 2026-09-30 +Closed: - + +## Problem + +Reproduced live on the vllm-qwen35-9b-exl3 container (Qwen3.5-9B-EXL3-4.00bpw, ROCm gfx1101, 2026-09-30). The checkpoint's config.json carries NO eos_token_id and ships no generation_config.json, and its tokenizer.json post_processor is a bare ByteLevel with no TemplateProcessing template, so Tokenizer::FromHfJson leaves eos_id_ = -1 (ExtractBosEos only reads post_processor). InputProcessor then finds no eos anywhere (input_processor.cpp:42-67) and every request goes out with eos_token_id unset: the model emits <|im_end|> (id 248046) at token 10 of a no-think reply and the engine ignores it, generating fake 'user\n\nassistant\n' turns to max_tokens (finish_reason=length). Upstream resolves the same id from the tokenizer itself: HF eos_token_id comes from tokenizer_config.json's eos_token ('<|im_end|>'), which FromHfJson never reads. User-visible symptom: thinking looks inconsistent — every answer ends by hallucinating more think blocks because <|im_end|> never terminates the turn. + +## Resolution + +FIXED 2026-09-30 on branch row/MODEL-QWEN35-EXL3-eos (commit c09275757). FromHfJson now reads sibling tokenizer_config.json/special_tokens_map.json and resolves eos_token/bos_token NAMES to ids where the post_processor left them unset. Verified LIVE on the vllm-qwen35-9b-exl3 container (rebuilt image): enable_thinking=false stops at <|im_end|> (finish_reason=stop, 11 tokens) where it previously ran to length hallucinating turns; thinking-on request stops at finish_reason=stop with content after the split; think_auto parser (compose adds --reasoning-parser auto) emits reasoning_content on the thinking arm. test_bpe 30/30 green incl. new naming-resolution cases. diff --git a/src/vllm/tokenizer/tokenizer.cpp b/src/vllm/tokenizer/tokenizer.cpp index c57c8c0027..a3682c15cd 100644 --- a/src/vllm/tokenizer/tokenizer.cpp +++ b/src/vllm/tokenizer/tokenizer.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -599,6 +600,105 @@ void ExtractBosEos(const json& doc, int32_t& bos, int32_t& eos) { eos = id_of(single->back()); } +// ─── tokenizer_config.json special-token names (ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77) +// +// ExtractBosEos resolves bos/eos ids ONLY from the post_processor's +// TemplateProcessing template. Every Qwen-family tokenizer.json ships a bare +// ByteLevel post_processor instead, so both ids come back -1 and the engine's +// InputProcessor (input_processor.cpp:42-67) ends up with NO eos at all when +// the model's config.json/generation_config.json also decline to name one: +// nothing stops generation, and <|im_end|> is generated then ignored +// (measured live on Qwen3.5-9B-EXL3: the model emits id 248046 at token 10 and +// the request runs to finish_reason=length hallucinating further turns). +// +// HF's resolved `tokenizer.eos_token_id` -- which is what upstream vLLM's +// InputProcessor reads -- comes from tokenizer_config.json's `eos_token` +// (a string, or an AddedToken object with a `content` field), optionally +// supplied by special_tokens_map.json. AutoTokenizer on the +// Qwen3.5-9B-EXL3-4.00bpw tokenizer_config.json resolves +// eos_token_id = 248046 (<|im_end|>), measured 2026-09-30 with +// transformers 5.17.0. So the sibling config is read here, as a FALLBACK +// applied only where the post_processor left the id unset. +// +// Scope, deliberately narrow: only the NAME->id resolution is taken, and only +// for bos/eos. `add_bos_token`/`add_eos_token` are NOT honoured -- the pinned +// DeepSeek-V2 measurement (test_bpe.cpp "tokenizer_config.json is NOT a +// special-token source") showed HF's loaded class ignores them, and +// EncodeWithSpecialTokens keeps reading only template_bos_/template_eos_. +// Precedence is post_processor first: where the template already named an id +// (OPT), that id stands -- the OPT-pinning subcase pairs a TemplateProcessing +// BOS with a conflicting config bos_token and checks BosId() == 20. + +// The tokenizer_config.json/special_tokens_map.json value for a special-token +// name is either a plain string ("eos_token": "<|im_end|>") or an AddedToken +// object ("eos_token": {"__type": "AddedToken", "content": "<|im_end|>"}). +// Anything else (null, a list) means the checkpoint did not declare one. +std::string SpecialTokenNameOf(const json& v) { + if (v.is_string()) return v.get(); + if (v.is_object()) { + const auto it = v.find("content"); + if (it != v.end() && it->is_string()) return it->get(); + } + return std::string(); +} + +// Resolves a special-token literal to its id inside `tok`: the added-token +// table first (special tokens are added tokens on every HF checkpoint in +// scope), then the stored vocab text -- tried both raw and byte-mapped, since +// tokenizer.json vocab keys live in the GPT-2 mapped alphabet while config +// names are plain text (EncodePlain does the same mapping). -1 when no token +// spells the name. +int32_t ConfigTokenId(const Tokenizer& tok, const std::string& text) { + if (text.empty()) return -1; + for (const SpecialToken& t : tok.AddedTokens()) { + if (t.text == text) return t.id; + } + const std::string mapped = MapBytesToUnicode(text); + for (int32_t id = 0; id < tok.VocabSize(); ++id) { + if (!tok.HasToken(id)) continue; + const std::string& stored = tok.TokenText(id); + if (stored == text || stored == mapped) return id; + } + return -1; +} + +// Reads the eos/bos NAMES declared in tokenizer_config.json (and the older +// special_tokens_map.json, which HF also consults) out of `model_dir`. +// tokenizer_config.json is read SECOND so it wins, the order transformers' +// from_pretrained merges them. A missing sibling is a no-op; a malformed one +// fails loudly like every other unreadable model file. The ids themselves are +// applied by Tokenizer::ApplyConfigSpecialIds (private-field access). +std::pair ConfigSpecialTokenNames( + const std::filesystem::path& model_dir, const std::string& source) { + std::string bos_name; + std::string eos_name; + for (const char* fname : + {"special_tokens_map.json", "tokenizer_config.json"}) { + const std::filesystem::path p = model_dir / fname; + if (!std::filesystem::exists(p)) continue; + std::ifstream in(p, std::ios::binary); + if (!in) continue; + json cfg; + try { + in >> cfg; + } catch (const json::exception& e) { + Fail("JSON parse error in " + p.string() + " (sibling of " + source + + "): " + e.what()); + } + if (!cfg.is_object()) continue; + // tokenizer_config.json is read SECOND so it wins over + // special_tokens_map.json, the order transformers' from_pretrained merges + // them. + if (bos_name.empty()) { + bos_name = SpecialTokenNameOf(cfg.value("bos_token", json())); + } + if (eos_name.empty()) { + eos_name = SpecialTokenNameOf(cfg.value("eos_token", json())); + } + } + return {bos_name, eos_name}; +} + // ---- GGUF kv access (FromGguf) ---- const GgufValue& RequireKv(const GgufFile& f, const char* key) { @@ -701,8 +801,26 @@ Tokenizer Tokenizer::FromHfJson(const std::string& tokenizer_json_path) { if (!in) Fail("cannot open " + tokenizer_json_path); std::string bytes((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - return FromHfJsonBytes( + Tokenizer tok = FromHfJsonBytes( std::string_view(bytes.data(), bytes.size()), tokenizer_json_path); + // A tokenizer.json on disk has a sibling tokenizer_config.json on every HF + // checkpoint in scope; it (and special_tokens_map.json) can name eos/bos the + // post_processor does not. FromHfJsonBytes is file-less, so the sibling read + // lives here and not inside the shared parse. The post_processor keeps + // precedence: a name only fills an id the template left at -1. + const std::filesystem::path dir = + std::filesystem::path(tokenizer_json_path).parent_path(); + if (!dir.empty()) { + const auto [bos_name, eos_name] = + ConfigSpecialTokenNames(dir, tokenizer_json_path); + if (tok.bos_id_ < 0 && !bos_name.empty()) { + tok.bos_id_ = ConfigTokenId(tok, bos_name); + } + if (tok.eos_id_ < 0 && !eos_name.empty()) { + tok.eos_id_ = ConfigTokenId(tok, eos_name); + } + } + return tok; } Tokenizer Tokenizer::FromHfJsonBytes(std::string_view tokenizer_json, diff --git a/tests/vllm/test_bpe.cpp b/tests/vllm/test_bpe.cpp index 5ef0ee3d0a..15d913c842 100644 --- a/tests/vllm/test_bpe.cpp +++ b/tests/vllm/test_bpe.cpp @@ -1089,13 +1089,24 @@ class TempTokenizerDir { } // namespace -TEST_CASE("tokenizer_config.json add_bos_token is NOT applied (DeepSeek-V2 shape)") { - SUBCASE("the exact DeepSeek-V2 shape adds no BOS") { +TEST_CASE("tokenizer_config.json add_bos_token is NOT applied, but the " + "bos/eos NAMES resolve (DeepSeek-V2 shape)") { + SUBCASE("the exact DeepSeek-V2 shape adds no BOS but the name resolves") { // Byte-for-byte the DeepSeek-V2-Lite situation: an AddedToken OBJECT for // bos_token, `add_bos_token: true`, `tokenizer_class: LlamaTokenizerFast`, // and a tokenizer.json whose post_processor is a plain ByteLevel. HF's - // resolved tokenizer adds NOTHING here (measured — see the block comment - // above), so neither may we. + // resolved tokenizer adds NOTHING to an encode here (measured — see the + // block comment above), so neither may we. + // + // What CHANGED (ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77): `add_bos_token` + // remains ignored, but the NAME resolves. HF's resolved + // `tokenizer.bos_token_id` is the config-named token's id even when + // `add_bos_token` resolves False — measured 2026-09-30 with transformers + // 5.17.0 on a checkpoint whose tokenizer_config.json declares bos_token as + // an AddedToken object (bos_token_id=248046 while add_bos_token=False and + // no BOS is prepended). BosId() naming that id is HF-faithful AND what + // lets InputProcessor stop on it; encoding is still the post_processor's + // alone. const TempTokenizerDir d(kTinyJson, R"json({ "tokenizer_class": "LlamaTokenizerFast", "add_bos_token": true, @@ -1103,7 +1114,7 @@ TEST_CASE("tokenizer_config.json add_bos_token is NOT applied (DeepSeek-V2 shape "bos_token": {"__type": "AddedToken", "content": "<|end|>", "normalized": true} })json"); const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); - CHECK(t.BosId() == -1); + CHECK(t.BosId() == 19); // the config-NAMED id; HF resolves it too CHECK(t.EncodeWithSpecialTokens("hello world") == t.Encode("hello world")); } @@ -1112,6 +1123,7 @@ TEST_CASE("tokenizer_config.json add_bos_token is NOT applied (DeepSeek-V2 shape "add_bos_token": false, "add_eos_token": true, "eos_token": "<|end|>" })json"); const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 19); // name resolves even though nothing is appended CHECK(t.EncodeWithSpecialTokens("hello") == t.Encode("hello")); } @@ -1141,3 +1153,72 @@ TEST_CASE("tokenizer_config.json add_bos_token is NOT applied (DeepSeek-V2 shape CHECK(t.EncodeWithSpecialTokens("hello world")[0] == 20); } } + +TEST_CASE("tokenizer_config.json eos_token/bos_token NAMES resolve when the " + "post_processor declares none (ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77)") { + // The Qwen3.5-9B-EXL3 shape: a ByteLevel post_processor (no TemplateProcessing + // ids) plus tokenizer_config.json naming eos_token. HF resolves + // eos_token_id from that name -- AutoTokenizer on the real checkpoint + // returns 248046 for "<|im_end|>" (measured 2026-09-30, transformers + // 5.17.0) -- so InputProcessor's tokenizer fallback must see it too. + // Without this the request carries NO eos id and <|im_end|> is generated + // then ignored. + SUBCASE("string-form eos_token resolves to the added-token id") { + const TempTokenizerDir d(kTinyJson, R"json({ + "eos_token": "<|end|>", + "bos_token": "" + })json"); + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 19); + CHECK(t.BosId() == 20); + // Encoding is untouched: the names set the reported ids only, and the + // post_processor still owns what gets added to a prompt. + CHECK(t.EncodeWithSpecialTokens("hello world") == + t.Encode("hello world")); + } + + SUBCASE("AddedToken object form resolves identically") { + const TempTokenizerDir d(kTinyJson, R"json({ + "eos_token": {"__type": "AddedToken", "content": "<|end|>", + "lstrip": false, "normalized": false, "rstrip": false, + "single_word": false, "special": true} + })json"); + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 19); + } + + SUBCASE("special_tokens_map.json names resolve too") { + const TempTokenizerDir d(kTinyJson, R"json({})json"); + // TempTokenizerDir only writes tokenizer_config.json; drop the map file + // beside it manually for the older-file arm. + std::filesystem::path dir = + std::filesystem::path(d.tokenizer_path()).parent_path(); + std::ofstream(dir / "special_tokens_map.json", std::ios::binary) + << R"json({"eos_token": "<|end|>"})json"; + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 19); + } + + SUBCASE("a post_processor id keeps precedence over the config name") { + // Same OPT shape as the pin above, mirrored on eos: TemplateProcessing + // says (20), the config says <|end|> (19); the post_processor wins. + const std::string tpl_json = ReplaceOnce( + kTinyJson, + R"("post_processor": {"type": "ByteLevel", "add_prefix_space": false, "trim_offsets": false, "use_regex": false})", + R"("post_processor": {"type": "TemplateProcessing", + "single": [{"Sequence": {"id": "A", "type_id": 0}}, + {"SpecialToken": {"id": "", "type_id": 0}}], + "pair": [], "special_tokens": {"": {"id": "", "ids": [20], "tokens": [""]}}})"); + const TempTokenizerDir d(tpl_json, R"json({"eos_token": "<|end|>"})json"); + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 20); + CHECK(t.BosId() == -1); // template's front is Sequence A -> -1, and the + // config names no bos_token, so it stays unset + } + + SUBCASE("an unresolvable name leaves the id unset") { + const TempTokenizerDir d(kTinyJson, R"json({"eos_token": "<|missing|>"})json"); + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == -1); + } +} From d5e6535bdbc9737115d2b2812d2047a3543f4bfe Mon Sep 17 00:00:00 2001 From: ghazni Date: Wed, 30 Sep 2026 20:42:24 +0400 Subject: [PATCH 2/3] fix(tokenizer): make tokenizer_config.json win over special_tokens_map.json Adversarial review of the eos/bos name fallback found the merge precedence inverted: the first-nonempty guard let special_tokens_map.json lock in a name before tokenizer_config.json was read, so the older file won. HF's from_pretrained applies the config's init kwargs after consuming the map file, so a declared key in tokenizer_config.json must overwrite the map's value. Unconditional overwrite regressed differently -- a config that does not declare the key would erase the map's name -- so contains() gates it: value() cannot distinguish absent from an explicit null. Verified empirically: the new 'tokenizer_config.json overrides special_tokens_map.json' subcase fails on the committed code (EosId -1 vs expected 19) and passes here; test_bpe is 30/30 green, 1020 assertions. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: OMP:devin/swe-2 [omp] --- src/vllm/tokenizer/tokenizer.cpp | 16 +++++++++------- tests/vllm/test_bpe.cpp | 13 +++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/vllm/tokenizer/tokenizer.cpp b/src/vllm/tokenizer/tokenizer.cpp index a3682c15cd..7515f28102 100644 --- a/src/vllm/tokenizer/tokenizer.cpp +++ b/src/vllm/tokenizer/tokenizer.cpp @@ -686,14 +686,16 @@ std::pair ConfigSpecialTokenNames( "): " + e.what()); } if (!cfg.is_object()) continue; - // tokenizer_config.json is read SECOND so it wins over - // special_tokens_map.json, the order transformers' from_pretrained merges - // them. - if (bos_name.empty()) { - bos_name = SpecialTokenNameOf(cfg.value("bos_token", json())); + // tokenizer_config.json is read SECOND and wins when it DECLARES the + // key: transformers' from_pretrained applies the config's init kwargs + // after the map file has already been consumed. contains() gates the + // overwrite because value() cannot distinguish "absent" from an explicit + // null. + if (cfg.contains("bos_token")) { + bos_name = SpecialTokenNameOf(cfg.at("bos_token")); } - if (eos_name.empty()) { - eos_name = SpecialTokenNameOf(cfg.value("eos_token", json())); + if (cfg.contains("eos_token")) { + eos_name = SpecialTokenNameOf(cfg.at("eos_token")); } } return {bos_name, eos_name}; diff --git a/tests/vllm/test_bpe.cpp b/tests/vllm/test_bpe.cpp index 15d913c842..a9ddd998ef 100644 --- a/tests/vllm/test_bpe.cpp +++ b/tests/vllm/test_bpe.cpp @@ -1199,6 +1199,19 @@ TEST_CASE("tokenizer_config.json eos_token/bos_token NAMES resolve when the " CHECK(t.EosId() == 19); } + SUBCASE("tokenizer_config.json overrides special_tokens_map.json") { + // HF from_pretrained applies the config file's kwargs AFTER the map file + // has been consumed, so the config name must win when both declare eos. + // Map says <|missing|> (unresolvable); config says <|end|> (19). + const TempTokenizerDir d(kTinyJson, R"json({"eos_token": "<|end|>"})json"); + std::filesystem::path dir = + std::filesystem::path(d.tokenizer_path()).parent_path(); + std::ofstream(dir / "special_tokens_map.json", std::ios::binary) + << R"json({"eos_token": "<|missing|>"})json"; + const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); + CHECK(t.EosId() == 19); + } + SUBCASE("a post_processor id keeps precedence over the config name") { // Same OPT shape as the pin above, mirrored on eos: TemplateProcessing // says (20), the config says <|end|> (19); the post_processor wins. From cacf4fcdff4b690d4b2446a6fbb16adf6a68f206 Mon Sep 17 00:00:00 2001 From: ghazni Date: Thu, 1 Oct 2026 14:52:32 +0400 Subject: [PATCH 3/3] fix(tokenizer): read sibling configs when the tokenizer path has no parent (MODEL-QWEN35-EXL3) FromHfJson("tokenizer.json") computed an empty parent_path and the !dir.empty() guard skipped the entire tokenizer_config.json / special_tokens_map.json lookup, so a bare relative filename left eos_id at -1 while "./tokenizer.json" resolved it (PR #3363 review). The guard is removed: joining a sibling name onto an empty path yields the bare sibling filename, which std::filesystem resolves against the CWD -- the same directory the bare tokenizer.json opened from. A new subcase loads both spellings from the fixture directory; reverting the fix fails it with EosId -1 vs 19. Also records the GitHub #3364 mirror in the row issue file, synced as part of this PR. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: OMP:devin/swe-2 [omp] --- .../ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md | 4 +-- src/vllm/tokenizer/tokenizer.cpp | 21 ++++++------ tests/vllm/test_bpe.cpp | 32 +++++++++++++++++++ 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md b/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md index 58a068a85f..a66263708c 100644 --- a/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md +++ b/.agents/issues/MODEL-QWEN35-EXL3/ISSUE-LOCAL-01M3RY20H90NMK37V74EH34Z77.md @@ -3,8 +3,8 @@ Title: No eos_token_id resolution from tokenizer_config.json: requests carry no Row: MODEL-QWEN35-EXL3 State: OPEN Kind: bug -GitHub: - -Mirror: PENDING +GitHub: 3364 +Mirror: SYNCED Availability: FULL Created: 2026-09-30 Updated: 2026-09-30 diff --git a/src/vllm/tokenizer/tokenizer.cpp b/src/vllm/tokenizer/tokenizer.cpp index 7515f28102..8d8221bbf5 100644 --- a/src/vllm/tokenizer/tokenizer.cpp +++ b/src/vllm/tokenizer/tokenizer.cpp @@ -810,17 +810,20 @@ Tokenizer Tokenizer::FromHfJson(const std::string& tokenizer_json_path) { // post_processor does not. FromHfJsonBytes is file-less, so the sibling read // lives here and not inside the shared parse. The post_processor keeps // precedence: a name only fills an id the template left at -1. + // parent_path() of a BARE filename is empty; joining the sibling name onto + // it yields just the sibling's filename, which std::filesystem resolves + // against the current working directory -- the same directory the bare + // tokenizer.json was opened from. So an empty dir must NOT skip the lookup: + // "tokenizer.json" and "./tokenizer.json" are the same file (PR #3363). const std::filesystem::path dir = std::filesystem::path(tokenizer_json_path).parent_path(); - if (!dir.empty()) { - const auto [bos_name, eos_name] = - ConfigSpecialTokenNames(dir, tokenizer_json_path); - if (tok.bos_id_ < 0 && !bos_name.empty()) { - tok.bos_id_ = ConfigTokenId(tok, bos_name); - } - if (tok.eos_id_ < 0 && !eos_name.empty()) { - tok.eos_id_ = ConfigTokenId(tok, eos_name); - } + const auto [bos_name, eos_name] = + ConfigSpecialTokenNames(dir, tokenizer_json_path); + if (tok.bos_id_ < 0 && !bos_name.empty()) { + tok.bos_id_ = ConfigTokenId(tok, bos_name); + } + if (tok.eos_id_ < 0 && !eos_name.empty()) { + tok.eos_id_ = ConfigTokenId(tok, eos_name); } return tok; } diff --git a/tests/vllm/test_bpe.cpp b/tests/vllm/test_bpe.cpp index a9ddd998ef..fa2246a657 100644 --- a/tests/vllm/test_bpe.cpp +++ b/tests/vllm/test_bpe.cpp @@ -1075,11 +1075,30 @@ class TempTokenizerDir { std::string tokenizer_path() const { return (dir_ / "tokenizer.json").string(); } + const std::filesystem::path& dir() const { return dir_; } + private: std::filesystem::path dir_; }; +// Holds the process working directory inside `new_cwd` for the enclosing +// scope: FromHfJson("tokenizer.json") resolves a BARE filename against the +// CWD, and its sibling lookup must follow the same resolution (PR #3363). +// Declared AFTER the TempTokenizerDir it scopes so destruction order restores +// the CWD before the directory is removed. +class ScopedCwd { + public: + explicit ScopedCwd(const std::filesystem::path& new_cwd) + : saved_(std::filesystem::current_path()) { + std::filesystem::current_path(new_cwd); + } + ~ScopedCwd() { std::filesystem::current_path(saved_); } + + private: + std::filesystem::path saved_; +}; + // NOTE: there is deliberately no `kTinySpecialId` constant here. It named // kTinyJson's added token id 19 ("<|end|>") as a stand-in BOS/EOS, but no case // below ever referenced it — the tests that use that id spell it literally @@ -1234,4 +1253,17 @@ TEST_CASE("tokenizer_config.json eos_token/bos_token NAMES resolve when the " const Tokenizer t = Tokenizer::FromHfJson(d.tokenizer_path()); CHECK(t.EosId() == -1); } + + SUBCASE("a bare filename resolves siblings from the CWD too (PR #3363)") { + // parent_path() of "tokenizer.json" is empty; the sibling join must still + // find tokenizer_config.json in the CWD. The unfixed `!dir.empty()` guard + // skipped the lookup on this spelling and left EosId() at -1 while + // "./tokenizer.json" resolved 19. + const TempTokenizerDir d(kTinyJson, R"json({"eos_token": "<|end|>"})json"); + const ScopedCwd cwd(d.dir()); + const Tokenizer bare = Tokenizer::FromHfJson("tokenizer.json"); + CHECK(bare.EosId() == 19); + const Tokenizer dotted = Tokenizer::FromHfJson("./tokenizer.json"); + CHECK(dotted.EosId() == 19); + } }