fix(filters): blank-typed browse 500 on id-valued fields (library/collection/added_by)

The browse-on-click blank sentinel was trimmed to empty (correct), but
the typed query had already been bound unconditionally — the id-source
suggestion query has no score CASE to reference it, leaving an unused
parameter Postgres rejects (42P18). Every click of an empty library /
collection / added-by box answered 'No matching values' via the error
path.

- ComposedFieldValues binds the typed query ONLY in branches that
  reference it and only when non-blank; the score expression is
  '0 AS score' for browse (no param) and the similarity CASE when
  typing. composedIDSourceValues takes the raw typed string and binds
  it only when the name filter is built. userBind already followed
  this discipline; audited every bind site in the file (fuzzy helpers,
  composed search, count) — all interpolate immediately.
- Tests: library browse under rules (exactly the surviving library,
  count pinned), series browse blank, added_by browse blank (no
  error), collection browse blank (offered with count) and under a
  zero-match rule (never offered). Verified end-to-end against the
  live dev data: author+Sapkowski AND narrator+Kenny → Audiobooks (8).
This commit is contained in:
John O'Keefe committed 2026-10-20 16:44:50 -04:00
1 parent 9c006a9a2f
commit ab713e2feb
2 files changed
+80 -14

No files matched your search

+52
View File
@@ -547,6 +547,58 @@ func TestFilterSuggestionsUnderRules(t *testing.T) {
} }
} }
assert.True(t, found, "the visible library is offered") assert.True(t, found, "the visible library is offered")
// BROWSE the library field UNDER rules (the blank-typed click): the
// id-source query must not choke on the unused typed parameter and
// must list exactly the library holding the surviving books.
status, results = suggest("library", " ", qfAll(qfRule("author", "contains", "Voss")))
require.Equal(t, http.StatusOK, status, "library browse under rules (blank typed)")
require.Len(t, results, 1, "exactly the fixture library survives the rules")
assert.Equal(t, libUUID.String(), results[0]["value"])
assert.InDelta(t, float64(2), results[0]["count"], 0.001, "both Voss books live there")
// Series browse (blank typed, under rules): full-membership source.
status, results = suggest("series", " ", qfAll(qfRule("author", "contains", "Voss")))
require.Equal(t, http.StatusOK, status)
assert.Contains(t, valuesOf(results), "Voss Saga")
// Added-by browse (blank typed): books without added_by → an empty
// menu, but NO error (the parameter-discipline regression net).
status, results = suggest("added_by", " ", "")
require.Equal(t, http.StatusOK, status)
// Collection browse (blank typed): the container sources ride the
// same id-source query — a collection holding a surviving book is
// offered with the surviving count; zero-match containers never are.
userA := getRegularUserID(t, setup.DB)
var collID pgtype.UUID
err = setup.DBPool.QueryRow(ctx,
`INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`,
pgtype.UUID{Bytes: [16]byte(userA), Valid: true},
fmt.Sprintf("Test Builder Sug Coll %d", time.Now().UnixNano())).Scan(&collID)
require.NoError(t, err)
t.Cleanup(func() { _ = setup.DB.DeleteCollection(ctx, collID) })
var tideID pgtype.UUID
err = setup.DBPool.QueryRow(ctx, `SELECT id FROM media_items WHERE title = $1`, "Sug Tide").Scan(&tideID)
require.NoError(t, err)
_, err = setup.DBPool.Exec(ctx,
`INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1, $2)`, collID, tideID)
require.NoError(t, err)
status, results = suggest("collection", " ", "")
require.Equal(t, http.StatusOK, status)
assert.True(t, func() bool {
for _, r := range results {
if r["value"] == collID.String() {
return true
}
}
return false
}(), "the collection holding a book is offered on browse")
status, results = suggest("collection", " ", qfAll(qfRule("genre", "is", "spacewestern")))
require.Equal(t, http.StatusOK, status)
assert.Empty(t, results, "zero-match containers never offered")
} }
func TestFilterFamilyLens(t *testing.T) { func TestFilterFamilyLens(t *testing.T) {
+28 -14
View File
@@ -869,15 +869,18 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
p.Typed = strings.TrimSpace(p.Typed) p.Typed = strings.TrimSpace(p.Typed)
b := newFilterBuilder(p.UserID) b := newFilterBuilder(p.UserID)
typedBind := b.bind(p.Typed)
limitBind := b.bind(p.Limit) limitBind := b.bind(p.Limit)
libBind := b.bind(p.LibraryID) libBind := b.bind(p.LibraryID)
// The typed filter differs per source family: substring for the // The typed filter differs per source family: substring for the
// viewer's own words (note/highlight), name-filter for the id // viewer's own words (note/highlight), name-filter for the id
// sources (library/collection/added_by — you find them by name), // sources (library/collection/added_by — you find them by name),
// word_similarity for metadata text. userBind is bound ONLY for the // word_similarity for metadata text.
// viewer-scoped sources — an unused parameter is a 42P18 error. //
// PARAMETER DISCIPLINE: every bind must be USED by the query it
// lands in — an unused parameter is a 42P18 error. The typed query
// is bound only in the branches that reference it, and only when
// non-blank (blank typed = browse: no typed filter, score 0).
var userBind string var userBind string
var valueSource, typedFilter string var valueSource, typedFilter string
switch field.Suggest { switch field.Suggest {
@@ -893,7 +896,7 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
WHERE t.media_item_id = mi.id AND t.user_id = %s WHERE t.media_item_id = mi.id AND t.user_id = %s
) AS src`, col, table, userBind) ) AS src`, col, table, userBind)
if p.Typed != "" { if p.Typed != "" {
typedFilter = " AND src.value ILIKE '%' || " + typedBind + " || '%'" typedFilter = " AND src.value ILIKE '%' || " + b.bind(p.Typed) + " || '%'"
} }
case suggestSeries: case suggestSeries:
valueSource = `CROSS JOIN LATERAL ( valueSource = `CROSS JOIN LATERAL (
@@ -901,20 +904,22 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
FROM unnest(ARRAY[mi.series, mi.alternate_info->>'alternate_series']) AS s FROM unnest(ARRAY[mi.series, mi.alternate_info->>'alternate_series']) AS s
WHERE s IS NOT NULL AND s <> '' WHERE s IS NOT NULL AND s <> ''
) AS src` ) AS src`
typedFilter = " AND word_similarity(" + typedBind + ", src.value) > 0.3" if p.Typed != "" {
typedFilter = " AND word_similarity(" + b.bind(p.Typed) + ", src.value) > 0.3"
}
case suggestLibrary: case suggestLibrary:
return s.composedIDSourceValues(ctx, b, p, "libraries l2", "l2.id::text", "l2.name", return s.composedIDSourceValues(ctx, b, p, "libraries l2", "l2.id::text", "l2.name",
"JOIN media_items mi ON mi.library_id = l2.id", typedBind, limitBind, libBind) "JOIN media_items mi ON mi.library_id = l2.id", limitBind, libBind)
case suggestCollection: case suggestCollection:
return s.composedIDSourceValues(ctx, b, p, return s.composedIDSourceValues(ctx, b, p,
"collections c JOIN collection_items ci ON ci.collection_id = c.id", "collections c JOIN collection_items ci ON ci.collection_id = c.id",
"c.id::text", "c.name", "c.id::text", "c.name",
"JOIN media_items mi ON ci.media_item_id = mi.id AND ci.excluded IS NOT TRUE", "JOIN media_items mi ON ci.media_item_id = mi.id AND ci.excluded IS NOT TRUE",
typedBind, limitBind, libBind) limitBind, libBind)
case suggestAddedBy: case suggestAddedBy:
return s.composedIDSourceValues(ctx, b, p, return s.composedIDSourceValues(ctx, b, p,
"users u JOIN media_items mi ON mi.added_by_admin_id = u.id", "users u JOIN media_items mi ON mi.added_by_admin_id = u.id",
"u.id::text", "u.username", "", typedBind, limitBind, libBind) "u.id::text", "u.username", "", limitBind, libBind)
default: default:
// Column / array sources: the value expr mirrors the field's // Column / array sources: the value expr mirrors the field's
// match column (tags via the *_search column, like the flat // match column (tags via the *_search column, like the flat
@@ -937,10 +942,17 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
} }
valueSource = "CROSS JOIN LATERAL (SELECT " + col + " AS value WHERE " + col + " IS NOT NULL AND " + col + " <> '') AS src" valueSource = "CROSS JOIN LATERAL (SELECT " + col + " AS value WHERE " + col + " IS NOT NULL AND " + col + " <> '') AS src"
} }
typedFilter = " AND word_similarity(" + typedBind + ", src.value) > 0.3" if p.Typed != "" {
typedFilter = " AND word_similarity(" + b.bind(p.Typed) + ", src.value) > 0.3"
} }
if p.Typed == "" { }
typedFilter = "" // no typing = the field's world, under the other rules
// Score: similarity ranking only when something is typed; browse
// orders by count alone (and binds nothing extra).
scoreExpr := "0 AS score"
if p.Typed != "" {
tb := b.bind(p.Typed)
scoreExpr = fmt.Sprintf("CASE WHEN %s = '' THEN 0 ELSE word_similarity(%s, src.value) END AS score", tb, tb)
} }
whereSQL := b.headWhere(p.LibraryID) + ` whereSQL := b.headWhere(p.LibraryID) + `
@@ -948,7 +960,7 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
treeWhere(p.Tree, b) + ` treeWhere(p.Tree, b) + `
AND src.value IS NOT NULL AND src.value <> ''` + typedFilter AND src.value IS NOT NULL AND src.value <> ''` + typedFilter
sqlStr := `SELECT src.value, COUNT(*) AS count, sqlStr := `SELECT src.value, COUNT(*) AS count,
CASE WHEN ` + typedBind + ` = '' THEN 0 ELSE word_similarity(` + typedBind + `, src.value) END AS score ` + scoreExpr + `
` + filterFrom + b.userIDBind + ` ` + filterFrom + b.userIDBind + `
` + valueSource + b.joinBlock() + ` ` + valueSource + b.joinBlock() + `
WHERE ` + whereSQL + ` WHERE ` + whereSQL + `
@@ -985,10 +997,12 @@ LIMIT ` + limitBind
// added_by): value = id, display = name, typing filters the NAME, and // added_by): value = id, display = name, typing filters the NAME, and
// counts are books-in-scope. Zero-count sources drop out (INNER JOIN). // counts are books-in-scope. Zero-count sources drop out (INNER JOIN).
func (s *SearchService) composedIDSourceValues(ctx context.Context, b *filterBuilder, p ComposedSuggestParams, func (s *SearchService) composedIDSourceValues(ctx context.Context, b *filterBuilder, p ComposedSuggestParams,
fromTables, valueExpr, displayExpr, joinMedia, typedBind, limitBind, libBind string) ([]FieldValue, error) { fromTables, valueExpr, displayExpr, joinMedia, limitBind, libBind string) ([]FieldValue, error) {
// The typed bind happens HERE and only when non-blank — an unused
// parameter is a 42P18 error (the blank-browse case binds nothing).
nameFilter := "" nameFilter := ""
if p.Typed != "" { if p.Typed != "" {
nameFilter = " AND " + displayExpr + " ILIKE '%' || " + typedBind + " || '%'" nameFilter = " AND " + displayExpr + " ILIKE '%' || " + b.bind(p.Typed) + " || '%'"
} }
sqlStr := `SELECT ` + valueExpr + ` AS value, ` + displayExpr + ` AS display, COUNT(mi.id) AS count sqlStr := `SELECT ` + valueExpr + ` AS value, ` + displayExpr + ` AS display, COUNT(mi.id) AS count
FROM ` + fromTables + ` FROM ` + fromTables + `