feat(assets): wire infinite scroll to the flat-output provider in the widget select dropdown#12780
Conversation
📝 WalkthroughWalkthroughThe PR adds an infinite-scroll capability to the asset select dropdown. A new ChangesInfinite Scroll Dropdown + Cursor Pagination
Sequence DiagramsequenceDiagram
participant User
participant WidgetSelectDropdown
participant FormDropdown
participant FormDropdownMenu
participant VirtualGrid
participant assetsStore
participant assetService
User->>VirtualGrid: scrolls near end of list
VirtualGrid->>FormDropdownMenu: emit approach-end
FormDropdownMenu->>FormDropdown: emit approach-end
FormDropdown->>WidgetSelectDropdown: emit approach-end
WidgetSelectDropdown->>WidgetSelectDropdown: handleApproachEnd (debounced, guards hasMore/loading)
WidgetSelectDropdown->>assetsStore: loadMoreFlatOutputs()
assetsStore->>assetService: getAssetsPageByTag('output', true, {limit, after: cursor})
assetService-->>assetsStore: AssetResponse {items, has_more, next_cursor}
assetsStore->>assetsStore: dedupe + append, update flatOutputNextCursor
assetsStore-->>FormDropdownMenu: isLoadingMore → renders loading row
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12780 +/- ##
===========================================
- Coverage 75.59% 62.90% -12.70%
===========================================
Files 1570 1459 -111
Lines 95272 74414 -20858
Branches 27359 19340 -8019
===========================================
- Hits 72021 46810 -25211
- Misses 22458 27260 +4802
+ Partials 793 344 -449
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1145 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…rg#12781) ## Summary Every `VirtualGrid` consumer (assets sidebar, manager dialog, widget select dropdown) is blind to discrete mouse-wheel scrolling: `useScroll`'s `throttle: 64` never reports scroll position, so the virtualization window stays frozen — users see blank space below the first viewport of items and `approach-end` (infinite scroll) never fires. Trackpad scrolling masks the bug by emitting events faster than the throttle window. ## Changes - **What**: drop the `throttle` option from `useScroll` in `VirtualGrid` and remove the `scrollThrottle` prop (no consumer passes it). Scroll events are frame-aligned and the handler is cheap, so the throttle bought nothing even when it worked. - **Root cause**: VueUse ≥14 `throttleFilter` with `leading=false` (what `useScroll` uses) marks spaced-out events as executed without executing them — each event re-arms an `isLeading` restore timer that makes the next event skip its invoke, and the trailing branch is unreachable when `elapsed > duration`. Regression of vueuse#2390; still present on vueuse `main`. ## Review Focus - Verified live against staging: with the throttle, sidebar scrolled to `scrollTop` 1250 while `scrollY` stayed 0 and the render window stayed at `[0..3)` of 27 (blank viewport); with this fix, `scrollY` tracks 1:1 and the window advances. Bare-vs-throttled `useScroll` compared side-by-side on the same element to isolate the cause. - Unblocks wheel-scroll for Comfy-Org#12780's dropdown infinite scroll with no changes there. - Fixes FE-990 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Calls the reusable Comfy-Org/github-workflows cursor-review.yml (single source of truth for panel, judge, prompts, scripts) instead of a standalone copy. Label-gated to the team; secret-bearing jobs skip fork PRs. Judge overridden to Opus 4.8.
dante01yoon
left a comment
There was a problem hiding this comment.
Read-only review at 5de4e08 (not approving, per request). Stacked on #12774 — base is matt/fe-985…, so this needs that to merge / a retarget to main first.
The event-forwarding chain (VirtualGrid → FormDropdownMenu → FormDropdown → WidgetSelectDropdown → loadMore) is clean, the debounce + hasMore/isLoadingMore guard mirrors the sidebar idiom, and the spinner row sizes its iconify icon with size-6 (correct — not a font-size class). Tests cover both the approach-end forward and the loading-more row toggle. One non-blocking question inline.
Heads-up: #12417 (FE-732) also edits WidgetSelectDropdown.vue; no logic overlap, but expect a small rebase conflict whichever lands second.
| } | ||
| } | ||
|
|
||
| const handleApproachEnd = useDebounceFn(async () => { |
There was a problem hiding this comment.
question (non-blocking): per the PR description, VueUse ≥14's throttleFilter(leading=false) makes useScroll's throttle: 64 in VirtualGrid drop discrete mouse-wheel events, so approach-end only fires for high-frequency (trackpad-style) scrolling. That leaves this wiring effectively inert for mouse-wheel users until the separate throttle bug is fixed. Is the intent to land the (harmless-but-inactive) wiring ahead of that fix, or to stack the throttle fix first so FE-988 ships actually-working for everyone? Either is defensible — just flagging that on merge the feature is non-functional for a large share of users.
There was a problem hiding this comment.
Confirmed on the wiring side: @approach-end is debounced 300ms and guarded by hasMore/isLoadingMore before loadMore(), with :loading-more driving the spinner — correct in isolation and a no-op when approach-end never fires. Your premise holds too: the event originates upstream in VirtualGrid's useScroll({ throttle: 64 }), and the leading=false wheel-event drop lives there, outside this diff.
So this lands as-is: the mouse-wheel dormancy is a pre-existing VirtualGrid throttle limitation that's tracked separately — not introduced or worsened by this PR, and trackpad scrolling works today. Not gating this change.
- fetchFlatOutputs threads next_cursor into after on loadMore, falling back to offset paging for backends that mint no cursors - hasMore is now server-authoritative (has_more) instead of the page-length heuristic, with empty-page and non-advancing-cursor guards - reset path clears the cursor and stays a from-the-top head fetch
- pin cursor preservation across a failed loadMore (retry resumes via after) - pin cursor clearing on refresh via the failed-refresh path (kills a surviving mutant on the reset-path clear) - rename error test to match actual preserve-on-error behavior - use vi.resetAllMocks to isolate per-test once-queues
… widget select dropdown - VirtualGrid approach-end forwards through FormDropdownMenu and FormDropdown to WidgetSelectDropdown, which loads the next page via the assets provider (cursor-paginated on cloud, jobs history on OSS) - guarded by hasMore/isLoadingMore and debounced, mirroring the assets sidebar idiom - loadingMore spinner row below the grid while a page is in flight
6a5a4ca to
00fc87f
Compare
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 7 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 2 |
| 🟢 Low | 4 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
…lat-output-provider-in-the
🎭 Playwright: ✅ 1671 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.44 MB gzip 🔴 +356 BDetailsSummary
Category Glance App Entry Points — 45.8 kB (baseline 45.8 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 95.3 kB (baseline 95.3 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 3 unchanged Panels & Settings — 523 kB (baseline 523 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 14 unchanged User & Accounts — 19.9 kB (baseline 19.9 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed / 3 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed / 1 unchanged UI Components — 57.2 kB (baseline 57.2 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 266 kB (baseline 266 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 12 added / 12 removed / 3 unchanged Utilities & Hooks — 3.32 MB (baseline 3.32 MB) • 🔴 +425 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed / 16 unchanged Vendor & Third-Party — 15.3 MB (baseline 15.3 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 10.4 MB (baseline 10.4 MB) • 🔴 +1.3 kBBundles that do not match a named category
Status: 62 added / 62 removed / 86 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-06-19T02:46:52.595Z",
"gitSha": "00ef00af64e9516e4cd5e3091cea64f45be681b5",
"branch": "matt/fe-988-fe-wire-infinite-scroll-to-the-flat-output-provider-in-the",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2021.1050000000341,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.574,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 442.243,
"heapDeltaBytes": -2907852,
"heapUsedBytes": 55834784,
"domNodes": 20,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 20.814000000000004,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2022.878999999989,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.486,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 435.95900000000006,
"heapDeltaBytes": -2539092,
"heapUsedBytes": 56153912,
"domNodes": 18,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 23.728,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1897.7140000000645,
"styleRecalcs": 77,
"styleRecalcDurationMs": 43.588,
"layouts": 12,
"layoutDurationMs": 3.952000000000001,
"taskDurationMs": 850.1460000000001,
"heapDeltaBytes": -14467016,
"heapUsedBytes": 54027960,
"domNodes": -211,
"jsHeapTotalBytes": 21671936,
"scriptDurationMs": 126.80799999999998,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1854.755999999952,
"styleRecalcs": 77,
"styleRecalcDurationMs": 43.507,
"layouts": 12,
"layoutDurationMs": 4.0600000000000005,
"taskDurationMs": 822.596,
"heapDeltaBytes": -7127332,
"heapUsedBytes": 51359608,
"domNodes": 61,
"jsHeapTotalBytes": 25427968,
"scriptDurationMs": 137.45200000000003,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1770.5220000000281,
"styleRecalcs": 32,
"styleRecalcDurationMs": 20.296000000000003,
"layouts": 6,
"layoutDurationMs": 0.6950000000000001,
"taskDurationMs": 437.17900000000003,
"heapDeltaBytes": -3685208,
"heapUsedBytes": 64494536,
"domNodes": -261,
"jsHeapTotalBytes": 11972608,
"scriptDurationMs": 38.675000000000004,
"eventListeners": -184,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1740.9059999999954,
"styleRecalcs": 30,
"styleRecalcDurationMs": 18.951000000000004,
"layouts": 6,
"layoutDurationMs": 0.72,
"taskDurationMs": 351.568,
"heapDeltaBytes": 1698800,
"heapUsedBytes": 60021708,
"domNodes": 78,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 30.476999999999997,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 572.9420000000118,
"styleRecalcs": 10,
"styleRecalcDurationMs": 6.761000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 354.614,
"heapDeltaBytes": 6902932,
"heapUsedBytes": 65548896,
"domNodes": 16,
"jsHeapTotalBytes": 18612224,
"scriptDurationMs": 59.86000000000001,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "dom-widget-clipping",
"durationMs": 607.0760000000064,
"styleRecalcs": 13,
"styleRecalcDurationMs": 10.103000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 389.465,
"heapDeltaBytes": 7623500,
"heapUsedBytes": 65939992,
"domNodes": 22,
"jsHeapTotalBytes": 18874368,
"scriptDurationMs": 71.89099999999999,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-idle",
"durationMs": 2023.9820000000464,
"styleRecalcs": 10,
"styleRecalcDurationMs": 11.924000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 664.7299999999999,
"heapDeltaBytes": -8883624,
"heapUsedBytes": 61136428,
"domNodes": 20,
"jsHeapTotalBytes": 11677696,
"scriptDurationMs": 119.61299999999999,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2041.8170000000373,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.405000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 631.087,
"heapDeltaBytes": -8630060,
"heapUsedBytes": 61857528,
"domNodes": 20,
"jsHeapTotalBytes": 11153408,
"scriptDurationMs": 119.24400000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2170.0060000000576,
"styleRecalcs": 69,
"styleRecalcDurationMs": 19.685000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1174.601,
"heapDeltaBytes": 10393012,
"heapUsedBytes": 83144012,
"domNodes": 16,
"jsHeapTotalBytes": 12115968,
"scriptDurationMs": 415.263,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2109.1330000000426,
"styleRecalcs": 69,
"styleRecalcDurationMs": 19.83,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1162.2839999999999,
"heapDeltaBytes": 10305916,
"heapUsedBytes": 83247912,
"domNodes": 18,
"jsHeapTotalBytes": 12378112,
"scriptDurationMs": 412.89500000000004,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3098.357999999962,
"styleRecalcs": 65,
"styleRecalcDurationMs": 20.207,
"layouts": 60,
"layoutDurationMs": 7.967999999999999,
"taskDurationMs": 1377.8400000000001,
"heapDeltaBytes": 13665764,
"heapUsedBytes": 68513436,
"domNodes": 14,
"jsHeapTotalBytes": 5767168,
"scriptDurationMs": 517.554,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3163.7719999999945,
"styleRecalcs": 67,
"styleRecalcDurationMs": 21.649,
"layouts": 60,
"layoutDurationMs": 8.19,
"taskDurationMs": 1403.7640000000001,
"heapDeltaBytes": -6883436,
"heapUsedBytes": 69317040,
"domNodes": 18,
"jsHeapTotalBytes": 9318400,
"scriptDurationMs": 510.44200000000006,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2031.0180000000173,
"styleRecalcs": 6,
"styleRecalcDurationMs": 5.233999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 677.337,
"heapDeltaBytes": -28446896,
"heapUsedBytes": 57671020,
"domNodes": -307,
"jsHeapTotalBytes": 102400,
"scriptDurationMs": 111.80999999999999,
"eventListeners": -197,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2028.9980000000014,
"styleRecalcs": 8,
"styleRecalcDurationMs": 9.440000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 675.918,
"heapDeltaBytes": -10223012,
"heapUsedBytes": 63563500,
"domNodes": 16,
"jsHeapTotalBytes": 7221248,
"scriptDurationMs": 111.699,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 617.4379999999928,
"styleRecalcs": 49,
"styleRecalcDurationMs": 14.666999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 418.07400000000007,
"heapDeltaBytes": 8228444,
"heapUsedBytes": 66940184,
"domNodes": 24,
"jsHeapTotalBytes": 19398656,
"scriptDurationMs": 141.22899999999998,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 571.9420000000355,
"styleRecalcs": 48,
"styleRecalcDurationMs": 13.171000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 367.088,
"heapDeltaBytes": 9974296,
"heapUsedBytes": 61019496,
"domNodes": 22,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 125.503,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-idle",
"durationMs": 2038.5890000000018,
"styleRecalcs": 10,
"styleRecalcDurationMs": 11.806000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 493.05899999999997,
"heapDeltaBytes": -9244236,
"heapUsedBytes": 59145700,
"domNodes": -313,
"jsHeapTotalBytes": 20361216,
"scriptDurationMs": 27.570000000000004,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 1999.3050000000494,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 421.11899999999997,
"heapDeltaBytes": -3035292,
"heapUsedBytes": 55781024,
"domNodes": 18,
"jsHeapTotalBytes": 25165824,
"scriptDurationMs": 20.046,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1691.1790000000337,
"styleRecalcs": 76,
"styleRecalcDurationMs": 38.485,
"layouts": 16,
"layoutDurationMs": 4.118,
"taskDurationMs": 724.77,
"heapDeltaBytes": 15374448,
"heapUsedBytes": 73965804,
"domNodes": 65,
"jsHeapTotalBytes": 24903680,
"scriptDurationMs": 102.40599999999999,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1715.750000000071,
"styleRecalcs": 75,
"styleRecalcDurationMs": 42.17999999999999,
"layouts": 16,
"layoutDurationMs": 4.9510000000000005,
"taskDurationMs": 761.247,
"heapDeltaBytes": -13691732,
"heapUsedBytes": 55251852,
"domNodes": -225,
"jsHeapTotalBytes": 16691200,
"scriptDurationMs": 103.82600000000001,
"eventListeners": -199,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-transition-enter",
"durationMs": 1399.234999999976,
"styleRecalcs": 16,
"styleRecalcDurationMs": 30.262000000000004,
"layouts": 4,
"layoutDurationMs": 15.197,
"taskDurationMs": 872.6899999999997,
"heapDeltaBytes": 4616604,
"heapUsedBytes": 80079084,
"domNodes": 13833,
"jsHeapTotalBytes": 17039360,
"scriptDurationMs": 44.72199999999999,
"eventListeners": 2529,
"totalBlockingTimeMs": 175,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "viewport-pan-sweep",
"durationMs": 8227.704000000016,
"styleRecalcs": 252,
"styleRecalcDurationMs": 58.324000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4026.35,
"heapDeltaBytes": -2645336,
"heapUsedBytes": 67436156,
"domNodes": 22,
"jsHeapTotalBytes": 19456000,
"scriptDurationMs": 1331.793,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "viewport-pan-sweep",
"durationMs": 8207.677999999987,
"styleRecalcs": 252,
"styleRecalcDurationMs": 62.897000000000006,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4118.713,
"heapDeltaBytes": -4049276,
"heapUsedBytes": 67427164,
"domNodes": 22,
"jsHeapTotalBytes": 17620992,
"scriptDurationMs": 1349.315,
"eventListeners": 20,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13652.467,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13633.516,
"heapDeltaBytes": -35542048,
"heapUsedBytes": 167620224,
"domNodes": -3308,
"jsHeapTotalBytes": 18321408,
"scriptDurationMs": 634.805,
"eventListeners": -16472,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 12861.16400000003,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12844.851999999999,
"heapDeltaBytes": -22423928,
"heapUsedBytes": 169612224,
"domNodes": -3308,
"jsHeapTotalBytes": 19369984,
"scriptDurationMs": 617.6310000000001,
"eventListeners": -16474,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 15156.518000000005,
"styleRecalcs": 72,
"styleRecalcDurationMs": 19.397,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 15136.993,
"heapDeltaBytes": -24965784,
"heapUsedBytes": 184376252,
"domNodes": -8329,
"jsHeapTotalBytes": 19775488,
"scriptDurationMs": 891.4050000000001,
"eventListeners": -16464,
"totalBlockingTimeMs": 60,
"frameDurationMs": 17.219999999999953,
"p95FrameDurationMs": 16.80000000000291
},
{
"name": "vue-large-graph-pan",
"durationMs": 14719.71199999996,
"styleRecalcs": 65,
"styleRecalcDurationMs": 18.224000000000018,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14693.831999999999,
"heapDeltaBytes": -37019256,
"heapUsedBytes": 167954328,
"domNodes": -3304,
"jsHeapTotalBytes": 18321408,
"scriptDurationMs": 917.4680000000001,
"eventListeners": -16472,
"totalBlockingTimeMs": 40,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 467.47800000002826,
"styleRecalcs": 17,
"styleRecalcDurationMs": 24.537999999999997,
"layouts": 5,
"layoutDurationMs": 1.7720000000000002,
"taskDurationMs": 134.406,
"heapDeltaBytes": 5260944,
"heapUsedBytes": 69615784,
"domNodes": 168,
"jsHeapTotalBytes": 4980736,
"scriptDurationMs": 24.530000000000005,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66666666666665,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "workflow-execution",
"durationMs": 456.9819999999254,
"styleRecalcs": 17,
"styleRecalcDurationMs": 23.558000000000003,
"layouts": 5,
"layoutDurationMs": 1.278,
"taskDurationMs": 126.07800000000002,
"heapDeltaBytes": 5237832,
"heapUsedBytes": 64885804,
"domNodes": 157,
"jsHeapTotalBytes": 2883584,
"scriptDurationMs": 22.352999999999998,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.660000000000007,
"p95FrameDurationMs": 16.800000000000182
}
]
} |
… during initial load - flatOutputHasMore now gates on fresh.length > 0 instead of batch.length > 0, so cursor pages whose items are all duplicates terminate pagination rather than looping infinitely on approach-end - handleApproachEnd adds !loading guard to prevent concurrent loadMore during the opening refresh fetch
|
Deployment failed with the following error: Learn More: https://vercel.com/uy-tieu-s-projects?upgradeToPro=build-rate-limit |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/stores/assetsStore.test.ts (1)
1734-1760: ⚡ Quick winStrengthen failed-refresh regression coverage.
This case validates call arguments but not resulting list integrity. Please assert that retrying after a failed refresh does not introduce duplicate asset IDs.
Suggested test tightening
it('restarts from the head when loadMore follows a failed refresh', async () => { vi.mocked(assetService.getAssetsPageByTag) .mockResolvedValueOnce( makePage([makeAsset('a1', 'f1.png')], { hasMore: true, nextCursor: 'cursor-1' }) ) .mockRejectedValueOnce(new Error('network down')) - .mockResolvedValueOnce(makePage([makeAsset('a2', 'f2.png')])) + // Head page again after refresh failure + .mockResolvedValueOnce(makePage([makeAsset('a1', 'f1.png')])) @@ await store.updateFlatOutputs() await store.updateFlatOutputs() await store.loadMoreFlatOutputs() @@ expect(assetService.getAssetsPageByTag).toHaveBeenLastCalledWith( 'output', true, { limit: FLAT_OUTPUT_PAGE_SIZE, offset: 0 } ) + const ids = store.flatOutputAssets.map((a) => a.id) + expect(new Set(ids).size).toBe(ids.length)As per coding guidelines, "Write tests for all changes, especially bug fixes to catch future regressions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/assetsStore.test.ts` around lines 1734 - 1760, The test validates that the correct API arguments are passed when restarting from the head after a failed refresh, but does not assert the integrity of the resulting asset list. Add an assertion after the loadMoreFlatOutputs call to verify that the store's flatOutputs (or equivalent collection being tested) contains the expected assets and does not have duplicate asset IDs. This will ensure that retrying after a failed refresh properly replaces the stale data instead of duplicating it.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/stores/assetsStore.test.ts`:
- Around line 1734-1760: The test validates that the correct API arguments are
passed when restarting from the head after a failed refresh, but does not assert
the integrity of the resulting asset list. Add an assertion after the
loadMoreFlatOutputs call to verify that the store's flatOutputs (or equivalent
collection being tested) contains the expected assets and does not have
duplicate asset IDs. This will ensure that retrying after a failed refresh
properly replaces the stale data instead of duplicating it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c65eb529-321e-4d25-ba83-fdf46c7cb99a
📒 Files selected for processing (6)
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vuesrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vuesrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.test.tssrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.vuesrc/stores/assetsStore.test.tssrc/stores/assetsStore.ts
dante01yoon
left a comment
There was a problem hiding this comment.
Adversarial re-review of the delta since my 5de4e08 review, now at 96c8b0e (not approving, per the read-only convention).
The delta is the batch.length → fresh.length fix for the infinite-loadMore loop — correct. One non-blocking suggestion:: the exact scenario that distinguishes the two (a loadMore page that is all-duplicate with has_more: true + an advancing cursor) has no regression test, so a revert to batch.length would stay green.
| nextCursor !== undefined && nextCursor === requestedAfter | ||
| flatOutputNextCursor = cursorStuck ? undefined : nextCursor | ||
| flatOutputHasMore.value = | ||
| fresh.length > 0 && page.has_more && !cursorStuck |
There was a problem hiding this comment.
suggestion (non-blocking): this is the exact fix for the infinite-loadMore loop (gating on fresh.length instead of batch.length), but the one scenario that distinguishes the two — a loadMore page whose items are all already-seen (batch.length > 0 yet fresh.length === 0) with has_more: true and an advancing cursor — has no test. The new 'empty page' test only covers batch.length === 0 (so batch.length > 0 and fresh.length > 0 are both false), 'appends and dedupes' returns has_more: false, and 'non-advancing cursor' terminates via cursorStuck, so none would fail if this clause regressed to batch.length. Worth adding: page 1 full + has_more: true, page 2 = entirely already-seen ids with an advancing next_cursor -> assert flatOutputHasMore is false and no further fetch fires. That locks in the behavior this commit introduced.
There was a problem hiding this comment.
Good catch. The missing case is: page 1 full + has_more: true + advancing cursor, page 2 = same ids (all already-seen) + has_more: true + new advancing cursor → assert flatOutputHasMore is false and list length unchanged.
Added the test locally (72/72 pass, hooks green). Branch is in the merge queue so the push is blocked — either dequeue to accept the commit or I can share the snippet for you to apply:
it('marks hasMore=false when all page items are already seen with an advancing cursor', async () => {
const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) =>
makeAsset(`a${i}`, `f${i}.png`)
)
vi.mocked(assetService.getAssetsPageByTag)
.mockResolvedValueOnce(
makePage(firstPage, { hasMore: true, nextCursor: 'cursor-1' })
)
.mockResolvedValueOnce(
makePage(firstPage, { hasMore: true, nextCursor: 'cursor-2' })
)
const store = useAssetsStore()
await store.updateFlatOutputs()
await store.loadMoreFlatOutputs()
expect(store.flatOutputHasMore).toBe(false)
expect(store.flatOutputAssets).toHaveLength(FLAT_OUTPUT_PAGE_SIZE)
})Confirmed this would fail if the guard regressed to batch.length > 0.
dante01yoon
left a comment
There was a problem hiding this comment.
Approving at 96c8b0e. The delta is the batch.length → fresh.length fix for the infinite-loadMore loop — correct. CI green. Only a non-blocking suggestion remains (add a regression test for the all-duplicate page + has_more + advancing-cursor case, which is the one scenario that distinguishes the two). LGTM.
|
Summary
Wires the previously-dormant
loadMorepath of the flat-output assets provider into the widget select dropdown, so cloud users with more than one page of outputs can actually reach them. Stacked on #12774 (FE-985); retargets tomainwhen that merges.Changes
VirtualGrid's existingapproach-endevent now forwards throughFormDropdownMenu→FormDropdown→WidgetSelectDropdown, which callsoutputMediaAssets.loadMore()guarded byhasMore/isLoadingMoreand debounced 300ms — the same idiom asAssetsSidebarTab. A spinner row (loadingMoreprop) renders below the grid while a page is in flight. On cloud this drives the FE-985 cursor walk; on OSS it drives the FE-962 jobs-history cursor walk viauseAssetsApi.Review Focus
Verified end-to-end against the dev server with a mocked 100-asset backend: scroll →
after=cur-40→after=cur-80→ stops athas_more:false(100/100, no duplicate fetches).Known platform limitation, not introduced here: VueUse ≥14's
throttleFilterwithleading=falsedrops events spaced wider than the throttle window, souseScroll'sthrottle: 64inVirtualGridnever reports discrete mouse-wheel scrolls — only high-frequency (trackpad-style) scrolling triggersapproach-end. This equally affects the assets sidebar and manager dialog today; bug filed separately with root cause. Fixing it makes this wiring work for wheel users with no further changes.Underfill edge:
approach-endcannot fire when loaded items don't overflow the viewport (shared VirtualGrid trait with the sidebar); with the 200-item page size this only matters for heavily-filtered media types.Fixes FE-988
🤖 Generated with Claude Code