Repository navigation
[misc] fix: Enable OpenAI streaming for HF tokenizers - #5739
Conversation
Signed-off-by: Yu Yao <yaoyu.094@gmail.com>
|
/ok to test 120e850 |
|
Light review - HFTokenizerAdapter streaming fix The change makes HFTokenizerAdapter compatible with MCore HuggingFaceFastIncrementalDetokenizer (OpenAI streaming path) by populating tokenizer / include_special_tokens attributes, guarded by hasattr so existing values are preserved. Scope is tight and the test exercises the previously-broken construction path. A couple of concerns:
Note: I could not read MCore HuggingFaceFastIncrementalDetokenizer source in this environment (3rdparty/Megatron-LM submodule not checked out), so finding 1 is a question, not a confirmed defect - please verify against the pinned MCore commit. Suggested test cases
|
| # MCore's incremental detokenizer treats ``_tokenizer`` as its Hugging Face wrapper. | ||
| if not hasattr(tokenizer, "tokenizer"): | ||
| tokenizer.tokenizer = tokenizer | ||
| if not hasattr(tokenizer, "include_special_tokens"): | ||
| tokenizer.include_special_tokens = force_skip_special_tokens is False |
There was a problem hiding this comment.
These attributes are set on tokenizer (the wrapped HF tokenizer, stored as self._tokenizer), but the object handed to HuggingFaceFastIncrementalDetokenizer in the new test is the adapter itself (self), not the wrapped tokenizer:
adapter = HFTokenizerAdapter(tokenizer)
detokenizer = HuggingFaceFastIncrementalDetokenizer(adapter, [])If MCore's detokenizer reads .tokenizer / .include_special_tokens off the argument it's constructed with, it will look at adapter.tokenizer / adapter.include_special_tokens — which are never set here (the adapter has no __getattr__ forwarding). It would only "work" via a getattr(..., default) fallback in MCore, in which case this whole block is dead code w.r.t. the adapter path.
Can you confirm MCore reads these off the wrapped HF tokenizer and not off the adapter? If it reads off the adapter, these should be self.tokenizer = tokenizer and self.include_special_tokens = ....
Problem
The maintained Bridge OpenAI-compatible server rejects supported
stream=truerequests even when it loaded a real Hugging Face fast tokenizer.build_tokenizer()stores the raw Hugging Face tokenizer atHFTokenizerAdapter._tokenizer. The pinned MCore streaming endpoints then treat that value as MCore's Hugging Face wrapper and look for_tokenizer.tokenizer. Standard fast tokenizers exposebackend_tokenizer/ internal_tokenizer, not.tokenizer, so both chat and text completion streaming return HTTP 400 before submitting generation:Non-streaming generation is unaffected. This is adjacent to the OpenAI adapter contract completed in #5091.
Fix
When the wrapped tokenizer does not already provide MCore's wrapper fields, expose:
tokenizerpointing to the real Hugging Face fast tokenizer;include_special_tokensderived from the adapter's detokenization policy.The second field keeps incremental streaming aligned with Bridge's normal special-token behavior; without it, construction succeeds but the EOS marker is emitted in the stream.
Regression evidence
An exact-source CPU reproducer passed a real
PreTrainedTokenizerFastthrough the audited Bridge adapter into the pinned MCore incremental detokenizer.The added unit regression exercises that same cross-package contract with a real fast tokenizer.
Focused and adjacent CPU validation (isolated package-init harness; exact checkout source paths):
Static validation:
The project-managed pre-commit invocation was blocked before hook execution because the pinned resiliency wheel requires a newer glibc platform; the isolated invocation above ran the repository's complete configured hook set.
Scope
This changes only the private inference tokenizer adapter and one CPU unit test. It does not change model weights, generation algorithms, dependencies, CI, public conversion APIs, or MCore source. No full-suite, GPU/model-quality, convergence, or performance validation is claimed.