Skip to content

[Bugfix] Honor generation controls in the DeepSeek V4.1 tokenizer - #57882

Closed
xijiaat wants to merge 1 commit into
vllm-project:mainfrom
xijiaat:codex/fix-dsv41-generation-controls
Closed

xijiaat wants to merge 1 commit into
vllm-project:mainfrom
xijiaat:codex/fix-dsv41-generation-controls

Conversation

@xijiaat

@xijiaat xijiaat commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

[Bugfix] Honor generation controls in the DeepSeek V4.1 tokenizer

Purpose

The independent deepseek_v41 tokenizer currently ignores
add_generation_prompt and continue_final_message. This produces identical
prompts with the generation prompt enabled or disabled, and leaves EOS at the
end of an assistant prefix even when continuation was requested.

This change forwards the controls to the V4.1 encoder and:

  • suppresses only the final generation cue, preserving historical separators;
  • opens a fresh assistant turn after a completed assistant message;
  • supports system-only generation;
  • removes only the final assistant EOS for continuation;
  • keeps a partial reasoning-only assistant message inside its thinking block;
  • rejects continuation without a final assistant message.

Continuation takes precedence over the generation prompt. This also handles
ChatCompletionRequest(continue_final_message=True) where the default
add_generation_prompt=True remains in the renderer kwargs. The request schema
already rejects callers that explicitly set both fields to true.

The reference fixture files are unchanged. Both fixtures end in a completed
assistant message, so the test now checks their exact reference encoding with
add_generation_prompt=False, and separately checks the new generation cue.

Related Work And Scope

Related: #46256 and #46257. The existing open PR #46257, by @fank, fixes the
older deepseek_v4 encoder; it does not modify the independent deepseek_v41
wrapper or encoder. This proposal is limited to V4.1 and follows the generation
control semantics discussed there. It does not claim to resolve the V4/V3.2
parts of #46256, and should not auto-close that issue.

Open-PR searches and the issue discussion were checked on 2026-09-21. This
follow-up changes only the independent V4.1 implementation, not the files in
#46257; its scope is also explained in the
issue discussion.

No NIXL, scheduling, DSpark, Triton, or CUDA code changes. This is not a claimed
fix for H20 worker deadlocks or production garbage output.

Test Plan

In a development environment installed according to the repository AGENTS.md:

.venv/bin/python -m pytest tests/tokenizers_/test_deepseek_v41.py -q
pre-commit run --files vllm/tokenizers/deepseek_v41.py vllm/tokenizers/deepseek_v41_encoding.py tests/tokenizers_/test_deepseek_v41.py

Coverage includes thinking on/off, final user/developer/system/assistant roles,
system-only prompts, preservation of historical EOS, partial reasoning,
request-to-renderer kwargs, caller-message immutability, and existing reference
tool/image/effort fixtures.

Test Result

Base main: 8ec00ec2bc2d294b742f7814041d3a3043e0a90e.

Tests executed in an isolated Linux container on an H20 host. Only the two
tokenizer modules from this main checkout were overlaid in the test process;
other dependencies came from nightly 0.29.1rc1.dev422+gd05da62e9. This is not a
claim of full-checkout integration testing.

  • New suite against unpatched main tokenizer modules: 18 failed, 30 passed.
  • Same suite against the patched modules: 48 passed.
  • Real downloaded DeepSeek-V4.1-Flash tokenizer: both thinking modes passed
    generation/continuation string-to-ID consistency checks; ordinary user-ended
    prompts retained identical token IDs before and after the patch.
  • Ruff 0.14.0, using this checkout's pyproject.toml: check and format check passed.
  • git diff --check: passed.
  • pre-commit run --files for the three changed files: passed (inapplicable
    hooks skipped), including mypy and the repository-specific checks.

The container regression runner loads the two checked-out tokenizer modules
process-locally and then invokes the existing test file with pytest. The
executed command was:

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 PYTHONDONTWRITEBYTECODE=1 \
  /tmp/dsv41-audit-venv/bin/python /work/tokenizer-pr-final/run_tests.py \
  --source /work/tokenizer-pr-final --tests /work/tokenizer-pr-final

Model Smoke Evaluation

An isolated H20 TP8 server loaded the real DeepSeek-V4.1-Flash model using the
unmodified nightly above. Baseline and patched tokenizers rendered the same
five synthetic conversations to token IDs; those IDs were sent serially to
/v1/completions. No live tokenizer modules were replaced.

Generation settings: temperature=0, seed=2026, max_tokens=128,
stream=false, skip_special_tokens=false. All 10 requests returned HTTP 200;
all five patched cases contained the expected answer fragment.

Case Baseline output Patched output
Normal user math question 323 323 (prompt IDs unchanged)
Continue assistant The result is Raw BOS marker, then # 17 * 19 = 323. 323.
Fresh turn after completed assistant Raw BOS marker and unrelated #sustainable agriculture#, then the answer The capital of France is Paris.
System-only Reply exactly OK. Empty text OK
Continue partial reasoning 17 * 19 = 17 * (20 - 1) = Generated # Invalid Request text after a raw BOS marker 340 - 17 = 323.</think>323

The baseline "Invalid Request" was generated model text, not an HTTP error.
This is a narrow prompt-control smoke evaluation, not a broad accuracy
benchmark or full patched /v1/chat/completions integration test. No
throughput or production-output correctness claim is made from these samples.

AI Assistance And Human Accountability

OpenAI Codex assisted with source review, implementation, test construction,
and test execution. The human submitter reviewed the changes and personally
reran the relevant tests successfully before submission, and is responsible
for the contribution. AI-executed validation is reported separately above.

Forward add_generation_prompt and continue_final_message to the independent V4.1 encoder. Preserve historical turn separators and EOS while honoring final assistant continuation, including partial reasoning. Add focused regression coverage to the existing tokenizer tests.

Co-authored-by: OpenAI Codex <noreply@openai.com>

Signed-off-by: ahmed xijiaat <52128022+xijiaat@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models bug Something isn't working labels Sep 21, 2026
@xijiaat

xijiaat commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Hi @DarkLight1337 and @njhill, could you please review this V4.1-only tokenizer
fix when you have a chance?

This is scoped to the independent V4.1 implementation; the related V4 work is
in #46257. The regression suite reproduces 18 failures before the fix and
passes all 48 tests after it. The PR also includes a small real-model
prompt-control evaluation and its limitations. The changes have been
human-reviewed and the relevant tests personally rerun before submission.

Thank you for your time.

@JohnZhao01

Copy link
Copy Markdown

Hi @xijiaat, thanks for the original fix. I carried it forward on current main in #62729 (with you as co-author), as the Python counterpart of #62189. Reviews welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants