-
Notifications
You must be signed in to change notification settings - Fork 67
fix compressed-diff workflow #3589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@bfintal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 2 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughReplaces a removed compressed-size workflow with a new "Build Size Comparison" GitHub Actions workflow that builds PR and base artifacts (including premium repo copies), computes per-file and total size diffs, posts a PR comment with a Markdown report, uploads both build zips, and adds two local debug side-effect logs in project files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant Runner as Actions Runner
participant Premium as bfintal/Stackable-Premium
participant npm as npm Registry
participant Artifact as Artifact Store
Dev->>GH: Open PR (target master/develop)
GH->>Runner: Trigger "Build Size Comparison" workflow
rect rgb(250,250,255)
note over Runner: Checkouts
Runner->>Runner: checkout PR repo (fetch-depth: 0)
Runner->>Runner: checkout base branch → `base-branch`
Runner->>Premium: checkout premium repo → `pro__premium_only` (PR)
Runner->>Premium: checkout premium repo → `base-branch/pro__premium_only` (base)
end
rect rgb(245,255,245)
note over Runner: Install & Build
Runner->>npm: npm ci (repo root) [PR]
Runner->>npm: npm ci (pro__premium_only) [PR]
Runner->>Runner: npm run build:no-translate [PR]
Runner->>npm: npm ci (base-branch root)
Runner->>npm: npm ci (base-branch/pro__premium_only)
Runner->>Runner: npm run build:no-translate (base)
end
rect rgb(255,250,240)
note over Runner: Package & Compare
Runner->>Runner: zip builds (exclude .map/node_modules)
Runner->>Runner: unzip into `current-files` and `base-files`
Runner->>Runner: run compare_files.py → generate JSON & summary
Runner->>GH: Upsert PR comment with Markdown report
Runner->>Artifact: upload build zips (artifact)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches🧪 Generate 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. Comment |
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/compressed-diff.yml (4)
21-27: Cache both lockfiles and switch to npm ci for reproducible installs.This enables caching for the Premium repo too and speeds installs while enforcing the lockfile.
with: node-version: 18.x cache: 'npm' + cache-dependency-path: | + package-lock.json + pro__premium_only/package-lock.json - - name: Install Dependencies + - name: Install Dependencies run: | - npm install - cd pro__premium_only && npm install + npm ci + cd pro__premium_only && npm ciPlease confirm both lockfiles are committed; otherwise npm ci will fail.
12-13: Ensure adequate git history for compressed-size comparisons.Add fetch-depth: 2 to the main checkout so the action can diff against the base; keep the Premium checkout shallow.
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + fetch-depth: 2 - - uses: actions/checkout@v4 # Checkout the Stackable Premium repo. + - uses: actions/checkout@v4 # Checkout the Stackable Premium repo. with: repository: 'bfintal/Stackable-Premium' ref: 'v3' path: 'pro__premium_only' token: '${{ secrets.ACCESS_KEY }}' + fetch-depth: 1If compressed-size-action doesn’t require history in your setup, feel free to skip this.
Also applies to: 14-18
14-18: Limit secret scope (defense-in-depth).Confirm ACCESS_KEY is a fine‑grained, read‑only PAT restricted to the Stackable‑Premium repo. Rotate if broader.
12-13: Optional: Pin actions to commit SHAs.For supply-chain hardening, pin actions (checkout, setup-node, compressed-size-action) to immutable SHAs.
Also applies to: 19-21, 28-28
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compressed-diff.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/compressed-diff.yml
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| - name: Setup Node | ||
| uses: actions/setup-node@v3 | ||
| with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade setup-node to v4 (v3 now blocked on GH runners).
actionlint flags v3 as too old. Move to v4 to unbreak the job.
- - name: Setup Node
- uses: actions/setup-node@v3
+ - name: Setup Node
+ uses: actions/setup-node@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Node | |
| uses: actions/setup-node@v3 | |
| with: | |
| - name: Setup Node | |
| uses: actions/setup-node@v4 | |
| with: |
🧰 Tools
🪛 actionlint (1.7.7)
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/compressed-diff.yml around lines 19 to 21 the workflow uses
actions/setup-node@v3 which is blocked on GitHub runners; update the action to
actions/setup-node@v4 by changing the version reference so the job uses the v4
release. Ensure there are no breaking input changes in the workflow (keep the
existing with: inputs the same) and run the workflow lint to confirm actionlint
no longer flags the version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/compressed-diff.yml (1)
19-23: Unblock the workflow: upgrade setup-node to v4 (v3 is blocked on GH runners).actionlint flags this and the job won’t run with v3. Update to v4.
- - name: Setup Node - uses: actions/setup-node@v3 + - name: Setup Node + uses: actions/setup-node@v4Optional: consider
node-version: 20.xto align with current LTS; verify build compatibility first.
🧹 Nitpick comments (3)
.github/workflows/compressed-diff.yml (3)
21-23: Extend npm cache to include Premium lockfile.Currently only the root lockfile is cached. Add both paths so installs in
pro__premium_onlybenefit from cache as well.with: - node-version: 18.x - cache: 'npm' + node-version: 18.x + cache: 'npm' + cache-dependency-path: | + package-lock.json + pro__premium_only/package-lock.json
24-28: Verify lockfiles exist; consider minor install hardening.
npm cirequirespackage-lock.jsonin both locations. Confirm both files are present. Optionally add--no-audit --no-fundto shave CI time/noise.- npm ci --legacy-peer-deps + npm ci --legacy-peer-deps --no-audit --no-fund cd pro__premium_only - npm ci --legacy-peer-deps + npm ci --legacy-peer-deps --no-audit --no-fund
12-13: Add explicit step names for clarity in logs.Small DX tweak to make steps easier to spot.
- - uses: actions/checkout@v4 - - uses: actions/checkout@v4 # Checkout the Stackable Premium repo. + - name: Checkout Stackable + uses: actions/checkout@v4 + - name: Checkout Stackable Premium + uses: actions/checkout@v4 # Checkout the Stackable Premium repo.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compressed-diff.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/compressed-diff.yml
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/compressed-diff.yml (1)
19-21: Upgrade setup-node to v4 (v3 is blocked on GH runners).actionlint flags v3 as too old; this breaks the job. Move to v4.
- - name: Setup Node - uses: actions/setup-node@v3 + - name: Setup Node + uses: actions/setup-node@v4
🧹 Nitpick comments (6)
.github/workflows/compressed-diff.yml (6)
22-23: Consider bumping Node to an LTS that’s still supported.Node 18 reached EOL on 2025-04-30. Prefer 20.x (or 22.x if compatible).
- node-version: 18.x + node-version: 20.xWould you like me to verify build compatibility across 18/20/22 with a quick matrix on this job?
22-28: Cache both lockfiles to speed installs.You install deps at repo root and in pro__premium_only. Add cache-dependency-path for both lockfiles so npm cache hits cover both.
with: node-version: 20.x cache: 'npm' + cache-dependency-path: | + package-lock.json + pro__premium_only/package-lock.json
24-28: Prefer dedicated steps with working-directory over inline cd.Minor readability win and cleaner logs. Functionally equivalent.
- - name: Install Dependencies - run: | - npm ci - cd pro__premium_only - npm ci + - name: Install Dependencies (root) + run: npm ci + - name: Install Dependencies (premium) + working-directory: pro__premium_only + run: npm ci
32-36: Pin third‑party action to a commit SHA.preactjs/compressed-size-action is a third-party action; pin to a specific SHA to reduce supply‑chain risk.
- - uses: preactjs/compressed-size-action@v2 + - uses: preactjs/compressed-size-action@<commit-sha>I can fetch the latest stable v2 commit SHA if you want.
7-11: Tighten default permissions for the job.Set minimal permissions for better security posture.
jobs: build: runs-on: ubuntu-latest + permissions: + contents: read
30-36: Configure compressed-size-action to handle installs and builds for both base and PR
Remove the standalone build step and pass your install- and build-scripts directly to the action:- - name: Build Project - run: npm run build:no-translate - - uses: preactjs/compressed-size-action@v2 + - uses: preactjs/compressed-size-action@v2 with: repo-token: "${{ secrets.GITHUB_TOKEN }}" pattern: "./build/stackable/**" exclude: "{**/*.map,**/node_modules/**}" + install-script: npm ci + build-script: npm run build:no-translateThe action will run your install-script and build-script for both the base ref and the PR head, ensuring accurate size baselines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compressed-diff.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/compressed-diff.yml
20-20: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/compressed-diff.yml (1)
12-13: LGTM on dual checkouts.Using checkout@v4 for both repos with a scoped path for Premium looks correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/compressed-diff.yml (1)
23-27: Upgrade setup-node to v4 and move off Node 18 (blocked/EOL).actionlint flags v3 as blocked; Node 18 is EOL. Bump to v4 and use an LTS (20 or 22). Also cache the sub-repo’s lockfile for faster CI.
- - name: Setup Node - uses: actions/setup-node@v3 + - name: Setup Node + uses: actions/setup-node@v4 with: - node-version: 18.x + node-version: 20.x cache: 'npm' + cache-dependency-path: | + package-lock.json + pro__premium_only/package-lock.json
🧹 Nitpick comments (2)
.github/workflows/compressed-diff.yml (2)
28-31: Slim the apt install.Minor: combine commands and avoid recommended extras to reduce time/image churn.
- - name: Install Build Tools - run: | - sudo apt-get update - sudo apt-get install -y build-essential + - name: Install Build Tools + run: sudo apt-get update && sudo apt-get install -y --no-install-recommends build-essential
32-36: Split installs, use working-directory, and skip premium deps on forks.Improves clarity, avoids cd, and prevents failures when secrets aren’t available on fork PRs.
- - name: Install Dependencies - run: | - npm ci --legacy-peer-deps - cd pro__premium_only - npm ci --legacy-peer-deps + - name: Install Dependencies (root) + run: npm ci --legacy-peer-deps --no-audit --no-fund + + - name: Install Premium Dependencies + if: ${{ !github.event.pull_request.head.repo.fork }} + working-directory: pro__premium_only + run: npm ci --legacy-peer-deps --no-audit --no-fundConfirm the workflow must build the premium package for all PRs; if not, we can fully skip those steps on forks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/compressed-diff.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/compressed-diff.yml
20-20: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
24-24: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/compressed-diff.yml (1)
12-14: Harden premium repo checkout: disable credential persistence & skip on forks
- Set
persist-credentials: falseto prevent the PAT from persisting in.git/config(actions/checkout defaults to true) (github.com, yossarian.net)- Add
if: ${{ ! github.event.pull_request.head.repo.fork }}so it won’t run whensecrets.ACCESS_KEYisn’t available in forked PRs (jerensl.com)- - uses: actions/checkout@v4 # Checkout the Stackable Premium repo. + - name: Checkout Stackable Premium + if: ${{ ! github.event.pull_request.head.repo.fork }} + uses: actions/checkout@v4 with: repository: 'bfintal/Stackable-Premium' ref: 'v3' path: 'pro__premium_only' token: '${{ secrets.ACCESS_KEY }}' + persist-credentials: falseIf you still need this checkout in forked PRs, you’ll need an alternate flow that doesn’t rely on repository secrets.
| - name: Setup Python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.9' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade setup-python to v5 (v4 now blocked on GH runners).
actionlint indicates v4 can’t run; bump the action version.
- - name: Setup Python
- uses: actions/setup-python@v4
+ - name: Setup Python
+ uses: actions/setup-python@v5
with:
python-version: '3.9'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Python | |
| uses: actions/setup-python@v4 | |
| with: | |
| python-version: '3.9' | |
| - name: Setup Python | |
| uses: actions/setup-python@v5 | |
| with: | |
| python-version: '3.9' |
🧰 Tools
🪛 actionlint (1.7.7)
20-20: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/compressed-diff.yml around lines 19 to 22: the workflow uses
actions/setup-python@v4 which is blocked on GH runners; update the action
reference to actions/setup-python@v5, keep the same inputs (python-version:
'3.9') and commit the change, then trigger or run the workflow to verify the
updated action works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (8)
.github/workflows/build-size-comparison.yml (8)
23-38: Premium repo checkout uses secrets; add explicit guards per step (defense-in-depth).Even with a job-level guard, also gate these steps so local/manual runs don’t accidentally fail.
- - name: Checkout Premium Repo (Current PR) + - name: Checkout Premium Repo (Current PR) + if: ${{ github.event.pull_request.head.repo.fork == false }} uses: actions/checkout@v4 with: repository: 'bfintal/Stackable-Premium' ref: 'v3' path: 'pro__premium_only' token: '${{ secrets.ACCESS_KEY }}' - - name: Checkout Premium Repo (Base Branch) + - name: Checkout Premium Repo (Base Branch) + if: ${{ github.event.pull_request.head.repo.fork == false }} uses: actions/checkout@v4 with: repository: 'bfintal/Stackable-Premium' ref: 'v3' path: 'base-branch/pro__premium_only' token: '${{ secrets.ACCESS_KEY }}'
45-57: Prefer working-directory over cd; drop legacy flag if not required.Minimize subshell churn and only keep
--legacy-peer-depsif you truly need it.- - name: Install Dependencies (Current PR) - run: | - npm ci --legacy-peer-deps - cd pro__premium_only - npm ci --legacy-peer-deps + - name: Install Dependencies (Current PR - root) + run: npm ci --no-audit --no-fund + - name: Install Dependencies (Current PR - premium) + run: npm ci --no-audit --no-fund + working-directory: pro__premium_only @@ - - name: Install Dependencies (Base Branch) - run: | - cd base-branch - npm ci --legacy-peer-deps - cd pro__premium_only - npm ci --legacy-peer-deps + - name: Install Dependencies (Base Branch - root) + run: npm ci --no-audit --no-fund + working-directory: base-branch + - name: Install Dependencies (Base Branch - premium) + run: npm ci --no-audit --no-fund + working-directory: base-branch/pro__premium_only
58-66: Set working-directory for builds.Keeps paths consistent and avoids implicit state from previous cd’s.
- - name: Build Current PR - run: | - npm run build:no-translate + - name: Build Current PR + run: npm run build:no-translate - - name: Build Base Branch - run: | - cd base-branch - npm run build:no-translate + - name: Build Base Branch + run: npm run build:no-translate + working-directory: base-branch
67-80: Use maximum compression and quieter zip; avoid fragile cd chains.Also prepares for later path changes without breaking.
- - name: Create Zip Files - run: | - # Create zip for current PR - cd build/stackable - zip -r ../../current-build.zip . -x "*.map" "node_modules/*" - - # Create zip for base branch - cd ../../base-branch/build/stackable - zip -r ../../../base-build.zip . -x "*.map" "node_modules/*" - - # Move zip files to root directory for easier access - cd ../../../ - ls -la *.zip + - name: Create Zip Files + run: | + zip -r9q current-build.zip build/stackable -x "*.map" "node_modules/*" + zip -r9q base-build.zip base-branch/build/stackable -x "*.map" "node_modules/*" + ls -la *.zip
123-130: Also list base-branch build contents for parity.Helps quickly spot packaging differences.
echo "### 📁 Build Contents" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "**Current PR build contents:**" >> $GITHUB_STEP_SUMMARY echo '```' >> $GITHUB_STEP_SUMMARY unzip -l current-build.zip | head -20 >> $GITHUB_STEP_SUMMARY echo '```' >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Base Branch build contents:**" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + unzip -l base-build.zip | head -20 >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY
11-22: Lock workflow permissions to least privilege.Not strictly required, but recommended.
jobs: build-and-compare: + permissions: + contents: read runs-on: ubuntu-latest
72-76: Trim trailing spaces and add a newline at EOF.Fix YAMLlint errors and keep diffs clean.
Also applies to: 85-85, 91-91, 96-96, 100-100, 104-104, 108-108, 114-114, 122-122, 138-138
23-30: Confirm secret scope and naming.
secrets.ACCESS_KEYshould be a fine‑grained PAT with read access tobfintal/Stackable-Premium. Consider renaming to a repo-specific name (e.g.,STACKABLE_PREMIUM_TOKEN) for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-size-comparison.yml(1 hunks).github/workflows/compressed-diff.yml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/compressed-diff.yml
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-size-comparison.yml
40-40: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/build-size-comparison.yml
[error] 72-72: trailing spaces
(trailing-spaces)
[error] 76-76: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 138-138: no new line character at the end of file
(new-line-at-end-of-file)
| build-and-compare: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Skip the entire job for forked PRs to avoid secret access failures.
pull_request from forks won’t receive ${{ secrets.ACCESS_KEY }}; the job will fail at premium checkouts. Gate the job to run only for non-fork PRs.
jobs:
build-and-compare:
+ if: ${{ github.event.pull_request.head.repo.fork == false }}
runs-on: ubuntu-latest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| build-and-compare: | |
| runs-on: ubuntu-latest | |
| build-and-compare: | |
| if: ${{ github.event.pull_request.head.repo.fork == false }} | |
| runs-on: ubuntu-latest |
🤖 Prompt for AI Agents
.github/workflows/build-size-comparison.yml around lines 8-9: the job currently
runs for pull_request events and will fail for forked PRs due to missing
secrets; add a job-level conditional to skip forked PRs by checking the source
repo. Modify the job to include an if condition such as: run if the event is not
a pull_request OR the pull request head repo equals the base repo (e.g. if:
github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository) so the job
only runs for non-fork PRs (and still runs for other events).
| - name: Setup Node | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 14.x | ||
| cache: 'npm' | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Update setup-node action and Node.js version (Node 14 is EOL; action v3 flagged).
Use actions/setup-node@v4 and a supported LTS (18.x or 20.x). Also cache all lockfiles via cache-dependency-path.
- - name: Setup Node
- uses: actions/setup-node@v3
+ - name: Setup Node
+ uses: actions/setup-node@v4
with:
- node-version: 14.x
- cache: 'npm'
+ node-version: 20.x
+ cache: 'npm'
+ cache-dependency-path: |
+ package-lock.json
+ pro__premium_only/package-lock.json
+ base-branch/package-lock.json
+ base-branch/pro__premium_only/package-lock.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Node | |
| uses: actions/setup-node@v3 | |
| with: | |
| node-version: 14.x | |
| cache: 'npm' | |
| - name: Setup Node | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 20.x | |
| cache: 'npm' | |
| cache-dependency-path: | | |
| package-lock.json | |
| pro__premium_only/package-lock.json | |
| base-branch/package-lock.json | |
| base-branch/pro__premium_only/package-lock.json |
🧰 Tools
🪛 actionlint (1.7.7)
40-40: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/build-size-comparison.yml around lines 39-44: the workflow
uses actions/setup-node@v3 with Node 14 (EOL) and the deprecated 'cache' option;
update the action to actions/setup-node@v4 and set node-version to a supported
LTS (e.g., 18.x or 20.x), and replace the 'cache' key with
'cache-dependency-path' listing your lockfiles (e.g., package-lock.json,
yarn.lock, pnpm-lock.yaml) so all lockfiles are cached correctly.
| - name: Compare Build Sizes | ||
| run: | | ||
| echo "## 📦 Build Size Comparison" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| # Check if files exist | ||
| if [ ! -f "current-build.zip" ]; then | ||
| echo "❌ **Error**: current-build.zip not found" >> $GITHUB_STEP_SUMMARY | ||
| exit 1 | ||
| fi | ||
| if [ ! -f "base-build.zip" ]; then | ||
| echo "❌ **Error**: base-build.zip not found" >> $GITHUB_STEP_SUMMARY | ||
| exit 1 | ||
| fi | ||
| # Get file sizes | ||
| CURRENT_SIZE=$(du -h current-build.zip | cut -f1) | ||
| BASE_SIZE=$(du -h base-build.zip | cut -f1) | ||
| # Get exact byte sizes for calculation | ||
| CURRENT_BYTES=$(stat -c%s current-build.zip) | ||
| BASE_BYTES=$(stat -c%s base-build.zip) | ||
| # Calculate difference | ||
| DIFF_BYTES=$((CURRENT_BYTES - BASE_BYTES)) | ||
| DIFF_PERCENT=$((DIFF_BYTES * 100 / BASE_BYTES)) | ||
| echo "| Build | Size |" >> $GITHUB_STEP_SUMMARY | ||
| echo "|-------|------|" >> $GITHUB_STEP_SUMMARY | ||
| echo "| **Current PR** | **$CURRENT_SIZE** |" >> $GITHUB_STEP_SUMMARY | ||
| echo "| Base Branch | $BASE_SIZE |" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| if [ $DIFF_BYTES -gt 0 ]; then | ||
| echo "📈 **Size increased by** $(numfmt --to=iec $DIFF_BYTES) (+$DIFF_PERCENT%)" >> $GITHUB_STEP_SUMMARY | ||
| elif [ $DIFF_BYTES -lt 0 ]; then | ||
| echo "📉 **Size decreased by** $(numfmt --to=iec $((DIFF_BYTES * -1))) (-$((DIFF_PERCENT * -1))%)" >> $GITHUB_STEP_SUMMARY | ||
| else | ||
| echo "✅ **No size change**" >> $GITHUB_STEP_SUMMARY | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against divide-by-zero and show precise percentages.
If base-build.zip is empty, the script will divide by zero. Also, render percentages to two decimals for clarity.
# Get exact byte sizes for calculation
CURRENT_BYTES=$(stat -c%s current-build.zip)
BASE_BYTES=$(stat -c%s base-build.zip)
# Calculate difference
- DIFF_BYTES=$((CURRENT_BYTES - BASE_BYTES))
- DIFF_PERCENT=$((DIFF_BYTES * 100 / BASE_BYTES))
+ DIFF_BYTES=$((CURRENT_BYTES - BASE_BYTES))
+ if [ "$BASE_BYTES" -eq 0 ]; then
+ echo "❌ **Error**: base-build.zip is empty (0 bytes), cannot compute percentage delta." >> $GITHUB_STEP_SUMMARY
+ exit 1
+ fi
+ DIFF_PERCENT=$(awk -v d="$DIFF_BYTES" -v b="$BASE_BYTES" 'BEGIN { printf("%.2f", (d*100.0)/b) }')
@@
- if [ $DIFF_BYTES -gt 0 ]; then
- echo "📈 **Size increased by** $(numfmt --to=iec $DIFF_BYTES) (+$DIFF_PERCENT%)" >> $GITHUB_STEP_SUMMARY
- elif [ $DIFF_BYTES -lt 0 ]; then
- echo "📉 **Size decreased by** $(numfmt --to=iec $((DIFF_BYTES * -1))) (-$((DIFF_PERCENT * -1))%)" >> $GITHUB_STEP_SUMMARY
+ if [ $DIFF_BYTES -gt 0 ]; then
+ echo "📈 **Size increased by** $(numfmt --to=iec $DIFF_BYTES) (+$DIFF_PERCENT%)" >> $GITHUB_STEP_SUMMARY
+ elif [ $DIFF_BYTES -lt 0 ]; then
+ # flip sign for display
+ POS_PCT=$(awk -v p="$DIFF_PERCENT" 'BEGIN { printf("%.2f", (p<0)?-p:p) }')
+ echo "📉 **Size decreased by** $(numfmt --to=iec $((DIFF_BYTES * -1))) (-$POS_PCT%)" >> $GITHUB_STEP_SUMMARY
else:
echo "✅ **No size change**" >> $GITHUB_STEP_SUMMARY
fiCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
.github/workflows/build-size-comparison.yml around lines 81-121: the script
currently divides by BASE_BYTES which can be zero and calculates integer
percentages; update it to guard against divide-by-zero by checking if BASE_BYTES
is zero before computing DIFF_PERCENT — if BASE_BYTES is 0 and CURRENT_BYTES is
also 0 then report "No size change", otherwise report percentage as "N/A" or "∞"
as appropriate — and compute the percentage with two decimal places using a
floating-point tool like awk or bc (e.g. DIFF_PERCENT=$(awk "BEGIN {printf
\"%.2f\", ($DIFF_BYTES*100)/$BASE_BYTES}")), ensure sign handling when
DIFF_BYTES is negative and use numfmt for human-readable byte diffs as already
used for display.
📦 Build Size Comparison
📈 Total size increased by 38.0B (+0.0%) Files changed: 2 📋 All File Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.github/workflows/build-size-comparison.yml (2)
8-9: Skip forked PRs to avoid secret access failures.Forks don’t get org secrets; premium checkouts will fail. Gate the job.
jobs: build-and-compare: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false }} runs-on: ubuntu-latest
39-44: Update setup-node action and Node version (v3 is disabled; Node 14 is EOL).Use actions/setup-node@v4 with Node 20.x and cache all lockfiles.
- - name: Setup Node - uses: actions/setup-node@v3 - with: - node-version: 14.x - cache: 'npm' + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: 20.x + cache: npm + cache-dependency-path: | + package-lock.json + pro__premium_only/package-lock.json + base-branch/package-lock.json + base-branch/pro__premium_only/package-lock.json
🧹 Nitpick comments (3)
.github/workflows/build-size-comparison.yml (3)
67-76: Zip exclude patterns miss nested source maps; quiet zip output.Use recursive globs and keep quoting to avoid shell expansion.
- cd build/stackable - zip -r ../../current-build.zip . -x "*.map" "node_modules/*" + cd build/stackable + zip -qr ../../current-build.zip . -x '**/*.map' '**/node_modules/*' @@ - cd ../../base-branch/build/stackable - zip -r ../../../base-build.zip . -x "*.map" "node_modules/*" + cd ../../base-branch/build/stackable + zip -qr ../../../base-build.zip . -x '**/*.map' '**/node_modules/*'
307-345: Use the hidden marker to upsert reliably and set a safer match.Matching on user.type can miss cases; use the marker injected above.
- const existingComment = comments.find(comment => - comment.user.type === 'Bot' && - comment.body.includes('📦 Build Size Comparison') - ); + const MARKER = '<!-- build-size-comparison -->'; + const existingComment = comments.find(c => c.body && c.body.includes(MARKER));
1-353: Clean trailing spaces and add a final newline.YAMLlint flagged many trailing spaces and missing EOF newline; trim whitespace and ensure a newline at end of file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-size-comparison.yml(1 hunks)plugin.php(1 hunks)src/disabled-blocks.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/disabled-blocks.js
- plugin.php
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-size-comparison.yml
40-40: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/build-size-comparison.yml
[error] 72-72: trailing spaces
(trailing-spaces)
[error] 76-76: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 163-163: trailing spaces
(trailing-spaces)
[error] 166-166: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 179-179: trailing spaces
(trailing-spaces)
[error] 185-185: trailing spaces
(trailing-spaces)
[error] 196-196: trailing spaces
(trailing-spaces)
[error] 203-203: trailing spaces
(trailing-spaces)
[error] 206-206: trailing spaces
(trailing-spaces)
[error] 212-212: trailing spaces
(trailing-spaces)
[error] 217-217: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[error] 228-228: trailing spaces
(trailing-spaces)
[error] 231-231: trailing spaces
(trailing-spaces)
[error] 236-236: trailing spaces
(trailing-spaces)
[error] 244-244: trailing spaces
(trailing-spaces)
[error] 251-251: trailing spaces
(trailing-spaces)
[error] 253-253: trailing spaces
(trailing-spaces)
[error] 255-255: trailing spaces
(trailing-spaces)
[error] 261-261: trailing spaces
(trailing-spaces)
[error] 269-269: trailing spaces
(trailing-spaces)
[error] 276-276: trailing spaces
(trailing-spaces)
[error] 282-282: trailing spaces
(trailing-spaces)
[error] 290-290: trailing spaces
(trailing-spaces)
[error] 297-297: trailing spaces
(trailing-spaces)
[error] 299-299: trailing spaces
(trailing-spaces)
[error] 303-303: trailing spaces
(trailing-spaces)
[error] 313-313: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
[error] 321-321: trailing spaces
(trailing-spaces)
[error] 322-322: trailing spaces
(trailing-spaces)
[error] 325-325: trailing spaces
(trailing-spaces)
[error] 353-353: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-compare
| name: Build Size Comparison | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ master, develop ] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add explicit permissions and concurrency to harden and speed up the workflow.
- Minimal permissions are safer, and comment posting needs issues: write.
- Concurrency cancels stale runs for the same PR.
name: Build Size Comparison
on:
pull_request:
branches: [ master, develop ]
+permissions:
+ contents: read
+ issues: write
+ pull-requests: write
+
+concurrency:
+ group: build-size-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: Build Size Comparison | |
| on: | |
| pull_request: | |
| branches: [ master, develop ] | |
| name: Build Size Comparison | |
| on: | |
| pull_request: | |
| branches: [ master, develop ] | |
| permissions: | |
| contents: read | |
| issues: write | |
| pull-requests: write | |
| concurrency: | |
| group: build-size-${{ github.event.pull_request.number || github.ref }} | |
| cancel-in-progress: true |
🤖 Prompt for AI Agents
.github/workflows/build-size-comparison.yml lines 1-6: add explicit
workflow-level permissions and a concurrency key to the YAML to tighten security
and cancel stale runs; set permissions to the minimal required (e.g.,
permissions: contents: read, issues: write) so the job can post comments, and
add a concurrency block (e.g., group: build-size-comparison-${{
github.event.pull_request.number }} and cancel-in-progress: true) to ensure only
the latest run for a PR proceeds.
| - name: Extract and Compare Files | ||
| run: | | ||
| # Extract both zip files | ||
| mkdir -p current-files base-files | ||
| unzip -q current-build.zip -d current-files/ | ||
| unzip -q base-build.zip -d base-files/ | ||
| # Create comparison script | ||
| cat > compare_files.py << 'EOF' | ||
| import os | ||
| import json | ||
| from pathlib import Path | ||
| def get_file_size(filepath): | ||
| if os.path.exists(filepath): | ||
| return os.path.getsize(filepath) | ||
| return 0 | ||
| def format_bytes(bytes): | ||
| for unit in ['B', 'KB', 'MB', 'GB']: | ||
| if bytes < 1024.0: | ||
| return f"{bytes:.1f}{unit}" | ||
| bytes /= 1024.0 | ||
| return f"{bytes:.1f}TB" | ||
| def compare_directories(current_dir, base_dir): | ||
| current_path = Path(current_dir) | ||
| base_path = Path(base_dir) | ||
| all_files = set() | ||
| # Get all files from both directories | ||
| for file_path in current_path.rglob('*'): | ||
| if file_path.is_file(): | ||
| rel_path = file_path.relative_to(current_path) | ||
| all_files.add(str(rel_path)) | ||
| for file_path in base_path.rglob('*'): | ||
| if file_path.is_file(): | ||
| rel_path = file_path.relative_to(base_path) | ||
| all_files.add(str(rel_path)) | ||
| changes = [] | ||
| total_current = 0 | ||
| total_base = 0 | ||
| for file_path in sorted(all_files): | ||
| current_file = current_path / file_path | ||
| base_file = base_path / file_path | ||
| current_size = get_file_size(current_file) | ||
| base_size = get_file_size(base_file) | ||
| total_current += current_size | ||
| total_base += base_size | ||
| if current_size != base_size: | ||
| diff = current_size - base_size | ||
| if base_size > 0: | ||
| percent = (diff / base_size) * 100 | ||
| else: | ||
| percent = 100 if current_size > 0 else 0 | ||
| status = "🆕" if base_size == 0 else "❌" if current_size == 0 else "📝" | ||
| changes.append({ | ||
| 'file': file_path, | ||
| 'current_size': current_size, | ||
| 'base_size': base_size, | ||
| 'diff': diff, | ||
| 'percent': percent, | ||
| 'status': status | ||
| }) | ||
| return changes, total_current, total_base | ||
| # Compare files | ||
| changes, total_current, total_base = compare_directories('current-files', 'base-files') | ||
| # Create summary | ||
| total_diff = total_current - total_base | ||
| total_percent = (total_diff / total_base * 100) if total_base > 0 else 0 | ||
| # Flag large changes (>10% or >100KB) | ||
| flagged_changes = [c for c in changes if abs(c['percent']) > 10 or abs(c['diff']) > 102400] | ||
| # Generate report | ||
| report = { | ||
| 'total_current': total_current, | ||
| 'total_base': total_base, | ||
| 'total_diff': total_diff, | ||
| 'total_percent': total_percent, | ||
| 'changes': changes, | ||
| 'flagged_changes': flagged_changes | ||
| } | ||
| with open('comparison_report.json', 'w') as f: | ||
| json.dump(report, f, indent=2) | ||
| # Print summary to console | ||
| print(f"Total size change: {format_bytes(total_diff)} ({total_percent:+.1f}%)") | ||
| print(f"Files changed: {len(changes)}") | ||
| print(f"Flagged changes: {len(flagged_changes)}") | ||
| EOF | ||
| python3 compare_files.py | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard total percentage against base=0 and standardize to 2 decimals.
Current total_percent becomes 0 when total_base==0, which is misleading. Report N/A (or ∞) and format with two decimals.
- total_diff = total_current - total_base
- total_percent = (total_diff / total_base * 100) if total_base > 0 else 0
+ total_diff = total_current - total_base
+ if total_base > 0:
+ total_percent = (total_diff / total_base) * 100.0
+ elif total_current == 0:
+ total_percent = 0.0
+ else:
+ total_percent = None # N/A: cannot compute percent vs 0 base
@@
- print(f"Total size change: {format_bytes(total_diff)} ({total_percent:+.1f}%)")
+ pct_str = "N/A" if total_percent is None else f"{total_percent:+.2f}%"
+ print(f"Total size change: {format_bytes(total_diff)} ({pct_str})")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Extract and Compare Files | |
| run: | | |
| # Extract both zip files | |
| mkdir -p current-files base-files | |
| unzip -q current-build.zip -d current-files/ | |
| unzip -q base-build.zip -d base-files/ | |
| # Create comparison script | |
| cat > compare_files.py << 'EOF' | |
| import os | |
| import json | |
| from pathlib import Path | |
| def get_file_size(filepath): | |
| if os.path.exists(filepath): | |
| return os.path.getsize(filepath) | |
| return 0 | |
| def format_bytes(bytes): | |
| for unit in ['B', 'KB', 'MB', 'GB']: | |
| if bytes < 1024.0: | |
| return f"{bytes:.1f}{unit}" | |
| bytes /= 1024.0 | |
| return f"{bytes:.1f}TB" | |
| def compare_directories(current_dir, base_dir): | |
| current_path = Path(current_dir) | |
| base_path = Path(base_dir) | |
| all_files = set() | |
| # Get all files from both directories | |
| for file_path in current_path.rglob('*'): | |
| if file_path.is_file(): | |
| rel_path = file_path.relative_to(current_path) | |
| all_files.add(str(rel_path)) | |
| for file_path in base_path.rglob('*'): | |
| if file_path.is_file(): | |
| rel_path = file_path.relative_to(base_path) | |
| all_files.add(str(rel_path)) | |
| changes = [] | |
| total_current = 0 | |
| total_base = 0 | |
| for file_path in sorted(all_files): | |
| current_file = current_path / file_path | |
| base_file = base_path / file_path | |
| current_size = get_file_size(current_file) | |
| base_size = get_file_size(base_file) | |
| total_current += current_size | |
| total_base += base_size | |
| if current_size != base_size: | |
| diff = current_size - base_size | |
| if base_size > 0: | |
| percent = (diff / base_size) * 100 | |
| else: | |
| percent = 100 if current_size > 0 else 0 | |
| status = "🆕" if base_size == 0 else "❌" if current_size == 0 else "📝" | |
| changes.append({ | |
| 'file': file_path, | |
| 'current_size': current_size, | |
| 'base_size': base_size, | |
| 'diff': diff, | |
| 'percent': percent, | |
| 'status': status | |
| }) | |
| return changes, total_current, total_base | |
| # Compare files | |
| changes, total_current, total_base = compare_directories('current-files', 'base-files') | |
| # Create summary | |
| total_diff = total_current - total_base | |
| total_percent = (total_diff / total_base * 100) if total_base > 0 else 0 | |
| # Flag large changes (>10% or >100KB) | |
| flagged_changes = [c for c in changes if abs(c['percent']) > 10 or abs(c['diff']) > 102400] | |
| # Generate report | |
| report = { | |
| 'total_current': total_current, | |
| 'total_base': total_base, | |
| 'total_diff': total_diff, | |
| 'total_percent': total_percent, | |
| 'changes': changes, | |
| 'flagged_changes': flagged_changes | |
| } | |
| with open('comparison_report.json', 'w') as f: | |
| json.dump(report, f, indent=2) | |
| # Print summary to console | |
| print(f"Total size change: {format_bytes(total_diff)} ({total_percent:+.1f}%)") | |
| print(f"Files changed: {len(changes)}") | |
| print(f"Flagged changes: {len(flagged_changes)}") | |
| EOF | |
| python3 compare_files.py | |
| # Create summary | |
| total_diff = total_current - total_base | |
| if total_base > 0: | |
| total_percent = (total_diff / total_base) * 100.0 | |
| elif total_current == 0: | |
| total_percent = 0.0 | |
| else: | |
| total_percent = None # N/A: cannot compute percent vs 0 base | |
| # Flag large changes (>10% or >100KB) | |
| flagged_changes = [c for c in changes if abs(c['percent']) > 10 or abs(c['diff']) > 102400] | |
| # Generate report | |
| report = { | |
| 'total_current': total_current, | |
| 'total_base': total_base, | |
| 'total_diff': total_diff, | |
| 'total_percent': total_percent, | |
| 'changes': changes, | |
| 'flagged_changes': flagged_changes | |
| } | |
| with open('comparison_report.json', 'w') as f: | |
| json.dump(report, f, indent=2) | |
| # Print summary to console | |
| pct_str = "N/A" if total_percent is None else f"{total_percent:+.2f}%" | |
| print(f"Total size change: {format_bytes(total_diff)} ({pct_str})") | |
| print(f"Files changed: {len(changes)}") | |
| print(f"Flagged changes: {len(flagged_changes)}") |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 133-133: trailing spaces
(trailing-spaces)
[error] 136-136: trailing spaces
(trailing-spaces)
[error] 143-143: trailing spaces
(trailing-spaces)
[error] 145-145: trailing spaces
(trailing-spaces)
[error] 154-154: trailing spaces
(trailing-spaces)
[error] 159-159: trailing spaces
(trailing-spaces)
[error] 163-163: trailing spaces
(trailing-spaces)
[error] 166-166: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 179-179: trailing spaces
(trailing-spaces)
[error] 185-185: trailing spaces
(trailing-spaces)
| - name: Generate PR Comment | ||
| id: pr_comment | ||
| run: | | ||
| # Read the comparison report | ||
| python3 -c " | ||
| import json | ||
| with open('comparison_report.json', 'r') as f: | ||
| report = json.load(f) | ||
| def format_bytes(bytes): | ||
| for unit in ['B', 'KB', 'MB', 'GB']: | ||
| if bytes < 1024.0: | ||
| return f'{bytes:.1f}{unit}' | ||
| bytes /= 1024.0 | ||
| return f'{bytes:.1f}TB' | ||
| # Generate comment | ||
| comment = '## 📦 Build Size Comparison\n\n' | ||
| # Overall summary | ||
| total_current = report['total_current'] | ||
| total_base = report['total_base'] | ||
| total_diff = report['total_diff'] | ||
| total_percent = report['total_percent'] | ||
| comment += '| Build | Size |\n' | ||
| comment += '|-------|------|\n' | ||
| comment += f'| **Current PR** | **{format_bytes(total_current)}** |\n' | ||
| comment += f'| Base Branch | {format_bytes(total_base)} |\n\n' | ||
| if total_diff > 0: | ||
| comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{total_percent:.1f}%)\n\n' | ||
| elif total_diff < 0: | ||
| comment += f'📉 **Total size decreased by** {format_bytes(abs(total_diff))} ({total_percent:.1f}%)\n\n' | ||
| else: | ||
| comment += '✅ **No total size change**\n\n' | ||
| # File changes summary | ||
| changes = report['changes'] | ||
| flagged = report['flagged_changes'] | ||
| comment += f'**Files changed:** {len(changes)}\n' | ||
| comment += f'**Large changes flagged:** {len(flagged)}\n\n' | ||
| if flagged: | ||
| comment += '## ⚠️ Large Changes (Requires Review)\n\n' | ||
| comment += '| File | Current | Base | Change | % | Status |\n' | ||
| comment += '|------|---------|------|--------|---|--------|\n' | ||
| for change in flagged: | ||
| status_icon = change['status'] | ||
| file_name = change['file'] | ||
| current_size = format_bytes(change['current_size']) | ||
| base_size = format_bytes(change['base_size']) | ||
| diff_size = format_bytes(abs(change['diff'])) | ||
| percent = change['percent'] | ||
| if change['diff'] > 0: | ||
| change_str = f'+{diff_size}' | ||
| elif change['diff'] < 0: | ||
| change_str = f'-{diff_size}' | ||
| else: | ||
| change_str = '0B' | ||
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | ||
| comment += '\n' | ||
| # All changes table (if not too many) | ||
| if len(changes) <= 50: | ||
| comment += '## 📋 All File Changes\n\n' | ||
| comment += '| File | Current | Base | Change | % | Status |\n' | ||
| comment += '|------|---------|------|--------|---|--------|\n' | ||
| for change in changes: | ||
| status_icon = change['status'] | ||
| file_name = change['file'] | ||
| current_size = format_bytes(change['current_size']) | ||
| base_size = format_bytes(change['base_size']) | ||
| diff_size = format_bytes(abs(change['diff'])) | ||
| percent = change['percent'] | ||
| if change['diff'] > 0: | ||
| change_str = f'+{diff_size}' | ||
| elif change['diff'] < 0: | ||
| change_str = f'-{diff_size}' | ||
| else: | ||
| change_str = '0B' | ||
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | ||
| else: | ||
| comment += f'## 📋 File Changes (showing first 50 of {len(changes)})\n\n' | ||
| comment += '| File | Current | Base | Change | % | Status |\n' | ||
| comment += '|------|---------|------|--------|---|--------|\n' | ||
| for change in changes[:50]: | ||
| status_icon = change['status'] | ||
| file_name = change['file'] | ||
| current_size = format_bytes(change['current_size']) | ||
| base_size = format_bytes(change['base_size']) | ||
| diff_size = format_bytes(abs(change['diff'])) | ||
| percent = change['percent'] | ||
| if change['diff'] > 0: | ||
| change_str = f'+{diff_size}' | ||
| elif change['diff'] < 0: | ||
| change_str = f'-{diff_size}' | ||
| else: | ||
| change_str = '0B' | ||
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | ||
| # Save comment to file | ||
| with open('pr_comment.md', 'w') as f: | ||
| f.write(comment) | ||
| print('PR comment generated') | ||
| " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle N/A totals, use 2‑decimal percents, and add a hidden marker for stable upserts.
Prepend a hidden marker and render percentages consistently; avoid formatting None.
- comment = '## 📦 Build Size Comparison\n\n'
+ comment = '<!-- build-size-comparison -->\n## 📦 Build Size Comparison\n\n'
@@
- if total_diff > 0:
- comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{total_percent:.1f}%)\n\n'
- elif total_diff < 0:
- comment += f'📉 **Total size decreased by** {format_bytes(abs(total_diff))} ({total_percent:.1f}%)\n\n'
+ def fmt_pct(p):
+ return 'N/A' if p is None else f'{p:+.2f}%'
+ if total_diff > 0:
+ comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{fmt_pct(total_percent).lstrip("+")})\n\n'
+ elif total_diff < 0:
+ comment += f'📉 **Total size decreased by** {format_bytes(abs(total_diff))} ({fmt_pct(total_percent)})\n\n'
else:
comment += '✅ **No total size change**\n\n'
@@
- comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n'
+ comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n'
@@
- comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n'
+ comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n'
@@
- comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n'
+ comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Generate PR Comment | |
| id: pr_comment | |
| run: | | |
| # Read the comparison report | |
| python3 -c " | |
| import json | |
| with open('comparison_report.json', 'r') as f: | |
| report = json.load(f) | |
| def format_bytes(bytes): | |
| for unit in ['B', 'KB', 'MB', 'GB']: | |
| if bytes < 1024.0: | |
| return f'{bytes:.1f}{unit}' | |
| bytes /= 1024.0 | |
| return f'{bytes:.1f}TB' | |
| # Generate comment | |
| comment = '## 📦 Build Size Comparison\n\n' | |
| # Overall summary | |
| total_current = report['total_current'] | |
| total_base = report['total_base'] | |
| total_diff = report['total_diff'] | |
| total_percent = report['total_percent'] | |
| comment += '| Build | Size |\n' | |
| comment += '|-------|------|\n' | |
| comment += f'| **Current PR** | **{format_bytes(total_current)}** |\n' | |
| comment += f'| Base Branch | {format_bytes(total_base)} |\n\n' | |
| if total_diff > 0: | |
| comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{total_percent:.1f}%)\n\n' | |
| elif total_diff < 0: | |
| comment += f'📉 **Total size decreased by** {format_bytes(abs(total_diff))} ({total_percent:.1f}%)\n\n' | |
| else: | |
| comment += '✅ **No total size change**\n\n' | |
| # File changes summary | |
| changes = report['changes'] | |
| flagged = report['flagged_changes'] | |
| comment += f'**Files changed:** {len(changes)}\n' | |
| comment += f'**Large changes flagged:** {len(flagged)}\n\n' | |
| if flagged: | |
| comment += '## ⚠️ Large Changes (Requires Review)\n\n' | |
| comment += '| File | Current | Base | Change | % | Status |\n' | |
| comment += '|------|---------|------|--------|---|--------|\n' | |
| for change in flagged: | |
| status_icon = change['status'] | |
| file_name = change['file'] | |
| current_size = format_bytes(change['current_size']) | |
| base_size = format_bytes(change['base_size']) | |
| diff_size = format_bytes(abs(change['diff'])) | |
| percent = change['percent'] | |
| if change['diff'] > 0: | |
| change_str = f'+{diff_size}' | |
| elif change['diff'] < 0: | |
| change_str = f'-{diff_size}' | |
| else: | |
| change_str = '0B' | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | |
| comment += '\n' | |
| # All changes table (if not too many) | |
| if len(changes) <= 50: | |
| comment += '## 📋 All File Changes\n\n' | |
| comment += '| File | Current | Base | Change | % | Status |\n' | |
| comment += '|------|---------|------|--------|---|--------|\n' | |
| for change in changes: | |
| status_icon = change['status'] | |
| file_name = change['file'] | |
| current_size = format_bytes(change['current_size']) | |
| base_size = format_bytes(change['base_size']) | |
| diff_size = format_bytes(abs(change['diff'])) | |
| percent = change['percent'] | |
| if change['diff'] > 0: | |
| change_str = f'+{diff_size}' | |
| elif change['diff'] < 0: | |
| change_str = f'-{diff_size}' | |
| else: | |
| change_str = '0B' | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | |
| else: | |
| comment += f'## 📋 File Changes (showing first 50 of {len(changes)})\n\n' | |
| comment += '| File | Current | Base | Change | % | Status |\n' | |
| comment += '|------|---------|------|--------|---|--------|\n' | |
| for change in changes[:50]: | |
| status_icon = change['status'] | |
| file_name = change['file'] | |
| current_size = format_bytes(change['current_size']) | |
| base_size = format_bytes(change['base_size']) | |
| diff_size = format_bytes(abs(change['diff'])) | |
| percent = change['percent'] | |
| if change['diff'] > 0: | |
| change_str = f'+{diff_size}' | |
| elif change['diff'] < 0: | |
| change_str = f'-{diff_size}' | |
| else: | |
| change_str = '0B' | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.1f}% | {status_icon} |\n' | |
| # Save comment to file | |
| with open('pr_comment.md', 'w') as f: | |
| f.write(comment) | |
| print('PR comment generated') | |
| " | |
| # Generate comment | |
| comment = '<!-- build-size-comparison -->\n## 📦 Build Size Comparison\n\n' | |
| # Overall summary | |
| total_current = report['total_current'] | |
| total_base = report['total_base'] | |
| total_diff = report['total_diff'] | |
| total_percent = report['total_percent'] | |
| - if total_diff > 0: | |
| - comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{total_percent:.1f}%)\n\n' | |
| - elif total_diff < 0: | |
| def fmt_pct(p): | |
| return 'N/A' if p is None else f'{p:+.2f}%' | |
| if total_diff > 0: | |
| comment += f'📈 **Total size increased by** {format_bytes(total_diff)} (+{fmt_pct(total_percent).lstrip("+")})\n\n' | |
| elif total_diff < 0: | |
| comment += f'📉 **Total size decreased by** {format_bytes(abs(total_diff))} ({fmt_pct(total_percent)})\n\n' | |
| else: | |
| comment += '✅ **No total size change**\n\n' | |
| @@ | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n' | |
| @@ | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n' | |
| @@ | |
| comment += f'| `{file_name}` | {current_size} | {base_size} | {change_str} | {percent:+.2f}% | {status_icon} |\n' |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 196-196: trailing spaces
(trailing-spaces)
[error] 203-203: trailing spaces
(trailing-spaces)
[error] 206-206: trailing spaces
(trailing-spaces)
[error] 212-212: trailing spaces
(trailing-spaces)
[error] 217-217: trailing spaces
(trailing-spaces)
[error] 224-224: trailing spaces
(trailing-spaces)
[error] 228-228: trailing spaces
(trailing-spaces)
[error] 231-231: trailing spaces
(trailing-spaces)
[error] 236-236: trailing spaces
(trailing-spaces)
[error] 244-244: trailing spaces
(trailing-spaces)
[error] 251-251: trailing spaces
(trailing-spaces)
[error] 253-253: trailing spaces
(trailing-spaces)
[error] 255-255: trailing spaces
(trailing-spaces)
[error] 261-261: trailing spaces
(trailing-spaces)
[error] 269-269: trailing spaces
(trailing-spaces)
[error] 276-276: trailing spaces
(trailing-spaces)
[error] 282-282: trailing spaces
(trailing-spaces)
[error] 290-290: trailing spaces
(trailing-spaces)
[error] 297-297: trailing spaces
(trailing-spaces)
[error] 299-299: trailing spaces
(trailing-spaces)
[error] 303-303: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/build-size-comparison.yml around lines 188-305, the PR
comment generator needs three fixes: prepend a hidden HTML marker to the top of
the generated comment (e.g. <!-- build-size-comparison -->) so updates are
idempotent; defensively handle None/absent totals and per-file percent values by
treating them as "N/A" instead of trying to format None; and render all percent
values with two decimals (use format like f'{percent:+.2f}%' or 'N/A' when
percent is None). Update the code to compute safe strings before interpolating
(e.g. total_percent_str = f'{total_percent:+.2f}%' if total_percent is not None
else 'N/A'), use those strings in the comment, and write the hidden marker as
the first line of the comment file.
Summary by CodeRabbit