Skip to content

chat: harden peg-native tool call parsing#24329

Merged
ServeurpersoCom merged 6 commits into
ggml-org:masterfrom
ServeurpersoCom:chat/harden-peg-tool-calls
Jun 15, 2026
Merged

chat: harden peg-native tool call parsing#24329
ServeurpersoCom merged 6 commits into
ggml-org:masterfrom
ServeurpersoCom:chat/harden-peg-tool-calls

Conversation

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Overview

While working we hit a silent bug on llama 3.3 dense: the assistant turn came back empty, no error shown. It only happened with tools enabled. The model emits a tool call in a variant format that the peg-native parser rejects, which blew up the whole turn. I landed a debug log first to confirm exactly what the model was emitting, then the actual fix: accept the "type": "function" variant, and fail soft on parse errors instead of tearing down the turn.

Additional information

A bug was discovered while working on this PR #23226 that allows the system to see when an error occurs; otherwise, it's silent (empty assistant turn). This PR adds the missing error if the PEG fails.

604615515-bd9a70de-baf7-44bb-99c4-9701cd714d17

Requirements

accept an optional leading type: function field in
build_json_tools_flat_keys so openai style tool calls parse on
templates whose serialization opens on the name field.

return a clean error and log the unparsed fragment on a final peg
parse failure instead of throwing the raw parser position and input.

keep the raw arguments string in func_args_not_string when it is not
valid json instead of aborting the prompt render.
@ServeurpersoCom ServeurpersoCom requested a review from a team as a code owner June 9, 2026 07:39
Comment thread common/chat.cpp Outdated
Comment thread common/chat.cpp Outdated
Comment thread common/chat-peg-parser.cpp Outdated
@pwilkin

pwilkin commented Jun 9, 2026

Copy link
Copy Markdown
Member

Yeah, I'd love to see the exact repro for this.

@ServeurpersoCom

ServeurpersoCom commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, I'd love to see the exact repro for this.

The problem look systematic with this model if tools are activated :
https://huggingface.co/unsloth/Llama-3.3-70B-Instruct-GGUF/tree/main

I will minimize the PR to a simple error message if the parser fails.

a final peg parse failure threw the raw parser position and input. log
the unparsed fragment and raise a clearer error instead, so a model
output that does not match the expected format no longer fails silently
with an empty assistant turn.

minimal change, no behavior change on successful parses.
@aldehir

aldehir commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

FWIW I think the old parsing also handled this variation. I would like to see it's unconstrained output first because we may also need to adjust the grammar triggers.

@ServeurpersoCom

ServeurpersoCom commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

FWIW I think the old parsing also handled this variation. I would like to see it's unconstrained output first because we may also need to adjust the grammar triggers.

I'll redo clean real captures from my server, but this model emits this:

get_weather:
{"type": "function", "name": "get_weather", "parameters": {"location": "Paris"}}
run_javascript:
{"type": "function", "name": "run_javascript", "parameters": {"code": "console.log(\"Salut !\");", "timeout_ms": 10000}}

EDIT, I confirm :

Clean captures, model unloaded after. Unconstrained output via /apply-template then raw /completion (no grammar, no parsing, temp 0) :

get_weather:
{"type": "function", "name": "get_weather", "parameters": {"location": "Paris"}}

run_javascript:
{"type": "function", "name": "run_javascript", "parameters": {"code": "console.log(\"Hello, world!\")", "timeout_ms": "1000"}}

two tools provided:
{"type": "function", "name": "get_weather", "parameters": {"location": "Paris"}}

It always opens with {"type": "function", ... and never the template's {"name": ..., so the lazy trigger on the name-field prefix never fires and generation stays unconstrained. The json itself is well formed, so this is a format mismatch (the type wrapper), not malformed output. Looks like the grammar triggers need to also engage on this opening.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

So, preferred direction: parse tolerance for the leading "type": "function", a trigger adjustment so the grammar constrains generation back to {"name": ...}, or both? Happy to implement whichever you prefer.

@aldehir

aldehir commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

The parsing changes are fine but we should add {"type": "function", as an additional trigger. No need to overly constrain it.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author
Sans titre

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

I did a little non-regression test by having my different family of models draw Mandelbrot in ASCII art :) The only one that was broken was this llama 3.3 now fixed

Comment thread common/chat-auto-parser-generator.cpp Outdated
Comment thread common/chat-peg-parser.cpp Outdated
Comment on lines +810 to +813
// Accept an optional leading "type": "function" field
auto type_field = optional(literal("\"type\"") + space() + literal(":") + space() +
literal("\"function\"") + space() + literal(",") + space());
auto ordered_body = tool_open(literal("{")) + space() + type_field;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Let's just finish up by wrapping this parsing logic in the new flag you made from the analysis.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Both the grammar trigger and the flat-keys parsing leniency are now gated on openai_wrapper_trigger, set only by the analysis workaround that matches Meta's JSON tool instruction. Other JSON_NATIVE templates no longer get the "type": "function" handling.

Thread accept_openai_wrapper from the generator to build_json_tools_flat_keys
so the leading "type": "function" field is accepted only when openai_wrapper_trigger is set.
@aldehir

aldehir commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@pwilkin

@pwilkin pwilkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Innocent enough :) would prob be better to have this in the main analysis route instead of a workaround (the criterion could be the same), but not a big deal.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Innocent enough :) would prob be better to have this in the main analysis route instead of a workaround (the criterion could be the same), but not a big deal.

Besides, it's an older model, not very efficient for toolcalling, but it's cleaner than a blank response.

@ServeurpersoCom ServeurpersoCom merged commit 581e8ec into ggml-org:master Jun 15, 2026
24 of 25 checks passed
am17an pushed a commit to am17an/llama.cpp that referenced this pull request Jun 20, 2026
* chat: harden peg-native tool call parsing

accept an optional leading type: function field in
build_json_tools_flat_keys so openai style tool calls parse on
templates whose serialization opens on the name field.

return a clean error and log the unparsed fragment on a final peg
parse failure instead of throwing the raw parser position and input.

keep the raw arguments string in func_args_not_string when it is not
valid json instead of aborting the prompt render.

* chat: surface peg-native parse failures

a final peg parse failure threw the raw parser position and input. log
the unparsed fragment and raise a clearer error instead, so a model
output that does not match the expected format no longer fails silently
with an empty assistant turn.

minimal change, no behavior change on successful parses.

* chat: handle openai style tool calls in peg-native

* nits

* common: scope OpenAI wrapper grammar trigger via autoparser flag

* chat: gate type:function parsing leniency on the analysis flag

Thread accept_openai_wrapper from the generator to build_json_tools_flat_keys
so the leading "type": "function" field is accepted only when openai_wrapper_trigger is set.
papamoose pushed a commit to papamoose/llama.cpp that referenced this pull request Jun 27, 2026
* chat: harden peg-native tool call parsing

accept an optional leading type: function field in
build_json_tools_flat_keys so openai style tool calls parse on
templates whose serialization opens on the name field.

return a clean error and log the unparsed fragment on a final peg
parse failure instead of throwing the raw parser position and input.

keep the raw arguments string in func_args_not_string when it is not
valid json instead of aborting the prompt render.

* chat: surface peg-native parse failures

a final peg parse failure threw the raw parser position and input. log
the unparsed fragment and raise a clearer error instead, so a model
output that does not match the expected format no longer fails silently
with an empty assistant turn.

minimal change, no behavior change on successful parses.

* chat: handle openai style tool calls in peg-native

* nits

* common: scope OpenAI wrapper grammar trigger via autoparser flag

* chat: gate type:function parsing leniency on the analysis flag

Thread accept_openai_wrapper from the generator to build_json_tools_flat_keys
so the leading "type": "function" field is accepted only when openai_wrapper_trigger is set.
adrianhoehne pushed a commit to adrianhoehne/llama.cpp that referenced this pull request Jul 5, 2026
* chat: harden peg-native tool call parsing

accept an optional leading type: function field in
build_json_tools_flat_keys so openai style tool calls parse on
templates whose serialization opens on the name field.

return a clean error and log the unparsed fragment on a final peg
parse failure instead of throwing the raw parser position and input.

keep the raw arguments string in func_args_not_string when it is not
valid json instead of aborting the prompt render.

* chat: surface peg-native parse failures

a final peg parse failure threw the raw parser position and input. log
the unparsed fragment and raise a clearer error instead, so a model
output that does not match the expected format no longer fails silently
with an empty assistant turn.

minimal change, no behavior change on successful parses.

* chat: handle openai style tool calls in peg-native

* nits

* common: scope OpenAI wrapper grammar trigger via autoparser flag

* chat: gate type:function parsing leniency on the analysis flag

Thread accept_openai_wrapper from the generator to build_json_tools_flat_keys
so the leading "type": "function" field is accepted only when openai_wrapper_trigger is set.
adromir pushed a commit to adromir/llama-cpp-turboquant that referenced this pull request Jul 8, 2026
* chat: harden peg-native tool call parsing

accept an optional leading type: function field in
build_json_tools_flat_keys so openai style tool calls parse on
templates whose serialization opens on the name field.

return a clean error and log the unparsed fragment on a final peg
parse failure instead of throwing the raw parser position and input.

keep the raw arguments string in func_args_not_string when it is not
valid json instead of aborting the prompt render.

* chat: surface peg-native parse failures

a final peg parse failure threw the raw parser position and input. log
the unparsed fragment and raise a clearer error instead, so a model
output that does not match the expected format no longer fails silently
with an empty assistant turn.

minimal change, no behavior change on successful parses.

* chat: handle openai style tool calls in peg-native

* nits

* common: scope OpenAI wrapper grammar trigger via autoparser flag

* chat: gate type:function parsing leniency on the analysis flag

Thread accept_openai_wrapper from the generator to build_json_tools_flat_keys
so the leading "type": "function" field is accepted only when openai_wrapper_trigger is set.

(cherry picked from commit 581e8ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants