Further prompt engineering to reduce token usage for PR comment handler#3712
Conversation
📝 WalkthroughWalkthroughThis PR updates a GitHub Actions workflow that automatically processes review comments on [Test Improver] PRs. The lockfile reduces push-to-PR limits and removes WebFetch from allowed tools. The procedure and documentation were rewritten to process up to 3 oldest [Test Improver] PRs via a bash-generated digest, early-exit when no actionable feedback, rebase-based conflict resolution, digest-driven classification with Claude, targeted file edits, one batched commit per PR, standardized summary comments, and repo-memory updates limited to discovered build/test/lint commands. Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GitHubAPI
participant BashDigest
participant ClaudeAgent
participant RepoMemory
GitHubActions->>GitHubAPI: list up to 3 oldest "[Test Improver]" PRs
GitHubActions->>BashDigest: fetch review states & top-level comments (truncated)
BashDigest->>ClaudeAgent: send digest for classification
ClaudeAgent->>GitHubActions: return classifications (clear/ambiguous/out-of-scope)
GitHubActions->>GitHubAPI: get_file_contents for clear items
GitHubActions->>GitHubAPI: push batched commit & create summary comment
GitHubActions->>RepoMemory: update discovered build/test/lint commands
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/pr-comment-handler.md (1)
87-87: 💤 Low valueClarify the placeholder syntax.
The placeholder
PR_NUMBER=<from above>requires the agent to substitute the actual PR number from the previous script's output. Consider using a more explicit template variable syntax (e.g.,PR_NUMBER=REPLACE_MEorPR_NUMBER=$PR_NUM) to make it clearer that this is a variable to be replaced programmatically.🤖 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 @.github/workflows/pr-comment-handler.md at line 87, Clarify the placeholder assignment for PR_NUMBER by replacing the ambiguous literal "PR_NUMBER=<from above>" with an explicit template or variable reference so it's clear it must be programmatically substituted; update the snippet to use a clear placeholder like PR_NUMBER=REPLACE_ME or a shell variable such as PR_NUMBER=$PR_NUM (and document that $PR_NUM should come from the previous step's output or environment), and ensure any consuming steps refer to the same symbol (PR_NUMBER) so the pipeline injects the actual PR number automatically.
🤖 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.
Inline comments:
In @.github/workflows/pr-comment-handler.lock.yml:
- Line 1276: The workflow now sets GH_AW_NOOP_REPORT_AS_ISSUE="true", which will
turn every early-exit/no-op message (e.g., "No open [Test Improver] PRs found")
into a created issue; decide and implement the intended behavior: either set
GH_AW_NOOP_REPORT_AS_ISSUE back to "false" to preserve silent early exits, or
keep it true but add conditional logic to only enable it for
non-scheduled/manual runs (or when a new ENV flag is explicitly set), and update
any documentation; key symbols to change are the GH_AW_NOOP_REPORT_AS_ISSUE
environment variable and the workflow step that checks for the "No open [Test
Improver] PRs found" early-exit path so it only creates issues under the
intended conditions.
In @.github/workflows/pr-comment-handler.md:
- Around line 109-115: Early-exit currently calls noop which, when
GH_AW_NOOP_REPORT_AS_ISSUE is "true", will create GitHub issues; modify the Step
2 logic so before calling noop it checks the GH_AW_NOOP_REPORT_AS_ISSUE env var
and if it's "true" perform a silent exit (exit 0) or skip creating an issue (do
not call noop) while preserving the original messages ("No open [Test Improver]
PRs found." and "No actionable feedback found on [Test Improver] PRs.");
implement this conditional around the noop invocation so the workflow won’t
create issues on scheduled runs unless issue-reporting is explicitly enabled.
- Around line 94-99: The current jq heuristic `resolved: (has("original_line") |
not)` in the gh api call that fetches PR comments incorrectly infers resolution
from `original_line`; replace it by fetching thread-level resolution
(PullRequestReviewThread.isResolved) and joining that data to individual
comments: either call the GraphQL API to query `pullRequest(number: $PR_NUMBER)
{ reviewThreads { nodes { id isResolved comments { nodes { id } } } } }` and map
thread isResolved to comment ids, or call the REST endpoint
`repos/.../pulls/$PR_NUMBER/review_threads` (via gh api) and merge `is_resolved`
into your comment objects instead of using `original_line`; remove the
`resolved: (has("original_line") | not)` expression and use the thread-level
`isResolved`/`is_resolved` value to set `resolved`.
---
Nitpick comments:
In @.github/workflows/pr-comment-handler.md:
- Line 87: Clarify the placeholder assignment for PR_NUMBER by replacing the
ambiguous literal "PR_NUMBER=<from above>" with an explicit template or variable
reference so it's clear it must be programmatically substituted; update the
snippet to use a clear placeholder like PR_NUMBER=REPLACE_ME or a shell variable
such as PR_NUMBER=$PR_NUM (and document that $PR_NUM should come from the
previous step's output or environment), and ensure any consuming steps refer to
the same symbol (PR_NUMBER) so the pipeline injects the actual PR number
automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d83efec1-08dc-41b5-9214-0bf4cba471fb
📒 Files selected for processing (2)
.github/workflows/pr-comment-handler.lock.yml.github/workflows/pr-comment-handler.md
| GH_AW_AGENT_CONCLUSION: ${{ needs.agent.result }} | ||
| GH_AW_NOOP_MESSAGE: ${{ steps.noop.outputs.noop_message }} | ||
| GH_AW_NOOP_REPORT_AS_ISSUE: "false" | ||
| GH_AW_NOOP_REPORT_AS_ISSUE: "true" |
There was a problem hiding this comment.
Behavior change: no-op messages will now create issues.
Setting GH_AW_NOOP_REPORT_AS_ISSUE to "true" means that when the workflow exits early (e.g., "No open [Test Improver] PRs found"), it will create a GitHub issue rather than silently completing. This increases visibility but may generate noise if the workflow runs frequently on a schedule with no PRs to process.
Confirm this is the intended behavior for early-exit scenarios.
🤖 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 @.github/workflows/pr-comment-handler.lock.yml at line 1276, The workflow now
sets GH_AW_NOOP_REPORT_AS_ISSUE="true", which will turn every early-exit/no-op
message (e.g., "No open [Test Improver] PRs found") into a created issue; decide
and implement the intended behavior: either set GH_AW_NOOP_REPORT_AS_ISSUE back
to "false" to preserve silent early exits, or keep it true but add conditional
logic to only enable it for non-scheduled/manual runs (or when a new ENV flag is
explicitly set), and update any documentation; key symbols to change are the
GH_AW_NOOP_REPORT_AS_ISSUE environment variable and the workflow step that
checks for the "No open [Test Improver] PRs found" early-exit path so it only
creates issues under the intended conditions.
| ## Step 2: Early Exit | ||
|
|
||
| 1. Check out the PR's head branch locally. | ||
| 2. Fetch the base branch (usually `master` or `main`) and rebase the PR branch onto it: | ||
| - Prefer rebase over merge to keep a clean history. | ||
| - If rebase is not safe (e.g., the branch has been force-pushed by a human), skip and post a comment asking for human assistance. | ||
| 3. Resolve any conflicts by favouring the intent of the PR's changes: | ||
| - Read the conflicting files carefully before resolving. | ||
| - If a conflict is in a test file added by Test Improver, keep the test changes and integrate with whatever changed on the base branch. | ||
| - If a conflict is ambiguous or risky (e.g., both sides changed the same logic significantly), do not guess — post a comment describing the conflict and ask for human help. | ||
| 4. After resolving, run the relevant tests to confirm nothing is broken. | ||
| 5. Commit with: | ||
| ``` | ||
| Resolve merge conflicts with base branch | ||
| If the bash output shows zero `[Test Improver]` PRs, call `noop` with message | ||
| "No open [Test Improver] PRs found." and stop immediately. | ||
|
|
||
| 🤖 PR Comment Handler | ||
| ``` | ||
| 6. Push to the PR branch. | ||
| 7. Post a brief comment on the PR: | ||
| ``` | ||
| 🤖 *PR Comment Handler here — I've rebased this branch onto the latest base branch to resolve merge conflicts. Please re-review if the conflict resolution affected your area of interest.* | ||
| ``` | ||
| If all PRs have no unresolved comments and no merge conflicts, call `noop` with | ||
| message "No actionable feedback found on [Test Improver] PRs." and stop. |
There was a problem hiding this comment.
Early exit will now create issues due to noop reporting change.
These early-exit conditions correctly implement the token-saving strategy from the PR objectives. However, note that with GH_AW_NOOP_REPORT_AS_ISSUE: "true" (line 1276 in the .lock.yml file), each early exit will create a GitHub issue rather than silently completing.
If this workflow runs on a daily schedule and frequently finds no work to do, it may generate many issues. Verify this is acceptable for your notification/issue tracking workflow.
🤖 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 @.github/workflows/pr-comment-handler.md around lines 109 - 115, Early-exit
currently calls noop which, when GH_AW_NOOP_REPORT_AS_ISSUE is "true", will
create GitHub issues; modify the Step 2 logic so before calling noop it checks
the GH_AW_NOOP_REPORT_AS_ISSUE env var and if it's "true" perform a silent exit
(exit 0) or skip creating an issue (do not call noop) while preserving the
original messages ("No open [Test Improver] PRs found." and "No actionable
feedback found on [Test Improver] PRs."); implement this conditional around the
noop invocation so the workflow won’t create issues on scheduled runs unless
issue-reporting is explicitly enabled.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3712 +/- ##
=========================================
Coverage 23.31% 23.31%
- Complexity 3897 3898 +1
=========================================
Files 921 921
Lines 55996 55996
Branches 6635 6635
=========================================
Hits 13055 13055
Misses 41249 41249
Partials 1692 1692 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
conroy-ricketts
left a comment
There was a problem hiding this comment.
Looks like the coderabbit comments may be worth addressing
b99a3c7
|
@coderabbitai full review please |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pr-comment-handler.md (2)
169-169: 💤 Low valueAdd language identifier to fenced code block.
The code block starting at line 169 lacks a language identifier. Adding
```markdownwould satisfy markdownlint and improve documentation rendering.🤖 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 @.github/workflows/pr-comment-handler.md at line 169, The markdown fenced code block in .github/workflows/pr-comment-handler.md currently uses plain backticks (```) with no language tag; update that fence to include the markdown language identifier by replacing the opening triple backticks with ```markdown so linting (markdownlint) and rendering correctly recognize the block as Markdown.
134-134: 💤 Low valueUse generic reference to base branch.
The phrase "rebase onto master" assumes the base branch is named
master, but it could bemainor another name. Consider rewording to "rebase onto the target base branch" or "rebase onto the PR's base branch" for clarity.🤖 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 @.github/workflows/pr-comment-handler.md at line 134, Update the instruction text that currently reads "Check out the PR branch and rebase onto master" to use a generic reference to the repository base branch (e.g., "rebase onto the PR's base branch" or "rebase onto the target base branch"); locate the exact string "Check out the PR branch and rebase onto master" and replace it with the new generic wording so it no longer assumes the base branch is named master.
🤖 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.
Inline comments:
In @.github/workflows/pr-comment-handler.md:
- Around line 130-164: The docs ambiguity: Steps "Step 4: Resolve Merge
Conflicts" and "Step 5: Classify and Implement Comments" imply two separate
pushes per PR which conflicts with the workflow's max: 3 PR limit; update the
procedure text to explicitly state the intended push semantics (either: always
batch conflict resolution + feedback into a single commit/push per PR, or allow
an extra push and reduce the max PRs processed), and add one clear rule (e.g.,
"Batch conflict resolution and subsequent feedback changes into one commit and
push per PR" or "If conflicts are resolved separately, reduce the workflow's max
processed PRs accordingly"). Edit the headings and bullet steps in Step 4 and
Step 5 to reflect that rule and adjust the sentence "Push once per PR" to the
chosen behavior so readers know whether Steps 4 and 5 are mutually exclusive;
ensure the change references the phrases "Resolve merge conflicts" and "Push
once per PR" so reviewers can find and validate the update.
---
Nitpick comments:
In @.github/workflows/pr-comment-handler.md:
- Line 169: The markdown fenced code block in
.github/workflows/pr-comment-handler.md currently uses plain backticks (```)
with no language tag; update that fence to include the markdown language
identifier by replacing the opening triple backticks with ```markdown so linting
(markdownlint) and rendering correctly recognize the block as Markdown.
- Line 134: Update the instruction text that currently reads "Check out the PR
branch and rebase onto master" to use a generic reference to the repository base
branch (e.g., "rebase onto the PR's base branch" or "rebase onto the target base
branch"); locate the exact string "Check out the PR branch and rebase onto
master" and replace it with the new generic wording so it no longer assumes the
base branch is named master.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dd0f2d49-2f52-4728-b194-54287a6a88f7
📒 Files selected for processing (2)
.github/workflows/pr-comment-handler.lock.yml.github/workflows/pr-comment-handler.md
Technical Description
Optimizes the PR Comment Handler workflow to reduce Anthropic API token usage, which was identified as consuming ~8M tokens/day when running every 4 hours (reduced to ~1.3M/day after the previous PR shifted it to daily). This PR focuses on reducing tokens per run.
Changes
ghCLI bash scripts first to collect a compact digest of PR data (comment bodies truncated to 300 chars) before making any GitHub MCP tool calls. Full file contents are only fetched for comments the agent has already classified as actionable, rather than loading all diffs upfront.[Test Improver]PRs exist or none have actionable feedback, the agent exits immediately after the bash step — zero GitHub MCP calls, minimal token usage.[Test Improver]PRs per run, prioritising oldest first. Previously unbounded.push-to-pull-request-branchmax reduced to 3: Aligns with the new 3-PR cap (one push per PR per run).Expected impact
The per-call input token count should drop significantly (estimated 40–60%) since large PR diffs and full comment threads are no longer loaded into the LLM context during the scanning phase. The saving compounds across all sequential API calls within a run since each call re-sends the full conversation history.
Also noting that this will require us to review test Improver PRs from oldest to newest, so that this workflow can pick up the related comments.