PR review agent: avoid approving eval-risk behavior changes#2246
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
API breakage checks (Griffe)Result: Failed Log excerpt (first 1000 characters) |
Agent server REST API breakage checks (OpenAPI)Result: Passed |
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟢 Good taste
This is a straightforward, pragmatic safety improvement that solves a real problem: preventing the PR review agent from auto-approving changes that could affect eval/benchmark performance.
Why this is sound:
- Clear real-world need: Prevents premature approval of risky changes (prompt templates, tool execution, loop logic, etc.)
- Simple approach: Just adds policy text with concrete examples - no complex logic
- Well-scoped: Lists specific categories that need human + eval verification
- No breaking changes: Only adds constraints; existing behavior unchanged
- Meta-level change: This is about PR review workflow, not task-solving agent behavior
Note on scope: This PR modifies a prompt template, but it's for the PR review agent (CI workflow), not the eval agents (SWE-bench, GAIA, etc.). It won't affect benchmark performance - it only makes the PR review process more conservative about approving changes that could affect benchmarks.
Verdict: ✅ Worth merging - improves PR review safety without unnecessary complexity.
Key insight: This is the kind of simple, defensive guardrail that prevents production issues. Good engineering taste.
| memory/condenser behavior, terminal/stdin/stdout handling, or evaluation harness code. | ||
|
|
||
| If a PR is in this category (or you are uncertain), leave a COMMENTED review and | ||
| explicitly flag it for a human maintainer to decide after running lightweight evals. |
There was a problem hiding this comment.
@enyst I think we should not put the prompt in this file, since it will affect all the repos using the codereview action, right? i think we should put it into a repository specific .agents/code-review.md
There was a problem hiding this comment.
I'm on it! enyst can track my progress at all-hands.dev
There was a problem hiding this comment.
Summary of work completed
- Identified that the “eval / benchmark risk” review decision policy was added to
examples/03_github_workflows/02_pr_review/prompt.py, which is used by the PR-review GitHub Action and therefore would affect all downstream repos that use the action. - Implemented the requested fix by moving that policy out of the shared example prompt and into this repository’s repo-specific review guidance at
.agents/skills/custom-codereview-guide.md.
Changes made (concise)
examples/03_github_workflows/02_pr_review/prompt.py- Removed the eval/benchmark-risk “do not APPROVE…” section so the action’s base prompt remains generic for all repos.
.agents/skills/custom-codereview-guide.md- Added a new “Review decision policy (eval / benchmark risk)” section that instructs reviewers to COMMENT instead of APPROVE when changes could plausibly affect benchmark/eval performance.
Verification / compliance checklist
- Addressed the reviewer concern about global impact by relocating policy to repo-specific rules.
- Kept changes minimal (only 2 files, no unrelated edits).
- Ran
uv run pre-commit run --files ...on the modified files (passed). - Commit message included
Co-authored-by: openhands <openhands@all-hands.dev>.
Delivery status
- PR PR review agent: avoid approving eval-risk behavior changes #2246 was already merged (and its branch no longer existed remotely), so I pushed the fix to a new branch and opened a follow-up PR:
This updates the PR review agent prompt to avoid submitting APPROVE reviews for PRs that could plausibly impact benchmark/evaluation performance (tool execution, loop logic, I/O/terminal handling, etc.).
For those PRs, the reviewer should leave a COMMENT (or REQUEST_CHANGES when appropriate) and flag for a human maintainer to decide after lightweight evals.
Change is confined to: examples/03_github_workflows/02_pr_review/prompt.py
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:59cf06a-pythonRun
All tags pushed for this build
About Multi-Architecture Support
59cf06a-python) is a multi-arch manifest supporting both amd64 and arm6459cf06a-python-amd64) are also available if needed