Move price badges to python nodes #7816
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughReplaces static node pricing with a JSONata-driven runtime pricing system: adds the jsonata dependency, introduces a price_badge schema, implements async compile/evaluate/caching in a new composable, and updates badge rendering to subscribe to pricing revisions and batch canvas redraws. Changes
Sequence DiagramssequenceDiagram
autonumber
participant Badge as Badge Renderer
participant Pricing as useNodePricing
participant Store as NodeDefStore
participant Cache as Compiled Rule Cache
participant Jsonata as JSONata Engine
participant UI as Canvas Renderer
Note over Badge,Pricing: getNodeDisplayPrice flow (fast-path vs async)
Badge->>Pricing: getNodeDisplayPrice(node)
alt cached label available
Pricing-->>Badge: return cached label
else
Pricing->>Store: fetch node definition (price_badge)
Store-->>Pricing: return price_badge
Pricing->>Cache: lookup compiled rule for node type
alt compiled rule cached
Cache-->>Pricing: compiled rule
else
Pricing->>Jsonata: compile rule.expr
Jsonata-->>Pricing: compiled rule
Pricing->>Cache: store compiled rule
end
Pricing->>Jsonata: evaluate rule with context (async)
Jsonata-->>Pricing: evaluation result
Pricing->>Pricing: format & cache display label
Pricing->>Badge: bump pricingRevision (reactive)
end
Badge->>UI: queue redraw (requestAnimationFrame)
UI->>UI: batch render on next frame
Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review full |
|
@DrJKL I'll perform a full review of all the changes in this PR. ✅ Actions performedFull review triggered. |
1 similar comment
|
@DrJKL I'll perform a full review of all the changes in this PR. ✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Fix all issues with AI Agents 🤖
In @src/composables/node/useNodePricing.ts:
- Around line 408-436: In getNodeDisplayPrice, the bare read pricingTick.value
is an intentional reactive dependency for Vue but looks like a no-op; add a
short inline comment immediately above or on the same line explaining that the
read is deliberate to trigger recomputation (e.g., "intentional reactive
dependency to bump computed re-evaluation; do not remove") so future readers
won't remove it; keep the comment concise and reference pricingTick.value and
the purpose of scheduling async evaluation via scheduleEvaluation.
- Around line 336-374: Add a timeout wrapper around the JSONata evaluation so
rule._compiled.evaluate(ctx) cannot hang indefinitely: create a withTimeout
helper that races the evaluation promise against a setTimeout rejection (e.g.,
1s or configurable), then replace the current
Promise.resolve(rule._compiled.evaluate(...)) call with withTimeout(...). Ensure
the resulting promise is still stored in inflight via inflight.set(node, { sig,
promise }) and that the existing .catch and .finally logic that updates cache,
desiredSig checks, and increments pricingTick.value handles timeout rejections
the same as other errors (i.e., log in development and cache an empty label for
the same sig).
- Around line 166-170: Remove the explicit any in the filter callback so
TypeScript infers the input type from node.inputs; update the connectedCount
computation to use a typed callback like node.inputs?.filter(inp =>
inp?.name?.startsWith(prefix) && inp?.link != null).length ?? 0 (or add a proper
Input type and annotate node.inputs as Input[] if needed) and leave g[groupName]
= connectedCount unchanged.
- Around line 151-160: The loop uses unsafe any types for widget/input lookups;
replace (x: any) with the actual types for node.widgets and node.inputs (e.g.,
Widget and Input or the project's defined interfaces) so the find callbacks and
subsequent access (x?.name, widget?.value, slot?.link) are type-checked; update
the variable declarations for widget and slot in the block referencing
rule.depends_on.widgets and rule.depends_on.inputs, ensure normalizeWidgetValue
accepts the Widget.value type, and adjust the i Record type if needed to reflect
the real Input shape rather than relying on any.
- Around line 255-263: compileRule currently forces the jsonata(...) result to
any which bypasses typings; change this to use jsonata's proper Expression type
(or create a local alias) and update CompiledJsonataPricingRule._compiled to
Expression | null; then replace the cast "as any" with a typed assignment like
const compiled: Expression = jsonata(rule.expr) and return { ...rule, _compiled:
compiled } so the code uses correct jsonata types without any casting while
keeping the catch path returning _compiled: null.
- Around line 453-472: Add a readonly price_badge?: PriceBadge property to the
ComfyNodeDefImpl class so price_badge is properly typed (remove the need for the
as any cast in getRelevantWidgetNames), and change the getRelevantWidgetNames
const arrow function into a named function declaration (function
getRelevantWidgetNames(nodeType: string): string[]) keeping the same body and
behavior; locate ComfyNodeDefImpl to add the property and replace the const
getRelevantWidgetNames = (...) => { ... } with a function declaration to match
codebase conventions.
In @src/schemas/nodeDefSchema.ts:
- Around line 222-228: The default value for the depends_on field on zPriceBadge
omits input_groups which can cause type mismatches with zPriceBadgeDepends;
update the default object used in zPriceBadge.depends_on to include
input_groups: [] (so the default becomes { widgets: [], inputs: [],
input_groups: [] }) and ensure the shape matches zPriceBadgeDepends to avoid
narrowing issues.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/composables/node/useNodeBadge.ts (1)
148-156: Clarify the intent of the discarded computed value.The call to
computedWithWidgetWatch(() => 0)registers widget listeners but discards the returned computed. While the comments explain this, the pattern is unusual and could confuse future maintainers.Consider extracting this into a dedicated function in
useWatchWidgetthat explicitly only registers listeners without returning a computed, or add a more prominent comment explaining why this pattern is necessary.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.jsonpnpm-workspace.yamlsrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
**/**/use[A-Z]*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables using the pattern
useXyz.ts
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tspackage.jsonsrc/composables/node/useNodePricing.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
🧠 Learnings (5)
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/composables/node/useNodeBadge.tssrc/schemas/nodeDefSchema.tssrc/composables/node/useNodePricing.ts
🧬 Code graph analysis (2)
src/composables/node/useNodeBadge.ts (3)
src/scripts/app.ts (1)
app(1772-1772)src/utils/migration/migrateReroute.ts (1)
node(91-107)src/stores/commandStore.ts (1)
label(52-54)
src/composables/node/useNodePricing.ts (3)
src/schemas/nodeDefSchema.ts (1)
PriceBadge(230-230)src/stores/commandStore.ts (1)
label(52-54)src/stores/nodeDefStore.ts (2)
useNodeDefStore(293-455)nodeDef(82-108)
🔇 Additional comments (9)
package.json (1)
174-174: LGTM!The
jsonatadependency addition aligns with the PR's objective to use JSONata expressions for pricing evaluation. The catalog reference correctly points to the version defined inpnpm-workspace.yaml.src/schemas/nodeDefSchema.ts (2)
200-215: Schema and documentation look good.The
zPriceBadgeDependsschema is well-documented, explaining the autogrow input group behavior and how the count is exposed in the JSONata context. The use ofoptional().default([])for array fields ensures safe access.
259-266: LGTM!The
price_badgefield is appropriately added as optional tozComfyNodeDefwith clear documentation explaining its purpose for API node pricing.src/composables/node/useNodeBadge.ts (3)
76-84: LGTM! Clean debouncing implementation.The
queueCanvasRedrawfunction properly batches multiple redraw requests within a single animation frame using a guard flag. This prevents excessive redraws when multiple pricing evaluations complete in quick succession.
86-93: LGTM! Reactive pricing revision watcher.The watcher correctly triggers canvas redraws only when
showApiPricingBadgeis enabled. Usingflush: 'sync'ensures immediate response to pricing changes.
158-170: LGTM! Efficient badge caching strategy.The
creditsBadgeGetterpattern correctly caches the badge instance and only recreates it when the price label actually changes. This avoids unnecessary object allocations on each badge access while maintaining reactivity.src/composables/node/useNodePricing.ts (2)
1-12: LGTM! Clear documentation of async behavior.The header comment effectively documents the JSONata v2.x async nature and explains the implementation strategy (sync getter, async evaluation, reactive tick). This context is valuable for future maintainers.
474-479: LGTM! Clean public API surface.The composable exposes a minimal, well-documented API: sync getter, config accessor, widget names helper, and a readonly reactive signal for UI updates.
pnpm-workspace.yaml (1)
67-67: LGTM!The
jsonataversion^2.1.0is appropriately pinned to the latest release in the workspace catalog. The caret allows for minor and patch updates, which is the correct approach. The placement maintains alphabetical ordering.
| ?.value | ||
| ) | ||
| // Reactive tick to force UI updates when async evaluations resolve. | ||
| // We purposely read pricingTick.value inside getNodeDisplayPrice to create a dependency. |
There was a problem hiding this comment.
Actionable comments posted: 7
Fix all issues with AI Agents 🤖
In @src/composables/node/useNodePricing.ts:
- Around line 151-160: The lookup loops use (x: any) which violates the no-any
rule; replace the explicit any annotations by using the correct domain types
(e.g., Widget and InputSlot or whatever concrete interfaces represent
node.widgets and node.inputs) or let TypeScript infer types by removing the
inline annotations; update the .find callbacks and local variables (widget,
slot, i) to use those types and keep the existing logic calling
normalizeWidgetValue and checking slot.link so behavior is unchanged (refer to
rule.depends_on.widgets, node.widgets, normalizeWidgetValue,
rule.depends_on.inputs, node.inputs, i, widget, slot to find the exact spots).
- Around line 166-170: The filter callback currently uses the unsafe any type
for inp; remove the ": any" and let TypeScript infer the type from node.inputs
(or explicitly cast node.inputs to the correct element type, e.g., Input[] or
NodeInput[] if that type exists) so the predicate becomes (inp) =>
inp?.name?.startsWith(prefix) && inp?.link != null; update the surrounding
typing of node or import/declare the proper input type if needed to ensure
node.inputs is strongly typed before computing connectedCount and assigning
g[groupName].
- Line 336: Repeated casts using (node.constructor as any)?.nodeData should be
replaced by a small typed helper to avoid any and centralize access; add a
getNodeData(node: LGraphNode): NodeData | undefined (with NodeData containing
name?, api_node?, price_badge?) that returns (node.constructor as { nodeData?:
NodeData })?.nodeData, then update all usages such as the nodeName assignment
(currently using (node.constructor as any)?.nodeData?.name ?? '') and other
occurrences (lines referencing api_node and price_badge) to call
getNodeData(node) and read properties from its result.
- Around line 255-263: The compileRule function currently uses an unsafe 'as
any' cast for the jsonata(rule.expr) result; import the jsonata Expression type
and change the compiled field typing so _compiled is typed as Expression | null
(update CompiledJsonataPricingRule accordingly) and cast jsonata(rule.expr) to
Expression instead of any, keeping the try/catch behavior in compileRule.
In @src/schemas/nodeDefSchema.ts:
- Around line 222-228: The default value for the depends_on property in
zPriceBadge omits input_groups, causing inconsistency with zPriceBadgeDepends;
update the default object in the zPriceBadge declaration (depends_on) to include
input_groups: [] alongside widgets: [] and inputs: [] so the default matches
zPriceBadgeDepends and avoids type narrowing issues.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/composables/node/useNodeBadge.ts (1)
148-156: Clarify the intent of discarding the computed return value.The
computedWithWidgetWatch(() => 0)call discards its return value, which is unusual. While the comment explains this registers widget listeners, consider adding a more explicit comment or using a dedicated function if the composable supports it.🔎 Suggested clarification
- // Ensure watchers are installed; ignore the returned value. - // (This call is what registers the widget listeners in most implementations.) - computedWithWidgetWatch(() => 0) + // Register widget change listeners to trigger canvas redraws. + // The returned computed value is intentionally unused; we only need + // the side effect of installing widget watchers. + void computedWithWidgetWatch(() => 0)
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.jsonpnpm-workspace.yamlsrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
package.jsonsrc/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
**/**/use[A-Z]*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables using the pattern
useXyz.ts
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
🧠 Learnings (13)
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/schemas/nodeDefSchema.tssrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Do not use `any` type or `as any` type assertions; fix the underlying type issue instead
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use function declarations instead of function expressions when possible
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-13T05:34:22.209Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:22.209Z
Learning: In Playwright tests in the Comfy-Org/ComfyUI_frontend repository, the `toPass()` assertion method implements incremental backoff during retries, so increasing the timeout value (e.g., to 5000ms) is sufficient for handling async operations without being overly aggressive.
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Applied to files:
src/composables/node/useNodePricing.ts
🧬 Code graph analysis (1)
src/composables/node/useNodePricing.ts (2)
src/schemas/nodeDefSchema.ts (1)
PriceBadge(230-230)src/stores/nodeDefStore.ts (2)
useNodeDefStore(293-455)nodeDef(82-108)
🔇 Additional comments (8)
package.json (1)
174-174: LGTM!The
jsonatadependency is appropriately added to runtime dependencies using the catalog specifier, consistent with the project's dependency management pattern.src/schemas/nodeDefSchema.ts (2)
200-215: LGTM!The
zPriceBadgeDependsschema is well-documented with clear explanations for each field, especially theinput_groupsfield which includes a helpful usage example for autogrow inputs.
259-265: LGTM!The
price_badgefield is appropriately added tozComfyNodeDefas an optional field with clear documentation explaining its purpose for API node pricing.src/composables/node/useNodePricing.ts (1)
408-436: LGTM with note on intentional reactive dependency.The bare
pricingTick.valueread on line 411 is an intentional reactive dependency pattern. Consider usingvoid pricingTick.valueto make the intent clearer and prevent accidental removal.The sync getter with async fallback pattern is well-designed for avoiding flicker while handling JSONata's async evaluation.
src/composables/node/useNodeBadge.ts (3)
76-93: LGTM!Good optimization using
requestAnimationFrameto batch canvas redraws. TheredrawQueuedflag prevents redundant RAF calls, and the watch with{ flush: 'sync' }ensures pricing updates are immediately reflected.
158-170: Mutable closure state for badge caching.The
lastLabelandlastBadgevariables create mutable closure state. This is acceptable here since it's a simple memoization pattern, but consider using aWeakMapfor consistency with the caching pattern inuseNodePricing.tsif this grows in complexity.
132-139: LGTM!The
hasDynamicPricingcheck correctly identifies nodes with widget or input dependencies, aligning with the JSONata-based pricing system.pnpm-workspace.yaml (1)
67-67: The jsonata version^2.1.0is the current stable release on npm and is actively maintained. The catalog entry is appropriate.
bigcat88
force-pushed
the
feat/backend-api-nodes-pricing-def
branch
from
55e67be to
7d3f77e
Compare
bigcat88
force-pushed
the
feat/backend-api-nodes-pricing-def
branch
from
7d3f77e to
110224b
Compare
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/07/2026, 03:39:24 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 01/07/2026, 03:44:32 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.26 MB (baseline 3.25 MB) • 🔴 +15.4 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 941 kB (baseline 1.01 MB) • 🟢 -66.4 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.63 kB (baseline 6.63 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 300 kB (baseline 300 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 198 kB (baseline 198 kB) • ⚪ 0 BReusable component library chunks
Status: 10 added / 10 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 9.44 MB (baseline 9.19 MB) • 🔴 +246 kBExternal libraries and shared vendor chunks
Status: 6 added / 6 removed Other — 3.5 MB (baseline 3.5 MB) • ⚪ 0 BBundles that do not match a named category
Status: 25 added / 25 removed |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
bigcat88
force-pushed
the
feat/backend-api-nodes-pricing-def
branch
from
9e81200 to
5ab4187
Compare
bigcat88
force-pushed
the
feat/backend-api-nodes-pricing-def
branch
from
5ab4187 to
ff49062
Compare
src/schemas/nodeDefSchema.ts
Outdated
| engine: z.literal('jsonata').optional().default('jsonata'), | ||
| depends_on: zPriceBadgeDepends | ||
| .optional() | ||
| .default({ widgets: [], inputs: [] }), |
There was a problem hiding this comment.
Maybe?
| .default({ widgets: [], inputs: [] }), | |
| .default({ widgets: [], inputs: [], input_groups: [] }), |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| widgets: Array<{ name: string; value: unknown }> = [], | ||
| inputs: Array<{ name: string; connected?: boolean }> = [] |
There was a problem hiding this comment.
Could these use the types defined in nodeDefSchema?
| "firebase": "catalog:", | ||
| "fuse.js": "^7.0.0", | ||
| "glob": "^11.0.3", | ||
| "jsonata": "catalog:", |
There was a problem hiding this comment.
[architecture] high Priority
Issue: New jsonata dependency addition without clear evaluation of alternatives
Context: Adding jsonata as a new runtime dependency for pricing calculations - should evaluate if existing libraries like lodash template functions or custom expression parser could handle these use cases
Suggestion: Document why JSONata was chosen over alternatives, consider bundle size impact, and evaluate if simpler expression evaluation patterns would suffice
| let lastBadge = priceBadge.getCreditsBadge(lastLabel) | ||
|
|
||
| const creditsBadgeGetter: () => LGraphBadge = () => { | ||
| const label = nodePricing.getNodeDisplayPrice(node) |
There was a problem hiding this comment.
[performance] medium Priority
Issue: Badge creation pattern may cause unnecessary re-evaluations
Context: The creditsBadgeGetter function is called on every render and manually caches lastLabel/lastBadge, but this could lead to inconsistent state if pricing updates occur between renders
Suggestion: Use a proper Vue computed property that depends on the pricing revision signal, or implement the caching within the pricing service itself
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: Move price badges to python nodes (#7816)
Impact: 829 additions, 4427 deletions across 8 files
Issue Distribution
- Critical: 1
- High: 2
- Medium: 3
- Low: 1
Category Breakdown
- Architecture: 1 issue
- Security: 1 issue
- Performance: 3 issues
- Code Quality: 2 issues
Key Findings
Architecture & Design
This PR represents a significant architectural shift from frontend-hardcoded pricing logic to backend-driven JSONata expressions. The change enables more flexible pricing configurations but introduces complexity in async evaluation patterns and expression compilation. The addition of jsonata as a dependency should be evaluated against simpler alternatives before being merged.
Security Considerations
Critical Issue: JSONata expression compilation from user-controlled data poses a security risk. The current implementation compiles arbitrary expressions without validation, potentially allowing code injection if malicious node definitions are loaded. This needs immediate attention before deployment.
Performance Impact
The new async pricing evaluation pattern with WeakMap-based caching introduces potential performance concerns:
- Synchronous getters triggering async operations during render cycles
- Complex state management across multiple WeakMaps
- Manual badge caching that may lead to inconsistent state
Bundle size impact from the new jsonata dependency should also be evaluated.
Integration Points
The changes maintain backward compatibility while introducing new JSONata-based pricing for API nodes. The schema updates properly extend the existing node definition structure. The integration with Vue's reactivity system through the pricingTick ref is well-designed.
Positive Observations
- Comprehensive test coverage: The test file shows thorough coverage of the new JSONata evaluation patterns
- Strong type safety: Excellent TypeScript typing throughout the new pricing system
- Error handling: Non-fatal error handling ensures UI stability even with bad expressions
- Clean separation: Good separation between compilation, caching, and evaluation logic
- Schema validation: Proper Zod schema validation for the new price badge structures
References
Next Steps
- Address critical security issue before merge - implement expression validation
- Evaluate bundle size impact of jsonata dependency
- Consider performance optimizations for the async evaluation pattern
- Add integration tests for the pricing badge rendering pipeline
- Document JSONata expression patterns for node developers
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
| type JsonataPricingRule = { | ||
| engine: 'jsonata' | ||
| depends_on: { | ||
| widgets: WidgetDependency[] | ||
| inputs: string[] | ||
| input_groups: string[] | ||
| } | ||
| expr: string | ||
| result_defaults?: CreditFormatOptions | ||
| } |
There was a problem hiding this comment.
Could this also use the type from the schema?
| if (duration === '5') { | ||
| return generateAudio ? formatCreditsLabel(0.7) : formatCreditsLabel(0.35) | ||
| } | ||
| const r = result as PricingResult |
There was a problem hiding this comment.
Nit: This would be a good place to have a real type guard.
There was a problem hiding this comment.
Could you type the parameter and enforce the type earlier on?
|
@coderabbitai review full |
|
@DrJKL I'll perform a full review of all the changes in this PR. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/composables/node/useNodeBadge.ts (1)
148-156: Clarify the intent of discarding the computed return value.The
computedWithWidgetWatch(() => 0)call is made solely to install widget listeners, with the return value intentionally discarded. While the comments explain this, consider extracting this pattern into a more explicit helper likeuseWidgetWatchersOnly(node, { widgetNames })to make the intent clearer.
🤖 Fix all issues with AI agents
In @src/composables/node/useNodePricing.ts:
- Line 367: Refactor the repeated unsafe cast by adding a typed helper (e.g.,
define a NodeData type with name?, api_node?, price_badge? and implement
getNodeData(node: LGraphNode): NodeData | undefined that returns
(node.constructor as { nodeData?: NodeData })?.nodeData) and replace all
occurrences of (node.constructor as any)?.nodeData (used when computing
nodeName, and at the other two sites around lines where api_node/price_badge are
accessed) to call getNodeData(node) and read properties from that typed result.
- Around line 439-442: The bare read of pricingTick.value inside
getNodeDisplayPrice is intended to create a reactive dependency but appears as
dead code; make that intent explicit by using a void read (e.g., `void
pricingTick.value`) so the reactive dependency is clear and linter readers
understand it’s deliberate—update the getNodeDisplayPrice function to replace
the bare pricingTick.value read with an explicit void usage.
In @src/schemas/nodeDefSchema.ts:
- Around line 236-238: The default value for depends_on is missing input_groups;
update the depends_on default object used with zPriceBadgeDepends to include
input_groups: [] so it matches zPriceBadgeDepends' shape (i.e., change the
default from { widgets: [], inputs: [] } to include input_groups: []). Ensure
the modification is applied where depends_on is defined so the schema default
aligns with zPriceBadgeDepends.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.jsonpnpm-workspace.yamlsrc/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.tssrc/stores/nodeDefStore.ts
🧰 Additional context used
📓 Path-based instructions (14)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
**/**/use[A-Z]*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables using the pattern
useXyz.ts
Files:
src/composables/node/useNodeBadge.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tspackage.jsonsrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/stores/nodeDefStore.ts
**/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores using the pattern
*Store.ts
Files:
src/stores/nodeDefStore.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Use unit/component tests intests-ui/orsrc/**/*.test.tswith Vitest framework
For mocking in tests, leverage Vitest utilities; keep module mocks contained and avoid global mutable state within test files
Do not write change detector tests or tests dependent on non-behavioral features like utility classes or styles
Aim for behavioral coverage of critical and new features in unit tests
Files:
src/composables/node/useNodePricing.test.ts
🧠 Learnings (17)
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/composables/node/useNodeBadge.tssrc/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.tssrc/composables/node/useNodePricing.tssrc/schemas/nodeDefSchema.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/stores/nodeDefStore.tssrc/composables/node/useNodePricing.test.ts
📚 Learning: 2025-12-10T03:09:13.807Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:13.807Z
Learning: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests validate actual user-facing behavior and accessibility, reducing reliance on implementation details like test IDs.
Applied to files:
src/composables/node/useNodePricing.test.ts
📚 Learning: 2025-12-30T01:31:04.927Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7797
File: tests-ui/tests/lib/litegraph/src/widgets/ComboWidget.test.ts:648-648
Timestamp: 2025-12-30T01:31:04.927Z
Learning: In Vitest v4, when mocking functions that may be called as constructors (using new), the mock implementation must use function() or class syntax rather than an arrow function. Arrow mocks can cause '<anonymous> is not a constructor' errors. This is a breaking change from Vitest v3 where mocks could use an arrow function. Apply this guideline to test files that mock constructor-like calls (e.g., in tests under tests-ui, such as ComboWidget.test.ts) and ensure mock implementations are defined with function() { ... } or class { ... } to preserve constructor behavior.
Applied to files:
src/composables/node/useNodePricing.test.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Do not use `any` type or `as any` type assertions; fix the underlying type issue instead
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-13T05:34:22.209Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7415
File: browser_tests/tests/mobileBaseline.spec.ts:7-15
Timestamp: 2025-12-13T05:34:22.209Z
Learning: In Playwright tests in the Comfy-Org/ComfyUI_frontend repository, the `toPass()` assertion method implements incremental backoff during retries, so increasing the timeout value (e.g., to 5000ms) is sufficient for handling async operations without being overly aggressive.
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2026-01-06T19:20:52.000Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7746
File: src/platform/assets/services/assetService.ts:484-491
Timestamp: 2026-01-06T19:20:52.000Z
Learning: In `src/platform/assets/services/assetService.ts`, prefer using `schema.safeParse()` over `schema.parse()` for validating API responses to avoid throwing ZodError with internal schema details; follow the existing pattern: call `safeParse()`, check `result.success`, and use `fromZodError(result.error)` to format error messages for logging while throwing user-friendly errors.
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.{ts,tsx,vue} : Use function declarations instead of function expressions when possible
Applied to files:
src/composables/node/useNodePricing.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/composables/node/useNodePricing.ts
🧬 Code graph analysis (2)
src/stores/nodeDefStore.ts (1)
src/schemas/nodeDefSchema.ts (1)
PriceBadge(242-242)
src/composables/node/useNodePricing.test.ts (4)
src/base/credits/comfyCredits.ts (1)
formatCreditsFromUsd(84-93)src/schemas/nodeDefSchema.ts (1)
PriceBadge(242-242)src/lib/litegraph/src/LGraphNode.ts (1)
inputs(3305-3309)src/composables/node/useNodePricing.ts (1)
useNodePricing(432-511)
🔇 Additional comments (17)
pnpm-workspace.yaml (1)
66-66: LGTM!The jsonata version ^2.1.0 is correctly added to the workspace catalog, enabling consistent version resolution across the monorepo.
src/schemas/nodeDefSchema.ts (2)
200-209: LGTM!The
zWidgetDependencyschema correctly captures widget dependencies with both name and type, enabling proper type-aware normalization in the pricing evaluation.
271-277: LGTM!The
price_badgefield is correctly integrated as an optional field inzComfyNodeDef, with clear documentation explaining its purpose for API node pricing.src/stores/nodeDefStore.ts (3)
17-19: LGTM!The
PriceBadgetype import is correctly separated from the non-type imports, following the repository's lint rules.
70-75: LGTM!The
price_badgefield is properly added as an optional readonly property with clear documentation explaining its purpose for JSONata-based pricing calculations.
144-144: LGTM!The
price_badgeinitialization correctly assigns from the input object, consistent with other optional fields in the constructor.src/composables/node/useNodeBadge.ts (2)
76-93: LGTM - Good batching pattern for canvas redraws.The
requestAnimationFramebatching prevents excessive redraws when multiple pricing updates occur in quick succession. Theflush: 'sync'on the watcher ensures immediate response to pricing revision changes.
158-170: Manual badge caching may have subtle timing issues.The
creditsBadgeGetterpattern cacheslastLabelandlastBadge, updating only when the label changes. This works but creates a closure that captures mutable state.Consider whether this could be simplified by having
priceBadge.getCreditsBadgeimplement its own memoization, or by using acomputedthat directly depends on the pricing revision signal.A previous review flagged potential inconsistencies with this caching pattern. The current implementation should work correctly since
getNodeDisplayPricehandles the async coordination, but the manual caching adds complexity.Likely an incorrect or invalid review comment.
src/composables/node/useNodePricing.test.ts (3)
26-57: LGTM - Well-structured test helper.The
createMockNodeWithPriceBadgehelper properly constructs mock nodes withprice_badgefor testing the JSONata-based pricing system. Using thePriceBadgetype from the schema ensures type consistency.
59-69: LGTM - Clean helper for constructing PriceBadge objects.The
priceBadgehelper provides sensible defaults while allowing customization of widgets, inputs, and input groups.
360-376: LGTM - Good edge case coverage.Tests for non-API nodes and nodes without
price_badgecorrectly verify that empty strings are returned, ensuring graceful degradation.src/composables/node/useNodePricing.ts (6)
1-21: LGTM - Clear module documentation.The header comment clearly explains the JSONata 2.x async behavior and the sync getter / async evaluation / reactive tick pattern. The imports are well-organized with type imports separated.
95-104: LGTM - Type definitions use schema types.The
JsonataPricingRuletype now correctly usesWidgetDependency[]for widgets, addressing the earlier review feedback about using types from the schema.
140-175: LGTM - Type-aware widget normalization.The
normalizeWidgetValuefunction correctly handles different widget types (INT, FLOAT, BOOLEAN, COMBO, STRING) with appropriate type conversions. The lowercase normalization for strings enables case-insensitive matching in JSONata expressions.
177-205: LGTM - Context building with proper types.The
buildJsonataContextfunction correctly usesIBaseWidgetandINodeInputSlottypes from litegraph, addressing the earlier review concern aboutanytypes in widget/input lookups.
286-293: LGTM - Expression type imported from jsonata.The
compileRulefunction now correctly uses theExpressiontype from jsonata, eliminating the need foras anycast on the compiled expression.
354-406: Async evaluation pipeline is well-designed.The
scheduleEvaluationfunction correctly handles:
- Deduplication via signature matching
- Stale result rejection via
desiredSigcheck- Non-fatal error handling with cache population
- Reactive updates via
pricingTickincrementThe WeakMap usage prevents memory leaks when nodes are removed.
| const lower = String(v).toLowerCase() | ||
| if (lower === 'true') return true | ||
| if (lower === 'false') return false | ||
| const nodeName = (node.constructor as any)?.nodeData?.name ?? '' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract helper to reduce repeated as any casts for nodeData access.
The pattern (node.constructor as any)?.nodeData is repeated at lines 367, 414, and 444-446. Consider extracting a typed helper function to isolate this cast.
This was flagged in a previous review. Suggested helper:
type NodeData = {
name?: string
api_node?: boolean
price_badge?: PriceBadge
}
function getNodeData(node: LGraphNode): NodeData | undefined {
return (node.constructor as { nodeData?: NodeData })?.nodeData
}🤖 Prompt for AI Agents
In @src/composables/node/useNodePricing.ts at line 367, Refactor the repeated
unsafe cast by adding a typed helper (e.g., define a NodeData type with name?,
api_node?, price_badge? and implement getNodeData(node: LGraphNode): NodeData |
undefined that returns (node.constructor as { nodeData?: NodeData })?.nodeData)
and replace all occurrences of (node.constructor as any)?.nodeData (used when
computing nodeName, and at the other two sites around lines where
api_node/price_badge are accessed) to call getNodeData(node) and read properties
from that typed result.
There was a problem hiding this comment.
@coderabbitai Are there other places in the codebase that could benefit from such a utility?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for patterns of accessing nodeData through constructor casts
echo "=== Pattern 1: (constructor as any)?.nodeData ==="
rg -n --type=ts -C2 '\(.*\.constructor as any\)\??\.nodeData'
echo ""
echo "=== Pattern 2: constructor as any (broader search) ==="
rg -n --type=ts -l 'constructor as any'
echo ""
echo "=== Pattern 3: Direct .nodeData access (to see context) ==="
rg -n --type=ts -C1 '\.nodeData\b' | head -100
Length of output: 7736
🏁 Script executed:
#!/bin/bash
# Check the other files that use "constructor as any"
echo "=== workflowStore.ts usage ==="
rg -n --type=ts -C3 'constructor as any' src/platform/workflow/management/stores/workflowStore.ts
echo ""
echo "=== LGraphNode.test.ts usage ==="
rg -n --type=ts -C3 'constructor as any' src/lib/litegraph/src/LGraphNode.test.ts
echo ""
echo "=== litegraphService.ts with type errors/casts ==="
rg -n --type=ts -B2 -A2 'ts-expect-error|nodeData as ComfyNodeDefImpl' src/services/litegraphService.ts | head -30
Length of output: 2845
@DrJKL Yes, there are several other places in the codebase that would benefit from a typed getNodeData helper:
Files that would benefit:
-
src/services/litegraphService.ts (line 205)
- Uses
node.constructor.nodeData as ComfyNodeDefImplwith a@ts-expect-errorcomment - Also has multiple direct
.nodeDataaccesses (lines 87, 302-303, 361, 379, 403-404, 409, 469, 492)
- Uses
-
src/stores/nodeDefStore.ts (line 360)
- Uses
node.constructor?.nodeData?.namedirectly
- Uses
-
src/utils/nodeFilterUtil.ts (line 11)
- Uses
node.constructor.nodeData?.output_nodedirectly
- Uses
-
src/utils/executableGroupNodeDto.ts (line 14)
- Uses
node.constructor?.nodeData?.[GROUP]directly
- Uses
-
src/extensions/core/nodeTemplates.ts (line 371)
- Uses
node?.constructor.nodeDatadirectly
- Uses
-
src/extensions/core/groupNode.ts (multiple lines)
- Has its own
nodeDataproperty management
- Has its own
Summary:
The typed helper would improve type safety in at least 6 files across the codebase. The most impactful would be litegraphService.ts, which has both an explicit cast with @ts-expect-error and numerous direct accesses to .nodeData.
I recommend creating this helper as a shared utility (e.g., in src/utils/nodeDataUtils.ts) so all these files can benefit from consistent, type-safe access to node metadata.
🧬 Code Graph Analysis Results
<src/schemas/nodeDefSchema.ts> (lines 242-242)
export type PriceBadge = z.infer<typeof zPriceBadge>
<src/base/credits/comfyCredits.ts> (lines 84-93)
export const formatCreditsFromUsd = ({
usd,
locale,
numberOptions
}: FormatFromUsdOptions): string =>
formatCredits({
value: usdToCredits(usd),
locale,
numberOptions
})
9698efc to
d78cd9e
Compare
Summary
WIP
Changes
Review Focus
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito