From 3af3799b25a67b782713c54274cef354a198c090 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 04:16:08 +0200 Subject: [PATCH 01/10] perf(feed): index the default published_at DESC NULLS LAST sort (E2 S1, PERF-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The feed's default `newest` sort is `published_at DESC NULLS LAST, id ASC`, but the only published_at index was a plain ASC btree — read backwards it satisfies DESC NULLS FIRST, not NULLS LAST — so every feed page fell back to a full `videos` seq scan + top-N sort. Migration 0067 adds the two matching indexes (global + per-channel) CONCURRENTLY and drops the now-redundant single-column ix_videos_channel_id (the channel composite leads with channel_id, a strict superset). Measured on the prod-sized localdev reseed (257,312 videos, prod planner): - global newest: 44,747 buffers / 154 ms -> 60 buffers / 0.32 ms - per-channel(12.6k): 7,604 buffers / 96 ms -> 54 buffers / 0.14 ms CONCURRENTLY build of both on 257k rows: 1.7s total (~0 downtime). This is the likely root cause of the "prod feed is slow" symptom. Model: drop the inline index=True on Video.channel_id (the composite supersedes it); the composites are migration-defined like the search_vector GIN. Gate green (334 pytest incl. the migration drift + downgrade/re-upgrade round-trip). --- .../versions/0067_feed_sort_indexes.py | 55 +++++++++++++++++++ backend/app/models.py | 6 +- 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 backend/alembic/versions/0067_feed_sort_indexes.py 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/app/models.py b/backend/app/models.py index 65078e47..d6ddbbcd 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -251,8 +251,12 @@ 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 From 8c08ea3133998bf356e75c85dfa50b14bcdd5db0 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 04:30:07 +0200 Subject: [PATCH 02/10] perf(feed): published_at NOT NULL + range-anchored keyset for deep pages (E2 S2, PERF-15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A nullable published_at forced the feed keyset into a non-sargable `... OR published_at IS NULL` disjunction: with the 0067 index the planner still applied it as a FILTER and scanned from the top of the index, so a deep `newest` page discarded every earlier row (O(depth)). - Migration 0068: backfill any NULLs (0 in prod) -> SET NOT NULL + DEFAULT now(). - Model: Video.published_at is now non-Optional, nullable=False, server_default now(). - feedquery: the `pub` key is nullable=False, and keyset_predicate is rebuilt recursively so a non-nullable leading key gets a non-strict range bound (`published_at <= v`) as an AND conjunct — an index range Cond instead of a filter. Nullable keys keep the NULL-aware OR form (a `<= v` bound would drop their NULL group). Equivalence proven by test_feed_keyset (all 13 sorts, cursor-walk == unpaged). - Ingest paths (stub_from_playlist_item, apply_video_details, playlists sync) fall back to now() so they never write an explicit NULL. Measured on the prod-sized localdev reseed at a ~50k-deep cursor (0067 index + prod planner): OLD `Rows Removed by Filter: 50001`, 25,687 buffers / 9.16 ms -> NEW `Index Cond: published_at <= v`, 1 row filtered, 47 buffers / 0.037 ms. SET NOT NULL on 257k rows: 0.32s. Gate green (334 pytest). --- .../versions/0068_published_at_not_null.py | 36 +++++++++++++++++++ backend/app/feedquery.py | 28 +++++++++++---- backend/app/models.py | 9 +++-- backend/app/sync/playlists.py | 4 ++- backend/app/sync/videos.py | 8 +++-- backend/tests/test_feed_keyset.py | 8 +++-- 6 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 backend/alembic/versions/0068_published_at_not_null.py 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/app/feedquery.py b/backend/app/feedquery.py index 93e82ac2..75b09b2a 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -339,7 +339,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 +383,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 +401,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 d6ddbbcd..89bc2f30 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -261,8 +261,13 @@ class Video(Base, TimestampMixin): 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) 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"), From 6a35e83deda8041451e0c2968ac4ddb9041fce39 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 04:41:16 +0200 Subject: [PATCH 03/10] perf(feed): un-block the search GIN + inner-join the state views (E2 S3, PERF-12/16) PERF-12: the search ORed `unaccent(channels.title) ILIKE '%q%'` straight into the per-video predicate, which forced a full scan + detoast of every search_vector and a per-candidate channel fetch. Pre-resolve the matching channel ids (a few-hundred-row scan) and match `Video.channel_id.in_(ids)` instead, so the planner keeps the search_vector GIN as a BitmapOr branch (alongside ix_videos_channel_pub_desc). Empty match -> false() collapses the OR. PERF-16: show=watched|hidden|in_progress used an outer join + coalesce(status, 'new'), so the planner couldn't start from the user's small video_states set. Those views only return rows that HAVE a state, so inner-join video_states and filter the raw column (status is NOT NULL on a present row, so it's identical). 'unwatched'/'all' keep the outer join. Dropped the now-dead position_expr. New regression tests (test_feed_views): search returns both text- and channel- name matches; the four watch-state views return the exact expected sets. Measured on the prod-sized localdev reseed (prod planner, 0067/0068 applied): - search /feed/count q=magyar: 663,463 -> 19,206 buffers (1,406 -> 31 ms; OLD seq-scan+detoast -> NEW BitmapOr GIN+channel index) - search feed page: 2,908 -> 1,479 buffers (32.6 -> 1.4 ms) - show=watched (user with 64 watched): 40,661 -> 264 buffers (~154x) Gate green (336 pytest, incl. the 2 new equivalence tests). --- backend/app/feedquery.py | 49 +++++++++++-------- backend/tests/test_feed_views.py | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 19 deletions(-) create mode 100644 backend/tests/test_feed_views.py diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 75b09b2a..1c6020c2 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -131,7 +131,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 +162,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 +252,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,13 +278,7 @@ 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 @@ -315,14 +325,15 @@ 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") diff --git a/backend/tests/test_feed_views.py b/backend/tests/test_feed_views.py new file mode 100644 index 00000000..161a29e2 --- /dev/null +++ b/backend/tests/test_feed_views.py @@ -0,0 +1,84 @@ +"""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 + +from app.models import Channel, Subscription, 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"} From 21a3aae700dd4324b3130ba039a9cc17feaf51fa Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 05:01:27 +0200 Subject: [PATCH 04/10] perf(feed): collapse count + facets into one /feed/summary pass (E2 S4, PERF-13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FE fired /feed/count (1 pass) + /facets (3 passes) on every filter change = 4 full scans of the subscribed set. New /api/feed/summary does ONE grouped query (the base filter with tags stripped, GROUP BY channel_id -> per-channel video counts) and derives the total count + every tag's distinct-channel facet count in Python on the ~few-hundred-row channel map, faithfully mirroring filtered_query's channel-based tag logic + the /facets drill-down (own category dropped, unless a conjunctive topic-AND). Returns {count, facets} — the same numbers the two old endpoints returned. FE: a single shared query (Feed reads .count, Sidebar reads .facets); same key + filters, so React Query dedupes to ONE request. Invalidations repointed to qk.feedSummary. The old /feed/count + /facets endpoints (and their now-unused FE api/qk) are kept as the equivalence-test oracle + safety net; a later cleanup can drop them. New equivalence test (test_feed_views, 8 filter combos incl. cross-category, disjunctive/conjunctive topic modes, search): /feed/summary count == /feed/count and facets == /facets, byte-identical. Structural win: 2 requests / 4-6 queries -> 1 request / 2-3 queries (the review measured 530 -> 144 ms). Gate green: 344 pytest, frontend tsc/eslint/knip/prettier/222 vitest. --- backend/app/feedquery.py | 76 +++++++++++++++++++ backend/app/routes/feed.py | 14 ++++ backend/tests/test_feed_views.py | 65 +++++++++++++++- frontend/src/components/Channels.tsx | 4 +- frontend/src/components/Feed.tsx | 17 +++-- .../src/components/NotificationsPanel.tsx | 2 +- frontend/src/components/OnboardingWizard.tsx | 2 +- frontend/src/components/Sidebar.tsx | 12 +-- frontend/src/lib/api/feed.ts | 4 + frontend/src/lib/queryKeys.test.ts | 2 + frontend/src/lib/queryKeys.ts | 2 + 11 files changed, 182 insertions(+), 18 deletions(-) diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 1c6020c2..32bc0b63 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -340,6 +340,82 @@ def filtered_query( 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, 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]] = {} + selected = params.get("tags") or [] + if selected: + for tid, cat in db.execute( + select(Tag.id, Tag.category).where(Tag.id.in_(selected)) + ).all(): + selected_by_cat.setdefault(cat, set()).add(tid) + + def passes(ch_tags: set[int], exclude_cat: str | None) -> bool: + 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 diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 49303b72..0598b8f6 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, @@ -174,6 +175,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/tests/test_feed_views.py b/backend/tests/test_feed_views.py index 161a29e2..69859150 100644 --- a/backend/tests/test_feed_views.py +++ b/backend/tests/test_feed_views.py @@ -9,7 +9,9 @@ Regression nets for two query rewrites that must stay behaviour-equivalent: """ from datetime import datetime, timezone -from app.models import Channel, Subscription, Video, VideoState +import pytest + +from app.models import Channel, ChannelTag, Subscription, Tag, Video, VideoState def _dt(day: int) -> datetime: @@ -82,3 +84,64 @@ def test_watch_state_views_return_the_right_sets(db, as_user, make_user): 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"), + } + 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 -> 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)) + # 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&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..6802759f 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, @@ -358,8 +360,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 +623,10 @@ export default function Feed({ )}