linear v2: Simple Mode #7734
Merged
linear v2: Simple Mode #7734
Conversation
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
Contributor
📝 WalkthroughWalkthroughAdds semantic color tokens, a linear/graph mode toggle with feature-flag gating, multiple Linear Mode UI components (controls, previews, history, drop zone), shared UI primitives (Popover, ZoomPane), state/store API adjustments, theming refactors, and small build/config and locale updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ModeToggle
participant CommandStore
participant CoreCommands
participant AppState
participant WorkflowStore
participant CanvasStore
User->>ModeToggle: Click toggle
ModeToggle->>CommandStore: dispatch(Comfy.ToggleLinear)
CommandStore->>CoreCommands: execute ToggleLinear
CoreCommands->>AppState: set app.rootGraph.extra.linearMode(newMode)
CoreCommands->>WorkflowStore: changeTracker.checkState()
CoreCommands->>CanvasStore: set canvasStore.linearMode(newMode)
CanvasStore-->>ModeToggle: reactive update (button variant)
sequenceDiagram
participant User
participant OutputHistory
participant NodeOutputStore
participant LinearPreview
participant WorkflowStore
User->>OutputHistory: scroll / select output
OutputHistory->>NodeOutputStore: update selectedItem/selectedOutput
NodeOutputStore-->>LinearPreview: selected output changes
LinearPreview->>WorkflowStore: load associated workflow (on rerun)
WorkflowStore-->>LinearPreview: workflow loaded / widgets executed
LinearPreview->>User: display preview / run feedback
Possibly related PRs
Suggested Reviewers
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/14/2026, 04:12:34 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests:
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.33 MB (baseline 3.33 MB) • 🔴 +872 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 1.15 MB (baseline 1.1 MB) • 🔴 +47.5 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.66 kB (baseline 6.63 kB) • 🔴 +39 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 372 kB (baseline 372 kB) • 🔴 +233 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 209 kB (baseline 209 kB) • 🔴 +257 BReusable component library chunks
Status: 9 added / 9 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.34 MB (baseline 9.19 MB) • 🔴 +151 kBExternal libraries and shared vendor chunks
Status: 7 added / 6 removed Other — 5.25 MB (baseline 5.25 MB) • 🔴 +622 BBundles that do not match a named category
Status: 21 added / 21 removed |
AustinMroz
force-pushed
the
austin/linear-v2
branch
from
89146e8 to
1f17f5f
Compare
Contributor
|
@coderabbitai review full |
Contributor
|
@DrJKL: I'll perform a full review of the PR changes now. ✅ Actions performedFull review triggered. |
![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: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/views/GraphView.vue (1)
219-226: Inconsistent linear mode handling between event handlers.The
onStatushandler (line 214) checks for both the assets sidebar andlinearMode, but theonExecutionSuccesshandler (line 223) only checks for the assets sidebar. This inconsistency means assets won't update after execution completes when in linear mode but the assets sidebar isn't open.🔧 Proposed fix
const onExecutionSuccess = async () => { await queueStore.update() - // Only update assets if the assets sidebar is currently open + // Update assets if the assets sidebar is open or linear mode is active // When sidebar is closed, AssetsSidebarTab.vue will refresh on mount - if (sidebarTabStore.activeSidebarTabId === 'assets') { + if (sidebarTabStore.activeSidebarTabId === 'assets' || linearMode.value) { await assetsStore.updateHistory() } }
🤖 Fix all issues with AI agents
In @packages/design-system/src/css/style.css:
- Line 248: The light-theme mapping for the CSS variable --success-background
currently uses var(--color-jade-600), which breaks the established pattern of
lighter shades for light themes; update the light-theme declaration(s) of
--success-background to use var(--color-jade-400) (replace occurrences where
--success-background is set to var(--color-jade-600) in the light-theme
block(s)) and keep the dark-theme mapping at var(--color-jade-600); after
change, verify contrast/accessibility and that any duplicated occurrences (the
other occurrence of --success-background) are updated only for the light-theme
variant.
In @src/components/sidebar/ModeToggle.vue:
- Line 16: The click handler currently sets canvasStore.linearMode directly
(e.g., in ModeToggle.vue) but does not update app.rootGraph.extra.linearMode,
causing state desync with the Comfy.ToggleLinear command; fix by updating both
locations whenever the toggle is changed (set canvasStore.linearMode and also
assign app.rootGraph.extra.linearMode = canvasStore.linearMode), or preferably
call the existing Comfy.ToggleLinear command (from
src/composables/useCoreCommands.ts) instead of directly mutating state so that
both canvasStore.linearMode and app.rootGraph.extra.linearMode are kept in sync.
- Around line 10-19: The icon-only Button components (e.g., the Button that
toggles canvasStore.linearMode in ModeToggle.vue) rely on title for tooltips but
lack accessible names; add an aria-label to each icon-only Button using the same
localized string used for title (e.g., t('linearMode.linearMode')) so screen
readers receive a proper label; apply the same change to the other icon-only
button(s) in the file (lines ~20-29) to ensure all icon buttons have aria-label
attributes.
- Line 11: The disabled:opacity-100 utility in ModeToggle.vue (the class on the
toggle buttons) forces full opacity for disabled buttons and makes the disabled
state ambiguous; remove the disabled:opacity-100 override from the button class
list and instead indicate selection via an explicit active state (e.g., an
"active"/"selected" class, different background/border, and aria-pressed or
aria-current attributes) or convert the control to an accessible radio-like
group so unselected options remain visually distinct without relying on disabled
styling; update the template and any CSS/variants used by the toggle buttons and
ensure accessibility attributes reflect the chosen approach.
In @src/components/sidebar/SideToolbar.vue:
- Around line 47-52: Call useFeatureFlags() in the <script setup> rather than in
the template: import/invoke the useFeatureFlags composable in the script setup,
destructure or create a computed (e.g., const { flags } = useFeatureFlags(); or
const linearToggleVisible = computed(() => flags.linearToggleEnabled ||
canvasStore.linearMode)) and then update the template ModeToggle v-if to use
that computed/destructured value (reference symbols: useFeatureFlags, flags,
linearToggleVisible, ModeToggle, canvasStore.linearMode).
In @src/components/ui/Popover.vue:
- Around line 26-30: The default trigger inside the <slot name="button"> renders
an icon-only <Button> which lacks an accessible label; update the Popover
component to add an aria-label on that default <Button> (e.g., aria-label="Open
popover") and expose a prop (prop name like defaultTriggerLabel or
triggerAriaLabel) so consumers can override the label; modify the template where
<Button size="icon"> is rendered and wire the new prop into the aria-label
attribute, ensuring the icon class logic (<i :class="icon ??
'icon-[lucide--ellipsis]'"/>) remains unchanged.
In @src/components/ui/ZoomPane.vue:
- Around line 1-63: ZoomPane lacks keyboard accessibility; add a keydown handler
and ARIA/tabindex attributes to enable keyboard panning, zooming and reset.
Implement a handleKeydown(KeyboardEvent) function that maps
ArrowLeft/Right/Up/Down to adjust panX and panY (respecting shift for larger
steps), '+'/'=' and '-' to change zoom, and '0'/'Home' to reset zoom/pan; ensure
it calls e.preventDefault() for handled keys. Attach the handler to the root
template element (the ref zoomPane) via @keydown and add tabindex="0", a
suitable role (e.g., role="img" or region), and an aria-label; also change
@wheel to use .prevent so keyboard focus + Ctrl/Shift interactions behave
consistently. Make sure to reference existing refs/refs names (zoom, panX, panY,
zoomPane) and update class names to include sensible focusable styles if needed.
- Line 56: The wheel event on ZoomPane.vue should prevent default page scrolling
during zoom: update the @wheel binding or the handler so that the wheel event's
default is prevented — either change the template to use the .prevent modifier
on the wheel listener or add event.preventDefault() at the start of the
handleWheel(event) method to stop the page from scrolling while zooming.
- Around line 40-44: handleMove currently assumes dragging remains true even if
the pointer button was released outside the element; update
handleMove(PointerEvent e) to check the primary button state (e.buttons & 1 or
e.buttons === 1) before applying movement to panX and panY, and if the primary
button is not pressed set dragging = false and return so you stop panning;
reference the existing handleMove function and the dragging, panX, and panY
reactive values when making this change.
- Line 55: In the ZoomPane.vue template the flex container uses an invalid
Tailwind utility `align-center`; replace `align-center` with the correct
`items-center` class so the element with class "contain-size flex justify-center
align-center" becomes "contain-size flex justify-center items-center" to achieve
vertical centering.
- Around line 21-31: Extract the magic numbers used in the zoom math into named
constants and use them in handleWheel and related zoom code: replace the literal
1.1 with a constant (e.g., ZOOM_BASE or ZOOM_FACTOR_BASE), replace -30 with a
constant for the divisor/sign used in the exponent (e.g., ZOOM_DELTA_DIVISOR or
ZOOM_SENSITIVITY with appropriate sign handling), and replace 30 (the opposite
sign divisor used elsewhere) with a corresponding constant (e.g.,
ZOOM_DELTA_DIVISOR_POS); define these constants at the top of the component (or
expose them as props/config) and update handleWheel to use those names in the
scaler calculation and any other zoom computations so the behavior is
configurable and maintainable (refer to function handleWheel, zoom.value
updates, and the scaler computation).
- Around line 33-44: The variable dragging is a plain JS boolean and not
reactive; change it to a Vue ref (e.g., import { ref } from 'vue' and const
dragging = ref(false)), then update all uses: in handleDown set dragging.value =
true (and capture pointer), in the pointer-up/cleanup code set dragging.value =
false, and in handleMove check if (!dragging.value) return and use movement
updates the same; also update any template bindings to use dragging.value or
unref(dragging) so Vue reactivity is preserved (references: dragging,
handleDown, handleMove).
In @src/composables/useCoreCommands.ts:
- Around line 1182-1186: The toggle command currently writes to
app.rootGraph.extra.linearMode without ensuring app.rootGraph exists; modify the
anonymous function handler to first check if app.rootGraph is present (e.g., if
(!app.rootGraph) { canvasStore.linearMode = newMode; return; }) before accessing
app.rootGraph.extra.linearMode, so you only access the rootGraph.extra when
rootGraph is defined and avoid the non-null assertion risk; update the function
that toggles canvasStore.linearMode and app.rootGraph.extra.linearMode
accordingly.
In @src/platform/workflow/templates/composables/useTemplateWorkflows.ts:
- Line 125: The function parameter sourceModule is being reassigned from
template.sourceModule; instead of mutating the parameter, introduce a new local
variable (e.g., resolvedSourceModule or sourceModuleResolved) and assign it
template.sourceModule, then use that new variable everywhere you currently use
the reassigned sourceModule; keep the useTemplateWorkflows function signature
unchanged and only replace internal references to the parameter with the new
local variable to avoid parameter mutation.
In @src/renderer/extensions/linearMode/DropZone.vue:
- Around line 29-34: The drop handler in DropZone.vue should await the
possibly-async onDragDrop callback before resetting canAcceptDrop: make the
inline drop listener async, call the onDragDrop callback (onDragDrop!(e)) and
await its result, then set canAcceptDrop = false only after the awaited Promise
resolves (or set it based on the returned boolean), and catch/log any rejection
so canAcceptDrop is still reliably reset on error; update the @drop handler
accordingly to use async/await and proper optional chaining.
- Line 45: The template currently uses @click.prevent="(e: MouseEvent) =>
dropIndicator!.onClick?.(e)" which always calls e.preventDefault() and may break
expected browser behaviors; change to plain @click="(e: MouseEvent) => { if
(dropIndicator?.shouldPreventDefault) e.preventDefault();
dropIndicator!.onClick?.(e) }" or simply remove .prevent to @click="(e:
MouseEvent) => dropIndicator!.onClick?.(e)" and, if needed, implement
conditional e.preventDefault() inside the dropIndicator.onClick implementation;
update DropZone.vue where dropIndicator!.onClick?.(e) is invoked and add a
boolean flag (e.g., shouldPreventDefault) to control preventing default behavior
if conditional prevention is required.
In @src/renderer/extensions/linearMode/ImagePreview.vue:
- Around line 10-12: The width and height refs (width, height) are initialized
to empty strings causing the UI to show " x " before the image loads; initialize
them to a sensible default like '0' or '–' (or use null) and update the template
that reads these refs (the dimensions display near the imageRef rendering) to
conditionally render the "W x H" text only when width/height are present (or
non-zero) so you don't see a dangling " x " pre-load.
- Around line 20-27: The @load handler only sets width/height from imageRef but
misses an @error path; add an @error handler (e.g., onImageError) in
ImagePreview.vue that sets a reactive imageLoadError flag, clears or sets
width/height to safe defaults, logs or emits the error, and update the template
to render a user-visible fallback/error message when imageLoadError is true;
reference the existing imageRef, width, and height so the error handler updates
the same state and optionally emits an event for parent components.
- Line 29: Replace the hardcoded dimension string in ImagePreview.vue with a
vue-i18n lookup and add an accessible label: add "linearMode.imageDimensions":
"{width} × {height}" to your locales (e.g., src/locales/en/main.json), then
change the span that currently uses v-text="`${width} x ${height}`" to use
$t('linearMode.imageDimensions', { width, height }) and include an appropriate
aria-label (or aria-hidden as needed) so screen readers get the localized,
semantic description; ensure the span retains its visual content while supplying
the i18n string for accessibility.
In @src/renderer/extensions/linearMode/LinearPreview.vue:
- Line 71: The template in LinearPreview.vue uses a plain string
`${allOutputs.length} asset` which fails to pluralize; change it to use the
app/i18n pluralization helper (or $t/$tc) instead of the hardcoded string so
counts are plural-aware (e.g., call the translation function with the key and
pass allOutputs.length as the count), add the corresponding translation key with
singular and plural forms in your locale files, and replace the current
expression that references allOutputs to use that translation helper.
- Around line 127-136: The download Button inside LinearPreview.vue is icon-only
and missing an accessibility label; update the Button (the component instance
using @click that calls downloadFile(selectedOutput.url)) to include an
appropriate aria-label (or title) such as "Download file" so screen readers can
announce its purpose; ensure the label text is descriptive and matches the
action (e.g., aria-label="Download file") on the Button element that renders the
<i class="icon-[lucide--download]">.
- Around line 71-73: The array build uses `(selectedOutput &&
mediaTypes[getMediaType(selectedOutput)]) ?? {}` which injects an empty object
that passes `!!i` but is an invalid entry; change that expression to return
undefined when no media entry exists (e.g., `selectedOutput ?
mediaTypes[getMediaType(selectedOutput)] : undefined`) or remove the `?? {}`
fallback, and then use a Boolean-narrowing filter like `.filter(Boolean)` (or a
type guard) so only real entries remain; update the array construction that
contains `allOutputs && { content: `${allOutputs.length} asset` }` and the
`mediaTypes[getMediaType(selectedOutput)]` reference accordingly.
In @src/renderer/extensions/linearMode/LinearWorkflow.vue:
- Line 155: Remove the dead Button element in the LinearWorkflow.vue template
(the <Button v-if="false"> showing t('menuLabels.publish')) or replace it with a
clear TODO comment describing the intended future publish feature and when it
should be re-enabled; locate the Button in the LinearWorkflow component template
and either delete the unused element or add a concise comment (e.g., TODO:
implement publish action and gate by feature flag) so the placeholder doesn't
remain as unreachable code.
- Line 220: Reviewer flagged usage of PrimeVue's ProgressSpinner in
LinearWorkflow.vue (the v-else branch rendering ProgressSpinner); replace that
PrimeVue component with either a project shared Spinner component (e.g.,
Spinner) or a Tailwind-based spinner div to comply with the guideline to avoid
new PrimeVue usage, and update the template where ProgressSpinner is referenced
to use the chosen Spinner symbol so all imports/registrations and tests reflect
the new component.
- Around line 43-48: The current use of shallowRef with graphNodes leads to
missed updates because assigning graphNodes.value = app.rootGraph.nodes keeps
the same array reference; change the assignment in the useEventListener callback
to replace the array with a new reference (e.g., spread into a new array) so
mutations to app.rootGraph.nodes trigger reactivity, or switch graphNodes to a
deep ref instead; update the callback that listens to app.rootGraph.events
'configured' (the function that sets graphNodes.value) to create and assign a
new array rather than the original reference.
- Around line 198-207: The interrupt icon-only Button rendered when
executionStore.isIdle is false lacks an accessible label; update the Button (the
one with v-if="!executionStore.isIdle" and
@click="commandStore.execute('Comfy.Interrupt')") to include a descriptive
accessible attribute such as title="Interrupt" and/or aria-label="Interrupt
workflow" (or a localized string) so screen readers can announce its purpose
while keeping the existing icon and behavior intact.
- Around line 106-134: runButtonClick currently swallows errors because there’s
no catch — add a catch(error) after the try around commandStore.execute to log
the error (console.error('Failed to queue prompt:', error)), surface a user
notification (call your existing toast/error helper or set a new reactive like
jobFinishedQueueError.value = true) and optionally reset any UI state (e.g.,
resetJobToastTimeout if needed); leave the finally to restore
jobFinishedQueue.value = true but ensure the new jobFinishedQueueError flag is
used in the template/UX to indicate failure.
In @src/renderer/extensions/linearMode/mediaTypes.ts:
- Around line 29-33: getMediaType currently returns an empty string when output
is missing which is not type-safe; change it to return undefined instead of ''
and update the function signature to return the union type MediaType | undefined
(or explicitly 'video'|'image'|'audio'|...|undefined) so callers get proper
typing; adjust handling inside getMediaType to return 'video' when
output.isVideo is true or return output.mediaType otherwise, and update any
callers of getMediaType to handle undefined, referencing the getMediaType
function, the ResultItemImpl type, and the mediaType property.
In @src/renderer/extensions/linearMode/OutputHistory.vue:
- Around line 48-55: The watch on selectedIndex currently emits
'updateSelection' using filteredOutputs.value[index] which can be undefined when
filteredOutputs is empty or index is out of range; modify the watch callback in
OutputHistory.vue to validate that filteredOutputs.value exists and index is
within 0 <= index < filteredOutputs.value.length before calling
emit('updateSelection'), and if invalid either skip emitting or emit a safe
fallback (e.g., null/undefined placeholders) so consumers of updateSelection
don't receive an out-of-bounds value.
- Line 32: The call outputs.fetchMediaList() is a floating promise that can
swallow errors; change the call to handle errors explicitly by either awaiting
it inside an async function (e.g., await outputs.fetchMediaList() within an
async setup/mounted handler) or attach a rejection handler like
outputs.fetchMediaList().catch(err => /* log or surface error */), and use the
project logger (or console.error) to record the error; ensure you reference
outputs.fetchMediaList() and update the surrounding lifecycle hook so the
promise is not left unhandled.
- Around line 3-4: OutputHistory.vue currently imports and uses PrimeVue's
ProgressSpinner (import ProgressSpinner ...) which violates the guideline;
replace it by removing the ProgressSpinner import and usage and instead import
and register the shared Tailwind-based Spinner component (use the component name
Spinner in template/registration), update the template to use <Spinner /> where
ProgressSpinner was used, and ensure any props or slots expected from
ProgressSpinner are mapped or adapted on the shared Spinner; leave other logic
that uses computed, ref, useTemplateRef, and watch untouched.
- Around line 243-254: The <img> in OutputHistory.vue (inside the block where
getMediaType(output) === 'images' and the element binds :src="output.url" and
sets @click to update selectedIndex) is missing an alt attribute; add a bound
alt (e.g., :alt="output.alt || output.title || `Image ${index}-${key}`" or a
sensible fallback) so screen readers get meaningful text (use an empty alt only
if the image is purely decorative). Ensure you update the <img> element where
getMediaType(output) and selectedIndex are used to include this :alt binding.
- Around line 140-141: The module-scoped CanvasPointer instance (pointer) and
scrollOffset cause leaks because pointer is created once with document.body and
never cleaned up; move creation into the component lifecycle (create the
CanvasPointer in mounted and store it on the component instance, e.g.
this.pointer) and perform cleanup in beforeUnmount/unmounted by calling the
CanvasPointer's teardown method (dispose()/destroy() or remove its event
listeners) and nulling the reference, and also reset scrollOffset on
mount/unmount so state is not retained across mounts.
In @src/renderer/extensions/linearMode/VideoPreview.vue:
- Around line 9-10: The width and height refs in VideoPreview.vue are
initialized to empty strings causing the UI to show " x " before metadata loads;
update the width and height refs (variables named width and height) to
initialize with a placeholder like '...' or modify the template to conditionally
render the dimension span only after metadata is set (e.g., check the same
reactive flag or that width/height are truthy) so the preview shows a
placeholder or hides dimensions until metadata is available.
- Around line 13-25: The video element currently has no error handling; add a
reactive error state (e.g., videoError) and implement a handler (e.g.,
handleVideoError) that runs on the video's @error event: set videoError to a
descriptive message, clear or reset width and height, and optionally emit an
'error' event or toggle a fallback UI flag; wire this handler into the <video
ref="videoRef" ... @error="handleVideoError" /> and update the template to show
fallback content or an error message when videoError is set.
In @src/views/GraphView.vue:
- Around line 212-214: Update the misleading comment above the if-check that
uses sidebarTabStore.activeSidebarTabId and linearMode.value in GraphView.vue:
change it to state that assets are updated when the assets sidebar is open OR
when linear mode is active (and keep the note that when the sidebar is closed,
AssetsSidebarTab.vue will refresh on mount). Ensure the comment mentions both
conditions (activeSidebarTabId === 'assets' and linearMode.value) so it
accurately reflects the current behavior.
In @src/views/LinearView.vue:
- Line 68: The two empty self-closing <div /> elements inside the SplitterPanel
blocks in LinearView.vue are unnecessary; remove those empty divs at the ends of
the SplitterPanel instances (the ones at lines referenced in the review) so the
template has no redundant empty elements—update the template in the LinearView
component to simply omit those <div /> nodes and ensure markup still renders
correctly.
- Around line 55-60: The two identical @update-selection inline handlers should
be extracted into a single function to avoid duplication: create a function
named handleUpdateSelection that accepts the event tuple and assigns the
unpacked values to selectedItem.value, selectedOutput.value, and
selectedIndex.value, then sets hasPreview.value = false; replace both inline
arrow handlers (the ones currently doing ;[selectedItem, selectedOutput,
selectedIndex] = e and hasPreview = false) with a call to handleUpdateSelection.
| --inverted-background-hover: var(--color-charcoal-600); | ||
| --warning-background: var(--color-gold-400); | ||
| --warning-background-hover: var(--color-gold-500); | ||
| --success-background: var(--color-jade-600); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify color choice consistency with theming pattern.
Both light and dark themes map --success-background to the same color value (--color-jade-600). This deviates from the established pattern where light themes typically use lighter shades (e.g., 400) and dark themes use darker shades (e.g., 600-700):
--warning-background: gold-400 (light) vs gold-600 (dark)--destructive-background: coral-500 (light) vs coral-700 (dark)--primary-background: azure-400 (light) vs azure-600 (dark)
Consider whether line 248 should use var(--color-jade-400) for consistency, though this may be an intentional design decision if jade-600 provides optimal contrast in both themes.
♻️ Proposed refactor for pattern consistency
- --success-background: var(--color-jade-600);
+ --success-background: var(--color-jade-400);Apply this change only to line 248 (light theme) if the lighter shade is preferred for consistency.
Also applies to: 374-374
🤖 Prompt for AI Agents
In @packages/design-system/src/css/style.css at line 248, The light-theme
mapping for the CSS variable --success-background currently uses
var(--color-jade-600), which breaks the established pattern of lighter shades
for light themes; update the light-theme declaration(s) of --success-background
to use var(--color-jade-400) (replace occurrences where --success-background is
set to var(--color-jade-600) in the light-theme block(s)) and keep the
dark-theme mapping at var(--color-jade-600); after change, verify
contrast/accessibility and that any duplicated occurrences (the other occurrence
of --success-background) are updated only for the light-theme variant.
Comment on lines
+26
to
+31
| <slot name="button"> | ||
| <Button size="icon"> | ||
| <i :class="icon ?? 'icon-[lucide--ellipsis]'" /> | ||
| </Button> | ||
| </slot> |
Contributor
There was a problem hiding this comment.
Add accessible label to default trigger button.
The default trigger button is icon-only and lacks an accessible label. Screen reader users won't know what the button does. Add an aria-label attribute or use the title attribute (though aria-label is preferred for accessibility).
♿ Proposed fix to add aria-label
<slot name="button">
- <Button size="icon">
+ <Button size="icon" aria-label="Open menu">
<i :class="icon ?? 'icon-[lucide--ellipsis]'" />
</Button>
</slot>Alternatively, allow consumers to provide the label via a prop if the popover's purpose varies by context.
🤖 Prompt for AI Agents
In @src/components/ui/Popover.vue around lines 26 - 30, The default trigger
inside the <slot name="button"> renders an icon-only <Button> which lacks an
accessible label; update the Popover component to add an aria-label on that
default <Button> (e.g., aria-label="Open popover") and expose a prop (prop name
like defaultTriggerLabel or triggerAriaLabel) so consumers can override the
label; modify the template where <Button size="icon"> is rendered and wire the
new prop into the aria-label attribute, ensuring the icon class logic (<i
:class="icon ?? 'icon-[lucide--ellipsis]'"/>) remains unchanged.
Comment on lines
+243
to
+259
| <img | ||
| v-if="getMediaType(output) === 'images'" | ||
| :class=" | ||
| cn( | ||
| 'p-1 rounded-lg aspect-square object-cover', | ||
| index === selectedIndex[0] && | ||
| key === selectedIndex[1] && | ||
| 'border-2' | ||
| ) | ||
| " | ||
| :src="output.url" | ||
| @click="selectedIndex = [index, key]" |
Contributor
There was a problem hiding this comment.
Image elements lack alt text for accessibility.
The <img> elements at line 243 are missing alt attributes, which is an accessibility issue for screen readers.
Proposed fix
<img
v-if="getMediaType(output) === 'images'"
:class="..."
:src="output.url"
+ :alt="output.filename || t('linearMode.outputImage')"
@click="selectedIndex = [index, key]"
/>🤖 Prompt for AI Agents
In @src/renderer/extensions/linearMode/OutputHistory.vue around lines 243 - 254,
The <img> in OutputHistory.vue (inside the block where getMediaType(output) ===
'images' and the element binds :src="output.url" and sets @click to update
selectedIndex) is missing an alt attribute; add a bound alt (e.g.,
:alt="output.alt || output.title || `Image ${index}-${key}`" or a sensible
fallback) so screen readers get meaningful text (use an empty alt only if the
image is purely decorative). Ensure you update the <img> element where
getMediaType(output) and selectedIndex are used to include this :alt binding.
Comment on lines
+9
to
+10
| const width = ref('') | ||
| const height = ref('') |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Initialize dimension refs with placeholder values to improve UX.
The width and height refs are initialized as empty strings, causing the span to render as " x " before the video metadata loads. Consider initializing with a placeholder like '...' or conditionally rendering the dimension display only after metadata is available.
Proposed improvement
-const width = ref('')
-const height = ref('')
+const width = ref<string | null>(null)
+const height = ref<string | null>(null)And update the template:
- <span class="self-center z-10" v-text="`${width} x ${height}`" />
+ <span v-if="width && height" class="self-center z-10" v-text="`${width} x ${height}`" />📝 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 width = ref('') | |
| const height = ref('') | |
| const width = ref<string | null>(null) | |
| const height = ref<string | null>(null) |
🤖 Prompt for AI Agents
In @src/renderer/extensions/linearMode/VideoPreview.vue around lines 9 - 10, The
width and height refs in VideoPreview.vue are initialized to empty strings
causing the UI to show " x " before metadata loads; update the width and height
refs (variables named width and height) to initialize with a placeholder like
'...' or modify the template to conditionally render the dimension span only
after metadata is set (e.g., check the same reactive flag or that width/height
are truthy) so the preview shows a placeholder or hides dimensions until
metadata is available.
Comment on lines
+13
to
+25
| <video | ||
| ref="videoRef" | ||
| :src | ||
| controls | ||
| v-bind="$attrs" | ||
| @loadedmetadata=" | ||
| () => { | ||
| if (!videoRef) return | ||
| width = `${videoRef.videoWidth}` | ||
| height = `${videoRef.videoHeight}` | ||
| } | ||
| " | ||
| /> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add error handling for video load failures.
The video element lacks error handling for scenarios where the video fails to load (e.g., network errors, unsupported formats, or CORS issues). Consider adding an @error event handler to provide feedback or fallback behavior.
Proposed error handler
Add a reactive error state:
+const hasError = ref(false)Add error handling to the template:
<video
ref="videoRef"
:src
controls
v-bind="$attrs"
@loadedmetadata="..."
+ @error="hasError = true"
/>
+ <span v-if="hasError" class="self-center z-10 text-error">
+ {{ $t('videoPreview.loadError', 'Failed to load video') }}
+ </span>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/renderer/extensions/linearMode/VideoPreview.vue around lines 13 - 25,
The video element currently has no error handling; add a reactive error state
(e.g., videoError) and implement a handler (e.g., handleVideoError) that runs on
the video's @error event: set videoError to a descriptive message, clear or
reset width and height, and optionally emit an 'error' event or toggle a
fallback UI flag; wire this handler into the <video ref="videoRef" ...
@error="handleVideoError" /> and update the template to show fallback content or
an error message when videoError is set.
Comment on lines
212
to
+218
| // Only update assets if the assets sidebar is currently open | ||
| // When sidebar is closed, AssetsSidebarTab.vue will refresh on mount | ||
| if (sidebarTabStore.activeSidebarTabId === 'assets') { | ||
| if (sidebarTabStore.activeSidebarTabId === 'assets' || linearMode.value) { |
Contributor
There was a problem hiding this comment.
Update comment to reflect linear mode condition.
The comment states "Only update assets if the assets sidebar is currently open" but the code now also updates assets when linear mode is active. Update the comment to accurately describe the behavior.
📝 Proposed fix
- // Only update assets if the assets sidebar is currently open
+ // Update assets if the assets sidebar is open or linear mode is active
// When sidebar is closed, AssetsSidebarTab.vue will refresh on mount
if (sidebarTabStore.activeSidebarTabId === 'assets' || linearMode.value) {📝 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
| // Only update assets if the assets sidebar is currently open | |
| // When sidebar is closed, AssetsSidebarTab.vue will refresh on mount | |
| if (sidebarTabStore.activeSidebarTabId === 'assets') { | |
| if (sidebarTabStore.activeSidebarTabId === 'assets' || linearMode.value) { | |
| // Update assets if the assets sidebar is open or linear mode is active | |
| // When sidebar is closed, AssetsSidebarTab.vue will refresh on mount | |
| if (sidebarTabStore.activeSidebarTabId === 'assets' || linearMode.value) { |
🤖 Prompt for AI Agents
In @src/views/GraphView.vue around lines 212 - 214, Update the misleading
comment above the if-check that uses sidebarTabStore.activeSidebarTabId and
linearMode.value in GraphView.vue: change it to state that assets are updated
when the assets sidebar is open OR when linear mode is active (and keep the note
that when the sidebar is closed, AssetsSidebarTab.vue will refresh on mount).
Ensure the comment mentions both conditions (activeSidebarTabId === 'assets' and
linearMode.value) so it accurately reflects the current behavior.
Contributor
There was a problem hiding this comment.
@DrJKL, that works too! Should I delete both comment lines, or would you like to keep the second line about AssetsSidebarTab.vue refreshing on mount? The first line is indeed redundant since the condition is self-documenting.
Comment on lines
55
to
111
| @update-selection=" | ||
| (e) => { | ||
| ;[selectedItem, selectedOutput, selectedIndex] = e | ||
| hasPreview = false | ||
| } | ||
| " |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate event handler logic should be extracted.
The @update-selection handlers at lines 55-60 and 109-114 are identical. Consider extracting to a function for maintainability.
Proposed refactor
Add to script setup:
function handleUpdateSelection(e: [AssetItem, ResultItemImpl, [number, number]]) {
;[selectedItem.value, selectedOutput.value, selectedIndex.value] = e
hasPreview.value = false
}Then in template:
- @update-selection="
- (e) => {
- ;[selectedItem, selectedOutput, selectedIndex] = e
- hasPreview = false
- }
- "
+ @update-selection="handleUpdateSelection"Also applies to: 109-114
🤖 Prompt for AI Agents
In @src/views/LinearView.vue around lines 55 - 60, The two identical
@update-selection inline handlers should be extracted into a single function to
avoid duplication: create a function named handleUpdateSelection that accepts
the event tuple and assigns the unpacked values to selectedItem.value,
selectedOutput.value, and selectedIndex.value, then sets hasPreview.value =
false; replace both inline arrow handlers (the ones currently doing
;[selectedItem, selectedOutput, selectedIndex] = e and hasPreview = false) with
a call to handleUpdateSelection.
DrJKL
reviewed
Jan 8, 2026
src/components/ui/ZoomPane.vue
Outdated
Comment on lines
12
to
19
| watch( | ||
| () => slots.default?.(), | ||
| () => { | ||
| zoom.value = 1 | ||
| panX.value = 0 | ||
| panY.value = 0 | ||
| } | ||
| ) |
DrJKL
reviewed
Jan 8, 2026
DrJKL
reviewed
Jan 8, 2026
Comment on lines
150
to
152
| const extra = app.rootGraph.extra | ||
| if (extra && extra.linearMode === undefined && fromLinearMode) | ||
| canvasStore.linearMode = extra.linearMode = true |
DrJKL
reviewed
Jan 8, 2026
Contributor
There was a problem hiding this comment.
Maybe this: https://vueuse.org/core/useDropZone/#usedropzone
Collaborator
Author
There was a problem hiding this comment.
Super sleek, but the existing litegraph handlers aren't compatible with useDropZone's checkValidity.
Maybe next time :(
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
@DrJKL, I'll open a tracking issue for this.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to track potential migration to VueUse's useDropZone
gh issue create \
--repo Comfy-Org/ComfyUI_frontend \
--title "Consider migrating DropZone to VueUse's useDropZone" \
--body "## Context
In PR #7734, a custom \`DropZone\` component was introduced at \`src/renderer/extensions/linearMode/DropZone.vue\` to handle drag-and-drop interactions.
## Suggestion
Use VueUse's [\`useDropZone\`](https://vueuse.org/core/useDropZone/#usedropzone) composable instead of the custom implementation for better maintainability and standardization.
## Blocker
The existing litegraph handlers are not compatible with \`useDropZone\`'s \`checkValidity\` function signature, making immediate migration not feasible.
## References
- PR: #7734
- Discussion: https://github.com/Comfy-Org/ComfyUI_frontend/pull/7734#discussion_r2673665454
- Requested by: @DrJKL" \
--assignee DrJKL
Length of output: 57
@DrJKL, I've created tracking issue #7971 for considering migration to VueUse's useDropZone in a future refactor. The issue includes context about the current incompatibility with litegraph handlers and has been assigned to you.
DrJKL
reviewed
Jan 8, 2026
| start: resetJobToastTimeout, | ||
| stop: stopJobTimeout | ||
| } = useTimeout(5000, { controls: true }) | ||
| stopJobTimeout() |
Contributor
There was a problem hiding this comment.
Should be able to do immediate: false
DrJKL
reviewed
Jan 8, 2026
DrJKL
reviewed
Jan 8, 2026
DrJKL
reviewed
Jan 8, 2026
DrJKL
reviewed
Jan 8, 2026
DrJKL
reviewed
Jan 8, 2026
AustinMroz
requested review from
a team,
KarryCharon,
Myestery,
Yorha4D,
christian-byrne,
comfyui-wiki and
shinshin86
as code owners
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/composables/graph/useGraphNodeManager.ts (1)
340-343: Potential duplicate extraction during initialization.When
syncWithGraphis called (line 561), it callsextractVueNodeDatafor all nodes (line 342). Then immediately after, the loop at lines 575-580 callsgraph.onNodeAddedfor each node, which triggershandleNodeAdded→extractVueNodeDataagain. This results in double extraction for all initial nodes.Proposed fix
Consider either:
- Removing the loop at lines 575-580 since
syncWithGraphalready processes existing nodes, or- Removing the
syncWithGraphcall and relying solely on the loop.// Set up event listeners immediately const cleanup = setupEventListeners() - // Process any existing nodes after event listeners are set up - if (graph._nodes && graph._nodes.length > 0) { - graph._nodes.forEach((node: LGraphNode) => { - if (graph.onNodeAdded) { - graph.onNodeAdded(node) - } - }) - }Or alternatively, remove
syncWithGraph()fromsetupEventListenersand keep the loop.
🤖 Fix all issues with AI agents
In @src/components/sidebar/ModeToggle.vue:
- Around line 4-8: Instantiate commandStore by calling useCommandStore() in the
same script section where canvasStore is created (move the useCommandStore()
call out of the template), e.g. add const commandStore = useCommandStore() next
to const canvasStore = useCanvasStore(), then update the template @click
handlers to reference commandStore instead of calling useCommandStore() inline
(ensure any places mentioned around the ModeToggle component's click handlers
use commandStore).
In @src/components/topbar/WorkflowTab.vue:
- Around line 19-22: The linear mode icon <i> rendered when
workflowOption.workflow.activeState?.extra?.linearMode is true lacks
accessibility semantics; update the <i> element in WorkflowTab.vue to either
include an aria-label using the i18n key sideToolbar.workflowTab.linearMode (add
the entry "linearMode": "Simple Mode" to src/locales/en/main.json) so screen
readers announce it, or mark the element as aria-hidden="true" if it is purely
decorative. Ensure the chosen approach is applied to the exact <i> node gated by
workflowOption.workflow.activeState?.extra?.linearMode.
In @src/composables/graph/useGraphNodeManager.ts:
- Line 218: The computed callback for reactiveComputed is mutating the shared
slotMetadata Map (slotMetadata) and never clears stale entries, causing impure
computed behavior and lingering data; refactor so the computed is pure by
creating and returning a new Map (or a plain object/array) of WidgetSlotMetadata
inside the reactiveComputed callback instead of mutating the outer slotMetadata,
or alternatively clear slotMetadata at the start of the update
(slotMetadata.clear()) and then populate it from fresh inputs; ensure any
watchers that rely on slotMetadata consume the new returned value or the updated
cleared map so no stale widget entries remain.
- Around line 210-211: The function extractVueNodeData is misleading because it
mutates the passed LGraphNode (replacing node.widgets and node.inputs with
reactive proxies); rename the function to something like createVueNodeData or
initializeVueNodeData (or alternatively update the JSDoc) to indicate it mutates
the input, and update all callers/imports to use the new name and ensure
tests/types reflect the change; specifically modify the exported function name
export function extractVueNodeData(...) to the chosen name and update any
references to that symbol across the codebase, or add a clear JSDoc above
extractVueNodeData stating that it mutates the node by reassigning
widgets/inputs to reactive proxies.
In @src/renderer/extensions/linearMode/LinearControls.vue:
- Around line 277-281: Remove the dead Teleport block (the Teleport element with
v-if="false" and its child div) from LinearControls.vue; delete the unused
:to="notesTo" binding and the entire Teleport node, and then run a quick search
for notesTo in the component to remove or repurpose the notesTo prop/variable if
it is now unused elsewhere.
- Line 193: Remove the dead Button element that uses v-if="false" in
LinearControls.vue; either delete the whole <Button v-if="false"> {{
t('menuLabels.publish') }} </Button> line, or replace the hardcoded false with a
real condition/prop (e.g., a showPublishButton prop or a feature-flag check) and
wire that prop into the component; if it's intended as a future placeholder,
remove the JSX and instead add a TODO comment and create an issue tracking the
feature.
- Around line 130-131: The Tailwind class string `*:[.min-w-0]:w-24
grid-cols-[auto_96px]!` inside LinearControls.vue uses fragile arbitrary
selector syntax and the `!` important prefix; remove the `*:[.min-w-0]:w-24` and
the trailing `!` and stop using arbitrary selector specificity. Instead,
refactor the widget usage to accept a dedicated class prop for the inner element
(e.g., add/consume an `innerClass` or `contentClass` prop on the widget
component used in LinearControls.vue) and move the needed classes (`min-w-0
w-24` and `grid-cols-[auto_96px]`) into that prop or apply them directly to the
inner element so styling is explicit and maintainable.
- Around line 79-84: The batchCountWidget object contains an unused value
property while v-model is bound to store state (batchCount), so remove the
redundant value: 1 from the batchCountWidget declaration; if the
SimplifiedWidget type currently requires value, either update SimplifiedWidget
to make value optional or remove the trailing as const on batchCountWidget so
the object shape matches usage; ensure WidgetInputNumberInput still receives the
widget options/name and v-model continues to use batchCount.
In @src/renderer/extensions/linearMode/LinearPreview.vue:
- Around line 87-96: The rerun function uses a brittle 500ms timeout to wait for
graph updates; instead, make the synchronization deterministic by having
loadWorkflow either return a Promise that resolves when app.rootGraph is fully
updated (e.g., after it emits a "ready" or "updated" event) or by awaiting a
graph-ready signal (listen for app.rootGraph.on('ready') or similar) or using
Vue's nextTick/watchEffect to detect stabilization; then replace the setTimeout
with awaiting that promise/event before calling
executeWidgetsCallback(collectAllNodes(app.rootGraph), 'afterQueued') and
finally invoke runButtonClick(e).
- Around line 119-122: Replace the unnecessary arrow wrapper on the click
handler in LinearPreview.vue; instead of @click="() =>
loadWorkflow(selectedItem)" call the method directly with the argument by using
@click="loadWorkflow(selectedItem)". This removes the redundant lambda while
preserving the selectedItem argument and keeps the handler bound to the
component method loadWorkflow.
- Around line 154-158: The ImagePreview rendering can crash because the v-if
uses the nullish coalescing (latentPreview ?? getMediaType(selectedOutput) ===
'images') but the src still force-unwraps selectedOutput with
selectedOutput!.url; change the rendering so src never reads selectedOutput when
it's undefined — e.g., make src use latentPreview ?? selectedOutput?.url and
tighten the v-if to either check latentPreview || (selectedOutput &&
getMediaType(selectedOutput) === 'images'), or split into two branches: one that
renders when latentPreview is present and another that renders only when
selectedOutput exists and is an image; update the ImagePreview usage accordingly
to avoid the non-null assertion on selectedOutput.
In @src/renderer/extensions/linearMode/OutputHistory.vue:
- Line 33: The call to outputs.fetchMediaList() in OutputHistory.vue currently
discards its promise with void, which will swallow any errors; update the call
to handle errors explicitly by either awaiting the promise inside an async
setup/method and wrapping it in try/catch (logging errors via the component
logger) or chain a .catch(...) to outputs.fetchMediaList() to log/report any
exception; if the fetchMediaList composable already handles errors internally,
add a comment clarifying that to avoid future silent failures.
- Around line 49-56: There are two watchers on selectedIndex; replace them with
a single watch(selectedIndex, (newVal, oldVal) => { ... }) that performs both
effects: extract the index (const [index] = newVal || selectedIndex.value), call
emit('updateSelection', [ outputs.media.value[index], selectedOutput.value,
index <= 0 ]) and also include whatever logic currently in the other
selectedIndex watcher so both behaviors run from this one handler; preserve
reactivity options (immediate/deep) used previously and keep the same variable
references (selectedIndex, outputs.media, selectedOutput, emit).
- Around line 295-307: The Button in OutputHistory.vue is an icon-only control
and needs an accessible name: add an aria-label prop to the Button that uses the
i18n string (e.g., $t('outputHistory.resetScroll') ) and wire the label to the
existing click handler resetOutputsScroll; then add the corresponding i18n
key/value (e.g., "outputHistory.resetScroll": "Scroll to top" or localized
translations) to your locale files so the aria-label resolves for all languages.
- Around line 150-151: The module-level mutable variables pointer and
scrollOffset create shared state across component instances; move their
declarations into the component instance scope (e.g., inside the component's
setup() or data() function) or convert them to reactive refs (ref/ reactive) so
each OutputHistory instance gets its own CanvasPointer and scrollOffset; update
any usages of pointer and scrollOffset in methods, lifecycle hooks, and cleanup
to reference the instance-scoped variables (e.g., this.pointer or local refs
returned from setup) and ensure CanvasPointer is constructed per-instance and
destroyed on unmount.
In @src/views/LinearView.vue:
- Around line 74-81: Extract the duplicated inline selection logic into a single
reusable function (e.g., handleSelectionUpdate) that accepts the tuple ([item,
output, canShow]) and sets selectedItem.value, selectedOutput.value,
canShowPreview.value, and resets hasPreview.value = false; then replace the
three inline @update-selection handlers with a reference to
handleSelectionUpdate so all OutputHistory components call the same handler.
- Line 119: Remove the unnecessary self-closing empty div elements found inside
the SplitterPanel instances in LinearView.vue (the occurrences of "<div />");
either delete those empty nodes or replace them by applying appropriate Tailwind
spacing/layout utilities to the parent SplitterPanel or its children (e.g.,
padding/margin/flex classes) so no empty placeholder elements remain; update
both occurrences where the empty div is present.
- Around line 29-33: The hasPreview boolean should be derived, not set via
side-effects: replace the whenever(...) / ref-based pattern with a computed that
returns Boolean(nodeOutputStore.latestPreview[0]) (or
nodeOutputStore.latestPreview.length > 0) and remove the manual hasPreview.value
assignments in the event handlers; update any code that previously reset
hasPreview to rely on the computed value instead (or clear
nodeOutputStore.latestPreview in the same handlers if intended), ensuring all
references now use the computed hasPreview.
Comment on lines
+4
to
+8
| import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' | ||
| import { useCommandStore } from '@/stores/commandStore' | ||
|
|
||
| const canvasStore = useCanvasStore() | ||
| </script> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move useCommandStore() instantiation to the script section for consistency.
canvasStore is correctly instantiated in the script section, but useCommandStore() is called inline within the template's @click handlers. For consistency and to avoid repeated store lookups in the template, instantiate commandStore alongside canvasStore.
♻️ Suggested refactor
import Button from '@/components/ui/button/Button.vue'
import { t } from '@/i18n'
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
import { useCommandStore } from '@/stores/commandStore'
const canvasStore = useCanvasStore()
+const commandStore = useCommandStore()Then update the click handlers:
- @click="useCommandStore().execute('Comfy.ToggleLinear')"
+ @click="commandStore.execute('Comfy.ToggleLinear')"Also applies to: 15-15, 23-23
🤖 Prompt for AI Agents
In @src/components/sidebar/ModeToggle.vue around lines 4 - 8, Instantiate
commandStore by calling useCommandStore() in the same script section where
canvasStore is created (move the useCommandStore() call out of the template),
e.g. add const commandStore = useCommandStore() next to const canvasStore =
useCanvasStore(), then update the template @click handlers to reference
commandStore instead of calling useCommandStore() inline (ensure any places
mentioned around the ModeToggle component's click handlers use commandStore).
Comment on lines
+19
to
+22
| <i | ||
| v-else-if="workflowOption.workflow.activeState?.extra?.linearMode" | ||
| class="icon-[lucide--panels-top-left] bg-primary-background" | ||
| /> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding accessibility attributes to the linear mode icon.
This icon conveys that the workflow is in linear mode, but screen reader users have no way to access this information. Based on learnings, elements without visible labels should have an aria-label.
♿ Suggested accessibility improvement
<i
v-else-if="workflowOption.workflow.activeState?.extra?.linearMode"
class="icon-[lucide--panels-top-left] bg-primary-background"
+ :aria-label="t('sideToolbar.workflowTab.linearMode')"
/>Add the corresponding i18n entry to src/locales/en/main.json:
{
"sideToolbar": {
"workflowTab": {
"linearMode": "Simple Mode"
}
}
}Alternatively, if this is purely a visual indicator and not essential information, use aria-hidden="true" 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
| <i | |
| v-else-if="workflowOption.workflow.activeState?.extra?.linearMode" | |
| class="icon-[lucide--panels-top-left] bg-primary-background" | |
| /> | |
| <i | |
| v-else-if="workflowOption.workflow.activeState?.extra?.linearMode" | |
| class="icon-[lucide--panels-top-left] bg-primary-background" | |
| :aria-label="t('sideToolbar.workflowTab.linearMode')" | |
| /> |
🤖 Prompt for AI Agents
In @src/components/topbar/WorkflowTab.vue around lines 19 - 22, The linear mode
icon <i> rendered when workflowOption.workflow.activeState?.extra?.linearMode is
true lacks accessibility semantics; update the <i> element in WorkflowTab.vue to
either include an aria-label using the i18n key
sideToolbar.workflowTab.linearMode (add the entry "linearMode": "Simple Mode" to
src/locales/en/main.json) so screen readers announce it, or mark the element as
aria-hidden="true" if it is purely decorative. Ensure the chosen approach is
applied to the exact <i> node gated by
workflowOption.workflow.activeState?.extra?.linearMode.
Comment on lines
+210
to
+211
| // Extract safe data from LiteGraph node for Vue consumption | ||
| export function extractVueNodeData(node: LGraphNode): VueNodeData { |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Function name is misleading - it mutates the input node.
extractVueNodeData suggests a pure extraction operation, but it actually mutates the passed node by redefining its widgets and inputs properties with reactive proxies. Consider renaming to better reflect this behavior (e.g., createVueNodeData or initializeVueNodeData), or document the mutation in the function's JSDoc.
🤖 Prompt for AI Agents
In @src/composables/graph/useGraphNodeManager.ts around lines 210 - 211, The
function extractVueNodeData is misleading because it mutates the passed
LGraphNode (replacing node.widgets and node.inputs with reactive proxies);
rename the function to something like createVueNodeData or initializeVueNodeData
(or alternatively update the JSDoc) to indicate it mutates the input, and update
all callers/imports to use the new name and ensure tests/types reflect the
change; specifically modify the exported function name export function
extractVueNodeData(...) to the chosen name and update any references to that
symbol across the codebase, or add a clear JSDoc above extractVueNodeData
stating that it mutates the node by reassigning widgets/inputs to reactive
proxies.
| ? String(node.graph.id) | ||
| : null | ||
| // Extract safe widget data | ||
| const slotMetadata = new Map<string, WidgetSlotMetadata>() |
Contributor
There was a problem hiding this comment.
Side effect inside reactiveComputed and stale slotMetadata entries.
The slotMetadata map is mutated inside the computed callback (lines 240-246), which violates the principle that computed values should be pure. Additionally, the map is never cleared between re-computations, so if an input previously had a widget but is later removed, the stale entry persists.
Proposed fix
const safeWidgets = reactiveComputed<SafeWidgetData[]>(() => {
+ slotMetadata.clear()
node.inputs?.forEach((input, index) => {
if (!input?.widget?.name) return
slotMetadata.set(input.widget.name, {
index,
linked: input.link != null
})
})
return node.widgets?.map(safeWidgetMapper(node, slotMetadata)) ?? []
})Also applies to: 239-248
🤖 Prompt for AI Agents
In @src/composables/graph/useGraphNodeManager.ts at line 218, The computed
callback for reactiveComputed is mutating the shared slotMetadata Map
(slotMetadata) and never clears stale entries, causing impure computed behavior
and lingering data; refactor so the computed is pure by creating and returning a
new Map (or a plain object/array) of WidgetSlotMetadata inside the
reactiveComputed callback instead of mutating the outer slotMetadata, or
alternatively clear slotMetadata at the start of the update
(slotMetadata.clear()) and then populate it from fresh inputs; ensure any
watchers that rely on slotMetadata consume the new returned value or the updated
cleared map so no stale widget entries remain.
Comment on lines
+220
to
+237
| const reactiveWidgets = shallowReactive<IBaseWidget[]>(node.widgets ?? []) | ||
| Object.defineProperty(node, 'widgets', { | ||
| get() { | ||
| return reactiveWidgets | ||
| }, | ||
| set(v) { | ||
| reactiveWidgets.splice(0, reactiveWidgets.length, ...v) | ||
| } | ||
| }) | ||
| const reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? []) | ||
| Object.defineProperty(node, 'inputs', { | ||
| get() { | ||
| return reactiveInputs | ||
| }, | ||
| set(v) { | ||
| reactiveInputs.splice(0, reactiveInputs.length, ...v) | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
Multiple calls to extractVueNodeData create redundant reactive wrappers.
This function is called multiple times on the same node (in syncWithGraph, handleNodeAdded, and onAfterGraphConfigured). On subsequent calls, node.widgets already returns the reactive array from the first call, which then gets wrapped in another shallowReactive. This creates nested reactivity and potential memory/performance issues.
Consider guarding against re-wrapping or extracting the reactive setup to a separate function called only once per node:
Proposed fix
+const REACTIVE_MARKER = Symbol('vueReactive')
+
export function extractVueNodeData(node: LGraphNode): VueNodeData {
const subgraphId =
node.graph && 'id' in node.graph && node.graph !== node.graph.rootGraph
? String(node.graph.id)
: null
const slotMetadata = new Map<string, WidgetSlotMetadata>()
- const reactiveWidgets = shallowReactive<IBaseWidget[]>(node.widgets ?? [])
- Object.defineProperty(node, 'widgets', {
- get() {
- return reactiveWidgets
- },
- set(v) {
- reactiveWidgets.splice(0, reactiveWidgets.length, ...v)
- }
- })
- const reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? [])
- Object.defineProperty(node, 'inputs', {
- get() {
- return reactiveInputs
- },
- set(v) {
- reactiveInputs.splice(0, reactiveInputs.length, ...v)
- }
- })
+ // Only wrap once to avoid nested reactivity
+ let reactiveWidgets: IBaseWidget[]
+ let reactiveInputs: INodeInputSlot[]
+
+ if ((node as any)[REACTIVE_MARKER]) {
+ reactiveWidgets = node.widgets ?? []
+ reactiveInputs = node.inputs ?? []
+ } else {
+ reactiveWidgets = shallowReactive<IBaseWidget[]>(node.widgets ?? [])
+ Object.defineProperty(node, 'widgets', {
+ get() { return reactiveWidgets },
+ set(v) { reactiveWidgets.splice(0, reactiveWidgets.length, ...v) }
+ })
+ reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? [])
+ Object.defineProperty(node, 'inputs', {
+ get() { return reactiveInputs },
+ set(v) { reactiveInputs.splice(0, reactiveInputs.length, ...v) }
+ })
+ ;(node as any)[REACTIVE_MARKER] = true
+ }
Comment on lines
+173
to
+185
| useEventListener(document.body, 'keydown', (e: KeyboardEvent) => { | ||
| if ( | ||
| (e.key !== 'ArrowDown' && e.key !== 'ArrowUp') || | ||
| e.target instanceof HTMLTextAreaElement || | ||
| e.target instanceof HTMLInputElement | ||
| ) | ||
| return | ||
|
|
||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| if (e.key === 'ArrowDown') gotoNextOutput() | ||
| else gotoPreviousOutput() | ||
| }) |
Contributor
There was a problem hiding this comment.
Global keyboard listener lacks cleanup and scope isolation.
The useEventListener on document.body captures all arrow key events globally. While input/textarea elements are filtered, this could still interfere with other components. Consider scoping to the component's root element or adding focus-based activation.
Note: useEventListener from VueUse automatically cleans up on component unmount, so there's no memory leak, but the global scope may cause unintended side effects.
Comment on lines
+295
to
+307
| <Button | ||
| :class=" | ||
| cn( | ||
| 'p-3 size-10 bg-base-foreground', | ||
| settingStore.get('Comfy.Sidebar.Location') === 'left' | ||
| ? 'left-4' | ||
| : 'right-4' | ||
| ) | ||
| " | ||
| @click="resetOutputsScroll" | ||
| > | ||
| <i class="icon-[lucide--arrow-up] size-4 bg-base-background" /> | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Icon-only button lacks accessible label.
The scroll-reset button contains only an icon without an aria-label. Per accessibility guidelines, icon-only buttons should have an accessible name.
♻️ Proposed fix
<Button
:class="..."
@click="resetOutputsScroll"
+ :aria-label="t('linearMode.scrollToTop')"
>
<i class="icon-[lucide--arrow-up] size-4 bg-base-background" />
</Button>Add the corresponding i18n key for the label.
🤖 Prompt for AI Agents
In @src/renderer/extensions/linearMode/OutputHistory.vue around lines 295 - 307,
The Button in OutputHistory.vue is an icon-only control and needs an accessible
name: add an aria-label prop to the Button that uses the i18n string (e.g.,
$t('outputHistory.resetScroll') ) and wire the label to the existing click
handler resetOutputsScroll; then add the corresponding i18n key/value (e.g.,
"outputHistory.resetScroll": "Scroll to top" or localized translations) to your
locale files so the aria-label resolves for all languages.
Comment on lines
+29
to
+33
| const hasPreview = ref(false) | ||
| whenever( | ||
| () => nodeOutputStore.latestPreview[0], | ||
| () => (hasPreview.value = true) | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using computed instead of whenever with side-effect.
The hasPreview state could be derived more declaratively. The current pattern sets hasPreview = true reactively but relies on event handlers to reset it, which is scattered logic.
The current approach works but tracking preview availability through both whenever and manual resets in three event handlers creates scattered state management.
🤖 Prompt for AI Agents
In @src/views/LinearView.vue around lines 29 - 33, The hasPreview boolean should
be derived, not set via side-effects: replace the whenever(...) / ref-based
pattern with a computed that returns Boolean(nodeOutputStore.latestPreview[0])
(or nodeOutputStore.latestPreview.length > 0) and remove the manual
hasPreview.value assignments in the event handlers; update any code that
previously reset hasPreview to rely on the computed value instead (or clear
nodeOutputStore.latestPreview in the same handlers if intended), ensuring all
references now use the computed hasPreview.
Comment on lines
+74
to
+81
| @update-selection=" | ||
| ([item, output, canShow]) => { | ||
| selectedItem = item | ||
| selectedOutput = output | ||
| canShowPreview = canShow | ||
| hasPreview = false | ||
| } | ||
| " |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate event handler logic for update-selection.
The same selection update logic is repeated three times for different OutputHistory instances. Extract to a reusable handler function.
♻️ Proposed refactor
function handleSelectionUpdate([item, output, canShow]: [
AssetItem | undefined,
ResultItemImpl | undefined,
boolean
]) {
selectedItem.value = item
selectedOutput.value = output
canShowPreview.value = canShow
hasPreview.value = false
}Then use in templates:
- @update-selection="
- ([item, output, canShow]) => {
- selectedItem = item
- selectedOutput = output
- canShowPreview = canShow
- hasPreview = false
- }
- "
+ @update-selection="handleSelectionUpdate"Also applies to: 104-111, 170-177
🤖 Prompt for AI Agents
In @src/views/LinearView.vue around lines 74 - 81, Extract the duplicated inline
selection logic into a single reusable function (e.g., handleSelectionUpdate)
that accepts the tuple ([item, output, canShow]) and sets selectedItem.value,
selectedOutput.value, canShowPreview.value, and resets hasPreview.value = false;
then replace the three inline @update-selection handlers with a reference to
handleSelectionUpdate so all OutputHistory components call the same handler.
| :toast-to="unrefElement(bottomLeftRef) ?? undefined" | ||
| :notes-to="unrefElement(topLeftRef) ?? undefined" | ||
| /> | ||
| <div /> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Empty <div /> elements appear to be unnecessary.
These self-closing divs at the end of SplitterPanels don't seem to serve a purpose. If they're spacers or layout fixers, consider using Tailwind utilities instead.
♻️ Proposed removal if not needed
/>
- <div />
</SplitterPanel>Also applies to: 179-179
🤖 Prompt for AI Agents
In @src/views/LinearView.vue at line 119, Remove the unnecessary self-closing
empty div elements found inside the SplitterPanel instances in LinearView.vue
(the occurrences of "<div />"); either delete those empty nodes or replace them
by applying appropriate Tailwind spacing/layout utilities to the parent
SplitterPanel or its children (e.g., padding/margin/flex classes) so no empty
placeholder elements remain; update both occurrences where the empty div is
present.
workflowData is a clone of an already initialized graph, as a result, cahnging it changes the initial state without modifying the active state of the graph. In the ideal case, where the linear flag should be set, this should be the other way around. A save should be possible to apply this modified flag, rather than discarding it. This flag is not set to false on load of a normal workflow. Setting linear to false requires explicit toggling of linear state. This ensures that workflows do not become modified on load unless the user is actively utilizing linear mode.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/platform/workflow/core/services/workflowService.ts:
- Around line 341-344: Cache the Pinia store once inside afterLoadNewGraph
instead of calling useCanvasStore() multiple times: assign const canvasStore =
useCanvasStore() at the top of afterLoadNewGraph and replace subsequent calls
(e.g., useCanvasStore().linearMode checks at lines where app.rootGraph.extra is
set) with canvasStore.linearMode so you reuse the singleton reference and avoid
redundant calls when setting app.rootGraph.extra.linearMode.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/platform/workflow/core/services/workflowService.ts
🧰 Additional context used
📓 Path-based instructions (7)
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/platform/workflow/core/services/workflowService.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/platform/workflow/core/services/workflowService.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/workflow/core/services/workflowService.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/platform/workflow/core/services/workflowService.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/platform/workflow/core/services/workflowService.ts
🧠 Learnings (7)
📚 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/platform/workflow/core/services/workflowService.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.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/platform/workflow/core/services/workflowService.ts
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/platform/workflow/core/services/workflowService.ts
🧬 Code graph analysis (1)
src/platform/workflow/core/services/workflowService.ts (2)
src/renderer/core/canvas/canvasStore.ts (1)
useCanvasStore(24-152)src/scripts/app.ts (1)
app(1746-1746)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: playwright-tests-chromium-sharded (7, 8)
- GitHub Check: playwright-tests-chromium-sharded (2, 8)
- GitHub Check: playwright-tests-chromium-sharded (6, 8)
- GitHub Check: playwright-tests-chromium-sharded (4, 8)
- GitHub Check: playwright-tests-chromium-sharded (8, 8)
- GitHub Check: playwright-tests-chromium-sharded (1, 8)
- GitHub Check: playwright-tests-chromium-sharded (3, 8)
- GitHub Check: playwright-tests-chromium-sharded (5, 8)
- GitHub Check: playwright-tests (mobile-chrome)
- GitHub Check: playwright-tests (chromium-2x)
- GitHub Check: playwright-tests (chromium-0.5x)
- GitHub Check: test
- GitHub Check: collect
- GitHub Check: lint-and-format
🔇 Additional comments (2)
src/platform/workflow/core/services/workflowService.ts (2)
14-14: LGTM!Import is correctly placed and necessary for the linearMode synchronization logic.
315-319: > Likely an incorrect or invalid review comment.
Comment on lines
+341
to
+344
| if (useCanvasStore().linearMode) { | ||
| app.rootGraph.extra ??= {} | ||
| app.rootGraph.extra.linearMode = true | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider caching the store reference.
The logic is correct, but useCanvasStore() is called multiple times within afterLoadNewGraph (lines 319 and 341). While Pinia stores are singletons, caching the reference is more efficient and cleaner.
♻️ Suggested refactor to cache store reference
At the top of afterLoadNewGraph:
const afterLoadNewGraph = async (
value: string | ComfyWorkflow | null,
workflowData: ComfyWorkflowJSON
) => {
const workflowStore = useWorkspaceStore().workflow
+ const canvasStore = useCanvasStore()
if (
workflowData.extra?.linearMode !== undefined ||
- !workflowData.nodes.length
+ !workflowData.nodes?.length
)
- useCanvasStore().linearMode = !!workflowData.extra?.linearMode
+ canvasStore.linearMode = !!workflowData.extra?.linearModeAnd later:
- if (useCanvasStore().linearMode) {
+ if (canvasStore.linearMode) {
app.rootGraph.extra ??= {}
app.rootGraph.extra.linearMode = true
}🤖 Prompt for AI Agents
In @src/platform/workflow/core/services/workflowService.ts around lines 341 -
344, Cache the Pinia store once inside afterLoadNewGraph instead of calling
useCanvasStore() multiple times: assign const canvasStore = useCanvasStore() at
the top of afterLoadNewGraph and replace subsequent calls (e.g.,
useCanvasStore().linearMode checks at lines where app.rootGraph.extra is set)
with canvasStore.linearMode so you reuse the singleton reference and avoid
redundant calls when setting app.rootGraph.extra.linearMode.
Collaborator
Author
|
Test failure is unrelated. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/locales/en/main.json:
- Line 1188: The menu label value for the key "toggle linear mode" should use
title case to match other menu labels; update the JSON entry so the value is
"Toggle Simple Mode" (i.e., change the value for the "toggle linear mode" key
from "toggle simple mode" to "Toggle Simple Mode") without altering the key
itself.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/locales/en/main.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/locales/en/main.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (3)
src/locales/en/main.json (3)
193-193: LGTM!The Vue i18n pluralization format is correct:
{zero} | {one} | {more}properly handles "0 assets", "1 asset", and "N assets" cases.
681-682: LGTM!New
filterTextkey follows the existing naming convention (filterImage,filterVideo,filterAudio,filter3D) and supports the PR's addition of non-image output handling.
2479-2487: LGTM!The new localization keys for Simple Mode are well-structured and provide clear, user-friendly labels for the new UI features (mode toggle, drag-and-drop, run controls, and batch actions).
| "Canvas Performance": "Canvas Performance", | ||
| "Help Center": "Help Center", | ||
| "toggle linear mode": "toggle linear mode", | ||
| "toggle linear mode": "toggle simple mode", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider capitalizing for consistency with other menu labels.
Other menu labels in menuLabels use title case (e.g., "Toggle Theme (Dark/Light)", "Toggle Queue Panel V2"). For consistency, this could be "Toggle Simple Mode".
💡 Suggested change
- "toggle linear mode": "toggle simple mode",
+ "toggle linear mode": "Toggle Simple Mode",📝 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
| "toggle linear mode": "toggle simple mode", | |
| "toggle linear mode": "Toggle Simple Mode", |
🤖 Prompt for AI Agents
In @src/locales/en/main.json at line 1188, The menu label value for the key
"toggle linear mode" should use title case to match other menu labels; update
the JSON entry so the value is "Toggle Simple Mode" (i.e., change the value for
the "toggle linear mode" key from "toggle simple mode" to "Toggle Simple Mode")
without altering the key itself.
You shouldn't see slots in linear mode
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/composables/graph/useGraphNodeManager.ts`:
- Line 296: The assignment for badges uses a direct reference (badges =
node.badges) while outputs is shallow-copied; change badges to be shallow-copied
like outputs to avoid external mutations leaking into VueNodeData.badges —
replace the direct reference with a shallow copy (e.g., [...node.badges]) where
VueNodeData.badges is constructed in useGraphNodeManager.ts so badges and
outputs are treated consistently.
- Around line 277-286: The reactiveComputed created for safeWidgets inside
extractVueNodeData is never disposed, causing orphaned subscriptions on repeated
calls; fix by tracking and stopping the previous computed before creating a new
one (e.g., store the stop/teardown function in a WeakMap keyed by the node or
attach a hidden property like node.__safeWidgetsStop), call that stop function
if present, then create the new reactiveComputed for safeWidgets and save its
stop handle; ensure this cleanup happens every time extractVueNodeData runs so
prior subscriptions are disposed.
♻️ Duplicate comments (12)
src/renderer/extensions/vueNodes/components/LGraphNode.vue (1)
43-43: Verify reactivity of theme-dependent background color.A past review flagged a potential reactivity gap: calling
applyLightThemeColor(nodeData.bgcolor)directly in the template only tracksnodeData.bgcoloras a Vue dependency, not any internal store access within the utility function. The AI summary indicates the utility was refactored to remove palette-store dependencies—please verify the updated implementation ensures reactivity when the color palette (light/dark theme) changes.#!/bin/bash # Check the implementation of applyLightThemeColor to verify reactivity handling echo "=== Checking applyLightThemeColor implementation ===" cat src/renderer/extensions/vueNodes/utils/nodeStyleUtils.ts echo "" echo "=== Checking if useColorPaletteStore is still used in nodeStyleUtils ===" rg -n "useColorPaletteStore|light_theme|completedActivePalette" src/renderer/extensions/vueNodes/utils/nodeStyleUtils.ts echo "" echo "=== Checking how applyLightThemeColor handles theme state ===" ast-grep --pattern 'function applyLightThemeColor($$$) { $$$ }' ast-grep --pattern 'const applyLightThemeColor = ($$$) => { $$$ }'src/locales/en/main.json (1)
1190-1190: Capitalize for consistency with other menu labels.Other entries in
menuLabelsuse title case (e.g., "Toggle Theme (Dark/Light)", "Toggle Queue Panel V2"). This should be "Toggle Simple Mode" for consistency.💡 Suggested fix
- "toggle linear mode": "toggle simple mode", + "toggle linear mode": "Toggle Simple Mode",src/renderer/extensions/linearMode/LinearControls.vue (8)
45-50:shallowRefassignment may not trigger reactivity on array mutations.Assigning the same array reference to
shallowRefwon't trigger updates whenapp.rootGraph.nodesis mutated in-place.🐛 Proposed fix
const graphNodes = shallowRef<LGraphNode[]>(app.rootGraph.nodes) useEventListener( app.rootGraph.events, 'configured', - () => (graphNodes.value = app.rootGraph.nodes) + () => (graphNodes.value = [...app.rootGraph.nodes]) )
81-86: Unusedvalueproperty and incorrectas constassertion.The
value: 1is never used sincev-model="batchCount"binds to store state. Also,as constassertion on an object with mutable semantics is misleading.♻️ Suggested fix
-const batchCountWidget: SimplifiedWidget<number> = { +const batchCountWidget = { options: { precision: 0, min: 1, max: isCloud ? 4 : 99 }, - value: 1, name: t('linearMode.runCount'), type: 'number' -} as const +} satisfies Omit<SimplifiedWidget<number>, 'value'>
88-118: MoveuseTelemetry()to setup level and add error handling.Two issues with
runButtonClick:
useTelemetry()is called inside the event handler—composables should be called at setup level- Errors from
commandStore.executeare silently swallowed (noted in TODO)🐛 Proposed fix
At setup level:
const telemetry = useTelemetry()Update the function:
async function runButtonClick(e: Event) { if (!jobFinishedQueue.value) return try { jobFinishedQueue.value = false resetJobToastTimeout() const isShiftPressed = 'shiftKey' in e && e.shiftKey const commandId = isShiftPressed ? 'Comfy.QueuePromptFront' : 'Comfy.QueuePrompt' - useTelemetry()?.trackUiButtonClicked({ + telemetry?.trackUiButtonClicked({ button_id: 'queue_run_linear' }) if (batchCount.value > 1) { - useTelemetry()?.trackUiButtonClicked({ + telemetry?.trackUiButtonClicked({ button_id: 'queue_run_multiple_batches_submitted' }) } await commandStore.execute(commandId, { metadata: { subscribe_to_run: false, trigger_source: 'button' } }) + } catch (error) { + console.error('Failed to queue workflow:', error) + // TODO: Show error toast to user } finally { - //TODO: Error state indicator for failed queue? jobFinishedQueue.value = true } }
132-133: Remove!Tailwind prefix per coding guidelines.The class
grid-cols-[auto_96px]!uses the forbidden!importantprefix. Also,*:[.min-w-0]:w-24is a fragile arbitrary selector. Consider refactoring the widget component to accept a class prop for inner elements instead.
145-153: Addaria-labelfor icon-only interrupt button.The interrupt button lacks an accessible label for screen readers.
♿ Proposed fix
<Button v-if="!executionStore.isIdle" variant="destructive" size="lg" class="w-10 p-2" + :aria-label="t('menu.interrupt')" `@click`="commandStore.execute('Comfy.Interrupt')" > <i class="icon-[lucide--x]" /> </Button>Apply the same fix to the duplicate button at lines 250-258.
195-195: Remove dead code: Button withv-if="false".This button will never render. Remove it and track the feature in an issue if needed.
♻️ Proposed removal
- <Button v-if="false"> {{ t('menuLabels.publish') }} </Button>
124-155: Consider extracting duplicated run button section.The mobile run section (lines 124-155) and desktop run section (lines 226-260) share nearly identical markup for batch count input, subscribe button, run button, and interrupt button. Extract to a reusable component to reduce maintenance burden.
Also applies to: 226-260
279-283: Remove dead Teleport withv-if="false".This Teleport block will never render and adds template complexity.
♻️ Proposed removal
- <Teleport v-if="false" defer :to="notesTo"> - <div - class="bg-base-background text-muted-foreground flex flex-col w-90 gap-2 rounded-2xl border-1 border-border-subtle py-3" - ></div> - </Teleport>src/composables/graph/useGraphNodeManager.ts (2)
258-275: Redundant reactive wrappers on repeated calls.This function is called multiple times on the same node (in
syncWithGraph,handleNodeAdded, andonAfterGraphConfigured). Each call wraps already-reactive arrays in newshallowReactivewrappers, creating nested reactivity.
277-286: Impure computed: mutatingslotMetadatainsidereactiveComputed.The computed callback mutates the outer
slotMetadataMap (lines 278-284), violating the principle that computed values should be pure. Additionally, stale entries persist between re-computations since the map is never cleared.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/composables/graph/useGraphNodeManager.tssrc/locales/en/main.jsonsrc/renderer/extensions/linearMode/LinearControls.vuesrc/renderer/extensions/vueNodes/components/LGraphNode.vue
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
src/**/*.vue: Use Vue 3 Single File Components (SFCs) with Composition API only
Use<script setup lang="ts">for component logic in Vue SFCs
Avoid<style>blocks in Vue components - use Tailwind 4 styling instead
Use vue-i18n for all string literals in Vue components - place translation entries insrc/locales/en/main.json
Use Tailwind utility classes instead ofdark:variant - use semantic values fromstyle.csstheme (e.g.,bg-node-component-surface)
Usecn()utility from@/utils/tailwindUtilfor merging Tailwind class names instead of:class="[]"or hardcoding
Never use!importantor!Tailwind prefix - fix interfering classes instead
Use Tailwind fraction utilities instead of arbitrary percentage values (e.g.,w-4/5instead ofw-[80%])
Use TypeScript Vue 3.5 style default prop declaration with reactive props destructuring - avoidwithDefaultsor runtime props
PreferdefineModelover separately defining a prop and emit for v-model bindings
Define slots via template usage, not viadefineSlots
Use same-name shorthand for slot prop bindings (e.g.,:isExpandedinstead of:is-expanded="isExpanded")
Do not import Vue macros unnecessarily
Avoid new usage of PrimeVue components
Use Tailwind's plurals system via i18n instead of hardcoding ...
Files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
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/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.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
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.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/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/composables/graph/useGraphNodeManager.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.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/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/composables/graph/useGraphNodeManager.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/composables/graph/useGraphNodeManager.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/graph/useGraphNodeManager.ts
src/composables/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables as
useXyz.ts(e.g.,useForm.ts)
Files:
src/composables/graph/useGraphNodeManager.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/composables/graph/useGraphNodeManager.ts
🧠 Learnings (43)
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use Tailwind utility classes instead of `dark:` variant - use semantic values from `style.css` theme (e.g., `bg-node-component-surface`)
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-11T03:55:51.755Z
Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:51.755Z
Learning: In Vue components under src/renderer/extensions/vueNodes (e.g., ImagePreview.vue and LGraphNode.vue), implement image gallery keyboard navigation so that it responds to the node's focus state rather than requiring focus inside the image preview wrapper. Achieve this by wiring keyEvent handling at the node focus level and injecting or propagating key events (arrow keys) to the gallery when the node is focused/selected. This improves accessibility and aligns navigation with node-level focus behavior.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Use `ref` for reactive state, `computed()` for derived values, and `watch`/`watchEffect` for side effects in Composition API
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Avoid using `ref` with `watch` if a `computed` would suffice - minimize refs and derived state
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-18T20:39:30.137Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI_frontend PR: 7621
File: src/components/load3d/Load3DScene.vue:4-4
Timestamp: 2025-12-18T20:39:30.137Z
Learning: In src/components/load3d/Load3DScene.vue, the scoped `<style>` block with `!important` declarations for the canvas element is necessary because Three.js dynamically creates the canvas with inline styles, preventing direct application of Tailwind classes. This is a valid exception to the Tailwind-only styling guideline.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-09T03:49:52.828Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:49:52.828Z
Learning: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML <button> elements. These components wrap PrimeVue with the project’s design system styling. Use only the common button components for consistency and theming, and import them from src/components/button/ as needed.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-09T21:40:12.361Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-16T22:26:49.463Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-22T21:36:08.369Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-08T02:26:18.357Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 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/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-18T02:07:38.870Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-18T21:15:46.862Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-21T01:06:02.786Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/renderer/extensions/vueNodes/components/LGraphNode.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.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/**/*.{vue,ts,tsx} : Follow Vue 3 composition API style guide
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize Vue 3's Teleport component when needed
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use `<script setup lang="ts">` for component logic in Vue SFCs
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use TypeScript Vue 3.5 style default prop declaration with reactive props destructuring - avoid `withDefaults` or runtime props
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-04T21:43:49.363Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/components/rightSidePanel/parameters/TabParameters.vue:10-0
Timestamp: 2025-12-04T21:43:49.363Z
Learning: Vue 3.5+ supports reactive props destructure in <script setup>. Destructuring props directly (e.g., `const { nodes } = defineProps<{ nodes: LGraphNode[] }>()`) maintains reactivity through compiler transformation. This is the recommended modern approach and does not require using `props.x` or `toRef`/`toRefs`.
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.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/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid mutable state - prefer immutability and assignment at point of declaration
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 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} : Take advantage of `TypedArray` `subarray` when appropriate
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-09T07:29:32.501Z
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:32.501Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vuesrc/composables/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-18T16:03:09.642Z
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:09.642Z
Learning: In the ComfyUI frontend queue system (src/stores/queueStore.ts), the `useQueuePendingTaskCountStore().count` includes the currently executing task. When count = 1, there is only the active/running task with no pending tasks. When count > 1, there is an active task plus pending tasks waiting in the queue. The "Clear Pending Tasks" button should only be enabled when count > 1 to avoid clearing the currently running task, which should be handled by the "Cancel current run" button instead.
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Leverage VueUse functions for performance-enhancing composables
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Never use `!important` or `!` Tailwind prefix - fix interfering classes instead
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use Tailwind fraction utilities instead of arbitrary percentage values (e.g., `w-4/5` instead of `w-[80%]`)
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Avoid `<style>` blocks in Vue components - use Tailwind 4 styling instead
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use `cn()` utility from `@/utils/tailwindUtil` for merging Tailwind class names instead of `:class="[]"` or hardcoding
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use Tailwind's plurals system via i18n instead of hardcoding pluralization in templates
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use Teleport/Suspense when needed for component rendering
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-01T23:42:30.894Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7084
File: src/renderer/extensions/vueNodes/components/SlotConnectionDot.vue:23-26
Timestamp: 2025-12-01T23:42:30.894Z
Learning: In the ComfyUI frontend codebase, Tailwind CSS is configured with Preflight enabled (default), which automatically provides `content: ''` for pseudo-elements when using `after:` or `before:` variants - no need to explicitly add `after:content-['']`.
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 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/**/{composables,components}/**/*.{ts,tsx,vue} : Clean up subscriptions in state management to prevent memory leaks
Applied to files:
src/composables/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-11T03:55:57.926Z
Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:57.926Z
Learning: In src/renderer/extensions/vueNodes/components/ImagePreview.vue and LGraphNode.vue, keyboard navigation for image galleries should respond to node-level focus (via keyEvent injection from LGraphNode), not require focus within the image preview wrapper itself. This allows users to navigate the gallery with arrow keys immediately when the node is focused/selected.
Applied to files:
src/composables/graph/useGraphNodeManager.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/composables/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/composables/graph/useGraphNodeManager.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/graph/useGraphNodeManager.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/graph/useGraphNodeManager.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/graph/useGraphNodeManager.ts
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/locales/en/main.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (7)
src/locales/en/main.json (2)
193-193: LGTM - Pluralization format is correct.The vue-i18n pipe-separated format correctly handles zero, singular, and plural cases:
{count} assets | {count} asset | {count} assetswill render "0 assets", "1 asset", and "2+ assets" respectively.
2481-2488: LGTM - New linearMode localization entries.The new keys for Simple Mode functionality are properly structured and consistent with the existing localization patterns.
src/renderer/extensions/linearMode/LinearControls.vue (4)
1-31: LGTM - Setup section follows Vue 3 Composition API guidelines.Proper use of
<script setup lang="ts">, store integration viastoreToRefs, and composable usage. Imports are well-organized.
33-37: LGTM - Props definition uses Vue 3.5 style.Correctly uses reactive props destructuring without
withDefaults, per coding guidelines.
156-194: LGTM - Workflow info section and notes popover.Good implementation of the workflow info header with conditional Popover for notes. The NodeWidgets rendering with proper styling is well-structured.
263-278: LGTM - Toast Teleport implementation.The queue toast Teleport correctly shows job status feedback based on
jobToastTimeoutandjobFinishedQueuestate.src/composables/graph/useGraphNodeManager.ts (1)
207-207: LGTM - reducing exported surface area.Making
safeWidgetMapperprivate aligns with the goal of minimizing module exports while centralizing the public API aroundextractVueNodeData.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+277
to
+286
| const safeWidgets = reactiveComputed<SafeWidgetData[]>(() => { | ||
| node.inputs?.forEach((input, index) => { | ||
| if (!input?.widget?.name) return | ||
| slotMetadata.set(input.widget.name, { | ||
| index, | ||
| linked: input.link != null | ||
| }) | ||
| }) | ||
| return node.widgets?.map(safeWidgetMapper(node, slotMetadata)) ?? [] | ||
| }) |
Contributor
There was a problem hiding this comment.
Memory leak: orphaned reactiveComputed subscriptions on repeated calls.
Each call to extractVueNodeData creates a new reactiveComputed subscription (line 277). When the function is called multiple times on the same node (which occurs in syncWithGraph, handleNodeAdded, and onAfterGraphConfigured), the previous computed subscriptions are never disposed—they remain active and continue tracking dependencies.
This violates the guideline to clean up subscriptions in state management to prevent memory leaks. Based on learnings, subscriptions should be cleaned up.
Possible approaches
- Guard against re-extraction: Use a marker (similar to the reactive marker suggestion) to skip re-computation if already set up.
- Store and dispose: Track the previous computed's stop function and call it before creating a new one.
- Extract once: Restructure callers to ensure
extractVueNodeDatais only called once per node lifetime.
🤖 Prompt for AI Agents
In `@src/composables/graph/useGraphNodeManager.ts` around lines 277 - 286, The
reactiveComputed created for safeWidgets inside extractVueNodeData is never
disposed, causing orphaned subscriptions on repeated calls; fix by tracking and
stopping the previous computed before creating a new one (e.g., store the
stop/teardown function in a WeakMap keyed by the node or attach a hidden
property like node.__safeWidgetsStop), call that stop function if present, then
create the new reactiveComputed for safeWidgets and save its stop handle; ensure
this cleanup happens every time extractVueNodeData runs so prior subscriptions
are disposed.
| 'Unknown' | ||
|
|
||
| const apiNode = node.constructor?.nodeData?.api_node ?? false | ||
| const badges = node.badges |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent isolation: badges is a direct reference while outputs is shallow-copied.
Line 312 creates a shallow copy of outputs ([...node.outputs]), but badges (line 296) is assigned as a direct reference. If node.badges is mutated externally, the VueNodeData.badges will reflect those changes unexpectedly. Consider applying consistent treatment:
- const badges = node.badges
+ const badges = node.badges ? [...node.badges] : undefined📝 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 badges = node.badges | |
| const badges = node.badges ? [...node.badges] : undefined |
🤖 Prompt for AI Agents
In `@src/composables/graph/useGraphNodeManager.ts` at line 296, The assignment for
badges uses a direct reference (badges = node.badges) while outputs is
shallow-copied; change badges to be shallow-copied like outputs to avoid
external mutations leaking into VueNodeData.badges — replace the direct
reference with a shallow copy (e.g., [...node.badges]) where VueNodeData.badges
is constructed in useGraphNodeManager.ts so badges and outputs are treated
consistently.
DrJKL
added
cloud/1.37
|
AustinMroz
added a commit
that referenced
this pull request
Jan 14, 2026
Manual backport of #7734 to `cloud/1.37`
AustinMroz
added a commit
that referenced
this pull request
Jan 14, 2026
Manual backport of #7734 to `cloud/1.37`
AustinMroz
added a commit
that referenced
this pull request
Jan 14, 2026
Manual backport of #7734 to `cloud/1.37` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8047-backport-cloud-1-37-linear-v2-Simple-Mode-2e86d73d365081948861debeae9604f0) by [Unito](https://www.unito.io)
This was referenced Jan 15, 2026
Merged
This was referenced Jan 22, 2026
jtydhr88
pushed a commit
that referenced
this pull request
Jan 28, 2026
## Summary Refactor DropZone to use VueUse useDropZone. (#7971, #7734) ## Changes - **What**: Replaced `@dragover`, `@dragleave`, and `@drop` bindings with `useDropZone`. ## Review Focus ## Screenshots ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8174-Refactor-DropZone-to-use-VueUse-useDropZone-2ee6d73d365081ed9d31d68f7cf42263) by [Unito](https://www.unito.io)
This was referenced Feb 2, 2026
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
A major, full rewrite of linear mode, now under the name "Simple Mode".
The primary means of accessing Simple Mode is a toggle button on the bottom right. This button is only shown if a feature flag is enabled, or the user has already seen linear mode during the current session. Simple Mode can also be accessed by
Known issues:
┆Issue is synchronized with this Notion page by Unito