Skip to content

fix: pr-review-feedback omits PR summary comment when no fixes applied - #25

Merged
ether-moon merged 2 commits into
mainfrom
montevideo-v2
May 20, 2026
Merged

fix: pr-review-feedback omits PR summary comment when no fixes applied#25
ether-moon merged 2 commits into
mainfrom
montevideo-v2

Conversation

@ether-moon

@ether-moon ether-moon commented May 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • pr-review-feedback now posts the PR summary comment unconditionally whenever ≥1 review comment was collected — independent of fix outcome and reviewer type (human / Codex / Claude / CodeRabbit / other).
  • CodeRabbit @coderabbitai resolve header is reframed as additive: detected from the PR's actual reviewer/commenter set via gh pr view --json reviews,comments,reviewThreads, not the post-filter collected items.
  • Phase 5.1 (commit) and 5.2 (summary comment) are now explicitly independent steps in the agent doc, so a no-op 5.1 never silences 5.2.

Changes

  • plugins/skill-set/agents/pr-review-feedback.md
    • Autofix Framework "PR-specific adaptations": split bot-resolution bullet into "Summary comment is universal" + "CodeRabbit resolve header is additive"
    • Phase 5 rewritten: 5.1/5.2 independent; 5.2 marked MANDATORY when Phase 1 collected ≥1 review comment; CodeRabbit detection moved to GitHub API reviewer set
    • Common Mistakes: added "Skipping Phase 5.2 When No Fixes Were Applied" and "Detecting CodeRabbit From the Filtered Comment Set"
    • Success Criteria updated to reflect unconditional comment posting and reviewer-set-based detection

Background

Before this change, /skill-set:pr:ship cycles that produced no fixes (e.g., all collected comments were non-actionable or already resolved) silently skipped posting the summary comment, and the @coderabbitai resolve header inherited the same gap because its detection ran on the filtered comment set. Reviewers got no acknowledgement, and CodeRabbit threads stayed open across cycles.

The fix lives entirely in pr-review-feedback so that /skill-set:pr:fix standalone gets the same correctness as the shipping-pr → resolving-pr-blockers → pr-review-feedback chain — single owner, no responsibility duplication.

Test Plan

  • Run /skill-set:pr:fix on a PR with only LGTM / non-actionable review comments — summary comment is posted.
  • Run /skill-set:pr:fix on a PR reviewed by CodeRabbit with no new actionable items — summary comment includes @coderabbitai resolve header as the first line.
  • Run /skill-set:pr:fix on a PR reviewed only by a human reviewer — summary comment is posted without the CodeRabbit header.
  • Run /skill-set:pr:ship end-to-end on a PR with CodeRabbit reviews — every cycle posts a summary comment, the resolve header appears once CodeRabbit is a reviewer.

Summary by CodeRabbit

  • Documentation
    • Updated PR review feedback workflow documentation to ensure summary comments are posted consistently when review activity is detected
    • Enhanced CodeRabbit integration with improved detection mechanisms and resolve header handling
    • Refined procedural steps and success criteria for better clarity on review completion requirements

Review Change Stack

Phase 5.2 (summary comment) was implicitly gated on having items to commit,
so PRs reviewed by humans, Codex, Claude, or CodeRabbit would receive no
acknowledgement when every collected comment was non-actionable or skipped.
The CodeRabbit resolve header inherited the same gap because its detection
ran on the post-filter comment set instead of the PR's actual reviewer set.

- Split Phase 5.1 (commit) and 5.2 (summary comment) into independent steps;
  5.2 now runs whenever Phase 1 collected >=1 review comment, regardless of
  reviewer type or fix outcome.
- Detect CodeRabbit via gh pr view --json reviews,comments,reviewThreads on
  the actual identities, before filtering. Resolve header is additive on top
  of the universal summary.
- Add Common Mistakes entries for both regressions and update Success
  Criteria accordingly.
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@ether-moon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 31 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2152df71-781d-467c-9e0b-5fe61706946b

📥 Commits

Reviewing files that changed from the base of the PR and between d5d18d8 and 72245fd.

📒 Files selected for processing (1)
  • plugins/skill-set/agents/pr-review-feedback.md
📝 Walkthrough

Walkthrough

The PR updates the pr-review-feedback workflow documentation to refine Phase 5 completion behavior. Phase 5 instructions are restructured so commit logic (Step 5.1) executes only when Phase 4 produced file changes, while summary posting (Step 5.2) runs unconditionally whenever review comments were collected. CodeRabbit resolve header detection switches from bot-based logic to GitHub API checks, with verification and retry logic added.

Changes

Phase 5 Completion and CodeRabbit Resolution

Layer / File(s) Summary
Phase 5 completion instructions and CodeRabbit detection logic
plugins/skill-set/agents/pr-review-feedback.md
Phase 5 instructions separated into Step 5.1 (conditional commit on file changes) and Step 5.2 (unconditional summary post on collected review comments). CodeRabbit resolve header detection replaced with GitHub API-driven identity checks over reviews, comments, and reviewThreads, including verification-and-retry to ensure the header is the first line. Common mistakes guidance extended to address Phase 5.2 execution independence and API-based CodeRabbit detection. Success criteria updated to reflect the new Step 5.1/5.2 split and API-based inclusion rule.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 Phase 5 now splits its dance with clarity supreme,
Step One commits when changes gleam,
Step Two posts summaries unconditional, always,
CodeRabbit found via APIs, no more maybes—
Two hearts beat true where once was one! 🎭

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main fix: ensuring the PR summary comment is posted unconditionally when review comments exist, rather than being omitted when no fixes are applied.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch montevideo-v2

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/skill-set/agents/pr-review-feedback.md (1)

239-249: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Incomplete verification logic for CodeRabbit resolve header.

Line 248 states: "If the CodeRabbit resolve header was included (HAS_CR > 0), verify it appears as the first line of the posted comment. If missing, re-post once and re-verify."

However, the verification script (lines 240-246) only retrieves and displays the comment but doesn't show how to programmatically:

  1. Check if the first line is @coderabbitai resolve
  2. Re-post the comment if the header is missing
  3. Re-verify after re-posting

The echo "$LAST_COMMENT" | head -5 command displays lines for manual inspection but doesn't implement the automated check-and-retry logic described.

📝 Suggested addition to show complete verification implementation

Add the following after line 246 to demonstrate the full verification and retry logic:

# If CodeRabbit header was included, verify it's the first line
if [ "$HAS_CR" -gt 0 ]; then
  FIRST_LINE=$(echo "$LAST_COMMENT" | head -1)
  if ! echo "$FIRST_LINE" | grep -q "`@coderabbitai` resolve"; then
    echo "Warning: CodeRabbit resolve header missing. Re-posting..."
    # Re-post with header
    gh pr comment "$PR_NUMBER" --body "$COMMENT_BODY"
    sleep 2
    # Re-verify
    LAST_COMMENT=$(gh pr view "$PR_NUMBER" --json comments --jq '.comments[-1].body')
    FIRST_LINE=$(echo "$LAST_COMMENT" | head -1)
    echo "$FIRST_LINE"  # Confirm header present
  fi
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/skill-set/agents/pr-review-feedback.md` around lines 239 - 249, The
verification step currently only prints LAST_COMMENT but does not enforce the
CodeRabbit resolve header; update the script to (1) when HAS_CR > 0 extract the
first line (e.g., FIRST_LINE=$(echo "$LAST_COMMENT" | head -1)), (2) test it for
the literal "`@coderabbitai` resolve" (use grep -q or a string comparison), (3) if
missing, re-post the comment with gh pr comment "$PR_NUMBER" --body
"$COMMENT_BODY", sleep briefly, re-fetch LAST_COMMENT via gh pr view ... --json
comments --jq '.comments[-1].body' and re-extract FIRST_LINE to confirm the
header is now present, and (4) surface an explicit warning/exit code if the
header still isn’t present after the retry; reference HAS_CR, LAST_COMMENT,
FIRST_LINE, COMMENT_BODY and the gh pr comment/view commands when locating where
to add this logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@plugins/skill-set/agents/pr-review-feedback.md`:
- Around line 239-249: The verification step currently only prints LAST_COMMENT
but does not enforce the CodeRabbit resolve header; update the script to (1)
when HAS_CR > 0 extract the first line (e.g., FIRST_LINE=$(echo "$LAST_COMMENT"
| head -1)), (2) test it for the literal "`@coderabbitai` resolve" (use grep -q or
a string comparison), (3) if missing, re-post the comment with gh pr comment
"$PR_NUMBER" --body "$COMMENT_BODY", sleep briefly, re-fetch LAST_COMMENT via gh
pr view ... --json comments --jq '.comments[-1].body' and re-extract FIRST_LINE
to confirm the header is now present, and (4) surface an explicit warning/exit
code if the header still isn’t present after the retry; reference HAS_CR,
LAST_COMMENT, FIRST_LINE, COMMENT_BODY and the gh pr comment/view commands when
locating where to add this logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 05cd9cef-9a45-4e04-aaac-1150dd1fcf24

📥 Commits

Reviewing files that changed from the base of the PR and between f0b9808 and d5d18d8.

📒 Files selected for processing (1)
  • plugins/skill-set/agents/pr-review-feedback.md

Step 5.3 of pr-review-feedback described the verification contract in
prose on line 248 (extract first line, check for the @coderabbitai
resolve marker, re-post once if missing, re-verify) but the bash
snippet beneath it only printed LAST_COMMENT and enforced none of it.

Replace the snippet with the full check-and-retry implementation so
the script is self-consistent with the prose:
- Extract FIRST_LINE from LAST_COMMENT when HAS_CR > 0
- grep for the @coderabbitai resolve marker
- Re-post the comment body and re-verify on failure
- Exit non-zero if the header is still absent after the retry

Resolves CodeRabbit outside-diff review comment on PR #25.
@ether-moon

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

PR 리뷰 코멘트 처리 결과

CodeRabbit 리뷰 코멘트 1건을 처리했습니다.

자동 적용 (OBVIOUS): 1건

  • plugins/skill-set/agents/pr-review-feedback.md (라인 239-249) — Step 5.3의 검증·재시도 스크립트를 라인 248의 문서화된 계약(첫 줄에서 @coderabbitai resolve 추출 → 없으면 재게시 → 재검증 → 그래도 없으면 오류 종료)에 맞춰 실제 bash로 구현. CodeRabbit이 제안한 스니펫을 기반으로 적용하되, 재시도 후에도 헤더가 없을 때는 exit 1로 명확히 실패를 노출하도록 보강. (reviewer: @coderabbitai)

논의 후 적용 (AMBIGUOUS): 0건

(없음)

건너뜀: 1건

  • CodeRabbit 워크스루(walkthrough) 자동 생성 코멘트 — 변경 사항 요약 및 pre-merge check 표시용 자동 코멘트로 실행 가능한 요청이 없어 건너뜀.

통계

  • 총 검토: 2건 (리뷰 1건 + PR 코멘트 1건)
  • 자동 적용: 1건
  • 논의 후 적용: 0건
  • 건너뜀: 1건

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List review threads on PR `#25` to find coderabbitai comments
gh api graphql -f query='
{
  repository(owner: "ether-moon", name: "skill-set") {
    pullRequest(number: 25) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              author { login }
              body
            }
          }
        }
      }
    }
  }
}'

Length of output: 70


@ether-moon: It looks like there are no open review threads from me on this PR — the thread list came back empty, so everything is already resolved (or no inline review threads were created). No further action needed on my end! 🐇

@ether-moon

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

PR 리뷰 처리 요약 (cycle 2)

리뷰어: @coderabbitai (Outside diff range comments, 2026-05-20T06:52:04Z)

자동 적용 (OBVIOUS)

(없음)

논의 후 적용 (AMBIGUOUS)

(없음)

건너뜀

  • plugins/skill-set/agents/pr-review-feedback.md 239-249 — 제안된 verify-and-retry 로직은 이미 구현되어 있습니다.
    CodeRabbit이 가리킨 검증 블록은 240-246줄 일부만 인용하지만, 동일 파일의 247-263줄에 다음이 이미 포함돼 있습니다:

    • if [ "$HAS_CR" -gt 0 ] 가드
    • FIRST_LINE=$(echo "$LAST_COMMENT" | head -1) 첫 줄 추출
    • grep -q "@coderabbitai resolve" 헤더 확인
    • 누락 시 gh pr comment ... --body "$COMMENT_BODY"로 재게시 → sleep 2 → 재조회 → 재검증
    • 재시도 후에도 헤더가 없으면 exit 1로 명시적 실패 처리 (CodeRabbit이 요청한 단순 echo보다 엄격)

    해당 로직은 이전 사이클 커밋 72245fd에서 이미 머지됐습니다. 이 사이클에서는 추가 변경 사항이 없습니다.

통계

  • 총 리뷰 항목: 1
  • 자동 적용: 0
  • 논의 후 적용: 0
  • 건너뜀 (이미 반영됨): 1

PR HEAD: 72245fd3c0cb00d13ef38e81db884c9c67ac7305 (변경 없음)

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List open review threads on PR `#25` from coderabbitai
gh api graphql -f query='
{
  repository(owner: "ether-moon", name: "skill-set") {
    pullRequest(number: 25) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              author { login }
              body
            }
          }
        }
      }
    }
  }
}'

Length of output: 70


@ether-moon: Confirmed — there are no open review threads from me on this PR. The thread list is empty, so everything is already resolved. Your explanation checks out: the verify-and-retry logic (the HAS_CR guard, first-line extraction, header check, re-post on miss, and exit 1 on second failure) was already in place at commit 72245fd before cycle 2 ran, making the suggestion a no-op. No further action needed! 🐇

@ether-moon

Copy link
Copy Markdown
Owner Author

@coderabbitai resolve

PR 리뷰 피드백 처리 결과

자동 적용 (OBVIOUS)

  • (없음)

논의 후 적용 (AMBIGUOUS)

  • (없음)

건너뜀 (SKIPPED)

  • plugins/skill-set/agents/pr-review-feedback.md (리뷰어: @coderabbitai, outside-diff 제안 lines 239-249) — 이미 반영됨. 현재 HEAD 72245fd의 247-263번 줄에 CodeRabbit이 제안한 verify-and-retry 스크립트가 그대로 구현되어 있으며, 재시도 후에도 헤더가 없으면 exit 1로 종료하는 추가 안전장치까지 포함되어 있습니다.

통계

  • 수집된 리뷰 항목: 1
  • 자동 적용: 0
  • 논의 후 적용: 0
  • 건너뜀 (이미 해결): 1

추가로 처리할 항목이 없어 새 커밋은 만들지 않았습니다. PR이 머지 가능한 상태입니다.

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List open review threads on PR `#25` from coderabbitai
gh api graphql -f query='
{
  repository(owner: "ether-moon", name: "skill-set") {
    pullRequest(number: 25) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              author { login }
              body
            }
          }
        }
      }
    }
  }
}'

Length of output: 70


@ether-moon: Confirmed again — there are no open review threads from me on this PR (thread list is empty). Your cycle 3 summary is accurate: the verify-and-retry logic is already fully implemented at lines 247–263 of commit 72245fd, with the exit 1 safety net on second failure going beyond what was originally suggested. No further action needed from my side! 🐇

@ether-moon
ether-moon merged commit 5038418 into main May 20, 2026
1 check passed
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