feat: add Free subscription tier support - #8864
Merged
Merged
Conversation
🎭 Playwright: ✅ 543 passed, 0 failed · 7 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request introduces support for a FREE subscription tier across the platform. Changes add 'FREE' to subscription tier type definitions, update subscription-related composables to expose a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
📦 Bundle: 4.41 MB gzip 🔴 +4.64 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 970 kB (baseline 970 kB) • 🔴 +31 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 68.8 kB) • 🔴 +3.4 kBTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 435 kB (baseline 436 kB) • 🟢 -489 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47.1 kB (baseline 46.9 kB) • 🔴 +130 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 2.54 MB (baseline 2.54 MB) • 🔴 +2.11 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 55.5 kB (baseline 58.3 kB) • 🟢 -2.75 kBHelpers, composables, and utility bundles
Status: 12 added / 13 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.69 MB (baseline 7.68 MB) • 🔴 +13 kBBundles that do not match a named category
Status: 66 added / 63 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/platform/cloud/onboarding/CloudLoginView.vue`:
- Around line 135-149: Replace the arrow-function expressions with function
declarations for the two handlers: change the const onShowEmailForm = () => {
... } to function onShowEmailForm() { ... } and const onBackToSocialLogin = ()
=> { ... } to function onBackToSocialLogin() { ... }; keep the same body
(setting showEmailForm.value and calling telemetry?.trackUiButtonClicked) and
ensure no other references rely on these being consts (they remain available in
the same scope).
In `@src/platform/cloud/onboarding/CloudSignupView.vue`:
- Around line 152-167: Replace the arrow function expressions for
onShowEmailForm and onBackToSocialLogin with plain function declarations named
onShowEmailForm and onBackToSocialLogin respectively; keep the body logic intact
(toggling showEmailForm.value and calling telemetry?.trackUiButtonClicked with
the same button_id values), and ensure their definitions remain in the same
scope where showEmailForm, telemetry, and other refs are available so behavior
doesn't change.
🧹 Nitpick comments (2)
src/platform/cloud/onboarding/CloudSignupView.vue (1)
25-93: Use$tdirectly in the template and remove theuseI18nimport.The
tfunction is only used in template interpolations, never in script logic. Using Vue's auto-injected$teliminates the redundantuseI18nimport and keeps the script cleaner.Suggested update
-import { useI18n } from 'vue-i18n' import { computed, onMounted, ref } from 'vue' import { useRoute, useRouter } from 'vue-router' ... -const { t } = useI18n() const router = useRouter()Then replace all
{{ t(...) }}with{{ $t(...) }}in the template.src/platform/cloud/onboarding/CloudLoginView.vue (1)
23-88: Use$tdirectly in the template and remove theuseI18nimport.Since the
tfunction is only used in the template, use the auto-injected$tinstead. This eliminates the unused import and the destructuring statement.Suggested update
-import { useI18n } from 'vue-i18n' import { useRoute, useRouter } from 'vue-router'-const { t } = useI18n() const router = useRouter()Replace all
t('key')with$t('key')in the template.
Comment on lines
+135
to
+149
| const showEmailForm = ref(false) | ||
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | ||
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | ||
| const toastStore = useToastStore() | ||
| const telemetry = useTelemetry() | ||
|
|
||
| const onShowEmailForm = () => { | ||
| showEmailForm.value = true | ||
| telemetry?.trackUiButtonClicked({ button_id: 'login_use_email_instead' }) | ||
| } | ||
|
|
||
| const onBackToSocialLogin = () => { | ||
| showEmailForm.value = false | ||
| telemetry?.trackUiButtonClicked({ button_id: 'login_back_to_social_login' }) | ||
| } |
Contributor
There was a problem hiding this comment.
Use function declarations for onShowEmailForm / onBackToSocialLogin.
Repository guidance prefers function declarations when possible.
♻️ Suggested update
-const onShowEmailForm = () => {
+function onShowEmailForm() {
showEmailForm.value = true
telemetry?.trackUiButtonClicked({ button_id: 'login_use_email_instead' })
}
-const onBackToSocialLogin = () => {
+function onBackToSocialLogin() {
showEmailForm.value = false
telemetry?.trackUiButtonClicked({ button_id: 'login_back_to_social_login' })
}Based on learnings "Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository."
📝 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.
Suggested change
| const showEmailForm = ref(false) | |
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | |
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | |
| const toastStore = useToastStore() | |
| const telemetry = useTelemetry() | |
| const onShowEmailForm = () => { | |
| showEmailForm.value = true | |
| telemetry?.trackUiButtonClicked({ button_id: 'login_use_email_instead' }) | |
| } | |
| const onBackToSocialLogin = () => { | |
| showEmailForm.value = false | |
| telemetry?.trackUiButtonClicked({ button_id: 'login_back_to_social_login' }) | |
| } | |
| const showEmailForm = ref(false) | |
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | |
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | |
| const toastStore = useToastStore() | |
| const telemetry = useTelemetry() | |
| function onShowEmailForm() { | |
| showEmailForm.value = true | |
| telemetry?.trackUiButtonClicked({ button_id: 'login_use_email_instead' }) | |
| } | |
| function onBackToSocialLogin() { | |
| showEmailForm.value = false | |
| telemetry?.trackUiButtonClicked({ button_id: 'login_back_to_social_login' }) | |
| } |
🤖 Prompt for AI Agents
In `@src/platform/cloud/onboarding/CloudLoginView.vue` around lines 135 - 149,
Replace the arrow-function expressions with function declarations for the two
handlers: change the const onShowEmailForm = () => { ... } to function
onShowEmailForm() { ... } and const onBackToSocialLogin = () => { ... } to
function onBackToSocialLogin() { ... }; keep the same body (setting
showEmailForm.value and calling telemetry?.trackUiButtonClicked) and ensure no
other references rely on these being consts (they remain available in the same
scope).
Comment on lines
+152
to
+167
| const showEmailForm = ref(false) | ||
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | ||
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | ||
| const userIsInChina = ref(false) | ||
| const toastStore = useToastStore() | ||
| const telemetry = useTelemetry() | ||
|
|
||
| const onShowEmailForm = () => { | ||
| showEmailForm.value = true | ||
| telemetry?.trackUiButtonClicked({ button_id: 'signup_use_email_instead' }) | ||
| } | ||
|
|
||
| const onBackToSocialLogin = () => { | ||
| showEmailForm.value = false | ||
| telemetry?.trackUiButtonClicked({ button_id: 'signup_back_to_social_login' }) | ||
| } |
Contributor
There was a problem hiding this comment.
Use function declarations for onShowEmailForm / onBackToSocialLogin.
Repository guidance prefers function declarations when possible.
♻️ Suggested update
-const onShowEmailForm = () => {
+function onShowEmailForm() {
showEmailForm.value = true
telemetry?.trackUiButtonClicked({ button_id: 'signup_use_email_instead' })
}
-const onBackToSocialLogin = () => {
+function onBackToSocialLogin() {
showEmailForm.value = false
telemetry?.trackUiButtonClicked({ button_id: 'signup_back_to_social_login' })
}Based on learnings "Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository."
📝 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.
Suggested change
| const showEmailForm = ref(false) | |
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | |
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | |
| const userIsInChina = ref(false) | |
| const toastStore = useToastStore() | |
| const telemetry = useTelemetry() | |
| const onShowEmailForm = () => { | |
| showEmailForm.value = true | |
| telemetry?.trackUiButtonClicked({ button_id: 'signup_use_email_instead' }) | |
| } | |
| const onBackToSocialLogin = () => { | |
| showEmailForm.value = false | |
| telemetry?.trackUiButtonClicked({ button_id: 'signup_back_to_social_login' }) | |
| } | |
| const showEmailForm = ref(false) | |
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) | |
| const showFreeTierBadge = !localStorage.getItem('comfy:hasAccount') | |
| const userIsInChina = ref(false) | |
| const toastStore = useToastStore() | |
| const telemetry = useTelemetry() | |
| function onShowEmailForm() { | |
| showEmailForm.value = true | |
| telemetry?.trackUiButtonClicked({ button_id: 'signup_use_email_instead' }) | |
| } | |
| function onBackToSocialLogin() { | |
| showEmailForm.value = false | |
| telemetry?.trackUiButtonClicked({ button_id: 'signup_back_to_social_login' }) | |
| } |
🤖 Prompt for AI Agents
In `@src/platform/cloud/onboarding/CloudSignupView.vue` around lines 152 - 167,
Replace the arrow function expressions for onShowEmailForm and
onBackToSocialLogin with plain function declarations named onShowEmailForm and
onBackToSocialLogin respectively; keep the body logic intact (toggling
showEmailForm.value and calling telemetry?.trackUiButtonClicked with the same
button_id values), and ensure their definitions remain in the same scope where
showEmailForm, telemetry, and other refs are available so behavior doesn't
change.
huntcsg
force-pushed
the
free-tier-initial
branch
from
February 20, 2026 03:38
9352ef8 to
378b590
Compare
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/platform/cloud/onboarding/CloudLoginView.vue`:
- Around line 141-149: Replace the arrow function expressions for
onShowEmailForm and onBackToSocialLogin with plain function declarations:
implement function onShowEmailForm() { showEmailForm.value = true;
telemetry?.trackUiButtonClicked({ button_id: 'login_use_email_instead' }) } and
function onBackToSocialLogin() { showEmailForm.value = false;
telemetry?.trackUiButtonClicked({ button_id: 'login_back_to_social_login' }) },
preserving the exact side effects and telemetry calls and keeping the same
symbol names so existing references still work.
In `@src/platform/cloud/onboarding/CloudSignupView.vue`:
- Around line 159-167: Convert the two handler function expressions
onShowEmailForm and onBackToSocialLogin into function declarations (replace the
const arrow functions with function onShowEmailForm() { ... } and function
onBackToSocialLogin() { ... }) while preserving their bodies: set
showEmailForm.value appropriately and keep the telemetry?.trackUiButtonClicked
calls unchanged; ensure the functions remain in the same module scope so
existing callers and reactivity (showEmailForm ref) continue to work and no
other references need updating.
huntcsg
force-pushed
the
free-tier-initial
branch
from
February 20, 2026 03:48
378b590 to
22eb1b3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/lib/litegraph/src/LGraphCanvas.ts (1)
3612-3626:⚠️ Potential issue | 🟡 MinorKeep selection-change notifications consistent on cancel.
On cancel, the selection changes via
deselect(node)but listeners won’t getonSelectionChange, while the confirm branch now does. That inconsistency can leave UI in a stale selection state. Consider firing the callback (or moving it after the branch to cover both).💡 Suggested fix
if (cancelled) { this.deselect(node) + this.onSelectionChange?.(this.selected_nodes) this.graph?.remove(node) } else { delete node.flags.ghost this.graph?.trigger('node:property:changed', { nodeId: node.id, property: 'flags.ghost', oldValue: true, newValue: false }) this.state.selectionChanged = true this.onSelectionChange?.(this.selected_nodes) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/litegraph/src/LGraphCanvas.ts` around lines 3612 - 3626, The cancel branch calls deselect(node) and updates the graph but does not notify listeners, causing inconsistent selection-change notifications compared to the confirm branch; update the logic so selection notifications always fire by either moving the state.selectionChanged = true and onSelectionChange?.(this.selected_nodes) lines after the if/else or by adding those two lines into the cancelled branch (ensure you reference deselect(node), state.selectionChanged, onSelectionChange, and selected_nodes when making the change) so both confirm and cancel paths notify listeners consistently.src/extensions/core/uploadImage.ts (1)
1-6:⚠️ Potential issue | 🟡 MinorSplit type-only imports from value imports.
This mixed import violates the repo rule that type-only imports must be in a separate
import typestatement.♻️ Proposed fix
-import { - type ComfyNodeDef, - type InputSpec, - isComboInputSpecV1 -} from '@/schemas/nodeDefSchema' +import type { ComfyNodeDef, InputSpec } from '@/schemas/nodeDefSchema' +import { isComboInputSpecV1 } from '@/schemas/nodeDefSchema'As per coding guidelines, use separate
import typestatements instead of inlinetypekeyword in mixed imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/extensions/core/uploadImage.ts` around lines 1 - 6, The mixed import combines type-only and value imports; split them so type-only symbols (LGraphNode, ComfyNodeDef, InputSpec) are imported with "import type" and value symbols (isComboInputSpecV1) are imported with a regular import. Update the top of src/extensions/core/uploadImage.ts to use one import type line for LGraphNode, ComfyNodeDef, InputSpec and a separate non-type import for isComboInputSpecV1 so type-only imports are not mixed with runtime values.src/extensions/core/groupNode.ts (1)
2000-2009:⚠️ Potential issue | 🟠 MajorStore group nodes under the canonical name, not
node.title.
node.titlecan diverge from the registered group name (e.g., user renames an instance), which can mis-keygroupNodesand break re-registration on reload. Use the group definition’s name (or derive fromnode.type) as the storage key.Suggested fix
- if (node.title && handler?.groupData?.nodeData) { - Workflow.storeGroupNode(node.title, handler.groupData.nodeData) - } + const groupName = handler?.groupData?.name + if (groupName && handler?.groupData?.nodeData) { + Workflow.storeGroupNode(groupName, handler.groupData.nodeData) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/extensions/core/groupNode.ts` around lines 2000 - 2009, The code in nodeCreated uses node.title as the storage key which can differ from the canonical group name; change the store key to the group definition's canonical name (from GroupNodeHandler or derived from node.type) instead of node.title: in nodeCreated, after creating the GroupNodeHandler and obtaining handler.groupData.nodeData, call Workflow.storeGroupNode using handler.groupData.name (or handler.getDefinitionName()) or fallback to node.type if the handler does not expose a name; keep storing the same nodeData (handler.groupData.nodeData) but replace node.title with the canonical name to ensure consistent re-registration.src/components/dialog/content/ConfirmationDialogContent.vue (1)
33-50:⚠️ Potential issue | 🟡 MinorUse
$tin the template and dropuseI18nhere.
useI18nis only supplyingtfor the template, so$tkeeps the setup leaner.🛠️ Proposed fix
- <label for="doNotAskAgain">{{ - t('missingModelsDialog.doNotAskAgain') - }}</label> + <label for="doNotAskAgain">{{ + $t('missingModelsDialog.doNotAskAgain') + }}</label>- {{ t('missingModelsDialog.reEnableInSettingsLink') }} + {{ $t('missingModelsDialog.reEnableInSettingsLink') }}-import { useI18n } from 'vue-i18n'-const { t } = useI18n()Based on learnings: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t in the template instead of importing useI18n and destructuring t in the script.
Also applies to: 118-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dialog/content/ConfirmationDialogContent.vue` around lines 33 - 50, The template is using the injected t from useI18n unnecessarily; remove the useI18n import/destructuring and any script-level t usage, then replace template calls like t('missingModelsDialog.doNotAskAgain') and t('missingModelsDialog.reEnableInSettingsLink') with the global $t('...') in the template; keep event handlers like openBlueprintOverwriteSetting unchanged and ensure no remaining references to the t variable or useI18n remain (also apply the same replacement for the other occurrences around the component where t is only used in the template).
🧹 Nitpick comments (25)
src/platform/assets/utils/createAssetWidget.ts (1)
85-91: Consider applyingfromZodError()consistently to filename validation as well.The asset item validation (line 71) now uses
fromZodError(), but the filename validation error at lines 86-91 still logs the rawerror.errorsarray. For consistency and improved log readability, consider updating this block too.♻️ Proposed fix for consistency
if (!validatedFilename.success) { console.error( 'Invalid asset filename:', - validatedFilename.error.errors, + fromZodError(validatedFilename.error).message, 'for asset:', validatedAsset.data.id )Based on learnings: "In assetService.ts, prefer using safeParse() ... and use fromZodError(result.error) to format error messages for logging."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/assets/utils/createAssetWidget.ts` around lines 85 - 91, The filename validation branch logs raw Zod errors; change it to format errors with fromZodError for consistency: when validatedFilename.success is false, call fromZodError(validatedFilename.error) and include that formatted message in the console.error (mirroring how validatedAsset is handled) so the log uses fromZodError instead of raw validatedFilename.error.errors; update the console.error invocation around validatedFilename and validatedAsset in createAssetWidget.ts accordingly.src/components/actionbar/ComfyRunButton/ComfyQueueButton.vue (1)
205-210: Consider replacing the scoped style with Tailwind utilities.Per coding guidelines, Vue components should use Tailwind 4 for styling and avoid
<style>blocks. The:deep()selector targeting PrimeVue's SplitButton dropdown could potentially be replaced with Tailwind utilities or handled via the component's class props.However, since this is styling a PrimeVue internal element that may not expose direct class customization, this override may be necessary for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/actionbar/ComfyRunButton/ComfyQueueButton.vue` around lines 205 - 210, The component currently uses a scoped style block in ComfyQueueButton.vue to target PrimeVue's internal dropdown via .comfyui-queue-button :deep(.p-splitbutton-dropdown) and set border-top-right-radius/bottom-right-radius to 0; replace this by removing the <style> block and, if possible, apply Tailwind utilities to the SplitButton wrapper (e.g., add classes on the ComfyQueueButton root or to the PrimeVue SplitButton via its class/style props) to achieve the same zero right-radius, otherwise keep the override but move it into a global CSS file (or a theme layer) with the same selector so the project avoids component-scoped <style> blocks; ensure you reference the .comfyui-queue-button wrapper and the :deep(.p-splitbutton-dropdown) selector when making the change.src/components/actionbar/ComfyRunButton/ComfyQueueButton.test.ts (1)
161-175: Remove redundant duplicate assertions.Lines 171-175 duplicate the assertions already made in lines 161-167. After verifying the mode transition and UI state once, re-asserting the same values adds noise without testing additional behavior.
♻️ Proposed simplification
expect(splitButtonWhileStopping.attributes('data-severity')).toBe('primary') expect(wrapper.find('.icon-\\[lucide--fast-forward\\]').exists()).toBe(true) expect(commandStore.execute).not.toHaveBeenCalled() - - const splitButton = wrapper.get('[data-testid="queue-button"]') - expect(queueSettingsStore.mode).toBe('instant-idle') - expect(splitButton.attributes('data-label')).toBe('Run (Instant)') - expect(splitButton.attributes('data-severity')).toBe('primary') - expect(wrapper.find('.icon-\\[lucide--fast-forward\\]').exists()).toBe(true) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/actionbar/ComfyRunButton/ComfyQueueButton.test.ts` around lines 161 - 175, Remove the redundant duplicate assertions that repeat the same checks for queueSettingsStore.mode, the queue-button attributes, and the fast-forward icon: after the initial checks using splitButtonWhileStopping, delete the second block that re-assigns splitButton and re-checks queueSettingsStore.mode, splitButton.attributes('data-label'), splitButton.attributes('data-severity'), and wrapper.find('.icon-\\[lucide--fast-forward\\]').exists(); keep the first assertions and the expect(commandStore.execute).not.toHaveBeenCalled() check only.src/components/ui/toggle-group/index.ts (1)
1-2: Consider whether this barrel file is needed.Per coding guidelines, barrel files are discouraged. The UI component guideline also states to "Import siblings directly (./Component.vue), not from barrel ('.')". If this is intended for external package consumption, it may be acceptable; otherwise, consumers should import directly from the component files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/toggle-group/index.ts` around lines 1 - 2, This barrel file exports ToggleGroup and ToggleGroupItem but barrels are discouraged; remove src/components/ui/toggle-group/index.ts and update all internal imports that reference the directory (e.g., import { ToggleGroup } from '.../toggle-group') to import the components directly from their sibling files (import ToggleGroup from './ToggleGroup.vue' and import ToggleGroupItem from './ToggleGroupItem.vue'); if the barrel is intentionally kept for external package consumers, instead add a clear comment above the exports explaining that this file exists only for package public API and leave it as-is.src/components/common/TextTicker.test.ts (1)
14-16: Consider using a more specific type for the wrapper variable.The current type
ReturnType<typeof mount>loses the component type information. Using a more specific type would improve IntelliSense and type safety.♻️ Suggested improvement
+import type { VueWrapper } from '@vue/test-utils' + describe(TextTicker, () => { let rafCallbacks: ((time: number) => void)[] - let wrapper: ReturnType<typeof mount> + let wrapper: VueWrapper<InstanceType<typeof TextTicker>>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TextTicker.test.ts` around lines 14 - 16, The wrapper variable is currently typed as ReturnType<typeof mount>, which loses the component's type info; update wrapper to a more specific wrapper type tied to TextTicker (e.g., use the test-utils wrapper generic or the framework-specific wrapper type) by changing its declaration to a typed wrapper such as ReactWrapper<TextTickerProps, TextTickerState, TextTicker> or VueWrapper<ComponentPublicInstance> (or by using mount<typeof TextTicker>() if mount supports generics) so IntelliSense and type safety reference the TextTicker component directly; adjust imports to pull in ReactWrapper/VueWrapper or the appropriate generic helper and update any usages accordingly.src/composables/sidebarTabs/useAssetsSidebarTab.test.ts (1)
26-26: Use function reference indescribefor Vitest rule alignment.♻️ Suggested change
-describe('useAssetsSidebarTab', () => { +describe(useAssetsSidebarTab, () => {Based on learnings: “In test files under src/**/*.test.ts, follow the vitest/prefer-describe-function-title rule by using describe(ComponentOrFunction, ...) instead of a string literal description when naming test suites.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/sidebarTabs/useAssetsSidebarTab.test.ts` at line 26, Replace the string literal test-suite title with the actual function reference to satisfy vitest/prefer-describe-function-title: change describe('useAssetsSidebarTab', ...) to describe(useAssetsSidebarTab, ...) and ensure the symbol useAssetsSidebarTab is imported from its module at the top of the test file so the reference resolves.src/composables/sidebarTabs/useAssetsSidebarTab.ts (1)
8-9: Prefer a function declaration for the composable.This is a pure function and can be expressed as a declaration for consistency with repo conventions.
♻️ Suggested refactor
-export const useAssetsSidebarTab = (): SidebarTabExtension => { +export function useAssetsSidebarTab(): SidebarTabExtension { return { id: 'assets', icon: 'icon-[comfy--image-ai-edit]', title: 'sideToolbar.assets', tooltip: 'sideToolbar.assets', label: 'sideToolbar.labels.assets', component: markRaw(AssetsSidebarTab), type: 'vue', iconBadge: () => { const settingStore = useSettingStore() if (!settingStore.get('Comfy.Queue.QPOV2')) { return null } const queueStore = useQueueStore() const count = queueStore.activeJobsCount return count > 0 ? count.toString() : null } } }Based on learnings: “Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/sidebarTabs/useAssetsSidebarTab.ts` around lines 8 - 9, Convert the exported const arrow function useAssetsSidebarTab into a named function declaration to match repository conventions for pure composables: change "export const useAssetsSidebarTab = (): SidebarTabExtension => { ... }" to a function declaration "export function useAssetsSidebarTab(): SidebarTabExtension { ... }" and keep the same body and return value so all callers and the SidebarTabExtension return type remain unchanged.src/components/common/SearchBoxV2.vue (1)
110-116: Consider initial emission behavior.
watchDebouncedwill emit on initial value changes. If the component mounts with an existingsearchTermvalue, it will emit asearchevent after the debounce delay. Verify this is the intended behavior, or add{ immediate: false }if initial emission should be suppressed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/SearchBoxV2.vue` around lines 110 - 116, The current watchDebounced on searchTerm will trigger an initial debounced emit of the 'search' event when the component mounts with a pre-filled value; if that initial emission is undesired, update the watchDebounced call (the watcher using searchTerm, emit, and debounceTime) to include immediate: false in the options (e.g., { debounce: debounceTime, immediate: false }) so it does not emit on initial mount, otherwise leave as-is if initial emission is intended.src/composables/node/useNodePreviewAndDrag.ts (1)
100-129: Potential timing issue with drag image cleanup.The drag image is appended to the document body and removed in a
requestAnimationFramecallback:document.body.appendChild(dragImage) e.dataTransfer.setDragImage(dragImage, 0, 0) requestAnimationFrame(() => { document.body.removeChild(dragImage) })While this pattern is commonly used,
requestAnimationFrameexecutes before the next paint, which should be sufficient forsetDragImageto capture the element. However, if the drag operation is cancelled very quickly, there's a small chance the element is removed before the browser fully processes the drag image.Consider using a slightly longer delay or keeping the element until
dragend:♻️ Safer cleanup approach
function handleDragStart(e: DragEvent) { if (!nodeDef.value) return isDragging.value = true isHovered.value = false startDrag(nodeDef.value, 'native') if (e.dataTransfer) { e.dataTransfer.effectAllowed = 'copy' e.dataTransfer.setData('application/x-comfy-node', nodeDef.value.name) const dragImage = createEmptyDragImage() document.body.appendChild(dragImage) e.dataTransfer.setDragImage(dragImage, 0, 0) - requestAnimationFrame(() => { - document.body.removeChild(dragImage) - }) + // Use setTimeout to ensure the drag image is captured + setTimeout(() => { + dragImage.remove() + }, 0) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePreviewAndDrag.ts` around lines 100 - 129, The temporary drag image may be removed too early by requestAnimationFrame in handleDragStart, risking the browser missing it; instead, append the drag image created by createEmptyDragImage and defer removal until the drag operation completes by storing the created element and removing it in a dragend listener (or as a fallback use a small timeout), i.e., attach a one-time 'dragend' handler that removes the dragImage from document.body and cleans up the listener so the element persists for the full drag lifecycle.src/components/searchbox/v2/NodeSearchInput.vue (1)
16-24: Consider using the shared Button component for the cancel filter button.Per coding guidelines, raw
<button>elements should be replaced with the shared Button component for consistent styling. However, this is a small icon-only button embedded within a tag chip, where inline styling may be acceptable for this compact use case.The
aria-labelis correctly provided for accessibility since this is an icon-only button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.vue` around lines 16 - 24, Replace the raw <button> in NodeSearchInput.vue with the shared Button component to enforce consistent styling: import and use the shared Button, preserve attributes data-testid="cancel-filter", type="button" and :aria-label="$t('g.remove')", and wire its click to emit('cancelFilter'); ensure the Button uses an icon-only/ghost/icon-size variant (or equivalent props) and renders the same <i class="pi pi-times text-xs" /> child so visual appearance remains unchanged.src/components/sidebar/tabs/nodeLibrary/NodeDragPreview.vue (1)
58-68: Use VueUseuseEventListenerfor drag/dragend listeners.It simplifies lifecycle cleanup and aligns with the repo’s VueUse guidance.
As per coding guidelines: src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/NodeDragPreview.vue` around lines 58 - 68, Replace the manual document.addEventListener / removeEventListener pattern with VueUse's useEventListener to auto-handle lifecycle and align with repo guidance: inside the setup where onMounted currently calls setupGlobalListeners and adds listeners, call useEventListener(document, 'drag', handleDrag) and useEventListener(document, 'dragend', handleDragEnd) (imported from '@vueuse/core'), then remove the explicit document.removeEventListener calls and the manual cleanup in onUnmounted (you can still call cleanupGlobalListeners if it does other work). Ensure you keep references to the existing handler functions handleDrag and handleDragEnd and remove the add/removeEventListener lines in NodeDragPreview.vue.src/components/searchbox/v2/NodeSearchInput.test.ts (2)
56-73: TypecreateWrapperprops withComponentProps<typeof NodeSearchInput>.This keeps the helper aligned with the component’s real props and matches the repo’s test helper typing convention.
🔧 Suggested change
import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ComponentProps } from 'vue-component-type-helpers' @@ -function createWrapper( - props: Partial<{ - filters: FuseFilterWithValue<ComfyNodeDefImpl, string>[] - activeFilter: FilterChip | null - searchQuery: string - filterQuery: string - }> = {} -) { +type NodeSearchInputProps = ComponentProps<typeof NodeSearchInput> + +function createWrapper(props: Partial<NodeSearchInputProps> = {}) { return mount(NodeSearchInput, { props: { filters: [], activeFilter: null, searchQuery: '', filterQuery: '', - ...props - }, + ...props + } as NodeSearchInputProps, global: { plugins: [testI18n] } }) }Based on learnings and coding guidelines: In test files, type helper props as Partial<ComponentProps>; **/*.ts: Derive component types using vue-component-type-helpers (ComponentProps, ComponentSlots).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 56 - 73, The createWrapper helper in NodeSearchInput.test.ts currently types its props inline; change the function signature to type props as Partial<ComponentProps<typeof NodeSearchInput>> (using the ComponentProps helper) so the test helper matches the component's actual props; update the import to bring in ComponentProps from the repo's vue-component-type-helpers (or the existing helper module) and leave the rest of createWrapper (props spreading into mount) unchanged, referencing the createWrapper function and NodeSearchInput component names when making the change.
111-135: Prefer accessible queries overdata-testidfor chip/cancel assertions.If the chip label/cancel button has visible text or role, query by accessible name instead of test IDs to keep tests closer to user behavior.
Based on learnings: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchInput.test.ts` around lines 111 - 135, Tests currently rely on data-testid selectors ('[data-testid="filter-chip"]' and '[data-testid="cancel-filter"]'); update the three specs to use accessible queries instead — e.g., in the "should hide/show filter chips" tests use createWrapper(...) then locate chips by their visible label/text or by role (findAll('button') and filter by text content or use a testing-library getByRole/getAllByRole with accessible name matching the chip label created via createFilter), and in "should emit cancelFilter" locate the cancel control by its visible text or aria-label/role instead of '[data-testid="cancel-filter"]' and trigger click; keep using createWrapper, createFilter, createActiveFilter and assert emitted events the same way.src/composables/node/useNodeDragToCanvas.ts (1)
67-83: Consider VueUseuseEventListenerfor global pointer/keyboard listeners.Using VueUse’s listener helpers simplifies cleanup and aligns with the repo’s composable guidelines.
As per coding guidelines: /composables//*.ts : Leverage VueUse functions for performance-enhancing composables.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodeDragToCanvas.ts` around lines 67 - 83, Replace the manual add/removeEventListener pattern in setupGlobalListeners/cleanupGlobalListeners with VueUse's useEventListener: import useEventListener from '@vueuse/core', call useEventListener(document, 'pointermove', updatePosition), useEventListener(document, 'pointerup', endDrag, { capture: true }), and useEventListener(document, 'keydown', handleKeydown) inside setupGlobalListeners, capture the returned stop functions (or disposers) into a local array or object keyed by listener, and then call those stop functions in cleanupGlobalListeners instead of removeEventListener; keep the listenersSetup flag and retain references to updatePosition, endDrag, and handleKeydown to ensure correct teardown.src/composables/node/useNodePreviewAndDrag.test.ts (1)
8-16: Avoid module-scope mutable mocks; usevi.hoisted.Hoisting keeps mocks contained and prevents leakage across tests.
🔧 Suggested change
-const mockStartDrag = vi.fn() -const mockHandleNativeDrop = vi.fn() +const { mockStartDrag, mockHandleNativeDrop } = vi.hoisted(() => ({ + mockStartDrag: vi.fn(), + mockHandleNativeDrop: vi.fn() +}))Based on learnings: Keep module mocks contained; do not use global mutable state within test files; use vi.hoisted() if necessary for per-test Arrange phase manipulation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePreviewAndDrag.test.ts` around lines 8 - 16, The module-scope mutable mocks mockStartDrag and mockHandleNativeDrop leak across tests; replace their top-level declarations with vi.hoisted(() => vi.fn()) so each test can arrange/reset them safely and use them inside the vi.mock for useNodeDragToCanvas (so startDrag and handleNativeDrop reference the hoisted fns), and ensure you reset or reassign those hoisted mocks in beforeEach/afterEach as needed to avoid cross-test state.src/components/queue/job/JobContextMenu.test.ts (1)
24-36: Prefer function declarations for test helpers.Suggested refactor
-const createEntries = (): MenuEntry[] => [ - { key: 'enabled', label: 'Enabled action', onClick: vi.fn() }, - { - key: 'disabled', - label: 'Disabled action', - disabled: true, - onClick: vi.fn() - }, - { kind: 'divider', key: 'divider-1' } -] +function createEntries(): MenuEntry[] { + return [ + { key: 'enabled', label: 'Enabled action', onClick: vi.fn() }, + { + key: 'disabled', + label: 'Disabled action', + disabled: true, + onClick: vi.fn() + }, + { kind: 'divider', key: 'divider-1' } + ] +} -const mountComponent = (entries: MenuEntry[]) => - mount(JobContextMenu, { - props: { entries }, - global: { - stubs: { - Popover: { - template: '<div class="popover-stub"><slot /></div>' - }, - Button: buttonStub - } - } - }) +function mountComponent(entries: MenuEntry[]) { + return mount(JobContextMenu, { + props: { entries }, + global: { + stubs: { + Popover: { + template: '<div class="popover-stub"><slot /></div>' + }, + Button: buttonStub + } + } + }) +}As per coding guidelines: Do not use function expressions if it's possible to use function declarations instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/queue/job/JobContextMenu.test.ts` around lines 24 - 36, The test file uses arrow function expressions for helper utilities; replace the createEntries and mountComponent arrow functions with equivalent function declarations (e.g., function createEntries(): MenuEntry[] { ... } and function mountComponent(entries: MenuEntry[]) { ... }) so they follow the coding guideline against function expressions; update any local references to these helpers but keep their names (createEntries, mountComponent) and usage with the JobContextMenu mount unchanged.src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts (1)
136-146: Multiple tick flushes may be fragile.The sequence
await nextTick(); await flushPromises(); await nextTick()on lines 138-140 suggests complex async timing requirements. While this works, it may be fragile if the component's internal timing changes. Consider documenting why this sequence is needed or exploring if the component can settle with fewer flushes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/EssentialNodesPanel.test.ts` around lines 136 - 146, The test "should expand all folders by default when expandedKeys is empty" relies on a fragile sequence of awaits (nextTick, flushPromises, nextTick); either simplify to a single stable await (prefer using flushPromises() once or await wrapper.vm.$nextTick()) so the component settles deterministically, or add a short comment above the sequence explaining why both nextTick and flushPromises are required; update the test around mountComponent(createMockRoot(), []), nextTick, and flushPromises usage accordingly and keep assertions against wrapper.findAll('.collapsible-root') unchanged.src/components/common/TreeExplorerV2Node.test.ts (1)
33-53: Consider usingsatisfiesfor type-safe mock construction.The
createMockItemhelper usesas RenderedTreeExplorerNode<ComfyNodeDefImpl>type assertion. Per codebase conventions, consider usingsatisfiesfor better type safety when constructing mock objects, though this works correctly for the current tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2Node.test.ts` around lines 33 - 53, createMockItem currently uses a type assertion ("as RenderedTreeExplorerNode<ComfyNodeDefImpl>") which bypasses excess property checks; replace that assertion with the TypeScript satisfies operator to ensure the object literal is checked against RenderedTreeExplorerNode<ComfyNodeDefImpl> while preserving its inferred narrower type (e.g. change the value declaration to use "satisfies RenderedTreeExplorerNode<ComfyNodeDefImpl>" instead of "as ..."); keep the returned FlattenedItem shape and other fields intact and ensure the file compiles with the project's TS version that supports satisfies.src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue (2)
97-97: Usecnfrom@/utils/tailwindUtilper repo guideline.The repo standardizes on the local
cnhelper to ensure consistent tailwind-merge behavior. As per coding guidelines “Usecn()utility from@/utils/tailwindUtilto merge class names; never use:class="[]"syntax.”♻️ Suggested refactor
-import { cn } from '@comfyorg/tailwind-utils' +import { cn } from '@/utils/tailwindUtil'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` at line 97, Replace the external cn import with the repo-standard local helper: change the import of cn in NodeLibrarySidebarTabV2.vue from '@comfyorg/tailwind-utils' to the local '@/utils/tailwindUtil' and update any class bindings in the component to use the cn(...) utility (instead of array-style :class="[...]") so tailwind-merge behavior is consistent; search for usages like cn(...) or :class="[" in this file and swap array bindings to cn calls where appropriate.
14-19: Swap the raw icon button for the shared Button component.Use the shared Button component for consistency with the design system and behavior. Based on learnings “In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue. Import and use it with appropriate variants (e.g., variant="link") to align with the design system. Apply this pattern across Vue components under src/components, ensuring consistent styling and behavior instead of ad-hoc button markup.”
♻️ Suggested refactor
+import Button from '@/components/ui/button/Button.vue' @@ - <button + <Button :aria-label="$t('g.sort')" class="flex size-10 shrink-0 cursor-pointer items-center justify-center rounded-lg bg-comfy-input hover:bg-comfy-input-hover border-none" > <i class="icon-[lucide--arrow-up-down] size-4" /> - </button> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 14 - 19, In NodeLibrarySidebarTabV2.vue replace the raw <button> element (the aria-label="$t('g.sort')" button with the <i class="icon-[lucide--arrow-up-down]">) with the shared Button component from src/components/ui/button/Button.vue: import Button and render it where the button was, passing aria-label="$t('g.sort')" and set the visual props (e.g., variant="link" or the equivalent props used across the codebase) and classes (size-10, rounded-lg, bg-comfy-input, hover:bg-comfy-input-hover, shrink-0, items-center, justify-center) so styling/behavior remain identical while removing the raw <button> markup and keeping the inner icon element intact. Ensure any event handlers or attributes on the original element are moved to the Button usage.src/components/searchbox/v2/NodeSearchCategorySidebar.vue (1)
5-30: Use the shared Button component for category items.The preset/source category actions should use the shared Button component for design-system consistency instead of raw
<button>tags. Based on learnings “In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue. Import and use it with appropriate variants (e.g., variant="link") to align with the design system. Apply this pattern across Vue components under src/components, ensuring consistent styling and behavior instead of ad-hoc button markup.”♻️ Suggested refactor
<script setup lang="ts"> -import { computed, ref } from 'vue' +import { computed, ref } from 'vue' +import Button from '@/components/ui/button/Button.vue'- <button + <Button v-for="preset in topCategories" :key="preset.id" type="button" :data-testid="`category-${preset.id}`" :aria-current="selectedCategory === preset.id || undefined" :class="categoryBtnClass(preset.id)" `@click`="selectCategory(preset.id)" > {{ preset.label }} - </button> + </Button>- <button + <Button v-for="preset in sourceCategories" :key="preset.id" type="button" :data-testid="`category-${preset.id}`" :aria-current="selectedCategory === preset.id || undefined" :class="categoryBtnClass(preset.id)" `@click`="selectCategory(preset.id)" > {{ preset.label }} - </button> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/searchbox/v2/NodeSearchCategorySidebar.vue` around lines 5 - 30, Top and source category list items currently render raw <button> elements; replace them with the shared Button component to ensure design-system consistency. Import the Button component from src/components/ui/button/Button.vue into NodeSearchCategorySidebar.vue, replace the v-for buttons for topCategories and sourceCategories with <Button> using variant="link" (or appropriate variant), keep the same :key, :data-testid, :aria-current binding, :class bound to categoryBtnClass(preset.id), and the `@click`="selectCategory(preset.id)" handler, and ensure the label stays as the slot/content so selectedCategory, categoryBtnClass, and selectCategory continue to work unchanged.src/composables/node/useNodePricing.test.ts (1)
1017-1033: Prefersatisfies ComfyNodeDefover type assertions for mock defs.This keeps type checking strict without forcing assertions. Based on learnings “In test files matching **/*.test.ts under src, when creating test helper functions that construct mock objects implementing an interface (e.g., AssetItem), prefer using satisfies InterfaceType for shape validation instead of type assertions like as Partial as InterfaceType or as any.”
♻️ Suggested refactor
- const createMockNodeDef = ( - overrides: Partial<ComfyNodeDef> = {} - ): ComfyNodeDef => - ({ - name: 'TestNode', - display_name: 'Test Node', - description: '', - category: 'test', - input: { required: {}, optional: {} }, - output: [], - output_name: [], - output_is_list: [], - python_module: 'test', - ...overrides - }) as ComfyNodeDef + const createMockNodeDef = ( + overrides: Partial<ComfyNodeDef> = {} + ): ComfyNodeDef => { + const nodeDef = { + name: 'TestNode', + display_name: 'Test Node', + description: '', + category: 'test', + input: { required: {}, optional: {} }, + output: [], + output_name: [], + output_is_list: [], + python_module: 'test', + ...overrides + } satisfies ComfyNodeDef + return nodeDef + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePricing.test.ts` around lines 1017 - 1033, The mock factory createMockNodeDef currently ends with a type assertion ("as ComfyNodeDef"); replace that assertion with a TypeScript "satisfies ComfyNodeDef" usage so the object literal is type-checked for the ComfyNodeDef shape without forcing a cast. Locate the createMockNodeDef function in this test and change the return expression to use "satisfies ComfyNodeDef" (preserving overrides: Partial<ComfyNodeDef> param) so invalid or missing properties are caught by the compiler while still allowing easy overrides in tests.src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue (1)
36-76: Avoid destructuringdefinePropsto preserve reactivity.Destructuring
fillNodeInfocan break reactivity if the prop ever changes; prefer a props object (ortoRefs) and referenceprops.fillNodeInfo. Based on learnings “In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity; prefer accessing props by name in the template. If you need to use props in the script, reference them via the defined props object rather than destructuring, or use toRefs when you intend to destructure while preserving reactivity.”♻️ Suggested refactor
-const { fillNodeInfo } = defineProps<{ +const props = defineProps<{ sections: NodeLibrarySection[] fillNodeInfo: (node: TreeNode) => RenderedTreeExplorerNode<ComfyNodeDefImpl> }>() @@ -const favoritesRoot = computed(() => - fillNodeInfo(nodeBookmarkStore.bookmarkedRoot) -) +const favoritesRoot = computed(() => + props.fillNodeInfo(nodeBookmarkStore.bookmarkedRoot) +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/tabs/nodeLibrary/AllNodesPanel.vue` around lines 36 - 76, The code destructures defineProps into fillNodeInfo which can break reactivity; change to capture the props object instead (e.g., const props = defineProps<...>()) and replace all uses of fillNodeInfo with props.fillNodeInfo (update favoritesRoot computed and any other references), or use toRefs if you need to destructure while preserving reactivity; ensure handleAddToFavorites, hasFavorites, and any other places reference props.fillNodeInfo so the prop remains reactive.src/components/common/TreeExplorerV2Node.vue (1)
62-62: Consider extracting the ref callback to avoid inline type assertion.The inline ref callback with type assertion works but could be cleaner:
:ref="(el) => (previewRef = el as HTMLElement)"This pattern is functional but the
as HTMLElementcast is needed because template refs can benull. Consider if this is intentional when the teleport condition is false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/TreeExplorerV2Node.vue` at line 62, Replace the inline ref callback that assigns previewRef with a named method to avoid the inline type assertion; create a function (e.g., setPreviewRef(el: HTMLElement | null)) that sets previewRef = el and use :ref="setPreviewRef" in the template, and ensure the function handles null when the teleport condition is false so no non-null assertion/cast is needed.src/composables/node/useNodePricing.ts (1)
499-504: Consider logging errors in catch block for debugging.The error is silently suppressed to avoid retry-spam, but this loses valuable debugging context. Consider adding a debug-level log:
🔧 Suggested improvement
- .catch(() => { + .catch((err) => { + if (import.meta.env.DEV) { + console.debug('[pricing/jsonata] evaluation failed:', err) + } // Cache empty to avoid retry-spam for same signature if (desiredSig.get(node) === sig) { cache.set(node, { sig, label: '' }) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/node/useNodePricing.ts` around lines 499 - 504, The catch block that currently swallows errors when fetching pricing (the block referencing desiredSig, cache, node, sig) should record the error at debug level before setting the empty-cache entry to avoid retry spam; update the .catch handler to accept the error parameter and call the module/logger debug method (include node and sig in the message for context) and then preserve the existing behavior of setting cache.set(node, { sig, label: '' }) only when desiredSig.get(node) === sig.
Comment on lines
+44
to
+50
| if (filled) { | ||
| return { | ||
| borderColor: borderStyle, | ||
| backgroundColor: `${borderStyle}33`, | ||
| color: borderStyle | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Potential issue: Filled state assumes hex color format for borderStyle.
The expression ${borderStyle}33 appends 33 (20% alpha in hex) directly to the borderStyle value. This works for hex colors like #f59e0b → #f59e0b33, but will produce invalid CSS for:
- CSS variables:
var(--color)33 - Named colors:
red33 - RGB/HSL:
rgb(255,0,0)33
Consider using CSS color-mix() or a helper function to handle different color formats, or document that borderStyle must be a hex color when filled is true.
💡 Alternative using CSS color-mix (if browser support permits)
if (filled) {
return {
borderColor: borderStyle,
- backgroundColor: `${borderStyle}33`,
+ backgroundColor: `color-mix(in srgb, ${borderStyle} 20%, transparent)`,
color: borderStyle
}
}📝 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.
Suggested change
| if (filled) { | |
| return { | |
| borderColor: borderStyle, | |
| backgroundColor: `${borderStyle}33`, | |
| color: borderStyle | |
| } | |
| } | |
| if (filled) { | |
| return { | |
| borderColor: borderStyle, | |
| backgroundColor: `color-mix(in srgb, ${borderStyle} 20%, transparent)`, | |
| color: borderStyle | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/BadgePill.vue` around lines 44 - 50, The filled branch
currently constructs backgroundColor by naively appending "33" to borderStyle
(in the block guarded by filled returning borderColor/backgroundColor/color),
which only works for hex colors; change this to detect hex via a simple regex
and append "33" for hex values, otherwise generate a proper translucent variant
using CSS color-mix (e.g., color-mix(in srgb, <borderStyle> 20%, transparent))
or convert rgb()/hsl() to an rgba()/hsla() equivalent as a fallback; update the
backgroundColor assignment in the filled return to use this helper logic so
borderStyle remains the source value for color and borderColor.
Comment on lines
+3
to
+4
| <TreeRoot | ||
| :expanded="[...expandedKeys]" |
Contributor
There was a problem hiding this comment.
Avoid defensive array copy for TreeRoot expanded binding.
Line 4 uses :expanded="[...expandedKeys]" which creates a defensive copy. Per established patterns for Reka UI Tree components, this breaks the reactivity contract and can cause state desynchronization.
Use v-model:expanded instead to let TreeRoot manage the controlled component pattern correctly.
🔧 Proposed fix
<ContextMenuRoot>
<TreeRoot
- :expanded="[...expandedKeys]"
+ v-model:expanded="expandedKeys"
:items="root.children ?? []"Based on learnings: "In TreeExplorerV2.vue and similar Reka UI Tree components, avoid defensive copies in bindings like :expanded='[...expandedKeys]'. Use v-model:expanded='expandedKeys' instead."
📝 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.
Suggested change
| <TreeRoot | |
| :expanded="[...expandedKeys]" | |
| <ContextMenuRoot> | |
| <TreeRoot | |
| v-model:expanded="expandedKeys" | |
| :items="root.children ?? []" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/TreeExplorerV2.vue` around lines 3 - 4, The binding for
TreeRoot currently uses a defensive copy (:expanded="[...expandedKeys]") which
breaks the Tree component's reactivity; change the binding to use the controlled
pattern by replacing the defensive copy with v-model:expanded bound to the
original state (use v-model:expanded="expandedKeys" on the TreeRoot component)
so TreeRoot can manage expansion state correctly and keep
reactivity/synchronization intact.
Comment on lines
+1
to
+5
| import { mount } from '@vue/test-utils' | ||
| import type { FlattenedItem } from 'reka-ui' | ||
| import { ref } from 'vue' | ||
| import { describe, expect, it, vi } from 'vitest' | ||
|
|
Contributor
There was a problem hiding this comment.
Missing beforeEach import from Vitest.
beforeEach is used on line 232 but is not included in the import statement on line 4.
🐛 Proposed fix
-import { describe, expect, it, vi } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'🧰 Tools
🪛 ESLint
[error] 1-1: Unable to resolve path to module '@vue/test-utils'.
(import-x/no-unresolved)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/common/TreeExplorerV2Node.test.ts` around lines 1 - 5, The
test file is missing the beforeEach import from Vitest used later; update the
top import from 'vitest' (where describe, expect, it, vi are imported) to also
include beforeEach so the test harness can run the setup block in
TreeExplorerV2Node.test.ts (refer to the beforeEach usage around line 232) — add
beforeEach to the named imports from 'vitest'.
| fallbackWarn: false | ||
| }) | ||
|
|
||
| describe('ConfirmationDialogContent', () => { |
Contributor
There was a problem hiding this comment.
Use component-based describe to satisfy the test rule.
Suggested fix
-describe('ConfirmationDialogContent', () => {
+describe(ConfirmationDialogContent, () => {Based on learnings: In test files under src/**/*.test.ts, follow the vitest/prefer-describe-function-title rule by using describe(ComponentOrFunction, ...) instead of a string literal description when naming test suites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/dialog/content/ConfirmationDialogContent.test.ts` at line 20,
The test suite uses a string literal in describe; change it to a component-based
title by replacing describe('ConfirmationDialogContent', ...) with
describe(ConfirmationDialogContent, ...) (or
describe(ConfirmationDialogContent.name, ...) if the symbol is not imported
directly) so the vitest/prefer-describe-function-title rule is satisfied; update
the import to reference ConfirmationDialogContent if needed and keep the inner
tests unchanged.
Comment on lines
+25
to
+40
| watch( | ||
| () => nodeDef.name, | ||
| (name) => { | ||
| if (!nodeDef.api_node) { | ||
| priceLabel.value = '' | ||
| return | ||
| } | ||
| const capturedName = name | ||
| evaluateNodeDefPricing(nodeDef) | ||
| .then((label) => { | ||
| if (nodeDef.name === capturedName) priceLabel.value = label | ||
| }) | ||
| .catch((e) => { | ||
| console.error('[NodePricingBadge] pricing evaluation failed:', e) | ||
| }) | ||
| }, |
Contributor
There was a problem hiding this comment.
Clear stale pricing label before async evaluation.
When switching between API nodes, the old price label can remain visible until the new evaluation resolves. Resetting the label at the start avoids showing stale pricing.
🛠️ Proposed fix
(name) => {
if (!nodeDef.api_node) {
priceLabel.value = ''
return
}
+ priceLabel.value = ''
const capturedName = name
evaluateNodeDefPricing(nodeDef)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/node/NodePricingBadge.vue` around lines 25 - 40, The watch
handler for nodeDef.name leaves the previous priceLabel visible while
evaluateNodeDefPricing runs; update the watcher (the function watching () =>
nodeDef.name) to immediately clear priceLabel.value (set to empty string) before
calling evaluateNodeDefPricing when nodeDef.api_node is truthy so stale labels
aren't shown, then proceed with the existing
evaluateNodeDefPricing(...).then(...).catch(...) logic that sets
priceLabel.value if nodeDef.name matches the capturedName.
Comment on lines
+1
to
+19
| <template> | ||
| <div class="flex items-center gap-2 px-2 py-1.5"> | ||
| <button | ||
| v-for="chip in chips" | ||
| :key="chip.key" | ||
| type="button" | ||
| :aria-pressed="activeChipKey === chip.key" | ||
| :class=" | ||
| cn( | ||
| 'cursor-pointer rounded-md border px-3 py-1 text-sm transition-colors flex-auto border-secondary-background', | ||
| activeChipKey === chip.key | ||
| ? 'bg-secondary-background text-foreground' | ||
| : 'bg-transparent text-muted-foreground hover:border-base-foreground/60 hover:text-base-foreground/60' | ||
| ) | ||
| " | ||
| @click="emit('selectChip', chip)" | ||
| > | ||
| {{ chip.label }} | ||
| </button> |
Contributor
There was a problem hiding this comment.
Use the shared Button component instead of raw <button>.
Suggested fix
- <div class="flex items-center gap-2 px-2 py-1.5">
- <button
+ <div class="flex items-center gap-2 px-2 py-1.5">
+ <Button
v-for="chip in chips"
:key="chip.key"
type="button"
:aria-pressed="activeChipKey === chip.key"
:class="
cn(
'cursor-pointer rounded-md border px-3 py-1 text-sm transition-colors flex-auto border-secondary-background',
activeChipKey === chip.key
? 'bg-secondary-background text-foreground'
: 'bg-transparent text-muted-foreground hover:border-base-foreground/60 hover:text-base-foreground/60'
)
"
`@click`="emit('selectChip', chip)"
>
{{ chip.label }}
- </button>
+ </Button>
</div> import { computed } from 'vue'
import { useI18n } from 'vue-i18n'
+import Button from '@/components/ui/button/Button.vue'
import { useNodeDefStore } from '@/stores/nodeDefStore'
import { cn } from '@/utils/tailwindUtil'Based on learnings: In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component located at src/components/ui/button/Button.vue.
📝 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.
Suggested change
| <template> | |
| <div class="flex items-center gap-2 px-2 py-1.5"> | |
| <button | |
| v-for="chip in chips" | |
| :key="chip.key" | |
| type="button" | |
| :aria-pressed="activeChipKey === chip.key" | |
| :class=" | |
| cn( | |
| 'cursor-pointer rounded-md border px-3 py-1 text-sm transition-colors flex-auto border-secondary-background', | |
| activeChipKey === chip.key | |
| ? 'bg-secondary-background text-foreground' | |
| : 'bg-transparent text-muted-foreground hover:border-base-foreground/60 hover:text-base-foreground/60' | |
| ) | |
| " | |
| @click="emit('selectChip', chip)" | |
| > | |
| {{ chip.label }} | |
| </button> | |
| <template> | |
| <div class="flex items-center gap-2 px-2 py-1.5"> | |
| <Button | |
| v-for="chip in chips" | |
| :key="chip.key" | |
| type="button" | |
| :aria-pressed="activeChipKey === chip.key" | |
| :class=" | |
| cn( | |
| 'cursor-pointer rounded-md border px-3 py-1 text-sm transition-colors flex-auto border-secondary-background', | |
| activeChipKey === chip.key | |
| ? 'bg-secondary-background text-foreground' | |
| : 'bg-transparent text-muted-foreground hover:border-base-foreground/60 hover:text-base-foreground/60' | |
| ) | |
| " | |
| `@click`="emit('selectChip', chip)" | |
| > | |
| {{ chip.label }} | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/searchbox/v2/NodeSearchFilterBar.vue` around lines 1 - 19,
Replace the raw <button> elements in NodeSearchFilterBar.vue with the shared
Button component: import and register the Button from
src/components/ui/button/Button.vue, then in the template swap the <button
v-for="chip in chips" ...> block to use <Button> (keep the v-for and
:key="chip.key"), forward attributes like type="button",
:aria-pressed="activeChipKey === chip.key", the dynamic :class binding, and the
`@click` handler to emit('selectChip', chip); ensure the Button receives the same
visual classes and accessibility attributes so chips and activeChipKey logic
continue to work.
Comment on lines
+6
to
+10
| <SearchBox | ||
| ref="searchBoxRef" | ||
| v-model="searchQuery" | ||
| :placeholder="$t('g.search') + '...'" | ||
| @search="handleSearch" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "NodeLibrarySidebarTabV2.vue" -type f 2>/dev/null | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
cat /tmp/repo-check.txt 2>/dev/null || echo "Repo path needed"
cd /tmp/repo && head -30 src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue 2>/dev/null || echo "File not found at expected path"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 195
🏁 Script executed:
# First, find the file
find . -type f -name "NodeLibrarySidebarTabV2.vue" 2>/dev/nullRepository: Comfy-Org/ComfyUI_frontend
Length of output: 127
🏁 Script executed:
# Check locale files structure
find . -type f -name "main.json" -o -name "*.json" | grep -i locale | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 367
🏁 Script executed:
# Search for search-related i18n keys
rg "g\.search" --type json -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for placeholder patterns in the codebase
rg "placeholder.*\$t|:\$t.*placeholder" -g "*.vue" -A 1 -B 1 | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Read the specific file around lines 6-10
head -20 ./src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue | cat -nRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1057
🏁 Script executed:
# Search for g.search in locale files
rg "g\.search" ./src/locales/en/main.json -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check if g.searchPlaceholder exists
grep -n "searchPlaceholder\|search.*\.\.\." ./src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend
Length of output: 360
🏁 Script executed:
# Look at the structure of g section in main.json
sed -n '/"g":/,/^ },$/p' ./src/locales/en/main.json | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1891
🏁 Script executed:
# Search for similar placeholder patterns with i18n in Vue files
rg "placeholder.*\+" ./src/components --type vue -B 2 -A 2 | head -40Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Check if g.search exists anywhere in main.json
grep -n '"search"' ./src/locales/en/main.json | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Look for the complete 'g' section more precisely
grep -n '"g":' ./src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend
Length of output: 80
🏁 Script executed:
# Get lines around the g section definition
sed -n '1,100p' ./src/locales/en/main.json | tail -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3126
🏁 Script executed:
# Search for where 'g' section ends to see all its keys
awk '/"g": \{/,/^ \}/' ./src/locales/en/main.json | tail -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2084
🏁 Script executed:
# Look at other Vue files using SearchBox or similar patterns to see the common approach
rg "SearchBox" ./src/components --type vue -A 5 | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Check if g.searchPlaceholder is already defined in main.json
grep -n '"searchPlaceholder"' ./src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend
Length of output: 360
🏁 Script executed:
# Count how many keys are in the g section to see if searchPlaceholder is there
awk '/"g": \{/,/^ \}/' ./src/locales/en/main.json | grep -c "search"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 71
🏁 Script executed:
# See the exact end of the g section
awk '/"g": \{/,/^ \}/' ./src/locales/en/main.json | tail -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 645
🏁 Script executed:
# Check if other SearchBox components in the codebase use similar concatenation
find ./src/components -name "*.vue" -exec grep -l "SearchBox" {} \; | head -5 | xargs -I {} grep -A 5 "SearchBox" {}Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3834
🏁 Script executed:
# Count usage of literal string concatenation with $t in placeholders across Vue files
rg '\$t\([^)]+\)\s*\+' ./src/components -g "*.vue" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 177
Use a dedicated i18n key for the search placeholder instead of concatenating a literal.
The placeholder concatenates a literal '...' with $t('g.search'), which violates the guideline that all user-facing strings must come from i18n. This prevents translations from adjusting punctuation and spacing appropriately. Add a g.searchPlaceholder key to src/locales/en/main.json with value "Search..." and reference it directly.
♻️ Suggested refactor
- :placeholder="$t('g.search') + '...'"
+ :placeholder="$t('g.searchPlaceholder')"Then add to src/locales/en/main.json in the g section:
"searchPlaceholder": "Search..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/tabs/NodeLibrarySidebarTabV2.vue` around lines 6 - 10,
Replace the concatenated placeholder in the SearchBox component with a dedicated
i18n key: add "searchPlaceholder": "Search..." under the "g" section in
src/locales/en/main.json, then change the placeholder prop on the SearchBox (the
component referenced by ref="searchBoxRef" and bound to searchQuery) to use
$t('g.searchPlaceholder') instead of $t('g.search') + '...'; ensure any other
usages of the concatenation are updated to use the new key.
Comment on lines
+871
to
+1223
| describe('formatPricingResult', () => { | ||
| describe('type: usd', () => { | ||
| it('should format usd result', () => { | ||
| const result = formatPricingResult({ type: 'usd', usd: 0.05 }) | ||
| expect(result).toBe('10.6 credits/Run') | ||
| }) | ||
|
|
||
| it('should return valueOnly format', () => { | ||
| const result = formatPricingResult( | ||
| { type: 'usd', usd: 0.05 }, | ||
| { valueOnly: true } | ||
| ) | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should handle approximate prefix in valueOnly mode', () => { | ||
| const result = formatPricingResult( | ||
| { type: 'usd', usd: 0.05, format: { approximate: true } }, | ||
| { valueOnly: true } | ||
| ) | ||
| expect(result).toBe('~10.6') | ||
| }) | ||
|
|
||
| it('should return empty for null usd', () => { | ||
| const result = formatPricingResult({ type: 'usd', usd: null as never }) | ||
| expect(result).toBe('') | ||
| }) | ||
| }) | ||
|
|
||
| describe('type: range_usd', () => { | ||
| it('should format range result', () => { | ||
| const result = formatPricingResult({ | ||
| type: 'range_usd', | ||
| min_usd: 0.05, | ||
| max_usd: 0.1 | ||
| }) | ||
| expect(result).toBe('10.6-21.1 credits/Run') | ||
| }) | ||
|
|
||
| it('should return valueOnly format', () => { | ||
| const result = formatPricingResult( | ||
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.1 }, | ||
| { valueOnly: true } | ||
| ) | ||
| expect(result).toBe('10.6-21.1') | ||
| }) | ||
|
|
||
| it('should collapse range when min equals max', () => { | ||
| const result = formatPricingResult( | ||
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.05 }, | ||
| { valueOnly: true } | ||
| ) | ||
| expect(result).toBe('10.6') | ||
| }) | ||
| }) | ||
|
|
||
| describe('type: list_usd', () => { | ||
| it('should format list result', () => { | ||
| const result = formatPricingResult({ | ||
| type: 'list_usd', | ||
| usd: [0.05, 0.1, 0.15] | ||
| }) | ||
| expect(result).toMatch(/\d+\.?\d*\/\d+\.?\d*\/\d+\.?\d* credits\/Run/) | ||
| }) | ||
|
|
||
| it('should return valueOnly format', () => { | ||
| const result = formatPricingResult( | ||
| { type: 'list_usd', usd: [0.05, 0.1] }, | ||
| { valueOnly: true } | ||
| ) | ||
| expect(result).toBe('10.6/21.1') | ||
| }) | ||
| }) | ||
|
|
||
| describe('type: text', () => { | ||
| it('should return text as-is', () => { | ||
| const result = formatPricingResult({ type: 'text', text: 'Free' }) | ||
| expect(result).toBe('Free') | ||
| }) | ||
| }) | ||
|
|
||
| describe('legacy format', () => { | ||
| it('should handle {usd: number} without type field', () => { | ||
| const result = formatPricingResult({ usd: 0.05 }) | ||
| expect(result).toBe('10.6 credits/Run') | ||
| }) | ||
|
|
||
| it('should return valueOnly for legacy format', () => { | ||
| const result = formatPricingResult({ usd: 0.05 }, { valueOnly: true }) | ||
| expect(result).toBe('10.6') | ||
| }) | ||
| }) | ||
|
|
||
| describe('invalid inputs', () => { | ||
| it('should return empty for invalid type', () => { | ||
| const result = formatPricingResult({ type: 'invalid' }) | ||
| expect(result).toBe('') | ||
| }) | ||
|
|
||
| it('should return empty for null', () => { | ||
| const result = formatPricingResult(null) | ||
| expect(result).toBe('') | ||
| }) | ||
|
|
||
| it('should return empty for undefined', () => { | ||
| const result = formatPricingResult(undefined) | ||
| expect(result).toBe('') | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // formatCreditsValue / Range / List Tests | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| describe('formatCreditsValue', () => { | ||
| it('should format USD to credits', () => { | ||
| expect(formatCreditsValue(0.05)).toBe('10.6') | ||
| expect(formatCreditsValue(1.0)).toBe('211') | ||
| }) | ||
| }) | ||
|
|
||
| describe('formatCreditsRangeValue', () => { | ||
| it('should format min-max range', () => { | ||
| expect(formatCreditsRangeValue(0.05, 0.1)).toBe('10.6-21.1') | ||
| }) | ||
|
|
||
| it('should collapse when min equals max', () => { | ||
| expect(formatCreditsRangeValue(0.05, 0.05)).toBe('10.6') | ||
| }) | ||
| }) | ||
|
|
||
| describe('formatCreditsListValue', () => { | ||
| it('should join values with separator', () => { | ||
| expect(formatCreditsListValue([0.05, 0.1])).toBe('10.6/21.1') | ||
| }) | ||
|
|
||
| it('should use custom separator', () => { | ||
| expect(formatCreditsListValue([0.05, 0.1], ' | ')).toBe('10.6 | 21.1') | ||
| }) | ||
| }) | ||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // evaluateNodeDefPricing Tests | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| describe('evaluateNodeDefPricing', () => { | ||
| const createMockNodeDef = ( | ||
| overrides: Partial<ComfyNodeDef> = {} | ||
| ): ComfyNodeDef => | ||
| ({ | ||
| name: 'TestNode', | ||
| display_name: 'Test Node', | ||
| description: '', | ||
| category: 'test', | ||
| input: { required: {}, optional: {} }, | ||
| output: [], | ||
| output_name: [], | ||
| output_is_list: [], | ||
| python_module: 'test', | ||
| ...overrides | ||
| }) as ComfyNodeDef | ||
|
|
||
| it('should return empty for node without price_badge', async () => { | ||
| const nodeDef = createMockNodeDef() | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('') | ||
| }) | ||
|
|
||
| it('should evaluate static expression', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'StaticPriceNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"usd","usd":0.05}', | ||
| depends_on: { widgets: [], inputs: [], input_groups: [] } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should use default value from input spec', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'DefaultValueNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"usd","usd": widgets.count * 0.01}', | ||
| depends_on: { | ||
| widgets: [{ name: 'count', type: 'INT' }], | ||
| inputs: [], | ||
| input_groups: [] | ||
| } | ||
| }, | ||
| input: { | ||
| required: { | ||
| count: ['INT', { default: 10 }] | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('21.1') // 10 * 0.01 = 0.1 USD = 21.1 credits | ||
| }) | ||
|
|
||
| it('should use first option for COMBO without default', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'ComboNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '(widgets.mode = "pro") ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | ||
| depends_on: { | ||
| widgets: [{ name: 'mode', type: 'COMBO' }], | ||
| inputs: [], | ||
| input_groups: [] | ||
| } | ||
| }, | ||
| input: { | ||
| required: { | ||
| mode: [['standard', 'pro'], {}] | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| // First option is "standard", not "pro", so should be 0.05 USD | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should use "original" as fallback for dynamic COMBO without input', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'DynamicComboNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: `( | ||
| $prices := {"original": 0.05, "720p": 0.03}; | ||
| {"type":"usd","usd": $lookup($prices, widgets.resolution)} | ||
| )`, | ||
| depends_on: { | ||
| widgets: [{ name: 'resolution', type: 'COMBO' }], | ||
| inputs: [], | ||
| input_groups: [] | ||
| } | ||
| }, | ||
| input: { | ||
| required: { | ||
| // resolution widget is NOT in inputs (dynamically created) | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| // Fallback to "original" = 0.05 USD | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should handle dynamic combo with options array', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'DynamicOptionsNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"usd","usd": widgets.model = "model_a" ? 0.05 : 0.10}', | ||
| depends_on: { | ||
| widgets: [{ name: 'model', type: 'COMFY_DYNAMICCOMBO_V3' }], | ||
| inputs: [], | ||
| input_groups: [] | ||
| } | ||
| }, | ||
| input: { | ||
| required: { | ||
| model: [ | ||
| 'COMFY_DYNAMICCOMBO_V3', | ||
| { options: [{ key: 'model_a' }, { key: 'model_b' }] } | ||
| ] | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| // First option key is "model_a" = 0.05 USD | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should assume inputs disconnected in preview', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'InputConnectedNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: 'inputs.image.connected ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | ||
| depends_on: { | ||
| widgets: [], | ||
| inputs: ['image'], | ||
| input_groups: [] | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| // In preview, inputs are assumed disconnected | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should assume inputGroups have 0 count in preview', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'InputGroupNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"usd","usd": 0.05 + inputGroups.videos * 0.02}', | ||
| depends_on: { | ||
| widgets: [], | ||
| inputs: [], | ||
| input_groups: ['videos'] | ||
| } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| // 0.05 + 0 * 0.02 = 0.05 USD | ||
| expect(result).toBe('10.6') | ||
| }) | ||
|
|
||
| it('should return empty on JSONata error', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'ErrorNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '$lookup(undefined, "key")', | ||
| depends_on: { widgets: [], inputs: [], input_groups: [] } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('') | ||
| }) | ||
|
|
||
| it('should handle range_usd result', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'RangeNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"range_usd","min_usd":0.05,"max_usd":0.10}', | ||
| depends_on: { widgets: [], inputs: [], input_groups: [] } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('10.6-21.1') | ||
| }) | ||
|
|
||
| it('should handle approximate format in valueOnly mode', async () => { | ||
| const nodeDef = createMockNodeDef({ | ||
| name: 'ApproximateNode', | ||
| price_badge: { | ||
| engine: 'jsonata', | ||
| expr: '{"type":"usd","usd":0.05,"format":{"approximate":true}}', | ||
| depends_on: { widgets: [], inputs: [], input_groups: [] } | ||
| } | ||
| }) | ||
| const result = await evaluateNodeDefPricing(nodeDef) | ||
| expect(result).toBe('~10.6') | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual test file to see imports and function availability
head -100 src/composables/node/useNodePricing.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2718
🏁 Script executed:
# Check if vitest/prefer-describe-function-title is actually configured/enforced
find . -name ".eslintrc*" -o -name "eslint.config.*" -o -name ".oxlintrc*" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1580
🏁 Script executed:
# Search for oxlint or ESLint configuration in the repo
cat .oxlintrc.json 2>/dev/null || cat eslint.config.ts 2>/dev/null || echo "No config found"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4012
🏁 Script executed:
# Check if other test files in src/ use function references in describe()
rg "describe\([a-zA-Z_][a-zA-Z0-9_]*,\s*\(\)" src --type ts -A 1 | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 972
Use function references in describe() titles.
The functions are already imported and available in scope. Use describe(formatPricingResult, ...), describe(formatCreditsValue, ...), and similar for the other functions to follow the repo's vitest convention and align with the pattern used elsewhere in the codebase.
♻️ Suggested refactor
-describe('formatPricingResult', () => {
+describe(formatPricingResult, () => {
@@
-describe('formatCreditsValue', () => {
+describe(formatCreditsValue, () => {
@@
-describe('formatCreditsRangeValue', () => {
+describe(formatCreditsRangeValue, () => {
@@
-describe('formatCreditsListValue', () => {
+describe(formatCreditsListValue, () => {
@@
-describe('evaluateNodeDefPricing', () => {
+describe(evaluateNodeDefPricing, () => {📝 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.
Suggested change
| describe('formatPricingResult', () => { | |
| describe('type: usd', () => { | |
| it('should format usd result', () => { | |
| const result = formatPricingResult({ type: 'usd', usd: 0.05 }) | |
| expect(result).toBe('10.6 credits/Run') | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'usd', usd: 0.05 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should handle approximate prefix in valueOnly mode', () => { | |
| const result = formatPricingResult( | |
| { type: 'usd', usd: 0.05, format: { approximate: true } }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('~10.6') | |
| }) | |
| it('should return empty for null usd', () => { | |
| const result = formatPricingResult({ type: 'usd', usd: null as never }) | |
| expect(result).toBe('') | |
| }) | |
| }) | |
| describe('type: range_usd', () => { | |
| it('should format range result', () => { | |
| const result = formatPricingResult({ | |
| type: 'range_usd', | |
| min_usd: 0.05, | |
| max_usd: 0.1 | |
| }) | |
| expect(result).toBe('10.6-21.1 credits/Run') | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.1 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6-21.1') | |
| }) | |
| it('should collapse range when min equals max', () => { | |
| const result = formatPricingResult( | |
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.05 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6') | |
| }) | |
| }) | |
| describe('type: list_usd', () => { | |
| it('should format list result', () => { | |
| const result = formatPricingResult({ | |
| type: 'list_usd', | |
| usd: [0.05, 0.1, 0.15] | |
| }) | |
| expect(result).toMatch(/\d+\.?\d*\/\d+\.?\d*\/\d+\.?\d* credits\/Run/) | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'list_usd', usd: [0.05, 0.1] }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6/21.1') | |
| }) | |
| }) | |
| describe('type: text', () => { | |
| it('should return text as-is', () => { | |
| const result = formatPricingResult({ type: 'text', text: 'Free' }) | |
| expect(result).toBe('Free') | |
| }) | |
| }) | |
| describe('legacy format', () => { | |
| it('should handle {usd: number} without type field', () => { | |
| const result = formatPricingResult({ usd: 0.05 }) | |
| expect(result).toBe('10.6 credits/Run') | |
| }) | |
| it('should return valueOnly for legacy format', () => { | |
| const result = formatPricingResult({ usd: 0.05 }, { valueOnly: true }) | |
| expect(result).toBe('10.6') | |
| }) | |
| }) | |
| describe('invalid inputs', () => { | |
| it('should return empty for invalid type', () => { | |
| const result = formatPricingResult({ type: 'invalid' }) | |
| expect(result).toBe('') | |
| }) | |
| it('should return empty for null', () => { | |
| const result = formatPricingResult(null) | |
| expect(result).toBe('') | |
| }) | |
| it('should return empty for undefined', () => { | |
| const result = formatPricingResult(undefined) | |
| expect(result).toBe('') | |
| }) | |
| }) | |
| }) | |
| // ----------------------------------------------------------------------------- | |
| // formatCreditsValue / Range / List Tests | |
| // ----------------------------------------------------------------------------- | |
| describe('formatCreditsValue', () => { | |
| it('should format USD to credits', () => { | |
| expect(formatCreditsValue(0.05)).toBe('10.6') | |
| expect(formatCreditsValue(1.0)).toBe('211') | |
| }) | |
| }) | |
| describe('formatCreditsRangeValue', () => { | |
| it('should format min-max range', () => { | |
| expect(formatCreditsRangeValue(0.05, 0.1)).toBe('10.6-21.1') | |
| }) | |
| it('should collapse when min equals max', () => { | |
| expect(formatCreditsRangeValue(0.05, 0.05)).toBe('10.6') | |
| }) | |
| }) | |
| describe('formatCreditsListValue', () => { | |
| it('should join values with separator', () => { | |
| expect(formatCreditsListValue([0.05, 0.1])).toBe('10.6/21.1') | |
| }) | |
| it('should use custom separator', () => { | |
| expect(formatCreditsListValue([0.05, 0.1], ' | ')).toBe('10.6 | 21.1') | |
| }) | |
| }) | |
| // ----------------------------------------------------------------------------- | |
| // evaluateNodeDefPricing Tests | |
| // ----------------------------------------------------------------------------- | |
| describe('evaluateNodeDefPricing', () => { | |
| const createMockNodeDef = ( | |
| overrides: Partial<ComfyNodeDef> = {} | |
| ): ComfyNodeDef => | |
| ({ | |
| name: 'TestNode', | |
| display_name: 'Test Node', | |
| description: '', | |
| category: 'test', | |
| input: { required: {}, optional: {} }, | |
| output: [], | |
| output_name: [], | |
| output_is_list: [], | |
| python_module: 'test', | |
| ...overrides | |
| }) as ComfyNodeDef | |
| it('should return empty for node without price_badge', async () => { | |
| const nodeDef = createMockNodeDef() | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('') | |
| }) | |
| it('should evaluate static expression', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'StaticPriceNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd":0.05}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should use default value from input spec', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DefaultValueNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": widgets.count * 0.01}', | |
| depends_on: { | |
| widgets: [{ name: 'count', type: 'INT' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| count: ['INT', { default: 10 }] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('21.1') // 10 * 0.01 = 0.1 USD = 21.1 credits | |
| }) | |
| it('should use first option for COMBO without default', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ComboNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '(widgets.mode = "pro") ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | |
| depends_on: { | |
| widgets: [{ name: 'mode', type: 'COMBO' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| mode: [['standard', 'pro'], {}] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // First option is "standard", not "pro", so should be 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should use "original" as fallback for dynamic COMBO without input', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DynamicComboNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: `( | |
| $prices := {"original": 0.05, "720p": 0.03}; | |
| {"type":"usd","usd": $lookup($prices, widgets.resolution)} | |
| )`, | |
| depends_on: { | |
| widgets: [{ name: 'resolution', type: 'COMBO' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| // resolution widget is NOT in inputs (dynamically created) | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // Fallback to "original" = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should handle dynamic combo with options array', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DynamicOptionsNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": widgets.model = "model_a" ? 0.05 : 0.10}', | |
| depends_on: { | |
| widgets: [{ name: 'model', type: 'COMFY_DYNAMICCOMBO_V3' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| model: [ | |
| 'COMFY_DYNAMICCOMBO_V3', | |
| { options: [{ key: 'model_a' }, { key: 'model_b' }] } | |
| ] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // First option key is "model_a" = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should assume inputs disconnected in preview', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'InputConnectedNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: 'inputs.image.connected ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | |
| depends_on: { | |
| widgets: [], | |
| inputs: ['image'], | |
| input_groups: [] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // In preview, inputs are assumed disconnected | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should assume inputGroups have 0 count in preview', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'InputGroupNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": 0.05 + inputGroups.videos * 0.02}', | |
| depends_on: { | |
| widgets: [], | |
| inputs: [], | |
| input_groups: ['videos'] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // 0.05 + 0 * 0.02 = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should return empty on JSONata error', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ErrorNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '$lookup(undefined, "key")', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('') | |
| }) | |
| it('should handle range_usd result', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'RangeNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"range_usd","min_usd":0.05,"max_usd":0.10}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('10.6-21.1') | |
| }) | |
| it('should handle approximate format in valueOnly mode', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ApproximateNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd":0.05,"format":{"approximate":true}}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('~10.6') | |
| }) | |
| describe(formatPricingResult, () => { | |
| describe('type: usd', () => { | |
| it('should format usd result', () => { | |
| const result = formatPricingResult({ type: 'usd', usd: 0.05 }) | |
| expect(result).toBe('10.6 credits/Run') | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'usd', usd: 0.05 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should handle approximate prefix in valueOnly mode', () => { | |
| const result = formatPricingResult( | |
| { type: 'usd', usd: 0.05, format: { approximate: true } }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('~10.6') | |
| }) | |
| it('should return empty for null usd', () => { | |
| const result = formatPricingResult({ type: 'usd', usd: null as never }) | |
| expect(result).toBe('') | |
| }) | |
| }) | |
| describe('type: range_usd', () => { | |
| it('should format range result', () => { | |
| const result = formatPricingResult({ | |
| type: 'range_usd', | |
| min_usd: 0.05, | |
| max_usd: 0.1 | |
| }) | |
| expect(result).toBe('10.6-21.1 credits/Run') | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.1 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6-21.1') | |
| }) | |
| it('should collapse range when min equals max', () => { | |
| const result = formatPricingResult( | |
| { type: 'range_usd', min_usd: 0.05, max_usd: 0.05 }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6') | |
| }) | |
| }) | |
| describe('type: list_usd', () => { | |
| it('should format list result', () => { | |
| const result = formatPricingResult({ | |
| type: 'list_usd', | |
| usd: [0.05, 0.1, 0.15] | |
| }) | |
| expect(result).toMatch(/\d+\.?\d*\/\d+\.?\d*\/\d+\.?\d* credits\/Run/) | |
| }) | |
| it('should return valueOnly format', () => { | |
| const result = formatPricingResult( | |
| { type: 'list_usd', usd: [0.05, 0.1] }, | |
| { valueOnly: true } | |
| ) | |
| expect(result).toBe('10.6/21.1') | |
| }) | |
| }) | |
| describe('type: text', () => { | |
| it('should return text as-is', () => { | |
| const result = formatPricingResult({ type: 'text', text: 'Free' }) | |
| expect(result).toBe('Free') | |
| }) | |
| }) | |
| describe('legacy format', () => { | |
| it('should handle {usd: number} without type field', () => { | |
| const result = formatPricingResult({ usd: 0.05 }) | |
| expect(result).toBe('10.6 credits/Run') | |
| }) | |
| it('should return valueOnly for legacy format', () => { | |
| const result = formatPricingResult({ usd: 0.05 }, { valueOnly: true }) | |
| expect(result).toBe('10.6') | |
| }) | |
| }) | |
| describe('invalid inputs', () => { | |
| it('should return empty for invalid type', () => { | |
| const result = formatPricingResult({ type: 'invalid' }) | |
| expect(result).toBe('') | |
| }) | |
| it('should return empty for null', () => { | |
| const result = formatPricingResult(null) | |
| expect(result).toBe('') | |
| }) | |
| it('should return empty for undefined', () => { | |
| const result = formatPricingResult(undefined) | |
| expect(result).toBe('') | |
| }) | |
| }) | |
| }) | |
| // ----------------------------------------------------------------------------- | |
| // formatCreditsValue / Range / List Tests | |
| // ----------------------------------------------------------------------------- | |
| describe(formatCreditsValue, () => { | |
| it('should format USD to credits', () => { | |
| expect(formatCreditsValue(0.05)).toBe('10.6') | |
| expect(formatCreditsValue(1.0)).toBe('211') | |
| }) | |
| }) | |
| describe(formatCreditsRangeValue, () => { | |
| it('should format min-max range', () => { | |
| expect(formatCreditsRangeValue(0.05, 0.1)).toBe('10.6-21.1') | |
| }) | |
| it('should collapse when min equals max', () => { | |
| expect(formatCreditsRangeValue(0.05, 0.05)).toBe('10.6') | |
| }) | |
| }) | |
| describe(formatCreditsListValue, () => { | |
| it('should join values with separator', () => { | |
| expect(formatCreditsListValue([0.05, 0.1])).toBe('10.6/21.1') | |
| }) | |
| it('should use custom separator', () => { | |
| expect(formatCreditsListValue([0.05, 0.1], ' | ')).toBe('10.6 | 21.1') | |
| }) | |
| }) | |
| // ----------------------------------------------------------------------------- | |
| // evaluateNodeDefPricing Tests | |
| // ----------------------------------------------------------------------------- | |
| describe(evaluateNodeDefPricing, () => { | |
| const createMockNodeDef = ( | |
| overrides: Partial<ComfyNodeDef> = {} | |
| ): ComfyNodeDef => | |
| ({ | |
| name: 'TestNode', | |
| display_name: 'Test Node', | |
| description: '', | |
| category: 'test', | |
| input: { required: {}, optional: {} }, | |
| output: [], | |
| output_name: [], | |
| output_is_list: [], | |
| python_module: 'test', | |
| ...overrides | |
| }) as ComfyNodeDef | |
| it('should return empty for node without price_badge', async () => { | |
| const nodeDef = createMockNodeDef() | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('') | |
| }) | |
| it('should evaluate static expression', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'StaticPriceNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd":0.05}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should use default value from input spec', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DefaultValueNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": widgets.count * 0.01}', | |
| depends_on: { | |
| widgets: [{ name: 'count', type: 'INT' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| count: ['INT', { default: 10 }] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('21.1') // 10 * 0.01 = 0.1 USD = 21.1 credits | |
| }) | |
| it('should use first option for COMBO without default', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ComboNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '(widgets.mode = "pro") ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | |
| depends_on: { | |
| widgets: [{ name: 'mode', type: 'COMBO' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| mode: [['standard', 'pro'], {}] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // First option is "standard", not "pro", so should be 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should use "original" as fallback for dynamic COMBO without input', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DynamicComboNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: `( | |
| $prices := {"original": 0.05, "720p": 0.03}; | |
| {"type":"usd","usd": $lookup($prices, widgets.resolution)} | |
| )`, | |
| depends_on: { | |
| widgets: [{ name: 'resolution', type: 'COMBO' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| // resolution widget is NOT in inputs (dynamically created) | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // Fallback to "original" = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should handle dynamic combo with options array', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'DynamicOptionsNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": widgets.model = "model_a" ? 0.05 : 0.10}', | |
| depends_on: { | |
| widgets: [{ name: 'model', type: 'COMFY_DYNAMICCOMBO_V3' }], | |
| inputs: [], | |
| input_groups: [] | |
| } | |
| }, | |
| input: { | |
| required: { | |
| model: [ | |
| 'COMFY_DYNAMICCOMBO_V3', | |
| { options: [{ key: 'model_a' }, { key: 'model_b' }] } | |
| ] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // First option key is "model_a" = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should assume inputs disconnected in preview', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'InputConnectedNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: 'inputs.image.connected ? {"type":"usd","usd":0.10} : {"type":"usd","usd":0.05}', | |
| depends_on: { | |
| widgets: [], | |
| inputs: ['image'], | |
| input_groups: [] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // In preview, inputs are assumed disconnected | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should assume inputGroups have 0 count in preview', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'InputGroupNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd": 0.05 + inputGroups.videos * 0.02}', | |
| depends_on: { | |
| widgets: [], | |
| inputs: [], | |
| input_groups: ['videos'] | |
| } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| // 0.05 + 0 * 0.02 = 0.05 USD | |
| expect(result).toBe('10.6') | |
| }) | |
| it('should return empty on JSONata error', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ErrorNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '$lookup(undefined, "key")', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('') | |
| }) | |
| it('should handle range_usd result', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'RangeNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"range_usd","min_usd":0.05,"max_usd":0.10}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('10.6-21.1') | |
| }) | |
| it('should handle approximate format in valueOnly mode', async () => { | |
| const nodeDef = createMockNodeDef({ | |
| name: 'ApproximateNode', | |
| price_badge: { | |
| engine: 'jsonata', | |
| expr: '{"type":"usd","usd":0.05,"format":{"approximate":true}}', | |
| depends_on: { widgets: [], inputs: [], input_groups: [] } | |
| } | |
| }) | |
| const result = await evaluateNodeDefPricing(nodeDef) | |
| expect(result).toBe('~10.6') | |
| }) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/composables/node/useNodePricing.test.ts` around lines 871 - 1223, The
describe blocks use string titles instead of function references; update each
top-level describe to pass the actual function identifier (e.g.,
describe(formatPricingResult, ...), describe(formatCreditsValue, ...),
describe(formatCreditsRangeValue, ...), describe(formatCreditsListValue, ...),
and describe(evaluateNodeDefPricing, ...)) so tests follow the repo's vitest
convention—replace the string names with the corresponding imported function
references while keeping the inner test bodies unchanged.
Comment on lines
+745
to
+749
| } else if (dep.type.toUpperCase() === 'COMBO') { | ||
| // For dynamic COMBO widgets without input spec, use a common default | ||
| // that works with most pricing expressions (e.g., resolution selectors) | ||
| rawValue = 'original' | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the file and understand the context around lines 745-749
sed -n '730,760p' src/composables/node/useNodePricing.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1340
🏁 Script executed:
# Search for COMBO-related code and pricing expressions
rg -n "COMBO" src/ --type ts --type vue -B 2 -A 2 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Search for 'original' references in pricing context
rg -n "'original'" src/ --type ts --type vue -B 2 -A 2 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Look for pricing widget configurations and COMBO options
rg -n "price_badge\|combo\|rawValue" src/ --type json -B 2 -A 5 | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Find the normalizeWidgetValue function to understand what happens with 'original'
rg -n "normalizeWidgetValue" src/ --type ts -B 3 -A 10 | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3946
🏁 Script executed:
# Search for COMBO widget definitions and valid options
rg -n "type.*COMBO\|COMBO.*type" src/ --type ts -B 2 -A 5 | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Look for pricing expressions that might depend on COMBO values
rg -n "depends_on\|priceBadge\|price_badge" src/ --type ts -B 2 -A 3 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check if there are test files related to useNodePricing
fd "useNodePricing\|pricing" --type f -e test -e spec | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for 'original' in the entire codebase to understand its usage context
rg -n "'original'" src/ --type ts -B 3 -A 1 | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5946
🏁 Script executed:
# Read the complete normalizeWidgetValue function
sed -n '201,240p' src/composables/node/useNodePricing.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1096
🏁 Script executed:
# Look for pricing rule definitions and COMBO widget dependencies
rg -n "depends_on" src/composables/node/useNodePricing.ts -B 5 -A 5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5290
🏁 Script executed:
# Search for widget type handling specifically for COMBO
rg -n "COMBO\|combo" src/composables/node/useNodePricing.ts -i -B 3 -A 3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Look at broader context - find where pricing rules are defined or loaded
rg -n "priceBadge\|price_badge" src/ --type ts -B 3 -A 10 | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for COMBO widget validation or handling anywhere in the codebase
rg -n "COMBO" src/ --type ts -i | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 6423
🏁 Script executed:
# Find ComboInputSpec definition
rg -n "ComboInputSpec\|type ComboInputSpec" src/ --type ts -B 2 -A 5 | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Look for COMBO widget options/values in actual implementations
rg -n "values\|options" src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts -B 2 -A 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for dynamic COMBO widgets (without predefined options)
rg -n "dynamic.*COMBO\|COMBO.*dynamic" src/ --type ts -i -B 3 -A 3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check extractDefaultFromSpec to understand how defaults are extracted
rg -n "extractDefaultFromSpec" src/composables/node/useNodePricing.ts -B 3 -A 15Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1632
🏁 Script executed:
# Look for actual dynamic COMBO examples in the codebase
rg -n "\"options\".*\[" src/composables/node/useNodePricing.ts -B 5 -A 10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for nodes that have COMBO dependencies in pricing rules
rg -n "depends_on.*widgets" src/ --type ts -A 5 | grep -A 5 COMBO | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 487
🏁 Script executed:
# Look for what happens when a COMBO value is not found in options
rg -n "validate\|check\|error" src/composables/node/useNodePricing.ts | head -40Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Search for examples of resolution selector or similar COMBO widgets
rg -n "resolution\|selector" src/ --type ts -i -B 3 -A 3 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Let's look at the full context of the pricing preview calculation
sed -n '720,800p' src/composables/node/useNodePricing.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2218
🏁 Script executed:
# Look at the test file to see if there are test cases for COMBO fallback
cat -n src/composables/node/useNodePricing.test.ts | head -200Repository: Comfy-Org/ComfyUI_frontend
Length of output: 7309
🏁 Script executed:
# Search for any validation or error cases related to COMBO or pricing
rg -n "test\|describe" src/composables/node/useNodePricing.test.ts -A 10 | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check if there are any examples of COMBO widgets with 'original' option
rg -n "'original'" src/ --type ts -B 5 -A 2 | grep -E "COMBO|combo|widget|option" -i -B 3 -A 3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3175
🏁 Script executed:
# Check if there are any error cases in tests related to invalid COMBO values
sed -n '200,400p' src/composables/node/useNodePricing.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 7050
🏁 Script executed:
# Look for any nodes that actually use dynamic COMBO with pricing
rg -n "price_badge.*depends_on" src/ --type ts -B 3 -A 5 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check if 'original' is ever used as a COMBO option in actual node definitions
rg -n "\"original\"" src/ --type ts -B 2 -A 2 | grep -i "combo\|option\|widget" -B 2 -A 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 911
🏁 Script executed:
# Read the complete test for dynamic COMBO fallback
sed -n '1098,1120p' src/composables/node/useNodePricing.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 808
🏁 Script executed:
# Check if there are any other tests that might contradict or expand on this
sed -n '1120,1150p' src/composables/node/useNodePricing.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 983
🏁 Script executed:
# Check if there are any error cases or edge cases in the tests related to this fallback
sed -n '1125,1200p' src/composables/node/useNodePricing.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2323
🏁 Script executed:
# Check the error handling and logging for pricing evaluation failures
sed -n '765,775p' src/composables/node/useNodePricing.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 430
🏁 Script executed:
# Search for nodes that use dynamic COMBO with pricing but might not have 'original' option
rg -n "price_badge" src/ --type ts -B 5 -A 15 | grep -E "COMBO|original|dynamic" -i -B 5 -A 5 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5692
🏁 Script executed:
# Check if there are any comments or documentation about this fallback limitation
rg -n "TODO\|FIXME\|BUG\|HACK" src/composables/node/useNodePricing.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
The 'original' fallback for dynamic COMBO widgets is intentionally designed but could fail silently if pricing expressions don't include it.
While there is a test confirming this pattern works for resolution selectors (lines 1098-1120 of useNodePricing.test.ts), the assumption that 'original' will exist in every pricing expression's lookup table is fragile. If a COMBO widget's pricing expression doesn't include 'original' as a valid key, the JSONata $lookup would fail, triggering the error handler that silently returns an empty string instead of a price.
Consider adding validation to ensure COMBO values exist in the pricing expression before evaluation, or documenting this assumption more explicitly for node definition authors who write pricing expressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/composables/node/useNodePricing.ts` around lines 745 - 749, The current
COMBO branch sets rawValue = 'original' when dep.type === 'COMBO', which can
silently fail if the pricing expression's lookup table doesn't include
'original'; update the logic that handles dep.type/COMBO (the block assigning
rawValue) to validate the chosen key against the pricing expression lookup
before evaluation: inspect the pricing expression's keys (or pre-evaluate the
lookup table) to confirm 'original' exists, and if it does not, either pick a
safe fallback (e.g., the first available lookup key) or log/error and skip
pricing for that node instead of relying on the try/catch that returns an empty
string; ensure any added branches reference dep.type, rawValue, and the
pricing-evaluation path so tests can be adjusted accordingly.
Comment on lines
+2175
to
+2180
| "outputs": { | ||
| "1": { | ||
| "name": "ÍNDICE", | ||
| "tooltip": null | ||
| } | ||
| ] | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pt="src/locales/pt-BR/nodeDefs.json"
en="src/locales/en-US/nodeDefs.json"
if [[ ! -f "$pt" || ! -f "$en" ]]; then
echo "Missing locale file(s): $pt or $en"
exit 1
fi
python3 - <<'PY'
import json
pt_path = "src/locales/pt-BR/nodeDefs.json"
en_path = "src/locales/en-US/nodeDefs.json"
nodes = ["CustomCombo", "Load3D"]
with open(pt_path, encoding="utf-8") as f:
pt = json.load(f)
with open(en_path, encoding="utf-8") as f:
en = json.load(f)
for node in nodes:
print(f"\n{node}")
print("pt-BR outputs:", json.dumps(pt.get(node, {}).get("outputs"), indent=2))
print("en-US outputs:", json.dumps(en.get(node, {}).get("outputs"), indent=2))
PYRepository: Comfy-Org/ComfyUI_frontend
Length of output: 160
🏁 Script executed:
find . -type f -name "nodeDefs.json" 2>/dev/null | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 447
🏁 Script executed:
find . -type d -name "locales" 2>/dev/nullRepository: Comfy-Org/ComfyUI_frontend
Length of output: 578
🏁 Script executed:
python3 - <<'PY'
import json
pt_path = "./src/locales/pt-BR/nodeDefs.json"
en_path = "./src/locales/en/nodeDefs.json"
nodes = ["CustomCombo", "Load3D"]
with open(pt_path, encoding="utf-8") as f:
pt = json.load(f)
with open(en_path, encoding="utf-8") as f:
en = json.load(f)
for node in nodes:
print(f"\n{'='*60}")
print(f"Node: {node}")
print(f"{'='*60}")
pt_outputs = pt.get(node, {}).get("outputs", {})
en_outputs = en.get(node, {}).get("outputs", {})
print(f"\npt-BR outputs keys: {sorted(pt_outputs.keys())}")
print(f"en outputs keys: {sorted(en_outputs.keys())}")
print(f"\npt-BR outputs:")
print(json.dumps(pt_outputs, indent=2))
print(f"\nen outputs:")
print(json.dumps(en_outputs, indent=2))
PYRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1173
Add missing output definitions to pt-BR locale for CustomCombo and Load3D.
Sparse output indices in pt-BR do not align with the canonical en locale definitions:
- CustomCombo: pt-BR defines only output
"1", but en defines outputs"0"and"1". The pt-BR locale is missing the definition for output"0". - Load3D: pt-BR defines only output
"6", but en defines outputs"0"through"6". The pt-BR locale is missing definitions for outputs"0"through"5"(image, mask, mesh_path, normal, camera_info, recording_video).
UI labels for missing outputs will not render correctly in Portuguese (Brazil). Add these output definitions to the pt-BR locale to match the canonical ordering, or verify this sparseness is intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/locales/pt-BR/nodeDefs.json` around lines 2175 - 2180, The pt-BR
nodeDefs.json is missing several output entries compared to the canonical en
locale for the CustomCombo and Load3D nodes; update the "outputs" objects for
those nodes to include the full set of keys that en provides (for CustomCombo
add "0" alongside existing "1"; for Load3D add "0" through "5" alongside
existing "6") using appropriate Portuguese labels/tooltip values (e.g., image,
mask, mesh_path, normal, camera_info, recording_video translated or placeholder
null tooltips) so the output indices and ordering match the canonical en locale.
huntcsg
force-pushed
the
free-tier-initial
branch
from
February 21, 2026 19:18
5b7609c to
2f7f330
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/locales/en/main.json (1)
2015-2016: Minor inconsistency in "Free Tier" capitalization.Line 2016 uses "Free Tier" (both words capitalized) while line 1999 uses "Free Tier" in the key name but "free credits" in the description. Consider whether "Free tier" or "Free Tier" should be the standard capitalization across all user-facing strings for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/locales/en/main.json` around lines 2015 - 2016, Standardize the capitalization of "Free Tier" across user-facing strings: update the value for the "emailNotEligibleForFreeTier" key (and any other locale entries referencing the free tier) to use the agreed form ("Free Tier" or "free tier") consistently; search for keys like "emailNotEligibleForFreeTier" and "personalDataConsentLabel" to locate occurrences and adjust the string values so all messages use the same capitalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/locales/en/main.json`:
- Around line 2015-2016: Standardize the capitalization of "Free Tier" across
user-facing strings: update the value for the "emailNotEligibleForFreeTier" key
(and any other locale entries referencing the free tier) to use the agreed form
("Free Tier" or "free tier") consistently; search for keys like
"emailNotEligibleForFreeTier" and "personalDataConsentLabel" to locate
occurrences and adjust the string values so all messages use the same
capitalization.
Comment on lines
+22
to
+24
| telemetry?.trackUiButtonClicked({ | ||
| button_id: `${source}_use_email_instead` | ||
| }) |
Contributor
There was a problem hiding this comment.
FYI this telemetry event is currently disabled due to it making us exceed our Mixpanel quota heavily. It can be re-enabled in the dynamic config by name though.
Contributor
Author
There was a problem hiding this comment.
I think we can gather the metrics other ways (just see increase in proportion of signups with Google), and enrich the user with their auth provider (if it doesn't already exist).
I'll drop the additional telemetry I think.
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Nit: The current structure for the 3-way dialog routing feels a bit awkward — show() was the original entry point, but now the PR renames it to showPricingTable, creates a new show that conditionally calls it, and then leaks showPricingTable to callers who re-implement the free-tier branching logic themselves (e.g. line 465 in SubscriptionPanelContentWorkspace.vue: isFreeTierPlan.value ? showPricingTable() : showSubscriptionDialog()). This defeats the purpose of the composable owning the routing.
Suggestion — single show() with a flag:
function show(options?: {
reason?: SubscriptionDialogReason
skipFreeTierDialog?: boolean // for "Upgrade" button in panel
}) {
if (isFreeTier.value && !options?.skipFreeTierDialog) {
showFreeTierDialog(options)
return
}
showPricingTable(options) // private, not exported
}
return { show, hide } // no showPricingTable leakThen SubscriptionPanelContentWorkspace.vue line 465 simplifies to show({ skipFreeTierDialog: true }), and all routing logic stays inside the composable.
Comment on lines
+11
to
+17
| const showFreeTierBadge = (() => { | ||
| try { | ||
| return !localStorage.getItem(HAS_ACCOUNT_KEY) | ||
| } catch { | ||
| return false | ||
| } | ||
| })() |
Contributor
There was a problem hiding this comment.
Consider something like:
import { useLocalStorage } from '@vueuse/core'
export function useFreeTierOnboarding(source: 'login' | 'signup') {
const hasAccount = useLocalStorage(HAS_ACCOUNT_KEY, null)
const showFreeTierBadge = computed(() => !hasAccount.value)
// ...
}
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
PR Review: Free Tier Support
Overall this is a solid PR with clear separation of concerns. The free tier routing, telemetry additions, and pricing table updates are well-structured. A few issues worth addressing below, ranging from dead code to a potential bug in the legacy panel.
Login/auth semantics: Verified — error handling, isInChina gating, and redirect/routing logic are all preserved correctly. The isInChina check now only appears in the email-form branch, which is correct since the restriction only applies to email signup (OAuth was always available to China users even before this PR).
Snake_case usage: Confirmed consistent with existing telemetry conventions — all Mixpanel property names in this codebase use snake_case (is_new_user, user_id, button_id, etc). The new free_tier_badge_shown, SubscriptionDialogReason values, and button_id strings all follow the established pattern correctly.
Telemetry: The new trackLoginOpened follows the exact same dispatch pattern as the existing trackSignupOpened. Implementation is consistent.
| } catch { | ||
| return false | ||
| } | ||
| })() |
Contributor
There was a problem hiding this comment.
Should use useLocalStorage from VueUse instead of raw localStorage.getItem(). The codebase consistently uses useLocalStorage in ~10 other places (apiKeyAuthStore, ComfyActionbar, NodeLibrarySidebarTab, TopMenuSection, etc.).
Current approach has two issues:
- Not reactive — if the user logs in during the session (which sets
HAS_ACCOUNT_KEYinfirebaseAuthStore.ts),showFreeTierBadgeremains stale since it's evaluated once at mount via IIFE. - Manual try/catch —
useLocalStoragehandles this automatically.
Suggested:
import { useLocalStorage } from '@vueuse/core'
const hasAccount = useLocalStorage(HAS_ACCOUNT_KEY, null)
const showFreeTierBadge = computed(() => !hasAccount.value)This also makes it reactive if the user logs in during the same session.
| utm_source?: string | ||
| utm_medium?: string | ||
| utm_campaign?: string | ||
| free_tier_badge_shown?: boolean |
Contributor
There was a problem hiding this comment.
free_tier_badge_shown was added to AuthMetadata but no trackAuth() call ever passes it. All four trackAuth() calls in firebaseAuthStore.ts only pass method, is_new_user, and user_id. This field is dead code in AuthMetadata — either wire it up in the auth tracking calls or remove it from this interface.
(The same field in AuthPageOpenedMetadata is correctly used by trackSignupOpened and trackLoginOpened.)
Contributor
Author
There was a problem hiding this comment.
I am adding a commit that re-works the telemetry for this feature.
Basically we want to track:
- how seeing the badge affects signup
- free tier -> subscribe
but within free tier -> subscribe there's three flows:
- ran out of credits -> modal CTA -> subscribe
- attempted to add credits -> modal CTA -> subscribe
- was managing plan and clicked upgrade
We'll track those properly now.
| import { useTelemetry } from '@/platform/telemetry' | ||
| import { HAS_ACCOUNT_KEY } from '@/stores/firebaseAuthStore' | ||
|
|
||
| export function useFreeTierOnboarding(source: 'login' | 'signup') { |
Contributor
There was a problem hiding this comment.
Missing tests. This new composable contains business logic (badge visibility based on localStorage, form toggling, telemetry tracking) but has no unit tests. Similarly, FreeTierDialogContent.vue is a new component with conditional rendering logic (reason-based title/subtitle switching) and no tests.
At minimum, useFreeTierOnboarding should have tests covering:
showFreeTierBadgereturnstruewhenHAS_ACCOUNT_KEYis absentshowFreeTierBadgereturnsfalsewhenHAS_ACCOUNT_KEYis setswitchToEmailForm/switchToSocialLogintoggleshowEmailFormand fire telemetry with correctbutton_id
|
|
||
| const { formattedRenewalDate } = useSubscription() | ||
|
|
||
| const freeTierCredits = computed(() => remoteConfig.value.free_tier_credits) |
Contributor
There was a problem hiding this comment.
Nit: freeTierCredits is computed independently in three places.
- Here:
computed(() => remoteConfig.value.free_tier_credits) useFreeTierOnboarding.ts:10: same expressiongetTierCredits('free')intierPricing.ts:69: readsremoteConfig.value.free_tier_credits ?? null
Consider using getTierCredits('free') here (it already exists for this purpose) to avoid the three-way duplication. Or expose it from useSubscription as a shared computed.
| isInsufficientCredits: true | ||
| }) | ||
| } | ||
| useDialogService().showTopUpCreditsDialog({ |
Contributor
There was a problem hiding this comment.
Behavioral change worth noting (not a bug): The old code silently swallowed Payment Required errors for users without an active subscription — the if (isActiveSubscription.value) guard meant non-subscribers saw nothing when hitting a payment wall. The new code always calls showTopUpCreditsDialog, which now redirects free-tier and non-subscribed users to the subscription required dialog.
This is better UX since free-tier users who exhaust credits now see a clear upgrade path instead of a silent no-op. Just confirming this behavioral change is intentional.
Contributor
Author
There was a problem hiding this comment.
I'm running this by Alex, but this is intentional in the new Free Tier context.
huntcsg
marked this pull request as ready for review
February 22, 2026 08:41
dosubot
Bot
added
the
size:XL
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
Comment on lines
+14
to
+21
| // users before HAS_ACCOUNT_KEY was introduced) | ||
| // Reactive via useLocalStorage so the badge hides if the user signs in | ||
| // during the current session. | ||
| const hasAccount = useLocalStorage<string | null>(HAS_ACCOUNT_KEY, null) | ||
| const previousWorkflow = useLocalStorage<string | null>( | ||
| 'Comfy.PreviousWorkflow', | ||
| null | ||
| ) |
Contributor
There was a problem hiding this comment.
we have a newUserService for detecting whether this is the first ever session.
huntcsg
force-pushed
the
free-tier-initial
branch
4 times, most recently
from
February 24, 2026 06:37
6bb1ff7 to
158e0e4
Compare
- Restructure login/signup pages: OAuth primary, progressive email disclosure - Add free tier badge on Google button, dynamic credit count from remote config - Add FREE subscription tier throughout type system and tier pricing - Track funnel events: login/signup opened with free_tier_badge_shown, email toggle clicks via trackUiButtonClicked - Disable top-up for free tier users (dialogService, purchaseCredits, popover) - Show subscription dialog instead of top-up on Payment Required for free tier - Show Upgrade button instead of Add Credits in user popover for free tier
The only caller (TopUpCreditsDialogContentLegacy) is already gated by showTopUpCreditsDialog, making the bottom-layer check unreachable. Also adds a doc comment to HAS_ACCOUNT_KEY explaining its purpose.
- Bump free tier description from text-xs to text-sm to match body text - Align header spacing (remove extra mt-6 from login) - Fix signup privacy link to use absolute URL matching login - Fix nested <p> in <div> — use sibling <p> elements for terms/contact - Keep badge at text-[10px] as intentional decorative size
Free tier users should use performSubscriptionCheckout (new subscription) instead of accessBillingPortal (Stripe upgrade), since they don't have an existing Stripe subscription to modify.
- Remove dead free_tier_badge_shown from AuthMetadata (never sent) - Remove disabled trackUiButtonClicked calls from useFreeTierOnboarding - Simplify getTierCredits to single signature (remove unused overloads) - Add isFreeTier guard to legacy panel Manage Subscription button - Enrich trackSubscription with current_tier and reason metadata to enable free-tier conversion funnel analysis
Replace IIFE with useLocalStorage for HAS_ACCOUNT_KEY and Comfy.PreviousWorkflow. Badge hides reactively if either key exists, covering both new deploys (existing users have PreviousWorkflow) and mid-session sign-ins (onAuthStateChanged sets HAS_ACCOUNT_KEY). Add unit tests for useFreeTierOnboarding composable.
- Change sign-up link to 'Sign up to try Cloud for free' - Remove free tier badge and teaser text from login page - Remove new-user detection (HAS_ACCOUNT_KEY, localStorage checks) - Make signup page badge/teaser unconditional (always shown) - Clean up useFreeTierOnboarding composable and telemetry types
New here? Sign up with a Gmail account to get 400 free credits every month. Only 'Sign up' is a hyperlink, rest is plain text.
…irst login - Add isFreeTier to BillingState interface and implement in both useLegacyBilling and useWorkspaceBilling so it reflects the active workspace's tier, not the user's personal tier. - Update dialogService and SubscriptionPanelContentWorkspace to use useBillingContext().isFreeTier instead of useSubscription().isFreeTier. - Guard FreeTierDialogContent in useSubscriptionDialog to only show in personal workspaces (free tier doesn't exist on team workspaces). - Fix balance race: re-fetch balance after parallel init when free tier credits were just lazily granted (balance === 0).
huntcsg
force-pushed
the
free-tier-initial
branch
from
February 25, 2026 04:03
56025bd to
c044392
Compare
huntcsg
added
cloud/1.40
needs-backport
christian-byrne
approved these changes
Feb 25, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Feb 25, 2026
## Summary Add frontend support for a Free subscription tier — login/signup page restructuring, telemetry instrumentation, and tier-aware billing gating. ## Changes - **What**: - Restructure login/signup pages: OAuth buttons promoted as primary sign-in method, email login available via progressive disclosure - Add Free tier badge on Google sign-up button with dynamic credit count from remote config - Add `FREE` subscription tier to type system (tier pricing, tier rank, registry types) - Add `isFreeTier` computed to `useSubscription()` - Disable credit top-up for Free tier users (dialogService, purchaseCredits, popover CTA) - Show subscription/upgrade dialog instead of top-up dialog when Free tier user hits out-of-credits - Add funnel telemetry: `trackLoginOpened`, enrich `trackSignupOpened` with `free_tier_badge_shown`, track email toggle clicks ## Review Focus - Tier gating logic: Free tier users should see "Upgrade" instead of "Add Credits" and never reach the top-up flow - Telemetry event design for Mixpanel funnel analysis - Progressive disclosure UX on login/signup pages ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8864-feat-add-Free-subscription-tier-support-3076d73d36508133b84ec5f0a67ccb03) by [Unito](https://www.unito.io)
Member
github-actions
Bot
removed
the
needs-backport
huntcsg
added a commit
that referenced
this pull request
Feb 25, 2026
Backport of #8864 to `cloud/1.40` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9190-backport-cloud-1-40-feat-add-Free-subscription-tier-support-3126d73d36508153b889ddc3c5d01f57) by [Unito](https://www.unito.io) Co-authored-by: Hunter <huntcsg@users.noreply.github.com>
dante01yoon
pushed a commit
that referenced
this pull request
Jul 27, 2026
## Summary Add frontend support for a Free subscription tier — login/signup page restructuring, telemetry instrumentation, and tier-aware billing gating. ## Changes - **What**: - Restructure login/signup pages: OAuth buttons promoted as primary sign-in method, email login available via progressive disclosure - Add Free tier badge on Google sign-up button with dynamic credit count from remote config - Add `FREE` subscription tier to type system (tier pricing, tier rank, registry types) - Add `isFreeTier` computed to `useSubscription()` - Disable credit top-up for Free tier users (dialogService, purchaseCredits, popover CTA) - Show subscription/upgrade dialog instead of top-up dialog when Free tier user hits out-of-credits - Add funnel telemetry: `trackLoginOpened`, enrich `trackSignupOpened` with `free_tier_badge_shown`, track email toggle clicks ## Review Focus - Tier gating logic: Free tier users should see "Upgrade" instead of "Add Credits" and never reach the top-up flow - Telemetry event design for Mixpanel funnel analysis - Progressive disclosure UX on login/signup pages ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8864-feat-add-Free-subscription-tier-support-3076d73d36508133b84ec5f0a67ccb03) by [Unito](https://www.unito.io)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add frontend support for a Free subscription tier — login/signup page restructuring, telemetry instrumentation, and tier-aware billing gating.
Changes
FREEsubscription tier to type system (tier pricing, tier rank, registry types)isFreeTiercomputed touseSubscription()trackLoginOpened, enrichtrackSignupOpenedwithfree_tier_badge_shown, track email toggle clicksReview Focus
┆Issue is synchronized with this Notion page by Unito