perf(ci): remove unnecessary pnpm install from merge-reports job#8030
perf(ci): remove unnecessary pnpm install from merge-reports job#8030
Conversation
The merge-reports job was running full setup-frontend (pnpm install) just to execute playwright merge-reports CLI. This added ~16-18s of overhead for a command that takes ~3s. Replace with npx which is pre-installed on GitHub runners, eliminating the need for any Node.js/pnpm setup in this job.
📝 WalkthroughWalkthroughReplaces the frontend setup step with Changes
Possibly related PRs
Suggested reviewers
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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/13/2026, 09:36:10 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
🎭 Playwright Tests:
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.32 MB (baseline 3.32 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 1.04 MB (baseline 1.04 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.63 kB (baseline 6.63 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 372 kB (baseline 372 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 200 kB (baseline 200 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 9.19 MB (baseline 9.19 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 5.25 MB (baseline 5.25 MB) • ⚪ 0 BBundles that do not match a named category
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
didn't work |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci-tests-e2e.yaml:
- Around line 159-165: The merge step currently invokes "pnpm dlx
@playwright/test merge-reports" without a version, causing potential Playwright
version mismatch with the test shards; update the merge job to either call the
exact installed Playwright version (e.g., replace "pnpm dlx @playwright/test"
with "pnpm dlx @playwright/test@1.57.0" to match the project's Playwright
1.57.0) or run "pnpm install --frozen-lockfile" in the merge job before invoking
"pnpm dlx @playwright/test merge-reports" so the merge uses the same locked
version and avoids blob report format incompatibilities.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-tests-e2e.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:24.164Z
Learning: The project has CI automation that handles code formatting (pnpm format) automatically, so manual formatting suggestions should not be provided in code reviews for the Comfy-Org/ComfyUI_frontend repository.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Run quality gates before PR: `pnpm lint`, `pnpm typecheck`, `pnpm knip`, and ensure relevant tests pass
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : E2E tests use Playwright and belong in `browser_tests/**/*.spec.ts`
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices for E2E testing
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
📚 Learning: 2025-12-12T23:02:37.473Z
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : Follow Playwright best practices for E2E testing
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to browser_tests/**/*.spec.ts : E2E tests use Playwright and belong in `browser_tests/**/*.spec.ts`
Applied to files:
.github/workflows/ci-tests-e2e.yaml
⏰ 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). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (1)
.github/workflows/ci-tests-e2e.yaml (1)
147-150: Good optimization — relies on runner's pre-installed Node.js.Using
pnpm/action-setup@v4withoutactions/setup-nodeis valid since GitHub runners have Node.js pre-installed. This effectively eliminates thepnpm installoverhead.
| - name: Merge into HTML Report | ||
| run: | | ||
| # Generate HTML report | ||
| pnpm exec playwright merge-reports --reporter=html ./all-blob-reports | ||
| pnpm dlx @playwright/test merge-reports --reporter=html ./all-blob-reports | ||
| # Generate JSON report separately with explicit output path | ||
| PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ | ||
| pnpm exec playwright merge-reports --reporter=json ./all-blob-reports | ||
| pnpm dlx @playwright/test merge-reports --reporter=json ./all-blob-reports |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat package.json | grep -A 1 -B 1 '@playwright/test'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 179
🏁 Script executed:
# Check for catalog definition in pnpm-workspace.yaml or top-level catalog in package.json
if [ -f "pnpm-workspace.yaml" ]; then
echo "=== pnpm-workspace.yaml ==="
grep -A 30 "catalog:" pnpm-workspace.yaml | head -50
fi
# Also check if there's a .pnpmfile.cjs
if [ -f ".pnpmfile.cjs" ]; then
echo "=== .pnpmfile.cjs ==="
cat .pnpmfile.cjs | head -30
fi
# Check pnpm-lock.yaml for the resolved version
echo "=== Checking pnpm-lock.yaml for @playwright/test ==="
grep -A 3 "'@playwright/test'" pnpm-lock.yaml | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1500
🏁 Script executed:
# Read the CI workflow file to see full context
cat -n .github/workflows/ci-tests-e2e.yaml | sed -n '150,180p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1447
🏁 Script executed:
# Find the test shard job definitions in the workflow
cat -n .github/workflows/ci-tests-e2e.yaml | head -160 | tail -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4032
🏁 Script executed:
# Search for how pnpm dlx behaves with unversioned packages
# Also check if the merge-reports job installs dependencies
cat -n .github/workflows/ci-tests-e2e.yaml | sed -n '139,160p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 775
🌐 Web query:
Playwright blob report format version compatibility merge-reports
💡 Result:
Short answer
- Playwright blob reports are ZIP blobs produced by the "blob" reporter and intended to be read/merged by the built-in CLI tool npx playwright merge-reports. [1][2][3]
- Best practice / compatibility rule: generate and merge blob reports with the same Playwright release (or at least the same minor release series). Mixing blobs produced by much older Playwright versions with a newer merge-reports tool can break or produce unexpected output. When in doubt, use the same Playwright version to create and to merge. [1][2][4]
Details / practical guidance
- What a blob is: the blob reporter writes report-.zip files that include all test metadata and attachments; merge-reports extracts those blobs and produces HTML/JSON/JUnit/etc. using the current reporter implementation. [2]
- When merging across OS / environments you may need an explicit merge config (to pick test-root, tags, etc.). [1]
- Known issues: users have reported sporadic/behavioral problems when merging (empty output, reporters writing to stdout vs file, etc.), which indicates the merge step can be sensitive to Playwright version, reporter choice, and CI workflow details. If you see missing output, ensure blobs are intact, use the same Playwright version, and try piping JSON/JUnit output to a file as a workaround. [4][5]
References
- Playwright docs — Test sharding / merging reports (blob + merge-reports). [Playwright Test sharding / merge-reports]. [1]
- Playwright docs — Blob reporter details (file name, contents). [Playwright Reporters / Blob]. [2]
- Playwright release notes introducing merge-reports (v1.37). [Release notes]. [3]
- Playwright GitHub issues reporting merge-reports problems (examples of failures / unexpected behavior). [GitHub issue examples]. [4][5]
If you want, I can:
- Inspect a sample report-.zip you have and tell you which Playwright version likely produced it, or
- Give a step-by-step CI workflow that reliably collects and merges blob reports (GitHub Actions example).
Pin @playwright/test version to match the project's installed version to ensure blob report compatibility.
The test shards use Playwright 1.57.0 (from pnpm install --frozen-lockfile in the test jobs), but the merge job uses pnpm dlx @playwright/test without a version pin. Since pnpm dlx fetches the latest available version from npm when no version is specified, mismatches can occur. Blob report format compatibility between different Playwright versions is not guaranteed, and version mismatches can cause merge failures or empty reports.
Pin the version to match the project:
- pnpm dlx @playwright/test merge-reports --reporter=html ./all-blob-reports
+ pnpm dlx @playwright/test@1.57.0 merge-reports --reporter=html ./all-blob-reports- pnpm dlx @playwright/test merge-reports --reporter=json ./all-blob-reports
+ pnpm dlx @playwright/test@1.57.0 merge-reports --reporter=json ./all-blob-reportsAlternatively, add pnpm install --frozen-lockfile to the merge job to use the version from package.json automatically.
🤖 Prompt for AI Agents
In @.github/workflows/ci-tests-e2e.yaml around lines 159 - 165, The merge step
currently invokes "pnpm dlx @playwright/test merge-reports" without a version,
causing potential Playwright version mismatch with the test shards; update the
merge job to either call the exact installed Playwright version (e.g., replace
"pnpm dlx @playwright/test" with "pnpm dlx @playwright/test@1.57.0" to match the
project's Playwright 1.57.0) or run "pnpm install --frozen-lockfile" in the
merge job before invoking "pnpm dlx @playwright/test merge-reports" so the merge
uses the same locked version and avoids blob report format incompatibilities.
Summary
setup-frontendaction frommerge-reportsjobnpx @playwright/testinstead ofpnpm exec playwrightWhy
The
merge-reportsjob was spending ~16-18s onpnpm installjust to run a CLI command that takes ~3s. Sincenpxis pre-installed on GitHub runners, we can eliminate the setup overhead entirely.Expected savings: ~16-18 seconds per CI run
Test Plan
┆Issue is synchronized with this Notion page by Unito