Feature: PR Review

[enyst]

What problem or use case are you trying to solve?
OpenHands has a resolver module which resolves GitHub issues, writes comments and reads comments, creates branches, commits and PRs.

We want to add to it a PR review functionality.

Describe the UX of the solution you'd like

When a PR is labeled with "review-pr", a github job to review the PR will be executed.

Implementation

• Read openhands/resolver and all its py files.
• Read the prompts/resolve sub-directory and its .jinja files.
• Read the github workflow file at ./github/workflows/openhands-resolver.yml (note that this is from the repo root)
• Think carefully how the resolver job works, its triggers, the repo it works with, how it reads a PR, how it comments on a PR, how it finds github comments and the corresponding source code. You can take the time to write out everything you find out because it helps you think. It's fine if you're throughout and your output is long. Keep it technical, don't pontificate.

Once you're done and understand the existing code, lets implement the new functionality of the resolver:

• make another job in the openhands-resolver workflow, beside auto-fix: review-pr
• it should be triggered for the label "review-pr"
• it should check out the pr, read its description, read its diff, and comments if any
• it should write a clear and comprehensive review as a comment to the PR.

For this feature implementation, you also need to:

• write a prompt in the openhands/resolver/prompts sub-directory for this, similar with existing prompts there (you can create its own folder)
• implement the necessary code in a similar way with the existing resolve-issue.py or other files you read previously, if it makes sense for the review-pr functionality
• however, do not add code if the review-pr functionality has no use for it. For example, a review is a comment, it doesn't make sense to "guess success" for it.
• add unit tests for the python functions you add.

IMPORTANT
We work on python 3.12, and so you can use builtin list, dict, | None, not the older variants.
Do NOT delete comments from the existing code.
Keep the new code you write aligned with the way existing code is written, style, comments, and patterns, otherwise it triggers my ocd.

[github-actions]

OpenHands started fixing the issue! You can monitor the progress here.

[github-actions]

An attempt was made to automatically fix this issue, but it was unsuccessful. A branch named 'openhands-fix-issue-5219' has been created with the attempted changes. You can view the branch here. Manual intervention may be required.

[github-actions]

OpenHands started fixing the issue! You can monitor the progress here.

[github-actions]

An attempt was made to automatically fix this issue, but it was unsuccessful. A branch named 'openhands-fix-issue-5219-try2' has been created with the attempted changes. You can view the branch here. Manual intervention may be required.

[github-actions]

This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[enyst]

I think we might want this feature, but I wrote this as a prompt to the LLM, out of curiosity to see what it does: and it was wildly successful at that. The result was basically a one-shot doing exactly as requested.

Since then, I've been looking further into this, and I used this to familiarize myself with the code of the resolver itself. And I think this might not be the way to go:

• it leads to a completely separate implementation from the rest of the resolver, when in fact that code already has some amount of PR review, since it reviews itself at the end of a PR; it feels like a better way is to reuse some of that
• it only creates one comment, there's nothing in this prompt about looking at each file diff, and inline comments would be more useful.

I'm going to close this, for these reasons. We may want to take another look at the issue with a renewed prompt. 😅