From 2c086345cdae7b1b8f26784d21eecb82d94326e9 Mon Sep 17 00:00:00 2001 From: blurbery <271863866+blurbery@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:24:20 +1000 Subject: [PATCH] fix(search): restore narrow title lookups --- internal/catalog/item_repo_test.go | 32 ++++++++++ internal/catalog/search_postgres_mixed.go | 10 +++ web/src/components/GlobalSearch.test.tsx | 42 +++++++++++- web/src/components/GlobalSearch.tsx | 1 + web/src/components/RequestPosterCard.test.tsx | 19 ++++++ web/src/components/RequestPosterCard.tsx | 10 ++- .../components/RequestToAddSection.test.tsx | 22 ++++++- web/src/components/RequestToAddSection.tsx | 64 +++++++++++++++++-- web/src/hooks/queries/catalog.ts | 1 + web/src/lib/mediaRequests.ts | 2 +- web/src/pages/Catalog.test.tsx | 30 ++++++++- web/src/pages/Catalog.tsx | 7 +- 12 files changed, 225 insertions(+), 15 deletions(-) diff --git a/internal/catalog/item_repo_test.go b/internal/catalog/item_repo_test.go index 790c6bfdf..b24fa83de 100644 --- a/internal/catalog/item_repo_test.go +++ b/internal/catalog/item_repo_test.go @@ -527,6 +527,38 @@ func TestItemRepo_Search_ShortFinalTokenUsesLeadingTitleIndexes(t *testing.T) { } } +// TestItemRepo_Search_NarrowTitlePathTypesSearchTextParameter guards a +// prepare-time failure that only appeared after the user finished a short +// final token: "breaking" used the regular FTS path, while "Breaking Bad" +// suppressed the overview arm and therefore left the still-bound $1 without a +// PostgreSQL type (SQLSTATE 42P18). Every physical source and both page/count +// statements must carry the explicit, parameter-only type guard. +func TestItemRepo_Search_NarrowTitlePathTypesSearchTextParameter(t *testing.T) { + repo := &ItemRepository{} + for _, test := range []struct { + name string + query string + itemTypes []string + }{ + {name: "mixed multiword", query: "Breaking Bad"}, + {name: "media multiword", query: "Breaking Bad", itemTypes: []string{"movie", "series"}}, + {name: "episode multiword", query: "Who Are You?", itemTypes: []string{"episode"}}, + {name: "mixed short title", query: "Up"}, + } { + t.Run(test.name, func(t *testing.T) { + dataSQL, countSQL, args := repo.buildSearchSQLWithTotal(test.query, test.itemTypes, 20, 0, AccessFilter{}, true) + if len(args) < 2 || args[0] != test.query { + t.Fatalf("unexpected fixed search arguments: %#v", args) + } + for _, sql := range []string{dataSQL, countSQL} { + if !strings.Contains(sql, "$1::text IS NOT NULL") { + t.Fatalf("narrow search must type bound $1 in both statements; got:\n%s", sql) + } + } + }) + } +} + // TestItemRepo_BuildFuzzySearchSQL asserts that the fuzzy fallback query scores // only on indexed normalized title/alias columns (no title tsvector rebuild), // matches via strict word similarity so long titles stay reachable, ranks by diff --git a/internal/catalog/search_postgres_mixed.go b/internal/catalog/search_postgres_mixed.go index 9ff908897..79ea8581b 100644 --- a/internal/catalog/search_postgres_mixed.go +++ b/internal/catalog/search_postgres_mixed.go @@ -276,6 +276,16 @@ func (r *ItemRepository) buildMixedSearchSQLFromParsed( } scoredCTE := "WITH scored AS (\n" + scoredBody + "\n)" postFilter := `FROM scored` + if narrowTitleLookup { + // Narrow title searches intentionally skip the overview branch. That + // branch is normally what gives $1 (searchText) its PostgreSQL type; + // without it, queries such as "Breaking Bad" reference $2 and later + // placeholders but fail at parse time with SQLSTATE 42P18 because $1 is + // untyped. This parameter-only guard is always true for a built search + // (empty input returned above), types $1 explicitly, and is planned as a + // one-time filter without widening either indexed title lookup. + postFilter += ` WHERE $1::text IS NOT NULL` + } pageTotalColumn := "" finalTotalColumn := "" diff --git a/web/src/components/GlobalSearch.test.tsx b/web/src/components/GlobalSearch.test.tsx index 6c9f67b82..b53933822 100644 --- a/web/src/components/GlobalSearch.test.tsx +++ b/web/src/components/GlobalSearch.test.tsx @@ -43,13 +43,15 @@ vi.mock("@/components/RequestToAddSection", () => ({ variant, query, libraryHadHits, + libraryResultsKnown, }: { variant: string; query: string; libraryHadHits: boolean; + libraryResultsKnown?: boolean; }) => (
- {`variant="${variant}" query="${query}" libraryHadHits="${String(libraryHadHits)}"`} + {`variant="${variant}" query="${query}" libraryHadHits="${String(libraryHadHits)}" libraryResultsKnown="${String(libraryResultsKnown)}"`}
), })); @@ -428,6 +430,7 @@ describe("GlobalSearch + RequestToAddSection wiring", () => { expect(markup).toContain('data-testid="request-section"'); expect(markup).toContain("libraryHadHits="true""); + expect(markup).toContain("libraryResultsKnown="true""); expect(markup).toContain("variant="dialog""); }); @@ -463,6 +466,43 @@ describe("GlobalSearch + RequestToAddSection wiring", () => { const markup = renderSearchMarkup({ defaultOpen: true, initialQuery: "ThisDoesNotExist" }); expect(markup).toContain("libraryHadHits="false""); + expect(markup).toContain("libraryResultsKnown="true""); + }); + + it("marks library results unknown while the local preview is still pending", () => { + mocks.useCanRequest.mockReturnValue({ + discoveryEnabled: true, + isResolving: false, + submitDisabledReason: null, + }); + mocks.useQuery.mockReturnValue({ + data: undefined, + isFetching: true, + isError: false, + }); + mocks.useRequestSearch.mockReturnValue({ + data: { + page: 1, + total_pages: 1, + total_results: 1, + results: [ + { + media_type: "movie", + tmdb_id: 1, + title: "X", + availability: "missing", + request: { requestable: true }, + }, + ], + }, + isLoading: false, + isError: false, + }); + + const markup = renderSearchMarkup({ defaultOpen: true, initialQuery: "Dune" }); + + expect(markup).toContain("libraryHadHits="false""); + expect(markup).toContain("libraryResultsKnown="false""); }); it("does not call useRequestSearch with enabled=true when discoveryEnabled is false", () => { diff --git a/web/src/components/GlobalSearch.tsx b/web/src/components/GlobalSearch.tsx index 248c33d55..fa5f13807 100644 --- a/web/src/components/GlobalSearch.tsx +++ b/web/src/components/GlobalSearch.tsx @@ -374,6 +374,7 @@ export function GlobalSearch({ variant="dialog" query={tmdbDebouncedQuery} libraryHadHits={items.length > 0} + libraryResultsKnown={!previewQuery.isFetching && !previewQuery.isError} /> )} diff --git a/web/src/components/RequestPosterCard.test.tsx b/web/src/components/RequestPosterCard.test.tsx index e84656599..41cc63b5d 100644 --- a/web/src/components/RequestPosterCard.test.tsx +++ b/web/src/components/RequestPosterCard.test.tsx @@ -59,4 +59,23 @@ describe("RequestPosterCard (discover variant)", () => { // its absence is the strongest signal that the button was suppressed. expect(markup).not.toContain(" { + const movieMarkup = renderToStaticMarkup( + + + , + ); + const seriesMarkup = renderToStaticMarkup( + + + , + ); + + expect(movieMarkup).toContain(">Movie<"); + expect(seriesMarkup).toContain(">Series<"); + }); }); diff --git a/web/src/components/RequestPosterCard.tsx b/web/src/components/RequestPosterCard.tsx index a518bbe0e..46b48a613 100644 --- a/web/src/components/RequestPosterCard.tsx +++ b/web/src/components/RequestPosterCard.tsx @@ -322,8 +322,16 @@ function CardMeta({ {hasMeta && (
{mediaType && ( - + <> + + {mediaType === "series" ? "Series" : "Movie"} + )} + {mediaType && year ? ( + + · + + ) : null} {year ? {year} : null} {(year || mediaType) && rating ? ( diff --git a/web/src/components/RequestToAddSection.test.tsx b/web/src/components/RequestToAddSection.test.tsx index 224dc3542..fbf372aa1 100644 --- a/web/src/components/RequestToAddSection.test.tsx +++ b/web/src/components/RequestToAddSection.test.tsx @@ -152,6 +152,24 @@ describe("RequestToAddSection (dialog variant)", () => { expect(markup).not.toContain("Request to Add"); }); + it("does not claim media is absent while the local lookup is unresolved or failed", () => { + mocks.useRequestSearch.mockReturnValue({ + data: { page: 1, total_pages: 1, total_results: 1, results: [missingResult()] }, + isLoading: false, + isError: false, + }); + const markup = render( + , + ); + expect(markup).toContain("Discovery matches:"); + expect(markup).not.toContain("Not in your library"); + }); + it("filters out results already available in the library", () => { // missingResult has tmdb_id 1, availableResult has tmdb_id 2. The DialogRow // renders item.title only as text content (never as a `title=` attribute), so @@ -237,8 +255,8 @@ describe("RequestToAddSection (dialog variant)", () => { expect(markup).toContain("Quota Capped Movie"); expect(markup).not.toContain("bg-amber-400/15"); - expect(markup).toContain("Limit reached"); - expect(markup).toContain('title="Limit reached"'); + expect(markup).toContain("Request limit reached"); + expect(markup).toContain('title="Request limit reached"'); }); it("prefers request status over reason when a row is already requested", () => { diff --git a/web/src/components/RequestToAddSection.tsx b/web/src/components/RequestToAddSection.tsx index 9fd26d9e7..ee04cb2b4 100644 --- a/web/src/components/RequestToAddSection.tsx +++ b/web/src/components/RequestToAddSection.tsx @@ -36,9 +36,19 @@ export type RequestToAddSectionProps = { query: string; /** True when the library search returned at least one hit. Drives header copy. */ libraryHadHits: boolean; + /** + * True only after the matching local-library query completed successfully. + * Loading and failed searches must not be presented as confirmed absences. + */ + libraryResultsKnown?: boolean; }; -export function RequestToAddSection({ variant, query, libraryHadHits }: RequestToAddSectionProps) { +export function RequestToAddSection({ + variant, + query, + libraryHadHits, + libraryResultsKnown = true, +}: RequestToAddSectionProps) { const { discoveryEnabled } = useCanRequest(); const search = useRequestSearch("all", query, 1, { enabled: discoveryEnabled, @@ -58,12 +68,32 @@ export function RequestToAddSection({ variant, query, libraryHadHits }: RequestT const visible = filtered.slice(0, limit); if (variant === "dialog") { - return ; + return ( + + ); } - return ; + return ( + + ); } -function HeaderCopy({ libraryHadHits, count }: { libraryHadHits: boolean; count: number }) { +function HeaderCopy({ + libraryHadHits, + libraryResultsKnown, + count, +}: { + libraryHadHits: boolean; + libraryResultsKnown: boolean; + count: number; +}) { if (libraryHadHits) { return (
@@ -75,6 +105,10 @@ function HeaderCopy({ libraryHadHits, count }: { libraryHadHits: boolean; count: ); } + if (!libraryResultsKnown) { + return
Discovery matches:
; + } + return (
Not in your library, but you can request: @@ -85,13 +119,19 @@ function HeaderCopy({ libraryHadHits, count }: { libraryHadHits: boolean; count: function DialogVariant({ items, libraryHadHits, + libraryResultsKnown, }: { items: RequestMediaResult[]; libraryHadHits: boolean; + libraryResultsKnown: boolean; }) { return (
- +
    {items.map((item) => (
  • @@ -154,9 +194,11 @@ function DialogRow({ item }: { item: RequestMediaResult }) { function GridVariant({ items, libraryHadHits, + libraryResultsKnown, }: { items: RequestMediaResult[]; libraryHadHits: boolean; + libraryResultsKnown: boolean; }) { const count = items.length; const createRequest = useCreateMediaRequest(); @@ -202,11 +244,19 @@ function GridVariant({
    - {libraryHadHits ? "Discover · Outside your library" : "Outside your library"} + {libraryHadHits + ? "Discover · Outside your library" + : libraryResultsKnown + ? "Outside your library" + : "Discovery"}

    - {libraryHadHits ? "Request to Add" : "Not in your library, but you can request"} + {libraryHadHits + ? "Request to Add" + : libraryResultsKnown + ? "Not in your library, but you can request" + : "More search matches"}

diff --git a/web/src/hooks/queries/catalog.ts b/web/src/hooks/queries/catalog.ts index 024979514..7be398fe6 100644 --- a/web/src/hooks/queries/catalog.ts +++ b/web/src/hooks/queries/catalog.ts @@ -332,6 +332,7 @@ export function useCatalogWindow( }, isLoading, isError: page0Result.isError, + isPlaceholderData: page0Result.isPlaceholderData, error: page0Result.error, refetch: page0Result.refetch, }; diff --git a/web/src/lib/mediaRequests.ts b/web/src/lib/mediaRequests.ts index 1a7a686a3..58fe29c11 100644 --- a/web/src/lib/mediaRequests.ts +++ b/web/src/lib/mediaRequests.ts @@ -98,7 +98,7 @@ export function formatRequestReason(reason?: string): string { case "blocked": return "Blocked"; case "quota_exceeded": - return "Limit reached"; + return "Request limit reached"; default: return "Unavailable"; } diff --git a/web/src/pages/Catalog.test.tsx b/web/src/pages/Catalog.test.tsx index 8dc9eb5f0..c6ecdf964 100644 --- a/web/src/pages/Catalog.test.tsx +++ b/web/src/pages/Catalog.test.tsx @@ -53,13 +53,15 @@ vi.mock("@/components/RequestToAddSection", () => ({ variant, query, libraryHadHits, + libraryResultsKnown, }: { variant: string; query: string; libraryHadHits: boolean; + libraryResultsKnown?: boolean; }) => (
- {`variant="${variant}" query="${query}" libraryHadHits="${String(libraryHadHits)}"`} + {`variant="${variant}" query="${query}" libraryHadHits="${String(libraryHadHits)}" libraryResultsKnown="${String(libraryResultsKnown)}"`}
), })); @@ -388,6 +390,7 @@ describe("Catalog page", () => { expect(markup).toContain('data-testid="request-section"'); expect(markup).toContain("variant="grid""); expect(markup).toContain("libraryHadHits="true""); + expect(markup).toContain("libraryResultsKnown="true""); }); it("renders the request grid variant with libraryHadHits=false when library has 0 hits", () => { @@ -426,6 +429,31 @@ describe("Catalog page", () => { ); expect(markup).toContain("libraryHadHits="false""); + expect(markup).toContain("libraryResultsKnown="true""); + }); + + it("marks library results unknown when the local search failed", () => { + mockUseCanRequest.mockReturnValue({ + discoveryEnabled: true, + isResolving: false, + submitDisabledReason: null, + }); + mockUseCatalogWindow.mockReturnValue({ + data: { title: 'Results for "heat"', totalItems: 0, pages: new Map() }, + isLoading: false, + isError: true, + isPlaceholderData: false, + refetch: vi.fn(), + }); + + const markup = renderToStaticMarkup( + + + , + ); + + expect(markup).toContain("libraryHadHits="false""); + expect(markup).toContain("libraryResultsKnown="false""); }); it("does not render the request section when source is not query", () => { diff --git a/web/src/pages/Catalog.tsx b/web/src/pages/Catalog.tsx index 9eb9db947..fcf46fc49 100644 --- a/web/src/pages/Catalog.tsx +++ b/web/src/pages/Catalog.tsx @@ -238,8 +238,10 @@ function CatalogResults({ }); const tmdbMissingCount = tmdbQuery.data?.results?.filter((result) => result.availability !== "available").length ?? 0; - const libraryHasResults = (catalogQuery.data?.totalItems ?? 0) > 0; - const libraryEmpty = !catalogQuery.isLoading && !libraryHasResults; + const libraryResultsKnown = + !catalogQuery.isLoading && !catalogQuery.isPlaceholderData && !catalogQuery.isError; + const libraryHasResults = libraryResultsKnown && (catalogQuery.data?.totalItems ?? 0) > 0; + const libraryEmpty = libraryResultsKnown && !libraryHasResults; // When the library is empty and the request section will (or might) render, // hide ItemGrid entirely. The previous approach pinned ItemGrid's `loading` // prop to true, which renders 24 skeleton tiles forever above the section. @@ -453,6 +455,7 @@ function CatalogResults({ variant="grid" query={tmdbDebouncedQ} libraryHadHits={libraryHasResults} + libraryResultsKnown={libraryResultsKnown} /> ) : null}