Add partially synthetic validation of option pairs that represent ranges#4949
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog Entry
To be copied to the draft changelog by merger:
vg giraffenow validates more options, including pairs of options describing min and max values of rangesDescription
Someone mentioned somewhere that Giraffe was able to crash instead of actually validating the values of some of the options that control mapping algorithm parameters.
This adds a couple more validation guards on individual options, and adds a new system for validating that paired min and max options actually describe a nonempty interval, though right now this happens after index loading, which is not ideal. Validating earlier would require exciting comparisons on the (unrelated)
Rangeobjects used for ticking through option combinations to make sure they're non-overlapping, in a way that the option parsing logic is not architected to support well. The right answer there might be telling the option parsing/range stuff when two options form a pair like this.This set of options missing validation was identified by the rigorous process of "asking Anthropic Claude Opus to find them", and so may be incomplete. I also let it write the calls to the validation function (and I then had to fix a missing min option it wanted to validate), and fix my syntax.