-
Notifications
You must be signed in to change notification settings - Fork 10.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests: Added integration tests for GBNF parser #6472
Conversation
9b3c89f
to
dae2d75
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
…rsing, as well as correctness of string matching. Intended for use to pin behavior while working on performance improvements.
… that needed functions are available via internal API.
5d9918e
to
b930945
Compare
Okay, all cleaned up. Was able to get rid of a hacky Usage / expected-behavior:
Note: This error message is a leftover from a hardcoded error message printed in the grammar parser. Rather than change that, I just added a message to alert the user that the error message is expected. Not the cleanest, but it's probably fine (?). Otherwise, this test feels reasonably clean, and I think I'm happy with it. Ready for review / merge whenever. Thank you! |
@ochafik Pinging you on this -- if you have interest I'd love to get your feedback on the usability of the string-vs.-grammar code in here. Does this work for your JSON test needs, or is there anything else we should do to clean it up more? I think there might be room to add some helper functions in here (or even in the core), but I didn't want to go crazy with that at first. "Duplicated code is better than the wrong abstraction", as they say. |
@HanClinto Sorry I'm late to the party, will look into integrating your code to the json tests this week ✌️ |
No rush -- whenever you get to it. Mainly wanted to make sure you knew I was following up on stuff we'd talked about earlier. Thanks so much! |
* Added integration tests for GBNF parser to validate correctness of parsing, as well as correctness of string matching. Intended for use to pin behavior while working on performance improvements. * Fixing whitespace errors and cleaning error message alert to be clearer. * Removing hacky include to llama.cpp from grammar integration test now that needed functions are available via internal API. * Comment cleanup. * Reorganizing tests for readability. * Cleaning up debug message to make a bit more sense.
Added integration tests for GBNF parser to validate correctness of grammar parsing, as well as correctness of string matching.
If we are going to increase the performance of the grammar matcher, then this will help us pin end-to-end behavior while we work on speed and memory improvements.
Might conflict a little bit with #5948, but if that one gets merged first then I'll adjust this one to match.