Skip to content

Add post-PR metrics script for GitHub actions#688

Open
saiganesh47 wants to merge 1 commit intothunderbird:mainfrom
saiganesh47:patch-2
Open

Add post-PR metrics script for GitHub actions#688
saiganesh47 wants to merge 1 commit intothunderbird:mainfrom
saiganesh47:patch-2

Conversation

@saiganesh47
Copy link
Copy Markdown

@saiganesh47 saiganesh47 commented Apr 18, 2026

Note

Low Risk
Low risk: adds a new GitHub Actions helper script that only posts/updates a PR comment based on environment variables and a baseline file, without touching production code paths.

Overview
Adds a new .github/scripts/optimized post-pr-metrics.test.js GitHub Actions script that posts (or updates) a tagged PR comment summarizing bundle gzip size and test coverage, optionally showing deltas vs a .metrics-baseline/metrics.json baseline.

The script validates/normalizes env-provided metrics, skips posting unless BUILD_OUTCOME is success and PREVIEW_READY is explicitly set, and uses a stable <!-- pr-metrics --> marker to update an existing comment instead of creating duplicates.

Reviewed by Cursor Bugbot for commit 6193de5. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Sai Ganesh Maganti <msaiganesh23@gmail.com>
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.


function diffText(current, baseline, formatter = v => v) {
if (current == null) return "—"
if (!baseline) return formatter(current)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Falsy check skips diff when baseline is zero

Medium Severity

The diffText function uses if (!baseline) to check for a missing baseline, but this falsiness check also matches when baseline is 0. A baseline bundle size of 0 or baseline coverage of 0% would skip the diff calculation entirely and display the value without any change indicator, as if no baseline existed. The check needs to be baseline == null to properly distinguish "no baseline" from a zero-valued baseline.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.

}
}

module.exports = { postPrMetrics }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production code in unreferenced file with test extension

Medium Severity

This file appears to be accidentally committed. It has a .test.js extension but contains production implementation code (not tests), its filename contains a space (optimized post-pr-metrics.test.js), and its postPrMetrics export is never imported or referenced anywhere in the codebase — the workflow still uses post-pr-metrics.cjs. The existing post-pr-metrics.test.js file contains the actual tests. This seems like a scratch/draft file that was not meant to be committed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.

async function postPrMetrics({ github, context }) {
// Skip conditions
if (process.env.BUILD_OUTCOME !== "success") return
if (process.env.PREVIEW_READY !== "true" && process.env.PREVIEW_READY !== "false") return
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated PREVIEW_READY guard suppresses all metrics posting

Medium Severity

The PREVIEW_READY env var controls whether the Render deploy preview is ready for Lighthouse testing, yet it's used here as a top-level gate for the entire function. This script doesn't even include Lighthouse metrics — it only posts bundle size and coverage. If the preview step is skipped or doesn't output a value, PREVIEW_READY will be undefined, causing the function to silently return without posting any metrics comment at all. The original post-pr-metrics.cjs correctly uses PREVIEW_READY only for the Lighthouse section, not as a top-level guard.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.

const coverage = readCoverage(process.env.COVERAGE)

const baseline = safeReadBaseline()
const baselineBundle = baseline?.bundleSize ?? null
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Baseline bundle size bypasses readNumber validation

Low Severity

baselineBundle is read directly from the JSON file without validation, while baselineCoverage on the next line is properly validated through readCoverage. The readNumber helper exists for exactly this purpose. If the baseline file contains a non-numeric bundleSize value, diffText would compute current - baseline producing NaN, and formatBytes(NaN) would render as "NaN KB" in the PR comment.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant