Skip to content

fix: refresh a stale score comment when the PR shrinks below the threshold - #89

Merged
3 commits merged into
mainfrom
fix/score-comment-stale
Sep 27, 2026
Merged

3 commits merged into
mainfrom
fix/score-comment-stale

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Problem

action.yml ran the "Post comment" step only when should_split == 'true'. Once a PR had been flagged and the author pushed it back under max-loc, the script took the _skip path, wrote no comment body, and the old <!-- pr-split-score --> comment with the now-wrong split plan stayed on the PR forever.

Fix

  • scripts/score_pr.py: comment writing is factored into _write_comment; the under-threshold _skip(..., within_limits=True) writes a short "within acceptable size limits" body carrying the marker. The two failure-path skips (pr-split error, missing plan) deliberately write no body, so a transient failure can never overwrite a valid plan comment.
  • action.yml: the step runs whenever comment_path is set, returns early on non-PR payloads, updates an existing marker comment regardless of should_split, and only creates one when SHOULD_SPLIT == 'true' (so small PRs that were never flagged stay quiet).

Tests

  • under-threshold skip writes a marker body and comment_path
  • failure skip writes no comment_path
  • main() with a failing stub pr-split on a 500-LOC diff writes no comment_path
  • main() with a stub that writes a 2-group plan produces a marker-prefixed body with the group table

Local review: 5/5 (the github-script branching was simulated for all five cases). 448 tests pass, ruff clean.

…shold

The comment step only ran when should_split was true, so once a PR was
pushed back under the limit its old '<!-- pr-split-score -->' comment
with the split plan stayed. The script now writes a 'within limits'
body on every skip, and the action updates an existing marker comment
regardless of should_split while still only creating one when a split
is suggested. Non-PR events are skipped in the step itself.
… path

Failure-path skips (pr-split error, missing plan) must not produce a
body, or a transient failure would overwrite a valid split-plan comment
with a false 'within limits' message.
@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 48 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3a58ac0c-5344-4550-b43d-34fb1f69e081

📥 Commits

Reviewing files that changed from the base of the PR and between 671a081 and d99ce23.

📒 Files selected for processing (4)
  • action.yml
  • scripts/score_pr.py
  • tests/test_score_pr_comment.py
  • tests/test_score_pr_events.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Main already factored out _write_comment and now flags oversized PRs with
a comment on planner failure, so only the under-threshold refresh remains:
_skip(within_limits=True) writes a marker comment via main's helper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w
@vitali87 vitali87 closed this pull request by merging all changes into main in 8ca9e3b Sep 27, 2026
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.

2 participants