Skip to content

feat(cursor-review): reuse check-pr-size classifier + discount comments in the size gate#59

Open
mattmillerai wants to merge 3 commits into
mainfrom
matt/cursor-review-reuse-pr-size-codegen
Open

feat(cursor-review): reuse check-pr-size classifier + discount comments in the size gate#59
mattmillerai wants to merge 3 commits into
mainfrom
matt/cursor-review-reuse-pr-size-codegen

Conversation

@mattmillerai

@mattmillerai mattmillerai commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Two related changes so cursor-review's size gate counts only the lines a human/AI reviewer actually needs to read: (1) reuse the PR-size classifier so generated code stops counting and stops being reviewed, and (2) discount blank/comment lines from the count so AI comment volume doesn't trip the cap.

ELI5

When a PR is huge, cursor-review skips it (too big for one useful pass), and whatever it does review, it feeds the models the whole diff. Two things inflate "huge": generated code (nobody hand-writes or reviews it) and, increasingly, AI-written comments (verbose, low-risk). Both were counting against the size limit and — for codegen — getting shoved at the review models.

This PR teaches the size gate to ignore both: codegen via the same detector the PR-size cap already uses, and comments via a per-language heuristic. Codegen is dropped from the reviewed diff too; comments are kept in the diff (a reviewer should still catch a wrong or misleading comment) — only the count ignores them.

Part 1 — reuse the classifier for codegen (commit 1)

cursor-review kept its own hand-typed diff_excludes pathspec list, weaker than and drifting from the PR-size cap's classifier (it matched filenames only, missing base-ref linguist-generated and the Go // Code generated … DO NOT EDIT. marker, while blunt directory excludes hid hand-written files).

  • check-pr-size gains an opt-in --generated-out flag (emits the classified generated paths; size verdict unchanged, pr-size.yml untouched).
  • diff-size builds/runs the tool once: over_cap drives within_cap, and the generated-paths list drives a single reviewed-diff artifact with codegen excluded.
  • Panel cells + judge consume that one artifact → no PR checkout in those jobs.
  • New extra_generated_globs / extra_lockfiles inputs mirror pr-size.yml; diff_excludes stays as a diff-only back-compat escape hatch.

Part 2 — discount blank/comment lines from the count (commit 2)

  • check-pr-size gains --ignore-comments (env PR_SIZE_IGNORE_COMMENTS): blank and comment-only changed lines are subtracted from the counted total. FileChange.Discounted carries the per-file count; Counted() = Changed() − Discounted, default 0 → behavior unchanged when off.
  • Heuristic, count-only: per-language single-line comment markers (//, #, --, <!-- -->) + single-line block comments + blanks. Multi-line block-comment bodies and string literals aren't lexed (documented); a comment token merely contained in code still counts. Generated files are never inspected; the reviewed diff is untouched.
  • Fails safe: any git/parse error leaves the count raw — a bug can only make the cap stricter, never sneak a big change under it.
  • cursor-review enables it via a new ignore_comments input (default true).

Behavior / back-compat

  • Both features are additive/opt-in at the tool level; pr-size.yml is not modified (the comment-discount capability is available for it to adopt later).
  • The old diff_excludes default (lockfiles + vendored dirs) now lives in the classifier built-ins + the extra_generated_globs default → equivalent-or-better exclusion, no diff-content change for default callers.
  • Migration for callers overriding diff_excludes: move generated entries into extra_generated_globs so they leave the size gate too. Bump the pinned SHA at the same time. Detector-caught codegen (Go markers, base-ref linguist-generated) needs no change.

Validation

  • go test ./... in scripts/check-pr-size — new suites (ParseDiscounts, IsInsignificantLine, CommentSyntaxFor, EvaluateDiscount, AnnotateDiscountsGit, WriteGeneratedPaths) + existing pass ✅
  • gofmt -l clean, go vet clean ✅
  • python3 -m unittest discover .github/cursor-review/tests (40 tests, unchanged) ✅
  • actionlint .github/workflows/cursor-review.yml (incl. shellcheck) ✅
  • Smoke: a synthetic PR of 5 code + 30 comment + 5 blank lines counts 41 → over a 20 cap without the flag, 5 (36 discounted) → passes with it; and a mockgen file caught only by the Go marker is excluded while the hand-written file survives the reviewed diff.

Coordination — two in-flight PRs touch this file

Draft until we pick the merge order.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c2d4cf3f-2753-450c-842c-0fc90e201e06

📥 Commits

Reviewing files that changed from the base of the PR and between 9a7e853 and ec880db.

📒 Files selected for processing (7)
  • .github/cursor-review/README.md
  • .github/workflows/cursor-review.yml
  • README.md
  • scripts/check-pr-size/main.go
  • scripts/check-pr-size/main_test.go
  • scripts/check-pr-size/size.go
  • scripts/check-pr-size/size_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch matt/cursor-review-reuse-pr-size-codegen
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch matt/cursor-review-reuse-pr-size-codegen

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

@mattmillerai mattmillerai added the cursor-review Multi-model cursor review label Jul 22, 2026
…n the size gate and the reviewed diff

cursor-review kept its own hand-maintained diff_excludes pathspec list to
decide what codegen to skip — weaker than, and drifting from, the PR-size
cap's classifier. It matched filename globs only, so generated files caught by
the cap's base-ref linguist-generated attribute or the canonical Go
`// Code generated ... DO NOT EDIT.` marker still counted toward the review
size cap and got fed to the panel, while blunt directory excludes hid
hand-written files that should have been reviewed.

Make the shared scripts/check-pr-size tool the single source of truth for
"what is codegen" in cursor-review too:

- check-pr-size gains an opt-in --generated-out flag that writes the classified
  generated paths (NUL-separated). The size verdict is unchanged; pr-size.yml is
  unaffected (additive, default off).
- diff-size builds and runs the tool once (GOWORK=off, mirroring pr-size.yml):
  its over_cap output (counted vs --max = DIFF_SIZE_CAP) drives within_cap, and
  the generated-paths list drives a single reviewed-diff artifact that excludes
  every classified generated file.
- the panel cells and the judge consume that one artifact instead of each
  regenerating the diff, so they need no PR checkout — the only PR-authored
  content they see is the diff text they review.
- new extra_generated_globs / extra_lockfiles inputs mirror pr-size and feed the
  classifier (count + diff). diff_excludes stays as a diff-only, back-compat
  escape hatch; its old lockfile/vendored default moves into the classifier and
  extra_generated_globs.

Fails open: if the tool errors, over_cap is unset (within_cap=true) and the diff
falls back to diff_excludes-only — never a silent skip.

Callers that listed generated paths in diff_excludes should move them to
extra_generated_globs to keep them out of the size gate too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mattmillerai
mattmillerai force-pushed the matt/cursor-review-reuse-pr-size-codegen branch from e9cda35 to 4077d73 Compare July 22, 2026 01:29

@github-actions github-actions 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.

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 8 finding(s).

Severity Count
🟠 High 1
🟡 Medium 5
🟢 Low 2

Panel: 8/8 reviewers contributed findings.

Comment thread .github/workflows/cursor-review.yml Outdated
Comment thread scripts/check-pr-size/main.go
Comment thread .github/workflows/cursor-review.yml Outdated
Comment thread .github/workflows/cursor-review.yml
Comment thread .github/workflows/cursor-review.yml
Comment thread .github/workflows/cursor-review.yml Outdated
Comment thread scripts/check-pr-size/main.go Outdated
Comment thread scripts/check-pr-size/main.go Outdated
@mattmillerai
mattmillerai marked this pull request as ready for review July 22, 2026 02:30
… (opt-in)

AI-generated code now leaves a non-trivial volume of comments, which inflate the
changed-line count and can push a PR over the review cap — skipping the review of
what may be modest actual logic.

Add an opt-in `--ignore-comments` flag (env PR_SIZE_IGNORE_COMMENTS) that excludes
blank and comment-only changed lines from the counted total:

- annotateDiscounts fetches the patch for the counted (non-generated, non-binary)
  paths only, and ParseDiscounts tallies each file's blank/comment-only added and
  removed lines. FileChange.Discounted carries the per-file total; Counted() =
  Changed() - Discounted (clamped ≥ 0). Default 0, so behavior is unchanged when
  the flag is off — Counted() == Changed() for every existing path and test.
- Heuristic and count-only by design: a per-language, single-line comment-marker
  table (C-family //, hash #, SQL/Lua --, HTML <!-- -->) plus single-line block
  comments and blank lines. Multi-line block-comment bodies and string literals
  are NOT lexed (documented limitation). A line whose content merely CONTAINS a
  comment token (e.g. a string literal) still counts. Generated files are never
  inspected, and the reviewed diff is untouched — only the count changes.
- Fails safe: any git/parse error leaves Discounted at 0 (count raw), so a bug can
  only make the cap stricter, never sneak a large change under it.

cursor-review enables it via a new `ignore_comments` input (default true): AI
comment volume no longer trips the diff-size gate and skips the review, but the
panel still sees the comments — so it can still flag wrong or misleading ones.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mattmillerai mattmillerai changed the title feat(cursor-review): reuse check-pr-size classifier to skip codegen in the size gate and reviewed diff feat(cursor-review): reuse check-pr-size classifier + discount comments in the size gate Jul 22, 2026
Address all 8 cursor-review panel threads on #59:

- Go generated marker is now honored only from the BASE blob for the
  review gate (new --marker-from-base flag): a PR can no longer hide
  hand-written code from the panel by prepending the one-line marker,
  matching the base-ref invariant of the linguist-generated path.
  pr-size.yml (flag off) is unchanged.
- The reviewed diff is built by the tool itself (--reviewed-diff-out):
  one full git diff streamed through a section filter keyed on the
  classified set (FilterPatch). No per-file argv pathspecs, so an
  unbounded generated set can't overflow ARG_MAX; a generated rename
  drops as one section (no orphaned old-path deletion); and a write
  failure exits 2 so counts are never paired with a missing/partial
  diff. (git diff has no --pathspec-from-file, hence post-filtering.)
- Degraded-path semantics made explicit and accurate: git-level
  breakage fails the diff-size job (fail closed); tool-level failure
  falls back to a RAW no-exclusions count gate before building the
  diff_excludes-only diff, so a degraded run can never feed an
  unbounded diff to the eight panel jobs.
- classify() applies path/attr rules to binary files (content read
  still skipped), so a binary matching extra_generated_globs (e.g.
  data/object_info.json.gz) leaves the reviewed diff again.
- globRegexp compiles **/ to (?:.*/)? (gitignore semantics): root-level
  node_modules/, dist/, *.min.js now match the default excludes.
- Pin every remaining mutable action tag in cursor-review.yml
  (checkout v6.0.3, upload-artifact v7.0.1, download-artifact v8.0.1,
  create-github-app-token v3.2.0).

Validation: go test ./... (new FilterPatch/parseDiffGitPaths/
marker-from-base/binary-extras/glob suites), gofmt, go vet, actionlint
(incl. shellcheck), python unittest (40 cursor-review tests), plus an
end-to-end smoke: head-added-marker attack file is counted AND
reviewed; base-generated mock excluded; root node_modules excluded;
diff_excludes stays diff-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mattmillerai

Copy link
Copy Markdown
Contributor Author

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

  • BE-4057 — Remove the committed check-pr-size binary from the repo and gitignore it

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

Labels

cursor-review Multi-model cursor review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant