CI: make the LLM reviewer terser and let it close out fixed findings - #131
Open
hauke wants to merge 2 commits into
Open
CI: make the LLM reviewer terser and let it close out fixed findings#131hauke wants to merge 2 commits into
hauke wants to merge 2 commits into
Conversation
The reviews the bot posts on openwrt/openwrt are dominated by text the maintainer already has. Review bodies recap the change instead of flagging anything: openwrt/openwrt#24915 opened with a "Verification performed:" list of three checks that all passed, #24916 followed "no new issues found" with a six-bullet "Re-verified on the new head:" rundown, and #24919 spent a paragraph on "the interdiff is confined to the previous round's feedback and each change checks out: ..." followed by every change in it. Inline comments run to several paragraphs with the reasoning chain spelled out, restate the line they are anchored to, and append unrelated "Separately: ..." findings that belong on their own anchor. Add a "Keep it short" section to both prompts — in the nightly digest it goes inside the sub-agent template, where the reviewing actually happens — and enforce it again at the two points that produce output: - Never recap the change. No description of what the PR does, what a commit changed, what was inspected or verified, or which earlier findings are now fixed. - Inline comments: three sentences at most before the optional suggestion block, no restating the commented line, no "this matters because ..." tail, several defects on one line get one short bullet each. - Review body: commit-check bullets only, one or two lines each — no summary, no verification log, no pointer to the inline comments, no praise for what is already correct. - One finding per comment. The nightly no-op body stays "Reviewed N new commits; no new issues found." — it is the marker the next run uses to detect re-review work — but it is now the entire body. docs/llm-review-setup.md still advertised a "1-2 sentence summary" the prompt has not asked for since 6013348; describe the current shape instead. Fixes: 2d30538 ("CI: add LLM-driven PR review workflows") Assisted-by: Claude:claude-opus-5 Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Once a contributor fixes an inline finding, nothing ever happens to the thread: the bot account only holds triage permission, so it cannot resolve conversations (that needs write access), and it never came back to the thread either. Threads from three rounds ago sit open next to code that has long since changed, and the maintainer has to open each one to find out whether it still applies. Have the reviewer reply on its own threads instead, once the review is posted. The reply is deliberately tiny — "fixed, thanks" when the finding was addressed as asked, one short sentence when it was addressed some other way, and nothing at all when it is unfixed or only half fixed, so the bot never nags and never repeats itself. It replies only when the thread is its own, is still unresolved, and holds exactly one comment: if the author or a maintainer has already answered there, the conversation is theirs and the bot stays out of it. This is also what replaces the "what the previous round fixed" prose that the preceding commit removed from the review body — the acknowledgement belongs on the thread it concerns, not in a summary. Mechanically this uses `add_reply_to_pull_request_comment`, added to the Tools paragraph of both prompts (and the nightly-digest sub-agent template). It takes the numeric REST comment ID, which `get_review_comments` does not return directly, so the prompt spells out that it is the number in the comment's `html_url` anchor (`#discussion_r3874849507`) rather than the GraphQL thread node ID (`PRRT_...`). Assisted-by: Claude:claude-opus-5 Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
hauke
changed the title
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
1 participant
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.
Two prompt changes for the LLM review routines, based on how the reviews
read on openwrt/openwrt.
Cut the review down to its findings. Bodies were recapping the change
instead of flagging anything — #24915 opened with a "Verification
performed:" list of checks that all passed, #24916 followed "no new issues
found" with a six-bullet "Re-verified on the new head:" rundown. Inline
comments ran to several paragraphs, restated the line they were anchored
to, and appended unrelated "Separately: ..." findings. Both prompts now
carry a "Keep it short" section: never recap the change, three sentences
max per inline comment, commit-check bullets only in the body, one finding
per comment.
Acknowledge findings the contributor fixed. Nothing ever happened to a
thread once its finding was fixed — the bot account only holds triage
permission, so it cannot resolve conversations. It now replies "fixed,
thanks" on its own threads instead, but only where the thread is still
unresolved and nobody else has replied. This also replaces the "what the
last round fixed" prose removed above: the acknowledgement belongs on the
thread, not in a summary.
Note these files are only the source of truth — the text has to be pasted
into the two routines at claude.ai/code/routines to take effect.