Skip to content

fix: update json schema for create_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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2025

Conversation

toby
Copy link
Member

@toby toby commented Apr 16, 2025

This updates the JSON schema for the create_pull_request_review tool to make all of the array properties required as per OpenAI's function calling API requirements, but make the optional properties nullable.

@Copilot Copilot AI review requested due to automatic review settings April 16, 2025 20:55
Copy link
Contributor

@Copilot Copilot AI left a 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"},

@SamMorrowDrums
Copy link
Collaborator

Just validating the resultant schema

{
  "description": "Create a review on a pull request",
  "inputSchema": {
    "type": "object",
    "properties": {
      "body": {
        "description": "Review comment text",
        "type": "string"
      },
      "comments": {
        "description": "Line-specific comments array of objects to place comments on pull request changes. Requires path and body. For line comments use line or position. For multi-line comments use start_line and line with optional side parameters.",
        "items": {
          "additionalProperties": false,
          "properties": {
            "body": {
              "description": "comment body",
              "type": "string"
            },
            "line": {
              "description": "line number in the file to comment on. For multi-line comments, the end of the line range",
              "type": [
                "number",
                "null"
              ]
            },
            "path": {
              "description": "path to the file",
              "type": "string"
            },
            "position": {
              "description": "position of the comment in the diff",
              "type": [
                "number",
                "null"
              ]
            },
            "side": {
              "description": "The side of the diff on which the line resides. For multi-line comments, this is the side for the end of the line range. (LEFT or RIGHT)",
              "type": [
                "string",
                "null"
              ]
            },
            "start_line": {
              "description": "The first line of the range to which the comment refers. Required for multi-line comments.",
              "type": [
                "number",
                "null"
              ]
            },
            "start_side": {
              "description": "The side of the diff on which the start line resides for multi-line comments. (LEFT or RIGHT)",
              "type": [
                "string",
                "null"
              ]
            }
          },
          "required": [
            "path",
            "body",
            "position",
            "line",
            "side",
            "start_line",
            "start_side"
          ],
          "type": "object"
        },
        "type": "array"
      },
      "commitId": {
        "description": "SHA of commit to review",
        "type": "string"
      },
      "event": {
        "description": "Review action to perform",
        "enum": [
          "APPROVE",
          "REQUEST_CHANGES",
          "COMMENT"
        ],
        "type": "string"
      },
      "owner": {
        "description": "Repository owner",
        "type": "string"
      },
      "pullNumber": {
        "description": "Pull request number",
        "type": "number"
      },
      "repo": {
        "description": "Repository name",
        "type": "string"
      }
    },
    "required": [
      "owner",
      "repo",
      "pullNumber",
      "event"
    ]
  },
  "name": "create_pull_request_review"
}

Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums left a 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"},
Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums left a 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!

@SamMorrowDrums SamMorrowDrums merged commit 22e4936 into main Apr 17, 2025
16 checks passed
@SamMorrowDrums SamMorrowDrums deleted the fix-pr-review branch April 17, 2025 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants