- Notifications
You must be signed in to change notification settings - Fork899
fix: update json schema forcreate_pull_request_review#300
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR updates the JSON schema for the create_pull_request_review tool to conform with OpenAI’s function calling API requirements. The changes ensure that all array properties are now required while the previously optional properties can be set to null.
- Updated the "required" array to include previously optional properties: "position", "line", "side", "start_line", and "start_side".
- Modified the type definitions for these properties to allow null values.
Comments suppressed due to low confidence (1)
pkg/github/pullrequests.go:911
- The inclusion of previously optional properties in the 'required' array forces all keys to be present, even if null. Ensure that clients sending data to this API are updated accordingly to prevent potential integration issues.
"required": []string{"path", "body", "position", "line", "side", "start_line", "start_side"},Just validating the resultant schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I've tested the changes and they work well.
| "properties": map[string]interface{}{ | ||
| "path": map[string]interface{}{ | ||
| "type": "string", | ||
| "description": "path to the file", | ||
| }, | ||
| "position": map[string]interface{}{ | ||
| "type": "number", | ||
| "type":[]string{"number", "null"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I tested this continues to work in VSCode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I appreciate this is aimed at other host applications, but I think if it doesn't break ones that work, then hopefully it is at worst a noop, and at best works with more tools. I'll try another now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's great testing this with itself!
22e4936 intomainUh oh!
There was an error while loading.Please reload this page.
This updates the JSON schema for the
create_pull_request_reviewtool to make all of the array properties required as per OpenAI's function calling API requirements,but make the optional properties nullable.