Skip to content

Commit 139cc62

Browse files
authored
json: restore default additionalProperties to false, fix some pattern escapes (ggml-org#8180)
* json: expand ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS charset * json: revert default of additionalProperties to false * Update README.md
1 parent e57dc62 commit 139cc62

File tree

6 files changed

+73
-48
lines changed

6 files changed

+73
-48
lines changed

common/json-schema-to-grammar.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ std::unordered_map<char, std::string> GRAMMAR_LITERAL_ESCAPES = {
316316
};
317317

318318
std::unordered_set<char> NON_LITERAL_SET = {'|', '.', '(', ')', '[', ']', '{', '}', '*', '+', '?'};
319-
std::unordered_set<char> ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = {'[', ']', '(', ')', '|', '{', '}', '*', '+', '?'};
319+
std::unordered_set<char> ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = {'^', '$', '.', '[', ']', '(', ')', '|', '{', '}', '*', '+', '?'};
320320

321321
template <typename Iterator>
322322
std::string join(Iterator begin, Iterator end, const std::string & separator) {
@@ -720,7 +720,7 @@ class SchemaConverter {
720720
}
721721
prop_names.push_back(prop_name);
722722
}
723-
if (!(additional_properties.is_boolean() && !additional_properties.get<bool>())) {
723+
if ((additional_properties.is_boolean() && additional_properties.get<bool>()) || additional_properties.is_object()) {
724724
std::string sub_name = name + (name.empty() ? "" : "-") + "additional";
725725
std::string value_rule =
726726
additional_properties.is_object() ? visit(additional_properties, sub_name + "-value")

examples/json_schema_to_grammar.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ def __init__(self, content: str, deps: list = None):
231231
GRAMMAR_LITERAL_ESCAPES = {'\r': '\\r', '\n': '\\n', '"': '\\"', '-': '\\-', ']': '\\]'}
232232

233233
NON_LITERAL_SET = set('|.()[]{}*+?')
234-
ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = set('[]()|{}*+?')
234+
ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = set('^$.[]()|{}*+?')
235235

236236

237237
class SchemaConverter:
@@ -602,7 +602,7 @@ def add_component(comp_schema, is_required):
602602
else:
603603
add_component(t, is_required=True)
604604

605-
return self._add_rule(rule_name, self._build_object_rule(properties, required, hybrid_name, additional_properties=[]))
605+
return self._add_rule(rule_name, self._build_object_rule(properties, required, hybrid_name, additional_properties=None))
606606

607607
elif schema_type in (None, 'array') and ('items' in schema or 'prefixItems' in schema):
608608
items = schema.get('items') or schema['prefixItems']
@@ -691,7 +691,7 @@ def _build_object_rule(self, properties: List[Tuple[str, Any]], required: Set[st
691691
required_props = [k for k in sorted_props if k in required]
692692
optional_props = [k for k in sorted_props if k not in required]
693693

694-
if additional_properties != False:
694+
if additional_properties is not None and additional_properties != False:
695695
sub_name = f'{name}{"-" if name else ""}additional'
696696
value_rule = self.visit(additional_properties, f'{sub_name}-value') if isinstance(additional_properties, dict) else \
697697
self._add_primitive('value', PRIMITIVE_RULES['value'])

examples/server/public/json-schema-to-grammar.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ const GRAMMAR_RANGE_LITERAL_ESCAPE_RE = /[\n\r"\]\-\\]/g;
259259
const GRAMMAR_LITERAL_ESCAPES = { '\r': '\\r', '\n': '\\n', '"': '\\"', '-': '\\-', ']': '\\]' };
260260

261261
const NON_LITERAL_SET = new Set('|.()[]{}*+?');
262-
const ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = new Set('[]()|{}*+?');
262+
const ESCAPED_IN_REGEXPS_BUT_NOT_IN_LITERALS = new Set('^$.[]()|{}*+?');
263263

264264
export class SchemaConverter {
265265
constructor(options) {
@@ -751,7 +751,7 @@ export class SchemaConverter {
751751
const requiredProps = sortedProps.filter(k => required.has(k));
752752
const optionalProps = sortedProps.filter(k => !required.has(k));
753753

754-
if (additionalProperties !== false) {
754+
if (additionalProperties) {
755755
const subName = `${name ?? ''}${name ? '-' : ''}additional`;
756756
const valueRule =
757757
additionalProperties != null && typeof additionalProperties === 'object' ? this.visit(additionalProperties, `${subName}-value`)

grammars/README.md

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ space ::= | " " | "\n" [ \t]{0,20}
182182

183183
Here is also a list of known limitations (contributions welcome):
184184

185+
- `additionalProperties` defaults to `false` (produces faster grammars + reduces hallucinations).
186+
- `"additionalProperties": true` may produce keys that contain unescaped newlines.
185187
- Unsupported features are skipped silently. It is currently advised to use the command-line Python converter (see above) to see any warnings, and to inspect the resulting grammar / test it w/ [llama-gbnf-validator](../examples/gbnf-validator/gbnf-validator.cpp).
186188
- Can't mix `properties` w/ `anyOf` / `oneOf` in the same type (https://github.com/ggerganov/llama.cpp/issues/7703)
187189
- [prefixItems](https://json-schema.org/draft/2020-12/json-schema-core#name-prefixitems) is broken (but [items](https://json-schema.org/draft/2020-12/json-schema-core#name-items) works)
@@ -203,10 +205,11 @@ And a non-exhaustive list of other unsupported features that are unlikely to be
203205
### A word about additionalProperties
204206

205207
> [!WARNING]
206-
> By default, `object`s accept [additional properties](https://json-schema.org/understanding-json-schema/reference/object#additionalproperties), which you might not want / not expect, and which will make sampling slower (not just because of the extra tokens, but also generates a slower grammar).
207-
> You can set `"additionalProperties": false` on the schema of any object to ensure only properties listed in `properties` are generated (not needed for non-`object` types, e.g. `array` or `string`).
208+
> The JSON schemas spec states `object`s accept [additional properties](https://json-schema.org/understanding-json-schema/reference/object#additionalproperties) by default.
209+
> Since this is slow and seems prone to hallucinations, we default to no additional properties.
210+
> You can set `"additionalProperties": true` in the the schema of any object to explicitly allow additional properties.
208211
209-
If you're using [Pydantic](https://pydantic.dev/) to generate schemas, you can disable additional properties with the `extra` config on each model class:
212+
If you're using [Pydantic](https://pydantic.dev/) to generate schemas, you can enable additional properties with the `extra` config on each model class:
210213

211214
```python
212215
# pip install pydantic
@@ -215,14 +218,14 @@ from typing import Annotated, List
215218
from pydantic import BaseModel, Extra, Field
216219
class QAPair(BaseModel):
217220
class Config:
218-
extra = 'forbid' # triggers additionalProperties: false in the JSON schema
221+
extra = 'allow' # triggers additionalProperties: true in the JSON schema
219222
question: str
220223
concise_answer: str
221224
justification: str
222225

223226
class Summary(BaseModel):
224227
class Config:
225-
extra = 'forbid'
228+
extra = 'allow'
226229
key_facts: List[Annotated[str, Field(pattern='- .{5,}')]]
227230
question_answers: List[Annotated[List[QAPair], Field(min_items=5)]]
228231

@@ -236,7 +239,7 @@ print(json.dumps(Summary.model_json_schema(), indent=2))
236239
{
237240
"$defs": {
238241
"QAPair": {
239-
"additionalProperties": false,
242+
"additionalProperties": true,
240243
"properties": {
241244
"question": {
242245
"title": "Question",
@@ -260,7 +263,7 @@ print(json.dumps(Summary.model_json_schema(), indent=2))
260263
"type": "object"
261264
}
262265
},
263-
"additionalProperties": false,
266+
"additionalProperties": true,
264267
"properties": {
265268
"key_facts": {
266269
"items": {
@@ -292,30 +295,40 @@ print(json.dumps(Summary.model_json_schema(), indent=2))
292295
```
293296

294297
```
295-
QAPair ::= "{" space QAPair-question-kv "," space QAPair-concise-answer-kv "," space QAPair-justification-kv "}" space
298+
QAPair ::= "{" space QAPair-question-kv "," space QAPair-concise-answer-kv "," space QAPair-justification-kv ( "," space ( QAPair-additional-kv ( "," space QAPair-additional-kv )* ) )? "}" space
299+
QAPair-additional-k ::= ["] ( [c] ([o] ([n] ([c] ([i] ([s] ([e] ([_] ([a] ([n] ([s] ([w] ([e] ([r] char+ | [^"r] char*) | [^"e] char*) | [^"w] char*) | [^"s] char*) | [^"n] char*) | [^"a] char*) | [^"_] char*) | [^"e] char*) | [^"s] char*) | [^"i] char*) | [^"c] char*) | [^"n] char*) | [^"o] char*) | [j] ([u] ([s] ([t] ([i] ([f] ([i] ([c] ([a] ([t] ([i] ([o] ([n] char+ | [^"n] char*) | [^"o] char*) | [^"i] char*) | [^"t] char*) | [^"a] char*) | [^"c] char*) | [^"i] char*) | [^"f] char*) | [^"i] char*) | [^"t] char*) | [^"s] char*) | [^"u] char*) | [q] ([u] ([e] ([s] ([t] ([i] ([o] ([n] char+ | [^"n] char*) | [^"o] char*) | [^"i] char*) | [^"t] char*) | [^"s] char*) | [^"e] char*) | [^"u] char*) | [^"cjq] char* )? ["] space
300+
QAPair-additional-kv ::= QAPair-additional-k ":" space value
296301
QAPair-concise-answer-kv ::= "\"concise_answer\"" space ":" space string
297302
QAPair-justification-kv ::= "\"justification\"" space ":" space string
298303
QAPair-question-kv ::= "\"question\"" space ":" space string
304+
additional-k ::= ["] ( [k] ([e] ([y] ([_] ([f] ([a] ([c] ([t] ([s] char+ | [^"s] char*) | [^"t] char*) | [^"c] char*) | [^"a] char*) | [^"f] char*) | [^"_] char*) | [^"y] char*) | [^"e] char*) | [q] ([u] ([e] ([s] ([t] ([i] ([o] ([n] ([_] ([a] ([n] ([s] ([w] ([e] ([r] ([s] char+ | [^"s] char*) | [^"r] char*) | [^"e] char*) | [^"w] char*) | [^"s] char*) | [^"n] char*) | [^"a] char*) | [^"_] char*) | [^"n] char*) | [^"o] char*) | [^"i] char*) | [^"t] char*) | [^"s] char*) | [^"e] char*) | [^"u] char*) | [^"kq] char* )? ["] space
305+
additional-kv ::= additional-k ":" space value
306+
array ::= "[" space ( value ("," space value)* )? "]" space
307+
boolean ::= ("true" | "false") space
299308
char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
309+
decimal-part ::= [0-9]{1,16}
300310
dot ::= [^\x0A\x0D]
311+
integral-part ::= [0] | [1-9] [0-9]{0,15}
301312
key-facts ::= "[" space (key-facts-item ("," space key-facts-item)*)? "]" space
302313
key-facts-item ::= "\"" "- " key-facts-item-1{5,} "\"" space
303314
key-facts-item-1 ::= dot
304315
key-facts-kv ::= "\"key_facts\"" space ":" space key-facts
316+
null ::= "null" space
317+
number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
318+
object ::= "{" space ( string ":" space value ("," space string ":" space value)* )? "}" space
305319
question-answers ::= "[" space (question-answers-item ("," space question-answers-item)*)? "]" space
306320
question-answers-item ::= "[" space question-answers-item-item ("," space question-answers-item-item){4,} "]" space
307321
question-answers-item-item ::= QAPair
308322
question-answers-kv ::= "\"question_answers\"" space ":" space question-answers
309-
root ::= "{" space key-facts-kv "," space question-answers-kv "}" space
323+
root ::= "{" space key-facts-kv "," space question-answers-kv ( "," space ( additional-kv ( "," space additional-kv )* ) )? "}" space
310324
space ::= | " " | "\n" [ \t]{0,20}
311325
string ::= "\"" char* "\"" space
326+
value ::= object | array | string | number | boolean | null
312327
```
313328

314329
</details>
315330

316-
If you're using [Zod](https://zod.dev/), you can make your objects explicitly strict w/ `z.object(...).strict()` or `z.strictObject(...)`.
317-
318-
Note however that [zod-to-json-schema](https://github.com/StefanTerdell/zod-to-json-schema) currently always seems to set `"additionalProperties": false` anyway (even w/ zod schemas on which `nonstrict()` / `passthrough()` was called).
331+
If you're using [Zod](https://zod.dev/), you can make your objects to explicitly allow extra properties w/ `nonstrict()` / `passthrough()` (or explicitly no extra props w/ `z.object(...).strict()` or `z.strictObject(...)`) but note that [zod-to-json-schema](https://github.com/StefanTerdell/zod-to-json-schema) currently always sets `"additionalProperties": false` anyway.
319332

320333
```js
321334
import { z } from 'zod';

tests/test-grammar-integration.cpp

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,40 @@ static void test_json_schema() {
993993
}
994994
);
995995

996+
test_schema(
997+
"simple pattern",
998+
// Schema
999+
R"""({
1000+
"pattern": "^[a-zA-Z0-9_-]*$"
1001+
})""",
1002+
// Passing strings
1003+
{
1004+
R"""("")""",
1005+
R"""("He_llo-12")""",
1006+
},
1007+
// Failing strings
1008+
{
1009+
R"""("!")""",
1010+
R"""("Hello World")""",
1011+
}
1012+
);
1013+
1014+
test_schema(
1015+
"pattern with escapes",
1016+
// Schema
1017+
R"""({
1018+
"pattern": "^a\\^\\$\\.\\[\\]\\(\\)\\|\\{\\}\\*\\+\\?b$"
1019+
})""",
1020+
// Passing strings
1021+
{
1022+
R"""("a^$.[]()|{}*+?b")""",
1023+
},
1024+
// Failing strings
1025+
{
1026+
R"""("ab")""",
1027+
}
1028+
);
1029+
9961030
test_schema(
9971031
"",
9981032
// Schema
@@ -1062,8 +1096,6 @@ static void test_json_schema() {
10621096
R"""({ "number": 1600, "street_name": "Pennsylvania" })""",
10631097
// "By extension, even an empty object is valid"
10641098
R"""({})""",
1065-
// "By default, providing additional properties is valid"
1066-
R"""({ "number": 1600, "street_name": "Pennsylvania", "street_type":"Avenue", "direction":"NW"})""",
10671099
R"""({ "number": 1600, "street_name": "Pennsylvania", "street_type": "Avenue" })""",
10681100
},
10691101
// Failing strings
@@ -1074,6 +1106,9 @@ static void test_json_schema() {
10741106
R"""({ "street_name": "Pennsylvania", "number": 1600 })""",
10751107
// Reorder properties
10761108
R"""({ "number": "1600", "street_name": "Pennsylvania", "street_type":"Avenue"})""",
1109+
// "Additional properties default to false for generation, even though the spec says true.
1110+
R"""({ "number": 1600, "street_name": "Pennsylvania", "street_type":"Avenue", "direction":"NW"})""",
1111+
10771112
}
10781113
);
10791114

tests/test-json-schema-to-grammar.cpp

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,28 +1120,15 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
11201120
R"""(
11211121
alternative-0 ::= foo
11221122
alternative-1 ::= bar
1123-
array ::= "[" space ( value ("," space value)* )? "]" space
1124-
bar ::= "{" space (bar-b-kv bar-b-rest | bar-additional-kv ( "," space bar-additional-kv )* )? "}" space
1125-
bar-additional-k ::= ["] ( [b] char+ | [^"b] char* )? ["] space
1126-
bar-additional-kv ::= bar-additional-k ":" space value
1123+
bar ::= "{" space (bar-b-kv )? "}" space
11271124
bar-b-kv ::= "\"b\"" space ":" space number
1128-
bar-b-rest ::= ( "," space bar-additional-kv )*
1129-
boolean ::= ("true" | "false") space
1130-
char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
11311125
decimal-part ::= [0-9]{1,16}
1132-
foo ::= "{" space (foo-a-kv foo-a-rest | foo-additional-kv ( "," space foo-additional-kv )* )? "}" space
1126+
foo ::= "{" space (foo-a-kv )? "}" space
11331127
foo-a-kv ::= "\"a\"" space ":" space number
1134-
foo-a-rest ::= ( "," space foo-additional-kv )*
1135-
foo-additional-k ::= ["] ( [a] char+ | [^"a] char* )? ["] space
1136-
foo-additional-kv ::= foo-additional-k ":" space value
11371128
integral-part ::= [0] | [1-9] [0-9]{0,15}
1138-
null ::= "null" space
11391129
number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
1140-
object ::= "{" space ( string ":" space value ("," space string ":" space value)* )? "}" space
11411130
root ::= alternative-0 | alternative-1
11421131
space ::= | " " | "\n" [ \t]{0,20}
1143-
string ::= "\"" char* "\"" space
1144-
value ::= object | array | string | number | boolean | null
11451132
)"""
11461133
});
11471134

@@ -1177,25 +1164,15 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
11771164
})""",
11781165
R"""(
11791166
a-kv ::= "\"a\"" space ":" space number
1180-
additional-k ::= ["] ( [a] char+ | [b] char+ | [c] char+ | [d] char+ | [^"abcd] char* )? ["] space
1181-
additional-kv ::= additional-k ":" space value
1182-
array ::= "[" space ( value ("," space value)* )? "]" space
11831167
b-kv ::= "\"b\"" space ":" space number
1184-
boolean ::= ("true" | "false") space
11851168
c-kv ::= "\"c\"" space ":" space number
1186-
c-rest ::= ( "," space additional-kv )*
1187-
char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
11881169
d-kv ::= "\"d\"" space ":" space number
1189-
d-rest ::= ( "," space c-kv )? c-rest
1170+
d-rest ::= ( "," space c-kv )?
11901171
decimal-part ::= [0-9]{1,16}
11911172
integral-part ::= [0] | [1-9] [0-9]{0,15}
1192-
null ::= "null" space
11931173
number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
1194-
object ::= "{" space ( string ":" space value ("," space string ":" space value)* )? "}" space
1195-
root ::= "{" space a-kv "," space b-kv ( "," space ( d-kv d-rest | c-kv c-rest | additional-kv ( "," space additional-kv )* ) )? "}" space
1174+
root ::= "{" space a-kv "," space b-kv ( "," space ( d-kv d-rest | c-kv ) )? "}" space
11961175
space ::= | " " | "\n" [ \t]{0,20}
1197-
string ::= "\"" char* "\"" space
1198-
value ::= object | array | string | number | boolean | null
11991176
)"""
12001177
});
12011178

0 commit comments

Comments
 (0)