Skip to content

fix(chat): accept </parameter> without the surrounding newlines - #232

Open
dongtran16092006 wants to merge 4 commits into
PrismML-Eng:prismfrom
dongtran16092006:fix/parameter-close-newlines
Open

dongtran16092006 wants to merge 4 commits into
PrismML-Eng:prismfrom
dongtran16092006:fix/parameter-close-newlines

Conversation

@dongtran16092006

Copy link
Copy Markdown

What happens

A tool call with more than one argument loses every argument after the first.

Reproduced with Qwen3.6-27B-UD-IQ2_XXS calling a 2-argument MCP tool
(entity_type: string, filters: array). What came back:

{
  "entity_type": "Note</parameter>\n<parameter=filters>\n[[\"updated_at\", \"between\", [\"2026-09-21T00:00:00Z\", \"2026-09-21T23:59:59Z\"]]]",
  "filters": null
}

The model's output was well-formed — the array is right there, intact. It ended up
inside entity_type because the parser never closed that argument.

Why

common/chat.cpp, the Qwen3.6 XML tool-call grammar:

auto arg_close  = p.tool_arg_close(p.literal("\n</parameter>\n"));
auto arg_string = p.rule("xml-arg-string",
    p.ac(p.tool_arg_string_value(p.until("\n</parameter>\n")) + arg_close, "\n</parameter>\n"));

The close delimiter is the exact literal "\n</parameter>\n". The template does ask for
that framing, and the model usually produces it — but not always. When it emits
Note</parameter> with no newline before the tag, until("\n</parameter>\n") does not
match at that position, so the scan continues to the next fully-framed occurrence and
swallows the <parameter=...> blocks in between. The first argument absorbs the rest of
the call and every later argument parses as null.

This is a partial failure, not a hard one: the call still returns, with silently wrong
arguments. In an agent loop the tool then errors, the model retries, emits the same
almost-right markup, and the loop does not terminate.

The change

Accept the looser spellings of the close tag as well, ordered longest first so the fully
framed form still wins wherever the model does produce it:

const std::vector<std::string> arg_close_delims = {
    "\n</parameter>\n", "\n</parameter>", "</parameter>\n", "</parameter>",
};
auto arg_close  = p.tool_arg_close(p.choice({ /* same four, as literals */ }));
auto arg_string = p.rule("xml-arg-string",
    p.ac(p.tool_arg_string_value(p.until_one_of(arg_close_delims)) + arg_close, arg_close_delims));

This matches what upstream already does for the </param> variant of the same grammar,
which closes on the bare tag with no newline requirement.

Effect

Same prompt, same model, same sampling, before and after:

entity_type filters
before "Note</parameter>\n<parameter=filters>\n[[...]]" null
after "Note" parsed as its own argument

Scope

Only the close delimiter is loosened. The open tag, the argument permutation and the JSON
branch are untouched. A value containing the literal text </parameter> would now close
early — that was already true for the framed form, and it is not a shape any JSON or
string argument produces on its own.

Not fixed here, and worth stating plainly: with the parser corrected, this model still
tends to write an object where the schema asks for a positional array
({"updated_at": "between", ...} instead of ["updated_at", "between", ...]). That is a
separate problem on the model side, not a parsing one.

The Qwen3.6 XML tool-call grammar closed an argument on the exact literal
"\n</parameter>\n". Models drop one of those newlines often enough to matter:
a value emitted as `Note</parameter>` never matches, so the scan runs on to the
next closing tag and swallows the intervening `<parameter=...>` blocks into the
first argument, leaving every later argument null.

Observed with Qwen3.6-27B calling a 2-argument tool: entity_type came back as
"Note</parameter>\n<parameter=filters>\n[[...]]" and filters as null.

Accept the looser spellings as well, longest first so the fully framed form
still wins where the model does emit it.
The previous commit covered a dropped newline. The same swallow happens whenever the
close tag is wrong in any other way — models also emit a tag named after the value,
`Project</Project>`, which matches none of the accepted spellings, so the scan runs on
and absorbs the following `<parameter=...>` blocks into the current argument.

Stop the value at the first of: any spelling of the close tag, which is consumed; or
`<parameter=` / `</function>`, which are not. Neither can occur inside a value, so
meeting one means the argument ended however the model chose to close it. The close tag
becomes optional for the same reason — by that point the argument is already delimited.

Observed on Qwen3.6-27B: entity_type came back as
"Project</Project>\n<parameter=filters>\n[[...]]" and is now "Project" with filters
parsed as its own argument.
imaami pushed a commit to imaami/llama.cpp that referenced this pull request Oct 13, 2026
Keep closing tags mandatory while accepting either framing newline independently. Check two adjacent string arguments with all four closing forms.

Source: PrismML-Eng#232
Original-commit: 41c32fb
Original-commit: 4115cea
Assisted-by: Codex (parser adaptation and regression tests)

Ported and rebased by Codex.
imaami pushed a commit to imaami/llama.cpp that referenced this pull request Oct 13, 2026
Keep closing tags mandatory while accepting either framing newline independently. Check two adjacent string arguments with all four closing forms.

Source: PrismML-Eng#232
Original-commit: 41c32fb
Original-commit: 4115cea
Assisted-by: Codex (parser adaptation and regression tests)

Ported and rebased by Codex.
@bri-prism

Copy link
Copy Markdown
Collaborator

With this change test-chat fails on prism. The generated grammar for the Qwen3.5 template no longer accepts the template's own canonical tool call: after \n</parameter> it expects < and rejects the \n before </function>. The ac rule ends at the first delimiter, so \n</parameter>\n becomes unreachable under grammar. Could you keep the canonical close reachable in the grammar (or apply the tolerant matching only in the parser), and add a test with the two-argument call from the description?

…er is tolerant

The previous commit added all four closing-tag variants to the grammar as well.
The grammar reads one character at a time and ends the parameter as soon as
any tag fully matches. `</parameter>` always fully matches before
`\n</parameter>\n` does (the long form still needs its final `\n`), so the
grammar ends the parameter too early. It then expects the `<` of the next tag,
but the template writes `\n` there. As a result, the grammar rejects the
template's own standard output, and at inference time the model is forced to
drop the newline in every parameter.

Fix: the grammar keeps only the standard closing tag, and the four closing-tag
variants are used by the parser only.
@dongtran16092006

Copy link
Copy Markdown
Author

With this change test-chat fails on prism. The generated grammar for the Qwen3.5 template no longer accepts the template's own canonical tool call: after \n</parameter> it expects < and rejects the \n before </function>. The ac rule ends at the first delimiter, so \n</parameter>\n becomes unreachable under grammar. Could you keep the canonical close reachable in the grammar (or apply the tolerant matching only in the parser), and add a test with the two-argument call from the description?

Fixed in 0545b92: the grammar keeps the canonical close tag, and only the parser is tolerant. Added a test with the two-argument call from the description; test-chat passes.

imaami pushed a commit to imaami/llama.cpp that referenced this pull request Nov 2, 2026
Keep closing tags mandatory while accepting either framing newline independently. Check two adjacent string arguments with all four closing forms.

Source: PrismML-Eng#232
Original-commit: 41c32fb
Original-commit: 4115cea
Assisted-by: Codex (parser adaptation and regression tests)

Ported and rebased by Codex.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants