fix(685): AddItemsDialog resolves by search instead of windowing the whole media-library type (#741)

docs/spa-conventions.md

@@ -178,10 +178,42 @@ the #644 follow-up got Class A right and Class B only half right):
 
   The helper owns both bounds: at most ONE `getLibraryBrowseItems` request per settled query, at most
   `LIBRARY_PICKER_RESULTS` (25) rows, and no request at all below `LIBRARY_PICKER_MIN_QUERY` (2)
-  characters. Selecting a media-library type must issue **zero** requests. There is no truncation, so
-  there is no truncation hint — the old `Showing the first 100 of 5000 — use search to narrow.` copy
-  is gone from these pickers along with the window it described. Prove the bound with a
-  **request-count assertion against a large (20k-row) fixture**, not by inspection.
+  characters. Selecting a media-library type must issue **zero** requests. The per-kind
+  `LIBRARY_PICKER_RESULTS` cap is the only truncation this class has — there is no whole-type window
+  left to hint at, so the old `Showing the first 100 of 5000 — use search to narrow.` copy is gone
+  from these pickers along with the window it described. Surfacing the per-kind cap is *permitted*
+  wherever it is reachable, and *required* only where bulk selection makes the count actionable — see
+  the `AddItemsDialog` sub-bullet below, which sums the cap across kinds and renders a `Showing N of
+  M matches` hint for exactly that reason. Prove the bound with a **request-count assertion against a
+  large (20k-row) fixture**, not by inspection.
+
+  - **`SearchPicker` is the single-select SHAPE, not the rule itself.** A MULTI-select picker
+    (`CollectionsScreen`'s `AddItemsDialog` — checkbox rows, many items added at once, fanned out
+    over several kinds) cannot render `SearchPicker` and must not be forced to. It satisfies this
+    section by taking the same *constraints* the helper enforces for single-select — the gate on
+    `LIBRARY_PICKER_MIN_QUERY`, `titleContainsQuery` the typed text, `LIBRARY_PICKER_RESULTS` per
+    kind — via `searchLibraryBrowseItems` (`web/src/api/libraryBrowse.ts`), a sibling of
+    `searchLibraryPickerOptions` that returns full `LibraryBrowseItem` rows plus `totalCount`
+    instead of `{id, name}`, so the bound lives in the helper rather than the caller (#685 review
+    finding 2). There is no post-fetch `slice`, but the per-kind cap can still truncate the real
+    match count — this is a bulk multi-select add, where "add the 40 matching episodes" is a
+    first-class use, so `AddItemsDialog` sums each kind's `totalCount` and renders a `Showing N of
+    M matches` hint once it exceeds the rendered rows (finding 4 — an earlier revision of this
+    bullet called the truncation nothing left to hint at). **The gate's home is the shared HELPER,
+    not the screen — however single-sink the screen's own function looks.** #685 got this wrong
+    twice in a row, and the second time is the instructive one: the check sat inside `runSearch`,
+    which genuinely IS the one sink both entry paths route through, so it read as correct. It was
+    still a duplicate of the helper's gate, and the two masked each other: as of `4be3f247d` —
+    which had no unit tests on the helper — deleting EITHER copy left the whole suite green, so the
+    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.
 
   - **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/api/libraryBrowse.test.ts

@@ -1,6 +1,7 @@
 import { afterEach, describe, expect, it, vi } from 'vitest';
 import {
   getLibraryBrowseItems,
+  searchLibraryBrowseItems,
   searchLibraryPickerOptions,
   titleContainsQuery,
   LIBRARY_PICKER_LUCENE_SPECIALS,
@@ -150,3 +151,53 @@ describe('searchLibraryPickerOptions (#651)', () => {
     expect(fetchMock).not.toHaveBeenCalled();
   });
 });
+
+describe('searchLibraryBrowseItems (#685 — AddItemsDialog sibling of searchLibraryPickerOptions)', () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  // The reviewer proved this helper was dead code to the suite: deleting its clamp, or deleting
+  // its gate, both left the whole suite green. These three tests mirror the ones above for
+  // searchLibraryPickerOptions so the same bound is pinned for the sibling helper.
+
+  it('#651 F4: CLAMPS an oversized pageSize rather than forwarding it', async () => {
+    const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(jsonResponse({ page: [], totalCount: 20000 }));
+
+    await searchLibraryBrowseItems('Episode', 'Alpha', 5000);
+
+    expect(browseUrl(fetchMock).searchParams.get('pageSize')).toBe(String(LIBRARY_PICKER_RESULTS));
+  });
+
+  it('issues NO request for a query below the minimum length, and resolves an empty result', async () => {
+    const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(jsonResponse({ page: [], totalCount: 0 }));
+
+    expect(await searchLibraryBrowseItems('Episode', 'a')).toEqual({ items: [], totalCount: 0 });
+    expect(await searchLibraryBrowseItems('Episode', '   ')).toEqual({ items: [], totalCount: 0 });
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
+  it('compiles/escapes the trimmed query, and returns the FULL row (mediaType present) plus totalCount', async () => {
+    const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(
+      jsonResponse({
+        page: [{ id: 1, mediaItemId: 7, mediaType: 'Movie', title: 'Show Alpha' }],
+        totalCount: 42
+      })
+    );
+
+    const result = await searchLibraryBrowseItems('Movie', '  Show Alpha  ');
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    const url = browseUrl(fetchMock);
+    expect(url.searchParams.get('query')).toBe(titleContainsQuery('Show Alpha'));
+    expect(url.searchParams.get('mediaType')).toBe('Movie');
+    expect(url.searchParams.get('pageNum')).toBe('0');
+    expect(url.searchParams.get('pageSize')).toBe(String(LIBRARY_PICKER_RESULTS));
+    // The reason this helper exists rather than reusing searchLibraryPickerOptions: the full row
+    // (mediaType included), not the {id, name} shape.
+    expect(result).toEqual({
+      items: [{ id: 1, mediaItemId: 7, mediaType: 'Movie', title: 'Show Alpha' }],
+      totalCount: 42
+    });
+  });
+});

web/src/api/libraryBrowse.ts

@@ -111,6 +111,41 @@ export function searchLibraryPickerOptions(
   );
 }
 
+export interface LibraryBrowseSearchResult {
+  items: LibraryBrowseItem[];
+  totalCount: number;
+}
+
+// Like `searchLibraryPickerOptions` above — same min-query gate, same clamp, same compiled query —
+// but for a MULTI-select caller (`CollectionsScreen`'s `AddItemsDialog`) that needs the full
+// `LibraryBrowseItem` row (mediaType + id, for `toAddItemsRequest`) rather than the `{id, name}`
+// shape a single-select `SearchPicker` renders, plus the response's `totalCount` so the caller can
+// surface how much of a match was actually returned. The gate/clamp/compile live HERE, not at the
+// call site, so no caller can accidentally skip them (§3b — "the bound belongs to the helper, not
+// the caller").
+export function searchLibraryBrowseItems(
+  mediaType: LibraryBrowseMediaType,
+  text: string,
+  pageSize: number = LIBRARY_PICKER_RESULTS
+): Promise<LibraryBrowseSearchResult> {
+  const trimmed = text.trim();
+  if (trimmed.length < LIBRARY_PICKER_MIN_QUERY) {
+    return Promise.resolve({ items: [], totalCount: 0 });
+  }
+
+  const boundedPageSize = Math.max(1, Math.min(Math.floor(pageSize), LIBRARY_PICKER_RESULTS));
+
+  return getLibraryBrowseItems({
+    mediaType,
+    pageNum: 0,
+    pageSize: boundedPageSize,
+    query: titleContainsQuery(trimmed)
+  }).then((result) => ({
+    items: result.page ?? [],
+    totalCount: result.totalCount ?? 0
+  }));
+}
+
 export function messageFromLibraryBrowseError(error: unknown, fallback = 'Unable to load library items'): string {
   if (error instanceof ApiError) {
     return error.detail ?? error.message;

web/src/api/pageSizeCallSites.guard.test.ts

@@ -83,12 +83,15 @@ import { scanPageSizeSites } from './pageSizeScan';
  *                          mechanism, and the row bound is a property of the CODE rather than of a
  *                          caller's discipline (a clamp inside the shared helper for #651's
  *                          library picker; a fixed small constant at the inline preview sites).
- *                          Nothing is list-loaded, so there is no truncation to surface and the
- *                          absence of a truncation hint is correct — which is why such a site
- *                          cannot be filed under 'class-b', whose defining evidence IS a surfaced
- *                          `totalCount`. Applies only where the query is genuinely required: a
- *                          site that degrades to an unfiltered browse when the query is empty is
- *                          NOT search-bounded (see 'deviation').
+ *                          Nothing is list-loaded, so a truncation hint is not REQUIRED here — but,
+ *                          unlike 'class-b', it is not the defining evidence either: a search-bounded
+ *                          site MAY surface its own per-kind cap (e.g. summed across kinds) once that
+ *                          count is actionable, without that hint reclassifying it as 'class-b'. What
+ *                          distinguishes the two classes is the SHAPE of the bound (one clamped
+ *                          search request per settled query vs. one bounded page of a list), not
+ *                          whether a hint is rendered. Applies only where the query is genuinely
+ *                          required: a site that degrades to an unfiltered browse when the query is
+ *                          empty is NOT search-bounded (see 'deviation').
  *   - 'class-b'         — one bounded page at (or under) the cap, with the real truncation
  *                          (`totalCount` vs items shown) surfaced to the user. Post-#651 this no
  *                          longer covers media-library pickers (those are 'search-bounded').
@@ -163,6 +166,35 @@ const REGISTRY: RegistryEntry[] = [
       'correctly absent). The bound is a clamp, not a default: Math.min(pageSize, ' +
       'LIBRARY_PICKER_RESULTS) inside the helper, so a caller cannot widen it.'
   },
+  {
+    file: 'api/libraryBrowse.ts',
+    kind: 'literal',
+    value: 'boundedPageSize',
+    classification: 'search-bounded',
+    note:
+      "searchLibraryBrowseItems — the #685 review fix that moved AddItemsDialog.runSearch's " +
+      'pageSize call site out of screens/CollectionsScreen.tsx and into this shared helper (same ' +
+      'gate/clamp/compile as searchLibraryPickerOptions above), so a multi-select caller needing ' +
+      'full LibraryBrowseItem rows plus totalCount cannot skip the bound either. A deliberate ' +
+      'second occurrence of the same (file, kind, value) identity — the multiset comparison ' +
+      'requires it be discovered twice. No request is issued below LIBRARY_PICKER_MIN_QUERY (a ' +
+      'blank form submit and a kind-chip click below the gate both resolve every kind to ' +
+      '{items: [], totalCount: 0} via the HELPER\'s own gate — CollectionsScreen no longer keeps a ' +
+      'second copy of this check; the #685 second review proved the two masked each other), the ' +
+      'typed text is compiled via titleContainsQuery rather than forwarded raw, and each kind is ' +
+      'bounded to one request per settled query at LIBRARY_PICKER_RESULTS rows. Unlike the #685 ' +
+      'first fix, this is NOT "nothing left to hint at": the per-kind cap can still truncate the ' +
+      "real match count below what totalCount reports, and AddItemsDialog now sums each kind's " +
+      "totalCount and renders a 'Showing N of M' hint when it exceeds the rendered rows. The only " +
+      'remaining client-side filter in AddItemsDialog (ADDABLE_TYPES.has(item.mediaType)) is inert, ' +
+      'not a silent drop, and this is now enforced by the type system rather than by convention on ' +
+      'BOTH ingress paths into the searched kinds — MediaKindFilter (the explicit-chip path) and ' +
+      'DEFAULT_SEARCH_KINDS (the `all` fan-out) are each derived from ADDABLE_TYPE_LIST via ' +
+      '`(typeof ADDABLE_TYPE_LIST)[number]`, so adding a non-addable kind to either is a compile ' +
+      'error. Enforcing only the first was the #685 round-3 review finding: the hint sums ' +
+      'PRE-filter totalCounts against POST-filter rows, so one unenforced ingress is enough to ' +
+      'overstate it with every row of that kind dropped.'
+  },
   {
     file: 'api/paging.ts',
     kind: 'shorthand',
@@ -246,22 +278,6 @@ const REGISTRY: RegistryEntry[] = [
       'Playout block history: forwards a user-adjustable `pageSize` state (persisted, backed by a ' +
       'page-size <Select>) to a real pager keyed off the response totalCount.'
   },
-  {
-    file: 'screens/CollectionsScreen.tsx',
-    kind: 'literal',
-    value: '50',
-    classification: 'deviation',
-    issue: 685,
-    note:
-      'TRACKED §3b VIOLATION — #685. AddItemsDialog.runSearch is reachable with an EMPTY query ' +
-      '(blank form submit, and a kind-chip click, which calls it immediately), and ' +
-      'getLibraryBrowseItems omits a falsy query — so it degrades to an unfiltered 50-row window ' +
-      'over the whole media-library type, per kind. Nothing surfaces it: totalCount is never read ' +
-      'here and no hint renders, and merged.slice(0, 50) drops up to 100 of 150 fetched rows even ' +
-      'for a real query. Under the cap, so #644 and #650 both missed it. NOT search-bounded (the ' +
-      'query is not required) and NOT class-b (no hint) — labelling it either would make this ' +
-      'registry vouch for behaviour that does not exist.'
-  },
   {
     file: 'screens/FillerPresetsScreen.tsx',
     kind: 'literal',
@@ -535,9 +551,11 @@ describe('pageSize call-site guard (#650)', () => {
   it("every 'deviation' entry names the issue tracking it", () => {
     const deviations = REGISTRY.filter((entry) => entry.classification === 'deviation');
 
-    // Anti-vacuity: if the deviations are ever all fixed, this must be deleted deliberately, not
-    // silently pass over an empty list while claiming to enforce something.
-    expect(deviations.length).toBeGreaterThan(0);
+    // No live 'deviation' entries as of #685 (the last one — CollectionsScreen.tsx's raw-50 window
+    // — was fixed and reclassified 'search-bounded' above). The anti-vacuity
+    // `expect(deviations.length).toBeGreaterThan(0)` this comment used to enforce is deleted
+    // DELIBERATELY here, per its own instruction, rather than left to silently pass over an empty
+    // list — re-add it the day a new 'deviation' entry is registered.
 
     for (const entry of deviations) {
       expect(entry.issue, `${entry.file}:${entry.value} is a deviation but names no tracking issue`).toEqual(
@@ -546,10 +564,12 @@ describe('pageSize call-site guard (#650)', () => {
       expect(entry.issue!).toBeGreaterThan(0);
     }
 
-    // The converse, so the field cannot drift into decoration: only a deviation carries one.
-    for (const entry of REGISTRY.filter((e) => e.classification !== 'deviation')) {
-      expect(entry.issue, `${entry.file}:${entry.value} is not a deviation but carries an issue`).toBeUndefined();
-    }
+    // The converse, so the field cannot drift into decoration: only a deviation carries one. With
+    // `deviations` currently empty, the loop above evaluates nothing — so this single bidirectional
+    // assertion is what actually gives the test teeth today: it fails the moment any non-deviation
+    // entry picks up an `issue` field, or a 'deviation' entry is added without one (#685 review
+    // finding 7).
+    expect(REGISTRY.filter((entry) => entry.issue !== undefined)).toEqual(deviations);
   });
 
   // Pins the report format, not the comparison key (#684 review M3): dropping the position from

web/src/screens/CollectionsScreen.test.tsx

@@ -1,5 +1,11 @@
 import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import {
+  LIBRARY_PICKER_LUCENE_SPECIALS,
+  LIBRARY_PICKER_MIN_QUERY,
+  LIBRARY_PICKER_RESULTS,
+  titleContainsQuery
+} from '../api';
 import { CollectionsScreen } from './CollectionsScreen';
 
 // The rule builder's field catalog normally comes from a live /api/v1/search/fields fetch; mock it
@@ -401,7 +407,7 @@ describe('CollectionsScreen', () => {
 
     const dialog = await openAddItemsDialog();
     fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
-      target: { value: 'a' }
+      target: { value: 'za' }
     });
     fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
 
@@ -419,7 +425,7 @@ describe('CollectionsScreen', () => {
 
     const dialog = await openAddItemsDialog();
     fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
-      target: { value: 'a' }
+      target: { value: 'za' }
     });
     fireEvent.click(within(dialog).getByRole('button', { name: 'Seasons' }));
 
@@ -451,7 +457,7 @@ describe('CollectionsScreen', () => {
 
     const dialog = await openAddItemsDialog();
     fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
-      target: { value: 'a' }
+      target: { value: 'za' }
     });
     fireEvent.click(within(dialog).getByRole('button', { name: 'Song' }));
 
@@ -475,6 +481,362 @@ describe('CollectionsScreen', () => {
     });
   });
 
+  it(
+    'backspacing the query below the min-query length after a search keeps the rows, checkmarks, and Add count (#685 review finding 1)',
+    async () => {
+      mockAddItemsApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+
+      fireEvent.click(within(dialog).getByText('Zathura'));
+      expect(within(dialog).getByRole('button', { name: /Add 1 item/ })).toBeInTheDocument();
+
+      // Backspace the LIVE input down to one character — below LIBRARY_PICKER_MIN_QUERY — WITHOUT
+      // re-submitting. Before the fix, the render condition keyed the min-query guidance to the
+      // live input alone, so this wiped the visible row list and its checkmark even though
+      // `results` and `selected` were untouched in state.
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'z' }
+      });
+
+      expect(within(dialog).getByText('Zathura')).toBeInTheDocument();
+      expect(within(dialog).getByText('Adventure Time')).toBeInTheDocument();
+      expect(
+        within(dialog).queryByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+      ).not.toBeInTheDocument();
+      const zathuraRow = within(dialog).getByText('Zathura').closest('button');
+      expect(zathuraRow).toHaveAttribute('aria-pressed', 'true');
+      expect(within(dialog).getByRole('button', { name: /Add 1 item/ })).toBeInTheDocument();
+    }
+  );
+
+  it(
+    'sums totalCount across kinds and hints at truncation once it exceeds the rendered rows (#685 review finding 4)',
+    { timeout: 15000 },
+    async () => {
+      mockLargeLibraryApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      await within(dialog).findByText('Movie Item 1');
+
+      // 3 kinds (Movie, TelevisionShow, Artist) each report a 20,000-row totalCount but only
+      // LIBRARY_PICKER_RESULTS (25) rows land per kind, so the sum (60,000) is far above the 75
+      // rows actually rendered.
+      expect(within(dialog).getByText(/Showing 75 of 60000 matches/)).toBeInTheDocument();
+    }
+  );
+
+  it('does not render a truncation hint when totalCount does not exceed the rendered rows', async () => {
+    mockAddItemsApi();
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+      target: { value: 'za' }
+    });
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+    await within(dialog).findByText('Zathura');
+
+    expect(within(dialog).queryByText(/Showing .* of .* matches/)).not.toBeInTheDocument();
+  });
+
+  it(
+    'labels the results with the query that produced them, and clearing without re-submitting keeps rows + label (#685 review)',
+    async () => {
+      mockAddItemsApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+      expect(
+        within(dialog).getByText(
+          (_content, element) => element?.tagName === 'P' && element.textContent === 'Results for “za”'
+        )
+      ).toBeInTheDocument();
+
+      // Clear the input WITHOUT re-submitting: the rows and their label must still describe the
+      // original ("za") search — nothing says which query produced an on-screen row otherwise.
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: '' }
+      });
+
+      expect(within(dialog).getByText('Zathura')).toBeInTheDocument();
+      expect(
+        within(dialog).getByText(
+          (_content, element) => element?.tagName === 'P' && element.textContent === 'Results for “za”'
+        )
+      ).toBeInTheDocument();
+    }
+  );
+
+  it(
+    'clearing the input and re-submitting shows the min-query guidance while the selection stays discoverable (#685 review)',
+    async () => {
+      mockAddItemsApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      expect(await within(dialog).findByText('Zathura')).toBeInTheDocument();
+      fireEvent.click(within(dialog).getByText('Zathura'));
+      expect(within(dialog).getByRole('button', { name: /Add 1 item/ })).toBeInTheDocument();
+
+      // Clear the input and re-submit: the helper's gate resolves every kind to
+      // { items: [], totalCount: 0 }, so the (now query-less) results are empty.
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: '' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      await waitFor(() => {
+        expect(
+          within(dialog).getByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+        ).toBeInTheDocument();
+      });
+      // `selected` persists across queries (correct for a multi-select picker) — it must never be
+      // invisible-but-counted, and the Add button's count is where that stays discoverable even
+      // though the previously-ticked row isn't in the (now empty) result set.
+      expect(within(dialog).getByRole('button', { name: /Add 1 item/ })).toBeInTheDocument();
+      // And the "Results for" heading is gone, not showing a stale/misleading query.
+      expect(within(dialog).queryByText(/^Results for /)).not.toBeInTheDocument();
+    }
+  );
+
+  it('clears the truncation hint on a failed search (#685 review)', async () => {
+    const fetchMock = mockApi({
+      onRequest: (url) => {
+        if (url.startsWith('/api/v1/library/browse')) {
+          return jsonResponse({ page: [], totalCount: 0 }, 500);
+        }
+        return null;
+      }
+    });
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+      target: { value: 'za' }
+    });
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+    await waitFor(() => {
+      expect(within(dialog).getByRole('alert')).toBeInTheDocument();
+    });
+    expect(within(dialog).queryByText(/Showing .* of .* matches/)).not.toBeInTheDocument();
+    expect(fetchMock).toHaveBeenCalled();
+
+    // A failed search must NOT fall into the min-query guidance branch: the user typed a valid
+    // 2-character query and got a 500, so telling them to "type at least 2 characters" states
+    // something false about what they just did. The error banner is the whole message (#685
+    // review round 3 — that branch conflated "nothing searched yet" with "the last search failed").
+    expect(
+      within(dialog).queryByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+    ).not.toBeInTheDocument();
+    // Nor "No results", which asserts a search that COMPLETED and found nothing. Both empty-state
+    // messages are suppressed on error, leaving the alert banner as the whole message. Pinned
+    // positively as well as negatively so a refactor cannot satisfy this by rendering nothing at
+    // all (#685 review round 4).
+    expect(within(dialog).queryByText('No results — try a search above.')).not.toBeInTheDocument();
+    expect(within(dialog).getByRole('alert')).toHaveTextContent(/500/);
+  });
+
+  /* ---------- add-items search bounds (#685) ---------- */
+
+  function buildLibraryFixture(mediaType: string, count: number): { id: number; mediaItemId: number; mediaType: string; title: string }[] {
+    return Array.from({ length: count }, (_, index) => ({
+      id: index + 1,
+      mediaItemId: index + 1,
+      mediaType,
+      title: `${mediaType} Item ${index + 1}`
+    }));
+  }
+
+  // Simulates a library with `rowsPerKind` rows of EACH media type (20,000 by default — large
+  // enough that "the client bounds itself" is the only thing keeping this fast), honoring the
+  // requested pageSize the way the real server-side clamp does (spa-conventions §3b).
+  function mockLargeLibraryApi(rowsPerKind = 20000) {
+    const fixtures = new Map<string, ReturnType<typeof buildLibraryFixture>>();
+
+    return mockApi({
+      onRequest: (url) => {
+        if (!url.startsWith('/api/v1/library/browse')) {
+          return null;
+        }
+
+        const params = new URL(url, 'http://localhost').searchParams;
+        const mediaType = params.get('mediaType') ?? 'Movie';
+        const pageSize = Number(params.get('pageSize') ?? '0');
+
+        if (!fixtures.has(mediaType)) {
+          fixtures.set(mediaType, buildLibraryFixture(mediaType, rowsPerKind));
+        }
+
+        const fixture = fixtures.get(mediaType) ?? [];
+        return jsonResponse({ page: fixture.slice(0, pageSize), totalCount: fixture.length });
+      }
+    });
+  }
+
+  function browseCallsOf(fetchMock: ReturnType<typeof mockApi>) {
+    return fetchMock.mock.calls.filter(([u]) => u.toString().startsWith('/api/v1/library/browse'));
+  }
+
+  it('issues zero requests for a blank submit, and renders the min-query guidance', async () => {
+    const fetchMock = mockLargeLibraryApi();
+
+    const dialog = await openAddItemsDialog();
+
+    expect(
+      within(dialog).getByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+    ).toBeInTheDocument();
+
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+    // No await-for-settle race: a blank submit never issues a fetch, so there is nothing async
+    // to wait on — assert immediately that the guidance is still the only thing shown.
+    expect(
+      within(dialog).getByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+    ).toBeInTheDocument();
+    expect(browseCallsOf(fetchMock).length).toBe(0);
+  });
+
+  it(
+    `a ${LIBRARY_PICKER_MIN_QUERY - 1}-character submit (one below the minimum) issues zero requests`,
+    async () => {
+      // §3b: unit-test the INCLUSIVE endpoint directly — existing coverage only exercised 0 chars
+      // (blank submit) and a valid 2-char query, so a `>` for `>=` slip in the gate would have
+      // passed the whole suite (#685 review finding 3).
+      const fetchMock = mockLargeLibraryApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'z' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      expect(
+        within(dialog).getByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+      ).toBeInTheDocument();
+      expect(browseCallsOf(fetchMock).length).toBe(0);
+    }
+  );
+
+  it('a kind-chip click below the min-query gate issues zero requests', async () => {
+    const fetchMock = mockLargeLibraryApi();
+
+    const dialog = await openAddItemsDialog();
+    fireEvent.click(within(dialog).getByRole('button', { name: 'Song' }));
+
+    expect(
+      within(dialog).getByText(`Type at least ${LIBRARY_PICKER_MIN_QUERY} characters to search.`)
+    ).toBeInTheDocument();
+    expect(browseCallsOf(fetchMock).length).toBe(0);
+  });
+
+  it(
+    'a valid query issues exactly one request per kind, bounded to LIBRARY_PICKER_RESULTS, with the compiled query',
+    { timeout: 15000 },
+    async () => {
+      const fetchMock = mockLargeLibraryApi();
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      await within(dialog).findByText('Movie Item 1');
+
+      const browseCalls = browseCallsOf(fetchMock);
+      // DEFAULT_SEARCH_KINDS fan-out: Movie, TelevisionShow, Artist — exactly one request each,
+      // never more.
+      expect(browseCalls.length).toBe(3);
+
+      const kindsRequested = browseCalls.map(
+        ([u]) => new URL(u.toString(), 'http://localhost').searchParams.get('mediaType')
+      );
+      expect(new Set(kindsRequested)).toEqual(new Set(['Movie', 'TelevisionShow', 'Artist']));
+
+      for (const [u] of browseCalls) {
+        const params = new URL(u.toString(), 'http://localhost').searchParams;
+        expect(params.get('pageSize')).toBe(String(LIBRARY_PICKER_RESULTS));
+        expect(params.get('query')).toBe(titleContainsQuery('za'));
+      }
+    }
+  );
+
+  it(
+    'compiles typed text and escapes a Lucene special character rather than forwarding it raw',
+    { timeout: 15000 },
+    async () => {
+      const fetchMock = mockLargeLibraryApi();
+      // This is a smoke check that the compiled query reaches the wire intact for ONE special
+      // character — it does not need to be exhaustive, because the exhaustive per-character case
+      // (every character in LIBRARY_PICKER_LUCENE_SPECIALS) already lives in
+      // web/src/api/libraryBrowse.test.ts, against titleContainsQuery directly. Picking element 0
+      // here is an arbitrary representative, not a hand-copied "every special" sample (#685 review
+      // finding 6).
+      const special = LIBRARY_PICKER_LUCENE_SPECIALS[0];
+      const typed = `ro${special}ck`;
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: typed }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      await within(dialog).findByText('Movie Item 1');
+
+      const browseCall = browseCallsOf(fetchMock)[0];
+      const params = new URL(String(browseCall?.[0]), 'http://localhost').searchParams;
+      const sentQuery = params.get('query');
+
+      expect(sentQuery).toBe(titleContainsQuery(typed));
+      // A raw, unescaped forward would still contain the bare special character; the compiled
+      // form never does (it is backslash-escaped instead).
+      expect(sentQuery).not.toBe(`title:*${typed}*`);
+      expect(sentQuery).toContain(`\\${special}`);
+    }
+  );
+
+  it(
+    'renders every merged row across all kinds — no post-fetch drop',
+    { timeout: 15000 },
+    async () => {
+      mockLargeLibraryApi(LIBRARY_PICKER_RESULTS);
+
+      const dialog = await openAddItemsDialog();
+      fireEvent.change(within(dialog).getByPlaceholderText('Search movies, shows, seasons, artists…'), {
+        target: { value: 'za' }
+      });
+      fireEvent.click(within(dialog).getByRole('button', { name: 'Search' }));
+
+      await within(dialog).findByText('Movie Item 1');
+
+      // Each of the 3 default-fan-out kinds returns a full LIBRARY_PICKER_RESULTS-row page;
+      // nothing fetched should be discarded, so all of them must render.
+      const titleEls = dialog.querySelectorAll('.ctv-collections-picker-row-title');
+      expect(titleEls.length).toBe(3 * LIBRARY_PICKER_RESULTS);
+    }
+  );
+
   /* ---------- reorder mode (#211) ---------- */
 
   function reorderPages(id: number) {

web/src/screens/CollectionsScreen.tsx

@@ -35,10 +35,11 @@ import {
   getCollectionItems,
   getCollectionItemsWithMeta,
   getCollections,
-  getLibraryBrowseItems,
   getSmartCollections,
+  LIBRARY_PICKER_MIN_QUERY,
   messageFromCollectionError,
   removeItemFromCollection,
+  searchLibraryBrowseItems,
   toAddItemsRequest,
   updateCollection,
   updateCollectionCustomOrder,
@@ -52,8 +53,13 @@ import { SmartCollectionDialog } from '../builder/SmartCollectionDialog';
 
 type Tab = 'manual' | 'smart';
 
-// Every kind that can be added to a manual collection from the picker.
-const ADDABLE_TYPE_LIST: LibraryBrowseItem['mediaType'][] = [
+// Every kind that can be added to a manual collection from the picker. `as const satisfies`
+// keeps this a literal-string tuple (rather than widening to the whole `mediaType` union) while
+// still checking each entry against it, so `MediaKindFilter` below can be DERIVED from this list:
+// adding a non-addable kind to `MEDIA_KIND_FILTERS` becomes a compile error instead of a
+// convention nothing enforces (#685 review — the sum-of-totalCount hint is only correct because
+// every filterable kind is addable, and this makes that structural rather than a comment).
+const ADDABLE_TYPE_LIST = [
   'Movie',
   'TelevisionShow',
   'TelevisionSeason',
@@ -64,15 +70,26 @@ const ADDABLE_TYPE_LIST: LibraryBrowseItem['mediaType'][] = [
   'OtherVideo',
   'Image',
   'RemoteStream'
-];
+] as const satisfies readonly LibraryBrowseItem['mediaType'][];
 const ADDABLE_TYPES = new Set<LibraryBrowseItem['mediaType']>(ADDABLE_TYPE_LIST);
 
 // The default fan-out excludes seasons so a multi-season show doesn't flood the
 // results with per-season rows (issue #180); seasons (and the other narrower kinds
 // added for #211) stay reachable via the explicit media-kind filter below.
-const DEFAULT_SEARCH_KINDS: LibraryBrowseItem['mediaType'][] = ['Movie', 'TelevisionShow', 'Artist'];
+// Derived from ADDABLE_TYPE_LIST for the same reason MediaKindFilter is: this is the SECOND
+// ingress into `kinds` (the `all` branch of runSearch), and the truncation hint sums pre-filter
+// totalCounts against post-ADDABLE_TYPES rows. A non-addable kind here would overstate the hint
+// with every one of its rows dropped, and typing it as the whole mediaType union let that through
+// (#685 review round 3 — one of two ingress paths was enforced, which is not an invariant).
+const DEFAULT_SEARCH_KINDS: AddableKind[] = ['Movie', 'TelevisionShow', 'Artist'];
 
-type MediaKindFilter = 'all' | LibraryBrowseItem['mediaType'];
+// The single derivation every ingress into the searched kinds goes through. Named rather than
+// spelled inline at each use so the "one list, all ingresses" property is visible at a glance —
+// a fourth ingress is most likely to be written by copying one of the existing three (#685
+// review round 4).
+type AddableKind = (typeof ADDABLE_TYPE_LIST)[number];
+
+type MediaKindFilter = 'all' | AddableKind;
 
 const MEDIA_KIND_FILTERS: { label: string; value: MediaKindFilter }[] = [
   { label: 'All', value: 'all' },
@@ -217,34 +234,64 @@ function AddItemsDialog({
 }) {
   const [query, setQuery] = useState('');
   const [results, setResults] = useState<LibraryBrowseItem[]>([]);
+  const [totalMatches, setTotalMatches] = useState(0);
+  // The query a *settled* search actually ran for — never the live input. Rendered as a heading
+  // over the results so a submitted-then-cleared input, or a selection made under an earlier
+  // query, is never shown without saying which query produced it (#685 review). Reset to '' when
+  // a search settles below LIBRARY_PICKER_MIN_QUERY (nothing was actually searched) or errors.
+  const [submittedQuery, setSubmittedQuery] = useState('');
   const [searching, setSearching] = useState(false);
   const [selected, setSelected] = useState<Map<string, LibraryBrowseItem>>(() => new Map());
   const [error, setError] = useState<string | null>(null);
   const [adding, setAdding] = useState(false);
   const [kindFilter, setKindFilter] = useState<MediaKindFilter>('all');
 
+  // `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").
+  // This screen used to keep a SECOND copy of the min-query check here — the #685 review proved
+  // 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.
   const runSearch = async (filter: MediaKindFilter = kindFilter) => {
+    const trimmed = query.trim();
+
     setSearching(true);
     setError(null);
 
     try {
-      const trimmed = query.trim();
       const kinds = filter === 'all' ? DEFAULT_SEARCH_KINDS : [filter];
-      const perKind = await Promise.all(
-        kinds.map((mediaType) => getLibraryBrowseItems({ pageSize: 50, query: trimmed, mediaType }))
-      );
+      const perKind = await Promise.all(kinds.map((mediaType) => searchLibraryBrowseItems(mediaType, trimmed)));
       const merged = perKind
-        .flatMap((result) => result.page ?? [])
+        .flatMap((result) => result.items)
         .filter((item) => ADDABLE_TYPES.has(item.mediaType))
         .sort((left, right) => left.title.localeCompare(right.title, undefined, { sensitivity: 'base' }));
-      setResults(merged.slice(0, 50));
+      // No slice: each kind is already bounded to LIBRARY_PICKER_RESULTS server-side by the helper,
+      // so nothing fetched is discarded here (#685 — merged.slice(0, 50) used to drop up to 100 of
+      // 150 rows). The per-kind cap can still truncate the real match count, though — surfaced
+      // below via totalMatches (#685 review finding 4).
+      setResults(merged);
+      setTotalMatches(perKind.reduce((sum, result) => sum + result.totalCount, 0));
+      // Below LIBRARY_PICKER_MIN_QUERY, the helper's own gate resolves every kind to
+      // { items: [], totalCount: 0 } without issuing a request — so `merged`/`totalMatches` above
+      // are already empty in that case, but `submittedQuery` must say "nothing was actually
+      // searched" rather than name the sub-gate text.
+      setSubmittedQuery(trimmed.length < LIBRARY_PICKER_MIN_QUERY ? '' : trimmed);
     } catch (searchError) {
       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).
+      setResults([]);
+      setTotalMatches(0);
+      setSubmittedQuery('');
     } finally {
       setSearching(false);
     }
   };
 
+  // A kind-chip click is the second path into runSearch (a blank form submit is the other), and it
+  // is deliberately NOT gated here: runSearch is the single sink and owns the min-query gate for
+  // every caller via searchLibraryBrowseItems. §3b is explicit that an unreachable guard is an
+  // untested one — the next path added would be the one nobody remembers to gate.
   const selectKindFilter = (filter: MediaKindFilter) => {
     setKindFilter(filter);
     void runSearch(filter);
@@ -341,13 +388,44 @@ function AddItemsDialog({
         &ldquo;All&rdquo; searches movies, shows and artists; pick a specific kind to add seasons, episodes,
         music videos, songs, other videos, images or remote streams.
       </p>
+      {/* Conditionally mounted, unlike the aria-live hint below — deliberately, not by oversight:
+          `role="alert"` is the one live-region role screen readers reliably announce on INSERTION,
+          so mounting it together with its content is right here and would be wrong there (#685
+          review round 4 flagged the divergence as unexplained). */}
       {error && (
         <span className="ctv-field-error" role="alert">
           {error}
         </span>
       )}
+      {submittedQuery !== '' && (
+        // Names the query the CURRENT results (or the empty-results/"no matches" state) belong
+        // to — never the live input, so a submitted-then-cleared query, or a selection made under
+        // an earlier query, is never shown without saying which search produced it (#685 review).
+        <p className="ctv-collections-picker-note">
+          Results for &ldquo;{submittedQuery}&rdquo;
+        </p>
+      )}
+      {/* The live region is mounted UNCONDITIONALLY with the condition inside it: most screen
+          readers only announce mutations to a region that already existed, so creating the region
+          and its text in the same commit announces nothing (#685 review round 3). */}
+      <p aria-live="polite" className="ctv-collections-picker-note">
+        {totalMatches > results.length ? `Showing ${results.length} of ${totalMatches} matches — narrow your search.` : ''}
+      </p>
       <div className="ctv-collections-picker-results">
-        {results.length === 0 && !searching ? (
+        {results.length === 0 && submittedQuery === '' && !error ? (
+          // Keyed to submittedQuery, not the live input: without it, backspacing the query back
+          // below the min-query length after a successful search wiped the rendered rows AND their
+          // checkmarks while `selected` (and the Add button's count) still held them (#685 review
+          // finding 1) — and, separately, submitting a blank/cleared query after a real selection
+          // must still say the selection persists rather than rendering as if nothing was ever
+          // searched (#685 second review).
+          <div className="ctv-collections-picker-empty">
+            Type at least {LIBRARY_PICKER_MIN_QUERY} characters to search.
+          </div>
+        ) : results.length === 0 && !searching && !error ? (
+          // Also suppressed on `error`: "No results" asserts a search that COMPLETED and found
+          // nothing, which is false when the request failed. The role="alert" banner above is the
+          // whole message in that state (#685 review round 4).
           <div className="ctv-collections-picker-empty">No results — try a search above.</div>
         ) : (
           results.map((item) => {