fix(pr-review): post reviews via Pull Request Reviews API, not issue comments#339
Open
m0nk111-post wants to merge 1 commit into
Open
Conversation
…comments
The github-pr-review skill and the agent prompt both documented the
correct API (POST /repos/{owner}/{repo}/pulls/{n}/reviews with a
comments[] array), but on real PRs the agent kept producing a single
gh pr comment (issue-level blob) instead of a proper review with
inline threads and suggestion blocks.
Two changes:
1. plugins/pr-review/scripts/prompt.py: insert a '## How to Post the
Review (CRITICAL — read first)' block right under the /github-pr-review
trigger. It includes a worked bash + JSON example, mandates the
Reviews API, and explicitly forbids gh pr comment / POST
/issues/{n}/comments. The {owner}/{repo}/{n} tokens inside the
example are escaped ({{...}}) so they survive str.format()
substitution; the existing pr_number placeholder stays as a real
substitution target.
2. skills/github-pr-review/SKILL.md: rewrite to align the spec with
the github-code-quality[bot] format. New sections: Pre-Review
Checks, 'One PR Review, One API Call, Many Inline Comments',
Inline Comment Body Format (## <Priority> <Category> + one-line
statement + --- separator + 'Best fix in this file' anchor + scope
confirmation + optional suggestion block), worked example, Good
vs Bad Inline Comments, edge cases, and a Summary Checklist.
The 'How Suggestions Actually Work' section heading is preserved
(test_skill_explains_replace_semantics depends on it).
3. skills/index.js: auto-regenerated by scripts/build-skills-catalog.mjs
to reflect the updated SKILL.md.
All 33 tests in test_pr_review_prompt.py, test_github_pr_review_skill.py,
test_pr_review_feedback.py, and test_pr_review_diff_payload.py pass.
Reference target: m0nklabs/cryptotrader#379
|
oke, human here. |
m0nk111
added a commit
to m0nk111/extensions
that referenced
this pull request
Jun 16, 2026
…PI and duplicate-review guard (#2) * feat(pr-review): add agent-canvas-automation v2.7 with Reviews API Tracks the standalone OpenHands Automations tarball that powers the github-pr-review automation on m0nklabs/cryptotrader. The v1 tarball (oh-internal://uploads/80f203b3-0678-404a-9b60-401782a0e4b9) told the agent NOT to use the Reviews API and posted results via the issue-comments API, which produced single-blob reviews instead of inline review threads. v2.7 (currently deployed) replaces the prompt template and the result-poster. Concretely: - Prompt mandates the GitHub Pull Request Reviews API and requires the agent to emit a ###REVIEW_JSON### block with the comment list. - Result-poster calls POST /repos/.../pulls/{n}/reviews with comments[]. - Path validation drops comments whose path is not in the PR diff. - REQUEST_CHANGES is downgraded to COMMENT when the bot is the PR author. - Re-review guard: closed (PR, label_event_id) pairs are skipped. - MCP-direct detection via _list_existing_reviews, no double-post. - Brace-counting JSON parser with last-marker-wins rule. Includes the source-of-truth v2.7 main.py and a README that documents the v1 -> v2.7 transition, the bug, the fix, and a real-world demonstration table comparing PR 380/381 (old format) with PR 400 (new format, clean e2e test). Refs OpenHands#339 Co-authored-by: openhands <openhands@all-hands.dev> * feat(pr-review): add agent-canvas-automation v2.8 with duplicate-review guard PR 400 demonstrated a race: the agent posted a review via the GitHub MCP (21:06:09), then the v2.7 script also posted from the parsed ###REVIEW_JSON### block (21:06:21), producing two reviews with identical content. The v2.7 script's MCP-direct detection only ran in the 'no JSON' path, not the 'have JSON' path. v2.8 fixes this: the script now calls _list_existing_reviews and _existing_bot_review_for_commit BEFORE deciding whether to post, regardless of whether the agent emitted a parseable JSON contract or used the GitHub MCP directly. If a m0nk111-post review already exists at the same commit_id, the script closes the state without re-posting. Verified on PR 402 (m0nklabs/cryptotrader): exactly 1 review lands, vs 2 on PR 400 with v2.7. Refs the PR 400 duplicate that the user reported. Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On real PRs, the
github-pr-reviewagent posts a singlegh pr comment(issue-level blob) instead of aPOST /repos/{owner}/{repo}/pulls/{n}/reviewscall with acomments[]array. That bypasses inline diff anchoring, breaks the suggestion-block UX, and produces no per-finding threads.Concrete observation: on m0nklabs/cryptotrader#379, the github-code-quality bot left 16 inline review threads (one per finding, each with a
Commit suggestionbutton). The OpenHands agent on the same PR left one Markdown blob in the PR conversation, with zero inline threads — i.e. it used the wrong API.The previous
SKILL.mdand prompt did say to bundle findings into one Reviews API call, but the agent routinely ignores the spec and falls back togh pr comment. The likely cause is that the prompt's primary instruction is "keep the review body brief unless your active review instructions require a longer structured format", which is weak and easily overridden by the agent's default behaviour. The spec was hidden several layers down inside the skill.Fix
Two changes:
plugins/pr-review/scripts/prompt.py— insert a## How to Post the Review (CRITICAL — read first)block directly under the/github-pr-reviewtrigger. It mandates the Reviews API, shows a workedgh api+ JSON example with one entry per finding, and explicitly forbidsgh pr comment/POST /issues/{n}/comments. The block is positioned before the existing "keep the review body brief" line so the strongest directive fires first.The example uses
{{owner}}/{{repo}}/{{n}}escapes (rendering as literal{owner}/{repo}/{n}) so the LLM sees a realistic-lookingghcommand. The realpr_numberplaceholder stays substituted.plugins/pr-review/skills/github-pr-review/SKILL.md— rewrite to align the spec with thegithub-code-quality[bot]format. New sections: Pre-Review Checks, Key Rule: One PR Review / One API Call / Many Inline Comments, Inline Comment Body Format, Worked example, Multi-Line Suggestion Blocks, How Suggestions Actually Work (Mandatory Verification), When to Use a Suggestion Block, Good vs Bad Inline Comments (borrowed from Aiderreviewer_tier1.md), Edge Cases, Summary Checklist.skills/index.js— auto-regenerated byscripts/build-skills-catalog.mjsto mirror the new SKILL.md content.Tests
All 33 tests in
test_pr_review_prompt.py,test_github_pr_review_skill.py,test_pr_review_feedback.py, andtest_pr_review_diff_payload.pypass. The full non-env-blocked suite (266 tests) is green.How to verify on PR #379
After this PR is merged, re-trigger the review on m0nklabs/cryptotrader#379 by adding the
review-thislabel. The new review should:path:linewithside: "RIGHT".## <Priority> <Category>shape, with 🔴 Critical / 🟠 Important / 🟡 Suggestion.```suggestion ```block where the fix is ≤ 5 lines and contiguous.Note on testing before merge
This change is already pushed to https://github.com/m0nk111-post/extensions/tree/pr-review/format-issues-as-review-threads. To run a live CI test against PR #379 before merging, change the
pr-review-by-openhands.ymlworkflow'suses:line fromOpenHands/extensions/plugins/pr-review@maintom0nk111-post/extensions/plugins/pr-review@pr-review/format-issues-as-review-threadsin a draft PR on the fork.Co-authored-by
Generated by an AI agent (OpenHands) on behalf of the maintainer.