Skip to content

[https://nvbugs/6227203][fix] Remove redundant TikTokenTokenizer shim from KimiK25InputProcessor - #14741

Merged
lancelly merged 1 commit into
NVIDIA:mainfrom
tianyuxbear:fix/6227203
Jun 9, 2026
Merged

[https://nvbugs/6227203][fix] Remove redundant TikTokenTokenizer shim from KimiK25InputProcessor#14741
lancelly merged 1 commit into
NVIDIA:mainfrom
tianyuxbear:fix/6227203

Conversation

@tianyuxbear

@tianyuxbear tianyuxbear commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Description

Remove the _ensure_k25_slow_tokenizer shim from KimiK25InputProcessor. The shim is redundant under the current AutoTokenizer default behavior, and its side effect of discarding the TransformersTokenizer wrapper is the direct cause of AttributeError: TikTokenTokenizer has no attribute tokenizer at openai_server.py:1142 (NVBug 6227203).

Timeline

  1. PR [TRTLLM-11375][feat] Add Kimi K2.5 multimodal vision support #12788 (transformers 5.3.0) — initial K2.5 multimodal. AutoTokenizer correctly returned the slow TikTokenTokenizer. No shim needed.
  2. PR [None][feat] Upgrade transformers dependency to 5.5.3 #13994 (5.3.0 → 5.5.3) — AutoTokenizer started routing K2.5 to a Rust fast backend that BPE-split K2.5 special tokens, breaking multimodal placeholder expansion. Filed as NVBug 6182617.
  3. PR [https://nvbugs/6182617][fix] Restore K2.5 multimodal dep8 accuracy test on transformers 5.5.x #14392 added the shim to force-load the slow class regardless of AutoTokenizer's choice (intended as version-independent).
  4. PR [None][chore] bump transformers to 5.5.4 #14456 (5.5.3 → 5.5.4) — upstream transformers fixed K2.5 AutoTokenizer routing. The slow class is now returned by default.
  5. PR [https://nvbugs/6248987][fix] Made the slow-tokenizer swap lazy and idempotent. __init__ now just sets `_slo #14846 (NVBug 6248987) — made the shim swap lazy and conditional, so that on the text-only thinking path the default tokenizer is kept (running the slow Python TikTokenTokenizer on the orchestrator GIL adds ~100 ms per _fetch_new_requests / broadcast_requests step, a 9x TPOT regression).

After step 4, AutoTokenizer's default output for K2.5 is already the slow TikTokenTokenizer. #14846 keeps that default tokenizer for the text-only path; the only additional work the shim does is to force-load a fresh slow class instance on the multimodal path — which produces a byte-identical tokenizer to the one already loaded by default. The shim machinery is therefore dead code.

This PR removes the entire shim (the original swap in #14392 plus the lazy/conditional machinery added by #14846), letting every path use the default AutoTokenizer output. NVBug 6227203 is resolved because the TransformersTokenizer wrapper is no longer overwritten; NVBug 6248987's perf concern is structurally resolved because no Python-side swap ever fires.

Why the shim is harmful, not just redundant

LLM builds a TransformersTokenizer wrapper whose .tokenizer inner attribute is the HF tokenizer, and passes the wrapper into KimiK25InputProcessor. The shim (whether eager or lazy) overwrites self._tokenizer with a bare slow class once it fires. LLM.tokenizer then reads the bare class back via self.input_processor.tokenizer and propagates it to the OpenAI server, where five sites in openai_server.py access self.tokenizer.tokenizer.<attr> directly — and raise AttributeError.

Test Coverage

  • test_e2e.py::test_openai_kv_cache_contamination — the failing test for NVBug 6227203; unwaived in waives.txt.
  • accuracy/test_llm_api_pytorch_multimodal.py::TestKimiK25::test_nvfp4[dep8] — the original NVBug 6182617 regression test. Re-ran 3× sequentially on 8x B200 with the shim removed: 3/3 PASS, MMMU within sample noise of the transformers 5.3 baseline, no OOM.

@tianyuxbear
tianyuxbear requested a review from a team as a code owner May 29, 2026 08:29
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR removes the slow tokenizer forcing mechanism from KimiK25InputProcessor by eliminating the _ensure_k25_slow_tokenizer method and its invocation in the constructor. The fallback path for token counting in get_num_tokens_per_video is also removed, now requiring image_processor.media_tokens_calculator to be available. A related test waiver is removed.

Changes

Kimi K2.5 Tokenizer and Token Count Simplification

Layer / File(s) Summary
Remove slow tokenizer forcing and fallback token counting
tensorrt_llm/_torch/models/modeling_kimi_k25.py
The _ensure_k25_slow_tokenizer method call is removed from __init__, the fallback path in get_num_tokens_per_video that computed token counts by treating temporal chunks as images is removed, and the _ensure_k25_slow_tokenizer method definition itself is deleted.
Remove test skip waiver
tests/integration/test_lists/waives.txt
The waiver entry for test_e2e.py::test_openai_kv_cache_contamination is removed, allowing the test to run without being skipped.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • NVIDIA/TensorRT-LLM#14392: Main PR removes _ensure_k25_slow_tokenizer and stops forcing the slow tokenizer, directly reversing the addition of this helper and its unconditional invocation.
  • NVIDIA/TensorRT-LLM#14452: Both PRs modify KimiK25InputProcessor tokenizer handling—this PR removes _ensure_k25_slow_tokenizer, while the retrieved PR ensures routing through the slow tokenizer.

Suggested reviewers

  • moraxu
  • syuoni
  • nv-guomingz
  • jieli-matrix
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: removing the redundant TikTokenTokenizer shim from KimiK25InputProcessor, with proper ticket reference and fix type.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR description thoroughly explains the issue, timeline of related PRs, root cause of the bug, and comprehensive test coverage with specific test cases and results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@tianyuxbear

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51017 [ run ] triggered by Bot. Commit: ef98819 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51017 [ run ] completed with state SUCCESS. Commit: ef98819
/LLM/main/L0_MergeRequest_PR pipeline #40465 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

… from KimiK25InputProcessor

Signed-off-by: Tianyu Xiong <117647511+tianyuxbear@users.noreply.github.com>
@tianyuxbear

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52571 [ run ] triggered by Bot. Commit: b8f5746 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52571 [ run ] completed with state SUCCESS. Commit: b8f5746
/LLM/main/L0_MergeRequest_PR pipeline #41853 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@tianyuxbear

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52627 [ run ] triggered by Bot. Commit: b8f5746 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #52627 [ run ] completed with state SUCCESS. Commit: b8f5746
/LLM/main/L0_MergeRequest_PR pipeline #41908 completed with status: 'SUCCESS'

CI Report

Link to invocation

@lancelly
lancelly merged commit 28845dd into NVIDIA:main Jun 9, 2026
13 checks passed
bo-deepinfra added a commit to deepinfra/TensorRT-LLM that referenced this pull request Jun 27, 2026
The rc18->rc19 rebase kept our rc18-based modeling_kimi_k25.py wholesale,
which missed rc19's own evolution of the (shared-lineage) file:
  - NVIDIA#13864 (EPD): get_prompt_token_ids() -> build_disagg_prefill_multimodal_inputs()
    returning DisaggPrefillMultimodalInputs. rc19's inputs/registry.py now
    REQUIRES this method for multimodal disagg; our old file only had the
    removed get_prompt_token_ids -> disaggregated multimodal Kimi would fail
    registry validation.
  - NVIDIA#14741: removed DI's lazy slow-tokenizer shim as redundant.
  - NVIDIA#15219: native vocab_size_padded delegating property.

Re-base our file on rc19's modeling_kimi_k25.py and re-apply only the DI
deltas that rc19 lacks:
  - tokenizer denylist discard ("kimi_k25") for correct gapped special-token
    ids (kept even though the slow-tokenizer shim was dropped; harmless no-op
    if upstream resolves the denylist entry);
  - root-logger filter silencing the per-encode "Calling super().encode" spam;
  - vision-tower MLP row chunking (TLLM_KIMI_VISION_MLP_CHUNK) to bound peak
    activation on large images;
  - keep MoonViT3d LayerNorm params in float32 (TRT-LLM LayerNorm.forward
    upcasts activations -> bf16 weights crash F.layer_norm);
  - data.image placeholder deficit top-up (drop the now-removed
    _ensure_k25_slow_tokenizer call);
  - spec-decode delegations rc19 lacks (spec_worker, spec_config,
    set_guided_decoder; rc19 already forwards the rest);
  - preserve original_input_ids across fuse_input_embeds for the Eagle3 drafter.

Net +110 lines over rc19 base. Aggregated and disaggregated multimodal paths
both supported now. Needs a vision smoke before prod.

Signed-off-by: Bo <bo@deepinfra.com>
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.

4 participants