From 21a3aae700dd4324b3130ba039a9cc17feaf51fa Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 18 Aug 2026 05:01:27 +0200 Subject: [PATCH] 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({ )}