Skip to content

Autoparser: add optional argument reshuffle capability#20171

Merged
pwilkin merged 3 commits intoggml-org:masterfrom
pwilkin:args-shuffle
Mar 6, 2026
Merged

Autoparser: add optional argument reshuffle capability#20171
pwilkin merged 3 commits intoggml-org:masterfrom
pwilkin:args-shuffle

Conversation

@pwilkin
Copy link
Contributor

@pwilkin pwilkin commented Mar 6, 2026

Requires #18675

Some of the models that use tagged parsers (i.e. those that use quasi-XML tags for the arguments and their values) prefer a certain argument order with certain common tools. However, currently the grammar enforces the order in which the arguments are declared. If the tool tries to fill one optional argument first, it will be unable due to the grammar constraint to fill the other argument.

This manifests most often in failed calls to read_file tools which have an optional limit and offset parameter.

Fixes #20164

@github-actions github-actions bot added documentation Improvements or additions to documentation script Script related testing Everything test related examples python python script changes server jinja parser Issues related to the jinja parser labels Mar 6, 2026
visorcraft added a commit to visorcraft/llama.cpp that referenced this pull request Mar 6, 2026
all_indices[i] = i;
}
args_seq = args_seq + build_opt_choices(all_indices);
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather just do this for any number of optional arguments. Presumably a model should know not to generate a duplicate argument, but we will accept it since the JSON parser doesn't care.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean just ignore the shuffle and use the "else" branch as the universal solution?

I think the overhead for building the permutations for 4 options is really small (only 24 branches) and it does add a bit of reliability for what I feel is a large number of cases. Unless you think we should just keep it uniform with the JSON parser.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. I simply don't see the need for the added complexity of permutations. A repetition of choice is more than good enough, IMO.

Copy link
Collaborator

@aldehir aldehir Mar 6, 2026

Choose a reason for hiding this comment

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

Maybe try it out and see if there are any problems?

This problem also exists for the JSON tool calling parsers that still enforce order because of json-schema-to-grammar. However, we can explode the arguments out like the XML parsers with a repetition that has a leading comma.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, the permutations are likely completely unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine, no shuffle.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Someone's gotta reel in your genius.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You meant "chaos", right? ;)

@MikeLP
Copy link

MikeLP commented Mar 6, 2026

Idk is it right place to report issue (latest unsloth qwen3.5 35b)

{"error":{"code":400,"message":"Unable to generate parser for this template. Automatic parser generation failed: \n------------\nWhile executing CallExpression at line 145, column 28 in source:\n... {%- else %}↵        {{-
raise_exception('Unexpected message role.') }}↵    {%- ...\n                                           ^\nError: Jinja Exception: Unexpected message role.","type":"invalid_request_error"}}

Is it known issue?

@aldehir
Copy link
Collaborator

aldehir commented Mar 6, 2026

@MikeLP, seems like your client is using a developer role which is only supported by gpt-oss. I'd look into seeing if this can be turned off, i.e. use the system role instead. It's not really relevant to this PR.

@pwilkin
Copy link
Contributor Author

pwilkin commented Mar 6, 2026

@MikeLP You have to modify your template to treat developer role as system. It's a known problem with using OpenAI tools, sadly.

@MikeLP
Copy link

MikeLP commented Mar 6, 2026

@MikeLP You have to modify your template to treat developer role as system. It's a known problem with using OpenAI tools, sadly.

Thanks for quick response.

#include "json-schema-to-grammar.h"
#include "nlohmann/json.hpp"

#include <functional>
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this anymore.

@pwilkin pwilkin merged commit 2f2923f into ggml-org:master Mar 6, 2026
72 of 75 checks passed
tehsiuhuang pushed a commit to tehsiuhuang/llama.cpp that referenced this pull request Mar 7, 2026
* Allow reshuffled arguments in tagged argument parser format tool calls.

* Remove shuffle just keep the optional parsers in any order

* Remove unnecessary import
bartowski1182 pushed a commit to bartowski1182/llama.cpp that referenced this pull request Mar 10, 2026
* Allow reshuffled arguments in tagged argument parser format tool calls.

* Remove shuffle just keep the optional parsers in any order

* Remove unnecessary import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation examples jinja parser Issues related to the jinja parser python python script changes script Script related server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eval bug: Tool calling may repeatedly fail under long context when a tool has multiple optional parameters (Qwen3.5-35B, Qwen3-Coder-Next)

4 participants