From 9bf7da2cbee64cb5c3f2e9e3748295a7313e5854 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 20:52:39 +0200 Subject: [PATCH] refactor(plex): dedup movie/show watch-state predicates into query.py (E3 S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `show` scope (all|watched|in_progress|unwatched) filter and the status CASE were hand-duplicated between unified_library (the grid) and facets.wsq — the exact drift risk the review flagged (a hidden movie kept out of the grid must not inflate facets). Extract them into four shared query.py helpers (movie_status_case, show_status_case, apply_movie_state_filter, apply_show_state_filter), model-agnostic (callers pass the joined state expressions), and point all three call sites at them. Pure behaviour-equivalence refactor: browse.py -67 lines, no query changes. Green under the E3 S1 parity net + query_conversion/facets/field-scoped/lang tests (348 DB-lane pass, ruff clean). --- backend/app/plex/query.py | 61 +++++++++++++++++++++++++++- backend/app/routes/plex/browse.py | 67 +++++-------------------------- 2 files changed, 71 insertions(+), 57 deletions(-) diff --git a/backend/app/plex/query.py b/backend/app/plex/query.py index bbc539c7..ca3b31ce 100644 --- a/backend/app/plex/query.py +++ b/backend/app/plex/query.py @@ -6,7 +6,7 @@ Pure query fragments — every function returns SQLAlchemy expressions or a para from dataclasses import dataclass from datetime import datetime, timedelta, timezone -from sqlalchemy import and_, func, or_, type_coerce +from sqlalchemy import and_, case, func, or_, type_coerce from sqlalchemy.dialects.postgresql import JSONB TS_CONFIG = "public.unaccent_simple" @@ -189,3 +189,62 @@ class LibraryFilters: def like_escape(s: str) -> str: return s.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + + +# --- Watch-state scope (shared by the /library grid AND the /facets aggregation) --------------- +# +# The `show` scope (all | watched | in_progress | unwatched) filters the browse result by per-user +# watch state. The SAME predicate has to be applied in two places — `unified_library` (the grid) and +# `facets.wsq` (so the facet counts reflect exactly the visible set) — and they MUST stay identical: +# a hidden movie kept out of the grid must not inflate the facets either. These helpers are the one +# home for that predicate so the two call sites can't drift (E3 S1; pinned by test_plex_facets_grid_ +# parity). Callers pass the already-joined state expressions, so this module stays model-agnostic: +# * movies — `st` is an aliased PlexState outer-joined on (item_id, user_id). +# * shows — `tot`/`wat`/`inp` are the coalesced totals from `rollup.agg_subquery`. + + +def movie_status_case(st): + """The per-movie status label (new | in_progress | watched) from its PlexState alias `st`.""" + return case( + (st.status == "watched", "watched"), + ( + and_(st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched")), + "in_progress", + ), + else_="new", + ) + + +def show_status_case(tot, wat, inp): + """The per-show status label rolled up from its episodes' totals (total/watched/in-progress).""" + return case( + (and_(tot > 0, wat >= tot), "watched"), + (or_(wat > 0, inp > 0), "in_progress"), + else_="new", + ) + + +def apply_movie_state_filter(query, st, show: str): + """Restrict a movie query to the `show` scope via its PlexState alias `st`. The default (`all`) + excludes hidden movies — the exclusion that must match between grid and facets.""" + if show == "watched": + return query.where(st.status == "watched") + if show == "in_progress": + return query.where( + st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched") + ) + if show == "unwatched": + return query.where(or_(st.status.is_(None), st.status.notin_(["watched", "hidden"]))) + return query.where(or_(st.status.is_(None), st.status != "hidden")) + + +def apply_show_state_filter(query, tot, wat, inp, show: str): + """Restrict a show query to the `show` scope via its rollup totals. `all` is a no-op (shows have + no hidden concept); the caller only joins the rollup for the three non-`all` scopes.""" + if show == "watched": + return query.where(tot > 0, wat >= tot) + if show == "in_progress": + return query.where(or_(wat > 0, inp > 0), wat < tot) + if show == "unwatched": + return query.where(wat == 0, inp == 0) + return query diff --git a/backend/app/routes/plex/browse.py b/backend/app/routes/plex/browse.py index 0a33bcb8..637dcb30 100644 --- a/backend/app/routes/plex/browse.py +++ b/backend/app/routes/plex/browse.py @@ -11,7 +11,6 @@ from sqlalchemy import ( Integer, String, and_, - case, cast, delete, func, @@ -50,10 +49,14 @@ from app.plex.query import ( TS_CONFIG, LibraryFilters, apply_meta_filters, + apply_movie_state_filter, + apply_show_state_filter, field_recheck, genres_col, like_escape, meta_filter_conds, + movie_status_case, + show_status_case, split_csv, title_col, vector_col, @@ -154,17 +157,7 @@ def unified_library( selects = [] if want_movies and grid_field: st = aliased(PlexState) - m_status = case( - (st.status == "watched", "watched"), - ( - and_( - st.position_seconds > 0, - or_(st.status.is_(None), st.status != "watched"), - ), - "in_progress", - ), - else_="new", - ) + m_status = movie_status_case(st) mq = ( select( PlexItem.rating_key.label("rk"), @@ -188,19 +181,7 @@ def unified_library( .outerjoin(st, and_(st.item_id == PlexItem.id, st.user_id == user.id)) .where(PlexItem.kind == "movie", PlexItem.library_id.in_(movie_lib_ids)) ) - if show == "watched": - mq = mq.where(st.status == "watched") - elif show == "in_progress": - mq = mq.where( - st.position_seconds > 0, - or_(st.status.is_(None), st.status != "watched"), - ) - elif show == "unwatched": - mq = mq.where( - or_(st.status.is_(None), st.status.notin_(["watched", "hidden"])) - ) - else: - mq = mq.where(or_(st.status.is_(None), st.status != "hidden")) + mq = apply_movie_state_filter(mq, st, show) mq = apply_meta_filters(mq, PlexItem, p, lang) if f.duration_min is not None: mq = mq.where(PlexItem.duration_s >= f.duration_min) @@ -216,11 +197,7 @@ def unified_library( tot = func.coalesce(agg.c.total, 0) wat = func.coalesce(agg.c.watched, 0) inp = func.coalesce(agg.c.inprog, 0) - s_status = case( - (and_(tot > 0, wat >= tot), "watched"), - (or_(wat > 0, inp > 0), "in_progress"), - else_="new", - ) + s_status = show_status_case(tot, wat, inp) sq = ( select( PlexShow.rating_key.label("rk"), @@ -244,12 +221,7 @@ def unified_library( .outerjoin(agg, agg.c.show_id == PlexShow.id) .where(PlexShow.library_id.in_(show_lib_ids)) ) - if show == "watched": - sq = sq.where(tot > 0, wat >= tot) - elif show == "in_progress": - sq = sq.where(or_(wat > 0, inp > 0), wat < tot) - elif show == "unwatched": - sq = sq.where(wat == 0, inp == 0) + sq = apply_show_state_filter(sq, tot, wat, inp, show) sq = apply_meta_filters(sq, PlexShow, p, lang) if tsq is not None: sq = sq.where(vector_match(PlexShow, lang, tsq)) @@ -393,23 +365,11 @@ def facets( query = query.where(model.duration_s >= pp["duration_min"]) if with_dur and pp.get("duration_max") is not None: query = query.where(model.duration_s <= pp["duration_max"]) - # Always join watch-state and mirror the grid's status branches (unified_library) exactly — + # Join watch-state and reuse the grid's exact movie state filter (unified_library) — # hidden movies never appear in the grid, so they must not inflate facet counts/bounds either. st = aliased(PlexState) query = query.outerjoin(st, and_(st.item_id == model.id, st.user_id == uid)) - if show == "watched": - query = query.where(st.status == "watched") - elif show == "in_progress": - query = query.where( - st.position_seconds > 0, - or_(st.status.is_(None), st.status != "watched"), - ) - elif show == "unwatched": - query = query.where( - or_(st.status.is_(None), st.status.notin_(["watched", "hidden"])) - ) - else: - query = query.where(or_(st.status.is_(None), st.status != "hidden")) + query = apply_movie_state_filter(query, st, show) else: if show in ("watched", "in_progress", "unwatched"): agg = rollup.agg_subquery(db, uid).subquery() @@ -417,12 +377,7 @@ def facets( tot = func.coalesce(agg.c.total, 0) wat = func.coalesce(agg.c.watched, 0) inp = func.coalesce(agg.c.inprog, 0) - if show == "watched": - query = query.where(tot > 0, wat >= tot) - elif show == "in_progress": - query = query.where(or_(wat > 0, inp > 0), wat < tot) - else: - query = query.where(wat == 0, inp == 0) + query = apply_show_state_filter(query, tot, wat, inp, show) return query genre_counts: dict[str, int] = {}