Skip to content

feat(pr-review): announce the round early and fix the turn budget - #97

Merged
Troublor merged 2 commits into
mainfrom
williamaaroncheung/feat/pr-review-early-status
Jul 28, 2026
Merged

feat(pr-review): announce the round early and fix the turn budget#97
Troublor merged 2 commits into
mainfrom
williamaaroncheung/feat/pr-review-early-status

Conversation

@Troublor

Copy link
Copy Markdown
Collaborator

Summary

The sticky status comment was only ever written by the publisher. A round takes several minutes, so a PR stayed completely silent for that whole window and then showed the detailed review and the status comment at the same moment — the status never actually preceded the review it is meant to frame.

Preparation now posts (or updates) the comment to 🔄 Review in progress before analysis begins. The status is on the PR from the start, and the review lands into an already-visible comment.

Three phases:

  • 🔄 Review in progress — set by preparation, with the scope being reviewed and a start timestamp. Items listed underneath are explicitly labelled as carried over from earlier rounds, so they are not misread as this round's output.
  • the finished verdict — unchanged.
  • 🛠️ Review did not finish — for a round that ends without publishing, naming the cause where one is known.

Two holes I made sure to close, since announcing early means something must always retire the phase:

  • A failed round retires it from the report step, which runs if: always().
  • A stale round (head moved mid-review, output discarded) retires it the same way.
  • A skip-mode round publishes nothing, so it is never announced in the first place.

Without those, a comment reading "Review in progress" would outlive the job that wrote it — worse than the current silence.

Announcing is best effort: set_sticky_phase swallows a GitHub failure and returns the previous comment id, so failing to announce never fails a review. The report step gains GH_TOKEN because it can now write.

Test plan

  • python3 -m unittest discover -s .github/actions/claude-pr-review -p 'test_*.py' — 72 pass, 6 new: in-progress rendering, prior open items staying visible and labelled during a round, failed-phase rendering, best-effort behaviour on a GitHub failure, and both close_out_sticky paths.
  • mise run lint — clean.
  • Rendered all three phases by hand to check wording and that no phase leaks another's counts.

Troublor added 2 commits July 28, 2026 16:28
The sticky status comment was only written by the publisher, so a PR stayed
silent for the several minutes a round takes and then showed the review and the
status comment at once. Preparation now posts or updates the comment to
"Review in progress" before analysis, so the status is there first and the
review lands into an already-visible comment.

Any path that ends without publishing retires the phase itself — a failure
diagnostic, or output discarded because the head moved — so the comment never
sits at "in progress" after the job ends. Skip-mode rounds publish nothing and
are not announced. Announcing is best effort: a GitHub failure while posting it
leaves the round unaffected.
Strong-tier reviews ran out of turns at 24. The driver is not diff size: the
prompt mandates six pipeline files plus repo guidance before the diff is read,
roughly ten turns, and a small diff inside a large file spends many more paging
through surrounding code. A 226-line change across four files exhausted the
budget while an 81-line new file finished comfortably.

Raise strong to 36 and deep to 56 so the ordering between them stays
meaningful.

Also stop replaying the failure: the retry reused the first attempt's budget,
so an exhausted-turns failure burned a second full model run that could not
succeed. It now gets half again as many turns, and its prompt says to favour
returning a well-supported result over exhaustive exploration.
@Troublor Troublor changed the title feat(pr-review): announce the round before analysis starts feat(pr-review): announce the round early and fix the turn budget Jul 28, 2026
@Troublor

Copy link
Copy Markdown
Collaborator Author

Added e4cc8c0, raising the turn budget, after this PR's own review died on error_max_turns.

Why 24 was too low. Not diff size — this PR is +226/−15 across 4 files, while an 81-line single-file PR reviewed comfortably. The driver is turns spent understanding context: the prompt mandates six pipeline files plus repo guidance (~10 turns) before the diff is read, and a small diff inside a 2700-line file costs many more paging through it. Classifying every recent pr-review failure: 3 of 9 in this repo were error_max_turns (this branch, and doc/complete-rpc-reference twice), and 0 of 14 across mega-evm and stateless-validator. So it is a real but shape-specific limit, not a broken default.

Strong 24 → 36. Deep 40 → 56, so it stays meaningfully above strong.

Separate bug fixed here too. The retry reused the first attempt's budget. Exhausting turns is deterministic, so the retry replayed the identical failure — visible twice in the failing log — burning a second full Opus run with no chance of success. The retry now gets 1.5× the turns, and its prompt says to favour returning a well-supported result over exhaustive exploration.

This cannot fix its own PR. The pr-review job pins the action to @main, so this PR is reviewed by main's code and will keep failing at 24 until it merges. The fix takes effect for subsequent PRs.

@mega-maxwell

mega-maxwell Bot commented Jul 28, 2026

Copy link
Copy Markdown

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; answer findings and questions in the review threads it links to.

✅ Review clean

Last reviewed: head e4cc8c01 · updated 2026-07-28T09:05:44+00:00

New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0

@Troublor
Troublor merged commit dc68b9c into main Jul 28, 2026
5 checks passed
@Troublor
Troublor deleted the williamaaroncheung/feat/pr-review-early-status branch July 28, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant