From 41dbcaa828c463e8ce164fa905c16e23106c8d02 Mon Sep 17 00:00:00 2001 From: Silo Server Migration Date: Mon, 25 May 2026 12:49:38 -0400 Subject: [PATCH] fix(catalog): gate search overview-only matches behind title FTS - Always apply stats CTE + CROSS JOIN so single-word queries no longer flood results with description-only hits - Require overview_rank >= 0.15 for overview-only fallback rows - Switch title gate from contiguous LIKE to title_rank > 0 so reordered-token title matches aren't demoted --- internal/catalog/item_repo.go | 102 ++++++++++++++------------ internal/catalog/item_repo_test.go | 69 ++++++++++++++--- internal/catalog/window_count_test.go | 69 +++++++++-------- 3 files changed, 149 insertions(+), 91 deletions(-) diff --git a/internal/catalog/item_repo.go b/internal/catalog/item_repo.go index c2ebcb23..9788a91c 100644 --- a/internal/catalog/item_repo.go +++ b/internal/catalog/item_repo.go @@ -35,6 +35,16 @@ func NewItemRepository(pool *pgxpool.Pool) *ItemRepository { return &ItemRepository{pool: pool} } +// overviewMatchFloor is the minimum ts_rank_cd score an overview-only match +// must achieve to be returned when no title FTS match exists in the candidate +// set. The overview tsvector is built without setweight(), so PostgreSQL's +// default D weight (0.1) applies: a single occurrence of a single-term query +// scores ~0.1. A floor of 0.15 effectively requires more than one occurrence +// (or a tightly clustered match for multi-term queries), which keeps niche +// description-only searches viable while suppressing the long tail of +// incidental one-mention hits that flooded results before. +const overviewMatchFloor = 0.15 + // itemColumns is the list of columns returned by all SELECT queries on media_items. const itemColumns = `content_id, type, title, sort_title, default_metadata_language, original_title, year, genres, content_rating, runtime, overview, tagline, @@ -773,17 +783,19 @@ func (r *ItemRepository) Search(ctx context.Context, query string, itemTypes []s // buildSearchSQL assembles the unified search query, returning the SQL string // and bound args (or empty string when the input parses to no searchable text). // -// The query always uses a single `WITH scored AS (...)` CTE that aggregates -// per-content_id ranking signals. The final SELECT off the CTE selects -// itemColumns plus COUNT(*) OVER () AS total_count, ordered by relevance and -// paged via LIMIT/OFFSET. When the parsed text spans multiple words, a strict -// title-match path is enabled: a `stats` CTE is computed off `scored`, and the -// final SELECT cross-joins it with a WHERE clause that suppresses non-matching -// rows when at least one row has a strong title match. Because the window -// count runs in the final SELECT (after the cross-joined WHERE), the total -// reflects the post-filter count automatically. +// The query uses two CTEs: `scored` aggregates per-content_id ranking signals +// (title_rank, overview_rank, phrase_rank, plus exact/contiguous/year match +// flags), and `stats` derives a single has_title_match boolean over the +// scored set. The final SELECT CROSS JOINs stats and applies a WHERE that +// keeps every row whose title FTS rank is positive, plus overview-only rows +// when no title match exists in the candidate set AND the overview rank +// clears overviewMatchFloor. This suppresses single-word body-only matches +// for queries like "obsession" without harming queries where the term truly +// only appears in descriptions (those still surface as the fallback bucket). +// COUNT(*) OVER () in the final SELECT means the returned total reflects +// the post-filter row count automatically. // -// Argument order is intentionally fixed across both paths: +// Argument order is intentionally fixed: // // $1 searchText (always) // itemType placeholders, allowed/disabled libraries, MaxContentRating @@ -791,10 +803,6 @@ func (r *ItemRepository) Search(ctx context.Context, query string, itemTypes []s // parsed.Year (or NULL) // parsed.Phrase // limit, offset -// -// The single ExactTitleHint binding is reused by both the strict-only -// contiguous_title_match LIKE filter inside the CTE and the exact_title_match -// equality predicate used for ranking. func (r *ItemRepository) buildSearchSQL(query string, itemTypes []string, limit, offset int, filter AccessFilter) (dataSQL, countSQL string, args []any) { parsed := parseSearchQuery(query) searchText := parsed.Text @@ -804,9 +812,6 @@ func (r *ItemRepository) buildSearchSQL(query string, itemTypes []string, limit, if searchText == "" { return "", "", nil } - normalizedSearchText := normalizeTitleForComparison(searchText) - strictTitleFilter := len(strings.Fields(normalizedSearchText)) > 1 - var conditions []string args = []any{searchText} argIdx := 2 @@ -873,9 +878,9 @@ func (r *ItemRepository) buildSearchSQL(query string, itemTypes []string, limit, whereClause := "WHERE " + strings.Join(conditions, " AND ") // Bind ExactTitleHint exactly once. The same arg index is referenced by - // both contiguous_title_match (used as a ranking signal in all paths and - // as the strict-title CROSS JOIN filter when strictTitleFilter is true) - // and exact_title_match (used as a ranking signal in all paths). + // both contiguous_title_match and exact_title_match (used as ranking + // signals in the ORDER BY). The post-CTE WHERE itself gates on title_rank + // > 0 (true title FTS match), not on contiguous_title_match. exactIdx := argIdx args = append(args, parsed.ExactTitleHint) argIdx++ @@ -957,39 +962,44 @@ func (r *ItemRepository) buildSearchSQL(query string, itemTypes []string, limit, // COUNT(*) OVER () runs after the GROUP BY in the scored CTE collapses // duplicates from the library JOIN, so the window count preserves the // COUNT(DISTINCT mi.content_id) semantics of the prior 2-query path. - // In strict-title mode, placing the window count in the final SELECT - // (after the cross-joined WHERE filter) means the total reflects the + // The stats CTE + CROSS JOIN below means the window count reflects the // post-filter row count automatically. // + // The stats CTE gates on title_rank > 0 (true title FTS match) rather + // than contiguous_title_match (LIKE substring), so reordered-token title + // queries like "order law" → "Law and Order" aren't wrongly demoted to + // the overview-fallback bucket. The post-CTE WHERE then keeps every title + // FTS hit, and admits overview-only rows only when no title hit exists + // anywhere in the candidate set AND the overview rank clears + // overviewMatchFloor. This suppresses single-occurrence body matches + // (which score at PostgreSQL's default D weight of 0.1) for common + // single-word queries like "obsession" that would otherwise flood + // results with description-only hits. + // // countSQL is a count-only sibling that omits LIMIT/OFFSET/ORDER BY. It is // invoked only as a fallback when the data SELECT returns an empty page // past offset 0 — COUNT(*) OVER () emits no rows in that case so total // would otherwise default to 0. - if strictTitleFilter { - statsCTE := ` - , stats AS ( - SELECT MAX(contiguous_title_match) AS has_strong_title_match - FROM scored - )` - dataSQL = scoredCTE + statsCTE + fmt.Sprintf(` - SELECT %s, COUNT(*) OVER () AS total_count + statsCTE := ` + , stats AS ( + SELECT MAX(CASE WHEN title_rank > 0 THEN 1 ELSE 0 END) AS has_title_match FROM scored - CROSS JOIN stats - WHERE COALESCE(stats.has_strong_title_match, 0) = 0 OR scored.contiguous_title_match = 1 - ORDER BY exact_title_match DESC, contiguous_title_match DESC, year_match DESC, phrase_rank DESC, title_rank DESC, overview_rank DESC, LOWER(title) ASC, content_id ASC - LIMIT $%d OFFSET $%d`, itemColumns, argIdx, argIdx+1) - countSQL = scoredCTE + statsCTE + ` - SELECT COUNT(*) FROM scored - CROSS JOIN stats - WHERE COALESCE(stats.has_strong_title_match, 0) = 0 OR scored.contiguous_title_match = 1` - } else { - dataSQL = scoredCTE + fmt.Sprintf(` - SELECT %s, COUNT(*) OVER () AS total_count - FROM scored - ORDER BY exact_title_match DESC, contiguous_title_match DESC, year_match DESC, phrase_rank DESC, title_rank DESC, overview_rank DESC, LOWER(title) ASC, content_id ASC - LIMIT $%d OFFSET $%d`, itemColumns, argIdx, argIdx+1) - countSQL = scoredCTE + `SELECT COUNT(*) FROM scored` - } + )` + // postFilter is the FROM + CROSS JOIN + WHERE shared by both dataSQL and + // countSQL. Keeping it in one string ensures the empty-page fallback count + // can never drift from the data query's filter. + postFilter := fmt.Sprintf(`FROM scored + CROSS JOIN stats + WHERE scored.title_rank > 0 + OR (COALESCE(stats.has_title_match, 0) = 0 AND scored.overview_rank >= %g)`, overviewMatchFloor) + dataSQL = scoredCTE + statsCTE + fmt.Sprintf(` + SELECT %s, COUNT(*) OVER () AS total_count + %s + ORDER BY exact_title_match DESC, contiguous_title_match DESC, year_match DESC, phrase_rank DESC, title_rank DESC, overview_rank DESC, LOWER(title) ASC, content_id ASC + LIMIT $%d OFFSET $%d`, itemColumns, postFilter, argIdx, argIdx+1) + countSQL = scoredCTE + statsCTE + fmt.Sprintf(` + SELECT COUNT(*) + %s`, postFilter) args = append(args, limit, offset) return dataSQL, countSQL, args } diff --git a/internal/catalog/item_repo_test.go b/internal/catalog/item_repo_test.go index b8c396ad..064bb116 100644 --- a/internal/catalog/item_repo_test.go +++ b/internal/catalog/item_repo_test.go @@ -1,6 +1,7 @@ package catalog import ( + "fmt" "strings" "testing" ) @@ -238,25 +239,73 @@ func TestItemRepo_Search_UsesWindowCount(t *testing.T) { } } -// TestItemRepo_Search_StrictTitleFilter_UsesWindowCount asserts the unified -// query also applies to the multi-word "strict title filter" path: the stats -// CTE is computed off the scored CTE, and the window count still runs on the -// final filtered result so the total reflects the strict-title CROSS JOIN -// filter rather than the broader pre-filter set. -func TestItemRepo_Search_StrictTitleFilter_UsesWindowCount(t *testing.T) { +// TestItemRepo_Search_TitleGate_UsesWindowCount asserts the unified query +// pairs the scored CTE with a stats CTE that derives has_title_match, and +// that the window count runs on the final filtered result so the total +// reflects the title-gate CROSS JOIN filter rather than the broader +// pre-filter set. +func TestItemRepo_Search_TitleGate_UsesWindowCount(t *testing.T) { repo := &ItemRepository{} sql, _, _ := repo.buildSearchSQL("the matrix reloaded", []string{"movie"}, 20, 0, AccessFilter{}) if !strings.Contains(sql, "COUNT(*) OVER ()") { - t.Fatalf("expected COUNT(*) OVER () in strict-title path; got %s", sql) + t.Fatalf("expected COUNT(*) OVER () in title-gate path; got %s", sql) } if strings.Count(sql, "WITH scored AS") != 1 { t.Fatalf("expected exactly one scored CTE; got %s", sql) } if !strings.Contains(sql, "stats AS") { - t.Fatalf("expected stats CTE for strict-title filtering; got %s", sql) + t.Fatalf("expected stats CTE; got %s", sql) } - if !strings.Contains(sql, "has_strong_title_match") { - t.Fatalf("expected strict-title filter predicate; got %s", sql) + if !strings.Contains(sql, "has_title_match") { + t.Fatalf("expected has_title_match predicate; got %s", sql) + } +} + +// TestItemRepo_Search_SingleWordEnablesTitleGate pins the bug fix for the +// "obsession returns 2000 results" report: even single-word queries must +// route through the stats CTE + CROSS JOIN, so overview-only matches are +// suppressed whenever any title match exists. Prior to this, single-word +// queries skipped the stats CTE entirely and returned every row where the +// search term appeared in the description. +func TestItemRepo_Search_SingleWordEnablesTitleGate(t *testing.T) { + repo := &ItemRepository{} + sql, _, _ := repo.buildSearchSQL("obsession", []string{"movie"}, 20, 0, AccessFilter{}) + if !strings.Contains(sql, "stats AS") { + t.Fatalf("expected single-word query to include the stats CTE; got %s", sql) + } + if !strings.Contains(sql, "CROSS JOIN stats") { + t.Fatalf("expected single-word query to CROSS JOIN stats; got %s", sql) + } + if !strings.Contains(sql, "scored.title_rank > 0") { + t.Fatalf("expected title gate to use title_rank > 0; got %s", sql) + } +} + +// TestItemRepo_Search_AppliesOverviewRankFloor pins that the overview-only +// fallback arm is gated by overviewMatchFloor, so weak single-occurrence +// description matches do not pass through when no title match exists. The +// floor literal is derived from the constant so the test stays in sync if +// the threshold is retuned. +func TestItemRepo_Search_AppliesOverviewRankFloor(t *testing.T) { + repo := &ItemRepository{} + dataSQL, countSQL, _ := repo.buildSearchSQL("obsession", []string{"movie"}, 20, 0, AccessFilter{}) + want := fmt.Sprintf("scored.overview_rank >= %g", overviewMatchFloor) + if !strings.Contains(dataSQL, want) { + t.Fatalf("expected %q in dataSQL; got %s", want, dataSQL) + } + if !strings.Contains(countSQL, want) { + t.Fatalf("expected %q in countSQL too (must mirror dataSQL); got %s", want, countSQL) + } +} + +// TestItemRepo_Search_EmptyQueryReturnsEmpty pins the early-return contract +// when input parses to no searchable text. Downstream callers rely on +// (dataSQL == "") to short-circuit without binding any args. +func TestItemRepo_Search_EmptyQueryReturnsEmpty(t *testing.T) { + repo := &ItemRepository{} + dataSQL, countSQL, args := repo.buildSearchSQL(" ", nil, 20, 0, AccessFilter{}) + if dataSQL != "" || countSQL != "" || args != nil { + t.Fatalf("expected (\"\", \"\", nil) for whitespace-only query; got (%q, %q, %v)", dataSQL, countSQL, args) } } diff --git a/internal/catalog/window_count_test.go b/internal/catalog/window_count_test.go index 4eaba97f..5f5c5f39 100644 --- a/internal/catalog/window_count_test.go +++ b/internal/catalog/window_count_test.go @@ -272,43 +272,42 @@ func TestBuildBrowseFavoritesPlan_CountSQL_OmitsLimitOffsetOrderBy(t *testing.T) // TestItemRepo_Search_CountSQL_OmitsLimitOffsetOrderBy pins the same // empty-page fallback contract for the Search path. The count sibling must -// preserve the strict-title CROSS JOIN filter so the recovered total -// reflects the post-filter row count (matching COUNT(*) OVER () semantics). +// preserve the title-gate CROSS JOIN filter so the recovered total reflects +// the post-filter row count (matching COUNT(*) OVER () semantics on the +// data SELECT). Single-word and multi-word queries share one SQL shape. func TestItemRepo_Search_CountSQL_OmitsLimitOffsetOrderBy(t *testing.T) { repo := &ItemRepository{} - // Single-word path: count from scored CTE. - _, countSQL, _ := repo.buildSearchSQL("avatar", []string{"movie"}, 20, 0, AccessFilter{}) - if !strings.Contains(countSQL, "WITH scored AS") { - t.Fatalf("countSQL must include scored CTE; got:\n%s", countSQL) - } - if !strings.Contains(countSQL, "SELECT COUNT(*) FROM scored") { - t.Fatalf("expected SELECT COUNT(*) FROM scored; got:\n%s", countSQL) - } - if strings.Contains(countSQL, "LIMIT ") { - t.Fatalf("countSQL must omit LIMIT; got:\n%s", countSQL) - } - if strings.Contains(countSQL, "OFFSET ") { - t.Fatalf("countSQL must omit OFFSET; got:\n%s", countSQL) - } - if strings.Contains(countSQL, "ORDER BY") { - t.Fatalf("countSQL must omit ORDER BY; got:\n%s", countSQL) - } - if strings.Contains(countSQL, "COUNT(*) OVER ()") { - t.Fatalf("countSQL must use plain COUNT(*); got:\n%s", countSQL) - } - - // Strict-title path: count must apply the same CROSS JOIN stats filter so - // the recovered total reflects post-filter row count (not the broader - // pre-filter set). Otherwise the fallback would over-count. - _, strictCountSQL, _ := repo.buildSearchSQL("the matrix reloaded", []string{"movie"}, 20, 0, AccessFilter{}) - if !strings.Contains(strictCountSQL, "stats AS") { - t.Fatalf("strict-title countSQL must include stats CTE; got:\n%s", strictCountSQL) - } - if !strings.Contains(strictCountSQL, "CROSS JOIN stats") { - t.Fatalf("strict-title countSQL must CROSS JOIN stats so the recovered total reflects the post-filter set; got:\n%s", strictCountSQL) - } - if !strings.Contains(strictCountSQL, "has_strong_title_match") { - t.Fatalf("strict-title countSQL must apply the strict-title predicate; got:\n%s", strictCountSQL) + for _, query := range []string{"avatar", "the matrix reloaded"} { + t.Run(query, func(t *testing.T) { + _, countSQL, _ := repo.buildSearchSQL(query, []string{"movie"}, 20, 0, AccessFilter{}) + if !strings.Contains(countSQL, "WITH scored AS") { + t.Fatalf("countSQL must include scored CTE; got:\n%s", countSQL) + } + if !strings.Contains(countSQL, "stats AS") { + t.Fatalf("countSQL must include stats CTE; got:\n%s", countSQL) + } + if !strings.Contains(countSQL, "CROSS JOIN stats") { + t.Fatalf("countSQL must CROSS JOIN stats so the recovered total reflects the post-filter set; got:\n%s", countSQL) + } + if !strings.Contains(countSQL, "has_title_match") { + t.Fatalf("countSQL must apply the title-gate predicate; got:\n%s", countSQL) + } + if !strings.Contains(countSQL, "SELECT COUNT(*)") { + t.Fatalf("expected SELECT COUNT(*); got:\n%s", countSQL) + } + if strings.Contains(countSQL, "LIMIT ") { + t.Fatalf("countSQL must omit LIMIT; got:\n%s", countSQL) + } + if strings.Contains(countSQL, "OFFSET ") { + t.Fatalf("countSQL must omit OFFSET; got:\n%s", countSQL) + } + if strings.Contains(countSQL, "ORDER BY") { + t.Fatalf("countSQL must omit ORDER BY; got:\n%s", countSQL) + } + if strings.Contains(countSQL, "COUNT(*) OVER ()") { + t.Fatalf("countSQL must use plain COUNT(*); got:\n%s", countSQL) + } + }) } }