Skip to content

llama: fix error on bad grammar#12628

Merged
JohannesGaessler merged 1 commit intoggml-org:masterfrom
JohannesGaessler:server-grammar-fix
Mar 28, 2025
Merged

llama: fix error on bad grammar#12628
JohannesGaessler merged 1 commit intoggml-org:masterfrom
JohannesGaessler:server-grammar-fix

Conversation

@JohannesGaessler
Copy link
Collaborator

On master passing a bad grammar to either llama-cli or the server currently does not result in an error, there is only a print indicating that the grammar could not be parsed and the grammar is otherwise ignored. However, I don't think that this is the intended behavior. The code checks if the return value of common_sampler_init is null, it's just that the return value is not null when passing a bad grammar. This PR makes it so that llama_sampler_init_grammar_impl returns null when grammar-str is not empty but could not be passed, this is then propagated upwards to return null with common_sampler_init. When passing a bad grammar to the server the server now returns an error, llama-cli terminates with an error message.

I used the following commands for testing:

export model_name=llama_3-8b && export quantization=q4_0
./build/bin/llama-cli --model models/opt/${model_name}-${quantization}.gguf -ngl 99 --n-predict 1 --prompt "1 2 3 4 " --grammar "root ::= \"6\""
./build/bin/llama-cli --model models/opt/${model_name}-${quantization}.gguf -ngl 99 --n-predict 1 --prompt "1 2 3 4 " --grammar "asdf"
./build/bin/llama-server --model models/opt/${model_name}-${quantization}.gguf --n-gpu-layers 99
curl --request POST --url http://localhost:8080/completion --data '{"prompt": "1 2 3 4 ", "n_predict": 1, "grammar": "root ::= \"6\""}' | python3 -m json.tool
curl --request POST --url http://localhost:8080/completion --data '{"prompt": "1 2 3 4 ", "n_predict": 1, "grammar": "asdf"}' | python3 -m json.tool

@JohannesGaessler JohannesGaessler merged commit dd373dd into ggml-org:master Mar 28, 2025
51 of 52 checks passed
SamuelOliveirads pushed a commit to SamuelOliveirads/llama.cpp that referenced this pull request Dec 29, 2025
* grammar : fix JSON Schema for string regex with top-level alt. (ggml-org#9903)

Prior to this commit, using a JSON Schema containing a string
with `pattern` regular expression that uses top-level alternation
(e.g. `"pattern": "^A|B|C|D$"`) would result in invalid JSON
output from the constrained sampling grammar, because it
ended up creating a grammar rule like this for the string:

```
thing ::= "\"" "A" | "B" | "C" | "D" "\"" space
```

Note that this rule will only match a starting quote for the "A" case,
and will only match an ending quote for the "D" case,
so this rule will always produce invalid JSON when used for sampling
(that is, the JSON will always be lacking the starting quote,
the ending quote, or both).

This was fixed in a simple way by adding parentheses to the
generated rule (for all string pattern rules, to keep it simple),
such that the new generated rule looks like this (correct):

```
thing ::= "\"" ("A" | "B" | "C" | "D") "\"" space
```

* grammars : add English-only grammar (ggml-org#10612)

* grammar : handle maxItems == 0 in JSON schema (ggml-org#13117)

Co-authored-by: Richard Lyons <frob@cloudstaff.com>

* grammar-parser : fix possible null-deref (ggml-org#9004)

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=70680

Signed-off-by: David Korczynski <david@adalogics.com>

* llama : fix typo in llama-grammar.h [no ci] (ggml-org#11816)

* * server: fix "--grammar-file" parameter (ggml-org#12285)

* common : use std::string_view now that we target c++17 (ggml-org#14319)

* json : support `enum` values within `allOf` (ggml-org#15830)

* grammar : use int64_t to avoid int overflows in int schema to grammar conversion logic (ggml-org#16626)

* grammar : support array references in json schema (ggml-org#16792)

* grammar : support array references in json schema

* Update json-schema-to-grammar.cpp

Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@scala.com>

* grammar : improve regex when naming ref derived rules

* grammar : replace non-conformant definitions array with anyOf test case

---------

Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@scala.com>
# Conflicts:
#	tests/test-json-schema-to-grammar.cpp

* merge fix

* llama : minor grammar refactor (ggml-org#10897)

* llama: fix error on bad grammar (ggml-org#12628)

* grammar : fix integer overflow (ggml-org#17381)

* Fix DoS / integer overflow

* Remove optional, use INT64_MAX instead as placeholder value (it's technically -1, so it fits :)

* White space

* Actually, since it's unsigned, use UINT64_MAX
# Conflicts:
#	src/llama-grammar.cpp

* grammar: fix regression caused by ggml-org#17381 (ggml-org#17412)

* grammar: fix regression caused by ggml-org#17381

* more readable
# Conflicts:
#	src/llama-grammar.cpp

* Merge Fix

* Fix warnings

---------

Signed-off-by: David Korczynski <david@adalogics.com>
Co-authored-by: Joe Eli McIlvain <joe.eli.mac@gmail.com>
Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Co-authored-by: frob <rick+github@frob.com.au>
Co-authored-by: Richard Lyons <frob@cloudstaff.com>
Co-authored-by: DavidKorczynski <david@adalogics.com>
Co-authored-by: Daniel Bevenius <daniel.bevenius@gmail.com>
Co-authored-by: firecoperana <firecoperana>
Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@scala.com>
Co-authored-by: Aldehir Rojas <hello@alde.dev>
Co-authored-by: Olivier Chafik <olivier.chafik@gmail.com>
Co-authored-by: Piotr Wilkin (ilintar) <piotr.wilkin@syndatis.com>
Co-authored-by: Xuan-Son Nguyen <son@huggingface.co>
Co-authored-by: Iwan Kawrakow <iwan.kawrakow@gmail.com>
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.

2 participants