test(SAMPLE-REASONING): pin the two qwen3 branches nothing caught, and correct a 2-core that was never possible (#605, #606)

.agents/specs/reasoning-parsers.md

@@ -142,6 +142,31 @@ The v4-alias sub-case is SKIPPED-with-reason in `test_deepseek_v3.cpp`
 input strings, non-streaming AND the per-delta streaming reconstruction,
 RED-first.
 
+### Coverage BEYOND upstream in `test_qwen3.cpp` (from scratch, recorded)
+
+Upstream drops nothing here — there is nothing to drop. `test_qwen3_reasoning_parser.py`
+has **no** `is_reasoning_end` case for qwen3 at all (the only one in the suite is
+`test_base_thinking_reasoning_parser.py:111`, for the base family), and its
+`THINKING_DISABLED_CASES` run through the adapter, which suppresses tool parsing.
+Both `Qwen3Parser` overrides therefore sit on branches upstream's own fixtures
+cannot reach, so three TEST_CASEs are written from scratch against the upstream
+SOURCE (`vllm/parser/qwen3.py:247,256-275`, `vllm/parser/seed_oss.py:24-29`):
+
+| our TEST_CASE | pins | mutation that proves it |
+|---|---|---|
+| `engine thinking-off passthrough survives an unskipped tool` | `qwen3.py:247` on the ENGINE, tool parsing NOT suppressed | drop the `extract_reasoning` override (#630) |
+| `is_reasoning_end (text form, incl. unpaired <tool_call>)` | `qwen3.py:256-275`, incl. the reasoning-REOPEN branch — a `<think>` AFTER the tool-call marker, which upstream's backwards walk meets first | drop `qwen3.cpp:55` `ps > p`; only `is_reasoning_end("<tool_call>x<think>y") == false` reds |
+| `seed_oss inherits the qwen3 thinking-off passthrough` | `get_parser_engine("seed_oss")` builds the shared `Qwen3Parser`, mirroring `class SeedOssParser(Qwen3Parser)` | route seed_oss back to a bare `ParserEngine` |
+
+The reopen and seed_oss rows are follow-up to the #630 fresh review's two LOW
+findings; before them, deleting `qwen3.cpp:55` left the focused suite fully green
+(7/7, 157/157), because all four existing tool-call assertions put `<think>`
+BEFORE the `<tool_call>`, where the guard cannot fire. Only the thinking-OFF arm
+can observe the seed_oss class change at all — the sole production call site,
+`OpenAIServingChat::MakeParserEngine` (`serving_chat.cpp:580`), takes the header
+default `thinking=true`, under which the refactor is byte-identical, so the
+production path was never affected.
+
 ## Gates
 
 - CORRECTNESS: doctest parity — each parser's `(reasoning, content)` over the

.agents/specs/serve-recipe-args.md

@@ -275,16 +275,23 @@ tests/vt/test_cpu_threadpool.cpp:539: ERROR: CHECK( ratio < 100.0 ) is NOT corre
 Attributed rather than assumed, to the same standard as #584. The guard divides
 two wall-clock medians (`over_us / fits_us`) and compares the ratio to a fixed
 100, so both ends are machine-shape dependent and the denominator is the problem:
-on the 2-core runner `fits_us` collapsed to 0.48 us, small enough that ordinary
+on the 4-core runner `fits_us` collapsed to 0.48 us, small enough that ordinary
 scheduler noise in the numerator moves the ratio by tens. Same commit, same code,
 three observations:
 
 | Box | `fits` | `over` | ratio |
 |---|---|---|---|
-| 2-core CI runner | 2 threads, 0.48 us | 5 threads, 48.752 us | **101.567** RED |
+| 4-core CI runner | 2 threads, 0.48 us | 5 threads, 48.752 us | **101.567** RED |
 | 20-core box (#631's table) | 10 threads, 7.213 us | 21 threads, 19.467 us | **2.699** GREEN |
 | 20-core box, this branch rebuilt at the reviewed head | 10 threads, 13.256 us | 21 threads, 13.135 us | **0.990872** GREEN |
 
+The runner's core count is read off the failure itself, not assumed: the case
+returns early at `test_cpu_threadpool.cpp:501` when `cores < 4`, so a box that
+produced a ratio at all has at least 4; and `fits = cores / 2` (`:512`) reporting
+2 threads with `over = cores + 1` (`:513`) reporting 5 pins it at exactly 4 —
+which is what GitHub gives `ubuntu-latest` on a public repo. An earlier revision
+of this section said 2-core, a number no run on that lane can produce.
+
 The last row is the one measured while repairing this record — `test_cpu_threadpool`
 9 cases / 9 passed, 19602 assertions / 0 failed, `Status: SUCCESS!`. That it sits
 2.7x below the middle row on the *same class of box* is itself the finding: the

tests/vllm/entrypoints/openai/reasoning_parsers/test_qwen3.cpp

@@ -17,6 +17,20 @@
 //     cannot carry request kwargs (same deviation as deepseek_v3, threading is
 //     W4 in specs/reasoning-parsers.md), so the thinking flag is taken through
 //     the public constructor — the same seam test_deepseek_v3.cpp uses.
+//
+// BEYOND UPSTREAM (written from scratch, no upstream case is dropped — there is
+// none to drop). Upstream's reasoning suite has NO qwen3 `is_reasoning_end`
+// test at all (only test_base_thinking_reasoning_parser.py:111 for the base
+// family), and its thinking-disabled cases run with tool parsing suppressed.
+// The last three TEST_CASEs are therefore ours, and cover branches upstream's
+// fixtures cannot reach:
+//   - "engine thinking-off passthrough survives an unskipped tool" — the
+//     qwen3.py:247 override on the ENGINE, not the adapter.
+//   - "is_reasoning_end (text form, ...)" — the qwen3.py:256 override,
+//     including the reasoning-REOPEN branch (`<tool_call>` then `<think>`).
+//   - "seed_oss inherits the qwen3 thinking-off passthrough" — the shared
+//     class, which only the thinking-off arm can observe.
+// Recorded in .agents/specs/reasoning-parsers.md § Tests to port.
 #include <doctest/doctest.h>
 
 #include <optional>
@@ -28,6 +42,7 @@
 #include "vllm/entrypoints/openai/reasoning_parsers/abstract.h"
 #include "vllm/entrypoints/openai/reasoning_parsers/parser_engine_adapter.h"
 #include "vllm/parser/engine/configs.h"
+#include "vllm/parser/parser_manager.h"
 #include "vllm/parser/qwen3.h"
 
 using namespace vllm::entrypoints::openai;
@@ -263,4 +278,50 @@ TEST_CASE("qwen3: is_reasoning_end (text form, incl. unpaired <tool_call>)") {
   CHECK(p->is_reasoning_end("thinking<tool_call>\n<function=bash>") == true);
   CHECK(p->is_reasoning_end("thinking<tool_call>x</tool_call>") == false);
   CHECK(p->is_reasoning_end("<think>a<tool_call>b") == true);
+  // The three above all put <think> BEFORE the tool-call marker, where the
+  // re-open guard cannot fire. This one puts it AFTER: upstream's backwards walk
+  // (qwen3.py:263-268) tests the reasoning-start id at EVERY index before it
+  // tests the tool-call id, so it meets <think> first and returns False —
+  // reasoning has re-opened and the earlier unpaired <tool_call> no longer ends
+  // it. It is the only assertion here that fails if `ps > p` (qwen3.cpp:55) is
+  // dropped.
+  CHECK(p->is_reasoning_end("<tool_call>x<think>y") == false);
+}
+
+// Upstream seed_oss is `class SeedOssParser(Qwen3Parser)` (seed_oss.py:24)
+// overriding only the four wrapper token strings, so it INHERITS the
+// thinking-off passthrough (qwen3.py:247). Since #630 (#605) our
+// `get_parser_engine` mirrors that by building the same Qwen3Parser class over
+// the `<seed:...>` spelling (parser_manager.cpp:28); before that, seed_oss got
+// a bare ParserEngine and the thinking-off arm ran the state machine instead.
+//
+// Nothing else pins the inheritance. The only production call site,
+// `OpenAIServingChat::MakeParserEngine` (serving_chat.cpp:580), takes the
+// header default thinking=true, under which a bare engine and Qwen3Parser are
+// byte-identical — so ONLY the thinking-off arm can tell the two apart, which
+// is why this case exists.
+TEST_CASE("seed_oss inherits the qwen3 thinking-off passthrough") {
+  namespace pe = vllm::parser::engine;
+  // seed_oss.py:26-29 — the qwen3 grammar with `seed:`-prefixed wrappers;
+  // `<function=>`/`<parameter=>` are byte-identical.
+  const std::string seed_tool_body =
+      "<seed:tool_call>\n<function=bash>\n<parameter=command>"
+      "\ncat /etc/hosts\n</parameter>\n</function>\n</seed:tool_call>";
+  const std::string output = "I need to read the file.\n\n" + seed_tool_body;
+
+  auto off = vllm::parser::get_parser_engine("seed_oss", /*thinking=*/false);
+  REQUIRE(off != nullptr);
+  const auto [reasoning, content] =
+      off->extract_reasoning(output, pe::ParserRequest{});
+  CHECK(reasoning == std::nullopt);
+  CHECK(content == Opt(output));
+
+  // Same name, thinking ON: the initial state is REASONING and the unpaired
+  // <seed:tool_call> is an implicit reasoning end, so the content span is NOT
+  // the whole output. That difference is exactly what the override suppresses.
+  auto on = vllm::parser::get_parser_engine("seed_oss", /*thinking=*/true);
+  REQUIRE(on != nullptr);
+  const auto [r2, c2] = on->extract_reasoning(output, pe::ParserRequest{});
+  CHECK(r2 == Opt("I need to read the file.\n\n"));
+  CHECK(c2 != Opt(output));
 }