fix(721,740): align Auto-Tune proposal rows on a grid; guard AddItemsDialog's async searches (#831)

Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

design-system/design_handoff_auto_tune/AutoTune.jsx

@@ -816,7 +816,7 @@
                     <div key={r.name} style={{ borderTop: i ? "1px solid var(--border-hairline)" : "none", opacity: r.alreadyExists ? 0.55 : 1 }}>
                       <div style={{ display: "flex", alignItems: "center", gap: 12, padding: "9px 14px" }}>
                         <Checkbox checked={on} disabled={r.alreadyExists} onChange={() => toggle(r.name)} />
-                        <span style={{ ...mono, minWidth: 42, font: "var(--text-sm) var(--font-mono)", color: "var(--text-secondary)" }}>{r.number}</span>
+                        <span style={{ ...mono, minWidth: 42, textAlign: "right", font: "var(--text-sm) var(--font-mono)", color: "var(--text-secondary)" }}>{r.number}</span>
                         <Bug name={name} initials={ov.bug && ov.bug.initials} ci={ov.bug && ov.bug.ci} size={28} />
                         <button type="button" onClick={() => !r.alreadyExists && toggleExp(r.name)} style={{ flex: 1, minWidth: 0, textAlign: "left", background: "transparent", border: "none", padding: 0, cursor: r.alreadyExists ? "default" : "pointer" }}>
                           <div style={{ font: "var(--weight-semibold) var(--text-sm)/1.2 var(--font-sans)", color: "var(--text-primary)", whiteSpace: "nowrap", overflow: "hidden", textOverflow: "ellipsis" }}>{name}</div>

design-system/templates/chicorytv-admin/AutoTune.jsx

@@ -816,7 +816,7 @@
                     <div key={r.name} style={{ borderTop: i ? "1px solid var(--border-hairline)" : "none", opacity: r.alreadyExists ? 0.55 : 1 }}>
                       <div style={{ display: "flex", alignItems: "center", gap: 12, padding: "9px 14px" }}>
                         <Checkbox checked={on} disabled={r.alreadyExists} onChange={() => toggle(r.name)} />
-                        <span style={{ ...mono, minWidth: 42, font: "var(--text-sm) var(--font-mono)", color: "var(--text-secondary)" }}>{r.number}</span>
+                        <span style={{ ...mono, minWidth: 42, textAlign: "right", font: "var(--text-sm) var(--font-mono)", color: "var(--text-secondary)" }}>{r.number}</span>
                         <Bug name={name} initials={ov.bug && ov.bug.initials} ci={ov.bug && ov.bug.ci} size={28} />
                         <button type="button" onClick={() => !r.alreadyExists && toggleExp(r.name)} style={{ flex: 1, minWidth: 0, textAlign: "left", background: "transparent", border: "none", padding: 0, cursor: r.alreadyExists ? "default" : "pointer" }}>
                           <div style={{ font: "var(--weight-semibold) var(--text-sm)/1.2 var(--font-sans)", color: "var(--text-primary)", whiteSpace: "nowrap", overflow: "hidden", textOverflow: "ellipsis" }}>{name}</div>

docs/spa-conventions.md

@@ -208,12 +208,28 @@ the #644 follow-up got Class A right and Class B only half right):
     min-query boundary test pinned nothing. Removing the screen's copy is what made the helper's
     gate load-bearing. **The invariant, not the count: every gate must have at least one test that
     reddens when that gate ALONE is removed.** A guard you cannot redden is not a guard, and "it's
-    the single sink" is not evidence that it is the only one. **Outstanding on this screen**: `AddItemsDialog` still lacks the monotonic `seqRef`
-    stale-response guard and `useIsMountedRef()` — the same class of guard "Debounced typeaheads"
-    below mandates there, applied to a debounced-while-typing fetch; `AddItemsDialog` is an explicit
-    Search-button submission, not a typeahead, so that mandate doesn't reach it directly, but the
-    same race (a superseded search settling after a newer one) can still occur here — tracked in
-    **ersatztv#740**, not yet fixed here.
+    the single sink" is not evidence that it is the only one. **The async guards below reach this
+    screen too** (#740), and debouncing is not what triggers either of them — **the two guards have
+    different triggers and neither subsumes the other**:
+    - `seqRef` is triggered by **concurrent reachability**: two or more entry points into one async
+      sink, so a superseded response can settle after a newer one. `AddItemsDialog` has exactly
+      that — a form submit fanning out over `DEFAULT_SEARCH_KINDS`, and a kind-chip click issuing a
+      single request — so `runSearch` carries it even though it is an explicit Search-button
+      submission rather than the debounced typeahead "Debounced typeaheads" mandates it for.
+    - `useIsMountedRef()` is triggered by **unmount-reachability**, which is present with a single
+      entry point too. Check whether the surface can close mid-request through a path that does not
+      consult the in-flight flag: a disabled submit button does not settle it, because `Dialog`
+      closes on Escape and on a backdrop click without consulting anything. Both `runSearch` and
+      `submit` carry it for that reason.
+
+    Apply the guard to this component's OWN state only. A parent callback invoked after the await —
+    `onAdded`, reloading the list after a write that succeeded — belongs to the parent, which owns
+    its own guarding; dropping it because THIS component went away leaves the list stale.
+
+    **`searching` is written under the seq guard too**, which is the half easily missed: a
+    superseded search's `finally` otherwise clears the spinner while the newer request is still in
+    flight, and the dialog claims to be idle mid-request. So is the **catch** path — an error from a
+    superseded search must not wipe the results a newer, successful one has already painted.
 
   - **Compile typed text; never forward raw Lucene.** Send `titleContainsQuery(text)` →
     `title:*<escaped>*`. The index's default field does not match bare title words (`Alpha` finds

web/src/components/forms.tsx

@@ -294,6 +294,12 @@ export interface CheckboxProps {
   checked?: boolean;
   onChange?: (next: boolean) => void;
   label?: string;
+  /**
+   * Keep `label` as the accessible name but do not render it visibly — same contract as `Switch`
+   * above. A caller that renders the name itself (a grid row with its own name column, #721) needs
+   * the control to stay named without emitting the text a second time.
+   */
+  hideLabel?: boolean;
   indeterminate?: boolean;
   disabled?: boolean;
   style?: CSSProperties;
@@ -303,6 +309,7 @@ export function Checkbox({
   checked = false,
   onChange,
   label,
+  hideLabel = false,
   indeterminate = false,
   disabled = false,
   style
@@ -328,7 +335,7 @@ export function Checkbox({
           <Check aria-hidden="true" size={12} strokeWidth={3} />
         ) : null}
       </button>
-      {label && <span>{label}</span>}
+      {label && !hideLabel && <span>{label}</span>}
     </span>
   );
 }

web/src/screens/AutoTuneScreen.test.tsx

@@ -149,6 +149,51 @@ describe('AutoTuneScreen', () => {
     expect(screen.getByText('Exists')).toBeInTheDocument();
   });
 
+  it('#721: renders each proposal name once and lays the row out as a fixed-track grid', async () => {
+    mockAutoTuneApi();
+    render(<AutoTuneScreen />);
+
+    fireEvent.click(screen.getByRole('button', { name: /Preview channels/ }));
+    await screen.findByText('TV Shows');
+
+    // The name used to render TWICE per row — once as the Checkbox's visible label, once in the
+    // detail column. The checkbox keeps it as its accessible name (asserted below), so this
+    // reddens on a re-added visible `label` without weakening the a11y guarantee.
+    expect(screen.getAllByText('The Office')).toHaveLength(1);
+    expect(screen.getAllByText('Friends')).toHaveLength(1);
+
+    const box = screen.getByRole('checkbox', { name: 'The Office' });
+    expect(box).toHaveAttribute('aria-label', 'The Office');
+
+    // The group header carried the same duplicate one line up: a visible "Select all <axis>" label
+    // immediately followed by the axis title itself. The accessible name survives, the text does not.
+    expect(screen.queryByText('Select all TV Shows')).not.toBeInTheDocument();
+    expect(screen.getByRole('checkbox', { name: 'Select all TV Shows' })).toBeInTheDocument();
+
+    // Alignment is a property of the TRACK LIST, not of any rendered text: jsdom computes no
+    // layout, so the only thing that can redden on a revert to the old flex row is the declared
+    // grid template itself. Matched as a pattern rather than a literal: the invariant is that the
+    // NUMBER track is a fixed `<n>ch` width — a content-sized track resolves per row (each row is
+    // its own grid container) and would not align — not the particular n, which may be retuned.
+    const row = box.closest('.ctv-checkbox')?.parentElement;
+    expect(row).not.toBeNull();
+    expect(row).toHaveStyle({ display: 'grid' });
+    expect(row?.style.gridTemplateColumns).toMatch(/^auto [1-9]\d*ch minmax\(0, 1fr\) auto$/);
+
+    // Right-alignment is what makes numbers of differing digit-length agree on their last digit.
+    // Without it the grid track aligns the column's left edge and the digits still ragged-right.
+    const numberCell = row?.children[1] as HTMLElement | undefined;
+    expect(numberCell?.textContent).toBe('500');
+    expect(numberCell).toHaveStyle({ textAlign: 'right' });
+    // No fixed track bounds a channel number (`Channel.NumberValidator` allows an unbounded integer
+    // part), so an over-long value must clip rather than overflow into the checkbox beside it.
+    expect(numberCell).toHaveStyle({ overflow: 'hidden' });
+    // `text-overflow: ellipsis` only applies to non-wrapping inline overflow, and the clipped value
+    // must stay recoverable — the ellipsis is the whole reason a fixed track is acceptable here.
+    expect(numberCell).toHaveStyle({ whiteSpace: 'nowrap' });
+    expect(numberCell).toHaveAttribute('title', '500');
+  });
+
   it('supports select-all / clear over the selectable proposals', async () => {
     mockAutoTuneApi();
     render(<AutoTuneScreen />);

web/src/screens/AutoTuneScreen.tsx

@@ -118,6 +118,40 @@ const eyebrowStyle: CSSProperties = {
 
 const monoStyle: CSSProperties = { fontFamily: 'var(--font-mono)', fontVariantNumeric: 'tabular-nums' };
 
+// Proposal rows are a grid so the channel-number column starts at the same x on every row (#721).
+// Each row is its OWN grid container, so the number track has to be a FIXED width — a content-sized
+// track (`auto`/`max-content`) is resolved per container and would go back to varying per row.
+//
+// No fixed width is a true upper bound on the value, and this track does not pretend to be one:
+// `AutoTuneNumberAllocator.Allocate` emits `int.ToString()` from a `StartingNumber` that
+// `PreviewAutoTuneChannelsHandler` only floors at 1, and a per-proposal override is free text
+// checked against `Channel.NumberValidator` (`^[0-9]+(\.[0-9]{1,2})?$`) — unbounded integer part
+// plus up to two decimals. So the track is sized for the range seen in practice and the cell CLIPS
+// beyond it: an extreme value degrades to an ellipsis with the full value on hover, rather than
+// overflowing a fixed track and overlapping the checkbox to its left.
+//
+// What actually stopped the column drifting with the name length is the checkbox track being
+// constant — a label-less Checkbox renders one fixed-size control — not the number track's width.
+const NUMBER_TRACK = '7ch';
+const proposalRowStyle: CSSProperties = {
+  display: 'grid',
+  gridTemplateColumns: `auto ${NUMBER_TRACK} minmax(0, 1fr) auto`,
+  alignItems: 'center',
+  columnGap: 12,
+  padding: '9px 14px'
+};
+
+// Right-aligned + tabular so 2-, 3- and 4-digit numbers agree on their last digit (#721).
+const proposalNumberStyle: CSSProperties = {
+  ...monoStyle,
+  font: 'var(--text-sm) var(--font-mono)',
+  color: 'var(--text-secondary)',
+  textAlign: 'right',
+  overflow: 'hidden',
+  textOverflow: 'ellipsis',
+  whiteSpace: 'nowrap'
+};
+
 // Selection identity — axis+value uniquely identifies a proposal. Keying on the display
 // name would couple two same-named proposals (e.g. a TV show titled "Comedy" and the
 // "Comedy" genre), flipping/creating both. An axis id never contains a space, so a single
@@ -1019,6 +1053,7 @@ function PreviewStep({
                 checked={allOn}
                 indeterminate={someOn && !allOn}
                 disabled={groupSelectable.length === 0}
+                hideLabel
                 label={`Select all ${axis.title}`}
                 onChange={() => setAll(rows, !allOn)}
               />
@@ -1047,17 +1082,18 @@ function PreviewStep({
                       opacity: row.alreadyExists ? 0.55 : 1
                     }}
                   >
-                    <div style={{ display: 'flex', alignItems: 'center', gap: 12, padding: '9px 14px' }}>
+                    <div style={proposalRowStyle}>
                       <Checkbox
                         checked={on && !row.alreadyExists}
                         disabled={row.alreadyExists}
+                        hideLabel
                         label={displayName}
                         onChange={() => toggle(key)}
                       />
-                      <span style={{ ...monoStyle, minWidth: 42, font: 'var(--text-sm) var(--font-mono)', color: 'var(--text-secondary)' }}>
+                      <span style={proposalNumberStyle} title={overrides[key]?.number?.trim() || row.number}>
                         {overrides[key]?.number?.trim() || row.number}
                       </span>
-                      <div style={{ flex: 1, minWidth: 0 }}>
+                      <div style={{ minWidth: 0 }}>
                         <div
                           style={{
                             font: 'var(--weight-semibold) var(--text-sm)/1.2 var(--font-sans)',
@@ -1073,26 +1109,30 @@ function PreviewStep({
                           from “{row.value}”
                         </div>
                       </div>
-                      {edited && (
-                        <Tag icon={<Pencil aria-hidden="true" size={11} />} tone="accent">
-                          Edited
-                        </Tag>
-                      )}
-                      <Badge tone="neutral">
-                        <span style={monoStyle}>{row.itemCount}</span>&nbsp;items
-                      </Badge>
-                      {row.alreadyExists ? (
-                        <Tag tone="neutral">Exists</Tag>
-                      ) : (
-                        <Button
-                          size="sm"
-                          variant="ghost"
-                          startIcon={<Settings2 aria-hidden="true" size={14} />}
-                          onClick={() => onConfigure(key)}
-                        >
-                          Configure
-                        </Button>
-                      )}
+                      {/* One grid cell, not three: the trailing controls are conditional, and a
+                          conditional child would shift every track after it from row to row. */}
+                      <div style={{ display: 'flex', alignItems: 'center', gap: 12, justifySelf: 'end' }}>
+                        {edited && (
+                          <Tag icon={<Pencil aria-hidden="true" size={11} />} tone="accent">
+                            Edited
+                          </Tag>
+                        )}
+                        <Badge tone="neutral">
+                          <span style={monoStyle}>{row.itemCount}</span>&nbsp;items
+                        </Badge>
+                        {row.alreadyExists ? (
+                          <Tag tone="neutral">Exists</Tag>
+                        ) : (
+                          <Button
+                            size="sm"
+                            variant="ghost"
+                            startIcon={<Settings2 aria-hidden="true" size={14} />}
+                            onClick={() => onConfigure(key)}
+                          >
+                            Configure
+                          </Button>
+                        )}
+                      </div>
                     </div>
                   </div>
                 );

web/src/screens/CollectionsScreen.guards.test.tsx

@@ -0,0 +1,310 @@
+import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { CollectionsScreen } from './CollectionsScreen';
+
+// #740: AddItemsDialog's stale-response and is-mounted guards.
+//
+// These live in their own file rather than alongside the rest of CollectionsScreen's tests because
+// both need a fetch mock whose /library/browse responses settle on demand — the defect only exists
+// in the window between two overlapping searches — and one needs a module mock of `../hooks` that
+// would otherwise record reads from every other component in the tree.
+
+// Every value AddItemsDialog read out of the shared is-mounted guard, in order. This is what makes
+// the unmount test non-vacuous (the same reason spelled out in schedules/pickers.test.tsx): an
+// unmounted tree renders nothing either way, so no DOM assertion can distinguish "the guard stopped
+// the update" from "React discarded it". Recording the reads proves the component actually CONSULTS
+// the guard, and specifically that it consults it once the late response lands, seeing `false`.
+const mountedReads: boolean[] = [];
+
+vi.mock('../hooks', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../hooks')>();
+  const { useRef } = await import('react');
+  return {
+    ...actual,
+    useIsMountedRef: () => {
+      const inner = actual.useIsMountedRef();
+      const proxyRef = useRef<{ readonly current: boolean } | null>(null);
+      proxyRef.current ??= {
+        get current() {
+          mountedReads.push(inner.current);
+          return inner.current;
+        }
+      };
+      return proxyRef.current;
+    }
+  };
+});
+
+vi.mock('../builder/rules/fieldCatalog', () => ({
+  useSearchFields: () => ({ byGroup: [], fields: [], fieldTypes: {}, status: 'success' })
+}));
+
+function jsonResponse(body: unknown): Response {
+  return new Response(JSON.stringify(body), {
+    headers: { 'Content-Type': 'application/json' },
+    status: 200
+  });
+}
+
+const manualCollections = [
+  { collectionType: 'Collection', id: 1, name: 'Favorites', state: 'Normal', useCustomPlaybackOrder: false }
+];
+
+const browseRows: Record<string, { id: number; mediaType: string; title: string }> = {
+  Artist: { id: 3, mediaType: 'Artist', title: 'Zapp Band' },
+  Movie: { id: 1, mediaType: 'Movie', title: 'Zathura' },
+  TelevisionShow: { id: 2, mediaType: 'TelevisionShow', title: 'Adventure Time' }
+};
+
+/** A `/library/browse` request parked until the test settles or fails it. */
+interface Parked {
+  fail: () => void;
+  mediaType: string;
+  settle: () => void;
+}
+
+/**
+ * fetch mock whose add-items browse requests — and, when `parkAdd` is set, the add-items POST —
+ * PARK instead of resolving. Everything else answers immediately, so the screen and the dialog
+ * reach their normal state.
+ */
+function mockDeferredApi(options: { parkAdd?: boolean } = {}): Parked[] {
+  const parked: Parked[] = [];
+
+  vi.spyOn(window, 'fetch').mockImplementation((input: RequestInfo | URL, init?: RequestInit) => {
+    const url = input.toString();
+    const method = (init?.method ?? 'GET').toUpperCase();
+
+    if (url === '/api/v1/collections' && method === 'GET') {
+      return Promise.resolve(jsonResponse(manualCollections));
+    }
+
+    if (url === '/api/v1/smart-collections' && method === 'GET') {
+      return Promise.resolve(jsonResponse([]));
+    }
+
+    if (url.startsWith('/api/v1/library/browse')) {
+      const params = new URL(url, 'http://localhost').searchParams;
+      const query = params.get('query') ?? '';
+      const mediaType = params.get('mediaType') ?? '';
+
+      // The manual-collection preview query is not an add-items search — never park it.
+      if (query.startsWith('collection:') || mediaType === '') {
+        return Promise.resolve(jsonResponse({ page: [], totalCount: 0 }));
+      }
+
+      return new Promise<Response>((resolve, reject) => {
+        const row = browseRows[mediaType];
+        parked.push({
+          fail: () => reject(new Error(`browse failed for ${mediaType}`)),
+          mediaType,
+          settle: () => resolve(jsonResponse(row ? { page: [row], totalCount: 1 } : { page: [], totalCount: 0 }))
+        });
+      });
+    }
+
+    if (/^\/api\/v1\/collections\/\d+\/items/.test(url) && method === 'GET') {
+      return Promise.resolve(jsonResponse({ page: [], totalCount: 0 }));
+    }
+
+    if (options.parkAdd === true && /^\/api\/v1\/collections\/\d+\/items$/.test(url) && method === 'POST') {
+      return new Promise<Response>((resolve, reject) => {
+        parked.push({
+          fail: () => reject(new Error('add failed')),
+          mediaType: 'ADD',
+          settle: () => resolve(new Response(null, { status: 204 }))
+        });
+      });
+    }
+
+    return Promise.resolve(new Response(null, { status: 204 }));
+  });
+
+  return parked;
+}
+
+async function openAddItemsDialog(): Promise<HTMLElement> {
+  render(<CollectionsScreen />);
+  await screen.findByText('Favorites');
+
+  fireEvent.click(screen.getByText('Favorites'));
+  await screen.findByRole('button', { name: 'Add items' });
+
+  fireEvent.click(screen.getByRole('button', { name: 'Add items' }));
+  return screen.getByRole('dialog');
+}
+
+function typeAndSearch(dialog: HTMLElement, text: string): void {
+  fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+    target: { value: text }
+  });
+  fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+}
+
+/**
+ * Flush the settled fetch promises and the React updates they schedule. Deliberately NOT keyed on
+ * anything a guard produces — a barrier that waits on the guard's own side effect would make every
+ * test in this file redden when any one guard is removed, which is the opposite of pinning them
+ * separately.
+ */
+async function flush(): Promise<void> {
+  await act(async () => {
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+beforeEach(() => {
+  mountedReads.length = 0;
+});
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+describe('AddItemsDialog async guards (#740)', () => {
+  it('a superseded fan-out settling LAST does not repaint the newer kind-filtered results', async () => {
+    const parked = mockDeferredApi();
+    const dialog = await openAddItemsDialog();
+
+    // Search A: the "All" fan-out over Movie / TelevisionShow / Artist.
+    typeAndSearch(dialog, 'za');
+    await waitFor(() => expect(parked).toHaveLength(3));
+    const fanOut = parked.splice(0, 3);
+
+    // Search B, issued while A is still in flight: the Movies chip, one request.
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Movies' }));
+    await waitFor(() => expect(parked).toHaveLength(1));
+    const chip = parked[0];
+    expect(chip.mediaType).toBe('Movie');
+
+    // B settles first and owns the list.
+    chip.settle();
+    expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+    // A settles second. Without the seq guard its 3 kinds overwrite B's results, leaving a list of
+    // shows and artists under an active "Movies" chip.
+    for (const request of fanOut) {
+      request.settle();
+    }
+    await flush();
+
+    expect(within(dialog).getByText('Zathura')).toBeInTheDocument();
+    expect(within(dialog).queryByText('Adventure Time')).not.toBeInTheDocument();
+    expect(within(dialog).queryByText('Zapp Band')).not.toBeInTheDocument();
+  });
+
+  it('a SUPERSEDED search settling first does not clear the spinner the newer one still owes', async () => {
+    const parked = mockDeferredApi();
+    const dialog = await openAddItemsDialog();
+
+    // Search A (fan-out), then search B (Movies chip) while A is still in flight.
+    typeAndSearch(dialog, 'za');
+    await waitFor(() => expect(parked).toHaveLength(3));
+    const fanOut = parked.splice(0, 3);
+
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Movies' }));
+    await waitFor(() => expect(parked).toHaveLength(1));
+    const chip = parked[0];
+
+    // A — now superseded — settles FIRST. `Promise.all` resolves, so its `finally` runs while B is
+    // still outstanding. Without the guard on `setSearching(false)` the spinner clears here and the
+    // dialog claims to be idle with a request in flight.
+    for (const request of fanOut) {
+      request.settle();
+    }
+    await flush();
+
+    // `loading` is what disables the Search button — there is no other disabled condition on it.
+    const searchButton = within(dialog).getByRole('button', { name: 'Search' });
+    expect(searchButton).toBeDisabled();
+
+    chip.settle();
+    await waitFor(() => expect(searchButton).toBeEnabled());
+    expect(within(dialog).getByText('Zathura')).toBeInTheDocument();
+  });
+
+  it('a SUPERSEDED search that FAILS does not wipe the newer results with an error', async () => {
+    const parked = mockDeferredApi();
+    const dialog = await openAddItemsDialog();
+
+    typeAndSearch(dialog, 'za');
+    await waitFor(() => expect(parked).toHaveLength(3));
+    const fanOut = parked.splice(0, 3);
+
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Movies' }));
+    await waitFor(() => expect(parked).toHaveLength(1));
+
+    // B succeeds and paints.
+    parked[0].settle();
+    expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+    // A — superseded — now REJECTS. Its catch clears results/totalMatches/submittedQuery and raises
+    // an error banner. Unguarded, a failure belonging to a search the user has already moved on
+    // from destroys the results they are currently looking at.
+    fanOut[0].fail();
+    fanOut[1].settle();
+    fanOut[2].settle();
+    await flush();
+
+    expect(within(dialog).getByText('Zathura')).toBeInTheDocument();
+    // Matched by ROLE, not by the fallback message text: `messageFromCollectionError` returns
+    // `error.message` for an `Error` throw and only reaches its fallback string for a non-Error,
+    // so asserting the fallback text here would be null-vs-null and pass with the guard removed.
+    expect(within(dialog).queryByRole('alert')).not.toBeInTheDocument();
+  });
+
+  it('a search resolving AFTER the dialog unmounts consults the is-mounted guard and is dropped', async () => {
+    const parked = mockDeferredApi();
+    const dialog = await openAddItemsDialog();
+
+    typeAndSearch(dialog, 'za');
+    await waitFor(() => expect(parked).toHaveLength(3));
+
+    // Cancel unmounts the dialog for real — the caller renders it under `key={add-${pickerOpen}}`.
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' }));
+    await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
+
+    // Only reads that happen from here on can prove the LATE response consulted the guard.
+    mountedReads.length = 0;
+    for (const request of parked) {
+      request.settle();
+    }
+
+    await waitFor(() => expect(mountedReads.length).toBeGreaterThan(0));
+    expect(mountedReads).toContain(false);
+    expect(mountedReads.every((read) => read === false)).toBe(true);
+  });
+
+  it('a FAILED add resolving after an Escape-close consults the is-mounted guard', async () => {
+    const parked = mockDeferredApi({ parkAdd: true });
+    const dialog = await openAddItemsDialog();
+
+    typeAndSearch(dialog, 'za');
+    await waitFor(() => expect(parked).toHaveLength(3));
+    for (const request of parked.splice(0, 3)) {
+      request.settle();
+    }
+
+    // Select a result, then submit.
+    fireEvent.click(await within(dialog).findByRole('button', { name: /Zathura/ }));
+    fireEvent.click(within(dialog).getByRole('button', { name: /Add 1 item/ }));
+    await waitFor(() => expect(parked).toHaveLength(1));
+
+    // The Add button is disabled while adding, but Escape is not gated on `adding` — this is the
+    // unmount-while-in-flight path a disabled submit button does NOT close.
+    fireEvent.keyDown(document, { key: 'Escape' });
+    await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
+
+    mountedReads.length = 0;
+    parked[0].fail();
+
+    // `submit` has exactly TWO post-await writes to this component's own state — `setError` in the
+    // catch and `setAdding` in the finally — and the invariant is that BOTH consult the guard.
+    // Counting them is what keeps the two from masking each other: asserting only that SOME read
+    // saw `false` stays green with either one removed, because the survivor still reads the ref.
+    await waitFor(() => expect(mountedReads).toHaveLength(2));
+    expect(mountedReads.every((read) => read === false)).toBe(true);
+  });
+});

web/src/screens/CollectionsScreen.tsx

@@ -48,6 +48,7 @@ import {
   type MediaCollection,
   type SmartCollection
 } from '../api';
+import { useIsMountedRef } from '../hooks';
 import { TYPE_LABEL } from '../media/mediaKinds';
 import { SmartCollectionDialog } from '../builder/SmartCollectionDialog';
 
@@ -245,6 +246,8 @@ function AddItemsDialog({
   const [error, setError] = useState<string | null>(null);
   const [adding, setAdding] = useState(false);
   const [kindFilter, setKindFilter] = useState<MediaKindFilter>('all');
+  const seqRef = useRef(0);
+  const mountedRef = useIsMountedRef();
 
   // `searchLibraryBrowseItems` owns the min-query gate, the pageSize clamp, and the
   // raw-Lucene-vs-compiled-query choice (§3b: "the bound belongs to the helper, not the caller").
@@ -252,8 +255,21 @@ function AddItemsDialog({
   // the two masked each other (deleting either one left the whole suite green, so the boundary
   // test pinned nothing) — so there is now exactly one gate, in the helper, and every caller
   // (blank form submit, kind-chip click) routes through it unconditionally.
+  //
+  // #740: `runSearch` is reachable twice concurrently — a form submit fans out over
+  // DEFAULT_SEARCH_KINDS while a kind-chip click issues a single-kind search — so the two guards
+  // §3b mandates for asynchronous pickers both apply. `seq` drops a SUPERSEDED response (the
+  // 3-kind batch settling after the 1-kind chip would otherwise repaint the list with all kinds
+  // while the active chip reads Movies), and `mountedRef` drops a response that lands after the
+  // dialog is unmounted (it genuinely is — `key={`add-${pickerOpen}`}` on the caller). Neither
+  // subsumes the other: a late response still matches the latest seq if no newer search started.
+  //
+  // `searching` is written under the same guard, which is what stops the spinner lying: without
+  // it, whichever search settles FIRST clears the flag while the other is still in flight.
   const runSearch = async (filter: MediaKindFilter = kindFilter) => {
     const trimmed = query.trim();
+    const seq = ++seqRef.current;
+    const isCurrent = () => mountedRef.current && seqRef.current === seq;
 
     setSearching(true);
     setError(null);
@@ -261,6 +277,9 @@ function AddItemsDialog({
     try {
       const kinds = filter === 'all' ? DEFAULT_SEARCH_KINDS : [filter];
       const perKind = await Promise.all(kinds.map((mediaType) => searchLibraryBrowseItems(mediaType, trimmed)));
+      if (!isCurrent()) {
+        return;
+      }
       const merged = perKind
         .flatMap((result) => result.items)
         .filter((item) => ADDABLE_TYPES.has(item.mediaType))
@@ -277,6 +296,9 @@ function AddItemsDialog({
       // searched" rather than name the sub-gate text.
       setSubmittedQuery(trimmed.length < LIBRARY_PICKER_MIN_QUERY ? '' : trimmed);
     } catch (searchError) {
+      if (!isCurrent()) {
+        return;
+      }
       setError(messageFromCollectionError(searchError, 'Unable to search library'));
       // A failed search must not leave a stale results/totalMatches pair rendering a confident
       // "Showing N of M matches" hint beside the error banner (#685 review).
@@ -284,7 +306,10 @@ function AddItemsDialog({
       setTotalMatches(0);
       setSubmittedQuery('');
     } finally {
-      setSearching(false);
+      // A superseded search must not clear the spinner a NEWER one is still owed.
+      if (isCurrent()) {
+        setSearching(false);
+      }
     }
   };
 
@@ -322,12 +347,25 @@ function AddItemsDialog({
 
     try {
       await addItemsToCollection(collection.id, toAddItemsRequest([...selected.values()]));
+      // NOT guarded: both belong to the still-mounted PARENT. `onAdded` reloads the collection's
+      // items, and dropping it because this dialog went away would leave the list stale after a
+      // write that succeeded.
       onAdded();
       onClose();
     } catch (addError) {
-      setError(messageFromCollectionError(addError, 'Unable to add items'));
+      // Guarded: these two are this component's own state. The dialog is unmount-reachable while a
+      // request is in flight even though the Add button is disabled — Escape, a backdrop click and
+      // the header close button all reach `onClose` without consulting `adding`
+      // (`components/overlay.tsx`), and the caller remounts on `key={`add-${pickerOpen}`}`. This is
+      // the is-mounted guard's own trigger, unmount-while-in-flight, and it needs no `seq`: the
+      // disabled Add button means there is never a second submission to be superseded by.
+      if (mountedRef.current) {
+        setError(messageFromCollectionError(addError, 'Unable to add items'));
+      }
     } finally {
-      setAdding(false);
+      if (mountedRef.current) {
+        setAdding(false);
+      }
     }
   };