diff --git a/backend/app/feedquery.py b/backend/app/feedquery.py index 32bc0b63..9fcdca4c 100644 --- a/backend/app/feedquery.py +++ b/backend/app/feedquery.py @@ -100,6 +100,16 @@ 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.""" + 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, @@ -281,12 +291,11 @@ def filtered_query( 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). @@ -377,17 +386,16 @@ def feed_summary(db: Session, user: User, **params) -> dict: 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) + # 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 diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 0598b8f6..c3ec31d4 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -128,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} @@ -147,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"): diff --git a/backend/tests/test_feed_views.py b/backend/tests/test_feed_views.py index 69859150..1a3eefd4 100644 --- a/backend/tests/test_feed_views.py +++ b/backend/tests/test_feed_views.py @@ -96,17 +96,22 @@ def _seed_tagged(db, make_user): "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 -> tag keys + # 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(): @@ -124,6 +129,8 @@ _SUMMARY_CASES = [ "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", ] diff --git a/frontend/src/lib/api/feed.ts b/frontend/src/lib/api/feed.ts index 016510c2..3a9845f7 100644 --- a/frontend/src/lib/api/feed.ts +++ b/frontend/src/lib/api/feed.ts @@ -113,6 +113,8 @@ function feedQuery(f: FeedFilters, cursor: string | null, limit: number): string export const feedApi = { feed: (f: FeedFilters, cursor: string | null, limit: number): Promise => req(`/api/feed?${feedQuery(f, cursor, limit)}`), + // SUPERSEDED by feedSummary (PERF-13) — no component calls this anymore; the backend /feed/count + // endpoint it targets is kept only as the /feed/summary equivalence-test oracle. Retire together. feedCount: (f: FeedFilters): Promise<{ count: number }> => req(`/api/feed/count?${filterParams(f).toString()}`), // Live YouTube search: results are materialised into the catalog and returned as feed cards. @@ -129,6 +131,8 @@ export const feedApi = { // the now-orphaned, un-kept videos). Returns how many videos were removed. clearSearch: (videoIds: string[]): Promise<{ removed: number }> => req("/api/search/clear", { method: "POST", body: JSON.stringify({ video_ids: videoIds }) }), + // SUPERSEDED by feedSummary (PERF-13) — no component calls this anymore; the backend /facets + // endpoint it targets is kept only as the /feed/summary equivalence-test oracle. Retire together. facets: (f: FeedFilters): Promise<{ counts: Record }> => req(`/api/facets?${filterParams(f).toString()}`), // The sidebar's total count + per-tag channel facets in ONE request + ONE grouped query, replacing