Skip to content

test: deflake templates locale coverage#7705

Merged
christian-byrne merged 4 commits intomainfrom
tests/fix-flaky-templates-test
Dec 28, 2025
Merged

test: deflake templates locale coverage#7705
christian-byrne merged 4 commits intomainfrom
tests/fix-flaky-templates-test

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Dec 22, 2025

Summary

Ensure the templates locale Playwright test validates localized UI text instead of waiting on a flaky network request.

Changes

  • What: Update Templates >> Uses proper locale files for templates to assert on French strings rendered in the dialog and confirm English fallback is absent

Review Focus

  • Confirm the chosen French strings always appear when the localized bundle loads so the test meaningfully covers the regression

┆Issue is synchronized with this Notion page by Unito

@christian-byrne christian-byrne requested a review from a team as a code owner December 22, 2025 12:12
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Dec 22, 2025
@github-actions
Copy link

github-actions bot commented Dec 22, 2025

🎨 Storybook Build Status

Build completed successfully!

⏰ Completed at: 12/28/2025, 01:19:13 AM UTC

🔗 Links


🎉 Your Storybook is ready for review!

@github-actions
Copy link

github-actions bot commented Dec 22, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 12/28/2025, 01:27:20 AM UTC

📈 Summary

  • Total Tests: 509
  • Passed: 499 ✅
  • Failed: 0
  • Flaky: 2 ⚠️
  • Skipped: 8 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 489 / ❌ 0 / ⚠️ 1 / ⏭️ 8
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 7 / ❌ 0 / ⚠️ 1 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

📝 Walkthrough

Walkthrough

Updated a browser test to stop waiting for the French locale network request and instead assert French-localized UI strings render and that the English fallback text is not present.

Changes

Cohort / File(s) Summary
Locale test assertion refactoring
browser_tests/tests/templates.spec.ts
Removed explicit wait/assertion on the French locale network request; replaced with direct UI assertions checking French-localized heading "Modèles" and button "Tous les modèles", and asserting the English "All Templates" text is not present.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tests/fix-flaky-templates-test

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 22, 2025

Bundle Size Report

Summary

  • Raw size: 17.3 MB baseline 17.3 MB — ⚪ 0 B
  • Gzip: 3.53 MB baseline 3.53 MB — ⚪ 0 B
  • Brotli: 2.7 MB baseline 2.7 MB — ⚪ 0 B
  • Bundles: 98 current • 98 baseline

Category Glance
Vendor & Third-Party ⚪ 0 B (9.1 MB) · Other ⚪ 0 B (3.46 MB) · App Entry Points ⚪ 0 B (3.19 MB) · Graph Workspace ⚪ 0 B (995 kB) · Panels & Settings ⚪ 0 B (296 kB) · UI Components ⚪ 0 B (196 kB) · + 3 more

Per-category breakdown
App Entry Points — 3.19 MB (baseline 3.19 MB) • ⚪ 0 B

Main entry bundles and manifests

File Before After Δ Raw Δ Gzip Δ Brotli
assets/index-BgDRQuno.js 193 kB 193 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/index-CvorPY1L.js 3 MB 3 MB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/index-STxi41Aq.js 345 B 345 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
Graph Workspace — 995 kB (baseline 995 kB) • ⚪ 0 B

Graph editor runtime, canvas, workflow orchestration

File Before After Δ Raw Δ Gzip Δ Brotli
assets/GraphView-CgS56sex.js 995 kB 995 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 B

Top-level views, pages, and routed surfaces

File Before After Δ Raw Δ Gzip Δ Brotli
assets/UserSelectView-CMduJEpS.js 6.54 kB 6.54 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Panels & Settings — 296 kB (baseline 296 kB) • ⚪ 0 B

Configuration panels, inspectors, and settings screens

File Before After Δ Raw Δ Gzip Δ Brotli
assets/AboutPanel-ByCOzSVb.js 9.16 kB 9.16 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/ExtensionPanel-NsCiuW6U.js 10.8 kB 10.8 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/KeybindingPanel-BAr8cUsD.js 13.6 kB 13.6 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/LegacyCreditsPanel-DTv-fsff.js 21.4 kB 21.4 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/ServerConfigPanel-CmfR2xQr.js 7.07 kB 7.07 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-AN2JfZVQ.js 21.7 kB 21.7 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-BhbWhsRg.js 101 B 101 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-BIdKi-OT.js 26.2 kB 26.2 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-Bu3OR-lX.js 24.6 kB 24.6 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-ByL6gy5c.js 25.4 kB 25.4 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-CjlRFMdL.js 32.8 kB 32.8 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-DkGwvylK.js 26.9 kB 26.9 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-Dyd027Dx.js 24.7 kB 24.7 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-wwBxqLH5.js 21.3 kB 21.3 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/settings-xx2Yb6R2.js 23.8 kB 23.8 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/UserPanel-D5du6HhC.js 6.23 kB 6.23 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
UI Components — 196 kB (baseline 196 kB) • ⚪ 0 B

Reusable component library chunks

File Before After Δ Raw Δ Gzip Δ Brotli
assets/ComfyQueueButton-C0nNlKMW.js 8.45 kB 8.45 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/LazyImage.vue_vue_type_script_setup_true_lang-BuF3mn96.js 59.9 kB 59.9 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/Load3D.vue_vue_type_script_setup_true_lang-B6bx6BEl.js 54.3 kB 54.3 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaTitle.vue_vue_type_script_setup_true_lang-Ci1ghGxp.js 897 B 897 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/UserAvatar.vue_vue_type_script_setup_true_lang-B6P2AFxg.js 1.34 kB 1.34 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetButton-CD0L1KzN.js 2.04 kB 2.04 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetInputNumber.vue_vue_type_script_setup_true_lang-BNIOLQAk.js 14.3 kB 14.3 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetLayoutField.vue_vue_type_script_setup_true_lang-CR6pnzPn.js 2.14 kB 2.14 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetSelect.vue_vue_type_script_setup_true_lang-DtvUkdso.js 48.8 kB 48.8 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetWithControl.vue_vue_type_script_setup_true_lang-DG1dAsz4.js 3.68 kB 3.68 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 B

Stores, services, APIs, and repositories

File Before After Δ Raw Δ Gzip Δ Brotli
assets/audioService-C4ee3b-O.js 2.2 kB 2.2 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/keybindingService-aPYrNVSU.js 7.51 kB 7.51 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/serverConfigStore-CNSmwya1.js 2.83 kB 2.83 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 B

Helpers, composables, and utility bundles

File Before After Δ Raw Δ Gzip Δ Brotli
assets/audioUtils-CUt_Q9Dg.js 1.41 kB 1.41 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Vendor & Third-Party — 9.1 MB (baseline 9.1 MB) • ⚪ 0 B

External libraries and shared vendor chunks

File Before After Δ Raw Δ Gzip Δ Brotli
assets/vendor-chart-DOvov576.js 452 kB 452 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-other-BKKxyZwz.js 3.86 MB 3.86 MB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-primevue-Btu8cl1B.js 1.95 MB 1.95 MB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-three-DYL0ZbEr.js 2.02 MB 2.02 MB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-tiptap-CUNFo3Ub.js 232 kB 232 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-vue-BlzIh1pE.js 160 kB 160 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/vendor-xterm-BF8peZ5_.js 420 kB 420 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
Other — 3.46 MB (baseline 3.46 MB) • ⚪ 0 B

Bundles that do not match a named category

File Before After Δ Raw Δ Gzip Δ Brotli
assets/AudioPreviewPlayer-ju-8XcLM.js 13.3 kB 13.3 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-bTEY9Mp6.js 13.8 kB 13.8 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-BWp4HdfU.js 101 B 101 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-CcfGaui5.js 14.4 kB 14.4 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-CisfgZf5.js 13.7 kB 13.7 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-CkU12Foh.js 13 kB 13 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-CoH2DJa6.js 14.2 kB 14.2 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-COSt-Bjx.js 14.9 kB 14.9 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-DalfIW5f.js 15.9 kB 15.9 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-DfTl0eCm.js 13.5 kB 13.5 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/commands-DwSJL865.js 13.7 kB 13.7 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/Load3D-DjiAqHCe.js 424 B 424 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-Bdc58rJq.js 97.1 kB 97.1 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-BRKE6VDs.js 91 kB 91 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-C9ZJBRdI.js 81.5 kB 81.5 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-CAL83XT3.js 84.6 kB 84.6 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-CHLLfvpG.js 82.4 kB 82.4 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-Cw9RZWRY.js 89 B 89 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-DDqR5EuX.js 71.3 kB 71.3 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-DLHyaEcz.js 92.1 kB 92.1 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-O7KfJeMO.js 79.9 kB 79.9 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/main-OzGsrlqJ.js 112 kB 112 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/Media3DBottom-C3Nasj2E.js 1.5 kB 1.5 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/Media3DTop-9huZkNiM.js 1.49 kB 1.49 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaAudioBottom-CAtPBbIK.js 1.51 kB 1.51 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaAudioTop-Bu3Uau7e.js 1.46 kB 1.46 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaImageBottom-BC-2HI_g.js 1.55 kB 1.55 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaImageTop-DTAowaFC.js 1.75 kB 1.75 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaVideoBottom-BQl2HlLG.js 1.5 kB 1.5 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/MediaVideoTop-BqwS3WNi.js 2.65 kB 2.65 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-aW9En70v.js 260 kB 260 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-BIckSVgU.js 273 kB 273 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-BiYpVi7D.js 263 kB 263 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-Bw_Jitw_.js 101 B 101 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-CCEXtYfM.js 243 kB 243 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-CvmVDWYd.js 323 kB 323 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-D_wreoPJ.js 267 kB 267 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-DIAYlJwM.js 246 kB 246 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-Dz-0ZIBN.js 297 kB 297 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/nodeDefs-VZsNmhG7.js 264 kB 264 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/SubscriptionRequiredDialogContent-Dl1KV5c6.js 28 kB 28 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/ValueControlPopover-DIg_X8FB.js 5.49 kB 5.49 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetAudioUI-CxBRFlKm.js 2.89 kB 2.89 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetChart-B3VlzWCa.js 2.48 kB 2.48 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetColorPicker-D927Vo08.js 3.41 kB 3.41 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetGalleria-BOG6ZBkS.js 4.1 kB 4.1 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetImageCompare-C_Tqin9v.js 3.18 kB 3.18 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetInputNumber-UKT-Xjb0.js 673 B 673 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetInputText-Cm3wYpf8.js 1.99 kB 1.99 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetLegacy-BfL1p4_s.js 364 B 364 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetMarkdown-BfaNBJeD.js 3.08 kB 3.08 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/widgetPropFilter-BIbGSUAt.js 1.28 kB 1.28 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetRecordAudio-mK8pG_J8.js 20.4 kB 20.4 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetSelect-BGmsUwfK.js 733 B 733 B ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetTextarea-CKkjY5d_.js 3.08 kB 3.08 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B
assets/WidgetToggleSwitch-B1Sdfqyv.js 1.76 kB 1.76 kB ⚪ 0 B ⚪ 0 B ⚪ 0 B

@christian-byrne
Copy link
Contributor Author

christian-byrne commented Dec 22, 2025

It is working correctly (per trace), I just used bad text to query for - will change to "Modèles"

image

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c46de5 and 2533f0d.

📒 Files selected for processing (1)
  • browser_tests/tests/templates.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)

browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports

Files:

  • browser_tests/tests/templates.spec.ts
**/*.{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 use any type or as any type 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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
browser_tests/**/*.spec.ts

📄 CodeRabbit inference engine (AGENTS.md)

browser_tests/**/*.spec.ts: Use E2E tests in browser_tests/**/*.spec.ts with Playwright framework
Do not use waitForTimeout in Playwright tests; use Locator actions and retrying assertions instead
Use tags like @mobile and @2x in Playwright tests for relevant test variations; tags are respected by config

Files:

  • browser_tests/tests/templates.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Minimize the surface area (exported values) of each module and composable

Files:

  • browser_tests/tests/templates.spec.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/locales/en/main.json:2257-2272
Timestamp: 2025-12-05T20:35:28.330Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, vue-i18n pluralization uses a two-parameter pattern: `t('key', { count: value })` where the `count` property in the interpolation object automatically triggers plural selection from pipe-separated locale strings (e.g., "no items | 1 item | {count} items"). This is the correct pattern used throughout the codebase, as validated by Playwright tests.
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: Create browser tests as appropriate after code changes
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:17-22
Timestamp: 2025-12-13T05:54:35.779Z
Learning: In browser_tests tests for the Comfy-Org/ComfyUI_frontend repository, the `comfyPage.loadWorkflow()` method already handles all necessary synchronization and waiting. No additional `await comfyPage.nextFrame()` call is needed before taking screenshots after loading a workflow.
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
📚 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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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} : Check assets/ directory for test data when writing tests

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-21T06:04:12.548Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.548Z
Learning: Applies to browser_tests/**/*.spec.ts : Do not use `waitForTimeout` in Playwright tests; use Locator actions and retrying assertions instead

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-21T06:04:12.548Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.548Z
Learning: Applies to **/*.test.ts : Do not write change detector tests or tests dependent on non-behavioral features like utility classes or styles

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{components,composables}/**/*.{ts,tsx,vue} : Use vue-i18n for ALL user-facing strings by adding them to `src/locales/en/main.json`

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-05T20:35:28.330Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/locales/en/main.json:2257-2272
Timestamp: 2025-12-05T20:35:28.330Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, vue-i18n pluralization uses a two-parameter pattern: `t('key', { count: value })` where the `count` property in the interpolation object automatically triggers plural selection from pipe-separated locale strings (e.g., "no items | 1 item | {count} items"). This is the correct pattern used throughout the codebase, as validated by Playwright tests.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-13T05:54:35.779Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:17-22
Timestamp: 2025-12-13T05:54:35.779Z
Learning: In browser_tests tests for the Comfy-Org/ComfyUI_frontend repository, the `comfyPage.loadWorkflow()` method already handles all necessary synchronization and waiting. No additional `await comfyPage.nextFrame()` call is needed before taking screenshots after loading a workflow.

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-13T05:34:15.488Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:15.488Z
Learning: In Playwright tests for the ComfyUI frontend, the toPass() assertion uses incremental backoff during retries. When a test may involve async operations, increasing the timeout (e.g., to 5000 ms) can be sufficient instead of aggressively extending timeouts. Apply this understanding to tests under browser_tests/tests/; if not resolved, review the toPass() backoff behavior and ensure timeouts align with expected async completion without masking issues.

Applied to files:

  • browser_tests/tests/templates.spec.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: test
  • GitHub Check: setup
  • GitHub Check: lint-and-format
  • GitHub Check: collect

DrJKL
DrJKL previously approved these changes Dec 22, 2025
await comfyPage.executeCommand('Comfy.BrowseTemplates')

const request = await requestPromise
const dialog = comfyPage.page.getByRole('dialog', { name: 'Modèles' })
Copy link
Contributor

Choose a reason for hiding this comment

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

I think maybe this:

const dialog = comfyPage.page.getByRole('dialog')
  .filter({ has: page.getByRole('heading', { name: 'Modèles' }) })

Copy link
Contributor Author

@christian-byrne christian-byrne Dec 28, 2025

Choose a reason for hiding this comment

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

Thank you, it worked!

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cf725b and 72799fc.

📒 Files selected for processing (1)
  • browser_tests/tests/templates.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)

browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports

Files:

  • browser_tests/tests/templates.spec.ts
**/*.{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 use any type or as any type 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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
browser_tests/**/*.spec.ts

📄 CodeRabbit inference engine (AGENTS.md)

browser_tests/**/*.spec.ts: Use E2E tests in browser_tests/**/*.spec.ts with Playwright framework
Do not use waitForTimeout in Playwright tests; use Locator actions and retrying assertions instead
Use tags like @mobile and @2x in Playwright tests for relevant test variations; tags are respected by config

Files:

  • browser_tests/tests/templates.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Minimize the surface area (exported values) of each module and composable

Files:

  • browser_tests/tests/templates.spec.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
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: Create browser tests as appropriate after code changes
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:17-22
Timestamp: 2025-12-13T05:54:35.779Z
Learning: In browser_tests tests for the Comfy-Org/ComfyUI_frontend repository, the `comfyPage.loadWorkflow()` method already handles all necessary synchronization and waiting. No additional `await comfyPage.nextFrame()` call is needed before taking screenshots after loading a workflow.
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
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
📚 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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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} : Check assets/ directory for test data when writing tests

Applied to files:

  • browser_tests/tests/templates.spec.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 : Do not write change detector tests or tests dependent on non-behavioral features like utility classes or styles

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-10T03:09:19.636Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-13T05:54:35.779Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:17-22
Timestamp: 2025-12-13T05:54:35.779Z
Learning: In browser_tests tests for the Comfy-Org/ComfyUI_frontend repository, the `comfyPage.loadWorkflow()` method already handles all necessary synchronization and waiting. No additional `await comfyPage.nextFrame()` call is needed before taking screenshots after loading a workflow.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-13T05:34:15.488Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:15.488Z
Learning: In Playwright tests for the ComfyUI frontend, the toPass() assertion uses incremental backoff during retries. When a test may involve async operations, increasing the timeout (e.g., to 5000 ms) can be sufficient instead of aggressively extending timeouts. Apply this understanding to tests under browser_tests/tests/; if not resolved, review the toPass() backoff behavior and ensure timeouts align with expected async completion without masking issues.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.

Applied to files:

  • browser_tests/tests/templates.spec.ts
📚 Learning: 2025-12-05T20:35:28.330Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/locales/en/main.json:2257-2272
Timestamp: 2025-12-05T20:35:28.330Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, vue-i18n pluralization uses a two-parameter pattern: `t('key', { count: value })` where the `count` property in the interpolation object automatically triggers plural selection from pipe-separated locale strings (e.g., "no items | 1 item | {count} items"). This is the correct pattern used throughout the codebase, as validated by Playwright tests.

Applied to files:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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:

  • browser_tests/tests/templates.spec.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: lint-and-format
  • GitHub Check: test
  • GitHub Check: setup
  • GitHub Check: collect

Comment on lines 111 to 133
test('Uses proper locale files for templates', async ({ comfyPage }) => {
// Load the templates dialog and wait for the French index file request
const requestPromise = comfyPage.page.waitForRequest(
'**/templates/index.fr.json'
)

// Set locale to French before opening templates
await comfyPage.setSetting('Comfy.Locale', 'fr')

await comfyPage.executeCommand('Comfy.BrowseTemplates')

const request = await requestPromise
const dialog = comfyPage.page.getByRole('dialog').filter({
has: comfyPage.page.getByRole('heading', { name: 'Modèles', exact: true })
})
await expect(dialog).toBeVisible()

// Verify French index was requested
expect(request.url()).toContain('templates/index.fr.json')
// Validate that French-localized strings from the templates index are rendered
await expect(
dialog.getByRole('heading', { name: 'Modèles', exact: true })
).toBeVisible()
await expect(
dialog.getByRole('button', { name: 'Tous les modèles', exact: true })
).toBeVisible()

await expect(comfyPage.templates.content).toBeVisible()
// Ensure the English fallback copy is not shown anywhere
await expect(
comfyPage.page.getByText('All Templates', { exact: true })
).toHaveCount(0)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Excellent improvement to test reliability!

The approach of validating rendered French UI strings is much more robust than waiting on network requests. The dialog filtering by heading "Modèles" and use of specific role-based selectors with exact: true aligns well with Playwright best practices.

Optional: Scope the negative assertion to the dialog for consistency.

Lines 122-127 correctly scope positive assertions to the dialog locator, but line 131 searches the entire page. For better test isolation and consistency, consider scoping the English fallback check to the dialog as well:

🔎 Proposed refinement
     // Ensure the English fallback copy is not shown anywhere
     await expect(
-      comfyPage.page.getByText('All Templates', { exact: true })
+      dialog.getByText('All Templates', { exact: true })
     ).toHaveCount(0)

This ensures the test only validates the dialog content (as stated in the PR objectives) and prevents false negatives if "All Templates" appears elsewhere on the page (e.g., in navigation or menus).

🤖 Prompt for AI Agents
In browser_tests/tests/templates.spec.ts around lines 111 to 133, the negative
assertion that English fallback text "All Templates" is checked against the
entire page (line ~131), which can produce false negatives; scope that negative
assertion to the dialog locator used above by replacing the page-level getByText
with dialog.getByText (or an equivalent dialog-scoped locator) so the test
verifies that "All Templates" is not present inside the dialog only.

@christian-byrne christian-byrne merged commit 33d8cb7 into main Dec 28, 2025
32 checks passed
@christian-byrne christian-byrne deleted the tests/fix-flaky-templates-test branch December 28, 2025 01:40
@christian-byrne christian-byrne added needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch core/1.36 Backport PRs for core 1.36 cloud/1.36 Backport PRs for cloud 1.36 and removed needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch labels Jan 13, 2026
@christian-byrne
Copy link
Contributor Author

Backporting to just improve test signal on RC

@github-actions
Copy link

@christian-byrne Commit 33d8cb706922af9c16f26356bb6d44dd0788c41d already exists on branch core/1.36. No backport needed.

@github-actions
Copy link

@christian-byrne Commit 33d8cb706922af9c16f26356bb6d44dd0788c41d already exists on branch cloud/1.36. No backport needed.

@github-actions
Copy link

@christian-byrne Commit 33d8cb706922af9c16f26356bb6d44dd0788c41d already exists on branch core/1.36. No backport needed.

@github-actions
Copy link

@christian-byrne Commit 33d8cb706922af9c16f26356bb6d44dd0788c41d already exists on branch cloud/1.36. No backport needed.

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

Labels

area:testing cloud/1.36 Backport PRs for cloud 1.36 core/1.36 Backport PRs for core 1.36 needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants