JSON Schema to GBNF integration tests #7790
Conversation
|
@ochafik Check out these latest tests -- I think you might like them. :) |
|
Awesome! Was plotting something similar in #7797 :-) I'll take a deeper look in the coming days! |
Haha -- we went down nearly identical roads. Your framework is much nicer than mine -- I'll pull your framework into this branch and update my tests accordingly. Then you should be able to pull it into your branch more easily if you want. Nicely done!! |
mofosyne
added
the
Review Complexity : Medium
HanClinto
force-pushed
the
tests-json-schema-integration
branch
from
bd3ebb6 to
cfccb6c
Compare
|
Okay, ready for review / merge. I incorporated the improvements from @ochafik 's test harness in #7797 , and also added a |
HanClinto
force-pushed
the
tests-json-schema-integration
branch
from
307619d to
488cbfa
Compare
ochafik
left a comment
ochafik
left a comment
There was a problem hiding this comment.
Amazing, thanks @HanClinto !
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
now ./llama-gbnf-validator :-D
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
Just indent across string boundary?
R"""({
"type": "string",
"minLength": 3
})""",
tests/test-grammar-integration.cpp
Outdated
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
I would split "data", "uuid", "time" and "date-time" test cases
tests/test-grammar-integration.cpp
Outdated
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
Thanks for preparing these!
I would just move them down to the failed branch (w/ caveat header), hopefully I'll move them back up very soon :-) (not a fan of #ifdef dead code)
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
nit: TBH I'd be tempted to use R"""({...})""" as soon as any backslash is needed, to make it easier to copy-paste things.
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
nit: exclusiveMinimum for number is on my roadmap :-)
tests/test-grammar-integration.cpp
Outdated
There was a problem hiding this comment.
uncomment & move it up / to passing?
HanClinto
force-pushed
the
tests-json-schema-integration
branch
from
488cbfa to
8713e57
Compare
…n validation against auto-generated JSON-schema grammars.
…ng the ability to automatically output improperly failing grammars to debug output files so they can more easily be examined in the gbnf-validator program.
…o are attempting to reproduce the bugs.
…n pass CI, but still be useful for other PRs that want to leverage the framework.
…failing cases, fixing some other strings.
HanClinto
force-pushed
the
tests-json-schema-integration
branch
from
8713e57 to
3bdf7c3
Compare
3 participants
This adds a number of FAILING tests to exercise JSON schema conversion. These failures are good, as they show bugs in our current schema-to-grammar system (as noted by #7703 and #7789 ).
I originally started writing these tests to help me dig into the root causes behind #7703 , but I ran into some other issues along the way (hence #7789 ).
Of the failing tests, in particular, there are a number of "pass" test cases that are marked with TODO that SHOULD pass and currently do NOT.
The "fail" test cases that are improperly passing are not as big of a deal (these are things that we can't really expect to support, such as numeric minimums or ensuring uniqueness of array entries), so feel free to ignore those TODO entries.
This is meant as an aide to anyone else who wants to chip in on any of the json-schema features / bugs and wants an easier way to debug the json schema generation and test it against sample generations. Very helpfully the json-schema.org documentation often includes examples of matching and non-matching strings for example schemas in their documentation, and I have begun copying those into these integration tests directly.