feat: improved playwright comment format #7882
Conversation
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughAdded Playwright failure extraction with trace URL generation, passed deployment/PR SHAs into CI steps via GITHUB_SHA, and refactored the PR comment script to post compact, collapsible summaries including per-browser reports and a consolidated failed-tests list with links. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Job as CI Job (deploy & tests)
participant Art as Artifacts (Playwright reports)
participant Extract as extract-playwright-counts.ts
participant Script as pr-playwright-deploy-and-comment.sh
participant PR as GitHub PR Comment
GH->>Job: trigger workflow (push/pr/workflow_run)
Job->>Art: run Playwright, upload report.json & traces
Job->>Extract: run extractor with reportDir and optional baseUrl
Extract-->>Job: return counts + failures (with trace URLs)
Job->>Script: invoke comment script with counts JSONs + deploy URL + GITHUB_SHA
Script->>PR: post/update compact, collapsible comment with summary, per-browser reports, and failed-tests (report & trace links)
Possibly related PRs
✨ Finishing touches
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/07/2026, 11:20:52 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
🎭 Playwright Tests:
|
|
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.
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 1.01 MB (baseline 1.01 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 — 300 kB (baseline 300 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 198 kB (baseline 198 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 — 3.5 MB (baseline 3.5 MB) • ⚪ 0 BBundles that do not match a named category
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @scripts/cicd/extract-playwright-counts.ts:
- Around line 75-87: processTest currently only treats a test as a failure when
any test.results entry has status 'failed' or 'timedOut', so flaky tests (which
have test.outcome === 'flaky') are skipped; update the logic in processTest (and
the failedResult selection) to also treat tests with test.outcome === 'flaky' as
failures — e.g., include a check for test.outcome === 'flaky' when computing
hasFailed and when picking failedResult (or fall back to the first result when
outcome is 'flaky') so counts.flaky > 0 leads to captured failed entries.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-tests-e2e-forks.yaml.github/workflows/ci-tests-e2e.yamlscripts/cicd/extract-playwright-counts.tsscripts/cicd/pr-playwright-deploy-and-comment.sh
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
scripts/cicd/extract-playwright-counts.ts
🧠 Learnings (14)
📓 Common learnings
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.
📚 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:
scripts/cicd/pr-playwright-deploy-and-comment.sh.github/workflows/ci-tests-e2e.yaml.github/workflows/ci-tests-e2e-forks.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:
scripts/cicd/pr-playwright-deploy-and-comment.sh.github/workflows/ci-tests-e2e.yamlscripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use E2E tests in `browser_tests/**/*.spec.ts` with Playwright framework
Applied to files:
scripts/cicd/pr-playwright-deploy-and-comment.sh.github/workflows/ci-tests-e2e.yamlscripts/cicd/extract-playwright-counts.ts
📚 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: 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} : Follow naming conventions 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 across multiple viewports
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use tags like `mobile` and `2x` in Playwright tests for relevant test variations; tags are respected by config
Applied to files:
.github/workflows/ci-tests-e2e.yaml
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-11T12:25:15.470Z
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:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
⏰ 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). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (8)
.github/workflows/ci-tests-e2e.yaml (1)
225-225: LGTM! Correct SHA propagation for PR head commit.Using
github.event.pull_request.head.shacorrectly captures the PR's head commit SHA, which will be used downstream to construct accurate GitHub file links for failed tests..github/workflows/ci-tests-e2e-forks.yaml (1)
6-6: LGTM! Workflow trigger name matches and SHA propagation is correct.The workflow name
'CI: Tests E2E'correctly matches the updated name inci-tests-e2e.yaml, ensuring theworkflow_runtrigger fires properly. TheGITHUB_SHAfromworkflow_run.head_shacorrectly references the forked PR's head commit.Also applies to: 84-84
scripts/cicd/extract-playwright-counts.ts (2)
187-247: HTML fallback paths don't extract failure details.The HTML parsing fallbacks (lines 187-287) only extract aggregate stats, not individual test failures. This means if
report.jsonis missing, users won't see the "Failed Tests" section with test names and trace links.This is acceptable as a degraded experience, but worth noting if JSON report generation ever fails.
69-141: Well-structured recursive failure extraction.The
extractFailedTestsfunction correctly:
- Recursively traverses nested suites
- Builds full test name paths with
›separator- Constructs trace viewer URLs from attachments
- Handles missing attachments gracefully
scripts/cicd/pr-playwright-deploy-and-comment.sh (4)
331-331: VerifyGITHUB_SHAis always available in the environment.The script constructs GitHub file URLs using
$GITHUB_SHA. This variable is now set in both workflow files (ci-tests-e2e.yamlline 225 andci-tests-e2e-forks.yamlline 84), so the links should work correctly.However, if someone runs this script locally or from another context without setting
GITHUB_SHA, the generated URLs will be malformed (e.g.,blob//browser_tests/...).Consider adding a fallback or validation:
Optional: Add fallback for GITHUB_SHA
+# Use HEAD SHA as fallback if GITHUB_SHA not set +: "${GITHUB_SHA:=$(git rev-parse HEAD 2>/dev/null || echo 'main')}" + # Convert file path to GitHub URL (relative to repo root) github_file_url="https://github.com/$GITHUB_REPOSITORY/blob/$GITHUB_SHA/$test_file"
312-346: Clean implementation of failed tests display.The failed tests section:
- Correctly iterates through all browsers' failures
- Constructs proper GitHub file links
- Handles missing trace URLs gracefully (checks for empty and "null")
- Uses jq's
// []and.[]?for safe null handling
137-154: Improved starting comment format with collapsible details.The new format is more concise and uses a collapsible section to reduce visual noise while still showing per-browser status.
348-403: Browser reports correctly moved to collapsible section.The completion comment now shows the key information (status, results, failed tests) upfront, with detailed per-browser reports in a collapsible section. This aligns with the PR objective of making comments "more concise and actionable."
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @scripts/cicd/extract-playwright-counts.ts:
- Around line 75-87: The failures extraction misses tests marked flaky because
processTest only checks result statuses; update the logic in processTest (the
hasFailed computation and the failedResult selection) to also consider
test.outcome === 'flaky' — e.g., change the hasFailed boolean to true if
test.outcome === 'flaky' OR any result has status 'failed'/'timedOut', and when
computing failedResult (used to build failure metadata) handle the flaky case by
using a representative result or tagging the failure as flaky so flaky tests are
included in the failures list.
- Around line 99-102: The construction of traceDataUrl and traceUrl assumes
baseUrl has no trailing slash which can produce double slashes; normalize
baseUrl (e.g., strip any trailing '/' from the baseUrl value before using it)
and then build traceDataUrl = `${normalizedBaseUrl}/data/${traceFile}` and
traceUrl =
`${normalizedBaseUrl}/trace/?trace=${encodeURIComponent(traceDataUrl)}` so the
URLs are correct whether baseUrl was provided with or without a trailing slash;
update references to baseUrl, traceDataUrl, and traceUrl accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/cicd/extract-playwright-counts.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
scripts/cicd/extract-playwright-counts.ts
🧠 Learnings (15)
📓 Common learnings
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: 2025-12-21T06:04:12.562Z
Learning: Include clear PR descriptions referencing linked issues, keep PRs concise and information-dense without emojis or excessive headers
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use E2E tests in `browser_tests/**/*.spec.ts` with Playwright framework
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-13T05:34:22.209Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:22.209Z
Learning: In Playwright tests in the Comfy-Org/ComfyUI_frontend repository, the `toPass()` assertion method implements incremental backoff during retries, so increasing the timeout value (e.g., to 5000ms) is sufficient for handling async operations without being overly aggressive.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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} : Prefer specific selectors in browser tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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} : Follow naming conventions for browser tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use tags like `mobile` and `2x` in Playwright tests for relevant test variations; tags are respected by config
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-11T12:25:15.470Z
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:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
⏰ 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). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (4)
scripts/cicd/extract-playwright-counts.ts (4)
13-55: LGTM! Well-structured interface definitions.The new interfaces properly model the Playwright report structure with appropriate optional fields and recursive suite handling. Type safety is maintained without using
any.
296-310: LGTM! CLI correctly handles optional baseUrl parameter.The CLI argument parsing and usage message properly reflect the optional
baseUrlparameter. The JSON output format is backward compatible, with thefailuresarray being an optional addition.
312-312: LGTM! Appropriate export additions.Exporting both
extractTestCountsandextractFailedTestsprovides good modularity and allows independent use of the failure extraction logic in other tooling.
187-262: Fallback HTML parsing won't extract detailed failures, but this is acceptable.The fallback parsing logic uses the
ReportDatainterface (lines 202, 229), which lacks thesuitesproperty needed byextractFailedTests. However,report.jsonis always explicitly generated in CI via the--reporter=jsonCLI flag (see.github/workflows/ci-tests-e2e.yaml, lines ~92-95 and ~121-124), so the fallback paths are defensive code that won't execute in practice.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @scripts/cicd/extract-playwright-counts.ts:
- Around line 100-103: The constructed traceDataUrl and traceUrl can end up with
double slashes when baseUrl has a trailing slash; normalize baseUrl inside the
if (baseUrl) block (e.g., strip trailing slashes from baseUrl) before building
traceDataUrl and traceUrl so you concatenate with single slashes (ensure you
still call encodeURIComponent(traceDataUrl) for traceUrl).
- Around line 75-88: In processTest, the failedResult selection only looks for
results with status 'failed' or 'timedOut' which leaves failedResult undefined
when isFlaky is true; update the logic in processTest (the failedResult
variable) to use the found failed/timedOut result when present, but if isFlaky
and none found, fall back to test.results[test.results.length - 1] so trace
attachment and error message extraction always have a result to read from; keep
the existing hasFailed/isFlaky checks and ensure subsequent trace/error handling
uses this fallback failedResult.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/cicd/extract-playwright-counts.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
scripts/cicd/extract-playwright-counts.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
scripts/cicd/extract-playwright-counts.ts
🧠 Learnings (15)
📓 Common learnings
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: 2025-12-21T06:04:12.562Z
Learning: Include clear PR descriptions referencing linked issues, keep PRs concise and information-dense without emojis or excessive headers
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use E2E tests in `browser_tests/**/*.spec.ts` with Playwright framework
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Tests should be cross-platform compatible using `path.resolve`, `path.join`, and `path.sep` for Windows, macOS, and Linux compatibility
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use tags like `mobile` and `2x` in Playwright tests for relevant test variations; tags are respected by config
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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} : Prefer specific selectors in browser tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-13T05:34:22.209Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:22.209Z
Learning: In Playwright tests in the Comfy-Org/ComfyUI_frontend repository, the `toPass()` assertion method implements incremental backoff during retries, so increasing the timeout value (e.g., to 5000ms) is sufficient for handling async operations without being overly aggressive.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 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} : Follow naming conventions for browser tests
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
scripts/cicd/extract-playwright-counts.ts
📚 Learning: 2025-12-11T12:25:15.470Z
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:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
scripts/cicd/extract-playwright-counts.ts
⏰ 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). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
| function processTest(test: TestCase, file: string, suitePath: string[]) { | ||
| // Check if test failed or is flaky | ||
| const hasFailed = test.results.some( | ||
| (r) => r.status === 'failed' || r.status === 'timedOut' | ||
| ) | ||
| const isFlaky = test.outcome === 'flaky' | ||
|
|
||
| if (hasFailed || isFlaky) { | ||
| const fullTestName = [...suitePath, test.title] | ||
| .filter(Boolean) | ||
| .join(' › ') | ||
| const failedResult = test.results.find( | ||
| (r) => r.status === 'failed' || r.status === 'timedOut' | ||
| ) |
There was a problem hiding this comment.
The flaky test capture fix is incomplete.
While you added the isFlaky check and updated the condition to if (hasFailed || isFlaky), the failedResult selection logic (lines 86-88) still only searches for results with 'failed' or 'timedOut' status. For edge cases where test.outcome === 'flaky' but no individual result has status 'failed' or 'timedOut' (which shouldn't happen in typical Playwright behavior, but the code allows for it), failedResult will be undefined, causing the trace attachment (line 92) and error message (line 112) to not be captured.
The past review suggested using a fallback to test.results[test.results.length - 1] when isFlaky is true to ensure a result is always available for trace/error extraction.
🔧 Complete the fix as suggested in the past review
const fullTestName = [...suitePath, test.title]
.filter(Boolean)
.join(' › ')
- const failedResult = test.results.find(
- (r) => r.status === 'failed' || r.status === 'timedOut'
- )
+ const failedResult = isFlaky && !hasFailed
+ ? test.results[test.results.length - 1]
+ : test.results.find(
+ (r) => r.status === 'failed' || r.status === 'timedOut'
+ )📝 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.
| function processTest(test: TestCase, file: string, suitePath: string[]) { | |
| // Check if test failed or is flaky | |
| const hasFailed = test.results.some( | |
| (r) => r.status === 'failed' || r.status === 'timedOut' | |
| ) | |
| const isFlaky = test.outcome === 'flaky' | |
| if (hasFailed || isFlaky) { | |
| const fullTestName = [...suitePath, test.title] | |
| .filter(Boolean) | |
| .join(' › ') | |
| const failedResult = test.results.find( | |
| (r) => r.status === 'failed' || r.status === 'timedOut' | |
| ) | |
| function processTest(test: TestCase, file: string, suitePath: string[]) { | |
| // Check if test failed or is flaky | |
| const hasFailed = test.results.some( | |
| (r) => r.status === 'failed' || r.status === 'timedOut' | |
| ) | |
| const isFlaky = test.outcome === 'flaky' | |
| if (hasFailed || isFlaky) { | |
| const fullTestName = [...suitePath, test.title] | |
| .filter(Boolean) | |
| .join(' › ') | |
| const failedResult = isFlaky && !hasFailed | |
| ? test.results[test.results.length - 1] | |
| : test.results.find( | |
| (r) => r.status === 'failed' || r.status === 'timedOut' | |
| ) |
🤖 Prompt for AI Agents
In @scripts/cicd/extract-playwright-counts.ts around lines 75 - 88, In
processTest, the failedResult selection only looks for results with status
'failed' or 'timedOut' which leaves failedResult undefined when isFlaky is true;
update the logic in processTest (the failedResult variable) to use the found
failed/timedOut result when present, but if isFlaky and none found, fall back to
test.results[test.results.length - 1] so trace attachment and error message
extraction always have a result to read from; keep the existing
hasFailed/isFlaky checks and ensure subsequent trace/error handling uses this
fallback failedResult.
|
|
||
| // Output as JSON for easy parsing in shell script | ||
| console.log(JSON.stringify(counts)) | ||
| process.stdout.write(JSON.stringify(counts) + '\n') |
|
SGTM |
Description
Improve Playwright PR comment format
Problem
The current Playwright PR comment format is verbose and doesn't provide easy access to failing test details.
Developers need to navigate multiple levels deep to:
Find which tests failed
Access test source code
View trace files for debugging
This makes debugging test failures tedious and time-consuming.
Solution
Improved the Playwright PR comment format to be concise and actionable by:
Modified extract-playwright-counts.ts to extract detailed failure information from Playwright JSON reports including test names, file paths, and trace URLs
Updated pr-playwright-deploy-and-comment.sh to generate concise comments with failed tests listed upfront
Modified ci-tests-e2e.yaml to pass GITHUB_SHA for source code links
Modified ci-tests-e2e-forks.yaml to pass GITHUB_SHA for forked PR workflow
Before:
Large multi-section layout with emoji-heavy headers
Summary section listing all counts vertically
Browser results displayed prominently with detailed counts
Failed test details only accessible through report links
No direct links to test source code or traces
After:
Concise single-line header with status
Single-line summary: "X passed, Y failed, Z flaky, W skipped (Total: N)"
Failed tests section (only shown when tests fail) with:
Direct links to test source code on GitHub
Direct links to trace viewer for each failure
Browser details collapsed in details section
Overall roughly half size reduction in visible text
Testing
Verified TypeScript extraction logic for parsing Playwright JSON reports
Validated shell script syntax
Confirmed GitHub workflow changes are properly formatted
Will be fully tested on next PR with actual test failures
┆Issue is synchronized with this Notion page by Unito