From b96e359b4ebe3e6aea68ce327d180578bf0b04d0 Mon Sep 17 00:00:00 2001 From: CoffeeKnyte <67730400+CoffeeKnyte@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:56:07 +0800 Subject: [PATCH] feat(catalog): typo-tolerant Postgres search fallback (did-you-mean) (#386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(catalog): typo-tolerant search for the Postgres (non-Meilisearch) path ## What this does (plain language) When someone searches the library and misspells a title — "intersteller", "godfathr", "jurasic" — the Postgres-backed search used to return nothing, because it only did exact full-text matching. This adds a "did you mean" fallback: when the normal search finds little or nothing, we run a second, typo-tolerant lookup and surface the closest titles. This only affects deployments that search via Postgres (the fallback path). Meilisearch already does its own typo tolerance and is left untouched. ## Why not just make the main query fuzzy The obvious approach — OR a trigram similarity match into the main search — is a performance trap. The trigram operator is "lossy", so Postgres re-checks every near-miss candidate by rebuilding three title search-vectors per row. On a real library that turned routine searches into multi-second queries. Measured on a 175k-title dev database: - exact full-text only: ~60 ms - fuzzy OR'd into the main query: ~217 ms (and far worse on prod-sized data) ## How it works The fuzzy arm is a completely separate query (buildFuzzySearchSQL). It matches only on the trigram-indexed title_normalized column and ranks only by similarity() on that same column — it never touches the title search-vectors, so it pays no per-row rebuild. It runs only when the exact search is "sparse" (fewer than 5 hits) and the query is long enough for the trigram index to help (>= 4 characters), so the common case stays on the fast exact path. It is wired into SearchPage (not just the thin Search wrapper) so the catalog search provider benefits too. ## Measured on the live 183k-title catalog (read-only EXPLAIN ANALYZE) - exact query for a typo: ~0.8 ms (0 hits -> triggers the fallback) - fuzzy fallback query: ~5-27 ms, always via the trigram index, with no search-vector rebuild - "intersteller" -> Interstellar (similarity 0.63) - "godfathr" -> GodFather (0.58), The Godfather - "breakin" (134 exact hits) -> fuzzy correctly does NOT fire Shared scope predicates (type / library / access / manga-exclusion) are extracted into appendSearchScopeFilters so the exact and fuzzy queries filter identically. Adapted from the earlier feat/search-fuzzy-fallback prototype onto main's current SearchPage / includeTotal architecture. * refactor(catalog): correct fuzzy-search pagination and parse the query once Follow-up to the fuzzy fallback, from an adversarial code review. Two things: a pagination correctness fix and a small performance/readability cleanup. Both were validated against the live 183k-title catalog. ## The pagination bug (plain language) Fuzzy results are shown after the exact results, as one combined list. The first version stitched that list together with page-offset math, and got the math wrong past the first page: - the reported result count grew as you paged (page 1 said "31 results", page 2 said "33"); - titles shown on page 1 could reappear on page 2; - paging far past the end still ran the (pointless) fuzzy query every time; - a tiny page size (e.g. an autocomplete asking for 3) could hide the fuzzy results behind a page the client was told did not exist. ## The fix Because the fuzzy fallback only runs when exact results are sparse (< 5) and the fuzzy part is capped at 50, the whole combined list is tiny. So instead of fragile per-page offset math, we now fetch that small combined list once and take the requested slice in memory. Every page is then correct by construction: stable total, no repeats, no wasted work past the end. Before -> after, typo search "intersteller" (21 results, page size 5): - total reported on page 2: 31 then 33 (drifting) -> 21 (stable) - repeated titles across pages: yes -> none - request past the end (offset 500): 2-3 DB queries -> 0 extra queries - autocomplete (page size 1): fuzzy hidden -> paginates correctly Cursor-style callers (that don't ask for a total) can't locate the boundary between the two blocks on a later page, so they now get the fuzzy results as a single terminal first page — no misleading "more results" flag. ## The cleanup The raw query string was being parsed three times per search (once for the eligibility check, once in each SQL builder). It is now parsed once in SearchPage and passed down; the shared search-text derivation is extracted so the two builders can't disagree; and the normalized form the eligibility gate needs is precomputed at parse time. ("Performance first", per the repo guidelines.) Also considered and rejected: excluding exact hits from the fuzzy query with a NOT(full-text) clause instead of by id. It reintroduced the search-vector rebuild the whole design avoids — measured ~51 ms vs ~20 ms on the worst case — so id-based exclusion stayed. Known limitation: the fuzzy path re-reads the small exact block in a second query, so a title written in the sub-millisecond gap between the two reads could be missed until the next search. Harmless and inherent to a multi-query design. * fix(catalog): close fuzzy-search library-scope leak and restore small-limit cursor recall Addresses two findings from the PR #386 review bots. ## Library-scope leak (Codex P1) The search scope helper shared by the FTS query and the trigram fuzzy fallback filtered libraries with `JOIN media_item_libraries mil` + `NOT (mil.media_folder_id = ANY($disabled))`. An item linked to BOTH a disabled and a non-disabled library fans out to two joined rows; the non-disabled row satisfies the deny check, GROUP BY collapses the item back, and it surfaces in search results despite the disabled library. Because the new fuzzy fallback reuses this helper, typo searches could leak disabled-library items too. appendSearchScopeFilters now delegates to the leak-safe appendLibraryAccessConditions (access_filter.go), which emits item-scoped EXISTS/NOT EXISTS subqueries — the same form GetByIDs/EnsureAccessible already use — and needs no membership JOIN. The disabled-only path keeps its argument-free positive-membership EXISTS so orphan items don't slip through a vacuous NOT EXISTS. The scored CTEs keep GROUP BY (now required only for the MAX() ranking aggregates). New regression test pins the EXISTS/NOT EXISTS shape and the absence of a JOIN for both the FTS and fuzzy builders. ## Small-limit cursor recall (Codex P2) In cursor mode (include_total=false) the FTS probe fetched only limit+1 rows. For a tiny caller limit (e.g. an autocomplete asking for 2) with a few incidental exact hits, that made ftsHasMore true, so the block never looked "sparse" and the typo fallback never fired — and subsequent offsets are barred from triggering it, so the fuzzy results were unreachable entirely. SearchPage now floors the cursor-mode probe at fuzzyFallbackThreshold rows, and execSearchBlock returns the pre-trim row count so sparsity is judged as `fetched < threshold` independent of the caller's page size. The returned page is still trimmed to limit with correct hasMore. Exact mode is unchanged (it judges sparsity by the page-independent window count). * fix(catalog): harden fuzzy-search fallback per adversarial review Addresses the confirmed findings from a deep review of the fuzzy-search fallback: - Cursor mode now enters the fallback only when the whole sparse FTS block fits the caller's page, so the terminal fuzzy page can never hide exact matches the plain hasMore path would have surfaced (jellycompat clients with EnableTotalRecordCount=false lost matches). - execSearchBlock takes a querier and returns its untrimmed rows; SearchPage hands the already-fetched block to the fallback instead of re-running an identical FTS query on every sparse search. - Fuzzy truncation is detected with LIMIT cap+1 instead of a COUNT(*) OVER () window count that only fed a debug log; truncated exact-mode responses now report total_exact=false rather than presenting the cap as an exact count. - The fuzzy query runs in a transaction pinning pg_trgm.similarity_threshold via SET LOCAL, so match quality cannot drift with cluster configuration. - When the FTS block has real hits, fuzzy augmentation demands similarity >= 0.45 so correctly-spelled sparse queries only gain near-identical titles instead of base-threshold trigram noise. - filterCatalogSearchItems no longer erases fuzzy matches on the filtered/sorted/prefix resolver path: a typo token is never a substring of the titles it matched, which left typo search returning zero results there while the plain search box showed matches. - The cursor probe floor applies only when the fallback can fire; cursor fuzzy fetches no more rows than the terminal page can serve. - slog.Debug -> slog.DebugContext (sloglint); reuse contentIDsFromMediaItems instead of a duplicate helper; document the title-only fuzzy scope. Verified against the dev deployment: stable totals across pages, no duplicates, small-limit cursor recall restored, filtered-path typo search working, ~160ms typo-path latency. Part of PR #386. Co-Authored-By: Claude Fable 5 * feat(catalog): reach long titles via strict word similarity in fuzzy search Full-string trigram similarity is diluted by every extra trigram a long title contributes, so a typo of one word could never reach titles like "Avengers: Endgame" ("avegners" scores ~0.38 against "avengers" but far below threshold against the full title). Swap the fuzzy predicate from % to <<% (strict_word_similarity), which scores the query against the best word-boundary extent of the title. At equal thresholds <<% is a strict superset of %, and the existing gin_trgm_ops index serves both — no migration needed. The SET LOCAL pin moves to pg_trgm.strict_word_similarity_threshold and is load-bearing: the 0.6 server default would reject ordinary one-edit typos outright. Ranking is strict word similarity first with whole-title similarity() as tie-break, so near-identical short titles ("The Avengers") sort above long titles that merely contain the matched word. The 0.45 augmentation floor deliberately stays on whole-title similarity(): word similarity rates embedded prefix words far too high ("coral" scores 0.5 against "coraline"), which dev testing showed would flood a correctly-spelled sparse query with 27 noise rows. Zero-hit (true typo) queries skip the floor, so the new long-title recall applies where it matters. Dev-verified: "avegners" now returns The Avengers first, then Avengers Grimm / Avengers: Endgame; "coraline" still returns exactly its 4 real titles; cursor small-limit recall, filtered-path typo search, pagination stability, and ~160ms typo-path latency all unchanged. Part of PR #386. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- internal/catalog/catalog_resolver.go | 10 + internal/catalog/item_repo.go | 505 +++++++++++++++++++++++---- internal/catalog/item_repo_test.go | 217 ++++++++++++ internal/catalog/search_query.go | 26 ++ 4 files changed, 693 insertions(+), 65 deletions(-) diff --git a/internal/catalog/catalog_resolver.go b/internal/catalog/catalog_resolver.go index 1e01dd11..22994fc3 100644 --- a/internal/catalog/catalog_resolver.go +++ b/internal/catalog/catalog_resolver.go @@ -2253,6 +2253,16 @@ func filterCatalogSearchItems(items []*models.MediaItem, raw string) []*models.M filtered = append(filtered, item) } } + if len(filtered) == 0 && len(items) > 0 && eligibleForFuzzy(parsed) { + // The strict per-token substring filter erases the repo layer's + // typo-tolerant (trigram) matches: a misspelled token is never a + // substring of the titles it fuzzy-matched, so a typo query that + // found results via the fuzzy fallback would be filtered to zero here + // while the plain search box shows matches. When a fuzzy-eligible + // query would be emptied entirely, trust the repo's relevance + // ordering instead. + return items + } return filtered } diff --git a/internal/catalog/item_repo.go b/internal/catalog/item_repo.go index f0620c36..452a0fdb 100644 --- a/internal/catalog/item_repo.go +++ b/internal/catalog/item_repo.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "strconv" "strings" @@ -131,6 +132,40 @@ func (r *ItemRepository) GetPosterPath(ctx context.Context, contentID string) (s // incidental one-mention hits that flooded results before. const overviewMatchFloor = 0.15 +// fuzzyFallbackThreshold is the FTS result count below which SearchPage reaches +// for the trigram fuzzy fallback (buildFuzzySearchSQL). At or above it the FTS +// result set is considered rich enough that a "did you mean" pass would only add +// noise, so common searches stay on the unchanged FTS path and the fuzzy query +// fires only in the sparse/misspelling case. +const fuzzyFallbackThreshold = 5 + +// fuzzyMaxResults caps how many trigram fuzzy matches the fallback contributes. +// The combined result the fuzzy path serves is therefore bounded at +// (fuzzyFallbackThreshold-1) FTS rows + fuzzyMaxResults fuzzy rows, small enough +// to materialize once and slice per page. A "did you mean" fallback has no need +// to paginate hundreds of low-similarity typo matches; when the true match count +// exceeds this, the overflow is logged and the total is reported as inexact. +const fuzzyMaxResults = 50 + +// trgmWordSimilarityThreshold pins pg_trgm.strict_word_similarity_threshold +// for the fuzzy query via SET LOCAL. The <<% operator's selectivity is +// otherwise governed by a cluster-wide GUC a DBA (or another application) can +// change, silently altering both match quality and scan cost per deployment — +// and its server default (0.6) would reject ordinary one-edit typos +// ("avegners" vs "avengers" scores ~0.38), so pinning is load-bearing, not +// just hygiene. +const trgmWordSimilarityThreshold = 0.30 + +// fuzzyAugmentSimilarityFloor is the minimum WHOLE-TITLE similarity demanded of +// fuzzy rows when the sparse FTS block is non-empty. A correctly spelled query +// with a few genuine hits ("coraline") should only gain near-identical titles, +// not every title clearing the permissive base threshold — word similarity is +// deliberately not used here, since it scores embedded prefix words far too +// high ("coral" is 0.5 to "coraline"). A zero-hit query (a likely misspelling) +// skips the floor entirely so typo recall, including word-extent matches into +// long titles, stays high. +const fuzzyAugmentSimilarityFloor = 0.45 + // itemColumnNames lists, in scan order, every column selected by media_items // item queries. Shared by itemColumns, qualifiedItemColumns, // qualifiedListItemColumns, and qualifiedItemColumnRefs so the select lists @@ -884,26 +919,241 @@ func (r *ItemRepository) SearchPage( if filter.AllowedLibraryIDs != nil && len(filter.AllowedLibraryIDs) == 0 { return []*models.MediaItem{}, 0, false, includeTotal, nil } + + parsed := parseSearchQuery(query) + + // FTS block (the common path). queryLimit fetches one extra row in cursor + // mode so execSearchBlock can derive hasMore without a separate count. When + // the fuzzy fallback below could fire (first page of a fuzzy-eligible + // query) the probe is additionally floored at fuzzyFallbackThreshold rows: + // a tiny caller limit (e.g. an autocomplete asking for 2) with a few + // incidental exact hits would otherwise report hasMore and hide the FTS + // block's true size, so the sparse test below could never fire. Fetching up + // to the threshold reveals a genuinely sparse block regardless of limit; + // the returned page is still trimmed to limit. Where the fallback cannot + // fire, fetching past limit+1 would be pure waste, so the floor is skipped. queryLimit := limit if !includeTotal { queryLimit = limit + 1 + if offset == 0 && eligibleForFuzzy(parsed) && queryLimit < fuzzyFallbackThreshold { + queryLimit = fuzzyFallbackThreshold + } } - sql, countSQL, args := r.buildSearchSQLWithTotal(query, itemTypes, queryLimit, offset, filter, includeTotal) + sql, countSQL, args := r.buildSearchSQLFromParsed(parsed, itemTypes, queryLimit, offset, filter, includeTotal) if sql == "" { return []*models.MediaItem{}, 0, false, includeTotal, nil } - - rows, err := r.pool.Query(ctx, sql, args...) + ftsItems, ftsTotal, ftsHasMore, ftsUntrimmed, err := r.execSearchBlock(ctx, r.pool, sql, countSQL, args, limit, offset, includeTotal) if err != nil { - return nil, 0, false, includeTotal, fmt.Errorf("searching media items: %w", err) + return nil, 0, false, includeTotal, err + } + + // Fuzzy fallback trigger. The trigram (typo-tolerant) arm is deliberately NOT + // part of the FTS query: OR-ing the pg_trgm % operator into that WHERE forces + // a lossy bitmap heap recheck that rebuilds the three title tsvectors for + // every one of the thousands of near-miss rows the % operator surfaces, + // making *every* search take seconds. Instead we reach for the trigram index + // only when the FTS result set is fully known and sparse (a likely + // misspelling or word-boundary miss). + // + // Exact mode judges sparsity by the window count (page-independent), so every + // page of a sparse query agrees and the combined result paginates fully. + // + // Cursor mode has no window count, so it uses the pre-trim size of a probe + // floored at fuzzyFallbackThreshold rows: fewer than the threshold came + // back ⇒ the whole FTS block is smaller than the threshold ⇒ sparse, + // independent of the caller's page size. It only trusts this on the first + // page (offset 0), where searchWithFuzzyFallback serves fuzzy as a terminal + // augmentation (no phantom next page a bare offset cursor couldn't locate), + // and only when the whole FTS block fits inside the caller's page: a + // terminal page must never hide exact matches the plain cursor path would + // have surfaced via hasMore. Past offset 0 an empty FTS page can't be told + // from a rich one, so sparse stays false. + var sparse bool + if includeTotal { + sparse = ftsTotal < fuzzyFallbackThreshold + } else if offset == 0 { + sparse = len(ftsUntrimmed) < fuzzyFallbackThreshold && len(ftsUntrimmed) <= limit + } + if !sparse || !eligibleForFuzzy(parsed) { + return ftsItems, ftsTotal, ftsHasMore, includeTotal, nil + } + + // Hand the fallback the FTS block when the rows just fetched provably form + // the complete block, sparing it an identical second FTS query. Cursor mode + // always qualifies here (the floored probe returned fewer rows than it + // asked for); exact mode qualifies on the first page when the window count + // fit within the page. + ftsBlock, haveFTSBlock := []*models.MediaItem(nil), false + if !includeTotal { + ftsBlock, haveFTSBlock = ftsUntrimmed, true + } else if offset == 0 && ftsTotal <= len(ftsItems) { + ftsBlock, haveFTSBlock = ftsItems, true + } + return r.searchWithFuzzyFallback(ctx, parsed, itemTypes, limit, offset, filter, includeTotal, ftsBlock, haveFTSBlock) +} + +// searchWithFuzzyFallback serves the combined [FTS block][fuzzy block] result +// for a sparse, fuzzy-eligible query. Because fuzzy only fires when the FTS +// block is sparse (< fuzzyFallbackThreshold rows) and the fuzzy contribution is +// capped at fuzzyMaxResults, the ENTIRE combined result is small +// (< fuzzyFallbackThreshold + fuzzyMaxResults rows). We materialize it once and +// slice the requested page in memory. That keeps two-block pagination correct on +// every page — stable total, no cross-page duplicates, no offset arithmetic +// straddling the block boundary — for both exact and cursor callers. The +// FTS-only path in SearchPage is untouched and still streams strictly per page. +// +// When haveFTSBlock is true, ftsBlock is the complete sparse FTS block the +// caller already fetched and no second FTS query is issued; otherwise (exact +// mode past the first page, or a first page smaller than the block) the block +// is re-fetched here at offset 0 — its ids drive fuzzy dedup below. +func (r *ItemRepository) searchWithFuzzyFallback( + ctx context.Context, + parsed parsedSearchQuery, + itemTypes []string, + limit, offset int, + filter AccessFilter, + includeTotal bool, + ftsBlock []*models.MediaItem, + haveFTSBlock bool, +) ([]*models.MediaItem, int, bool, bool, error) { + if !haveFTSBlock { + ftsSQL, ftsCountSQL, ftsArgs := r.buildSearchSQLFromParsed(parsed, itemTypes, fuzzyFallbackThreshold, 0, filter, true) + var err error + ftsBlock, _, _, _, err = r.execSearchBlock(ctx, r.pool, ftsSQL, ftsCountSQL, ftsArgs, fuzzyFallbackThreshold, 0, true) + if err != nil { + return nil, 0, false, includeTotal, err + } + } + + // Fuzzy block, excluding every FTS-block id so no title appears in both + // blocks and the combined total is stable per page. Cursor mode serves the + // combined result as a single terminal page, so it never needs more fuzzy + // rows than the page has room for; exact mode materializes up to the cap. + // Either way one extra row is fetched so truncation is detectable without + // paying a COUNT(*) OVER () window count over the whole trigram match set. + // When the FTS block found real hits the fuzzy arm demands near-certain + // corrections (fuzzyAugmentSimilarityFloor) instead of base-threshold noise. + fuzzyLimit := fuzzyMaxResults + if !includeTotal && limit-len(ftsBlock) < fuzzyLimit { + fuzzyLimit = limit - len(ftsBlock) + } + minSimilarity := 0.0 + if len(ftsBlock) > 0 { + minSimilarity = fuzzyAugmentSimilarityFloor + } + var fuzzyBlock []*models.MediaItem + fuzzyTruncated := false + if fuzzyLimit > 0 { + fuzzySQL, _, fuzzyArgs := r.buildFuzzySearchFromParsed(parsed, itemTypes, fuzzyLimit+1, 0, filter, false, contentIDsFromMediaItems(ftsBlock), minSimilarity) + if fuzzySQL != "" { + var err error + fuzzyBlock, fuzzyTruncated, err = r.execFuzzyBlock(ctx, fuzzySQL, fuzzyArgs, fuzzyLimit) + if err != nil { + return nil, 0, false, includeTotal, err + } + if fuzzyTruncated { + slog.DebugContext(ctx, "fuzzy search fallback truncated to cap", + "query", parsed.Text, "cap", fuzzyLimit) + } + } + } + + combined := make([]*models.MediaItem, 0, len(ftsBlock)+len(fuzzyBlock)) + combined = append(combined, ftsBlock...) + combined = append(combined, fuzzyBlock...) + total := len(combined) + + // Slice the requested page out of the materialized list, clamped so a + // caller passing a negative offset or an overflowing limit (the exported + // SearchPage is reachable outside the HTTP parser, which otherwise + // guarantees offset>=0 and limit>0) gets a sane page instead of a + // reversed-slice panic. + lo := min(max(offset, 0), total) + hi := lo + max(limit, 0) + if hi < lo || hi > total { // hi < lo: lo+limit overflowed; serve the rest + hi = total + } + page := combined[lo:hi] + + if !includeTotal { + // Cursor mode reaches here only at offset 0 with the whole FTS block + // inside the page (see SearchPage gate), so no exact match is hidden; + // only fuzzy augmentation past the page is cut. Serve as a terminal + // page: report only what we return and no next page, since a bare + // offset cursor can't locate the FTS/fuzzy boundary on a follow-up + // request. + return page, len(page), false, false, nil + } + // When the fuzzy arm hit its cap the true match count is larger than the + // materialized list, so surface the total as inexact rather than presenting + // the cap as an exact count. + return page, total, hi < total, !fuzzyTruncated, nil +} + +// execFuzzyBlock runs the fuzzy data query inside a transaction that pins +// pg_trgm.strict_word_similarity_threshold, so the <<% operator's selectivity +// cannot drift with cluster configuration (its 0.6 server default would reject +// ordinary typos outright). The query was built with LIMIT fuzzyLimit+1; the +// extra row only signals truncation and is trimmed from the returned block. +func (r *ItemRepository) execFuzzyBlock(ctx context.Context, dataSQL string, args []any, fuzzyLimit int) ([]*models.MediaItem, bool, error) { + tx, err := r.pool.Begin(ctx) + if err != nil { + return nil, false, fmt.Errorf("beginning fuzzy search tx: %w", err) + } + defer func() { _ = tx.Rollback(ctx) }() + + if _, err := tx.Exec(ctx, fmt.Sprintf("SET LOCAL pg_trgm.strict_word_similarity_threshold = %g", trgmWordSimilarityThreshold)); err != nil { + return nil, false, fmt.Errorf("pinning trigram word-similarity threshold: %w", err) + } + block, _, truncated, _, err := r.execSearchBlock(ctx, tx, dataSQL, "", args, fuzzyLimit, 0, false) + if err != nil { + return nil, false, err + } + if err := tx.Commit(ctx); err != nil { + return nil, false, fmt.Errorf("committing fuzzy search tx: %w", err) + } + return block, truncated, nil +} + +// searchQuerier is the subset of pgxpool.Pool / pgx.Tx that execSearchBlock +// needs, so a block can run either directly on the pool or inside a +// transaction (the fuzzy block pins a GUC with SET LOCAL). +type searchQuerier interface { + Query(ctx context.Context, sql string, args ...any) (pgx.Rows, error) + QueryRow(ctx context.Context, sql string, args ...any) pgx.Row +} + +// execSearchBlock runs one search data query (FTS or fuzzy fallback) and its +// count-fallback sibling, returning that block's page. It is the shared engine +// behind SearchPage's two blocks so both derive hasMore/total identically. +// +// In cursor mode (includeTotal == false) the caller fetches limit+1 rows so +// hasMore is len(items) > limit; the extra row is trimmed here. In exact mode +// the data query carries COUNT(*) OVER () for the total, which emits no rows on +// an empty page (e.g. OFFSET past the last row); only then, and only past +// offset 0, is the count sibling re-run to recover the real total. Both queries +// must end with the trailing limit/offset args so the count sibling can drop +// them. +// +// The fourth return value is the untrimmed row set the data query actually +// returned (before the cursor-mode +1 row is trimmed; in exact mode it equals +// the page). SearchPage uses it in cursor mode to judge FTS-block sparsity +// independently of the caller's page size and to hand the fuzzy fallback the +// complete block without a second FTS query. +func (r *ItemRepository) execSearchBlock(ctx context.Context, q searchQuerier, dataSQL, countSQL string, args []any, limit, offset int, includeTotal bool) ([]*models.MediaItem, int, bool, []*models.MediaItem, error) { + rows, err := q.Query(ctx, dataSQL, args...) + if err != nil { + return nil, 0, false, nil, fmt.Errorf("searching media items: %w", err) } defer rows.Close() if !includeTotal { items, err := scanItems(rows) if err != nil { - return nil, 0, false, false, err + return nil, 0, false, nil, err } + untrimmed := items hasMore := len(items) > limit if hasMore { items = items[:limit] @@ -912,28 +1162,23 @@ func (r *ItemRepository) SearchPage( if hasMore { total++ } - return items, total, hasMore, false, nil + return items, total, hasMore, untrimmed, nil } items, total, err := scanItemsWithTotal(rows) if err != nil { - return nil, 0, false, true, err + return nil, 0, false, nil, err } hasMore := total > offset+len(items) - // COUNT(*) OVER () emits no rows when the data SELECT is empty, so total - // stays 0 even when the broader result set has matching rows (e.g. OFFSET - // past the last page). Re-query the count to give callers the real total. - // Skip when offset == 0 because in that case an empty page genuinely means - // total = 0. if len(items) == 0 && offset > 0 { // Drop the trailing limit/offset args from the data query. countArgs := args[:len(args)-2] - if err := r.pool.QueryRow(ctx, countSQL, countArgs...).Scan(&total); err != nil { - return nil, 0, false, true, fmt.Errorf("count fallback for empty search page: %w", err) + if err := q.QueryRow(ctx, countSQL, countArgs...).Scan(&total); err != nil { + return nil, 0, false, nil, fmt.Errorf("count fallback for empty search page: %w", err) } hasMore = total > offset+len(items) } - return items, total, hasMore, true, nil + return items, total, hasMore, items, nil } // buildSearchSQL assembles the unified search query, returning the SQL string @@ -960,16 +1205,32 @@ func (r *ItemRepository) SearchPage( // parsed.Year (or NULL) // parsed.Phrase // limit, offset +// +// searchTextFromParsed derives the effective search text shared by the FTS and +// fuzzy builders: the parsed Text, or (when the query was only quotes/whitespace +// that parsed to empty Text) a whitespace-collapsed, quote-stripped fallback off +// the raw query. Centralized so the two builders can never search different text. +func searchTextFromParsed(parsed parsedSearchQuery) string { + if parsed.Text != "" { + return parsed.Text + } + return collapseSearchWhitespace(strings.ReplaceAll(strings.TrimSpace(parsed.Raw), "\"", " ")) +} + func (r *ItemRepository) buildSearchSQL(query string, itemTypes []string, limit, offset int, filter AccessFilter) (dataSQL, countSQL string, args []any) { return r.buildSearchSQLWithTotal(query, itemTypes, limit, offset, filter, true) } func (r *ItemRepository) buildSearchSQLWithTotal(query string, itemTypes []string, limit, offset int, filter AccessFilter, includeTotal bool) (dataSQL, countSQL string, args []any) { - parsed := parseSearchQuery(query) - searchText := parsed.Text - if searchText == "" { - searchText = collapseSearchWhitespace(strings.ReplaceAll(strings.TrimSpace(query), "\"", " ")) - } + return r.buildSearchSQLFromParsed(parseSearchQuery(query), itemTypes, limit, offset, filter, includeTotal) +} + +// buildSearchSQLFromParsed is the FTS query builder proper; the string-taking +// wrappers above parse first. SearchPage parses once and calls this directly so +// the same parsedSearchQuery feeds the FTS builder, the fuzzy builder, and the +// eligibility gate without re-parsing. +func (r *ItemRepository) buildSearchSQLFromParsed(parsed parsedSearchQuery, itemTypes []string, limit, offset int, filter AccessFilter, includeTotal bool) (dataSQL, countSQL string, args []any) { + searchText := searchTextFromParsed(parsed) if searchText == "" { return "", "", nil } @@ -1000,48 +1261,14 @@ func (r *ItemRepository) buildSearchSQLWithTotal(query string, itemTypes []strin overviewMatch := fmt.Sprintf("%s @@ websearch_to_tsquery('english', $1)", overviewVector) // Keep the base match condition index-friendly; exact-title logic is used as - // a ranking boost later, not as an additional scan predicate. + // a ranking boost later, not as an additional scan predicate. The trigram + // (typo-tolerant) arm intentionally lives in buildFuzzySearchSQL, not here: + // OR-ing the % operator into this WHERE forced a lossy bitmap recheck that + // rebuilt the title tsvectors for thousands of near-miss rows. SearchPage + // invokes the fuzzy path separately only when this FTS query is sparse. conditions = append(conditions, fmt.Sprintf("(%s OR %s OR %s)", titleMatch, titlePrefixMatch, overviewMatch)) - if len(itemTypes) > 0 { - placeholders := make([]string, 0, len(itemTypes)) - for _, itemType := range itemTypes { - if strings.TrimSpace(itemType) == "" { - continue - } - placeholders = append(placeholders, fmt.Sprintf("$%d", argIdx)) - args = append(args, strings.ToLower(strings.TrimSpace(itemType))) - argIdx++ - } - if len(placeholders) > 0 { - conditions = append(conditions, fmt.Sprintf("mi.type IN (%s)", strings.Join(placeholders, ", "))) - } - } - - needsLibJoin := filter.AllowedLibraryIDs != nil || len(filter.DisabledLibraryIDs) > 0 - fromClause := "media_items mi" - if filter.AllowedLibraryIDs != nil { - // Caller (Search) is expected to short-circuit when len == 0; we still - // guard here so the builder is safe to invoke from tests. - if len(filter.AllowedLibraryIDs) > 0 { - conditions = append(conditions, fmt.Sprintf("mil.media_folder_id = ANY($%d)", argIdx)) - args = append(args, filter.AllowedLibraryIDs) - argIdx++ - } - } - if len(filter.DisabledLibraryIDs) > 0 { - conditions = append(conditions, fmt.Sprintf("NOT (mil.media_folder_id = ANY($%d))", argIdx)) - args = append(args, filter.DisabledLibraryIDs) - argIdx++ - } - if needsLibJoin { - fromClause = "media_items mi JOIN media_item_libraries mil ON mi.content_id = mil.content_id" - } - applyAccessFilter("mi", AccessFilter{MaxContentRating: filter.MaxContentRating, ExcludedMediaTypes: filter.ExcludedMediaTypes}, &conditions, &args, &argIdx) - - // Manga chapters (type='ebook' rows linked into a manga series) are internal - // sub-units and must never surface as standalone search results. - conditions = append(conditions, MangaChapterExclusionWhere("mi")) + fromClause := appendSearchScopeFilters(itemTypes, filter, &conditions, &args, &argIdx) whereClause := "WHERE " + strings.Join(conditions, " AND ") @@ -1096,11 +1323,11 @@ func (r *ItemRepository) buildSearchSQLWithTotal(query string, itemTypes []strin fmt.Sprintf(normalizedTitleExpr, "mi.sort_title"), exactIdx, ) - // Use qualified column names inside the CTE to avoid ambiguity when - // the FROM clause includes a JOIN to media_item_libraries. The select - // list aliases coalesced columns back to their own names (poster_path - // etc.) so the outer query can re-reference them; GROUP BY needs the - // raw references because output aliases are invalid there. + // Qualified column names inside the CTE, grouped so the MAX(...) ranking + // aggregates below are legal. The select list aliases coalesced columns back + // to their own names (poster_path etc.) so the outer query can re-reference + // them; GROUP BY needs the raw references because output aliases are invalid + // there. qualifiedCols := qualifiedItemColumns("mi") groupByCols := qualifiedItemColumnRefs("mi") scoredCTE := fmt.Sprintf(` @@ -1185,6 +1412,154 @@ func (r *ItemRepository) buildSearchSQLWithTotal(query string, itemTypes []strin return dataSQL, countSQL, args } +// appendSearchScopeFilters appends the scope predicates shared by the FTS search +// (buildSearchSQLWithTotal) and the trigram fuzzy fallback (buildFuzzySearchSQL): +// item type, allowed/disabled libraries, the access filter, and the +// manga-chapter exclusion. It mutates conditions/args/argIdx in place and +// returns the FROM clause (always "media_items mi"; library scoping is enforced +// with independent EXISTS/NOT EXISTS subqueries rather than a JOIN). Both callers +// append these in the same order so a single helper keeps the two queries' +// filtering provably identical. +func appendSearchScopeFilters(itemTypes []string, filter AccessFilter, conditions *[]string, args *[]any, argIdx *int) (fromClause string) { + fromClause = "media_items mi" + + if len(itemTypes) > 0 { + placeholders := make([]string, 0, len(itemTypes)) + for _, itemType := range itemTypes { + if strings.TrimSpace(itemType) == "" { + continue + } + placeholders = append(placeholders, fmt.Sprintf("$%d", *argIdx)) + *args = append(*args, strings.ToLower(strings.TrimSpace(itemType))) + *argIdx++ + } + if len(placeholders) > 0 { + *conditions = append(*conditions, fmt.Sprintf("mi.type IN (%s)", strings.Join(placeholders, ", "))) + } + } + + // Library allow/deny via the shared leak-safe helper: an item linked to both a + // passing and a disabled library must not slip through. The old JOIN + + // NOT(mil.media_folder_id = ANY(...)) form let the passing membership row + // satisfy the deny check (audit 2026-05-01 §3.3), so both the FTS search and + // the fuzzy fallback that share this helper used it. appendLibraryAccessConditions + // emits independent EXISTS/NOT EXISTS subqueries keyed on mi.content_id and + // needs no JOIN. + appendLibraryAccessConditions("mi.content_id", filter, conditions, args, argIdx) + + applyAccessFilter("mi", AccessFilter{MaxContentRating: filter.MaxContentRating, ExcludedMediaTypes: filter.ExcludedMediaTypes}, conditions, args, argIdx) + + // Manga chapters (type='ebook' rows linked into a manga series) are internal + // sub-units and must never surface as standalone search results. + *conditions = append(*conditions, MangaChapterExclusionWhere("mi")) + return fromClause +} + +// buildFuzzySearchSQL assembles the trigram fuzzy-fallback query invoked by +// SearchPage only when the FTS query is sparse. Unlike buildSearchSQLWithTotal, +// the sole match predicate is the pg_trgm strict-word-similarity operator +// (<<%) against the indexed title_normalized generated column (migration 105's +// gin_trgm_ops index serves it), and the ranking signals are +// strict_word_similarity()/similarity() on that same column. Crucially it +// never references the title tsvectors, so the low-similarity rows the +// operator can surface never pay a per-row tsvector rebuild — that rebuild +// over the fuzzy candidate set was the entire cause of the multi-second search +// regression. The operator's base selectivity is pinned by the caller +// (execFuzzyBlock sets pg_trgm.strict_word_similarity_threshold via SET LOCAL) +// so it cannot drift with cluster configuration. +// +// Scope note: only title_normalized — a generated column over `title` — is +// fuzzy-matched, because it is the only column with a gin_trgm_ops index +// (migration 105). Typos of original_title or sort_title, which the FTS arm +// covers exactly, are deliberately out of the fuzzy arm's scope; extending it +// would need matching normalized columns + trigram indexes (follow-up). +// +// includeTotal toggles the COUNT(*) OVER () window total the same way +// buildSearchSQLWithTotal does, so the fuzzy block honors the caller's cursor +// vs. exact-count mode. excludeContentIDs drops rows already returned by the FTS +// block so a title matching both is not shown twice. minSimilarity > 0 adds an +// explicit similarity floor above the base threshold — used when the FTS block +// had real hits so augmentation only admits near-certain corrections. Argument +// order: $1 searchText, then the similarity floor (if any), then the shared +// scope placeholders, then the exclusion array, then limit/offset. +func (r *ItemRepository) buildFuzzySearchSQL(query string, itemTypes []string, limit, offset int, filter AccessFilter, includeTotal bool, excludeContentIDs []string, minSimilarity float64) (dataSQL, countSQL string, args []any) { + return r.buildFuzzySearchFromParsed(parseSearchQuery(query), itemTypes, limit, offset, filter, includeTotal, excludeContentIDs, minSimilarity) +} + +// buildFuzzySearchFromParsed is the fuzzy query builder proper; the string-taking +// wrapper above parses first. SearchPage parses once and calls this directly. +func (r *ItemRepository) buildFuzzySearchFromParsed(parsed parsedSearchQuery, itemTypes []string, limit, offset int, filter AccessFilter, includeTotal bool, excludeContentIDs []string, minSimilarity float64) (dataSQL, countSQL string, args []any) { + searchText := searchTextFromParsed(parsed) + if searchText == "" { + return "", "", nil + } + + args = []any{searchText} + argIdx := 2 + // Strict word similarity (<<%) rather than full-string similarity (%): a + // typo of one word must still reach long titles ("avegners" → "Avengers: + // Endgame"), where full-string similarity is diluted by every extra trigram + // the rest of the title contributes. <<% scores the query against the best + // word-boundary extent of the title instead, and at equal thresholds it is + // a strict superset of % (the whole string is itself a valid extent). The + // same gin_trgm_ops index serves both operators. + conditions := []string{"public.normalize_search_text($1) <<% mi.title_normalized"} + if minSimilarity > 0 { + // The floor is whole-title similarity, NOT word similarity: augmenting + // a query that already has hits must only admit near-identical titles, + // and word similarity rates embedded prefix words far too high (see + // fuzzyAugmentSimilarityFloor). + conditions = append(conditions, fmt.Sprintf("similarity(public.normalize_search_text($1), mi.title_normalized) >= $%d", argIdx)) + args = append(args, minSimilarity) + argIdx++ + } + + fromClause := appendSearchScopeFilters(itemTypes, filter, &conditions, &args, &argIdx) + + if len(excludeContentIDs) > 0 { + conditions = append(conditions, fmt.Sprintf("NOT (mi.content_id = ANY($%d))", argIdx)) + args = append(args, excludeContentIDs) + argIdx++ + } + + whereClause := "WHERE " + strings.Join(conditions, " AND ") + + // GROUP BY is required so the MAX(similarity(...)) ranking aggregate is legal, + // mirroring buildSearchSQLWithTotal; COUNT(*) OVER () then counts distinct + // content_ids. qualifiedItemColumns aliases the coalesced columns so the outer + // SELECT can re-reference them; GROUP BY uses the raw refs. + qualifiedCols := qualifiedItemColumns("mi") + groupByCols := qualifiedItemColumnRefs("mi") + // fuzzy_rank orders by how well the query matches SOME word extent of the + // title; fuzzy_full_rank tie-breaks by whole-title closeness so "The + // Avengers" sorts above "Avengers: Endgame" for the typo "avegners". Both + // are computed only over the matched candidate set. + scoredCTE := fmt.Sprintf(` + WITH scored AS ( + SELECT + %s, + MAX(strict_word_similarity(public.normalize_search_text($1), mi.title_normalized)) AS fuzzy_rank, + MAX(similarity(public.normalize_search_text($1), mi.title_normalized)) AS fuzzy_full_rank + FROM %s + %s + GROUP BY %s + ) + `, qualifiedCols, fromClause, whereClause, groupByCols) + + totalColumn := "" + if includeTotal { + totalColumn = ", COUNT(*) OVER () AS total_count" + } + dataSQL = scoredCTE + fmt.Sprintf(` + SELECT %s%s + FROM scored + ORDER BY fuzzy_rank DESC, fuzzy_full_rank DESC, LOWER(title) ASC, content_id ASC + LIMIT $%d OFFSET $%d`, itemColumns, totalColumn, argIdx, argIdx+1) + countSQL = scoredCTE + `SELECT COUNT(*) FROM scored` + args = append(args, limit, offset) + return dataSQL, countSQL, args +} + // ListUnmatchedByFolderAndPathPrefix returns content IDs for unmatched-style // items that are linked to at least one present file within the given folder // subtree. This intentionally includes ambiguous items so a library scan can diff --git a/internal/catalog/item_repo_test.go b/internal/catalog/item_repo_test.go index cbedbe48..b01cb275 100644 --- a/internal/catalog/item_repo_test.go +++ b/internal/catalog/item_repo_test.go @@ -382,6 +382,223 @@ func TestItemRepo_Search_EmptyQueryReturnsEmpty(t *testing.T) { } } +// TestEligibleForFuzzy pins the min-token gate that guards the fuzzy title +// fallback: the longest normalized token must clear fuzzyMinTokenLen, so short, +// non-selective queries stay on the exact FTS/prefix path. +func TestEligibleForFuzzy(t *testing.T) { + cases := []struct { + query string + want bool + }{ + {"avegners", true}, // 8-char typo + {"sponge bob", true}, // longest token "sponge" (6) + {"a vengers", true}, // judged on "vengers" (7), not "a" + {"dune", true}, // exactly at the floor + {"the", false}, // 3 chars + {"a b c", false}, // all short + {"and the", false}, // "and" normalized away, "the" is 3 + {" ", false}, // empty + } + for _, tc := range cases { + if got := eligibleForFuzzy(parseSearchQuery(tc.query)); got != tc.want { + t.Errorf("eligibleForFuzzy(%q) = %v, want %v", tc.query, got, tc.want) + } + } +} + +// TestSearchTextFromParsed pins the shared searchText derivation used by both +// the FTS and fuzzy builders: parsed Text when present, else a quote-stripped +// fallback off the raw query. Both builders must agree so they search the same +// text. +func TestSearchTextFromParsed(t *testing.T) { + cases := []struct { + raw string + want string + }{ + {"interstellar", "interstellar"}, + {`"the matrix"`, "the matrix"}, + {" spaced out ", "spaced out"}, + {`"`, ""}, + {" ", ""}, + } + for _, tc := range cases { + if got := searchTextFromParsed(parseSearchQuery(tc.raw)); got != tc.want { + t.Errorf("searchTextFromParsed(%q) = %q, want %q", tc.raw, got, tc.want) + } + } +} + +// TestParseSearchQuery_NormalizedText asserts parseSearchQuery precomputes the +// full normalized text that eligibleForFuzzy's token gate reads (so the gate +// does not re-normalize on every sparse search). +func TestParseSearchQuery_NormalizedText(t *testing.T) { + if got := parseSearchQuery("The Avengers").NormalizedText; got != "the avengers" { + t.Fatalf("NormalizedText = %q, want %q", got, "the avengers") + } + // "and" is dropped and folded, matching normalizeTitleForComparison. + if got := parseSearchQuery("law and order").NormalizedText; got != "law order" { + t.Fatalf("NormalizedText = %q, want %q", got, "law order") + } +} + +// TestItemRepo_Search_FTSQueryHasNoFuzzyArm asserts that the FTS query never +// carries the trigram % arm or its similarity ranking. Fusing them forced a +// lossy bitmap recheck that rebuilt the title tsvectors for thousands of +// near-miss rows on every search; the fuzzy path is now a separate query +// (buildFuzzySearchSQL) invoked only as a fallback by SearchPage. +func TestItemRepo_Search_FTSQueryHasNoFuzzyArm(t *testing.T) { + repo := &ItemRepository{} + dataSQL, countSQL, _ := repo.buildSearchSQL("avegners", []string{"movie"}, 20, 0, AccessFilter{}) + for _, sql := range []string{dataSQL, countSQL} { + if strings.Contains(sql, "% mi.title_normalized") { + t.Fatalf("FTS query must not include the trigram %% arm; got:\n%s", sql) + } + if strings.Contains(sql, "fuzzy_rank") { + t.Fatalf("FTS query must not include fuzzy_rank; got:\n%s", sql) + } + if strings.Contains(sql, "similarity(") { + t.Fatalf("FTS query must not compute similarity(); got:\n%s", sql) + } + } +} + +// TestItemRepo_BuildFuzzySearchSQL asserts that the fuzzy fallback query scores +// only on the indexed title_normalized column (no title tsvector rebuild), +// matches via strict word similarity so long titles stay reachable, ranks by +// descending word similarity with whole-title closeness as tie-break, excludes +// already-seen content_ids, and applies the same scope filters (type, manga +// exclusion) as the FTS query. +func TestItemRepo_BuildFuzzySearchSQL(t *testing.T) { + repo := &ItemRepository{} + dataSQL, countSQL, args := repo.buildFuzzySearchSQL("avegners", []string{"movie"}, 20, 0, AccessFilter{}, true, []string{"abc", "def"}, 0) + + // <<%, not %: full-string similarity is diluted by long titles ("avegners" + // must reach "Avengers: Endgame"); the same gin_trgm_ops index serves both. + if !strings.Contains(dataSQL, "public.normalize_search_text($1) <<% mi.title_normalized") { + t.Fatalf("expected strict-word-similarity arm against title_normalized; got:\n%s", dataSQL) + } + if !strings.Contains(dataSQL, "MAX(strict_word_similarity(public.normalize_search_text($1), mi.title_normalized)) AS fuzzy_rank") { + t.Fatalf("expected strict word similarity ranking on title_normalized; got:\n%s", dataSQL) + } + if !strings.Contains(dataSQL, "MAX(similarity(public.normalize_search_text($1), mi.title_normalized)) AS fuzzy_full_rank") { + t.Fatalf("expected whole-title similarity tie-break rank; got:\n%s", dataSQL) + } + // The whole point of the separate query: it must never rebuild the title + // tsvectors that made the fused query slow. + if strings.Contains(dataSQL, "to_tsvector") || strings.Contains(dataSQL, "ts_rank_cd") { + t.Fatalf("fuzzy query must not rebuild title tsvectors; got:\n%s", dataSQL) + } + if !strings.Contains(dataSQL, "ORDER BY fuzzy_rank DESC, fuzzy_full_rank DESC, LOWER(title) ASC, content_id ASC") { + t.Fatalf("expected similarity-ordered results; got:\n%s", dataSQL) + } + if !strings.Contains(dataSQL, "NOT (mi.content_id = ANY($") { + t.Fatalf("expected exclusion of already-seen content_ids; got:\n%s", dataSQL) + } + if !strings.Contains(dataSQL, "mi.type IN ($") { + t.Fatalf("expected shared type scope filter; got:\n%s", dataSQL) + } + if !strings.Contains(countSQL, "SELECT COUNT(*) FROM scored") { + t.Fatalf("expected count sibling over the scored CTE; got:\n%s", countSQL) + } + // Arg order: $1 searchText, $2 type, $3 exclusion array, $4 limit, $5 offset. + if len(args) != 5 { + t.Fatalf("expected 5 args, got %d: %#v", len(args), args) + } + if args[0] != "avegners" { + t.Fatalf("expected $1=searchText; got %#v", args[0]) + } + if args[len(args)-2] != 20 || args[len(args)-1] != 0 { + t.Fatalf("expected trailing limit/offset args; got %#v", args[len(args)-2:]) + } +} + +// TestItemRepo_BuildFuzzySearchSQL_CursorModeOmitsWindowCount asserts that the +// fuzzy fallback honors cursor mode: with includeTotal=false it must not carry +// COUNT(*) OVER (), matching buildSearchSQLWithTotal's skip-total contract so +// SearchPage's cursor path stays a pure keyset page. +func TestItemRepo_BuildFuzzySearchSQL_CursorModeOmitsWindowCount(t *testing.T) { + repo := &ItemRepository{} + dataSQL, _, _ := repo.buildFuzzySearchSQL("avegners", []string{"movie"}, 21, 0, AccessFilter{}, false, nil, 0) + if strings.Contains(dataSQL, "COUNT(*) OVER") { + t.Fatalf("cursor-mode fuzzy query must omit window count; got:\n%s", dataSQL) + } + if strings.Contains(dataSQL, "total_count") { + t.Fatalf("cursor-mode fuzzy query must not select total_count; got:\n%s", dataSQL) + } +} + +// TestItemRepo_BuildFuzzySearchSQL_SimilarityFloor asserts the augment floor +// contract: minSimilarity > 0 adds an explicit similarity() >= $n predicate +// (used when the FTS block had real hits, so augmentation only admits +// near-certain corrections), and minSimilarity == 0 omits it so zero-hit typo +// queries keep the base pinned threshold's recall. +func TestItemRepo_BuildFuzzySearchSQL_SimilarityFloor(t *testing.T) { + repo := &ItemRepository{} + + floorSQL, _, floorArgs := repo.buildFuzzySearchSQL("avegners", []string{"movie"}, 20, 0, AccessFilter{}, true, nil, fuzzyAugmentSimilarityFloor) + // Whole-title similarity, not word similarity: the augment floor must not + // admit embedded prefix words ("coral" scores 0.5 word-similarity to + // "coraline"). + if !strings.Contains(floorSQL, "AND similarity(public.normalize_search_text($1), mi.title_normalized) >= $2") { + t.Fatalf("expected explicit whole-title similarity floor predicate as $2; got:\n%s", floorSQL) + } + if len(floorArgs) < 2 || floorArgs[1] != fuzzyAugmentSimilarityFloor { + t.Fatalf("expected $2 = fuzzyAugmentSimilarityFloor; got args %#v", floorArgs) + } + // Trailing limit/offset must still close the arg list for the count sibling. + if floorArgs[len(floorArgs)-2] != 20 || floorArgs[len(floorArgs)-1] != 0 { + t.Fatalf("expected trailing limit/offset args; got %#v", floorArgs[len(floorArgs)-2:]) + } + + baseSQL, _, baseArgs := repo.buildFuzzySearchSQL("avegners", []string{"movie"}, 20, 0, AccessFilter{}, true, nil, 0) + if strings.Contains(baseSQL, "AND similarity(public.normalize_search_text($1), mi.title_normalized) >= $2") { + t.Fatalf("zero floor must not add a similarity predicate; got:\n%s", baseSQL) + } + if len(baseArgs) != len(floorArgs)-1 { + t.Fatalf("zero floor should bind one fewer arg: base %d vs floor %d", len(baseArgs), len(floorArgs)) + } +} + +// TestItemRepo_BuildFuzzySearchSQL_EmptyQueryReturnsEmpty guards the same +// empty-input contract as buildSearchSQL. +func TestItemRepo_BuildFuzzySearchSQL_EmptyQueryReturnsEmpty(t *testing.T) { + repo := &ItemRepository{} + dataSQL, countSQL, args := repo.buildFuzzySearchSQL(" ", []string{"movie"}, 20, 0, AccessFilter{}, true, nil, 0) + if dataSQL != "" || countSQL != "" || args != nil { + t.Fatalf("expected empty result for blank query; got dataSQL=%q countSQL=%q args=%#v", dataSQL, countSQL, args) + } +} + +// TestItemRepo_Search_LibraryScopeUsesIndependentExistsPredicates pins the +// leak-safe library scoping shared by the FTS search and the fuzzy fallback via +// appendSearchScopeFilters. The prior JOIN + NOT(mil.media_folder_id = ANY(...)) +// form let an item linked to BOTH a disabled and a non-disabled library survive +// the deny check on the passing membership row, so a typo (fuzzy) or exact +// search could surface items from a disabled library. Both builders must instead +// emit independent EXISTS/NOT EXISTS subqueries and no membership JOIN. +func TestItemRepo_Search_LibraryScopeUsesIndependentExistsPredicates(t *testing.T) { + repo := &ItemRepository{} + filter := AccessFilter{DisabledLibraryIDs: []int{9}} + + ftsSQL, _, _ := repo.buildSearchSQL("avatar", []string{"movie"}, 20, 0, filter) + fuzzySQL, _, _ := repo.buildFuzzySearchSQL("avatar", []string{"movie"}, 20, 0, filter, true, nil, 0) + + for name, sql := range map[string]string{"fts": ftsSQL, "fuzzy": fuzzySQL} { + if strings.Contains(sql, "JOIN media_item_libraries") { + t.Fatalf("%s query must not JOIN media_item_libraries for scoping; got:\n%s", name, sql) + } + // Disabled-only path requires positive membership (argument-free EXISTS) + // so orphan items don't slip through the vacuous NOT EXISTS... + if !strings.Contains(sql, "EXISTS (SELECT 1 FROM media_item_libraries mil WHERE mil.content_id = mi.content_id)") { + t.Fatalf("%s query must require library membership via an argument-free EXISTS; got:\n%s", name, sql) + } + // ...plus the disabled NOT EXISTS. + if !strings.Contains(sql, "NOT EXISTS (SELECT 1 FROM media_item_libraries mil WHERE mil.content_id = mi.content_id AND mil.media_folder_id = ANY($") { + t.Fatalf("%s query must exclude disabled libraries via NOT EXISTS; got:\n%s", name, sql) + } + } +} + // TestItemRepo_Search_UsesTitleNormalizedColumn asserts that buildSearchSQL // reads the mi.title_normalized stored generated column for the title rank // arms (exact_title_match and contiguous_title_match), so the LIKE diff --git a/internal/catalog/search_query.go b/internal/catalog/search_query.go index 788ab333..d487d7e0 100644 --- a/internal/catalog/search_query.go +++ b/internal/catalog/search_query.go @@ -11,6 +11,11 @@ type parsedSearchQuery struct { Text string Phrase string ExactTitleHint string + // NormalizedText is normalizeTitleForComparison(Text) computed once at parse + // time. Text already folds phrase + remainder together, so this is the full + // normalized query used by eligibleForFuzzy's token gate (which would + // otherwise re-normalize on every sparse search). + NormalizedText string Year *int } @@ -37,10 +42,31 @@ func parseSearchQuery(raw string) parsedSearchQuery { Text: text, Phrase: phrase, ExactTitleHint: normalizeTitleForComparison(firstNonEmptySearchValue(phrase, text)), + NormalizedText: normalizeTitleForComparison(text), Year: year, } } +// fuzzyMinTokenLen is the shortest normalized token that may enable the trigram +// fuzzy title fallback. A token shorter than this forms too few trigrams to use +// the gin_trgm_ops index selectively (and a 1-2 char token can't use it at +// all), so for short queries the fuzzy fallback is skipped and search stays on +// the exact FTS/prefix path. The gate is applied to the longest token so a stray +// short token ("a vengers") is judged on "vengers", not "a". +const fuzzyMinTokenLen = 4 + +// eligibleForFuzzy reports whether a parsed query clears the min-token gate for +// the trigram fuzzy title fallback. +func eligibleForFuzzy(parsed parsedSearchQuery) bool { + longest := 0 + for _, tok := range strings.Fields(parsed.NormalizedText) { + if n := len([]rune(tok)); n > longest { + longest = n + } + } + return longest >= fuzzyMinTokenLen +} + func extractBalancedPhrase(input string) (string, string) { start := strings.Index(input, "\"") if start == -1 {