feat(frontend): add basetenkenizer backend - #12376
Conversation
Signed-off-by: Mahesh Bapatu <maheshbapatu@gmail.com>
WalkthroughThe change adds the ChangesBasetenkenizer tokenizer support
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/src/dynamo/frontend/frontend_args.py (1)
91-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
frozensetto satisfy Ruff RUF012 on the shared class attribute.Static analysis flags
_VALID_TOKENIZER_BACKENDSas a mutable class-level default (RUF012). Since it's only read via membership checks, switching to afrozensetfixes the lint with no import changes needed.♻️ Proposed fix
- _VALID_TOKENIZER_BACKENDS = {"default", "fastokens", "basetenkenizer"} + _VALID_TOKENIZER_BACKENDS = frozenset({"default", "fastokens", "basetenkenizer"})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/src/dynamo/frontend/frontend_args.py` at line 91, Update the shared class attribute _VALID_TOKENIZER_BACKENDS to use frozenset instead of a mutable set, preserving its existing tokenizer backend values and membership-check behavior.Source: Linters/SAST tools
lib/llm/src/model_card.rs (1)
1260-1307: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated alt-backend load/fallback logic.
The
FastokensandBasetenkenizerarms are structurally identical (UTF-8 path check → try load → log → fallback towrap_hf(hf)), differing only in the concrete tokenizer type and log strings. Consider factoring this into a small helper closure/function parameterized by a loaderFnOnce(&str) -> Result<Arc<dyn Tokenizer>, E>and a backend label, to avoid tripling this boilerplate if a third backend is added later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/llm/src/model_card.rs` around lines 1260 - 1307, Extract the duplicated alternate-backend loading flow from the Fastokens and Basetenkenizer arms into a reusable local helper or function parameterized by the backend label and loader, while preserving the UTF-8 path validation, success logging, failure logging, and wrap_hf(hf) fallback behavior. Update both TokenizerBackend branches to use the helper and retain their concrete loaders and labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/fern/components/frontend/Tokenizer.md`:
- Around line 24-28: Update the link in the “basetenkenizer Native Encoder and
Decoder” section to remove the invalid Baseten Tokenizer repository URL or
replace it with the correct public reference, while preserving the surrounding
backend description.
---
Nitpick comments:
In `@components/src/dynamo/frontend/frontend_args.py`:
- Line 91: Update the shared class attribute _VALID_TOKENIZER_BACKENDS to use
frozenset instead of a mutable set, preserving its existing tokenizer backend
values and membership-check behavior.
In `@lib/llm/src/model_card.rs`:
- Around line 1260-1307: Extract the duplicated alternate-backend loading flow
from the Fastokens and Basetenkenizer arms into a reusable local helper or
function parameterized by the backend label and loader, while preserving the
UTF-8 path validation, success logging, failure logging, and wrap_hf(hf)
fallback behavior. Update both TokenizerBackend branches to use the helper and
retain their concrete loaders and labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 691f014a-9e52-4080-9e37-cf5656ec3cfc
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlcomponents/src/dynamo/frontend/frontend_args.pycomponents/src/dynamo/frontend/tests/test_frontend_args.pydocs/fern/components/frontend/Tokenizer.mddocs/fern/components/frontend/frontend-config-reference.mdxdocs/fern/features/tokenizer/README.mdlib/llm/src/local_model/runtime_config.rslib/llm/src/model_card.rslib/llm/tests/model_card.rs
This comment has been minimized.
This comment has been minimized.
Refresh the Python bindings lockfile for dynamo-tokenizers 1.6.0 and remove the invalid Baseten Tokenizer documentation link. Signed-off-by: maheshrbapatu <maheshbapatu@gmail.com>
Remove the frontend argument test file as requested in review. Signed-off-by: maheshrbapatu <maheshbapatu@gmail.com>
Resolve dependency conflicts by retaining upstream dynamo-tokenizers 1.7.0 and dynamo-renderer 5.0.0. Signed-off-by: maheshrbapatu <maheshbapatu@gmail.com>
Signed-off-by: maheshrbapatu <maheshbapatu@gmail.com>
|
/ok-to-test e28d719 |
dyn-3691-extract-shared-target-pid-cuda-customstorage-operation-layer * 'main' of https://github.com/ai-dynamo/dynamo: (50 commits) docs(cli): correct removed vLLM prefill-worker flag reference (#12581) docs(operator): reserve webhook Ignore for emergencies (#12563) ci(docs): make previews and checks match what actually publishes (#12339) refactor(vllm): organize custom encoder modules (#12416) feat(llm): Select reasoning output field via env var (#11464) feat(runtime): add TLS support to TCP request plane (#10921) fix: convert conditional disagg sglang warning to httperror 400 (#12578) feat(operator): add runtime feature gates (#12421) refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait (#12447) feat(replay): add deterministic canonical offline reports (#12363) build: bump ModelExpress to 0.5.0(OPS-7978) (#12455) fix(mocker): use logical KV tokens for decode timing (#12583) fix(examples): update Triton example for CUDA 13 + fix libdcgm copy (DYN-3697) (#12577) refactor(operator): implement composition-first DGD reconciliation (#12283) feat(frontend): add basetenkenizer backend (#12376) fix(profiler): configure rapid mocker without planner (#12573) docs(vllm): correct worker-role flags and document --kv-transfer-config (#12568) ci: add Kubernetes deploy test to nightly (#12090) fix(container): reuse pinned protoc in runtime image (#12535) feat(self-host): flip DYN_SELF_HOST_METADATA default to ON (gh-8749) (#11417) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Overview:
Adds Baseten Tokenizer as a selectable Dynamo frontend tokenizer backend using
BasetenTokenizerexposed by the current upstreamdynamo-tokenizers1.7.0. Operators can select it with--tokenizer basetenkenizerorDYN_TOKENIZER=basetenkenizer.Details:
BasetenTokenizerfrom the current upstreamdynamo-tokenizers1.7.0.basetenkenizerthrough the Python CLI/environment configuration, JSON runtime configuration, and Rust string parsing.tokenizer.jsonfiles while preserving HuggingFace fallback behavior.tokenizer_config.jsonfiles and wrap the selected backend with the existing L1 prefix cache.Where should the reviewer start?
lib/llm/src/model_card.rsfor backend construction, caching, and fallback.lib/llm/src/local_model/runtime_config.rsfor configuration parsing and precedence.lib/llm/tests/model_card.rsfor parity, cache, special-token, segmented-encoding, and fallback coverage.Related Issues
Validation
cargo check -p dynamo-llm --lib --lockedcargo test -p dynamo-llm --lib tokenizer_backend --locked(4 passed)cargo test -p dynamo-llm --test model_card --locked(10 passed)cargo test -p dynamo-llm --test tokenizers --locked(22 passed)git diff --checkEnd-to-end performance validation
AIPerf exercised all three Dynamo frontend tokenizer backends against the same live
Qwen/Qwen3-8BvLLM worker.Hardware and software
97,871 MiBHBM reported bynvidia-smi) and one 72-core NVIDIA Grace/Arm Neoverse-V2 CPU.6.8.0-111-generic; NVIDIA driver610.43.02.337655f924usingdynamo-tokenizers1.6.0. Current upstream provides 1.7.0; itsBasetenTokenizerimplementation is unchanged from 1.6.0, and the merged tree passed the correctness checks above.0.11.0; vLLM0.22.1rc1.dev144+g25f921b00.Workload and method
Qwen/Qwen3-8B, BF16, tensor parallelism 1, max model length 131,072 with static YaRN, vLLM prefix caching enabled, and 8,192-token chunked prefill.DYN_TOKENIZER_CACHE=0, confirmed by zero tokenizer-cache hit and miss counters.Mean time to first token
Mean Dynamo frontend tokenization time
Measured from
dynamo_frontend_tokenize_seconds:In this setup, Basetenkenizer reduced measured frontend tokenization time by 85.7-89.0% relative to HuggingFace and 2.1-18.5% relative to Fastokens. End-to-end Basetenkenizer and Fastokens remained close: Fastokens mean TTFT was 0.4-2.4% lower in these runs. The 36.35% result is specifically Basetenkenizer versus HuggingFace at approximately 99% model KV-cache reuse; it is not a general across-workload speedup. With a cold model KV cache, GPU prefill dominated and the TTFT reduction versus HuggingFace was 1.11%.
Summary by CodeRabbit
New Features
basetenkenizertokenizer backend.--tokenizeroption andDYN_TOKENIZERenvironment variable.Documentation
Bug Fixes