Skip to content

Fix Claude Code Review CI quota exhaustion#1353

Merged
sbryngelson merged 5 commits intoMFlowCode:masterfrom
sbryngelson:fix-claude-review-ci
Apr 4, 2026
Merged

Fix Claude Code Review CI quota exhaustion#1353
sbryngelson merged 5 commits intoMFlowCode:masterfrom
sbryngelson:fix-claude-review-ci

Conversation

@sbryngelson
Copy link
Copy Markdown
Member

@sbryngelson sbryngelson commented Apr 4, 2026

Summary

  • Remove synchronize event trigger that caused the Claude review to run on every push to a PR branch, exhausting the API quota (You've hit your limit errors)
  • Remove all incremental review mode logic (dead code after synchronize removal)
  • Filter out tests/ directory from review inputs — golden files don't need code review

Test plan

  • Verify the Claude Code Review workflow runs on PR open/reopen/ready_for_review/label events
  • Verify the workflow does NOT run on push (synchronize) events
  • Verify PRs that only change tests/ files produce an empty review

Remove the synchronize event trigger that was causing the review to run on
every push to a PR branch, exhausting the API quota. Also remove all
incremental review mode logic (now dead code) and filter out tests/
directory from review inputs since golden files don't need code review.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Claude Code Review

Head SHA: 8cf4f11

Files changed:

  • 1
  • .github/workflows/claude-code-review.yml

Findings:

  • Missing guard for all-tests PR: The previous workflow had an explicit empty-diff fallback guard (if [[ ! -s .claude-review/review.diff ]] || [[ ! -s .claude-review/changed_files.txt ]]) that was removed in this PR. Combined with the new tests/ filtering, a PR that only modifies files under tests/ will produce empty review.diff and changed_files.txt after filtering — Claude then runs with no inputs, wastes CI minutes, and produces no useful review. A short-circuit exit before invoking Claude (e.g., checking if review.diff is empty after filtering) would prevent this.

@sbryngelson sbryngelson marked this pull request as ready for review April 4, 2026 13:44
@sbryngelson sbryngelson merged commit 8bf2296 into MFlowCode:master Apr 4, 2026
19 of 28 checks passed
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix Claude Code Review CI quota exhaustion and streamline workflow

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Remove synchronize event trigger causing quota exhaustion
• Eliminate incremental review mode logic (now dead code)
• Filter tests/ directory from review inputs
• Prioritize src/ files for context and increase file size cap
• Allow Claude review to soft-fail without blocking CI
Diagram
flowchart LR
  A["Remove synchronize trigger"] --> B["Eliminate incremental mode"]
  B --> C["Filter tests/ directory"]
  C --> D["Prioritize src/ context"]
  D --> E["Soft-fail on error"]
  E --> F["Simplified review prompt"]
Loading

Grey Divider

File Changes

1. .github/workflows/claude-code-review.yml 🐞 Bug fix +30/-89

Fix quota exhaustion and streamline review workflow

• Removed synchronize from pull_request_target event types to prevent quota exhaustion
• Removed all incremental review mode logic including mode determination and delta diff comparison
• Added filtering to exclude tests/ directory from both diff and changed files list
• Prioritized src/ files for context fetching and increased file size cap from 400 to 1500 lines
• Reduced max-turns from 90 to 40 and added continue-on-error: true for soft failure
• Simplified review prompt by removing mode-related variables and consolidating output format
• Added inline review priorities based on CLAUDE.md standards

.github/workflows/claude-code-review.yml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Apr 4, 2026

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Rename diff wrongly dropped 🐞 Bug ≡ Correctness
Description
The AWK filter strips any diff whose *old* path starts with tests/ (diff --git a/tests/...),
which will incorrectly drop diffs for renames/moves from tests/ to non-tests/ paths (e.g.,
tests/foo -> src/foo). This can cause the review to miss real (non-test) changes while
changed_files.txt still includes the new non-test path.
Code

.github/workflows/claude-code-review.yml[R116-120]

+          awk '
+            /^diff --git a\/tests\// { skip=1; next }
+            /^diff --git /           { skip=0 }
+            !skip                    { print }
+          ' .claude-review/review.diff.raw > .claude-review/review.diff
Evidence
The workflow filters the changed-file list by current path (grep -v '^tests/'), but strips diff
sections by matching a/tests/ in the unified diff header. In unified diffs, a/ is the old path
and b/ is the new path, so renames from tests/ to src/ will be removed from the diff even
though the file is not filtered out of changed_files.txt.

.github/workflows/claude-code-review.yml[110-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The AWK rule that strips `tests/` diffs matches `diff --git a/tests/...`, which incorrectly removes diffs for files renamed/moved from `tests/` to non-`tests/` paths.

### Issue Context
`changed_files.txt` is filtered based on the post-change path list (`.files[].path`), so the diff filtering should key off the *new* path (`b/...`) to stay consistent.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[113-120]

### Suggested change
Update the AWK logic to set `skip` when the `diff --git` header indicates the **new** path is under `tests/` (e.g., match `b/tests/`). For example:
- On `^diff --git ` lines, compute `skip = ($0 ~ /^diff --git a\/[^ ]+ b\/tests\//)`
- Print lines only when `skip==0`
This keeps renames like `tests/foo -> src/foo` in the diff while excluding files whose new location is `tests/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Hidden Claude step failures 🐞 Bug ✧ Quality
Description
continue-on-error: true on the Claude step prevents the job from failing, so the existing `if:
failure()` fallback summary will never run and Claude outages/quota failures become silent. This can
leave PRs appearing “successfully reviewed” while no review is published.
Code

.github/workflows/claude-code-review.yml[R175-177]

      - name: Run Claude Code Review
+        continue-on-error: true
        uses: anthropics/claude-code-action@v1
Evidence
The workflow explicitly enables continue-on-error: true for the Claude action, and the only
failure-reporting mechanism at the end is gated on if: failure(). With continue-on-error, Claude
failures do not set the job to a failing state, so the fallback step won’t execute.

.github/workflows/claude-code-review.yml[175-177]
.github/workflows/claude-code-review.yml[304-315]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`continue-on-error: true` makes Claude failures non-fatal, but then `if: failure()` never triggers, so failures are not surfaced anywhere.

### Issue Context
You likely want to avoid blocking CI, but still need visibility when the review step fails (quota/auth/outage).

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[175-177]
- .github/workflows/claude-code-review.yml[266-315]

### Suggested change
1) Give the Claude step an `id` (e.g., `id: claude`).
2) Add a follow-up step that runs **always** (`if: always()`) and checks `steps.claude.outcome` / `steps.claude.conclusion`.
3) If it failed, write a clear message to `$GITHUB_STEP_SUMMARY` (and/or post/update the primary comment with an error marker) so failures are visible even though the job stays green.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Unstable context file ordering 🐞 Bug ☼ Reliability
Description
The src/ prioritization uses for(i in rest) to print non-src/ files from an AWK associative
array, which is not order-stable. With the later 10-file cap, the set/order of non-src/ context
files can vary across runs for the same PR.
Code

.github/workflows/claude-code-review.yml[R141-144]

+          # Prioritize src/ files for context (most likely to need review)
+          sort -t/ -k1,1 -s < "${{ steps.review_input.outputs.changed_files_path }}" | \
+            awk '/^src\//{print; next} {rest[NR]=$0} END{for(i in rest) print rest[i]}' > .claude-review/sorted_files.txt
+
Evidence
The workflow builds .claude-review/sorted_files.txt by printing src/ files immediately, but
stores all other paths in an associative array and prints them via for(i in rest), which does not
guarantee numeric key order. Because the subsequent loop stops after 10 files, this can change which
non-src/ files are included as context between runs.

.github/workflows/claude-code-review.yml[141-149]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Non-`src/` paths are emitted in non-deterministic order due to `for(i in rest)` over an AWK associative array.

### Issue Context
Only the first 10 files are fetched for context, so ordering impacts which files get included.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[141-149]

### Suggested change
Ensure deterministic ordering by preserving numeric order, e.g.:
- Use `END { for (i=1; i<=NR; i++) if (i in rest) print rest[i] }`
Or avoid associative-array iteration entirely by producing a sortable key and using `sort` (e.g., prefix `0	` for `src/`, `1	` for others, then sort and strip the prefix).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Claude runs on empty inputs 🐞 Bug ➹ Performance
Description
After filtering out tests/, the workflow can produce an empty changed_files.txt and/or
review.diff but still always invokes the Claude action, spending quota even though the publish
step will later skip posting. This undermines the quota-reduction goal for test-only PRs.
Code

.github/workflows/claude-code-review.yml[R113-125]

+          # Filter out tests/ directory — golden files don't need code review
+          grep -v '^tests/' .claude-review/changed_files.txt.raw > .claude-review/changed_files.txt || true
+          # Strip diff hunks for tests/ files
+          awk '
+            /^diff --git a\/tests\// { skip=1; next }
+            /^diff --git /           { skip=0 }
+            !skip                    { print }
+          ' .claude-review/review.diff.raw > .claude-review/review.diff

          sed -i '/^[[:space:]]*$/d' .claude-review/changed_files.txt || true

-          if [[ ! -s .claude-review/review.diff ]] || [[ ! -s .claude-review/changed_files.txt ]]; then
-            echo "Prepared diff or changed file list is empty; falling back to full review."
-            build_full
-            sed -i '/^[[:space:]]*$/d' .claude-review/changed_files.txt || true
-          fi
-
          echo "review_diff_path=.claude-review/review.diff" >> "$GITHUB_OUTPUT"
          echo "changed_files_path=.claude-review/changed_files.txt" >> "$GITHUB_OUTPUT"
Evidence
The workflow filters tests/ out of both the changed-file list and the diff, but does not add any
gating if: or shell check to skip the Claude step when the filtered files are empty. The publish
step explicitly skips posting when output is empty, which confirms the pipeline expects “no review”,
but quota is still consumed by running Claude.

.github/workflows/claude-code-review.yml[110-125]
.github/workflows/claude-code-review.yml[175-199]
.github/workflows/claude-code-review.yml[283-286]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Claude is invoked even when the filtered diff / file list is empty (e.g., PR only touches `tests/`).

### Issue Context
The publish step already treats empty output as “nothing to publish”, so it’s safe (and cheaper) to short-circuit before calling Claude.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[97-126]
- .github/workflows/claude-code-review.yml[175-177]

### Suggested change
Add a step after `Build review diff...` to detect emptiness:
- If `changed_files.txt` is empty OR `review.diff` is empty, write a message to `$GITHUB_STEP_SUMMARY` and exit 0.
Then gate the Claude step with `if:` using an output from that check (e.g., `steps.review_input_check.outputs.should_run == 'true'`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

5. Context size cap increased 🐞 Bug ➹ Performance
Description
Raising the per-file context inclusion limit from 400 to 1500 lines can significantly increase
tokens sent to Claude, increasing latency and quota usage. This change may partially offset the
intended quota savings from reduced triggers.
Code

.github/workflows/claude-code-review.yml[R161-164]

+            if [[ "$LINE_COUNT" -le 1500 ]]; then
              printf '%s' "$CONTENT" > ".claude-review/context/$SAFE_NAME"
            fi
-          done < "${{ steps.review_input.outputs.changed_files_path }}"
+          done < .claude-review/sorted_files.txt
Evidence
The workflow now writes full-file content into .claude-review/context/ for files up to 1500 lines,
which directly increases prompt size when those files are included (up to 10 files). Larger inputs
generally increase model runtime and consumption.

.github/workflows/claude-code-review.yml[157-164]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The context capture threshold was increased to 1500 lines, which can increase token usage and quota.

### Issue Context
This workflow’s primary pain point is quota exhaustion; larger context may counteract savings.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[157-164]

### Suggested change
Consider reverting closer to the prior limit (e.g., 400) or making it conditional (e.g., higher only for `src/` files, or cap by bytes/tokens rather than lines).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@sbryngelson sbryngelson deleted the fix-claude-review-ci branch April 4, 2026 13:44
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0eede749-e01b-46c4-b1ab-7ade00cb3e5a

📥 Commits

Reviewing files that changed from the base of the PR and between 5bdc23e and 8cf4f11.

📒 Files selected for processing (1)
  • .github/workflows/claude-code-review.yml

📝 Walkthrough

Walkthrough

This pull request modifies the GitHub Actions workflow for code review by simplifying its trigger logic and review mechanism. The workflow's pull_request_target trigger no longer runs on synchronize events. Review mode selection logic and delta-diff preparation have been removed entirely. The workflow now consistently uses the full PR diff via gh pr diff and filters the changed-files list to exclude tests/ paths. File context selection was adjusted to prioritize src/ paths and increase the maximum file length threshold from 400 to 1500 lines. The Claude API invocation was updated with continue-on-error: true, reduced --max-turns from 90 to 40, and simplified prompting for full-diff review only.


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.

Comment on lines +116 to +120
awk '
/^diff --git a\/tests\// { skip=1; next }
/^diff --git / { skip=0 }
!skip { print }
' .claude-review/review.diff.raw > .claude-review/review.diff
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Rename diff wrongly dropped 🐞 Bug ≡ Correctness

The AWK filter strips any diff whose *old* path starts with tests/ (diff --git a/tests/...),
which will incorrectly drop diffs for renames/moves from tests/ to non-tests/ paths (e.g.,
tests/foo -> src/foo). This can cause the review to miss real (non-test) changes while
changed_files.txt still includes the new non-test path.
Agent Prompt
### Issue description
The AWK rule that strips `tests/` diffs matches `diff --git a/tests/...`, which incorrectly removes diffs for files renamed/moved from `tests/` to non-`tests/` paths.

### Issue Context
`changed_files.txt` is filtered based on the post-change path list (`.files[].path`), so the diff filtering should key off the *new* path (`b/...`) to stay consistent.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[113-120]

### Suggested change
Update the AWK logic to set `skip` when the `diff --git` header indicates the **new** path is under `tests/` (e.g., match `b/tests/`). For example:
- On `^diff --git ` lines, compute `skip = ($0 ~ /^diff --git a\/[^ ]+ b\/tests\//)`
- Print lines only when `skip==0`
This keeps renames like `tests/foo -> src/foo` in the diff while excluding files whose new location is `tests/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 175 to 177
- name: Run Claude Code Review
continue-on-error: true
uses: anthropics/claude-code-action@v1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Hidden claude step failures 🐞 Bug ✧ Quality

continue-on-error: true on the Claude step prevents the job from failing, so the existing `if:
failure()` fallback summary will never run and Claude outages/quota failures become silent. This can
leave PRs appearing “successfully reviewed” while no review is published.
Agent Prompt
### Issue description
`continue-on-error: true` makes Claude failures non-fatal, but then `if: failure()` never triggers, so failures are not surfaced anywhere.

### Issue Context
You likely want to avoid blocking CI, but still need visibility when the review step fails (quota/auth/outage).

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[175-177]
- .github/workflows/claude-code-review.yml[266-315]

### Suggested change
1) Give the Claude step an `id` (e.g., `id: claude`).
2) Add a follow-up step that runs **always** (`if: always()`) and checks `steps.claude.outcome` / `steps.claude.conclusion`.
3) If it failed, write a clear message to `$GITHUB_STEP_SUMMARY` (and/or post/update the primary comment with an error marker) so failures are visible even though the job stays green.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant