refactor(ui): item-owned reveal state machine with explicit selection identity

invokeai/frontend/web/src/app/store/middleware/listenerMiddleware/listeners/boardIdSelected.test.ts

@@ -1,28 +1,38 @@
 import { configureStore, createListenerMiddleware } from '@reduxjs/toolkit';
-import type { AppStartListening } from 'app/store/store';
+import type { AppStartListening, RootState } from 'app/store/store';
+import { $gallerySelection, resetGallerySelectionSource } from 'features/gallery/store/gallerySelectionSource';
+import { selectGalleryItemNamesQueryArgs } from 'features/gallery/store/gallerySelectors';
 import {
+  boardIdSelected,
   gallerySliceConfig,
   galleryViewChanged,
   imageSelected,
   selectionChanged,
 } from 'features/gallery/store/gallerySlice';
 import { api } from 'services/api';
+import { galleryApi } from 'services/api/endpoints/gallery';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { addBoardIdSelectedListener } from './boardIdSelected';
+import { addGallerySelectionSourceListener } from './gallerySelectionSource';
 
 // The listener waits for the board's item list before auto-selecting, so the store needs the API
-// slice present (the query is never fulfilled here — the point is what happens meanwhile).
-const buildStore = () => {
+// slice present (in most tests here the query is never fulfilled — the point is what happens
+// meanwhile). `withSelectionSource` also registers the listener that publishes selections to the
+// viewer, for the tests that care whether the probe's own write reads as a user pick.
+const buildStore = ({ withSelectionSource = false }: { withSelectionSource?: boolean } = {}) => {
   const listenerMiddleware = createListenerMiddleware();
   addBoardIdSelectedListener(listenerMiddleware.startListening as unknown as AppStartListening);
+  if (withSelectionSource) {
+    addGallerySelectionSourceListener(listenerMiddleware.startListening as unknown as AppStartListening);
+  }
   return configureStore({
     reducer: {
       gallery: gallerySliceConfig.slice.reducer,
       [api.reducerPath]: api.reducer,
     },
     middleware: (getDefaultMiddleware) =>
-      getDefaultMiddleware({ serializableCheck: false }).prepend(listenerMiddleware.middleware),
+      getDefaultMiddleware({ serializableCheck: false }).prepend(listenerMiddleware.middleware).concat(api.middleware),
   });
 };
 
@@ -64,8 +74,8 @@ describe('addBoardIdSelectedListener', () => {
   });
 
   it('does not overwrite a selection made through the gallery grid either', () => {
-    // Thumbnail clicks and keyboard navigation dispatch selectionChanged, not imageSelected, so
-    // matching on the action type alone leaves the ordinary path exposed.
+    // Ctrl/shift-clicks and the delete flow's selection pruning dispatch selectionChanged, not
+    // imageSelected, so matching on the action type alone leaves those paths exposed.
     const store = buildStore();
 
     store.dispatch(galleryViewChanged('images'));
@@ -114,4 +124,49 @@ describe('addBoardIdSelectedListener', () => {
       expect(store.getState().gallery.selection).toEqual(['a.png']);
     });
   });
+
+  it("auto-selects the board's first item without that reading as a user pick", async () => {
+    // The probe picks *for* the user, so its write must not publish as a pick when it lands on the
+    // item already displayed: NoBoardBoard re-dispatches boardIdSelected even when its board is
+    // already selected, and the viewer answers a pick by lifting a running generation's progress
+    // overlay off the item for two seconds — a stale flash for a click on the current board.
+    resetGallerySelectionSource();
+    const store = buildStore({ withSelectionSource: true });
+
+    store.dispatch(boardIdSelected({ boardId: 'none' }));
+    // Fulfil the item-name query the probe is waiting on, under the same cache key it computes.
+    // The store here carries only the two slices this listener needs, so the selector — typed
+    // against the whole RootState — has to be told that is enough.
+    const queryArgs = selectGalleryItemNamesQueryArgs(store.getState() as unknown as RootState);
+    // The upsert has to be flushed through the fake timers before it is awaited, or its fulfilled
+    // action lands after the probe's 5s give-up.
+    const upsert = store.dispatch(
+      galleryApi.util.upsertQueryData('listGalleryItemNames', queryArgs, {
+        item_names: ['already-showing.png'],
+        starred_count: 0,
+        total_count: 1,
+      })
+    );
+    await vi.advanceTimersByTimeAsync(0);
+    await upsert;
+    await vi.advanceTimersByTimeAsync(6000);
+
+    // The probe really does select for the user — this is also the only coverage of that path.
+    expect(store.getState().gallery.selection).toEqual(['already-showing.png']);
+    const generationAfterFirstProbe = $gallerySelection.get().generation;
+    expect(generationAfterFirstProbe, 'moving the viewer to a new item is worth publishing').toBeGreaterThan(0);
+
+    // Re-select the same board. The probe runs again and lands on the item already displayed.
+    store.dispatch(boardIdSelected({ boardId: 'none' }));
+    // The listener's `condition` only re-evaluates its predicate when an action is dispatched, so a
+    // test that merely advances time would watch this second probe time out and clear the selection
+    // — the give-up path, not the path under test. Any action wakes it; this one touches nothing.
+    store.dispatch({ type: 'test/tick' });
+    await vi.advanceTimersByTimeAsync(6000);
+
+    expect(store.getState().gallery.selection).toEqual(['already-showing.png']);
+    expect($gallerySelection.get().generation, 'nothing moved, so there is nothing to reveal').toBe(
+      generationAfterFirstProbe
+    );
+  });
 });

invokeai/frontend/web/src/app/store/middleware/listenerMiddleware/listeners/boardIdSelected.ts

@@ -1,7 +1,7 @@
 import { isAnyOf } from '@reduxjs/toolkit';
 import type { AppStartListening } from 'app/store/store';
 import { selectGalleryItemNamesQueryArgs, selectSelection } from 'features/gallery/store/gallerySelectors';
-import { boardIdSelected, galleryViewChanged, imageSelected } from 'features/gallery/store/gallerySlice';
+import { boardIdSelected, galleryViewChanged, selectionChanged } from 'features/gallery/store/gallerySlice';
 import { galleryApi } from 'services/api/endpoints/gallery';
 
 /** The actions that ask this listener to pick an item for the user. */
@@ -11,10 +11,11 @@ export const addBoardIdSelectedListener = (startAppListening: AppStartListening)
   startAppListening({
     // Two jobs, so this cannot be a plain action matcher. The probe below is started by a board or
     // view change — but it must also be *cancelled* by any selection that lands while it waits,
-    // and a selection arrives through several actions: imageSelected from the gallery's auto-switch
-    // and keyboard navigation, selectionChanged from thumbnail clicks, boardIdSelected carrying a
-    // selection. Matching the resulting change of the selection covers all of them, including any
-    // writer added later — an action list would silently miss it.
+    // and a selection arrives through several actions: imageSelected from the gallery's auto-switch,
+    // plain thumbnail clicks and keyboard navigation, selectionChanged from ctrl/shift-clicks, the
+    // delete flow's pruning and this listener's own probe, boardIdSelected carrying a selection.
+    // Matching the resulting change of the selection covers all of them, including any writer added
+    // later — an action list would silently miss it.
     //
     // The whole selection, not just its active item: removing one of several selected thumbnails,
     // or re-picking the one already active, leaves the last item unchanged while still being the
@@ -53,15 +54,30 @@ export const addBoardIdSelectedListener = (startAppListening: AppStartListening)
       // must use getState() to ensure we do not have stale state
       const isSuccess = await condition(() => selectQuery(getState()).isSuccess, 5000);
 
+      // The probe picks an item *for* the user, so it writes the selection with the mutation
+      // action rather than `imageSelected`. The state is identical either way, but `imageSelected`
+      // means "the user asked to see this", and while a generation is running the viewer answers
+      // that by lifting the progress overlay off the item for a couple of seconds — so a write
+      // that changes nothing must not announce itself as a pick. NoBoardBoard and the view tabs
+      // dispatch even when nothing changed (unlike GalleryBoard and VirtualBoardItem), which
+      // re-runs this probe; when it lands back on the item already displayed, the mutation action
+      // is what keeps it silent. A write that genuinely moves the displayed item still reveals,
+      // through the change-of-active-item clause. See gallerySelectionSource.
+      //
+      // This does NOT stop that re-run from *replacing* a selection further down the list with
+      // `item_names[0]` — a real bug, but an older and wider one than this file, tracked in its own
+      // issue along with the give-up branch below clearing a good selection whenever `condition`
+      // gets no wake-up within 5s.
       if (!isSuccess) {
-        dispatch(imageSelected(null));
+        dispatch(selectionChanged([]));
         return;
       }
 
       // the board was just changed - we can select the first gallery item (image or video)
       const itemNames = selectQuery(getState()).data?.item_names;
+      const firstItemName = itemNames?.[0];
 
-      dispatch(imageSelected(itemNames?.[0] ?? null));
+      dispatch(selectionChanged(firstItemName ? [firstItemName] : []));
     },
   });
 };

invokeai/frontend/web/src/app/store/middleware/listenerMiddleware/listeners/gallerySelectionSource.test.ts

@@ -0,0 +1,97 @@
+import { configureStore, createListenerMiddleware } from '@reduxjs/toolkit';
+import type { AppStartListening } from 'app/store/store';
+import {
+  $gallerySelection,
+  markNextSelectionAutoSwitched,
+  resetGallerySelectionSource,
+} from 'features/gallery/store/gallerySelectionSource';
+import {
+  boardIdSelected,
+  gallerySliceConfig,
+  imageSelected,
+  selectionChanged,
+} from 'features/gallery/store/gallerySlice';
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import { addGallerySelectionSourceListener } from './gallerySelectionSource';
+
+const buildStore = () => {
+  const listenerMiddleware = createListenerMiddleware();
+  addGallerySelectionSourceListener(listenerMiddleware.startListening as unknown as AppStartListening);
+  return configureStore({
+    reducer: { gallery: gallerySliceConfig.slice.reducer },
+    middleware: (getDefaultMiddleware) => getDefaultMiddleware().prepend(listenerMiddleware.middleware),
+  });
+};
+
+describe('addGallerySelectionSourceListener', () => {
+  beforeEach(() => {
+    resetGallerySelectionSource();
+  });
+
+  it('publishes a click made through the gallery grid', () => {
+    const store = buildStore();
+    store.dispatch(imageSelected('clicked.png'));
+    expect($gallerySelection.get()).toMatchObject({ name: 'clicked.png', isAutoSwitch: false });
+  });
+
+  it('does not publish a multi-select mutation that leaves the active item in place', () => {
+    // Active item b, selection [a, b]: ctrl-clicking `a` off the selection dispatches
+    // selectionChanged([b]). Nothing the viewer shows changes — publishing it would flash the
+    // progress overlay off for bookkeeping aimed at a different item.
+    const store = buildStore();
+    store.dispatch(selectionChanged(['a.png', 'b.png']));
+    const beforeDeselect = $gallerySelection.get().generation;
+
+    store.dispatch(selectionChanged(['b.png']));
+
+    expect($gallerySelection.get().generation).toBe(beforeDeselect);
+  });
+
+  it('publishes a multi-select mutation that moves the active item', () => {
+    // Ctrl-clicking an unselected item appends it and makes it active. selectionChanged is not in
+    // the pick list, so this relies on the change-of-active-item clause.
+    const store = buildStore();
+    store.dispatch(imageSelected('a.png'));
+    store.dispatch(selectionChanged(['a.png', 'b.png']));
+    expect($gallerySelection.get()).toMatchObject({ name: 'b.png', isAutoSwitch: false });
+  });
+
+  it('publishes a re-selection of the item already active as a new selection', () => {
+    // Nothing in the state changes, so a state-transition-only predicate would miss it — and the
+    // viewer would have no way to make a repeat click on the displayed item visible.
+    const store = buildStore();
+    store.dispatch(imageSelected('a.png'));
+    const first = $gallerySelection.get().generation;
+    store.dispatch(imageSelected('a.png'));
+    expect($gallerySelection.get().generation).toBeGreaterThan(first);
+  });
+
+  it('attributes an auto-switch that carries its own board change', () => {
+    const store = buildStore();
+    markNextSelectionAutoSwitched();
+    store.dispatch(boardIdSelected({ boardId: 'other', select: { selection: ['auto.png'], galleryView: 'images' } }));
+    expect($gallerySelection.get()).toMatchObject({ name: 'auto.png', isAutoSwitch: true });
+  });
+
+  it('publishes a selection cleared by an action it does not name', () => {
+    // logout is not in the action list; the active-item clause is what covers it.
+    const store = buildStore();
+    store.dispatch(imageSelected('a.png'));
+    store.dispatch({ type: 'auth/logout' });
+    expect($gallerySelection.get().name).toBeNull();
+  });
+
+  it('does not treat a bare board click as the user picking the item that stays selected', () => {
+    // Clicking a board in the boards list dispatches boardIdSelected with no selection payload and
+    // leaves the selection alone. Counting it would make the viewer reveal an item the user never
+    // clicked, over the live progress preview.
+    const store = buildStore();
+    store.dispatch(imageSelected('a.png'));
+    const beforeBoardClick = $gallerySelection.get().generation;
+
+    store.dispatch(boardIdSelected({ boardId: 'some-other-board' }));
+
+    expect($gallerySelection.get().generation).toBe(beforeBoardClick);
+  });
+});