feat: unified PR report combining bundle size and runtime perf#9911
Open
christian-byrne wants to merge 2 commits intomainfrom
Open
feat: unified PR report combining bundle size and runtime perf#9911christian-byrne wants to merge 2 commits intomainfrom
christian-byrne wants to merge 2 commits intomainfrom
Conversation
dosubot
bot
added
the
size:XL
🎭 Playwright: ✅ 550 passed, 0 failed · 10 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
Contributor
📝 WalkthroughWalkthroughDeletes two legacy PR-reporting workflows, updates CI workflows to save PR head SHA metadata, adds a new unified PR workflow that aggregates size and perf artifacts and posts a single PR comment, and introduces a script to generate the combined size+performance report. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant CI_Size as CI: Size Data
participant CI_Perf as CI: Performance Report
participant Artifact as Artifact Storage
participant PR_Workflow as PR: Unified Report
participant Script as unified-report.js
participant GitHub as GitHub (PR)
Dev->>CI_Size: push / run CI
Dev->>CI_Perf: push / run CI
CI_Size->>Artifact: upload size data + head-sha.txt
CI_Perf->>Artifact: upload perf metrics + head-sha.txt
CI_Size->>PR_Workflow: workflow_run complete
CI_Perf->>PR_Workflow: workflow_run complete
PR_Workflow->>CI_Size: fetch run metadata/status
PR_Workflow->>CI_Perf: fetch run metadata/status
PR_Workflow->>Artifact: download current & baseline size data
PR_Workflow->>Artifact: download current & baseline perf metrics
PR_Workflow->>Script: exec unified-report.js --size-status --perf-status
Script->>Script: generate Bundle Size section (calls size-report.js if ready)
Script->>Script: generate Performance section (calls perf-report.ts if ready)
Script-->>PR_Workflow: pr-report.md
PR_Workflow->>GitHub: remove legacy comments
PR_Workflow->>GitHub: post unified report comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
📦 Bundle: 4.99 MB gzip 🔴 +317 BDetailsSummary
Category Glance App Entry Points — 21.7 kB (baseline 21.7 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.08 MB (baseline 1.08 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 75.3 kB (baseline 75.3 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 460 kB (baseline 460 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.6 kB (baseline 16.6 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 81.8 kB (baseline 81.8 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 59 kB (baseline 59 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.17 MB (baseline 3.17 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 15 added / 15 removed Utilities & Hooks — 68.9 kB (baseline 68.9 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 9.78 MB (baseline 9.78 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 8.21 MB (baseline 8.21 MB) • ⚪ 0 BBundles that do not match a named category
Status: 52 added / 52 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/pr-report.yaml (1)
195-211: Legacy comment deletion won't paginate beyond 100 comments.
listCommentswithper_page: 100will only return the first 100 comments. For PRs with more comments, legacy markers beyond the first page won't be deleted. This is likely acceptable for most cases, but worth noting if legacy cleanup completeness is important.♻️ Optional: Add pagination for complete cleanup
- const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - per_page: 100, - }); + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + per_page: 100, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-report.yaml around lines 195 - 211, The current cleanup only fetches the first page because github.rest.issues.listComments is called with per_page: 100; replace that call with Octokit's pagination (e.g., github.paginate or octokit.paginate) to retrieve all comments for the PR before iterating, then keep the existing loop that checks legacyMarkers and calls github.rest.issues.deleteComment for each match; update the variable usage around the returned collection (comments) so the for (const comment of comments) logic operates on the fully paginated list instead of a single page.scripts/unified-report.js (1)
21-27: Consider adding error handling for subprocess failures.
execFileSyncwill throw if the child process exits with a non-zero code. While the workflow may handle step failures, wrapping these calls in try-catch would allow for graceful fallback messages in the report rather than workflow failures.♻️ Optional: Add try-catch for graceful degradation
if (sizeStatus === 'ready') { - const sizeReport = execFileSync( - 'node', - ['scripts/size-report.js'], - { encoding: 'utf-8' } - ).trimEnd() - lines.push(sizeReport) + try { + const sizeReport = execFileSync( + 'node', + ['scripts/size-report.js'], + { encoding: 'utf-8' } + ).trimEnd() + lines.push(sizeReport) + } catch { + lines.push('## 📦 Bundle Size') + lines.push('') + lines.push('> ⚠️ Size report generation failed. Check the CI workflow logs.') + } } else if (sizeStatus === 'failed') {Apply similar wrapping for the perf section (lines 42-47).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/unified-report.js` around lines 21 - 27, Wrap the synchronous child-process calls that build the report in try-catch blocks to avoid crashing the whole script on non-zero exits: specifically, surround the execFileSync call that runs 'node' with ['scripts/size-report.js'] (used when sizeStatus === 'ready' and assigns sizeReport, then pushes to lines) with try/catch and on error push a clear fallback message (including error.message) to lines instead of letting the exception propagate; do the same for the perf execFileSync call that runs ['scripts/perf-report.js'] (the code that assigns perfReport and pushes to lines) so both sections gracefully degrade and include error details in the report.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/pr-report.yaml:
- Around line 195-211: The current cleanup only fetches the first page because
github.rest.issues.listComments is called with per_page: 100; replace that call
with Octokit's pagination (e.g., github.paginate or octokit.paginate) to
retrieve all comments for the PR before iterating, then keep the existing loop
that checks legacyMarkers and calls github.rest.issues.deleteComment for each
match; update the variable usage around the returned collection (comments) so
the for (const comment of comments) logic operates on the fully paginated list
instead of a single page.
In `@scripts/unified-report.js`:
- Around line 21-27: Wrap the synchronous child-process calls that build the
report in try-catch blocks to avoid crashing the whole script on non-zero exits:
specifically, surround the execFileSync call that runs 'node' with
['scripts/size-report.js'] (used when sizeStatus === 'ready' and assigns
sizeReport, then pushes to lines) with try/catch and on error push a clear
fallback message (including error.message) to lines instead of letting the
exception propagate; do the same for the perf execFileSync call that runs
['scripts/perf-report.js'] (the code that assigns perfReport and pushes to
lines) so both sections gracefully degrade and include error details in the
report.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f3578dd-819d-4e0a-a1c2-07a878ad33f1
📒 Files selected for processing (6)
.github/workflows/ci-perf-report.yaml.github/workflows/ci-size-data.yaml.github/workflows/pr-perf-report.yaml.github/workflows/pr-report.yaml.github/workflows/pr-size-report.yamlscripts/unified-report.js
💤 Files with no reviewable changes (2)
- .github/workflows/pr-size-report.yaml
- .github/workflows/pr-perf-report.yaml
⚡ Performance Report
All metrics
Historical variance (last 2 runs)
Raw data{
"timestamp": "2026-03-15T10:42:42.426Z",
"gitSha": "6287e3c25135e8c88009c7394c2cf2258e42019a",
"branch": "feat/unified-pr-perf-report",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2030.7090000000017,
"styleRecalcs": 15,
"styleRecalcDurationMs": 15.303999999999998,
"layouts": 1,
"layoutDurationMs": 0.30099999999999993,
"taskDurationMs": 381.31299999999993,
"heapDeltaBytes": 1266604,
"domNodes": 68,
"jsHeapTotalBytes": 18874368,
"scriptDurationMs": 23.270000000000003,
"eventListeners": 23
},
{
"name": "canvas-idle",
"durationMs": 2028.9999999999964,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.059999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.424,
"heapDeltaBytes": 1286412,
"domNodes": 22,
"jsHeapTotalBytes": 18350080,
"scriptDurationMs": 27.266,
"eventListeners": 6
},
{
"name": "canvas-idle",
"durationMs": 2037.7480000000219,
"styleRecalcs": 11,
"styleRecalcDurationMs": 9.665999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 421.427,
"heapDeltaBytes": 542756,
"domNodes": 21,
"jsHeapTotalBytes": 18874368,
"scriptDurationMs": 30.063,
"eventListeners": 6
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1820.0630000000046,
"styleRecalcs": 78,
"styleRecalcDurationMs": 43.052,
"layouts": 13,
"layoutDurationMs": 3.826,
"taskDurationMs": 764.972,
"heapDeltaBytes": -2775092,
"domNodes": 104,
"jsHeapTotalBytes": 19922944,
"scriptDurationMs": 122.88399999999999,
"eventListeners": 21
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1888.3789999999863,
"styleRecalcs": 79,
"styleRecalcDurationMs": 52.656,
"layouts": 13,
"layoutDurationMs": 4.1419999999999995,
"taskDurationMs": 832.1999999999999,
"heapDeltaBytes": -3339944,
"domNodes": 106,
"jsHeapTotalBytes": 19660800,
"scriptDurationMs": 132.64,
"eventListeners": 21
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1831.875000000025,
"styleRecalcs": 77,
"styleRecalcDurationMs": 42.87,
"layouts": 12,
"layoutDurationMs": 3.9489999999999994,
"taskDurationMs": 797.7529999999999,
"heapDeltaBytes": -2774472,
"domNodes": 60,
"jsHeapTotalBytes": 18350080,
"scriptDurationMs": 126.554,
"eventListeners": 4
},
{
"name": "dom-widget-clipping",
"durationMs": 551.2219999999957,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.172999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 341.774,
"heapDeltaBytes": 12550500,
"domNodes": 22,
"jsHeapTotalBytes": 17825792,
"scriptDurationMs": 60.24499999999999,
"eventListeners": 2
},
{
"name": "dom-widget-clipping",
"durationMs": 571.4760000000183,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.575000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 352.123,
"heapDeltaBytes": 12770676,
"domNodes": 22,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 63.56,
"eventListeners": 2
},
{
"name": "dom-widget-clipping",
"durationMs": 549.5999999999981,
"styleRecalcs": 13,
"styleRecalcDurationMs": 8.476,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 339.8059999999999,
"heapDeltaBytes": 7003736,
"domNodes": 23,
"jsHeapTotalBytes": 12845056,
"scriptDurationMs": 61.224000000000004,
"eventListeners": 2
},
{
"name": "large-graph-idle",
"durationMs": 2021.9260000000077,
"styleRecalcs": 14,
"styleRecalcDurationMs": 14.335,
"layouts": 1,
"layoutDurationMs": 0.19399999999999973,
"taskDurationMs": 547.071,
"heapDeltaBytes": 16980216,
"domNodes": 69,
"jsHeapTotalBytes": 8101888,
"scriptDurationMs": 100.65000000000002,
"eventListeners": 23
},
{
"name": "large-graph-idle",
"durationMs": 2047.7779999999939,
"styleRecalcs": 14,
"styleRecalcDurationMs": 15.286999999999999,
"layouts": 1,
"layoutDurationMs": 0.2179999999999999,
"taskDurationMs": 613.2249999999999,
"heapDeltaBytes": 16362432,
"domNodes": 68,
"jsHeapTotalBytes": 8712192,
"scriptDurationMs": 102.984,
"eventListeners": 23
},
{
"name": "large-graph-idle",
"durationMs": 2046.1950000000115,
"styleRecalcs": 14,
"styleRecalcDurationMs": 13.098,
"layouts": 1,
"layoutDurationMs": 0.21699999999999975,
"taskDurationMs": 543.3760000000001,
"heapDeltaBytes": -10146900,
"domNodes": 69,
"jsHeapTotalBytes": 8364032,
"scriptDurationMs": 93.593,
"eventListeners": 23
},
{
"name": "large-graph-pan",
"durationMs": 2100.732999999991,
"styleRecalcs": 70,
"styleRecalcDurationMs": 18.526999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1081.658,
"heapDeltaBytes": 2871740,
"domNodes": 22,
"jsHeapTotalBytes": 9236480,
"scriptDurationMs": 383.37,
"eventListeners": 6
},
{
"name": "large-graph-pan",
"durationMs": 2101.62600000001,
"styleRecalcs": 70,
"styleRecalcDurationMs": 17.566,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1130.7310000000002,
"heapDeltaBytes": 3287232,
"domNodes": 20,
"jsHeapTotalBytes": 9760768,
"scriptDurationMs": 415.835,
"eventListeners": 6
},
{
"name": "large-graph-pan",
"durationMs": 2110.810000000015,
"styleRecalcs": 71,
"styleRecalcDurationMs": 18.785000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1118.542,
"heapDeltaBytes": 3130580,
"domNodes": 22,
"jsHeapTotalBytes": 8450048,
"scriptDurationMs": 403.406,
"eventListeners": 6
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 586.593999999991,
"styleRecalcs": 52,
"styleRecalcDurationMs": 16.939999999999998,
"layouts": 1,
"layoutDurationMs": 0.2840000000000001,
"taskDurationMs": 379.707,
"heapDeltaBytes": 13264068,
"domNodes": 68,
"jsHeapTotalBytes": 16515072,
"scriptDurationMs": 118.099,
"eventListeners": 25
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 580.8480000000031,
"styleRecalcs": 48,
"styleRecalcDurationMs": 12.134,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 388.90799999999996,
"heapDeltaBytes": 12811388,
"domNodes": 22,
"jsHeapTotalBytes": 13369344,
"scriptDurationMs": 130.67199999999997,
"eventListeners": 8
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 567.6339999999982,
"styleRecalcs": 50,
"styleRecalcDurationMs": 13.472999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 374.855,
"heapDeltaBytes": 11794232,
"domNodes": 25,
"jsHeapTotalBytes": 17039360,
"scriptDurationMs": 123.152,
"eventListeners": 8
},
{
"name": "subgraph-idle",
"durationMs": 2015.8680000000118,
"styleRecalcs": 14,
"styleRecalcDurationMs": 14.675,
"layouts": 1,
"layoutDurationMs": 0.27200000000000013,
"taskDurationMs": 368.004,
"heapDeltaBytes": 761124,
"domNodes": 68,
"jsHeapTotalBytes": 18874368,
"scriptDurationMs": 19.338,
"eventListeners": 23
},
{
"name": "subgraph-idle",
"durationMs": 2000.4480000000058,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.306000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 383.275,
"heapDeltaBytes": 260808,
"domNodes": 22,
"jsHeapTotalBytes": 18350080,
"scriptDurationMs": 20.907000000000004,
"eventListeners": 6
},
{
"name": "subgraph-idle",
"durationMs": 2007.5969999999757,
"styleRecalcs": 12,
"styleRecalcDurationMs": 10.6,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 368.59600000000006,
"heapDeltaBytes": 584948,
"domNodes": 24,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 20.948999999999995,
"eventListeners": 6
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1731.5810000000056,
"styleRecalcs": 77,
"styleRecalcDurationMs": 46.457,
"layouts": 16,
"layoutDurationMs": 5.747999999999999,
"taskDurationMs": 810.9699999999999,
"heapDeltaBytes": 19307876,
"domNodes": 65,
"jsHeapTotalBytes": 19922944,
"scriptDurationMs": 105.666,
"eventListeners": 6
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1712.2810000000186,
"styleRecalcs": 79,
"styleRecalcDurationMs": 45.196,
"layouts": 17,
"layoutDurationMs": 5.154000000000001,
"taskDurationMs": 764.544,
"heapDeltaBytes": -7217048,
"domNodes": 107,
"jsHeapTotalBytes": 19398656,
"scriptDurationMs": 97.54,
"eventListeners": 21
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1693.853000000047,
"styleRecalcs": 79,
"styleRecalcDurationMs": 41.961,
"layouts": 17,
"layoutDurationMs": 4.774000000000001,
"taskDurationMs": 709.518,
"heapDeltaBytes": -7221904,
"domNodes": 107,
"jsHeapTotalBytes": 19398656,
"scriptDurationMs": 90.727,
"eventListeners": 21
}
]
} |
Replace separate pr-size-report.yaml and pr-perf-report.yaml with a single pr-report.yaml aggregator that triggers on both CI workflow completions. Produces one combined PR comment with bundle size and performance sections, showing 'pending' placeholders for whichever section hasn't completed yet. Key changes: - Add head-sha metadata to both CI workflows for stale-run detection - Add concurrency to ci-size-data.yaml (matches ci-perf-report.yaml) - Create unified-report.js that shells out to existing size-report.js and perf-report.ts, combining their output - Clean up legacy separate comment markers on first run - Use PR head SHA as concurrency key to prevent race conditions
christian-byrne
force-pushed
the
feat/unified-pr-perf-report
branch
from
70269a8 to
8b7a7e4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/pr-report.yaml (1)
195-211: Consider implementing pagination for legacy comment cleanup.The code fetches comments without a
pageparameter, retrieving only the first ~100 comments. PRs with longer discussion threads will retain legacy markers beyond the first page.For best results, use octokit's built-in pagination helper instead of manual looping:
const comments = await github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, per_page: 100, });This automatically handles all pages. For memory-sensitive scenarios, use
github.paginate.iterator()to process page-by-page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-report.yaml around lines 195 - 211, The current cleanup fetches only the first page via github.rest.issues.listComments (per_page:100) so legacyMarkers and comment deletion loop can miss later pages; replace that call with octokit's pagination (use github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, per_page: 100 }) to get all comments) and then iterate the returned comments array checking legacyMarkers.some(...) and calling github.rest.issues.deleteComment for matching comment.id; alternatively, for large PRs use github.paginate.iterator to process pages one-by-one to avoid high memory usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-report.yaml:
- Around line 52-63: Add a second head-SHA revalidation immediately before the
Post PR comment step: call github.rest.pulls.get to fetch the live PR (same
owner/repo/pull_number used earlier) and compare livePr.head.sha against the
previously stored head SHA (the value you saved from
context.payload.workflow_run.head_sha or earlier variable). If they differ, log
a message and set core.setOutput('skip','true') (and return/exit the job path)
so the posting step is skipped; otherwise proceed to post the comment as normal.
In `@scripts/unified-report.js`:
- Around line 21-27: The execFileSync invocation that runs the size report when
sizeStatus === 'ready' (calling 'node' with 'scripts/size-report.js' and pushing
sizeReport into lines) must be wrapped in a try-catch so a non-zero exit doesn't
abort the whole pipeline; catch the error and push a clear placeholder line
(e.g. a section-level failure message mentioning the size report) into lines
instead. Do the same for the performance report execFileSync call (the analogous
call around line ~40 that runs 'scripts/perf-report.js' or similar): wrap in
try-catch and push a performance-section failure placeholder on error, ensuring
the rest of the unified report continues to run. Ensure you reference the same
function/variable names (sizeStatus, lines, sizeReport, and the performance
report variables) when making the change.
---
Nitpick comments:
In @.github/workflows/pr-report.yaml:
- Around line 195-211: The current cleanup fetches only the first page via
github.rest.issues.listComments (per_page:100) so legacyMarkers and comment
deletion loop can miss later pages; replace that call with octokit's pagination
(use github.paginate(github.rest.issues.listComments, { owner:
context.repo.owner, repo: context.repo.repo, issue_number: prNumber, per_page:
100 }) to get all comments) and then iterate the returned comments array
checking legacyMarkers.some(...) and calling github.rest.issues.deleteComment
for matching comment.id; alternatively, for large PRs use
github.paginate.iterator to process pages one-by-one to avoid high memory usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 048b7893-925a-43f4-86f9-53906daccd2a
📒 Files selected for processing (6)
.github/workflows/ci-perf-report.yaml.github/workflows/ci-size-data.yaml.github/workflows/pr-perf-report.yaml.github/workflows/pr-report.yaml.github/workflows/pr-size-report.yamlscripts/unified-report.js
💤 Files with no reviewable changes (2)
- .github/workflows/pr-perf-report.yaml
- .github/workflows/pr-size-report.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-perf-report.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/unified-report.js (1)
21-25:⚠️ Potential issue | 🟠 MajorAdd section-level failure handling around subprocess execution.
Line 22 and Line 40 call
execFileSyncwithout local handling. If either command exits non-zero, unified report generation aborts instead of emitting the section fallback.Suggested patch
import { execFileSync } from 'node:child_process' import { existsSync } from 'node:fs' @@ /** `@type` {string[]} */ const lines = [] + +/** + * `@param` {string} command + * `@param` {string[]} commandArgs + * `@param` {string[]} fallbackLines + */ +function appendReportOrFallback(command, commandArgs, fallbackLines) { + try { + const out = execFileSync(command, commandArgs, { encoding: 'utf-8' }).trimEnd() + lines.push(out) + } catch { + lines.push(...fallbackLines) + } +} @@ if (sizeStatus === 'ready') { - const sizeReport = execFileSync('node', ['scripts/size-report.js'], { - encoding: 'utf-8' - }).trimEnd() - lines.push(sizeReport) + appendReportOrFallback('node', ['scripts/size-report.js'], [ + '## 📦 Bundle Size', + '', + '> ⚠️ Failed to render bundle size report. Check the CI workflow logs.' + ]) @@ if (perfStatus === 'ready' && existsSync('test-results/perf-metrics.json')) { - const perfReport = execFileSync( - 'pnpm', - ['exec', 'tsx', 'scripts/perf-report.ts'], - { encoding: 'utf-8' } - ).trimEnd() - lines.push(perfReport) + appendReportOrFallback('pnpm', ['exec', 'tsx', 'scripts/perf-report.ts'], [ + '## ⚡ Performance', + '', + '> ⚠️ Failed to render performance report. Check the CI workflow logs.' + ])#!/bin/bash # Verify subprocess calls exist and are not locally wrapped by try/catch in this file. rg -n -C3 'execFileSync\(' scripts/unified-report.js rg -n -C3 '^\s*try\s*\{' scripts/unified-report.jsAlso applies to: 40-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/unified-report.js` around lines 21 - 25, Wrap each execFileSync invocation (e.g., the call that assigns sizeReport and the other execFileSync at lines ~40-45) in a local try/catch so a non-zero exit does not abort the whole report; on error catch the exception and push a fallback section into lines (using the same section boundary format as the successful output) that includes a short failure message and the error.message/exit code, and continue execution. Ensure you still call .trimEnd() on successful outputs (e.g., sizeReport) and keep encoding:'utf-8' when invoking execFileSync; reference the variables sizeReport and the array lines when inserting the fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/unified-report.js`:
- Around line 39-54: The current branch treats perfStatus === 'ready' with a
missing 'test-results/perf-metrics.json' as "in progress"; change the branching
so that a missing metrics file when perfStatus is 'ready' is considered
unavailable/failed: adjust the condition around the
existsSync('test-results/perf-metrics.json') and perfStatus checks so that if
perfStatus === 'failed' OR (perfStatus === 'ready' && !existsSync(...)) you push
the unavailable/failed placeholder (the same block that currently handles
perfStatus === 'failed'), and only run execFileSync and append perfReport when
perfStatus === 'ready' AND the metrics file exists; update the messages produced
by lines.push in the unavailable branch accordingly.
---
Duplicate comments:
In `@scripts/unified-report.js`:
- Around line 21-25: Wrap each execFileSync invocation (e.g., the call that
assigns sizeReport and the other execFileSync at lines ~40-45) in a local
try/catch so a non-zero exit does not abort the whole report; on error catch the
exception and push a fallback section into lines (using the same section
boundary format as the successful output) that includes a short failure message
and the error.message/exit code, and continue execution. Ensure you still call
.trimEnd() on successful outputs (e.g., sizeReport) and keep encoding:'utf-8'
when invoking execFileSync; reference the variables sizeReport and the array
lines when inserting the fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97ec0b78-97fb-4270-9b3b-1faa838ed258
📒 Files selected for processing (1)
scripts/unified-report.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces two separate PR comment workflows (bundle size + performance) with a single unified report that posts one combined comment per PR.
Changes
pr-report.yamlaggregator workflow triggers on bothCI: Size DataandCI: Performance Reportcompletions. Finds sibling workflow runs by PR head SHA. Renders combined report viaunified-report.js(shells out to existingsize-report.jsandperf-report.ts). Sections show "pending" or "failed" placeholders when data is unavailable.pr-size-report.yamlandpr-perf-report.yaml. Legacy<!-- COMFYUI_FRONTEND_SIZE -->and<!-- COMFYUI_FRONTEND_PERF -->comments are auto-cleaned on first run.Review Focus
head_shaso the later-completing workflow cancels the earlier report run, ensuring the final comment always has both sections.workflow_dispatchre-trigger path frompr-size-report.yamlis not carried forward — the unified workflow handles re-trigger naturally via its dual-trigger design.┆Issue is synchronized with this Notion page by Unito