feat(filters): scopes + episode fields — the engine becomes the single query system (item 3.1)

The qf tree gains an optional top-level `scope` ("books" default |
"episodes" — absent = books, so every existing saved tree and legacy
client stays valid). The row head is scope-aware AND this closes a
real leak: the composed engine's head had NO podcasts exclusion, so
episodes already surfaced on All-Books qf queries (plan §9 violation —
the sqlc flat head excluded them, the composed head didn't). Books
scope now says library_type_name <> 'podcasts'; episodes scope is
source = 'podcast_episode'.

The registry fields declare their scopes (the plan's "reusable
template with conditionals" = scope-driven rendering in 3.3): episode
fields declare exactly the episodes scope and the books scope REJECTS
them at parse (the "hide podcasts from the rules on the all books
page" rule enforced server-side); books fields inside episodes trees
are allowed and honestly no-op on NULLs.

New fields (the item-2.4 rule vocabulary expressed in SQL):
- feed — KindIdentifier over podcast_feeds (RLS-governed suggestion
  source and match; unsubscribed feeds match nothing, by the engine)
- downloaded — is_true/is_false on download_state = 'downloaded'
- played — THE canonical finished predicate (viewer progress >= 0.999,
  the hide_finished definition shared with item 2.4), reading_progress
  join per viewer
- published_date — clauseDateFactory like added_at, within_days
  default op (the published_within rule's SQL form)

Engine hygiene: the stray ComposedSearch SQL debug printf is gone.

Pinned by TestFilterScopeValidation (unit: scope grammar, books×
episode-field 400s, legacy no-scope trees) and
TestFilterScopesAndEpisodeFields (integration: books head excludes
podcasts including the 404-no-results contract, episodes head is
episodes only, feed-in/downloaded/played/published-date evaluate in
SQL against a subscribed-feed fixture with a played viewer).
This commit is contained in:
John O'Keefe committed 2026-10-30 18:19:55 -04:00
1 parent bc91ce68d4
commit 56871b65ed
4 files changed
+304 -13

No files matched your search

+109
View File
@@ -295,6 +295,115 @@ func TestFilterComposedSemantics(t *testing.T) {
assert.Equal(t, []string{"Sem Tide"}, got, "published_year at_least")
}
// TestFilterScopesAndEpisodeFields pins item 3.1: the books scope
// excludes podcasts (the leak the composed head used to have), the
// episodes scope is episodes only, episode fields evaluate in SQL
// (feed-in, downloaded, played, published-date-within), and episode
// fields are 400s in books trees.
func TestFilterScopesAndEpisodeFields(t *testing.T) {
setup := setupTestServer(t)
t.Cleanup(func() { _ = setup.Close() })
regUUID := getRegularUserID(t, setup.DB)
builderFixtureLib(t, setup, regUUID, "Scope")
// Podcasts library + a subscribed feed with three downloaded
// episodes: two fresh/unplayed, one played (~finished) — the
// played-rule fixture.
feedLib, _ := createPodcastsLibrary(t, setup, "test Scope Podcasts")
feed, err := setup.DB.CreatePodcastFeed(adminSeedCtx(t, setup), database.CreatePodcastFeedParams{
LibraryID: feedLib.ID,
FeedUrl: "https://feed.invalid/scope-probe.xml",
Title: "Scope Probe Feed",
})
require.NoError(t, err)
epFresh := upsertTestEpisode(t, setup, feed, "scope-fresh", "Scope Fresh Episode", 1)
epPlayed := upsertTestEpisode(t, setup, feed, "scope-played", "Scope Played Episode", 2)
upsertTestEpisode(t, setup, feed, "scope-pending", "Scope Pending Episode", 3)
subscribeRaw(t, setup, regUUID, feed.ID)
adminScopeExec(t, setup, adminCtxValue(t, setup), `
UPDATE media_items SET download_state = 'downloaded', file_size = 1000,
date_published = CURRENT_DATE WHERE id IN ($1, $2)`, epFresh.ID, epPlayed.ID)
adminScopeExec(t, setup, adminCtxValue(t, setup), `
INSERT INTO reading_progress (media_item_id, user_id, percentage)
VALUES ($1, $2, 1.0) ON CONFLICT (media_item_id, user_id)
DO UPDATE SET percentage = 1.0`, epPlayed.ID, regUUID)
feedID := uuid.UUID(feed.ID.Bytes).String()
qfScoped := func(scope string, children ...map[string]interface{}) url.Values {
b, _ := json.Marshal(map[string]interface{}{"scope": scope, "mode": "all", "children": children})
return url.Values{"qf": {string(b)}}
}
const booksScope = "books"
const episodesScope = "episodes"
search := func(p url.Values) []string {
rows := p
status, body := qfSearch(t, setup, setup.RegularToken, rows)
// 404 is the endpoint's pinned no-results contract — an empty set.
require.Contains(t, []int{http.StatusOK, http.StatusNotFound}, status, "qf must parse: %v", rows)
return qfTitles(t, body)
}
t.Run("books_scope_excludes_podcasts", func(t *testing.T) {
// An exact-ish title hits BOTH a book fixture ("Scope Tide") and
// the episode fixture if the head leaks; books scope must return
// the book, never the episodes.
got := search(qfScoped(booksScope, qfRule("title", "is", "Scope Tide")))
assert.Equal(t, []string{"Scope Tide"}, got, "books scope returns the book")
got = search(qfScoped(booksScope, qfRule("title", "is", "Scope Fresh Episode")))
assert.Empty(t, got,
"books scope must never return podcast episodes (the composed-head leak is CLOSED)")
})
t.Run("episodes_scope_is_episodes_only", func(t *testing.T) {
got := search(qfScoped(episodesScope, qfRule("title", "is", "Scope Fresh Episode")))
assert.Equal(t, []string{"Scope Fresh Episode"}, got,
"episodes scope: only episodes — books drop")
})
t.Run("episode_rules_evaluate_in_sql", func(t *testing.T) {
got := search(qfScoped(episodesScope,
qfRule("feed", "in", feedID)))
assert.ElementsMatch(t, []string{"Scope Fresh Episode", "Scope Played Episode", "Scope Pending Episode"},
got, "feed-in matches every episode of the feed (download state is a separate rule)")
got = search(qfScoped(episodesScope,
qfRule("downloaded", "is_true")))
assert.ElementsMatch(t, []string{"Scope Fresh Episode", "Scope Played Episode"},
got, "downloaded:true drops the pending episode")
got = search(qfScoped(episodesScope,
qfRule("played", "is_false"),
qfRule("downloaded", "is_true")))
assert.ElementsMatch(t, []string{"Scope Fresh Episode"}, got,
"played:false + downloaded = the unplayed playable episode (viewer progress)")
got = search(qfScoped(episodesScope,
qfRule("played", "is_true")))
assert.ElementsMatch(t, []string{"Scope Played Episode"}, got,
"played:true matches the finished episode")
got = search(qfScoped(episodesScope,
qfRule("published_date", "within_days", "7"),
qfRule("downloaded", "is_true")))
assert.ElementsMatch(t, []string{"Scope Fresh Episode", "Scope Played Episode"},
got, "published-within includes the dated episodes")
})
t.Run("episode_fields_are_400_in_books_scope", func(t *testing.T) {
p := qfScoped(booksScope, qfRule("played", "is_true"))
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/media-items/search?"+p.Encode(), nil)
req.Header.Set("Authorization", "Bearer "+setup.RegularToken)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
defer func() { _ = resp.Body.Close() }()
assert.Equal(t, http.StatusBadRequest, resp.StatusCode,
"episode-only fields are rejected in books trees at parse time")
})
}
func TestFilterUserRelativeFields(t *testing.T) {
setup := setupTestServer(t)
t.Cleanup(func() { _ = setup.Close() })
+68 -8
View File
@@ -75,11 +75,20 @@ JOIN libraries l ON mi.library_id = l.id
JOIN library_types lt ON l.library_type_id = lt.id
LEFT JOIN library_visibility lv ON l.id = lv.library_id AND lv.user_id = `
// headWhere renders the mandatory WHERE head: the family lens +
// integrity facts, plus the optional library scope (validated upstream —
// an explicit out-of-scope library never reaches the SQL).
func (b *filterBuilder) headWhere(libraryID pgtype.UUID) string {
// headWhere renders the mandatory WHERE head: the SCOPE (books excludes
// podcasts; episodes is episodes only — plan item 3.1 closes the leak
// where the composed head happily returned episodes on All-Books
// queries), the family lens, integrity facts, plus the optional library
// scope (validated upstream — an explicit out-of-scope library never
// reaches the SQL).
func (b *filterBuilder) headWhere(libraryID pgtype.UUID, scope string) string {
w := "COALESCE(lv.is_visible, true) = true AND mi.archived_at IS NULL AND mi.missing_scan_count = 0"
switch scope {
case ScopeEpisodes:
w += " AND mi.source = 'podcast_episode'"
default: // books — the default legacy scope
w += " AND mi.library_type_name <> 'podcasts'"
}
if libraryID.Valid {
w += " AND mi.library_id = " + b.bind(libraryID)
}
@@ -637,6 +646,50 @@ func clauseLibrary(r FilterRule, b *filterBuilder) string {
// ─── tree → SQL ──────────────────────────────────────────────────────
// clauseFeed — the episode's source feed (episodes scope, item 3.1).
// Values are podcast-feed UUIDs; visibility rides RLS (podcast_feeds is
// governed by the same subscription predicate as episodes, so a rule
// naming an unsubscribed feed matches nothing).
func clauseFeed(r FilterRule, b *filterBuilder) string {
b.needJoin("pf", "LEFT JOIN podcast_feeds pf ON pf.id = mi.source_feed_id")
ids := make([]uuid.UUID, 0, len(r.Values))
for _, v := range r.Values {
if id, err := uuid.Parse(v); err == nil {
ids = append(ids, id)
}
}
in := "(pf.id = ANY(" + b.bind(ids) + "::uuid[]))"
switch r.Op {
case OpIs, OpIn:
return in
default:
return "(NOT " + in + ")"
}
}
// clauseDownloaded — the episode's download state (episodes scope).
// is_true = the playable file is on disk; is_false = anything else.
func clauseDownloaded(r FilterRule, b *filterBuilder) string {
if r.Op == OpIsTrue {
return "(mi.download_state = 'downloaded')"
}
return "(mi.download_state <> 'downloaded')"
}
// clausePlayed — THE episode finished predicate (episodes scope): the
// viewer's progress >= 0.999, the same definition as hide_finished and
// the item-2.4 rules engine. Ties to the viewer via the shared
// reading_progress join (R2/D5 lineage: progress is per-user).
func clausePlayed(r FilterRule, b *filterBuilder) string {
b.readingProgress()
if r.Op == OpIsTrue {
return "(COALESCE(rp.percentage, 0) >= 0.999)"
}
return "(COALESCE(rp.percentage, 0) < 0.999)"
}
// ─── tree → SQL (end) ─────────────────────────────────────────────────
// buildGroupClause renders a group: children joined by the group mode,
// each child a rule clause or a nested group. A group ANDs into its
// parent.
@@ -742,14 +795,13 @@ func (s *SearchService) ComposedSearch(ctx context.Context, params SearchParams)
// Build the WHERE first — clause construction requests the
// user-relative JOINs lazily, and joinBlock() must render AFTER
// they are collected.
whereSQL := b.headWhere(params.LibraryID) + treeWhere(params.FilterTree, b) + `
whereSQL := b.headWhere(params.LibraryID, params.FilterTree.FilterScope()) + treeWhere(params.FilterTree, b) + `
AND (` + libBind + `::uuid IS NULL OR mi.library_id = ` + libBind + `::uuid)
AND (` + qBind + ` = '' OR ` + composedQueryBlock(qBind, exactBind) + `)`
sqlStr := composedSelect + filterFrom + b.userIDBind +
b.joinBlock() + `
WHERE ` + whereSQL +
composedSortOrder(sortBind, qBind, exactBind) + ` ` + limitBind + ` OFFSET ` + offsetBind
fmt.Printf("[DEBUG COMPOSED] %s\n", sqlStr)
rows, err := s.raw.Query(ctx, sqlStr, b.args...)
if err != nil {
@@ -826,7 +878,7 @@ func composedQueryBlock(qBind, exactBind string) string {
func (s *SearchService) ComposedCount(ctx context.Context, userID, libraryID pgtype.UUID, tree *FilterTree) (int, error) {
b := newFilterBuilder(userID)
libBind := b.bind(libraryID)
whereSQL := b.headWhere(libraryID) + treeWhere(tree, b) + `
whereSQL := b.headWhere(libraryID, tree.FilterScope()) + treeWhere(tree, b) + `
AND (` + libBind + `::uuid IS NULL OR mi.library_id = ` + libBind + `::uuid)`
sqlStr := `SELECT COUNT(*) ` + filterFrom + b.userIDBind +
b.joinBlock() + `
@@ -921,6 +973,14 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
return s.composedIDSourceValues(ctx, b, p,
"users u JOIN media_items mi ON mi.added_by_admin_id = u.id",
"u.id::text", "u.username", "", limitBind, libBind)
case suggestFeed:
// The viewer's feed universe — RLS (podcast_feeds subscription
// predicate + admin escape) scopes both suggestions and matches.
return s.composedIDSourceValues(ctx, b, p,
"podcast_feeds pf",
"pf.id::text", "pf.title",
"JOIN media_items mi ON mi.source_feed_id = pf.id",
limitBind, libBind)
default:
// Column / array sources: the value expr mirrors the field's
// match column (tags via the *_search column, like the flat
@@ -956,7 +1016,7 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge
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, p.Tree.FilterScope()) + `
AND (` + libBind + `::uuid IS NULL OR mi.library_id = ` + libBind + `::uuid)` +
treeWhere(p.Tree, b) + `
AND src.value IS NOT NULL AND src.value <> ''` + typedFilter
+85 -5
View File
@@ -87,6 +87,7 @@ const (
suggestHighlight SuggestSource = "highlight_text"
suggestLibrary SuggestSource = "library" // visibility-scoped libraries
suggestCollection SuggestSource = "collection" // the viewer's collections
suggestFeed SuggestSource = "feed" // the viewer's subscribed feeds (RLS-governed)
suggestAddedBy SuggestSource = "added_by" // admins who added books in scope
)
@@ -99,8 +100,27 @@ const (
GroupComics = "Comics & manga"
GroupMyReading = "My reading"
GroupLibrary = "Library"
// Episodes (plan item 3.1, filter-system unification): fields that
// only make sense on podcast-episode rows. Hidden in the books
// scope ("hide podcasts from the rules on the all books page, but
// allow them in collections").
GroupEpisodes = "Episodes"
)
// Filter scopes (plan item 3.1): which row universe a tree matches.
// The top-level `scope` key on the qf tree declares it; absent = books
// (every existing saved tree and every legacy client stays valid).
const (
ScopeBooks = "books"
ScopeEpisodes = "episodes"
)
// episodeOnlyFields are meaningful ONLY in the episodes scope — a
// books-scope tree using one is a user error (400 at parse).
var episodeOnlyFields = map[string]bool{
"feed": true, "downloaded": true, "played": true,
}
// FilterRule is one leaf: field + operator + values (all bound params).
type FilterRule struct {
Field string
@@ -112,10 +132,21 @@ type FilterRule struct {
// children are rules or nested groups. Groups AND/OR their children
// together; a group ANDs into its parent.
type FilterTree struct {
Scope string `json:"scope,omitempty"`
Mode string `json:"mode"`
Children []FilterChild `json:"children"`
}
// FilterScope resolves the tree's row universe: an absent scope key is
// the books scope (legacy trees, saved filters, every pre-unification
// client). nil-safe (suggest paths can carry a nil tree).
func (t *FilterTree) FilterScope() string {
if t != nil && t.Scope == ScopeEpisodes {
return ScopeEpisodes
}
return ScopeBooks
}
// FilterChild is one node: a rule (Field set) or a group (Mode set).
type FilterChild struct {
Field string `json:"field,omitempty"`
@@ -135,6 +166,24 @@ type FilterField struct {
DefaultOp string // operator a new rule starts on (field-switch reset)
Clause func(r FilterRule, b *filterBuilder) string
Suggest SuggestSource
// Scopes limits the field to the listed scopes (nil = available in
// every scope — most book fields no-op against episode rows since
// episode rows carry NULLs there). Episode-only fields declare
// exactly [ScopeEpisodes]; the books scope rejects them at parse.
Scopes []string
}
// InScope reports whether the field may appear in a scope tree.
func (f FilterField) InScope(scope string) bool {
if len(f.Scopes) == 0 {
return true
}
for _, s := range f.Scopes {
if s == scope {
return true
}
}
return false
}
// opsWithValue / opsWithoutValue drive validation.
@@ -176,6 +225,9 @@ var filterFields = []FilterField{
{Key: "published_year", Label: "Published year", Group: GroupPublish, Kind: KindNumber,
Ops: []string{OpBetween, OpAtLeast, OpAtMost, OpEmpty}, DefaultOp: OpAtLeast,
Clause: clausePublishedYear, Suggest: suggestNone},
{Key: "published_date", Label: "Published date", Group: GroupPublish, Kind: KindDate,
Ops: []string{OpWithinDays, OpBetween, OpEmpty}, DefaultOp: OpWithinDays,
Clause: clauseDateFactory("mi.date_published"), Suggest: suggestNone},
// ── Topics ────────────────────────────────────────────────────
{Key: "genre", Label: "Genre", Group: GroupTopics, Kind: KindScalarFuzzy,
@@ -349,6 +401,23 @@ var filterFields = []FilterField{
Ops: []string{OpIn, OpNotIn, OpInAny, OpInNone}, DefaultOp: OpIn,
Clause: clauseCollection, Suggest: suggestCollection},
// ── Episodes (podcast-episode rows; episodes scope only) ──────
// Plan item 3.1 / §0 item 3: the filter engine gains the episode
// fields so smart episode collections can ride the SAME engine.
// `feed` suggests the viewer's subscribed feeds (RLS governs both
// suggest and match); `downloaded` reads the download state;
// `played` is THE finished predicate (viewer progress >= 0.999 —
// the same definition as hide_finished, item 2.4).
{Key: "feed", Label: "Feed", Group: GroupEpisodes, Kind: KindIdentifier,
Ops: []string{OpIs, OpIsNot, OpIn, OpNotIn, OpHas, OpMissing}, DefaultOp: OpIn,
Clause: clauseFeed, Suggest: suggestFeed, Scopes: []string{ScopeEpisodes}},
{Key: "downloaded", Label: "Downloaded", Group: GroupEpisodes, Kind: KindBool,
Ops: []string{OpIsTrue, OpIsFalse}, DefaultOp: OpIsTrue,
Clause: clauseDownloaded, Suggest: suggestNone, Scopes: []string{ScopeEpisodes}},
{Key: "played", Label: "Played", Group: GroupEpisodes, Kind: KindBool,
Ops: []string{OpIsTrue, OpIsFalse}, DefaultOp: OpIsFalse,
Clause: clausePlayed, Suggest: suggestNone, Scopes: []string{ScopeEpisodes}},
// ── Library (holding structure + family lens) ─────────────────
{Key: "library", Label: "Library", Group: GroupLibrary, Kind: KindLibrary,
// Values are library IDs; the menu only ever offers the viewer's
@@ -426,13 +495,16 @@ func ParseFilterTree(raw string) (*FilterTree, error) {
if err := json.Unmarshal([]byte(raw), &t); err != nil {
return nil, fmt.Errorf("invalid qf JSON: %w", err)
}
if err := validateGroup(&FilterChild{Mode: t.Mode, Children: t.Children}, 1); err != nil {
if t.Scope != "" && t.Scope != ScopeBooks && t.Scope != ScopeEpisodes {
return nil, fmt.Errorf("filter scope must be books|episodes, got %q", t.Scope)
}
if err := validateGroup(&FilterChild{Mode: t.Mode, Children: t.Children}, 1, t.FilterScope()); err != nil {
return nil, err
}
return &t, nil
}
func validateGroup(g *FilterChild, depth int) error {
func validateGroup(g *FilterChild, depth int, scope string) error {
if depth > maxFilterDepth {
return fmt.Errorf("filter group nesting deeper than %d", maxFilterDepth)
}
@@ -444,18 +516,19 @@ func validateGroup(g *FilterChild, depth int) error {
}
for _, c := range g.Children {
if c.Field == "" && c.Mode != "" {
if err := validateGroup(&c, depth+1); err != nil {
if err := validateGroup(&c, depth+1, scope); err != nil {
return err
}
continue
}
if err := validateRule(c); err != nil {
if err := validateRule(c, scope); err != nil {
return err
}
}
return nil
}
// numericParsers map each numeric field's wire values to the Go value
// bound into SQL — validation and clause-building share one parser per
// field, so a value that passes validation always binds.
@@ -475,11 +548,18 @@ var numericParsers = map[string]func(string) (any, error){
"pages_read": parseNumberFloat,
}
func validateRule(c FilterChild) error {
func validateRule(c FilterChild, scope string) error {
f, ok := FilterFieldByKey(c.Field)
if !ok {
return fmt.Errorf("unknown filter field %q", c.Field)
}
// Episode-only fields are a user error in a books tree (plan item
// 3.1: "hide podcasts from the rules on the all books page"). The
// reverse is NOT an error: book fields that no-op against episode
// rows (NULLs) are allowed in episodes trees.
if scope == ScopeBooks && episodeOnlyFields[c.Field] {
return fmt.Errorf("field %q only exists in the episodes scope", c.Field)
}
if !fieldOpsAllowed(f, c.Op) {
return fmt.Errorf("operator %q not allowed on field %q", c.Op, c.Field)
}
+42
View File
@@ -55,6 +55,15 @@ func TestFilterRegistryCompleteness(t *testing.T) {
"pages_read", "collection",
// holding structure
"library",
// episode quantifier (item 3.1)
"published_date",
}
// Episode-scope fields (item 3.1): they declare exactly the
// episodes scope; no other field declares a scope restriction.
for _, key := range []string{"feed", "downloaded", "played"} {
assert.True(t, seen[key], "episode filter field %q missing from the registry", key)
f, _ := FilterFieldByKey(key)
assert.Equal(t, []string{ScopeEpisodes}, f.Scopes, "%q must be episodes-scope only", key)
}
for _, key := range mandated {
assert.True(t, seen[key], "mandated filter field %q missing from the registry", key)
@@ -102,6 +111,39 @@ func TestFilterParseValidation(t *testing.T) {
invalid(`{"mode":"all","children":[{"field":"author","op":"contains"}]}`, "needs a value")
}
func TestFilterScopeValidation(t *testing.T) {
valid := func(raw string) {
_, err := ParseFilterTree(raw)
require.NoError(t, err, "expected %s to parse", raw)
}
invalid := func(raw, wantContain string) {
_, err := ParseFilterTree(raw)
require.Error(t, err, "expected %s to be rejected", raw)
require.Contains(t, err.Error(), wantContain)
}
// Legacy trees (no scope key) stay valid — the default is books.
valid(`{"mode":"all","children":[{"field":"author","op":"contains","values":["Voss"]}]}`)
// Books trees REJECT episode-only fields (the item-3 hide rule).
invalid(`{"scope":"books","mode":"all","children":[{"field":"played","op":"is_true"}]}`,
"only exists in the episodes scope")
invalid(`{"mode":"all","children":[{"field":"downloaded","op":"is_true"}]}`,
"only exists in the episodes scope")
// Episodes trees carry the episode fields.
valid(`{"scope":"episodes","mode":"all","children":[
{"field":"feed","op":"in","values":["12345678-1234-1234-1234-123456789012"]},
{"field":"downloaded","op":"is_true"},
{"field":"played","op":"is_false"},
{"field":"published_date","op":"within_days","values":["7"]},
{"field":"title","op":"contains","values":["news"]}]}`)
// Unknown scope values are rejected.
invalid(`{"scope":"manga","mode":"all","children":[{"field":"title","op":"contains","values":["x"]}]}`,
"scope must be books|episodes")
}
func TestFilterParseDepthCap(t *testing.T) {
// Nesting beyond maxFilterDepth is rejected.
raw := `{"mode":"all","children":[`