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

New issue

Closed

opened 2026-10-26 22:35:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/59
Author: @mattmillerai
Created: 7/22/2026
Status: ✅ Merged
Merged: 8/11/2026
Merged by: @mattmillerai

Base: main ← Head: matt/cursor-review-reuse-pr-size-codegen

📝 Commits (9)

4077d73 feat(cursor-review): reuse check-pr-size classifier to skip codegen in the size gate and the reviewed diff
3bfe469 feat(check-pr-size): discount blank/comment lines from the size count (opt-in)
ec880db fix(cursor-review): resolve panel findings on classifier reuse (BE-1916)
b0de184 Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen
6da34ac Merge origin/main, resolve cursor-review.yml checkout-pin conflict
809971d Merge origin/main into matt/cursor-review-reuse-pr-size-codegen
3763645 Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen
607f093 Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen
6bd8b6d Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen

📊 Changes

7 files changed (+1392 additions, -179 deletions)

View changed files

📝 .github/cursor-review/README.md (+5 -2)
📝 .github/workflows/cursor-review.yml (+266 -101)
📝 README.md (+1 -1)
📝 scripts/check-pr-size/main.go (+199 -36)
📝 scripts/check-pr-size/main_test.go (+187 -5)
📝 scripts/check-pr-size/size.go (+370 -29)
📝 scripts/check-pr-size/size_test.go (+364 -5)

📄 Description

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

#58 (XL mode, draft) is complementary: a manual cursor-review-xl label that bypasses the cap for a genuinely huge PR. This PR shrinks what counts toward the cap in the first place. They compose; whoever lands second rebases diff-size.
#33 (structured output) rewrites the panel/judge finding-extraction. This PR rewrites the diff-generation + checkout of the same jobs (different steps). Conflicts are mechanical.

Draft until we pick the merge order.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/59 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/22/2026 **Status:** ✅ Merged **Merged:** 8/11/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/cursor-review-reuse-pr-size-codegen` --- ### 📝 Commits (9) - [`4077d73`](https://github.com/Comfy-Org/github-workflows/commit/4077d73d2925cfc88487ee767c90d2df450c60e0) feat(cursor-review): reuse check-pr-size classifier to skip codegen in the size gate and the reviewed diff - [`3bfe469`](https://github.com/Comfy-Org/github-workflows/commit/3bfe469263b186875ec74f401e90becc68462819) feat(check-pr-size): discount blank/comment lines from the size count (opt-in) - [`ec880db`](https://github.com/Comfy-Org/github-workflows/commit/ec880db4ad6395dbed4d793d00397ed523b7ae44) fix(cursor-review): resolve panel findings on classifier reuse (BE-1916) - [`b0de184`](https://github.com/Comfy-Org/github-workflows/commit/b0de184c362bc1712a3f4dc4cc415b8d88b5cb5e) Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen - [`6da34ac`](https://github.com/Comfy-Org/github-workflows/commit/6da34ac597d73f029e892a9645667fd11eccf028) Merge origin/main, resolve cursor-review.yml checkout-pin conflict - [`809971d`](https://github.com/Comfy-Org/github-workflows/commit/809971d7ccf4e233f95c322f57762aa11d99cf2a) Merge origin/main into matt/cursor-review-reuse-pr-size-codegen - [`3763645`](https://github.com/Comfy-Org/github-workflows/commit/376364549741334ba16da4bd3d4a048ead80ffdb) Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen - [`607f093`](https://github.com/Comfy-Org/github-workflows/commit/607f093b633f54ed8b410262cb6c83dbdf265f52) Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen - [`6bd8b6d`](https://github.com/Comfy-Org/github-workflows/commit/6bd8b6d9c1f0bde786dd6a9a118d593112c78420) Merge remote-tracking branch 'origin/main' into matt/cursor-review-reuse-pr-size-codegen ### 📊 Changes **7 files changed** (+1392 additions, -179 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+5 -2) 📝 `.github/workflows/cursor-review.yml` (+266 -101) 📝 `README.md` (+1 -1) 📝 `scripts/check-pr-size/main.go` (+199 -36) 📝 `scripts/check-pr-size/main_test.go` (+187 -5) 📝 `scripts/check-pr-size/size.go` (+370 -29) 📝 `scripts/check-pr-size/size_test.go` (+364 -5) </details> ### 📄 Description 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 - **#58 (XL mode, draft)** is complementary: a manual `cursor-review-xl` label that *bypasses* the cap for a genuinely huge PR. This PR shrinks what counts toward the cap in the first place. They compose; whoever lands second rebases `diff-size`. - **#33 (structured output)** rewrites the panel/judge finding-extraction. This PR rewrites the diff-generation + checkout of the same jobs (different steps). Conflicts are mechanical. Draft until we pick the merge order. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:53 +00:00