feat(SAMPLE-REASONING): --reasoning-parser qwen3 (and its mimo alias) resolve (#605) - #630
Merged
Merged
Conversation
…605) FOLLOWING_AGENTS_PROTOCOL The work breakdown numbered its waves without demand data. The recipe-surface sweep supplies it and inverts the order. Across the 157 official recipes, --reasoning-parser is passed 76 times over 20 distinct values. We resolve 15; the other 61 abort startup with "unknown reasoning parser". W3 -- the engine-backed adapters -- covers 43 of those 76 uses. W2 covers 18, and four of its names (ernie45, granite, cohere_command3/4, openai_gptoss) have ZERO recipe demand. qwen3 alone is 18 uses, more than every W2 name combined except kimi_k2 and poolside_v1. qwen3 is also what the published Qwen3.5 and Qwen3.6 recipes pass to models we already ship token-exact and gated, so the engine currently serves the model and rejects its own recipe's flag. That is the argument for resequencing, and it is why the first brick inside W3 is Qwen3ParserReasoningAdapter: __init__.py:87 registers mimo onto the same class, so two names land for one port, and glm45/glm47 share Glm47MoeParserReasoningAdapter the same way. Three names were missing from W3 and are added: nano_v3, plus kimi_k3 and ling3, which are POST-PIN and recorded so they are not rediscovered -- they land with the next pin advance, not before. W2 is resequenced, not cancelled; the zero-demand names stay in scope because upstream registers them and we mirror upstream. Spec only, no code and no row-state change. VERIFICATION, stated honestly: check-pr-size clean; staged preflight FAILED one gate, test_cpu_x86_llamacpp_floor test_a_contended_leg_is_discarded_and_never_summarised, "AssertionError: 4 != 2". That gate is load-sensitive and this box is at load average 79 from parallel builds. Reproduced on the SHARED CHECKOUT at clean main with no changes at all ("NO_QUIET_WINDOW after 30s, busy=125% load=48.12"), so it is the instrument, not this diff -- which is one markdown file and cannot reach a CPU floor harness. Not claimed as green; re-verify on a quiet box or on CI. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
… resolve (#605) FOLLOWING_AGENTS_PROTOCOL qwen3 is the single most common --reasoning-parser value in the whole official recipe corpus (18 of 76 uses), and it is what the published Qwen3.5 and Qwen3.6 recipes pass to models we already ship token-exact and gated. Until now the engine served the model and aborted on its own recipe's flag with "unknown reasoning parser". This is W3 brick 1 of specs/reasoning-parsers.md, whose ordering amendment (#605, 3b40aa5) authorises W3 ahead of W2 on that demand data. TWO NAMES, ONE PORT. vllm/reasoning/__init__.py registers "qwen3" (:115) and "mimo" (:87) onto the SAME class, Qwen3ParserReasoningAdapter, so both land here. Coverage 10 -> 12 registered names. NOT AN ALIAS OF think_auto. Checked before writing anything, because it looked like one. Upstream's qwen3 config is initial_state=REASONING, so a marker-less stream is ENTIRELY reasoning (test_qwen3_reasoning_parser.py:66 WITHOUT_THINK expects reasoning="This is the rest", content=None). think_auto.cpp:30 does the exact opposite by design: no marker => everything is CONTENT, because a generic <think> template covers hybrid-thinking models that may answer with no think block. Both are right for their own selection path; they are not interchangeable, and qwen3 gets a real port. qwen3 also ends reasoning on a <tool_call> with no </think> at all (qwen3.py:137), which no <think>-splitting text parser can see. Ported over the ALREADY-LANDED parser engine, not as a fresh text parser -- that reuse is the point of W3 and a hand-rolled parallel path is what the shared-seam rule forbids: - reasoning_parsers/parser_engine_adapter.{h,cpp}: ParserEngineReasoningAdapter (adapters.py:35), the reusable base every remaining W3 name plugs into, including the _skip_tool_parsing scope that keeps the tool body verbatim content for the tool parser; plus Qwen3ParserReasoningAdapter (registered_adapters.py:48). - parser/qwen3.{h,cpp}: Qwen3Parser (qwen3.py:201), the engine subclass carrying the two behaviours the config cannot express -- thinking-off passthrough (:247) and the unpaired-<tool_call> reasoning end (:256). Both read their literals from the config's terminals, so seed_oss -- which upstream spells `class SeedOssParser(Qwen3Parser)` with four different wrapper strings -- now shares this class instead of a second parallel construction. Tool assembly is untouched; neither override is on that path. - ParserEngine gains extract_reasoning_streaming (parser_engine.py:519) and a virtual is_reasoning_end (parser_engine.py:595) in the TEXT form our reasoning seam documents, since the token-ID methods are dropped there. No detect.cpp marker row: qwen3 has no template-stable literal distinct from the existing generic "<think>" row, and that row must keep resolving to think_auto for exactly the hybrid-thinking reason above. Explicit selection is correct. TESTS. tests/.../test_qwen3.cpp ports tests/reasoning/test_qwen3_reasoning_parser.py @ 555967922 verbatim: all 10 TEST_CASES fixtures x non-streaming AND streaming, the 5 MULTI_TOKEN_DELTA_CASES, THINKING_DISABLED_CASES, and is_reasoning_end. RED first (both names nullptr), then green: 7 cases / 157 assertions. Two things stated honestly. First, the three THINKING_DISABLED_CASES do NOT distinguish the qwen3.py:247 override: the adapter always suppresses tool parsing, under which a suppressed <tool_call> already degrades to content, so they pass with the override deleted -- upstream's own test has the same blind spot. A direct engine-level case was added that does distinguish it, and it is what caught the mutation. Second, the pinned name-count in test_detect.cpp moves 10 -> 12; that pin is what forces the README / USAGE / STATUS counts to move in this same change. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
FOLLOWING_AGENTS_PROTOCOL main advanced while this branch was open: #616's spec amendment squash-landed, along with the four other recipe-sweep PRs. Two conflicts, resolved deliberately. reasoning-parsers.md: this branch carries #616's amendment (it was based on that branch) PLUS its own W3 status -- nano_v3 added to the wave list and the "Brick 1 DONE" record. Taking this side is correct because it is a strict superset; verified by confirming the amendment section and all names survive, not by assuming supersetness from the diff shape. roadmap_v1.md: the check-agent-record gate refused with "issue #605 listed twice" -- and it was RIGHT. #612 landed an intake row for #605 while this branch carried its own, so the merge would have defined one key twice. That is the fourth time this campaign that two branches wrote the same keyed record; it is the first time a gate caught it rather than a human, which is worth recording. Resolved per AGENTS.md: kept main's row in main's position and folded this branch's content into it as a scoped update, rather than appending a second row. Not resolved by taking either side wholesale: the surviving row now states both the original defect AND that brick 1 landed, because after this merge both are true and a keyed record must describe its key's current state. scripts/agent-preflight.sh --staged: exit 0, all gates green. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part of #605 — the first brick of wave W3 in
specs/reasoning-parsers.md.Builds on
row/reasoning-recipe-order(commit3b40aa56, the committed specamendment that authorises W3 ahead of W2 on measured recipe demand). That commit
is included in this branch; merge it or this branch, not both bases.
Why this one first
--reasoning-parseris passed 76 times across 20 values in the 157 officialvLLM recipes. We resolved 15 of those uses; 61 aborted startup.
qwen3alone is18 — the single most common value in the corpus — and it is what the published
Qwen3.5 and Qwen3.6 recipes pass to models we already ship token-exact and gated.
The engine served the model and rejected its own recipe's flag.
Coverage 10 → 12 registered names:
vllm/reasoning/__init__.pyregistersqwen3(:115) andmimo(:87) onto the SAME class, so two names land for oneport.
It is not think_auto with a different name
That was the first thing checked, since it looked like an alias.
think_auto(think_auto.cpp:30)qwen3(qwen3.py:100)initial_state=REASONING)<tool_call>with no</think>qwen3.py:137)</think>in contentqwen3.py:132)Upstream's own fixture pins the disagreement:
WITHOUT_THINK(
test_qwen3_reasoning_parser.py:66) expectsreasoning="This is the rest", content=None;think_autoreturns the exact opposite, deliberately, becausethe generic
<think>template row must cover hybrid-thinking models that answerwith no think block. Both are right on their own selection path.
qwen3gets areal port.
Shape
Ported as a reasoning FACE over the already-landed
src/vllm/parser/engine/parser (
TOOLS-STREAMING-PARSER), which is what W3 is for — a hand-rolled<think>splitter would be the parallel path the shared-seam rule forbids.reasoning_parsers/parser_engine_adapter.{h,cpp}—ParserEngineReasoningAdapter(ports
vllm/parser/engine/adapters.py:35), the reusable base everyremaining W3 name plugs into, including the
_skip_tool_parsingscope thatkeeps the tool body verbatim content; plus
Qwen3ParserReasoningAdapter(
registered_adapters.py:48).parser/qwen3.{h,cpp}—Qwen3Parser(qwen3.py:201), the engine subclasscarrying the two behaviours the declarative config cannot express: thinking-off
passthrough (
:247) and the unpaired-<tool_call>reasoning end (:256).Both read their literals from the config's terminals, so
seed_oss— upstreamliterally
class SeedOssParser(Qwen3Parser)with four different wrapperstrings — now shares this class instead of a second parallel construction. Tool
assembly is untouched; neither override is on that path.
ParserEnginegainsextract_reasoning_streaming(parser_engine.py:519) anda virtual
is_reasoning_end(parser_engine.py:595) in the TEXT form ourreasoning seam documents.
No
detect.cppmarker row.qwen3has no template-stable literal distinctfrom the existing generic
<think>row, and that row must keep resolving tothink_autofor the reason above. Explicit selection is correct here.Verification
tests/vllm/entrypoints/openai/reasoning_parsers/test_qwen3.cppportstests/reasoning/test_qwen3_reasoning_parser.py@555967922verbatim: all 10TEST_CASESfixtures × non-streaming AND streaming, the 5MULTI_TOKEN_DELTA_CASES,THINKING_DISABLED_CASES, andis_reasoning_end.nullptr, 5 cases / 6 assertions failed.7 passed | 0 failed,157 assertions | 157 passed.cmake --build buildclean-Werror,ctest403/404;the one red was
test_engine_core_proc, a knownctest -jstarvation, and itpasses serially.
scripts/agent-preflight.sh --staged— all gates green, exit 0.check-pr-size.py— OK.Mutations (restored byte-for-byte, sha256-verified)
SkipToolParsingscopeCHECK(got.content == c.content)on bothtool_call_*fixtures (test_qwen3.cpp:162,:177)qwen3.py:247)CHECK(content == Opt(output))attest_qwen3.cpp:234<tool_call>no longer ends reasoningis_reasoning_end("thinking<tool_call>…")and("<think>a<tool_call>b")at:263,:265is_reasoning_end("a</think>b<think>c")at:260Stated honestly: the three
THINKING_DISABLED_CASESdo not distinguishthe
qwen3.py:247override. The adapter always suppresses tool parsing, underwhich a suppressed
<tool_call>already degrades to content, so they pass withthe override deleted — upstream's own test has the same blind spot. A direct
engine-level case (
test_qwen3.cpp:225) was added that does distinguish it, andthat is the assertion the mutation table cites.
Records
test_detect.cpp's pinned name count moves 10 → 12, which is what forcesREADME.md,docs/USAGE.mdanddocs/STATUS.mdto move in the same change..agents/engine-matrix.md,.agents/roadmap_v1.md(issue table + row 6) and thespec's wave status / tests table are reconciled here too. The spec's tracked
README-count risk is discharged.
🤖 Generated with Claude Code