Skip to content

ci(github): exclude test files from mixed PR size calculation#1105

Merged
juliusmarminge merged 4 commits intopingdotgg:mainfrom
binbandit:t3code/ignore-test-files-change-size
Mar 24, 2026
Merged

ci(github): exclude test files from mixed PR size calculation#1105
juliusmarminge merged 4 commits intopingdotgg:mainfrom
binbandit:t3code/ignore-test-files-change-size

Conversation

@binbandit
Copy link
Copy Markdown
Contributor

@binbandit binbandit commented Mar 15, 2026 •
edited by macroscopeapp bot
Loading

What Changed

  • Switched PR size labeling from aggregate PR additions/deletions to per-file diff stats from the GitHub pull request files API.
  • Excluded test-file line changes when a PR also changes non-test files.
  • Preserved test-file line counting when a PR only changes tests.
  • Added logging for test vs non-test lines and a warning when GitHub's file listing may truncate at 3,000 files.

Why

  • Mixed PRs with substantial test updates were receiving inflated size labels even when the production-code review surface stayed small.
  • Test-only PRs should still reflect their actual review size instead of collapsing to the smallest label.

Testing

  • bun fmt
  • bun lint
  • bun typecheck
  • Local scenario validation for mixed code+tests, test-only, browser-test, and apps/server/integration fixture changes.

Note

Exclude test files from PR size label calculation in mixed PRs

  • Replaces the simple additions + deletions count from the event payload with a per-file aggregation using pulls.listFiles to detect and separate test vs. non-test changed lines.
  • For mixed PRs (test + non-test files), size labels are based only on non-test changed lines; for test-only PRs, the full test line count is used.
  • Test files are detected via a set of regex patterns applied per file in pr-size.yml.
  • Action logs now report non-test lines, test lines, effective lines, and a classification category.
  • Behavioral Change: PRs with test file changes alongside production code will now receive a smaller size label than before.

Macroscope summarized 3dedd41.

@github-actions github-actions bot added size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Mar 15, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b6a1b4fc-2702-43cd-9133-c60ccdd7cdbe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

@binbandit binbandit changed the title Exclude test files from mixed PR size calculation ci(github): exclude test files from mixed PR size calculation Mar 15, 2026
@Noojuno
Copy link
Copy Markdown
Contributor

Noojuno commented Mar 15, 2026

This is obv up to Julius and Theo (and team), but if it were me I don't think I'd want this tbh. Tests still need to be reviewed, so if a PR adds a shitload of tests, the effort to review is still decently high, especially because of the crazy rate of shit and useless tests that LLM's love to write

@binbandit
Copy link
Copy Markdown
Contributor Author

binbandit commented Mar 15, 2026

The problem is that a small change to add a tiny feature might need a whole new test file written. Tests are pretty effortless to review, but the actual implementation is more effort.

So we should base the "size" on the actual feature / fix being added, and not base it on the tests.

That being said, if the PR is to just add tests, then we will use that as a gauge for how large the PR is

@UtkarshUsername
Copy link
Copy Markdown
Contributor

UtkarshUsername commented Mar 16, 2026

The problem is that a small change to add a tiny feature might need a whole new test file written.

I agree, because of this it would be better to exclude test files from the labels, when the PR is not only for tests.

@jamesx0416
Copy link
Copy Markdown
Contributor

jamesx0416 commented Mar 18, 2026 •
edited
Loading

I wonder if there's a way to set modified lines as 1 line, instead of 2(+1 -1). And maybe don't count empty lines.
Good pr btw, should get Julius to have a look.

@binbandit
Copy link
Copy Markdown
Contributor Author

binbandit commented Mar 24, 2026

@juliusmarminge lets get this merged

@juliusmarminge juliusmarminge merged commit e823f8f into pingdotgg:main Mar 24, 2026
8 of 9 checks passed
Chrono-byte pushed a commit to Chrono-byte/t3code that referenced this pull request Mar 24, 2026
@binbandit @Chrono-byte
ci(github): exclude test files from mixed PR size calculation (pingdo…
597f0d8 
…tgg#1105)
aaditagrawal added a commit to aaditagrawal/t3code that referenced this pull request Mar 24, 2026
@aaditagrawal
ci(github): exclude test files from mixed PR size calculation (pingdo…
12bc8dd 
…tgg#1105)
@aaditagrawal aaditagrawal mentioned this pull request Mar 24, 2026
Merged
4 tasks
ranvier2d2 added a commit to Ranvier-Technologies/t3code-OTP that referenced this pull request Mar 24, 2026
@ranvier2d2 @claude
chore: sync 4 upstream commits (no-conflict)
26bfc59 
Cherry-picked from pingdotgg/t3code:
- 843d6d8 fix: add license field to npm package (pingdotgg#1272)
- e11fb6e fix(web): avoid false draft attachment persistence warnings (pingdotgg#1153)
- 2b08d86 Add resizable chat sidebar (pingdotgg#1347)
- e823f8f ci(github): exclude test files from mixed PR size calculation (pingdotgg#1105)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@ranvier2d2 ranvier2d2 mentioned this pull request Mar 24, 2026
Merged
3 tasks
ranvier2d2 added a commit to Ranvier-Technologies/t3code-OTP that referenced this pull request Mar 24, 2026
@ranvier2d2 @claude
chore: sync 4 upstream commits (no-conflict)
96825c6 
Cherry-picked from pingdotgg/t3code:
- 843d6d8 fix: add license field to npm package (pingdotgg#1272)
- e11fb6e fix(web): avoid false draft attachment persistence warnings (pingdotgg#1153)
- 2b08d86 Add resizable chat sidebar (pingdotgg#1347)
- e823f8f ci(github): exclude test files from mixed PR size calculation (pingdotgg#1105)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
gabrielMalonso pushed a commit to gabrielMalonso/t3code that referenced this pull request Mar 24, 2026
@binbandit @gabrielMalonso
ci(github): exclude test files from mixed PR size calculation (pingdo…
e1fe1a3 
…tgg#1105)
apexsloth pushed a commit to apexslothforks/t3code that referenced this pull request Mar 26, 2026
@binbandit
ci(github): exclude test files from mixed PR size calculation (pingdo…
2e0c7da 
…tgg#1105)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@UtkarshUsername UtkarshUsername UtkarshUsername approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@binbandit @Noojuno @UtkarshUsername @jamesx0416 @juliusmarminge