feat(TOOLS-PARSER-BREADTH): register inkling — the ported engine had no registry face (#608 W1)

[localai-bot]

Row: TOOLS-PARSER-BREADTH · Issue: #608 (W1) · Spec: .agents/specs/tool-parser-breadth.md

Review repair, 2026-08-14

The fresh review returned FAIL on the record, not the code: the shipped C++ was found sound, faithful to upstream, and covered by every mutation it tried. The implementation is not touched. Six findings — all in what this change said about itself — are repaired in an additional commit, and this body is corrected in place with the corrections called out inline.

origin/main was merged in first (it had moved six commits and touched both CMakeLists.txt files this branch also touches), and merged again at the end because it moved six more while the repair was being verified. Every anchor below was re-derived after the last merge, not before, and the gates below ran on the exact SHA that is pushed. Verification ran on aarch64, not x86_64 — see Gates.

What this is

--tool-call-parser inkling now resolves. Registry goes 41 → 42 accepted names, 37 → 38 families.

The gap was not what the spec thought

W1 was scoped as "port a parser". It was not. The Inkling dialect was already ported and already gated against the pinned oracle:

• src/vllm/parser/inkling.cpp — the InklingParser engine subclass
• engine/configs.cpp — inkling_config() and its prefix-stable JSON-span arg carver
• parser::get_parser_engine("inkling") — already dispatched it
• test_parser_engine_assembly — golden scenarios inkling_think_tool_text_*, inkling_nonobject_args_*, captured from vLLM 555967922

What was missing was the tool-parser registry face. ResolveToolParserName("inkling", …) calls get_tool_parser, which returned nullptr, so the flag threw at startup and a fully ported, oracle-gated dialect was unreachable.

The change

tool_parsers/parser_engine_adapter.{h,cpp}	ParserEngineToolAdapter (adapters.py:128 — the TOOL half of make_adapters, twin of the reasoning adapter we already carried) + InklingEngineToolParser (inkling_tool_parser.py:7, registered at __init__.py:177)
abstract.cpp	the factory branch and the tool_parser_names() entry, in the same change, as that file's own comment demands
parser_engine.{h,cpp} / serving_chat.cpp	ParserRequestFromChatCompletion moved out of an anonymous namespace to sit beside ParserRequest, so the adapter and the serving path share one request projection instead of two copies

All upstream anchors read at 555967922.

inkling is EXPLICIT-ONLY in detect.cpp — and for a reason no other row has

Every other EXPLICIT-ONLY family is excluded for marker collision. Inkling is excluded because there is nothing to sniff: that table matches a chat template string, and Inkling has no jinja template at the pin. Rendering is vllm/renderers/inkling_encoding.py (mirrored by the Rust rust/src/chat/src/renderer/inkling/), and examples/ carries no tool_chat_template_inkling.jinja.

Its <|content_invoke_tool_json|> marker collides with nothing, so a row would look harmless — while being unreachable through the only input the function gets, and claiming a template-stability we cannot demonstrate.

No structural-tag row either: upstream sets structural_tag_model = None / supports_required_and_named = False, which our unmapped-family nullopt already is. Repaired: that equivalence is now gated rather than argued — test_adapters_resolve checks ToolChoiceStructuralTagSpecFor("inkling", …) is nullopt for auto, required and named, and a mutation that gives inkling the hermes spec is caught.

Tests

The spec said the test was ours to author, on the assumption that a missing tests/tool_parsers/test_inkling_tool_parser.py meant no upstream test. There is one, in the other place — tests/parser/engine/test_inkling.py — because upstream's class is a bare make_adapters(InklingParser) subclass with no behaviour of its own.

F1 (MEDIUM) — a false claim about upstream, and the four cases it hid

This body, the spec, the landing commit and the test file's header all said upstream "drives InklingParser directly and never constructs the adapter". It does construct it — test_inkling.py:487 class TestRegisteredAdapters resolves through ToolParserManager and calls extract_tool_calls on the result. The claim was not merely wrong, it was load-bearing: it is the sentence that justified not looking for tool-facing cases outside TestNonStreaming/TestStreaming, and two whole classes went unassessed.

Per-case status for both classes:

Upstream case	Status
:488 test_adapters_resolve	PORTED. Adaptation 3: upstream asserts on the registered CLASS (tool_cls._parser_engine_cls is InklingParser); our registry hands back an INSTANCE, so the same fact is a dynamic_cast to InklingEngineToolParser, whose ctor is get_parser_engine("inkling") and admits no other engine (parser_engine_adapter.cpp:49). supports_required_and_named is False is asserted through the surface that flag controls here. Its reasoning half is DECLINED — we have no inkling reasoning registry row (#703, below).
:498 test_adapter_round_trip	PORTED verbatim.
:465 test_tool_choice_none_non_streaming	PORTED, adaptation 4: upstream's parse() returns reasoning too and this seam does not, so the THINK block and its assertion are dropped; content and tool suppression are kept verbatim. This is the case that exercises the tool_choice field of the projection this PR moved.
:477 test_tool_choice_none_streaming	PORTED verbatim.
:436 test_skip_tool_parsing_round_trip	DECLINED, with reason at its site. It sets skip_tool_parsing = True on one InklingParser, calls extract_reasoning, then re-extracts with a second parser. Our ToolParser ABC exposes neither a skip-tool-parsing setter nor extract_reasoning, and the reasoning half has no registry face at all; porting it would mean driving ParserEngine directly — the engine layer this file deliberately does not re-gate.

Corrected split: 22 cases, 19 PORTED with upstream names verbatim, 3 AUTHORED — not the "15 of 18" claimed in four places. Four documented harness adaptations, not two:

1. a <|message_model|> prefix reproduces upstream's MESSAGE_HEADER initial state on the CONTENT-seeded tool adapter, so assertions are preserved rather than weakened;
2. streaming uses upstream's _stream_text_only character-chunk harness (our ToolParser seam is text-only) — a strictly stronger split than token boundaries, since chunks slice markers mid-marker;
3. registered-CLASS assertion → instance dynamic_cast;
4. the tool_choice="none" case drops the reasoning third of its assertion.

TestArgConverter and the token-id/reasoning cases remain deliberately un-ported: they gate the engine layer the assembly goldens already gate.

The authored CONTENT-seed case survives the correction; its stated reason did not. Upstream does construct the adapter — what no upstream case reaches is the seed, which lives only in extract_tool_calls_streaming (adapters.py:178) while extract_tool_calls (:158) delegates to extract_tool_calls_from_content with no seed at all.

F5 (LOW) — two coverage gaps: one closed, one measured and left open

• Closed. StreamTextOnly omitted the trailing parser.finish_streaming() that upstream's _stream_text_only calls and appends, and compensated by appending <|end_message|> to the authored case's input. The harness now calls it; the compensation is gone.
• Open, and now measured. That gives ParserEngineToolAdapter::finish_streaming() its only caller — it has none in src/, include/ or examples/, and will have none while MakeToolParser routes engine-backed names away from this adapter (serving_chat.cpp:546-548) — but not a guarantee. Mutating the body to return std::nullopt leaves all 22 cases green. A streaming twin of test_text_after_tool_call was authored to try to move that mutation, did not, and was deleted rather than shipped with a false rationale: nothing in the ported set is deferred past the last delta. The method ships functionally ungated, and the header, the harness comment and the spec all say so. An unmoved mutation is a measurement, not a licence to claim coverage.

F3 (MEDIUM) — a comment asserting a record that does not exist

test_inkling.cpp claimed the authored case was "recorded as from-scratch in porting-inventory.md §9". No such record exists — and none should: §9 enumerates forced structural deviations of the port, not test authorship, and the reviewer agreed with that reading. No §9 entry is added. The sentence is deleted, and the spec now records that the review agreed and what it actually flagged.

F4 (LOW) — a header claiming coverage that does not exist

parser_engine_adapter.h said adjust_request is unmodelled because "the engine-backed serving path handles it". Nothing handles it. The shared ToolParser seam has no adjust_request dispatch site at all (KimiK2ToolParser::adjust_request, kimi_k2.cpp:87, has no callers either; reasoning_parsers/muse_glimmer.h already records the same seam gap), and serving_chat.cpp applies no override.

It is material for Inkling, whose entire grammar is special tokens: skip_special_tokens defaults true (protocol.h:240/:461), is forwarded verbatim by to_sampling_params (protocol.cpp:583) and honoured at v1/engine/detokenizer.cpp:68, so at server defaults the markers are stripped before the parser runs. The header now uses the sibling "dropped, and that is an OPEN GAP" wording, and docs/USAGE.md tells a user the one thing they can do about it today (pass "skip_special_tokens": false). Tracked as #695; not fixed here.

RED → GREEN

RED was captured with the implementation stashed (git stash push -- src include CMakeLists.txt), so the test compiled against unmodified main and failed on behaviour, not on a missing header.

RED  (implementation stashed, 17 cases at the time):
    test cases: 17 |  1 passed | 16 failed
    assertions: 20 |  2 passed | 18 failed   Status: FAILURE!
    → every failure at REQUIRE( p != nullptr ) in MakeParser()

GREEN (repair head, aarch64, clean build):
    test cases:  22 |  22 passed | 0 failed
    assertions: 133 | 133 passed | 0 failed  Status: SUCCESS!

The one case passing at RED is the EXPLICIT-ONLY autodetect assertion, a control: it must hold both before and after.

Mutations

F6 (INFO) — every catching line re-derived at the repair head, after the merge. The previously quoted :174 / :248 / :138 were already wrong before the merge and moved again after it; docs/FEATURES.md alone slid :230 → :232 in the same change that documents the hazard.

Original four (each restored byte-for-byte, sha256-verified):

Mutation	Caught by
build a base ParserEngine over inkling_config() instead of InklingParser — drops the trailing-text flush hook (inkling.py:376)	test_inkling.cpp:385 (test_text_after_tool_call, case at :373)
drop "inkling" from tool_parser_names(), keep the factory branch	test_inkling.cpp:243 (case at :233) — and the pre-existing count pin test_detect.cpp:222
drop the adapter's initialize_streaming(CONTENT) seed	SURVIVED the first suite. Caught only after adding the authored case, at test_inkling.cpp:284 (case at :256). That is why the case exists.
inkling_arg_converter returns the raw {"name":…,"args":…} wrapper	new suite — and the pre-existing test_parser_engine_assembly goldens (3 cases / 33 assertions), which own it first since the carver is engine-layer

Four more for the cases this repair adds (aarch64):

Mutation	Result
get_tool_parser("inkling") returns HermesToolParser (breaks the engine-class identity)	CAUGHT — :539/:540, :575/:576, and the 18 MakeParser() cases at :121. 2 passed / 20 failed
ToolChoiceStructuralTagSpecFor gives inkling the hermes spec (supports_required_and_named = True)	CAUGHT — :557, all three modes
ParserRequestFromChatCompletion drops tool_choice, hardcodes "auto" (the moved projection)	CAUGHT — :510/:511 and :522. 20 passed / 2 failed
ParserEngineToolAdapter::finish_streaming() returns std::nullopt	SURVIVED, 22/22 green — recorded, not hidden (F5)

The first run of that second table was defective and is worth the warning. Restoring a mutated file with cp -p preserves its mtime, ninja skips the relink, and the next mutation runs against the previous one's object — four mutations produced four identical-looking failures, including a "restored" run that was still red. Restore is cp + touch, and each restore re-runs the suite before the next mutation is applied.

Counts, re-derived not incremented

At the repair head: tool_parser_names() holds 42 entries; get_tool_parser has 42 name == branches over 38 distinct std::make_unique<T> classes; 27 marker rows (unchanged); 39 tool-parser test files.

F2 (MEDIUM) — a spec sentence contradicted by its own diff

The spec said "README.md and docs/FEATURES.md carry no parser count, so neither is owed." False for both.

• docs/FEATURES.md does carry it, and this PR changed it 37 → 38 families.
• README.md carries it twice (:81, :219) and both were already stale before this PR: at 43a6c5518 they read 36 families / 40 names against a true 37 / 41, so README had also missed muse_glimmer. This PR widens the drift to 38 / 42.

README is not corrected here, and that is a gate decision, not a preference. check-doc-checkpoint.py --staged refuses any README edit in a change that does not touch a landing source (.agents/mission.md, CMakeLists.txt, the three benchmarks/demo/*.json, examples/{cli,server}/main.cpp) — "Co-edited public projections can NEVER justify README churn", a rule the checker's own comment calls deliberate and directly tested. Weakening a checker to make a transition pass is forbidden, so the correction is filed as #704 and linked from the roadmap_v1.md issue table, which is the other option the review offered. #704 records the part worth knowing next: the same rule refuses a README-only fix too, so closing it needs either a change that legitimately moves a landing source or an argued exception in its own commit message.

Neither check-readme-structure.py nor check-public-doc-tables.py cross-checks a prose count against the registry, which is why the drift survived two waves. #649 covers only the TOOLS-CALLING-CORE engine-matrix row — a different surface — and is still deliberately not touched here.

Moved with this PR: test_detect.cpp:222, docs/USAGE.md:1126, docs/FEATURES.md:232, docs/STATUS.md, .agents/engine-matrix.md, and the roadmap's --tool-call-parser sweep row (82/90 → 84/90 recipe uses).

Owed, not fixed here

• --reasoning-parser inkling still throws while --tool-call-parser inkling now resolves (asymmetry shipped by #608 W1) #703 (filed by this repair) — --reasoning-parser inkling still throws. Upstream registers inkling in both registries: vllm/reasoning/__init__.py:131 names InklingParserReasoningAdapter, the reasoning half of the same make_adapters(InklingParser) call (registered_adapters.py:67-70) whose tool half this PR lands. Ours has only the tool half, so a user following an Inkling recipe now gets a tool parser that resolves and a hard startup abort on the reasoning flag in the same command line — an asymmetry this PR creates (before it, both flags refused, which was at least consistent). The fix belongs to SAMPLE-REASONING (vllm-serve rejects 61 of 76 official-recipe --reasoning-parser uses, including qwen3 on our own gate models #605), whose table already lists inkling; out of scope here, recorded in the spec's "Owed by W1's landing", and the reason the reasoning half of test_adapters_resolve had to be declined.
• PR #683: engine-backed tool parsers (inkling) run skip_special_tokens=true, stripping structural markers before ParserEngine #695 — engine-backed names run with skip_special_tokens=true. See F4.

The SGLang secondary-oracle check for openai — run, and answered NO

Worth reading before anyone picks up the rest of W1. Under AGENTS.md §"When vLLM has no implementation", the question was whether SGLang (registered oracle sglang, pin f63458b5b… / v0.5.15; local checkout clean at exactly that SHA) should be the source for gpt-oss tool parsing, since it carries harmony_parser.py and a test for it.

It should not, and reaching for it would have been a rule violation. That section admits a secondary oracle only where vLLM implements nothing. vLLM implements this path: vllm/parser/harmony.py is a 358-line vLLM-owned HarmonyParser(DelegatingParser), tested by tests/parser/test_harmony.py, and is exactly what GptOssToolParser's docstring defers to. The stub is a delegation inside vLLM, not an absence of it.

What the check did establish: the residual gap is the out-of-tree openai_harmony package under that wrapper — the same shape as Cohere's cohere_melody, i.e. a W2-class grammar decision, not the "register the name with an explicit refusal" the spec had assumed. Recorded in the spec so the next agent does not re-run the check.

Gates

Verification for the repair ran on kairos-4db2, aarch64 (14 cores, CPU-only build, -DVLLM_CPP_CUDA=OFF), not x86_64. aarch64 is a supported lane — CI carries build-test-cpu-arm64 — so this is legitimate coverage, but it is stated rather than presented as x86.

Gate	Result
Clean configure + full build at the push SHA (aarch64, -DVLLM_CPP_CUDA=OFF)	FINAL_BUILD_EXIT=0, 0 warnings, 0 errors, 0 ENOSPC markers
Full ctest -j 8 at the push SHA	432/433 passed, 2 skipped. The 1 failure is test_engine_core_proc, one of the four known -j starvation tests; passes serially (ctest -R '^test_engine_core_proc$' → 1/1, exit 0). An earlier full run on the same tree minus main's last six commits was 430/430, exit 0, with that same test green — it starves nondeterministically.
Focused test_tool_parser_inkling	22/22 cases, 133/133 assertions, Status: SUCCESS!
test_tool_parser_detect (the 42-name count pin)	16/16 cases, 352/352 assertions
test_parser_engine_assembly (the pre-existing oracle goldens)	3/3 cases, 5038/5038 assertions
scripts/agent-preflight.sh	OK
scripts/agent-preflight.sh --staged	OK — it caught a real one first (see F2: it refused the README edit, and that refusal is why README is filed rather than fixed)
check-commit-trailers.py --range origin/main..HEAD	OK (run explicitly — #653 makes preflight skip it silently when behind main; it also correctly refused while origin/main had moved ahead, which is why main is merged in twice)
check-device-leakage.py	OK, ratchet holds (DSR 32 == baseline 32). This PR's files contribute zero
check-windows-portability.py	RED, inherited — src/vllm/multimodal/video_engine.cpp:21,59,64, #648 / #646. Zero hits in this PR's files. Not this PR.

The push SHA is 4b319ba7c. test_engine_core_proc, test_async_llm, test_serve_low_tools and test_openai_conformance are the known -j starvation set; only the first starved here, and it was re-run serially rather than attributed.

Deliberately deferred

minimax_m3, openai, and both Cohere names. Shipping inkling alone keeps this reviewable. openai in particular is now known to be a Harmony-seam row, materially larger than this one.