feat(action): add base branch size comparison in PR comments#2796
feat(action): add base branch size comparison in PR comments#2796
Conversation
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a compare module that fetches base-branch file contents via GitHub, computes per-file and aggregate size changes for minified assets, integrates comparisons into PR comment generation, and includes extensive tests for comparison behaviors and edge cases. Changes
Sequence DiagramsequenceDiagram
participant Action as GitHub Action
participant Compare as Compare Module
participant Octokit as Octokit Client
participant GitHub as GitHub API
participant Reporter as PR Comment Reporter
Action->>Compare: compareWithBase(result, githubToken)
Compare->>Compare: Verify PR context & token
loop per file
Compare->>Octokit: repos.getContent(owner, repo, path, ref=base)
Octokit->>GitHub: HTTP request
GitHub-->>Octokit: 200 / 404 / error
Octokit-->>Compare: content or error
Compare->>Compare: compute baseSize, currentSize, change, isNew
end
Compare-->>Action: ComparisonResult[]
Action->>Reporter: postPRComment(result, githubToken, comparisons)
Reporter->>Reporter: generateCommentBody(result, comparisons)
Reporter->>Reporter: formatChange(comparison)
Reporter->>GitHub: create/update PR comment via Octokit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/action/src/compare.ts`:
- Around line 190-228: The calculateTotalChange function currently computes
totalCurrentSize only from comparable (non-new) items in the main branch; change
it so totalCurrentSize always sums c.currentSize for all comparisons (use
comparisons.reduce((sum,c)=>sum+c.currentSize,0)), while keeping totalBaseSize
computed from comparable (c.baseSize) and preserving totalChangePercent logic
(null when there are no comparable entries, otherwise compute percent using
totalBaseSize and the always-included totalCurrentSize). Update references in
calculateTotalChange to use the new totalCurrentSize calculation and leave the
comparable-based totalBaseSize and percent logic intact.
🧹 Nitpick comments (3)
packages/action/src/compare.ts (2)
43-53: Consider parallel file comparison for better performance.The sequential
for...ofloop makes API calls one at a time. For PRs with many files, this could be slow. Consider usingPromise.allfor parallel requests.♻️ Proposed parallel processing
- const results: ComparisonResult[] = []; - - for (const fileResult of result.files) { - const comparison = await compareFile( - octokit, - owner, - repo, - baseBranch, - fileResult.file, - fileResult.minifiedSize - ); - results.push(comparison); - } - - return results; + const results = await Promise.all( + result.files.map((fileResult) => + compareFile( + octokit, + owner, + repo, + baseBranch, + fileResult.file, + fileResult.minifiedSize + ) + ) + ); + + return results;As per coding guidelines, use
Promise.allfor parallel operations where all must succeed.
161-170: Zero change shows success emoji — verify this is intentional.When
change === 0, the function returns+0.0% ✅. The success emoji for "no change" is reasonable, but the+sign for zero might be unexpected. Consider whether0.0% ✅(without the+) would be clearer.♻️ Optional: Remove plus sign for zero change
- const sign = comparison.change >= 0 ? "+" : ""; + const sign = comparison.change > 0 ? "+" : "";packages/action/__tests__/compare.test.ts (1)
1-221: Consider adding tests forcompareWithBase.The main exported function
compareWithBaselacks test coverage. While it requires mocking@actions/github, testing the orchestration logic would improve confidence.Do you want me to generate test cases for
compareWithBaseusing Vitest mocks for@actions/github?
Greptile OverviewGreptile SummaryThis PR adds base branch size comparison to the GitHub Action's PR comments. The implementation fetches minified file sizes from the base branch using the GitHub API and displays changes as a new "vs Base" column with visual indicators ( Key changes:
The implementation is backward-compatible and handles edge cases well (404s for new files, directory responses, symlinks, network errors). The comparison feature only runs on PRs with a valid GitHub token, gracefully skipping otherwise. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Action as GitHub Action
participant Index as index.ts
participant Compare as compare.ts
participant GitHub as GitHub API
participant Comment as comment.ts
Action->>Index: run()
Index->>Index: runMinification(inputs)
Index-->>Index: MinifyResult
alt is PR and reportPRComment enabled
Index->>Compare: compareWithBase(result, token)
alt has token and is PR
loop for each file
Compare->>GitHub: getContent(path, baseBranch)
alt file exists
GitHub-->>Compare: {type: "file", size: baseSize}
Compare->>Compare: calculate change %
else file not found (404)
GitHub-->>Compare: 404 error
Compare->>Compare: mark as new file
else other error
GitHub-->>Compare: error
Compare->>Compare: log warning, mark as new
end
end
Compare-->>Index: ComparisonResult[]
else no token or not PR
Compare-->>Index: []
end
Index->>Comment: postPRComment(result, token, comparisons)
Comment->>Comment: generateCommentBody(result, comparisons)
Comment->>Comment: build table with "vs Base" column
Comment->>GitHub: create/update PR comment
GitHub-->>Comment: success
Comment-->>Index: complete
end
Index-->>Action: success
|
packages/action/src/compare.ts
Outdated
| for (const fileResult of result.files) { | ||
| const comparison = await compareFile( | ||
| octokit, | ||
| owner, | ||
| repo, | ||
| baseBranch, | ||
| fileResult.file, | ||
| fileResult.minifiedSize | ||
| ); | ||
| results.push(comparison); | ||
| } |
There was a problem hiding this comment.
Sequential API calls - use Promise.all for parallel execution.
| for (const fileResult of result.files) { | |
| const comparison = await compareFile( | |
| octokit, | |
| owner, | |
| repo, | |
| baseBranch, | |
| fileResult.file, | |
| fileResult.minifiedSize | |
| ); | |
| results.push(comparison); | |
| } | |
| const results = await Promise.all( | |
| result.files.map((fileResult) => | |
| compareFile( | |
| octokit, | |
| owner, | |
| repo, | |
| baseBranch, | |
| fileResult.file, | |
| fileResult.minifiedSize | |
| ) | |
| ) | |
| ); | |
| return results; |
Context Used: Context from dashboard - AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/action/src/compare.ts
Line: 43:53
Comment:
Sequential API calls - use `Promise.all` for parallel execution.
```suggestion
const results = await Promise.all(
result.files.map((fileResult) =>
compareFile(
octokit,
owner,
repo,
baseBranch,
fileResult.file,
fileResult.minifiedSize
)
)
);
return results;
```
**Context Used:** Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=1faf77f6-13aa-4727-b03f-89976b6a776a))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/action/src/compare.ts">
<violation number="1" location="packages/action/src/compare.ts:194">
P2: totalChangePercent now uses the current size from all files, including new ones, while totalBaseSize is only for comparable files. This inflates the percent change whenever new files are present. Calculate the percent change using a current size total from comparable files.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Previously totalChangePercent incorrectly included new files' sizes in the current total, inflating the percent when new files were present. Now uses only comparable files' current sizes for accurate before/after comparison.
|
@greptile |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2796 +/- ##
===========================================
+ Coverage 99.37% 99.40% +0.02%
===========================================
Files 63 64 +1
Lines 1287 1345 +58
Branches 397 419 +22
===========================================
+ Hits 1279 1337 +58
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
User description
Summary
compare.tsmodule to fetch and compare minified file sizes against base branchCompletes Phase 3 of the GitHub Action implementation plan.
PR Type
Enhancement
Description
Add base branch size comparison module with file fetching logic
Implement comparison formatting with visual indicators (✅/⚠️ )
Update PR comments to display "vs Base" column with change percentages
Add 14 comprehensive unit tests for comparison functions
Diagram Walkthrough
File Walkthrough
compare.ts
Base branch comparison logic and formattingpackages/action/src/compare.ts
compareWithBase()to fetch and compare files against basebranch using GitHub API
computation
formatChange()to format comparisons with emoji indicators(
hasIncrease()andcalculateTotalChange()utility functions foranalysis
index.ts
Integrate base comparison into main workflowpackages/action/src/index.ts
compareWithBasefrom new compare modulecompareWithBase()before posting PR comment to fetch base branchcomparisons
postPRComment()functioncomment.ts
Display comparison data in PR commentspackages/action/src/reporters/comment.ts
postPRComment()signature to accept optionalcomparisonsparameter
generateCommentBody()to accept and process comparisonresults
indicators
rendering
compare.test.ts
Unit tests for comparison functionspackages/action/tests/compare.test.ts
formatChange()with size increases, decreases, no change, andnew files
hasIncrease()to detect any file size increases acrosscomparisons
calculateTotalChange()with various scenarios including newfiles and empty arrays
Summary by cubic
Adds base-branch size comparison to the GitHub Action and displays a “vs Base” column in PR comments so reviewers can quickly spot increases or reductions. Runs only on PRs with a GitHub token and gracefully skips otherwise.
New Features
Bug Fixes
Written for commit 0e8ac02. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.