Clarify create_pull_request_review input schema definition for: 'position'

[jamipouchi]

Why

I have been playing with github's mcp server, and have found that it is incapable of adding comments due to:

McpError: MCP error -32603: failed to create pull request review: POST https://api.github.com/repos/owner/name/pulls/pullId/reviews: 422 Unprocessable Entity [{Resource: Field: Code: Message:Pull request review thread position is invalid and Pull request review thread diff hunk can't be blank}]

This is because the current definition for the position input parameter is wrong:

    line number in the file

This is a clear misconception, as even the github api documentation states a Note specifying that the position is NOT the line number in the file, but of the diff.

What

I have changed the description to better reflect what the input actually is. I have based it on the REST API documentation, and my tests.

How

I have used: claude-3-5-sonnet-latest with a simple loop:

• it is given a pr and the tool to get the file for more context
• When all files have been looked at, it is asked to create a pr review

The description is a bit longer than I would have liked, but I have seen that it has a hard time understanding what this position actually means, and this has given the best results.

Alternatives

• Changing this position to be the line of the file. Thought that would require changing how the REST API endpoint works, and is not so simple, as a single fine line, might have multiple positions in a diff (ex. when there is a deletion and addition)
• A similar description that works better (that requires testing a broader set of cases than I have)

[juruen]

Thank you for your contribution and for bringing this to our attention. I believe this is being addressed in #118, where we also introduce the use of new line-related parameters.

Having said that, let's do some tests and see if we need to update the new description in #118 to make it succeed.

Note position is deprecated, and we'll eventually drop it in favor of line, start_line, etc.

[jamipouchi]

Thank you for your contribution and for bringing this to our attention. I believe this is being addressed in #118, where we also introduce the use of new line-related parameters.

Having said that, let's do some tests and see if we need to update the new description in #118 to make it succeed.

Note position is deprecated, and we'll eventually drop it in favor of line, start_line, etc.

Yep, that's right, should have seen that it was already being addressed 😅
Closing this, happy to test the new changes and see if it works well.