diff --git a/invokeai/frontend/web/package.json b/invokeai/frontend/web/package.json index 6c7ea3f65ca..2ba037653e5 100644 --- a/invokeai/frontend/web/package.json +++ b/invokeai/frontend/web/package.json @@ -142,6 +142,7 @@ "eslint-plugin-storybook": "^10.3.6", "eslint-plugin-unused-imports": "^4.4.1", "globals": "^16.5.0", + "happy-dom": "^20.11.1", "knip": "^5.77.4", "magic-string": "^0.30.21", "openapi-types": "^12.1.3", diff --git a/invokeai/frontend/web/pnpm-lock.yaml b/invokeai/frontend/web/pnpm-lock.yaml index 4901ad00405..b45e368e497 100644 --- a/invokeai/frontend/web/pnpm-lock.yaml +++ b/invokeai/frontend/web/pnpm-lock.yaml @@ -297,6 +297,9 @@ importers: globals: specifier: ^16.5.0 version: 16.5.0 + happy-dom: + specifier: ^20.11.1 + version: 20.11.1 knip: specifier: ^5.77.4 version: 5.77.4(@types/node@22.19.3)(typescript@5.9.3) @@ -338,7 +341,7 @@ importers: version: 1.8.1(eslint@9.39.2(jiti@2.6.1))(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) vitest: specifier: ^4.1.5 - version: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) + version: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(happy-dom@20.11.1)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) packages: @@ -1969,6 +1972,12 @@ packages: '@types/uuid@10.0.0': resolution: {integrity: sha512-7gqG38EyHgyP1S+7+xomFtL+ZNHcKv6DwNaCZmJmo1vgMugyF3TCnXVg4t1uk89mLNwnLtnY3TpOpCOyp1/xHQ==} + '@types/whatwg-mimetype@3.0.2': + resolution: {integrity: sha512-c2AKvDT8ToxLIOUlN51gTiHXflsfIFisS4pO7pDPoKouJCESkhZnEy623gwP9laCy5lnLDAw1vAzu2vM2YLOrA==} + + '@types/ws@8.18.1': + resolution: {integrity: sha512-ThVF6DCVhA8kUGy+aazFQ4kXQ7E1Ty7A3ypFOe0IcJV8O/M511G99AW24irKrW56Wt44yG9+ij8FaqoBGkuBXg==} + '@typescript-eslint/eslint-plugin@8.59.2': resolution: {integrity: sha512-j/bwmkBvHUtPNxzuWe5z6BEk3q54YRyGlBXkSsmfoih7zNrBvl5A9A98anlp/7JbyZcWIJ8KXo/3Tq/DjFLtuQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} @@ -2331,6 +2340,10 @@ packages: engines: {node: ^6 || ^7 || ^8 || ^9 || ^10 || ^11 || ^12 || >=13.7} hasBin: true + buffer-image-size@0.6.4: + resolution: {integrity: sha512-nEh+kZOPY1w+gcCMobZ6ETUp9WfibndnosbpwB1iJk/8Gt5ZF2bhS6+B6bPYz424KtwsR6Rflc3tCz1/ghX2dQ==} + engines: {node: '>=4.0'} + buffer@5.7.1: resolution: {integrity: sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ==} @@ -2660,6 +2673,10 @@ packages: resolution: {integrity: sha512-HqD3yTBfnBxIrbnM1DoD6Pcq8NECnh8d4As1Qgh0z5Gg3jRRIqijury0CL3ghu/edArpUYiYqQiDUQBIs4np3Q==} engines: {node: '>=10.0.0'} + entities@7.0.1: + resolution: {integrity: sha512-TWrgLOFUQTH994YUyl1yT4uyavY5nNB5muff+RtWaqNVCAK408b5ZnnbNAUEWLTCpum9w6arT70i1XdQ4UeOPA==} + engines: {node: '>=0.12'} + error-ex@1.3.4: resolution: {integrity: sha512-sqQamAnR14VgCr1A618A3sGrygcpK+HEbenA/HiEAkkUwcZIIB/tgWqHFxWgOyDh4nB4JCRimh79dR5Ywc9MDQ==} @@ -3070,6 +3087,10 @@ packages: graceful-fs@4.2.11: resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} + happy-dom@20.11.1: + resolution: {integrity: sha512-XSt8tMzbW9ymE7687xztkO1ckR7qJNQ3LywY9vlYGhGi3zXrGBHuUo2Cl1ztZaICW+1eAGdkLbj6iwVqDT33kg==} + engines: {node: '>=20.0.0'} + has-bigints@1.1.0: resolution: {integrity: sha512-R3pbpkcIqv2Pm3dUwgjclDRVmWpTJW2DcMzcIhEXEx1oh/CEMObMm3KLmRJOdvhM7o4uQBnwr8pzRK2sJWIqfg==} engines: {node: '>= 0.4'} @@ -4854,6 +4875,10 @@ packages: webpack-virtual-modules@0.6.2: resolution: {integrity: sha512-66/V2i5hQanC51vBQKPH4aI8NMAcBW59FVBs+rC7eGHupMyfn34q7rZIE+ETlJ+XTevqfUhVVBgSUNSW2flEUQ==} + whatwg-mimetype@3.0.0: + resolution: {integrity: sha512-nt+N2dzIutVRxARx1nghPKGv1xHikU7HKdfafKkLNLindmPU/ch3U31NOCGGA/dmPcmb1VlofO0vnKAcsm0o/Q==} + engines: {node: '>=12'} + whatwg-url@5.0.0: resolution: {integrity: sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw==} @@ -4919,6 +4944,18 @@ packages: utf-8-validate: optional: true + ws@8.21.1: + resolution: {integrity: sha512-+0NTnW77fFN/DjQi6k/Sq/Yvk4Sgajw7urW8V+asjXnRgDs9gyGkdb7EzgfhA4goXsRIZKE28fzIXBHEzhuiWw==} + engines: {node: '>=10.0.0'} + peerDependencies: + bufferutil: ^4.0.1 + utf-8-validate: '>=5.0.2' + peerDependenciesMeta: + bufferutil: + optional: true + utf-8-validate: + optional: true + wsl-utils@0.1.0: resolution: {integrity: sha512-h3Fbisa2nKGPxCpm89Hk33lBLsnaGBvctQopaBSOW/uIs6FTe1ATyAnKFJrzVs9vpGdsTe73WF3V4lIsk4Gacw==} engines: {node: '>=18'} @@ -6497,7 +6534,7 @@ snapshots: '@testing-library/dom@10.4.0': dependencies: - '@babel/code-frame': 7.29.0 + '@babel/code-frame': 7.29.7 '@babel/runtime': 7.29.2 '@types/aria-query': 5.0.4 aria-query: 5.3.0 @@ -6635,6 +6672,12 @@ snapshots: '@types/uuid@10.0.0': {} + '@types/whatwg-mimetype@3.0.2': {} + + '@types/ws@8.18.1': + dependencies: + '@types/node': 22.19.3 + '@typescript-eslint/eslint-plugin@8.59.2(@typescript-eslint/parser@8.59.2(eslint@9.39.2(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.2(jiti@2.6.1))(typescript@5.9.3)': dependencies: '@eslint-community/regexpp': 4.12.2 @@ -6797,7 +6840,7 @@ snapshots: obug: 2.1.1 std-env: 4.1.0 tinyrainbow: 3.1.0 - vitest: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) + vitest: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(happy-dom@20.11.1)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) '@vitest/expect@3.2.4': dependencies: @@ -6859,7 +6902,7 @@ snapshots: sirv: 3.0.2 tinyglobby: 0.2.16 tinyrainbow: 3.1.0 - vitest: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) + vitest: 4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(happy-dom@20.11.1)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) '@vitest/utils@3.2.4': dependencies: @@ -7110,6 +7153,10 @@ snapshots: node-releases: 2.0.38 update-browserslist-db: 1.2.3(browserslist@4.28.2) + buffer-image-size@0.6.4: + dependencies: + '@types/node': 22.19.3 + buffer@5.7.1: dependencies: base64-js: 1.5.1 @@ -7445,6 +7492,8 @@ snapshots: engine.io-parser@5.2.3: {} + entities@7.0.1: {} + error-ex@1.3.4: dependencies: is-arrayish: 0.2.1 @@ -7992,6 +8041,19 @@ snapshots: graceful-fs@4.2.11: {} + happy-dom@20.11.1: + dependencies: + '@types/node': 22.19.3 + '@types/whatwg-mimetype': 3.0.2 + '@types/ws': 8.18.1 + buffer-image-size: 0.6.4 + entities: 7.0.1 + whatwg-mimetype: 3.0.0 + ws: 8.21.1 + transitivePeerDependencies: + - bufferutil + - utf-8-validate + has-bigints@1.1.0: {} has-flag@4.0.0: {} @@ -9744,7 +9806,7 @@ snapshots: fsevents: 2.3.3 jiti: 2.6.1 - vitest@4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)): + vitest@4.1.5(@types/node@22.19.3)(@vitest/coverage-v8@4.1.5)(@vitest/ui@4.1.5)(happy-dom@20.11.1)(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)): dependencies: '@vitest/expect': 4.1.5 '@vitest/mocker': 4.1.5(vite@8.0.11(@types/node@22.19.3)(esbuild@0.27.7)(jiti@2.6.1)) @@ -9770,6 +9832,7 @@ snapshots: '@types/node': 22.19.3 '@vitest/coverage-v8': 4.1.5(vitest@4.1.5) '@vitest/ui': 4.1.5(vitest@4.1.5) + happy-dom: 20.11.1 transitivePeerDependencies: - msw @@ -9785,6 +9848,8 @@ snapshots: webpack-virtual-modules@0.6.2: {} + whatwg-mimetype@3.0.0: {} + whatwg-url@5.0.0: dependencies: tr46: 0.0.3 @@ -9858,6 +9923,8 @@ snapshots: ws@8.20.0: {} + ws@8.21.1: {} + wsl-utils@0.1.0: dependencies: is-wsl: 3.1.1 diff --git a/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.test.ts b/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.test.ts new file mode 100644 index 00000000000..2674f340664 --- /dev/null +++ b/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest'; + +import { coalesceRanges } from './useBoundedRangeRetry'; + +describe('coalesceRanges', () => { + it('returns empty and single-range inputs as-is', () => { + expect(coalesceRanges([])).toEqual([]); + expect(coalesceRanges([{ startIndex: 3, endIndex: 7 }])).toEqual([{ startIndex: 3, endIndex: 7 }]); + }); + + it('merges overlapping ranges', () => { + expect( + coalesceRanges([ + { startIndex: 0, endIndex: 5 }, + { startIndex: 3, endIndex: 8 }, + ]) + ).toEqual([{ startIndex: 0, endIndex: 8 }]); + }); + + it('merges adjacent ranges', () => { + expect( + coalesceRanges([ + { startIndex: 0, endIndex: 2 }, + { startIndex: 3, endIndex: 5 }, + ]) + ).toEqual([{ startIndex: 0, endIndex: 5 }]); + }); + + it('collapses duplicates — the per-retry-cycle growth case', () => { + // Pre-change, each retry cycle appended the viewport range again, so the pending state grew + // by a duplicate entry per cycle for as long as the failure persisted. + const range = { startIndex: 10, endIndex: 30 }; + expect(coalesceRanges([range, range, range, range])).toEqual([range]); + }); + + it('absorbs contained ranges', () => { + expect( + coalesceRanges([ + { startIndex: 0, endIndex: 10 }, + { startIndex: 2, endIndex: 4 }, + ]) + ).toEqual([{ startIndex: 0, endIndex: 10 }]); + }); + + it('keeps disjoint ranges separate and sorts them', () => { + expect( + coalesceRanges([ + { startIndex: 6, endIndex: 8 }, + { startIndex: 0, endIndex: 2 }, + ]) + ).toEqual([ + { startIndex: 0, endIndex: 2 }, + { startIndex: 6, endIndex: 8 }, + ]); + }); + + it('does not mutate its input', () => { + const input = [ + { startIndex: 0, endIndex: 5 }, + { startIndex: 3, endIndex: 8 }, + ]; + coalesceRanges(input); + expect(input).toEqual([ + { startIndex: 0, endIndex: 5 }, + { startIndex: 3, endIndex: 8 }, + ]); + }); +}); diff --git a/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.ts b/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.ts new file mode 100644 index 00000000000..370439b8af6 --- /dev/null +++ b/invokeai/frontend/web/src/common/hooks/useBoundedRangeRetry.ts @@ -0,0 +1,119 @@ +import { useCallback, useEffect, useRef } from 'react'; +import type { ListRange } from 'react-virtuoso'; + +const RETRY_INITIAL_DELAY_MS = 1_000; +const RETRY_MAX_DELAY_MS = 16_000; +const RETRY_MAX_ATTEMPTS = 5; + +/** + * Merge overlapping or adjacent ranges into a minimal, sorted, disjoint set. + * + * This is what bounds the retry state: failed ranges are accumulated as a coalesced union, so + * repeated failures over the same viewport collapse into one entry instead of growing by a + * duplicate range per retry cycle. + */ +export const coalesceRanges = (ranges: ListRange[]): ListRange[] => { + if (ranges.length <= 1) { + return ranges; + } + const sorted = [...ranges].sort((a, b) => a.startIndex - b.startIndex); + const first = sorted[0]!; + const coalesced: ListRange[] = [{ startIndex: first.startIndex, endIndex: first.endIndex }]; + for (let i = 1; i < sorted.length; i++) { + const range = sorted[i]!; + const last = coalesced[coalesced.length - 1]!; + if (range.startIndex <= last.endIndex + 1) { + last.endIndex = Math.max(last.endIndex, range.endIndex); + } else { + coalesced.push({ startIndex: range.startIndex, endIndex: range.endIndex }); + } + } + return coalesced; +}; + +interface UseBoundedRangeRetryReturn { + /** + * Report a failed bulk fetch, with the ranges it was fetching. Schedules a single retry with + * exponential backoff (1s, 2s, ... capped at 16s); while one is already scheduled, additional + * failures only merge their ranges into it. After RETRY_MAX_ATTEMPTS consecutive failures the + * hook gives up until the budget is reset. + */ + onFetchFailure: (ranges: ListRange[]) => void; + /** + * End the current failure streak. Call when a fetch succeeds (the backend is answering again) + * and on new user input (a fresh range report), so a list that gave up resumes retrying as the + * user scrolls. + */ + resetRetryBudget: () => void; +} + +/** + * Bounded, backoff-driven retry of failed range fetches. + * + * The range-based fetching hooks are the ONLY fetcher for their rows (the row components consume + * the cache with `skip: isUninitialized`), so a failed bulk fetch must be retried or the rows stay + * placeholders until the user happens to scroll. But an unbounded retry is a fixed-rate request + * storm from every open tab against a backend that is trying to come back up. This hook bounds it: + * exponential backoff between attempts, a cap on consecutive failures, and coalesced accumulation + * of the failed ranges. + * + * `restoreRanges` is invoked when a retry fires, with the coalesced union of every range that + * failed since the last retry. It must be referentially stable (wrap it in `useCallback`). + */ +export const useBoundedRangeRetry = ( + restoreRanges: (failedRanges: ListRange[]) => void +): UseBoundedRangeRetryReturn => { + const stateRef = useRef<{ + attempts: number; + failedRanges: ListRange[]; + timeoutId: ReturnType | null; + }>({ attempts: 0, failedRanges: [], timeoutId: null }); + + useEffect(() => { + const state = stateRef.current; + return () => { + if (state.timeoutId !== null) { + clearTimeout(state.timeoutId); + // Null the sentinel too: effect cleanup can run while the instance (and this ref) + // survives — Fast Refresh, or a re-suspending Suspense/Activity boundary. A stale + // non-null timeoutId would make every future onFetchFailure early-return, silently + // disabling retry for the lifetime of the instance. + state.timeoutId = null; + } + }; + }, []); + + const onFetchFailure = useCallback( + (ranges: ListRange[]) => { + const state = stateRef.current; + state.failedRanges = coalesceRanges([...state.failedRanges, ...ranges]); + if (state.timeoutId !== null) { + // A retry is already scheduled; it will pick up the merged ranges when it fires. + return; + } + if (state.attempts >= RETRY_MAX_ATTEMPTS) { + // Budget exhausted — abandon these ranges rather than letting them accumulate. The rows + // still in view are re-reported by the next range change, which also resets the budget. + state.failedRanges = []; + return; + } + state.attempts += 1; + const delay = Math.min(RETRY_INITIAL_DELAY_MS * 2 ** (state.attempts - 1), RETRY_MAX_DELAY_MS); + state.timeoutId = setTimeout(() => { + state.timeoutId = null; + const failedRanges = state.failedRanges; + state.failedRanges = []; + if (failedRanges.length > 0) { + restoreRanges(failedRanges); + } + }, delay); + }, + [restoreRanges] + ); + + const resetRetryBudget = useCallback(() => { + stateRef.current.attempts = 0; + }, []); + + return { onFetchFailure, resetRetryBudget }; +}; diff --git a/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.test.ts b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.test.ts index 6cec16aa043..eeba69648aa 100644 --- a/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.test.ts +++ b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.test.ts @@ -1,6 +1,317 @@ -import { describe, expect, it } from 'vitest'; +// @vitest-environment happy-dom +import { act, createElement, type FC } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { ListRange } from 'react-virtuoso'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { getVideoPrefetchOptions, hasCachedVideoDTO } from './useRangeBasedImageFetching'; +import { getVideoPrefetchOptions, hasCachedVideoDTO, useRangeBasedImageFetching } from './useRangeBasedImageFetching'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const mocks = vi.hoisted(() => ({ + // Args of every getImageDTOsByNames call, in order. + imageFetches: [] as string[][], + // Names with a getImageDTO cache entry, as reported by selectCachedArgsForQuery. + cachedImageNames: [] as string[], + // When true, a successful fetch upserts the requested names into the cache, like + // getImageDTOsByNames.onQueryStarted does. When false, requested names never land in the + // cache — the deleted-image / multiuser-filtered case that drove the pre-fix request stream. + cacheLands: true, + // When true, the mutation rejects, like a backend restart or a 502 from a reverse proxy. + failFetches: false, +})); + +vi.mock('app/store/storeHooks', () => { + const store = { getState: () => ({}), dispatch: () => undefined }; + return { useAppStore: () => store }; +}); + +vi.mock('features/gallery/store/types', () => ({ + isVideoName: (name: string) => name.endsWith('.mp4'), +})); + +vi.mock('services/api/endpoints/images', () => { + const trigger = (arg: { image_names: string[] }) => { + mocks.imageFetches.push(arg.image_names); + // Like the real mutation: onQueryStarted upserts when the request fulfills, whether or not + // the caller unwraps, and only the promise returned by unwrap() surfaces the rejection. + const settled = mocks.failFetches + ? Promise.reject(new Error('fetch failed')) + : Promise.resolve().then(() => { + if (mocks.cacheLands) { + mocks.cachedImageNames.push(...arg.image_names); + } + return []; + }); + settled.catch(() => undefined); + return { unwrap: () => settled.then((r) => r) }; + }; + // RTK Query's mutation trigger is referentially stable across renders; the hook's fetchItems + // callback (and therefore its throttle and effect) depend on that. + const result = [trigger]; + return { + imagesApi: { util: { selectCachedArgsForQuery: () => mocks.cachedImageNames } }, + useGetImageDTOsByNamesMutation: () => result, + }; +}); + +vi.mock('services/api/endpoints/videos', () => ({ + videosApi: { + util: { selectCachedArgsForQuery: () => [] }, + endpoints: { getVideoDTO: { select: () => () => ({ data: undefined }), initiate: () => ({ type: 'noop' }) } }, + }, +})); + +const IMAGE_NAMES = ['a.png', 'b.png', 'c.png']; +const THROTTLE_MS = 500; + +describe('useRangeBasedImageFetching', () => { + let root: Root | null = null; + let renderCount = 0; + let hookReturn: ReturnType; + + const renderHook = (imageNames: string[], enabled: boolean) => { + const Harness: FC = () => { + renderCount++; + hookReturn = useRangeBasedImageFetching({ imageNames, enabled }); + return null; + }; + root = createRoot(document.createElement('div')); + act(() => { + root!.render(createElement(Harness)); + }); + }; + + const scrollTo = (range: ListRange) => { + act(() => { + hookReturn.onRangeChanged(range); + }); + }; + + // Advance fake time in small steps, flushing React work (renders + effects) between steps. A + // single long advance would defer all effect re-runs to the end of the act scope, which breaks + // the feedback cycle this suite exists to detect: state update -> effect -> throttle -> fetch -> + // state update. Stepping mimics real event-loop turns, letting a loop sustain itself if the + // code allows one. + const advance = async (ms: number) => { + const step = 250; + for (let elapsed = 0; elapsed < ms; elapsed += step) { + await act(async () => { + await vi.advanceTimersByTimeAsync(step); + }); + } + }; + + beforeEach(() => { + vi.useFakeTimers(); + mocks.imageFetches = []; + mocks.cachedImageNames = []; + mocks.cacheLands = true; + mocks.failFetches = false; + renderCount = 0; + }); + + afterEach(() => { + if (root) { + act(() => { + root!.unmount(); + }); + root = null; + } + vi.useRealTimers(); + }); + + it('fetches uncached names for a reported range, then goes quiet', async () => { + renderHook(IMAGE_NAMES, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 2); + + expect(mocks.imageFetches).toEqual([IMAGE_NAMES]); + + // Regression: clearing pendingRanges with a fresh `[]` (a new identity every time) re-ran the + // effect, re-armed the throttle, and cleared again — a self-sustaining render loop that + // re-rendered every ~500ms for as long as the grid was mounted, with no user input. Once the + // range has been handled and the throttle has drained, both renders and fetches must stop. + const settledRenders = renderCount; + await advance(THROTTLE_MS * 10); + expect(renderCount).toBe(settledRenders); + expect(mocks.imageFetches).toEqual([IMAGE_NAMES]); + }); + + it('does not loop even while the grid is mounted with nothing to fetch', async () => { + // Pre-fix, the loop ran from mount even with no ranges reported, because the clear was + // unconditional and every pass installed a new [] identity. + renderHook(IMAGE_NAMES, true); + const settledRenders = renderCount; + await advance(THROTTLE_MS * 10); + expect(renderCount).toBe(settledRenders); + expect(mocks.imageFetches).toEqual([]); + }); + + it('stops re-requesting names that never land in the cache', async () => { + // onQueryStarted upserts only the DTOs the server actually returned, so a requested name that + // comes back missing (deleted image, multiuser ownership filter) never gets a cache entry. + // Pre-fix, the render loop re-requested such names every ~500ms, forever. + mocks.cacheLands = false; + renderHook(IMAGE_NAMES, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + + // The range-change pass fetches once, and clearing pendingRanges ([range] -> EMPTY_ARRAY) is a + // real state change, so one follow-up pass may re-check the cache and re-request the + // still-missing names. After that the state is stable and the stream must stop — pre-fix it + // continued at one request per throttle window, forever. + expect(mocks.imageFetches.length).toBeGreaterThanOrEqual(1); + expect(mocks.imageFetches.length).toBeLessThanOrEqual(2); + const settledFetches = mocks.imageFetches.length; + await advance(THROTTLE_MS * 10); + expect(mocks.imageFetches.length).toBe(settledFetches); + }); + + it('retries a failed fetch until it succeeds, then goes quiet', async () => { + // The pre-fix loop was also an accidental retry, and this bulk fetch is the only fetcher for + // these rows (ImageAtPosition subscribes with `skip: isUninitialized`). Without an explicit + // retry, a transient failure would leave grey placeholders until the user happens to scroll. + mocks.failFetches = true; + renderHook(IMAGE_NAMES, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 4); + + // The initial failure produces a fetch at the leading and trailing edges of the throttle + // window, and the first backoff retry (1s) restores the ranges for at least one more pass. + // Without the retry, clearing pendingRanges after the failed fetch still re-runs the effect + // once, so the count caps at two — three or more requires the retry restoring the ranges. + expect(mocks.imageFetches.length).toBeGreaterThanOrEqual(3); + expect(mocks.cachedImageNames).toEqual([]); + + mocks.failFetches = false; + await advance(THROTTLE_MS * 4); + expect(mocks.cachedImageNames).toEqual(IMAGE_NAMES); + + const fetchesAfterRecovery = mocks.imageFetches.length; + const settledRenders = renderCount; + await advance(THROTTLE_MS * 10); + expect(mocks.imageFetches.length).toBe(fetchesAfterRecovery); + expect(renderCount).toBe(settledRenders); + }); + + it('stops retrying when failure is sustained, instead of storming', async () => { + // Review finding on the original retry: restoring the ranges immediately meant a sustained + // backend outage produced a request every throttle window, forever — a fixed-rate storm from + // every open tab against a backend trying to come back up. The bounded retry backs off + // (1s, 2s, 4s, 8s, 16s) and gives up after five consecutive scheduled retries, so the request + // stream must terminate. Each retry pass produces at most a leading and a trailing fetch, + // bounding the total at 12; six requires every backoff retry to have actually fired. + mocks.failFetches = true; + renderHook(IMAGE_NAMES, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(35_000); + + expect(mocks.imageFetches.length).toBeGreaterThanOrEqual(6); + expect(mocks.imageFetches.length).toBeLessThanOrEqual(12); + + const settledFetches = mocks.imageFetches.length; + const settledRenders = renderCount; + await advance(30_000); + expect(mocks.imageFetches.length).toBe(settledFetches); + expect(renderCount).toBe(settledRenders); + }); + + it('resumes retrying after giving up when the user scrolls', async () => { + mocks.failFetches = true; + renderHook(IMAGE_NAMES, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(35_000); + const fetchesAfterGiveUp = mocks.imageFetches.length; + + // A new range report is fresh user input: it restarts the retry budget, so the grid does not + // stay dead until reload. With the budget still exhausted, only the scroll-triggered fetch and + // its trailing companion would fire — three or more new fetches requires the backoff schedule + // to have restarted. + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(2_000); + expect(mocks.imageFetches.length).toBeGreaterThanOrEqual(fetchesAfterGiveUp + 3); + }); + + it('recovers a range that failed while the user was scrolling elsewhere', async () => { + // Review finding on the original retry: the catch (`prev.length > 0 ? prev : ranges`) dropped + // the failed range whenever another range had been reported in the meantime — rows the user + // had scrolled past stayed grey placeholders. The retry now merges the failed ranges with + // whatever is pending instead of choosing one side, so both ranges end up fetched with no + // further user input. + const names = ['a.png', 'b.png', 'c.png', 'd.png', 'e.png', 'f.png', 'g.png', 'h.png', 'i.png']; + mocks.failFetches = true; + renderHook(names, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + // The fetches for the failed range land at t=500 (throttle edges), scheduling the 1s backoff + // retry for t=1500. + await advance(1_250); + + // The backend recovers, and the user scrolls to a disjoint range. The first report fires on + // the throttle's leading edge (t=1250); the second lands in pendingRanges and stays there + // until the trailing edge (t=1750) — so the backoff retry at t=1500 finds a non-empty + // pendingRanges and must merge into it rather than pick a side. + mocks.failFetches = false; + scrollTo({ startIndex: 6, endIndex: 8 }); + scrollTo({ startIndex: 6, endIndex: 8 }); + await advance(3_000); + + // Both the failed range (a-c) and the new one (g-i) land, with no user input beyond the one + // scroll — and nothing outside the reported ranges is fetched. + expect([...mocks.cachedImageNames].sort()).toEqual(['a.png', 'b.png', 'c.png', 'g.png', 'h.png', 'i.png']); + }); + + it('still fetches for new ranges after settling', async () => { + const names = ['a.png', 'b.png', 'c.png', 'd.png', 'e.png', 'f.png']; + renderHook(names, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + expect(mocks.imageFetches).toEqual([['a.png', 'b.png', 'c.png']]); + + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.imageFetches).toEqual([ + ['a.png', 'b.png', 'c.png'], + ['d.png', 'e.png', 'f.png'], + ]); + }); + + it('fetches every range reported within a throttle window, not just the last', async () => { + // onRangeChanged accumulates ranges into pendingRanges precisely so that ranges reported + // mid-window are not dropped when the trailing invocation only sees the latest call's args. + const names = ['a.png', 'b.png', 'c.png', 'd.png', 'e.png', 'f.png']; + renderHook(names, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.imageFetches).toEqual([['a.png', 'b.png', 'c.png', 'd.png', 'e.png', 'f.png']]); + }); + + it('drops handled ranges instead of accumulating them', async () => { + // A handled range must not be re-scanned by later passes. Pre-fix, the queue variant of this + // hook returned early without clearing when everything was cached, so ranges accumulated for + // the lifetime of the list and a later pass would re-request an item evicted from a range + // handled long ago. + const names = ['a.png', 'b.png', 'c.png', 'd.png', 'e.png', 'f.png']; + mocks.cachedImageNames = ['a.png', 'b.png', 'c.png']; + renderHook(names, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + expect(mocks.imageFetches).toEqual([]); + + mocks.cachedImageNames = ['a.png', 'c.png']; + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.imageFetches).toEqual([['d.png', 'e.png', 'f.png']]); + }); + + it('does not fetch when disabled', async () => { + renderHook(IMAGE_NAMES, false); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 4); + expect(mocks.imageFetches).toEqual([]); + }); +}); describe('video range prefetch', () => { it('does not retain an RTK Query subscription', () => { diff --git a/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts index eab38776e5b..91ca4884906 100644 --- a/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts +++ b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts @@ -1,4 +1,6 @@ +import { EMPTY_ARRAY } from 'app/store/constants'; import { useAppStore } from 'app/store/storeHooks'; +import { coalesceRanges, useBoundedRangeRetry } from 'common/hooks/useBoundedRangeRetry'; import { isVideoName } from 'features/gallery/store/types'; import { useCallback, useEffect, useState } from 'react'; import type { ListRange } from 'react-virtuoso'; @@ -51,7 +53,14 @@ export const useRangeBasedImageFetching = ({ const store = useAppStore(); const [getImageDTOsByNames] = useGetImageDTOsByNamesMutation(); const [lastRange, setLastRange] = useState(null); - const [pendingRanges, setPendingRanges] = useState([]); + const [pendingRanges, setPendingRanges] = useState(EMPTY_ARRAY); + + const restoreFailedRanges = useCallback((failedRanges: ListRange[]) => { + // Merge with whatever is pending — replacing either side would drop ranges the user reported + // while the failed fetch was in flight, or ranges that failed while the user was scrolling. + setPendingRanges((prev) => (prev.length > 0 ? coalesceRanges([...prev, ...failedRanges]) : failedRanges)); + }, []); + const { onFetchFailure, resetRetryBudget } = useBoundedRangeRetry(restoreFailedRanges); const fetchItems = useCallback( (ranges: ListRange[], allNames: string[]) => { @@ -64,7 +73,17 @@ export const useRangeBasedImageFetching = ({ const cachedImageNames = imagesApi.util.selectCachedArgsForQuery(state, 'getImageDTO'); const uncachedImageNames = getUncachedNames(allNames, cachedImageNames, ranges).filter((n) => !isVideoName(n)); if (uncachedImageNames.length > 0) { - getImageDTOsByNames({ image_names: uncachedImageNames }); + getImageDTOsByNames({ image_names: uncachedImageNames }) + .unwrap() + .then(resetRetryBudget) + .catch(() => { + // This bulk fetch is the ONLY fetcher for these rows: `ImageAtPosition` consumes the + // cache with `skip: isUninitialized`, so a row whose DTO never arrived does not fetch + // for itself, and images (unlike videos) have no retry affordance. Hand the ranges to + // the bounded retry so they are restored after a backoff — otherwise a transient + // failure leaves grey placeholders until the user happens to scroll. + onFetchFailure(ranges); + }); } // Videos — fetch one at a time (no batch endpoint yet). Each `initiate()` is a no-op for @@ -77,17 +96,29 @@ export const useRangeBasedImageFetching = ({ store.dispatch(videosApi.endpoints.getVideoDTO.initiate(videoName, getVideoPrefetchOptions())); } - setPendingRanges([]); + // Clear with a stable reference. `pendingRanges` is a dependency of the effect that + // calls this function, so a fresh `[]` — a new identity every time — re-runs the + // effect, which re-arms the throttle, which calls this again: a self-sustaining + // render loop, running as fast as the throttle allows, for as long as the grid is + // mounted and with no user input. Setting state to the value it already holds makes + // React bail out instead. + setPendingRanges(EMPTY_ARRAY); }, - [enabled, getImageDTOsByNames, store] + [enabled, getImageDTOsByNames, onFetchFailure, resetRetryBudget, store] ); const throttledFetchItems = useThrottledCallback(fetchItems, 500); - const onRangeChanged = useCallback((range: ListRange) => { - setLastRange(range); - setPendingRanges((prev) => [...prev, range]); - }, []); + const onRangeChanged = useCallback( + (range: ListRange) => { + // A new range report is fresh user input — restart the retry budget so a grid that gave up + // after sustained failure resumes retrying as the user scrolls. + resetRetryBudget(); + setLastRange(range); + setPendingRanges((prev) => [...prev, range]); + }, + [resetRetryBudget] + ); useEffect(() => { const combinedRanges = lastRange ? [...pendingRanges, lastRange] : pendingRanges; diff --git a/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.test.ts b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.test.ts new file mode 100644 index 00000000000..6efc5e77f73 --- /dev/null +++ b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.test.ts @@ -0,0 +1,290 @@ +// @vitest-environment happy-dom +import { act, createElement, type FC } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import type { ListRange } from 'react-virtuoso'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useRangeBasedQueueItemFetching } from './useRangeBasedQueueItemFetching'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const mocks = vi.hoisted(() => ({ + // Args of every getQueueItemDTOsByItemIds call, in order. + queueFetches: [] as number[][], + // Item ids with a getQueueItem cache entry, as reported by selectCachedArgsForQuery. + cachedItemIds: [] as number[], + // When true, a successful fetch upserts the requested ids into the cache, like the mutation's + // onQueryStarted does. When false, requested ids never land in the cache. + cacheLands: true, + // When true, the mutation rejects, like a backend restart or a 502 from a reverse proxy. + failFetches: false, +})); + +vi.mock('app/store/storeHooks', () => { + const store = { getState: () => ({}), dispatch: () => undefined }; + return { useAppStore: () => store }; +}); + +vi.mock('services/api/endpoints/queue', () => { + const trigger = (arg: { item_ids: number[] }) => { + mocks.queueFetches.push(arg.item_ids); + // Like the real mutation: onQueryStarted upserts when the request fulfills, whether or not + // the caller unwraps, and only the promise returned by unwrap() surfaces the rejection. + const settled = mocks.failFetches + ? Promise.reject(new Error('fetch failed')) + : Promise.resolve().then(() => { + if (mocks.cacheLands) { + mocks.cachedItemIds.push(...arg.item_ids); + } + return []; + }); + settled.catch(() => undefined); + return { unwrap: () => settled.then((r) => r) }; + }; + // RTK Query's mutation trigger is referentially stable across renders; the hook's + // fetchQueueItems callback (and therefore its throttle and effect) depend on that. + const result = [trigger]; + return { + queueApi: { util: { selectCachedArgsForQuery: () => mocks.cachedItemIds } }, + useGetQueueItemDTOsByItemIdsMutation: () => result, + }; +}); + +const ITEM_IDS = [1, 2, 3]; +const THROTTLE_MS = 500; + +describe('useRangeBasedQueueItemFetching', () => { + let root: Root | null = null; + let renderCount = 0; + let hookReturn: ReturnType; + + const renderHook = (itemIds: number[], enabled: boolean) => { + const Harness: FC = () => { + renderCount++; + hookReturn = useRangeBasedQueueItemFetching({ itemIds, enabled }); + return null; + }; + root = createRoot(document.createElement('div')); + act(() => { + root!.render(createElement(Harness)); + }); + }; + + const scrollTo = (range: ListRange) => { + act(() => { + hookReturn.onRangeChanged(range); + }); + }; + + // Advance fake time in small steps, flushing React work (renders + effects) between steps. A + // single long advance would defer all effect re-runs to the end of the act scope, which breaks + // the feedback cycle this suite exists to detect: state update -> effect -> throttle -> fetch -> + // state update. Stepping mimics real event-loop turns, letting a loop sustain itself if the + // code allows one. + const advance = async (ms: number) => { + const step = 250; + for (let elapsed = 0; elapsed < ms; elapsed += step) { + await act(async () => { + await vi.advanceTimersByTimeAsync(step); + }); + } + }; + + beforeEach(() => { + vi.useFakeTimers(); + mocks.queueFetches = []; + mocks.cachedItemIds = []; + mocks.cacheLands = true; + mocks.failFetches = false; + renderCount = 0; + }); + + afterEach(() => { + if (root) { + act(() => { + root!.unmount(); + }); + root = null; + } + vi.useRealTimers(); + }); + + it('fetches uncached items for a reported range, then goes quiet', async () => { + renderHook(ITEM_IDS, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 2); + + expect(mocks.queueFetches).toEqual([ITEM_IDS]); + + // Regression: clearing pendingRanges with a fresh `[]` (a new identity every time) re-ran the + // effect, re-armed the throttle, and cleared again — a self-sustaining render loop. Once the + // range has been handled and the throttle has drained, both renders and fetches must stop. + const settledRenders = renderCount; + await advance(THROTTLE_MS * 10); + expect(renderCount).toBe(settledRenders); + expect(mocks.queueFetches).toEqual([ITEM_IDS]); + }); + + it('stops re-requesting items that never land in the cache', async () => { + // A requested id the server does not return never gets a getQueueItem cache entry, so it is + // uncached on every pass. Pre-fix, that sustained the loop: the list re-requested such ids + // every ~500ms for as long as it was mounted. + mocks.cacheLands = false; + renderHook(ITEM_IDS, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + + // The range-change pass fetches once, and clearing pendingRanges ([range] -> EMPTY_ARRAY) is a + // real state change, so one follow-up pass may re-check the cache and re-request the + // still-missing ids. After that the state is stable and the stream must stop — pre-fix it + // continued at one request per throttle window, forever. + expect(mocks.queueFetches.length).toBeGreaterThanOrEqual(1); + expect(mocks.queueFetches.length).toBeLessThanOrEqual(2); + const settledFetches = mocks.queueFetches.length; + await advance(THROTTLE_MS * 10); + expect(mocks.queueFetches.length).toBe(settledFetches); + }); + + it('retries a failed fetch until it succeeds, then goes quiet', async () => { + // This bulk fetch is the only fetcher for these rows (QueueItemAtPosition subscribes with + // `skip: isUninitialized`), so a transient failure must be retried or the placeholders stay + // empty until the user happens to scroll. + mocks.failFetches = true; + renderHook(ITEM_IDS, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 4); + + // The initial failure produces a fetch at the leading and trailing edges of the throttle + // window, and the first backoff retry (1s) restores the ranges for at least one more pass. + // Without the retry, clearing pendingRanges after the failed fetch still re-runs the effect + // once, so the count caps at two — three or more requires the retry restoring the ranges. + expect(mocks.queueFetches.length).toBeGreaterThanOrEqual(3); + expect(mocks.cachedItemIds).toEqual([]); + + mocks.failFetches = false; + await advance(THROTTLE_MS * 4); + expect(mocks.cachedItemIds).toEqual(ITEM_IDS); + + const fetchesAfterRecovery = mocks.queueFetches.length; + const settledRenders = renderCount; + await advance(THROTTLE_MS * 10); + expect(mocks.queueFetches.length).toBe(fetchesAfterRecovery); + expect(renderCount).toBe(settledRenders); + }); + + it('stops retrying when failure is sustained, instead of storming', async () => { + // Review finding on the original retry: restoring the ranges immediately meant a sustained + // backend outage produced a request every throttle window, forever — a fixed-rate storm from + // every open tab against a backend trying to come back up. The bounded retry backs off + // (1s, 2s, 4s, 8s, 16s) and gives up after five consecutive scheduled retries, so the request + // stream must terminate. Each retry pass produces at most a leading and a trailing fetch, + // bounding the total at 12; six requires every backoff retry to have actually fired. + mocks.failFetches = true; + renderHook(ITEM_IDS, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(35_000); + + expect(mocks.queueFetches.length).toBeGreaterThanOrEqual(6); + expect(mocks.queueFetches.length).toBeLessThanOrEqual(12); + + const settledFetches = mocks.queueFetches.length; + const settledRenders = renderCount; + await advance(30_000); + expect(mocks.queueFetches.length).toBe(settledFetches); + expect(renderCount).toBe(settledRenders); + }); + + it('resumes retrying after giving up when the user scrolls', async () => { + mocks.failFetches = true; + renderHook(ITEM_IDS, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(35_000); + const fetchesAfterGiveUp = mocks.queueFetches.length; + + // A new range report is fresh user input: it restarts the retry budget, so the list does not + // stay dead until reload. With the budget still exhausted, only the scroll-triggered fetch and + // its trailing companion would fire — three or more new fetches requires the backoff schedule + // to have restarted. + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(2_000); + expect(mocks.queueFetches.length).toBeGreaterThanOrEqual(fetchesAfterGiveUp + 3); + }); + + it('recovers a range that failed while the user was scrolling elsewhere', async () => { + // Review finding on the original retry: the catch (`prev.length > 0 ? prev : ranges`) dropped + // the failed range whenever another range had been reported in the meantime — rows the user + // had scrolled past stayed blank placeholders. The retry now merges the failed ranges with + // whatever is pending instead of choosing one side, so both ranges end up fetched with no + // further user input. + const itemIds = [1, 2, 3, 4, 5, 6, 7, 8, 9]; + mocks.failFetches = true; + renderHook(itemIds, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + // The fetches for the failed range land at t=500 (throttle edges), scheduling the 1s backoff + // retry for t=1500. + await advance(1_250); + + // The backend recovers, and the user scrolls to a disjoint range. The first report fires on + // the throttle's leading edge (t=1250); the second lands in pendingRanges and stays there + // until the trailing edge (t=1750) — so the backoff retry at t=1500 finds a non-empty + // pendingRanges and must merge into it rather than pick a side. + mocks.failFetches = false; + scrollTo({ startIndex: 6, endIndex: 8 }); + scrollTo({ startIndex: 6, endIndex: 8 }); + await advance(3_000); + + // Both the failed range (1-3) and the new one (7-9) land, with no user input beyond the one + // scroll — and nothing outside the reported ranges is fetched. + expect([...mocks.cachedItemIds].sort((a, b) => a - b)).toEqual([1, 2, 3, 7, 8, 9]); + }); + + it('still fetches for new ranges after settling', async () => { + const itemIds = [1, 2, 3, 4, 5, 6]; + renderHook(itemIds, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + expect(mocks.queueFetches).toEqual([[1, 2, 3]]); + + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.queueFetches).toEqual([ + [1, 2, 3], + [4, 5, 6], + ]); + }); + + it('fetches every range reported within a throttle window, not just the last', async () => { + // onRangeChanged accumulates ranges into pendingRanges precisely so that ranges reported + // mid-window are not dropped when the trailing invocation only sees the latest call's args. + const itemIds = [1, 2, 3, 4, 5, 6]; + renderHook(itemIds, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.queueFetches).toEqual([[1, 2, 3, 4, 5, 6]]); + }); + + it('drops handled ranges instead of accumulating them', async () => { + // A handled range must not be re-scanned by later passes. Pre-fix, this hook returned early + // without clearing when everything was cached, so ranges accumulated for the lifetime of the + // list and a later pass would re-request an item evicted from a range handled long ago. + const itemIds = [1, 2, 3, 4, 5, 6]; + mocks.cachedItemIds = [1, 2, 3]; + renderHook(itemIds, true); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 10); + expect(mocks.queueFetches).toEqual([]); + + mocks.cachedItemIds = [1, 3]; + scrollTo({ startIndex: 3, endIndex: 5 }); + await advance(THROTTLE_MS * 2); + expect(mocks.queueFetches).toEqual([[4, 5, 6]]); + }); + + it('does not fetch when disabled', async () => { + renderHook(ITEM_IDS, false); + scrollTo({ startIndex: 0, endIndex: 2 }); + await advance(THROTTLE_MS * 4); + expect(mocks.queueFetches).toEqual([]); + }); +}); diff --git a/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts index b2d4c4ac813..58cccd696e0 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts @@ -1,4 +1,6 @@ +import { EMPTY_ARRAY } from 'app/store/constants'; import { useAppStore } from 'app/store/storeHooks'; +import { coalesceRanges, useBoundedRangeRetry } from 'common/hooks/useBoundedRangeRetry'; import { useCallback, useEffect, useState } from 'react'; import type { ListRange } from 'react-virtuoso'; import { queueApi, useGetQueueItemDTOsByItemIdsMutation } from 'services/api/endpoints/queue'; @@ -41,7 +43,14 @@ export const useRangeBasedQueueItemFetching = ({ const store = useAppStore(); const [getQueueItemDTOsByItemIds] = useGetQueueItemDTOsByItemIdsMutation(); const [lastRange, setLastRange] = useState(null); - const [pendingRanges, setPendingRanges] = useState([]); + const [pendingRanges, setPendingRanges] = useState(EMPTY_ARRAY); + + const restoreFailedRanges = useCallback((failedRanges: ListRange[]) => { + // Merge with whatever is pending — replacing either side would drop ranges the user reported + // while the failed fetch was in flight, or ranges that failed while the user was scrolling. + setPendingRanges((prev) => (prev.length > 0 ? coalesceRanges([...prev, ...failedRanges]) : failedRanges)); + }, []); + const { onFetchFailure, resetRetryBudget } = useBoundedRangeRetry(restoreFailedRanges); const fetchQueueItems = useCallback( (ranges: ListRange[], itemIds: number[]) => { @@ -50,21 +59,45 @@ export const useRangeBasedQueueItemFetching = ({ } const cachedItemIds = queueApi.util.selectCachedArgsForQuery(store.getState(), 'getQueueItem'); const uncachedItemIds = getUncachedItemIds(itemIds, cachedItemIds, ranges); - if (uncachedItemIds.length === 0) { - return; + if (uncachedItemIds.length > 0) { + getQueueItemDTOsByItemIds({ item_ids: uncachedItemIds }) + .unwrap() + .then(resetRetryBudget) + .catch(() => { + // This bulk fetch is the ONLY fetcher for these rows: `QueueItemAtPosition` consumes + // the cache with `skip: isUninitialized`, so a row whose DTO never arrived does not + // fetch for itself. Hand the ranges to the bounded retry so they are restored after a + // backoff — otherwise a transient failure leaves placeholders until the user happens + // to scroll. + onFetchFailure(ranges); + }); } - getQueueItemDTOsByItemIds({ item_ids: uncachedItemIds }); - setPendingRanges([]); + // Clear unconditionally. Returning early without clearing (the previous behaviour when + // everything was already cached) let ranges accumulate for the lifetime of the list, + // growing the scan on every subsequent pass. + // + // Clear with a stable reference. `pendingRanges` is a dependency of the effect that calls + // this function, so a fresh `[]` — a new identity every time — re-runs the effect, which + // re-arms the throttle, which calls this again. The old early return happened to prevent + // that while everything was cached, so the loop only ran while items were genuinely + // uncached; clearing on both paths means the stable reference is now what stops it. + setPendingRanges(EMPTY_ARRAY); }, - [enabled, getQueueItemDTOsByItemIds, store] + [enabled, getQueueItemDTOsByItemIds, onFetchFailure, resetRetryBudget, store] ); const throttledFetchQueueItems = useThrottledCallback(fetchQueueItems, 500); - const onRangeChanged = useCallback((range: ListRange) => { - setLastRange(range); - setPendingRanges((prev) => [...prev, range]); - }, []); + const onRangeChanged = useCallback( + (range: ListRange) => { + // A new range report is fresh user input — restart the retry budget so a list that gave up + // after sustained failure resumes retrying as the user scrolls. + resetRetryBudget(); + setLastRange(range); + setPendingRanges((prev) => [...prev, range]); + }, + [resetRetryBudget] + ); useEffect(() => { const combinedRanges = lastRange ? [...pendingRanges, lastRange] : pendingRanges;