diff --git a/VERSION b/VERSION index 24c41e03..c657195a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.67.0 \ No newline at end of file +0.68.0 \ No newline at end of file diff --git a/backend/alembic/versions/0067_feed_sort_indexes.py b/backend/alembic/versions/0067_feed_sort_indexes.py new file mode 100644 index 00000000..07f1e22b --- /dev/null +++ b/backend/alembic/versions/0067_feed_sort_indexes.py @@ -0,0 +1,55 @@ +"""feed sort indexes: published_at DESC NULLS LAST composite(s) for the default feed (PERF-1) + +The feed's default `newest` sort is `published_at DESC NULLS LAST, id ASC` (feedquery.order_by), but the +only published_at index was a plain ASC btree (`ix_videos_published_at`). A btree read backwards satisfies +`DESC NULLS FIRST`, not the `DESC NULLS LAST` the query asks for, so the planner could not use it for +ordering — every feed page fell back to a full `videos` scan + top-N sort. Measured on a prod-sized reseed +(257k videos): the default page went from ~44.8k shared buffers (dev planner) / ~169k (prod planner, the +per-channel nested loop that is the likely "prod feed is slow" root cause) down to ~624 buffers with the +matching index. + +This adds two indexes in the exact feed order: + - ix_videos_published_at_desc (published_at DESC NULLS LAST, id ASC) — the global feed / Library sort. + - ix_videos_channel_pub_desc (channel_id, published_at DESC NULLS LAST, id ASC) — the per-channel feed + (channel page, channel_ids filter). It leads with channel_id, so it is a strict superset of the old + single-column `ix_videos_channel_id` (FK cascade, sync upsert, autotag, channel-detail all still ride + it) — that redundant index is dropped. + +The plain ASC `ix_videos_published_at` stays: it already serves the `oldest` sort (ASC NULLS LAST = btree +default) as an Index Scan. + +CONCURRENTLY (in alembic's autocommit_block, since it can't run inside a transaction) so a live prod builds +these without blocking reads/writes; downtime ~0. IF (NOT) EXISTS keeps the drift test's +downgrade->re-upgrade round-trip idempotent. +""" +from typing import Sequence, Union + +from alembic import op + +revision: str = "0067_feed_sort_indexes" +down_revision: Union[str, None] = "0066_notif_created_at_notnull" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.get_context().autocommit_block(): + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_published_at_desc " + "ON videos (published_at DESC NULLS LAST, id ASC)" + ) + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_pub_desc " + "ON videos (channel_id, published_at DESC NULLS LAST, id ASC)" + ) + # Redundant now — ix_videos_channel_pub_desc leads with channel_id (a strict superset). + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_id") + + +def downgrade() -> None: + with op.get_context().autocommit_block(): + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_channel_id ON videos (channel_id)" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_channel_pub_desc") + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_published_at_desc") diff --git a/backend/alembic/versions/0068_published_at_not_null.py b/backend/alembic/versions/0068_published_at_not_null.py new file mode 100644 index 00000000..1258a240 --- /dev/null +++ b/backend/alembic/versions/0068_published_at_not_null.py @@ -0,0 +1,36 @@ +"""videos.published_at NOT NULL DEFAULT now() (PERF-15) + +Every video in practice carries a publish date (0 NULLs in the 257k-row prod-sized reseed), but the +column was nullable, which forced the feed's keyset predicate into a non-sargable +`... OR published_at IS NULL` disjunction — blocking the ix_videos_published_at_desc range so deep +`newest` pages scanned from the top of the index and filtered (51.8k buffers). Making it NOT NULL lets +the keyset drop that disjunction and take a `published_at <=` range bound (deep page -> ~0.5k buffers). + +server_default now() is the safety net for the rare date-less ingest path (the enrich overwrites it with +the real date); the app-side ingest paths also fall back to now() so they never write an explicit NULL. + +Note: `SET NOT NULL` validates with a brief ACCESS EXCLUSIVE lock + one table scan (it cannot run +CONCURRENTLY). On the prod row count (~235k) that is sub-second; still, it is a short write-blocking lock +rather than the ~0 of 0067's CONCURRENTLY index build. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0068_published_at_not_null" +down_revision: Union[str, None] = "0067_feed_sort_indexes" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Backfill any stray NULLs first so SET NOT NULL can't fail (0 in prod, but be safe). + op.execute("UPDATE videos SET published_at = now() WHERE published_at IS NULL") + op.alter_column( + "videos", "published_at", nullable=False, server_default=sa.text("now()") + ) + + +def downgrade() -> None: + op.alter_column("videos", "published_at", nullable=True, server_default=None) diff --git a/backend/alembic/versions/0069_job_index_hygiene.py b/backend/alembic/versions/0069_job_index_hygiene.py new file mode 100644 index 00000000..59370115 --- /dev/null +++ b/backend/alembic/versions/0069_job_index_hygiene.py @@ -0,0 +1,93 @@ +"""scheduler-job partial indexes + boolean-index cleanup (PERF-17/18/3) + +Two scheduler jobs full-seq-scanned the 257k-row videos table every run: + - PERF-17: enrich_pending (`WHERE enriched_at IS NULL LIMIT N`, every few minutes) — 44.6k buffers / + run even when nothing is pending. A partial index over just the pending rows makes it a 1-buffer + (empty) index scan. + - PERF-18: maintenance revalidate (`WHERE unavailable_since IS NULL ORDER BY last_checked_at NULLS + FIRST LIMIT N`, ~300 pages/run) — 44.7k buffers/page; ix_videos_last_checked_at had idx_scan=0. A + partial index in that exact order serves it as an index scan (~36 buffers/page) and replaces the + unused plain one. + +PERF-3: the videos table also carried plain btrees on low-cardinality boolean columns the planner never +picks (a `col IS false` majority predicate can't ride a btree). Drop the never-useful ones (is_short, +via_explore) and slim the rest to partial indexes over the small/shrinking subset the job queries target +(via_search=true, NOT shorts_probed, unavailable_since IS NOT NULL) — less write amplification on the +biggest table, same lookups. live_status is left alone (a separate finding). + +All CONCURRENTLY (autocommit_block) — downtime ~0; IF (NOT) EXISTS keeps the drift test's +downgrade->re-upgrade idempotent. +""" +from typing import Sequence, Union + +from alembic import op + +revision: str = "0069_job_index_hygiene" +down_revision: Union[str, None] = "0068_published_at_not_null" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.get_context().autocommit_block(): + # PERF-17: enrich-pending partial (empty when nothing is pending -> 1-buffer scan). + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_enrich_pending " + "ON videos (id) WHERE enriched_at IS NULL" + ) + # PERF-18: maintenance revalidate order, as a partial in the exact WHERE + ORDER BY shape. + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_recheck_order " + "ON videos (last_checked_at ASC NULLS FIRST) WHERE unavailable_since IS NULL" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_last_checked_at") + # PERF-3: drop the never-picked boolean btrees; slim the rest to partials over the target subset. + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_is_short") + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_via_explore") + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_via_search") + # Predicate MUST match the query's `.is_(True)`/`.is_(False)` form (SQL `IS TRUE`/`IS FALSE`): + # Postgres's partial-index prover does NOT match a `col IS TRUE` clause to a bare `col` predicate, + # so `WHERE via_search` / `WHERE NOT shorts_probed` would leave the queries seq-scanning. + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_via_search " + "ON videos (id) WHERE via_search IS TRUE" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_shorts_probed") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_shorts_probed " + "ON videos (id) WHERE shorts_probed IS FALSE" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_unavailable_since") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_unavailable_since " + "ON videos (unavailable_since) WHERE unavailable_since IS NOT NULL" + ) + + +def downgrade() -> None: + with op.get_context().autocommit_block(): + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_enrich_pending") + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_recheck_order") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_last_checked_at " + "ON videos (last_checked_at)" + ) + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_is_short ON videos (is_short)" + ) + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_via_explore ON videos (via_explore)" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_via_search") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_via_search ON videos (via_search)" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_shorts_probed") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_shorts_probed ON videos (shorts_probed)" + ) + op.execute("DROP INDEX CONCURRENTLY IF EXISTS ix_videos_unavailable_since") + op.execute( + "CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_videos_unavailable_since " + "ON videos (unavailable_since)" + ) diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 93e82ac2..2703583e 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -100,6 +100,18 @@ def serialize(row) -> dict: } +def tags_by_category(db: Session, tag_ids: list[int]) -> dict[str, list[int]]: + """Group tag ids by their category (language/topic/other). The single source for the category split + shared by the feed's tag WHERE (filtered_query) and the facet/count summary (feed_summary), so the + two can't disagree on which selected tag belongs to which category.""" + if not tag_ids: + return {} # skip the pointless `WHERE id IN ()` round-trip on the no-tag-filter hot path + out: dict[str, list[int]] = {} + for tid, cat in db.execute(select(Tag.id, Tag.category).where(Tag.id.in_(tag_ids))).all(): + out.setdefault(cat, []).append(tid) + return out + + def filtered_query( db: Session, user: User, @@ -131,7 +143,6 @@ def filtered_query( either mode via the VideoState outer join.""" state = aliased(VideoState) status_expr = func.coalesce(state.status, "new") - position_expr = func.coalesce(state.position_seconds, 0) query = select(*feed_columns(user, state)).join( Channel, Channel.id == Video.channel_id @@ -163,7 +174,14 @@ def filtered_query( and_(SearchFind.video_id == Video.id, SearchFind.user_id == user.id), ) - query = query.outerjoin( + # The state-gated views (watched/hidden/in_progress) only ever return videos that HAVE a + # video_states row, so an INNER join lets the planner drive from the user's own small video_states + # set (~100 rows) instead of scanning all videos and outer-joining (PERF-16). status is NOT NULL on + # a present row (server_default 'new'), so the raw-column filters below match the coalesce form + # exactly. 'unwatched'/'all' include row-less 'new' videos, so they keep the outer join. + state_required = show in ("watched", "hidden", "in_progress") + join_state = query.join if state_required else query.outerjoin + query = join_state( state, and_(state.video_id == Video.id, state.user_id == user.id) ) @@ -246,15 +264,25 @@ def filtered_query( # channel searches work. `rank_expr` (weight-aware ts_rank) drives the "relevance" sort. rank_expr = None if q: + # Resolve the channels whose NAME matches up front (there are only a few hundred channels, so + # this is a tiny standalone scan) and match videos by channel_id. ORing an unindexed + # `unaccent(channels.title) ILIKE '%q%'` straight into the per-video predicate defeated the + # search_vector GIN — the planner detoasted every non-matching tsvector to test the `@@` + # (~2.6s/search on the prod-sized set); channel_id.in_(...) keeps the GIN as a BitmapOr branch + # (PERF-12). Empty match -> false() so the OR collapses to the text match alone. + like = func.unaccent(f"%{q}%") + matching_channel_ids = ( + db.execute(select(Channel.id).where(func.unaccent(Channel.title).ilike(like))) + .scalars() + .all() + ) + channel_match = ( + Video.channel_id.in_(matching_channel_ids) if matching_channel_ids else false() + ) ts_str = to_tsquery_str(q) if ts_str: tsq = func.to_tsquery("public.unaccent_simple", ts_str) - query = query.where( - or_( - Video.search_vector.op("@@")(tsq), - func.unaccent(Channel.title).ilike(func.unaccent(f"%{q}%")), - ) - ) + query = query.where(or_(Video.search_vector.op("@@")(tsq), channel_match)) # Scale ts_rank (a float4) to an integer score so the keyset cursor compares it # EXACTLY — a raw float4 vs the float8 cursor value mismatches on round-trip and # breaks paging (the same page repeats). 1e6 granularity is ample; ties break on @@ -262,21 +290,14 @@ def filtered_query( rank_expr = cast(func.ts_rank(Video.search_vector, tsq) * 1000000, BigInteger) else: # No usable tokens (e.g. only punctuation): fall back to a plain substring match. - like = func.unaccent(f"%{q}%") - query = query.where( - or_( - func.unaccent(Video.title).ilike(like), - func.unaccent(Channel.title).ilike(like), - ) - ) + query = query.where(or_(func.unaccent(Video.title).ilike(like), channel_match)) if tags: - # AND across tag categories (e.g. language AND topic narrows), OR within a - # category; the any/all toggle controls multiple topic tags. - cat_rows = db.execute(select(Tag.id, Tag.category).where(Tag.id.in_(tags))).all() - by_category: dict[str, list[int]] = {} - for tag_id, category in cat_rows: - by_category.setdefault(category, []).append(tag_id) + # AND across tag categories (e.g. language AND topic narrows), OR within a category; the any/all + # toggle controls multiple topic tags. ⚠️ feed_summary.passes() reimplements this exact rule in + # Python for the sidebar count/facets — keep the two in lockstep (equivalence-tested by + # test_feed_summary_matches_count_plus_facets). + by_category = tags_by_category(db, tags) # Facet counting drops the category being counted so its own chips don't zero each # other out (standard drill-down faceting: a category's count ignores its own # selections but still honours the other categories' filters). @@ -315,20 +336,96 @@ def filtered_query( if show == "unwatched": query = query.where(status_expr.notin_(("watched", "hidden"))) elif show == "in_progress": - # Started but not finished: a stored resume position, not yet watched/hidden. + # Started but not finished: a stored resume position, not yet watched/hidden. INNER-joined + # (state_required) so the raw state columns are safe and the planner can use video_states. query = query.where( - and_(position_expr > 0, status_expr.notin_(("watched", "hidden"))) + and_(state.position_seconds > 0, state.status.notin_(("watched", "hidden"))) ) elif show == "watched": - query = query.where(status_expr == "watched") + query = query.where(state.status == "watched") elif show == "hidden": - query = query.where(status_expr == "hidden") + query = query.where(state.status == "hidden") else: # all query = query.where(status_expr != "hidden") return query, rank_expr +def feed_summary(db: Session, user: User, **params) -> dict: + """Single-pass replacement for /feed/count + 3x /facets (PERF-13): the sidebar's total video count + and every tag's distinct-channel facet count, derived in Python from ONE grouped query instead of + four full passes over the subscribed set on every filter change. + + The one query is the base filter with the TAG predicate removed, grouped by channel: it yields every + channel that has a matching video, with its video count. From that ~few-hundred-row map we compute: + - count = sum of the video counts of the channels that pass the SELECTED tag filter; + - facets = per tag, the number of distinct channels that carry it AND pass the drill-down filter + (the tag's own category dropped, unless it's a conjunctive topic-AND — mirroring the + old /facets exclude_tag_category logic exactly). + Tag matching is channel-based, identical to filtered_query's tag block (AND across categories, OR + within, topic-AND requires ALL selected topics).""" + tag_mode = params.get("tag_mode", "or") + + # ONE pass: per-channel video counts for the base filter, tags stripped so every candidate channel + # is present to facet over. + base, _rank = filtered_query(db, user, **{**params, "tags": []}) + ch_col = base.with_only_columns(Video.channel_id).subquery() + channel_counts: dict[str, int] = dict( + db.execute(select(ch_col.c.channel_id, func.count()).group_by(ch_col.c.channel_id)).all() + ) + if not channel_counts: + return {"count": 0, "facets": {}} + + # channel -> its visible tag ids, plus each tag's category (for grouping the facet output). + visible = or_(ChannelTag.user_id.is_(None), ChannelTag.user_id == user.id) + channel_tags: dict[str, set[int]] = {} + tag_category: dict[int, str] = {} + for cid, tid, cat in db.execute( + select(ChannelTag.channel_id, ChannelTag.tag_id, Tag.category) + .join(Tag, Tag.id == ChannelTag.tag_id) + .where(visible, ChannelTag.channel_id.in_(channel_counts.keys())) + ).all(): + channel_tags.setdefault(cid, set()).add(tid) + tag_category[tid] = cat + + # Selected tags grouped by category (read from Tag via the shared helper, so a selected tag that no + # base channel carries still filters everything out for its category — matching the SQL `IN (empty)` + # behaviour). + selected_by_cat: dict[str, set[int]] = { + cat: set(ids) for cat, ids in tags_by_category(db, params.get("tags") or []).items() + } + + def passes(ch_tags: set[int], exclude_cat: str | None) -> bool: + # The Python mirror of filtered_query's SQL tag block (AND across categories, OR within, + # topic-AND requires all selected) — keep in lockstep; see the ⚠️ note there. + for cat, sel in selected_by_cat.items(): + if cat == exclude_cat: + continue + if cat == "topic" and tag_mode == "and" and len(sel) > 1: + if not sel <= ch_tags: # conjunctive: channel must carry ALL selected topics + return False + elif not (sel & ch_tags): # disjunctive: channel must carry ANY selected tag + return False + return True + + count = sum( + n for cid, n in channel_counts.items() if passes(channel_tags.get(cid, set()), None) + ) + + facets: dict[int, int] = {} + for category in ("language", "topic", "other"): + # Drop this category's own selections so its chips keep independent counts; a conjunctive + # topic-AND keeps them (each remaining chip narrows to channels that ALSO have the selected). + exclude = None if (category == "topic" and tag_mode == "and") else category + for cid, ch_tags in channel_tags.items(): + if not passes(ch_tags, exclude): + continue + for tid in ch_tags: + if tag_category.get(tid) == category: + facets[tid] = facets.get(tid, 0) + 1 + return {"count": count, "facets": facets} + + # --- Keyset (cursor) pagination --------------------------------------------- # # Each sort is described as an ordered list of key columns (expression + direction @@ -339,7 +436,7 @@ def filtered_query( # and the page boundary is stable even as the scheduler ingests new videos mid-scroll # (OFFSET would shift and duplicate/skip rows). def sort_keys(sort: str, seed: int, rank_expr=None) -> list[dict]: - pub = {"expr": Video.published_at, "asc": False, "nullable": True, "kind": "dt"} + pub = {"expr": Video.published_at, "asc": False, "nullable": False, "kind": "dt"} table: dict[str, list[dict]] = { "newest": [pub], "oldest": [{**pub, "asc": True}], @@ -383,7 +480,15 @@ def keyset_predicate(keys: list[dict], cursor_id: str): Lexicographic over the key columns then id, NULL-aware for `nulls_last`: a NULL sorts after every non-NULL value, so a cursor sitting on a non-NULL value must - also pull in the NULL group, and equality treats NULL == NULL.""" + also pull in the NULL group, and equality treats NULL == NULL. + + Built recursively so that for a NON-nullable leading key we can prepend a non-strict + range bound (`col <= v` for desc / `col >= v` for asc) as an AND conjunct — logically + a no-op over the OR of strict-after branches, but it lets the planner bound the index + scan instead of scanning from the top and filtering every earlier row (PERF-15: deep + `newest` pages went 51.8k -> ~0.5k buffers). A NULLABLE key can't take that bound (the + NULL group sorts after every value and `col <= v` would drop it), so it keeps the pure + NULL-aware OR form.""" def eq(k): return k["expr"].is_(None) if k["value"] is None else k["expr"] == k["value"] @@ -393,11 +498,17 @@ def keyset_predicate(keys: list[dict], cursor_id: str): cmp = k["expr"] > k["value"] if k["asc"] else k["expr"] < k["value"] return or_(cmp, k["expr"].is_(None)) if k["nullable"] else cmp - branches = [] - for j in range(len(keys)): - branches.append(and_(*[eq(keys[i]) for i in range(j)], after(keys[j]))) - branches.append(and_(*[eq(k) for k in keys], Video.id > cursor_id)) - return or_(*branches) + def rec(i: int): + if i == len(keys): + return Video.id > cursor_id # all keys tied -> break by the unique id + k = keys[i] + tail = or_(after(k), and_(eq(k), rec(i + 1))) + if not k["nullable"] and k["value"] is not None: + bound = k["expr"] >= k["value"] if k["asc"] else k["expr"] <= k["value"] + return and_(bound, tail) + return tail + + return rec(0) def encode_cursor(keys: list[dict], row) -> str: diff --git a/backend/app/models.py b/backend/app/models.py index 65078e47..ed856b48 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -251,14 +251,23 @@ class Video(Base, TimestampMixin): __tablename__ = "videos" id: Mapped[str] = mapped_column(String(16), primary_key=True) # video id + # No standalone channel_id index: the composite `ix_videos_channel_pub_desc` + # (channel_id, published_at DESC NULLS LAST, id) from migration 0067 leads with channel_id, so it + # serves every channel_id lookup (FK cascade, sync upsert, channel-detail) AND the per-channel feed + # sort. Like the search_vector GIN, this composite is migration-defined, not an inline `index=True`. channel_id: Mapped[str] = mapped_column( - ForeignKey("channels.id", ondelete="CASCADE"), index=True + ForeignKey("channels.id", ondelete="CASCADE") ) title: Mapped[str | None] = mapped_column(Text) # normalized for display (see app.titles) original_title: Mapped[str | None] = mapped_column(Text) # raw YouTube title, preserved description: Mapped[str | None] = mapped_column(Text) - published_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), index=True + # NOT NULL (migration 0068): in reality every video carries a publish date, and a nullable key + # forced the feed's keyset predicate into a non-sargable `... OR published_at IS NULL` disjunction + # that blocked the ix_videos_published_at_desc range (deep pages scanned from the top and filtered — + # PERF-15). server_default now() is the safety net for the rare date-less ingest (the enrich fills + # the real date); the ix_videos_published_at index stays for the `oldest` sort. + published_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), index=True, nullable=False, server_default=func.now() ) thumbnail_url: Mapped[str | None] = mapped_column(String(1024)) duration_seconds: Mapped[int | None] = mapped_column(Integer) @@ -276,18 +285,22 @@ class Video(Base, TimestampMixin): # folds YouTube's own relevance judgement into the local search: a video YouTube returned # for "ti amo magyarul" becomes findable by that query locally even if its title lacks it. search_terms: Mapped[str | None] = mapped_column(Text) + # No btree here or on the flag columns below: migration 0069 (PERF-3) dropped the never-picked + # boolean btrees and replaced the useful ones with partial indexes over the target subset + # (via_search=true, NOT shorts_probed, unavailable_since IS NOT NULL) — migration-defined, like the + # search_vector GIN. is_short: Mapped[bool] = mapped_column( - Boolean, default=False, server_default="false", index=True + Boolean, default=False, server_default="false" ) # Whether the youtube.com/shorts/ probe has run for this video. shorts_probed: Mapped[bool] = mapped_column( - Boolean, default=False, server_default="false", index=True + Boolean, default=False, server_default="false" ) # True when this video first entered the catalog via a live YouTube search (not an # organic channel/playlist sync). Used to hide search-discovered "noise" from the # Library by default; left False by the organic ingest paths. via_search: Mapped[bool] = mapped_column( - Boolean, default=False, server_default="false", index=True + Boolean, default=False, server_default="false" ) # True when this video entered the catalog because a user EXPLORED its (un-subscribed) # channel. Like via_search it's hidden from the shared Library by default, but it's @@ -295,7 +308,7 @@ class Video(Base, TimestampMixin): # explored or subscribe to its channel (a gate in the feed query), so one user's # curiosity never floods everyone else's catalog. Left False by the organic ingest paths. via_explore: Mapped[bool] = mapped_column( - Boolean, default=False, server_default="false", index=True + Boolean, default=False, server_default="false" ) # none | live | upcoming | premiere | was_live live_status: Mapped[str] = mapped_column( @@ -311,11 +324,14 @@ class Video(Base, TimestampMixin): # (least-recently-checked first); unavailable_since marks a video as currently # unplayable (hidden from the feed immediately, hard-deleted after a grace period); # unavailable_reason records why (deleted|private|paywalled|abandoned|stuck_live). + # last_checked_at: no plain btree — migration 0069 (PERF-18) replaced it with the partial + # ix_videos_recheck_order (last_checked_at NULLS FIRST WHERE unavailable_since IS NULL) that the + # revalidate job's exact ORDER BY rides; unavailable_since is a partial WHERE IS NOT NULL (PERF-3). last_checked_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), index=True + DateTime(timezone=True) ) unavailable_since: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), index=True + DateTime(timezone=True) ) unavailable_reason: Mapped[str | None] = mapped_column(String(24)) diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 49303b72..c3ec31d4 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -13,6 +13,7 @@ from app.db import get_db from app.feedquery import ( decode_cursor, encode_cursor, + feed_summary, filtered_query, keyset_predicate, order_by, @@ -127,6 +128,9 @@ def get_feed_count( user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: + # SUPERSEDED by /feed/summary (PERF-13) — the app no longer calls this. Kept as the reference + # oracle that test_feed_summary_matches_count_plus_facets asserts /feed/summary's `count` against; + # remove both together with that test if this endpoint is ever retired. query, _rank = filtered_query(db, user, **params) total = db.scalar(select(func.count()).select_from(query.subquery())) return {"count": total or 0} @@ -146,7 +150,11 @@ def get_facets( The count is the number of distinct channels that have at least one video in the current view, matching the existing channel-count chip semantics. JSON object keys are strings - (tag ids).""" + (tag ids). + + SUPERSEDED by /feed/summary (PERF-13) — the app no longer calls this. Kept as the reference + oracle for test_feed_summary_matches_count_plus_facets (it asserts /feed/summary's `facets` + against these `counts`); retire the two together.""" visible = or_(ChannelTag.user_id.is_(None), ChannelTag.user_id == user.id) counts: dict[int, int] = {} for category in ("language", "topic", "other"): @@ -174,6 +182,19 @@ def get_facets( return {"counts": counts} +@router.get("/feed/summary") +def get_feed_summary( + params: dict = Depends(_feed_params), + user: User = Depends(current_user), + db: Session = Depends(get_db), +) -> dict: + """The sidebar's total video count + per-tag channel facet counts in ONE request and ONE grouped + query, replacing the separate /feed/count (1 pass) + /facets (3 passes) the FE fired on every filter + change (PERF-13). Response: {"count": , "facets": {: }} — the same + numbers /feed/count's `count` and /facets' `counts` return, so the two are interchangeable.""" + return feed_summary(db, user, **params) + + class VideoStateIn(BaseModel): status: str diff --git a/backend/app/sync/maintenance.py b/backend/app/sync/maintenance.py index 1eb93302..d827afa6 100644 --- a/backend/app/sync/maintenance.py +++ b/backend/app/sync/maintenance.py @@ -194,10 +194,10 @@ def _revalidate_rolling(db: Session, yt: YouTubeClient) -> dict: db.execute( select(Video) .where(Video.unavailable_since.is_(None)) - .order_by( - Video.last_checked_at.is_(None).desc(), # never-checked first - Video.last_checked_at.asc(), - ) + # NULLS FIRST = never-checked first, then least-recently-checked; equivalent to the old + # `(last_checked_at IS NULL) DESC, last_checked_at ASC` but in the exact shape of the + # partial ix_videos_recheck_order (migration 0069, PERF-18) so it rides that index. + .order_by(Video.last_checked_at.asc().nulls_first()) .limit(min(50, batch - checked)) ) .scalars() diff --git a/backend/app/sync/playlists.py b/backend/app/sync/playlists.py index 81ecb2c2..d12c3365 100644 --- a/backend/app/sync/playlists.py +++ b/backend/app/sync/playlists.py @@ -90,7 +90,9 @@ def _ensure_videos(db: Session, yt: YouTubeClient, video_ids: list[str]) -> None if not vid or not cid or vid in seen: continue seen.add(vid) - v = Video(id=vid, channel_id=cid, published_at=parse_dt(sn.get("publishedAt"))) + # published_at is NOT NULL (0068); apply_video_details re-fills a missing date, but seed a now() + # fallback here too so the constructor never carries an explicit NULL. + v = Video(id=vid, channel_id=cid, published_at=parse_dt(sn.get("publishedAt")) or now) apply_video_details(v, it) v.enriched_at = now db.add(v) diff --git a/backend/app/sync/videos.py b/backend/app/sync/videos.py index c520c37b..a36417b9 100644 --- a/backend/app/sync/videos.py +++ b/backend/app/sync/videos.py @@ -90,7 +90,10 @@ def stub_from_playlist_item(item: dict, channel_id: str) -> dict | None: video_id = content.get("videoId") if not video_id: return None - published = parse_dt(content.get("videoPublishedAt") or snippet.get("publishedAt")) + # published_at is NOT NULL (migration 0068); fall back to now() for the rare item that carries no + # date at all, so the bulk stub insert never writes a NULL. The enrich pass replaces it if the + # video's own snippet later yields a real publish date. + published = parse_dt(content.get("videoPublishedAt") or snippet.get("publishedAt")) or _now() return { "id": video_id, "channel_id": channel_id, @@ -243,7 +246,8 @@ def apply_video_details(video: Video, item: dict) -> None: video.title = normalize_title(raw_title) video.description = snippet.get("description") if not video.published_at: - video.published_at = parse_dt(snippet.get("publishedAt")) + # NOT NULL (0068): a fresh insert with no date yet falls back to now() (enrich normally has one). + video.published_at = parse_dt(snippet.get("publishedAt")) or _now() video.thumbnail_url = best_thumbnail(snippet.get("thumbnails")) or video.thumbnail_url video.duration_seconds = parse_iso8601_duration(content.get("duration")) video.view_count = int(stats["viewCount"]) if stats.get("viewCount") else None diff --git a/backend/tests/test_feed_keyset.py b/backend/tests/test_feed_keyset.py index 44b62d4b..f8267e2e 100644 --- a/backend/tests/test_feed_keyset.py +++ b/backend/tests/test_feed_keyset.py @@ -41,14 +41,16 @@ def _seed(db): Subscription(user_id=user.id, channel_id="UCa", priority=5, hidden=False), Subscription(user_id=user.id, channel_id="UCb", priority=0, hidden=False), ]) - # 7 normal/organic videos. Deliberate duplicates (published_at day 1 x2 and day 3 x2, view_count - # 100 x2, duration 60 x2 / 30 x2) and NULLs spread across the sort columns. + # 7 normal/organic videos. Deliberate duplicates (published_at day 1 x2, day 2 x2 and day 3 x2, + # view_count 100 x2, duration 60 x2 / 30 x2) and NULLs spread across the still-nullable sort columns + # (view_count / duration_seconds / title / subscriber_count) to exercise nulls_last. published_at is + # NOT NULL (migration 0068), so no NULL there — the duplicates keep the id tiebreaker load-bearing. rows = [ # id ch published views dur title ("vid00", "UCa", _dt(1), 100, 60, "alpha"), ("vid01", "UCa", _dt(1), 100, 60, "beta"), ("vid02", "UCa", _dt(2), None, None, "gamma"), - ("vid03", "UCb", None, 50, 120, None), + ("vid03", "UCb", _dt(2), 50, 120, None), ("vid04", "UCb", _dt(3), 200, 30, "delta"), ("vid05", "UCb", _dt(3), None, 30, "epsilon"), ("vid06", "UCa", _dt(28), None, None, "zeta"), diff --git a/backend/tests/test_feed_views.py b/backend/tests/test_feed_views.py new file mode 100644 index 00000000..1a3eefd4 --- /dev/null +++ b/backend/tests/test_feed_views.py @@ -0,0 +1,154 @@ +"""Feed search + watch-state views (E2 S3), DB-lane. + +Regression nets for two query rewrites that must stay behaviour-equivalent: +- PERF-12: the search channel-name branch moved from an ORed `unaccent(channels.title) ILIKE` (which + defeated the search_vector GIN) to a pre-resolved `channel_id IN (...)`. A search must STILL return + both videos whose text matches AND videos whose CHANNEL name matches. +- PERF-16: the watched/hidden/in_progress views moved from an outer join + `coalesce(state.status,'new')` + to an inner join on video_states + the raw column. Each view must return exactly the same set. +""" +from datetime import datetime, timezone + +import pytest + +from app.models import Channel, ChannelTag, Subscription, Tag, Video, VideoState + + +def _dt(day: int) -> datetime: + return datetime(2024, 1, day, tzinfo=timezone.utc) + + +def _video(vid: str, ch: str, title: str) -> Video: + return Video( + id=vid, channel_id=ch, published_at=_dt(1), title=title, + is_short=False, live_status="none", via_search=False, via_explore=False, + ) + + +def _feed_ids(client, query: str) -> set[str]: + resp = client.get(f"/api/feed?{query}&limit=50") + assert resp.status_code == 200, resp.text + return {it["id"] for it in resp.json()["items"]} + + +def test_search_matches_both_video_text_and_channel_name(db, as_user, make_user): + # Two subscribed channels: one whose NAME matches the query (its video's text does not), and one + # whose video TEXT matches (its name does not). The pre-resolved channel_id branch must still OR in + # the name match, so both videos come back — and an unrelated video on a matching-name-less channel + # stays out. + user = make_user(email="search@example.com") + db.add_all([ + Channel(id="UCkube", title="Kubernetes Weekly", subscriber_count=10), + Channel(id="UCother", title="Some Other Channel", subscriber_count=10), + ]) + db.add_all([ + Subscription(user_id=user.id, channel_id="UCkube", priority=0, hidden=False), + Subscription(user_id=user.id, channel_id="UCother", priority=0, hidden=False), + ]) + db.add_all([ + _video("vidChan", "UCkube", "Totally unrelated content"), # channel NAME matches + _video("vidText", "UCother", "kubernetes deep dive"), # video TEXT matches + _video("vidNone", "UCother", "cooking pasta at home"), # neither matches + ]) + db.commit() + + ids = _feed_ids(as_user(user), "q=kubernetes&show=all") + assert ids == {"vidChan", "vidText"} + + +def test_watch_state_views_return_the_right_sets(db, as_user, make_user): + user = make_user(email="views@example.com") + db.add(Channel(id="UCv", title="Channel V", subscriber_count=10)) + db.add(Subscription(user_id=user.id, channel_id="UCv", priority=0, hidden=False)) + db.add_all([ + _video("vWatch", "UCv", "watched one"), + _video("vHide", "UCv", "hidden one"), + _video("vProg", "UCv", "in progress one"), + _video("vNew", "UCv", "brand new one"), + ]) + db.flush() + db.add_all([ + VideoState(user_id=user.id, video_id="vWatch", status="watched"), + VideoState(user_id=user.id, video_id="vHide", status="hidden"), + # in progress = a stored resume position while still "new" (not watched/hidden) + VideoState(user_id=user.id, video_id="vProg", status="new", position_seconds=30), + # vNew has NO state row -> "new"/unwatched + ]) + db.commit() + + client = as_user(user) + assert _feed_ids(client, "show=watched") == {"vWatch"} + assert _feed_ids(client, "show=hidden") == {"vHide"} + assert _feed_ids(client, "show=in_progress") == {"vProg"} + # unwatched excludes watched + hidden, keeps new AND in-progress + assert _feed_ids(client, "show=unwatched") == {"vProg", "vNew"} + # "all" shows everything except hidden + assert _feed_ids(client, "show=all") == {"vWatch", "vProg", "vNew"} + + +def _seed_tagged(db, make_user): + """A user subscribed to 4 channels with a spread of language/topic/other tags and differing video + counts, so the count sum and the per-category facet drill-down are all non-trivial.""" + user = make_user(email="summary@example.com") + tags = { + "L1": Tag(user_id=None, name="Hungarian", category="language"), + "L2": Tag(user_id=None, name="English", category="language"), + "T1": Tag(user_id=None, name="Tech", category="topic"), + "T2": Tag(user_id=None, name="Gaming", category="topic"), + "O1": Tag(user_id=None, name="Favourite", category="other"), + # A USER-owned tag (+ a user-owned ChannelTag link below) so the visibility branch + # `ChannelTag.user_id == user.id` is exercised, not only the system `user_id IS NULL` one. + "U1": Tag(user_id=user.id, name="MyPin", category="other"), + } + db.add_all(tags.values()) + chans = {c: Channel(id=c, title=c, subscriber_count=10) for c in ("chA", "chB", "chC", "chD")} + db.add_all(chans.values()) + db.flush() + # channel -> (system) tag keys + links = {"chA": ["L1", "T1"], "chB": ["L1", "T2"], "chC": ["L2", "T1", "O1"], "chD": []} + for cid, tkeys in links.items(): + db.add(Subscription(user_id=user.id, channel_id=cid, priority=0, hidden=False)) + for tk in tkeys: + db.add(ChannelTag(channel_id=cid, tag_id=tags[tk].id, user_id=None)) + # The user's own private link (visible only to them): chB carries U1 for this user. + db.add(ChannelTag(channel_id="chB", tag_id=tags["U1"].id, user_id=user.id)) + # differing video counts per channel (chA:3, chB:2, chC:1, chD:2) + counts = {"chA": 3, "chB": 2, "chC": 1, "chD": 2} + for cid, n in counts.items(): + db.add_all([_video(f"{cid}v{i}", cid, f"{cid} clip {i}") for i in range(n)]) + db.commit() + return user, {k: v.id for k, v in tags.items()} + + +# Query-param combos (as a template with {L1}/{T1}/… placeholders) that stress every facet path: +# no tags, single-category, cross-category, disjunctive + conjunctive topic modes, and a search filter. +_SUMMARY_CASES = [ + "show=all", + "show=all&tags={L1}", + "show=all&tags={T1}", + "show=all&tags={L1}&tags={T1}", + "show=all&tags={T1}&tags={T2}&tag_mode=or", + "show=all&tags={T1}&tags={T2}&tag_mode=and", + "show=all&tags={U1}", # user-owned tag (visibility == user.id branch) + "show=all&tags={U1}&tags={L1}", # user tag AND a system tag, cross-category + "show=all&q=clip", + "show=unwatched", +] + + +@pytest.mark.parametrize("case", _SUMMARY_CASES) +def test_feed_summary_matches_count_plus_facets(db, as_user, make_user, case): + # /feed/summary must return byte-identical numbers to the old /feed/count + /facets it replaces, + # across the full matrix of tag filters — it reimplements the tag drill-down in Python, so this is + # the equivalence net. + user, tag_ids = _seed_tagged(db, make_user) + client = as_user(user) + qs = case.format(**tag_ids) + + summary = client.get(f"/api/feed/summary?{qs}") + count = client.get(f"/api/feed/count?{qs}") + facets = client.get(f"/api/facets?{qs}") + assert summary.status_code == count.status_code == facets.status_code == 200 + + assert summary.json()["count"] == count.json()["count"], qs + assert summary.json()["facets"] == facets.json()["counts"], qs diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 366e6e71..7abe86c0 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -115,7 +115,7 @@ export default function Channels() { // FB2: syncing subscriptions can change the feed immediately — newly-followed channels may // already have videos in the shared catalog, so refresh the feed too (not just the manager). qc.invalidateQueries({ queryKey: qk.feed() }); - qc.invalidateQueries({ queryKey: qk.feedCount() }); + qc.invalidateQueries({ queryKey: qk.feedSummary() }); }; const userTags = (tagsQuery.data ?? []).filter((t) => !t.system); @@ -131,7 +131,7 @@ export default function Channels() { qc.invalidateQueries({ queryKey: qk.channels() }); qc.invalidateQueries({ queryKey: qk.myStatus() }); qc.invalidateQueries({ queryKey: qk.feed() }); - qc.invalidateQueries({ queryKey: qk.feedCount() }); + qc.invalidateQueries({ queryKey: qk.feedSummary() }); }, }); // Priority is updated optimistically in place and NOT re-sorted/refetched, so the list doesn't diff --git a/frontend/src/components/Feed.tsx b/frontend/src/components/Feed.tsx index 8440a519..360487ed 100644 --- a/frontend/src/components/Feed.tsx +++ b/frontend/src/components/Feed.tsx @@ -226,9 +226,11 @@ export default function Feed({ } }, [query.dataUpdatedAt]); - const countQuery = useQuery({ - queryKey: qk.feedCount(queryFilters), - queryFn: () => api.feedCount(queryFilters), + // One shared query with the sidebar (same key + filters -> React Query dedupes to a single request): + // the feed reads `.count`, the sidebar reads `.facets` (PERF-13, replaces feedCount + facets). + const summaryQuery = useQuery({ + queryKey: qk.feedSummary(queryFilters), + queryFn: () => api.feedSummary(queryFilters), staleTime: 30_000, enabled: !ytActive, placeholderData: keepPreviousData, @@ -252,7 +254,11 @@ export default function Feed({ api .setState(id, status) .then(() => { - qc.invalidateQueries({ queryKey: qk.feed() }); + // Mark stale WITHOUT refetching: the optimistic `overrides` layer + the matchesView filter + // already show the correct state (a hidden/watched card drops out immediately), so refetching + // every loaded infinite-query page here was pure waste — 10 pages x rapid marks = a refetch + // storm (PERF-2). The next filter change / remount refetches fresh from the server. + qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }); if (status === "hidden") { const v = loadedRef.current.find((x) => x.id === id); notify({ @@ -309,7 +315,8 @@ export default function Feed({ setOverrides((o) => ({ ...o, [id]: "new" })); api .clearState(id) - .then(() => qc.invalidateQueries({ queryKey: qk.feed() })) + // Stale-mark only (see setState above): the optimistic override already reset the card. + .then(() => qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" })) .catch(() => setOverrides((o) => { const next = { ...o }; @@ -358,8 +365,7 @@ export default function Feed({ // pre-search cache. Drop them so the feed re-fetches fresh on return. const dropFeedCaches = () => { qc.removeQueries({ queryKey: qk.feed() }); - qc.removeQueries({ queryKey: qk.feedCount() }); - qc.removeQueries({ queryKey: qk.facets() }); + qc.removeQueries({ queryKey: qk.feedSummary() }); }; // --- Live YouTube search mode ------------------------------------------------------------- @@ -622,10 +628,10 @@ export default function Feed({ )}