Skip to content

fix: raise ValueError instead of assert in DeepSeek DSML completion parsing - #5012

Open
pujitha24 wants to merge 1 commit into
InternLM:mainfrom
pujitha24:auto/issue-5011
Open

pujitha24 wants to merge 1 commit into
InternLM:mainfrom
pujitha24:auto/issue-5011

Conversation

@pujitha24

Copy link
Copy Markdown

Motivation

parse_message_from_completion_text in deepseek_v32_encoding.py / deepseek_v4_encoding.py documents that it raises ValueError for malformed output, but enforced the format with assert. It raised AssertionError, and under python -O the checks vanish so malformed output (e.g. missing EOS) parses as a valid message. No production caller was found (tests only), so the benefit is the documented contract plus -O safety.

Modification

Convert the assert-based format checks in the parse functions of both modules to raise ValueError with the same messages. Encode-side asserts untouched. Divergences 2 and 3 in the issue (batch vs streaming grammar, duplicate params) are not addressed.

BC-breaking (Optional)

Callers catching AssertionError from these parse functions would now need ValueError (none found in the repo).

Use cases (Optional)

N/A

Checklist

  1. No linter was available locally (ruff/flake8 not installed); files py_compile cleanly.
  2. Added a malformed-input test per module. Not run under pytest locally (lmdeploy import needs torch/tqdm). Instead a stdlib-only script showed AssertionError (silent success under -O) before, ValueError after.
  3. N/A
  4. Docstring already documented ValueError.

Fixes #5011

Fixes #5011

…arsing

Motivation: parse_message_from_completion_text in the DeepSeek V3.2 and V4
encoding modules documents that it raises ValueError for malformed output,
but enforced the format with assert. It therefore raised AssertionError, and
under `python -O` the checks are stripped so malformed output (e.g. a
completion with no EOS token) parsed into a well-formed-looking message.
Only the V4 parse_tool_calls already used ValueError.

Approach: convert the assert-based format checks in
parse_message_from_completion_text (both modules) and parse_tool_calls
(V3.2) into explicit `raise ValueError` with the same messages. Encode-side
asserts are untouched. No production caller of these functions was found
(tests only), so the benefit is that the documented contract holds and the
checks survive -O; the streaming-vs-batch grammar and duplicate-parameter
divergences from the issue are not addressed here.

Validation:
- Ran a stdlib-only script that loads both modules standalone and parses
  malformed completions: before the change it gave AssertionError (and no
  error at all under `python3 -O`); after, ValueError, including under -O.
- Added tests for both modules; they were only py_compile'd, not run under
  pytest, because importing lmdeploy locally needs deps (torch, tqdm) that
  are not installed here. No linter was available locally either.

Report: InternLM#5011
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
Copilot AI balanced review requested due to automatic review settings September 30, 2026 16:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

2 participants