fix(ui): stop range-based fetching hooks from spinning in a render loop

[lstein]

Summary

Fix (performance). useRangeBasedImageFetching and useRangeBasedQueueItemFetching spin in a self-sustaining render loop for as long as the gallery grid / queue list is mounted.

The shape of the bug, in useRangeBasedImageFetching:

const fetchItems = useCallback((ranges, allNames) => {
  ...
  setPendingRanges([]);           // ← new array identity, every call
}, [enabled, getImageDTOsByNames, store]);

const throttledFetchItems = useThrottledCallback(fetchItems, 500);

useEffect(() => {
  throttledFetchItems([...pendingRanges, lastRange], imageNames);
}, [imageNames, lastRange, pendingRanges, throttledFetchItems]);
//                         ^^^^^^^^^^^^^ dependency

setPendingRanges([]) installs a fresh array, which is never Object.is-equal to the previous one, so the effect re-runs. useThrottledCallback resolves to {maxWait: 500, leading: true, trailing: true}, so the re-entry schedules a trailing invocation, which calls fetchItems, which clears again. Round and round, several times a second, indefinitely, with no user input. In the gallery hook the clear is unconditional, so the loop runs from mount even when there is nothing to fetch; the queue hook returned early before clearing when everything was cached, so there it only ran while items were genuinely uncached.

Most of the time this only burns CPU and re-renders. It turns into a permanent 2Hz request stream whenever a name in the visible range never lands in the cache — because getImageDTOsByNames.onQueryStarted upserts only the DTOs the server actually returned:

for (const imageDTO of imageDTOs) {
  updates.push({ endpointName: 'getImageDTO', arg: imageDTO.image_name, value: imageDTO });
}

A requested name that comes back missing (a deleted image still present in the name list, an item filtered out by ownership in multiuser mode) therefore never gets a getImageDTO cache entry, selectCachedArgsForQuery never reports it, and it is re-requested on every pass — twice a second, forever.

The change: clear with the shared stable EMPTY_ARRAY reference from app/store/constants.ts, which is already used across the app for exactly this purpose. Setting state to the value it already holds makes React bail out instead of re-running the effect. Real range changes still flow through onRangeChanged, which sets lastRange to a new object, so fetching on scroll is unaffected.

The queue variant also returned early without clearing when nothing was uncached, letting ranges accumulate for the lifetime of the list and growing the scan on every subsequent pass. It now clears on both paths — the ranges have been handled either way.

The loop was also an accidental retry, so the retry is now explicit. These bulk fetches are the only fetcher for their rows. ImageAtPosition and QueueItemAtPosition both consume the cache with the documented "subscribe once it has data" hack:

imagesApi.endpoints.getImageDTO.useQuerySubscription(isVideo ? '' : imageName, {
  skip: isVideo || imageState.isUninitialized,
});

so a row whose DTO never arrived does not fetch for itself, onQueryStarted swallows the failure in catch {}, and nobody reads the mutation's error state. Videos have a retry button; images and queue items do not. Pre-fix, a failed bulk fetch was simply re-attempted by the loop until it succeeded. Removing the loop without replacing that would mean a transient failure — a backend restart, a 502 from a reverse proxy — leaves grey placeholders until the user happens to scroll, since nothing else changes any dependency of the effect (RTK Query's structuralSharing preserves the imageNames reference even across a refetch). So the failure path now hands the ranges to a shared bounded retry (common/hooks/useBoundedRangeRetry.ts): exponential backoff between attempts (1s, 2s, 4s, 8s, capped at 16s), giving up after 5 consecutive scheduled retries, with the failed ranges accumulated as a coalesced (sorted, disjoint) union that is merged into whatever is pending rather than replacing it. Any new range report resets the retry budget, so scrolling revives a list that gave up. (The first iteration restored the ranges immediately, bounded only by the throttle; Pfannkuchensack's review correctly flagged that as a fixed-rate request storm under sustained failure, with unbounded state growth and a window where a failed range could be dropped mid-scroll — the bounded design resolves all three.)

Related Issues / Discussions

Found while investigating an unrelated report of repeated socket connections; see #9438 for that one. No existing issue.

QA Instructions

The loop is easiest to see with React DevTools:

1. Open the gallery with a board that has enough images to virtualize.
2. In React DevTools → Components → Settings, enable "Highlight updates when components render".
3. Before this change: the gallery re-renders continuously, twice a second, with the mouse untouched. After: it goes quiet once scrolling stops.

For the network half, you need a name that the server will not return — e.g. delete an image directly from the DB (or via another client) so it stays in the cached name list, then scroll it into view. Before: POST /api/v1/images/images_by_names repeats every ~500ms indefinitely in the Network tab. After: it fires once per range change.

Regression checks:

• Scroll the gallery fast through un-fetched regions; thumbnails should still resolve, with no gaps.
• Same for the queue list with many pending items.
• Both lists should still fetch correctly after switching boards / filters (which changes imageNames).
• Self-healing after a failed fetch: stop the backend while the gallery is open, scroll to an un-fetched region so the bulk fetch fails, then restart the backend. The placeholders should fill in on their own, without scrolling. This is the behaviour the loop was providing accidentally.

Notes for reviewers

Two things worth knowing, both found by adversarially reviewing this diff:

• The retry-on-failure is not gold-plating; it is preserving existing behaviour. Without it this change introduces a real regression (permanent placeholders after any transient bulk-fetch failure), because the loop it removes was the only retry these rows had.
• Out of scope, but adjacent: EMPTY_ARRAY in app/store/constants.ts is never[] — mutable, unfrozen, and now referenced from ~30 files and held as component state by these hooks. Nothing mutates it today (audited), but a future pendingRanges.push(...) would compile fine and silently corrupt unrelated selectors app-wide. Object.freeze([]) there would close that off without any type churn. Happy to do it separately if wanted.

Automated: regression tests added for both hooks (useRangeBasedImageFetching.test.ts, useRangeBasedQueueItemFetching.test.ts). They render the real hooks with React act + fake timers in a happy-dom environment (scoped per-file via a @vitest-environment docblock — happy-dom is the only new dev dependency; the rest of the suite stays in the node environment), mocking only the thin API-endpoint modules so the actual state/effect/throttle cycle is exercised. Covered per hook: a reported range fetches its uncached items once and then renders and fetches go quiet; never-cached items (the deleted-image / multiuser-filter case) are re-requested boundedly rather than forever; a failed bulk fetch retries until it succeeds, then goes quiet; every range reported within a throttle window is fetched, not just the last; handled ranges are dropped rather than accumulated (an item evicted from a long-handled range is not re-requested — the queue hook's pre-fix early return regressed exactly this); new ranges after settling still fetch; enabled: false fetches nothing. Also covered: sustained failure terminates (fetch count bounded, then frozen over a further 30 simulated seconds); scrolling revives a given-up list; a range that fails while the user is scrolling elsewhere is recovered; and unit tests for coalesceRanges, including the duplicate-per-cycle growth case. Mutation-verified: reverting the EMPTY_ARRAY clears, restoring the queue hook's early return, dropping onRangeChanged's accumulation, removing the retry, removing the backoff/cap, removing the budget reset, or replacing the merge-on-restore with either side each makes at least one test fail; all 30 pass with the fix in place.

pnpm test:no-watch — full suite passes (1817 tests). pnpm lint:eslint, pnpm lint:prettier, pnpm lint:tsc, pnpm lint:knip all clean.

Merge Plan

Ordinary merge. No redux slice changes, so no migration.

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Tests added / updated (if applicable)
• ❗Changes to a redux slice have a corresponding migration — n/a
• Documentation added / updated (if applicable) — n/a
• Updated What's New copy (if doing a release after this PR) — n/a

[Pfannkuchensack]

PR #9439 — Stop range-based fetching hooks from spinning in a render loop

• The new retry is itself an unbounded request storm. invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts:76 (and queue :62) closes the loop: catch reinstalls ranges → pendingRanges is a dependency of the effect (:111) → effect re-runs → throttle → fetch fails → catch. Measured: 120 requests in 30 simulated seconds, exactly 4/s, linear, still climbing — the same order as the 3.79/s measured live for the pre-fix loop. There is no attempt counter, no backoff, and no retry()/maxRetries anywhere in the API layer (verified). Trigger is the scenario the PR's own comment names: backend restart, OOM kill, 502 from a reverse proxy. Not a regression against main, but the pathology is preserved on the failure path and now locked in by a test. (401 is not a vector — it self-terminates via sessionExpiredLogout.)
• The retry state grows without bound while the failure persists. Each cycle appends lastRange again and nothing truncates: measured +8 range entries per 500 ms cycle, monotonic — linear scan cost, quadratic total work. This is precisely the pathology the PR's own queue-hook comment cites as the reason to clear unconditionally.
• prev.length > 0 ? prev : ranges drops failed ranges during a scroll. Reproduced: range A dispatches; 100 ms later onRangeChanged pushes B into pendingRanges; A then rejects; the catch sees prev.length > 0, keeps B, and A is lost. Those rows stay grey placeholders — unlike videos, images have no error tile and no retry button (invokeai/frontend/web/src/features/gallery/components/GalleryImageGrid.tsx:81-95). Virtuoso fires onRangeChanged many times per second, so the window is wide open exactly while the user scrolls. The retry test scrolls once and then leaves the hook idle.
• Consider removing the retry entirely by giving images the error tile + manual retry button videos already have. That would resolve all three items above. The PR went the other way without stating why.
• The tests mock away the assumption the fix rests on. Both suites hardcode a referentially stable mutation trigger. If a future @reduxjs/toolkit returned a fresh trigger identity per render, the render loop would come straight back and all 17 tests would still pass. Assert trigger stability across a forced re-render instead.
• Missing negative-path coverage: no test asserts that sustained failure terminates, that the retry state stops growing, or that a range failing during a concurrent scroll is ever recovered.
• Open question: does the gallery panel stay mounted when its dockview panel is hidden or the user switches tabs? If so, the storm continues off-screen and invisible.

[Pfannkuchensack]

Looks good. we need to check more for if a version bump is needed. Wrong pr

Wrong pr

[lstein]

Thanks for this review — the three findings on the retry path are all real, and they're now addressed in e464cf3. On the headline suggestion (remove the retry, add a retry button): I went the other way deliberately, and here's the reasoning.

Why not a retry button (yet): the video error tile works because each video row owns a real getVideoDTO query whose state transitions to isError. Image and queue rows deliberately have no per-row query on failure — the whole skip: isUninitialized design means a row whose DTO never arrived via the bulk mutation is uninitialized, indistinguishable from "not fetched yet". There is no error signal to render a tile from, and the batch mutation fails as a unit, so it can't say which names failed. Building that plumbing (or letting rows fall back to self-fetching on error, which reintroduces the request-per-row pattern the bulk hook exists to avoid) is a real redesign, not a drop-in. It also only covers half the surface: the queue hook has the identical retry, and queue rows are text rows with no tile to hang a button on. And since these failures are overwhelmingly transient (backend restart, brief 502), button-only would turn every blip into dozens of dead tiles needing individual clicks. An error tile after retries exhaust would be a fine complement — happy to see that as a follow-up — but it shouldn't replace automatic recovery for the transient case.

What the findings actually warranted, and what changed — the retry is now bounded via a shared useBoundedRangeRetry hook:

• Storm → exponential backoff (1s, 2s, 4s, 8s, capped at 16s), giving up after 5 consecutive scheduled retries. A sustained outage now produces ~6–12 requests total and then goes quiet, instead of 2/s per hook forever. (One clarification on the measurement: the pre-fix(ui): stop range-based fetching hooks from spinning in a render loop #9439 loop ran unconditionally, even on success; the retry only ran while requests were actively failing. But "fixed-rate, no backoff, forever, from every open tab" was still wrong, agreed.)
• Unbounded state growth → failed ranges accumulate as a coalesced union (sorted, disjoint, deduplicated), so repeated failures over the same viewport collapse to one entry instead of growing per cycle.
• Dropped ranges during scroll → the restore now merges the failed ranges into whatever is pending (coalesceRanges([...prev, ...failed])) instead of prev.length > 0 ? prev : ranges. Worth noting the practical severity was lower than it looks: lastRange is appended to every fetch pass and virtuoso re-reports ranges on scroll-back, so permanently-grey rows required the narrow overscan case — but the either/or was still wrong and is gone.
• Given-up ≠ dead: any new onRangeChanged resets the retry budget, so scrolling revives a list that gave up.

Negative-path coverage (your bullet 6) is now in both hook suites: sustained failure terminates (fetch count bounded, then frozen over a further 30 simulated seconds); scrolling revives a given-up list; and a range that fails while the user is scrolling elsewhere is recovered — that last one is timed so the backoff retry fires while another scroll report is mid-throttle-window, which is exactly the interleaving that kills the either/or restore. All of these are mutation-verified: removing the backoff/cap, the budget reset, the merge-on-restore, or the retry itself each fails at least one test. coalesceRanges has its own unit tests including the duplicate-per-cycle growth case.

On the mock stability point (bullet 5): the trigger's referential stability is RTK Query's documented contract (the trigger is useCallback'd on [dispatch, initiate, fixedCacheKey]), and the mock mirrors that contract rather than assuming it away. A test asserting the real trigger's identity across renders would be testing the library — and if a future RTK Query broke it, every useCallback depending on a trigger across the app would churn, not just these hooks. I'd rather catch that in a toolkit upgrade PR than pin it here.

Open question (bullet 7): enabled is !isLoading at both call sites — it tracks query state, not panel visibility — so yes, a mounted-but-hidden panel kept retrying pre-change. With the bound it self-terminates within ~31s regardless of visibility, and nothing re-arms it until the user actually scrolls again.

[Pfannkuchensack]

Findings

1. Medium - the bounded retry gives up permanently after ~31s and nothing re-arms it

invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.ts:94-99

A backend outage longer than the retry budget leaves grey tiles with no automatic recovery and no user affordance.

Chain:

1. Delays are 1/2/4/8/16s (useBoundedRangeRetry.ts:4-6,101), so give-up lands ~31s after the first failed fetch.
2. On give-up the hook drops state.failedRanges and returns (useBoundedRangeRetry.ts:94-99). The only re-arms are resetRetryBudget from onRangeChanged (useRangeBasedImageFetching.ts:112-121) or a successful fetch (:78).
3. A fetch only fires when an effect dependency changes: imageNames, lastRange, pendingRanges, throttledFetchItems (useRangeBasedImageFetching.ts:123-126).
4. On backend recovery in a production build, app/store/middleware/listenerMiddleware/listeners/socketConnected.ts:41-43 resets the API state only in development; production reaches :68-70 and returns early unless the queue status changed.
5. Even when FetchOnReconnect is invalidated (:74), RTK Query structural sharing preserves currentData.items, so the useMemo in features/gallery/components/use-gallery-image-names.ts:72-75 returns the same imageNames reference. No dependency changes.
6. enabled is !isLoading (GalleryImageGrid.tsx:402-405); a refetch sets isFetching, not isLoading, so enabled does not toggle either.
7. The row has no fallback: GalleryImageGrid.tsx:103-106 renders a bare placeholder for images, with no error state and no retry button - unlike the video branch at :81-95.

Verified with a probe test against the PR head: after 35s of failures the hook has given up; the backend then recovers and the component re-renders with identical props; 120 further simulated seconds produce zero fetches and zero cached DTOs.

This is a behaviour regression against main, where the render loop retried at ~2Hz forever and therefore always recovered. It also means the PR's own QA step - "stop the backend ... then restart the backend. The placeholders should fill in on their own, without scrolling" - only holds for restarts under ~31s, which an InvokeAI backend restart (config load, DB migrations, model scan) frequently exceeds.

Blast radius is narrowed by the fact that an actively-generating user heals incidentally (a new image changes imageNames, and lastRange is appended to every pass); an idle user watching the gallery during a restart does not. Dev builds heal via resetApiState().

socketConnected / $isConnected is an existing signal that would close this in one line.

To expose this issue, add a test that exhausts the retry budget under sustained failure, then makes the fetch succeed again without reporting a new range, and asserts the rows are eventually fetched. The existing resumes retrying after giving up when the user scrolls (useRangeBasedImageFetching.test.ts:220-234) bakes the user input in as a precondition, so it passes either way.

2. Low - a backoff timer is armed on an unmounted instance and nothing clears it

invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.ts:72-84

The cleanup effect clears and nulls timeoutId at unmount. An in-flight mutation that rejects after that reaches .catch() at useRangeBasedImageFetching.ts:79-86, calls onFetchFailure, finds timeoutId === null and attempts < 5, and schedules a fresh setTimeout of up to 16s (useBoundedRangeRetry.ts:102-109) that no cleanup will ever reach.

Measured directly: vi.getTimerCount() goes 0 -> 1 across the post-unmount rejection. Consequence is a leaked timer holding the closure plus a no-op setPendingRanges on an unmounted root (silent in React 18/19). Triggered by closing the gallery or switching tabs while the backend is down. The comment at :77-81 shows cleanup-without-unmount was considered; failure-after-unmount was not.

To expose this issue, add a test that unmounts the hook while a bulk fetch is in flight, then rejects it, and asserts no timer remains scheduled.

3. Low - the !enabled path still returns early without clearing

invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts:57-59 and invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts:67-69

This is the exact defect the PR names and fixes on the cached path. The comment the PR adds at useRangeBasedQueueItemFetching.ts:75-78 states that returning early without clearing "let ranges accumulate for the lifetime of the list, growing the scan on every subsequent pass." The !enabled guard sits above the clear at :84 and does precisely that.

Practically bounded by how long isLoading stays true and flushed when enabled flips, so severity is low - but it is the same pattern, left in both hooks. The does not fetch when disabled tests (useRangeBasedImageFetching.test.ts:308-313, useRangeBasedQueueItemFetching.test.ts:284-289) only assert zero fetches, never that state stays bounded.

To expose this issue, add a test that reports many ranges while enabled: false, then enables, and asserts the resulting fetch covers only the current viewport rather than every range reported while disabled.

4. Low - coverage gap: the queue suite is missing the mount-time no-loop test

invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.test.ts

useRangeBasedImageFetching.test.ts:141-149 asserts the grid does not loop when mounted with nothing to fetch. The queue hook previously could not loop on that path because it returned early before clearing; this PR makes its clear unconditional (useRangeBasedQueueItemFetching.ts:84), so the queue hook now depends on the EMPTY_ARRAY identity for exactly that case - newly introduced, and untested there.

To expose this issue, add a test that mounts the queue hook with nothing to fetch and asserts render count and fetch count stay frozen over several throttle windows.

Open Questions

• useBoundedRangeRetry documents that restoreRanges must be referentially stable (useBoundedRangeRetry.ts:61) but nothing enforces it, and an unstable callback would churn onFetchFailure -> fetchItems -> throttledFetchItems -> the effect on every render. I could not construct a case where that recreates the 2Hz loop today - the EMPTY_ARRAY bail-out still breaks the self-sustaining cycle - so this is a contract risk on a newly shared hook, not a proven defect.
• Is ~31s the intended ceiling? Raising RETRY_MAX_ATTEMPTS is a one-line partial mitigation for Finding 1, but it does not remove the unbounded-outage case; a reconnect-driven re-arm does.