Skip to content

[Bugfix][Parser] Seed-OSS turn-boundary tokens + boundary-fallback tests - #54264

Merged
sfeng33 merged 3 commits into
vllm-project:mainfrom
Xarbirus:seedoss-reasoning-parser-fix
Sep 9, 2026
Merged

sfeng33 merged 3 commits into
vllm-project:mainfrom
Xarbirus:seedoss-reasoning-parser-fix

Conversation

@Xarbirus

Copy link
Copy Markdown
Contributor

Purpose

Follow-up to #54089, covering two items:

  1. SeedOSS turn boundaries. SeedOssParser inherits the ChatML turn boundaries (<|im_start|>/<|im_end|>) from Qwen3Parser, but they are absent from the Seed vocab, so the current-turn scoping from [Bugfix][Parser] Scope reasoning-end detection to the current turn via turn-boundary tokens #54089 was silently disabled there. Override TURN_BOUNDARIES with <seed:bos>/<seed:eos> (verified against the real ByteDance-Seed/Seed-OSS-36B-Instruct tokenizer: <seed:bos> (id 0) / <seed:eos> (id 2) are single vocab tokens).

  2. Pin the boundary-fallback CONTENT arm. The fallback returns initial_state != REASONING, but all boundary tests in [Bugfix][Parser] Scope reasoning-end detection to the current turn via turn-boundary tokens #54089 ran with thinking enabled. Add focused tests for thinking disabled plus an enabled-state control.

Test Plan

pytest tests/parser/engine/test_seed_oss.py -v
pytest tests/parser/engine/test_qwen3_reasoning.py -v
pytest tests/parser -q
pre-commit run --files <changed files>

Test Result

Tested on H100

  • tests/parser/engine/test_seed_oss.py: 11 passed (incl. 3 new turn-boundary tests)
  • tests/parser/engine/test_qwen3_reasoning.py: 59 passed (incl. 2 new fallback tests)
  • full tests/parser suite: 4091 passed, 0 failures
  • pre-commit on changed files: all hooks passed

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

…r disabled thinking

Follow-up to vllm-project#54089: the turn-boundary fallback returns `initial_state != REASONING`, but all existing boundary tests ran with thinking enabled. Add a focused test for the CONTENT arm (thinking disabled + marker-free prompt tail -> reasoning ended) and a control for the REASONING arm

Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.com>
…ng-end detection

Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.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.

@sfeng33 sfeng33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@sfeng33 sfeng33 added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

✅ @Xarbirus, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@sfeng33

sfeng33 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

/ci run

@sfeng33
sfeng33 enabled auto-merge (squash) September 9, 2026 22:16
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88002 for commit bd43e1b3ff82.

@sfeng33
sfeng33 merged commit 65f3fca into vllm-project:main Sep 9, 2026
73 of 74 checks passed
ItsRoy69 pushed a commit to ItsRoy69/vllm that referenced this pull request Sep 10, 2026
…sts (vllm-project#54264)

Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.com>
Co-authored-by: Flora Feng <4florafeng@gmail.com>
Signed-off-by: Jyotirmoy Roy <jyotirmoyroy649@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants