Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR significantly refactors and extends the linear mode UI by introducing new components, a centralized Pinia store for output state management, and utility functions for handling job execution, latent previews, and output history. The changes consolidate output preview logic, add progress tracking, and restructure component interactions around a unified selection model. Changes
Sequence DiagramsequenceDiagram
participant Execution as Execution Engine
participant Store as linearOutputStore
participant JobPreview as JobPreviewStore
participant OutputHistory as OutputHistory Component
participant UI as UI Components
Execution->>Store: onJobStart(jobId)
Store->>Store: Create skeleton item, auto-select
Store->>UI: Update inProgressItems
Execution->>JobPreview: Update latent preview
JobPreview->>Store: onLatentPreview(jobId, url)
Store->>Store: Create latent item if tracking job
Store->>UI: Emit selection update
Execution->>Store: onNodeExecuted(jobId, outputs)
Store->>Store: Flatten outputs, convert skeleton to images
Store->>UI: Update inProgressItems with image items
Execution->>Store: onJobComplete(jobId)
Store->>Store: Remove non-image items, mark resolved
UI->>OutputHistory: User selects output item
OutputHistory->>Store: handleSelection(selection)
Store->>Store: Update selectedId, canShowPreview
Store->>UI: Emit new selection with metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: ✅ Built — View Storybook |
🎭 Playwright: ✅ 531 passed, 0 failed · 2 flaky📊 Browser Reports
|
![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: 4
🧹 Nitpick comments (3)
packages/design-system/src/css/style.css (1)
1884-1900: Consider theme-aware colors for skeleton shimmer.The skeleton shimmer uses hardcoded colors (
rgb(188 188 188 / 0.5)andrgb(0 0 0 / 0.5)). This works but won't adapt to the dark theme. If you want consistent theming, consider using CSS variables or adding a.dark-theme .skeleton-shimmervariant.This is optional since the current implementation provides adequate visual feedback in both themes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/design-system/src/css/style.css` around lines 1884 - 1900, Update the .skeleton-shimmer and `@keyframes` skeleton-shimmer to use theme-aware CSS variables instead of hardcoded RGB values (e.g., --skeleton-base and --skeleton-highlight) or provide an override rule for .dark-theme .skeleton-shimmer that sets alternative variables; replace references to rgb(188 188 188 / 0.5) and rgb(0 0 0 / 0.5) with the variables so the shimmer adapts to light/dark themes while keeping the existing animation/keyframe names intact.src/renderer/extensions/linearMode/linearOutputStore.test.ts (1)
9-15: Avoid module-level mutable refs in mocks.
activeJobIdRef,previewsRef, andisAppModeRefare global mutable state. Prefer avi.hoisted()container to keep mock state contained and predictable across tests.Based on learnings: Keep module mocks contained; do not use global mutable state within test files; use vi.hoisted() if necessary for per-test Arrange phase manipulation.🧪 Suggested refactor
-const activeJobIdRef = ref<string | null>(null) -const previewsRef = ref<Record<string, string>>({}) -const isAppModeRef = ref(true) - -const { apiTarget } = vi.hoisted(() => ({ - apiTarget: new EventTarget() -})) +const { apiTarget, activeJobIdRef, previewsRef, isAppModeRef } = vi.hoisted( + () => ({ + apiTarget: new EventTarget(), + activeJobIdRef: ref<string | null>(null), + previewsRef: ref<Record<string, string>>({}), + isAppModeRef: ref(true) + }) +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearOutputStore.test.ts` around lines 9 - 15, activeJobIdRef, previewsRef and isAppModeRef are defined as module-level mutable refs causing shared state between tests; move their creation into a vi.hoisted() container so each test can control/reset them predictably. Replace the top-level refs with something like a hoisted object (using vi.hoisted(() => ({ activeJobIdRef: ref(...), previewsRef: ref(...), isAppModeRef: ref(...) }))) and update tests/mocks to read/write those properties (referencing the hoisted container and its activeJobIdRef, previewsRef, isAppModeRef) so state is isolated per test.src/renderer/extensions/linearMode/OutputHistory.vue (1)
206-269: Extract inline event handlers into named functions.
This aligns with the repo’s preference for function declarations and keeps handlers reusable/testable.♻️ Suggested refactor
const pointer = new CanvasPointer(document.body) let scrollOffset = 0 -useEventListener( - document.body, - 'wheel', - function (e: WheelEvent) { - if (!e.ctrlKey && !e.metaKey) return - e.preventDefault() - e.stopPropagation() - - if (!pointer.isTrackpadGesture(e)) { - if (e.deltaY > 0) navigateToAdjacent(1) - else navigateToAdjacent(-1) - return - } - scrollOffset += e.deltaY - while (scrollOffset >= 60) { - scrollOffset -= 60 - navigateToAdjacent(1) - } - while (scrollOffset <= -60) { - scrollOffset += 60 - navigateToAdjacent(-1) - } - }, - { capture: true, passive: false } -) - -useEventListener( - outputsRef, - 'wheel', - function (e: WheelEvent) { - if (e.ctrlKey || e.metaKey || e.deltaY === 0) return - e.preventDefault() - if (outputsRef.value) outputsRef.value.scrollLeft += e.deltaY - }, - { passive: false } -) - -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') navigateToAdjacent(1) - else navigateToAdjacent(-1) -}) +function onGlobalWheel(e: WheelEvent) { + if (!e.ctrlKey && !e.metaKey) return + e.preventDefault() + e.stopPropagation() + + if (!pointer.isTrackpadGesture(e)) { + if (e.deltaY > 0) navigateToAdjacent(1) + else navigateToAdjacent(-1) + return + } + scrollOffset += e.deltaY + while (scrollOffset >= 60) { + scrollOffset -= 60 + navigateToAdjacent(1) + } + while (scrollOffset <= -60) { + scrollOffset += 60 + navigateToAdjacent(-1) + } +} + +function onOutputsWheel(e: WheelEvent) { + if (e.ctrlKey || e.metaKey || e.deltaY === 0) return + e.preventDefault() + if (outputsRef.value) outputsRef.value.scrollLeft += e.deltaY +} + +function onGlobalKeydown(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') navigateToAdjacent(1) + else navigateToAdjacent(-1) +} + +useEventListener(document.body, 'wheel', onGlobalWheel, { + capture: true, + passive: false +}) +useEventListener(outputsRef, 'wheel', onOutputsWheel, { passive: false }) +useEventListener(document.body, 'keydown', onGlobalKeydown)As per coding guidelines: Do not use function expressions if it's possible to use function declarations instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistory.vue` around lines 206 - 269, The inline anonymous event handler functions passed to useEventListener (the wheel handler on document.body that uses pointer.isTrackpadGesture and scrollOffset, the wheel handler on outputsRef that updates outputsRef.value.scrollLeft, and the keydown handler on document.body that calls navigateToAdjacent) should be extracted into named function declarations to match the repo style; create e.g. handleDocumentWheel(e: WheelEvent), handleOutputsWheel(e: WheelEvent), and handleDocumentKeydown(e: KeyboardEvent) that encapsulate the existing logic (use pointer.isTrackpadGesture, scrollOffset mutation, navigateToAdjacent, and checks for ctrl/meta and input/textarea targets), then pass those named functions to useEventListener instead of inline functions and keep references to listboxRef, outputsRef, pointer, and navigateToAdjacent as used by the handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/extensions/linearMode/flattenNodeOutput.test.ts`:
- Line 12: The test suite currently uses a string title in describe; change it
to use the function reference to satisfy the
vitest/prefer-describe-function-title rule by replacing
describe('flattenNodeOutput', ...) with describe(flattenNodeOutput, ...) so the
suite title is the actual function reference (flattenNodeOutput) used in the
file, ensuring the test import or symbol is in scope for the describe call.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 101-118: Add accessible names to the two icon-only action buttons
by supplying aria-labels via vue-i18n keys: for the download button (the Button
rendered when selectedOutput and invoking downloadFile(selectedOutput.url)) add
an aria-label using a new i18n key like "actions.download" and for the interrupt
button (the Button rendered when !executionStore.isIdle && !selectedItem and
invoking commandStore.execute('Comfy.Interrupt')) add an aria-label using a key
like "actions.interrupt"; implement the labels using the Composition API i18n
(e.g., useI18n/t function inside the component) and add the corresponding
entries to src/locales/en/main.json.
In `@src/renderer/extensions/linearMode/OutputHistoryItem.vue`:
- Around line 14-25: The img preview rendered when getMediaType(output) ===
'images' is missing an alt attribute; update the <img> element in
OutputHistoryItem.vue to include an alt (e.g., alt="" for decorative images or
alt="Preview" / derived text if available) so accessibility and HTML validity
are satisfied; locate the img that binds :src="output.url" (and is controlled by
getMediaType(output)) and add the alt attribute there.
In `@src/renderer/extensions/linearMode/OutputPreviewItem.vue`:
- Around line 10-14: The <img> in the OutputPreviewItem.vue component (rendered
when latentPreview is truthy) lacks an alt attribute; update the img element to
include an appropriate alt: use alt="" if the image is purely decorative or bind
a descriptive string via i18n (e.g., :alt="$t('...')" or a prop) when it conveys
meaning, ensuring the change references the latentPreview-rendering <img>
element so screen readers receive correct semantics.
---
Nitpick comments:
In `@packages/design-system/src/css/style.css`:
- Around line 1884-1900: Update the .skeleton-shimmer and `@keyframes`
skeleton-shimmer to use theme-aware CSS variables instead of hardcoded RGB
values (e.g., --skeleton-base and --skeleton-highlight) or provide an override
rule for .dark-theme .skeleton-shimmer that sets alternative variables; replace
references to rgb(188 188 188 / 0.5) and rgb(0 0 0 / 0.5) with the variables so
the shimmer adapts to light/dark themes while keeping the existing
animation/keyframe names intact.
In `@src/renderer/extensions/linearMode/linearOutputStore.test.ts`:
- Around line 9-15: activeJobIdRef, previewsRef and isAppModeRef are defined as
module-level mutable refs causing shared state between tests; move their
creation into a vi.hoisted() container so each test can control/reset them
predictably. Replace the top-level refs with something like a hoisted object
(using vi.hoisted(() => ({ activeJobIdRef: ref(...), previewsRef: ref(...),
isAppModeRef: ref(...) }))) and update tests/mocks to read/write those
properties (referencing the hoisted container and its activeJobIdRef,
previewsRef, isAppModeRef) so state is isolated per test.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 206-269: The inline anonymous event handler functions passed to
useEventListener (the wheel handler on document.body that uses
pointer.isTrackpadGesture and scrollOffset, the wheel handler on outputsRef that
updates outputsRef.value.scrollLeft, and the keydown handler on document.body
that calls navigateToAdjacent) should be extracted into named function
declarations to match the repo style; create e.g. handleDocumentWheel(e:
WheelEvent), handleOutputsWheel(e: WheelEvent), and handleDocumentKeydown(e:
KeyboardEvent) that encapsulate the existing logic (use
pointer.isTrackpadGesture, scrollOffset mutation, navigateToAdjacent, and checks
for ctrl/meta and input/textarea targets), then pass those named functions to
useEventListener instead of inline functions and keep references to listboxRef,
outputsRef, pointer, and navigateToAdjacent as used by the handlers.
pythongosssss
force-pushed
the
pysssss/appmode/welcome-page.3
branch
from
6be06e7 to
e0f2de5
Compare
pythongosssss
force-pushed
the
pysssss/appmode/linear-run.3.5
branch
from
b37e835 to
6ec4e0f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue (1)
6-24: Avoid destructuringdefinePropsto preserve reactivity. Use the props object (or rely on template prop exposure) instead.Based on learnings: In Vue 3 script setup, props defined with defineProps are automatically available by name in the template without destructuring. Destructuring the result of defineProps inside script can break reactivity.♻️ Proposed refactor
-const { rootClass } = defineProps<{ +defineProps<{ widget: Pick< SimplifiedWidget<string | number | undefined>, 'name' | 'label' | 'borderStyle' > rootClass?: string }>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue` around lines 6 - 24, The code destructures defineProps (extracting rootClass) which can break Vue reactivity; change to capture the whole props object via defineProps (e.g., const props = defineProps<...>() or rely on the template-exposed prop names) and update usages of rootClass in this file to reference props.rootClass (or use the template-provided rootClass directly) so reactivity for rootClass and other props is preserved; ensure useHideLayoutField() remains unchanged and any references to rootClass in the template or script are updated to the non-destructured form.src/views/LinearView.vue (1)
6-22: Prefer$tin the template when i18n isn’t used in script.Based on learnings: In Vue single-file components where the i18n t function is only used within the template, prefer using the built-in $t in the template instead of importing useI18n and destructuring t in the script.♻️ Suggested refactor
-import { useI18n } from 'vue-i18n' @@ -const { t } = useI18n() @@ - <div v-text="t('linearMode.beta')" /> + <div v-text="$t('linearMode.beta')" />Also applies to: 71-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/LinearView.vue` around lines 6 - 22, The file imports useI18n and destructures t via "const { t } = useI18n()" even though t is only used in the template; remove the useI18n import and the "const { t } = useI18n()" statement and update the template to call $t(...) instead of t(...). Search for other occurrences of destructured t or useI18n in this component (including the other occurrence referenced) and apply the same change so all template translations use $t and no unused i18n import or variable remains.src/renderer/extensions/linearMode/LinearPreview.vue (1)
101-134: Avoid inline function expressions in template handlers.As per coding guidelines: Do not use function expressions if it's possible to use function declarations instead.♻️ Suggested refactor
+function downloadSelected() { + if (selectedOutput.value?.url) downloadFile(selectedOutput.value.url) +} + +function interruptExecution() { + commandStore.execute('Comfy.Interrupt') +} + +function downloadAllSelected() { + downloadAsset(selectedItem.value) +} + +function deleteSelectedAsset() { + mediaActions.deleteAssets(selectedItem.value!) +} @@ <Button v-if="selectedOutput" size="icon" - `@click`=" - () => { - if (selectedOutput?.url) downloadFile(selectedOutput.url) - } - " + `@click`="downloadSelected" > @@ <Button v-if="!executionStore.isIdle && !selectedItem" variant="destructive" size="icon" - `@click`="commandStore.execute('Comfy.Interrupt')" + `@click`="interruptExecution" > @@ - command: () => downloadAsset(selectedItem!) + command: downloadAllSelected }, @@ - command: () => mediaActions.deleteAssets(selectedItem!) + command: deleteSelectedAsset }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/LinearPreview.vue` around lines 101 - 134, The template currently uses inline arrow functions for click handlers (e.g., the download button uses "() => { if (selectedOutput?.url) downloadFile(selectedOutput.url) }", the download-all and delete entries use "command: () => downloadAsset(selectedItem!)" and "command: () => mediaActions.deleteAssets(selectedItem!)"), and the interrupt button directly calls commandStore.execute; move these inline expressions into named component methods (for example create methods like onDownloadSelected(), onInterrupt(), onDownloadItem(), onDeleteItem()) inside the component's methods block, reference selectedOutput, selectedItem, downloadFile, downloadAsset, mediaActions.deleteAssets and commandStore.execute from those methods, and update the template/Popover entries to call the method names instead of arrow functions so handlers are declared rather than inlined.src/renderer/extensions/linearMode/OutputHistory.vue (2)
218-244: CanvasPointer instantiation at module level assumes DOM availability.
new CanvasPointer(document.body)runs during component setup. This is fine for client-side only usage but would fail in SSR. If this component is ever used in an SSR context, consider wrapping inonMountedor checking fortypeof document !== 'undefined'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistory.vue` around lines 218 - 244, The CanvasPointer is instantiated at module load (new CanvasPointer(document.body)), which breaks SSR; change to declare let pointer: CanvasPointer | null = null and defer initialization to onMounted (or guard with typeof document !== 'undefined') so the instance is created only in the browser; update the wheel handler (used with useEventListener) to check pointer is non-null before calling pointer.isTrackpadGesture and ensure any listener registration happens after mounting (or short-circuit if document is undefined) and clean up/reset pointer on unmount.
85-88: Consider typing the callback parameter.The
as SelectionValue | undefinedassertion could be avoided if reka-ui'sListboxRootexposes a generic or typed@update:model-valuecallback. If the library doesn't support this, the assertion is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistory.vue` around lines 85 - 88, The callback parameter is currently untyped (val: unknown) in onSelectionChange and uses a type assertion; change the signature to accept the correct type (e.g., function onSelectionChange(val: SelectionValue | undefined)) so you can remove the "as SelectionValue | undefined" assertion and call store.select(val?.id ?? null) directly; if reka-ui's ListboxRoot exposes a generic typed `@update`:model-value, prefer using that generic in the component's prop/event typing to propagate SelectionValue into the handler, otherwise keep the explicit typed parameter as shown.src/renderer/extensions/linearMode/linearOutputStore.ts (1)
258-271: Consider whether lifecycle actions should be exposed.Exposing
onJobStart,onLatentPreview,onNodeExecuted, andonJobCompleteallows any consumer to call them out of order, potentially corrupting state. If these are only meant to be triggered by internal watchers and event listeners, consider keeping them private. If external calls are intentional (e.g., for testing), this is fine.Based on learnings: "Only expose state and actions that are used externally; keep internal state private."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearOutputStore.ts` around lines 258 - 271, The lifecycle handlers onJobStart, onLatentPreview, onNodeExecuted, and onJobComplete are being exported from the linearOutputStore even though they appear to be internal-only and can be called out of order; stop exposing them publicly by removing them from the returned object and keep the functions as internal helpers inside linearOutputStore (leave externally-used API like inProgressItems, selectedId, trackedJobId, pendingResolve, select, selectAsLatest, resolveIfReady untouched). If you need external access for tests, provide a clearly-named test-only accessor (e.g., getInternalHooksForTest) or export a thin, controlled wrapper that enforces ordering/validation rather than returning the raw handlers. Ensure references to onJobStart/onLatentPreview/onNodeExecuted/onJobComplete within the module still call the internal functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 157-162: The Tailwind `!` important modifier on the
WidgetInputNumberInput instance should be removed—edit the class string
`class="*:[.min-w-0]:w-24 grid-cols-[auto_96px]!"` to drop the trailing `!` and
instead adjust the base/root classes (e.g. `root-class="text-base-foreground"`)
or component styles to achieve the intended width; also locate the other
WidgetInputNumberInput usage with the same `!` modifier later in the file (the
block around the second occurrence) and remove the `!`, ensuring layout is
preserved by reordering classes or using a stronger selector in the component
CSS rather than using `!important`.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 336-344: The ListboxItem currently uses a non-null assertion on
selectionMap.get(`history:${asset.id}:${key}`)! which can throw if the map is
out of sync; update the rendering to defensively handle undefined by resolving
the selection into a local const (using selectionMap.get(...)) and either skip
rendering the ListboxItem when that value is undefined (v-if) or provide a safe
fallback selection value before passing it to ListboxItem; adjust the loop over
toValue(allOutputs(asset)) accordingly so OutputHistoryItem is only rendered
paired with a valid selection to avoid runtime exceptions.
- Around line 313-324: The ListboxItem and OutputHistoryItem usage assumes
selectionMap.get(`slot:${item.id}`)! and item.output! are always present which
can throw on race conditions; change to defensive access by checking
selectionMap.has(`slot:${item.id}`) before using its value (or pass
undefined/null when missing) and render OutputHistoryItem only when item.output
is defined (or pass a safe fallback), i.e., update the v-for rendering logic
around ListboxItem, selectionMap and OutputHistoryItem to guard against missing
entries instead of using non-null assertions so the template won't access
undefined values during async updates.
---
Duplicate comments:
In `@src/renderer/extensions/linearMode/flattenNodeOutput.test.ts`:
- Line 12: The test suite currently uses a string title; change
describe('flattenNodeOutput', ...) to use the function reference
describe(flattenNodeOutput, ...) to satisfy
vitest/prefer-describe-function-title; ensure the flattenNodeOutput symbol is
imported or referenced in the test file (from the module supplying
flattenNodeOutput) so the identifier is available to describe.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 101-118: The two icon-only Button instances (the download button
tied to selectedOutput and downloadFile, and the interrupt button that calls
commandStore.execute('Comfy.Interrupt') when !executionStore.isIdle &&
!selectedItem) need accessible labels: add an aria-label prop bound to vue-i18n
(e.g. :aria-label="$t('linearPreview.download')" and
:aria-label="$t('linearPreview.interrupt')") on those Button components, and add
corresponding entries in src/locales/en/main.json under a new linearPreview
namespace (e.g. "download" and "interrupt"); do not add aria-labels to buttons
that already have visible text.
In `@src/renderer/extensions/linearMode/OutputHistoryItem.vue`:
- Around line 14-21: The image tag in OutputHistoryItem.vue lacks an alt
attribute which harms accessibility; update the <img> used when
getMediaType(output) === 'images' to include a meaningful alt (e.g., use
output.alt, output.description, or fallback like `Preview image`), ensuring the
template binds :alt to the chosen field (and add a safe fallback if the output
object may not have that property).
In `@src/renderer/extensions/linearMode/OutputPreviewItem.vue`:
- Around line 10-14: The <img> used for the latent preview in the
OutputPreviewItem.vue component is missing an alt attribute which harms
accessibility; update the <img v-if="latentPreview" ... :src="latentPreview" />
element to include an appropriate alt attribute (use alt="" if the image is
purely decorative or a short descriptive string if it conveys information) so
the latentPreview image is accessible to assistive technologies.
---
Nitpick comments:
In `@src/renderer/extensions/linearMode/linearOutputStore.ts`:
- Around line 258-271: The lifecycle handlers onJobStart, onLatentPreview,
onNodeExecuted, and onJobComplete are being exported from the linearOutputStore
even though they appear to be internal-only and can be called out of order; stop
exposing them publicly by removing them from the returned object and keep the
functions as internal helpers inside linearOutputStore (leave externally-used
API like inProgressItems, selectedId, trackedJobId, pendingResolve, select,
selectAsLatest, resolveIfReady untouched). If you need external access for
tests, provide a clearly-named test-only accessor (e.g.,
getInternalHooksForTest) or export a thin, controlled wrapper that enforces
ordering/validation rather than returning the raw handlers. Ensure references to
onJobStart/onLatentPreview/onNodeExecuted/onJobComplete within the module still
call the internal functions.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 101-134: The template currently uses inline arrow functions for
click handlers (e.g., the download button uses "() => { if (selectedOutput?.url)
downloadFile(selectedOutput.url) }", the download-all and delete entries use
"command: () => downloadAsset(selectedItem!)" and "command: () =>
mediaActions.deleteAssets(selectedItem!)"), and the interrupt button directly
calls commandStore.execute; move these inline expressions into named component
methods (for example create methods like onDownloadSelected(), onInterrupt(),
onDownloadItem(), onDeleteItem()) inside the component's methods block,
reference selectedOutput, selectedItem, downloadFile, downloadAsset,
mediaActions.deleteAssets and commandStore.execute from those methods, and
update the template/Popover entries to call the method names instead of arrow
functions so handlers are declared rather than inlined.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 218-244: The CanvasPointer is instantiated at module load (new
CanvasPointer(document.body)), which breaks SSR; change to declare let pointer:
CanvasPointer | null = null and defer initialization to onMounted (or guard with
typeof document !== 'undefined') so the instance is created only in the browser;
update the wheel handler (used with useEventListener) to check pointer is
non-null before calling pointer.isTrackpadGesture and ensure any listener
registration happens after mounting (or short-circuit if document is undefined)
and clean up/reset pointer on unmount.
- Around line 85-88: The callback parameter is currently untyped (val: unknown)
in onSelectionChange and uses a type assertion; change the signature to accept
the correct type (e.g., function onSelectionChange(val: SelectionValue |
undefined)) so you can remove the "as SelectionValue | undefined" assertion and
call store.select(val?.id ?? null) directly; if reka-ui's ListboxRoot exposes a
generic typed `@update`:model-value, prefer using that generic in the component's
prop/event typing to propagate SelectionValue into the handler, otherwise keep
the explicit typed parameter as shown.
In
`@src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue`:
- Around line 6-24: The code destructures defineProps (extracting rootClass)
which can break Vue reactivity; change to capture the whole props object via
defineProps (e.g., const props = defineProps<...>() or rely on the
template-exposed prop names) and update usages of rootClass in this file to
reference props.rootClass (or use the template-provided rootClass directly) so
reactivity for rootClass and other props is preserved; ensure
useHideLayoutField() remains unchanged and any references to rootClass in the
template or script are updated to the non-destructured form.
In `@src/views/LinearView.vue`:
- Around line 6-22: The file imports useI18n and destructures t via "const { t }
= useI18n()" even though t is only used in the template; remove the useI18n
import and the "const { t } = useI18n()" statement and update the template to
call $t(...) instead of t(...). Search for other occurrences of destructured t
or useI18n in this component (including the other occurrence referenced) and
apply the same change so all template translations use $t and no unused i18n
import or variable remains.
e0f2de5 to
b7071e5
Compare
pythongosssss
force-pushed
the
pysssss/appmode/linear-run.3.5
branch
from
c0a9be1 to
3234ab0
Compare
AustinMroz
reviewed
Feb 21, 2026
Collaborator
AustinMroz
left a comment
AustinMroz
left a comment
There was a problem hiding this comment.
A dedicated skeleton component was recently added in #8979 . I'm undecided on which animation is better, but they should probably be consolidated at some point.
PR has my tentative approval, but it's complex enough that I want to sleep on it.
| return user_metadata?.outputCount ?? 0 | ||
| } | ||
| const selectionMap = computed( | ||
| () => new Map(selectableItems.value.map((v) => [v.id, v])) |
Collaborator
There was a problem hiding this comment.
I'm not a fan of the amount of recomputing done here when a selectableItem is created, but it's probably fine.
| // the global keybinding handler on window. Intercept in capture phase | ||
| // and re-dispatch from above the Listbox. | ||
| function onModifierEnter(e: KeyboardEvent) { | ||
| if (e.key !== 'Enter' || !(e.ctrlKey || e.metaKey || e.shiftKey)) return |
Collaborator
There was a problem hiding this comment.
I hate this, but I'm not seeing a cleaner fix.
pythongosssss
force-pushed
the
pysssss/appmode/welcome-page.3
branch
2 times, most recently
from
8307211 to
7062141
Compare
pythongosssss
force-pushed
the
pysssss/appmode/linear-run.3.5
branch
from
3234ab0 to
6a8cee2
Compare
AustinMroz
previously approved these changes
Feb 21, 2026
dosubot
bot
added
the
size:XXL
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
pythongosssss
force-pushed
the
pysssss/appmode/welcome-page.3
branch
from
7062141 to
052f890
Compare
pythongosssss
dismissed
AustinMroz’s stale review
The base branch was changed.
dosubot
bot
added
size:XL
… → image) with follow/browse selection modes - Extract flattenNodeOutput, useOutputHistory, and shared types into dedicated modules - Decompose OutputHistory.vue into smaller components (OutputHistoryItem, OutputPreviewItem, LatentPreview, LinearProgressBar) - Simplify LinearView/LinearPreview by moving execution and selection state into the store
- fix event types - fix tests
remove log
pythongosssss
force-pushed
the
pysssss/appmode/linear-run.3.5
branch
from
6a8cee2 to
2f911ff
Compare
dosubot
bot
added
size:XXL
📦 Bundle: 4.37 MB gzip 🔴 +3.22 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 943 kB (baseline 932 kB) • 🔴 +10.2 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 12 added / 12 removed Data & Services — 2.52 MB (baseline 2.52 MB) • 🟢 -443 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 58.3 kB (baseline 58.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed Vendor & Third-Party — 8.84 MB (baseline 8.83 MB) • 🔴 +5.35 kBExternal libraries and shared vendor chunks
Status: 6 added / 6 removed Other — 7.62 MB (baseline 7.62 MB) • 🔴 +117 BBundles that do not match a named category
Status: 78 added / 78 removed |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/extensions/linearMode/OutputHistory.vue (1)
220-244:⚠️ Potential issue | 🟠 MajorGlobal
Ctrl+wheelcapture ondocument.bodyprevents browser zoom site-wide.The handler at line 224-226 calls
preventDefault()+stopPropagation()on everyCtrl+wheelevent, which disables the browser's native pinch/zoom across the entire page — not just over the output history area. Users who rely on Ctrl+scroll to zoom the page will be unable to do so while this component is mounted.Consider scoping the capture to the
outputsRefelement instead ofdocument.body, or adding a check that the event target is within the outputs area.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistory.vue` around lines 220 - 244, The wheel handler currently registered with useEventListener(document.body, ...) calls preventDefault()/stopPropagation() for every Ctrl+wheel and thus blocks browser zoom globally; update the listener to only act when the event originates inside the output area (e.g., scope the listener to outputsRef instead of document.body or add an early guard like if (!outputsRef?.value?.contains(e.target as Node)) return), and only call e.preventDefault() / e.stopPropagation() and run pointer.isTrackpadGesture / navigateToAdjacent / scrollOffset logic when the event is within that outputsRef element; keep the same options ({ capture: true, passive: false }) when attaching to the scoped element.
♻️ Duplicate comments (3)
src/renderer/extensions/linearMode/LinearControls.vue (1)
158-163:!-important modifier removed — styling split looks correct.
root-classnow carries the grid overrides whileclasstargets the inner input wrapper;tailwind-mergeinsidecn()correctly letsgrid-cols-[auto_96px]supersedegrid-cols-subgrid, andw-24(96 px) is consistent with the 96 px grid column. Resolves the prior review flag.Also applies to: 266-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/LinearControls.vue` around lines 158 - 163, The styling change removed the `!`-important modifier and split layout rules correctly between props: ensure the WidgetInputNumberInput usage sets the grid override on the root with `root-class="text-base-foreground grid-cols-[auto_96px]"` and places the input wrapper width on `class="*:[.min-w-0]:w-24"` so tailwind-merge via cn() lets `grid-cols-[auto_96px]` supersede `grid-cols-subgrid` and `w-24` equals 96px; apply the identical adjustment to the other occurrence of WidgetInputNumberInput (the one around lines 266-271) so both instances use `root-class` for grid overrides and `class` for the inner input wrapper.src/renderer/extensions/linearMode/OutputPreviewItem.vue (1)
10-14:⚠️ Potential issue | 🟡 MinorAdd
altattribute to the<img>element.The image is missing an
altattribute. For a purely decorative latent preview, usealt=""to suppress screen-reader noise. If the image carries meaningful content, provide a descriptive i18n string.🔧 Proposed fix
<img v-if="latentPreview" class="block size-10 rounded-sm object-cover" :src="latentPreview" + alt="" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputPreviewItem.vue` around lines 10 - 14, The <img> in OutputPreviewItem.vue that uses v-if="latentPreview" and :src="latentPreview" is missing an alt attribute; add an alt attribute to that <img> — if the preview is decorative set alt="" to hide it from screen readers, otherwise bind a localized descriptive string (e.g. :alt="$t('…')" or a computed/prop like :alt="latentPreviewAlt") so the image always has an accessible alt value.src/renderer/extensions/linearMode/OutputHistoryItem.vue (1)
14-21:⚠️ Potential issue | 🟡 MinorAdd an
altattribute to the<img>element.The image is missing an
altattribute, which is needed for accessibility and HTML validity. Usealt=""if purely decorative.🛠️ Suggested fix
<img v-if="getMediaType(output) === 'images'" class="block size-10 rounded-sm object-cover bg-secondary-background" loading="lazy" width="40" height="40" + alt="" :src="output.url" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistoryItem.vue` around lines 14 - 21, The <img> in OutputHistoryItem.vue rendered when getMediaType(output) === 'images' is missing an alt attribute; update the <img> element used in that v-if branch to include an appropriate alt (use alt="" if the image is decorative or bind a meaningful value like :alt="output.alt || ''") so the markup is accessible and valid, keeping the existing attributes (class, loading, width, height, :src) unchanged.
🧹 Nitpick comments (9)
src/renderer/extensions/linearMode/flattenNodeOutput.test.ts (1)
36-60: Missing isolated test for thevideomedia type.Every other handled media type (
images,audio,gifs,3d) has its own dedicated test exercisingmediaTypeandnodeId. The video path is only incidentally covered in the "multiple media types" test, which doesn't assert individual item attributes.✅ Suggested additional test
it('flattens video outputs', () => { const output = makeOutput({ video: [{ filename: 'clip.mp4', subfolder: '', type: 'output' }] }) const result = flattenNodeOutput(['99', output]) expect(result).toHaveLength(1) expect(result[0].mediaType).toBe('video') expect(result[0].nodeId).toBe('99') expect(result[0].filename).toBe('clip.mp4') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/flattenNodeOutput.test.ts` around lines 36 - 60, Add an isolated test that exercises the video path of flattenNodeOutput: create an output via makeOutput with a single video entry (e.g., filename 'clip.mp4'), call flattenNodeOutput with a nodeId like '99', and assert the returned array has length 1 and that result[0].mediaType === 'video', result[0].nodeId === '99', and result[0].filename === 'clip.mp4'; place this alongside the other media-type tests to mirror the existing audio/images/gifs/3d tests.src/renderer/extensions/linearMode/linearModeTypes.ts (1)
12-17: Consider makingOutputSelectiona discriminated union for stronger type safety.With both
assetandoutputoptional and no discriminant, the type allows both fields to be simultaneously set or simultaneously absent — neither of which appears intentional. TypeScript won't help callers narrow to the correct branch.♻️ Optional refactor to discriminated union
-export interface OutputSelection { - asset?: AssetItem - output?: ResultItemImpl - canShowPreview: boolean - latentPreviewUrl?: string -} +export type OutputSelection = + | { kind: 'asset'; asset: AssetItem; output?: never; canShowPreview: boolean; latentPreviewUrl?: string } + | { kind: 'output'; output: ResultItemImpl; asset?: never; canShowPreview: boolean; latentPreviewUrl?: string } + | { kind: 'latent'; asset?: never; output?: never; canShowPreview: boolean; latentPreviewUrl: string }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearModeTypes.ts` around lines 12 - 17, OutputSelection is currently a loose interface allowing asset and output to be both present or both absent; change it to a discriminated union to enforce exactly-one semantics. Replace OutputSelection with two interfaces discriminated by a literal field (e.g. kind: 'asset' | 'output') — one variant containing asset: AssetItem (and any asset-specific fields like latentPreviewUrl) and the other containing output: ResultItemImpl; include shared fields such as canShowPreview on both variants if needed. Update any code that constructs or narrows OutputSelection to switch/if on the discriminant (kind) instead of checking optional properties.src/renderer/extensions/linearMode/linearOutputStore.test.ts (2)
64-76: Consider usingsatisfiesinstead ofasfor the mock factory return.Per repository convention, test helpers constructing mock objects should prefer
satisfiesfor shape validation over type assertions.However, if
ExecutedWsMessagehas required fields not present here, theascast is pragmatic. Verify whether the full type is satisfied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearOutputStore.test.ts` around lines 64 - 76, The mock factory makeExecutedDetail currently returns the object using a type assertion ("as ExecutedWsMessage"); replace this with TypeScript's "satisfies ExecutedWsMessage" to validate the shape at compile time (i.e., change the return expression to use satisfies instead of as) and, if the compiler reports missing required fields on ExecutedWsMessage, add those minimal required properties to the returned object so the satisfies check passes while preserving the test intent.
78-89:afterEachdoesn't resetisAppModeRef, creating an inconsistency withbeforeEach.
beforeEachsetsisAppModeRef.value = true, butafterEachonly resetsactiveJobIdRefandpreviewsRef. While this is currently harmless (the nextbeforeEachre-sets it), it's inconsistent and could mask leaks if a test modifiesisAppModeRefwithout cleanup.🛠️ Suggested fix
afterEach(() => { activeJobIdRef.value = null previewsRef.value = {} + isAppModeRef.value = true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearOutputStore.test.ts` around lines 78 - 89, The afterEach cleanup is missing reset for isAppModeRef, causing inconsistency with beforeEach; update the afterEach block (alongside activeJobIdRef and previewsRef) to also reset isAppModeRef.value (e.g., set isAppModeRef.value = true) so tests that modify isAppModeRef are properly cleaned up; adjust the afterEach in the linearOutputStore.test to include this change.src/renderer/extensions/linearMode/linearOutputStore.ts (1)
233-243:{ deep: true }onpreviewsByPromptIdmay be expensive if the record grows large.The deep watcher traverses the entire
previewsByPromptIdrecord on every change. If this record accumulates many job entries, consider a more targeted approach (e.g., watching a computed that extracts only the active job's preview).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/linearOutputStore.ts` around lines 233 - 243, The current deep watcher on jobPreviewStore.previewsByPromptId is expensive; instead, watch a computed that selects only the active job's preview so you avoid full-record traversal: replace the watch call that references jobPreviewStore.previewsByPromptId with a watcher on a computed which reads executionStore.activeJobId and returns jobPreviewStore.previewsByPromptId[activeJobId], then in the watcher body keep the same guard using appModeStore.isAppMode and call onLatentPreview(jobId, url) when url exists; update references to watch, jobPreviewStore.previewsByPromptId, executionStore.activeJobId, appModeStore.isAppMode, and onLatentPreview accordingly.src/renderer/extensions/linearMode/useOutputHistory.ts (1)
21-61:outputsCacheis never invalidated — stale entries persist after job resolution.Once an asset's outputs are cached on line 24's early return, the entry is never evicted or refreshed. After a job resolves via the
pendingResolvepath (line 36), the cached in-progress-derived outputs remain even though the job has completed and history data may differ. In practice theResultItemImplobjects should be identical, but the cache also grows without bound across sessions.Consider clearing entries for a
jobIdwhen it resolves, or using aMapwith a bounded size / LRU eviction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/useOutputHistory.ts` around lines 21 - 61, The outputsCache in allOutputs keeps stale entries after jobs move from linearStore.pendingResolve to completed, causing incorrect/ever-growing cache; update the implementation to invalidate or refresh cache entries for the corresponding jobId when a job resolves (e.g., listen for the resolution event that removes jobId from linearStore.pendingResolve or hooks that update linearStore.inProgressItems and then delete outputsCache[itemId] for items whose user_metadata.jobId matches), and/or replace outputsCache with a Map implementing bounded size or LRU eviction to avoid unbounded growth; refer to outputsCache, allOutputs, user_metadata.jobId, linearStore.pendingResolve and linearStore.inProgressItems to locate where to add eviction logic.src/views/LinearView.vue (1)
102-104: Verify the+16pxoffset covers all gutter widths.The
w-[calc(100%+16px)]presumably compensates for the Splitter gutter (w-4 -mx-1). If gutter sizing changes, this value will silently drift. Consider leaving a brief note (or deriving from a shared constant) if this coupling is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/LinearView.vue` around lines 102 - 104, The hardcoded w-[calc(100%+16px)] on the LinearProgressBar is compensating for the Splitter gutter (w-4 -mx-1) and will drift if gutter sizing changes; update the LinearProgressBar usage to either derive the offset from a shared CSS variable/constant (e.g., --splitter-gutter or a shared spacing token) and use w-[calc(100%+var(--splitter-gutter))] or add an inline comment next to the LinearProgressBar class explaining the coupling to the Splitter gutter (and reference the Splitter's class names) so future changes to Splitter gutter size are noticed and kept in sync.src/renderer/extensions/linearMode/LinearPreview.vue (1)
73-82: The 500ms timeout is fragile for seed updates.The FIXME is already acknowledged, but
await new Promise((r) => setTimeout(r, 500))with the note that seeds still fail to update suggests this workaround is unreliable. Consider tracking this as a follow-up issue.Do you want me to open an issue to track replacing this timeout with a proper synchronization mechanism?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/LinearPreview.vue` around lines 73 - 82, The 500ms sleep in rerun is a fragile workaround; replace it with a deterministic synchronization: have loadWorkflow (or the module that applies seeds) return or emit a promise/event that resolves when seeds/state are fully applied, then await that promise in rerun before calling executeWidgetsCallback and runButtonClick; specifically update loadWorkflow(selectedItem.value) to expose a completion signal (or add a new function like waitForSeedsApplied) and use that in rerun instead of setTimeout, and if you cannot implement this immediately, open a follow-up issue and replace the timeout line with a clear TODO referencing that issue while keeping behavior unchanged.src/renderer/extensions/linearMode/OutputHistory.vue (1)
49-51:visibleHistorycallsallOutputs()for every asset on each recomputation.Each call to
allOutputs(a)may triggeruseAsyncState(for uncached items), potentially creating a waterfall of network requests when many assets are loaded. The cache inuseOutputHistorymitigates repeated calls, but the initial load of a large history could be heavy.This is likely acceptable for typical workloads but worth monitoring.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/linearMode/OutputHistory.vue` around lines 49 - 51, visibleHistory currently calls allOutputs(a) on every recompute which can trigger useAsyncState per asset and cause many initial network requests; to fix, stop invoking allOutputs inside the filter and instead precompute/cache allOutputs results once for the current outputs.media (e.g. build a computed/map of asset -> allOutputs result using the useOutputHistory cache or Promise.all for initial load) and then let visibleHistory filter based on those cached/precomputed values; reference visibleHistory, allOutputs, useAsyncState and useOutputHistory to locate and change the logic so repeated recomputations reuse the cached allOutputs results rather than re-calling allOutputs for each asset.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/extensions/linearMode/flattenNodeOutput.ts`:
- Around line 8-13: The flattenNodeOutput logic currently assigns mediaType
'gifs' for GIF outputs which has no matching key in mediaTypes, causing missing
icons; update the branch in flattenNodeOutput that handles nodeOutput.gifs so
that GIF items are mapped to mediaType 'images' (i.e., when building
knownOutputs and the ResultItem for each gif, set mediaType='images' instead of
'gifs'), keeping the existing casts (as ResultItem[]) and preserving other
fields on the output; alternatively (less preferred) add a 'gifs' entry to
mediaTypes, but implement the preferred remap in flattenNodeOutput (refer to the
knownOutputs variable and the nodeOutput.gifs handling).
In `@src/renderer/extensions/linearMode/linearOutputStore.ts`:
- Around line 90-93: The expression that computes nodeId in onNodeExecuted uses
the || operator which wrongly treats falsy-but-valid values (like 0 or '') as
absent; change the fallback from using || to the nullish coalescing operator ??
so nodeId is computed as String(detail.display_node ?? detail.node) to preserve
valid falsy values while still falling back when display_node is null or
undefined.
---
Outside diff comments:
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 220-244: The wheel handler currently registered with
useEventListener(document.body, ...) calls preventDefault()/stopPropagation()
for every Ctrl+wheel and thus blocks browser zoom globally; update the listener
to only act when the event originates inside the output area (e.g., scope the
listener to outputsRef instead of document.body or add an early guard like if
(!outputsRef?.value?.contains(e.target as Node)) return), and only call
e.preventDefault() / e.stopPropagation() and run pointer.isTrackpadGesture /
navigateToAdjacent / scrollOffset logic when the event is within that outputsRef
element; keep the same options ({ capture: true, passive: false }) when
attaching to the scoped element.
---
Duplicate comments:
In `@src/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 158-163: The styling change removed the `!`-important modifier and
split layout rules correctly between props: ensure the WidgetInputNumberInput
usage sets the grid override on the root with `root-class="text-base-foreground
grid-cols-[auto_96px]"` and places the input wrapper width on
`class="*:[.min-w-0]:w-24"` so tailwind-merge via cn() lets
`grid-cols-[auto_96px]` supersede `grid-cols-subgrid` and `w-24` equals 96px;
apply the identical adjustment to the other occurrence of WidgetInputNumberInput
(the one around lines 266-271) so both instances use `root-class` for grid
overrides and `class` for the inner input wrapper.
In `@src/renderer/extensions/linearMode/OutputHistoryItem.vue`:
- Around line 14-21: The <img> in OutputHistoryItem.vue rendered when
getMediaType(output) === 'images' is missing an alt attribute; update the <img>
element used in that v-if branch to include an appropriate alt (use alt="" if
the image is decorative or bind a meaningful value like :alt="output.alt || ''")
so the markup is accessible and valid, keeping the existing attributes (class,
loading, width, height, :src) unchanged.
In `@src/renderer/extensions/linearMode/OutputPreviewItem.vue`:
- Around line 10-14: The <img> in OutputPreviewItem.vue that uses
v-if="latentPreview" and :src="latentPreview" is missing an alt attribute; add
an alt attribute to that <img> — if the preview is decorative set alt="" to hide
it from screen readers, otherwise bind a localized descriptive string (e.g.
:alt="$t('…')" or a computed/prop like :alt="latentPreviewAlt") so the image
always has an accessible alt value.
---
Nitpick comments:
In `@src/renderer/extensions/linearMode/flattenNodeOutput.test.ts`:
- Around line 36-60: Add an isolated test that exercises the video path of
flattenNodeOutput: create an output via makeOutput with a single video entry
(e.g., filename 'clip.mp4'), call flattenNodeOutput with a nodeId like '99', and
assert the returned array has length 1 and that result[0].mediaType === 'video',
result[0].nodeId === '99', and result[0].filename === 'clip.mp4'; place this
alongside the other media-type tests to mirror the existing audio/images/gifs/3d
tests.
In `@src/renderer/extensions/linearMode/linearModeTypes.ts`:
- Around line 12-17: OutputSelection is currently a loose interface allowing
asset and output to be both present or both absent; change it to a discriminated
union to enforce exactly-one semantics. Replace OutputSelection with two
interfaces discriminated by a literal field (e.g. kind: 'asset' | 'output') —
one variant containing asset: AssetItem (and any asset-specific fields like
latentPreviewUrl) and the other containing output: ResultItemImpl; include
shared fields such as canShowPreview on both variants if needed. Update any code
that constructs or narrows OutputSelection to switch/if on the discriminant
(kind) instead of checking optional properties.
In `@src/renderer/extensions/linearMode/linearOutputStore.test.ts`:
- Around line 64-76: The mock factory makeExecutedDetail currently returns the
object using a type assertion ("as ExecutedWsMessage"); replace this with
TypeScript's "satisfies ExecutedWsMessage" to validate the shape at compile time
(i.e., change the return expression to use satisfies instead of as) and, if the
compiler reports missing required fields on ExecutedWsMessage, add those minimal
required properties to the returned object so the satisfies check passes while
preserving the test intent.
- Around line 78-89: The afterEach cleanup is missing reset for isAppModeRef,
causing inconsistency with beforeEach; update the afterEach block (alongside
activeJobIdRef and previewsRef) to also reset isAppModeRef.value (e.g., set
isAppModeRef.value = true) so tests that modify isAppModeRef are properly
cleaned up; adjust the afterEach in the linearOutputStore.test to include this
change.
In `@src/renderer/extensions/linearMode/linearOutputStore.ts`:
- Around line 233-243: The current deep watcher on
jobPreviewStore.previewsByPromptId is expensive; instead, watch a computed that
selects only the active job's preview so you avoid full-record traversal:
replace the watch call that references jobPreviewStore.previewsByPromptId with a
watcher on a computed which reads executionStore.activeJobId and returns
jobPreviewStore.previewsByPromptId[activeJobId], then in the watcher body keep
the same guard using appModeStore.isAppMode and call onLatentPreview(jobId, url)
when url exists; update references to watch, jobPreviewStore.previewsByPromptId,
executionStore.activeJobId, appModeStore.isAppMode, and onLatentPreview
accordingly.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 73-82: The 500ms sleep in rerun is a fragile workaround; replace
it with a deterministic synchronization: have loadWorkflow (or the module that
applies seeds) return or emit a promise/event that resolves when seeds/state are
fully applied, then await that promise in rerun before calling
executeWidgetsCallback and runButtonClick; specifically update
loadWorkflow(selectedItem.value) to expose a completion signal (or add a new
function like waitForSeedsApplied) and use that in rerun instead of setTimeout,
and if you cannot implement this immediately, open a follow-up issue and replace
the timeout line with a clear TODO referencing that issue while keeping behavior
unchanged.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 49-51: visibleHistory currently calls allOutputs(a) on every
recompute which can trigger useAsyncState per asset and cause many initial
network requests; to fix, stop invoking allOutputs inside the filter and instead
precompute/cache allOutputs results once for the current outputs.media (e.g.
build a computed/map of asset -> allOutputs result using the useOutputHistory
cache or Promise.all for initial load) and then let visibleHistory filter based
on those cached/precomputed values; reference visibleHistory, allOutputs,
useAsyncState and useOutputHistory to locate and change the logic so repeated
recomputations reuse the cached allOutputs results rather than re-calling
allOutputs for each asset.
In `@src/renderer/extensions/linearMode/useOutputHistory.ts`:
- Around line 21-61: The outputsCache in allOutputs keeps stale entries after
jobs move from linearStore.pendingResolve to completed, causing
incorrect/ever-growing cache; update the implementation to invalidate or refresh
cache entries for the corresponding jobId when a job resolves (e.g., listen for
the resolution event that removes jobId from linearStore.pendingResolve or hooks
that update linearStore.inProgressItems and then delete outputsCache[itemId] for
items whose user_metadata.jobId matches), and/or replace outputsCache with a Map
implementing bounded size or LRU eviction to avoid unbounded growth; refer to
outputsCache, allOutputs, user_metadata.jobId, linearStore.pendingResolve and
linearStore.inProgressItems to locate where to add eviction logic.
In `@src/views/LinearView.vue`:
- Around line 102-104: The hardcoded w-[calc(100%+16px)] on the
LinearProgressBar is compensating for the Splitter gutter (w-4 -mx-1) and will
drift if gutter sizing changes; update the LinearProgressBar usage to either
derive the offset from a shared CSS variable/constant (e.g., --splitter-gutter
or a shared spacing token) and use w-[calc(100%+var(--splitter-gutter))] or add
an inline comment next to the LinearProgressBar class explaining the coupling to
the Splitter gutter (and reference the Splitter's class names) so future changes
to Splitter gutter size are noticed and kept in sync.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/design-system/src/css/style.csssrc/locales/en/main.jsonsrc/renderer/extensions/linearMode/LatentPreview.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/LinearProgressBar.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/OutputHistoryItem.vuesrc/renderer/extensions/linearMode/OutputPreviewItem.vuesrc/renderer/extensions/linearMode/flattenNodeOutput.test.tssrc/renderer/extensions/linearMode/flattenNodeOutput.tssrc/renderer/extensions/linearMode/linearModeTypes.tssrc/renderer/extensions/linearMode/linearOutputStore.test.tssrc/renderer/extensions/linearMode/linearOutputStore.tssrc/renderer/extensions/linearMode/mediaTypes.tssrc/renderer/extensions/linearMode/useOutputHistory.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vuesrc/utils/dateTimeUtil.tssrc/views/LinearView.vue
💤 Files with no reviewable changes (1)
- src/utils/dateTimeUtil.ts
Comment on lines
+8
to
+13
| const knownOutputs: Record<string, ResultItem[]> = {} | ||
| if (nodeOutput.audio) knownOutputs.audio = nodeOutput.audio | ||
| if (nodeOutput.images) knownOutputs.images = nodeOutput.images | ||
| if (nodeOutput.video) knownOutputs.video = nodeOutput.video | ||
| if (nodeOutput.gifs) knownOutputs.gifs = nodeOutput.gifs as ResultItem[] | ||
| if (nodeOutput['3d']) knownOutputs['3d'] = nodeOutput['3d'] as ResultItem[] |
Contributor
There was a problem hiding this comment.
'gifs' mediaType will silently produce missing icons in UI consumers.
flattenNodeOutput writes mediaType = 'gifs' for gif outputs (line 12), but mediaTypes in mediaTypes.ts has no 'gifs' key. Any component that does mediaTypes[getMediaType(output)]?.iconClass for a GIF item receives undefined, so no icon is displayed. Since getMediaType delegates to output.mediaType for non-video items, GIFs are unresolvable.
Fix options:
- A (preferred): map GIF outputs to
mediaType = 'images'here (GIFs are images), relying onisImageBySuffixfor rendering. - B: add a
'gifs'entry tomediaTypes.tswith the image icon.
🔧 Option A — remap gifs to 'images' media type
- if (nodeOutput.gifs) knownOutputs.gifs = nodeOutput.gifs as ResultItem[]
+ if (nodeOutput.gifs) knownOutputs.images = [
+ ...(knownOutputs.images ?? []),
+ ...(nodeOutput.gifs as ResultItem[])
+ ]The as ResultItem[] casts on lines 12–13 are needed because gifs/3d schema types differ from ResultItem[]. This is not as any, so it's within type-safety guidelines — but note that any schema-specific fields on those items (e.g. format, frame_rate) are still passed through at runtime via the spread { ...output, ... }.
📝 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 knownOutputs: Record<string, ResultItem[]> = {} | |
| if (nodeOutput.audio) knownOutputs.audio = nodeOutput.audio | |
| if (nodeOutput.images) knownOutputs.images = nodeOutput.images | |
| if (nodeOutput.video) knownOutputs.video = nodeOutput.video | |
| if (nodeOutput.gifs) knownOutputs.gifs = nodeOutput.gifs as ResultItem[] | |
| if (nodeOutput['3d']) knownOutputs['3d'] = nodeOutput['3d'] as ResultItem[] | |
| const knownOutputs: Record<string, ResultItem[]> = {} | |
| if (nodeOutput.audio) knownOutputs.audio = nodeOutput.audio | |
| if (nodeOutput.images) knownOutputs.images = nodeOutput.images | |
| if (nodeOutput.video) knownOutputs.video = nodeOutput.video | |
| if (nodeOutput.gifs) knownOutputs.images = [ | |
| ...(knownOutputs.images ?? []), | |
| ...(nodeOutput.gifs as ResultItem[]) | |
| ] | |
| if (nodeOutput['3d']) knownOutputs['3d'] = nodeOutput['3d'] as ResultItem[] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/extensions/linearMode/flattenNodeOutput.ts` around lines 8 - 13,
The flattenNodeOutput logic currently assigns mediaType 'gifs' for GIF outputs
which has no matching key in mediaTypes, causing missing icons; update the
branch in flattenNodeOutput that handles nodeOutput.gifs so that GIF items are
mapped to mediaType 'images' (i.e., when building knownOutputs and the
ResultItem for each gif, set mediaType='images' instead of 'gifs'), keeping the
existing casts (as ResultItem[]) and preserving other fields on the output;
alternatively (less preferred) add a 'gifs' entry to mediaTypes, but implement
the preferred remap in flattenNodeOutput (refer to the knownOutputs variable and
the nodeOutput.gifs handling).
Comment on lines
+90
to
+93
| function onNodeExecuted(jobId: string, detail: ExecutedWsMessage) { | ||
| const nodeId = String(detail.display_node || detail.node) | ||
| const newOutputs = flattenNodeOutput([nodeId, detail.output]) | ||
| if (newOutputs.length === 0) return |
Contributor
There was a problem hiding this comment.
|| may discard falsy-but-valid display_node values — consider ??.
String(detail.display_node || detail.node) falls through to detail.node when display_node is 0, '', or null. If node IDs can be 0 (numeric), the || operator would skip a valid value. Using ?? preserves 0 and '' while only falling back on null/undefined.
🛠️ Suggested fix
- const nodeId = String(detail.display_node || detail.node)
+ const nodeId = String(detail.display_node ?? detail.node)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/extensions/linearMode/linearOutputStore.ts` around lines 90 -
93, The expression that computes nodeId in onNodeExecuted uses the || operator
which wrongly treats falsy-but-valid values (like 0 or '') as absent; change the
fallback from using || to the nullish coalescing operator ?? so nodeId is
computed as String(detail.display_node ?? detail.node) to preserve valid falsy
values while still falling back when display_node is null or undefined.
AustinMroz
approved these changes
Feb 23, 2026
This was referenced Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds new store for tracking outputs lin linear mode and reworks outputs to display the following: skeleton -> latent preview -> node output -> job result.
Changes
Review Focus
Getting all the events and stores aligned to happen consistently and in the correct order was a challenge, unifying the various sources. so suggestions there would be good
Screenshots (if applicable)
app.mode.progress.mp4
┆Issue is synchronized with this Notion page by Unito