chat: fix whitespace problems once and for all#24624
Conversation
pwilkin
requested review from
CISC and
aldehir
and removed request for
a team and
ggerganov
| common_peg_parser common_peg_parser_builder::double_quoted_string() { | ||
| return rule("double-quoted-string", [this]() { | ||
| return sequence({literal("\""), string_content('"'), literal("\""), space()}); | ||
| return sequence({literal("\""), string_content('"'), literal("\"")}); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Need to make sure the json-to-grammar rules align with this, which is the only reason I had the space in there.
There was a problem hiding this comment.
Yeah, but the key point is, this is just resolving ambiguity. Since both the array and object parsers already have enclosing whitespaces, this leaves just straight out constants. And having a constant have a space at the end was producing a parsing ambiguity that actually caused an error with <number><newline>, because the newline got consumed as part of the number and wasn't available to be consumed as part of the marker (and I stopped stripping the newlines from markers so it wasn't valid).
I first wanted to go with a new parser space_no_newline(), but realized that I can just remove the extra space() from there and the result would be the same. The only place this potentially mattered was the json-schema-to-grammar for pure json-schema output, which had wrapping spaces that I had to add directly to the parser.
|
Oh my. Ok, save off the diff and let's work the JSON schema in a separate PR. |
This reverts commit b0827ec.
|
Aight, reverted. |
|
@ggml-org/maintainers need 1 more approval please |
pwilkin
added
the
merge ready
|
This reproducibly breaks (some) tool calls when used from maki with qwen3.6 27B (didn't test others yet). Reverting just this commit makes it work again. Specifically read tool calls were working fine but not write, which might be because the content was a C source file with lots of indentation and other whitespace. I can create an issue with more info if needed and also include what goes over the OpenAI API |
|
@sdroege yes, please do so - best option would be to also include the newly available "save prompts to directory" server option and provide the prompts. |
* chat: fix whitespace problems once and for all * Purge trailing spaces from grammar generation * Revert "Purge trailing spaces from grammar generation" This reverts commit b0827ec.
* chat: fix whitespace problems once and for all * Purge trailing spaces from grammar generation * Revert "Purge trailing spaces from grammar generation" This reverts commit b0827ec.
* chat: fix whitespace problems once and for all * Purge trailing spaces from grammar generation * Revert "Purge trailing spaces from grammar generation" This reverts commit 76efdaabb1fa6fda939646f1fca5358c34583a43.
4 participants
Overview
I finally got around to checking the problems with whitespace parsing and I think this fixes most issues.
Additional information
Removed spaces from all JSON parsers, removed space trimming from mapper (which was causing the error described in #23602 ).
Requirements