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/features/gallery/hooks/useRangeBasedImageFetching.test.ts b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.test.ts index 6cec16aa043..8194c5c965c 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,250 @@ -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 catch-driven retry produces a fetch per throttle window. Without it, clearing + // pendingRanges after the failed fetch still re-runs the effect once, so the count caps at + // two — three or more requires the catch handler 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('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..6264189747f 100644 --- a/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts +++ b/invokeai/frontend/web/src/features/gallery/hooks/useRangeBasedImageFetching.ts @@ -1,3 +1,4 @@ +import { EMPTY_ARRAY } from 'app/store/constants'; import { useAppStore } from 'app/store/storeHooks'; import { isVideoName } from 'features/gallery/store/types'; import { useCallback, useEffect, useState } from 'react'; @@ -51,7 +52,7 @@ 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 fetchItems = useCallback( (ranges: ListRange[], allNames: string[]) => { @@ -64,7 +65,16 @@ 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() + .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. Put the ranges back + // so the effect re-runs and tries again — otherwise a transient failure leaves grey + // placeholders until the user happens to scroll. The throttle bounds the retry rate. + setPendingRanges((prev) => (prev.length > 0 ? prev : ranges)); + }); } // Videos — fetch one at a time (no batch endpoint yet). Each `initiate()` is a no-op for @@ -77,7 +87,13 @@ 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] ); 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..fb40bd55d35 --- /dev/null +++ b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.test.ts @@ -0,0 +1,223 @@ +// @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 catch-driven retry produces a fetch per throttle window. Without it, clearing + // pendingRanges after the failed fetch still re-runs the effect once, so the count caps at + // two — three or more requires the catch handler 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('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..33697875542 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/useRangeBasedQueueItemFetching.ts @@ -1,3 +1,4 @@ +import { EMPTY_ARRAY } from 'app/store/constants'; import { useAppStore } from 'app/store/storeHooks'; import { useCallback, useEffect, useState } from 'react'; import type { ListRange } from 'react-virtuoso'; @@ -41,7 +42,7 @@ 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 fetchQueueItems = useCallback( (ranges: ListRange[], itemIds: number[]) => { @@ -50,11 +51,27 @@ 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() + .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. Put the ranges back so the effect re-runs and tries again — + // otherwise a transient failure leaves placeholders until the user happens to scroll. + setPendingRanges((prev) => (prev.length > 0 ? prev : 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] );