Port guidance's JSON implementation #48
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
parser/src/json/numeric.rs
Outdated
| right: Option<f64>, | ||
| left_inclusive: bool, | ||
| right_inclusive: bool, | ||
| ) -> String { |
There was a problem hiding this comment.
Should be easy enough to use anyhow::Result<String> and than ensure!() instead of assert!() and bail!() instead of panic!(), at least in the top-level function. The inner asserts should probably never happen.
There was a problem hiding this comment.
Agreed. This particular file is thankfully the result of chatgpt converting your python to rust (with some annoying debugging since it took it upon itself to incorrectly modify some of the logic...)
Thanks! Please don't approve until it's done though 😆 |
hudson-ai
force-pushed
the
json_extension
branch
from
bb7b03f
to
be74d89
Compare
…hat have already been normalized
parser/src/json/schema.rs
Outdated
| @@ -247,6 +251,7 @@ struct Context<'a> { | |||
| // TODO: actually use this to handle draft-specific behavior | |||
| draft: Draft, | |||
| defs: Rc<RefCell<HashMap<String, Schema>>>, | |||
| seen: Rc<RefCell<HashSet<String>>>, | |||
There was a problem hiding this comment.
at some point, you will probably want Rc<RefCell<InnerContext>> (probably when adding normalize counters)
There was a problem hiding this comment.
Oh thanks, I'll look into it. Was just about to start adding said counters to track the number of Schema nodes that are being constructed. Probably not to normalize itself though, as I just made that "shallow".
|
@mmoskal re: consts and enums... "Upstairs" in the grammar compiler, I'm already transforming non-atomic const nodes (objects and arrays) into strict schemas that are then being passed to gen_json_*. This is so that they're actually properly lexed and have the correct whitespace behavior. I could do this earlier in the Schema builder, doing it for "atomic" types too, e.g. This would allow our existing intersection logic to handle these cases rather than using a full JSON validator. It would be more limited in its capabilities, but it would solve the problem of not having a real schema object when we're merging. The only complication here is bools, which have no built-in mechanism for constraining them. So I think I would just have to add a LiteralBool type. |
|
Hoping to merge this ASAP though, and I think I will worry about consts, enums, and refs in a follow-up PR. Will work on counters and unnecessary cloning today |
|
I like the const->schema idea. We just need to make sure we get similarly efficient schemas from out min/max etc logic. Also, for the case of enum it would be ideal to generate a single regex for the enum, not a grammar (grammars have a limit of at most ~200 items in a select, but regexes don't). But all good for later PRs, let's marge this! |
hudson-ai
changed the title
Working towards resolving #44
Super draft PR, just opening for feedback and help with my extremely amateurish rust (thanks @mmoskal!)
To run guidance's JSON tests with this as the backend, check out my (also very rough) guidance branch
https://github.com/hudson-ai/guidance/tree/llguidance_json