Fix qwen3coder tool parser keeping quotes in function and parameter names

[lusoris]

🛠 Summary

JIRA/Issue: #4487

Symptom. When the model emits the XML tag attribute quoted — <parameter="command"> instead
of the spec's <parameter=command> — the quotes end up inside the emitted JSON argument key. The
client receives {"\"command\"": "..."} and OpenAI-schema validation rejects the tool call. The
same defect exists on the function name (<function="bash"> yields the tool name "bash").

Measured impact. Restating the figure from #4487 precisely, because the headline number is
easy to misread: a sample of 600 opencode runs on 2026-08-29 produced 171 tool calls that
failed client-side schema validation, spread across 88 runs. 54 of those 171 carried a quoted
argument key — the largest single malformed-key shape, roughly a third of the total. The other
117 are different shapes and are not evidence for this PR. Caveat, as already stated on the
issue on 2026-09-03: that 54/171 count aggregates more than one Qwen lane, and because request
bodies and generated fragments were not retained, individual rows cannot be attributed to one
model or one card. Please treat it as symptom evidence for the shape, not as a per-model
reproducer. We currently carry a LiteLLM post-call hook that rewrites these keys; that is defence
in depth, and the fix belongs in the parser.

Lanes that can carry this shape. From retained configuration, not from attribution: three
models in our fleet are configured with --tool_parser qwen3coder --reasoning_parser qwen3 on
Intel Arc (xe) — qwen3-8-27b, qwen3-6-35b-a3b and qwythos-9b-v2. The one I can name
concretely for the reported symptom is qwen3-8-27b: artifact OpenVINO/Qwen3.8-27B-int4-ov,
OVMS 2026.4, Intel Arc Pro B60, --target_device=GPU. I cannot claim the 54 rows all came from
that lane.

Root cause. src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp, in
Qwen3CoderToolParserImpl::parseUntilStateChange (line numbers against main
3607c3572c58de28ba161d7325b6723d0ff85756):

• :157 — case State::InsideFunctionName:
  this->currentFunction.name = streamContent.substr(this->lastProcessedPosition, pos - this->lastProcessedPosition);
• :177 — case State::InsideParameterName:
  this->currentParameterName = streamContent.substr(this->lastProcessedPosition, pos - this->lastProcessedPosition);

pos comes from DEFINE_TAG_POSITION_AND_BREAK_IF_NOT_FOUND (:68-73), a bare find(">"), so
the attribute is taken verbatim. Qwen3CoderToolParser::PARAMETER_NAME_TAG / FUNCTION_NAME_TAG
(:34, :32) carry no quote characters, so nothing consumes them.

Second-order effect, which is why this is more than cosmetic. Both names are the lookup keys
into toolsParametersTypeMap: currentFunction.name at :79, and currentParameterName at
:86 (gating normalizeBooleanString) and :109 (gating enforceStringValue). A quoted name
makes every one of those lookups miss, so schema-driven type handling is silently skipped for the
whole call: a parameter declared type: string is no longer coerced, and True/False is no
longer normalized. With this patch a quoted-attribute call gets the same typing as a clean one.

Scope within this repo — one parser, not five. qwen3coder is the only tool parser here whose
name terminator is a bare > (Qwen3CoderToolParser::XML_TAG_END = ">", :33). The two other
parsers with the same substr-until-terminator shape embed the quote in the terminator itself —
OnyxToolParser::NAME_ATTR_END_TAG = "\">" and Minicpm5ToolParser::XML_TAG_END = "\">" — so a
quoted attribute is consumed there by construction and neither is affected. (minicpm5
additionally has an explicit quote-aware extractNameAttribute().) No other parser in
src/llm/io_processing/ uses XML attribute-style name tags at all. This change therefore needs to
land in exactly one place.

On whether the model is at fault. It is — and the PR does not claim otherwise. Qwen's official
templates render the attribute unquoted on both the prompt-spec side and the assistant-rendering
side (Qwen/Qwen3-Coder-30B-A3B-Instruct chat_template.jinja:88,
Qwen/Qwen3.8-27B chat_template.jinja:137: {{- '<parameter=' + args_name + '>\n' }}), so
<parameter="name"> is a model deviation, not a dialect variant.

The argument for handling it in the parser is that this file has already absorbed exactly this
class of deviation once, in the value position: #3727 (merged 2025-11-05, @atobiszei, same file)
split the old escapeString() into escapeQuotes() + escapeNewline() so that model-emitted
quotes inside string parameter values stopped corrupting the JSON. To be accurate about the
current tree: escapeQuotes() no longer exists — grep -rn escapeQuotes src/ returns nothing on
main — because the value path was later refactored to go through rapidjson Parse() plus
enforceStringValue() (src/llm/io_processing/utils.cpp:164 on main; :179 on this branch,
which adds the new helper above it). The precedent is what I am
citing, not the helper. Still live today: addParameterToCurrentFunctionDoc calls trimNewline()
on values to strip template artifacts, guarded on removeNewlineAroundParameters, which is
= true for this parser (qwen3coder_tool_parser.hpp:109). The name is the one position that
never got equivalent treatment.

To be explicit about what this is not: no other qwen3-coder parser normalizes the name today
(vLLM's Rust take_until(1.., ">") and Python ([^>]*), SGLang's p_match[:p_idx] all take it
verbatim), so this is a proposed improvement rather than convergence with peers.

The nearest precedent in shape is vLLM's Gemma4 parser, which strips STRING_DELIM from
string-quoted keys on main — the
if key.startswith(STRING_DELIM) and key.endswith(STRING_DELIM) guard inside
_parse_gemma4_args — after vllm#44715. I want to hedge
that comparison rather than have you point it out: Gemma4's STRING_DELIM is <|"|>, its own
dialect's legal string delimiter, so that fix was a parser learning to consume a delimiter its
own spec defines. This PR asks OVMS to tolerate a character the Qwen template does not emit at
all. Same failure surface, weaker precedent.

Scope of the change

trimSurroundingQuotes() added to src/llm/io_processing/utils.{hpp,cpp}, next to the existing
trimNewline / normalizeBooleanString tag-parser helpers, and called at both extraction sites.

Production diffstat, so the number matches what you see: +25 lines across three production
files, 0 deletions — qwen3coder_tool_parser.cpp +4 (two call sites, each one line plus one
comment line), utils.cpp +15 (the helper), utils.hpp +6 (declaration plus comment). The rest
of the diff (+339) is tests.

The rule is deliberately narrow: a quote pair is a delimiter only when the same quote character
(" or ') is both the first and the last character of the extracted name and does not occur
anywhere in between. Consequences:

• <parameter=command> → command — byte-identical, no behaviour change for clean input.
• <parameter="command"> / <parameter='command'> → command.
• <parameter=arg"1> → arg"1 — an internal quote is part of the name, never trimmed.
• <parameter="a" "b"> → unchanged — stripping would silently merge two quoted tokens.
• <parameter=parameter=command> (a leaked tag prefix, another shape our downstream hook
  normalizes) is out of scope — we have no established cause for it, and stripping a parameter=
  prefix from a name would be a real over-trim hazard.

I fixed the function name as well as the parameter name: it is the same verbatim-substr
shape, the same dialect, and it additionally un-types every parameter of the call via the :79
lookup. Happy to split it out if you would rather keep the PR to the reported symptom only.

An alternative implementation would be a quote-aware scan for the closing > instead of a
post-hoc trim. It would additionally recover <parameter="a>b"> (today the bare find(">")
terminates the name early at the inner >), but it changes when the state machine can advance
during streaming — a name whose opening quote has arrived and whose closing quote has not would
stall. I chose the trim because it cannot change the streaming state machine's timing at all.
Say the word if you prefer the scan.

What this deliberately does NOT fix

I ran each of these through the real Qwen3CoderToolParserImpl on both pristine main
3607c357 and this branch, so these are observed outputs, not predictions.

• Whitespace-padded attributes are still not normalized. <parameter= "arg1" > yields the key
  " \"arg1\" " on both trees — the padding defeats the first/last-character test. Being blunt
  about the operational consequence: our downstream LiteLLM key normalisation (which strips
  whitespace, quotes and a parameter= prefix) therefore remains load-bearing on our side even
  after this lands. This PR removes one shape, not the need for the hook.
• An unmatched quote is deliberately out of scope. <parameter="command> → "command on both
  trees. An unpaired quote is not a delimiter pair, and the parser cannot distinguish a dropped
  closing quote from a name that genuinely starts with one. We do see this shape in production but
  did not retain the raw model text, so we cannot evidence its cause. There is a test pinning the
  current behaviour, so extending the rule later is a one-predicate, deliberate change rather than
  an accident.
• Typographic quotes pass through. <parameter=“arg1”> (U+201C/U+201D) yields the key
  “arg1” on both trees. The helper compares raw char values against '"' and '\'' only; it
  is ASCII-only by design and does not decode UTF-8. The header comment names both accepted
  characters explicitly, so I left it as written.
• Colliding spellings now silently de-duplicate — a real behaviour change.
  <parameter=arg1>first</parameter><parameter="arg1">second</parameter> gives
  {"arg1":"first","\"arg1\"":"second"} on main (both keys survive) and {"arg1":"first"} on
  this branch. Once both spellings normalize to the same key, the existing
  if (!currentFunctionArgsDoc.HasMember(keyVal)) guard drops the second and logs
  Parameter: {} already exists in document. First occurrence wins. I think this is the desirable
  outcome — a duplicate key was never valid — but it is a change, not a no-op, so I am flagging it
  rather than burying it.
• <function=""> now yields an empty tool name. On main it yields the two-character name
  ""; on this branch it yields an empty name — which is exactly what <function=> already does
  on main today. The patch routes one more degenerate input into an existing path; it does not
  create a new one, and it does not add error handling for it.

Tests added

src/test/llm/io_processing_utils_test.cpp (12 cases, no tokenizer and no model needed —
this target depends only on //src/llm:io_processing_utils): the helper's full truth table,
including every no-over-trim boundary listed above.

src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp (11 cases, existing fixture and
assertion idiom, no BUILD change — the target globs the directory):

• TestJustParserImplQuotedParameterName — the reported bug, unary.
• TestJustParserImplSingleQuotedParameterName
• TestJustParserImplQuotedFunctionName
• TestJustParserImplQuotedNamesKeepStringEnforcement — proves the schema lookup is restored:
  string_int_tool with quoted names yields {"arg1":"42","arg2":7}, i.e. enforceStringValue
  runs again (before the patch: {"\"arg1\"":42,"\"arg2\"":7}).
• TestJustParserImplQuotedParameterNameKeepsBooleanNormalization — True → true again
  (before the patch: {"\"arg1\"":"True"}).
• TestJustParserImplMixedQuotingOfParameterNames — quoted, unquoted and single-quoted names in
  one call.
• TestJustParserImplTwoToolCallsWithQuotedNames — ordering and per-call allocator handling.
• TestJustParserImplParameterNameWithInnerQuoteKept — no over-trim.
• TestJustParserImplParameterNameWithUnmatchedQuoteKept — pins the out-of-scope boundary.
• TestJustParserImplStreamStepWithQuotedFunctionName — getCurrentFunctionName() feeds the
  first streamed delta, so it must already be clean.
• StreamingQuotedFunctionAndParameterNames — end-to-end through OutputParser: neither the
  first delta's name nor the final delta's argument keys carry quotes.

No existing expectation changes: grep -rn 'parameter="' src/test/llm/output_parsers/ and
'function="' return no hits on main, so no current test feeds a quoted attribute.

Validation

What I actually ran, and where it stops:

• clang-format 6.0.1 (the version pinned in ci/style_requirements.txt) with -style=file
  on all five touched files — no diff.
• cpplint 1.4.3 with the exact STYLE_CHECK_OPTS from the Makefile on all five touched
  files — no warnings, exit 0.
• codespell 2.3.0 (also the pinned version) on the touched files — no new hits. To be
  precise rather than to claim "clean": it does report
  qwen3coder_output_parser_test.cpp:995: paramete ==> parameter and exits 65. That hit is
  pre-existing, not mine — it is the deliberately split "<paramete" streaming chunk, which sits
  at line 739 of the same file on pristine main and produces the identical codespell hit there.
  My patch only shifted its line number. make spell filters it via the file-level entry for
  src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp at spelling-whitelist.txt:28
  (the target pipes codespell through grep -vFf spelling-whitelist.txt), so the gate is
  unaffected.
• The real io_processing_utils unit test, compiled and run natively. The unmodified
  src/llm/io_processing/utils.cpp and the unmodified-plus-new
  src/test/llm/io_processing_utils_test.cpp built with g++ -std=c++17 against system gtest and
  upstream rapidjson: 33/33 pass (21 pre-existing FindInStringTest + 12 new
  TrimSurroundingQuotesTest).
• The real parser translation unit, compiled and driven natively, with a negative control.
  qwen3coder_tool_parser.cpp, utils.cpp, base_output_parser.cpp and status.cpp built as-is
  with g++ -std=c++17; the only substitution was a 35-line stub for
  openvino/genai/{tokenizer,generation_handle}.hpp, which Qwen3CoderToolParserImpl does not
  use. Every TestJustParserImpl* body plus the parametrized suite was lifted verbatim out of
  qwen3coder_output_parser_test.cpp (TEST_F → TEST was the only edit) and run against both
  trees:
  • pristine main 3607c357: 29/37 pass, 8 fail — exactly the eight new quoted-attribute
    tests, failing with the production shapes ({"\"arg1\"":"value1"}, {"'arg1'":"value1"},
    tool name "string_tool", {"\"arg1\"":"True"}).
  • this branch: 37/37 pass.
    So every pre-existing parser-level test still passes, and every new test is a real regression
    test rather than a vacuous one. The "does NOT fix" cases above were driven through the same
    harness on both trees.

Not verified locally, please rely on CI: I could not run
bazel test --test_filter='Qwen3CoderOutputParserTest.*' //src:ovms_test. That needs the
openvino/model_server-build container and the Qwen/Qwen3-8B tokenizer at
/ovms/src/test/llm_testing, which I do not have here. Consequently the two tests that go through
the real OutputParser — StreamingQuotedFunctionAndParameterNames and the fixture wiring of the
new TEST_Fs — are unrun; only their Qwen3CoderToolParserImpl-level equivalents were
executed. I also have not run the full //src:ovms_test, make style, make sdl-check, or any
end-to-end serving test against a real model with this build.

On the deployed artifact's template. I have now read the template shipped by the deployed
artifact repository itself, not just the stock one: OpenVINO/Qwen3.8-27B-int4-ov at main
(repo revision 1842e0d3, last modified 2026-08-21) ships a chat_template.jinja that is
byte-identical to Qwen/Qwen3.8-27B's — both sha256 c3cf9e34abf4f9e36c2d72165aa9c132d3e2a725b6c2586aaa3a8af9d7a81041 —
and its line 137 renders <parameter= unquoted. So on the evidence I have, no template on our
side instructs the quoted form. The residual uncertainty I cannot close: I read the repository's
current main, not the copy inside the IR we staged to object storage on 2026-08-17, and the
repository was modified after that date, so I cannot fully exclude revision drift between the two.
For completeness, an abliterated Qwen3.8-27B derivative exists in our model catalogue but was
not serving during the 2026-08-29 measurement — its record was only created on 2026-09-01, three
days later, and it has never been promoted past discovered — so it is not a candidate
explanation for these rows.

🧪 Checklist

• Unit tests added. 23 cases across two files; 12 of them need no tokenizer and no model.
• The documentation updated. Not applicable, so left unchecked rather than ticked: no
  user-facing behaviour, flag or API changes — the change only affects how malformed model
  output is normalized internally. Happy to add a note under docs/ if you would prefer the
  tolerated-input behaviour documented.
• Change follows security best practices. No new allocation paths, no unbounded reads, no
  parsing of untrusted input beyond what the state machine already does; the helper only ever
  shrinks a std::string and is bounds-checked for sizes below 2.

[Copilot]

🟢 Approval recommended

The functional change is small and well-covered by targeted tests; the only remaining feedback is a minor performance optimization opportunity in the new helper.

Pull request overview

This PR hardens the Qwen3Coder tool-call XML parser so that function and parameter names extracted from quoted attributes (e.g. <function="bash">, <parameter="command">) are normalized before being used as tool names / JSON argument keys and before schema-driven type handling lookups.

Changes:

• Added trimSurroundingQuotes() helper to normalize quoted XML-style attribute values.
• Applied quote trimming to both extracted function names and parameter names in the qwen3coder tool parser.
• Added focused unit and regression tests (including streaming) to prevent quoted-name regressions and to ensure schema-driven coercions still apply.

File summaries

File	Description
src/llm/io_processing/qwen3coder/qwen3coder_tool_parser.cpp	Trim surrounding quotes for extracted function/parameter names before downstream lookup/serialization.
src/llm/io_processing/utils.hpp	Declare trimSurroundingQuotes() helper with documented trimming rules.
src/llm/io_processing/utils.cpp	Implement trimSurroundingQuotes() normalization logic.
src/test/llm/io_processing_utils_test.cpp	Add unit tests covering the helper’s boundary conditions and no-over-trim behavior.
src/test/llm/output_parsers/qwen3coder_output_parser_test.cpp	Add regression tests ensuring quoted attributes don’t leak into tool names/argument keys and that schema coercion still works.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.